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 <mich...@walle.cc> [snip] > diff --git a/board/kontron/sl28/MAINTAINERS b/board/kontron/sl28/MAINTAINERS > new file mode 100644 > index 0000000000..047a057646 > --- /dev/null > +++ b/board/kontron/sl28/MAINTAINERS > @@ -0,0 +1,6 @@ > +Kontron SMARC-sAL28 board > +M: Michael Walle <mich...@walle.cc> > +S: Maintained > +F: board/kontron/sl28/ > +F: include/configs/kontron_sl28.h > +F: configs/kontron_sl28_defconfig
You should list the DTS files here too. [snip] > diff --git a/board/kontron/sl28/README b/board/kontron/sl28/README > new file mode 100644 > index 0000000000..3ddd9aeeb8 > --- /dev/null > +++ b/board/kontron/sl28/README This should be rST and in doc/board/ somewhere please. [snip] > 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? [snip] > +#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? > + > +#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. > +#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. > +#define CONFIG_EXTRA_ENV_SETTINGS \ > + "fdt_high=0xffffffffffffffff\0" \ > + "initrd_high=0xffffffffffffffff\0" \ You cannot disable fdt relocation by default and you likely shouldn't for initrd. If we would be placing the device tree outside of visible to the kernel memory use bootm_size to limit relocation but it's so easy to construct real life cases where the device tree is not 8 byte aligned and Linux blows up in non-obvious ways. Thanks. -- Tom
signature.asc
Description: PGP signature
