Re: [PATCH 0/8] env: mmc: improvements and corrections

[Tom Rini <trini@konsulko.com>]

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

On Thu, Nov 10, 2022 at 01:40:32PM -0700, Simon Glass wrote:
> Hi Patrick,
> 
> On Thu, 10 Nov 2022 at 03:49, Patrick Delaunay
> <patrick.delaunay@foss.st.com> wrote:
> >
> >
> > Update in U-Boot env mmc backend with several cosmetic changes or
> > corrections and 2 new features:
> >
> > 1/ CONFIG_ENV_MMC_USE_DT = no more use CONFIG_ENV_OFFSET
> >    in the mmc ENV backend when this config is activated.
> >
> >    Requested by the STM32MP STMicroelectronics boards which activate
> >    several ENV_IS_IN_XXX; the value of CONFIG_ENV_OFFSET is invalid for
> >    SD-Card / eMMC boot; this offset should only used in SPIFlash backend
> >    (sf.c) for SPI-NOR boot.
> >
> >    If this offset is used on mmc backend, when partition name in GPT is
> >    not aligned with  U-Boot DT: "u-boot,mmc-env-partition", the behavior
> >    is difficult to debug: a partition is corrupted on 'env save' command.
> >
> > 2/ selects the GPT env partition by the "u-boot-env" type GUID introduced
> >    by the commit c0364ce1c695 ("doc/README.gpt: define partition
> >    type GUID for U-Boot environment")
> >
> >    This feature can also avoid issue when 'u-boot-env' partition name
> >    change in GPT partitioning but not in the U-Boot DT with
> >    "u-boot,mmc-env-partition"
> >
> > Few check patch warnings remained in the series,
> > but after check I can't remove them :
> >
> > - IS_ENABLED(ENV_MMC_HWPART_REDUND) is normally used as
> >   IS_ENABLED(CONFIG_ENV_MMC_HWPART_REDUND)
> >   => ENV_MMC_HWPART_REDUND is locally defined in this file it is not
> >      a real CONFIG but I can use the IS_ENABLED() macro as it is defined
> >      to 1
> >
> > - Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> >   possible
> >   + CONFIG_PARTITION_TYPE_GUID => info.type_guid existence
> >   + CONFIG_ENV_OFFSET_REDUND and CONFIG_ENV_MMC_USE_DT => only for define
> >
> > As I miss the merge window, not targeted for v2023.01 but for next
> > v2023.04.
> 
> Shouldn't this all move to device tree? Using CONFIG options is such a
> mess. We have the devices in DT so can indicate which ones have an
> environment and what the parameters are for each.

And there's already the
Documentation/devicetree/bindings/nvmem/u-boot,env.yaml to build upon.

-- 
Tom

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