Hi,
On Tue, Jan 09, 2018 at 11:05:51AM -0500, Tom Rini wrote:
> On Tue, Jan 09, 2018 at 02:10:47PM +0100, Maxime Ripard wrote:
> > Hi,
> >
> > On Sun, Jan 07, 2018 at 09:52:23PM -0700, Simon Glass wrote:
> > > >> On 28 November 2017 at 03:24, 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: Lukasz Majewski <lu...@denx.de>
> > > >> > Signed-off-by: Maxime Ripard <maxime.rip...@free-electrons.com>
> > > >> > ---
> > > >> > env/env.c | 2 +-
> > > >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >> >
> > > >> > diff --git a/env/env.c b/env/env.c
> > > >> > index b4d8886e7a69..9b8b38cf3c2b 100644
> > > >> > --- a/env/env.c
> > > >> > +++ b/env/env.c
> > > >> > @@ -62,7 +62,7 @@ static enum env_location env_locations[] = {
> > > >> >
> > > >> > static enum env_location env_load_location;
> > > >> >
> > > >> > -static enum env_location env_get_location(enum env_operation op,
> > > >> > int prio)
> > > >> > +__weak enum env_location env_get_location(enum env_operation op,
> > > >> > int prio)
> > > >>
> > > >> Is it possible instead to make this deterministic, so that the board
> > > >> sets up the location during boot?
> > > >
> > > > I'm not really sure what you mean here. The default implementation is
> > > > deterministic, and the board implementations should make sure that it
> > > > is if it's of any value to them.
> > > >
> > > > Do you want me to add a comment to make that clearer?
> > >
> > > I mean that the board presumably knows the location, so could set it
> > > up (perhaps in global_data). Then env_get_location() can use it.
> >
> > Well, it's pretty much what happens when you only have a single
> > environment selected in Kconfig, except that you don't need to change
> > anything in all the boards, which would be the case with your solution
> > I guess?
> >
> > > Making functions weak means everything becomes non-deterministic -
> > > e.g. I cannot tell from the code what it is going to do.
> >
> > One of the point of this serie is to allow boards to select the
> > environment locations based on data of their choice, so it's kind of
> > expected that that function should be overriden I guess?
>
> I am generally a fan of using __weak functions as well, with the weak
> version being clear about what an overriding version is supposed to do,
> when it needs to be overridden.I'll add a comment then to detail what is expected from this function, and any function that might override it. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
