On 26.01.20 00:15, Denis 'GNUtoo' Carikli wrote: > On Sat, 25 Jan 2020 20:52:36 +0100 > Soeren Moch <sm...@web.de> wrote: > >> Hi Denis, > Hi, > >> thanks for your patch. In general I think it could be a good idea to >> support distroboot on this board, especially if we can maintain >> compatibility with the existing boot procedure. However, since this >> board repeatedly has size problems with the u-boot binary, we >> carefully need to check binary size. > I saw that. > > I also experienced size issues with the stock tbs2910_defconfig, and > for now I just locally removed support for things that eats up too much > space like PCIe support. > >> There in no SPL for tbs2910. So this is not required. > Ahh ok, now I understand. That probably explains the repeated size > issues. Why? With SPL+u-boot proper it would be even worse, since then there is a gap between SPL and u-boot, u-boot starts at higher address in eMMC, and it would not be smaller. And this 2-stage boot would break the documented u-boot update procedure, since we have to flash 2 files then. > > For my patch, I could guard the code addition with some ifdefs for > CONFIG_DISTRO_DEFAULTS if necessary. Note that CONFIG_DISTRO_DEFAULTS is > not set in the tbs2910_defconfig, so if done correctly it should not > make things worse. That means, without this change in defconfig the whole distroboot does not work? I will only take this patch if distroboot really works with defconfig then. > > In the long run it's probably worth looking into adding SPL support. > For instance the Wandboard has it. I'll try to find the time to look > into it but I can't guarantee anything. SPL only is required to detect i.MX6 flavour and SDRAM size and location. This is not necessary for tbs2910, since this board is only available with i.MX6Q. > > As for the rest of the questions: >> Why do you define CONFIG_BOOTCOMMAND here instead of modifying the >> existing one? >>> +#define CONFIG_BOOTCOMMAND \ >>> + "mmc rescan; " \ >>> + "if run bootcmd_up1; then " \ >>> + "run bootcmd_up2; " \ >>> + "else " \ >>> + "run bootcmd_mmc || run distro_bootcmd; " \ >>> + "fi" >>> + >> Why do you define CONFIG_BOOTCOMMAND here instead of modifying the >> existing one? > I'm a bit confused with it: There seem to be many way to do the same > thing and I'm not sure which one is the best. > > Because of that, I just kept it in the code as it was (instead of > moving it to tbs2910_defconfig). This is ok. What I mean: You moved it in the file, and therefore you unnecessarily changed a lot of lines without actually changing most of it's contents. > > I'm also not sure if it's best to run distro_bootcmd before or > after the bootcmd_up1/bootcmd_up2/bootcmd_mmc commands, which are > probably used to boot some distribution like LibreElec. > You did it right, as last boot option. Otherwise you would break compatibility to the existing boot flow.
One additional comment: currently we want to strip down the embedded dtb to reduce size. This means we cannot pass this dtb to linux, and always need to load a different one (together with kernel and initrd). This is no problem for extlinux.conf, but maybe we can omit to define fdtfile just to make sure we have to load a different one. I'm not sure, however, if distroboot scripts work without this default dtb. Soeren
