Hi Tom, On 21.07.21 15:33, Tom Rini wrote: > On Wed, Jul 21, 2021 at 10:03:26AM +0200, Frieder Schrempf wrote: >> From: Frieder Schrempf <[email protected]> >> >> This adds support for i.MX6UL/ULL-based evaluation kits with SoMs by >> Kontron Electronics GmbH. >> >> Currently there are the following SoM flavors (SoM-Line): >> * N6310: SOM with i.MX6UL-2, 256MB RAM, 256MB SPI NAND >> * N6311: SOM with i.MX6UL-2, 512MB RAM, 512MB SPI NAND >> * N6411: SOM with i.MX6ULL, 512MB RAM, 512MB SPI NAND >> >> And the according evaluation boards (Board-Line): >> * N6310-S: Baseboard with SOM N6310, eMMC, display (optional), ... >> * N6311-S: Baseboard with SOM N6311, eMMC, display (optional), ... >> * N6411-S: Baseboard with SOM N6411, eMMC, display (optional), ... >> >> Currently U-Boot describes i.MX6UL and i.MX6ULL through separate config >> options at compile-time. Though the differences are so minor, that for >> the scope of these SoMs we just use a single defconfig that is compatible >> with both SoCs. >> >> Signed-off-by: Frieder Schrempf <[email protected]> >> Reviewed-by: Stefano Babic <[email protected]> > [snip] >> +/* >> + * ####################################### >> + * ### ENVIRONMENT ### >> + * ####################################### >> + */ >> +#define CONFIG_EXTRA_ENV_SETTINGS \ >> + "bootargs_base=" KONTRON_ENV_KERNEL_MTDPARTS "\0" \ >> + "script=boot.scr\0" \ >> + "kernel_addr_r=" KONTRON_ENV_KERNEL_ADDR "\0" \ >> + "fdt_addr_r=" KONTRON_ENV_FDT_ADDR "\0" \ >> + "ramdisk_addr_r=" KONTRON_ENV_RAMDISK_ADDR "\0" \ >> + "pxefile_addr_r=" KONTRON_ENV_PXE_ADDR "\0" \ >> + "scriptaddr=" KONTRON_ENV_PXE_ADDR "\0" \ > > I assume there's more platforms coming in, and thus the common and mx6 > split. But I still see some problems.
Yes, probably. But I admit that the use of the shared header file is rather limited. I'm thinking of getting rid of it in the next iteration of this patch. > >> + "bootdir=\0" \ >> + "bootdelay=3\0" \ >> + "ipaddr=192.168.1.11\0" \ >> + "serverip=192.168.1.10\0" \ >> + "gatewayip=192.168.1.10\0" \ >> + "netmask=255.255.255.0\0" \ > > It's really really frowned upon to put the ipaddr/etc in to the > hard-coded default environment. Is this absolutely necessary? Indeed! No it's not necessary and it's rather a historical relict. I will remove all those variables. > [snip] >> +#define KONTRON_ENV_KERNEL_MTDPARTS >> "mtdparts=spi1.0:128k(spl),832k(u-boot),64k(env);spi4.0:192m(rootfs),-(user)" > > This should just be in the device tree (for Linux!) and we should be > parsing that. Agree, I will also remove this. > >> +#define KONTRON_ENV_KERNEL_ADDR "0x82000000" >> +#define KONTRON_ENV_FDT_ADDR "0x83000000" > > 16MB for kernel + BSS is very very small. You're likely to hit > overlaps. > >> +#define KONTRON_ENV_PXE_ADDR "0x83100000" >> +#define KONTRON_ENV_RAMDISK_ADDR "0x83200000" > > What is the smallest amount of memory you need to work with on these > platforms? The header I referenced in the other thread spells out a > bunch of safe sizes / offsets to use, but that can be tricky on smaller > memory platforms. As this board has a minimum of 256 MB DDR, I think I will just use the layout from the reference you provided. Thanks for reviewing! Frieder
