[U-Boot] [PATCH 2/2] env: Add back default action of get_char in env_get_char()

[Maxime Ripard]

On Thu, Feb 08, 2018 at 10:52:20AM +0100, Simon Goldschmidt wrote:
> On 08.02.2018 09:47, Maxime Ripard wrote:
> > On Wed, Feb 07, 2018 at 02:17:12PM -0800, York Sun wrote:
> > > Commit 8a3a7e2270b3 ("env: Pass additional parameters to the env
> > > lookup function") dropped the default action if driver doesn't have
> > > get_char() defined. This causes failure to get environmental
> > > variables from NOR flash. Add back this default action for now.
> > > 
> > > Signed-off-by: York Sun <york.sun at nxp.com>
> > > CC: Maxime Ripard <maxime.ripard at free-electrons.com>
> > > ---
> > > Limited test on LS1043ARDB.
> > > 
> > >   env/env.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/env/env.c b/env/env.c
> > > index edfb575..210bae2 100644
> > > --- a/env/env.c
> > > +++ b/env/env.c
> > > @@ -159,7 +159,7 @@ int env_get_char(int index)
> > >   		int ret;
> > >   		if (!drv->get_char)
> > > -			continue;
> > > +			return *(uchar *)(gd->env_addr + index);
> > Thinking more about this, I think this would break the case where the
> > first environment in your list has no get_char method, but the second
> > might.
> 
> 
> How can we decide which way is wanted? With your patch below, we might end
> up loading chars from a low-prio environment (which is not CRC checked) in
> the early boot stage. Later, we load the environment from another env driver
> with higher priority.

Ah, right.

> That's why I suggested removing the 'get_char' callback from the env drivers
> :-) Early boot stage environment lookups would still work the old way
> reading from 'gd->env_addr + index'.

If that works on York's board, I'm all in.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com