[PATCH 0/5] CMD_SAVEENV ifdef cleanup

[Rasmus Villemoes]

On 19/02/2020 14.25, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <20200219094726.26798-1-rasmus.villemoes at prevas.dk> you wrote:
>>
>> [1] Here's the current conditions for which these three drivers
>> provide .save:
>>
>>           SPL                     U-Boot
>> ext4.c    CONFIG_CMD_SAVEENV=y    CONFIG_CMD_SAVEENV=y
>> fat.c     never                   CONFIG_CMD_SAVEENV=y
>> sf.c      never                   CONFIG_CMD_SAVEENV=y [2]
> 
> Some questions:
> 
> 
> 1) I'm not sure if your changes cover the situation that you want to
>    have "saveenv" available in U-Boot proper, but do NOT want to
>    have it in SPL. 

They do, that's the whole point of introducing the simple
CONFIG_IS_ENABLED(SAVEENV) - for answering the question "do we want to
enable saving the environment in this context". Then the .save method
gets built and linked in precisely if that's the case, so one gets a
consistent matrix that says

           SPL                     U-Boot
 ext4.c    CONFIG_SPL_SAVEENV=y    CONFIG_CMD_SAVEENV=y
 fat.c     CONFIG_SPL_SAVEENV=y    CONFIG_CMD_SAVEENV=y
 sf.c      CONFIG_SPL_SAVEENV=y    CONFIG_CMD_SAVEENV=y

But I can't fix the whole world in one go, so only the above three get
fixed to that state for now.

>    It is mandatory that this possibility is kept.

Of course, and _nothing_ changes in that regard. [It is of course
possible that I messed up with the implementation, but it is certainly
the intention to keep it that way.]

> 
> 2) It seems wrong to me to make such cleanup in any way dependent on
>    file system type or a mix of arbitrary storage driver types.
>    this should be handled in two independent, orthogonal steps:
>    a) clean up any drivers or file system accessors that do not fit
>       into the general model
>    b) adapt the general model to such changes
> 
>    Maybe it makes sense to change the order of these steps, if this
>    results in less intrusive patches - I have no ides.
> 
>    In any case testing must _also_ include all the other many ways
>    of storing the environment, including parallel or SPI NOR flash,
>    NAND flash, UBI, UNIFS, etc.

See above, I can't fix, much less test, all those backend drivers.
Nothing changes for those, they provide a .save method under exactly the
same (probably mutually inconsistent...) conditions as they used to. So
I expect that when someone else runs into wanting CONFIG_SPL_SAVEENV
honoured with, say, mmc backend, they probably quickly discover that
doesn't work at all, and then they can fix mmc.c to make it work.

But it's not in all cases as simply as just removing the
custom/arbitrary ifdef logic, sometimes those ifdefs cover code that
depends on certain macros or whatnot that are not available for an
SPL_BUILD.

Rasmus