[U-Boot] [RFC] Make U-Boot log great again

[Maxime Ripard]

On Wed, Feb 21, 2018 at 08:35:47AM -0500, Tom Rini wrote:
> On Tue, Feb 20, 2018 at 10:59:33AM +0100, Lukasz Majewski wrote:
> > On Tue, 20 Feb 2018 09:00:56 +0100
> > Maxime Ripard <maxime.ripard at bootlin.com> wrote:
> > 
> > > On Mon, Feb 19, 2018 at 07:47:52PM +0200, Sam Protsenko wrote:
> > > > On 18 February 2018 at 23:22, Wolfgang Denk <wd at denx.de> wrote:  
> > > > > Dear Sam,
> > > > >
> > > > > In message
> > > > > <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA at mail.gmail.com>
> > > > > you wrote:  
> > > > >>
> > > > >> Right now U-Boot and SPL logs are cluttered with bogus warnings
> > > > >> like these (on X15 board, but I'm pretty sure it should appear
> > > > >> on many others):
> > > > >>
> > > > >>     Loading Environment from FAT...
> > > > >>     *** Warning - bad CRC, using default environment  
> > > > >
> > > > > I donpt want to question the purpose of your patch series in
> > > > > genral, but:
> > > > >  
> > > > 
> > > > Oh, it's merely a discussion, not a patch series. I probably
> > > > shouldn't have been added that RFC tag, it's confusing, sorry.
> > > >   
> > > > > This is NOT a bogus warning - actually it is something which is
> > > > > not supposed to happen on any sane system.  If it does on your
> > > > > board even after first boot and running "env save" at least once,
> > > > > then you have some problem either in the design or implementation
> > > > > of your board code.
> > > > >
> > > > > So this is a very valid warning which means: FIX ME!
> > > > >  
> > > > 
> > > > AFAIU, that behavior was changed in the mentioned patch (please see
> > > > my original message). Let me elaborate a bit. In v2018.01 everything
> > > > works fine and none errors/warnings are present on my boards (AM57x
> > > > EVM and X15 board). The problem appears after commit fb69464eae1e
> > > > ("env: Allow to build multiple environments in Kconfig"). Now U-Boot
> > > > tries to load the environment from SD card first (uEnv.txt file on
> > > > FAT partition), and then from eMMC partition. In case when SD card
> > > > is not inserted, I observe mentioned errors. So I'm not sure how to
> > > > handle this properly, that's why I created this thread... Let me
> > > > try and explain my concerns better:
> > > >  1. On the one hand, it's good to check the environment on both SD
> > > > card and eMMC (that was done in mentioned patch). This case seems to
> > > > be legit (at least as far as I understand it), i.e. when SD card is
> > > > not inserted, it's fine, we just check the env on eMMC
> > > >  2. But on the other hand, errors shouldn't appear in boot log, if
> > > > it's legit case, it's confusing the user  
> > > 
> > > That patch intent was to keep the current behaviour as is for all
> > > users, so the fact that you now have the FAT environment enabled is an
> > > unwanted side-effect.
> > 
> > The same situation is on Beagle Bone Black. Even though with OE it is
> > built to use eMMC for storing its envs, by default it also has envs in
> > FAT support enabled.
> > 
> > For that reason, u-boot on this board looks for envs in FAT first and
> > similar message is printed.
> > 
> > IMHO, we now have (unintentionally) the situation where implicitly
> > reading envs from FAT has the highest priority.
> 
> It's not so much unintentional but rather that the mechanism to define
> the priority order isn't being provided specifically by
> board/ti/am335x/board.c so we get the default order.

It really was unintentional to me :)

The point of that commit really was to not introduce new environments
to anyone. The fact that we now have FAT being higher priority than
MMC is a side effect of that since we now have two environments
enabled. If we only had the MMC, we wouldn't have any issue with it,
and that was my intent.

Is there an easy way (one in tools/ ?) to try to diff two configs
between revisions?

> And one thing that I think does need to happen now is that the error
> messages about "didn't find valid environment in ..." need to be
> rethought a bit.  It would probably also make sense to move from every
> env operation tries every possible env location to env init finds the
> first valid location, tells the user clearly it's using that, and then
> always uses it.

Not all environments have an init callback, so we'd rather need to do
that at load time. However, I guess we also want to inform users if a
higher priority load has failed somehow.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180221/c853ae5e/attachment.sig>