From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Wed, 21 Feb 2018 09:54:41 -0500 Subject: [U-Boot] [RFC] Make U-Boot log great again In-Reply-To: <2384a17f-254d-8430-3207-b5af1ac0aff6@de.pepperl-fuchs.com> References: <20180218212214.2D64A24018D@gemini.denx.de> <20180220080056.ngdwvniujrsw6klb@flea.lan> <20180220105933.7d402043@jawa> <20180221133547.GH4311@bill-the-cat> <2384a17f-254d-8430-3207-b5af1ac0aff6@de.pepperl-fuchs.com> Message-ID: <20180221145441.GM4311@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 wrote: > >> > >>>On Mon, Feb 19, 2018 at 07:47:52PM +0200, Sam Protsenko wrote: > >>>>On 18 February 2018 at 23:22, Wolfgang Denk wrote: > >>>>>Dear Sam, > >>>>> > >>>>>In message > >>>>> > >>>>>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: