[PATCH] env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT

[Tom Rini]

On Wed, Jan 13, 2021 at 03:50:17PM +0100, Michal Simek wrote:
> 
> 
> On 13. 01. 21 15:43, Tom Rini wrote:
> > On Wed, Jan 13, 2021 at 03:24:24PM +0100, Michal Simek wrote:
> >> On 13. 01. 21 15:02, Tom Rini wrote:
> >>> On Wed, Jan 13, 2021 at 01:26:27PM +0100, Michal Simek wrote:
> >>>
> >>>> CONFIG_SYS_REDUNDAND_ENVIRONMENT is changing in env_internal.h how u-boot
> >>>> works with variables. struct environment_s has one byte flags property
> >>>> which also affects ENV_SIZE macro.
> >>>>
> >>>> I have reached the case where CONFIG_ENV_IS_NOWHERE is default setup
> >>>> but custom scripts can be designed in a way that u-boot is asked to
> >>>> import/export variables from/to file which can be in certain format.
> >>>> That's why also for this configuration make sense to enable
> >>>> CONFIG_SYS_REDUNDAND_ENVIRONMENT because it depends on environment file
> >>>> format.
> >>>>
> >>>> The patch is removing dependency on this configuration to support selecting
> >>>> environment file format without any specific dependency where variables are
> >>>> stored.
> >>>
> >>> Are you importing a binary of the environment in, which was generated
> >>> with redundant env set, is what the problem is?
> >>
> >> Yes exactly.
> > 
> > OK, so env import/export -b require compatible env structs, that makes
> > sense.  I assume you've ruled out env import/export -t instead already.
> 
> Yes that's not an option.
> 
> > I would rather see the struct be identical (so, always have flags)
> > rather than say that we can enable redundant environment in all cases
> > (since functionally, we can't for "nowhere" and don't for some others)
> > as it also means that for your case you would still need to know to
> > enable the redundant feature to get what you're aiming for to work.
> > Does that make sense?  Thanks!
> 
> I have not a problem to enable this feature for all but when this is
> simply enabled for everybody boards which don't have this enabled will
> fail. Maybe variables without that flags can be fall back option that
> you can read them but when you save you will add there flag field.
> 
> As of I now I know that I want to enable this feature for certain board
> because boot variables are generated by OS.

OK, so yes, maybe we don't want to enable it globally unconditionally.
Can you do a v2 where you update the help text to note that this changes
the binary environment structure U-Boot uses?  My main concern is that
the problem you've tracked down and solved here should be easier to spot
for the next person that goes down this path.  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/20210114/77562f68/attachment.sig>