| Message ID | 20180717220912.11358-1-semen.protsenko@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,1/2] env: Drop error messages when loading environment | expand |
Dear Sam, In message <20180717220912.11358-1-semen.protsenko@linaro.org> you wrote: > This is just a draft to discuss ideas related to "Make U-Boot log great > again" thread. > > With this patch we will have something like this: > > Loading Environment from FAT... Failed (-5) > Loading Environment from MMC... OK > > instead of this: > > Loading Environment from FAT... MMC: no card present > ** Bad device mmc 0 ** > Failed (-5) > Loading Environment from MMC... OK Why do you think an error message "Failed (-5)" looks great? From a user's point of view, the "MMC: no card present" is _much_ better (but of course it could still be improved). Printing cryptic error codes has never been a good idea and is definitly not a "great" idea. Best regards, Wolfgang Denk
On Wed, Jul 18, 2018 at 9:19 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear Sam, > > In message <20180717220912.11358-1-semen.protsenko@linaro.org> you wrote: >> This is just a draft to discuss ideas related to "Make U-Boot log great >> again" thread. >> >> With this patch we will have something like this: >> >> Loading Environment from FAT... Failed (-5) >> Loading Environment from MMC... OK >> >> instead of this: >> >> Loading Environment from FAT... MMC: no card present >> ** Bad device mmc 0 ** >> Failed (-5) >> Loading Environment from MMC... OK > > Why do you think an error message "Failed (-5)" looks great? > As I mentioned in commit message, this RFC series is merely for discussing ideas. I don't think it's "great" (for the same reasons you mentioned), but this is one of things we *could* do technically, to make boot log straight. What I mean "technically" doable: for example we would like to see log like this: Loading Environment from FAT... Failed (-5): -> MMC: no card present -> ** Bad device mmc 0 ** Loading Environment from MMC... OK But to do so, we would probably need to do one of these: 1. Rework the code for all boot sources (like drivers/mmc/mmc.c), so that that code doesn't print warnings to console, but instead filling some error buffer, so we can print that buffer later from env.c. I'm not sure it's a sane idea 2. Issuing some escape codes for moving cursor up and down, to modify already printed message, also has a lot of implications and I don't thing it's sane as well... 3. Messing with GD_FLG_RECORD and GD_FLG_SILENT also doesn't seem to be right here, as much of platforms don't enable CONFIG_CONSOLE_RECORD and CONFIG_SILENT_CONSOLE So *technically*, we can only do what I did in two RFC patches I sent. The only other way I see, is to make boot log look like this: --> Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 ** Failed (-5): --> Loading Environment from MMC... OK Not sure if I like this way, but it's doable. If you have any preferences about what I said, or if you have any other ideas on how to approach this, please share. That's the whole reason why I sent this RFC series :) > From a user's point of view, the "MMC: no card present" > is _much_ better (but of course it could still be improved). > > Printing cryptic error codes has never been a good idea and is > definitly not a "great" idea. > > 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@denx.de > Fools ignore complexity. Pragmatists suffer it. Some can avoid it. > Geniuses remove it. > - Perlis's Programming Proverb #58, SIGPLAN Notices, Sept. 1982
On Wed, Jul 18, 2018 at 08:19:50AM +0200, Wolfgang Denk wrote: > Dear Sam, > > In message <20180717220912.11358-1-semen.protsenko@linaro.org> you wrote: > > This is just a draft to discuss ideas related to "Make U-Boot log great > > again" thread. > > > > With this patch we will have something like this: > > > > Loading Environment from FAT... Failed (-5) > > Loading Environment from MMC... OK > > > > instead of this: > > > > Loading Environment from FAT... MMC: no card present > > ** Bad device mmc 0 ** > > Failed (-5) > > Loading Environment from MMC... OK > > Why do you think an error message "Failed (-5)" looks great? > > From a user's point of view, the "MMC: no card present" > is _much_ better (but of course it could still be improved). > > Printing cryptic error codes has never been a good idea and is > definitly not a "great" idea. Agreed, I don't think we want to go down the path of just suppressing some of these error messages, that's not helpful. -- Tom
diff --git a/env/env.c b/env/env.c index 5c0842ac07..85598fa5d4 100644 --- a/env/env.c +++ b/env/env.c @@ -187,6 +187,7 @@ int env_load(void) for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) { int ret; + unsigned long have_console = gd->have_console; if (!drv->load) continue; @@ -195,7 +196,11 @@ int env_load(void) continue; printf("Loading Environment from %s... ", drv->name); + + /* Suppress console output for drv->load() */ + gd->have_console = 0; ret = drv->load(); + gd->have_console = have_console; if (ret) printf("Failed (%d)\n", ret); else
This is just a draft to discuss ideas related to "Make U-Boot log great again" thread. With this patch we will have something like this: Loading Environment from FAT... Failed (-5) Loading Environment from MMC... OK instead of this: Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 ** Failed (-5) Loading Environment from MMC... OK Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> --- env/env.c | 5 +++++ 1 file changed, 5 insertions(+)