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.
