Hi Tom,
thanks for the review!
Am 2020-08-31 18:08, schrieb Tom Rini:
On Sun, Aug 30, 2020 at 10:03:00PM +0200, Michael Walle wrote:
Add basic support for the Kontron SMARC-sAL28 board. This includes just the bare minimum to be able to bring up the board and boot linux.
For now, the Single and Dual PHY variant is supported. Other variants will fall back to the basic variant.
In particular, there is no watchdog support for now. This means that you have to disable the default watchdog, otherwise you'll end up in the recovery bootloader. See the board README for details.
Signed-off-by: Michael Walle michael@walle.cc
..
diff --git a/include/configs/kontron_sl28.h b/include/configs/kontron_sl28.h new file mode 100644 index 0000000000..a5e3219560 --- /dev/null +++ b/include/configs/kontron_sl28.h @@ -0,0 +1,122 @@ +/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef __SL28_H +#define __SL28_H
+#include <asm/arch/stream_id_lsch3.h> +#include <asm/arch/config.h> +#include <asm/arch/soc.h>
Do we need these in here?
They are included in the ls1028a_common.h, but I cannot include this common file because its is really just a "common across freescale development boards" file. Therefore, I had to duplicate it here. Removing these includes results in build errors, eg. DCFG_BASE is undefined etc. I really don't want to go down that rabbit hole and fixing all the layerscape/freescale stuff.
+#ifdef CONFIG_SPL_BUILD +#define CONFIG_SYS_MONITOR_BASE CONFIG_SPL_TEXT_BASE +#else +#define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE +#endif
I don't like this, but I see it's copied from other layerscapes. Is it really needed?
I don't think so. I'll remove it.
+#define CONFIG_SYS_LOAD_ADDR (CONFIG_SYS_DDR_SDRAM_BASE + 0x10000000)
+/* environment */ +#define CONFIG_LOADADDR 0x81000000 +#define ENV_MEM_LAYOUT_SETTINGS \
- "scriptaddr=0x90000000\0" \
- "pxefile_addr_r=0x90100000\0" \
- "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \
- "fdt_addr_r=0x83000000\0" \
- "ramdisk_addr_r=0x83100000\0" \
- "fdtfile=freescale/fsl-ls1028a-kontron-sl28.dtb\0"
These are too small of a range of kernel/device tree/ramdisk addresses, and there will be overlap/overwrites at some point. I've been pointing people at the big block comment in include/configs/ti_armv7_common.h as it's still correct for minimum sizes for aarch64 as well.
thanks for pointing that out.
+#ifndef CONFIG_SPL_BUILD +#define BOOT_TARGET_DEVICES(func) \
- func(MMC, mmc, 1) \
- func(MMC, mmc, 0) \
- func(NVME, nvme, 0) \
- func(USB, usb, 0) \
- func(DHCP, dhcp, 0) \
- func(PXE, pxe, 0)
+#include <config_distro_bootcmd.h> +#else +#define BOOTENV +#endif
Is env in SPL enabled? This is another case that I don't really like to see done.
No it's not and it seems to build without the else case. IIRC it wasn't building a year ago when I did the original port.
I'll post a new version with your suggested changes, soon.
-michael