[PATCH v3 3/8] bootstd: Avoid depending on BLK

Wed Nov 13 15:39:53 CET 2024

Hi Tom,

On Fri, 18 Oct 2024 at 12:48, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Tom,
>
> On Fri, 18 Oct 2024 at 12:29, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Fri, Oct 18, 2024 at 12:06:46PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 18 Oct 2024 at 11:52, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Fri, Oct 18, 2024 at 11:20:40AM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Fri, 18 Oct 2024 at 09:11, Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > On Fri, Oct 18, 2024 at 09:00:49AM -0600, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Thu, 17 Oct 2024 at 22:39, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Thu, Oct 17, 2024 at 04:12:02PM -0700, Simon Glass wrote:
> > > > > > > >
> > > > > > > > > In principle bootstd can work without block devices, even if it does
> > > > > > > > > require driver model to be enabled in that case.
> > > > > > > > >
> > > > > > > > > The use of a 'depends on BLK' for BOOTSTD conflicts with the way 'BLK'
> > > > > > > > > is now defined, producing recursive errors through multiple different
> > > > > > > > > paths, one of which is this (with Linksprite_pcDuino3 and
> > > > > > > > > BOOTSTD_DEFAULTS enabled):
> > > > > > > > >
> > > > > > > > >   arch/arm/Kconfig:7:error: recursive dependency detected!
> > > > > > > > >   arch/arm/Kconfig:7: symbol ARM64 is selected by ARCH_UNIPHIER_V8_MULTI
> > > > > > > > >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> > > > > > > > >      part of choice <choice>
> > > > > > > > >   arch/arm/mach-uniphier/Kconfig:6: choice <choice> contains symbol
> > > > > > > > >      ARCH_UNIPHIER_V8_MULTI
> > > > > > > > >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> > > > > > > > >      part of choice SPL
> > > > > > > > >   arch/arm/mach-stm32mp/Kconfig:3: symbol SPL depends on SUPPORT_SPL
> > > > > > > > >   common/spl/Kconfig:1: symbol SUPPORT_SPL is selected by ASPEED_AST2600
> > > > > > > > >   arch/arm/mach-aspeed/Kconfig:26: symbol ASPEED_AST2600 is part of
> > > > > > > > >      choice <choice>
> > > > > > > > >   arch/arm/mach-aspeed/Kconfig:12: choice <choice> contains symbol
> > > > > > > > >      ASPEED_AST2500
> > > > > > > > >   arch/arm/mach-aspeed/Kconfig:17: symbol ASPEED_AST2500 is part of
> > > > > > > > >      choice DM_RESET
> > > > > > > > >   arch/arm/mach-renesas/Kconfig.rcar3:197: symbol DM_RESET is selected
> > > > > > > > >      by CLK_RCAR_GEN3
> > > > > > > > >   drivers/clk/renesas/Kconfig:53: symbol CLK_RCAR_GEN3 depends on
> > > > > > > > >      CLK_RENESAS
> > > > > > > > >   drivers/clk/renesas/Kconfig:1: symbol CLK_RENESAS depends on CLK
> > > > > > > > >   drivers/clk/Kconfig:3: symbol CLK is selected by IMX8M_POWER_DOMAIN
> > > > > > > > >   drivers/power/domain/Kconfig:35: symbol IMX8M_POWER_DOMAIN depends on
> > > > > > > > >      POWER_DOMAIN
> > > > > > > > >   drivers/power/domain/Kconfig:3: symbol POWER_DOMAIN is selected by
> > > > > > > > >      BCM6318_USBH_PHY
> > > > > > > > >   drivers/phy/Kconfig:83: symbol BCM6318_USBH_PHY depends on PHY
> > > > > > > > >   drivers/phy/Kconfig:4: symbol PHY is selected by USB_EHCI_MX7
> > > > > > > > >   drivers/usb/host/Kconfig:211: symbol USB_EHCI_MX7 depends on USB
> > > > > > > > >   drivers/usb/Kconfig:1: symbol USB is selected by BOOTSTD_DEFAULTS
> > > > > > > > >   boot/Kconfig:455: symbol BOOTSTD_DEFAULTS depends on BOOTSTD
> > > > > > > > >   boot/Kconfig:398: symbol BOOTSTD depends on BLK
> > > > > > > > >   drivers/block/Kconfig:1: symbol BLK is selected by PVBLOCK
> > > > > > > > >   drivers/xen/Kconfig:1: symbol PVBLOCK depends on XEN
> > > > > > > > >   Kconfig:176: symbol XEN depends on ARM64
> > > > > > > > >
> > > > > > > > > We don't want to revert the change to BLK, which has been in place for
> > > > > > > > > a year now. We don't want to select BLK in BOOTSTD since it should
> > > > > > > > > support booting without block devices. The only realistic option is to
> > > > > > > > > remove BOOTSTD's dependency on BLK.
> > > > > > > > >
> > > > > > > > > Disable standard boot on the one board which fails.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > Changes in v3:
> > > > > > > > > - Drop wip (work-in-progress) comment in commit
> > > > > > > > >
> > > > > > > > > Changes in v2:
> > > > > > > > > - Add new patch to resolve BOOTSTD->BLK recursion with Kconfig
> > > > > > > > >
> > > > > > > > >  boot/Kconfig                                   | 2 +-
> > > > > > > > >  configs/gardena-smart-gateway-mt7688_defconfig | 1 +
> > > > > > > > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > > > > > > >
> > > > > > > > > Applied to u-boot-dm, thanks!
> > > > > > > >
> > > > > > > > No, this is wrong, it's pulling in BOOTSTD on a ton of platforms that
> > > > > > > > didn't have it before.
> > > > > > >
> > > > > > > This affects 103 boards and the size growth is considerable:
> > > > > > >
> > > > > > >    aarch64: (for 22/22 boards) all +15697.0 bss +26.2 data +710.9
> > > > > > > rodata +1822.1 text +13137.8
> > > > > > >        arc: (for 4/4 boards) all +15338.0 bss +4.0 data +526.0 rodata
> > > > > > > +2096.0 text +12712.0
> > > > > > >        arm: (for 26/26 boards) all +18882.8 data +619.2 rodata +1998.8
> > > > > > > text +16264.8
> > > > > > >       m68k: (for 17/17 boards) all +23859.9 bss +3.5 data +1595.8
> > > > > > > rodata +1722.8 text +20537.8
> > > > > > > microblaze: (for 1/1 boards) all +24713.0 bss -88.0 data +1248.0
> > > > > > > rodata +2288.0 text +21265.0
> > > > > > >       mips: (for 23/23 boards) all +21659.1 bss +39.5 data +746.6
> > > > > > > rodata +2155.7 text +18717.4
> > > > > > >      nios2: (for 2/2 boards) all +24856.0 bss +4.0 data +704.0 text +24148.0
> > > > > > >    powerpc: (for 13/13 boards) all +21746.6 data +836.3 rodata +807.2
> > > > > > > text +20103.2
> > > > > > >
> > > > > > > Added options are:
> > > > > > >
> > > > > > >    + all: CONFIG_BOOTDEV_ETH=1 CONFIG_BOOTMETH_EXTLINUX=1
> > > > > > > CONFIG_BOOTMETH_GLOBAL=1 CONFIG_BOOTMETH_VBE=1
> > > > > > > CONFIG_BOOTMETH_VBE_REQUEST=1 CONFIG_BOOTMETH_VBE_SIMPLE=1
> > > > > > > CONFIG_BOOTMETH_VBE_SIMPLE_OS=1 CONFIG_BOOTSTD=1
> > > > > > > CONFIG_BOOTSTD_BOOTCOMMAND=1 CONFIG_CMD_BOOTFLOW=1 CONFIG_MENU=1
> > > > > > > CONFIG_PXE_UTILS=1
> > > > > > >
> > > > > > > Should I manually disable it on those platforms? I've reached the
> > > > > > > point where bootstd needs to be available when BLK is not enabled, so
> > > > > > > need some sort of solution.
> > > > > >
> > > > > > What is the use case for needing all of this, without BLK being set? I'm
> > > > > > not sure which platforms that grew here also need bootstd?
> > > > >
> > > > > It is just that some XPL (I can say that now) platforms don't enable
> > > > > BLK but do need to read from a device, e.g. MMC.
> > > >
> > > > Er, how is that an issue for any of the XPL phases? We have
> > > > {SPL,TPL,VPL}_BLK symbols. And BLK itself is def_bool'd on DM and MMC,
> > > > etc. And all boards enable DM now.
> > >
> > > I was using MMC_TINY... I still have one more series for rk3399...so
> > > if things build OK I can perhaps just drop this patch and figure this
> > > out later?
> >
> > Yeah, please drop this for now and re-visit things. MMC_TINY is only for
> > SPL (but tested as CONFIG_IS_ENABLED(MMC_TINY) because maybe it makes
> > sense in the TPL sense? But it's distinctly not BLK and DM, iirc).
>
> Yes, I meant XPL_BLK, not BLK, oops.

This objection is one of a few things blocking the sunxi bootstd
conversion. If you want to see the problem for yourself, try building
Linksprite_pcDuino3 with this series applied sans this patch.

The other thing blocking it is bootmgr, which we either need to
disable for sunxi, or revert a patch which the author doesn't want
reverted.

So for now I'll just send a v4 with patches for those two things and
we can discuss it again.

Regards,
Simon