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

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

[Sam Protsenko]

Hi guys,

TL;DR This is a suggestion about fixing U-Boot log, which has got
worse recently.

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
    Failed (-5)

Those are the consequences of next commit:

    fb69464eae1e ("env: Allow to build multiple environments in Kconfig")

Because of this commit, I can see following changes in my .config file:

    +CONFIG_ENV_IS_IN_FAT=y
    +CONFIG_ENV_FAT_INTERFACE="mmc"
    +CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
    +CONFIG_ENV_FAT_FILE="uboot.env"

which led U-Boot to try and load the environment from different
sources. I agree that it's good thing to do and can be useful, but the
problem is that the code for loading the environment wasn't changed to
handle errors properly.

How I suggest to handle that case:

 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
 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)
 3. And only print the error message in case when U-Boot environment
wasn't found at all (on all possible sources).

I don't have enough time to fix this by my own right now. But let's
discuss how to approach this issue in a best way possible. And if
someone wants to step forward and do that -- would be really nice. If
no -- I can look into that later. But let's collect some opinions
here, first.

Thanks.

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

[Robert Nelson]

On Fri, Feb 16, 2018 at 1:01 PM, Sam Protsenko
<semen.protsenko@linaro.org> wrote:
> Hi guys,
>
> TL;DR This is a suggestion about fixing U-Boot log, which has got
> worse recently.
>
> 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
>     Failed (-5)

This one seems to cause the most confusion with end users.  They like
to think it's a real bug, when in reality, "saveenv" and friends was
just never run from within u-boot or a separate environment partition
wasn't pre-programmed.

It's one of those bugs, users asked 10 years ago when the Beagle first
launched, yet users still ask from time to time..

>
> Those are the consequences of next commit:
>
>     fb69464eae1e ("env: Allow to build multiple environments in Kconfig")
>
> Because of this commit, I can see following changes in my .config file:
>
>     +CONFIG_ENV_IS_IN_FAT=y
>     +CONFIG_ENV_FAT_INTERFACE="mmc"
>     +CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
>     +CONFIG_ENV_FAT_FILE="uboot.env"
>
> which led U-Boot to try and load the environment from different
> sources. I agree that it's good thing to do and can be useful, but the
> problem is that the code for loading the environment wasn't changed to
> handle errors properly.
>
> How I suggest to handle that case:
>
>  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
>  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)
>  3. And only print the error message in case when U-Boot environment
> wasn't found at all (on all possible sources).
>
> I don't have enough time to fix this by my own right now. But let's
> discuss how to approach this issue in a best way possible. And if
> someone wants to step forward and do that -- would be really nice. If
> no -- I can look into that later. But let's collect some opinions
> here, first.

I've also found, when you are dealing with multiple sources, it's nice
to print "where" you came from.

Otherwise, I've been known to make things way to verbose, but it's
easier to debug years later, when your adding a new family, or
overhauling when/how "overlays" are handled...

Regards,

-- 
Robert Nelson
https://rcn-ee.com/

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

[Bin Meng]

Hi,

On Sat, Feb 17, 2018 at 3:49 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
> On Fri, Feb 16, 2018 at 1:01 PM, Sam Protsenko
> <semen.protsenko@linaro.org> wrote:
>> Hi guys,
>>
>> TL;DR This is a suggestion about fixing U-Boot log, which has got
>> worse recently.
>>
>> 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
>>     Failed (-5)
>

Do we plan to fix this "Failed (-5)" message? It's very confusing.
Like previous U-Boot just printing "*** Warning - bad CRC, using
default environment" is enough I think.

