[PATCH V2 4/7] env: Fix invalid env handling in env_init()

[Marek Vasut <marex@denx.de>]

On 7/28/20 2:39 PM, Tom Rini wrote:
[...]
>>> So, I am not sure about this change.  Given the whole thread that ends
>>> in https://lists.denx.de/pipermail/u-boot/2020-June/417433.html this
>>> particular part of the function is being too clever.  I think it's
>>> intentionally not doing what you're adding right here for some use
>>> cases.
>>
>> Which use-cases ?
> 
> flash specifically sets env_valid to ENV_INVALID and returns 0, and uses
> the default environment location, when env is invalid.  NAND, when
> embedded, sets addr to 0 when invalid, ENV_INVALID and returns 0.
> NOWHERE is the real funny case, it says ENV_INVALID and
> default_environment and returns 0.
> 
> Which all brings me back to my "too clever" statement.  Only REMOTE
> ever returns non-zero in it's init function.

Which makes me think there is probably no good way out which would not
break one usecase or the other.

>>> I think that to cleanly achieve the goals of your series we need
>>> to stop letting drv->init be optional so that we then stop doing the
>>> particular we're doing with "ENOENT means runs this common code path for
>>> many envs".  We may also need to make sure the link order in
>>> env/Makefile has nowhere first in all cases, rather than just most cases
>>> like it does now, with a big comment on why.
>>
>> So, would it make sense to apply the rest and revisit this patch
>> separately (likely with ST on CC) so the writeable list won't miss this
>> release ?
> 
> If you're OK with that, yes.

Yes, fine.