[U-Boot] [PATCH v2 0/2] env: Make environment loading log more clear

[Sam Protsenko]

Hi Yaniv,

On Wed, Jul 25, 2018 at 2:05 PM, Yaniv Levinsky
<yaniv.levinsky at compulab.co.il> wrote:
> On 07/20/2018 06:18 PM, Sam Protsenko wrote:
>> This patch series intended to make boot log better. Basically here we
>> just remove unwanted error messages, relying on the message from most
>> deep API to be printed (like mmc subsystem). At the moment this looks
>> like most clean solution to cluttered log problem, as any other solution
>> will be hackish.
>>
>> With this patch set applied we will see something like this:
>>
>>     Loading Environment from FAT... MMC: no card present
>>     Loading Environment from MMC... OK
>>
>> instead of:
>>
>>     Loading Environment from FAT... MMC: no card present
>>     ** Bad device mmc 0 **
>>     Failed (-5)
>>     Loading Environment from MMC... OK
>>
>> Sam Protsenko (2):
>>   env: Don't print "Failed" error message
>>   disk: part: Don't show redundant error message
>>
>>  disk/part.c |  2 +-
>>  env/env.c   | 12 +++++++-----
>>  2 files changed, 8 insertions(+), 6 deletions(-)
>>
>
> Hi Sam,
>
> After following the discussion from "Make U-Boot log great again" to
> here, it made me wonder: Does the user really need to be exposed to all
> the failed attempts to load the environment if it succeeded eventually?
>
> Maybe the maintainers are willing the consider a more drastic solution
> for clearing the console clutter when the environment loads.
>
> What if the only thing the user would see on a successful load is this:
>         ENV:   Loaded from MMC
> And the rest of the usual clutter would be visible only if DEBUG is set.
>

I believe we discussed something similar, and came to conclusion than
it's not good to hide useful warnings, user should see it. You may
want to go through discussion on my RFC patches I sent, I guess
Wolfgang mentioned that and we agreed.

> It shouldn't be too hard to implement (Rising GD_FLG_SILENT if DEBUG not
> defined) and it is very consistent with the rest of the printed messages
> on boot. The problem is how and what to print on a failed load.
>

On one of my RFC patches I considered using GD_FLG_SILED, but it won't
work if CONFIG_SILENT_CONSOLE is not defined (look at
common/console.c, in puts() implementation). And a lot of platforms
don't define that config option, so we can't rely on it.

Moreover, as Wolfgang mentioned in my RFC patches, we shouldn't do
post-beautifying the log, but we should instead disable all not
desired messages, and only leave most relevant one (that's what I'm
trying to do in patch series).

> I think it would be best if we could keep the above pattern like so:
>         ENV:   Failed to load from FAT - MMC: No card present (-5)
>         ENV:   Failed to load from MMC - No MMC card found (-5)
>         ENV:   Using default environment
> The last line would print only if (gd->flags & GD_FLG_ENV_DEFAULT)
>

The same problem here. We know return code only when all errors were
already printed. So it would require doing some post-cleanup, which is
not possible for all platforms. Also Wolfgang is obviously against
that approach (and I can see his point, it's better and easier to just
cleanup errors reporting). This way we will have pretty much the same
as you suggested. With my patches you'll see something like this:

    Loading Environment from FAT... MMC: no card present
    Loading Environment from MMC... OK


> This might be harder to implement, but do you think it could work?
>

Overall: please check my RFC patches I sent recently. I believe they
implement quite similar feature you suggest, but maintainers don't
think it's the best approach to the problem.

> Yaniv