On Mon, Jan 22, 2018 at 09:48:26AM -0700, Simon Glass wrote: > Hi, > > On 22 January 2018 at 09:36, Tom Rini <tr...@konsulko.com> wrote: > > On Mon, Jan 22, 2018 at 04:57:41PM +0100, Maxime Ripard wrote: > >> On Mon, Jan 22, 2018 at 07:49:41AM -0500, Tom Rini wrote: > >> > On Mon, Jan 22, 2018 at 01:46:46PM +0100, Maxime Ripard wrote: > >> > > Hi, > >> > > > >> > > On Sun, Jan 21, 2018 at 05:29:56PM -0700, Simon Glass wrote: > >> > > > > On Wed, Jan 17, 2018 at 03:07:58PM -0700, Simon Glass wrote: > >> > > > >> On 16 January 2018 at 01:16, Maxime Ripard > >> > > > >> <maxime.rip...@free-electrons.com> wrote: > >> > > > >> > Allow boards and architectures to override the default > >> > > > >> > environment lookup > >> > > > >> > code by overriding env_get_location. > >> > > > >> > > >> > > > >> > Reviewed-by: Andre Przywara <andre.przyw...@arm.com> > >> > > > >> > Reviewed-by: Lukasz Majewski <lu...@denx.de> > >> > > > >> > Signed-off-by: Maxime Ripard <maxime.rip...@free-electrons.com> > >> > > > >> > --- > >> > > > >> > env/env.c | 20 +++++++++++++++++++- > >> > > > >> > 1 file changed, 19 insertions(+), 1 deletion(-) > >> > > > >> > > >> > > > >> > >> > > > >> I still don't really understand why this needs to be a weak > >> > > > >> function. > >> > > > >> If the board knows the priority order, can it not put it into > >> > > > >> global_data? We could have a little u8 array of 4 items with a > >> > > > >> terminator? > >> > > > > > >> > > > > Sure that would be simpler, but that would also prevent us from > >> > > > > doing > >> > > > > "smart" things based on data other than just whether the previous > >> > > > > environment is usable. Things based for example on a GPIO state, > >> > > > > or a > >> > > > > custom algorithm to transition (or duplicate) the environment. > >> > > > > >> > > > In that case the board could read the GPIO state, or the algorithm, > >> > > > and then set up the value. > >> > > > > >> > > > Basically I am saying it could set up the priority order in advance > >> > > > of > >> > > > it being needed, rather than having a callback. > >> > > > >> > > Aren't we kind of stuck here? > >> > > > >> > > On the previous iterations, we already discussed this and Tom > >> > > eventually told he was in favour of __weak functions, and the > >> > > discussion stopped there. I assumed you were ok with it. > >> > > > >> > > I'd really want to move forward on that. This is something that is > >> > > really biting us *now* and I'd hate to miss yet another merge window > >> > > because of debates like this. > >> > > >> > Yes, I think this is where we want things to be weak, with a reasonable > >> > default. If we start to see that "everyone" does the same thing by and > >> > large we can re-evaluate. > >> > >> Ok. > >> > >> I've fixed the bug I mentionned the other day on IRC, should I send a PR? > > > > Lets give Simon a chance to provide any other feedback here, or another > > argument to convince me that no, we don't want to have this abstracted > > by a weak function but instead ..., thanks! > > I suspect there is a reason why this is better than what I propose. > Perhaps when I try it out it will become apparent. > > So let's go ahead and revisit later if we have new information. > > Reviewed-by: Simon Glass <s...@chromium.org>
Thanks! Maxime, please re-spin with the bugfix (or wait another day or two for other feedback), repost and I'll take it in Thurs/Fri or so. -- Tom
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot