Hi Tom, Many thanks for the review.
On Wed, Mar 23, 2022 at 09:21:42AM -0400, Tom Rini wrote:
On Tue, Mar 22, 2022 at 10:41:18AM +0000, Rui Miguel Silva wrote:
Corstone1000 is a platform from arm, which includes pre verified Corstone SSE710 sub-system that combines Cortex-A and Cortex-M processors [0].
This code adds the support for the Cortex-A35 implementation at host side, it contains also the necessary bits to support the Corstone 1000 FVP (Fixed Virtual Platform) [1] and also the FPGA MPS3 board implementation of this platform. [2]
0: https://documentation-service.arm.com/static/619e02b1f45f0b1fbf3a8f16 1: https://developer.arm.com/tools-and-software/open-source-software/arm-platfo... 2: https://documentation-service.arm.com/static/61f3f4d7fa8173727a1b71bf
Signed-off-by: Abdellatif El Khlifi abdellatif.elkhlifi@arm.com Signed-off-by: Rui Miguel Silva rui.silva@linaro.org
arch/arm/Kconfig | 8 ++ arch/arm/dts/Makefile | 3 + arch/arm/dts/corstone1000-fvp.dts | 33 +++++ arch/arm/dts/corstone1000-mps3.dts | 41 ++++++ arch/arm/dts/corstone1000.dtsi | 167 +++++++++++++++++++++++
What is the status of these dts files with upstream Linux? They need to be in linux-next at least before we take them.
Hrr, did not know that one. I will do that and spin a v2 when these device trees get in linux-next. Also will address all yours other comments in this series.
Cheers, Rui
[snip]
@@ -2230,6 +2236,8 @@ source "arch/arm/mach-nexell/Kconfig"
source "board/armltd/total_compute/Kconfig"
+source "board/armltd/corstone1000/Kconfig"
source "board/bosch/shc/Kconfig" source "board/bosch/guardian/Kconfig" source "board/Marvell/octeontx/Kconfig"
There shouldn't be a space today where there is, so this entry can just fill in that gap.
[snip]
+/*
- Board specific ethernet initialization routine.
- */
+int board_eth_init(struct bd_info *bis) +{
- int rc = 0;
+#ifndef CONFIG_DM_ETH +#ifdef CONFIG_SMC91111
- rc = smc91111_initialize(0, CONFIG_SMC91111_BASE);
+#endif +#ifdef CONFIG_SMC911X
- rc = smc911x_initialize(0, CONFIG_SMC911X_BASE);
+#endif +#endif
- return rc;
+}
DM_ETH should always be set, so please clean this up.
[snip]
+#define V2M_SRAM0 0x02000000 +#define V2M_QSPI 0x08000000
+#define V2M_DEBUG 0x10000000 +#define V2M_BASE_PERIPH 0x1A000000
+#define V2M_BASE 0x80000000
+#define V2M_PERIPH_OFFSET(x) ((x) << 16)
+#define V2M_SYSID (V2M_BASE_PERIPH) +#define V2M_SYSCTL (V2M_BASE_PERIPH + V2M_PERIPH_OFFSET(1))
+#define V2M_COUNTER_CTL (V2M_BASE_PERIPH + V2M_PERIPH_OFFSET(32)) +#define V2M_COUNTER_READ (V2M_BASE_PERIPH + V2M_PERIPH_OFFSET(33))
+#define V2M_TIMER_CTL (V2M_BASE_PERIPH + V2M_PERIPH_OFFSET(34)) +#define V2M_TIMER_BASE0 (V2M_BASE_PERIPH + V2M_PERIPH_OFFSET(35))
+#define V2M_UART0 (V2M_BASE_PERIPH + V2M_PERIPH_OFFSET(81)) +#define V2M_UART1 (V2M_BASE_PERIPH + V2M_PERIPH_OFFSET(82))
Please find someplace better than the board config.h file for these values. Either some SoC header file, or pulled out of the dt. And you're probably not the first board to do this, but it does need cleaning up.
+/*
- config_distro_bootcmd define the boot command to distro_bootcmd, but we here
- want to first try to load a kernel if exists, override that config then
- */
+#undef CONFIG_BOOTCOMMAND
+#define CONFIG_BOOTCOMMAND \
"run retrieve_kernel_load_addr;" \"echo Loading kernel from $kernel_addr to memory ... ;" \"loadm $kernel_addr $kernel_addr_r 0xc00000;" \"usb start; usb reset;" \"run distro_bootcmd;" \"bootefi $kernel_addr_r $fdtcontroladdr;"CONFIG_BOOTCOMMMAND is part of Kconfig, so this needs to be set in the defconfig. I didn't see any other options that need to be migrated, but, CI will fail (at least, -next has the test fixed) over migrated options being in the board config.h file.
-- Tom