> This one seems to cause the most confusion with end users.  They like
> to think it's a real bug, when in reality, "saveenv" and friends was
> just never run from within u-boot or a separate environment partition
> wasn't pre-programmed.
>
> It's one of those bugs, users asked 10 years ago when the Beagle first
> launched, yet users still ask from time to time..
>
>>
>> Those are the consequences of next commit:
>>
>>     fb69464eae1e ("env: Allow to build multiple environments in Kconfig")
>>
>> Because of this commit, I can see following changes in my .config file:
>>
>>     +CONFIG_ENV_IS_IN_FAT=y
>>     +CONFIG_ENV_FAT_INTERFACE="mmc"
>>     +CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
>>     +CONFIG_ENV_FAT_FILE="uboot.env"
>>
>> which led U-Boot to try and load the environment from different
>> sources. I agree that it's good thing to do and can be useful, but the
>> problem is that the code for loading the environment wasn't changed to
>> handle errors properly.
>>
>> How I suggest to handle that case:
>>
>>  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
>>  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)
>>  3. And only print the error message in case when U-Boot environment
>> wasn't found at all (on all possible sources).
>>
>> I don't have enough time to fix this by my own right now. But let's
>> discuss how to approach this issue in a best way possible. And if
>> someone wants to step forward and do that -- would be really nice. If
>> no -- I can look into that later. But let's collect some opinions
>> here, first.
>
> I've also found, when you are dealing with multiple sources, it's nice
> to print "where" you came from.
>
> Otherwise, I've been known to make things way to verbose, but it's
> easier to debug years later, when your adding a new family, or
> overhauling when/how "overlays" are handled...
>

Regards,
Bin

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

[Bin Meng]

Hello,

On Fri, Mar 23, 2018 at 1:44 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi,
>
> On Sat, Feb 17, 2018 at 3:49 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
>> On Fri, Feb 16, 2018 at 1:01 PM, Sam Protsenko
>> <semen.protsenko@linaro.org> wrote:
>>> Hi guys,
>>>
>>> TL;DR This is a suggestion about fixing U-Boot log, which has got
>>> worse recently.
>>>
>>> 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
>>>     Failed (-5)
>>
>
> Do we plan to fix this "Failed (-5)" message? It's very confusing.
> Like previous U-Boot just printing "*** Warning - bad CRC, using
> default environment" is enough I think.
>

The v2018.07 release still has this "Failed (-5)" message on boot.
When do we plan to fix this?

>> This one seems to cause the most confusion with end users.  They like
>> to think it's a real bug, when in reality, "saveenv" and friends was
>> just never run from within u-boot or a separate environment partition
>> wasn't pre-programmed.
>>
>> It's one of those bugs, users asked 10 years ago when the Beagle first
>> launched, yet users still ask from time to time..
>>
>>>
>>> Those are the consequences of next commit:
>>>
>>>     fb69464eae1e ("env: Allow to build multiple environments in Kconfig")
>>>
>>> Because of this commit, I can see following changes in my .config file:
>>>
>>>     +CONFIG_ENV_IS_IN_FAT=y
>>>     +CONFIG_ENV_FAT_INTERFACE="mmc"
>>>     +CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
>>>     +CONFIG_ENV_FAT_FILE="uboot.env"
>>>
>>> which led U-Boot to try and load the environment from different
>>> sources. I agree that it's good thing to do and can be useful, but the
>>> problem is that the code for loading the environment wasn't changed to
>>> handle errors properly.
>>>
>>> How I suggest to handle that case:
>>>
>>>  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
>>>  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)
>>>  3. And only print the error message in case when U-Boot environment
>>> wasn't found at all (on all possible sources).
>>>
>>> I don't have enough time to fix this by my own right now. But let's
>>> discuss how to approach this issue in a best way possible. And if
>>> someone wants to step forward and do that -- would be really nice. If
>>> no -- I can look into that later. But let's collect some opinions
>>> here, first.
>>
>> I've also found, when you are dealing with multiple sources, it's nice
>> to print "where" you came from.
>>
>> Otherwise, I've been known to make things way to verbose, but it's
>> easier to debug years later, when your adding a new family, or
>> overhauling when/how "overlays" are handled...
>>

