Re: [PATCH v3 31/31] RFC: Switch rpi over to use bootstd

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4498 bytes --]

On Wed, Jan 19, 2022 at 09:09:51AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 19 Jan 2022 at 08:01, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Jan 19, 2022 at 07:37:36AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 19 Jan 2022 at 07:04, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Tue, Jan 18, 2022 at 06:43:15PM -0700, Simon Glass wrote:
> > > >
> > > > > Switch this over, for testing purposes.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > > (no changes since v1)
> > > > >
> > > > >  boot/Kconfig          |  3 ++-
> > > > >  include/configs/rpi.h | 39 ++-------------------------------------
> > > > >  2 files changed, 4 insertions(+), 38 deletions(-)
> > > > >
> > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > index 9cf1d013f20..eab3c0f3467 100644
> > > > > --- a/boot/Kconfig
> > > > > +++ b/boot/Kconfig
> > > > > @@ -1124,7 +1124,8 @@ config USE_BOOTCOMMAND
> > > > >  config BOOTCOMMAND
> > > > >       string "bootcmd value"
> > > > >       depends on USE_BOOTCOMMAND && !USE_DEFAULT_ENV_FILE
> > > > > -     default "run distro_bootcmd" if DISTRO_DEFAULTS
> > > > > +     default "bootflow scan -lb" if BOOTSTD
> > > > > +     default "run distro_bootcmd" if !BOOTSTD && DISTRO_DEFAULTS
> > > > >       help
> > > > >         This is the string of commands that will be used as bootcmd and if
> > > > >         AUTOBOOT is set, automatically run.
> > > > > diff --git a/include/configs/rpi.h b/include/configs/rpi.h
> > > > > index d5e064fb379..ea373d0c221 100644
> > > > > --- a/include/configs/rpi.h
> > > > > +++ b/include/configs/rpi.h
> > > > > @@ -133,47 +133,12 @@
> > > > >       "fdt_addr_r=0x02600000\0" \
> > > > >       "ramdisk_addr_r=0x02700000\0"
> > > > >
> > > > > -#if CONFIG_IS_ENABLED(CMD_MMC)
> > > > > -     #define BOOT_TARGET_MMC(func) \
> > > > > -             func(MMC, mmc, 0) \
> > > > > -             func(MMC, mmc, 1) \
> > > > > -             func(MMC, mmc, 2)
> > > > > -#else
> > > > > -     #define BOOT_TARGET_MMC(func)
> > > > > -#endif
> > > > > -
> > > > > -#if CONFIG_IS_ENABLED(CMD_USB)
> > > > > -     #define BOOT_TARGET_USB(func) func(USB, usb, 0)
> > > > > -#else
> > > > > -     #define BOOT_TARGET_USB(func)
> > > > > -#endif
> > > > > -
> > > > > -#if CONFIG_IS_ENABLED(CMD_PXE)
> > > > > -     #define BOOT_TARGET_PXE(func) func(PXE, pxe, na)
> > > > > -#else
> > > > > -     #define BOOT_TARGET_PXE(func)
> > > > > -#endif
> > > > > -
> > > > > -#if CONFIG_IS_ENABLED(CMD_DHCP)
> > > > > -     #define BOOT_TARGET_DHCP(func) func(DHCP, dhcp, na)
> > > > > -#else
> > > > > -     #define BOOT_TARGET_DHCP(func)
> > > > > -#endif
> > > > > -
> > > > > -#define BOOT_TARGET_DEVICES(func) \
> > > > > -     BOOT_TARGET_MMC(func) \
> > > > > -     BOOT_TARGET_USB(func) \
> > > > > -     BOOT_TARGET_PXE(func) \
> > > > > -     BOOT_TARGET_DHCP(func)
> > > > > -
> > > > > -#include <config_distro_bootcmd.h>
> > > > > -
> > > > >  #define CONFIG_EXTRA_ENV_SETTINGS \
> > > > >       "dhcpuboot=usb start; dhcp u-boot.uimg; bootm\0" \
> > > > > +     "boot_targets=mmc0 mmc1 usb0 pxe dhcp\0" \
> > > >
> > > > We have the indirect defines to func(...) everywhere so that if a
> > > > feature is disabled we still build + function, as otherwise it's a loud
> > > > link error.  I assume with this we just get a try and fail move to the
> > > > next target at run time, yes?
> > >
> > > Yes, that's right. But I hope that boards actually set up the boot
> > > targets correctly so that only those that are actually supported are
> > > enabled.
> >
> > What do you mean?  They always do that, even today, which is why we have
> > those #ifdef's in the distro boot board code.  If the user ends up
> > disabling something, it should still build and work.  And honestly,
> > given that dependencies are now in Kconfig, the distro_boot.h stuff
> > could get away with dropping all of the
> > "Enabled_foo_without_CONFIG_CMD_foo" stuff I bet.
> 
> Then I don't think I understand your original question. With bootstd
> if USB is disabled (for example) then the USB bootdev won't exist so
> it won't look at USB. It is not so much a link error, just that the
> device is not there.

I mean, with "boot_targets=mmc0 mmc1 usb0 pxe dhcp0" but CONFIG_MMC
disabled, bootstd will gracefully fail, gracefully fail, boot from USB.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]