[PATCH 01/12] env: Complete generic support for writable list

[Marek Vasut]

On 5/29/22 17:37, Jan Kiszka wrote:
> On 29.05.22 03:46, Marek Vasut wrote:
>> On 5/28/22 15:02, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka at siemens.com>
>>>
>>> This completes what 890feecaab72 started by selecting ENV_APPEND and
>>> ENV_IS_NOWHERE and by moving this driver to top if the list. This
>>
>> s at if the list at of the list@
>>
>>> ensures that load operations pick up both the default env and the
>>> permitted parts of the next-prio location. When writing though, we must
>>> not use NOWHERE and rather need to forward directly to the first
>>> external location.
>>
>> Isn't the env load order a board-specific setting ?
>>
> 
> There can always be special cases, but I don't see why it should be
> _always_ board-specific.

Because this is a special case, right ?

>>> With this change, boards only need to define the list of writable
>>> variables but no longer have to provide a custom env_get_location
>>> implementation.
>>
>> This also brings in a lot of ifdeffery and extra complexity. If you
>> implement this as a board-specific env_get_location() override, you can
>> avoid that. Try:
>>
>> enum env_location env_get_location(enum env_operation op, int prio)
>> {
>>           if (op == ENVOP_SAVE) {
>>                   if (prio == 0)
>>                           return ENVL_SPI_FLASH;
>>           } else {
>>                   if (prio == 0)
>>                           return ENVL_NOWHERE;
>>                   if (prio == 1)
>>                           return ENVL_SPI_FLASH;
>>           }
>>
>>           return ENVL_UNKNOWN;
>> }
> 
> This is exactly what I would like to avoid, having to carry such a
> generic function (minus that hard-coded the choice of storage - this can
> be controlled via .config) in every board.

Just for a quick statistics -- how many boards would make use of this 
generic function ?

If you want to make this into a generic patch, can you somehow reduce 
the ever-growing ifdeffery, so that the patch won't add to it so much ? 
I suspect the code above can help with that, maybe it can be used to 
remove at least the env_locations[] reordering ifdeffery ?