Regards,
Bin

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

[Tom Rini]

On Tue, Jul 10, 2018 at 11:01:14AM +0800, Bin Meng wrote:
> Hello,
> 
> On Fri, Mar 23, 2018 at 1:44 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> > Hi,
> >
> > On Sat, Feb 17, 2018 at 3:49 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
> >> On Fri, Feb 16, 2018 at 1:01 PM, Sam Protsenko
> >> <semen.protsenko@linaro.org> wrote:
> >>> Hi guys,
> >>>
> >>> TL;DR This is a suggestion about fixing U-Boot log, which has got
> >>> worse recently.
> >>>
> >>> 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
> >>>     Failed (-5)
> >>
> >
> > Do we plan to fix this "Failed (-5)" message? It's very confusing.
> > Like previous U-Boot just printing "*** Warning - bad CRC, using
> > default environment" is enough I think.
> >
> 
> The v2018.07 release still has this "Failed (-5)" message on boot.
> When do we plan to fix this?

I don't mean for this to sound glib, but I was expecting patches to get
posted to change this particular area at least.  I thought it had been
talked to the point where it was understood what the next steps should
look like?  Thanks/sorry!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180710/871d3437/attachment.sig>

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

[Sam Protsenko]

On Tue, Jul 10, 2018 at 5:38 PM, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Jul 10, 2018 at 11:01:14AM +0800, Bin Meng wrote:
>> Hello,
>>
>> On Fri, Mar 23, 2018 at 1:44 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> > Hi,
>> >
>> > On Sat, Feb 17, 2018 at 3:49 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
>> >> On Fri, Feb 16, 2018 at 1:01 PM, Sam Protsenko
>> >> <semen.protsenko@linaro.org> wrote:
>> >>> Hi guys,
>> >>>
>> >>> TL;DR This is a suggestion about fixing U-Boot log, which has got
>> >>> worse recently.
>> >>>
>> >>> 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
>> >>>     Failed (-5)
>> >>
>> >
>> > Do we plan to fix this "Failed (-5)" message? It's very confusing.
>> > Like previous U-Boot just printing "*** Warning - bad CRC, using
>> > default environment" is enough I think.
>> >
>>
>> The v2018.07 release still has this "Failed (-5)" message on boot.
>> When do we plan to fix this?
>
> I don't mean for this to sound glib, but I was expecting patches to get
> posted to change this particular area at least.  I thought it had been
> talked to the point where it was understood what the next steps should
> look like?  Thanks/sorry!
>

Ok, as nobody volunteered, I'll look into this in my spare time. This
thread has all sorts of suggestions, not only about error messages. I
guess we need to see some code to have more meaningful discussion. Let
me prepare some patch, and then we can discuss if it's a way to go.

Thanks.

> --
> Tom

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

[Sam Protsenko]

On Tue, Jul 10, 2018 at 5:38 PM, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Jul 10, 2018 at 11:01:14AM +0800, Bin Meng wrote:
>> Hello,
>>
>> On Fri, Mar 23, 2018 at 1:44 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> > Hi,
>> >
>> > On Sat, Feb 17, 2018 at 3:49 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
>> >> On Fri, Feb 16, 2018 at 1:01 PM, Sam Protsenko
>> >> <semen.protsenko@linaro.org> wrote:
>> >>> Hi guys,
>> >>>
>> >>> TL;DR This is a suggestion about fixing U-Boot log, which has got
>> >>> worse recently.
>> >>>
>> >>> 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
>> >>>     Failed (-5)
>> >>
>> >
>> > Do we plan to fix this "Failed (-5)" message? It's very confusing.
>> > Like previous U-Boot just printing "*** Warning - bad CRC, using
>> > default environment" is enough I think.
>> >
>>
>> The v2018.07 release still has this "Failed (-5)" message on boot.
>> When do we plan to fix this?
>
> I don't mean for this to sound glib, but I was expecting patches to get
> posted to change this particular area at least.  I thought it had been
> talked to the point where it was understood what the next steps should
> look like?  Thanks/sorry!
>

