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

[Sam Protsenko]

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

Please correct me if I'm wrong, I'm just trying to come up with
correct perspective about what should be done about it. Like:
 1. Just disable SD card environment (i.e. revert that patch changes
for my board). But it's probably a useful feature?
 2. Fix errors handling when multiple environments are enabled
 3. Some other solution (which I probably don't know about, yet)

>>  1. If we have two sources for the environment (e.g. FAT partition on
>> SD card and some raw partition on eMMC), we shouldn't print error
>> messages if we were unable to load the environment from one source
>
> This needs specification.
>
> If there are several potential sources for the environment, it can
> be assumed that the first one is supposed to be the primary one, so
> if this does not work, there _should_ be a warning (or even error)
> message before U-Boot falls back to alternative storage.
>
> However, in such a case the "using default environment" is
> apparently wrong...
>

In my case, we have two sources for the env: SD card and eMMC. And we
have 2 possible state of the board:
 1. SD card is inserted. U-Boot will be ran from SD card
 2. SD card is not inserted. U-Boot will be ran from eMMC

The question is, how we are supposed to handle environment for those
two cases? Right now it's being tested from SD card first, then (if
not found), it's going to be tested from eMMC. That's where the error
appears (if SD card is not inserted).

>
>>  2. We should probably print some human-readable information that we
>> didn't find the environment there, let's skip and look for next source
>> (but don't print those warnings/failed messages)
>
> For any attempt to read the environment from some storage device
> which fails, a warning SHALL be printed, because this is not
> supposed to happen.
>

Ok, you are saying that the error shouldn't happen. But the only way I
see to change that, is to implement conditional probing for the
environment. Like:
 1. If we are being booted from SD card, load the environment only from SD card
 2. If we are being booted from eMMC, load the environment only from eMMC

Am I correct? If so, it's not how it's implemented right now (judging
from those errors in logs)... I want to be absolutely clear about how
do we want to see this, before doing something...

Thanks.

>>  3. And only print the error message in case when U-Boot environment
>> wasn't found at all (on all possible sources).
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> When in doubt, mumble;   when in trouble, delegate;  when in  charge,
> ponder.                                             -- James H. Boren