[PATCH] Convert CONFIG_SYS_MMC_ENV_DEV et al to Kconfig

[Tom Rini <trini@konsulko.com>]

On Wed, Aug 05, 2020 at 02:22:35PM +0100, David Woodhouse wrote:

> On 5 August 2020 14:14:58 BST, Tom Rini <trini@konsulko.com> wrote:
> >On Wed, Aug 05, 2020 at 02:04:02PM +0100, David Woodhouse wrote:
> >> 
> >> 
> >> On 5 August 2020 13:51:43 BST, Tom Rini <trini@konsulko.com> wrote:
> >> >On Wed, Aug 05, 2020 at 11:07:15AM +0200, Stefan Roese wrote:
> >> >> Hi Rasmus,
> >> >> 
> >> >> On 05.08.20 10:47, Rasmus Villemoes wrote:
> >> >> > On 24/07/2020 23.14, Tom Rini wrote:
> >> >> > > This converts the following to Kconfig:
> >> >> > >     CONFIG_SYS_MMC_ENV_DEV
> >> >> > >     CONFIG_SYS_MMC_ENV_PART
> >> >> > 
> >> >> > Yes, please! That's the only thing preventing us from running a
> >> >vanilla
> >> >> > upstream U-Boot on a number of our boards where we have the
> >> >defconfig
> >> >> > out of tree - we currently have a single patch touching a few
> >> >config
> >> >> > headers.
> >> >> > 
> >> >> > > Cc: Priyanka Jain <priyanka.jain@nxp.com>
> >> >> > > Cc: Madalin Bucur <madalin.bucur@oss.nxp.com>
> >> >> > > Signed-off-by: Tom Rini <trini@konsulko.com>
> >> >> > > ---
> >> >> > > Due to some non-environment usage of the variables, cc'ing a
> >few
> >> >folks.
> >> >> > 
> >> >> > Yes, and those are the reason I didn't already send a patch.
> >> >> > 
> >> >> > > diff --git a/env/Kconfig b/env/Kconfig
> >> >> > > index 38e7fadbb93a..2eb2fe48da3d 100644
> >> >> > > --- a/env/Kconfig
> >> >> > > +++ b/env/Kconfig
> >> >> > > @@ -567,6 +567,25 @@ config SYS_RELOC_GD_ENV_ADDR
> >> >> > >   	  Relocate the early env_addr pointer so we know it is not
> >> >inside
> >> >> > >   	  the binary. Some systems need this and for the rest, it
> >> >doesn't hurt.
> >> >> > > +config SYS_MMC_ENV_DEV
> >> >> > > +	int "mmc device number"
> >> >> > > +	depends on ENV_IS_IN_MMC || ENV_IS_IN_FAT ||
> >> >SYS_LS_PPA_FW_IN_MMC || \
> >> >> > > +		CMD_MVEBU_BUBT || FMAN_ENET || QE
> >> >> > 
> >> >> > Slight yuck. But I suppose that those (ab)uses of
> >> >CONFIG_SYS_MMC_ENV_DEV
> >> >> > can grow their own proper config symbols piecemeal and then get
> >> >removed
> >> >> > from that list, rather than having to wean off all of them
> >before
> >> >> > SYS_MMC_ENV_* can get converted.
> >> >> > 
> >> >> > Why is ENV_IS_IN_FAT in that list? That has its own
> >> >already-existing
> >> >> > ENV_FAT_DEVICE_AND_PART.
> >> >
> >> >ENV_IS_IN_FAT is in the list because with 6731bef6966e ("env/fat.c:
> >> >allow loading from a FAT partition on the MMC boot device") we can
> >pick
> >> >up the device dynamically and hard-code the partition.  But that in
> >> >turn
> >> >means needing a default device.  This even gets expanded to cover
> >ext4
> >> >here:
> >>
> >>http://patchwork.ozlabs.org/project/uboot/patch/20200804090547.2175076-2-dwmw2 at infradead.org/
> >> 
> >> 
> >> FAT and ext4 don't need to grow their own config symbols because they
> >already *have* them. The only reason they're involved here is because
> >of the case where they explicitly want to *abdicate* responsibility and
> >let platform code determine the device to use, not hard-coded config.
> >Like the MT7623 platforms where the preloader tells U-Boot if it was
> >actually loaded from the internal eMMC or external SD card.
> >> 
> >> If someone is setting up the ENV_FAT_DEVICE_AND_PART to start with a
> >colon and thus abdicate the device part, but then that decision is only
> >coming from a hard-coded SYS_MMC_ENV_DEV option, then they're probably
> >somewhat confused.
> >
> >Right, but the default mmc_get_env_dev() you added uses
> >CONFIG_SYS_MMC_ENV_DEV.
> 
> Perhaps I could revamp the FAT/ext4 patches so that they don't use the default weak function, and if you use the "get it at runtime" behaviour on any build which doesn't actually have a real platform mmc_get_env_dev() function, the build fails.  That would be sane, but might involve a separate explicit config option instead of just "the DEVICE_AND_PART option starts with a colon".

That sounds like a good idea, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200805/fd1a6c92/attachment.sig>