Just to have something specific to discuss, I've sent two RFC patches:
[1,2]. I suggest we start with messages beautifying first, as this
seems to be main concern for most people.

Tom, what do you think about [1,2]? Does it have right to exist at
all, or should we go some other way?

Thanks.

[1] https://lists.denx.de/pipermail/u-boot/2018-July/335071.html
[2] https://lists.denx.de/pipermail/u-boot/2018-July/335072.html

> --
> Tom

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

[Sam Protsenko]

On Wed, Jul 18, 2018 at 1:14 AM, Sam Protsenko
<semen.protsenko@linaro.org> wrote:
> On Tue, Jul 10, 2018 at 5:38 PM, Tom Rini <trini@konsulko.com> wrote:
>> On Tue, Jul 10, 2018 at 11:01:14AM +0800, Bin Meng wrote:
>>> Hello,
>>>
>>> On Fri, Mar 23, 2018 at 1:44 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> > Hi,
>>> >
>>> > On Sat, Feb 17, 2018 at 3:49 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
>>> >> On Fri, Feb 16, 2018 at 1:01 PM, Sam Protsenko
>>> >> <semen.protsenko@linaro.org> wrote:
>>> >>> Hi guys,
>>> >>>
>>> >>> TL;DR This is a suggestion about fixing U-Boot log, which has got
>>> >>> worse recently.
>>> >>>
>>> >>> 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
>>> >>>     Failed (-5)
>>> >>
>>> >
>>> > Do we plan to fix this "Failed (-5)" message? It's very confusing.
>>> > Like previous U-Boot just printing "*** Warning - bad CRC, using
>>> > default environment" is enough I think.
>>> >
>>>
>>> The v2018.07 release still has this "Failed (-5)" message on boot.
>>> When do we plan to fix this?
>>
>> I don't mean for this to sound glib, but I was expecting patches to get
>> posted to change this particular area at least.  I thought it had been
>> talked to the point where it was understood what the next steps should
>> look like?  Thanks/sorry!
>>
>
> Just to have something specific to discuss, I've sent two RFC patches:
> [1,2]. I suggest we start with messages beautifying first, as this
> seems to be main concern for most people.
>
> Tom, what do you think about [1,2]? Does it have right to exist at
> all, or should we go some other way?
>

After some discussion, new RFC patch is ready for review: [1].
Personally, I'd also like to prepend error messages with at least two
spaces or some other prefix. But that would take introducing new
pointer in gd structure. Please let me know what you think.

[1] https://lists.denx.de/pipermail/u-boot/2018-July/335223.html

> Thanks.
>
> [1] https://lists.denx.de/pipermail/u-boot/2018-July/335071.html
> [2] https://lists.denx.de/pipermail/u-boot/2018-July/335072.html
>
>> --
>> Tom

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

[Wolfgang Denk]

Dear Sam,

In message <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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:

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!

>  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...


>  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.

>  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

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

[Sam Protsenko]

On 18 February 2018 at 23:22, Wolfgang Denk <wd@denx.de> wrote:
> Dear Sam,
>
> In message <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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

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

[Wolfgang Denk]

Dear Sam,

In message <CAKaJLVvL-E7b5XKqixM4yn=2rXW5qzRRS5ijkWRjmdetdakZ2Q@mail.gmail.com> you wrote:
> 
> 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:

I think we both are talking about the same thing.  I just see it
differently :-)

If the user configures the system to try the SDCard first for the
environment, and it cannot find it there, then this should be
considered an error situation (even if recoverable by falling back
to secondary storage), and at least a warning must be printed (or an
error, if there is no secondary storage configured).

I agree with you that both the part about the CRC error and the part
about using the default environment are wrong in this case, so the
tezt of the warning message needs to be fixed - it should say
clearly that the attempt to _read_ the environment from storage
device _XXX_ failed.

