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.