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.rip...@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 <w...@denx.de> wrote:
Dear Sam,

In message
<CAKaJLVsWKpGeEuS=iZ7QCtZrDfUSA=8gzo3zjdr-vgw-muc...@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 mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to