>  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

If this is what is configured, yes.  In this case I would expoect to
see something like this:

# Warning: Can't read environment from SDCard, trying now: eMMC.

or such [eventually - and if available - with additional information
like "no card present"].

>  2. But on the other hand, errors shouldn't appear in boot log, if
> it's legit case, it's confusing the user

Well, the "legit case" is that the primary storage for the
environment works.  If it is not available, this is something that
the user should be made aware of.

> 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?

It is definitely a useful feature in many situations.

On the other hand, as always you should know what you are doing. For
example:

Assume no error/warning is printed when no SDCard is present.  Then
the user might not even be aware that the SDCard is being probed.
He may assume he can secure his system for example by setting
bootdelay=0 to prevent commandline access in U-Boot.  And apparently
this will work fine for him - until someone comes along wh inserts a
SDCard with different environment settings.

I think it is a good ide to inform the user what is actually going
on...

>  2. Fix errors handling when multiple environments are enabled

THis is definitely needed, agreed.

> 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).

I think this is pretty clear: the configuration apparently selects
SDCard as the primary storage for the environment and probes it
first.  When it is not present, it tries the second best option,
eMMC.  In this case a warning is appropriate.  And i eMMC does not
work either, then an error message must be printed.

> 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

This is another configuration.  it is as valid as the other one.  I
assume you can insert a SDCard without SPL/U-Boot on it, so you will
still boot from eMMC, but read the environment from SDCard?

There are many configurations which could be implemented, but they
all should be explicit about the "search order" for the environment.

Of course ou can make this as complicated as you like.  One could
also define a policy if all storage locations are equal, so you
don't care where the environment is coming from as long as any of
them works.  Or you define a clear sequence, in which case you will
want to be informed (warning) when the primary location does not
work.  One could even make this behavious switchable at run-time.
But: do we really need that?

I would expect, that the strict order primary forst, then try next
storage devices in strict sequence, and warn for not working ones,
is what most users expect and find useful.


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
The White Rabbit put on his spectacles. "Where shall I begin,  please
your Majesty ?" he asked.
"Begin at the beginning,", the King said, very gravely,  "and  go  on
till you come to the end: then stop."                -- Lewis Carroll

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

[Maxime Ripard]

On Mon, Feb 19, 2018 at 07:47:52PM +0200, Sam Protsenko wrote:
> On 18 February 2018 at 23:22, Wolfgang Denk <wd@denx.de> wrote:
> > Dear Sam,
> >
> > In message <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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.

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/20180220/98b38f7b/attachment.sig>

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

[Lukasz Majewski]

On Tue, 20 Feb 2018 09:00:56 +0100
Maxime Ripard <maxime.ripard@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@denx.de> wrote:  
> > > Dear Sam,
> > >
> > > In message
> > > <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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.

> 
> Maxime
> 




Best regards,

Lukasz Majewski

--

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180220/9c42f728/attachment.sig>

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

[Tom Rini]

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@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@denx.de> wrote:  
> > > > Dear Sam,
> > > >
> > > > In message
> > > > <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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.

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180221/71ceb179/attachment.sig>

[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@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@denx.de> wrote:  
> > > > > Dear Sam,
> > > > >
> > > > > In message
> > > > > <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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>

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

[Tom Rini]

On Wed, Feb 21, 2018 at 02:55:29PM +0100, Maxime Ripard wrote:
> 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@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@denx.de> wrote:  
> > > > > > Dear Sam,
> > > > > >
> > > > > > In message
> > > > > > <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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?

So, what happened here is that the defconfig file says ENV_IS_IN_MMC but
we also have 'default y if MMC_OMAP_HS && TI_COMMON_CMD_OPTIONS' in
env/Kconfig.

> > 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.

Riffing off what I just said to Simon, I was thinking that at env_init()
we set the default env_driver, and then introduce a new sub-command to
'env' so that a user can specify where they want the env to be stored
(or if you do env default -f -a, re-read).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180221/264a440b/attachment.sig>

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

[Simon Goldschmidt]

On 21.02.2018 14:35, 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@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@denx.de> wrote:
>>>>> Dear Sam,
>>>>>
>>>>> In message
>>>>> <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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.
>
> 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.

But it's like that already, isnt' it? Env load selects the first valid 
location in the list and env save uses that location. All other env 
operations work on the environment in memory.

Also, for transitioning e.g. from MMC to FAT, we would need a mechanism 
to store to an environment place other than the one selected at load time.

Simon

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

[Tom Rini]

On Wed, Feb 21, 2018 at 03:38:42PM +0100, Simon Goldschmidt wrote:
> On 21.02.2018 14:35, 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@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@denx.de> wrote:
> >>>>>Dear Sam,
> >>>>>
> >>>>>In message
> >>>>><CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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.
> >
> >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.
> 
> But it's like that already, isnt' it? Env load selects the first valid
> location in the list and env save uses that location. All other env
> operations work on the environment in memory.

For saveenv we have:
for (prio = 0; (drv = env_driver_lookup(ENVOP_SAVE, prio)); prio++) {
...

> Also, for transitioning e.g. from MMC to FAT, we would need a mechanism to
> store to an environment place other than the one selected at load time.

Ah, so we have different valid use cases.  Maybe a new env sub-command,
switch?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180221/9d0bab1e/attachment.sig>

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

[Simon Goldschmidt]

On 21.02.2018 15:54, Tom Rini wrote:
> On Wed, Feb 21, 2018 at 03:38:42PM +0100, Simon Goldschmidt wrote:
>> On 21.02.2018 14:35, 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@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@denx.de> wrote:
>>>>>>> Dear Sam,
>>>>>>>
>>>>>>> In message
>>>>>>> <CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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.
>>>
>>> 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.
>> But it's like that already, isnt' it? Env load selects the first valid
>> location in the list and env save uses that location. All other env
>> operations work on the environment in memory.
> For saveenv we have:
> for (prio = 0; (drv = env_driver_lookup(ENVOP_SAVE, prio)); prio++) {
> ...

Yes, but I think env_get_location() called from env_driver_lookup() 
returns gd->env_load_location for saving...
That seems a little hidden though...

>
>> Also, for transitioning e.g. from MMC to FAT, we would need a mechanism to
>> store to an environment place other than the one selected at load time.
> Ah, so we have different valid use cases.  Maybe a new env sub-command,
> switch?
>

Something like that, yes.

Simon

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

[Maxime Ripard]

On Wed, Feb 21, 2018 at 09:54:41AM -0500, Tom Rini wrote:
> On Wed, Feb 21, 2018 at 03:38:42PM +0100, Simon Goldschmidt wrote:
> > On 21.02.2018 14:35, 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@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@denx.de> wrote:
> > >>>>>Dear Sam,
> > >>>>>
> > >>>>>In message
> > >>>>><CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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.
> > >
> > >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.
> > 
> > But it's like that already, isnt' it? Env load selects the first valid
> > location in the list and env save uses that location. All other env
> > operations work on the environment in memory.
> 
> For saveenv we have:
> for (prio = 0; (drv = env_driver_lookup(ENVOP_SAVE, prio)); prio++) {
> ...

In the default case, env_driver_lookup will return the location where
the environment with the highest priority has been loaded.

> > Also, for transitioning e.g. from MMC to FAT, we would need a mechanism to
> > store to an environment place other than the one selected at load time.
> 
> Ah, so we have different valid use cases.  Maybe a new env sub-command,
> switch?

We implemented it by overwriting the priority look up.

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/8f84f0d0/attachment.sig>

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

[Tom Rini]

On Wed, Feb 21, 2018 at 04:07:03PM +0100, Maxime Ripard wrote:
> On Wed, Feb 21, 2018 at 09:54:41AM -0500, Tom Rini wrote:
> > On Wed, Feb 21, 2018 at 03:38:42PM +0100, Simon Goldschmidt wrote:
> > > On 21.02.2018 14:35, 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@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@denx.de> wrote:
> > > >>>>>Dear Sam,
> > > >>>>>
> > > >>>>>In message
> > > >>>>><CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8GZo3zJDr-VgW-MUCFzA@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.
> > > >
> > > >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.
> > > 
> > > But it's like that already, isnt' it? Env load selects the first valid
> > > location in the list and env save uses that location. All other env
> > > operations work on the environment in memory.
> > 
> > For saveenv we have:
> > for (prio = 0; (drv = env_driver_lookup(ENVOP_SAVE, prio)); prio++) {
> > ...
> 
> In the default case, env_driver_lookup will return the location where
> the environment with the highest priority has been loaded.
> 
> > > Also, for transitioning e.g. from MMC to FAT, we would need a mechanism to
> > > store to an environment place other than the one selected at load time.
> > 
> > Ah, so we have different valid use cases.  Maybe a new env sub-command,
> > switch?
> 
> We implemented it by overwriting the priority look up.

The general sunxi case is "We need to move from offset to FAT file" as
there's not really generally other valid cases.  But on other SoCs we
can both say that it's valid to move from eMMC (we put env in an
unbootable by the SoC boot partition) to FAT, or not want to move it at
all.

Another case I've been thinking of, but would be handled by the code
as-is, is the "Beagleboard vs Beagleboard xM" case where the board code
needs to check which we're on and keep env on NAND or finally have env
on file on FAT".  But even then, allowing an explicit switch might be
needed, if people want to migrate env to FAT, even on original
Beagleboard.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180221/48df2d55/attachment.sig>

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

[Maxime Ripard]

On Wed, Feb 21, 2018 at 10:14:47AM -0500, Tom Rini wrote:
> > > > Also, for transitioning e.g. from MMC to FAT, we would need a mechanism to
> > > > store to an environment place other than the one selected at load time.
> > > 
> > > Ah, so we have different valid use cases.  Maybe a new env sub-command,
> > > switch?
> > 
> > We implemented it by overwriting the priority look up.
> 
> The general sunxi case is "We need to move from offset to FAT file" as
> there's not really generally other valid cases.  But on other SoCs we
> can both say that it's valid to move from eMMC (we put env in an
> unbootable by the SoC boot partition) to FAT, or not want to move it at
> all.
> 
> Another case I've been thinking of, but would be handled by the code
> as-is, is the "Beagleboard vs Beagleboard xM" case where the board code
> needs to check which we're on and keep env on NAND or finally have env
> on file on FAT".  But even then, allowing an explicit switch might be
> needed, if people want to migrate env to FAT, even on original
> Beagleboard.

Ah, right. That definitely makes sense.

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/20180222/0e27a32c/attachment.sig>

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

[Wolfgang Denk]

Dear Lukasz,

In message <20180220105933.7d402043@jawa> you wrote:
>
> > 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.

This makes clear that this area needs more than just a casual "fix".
We must make sure to implement a policy that is consistent and
useful:

1. The user should be able to select which devices are probed for
   the environment.
2. He should be able to specify the exact order in which this
   probing takes place.
3. Enough information should be printed so the user knows where his
   environment is coming from, and which devices may have been
   probed without success.  The messages should be clear and easy
   to understand.

Obviously it is not possible to adjust this configuration at
run-time - accessing some environment variable to determine where to
read the environment from is a nice chicken-and-egg problem.  So
this is some _compile time_ configuration, i. e. it probably
requires some Kconfig magic plus appropriate code changes.

Just fixing an error message is not enough.

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
If I had to live my life again,  I'd  make  the  same  mistakes, only
sooner.                                          -- Tallulah Bankhead