env: Drop error messages when loading environment

RFC New Sam Protsenko 3 participants 15 messages Jul 17, 2018 – Jul 19, 2018 Generic include/asm header files

lore

Share & tools

lore

Sam Protsenko

author

2018-07-17 22:09 UTC #0 lore raw

RFC 1/2 env: Drop error messages when loading environment

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 <[email protected]>

---
 env/env.c | 5 +++++
 1 file changed, 5 insertions(+)

env/env.c +5 -0

index 5c0842ac07..85598fa5d4 100644
@@ -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

Signature · 1 line

2.18.0

Sam Protsenko

author

2018-07-17 22:09 UTC #1 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

This is just a draft to discuss ideas related to "Make U-Boot log great again" thread.

With this patch we will see something like:

    Loading Environment from FAT...
       --> MMC: no card present
       --> ** Bad device mmc 0 **
       --> Failed (-5)
    Loading Environment from MMC...
       --> OK

instead of:

Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK

Signed-off-by Sam Protsenko <[email protected]>

---
 common/console.c                  | 8 ++++++++
 env/env.c                         | 4 +++-
 include/asm-generic/global_data.h | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

common/console.c +8 -0

index 2ba33dc574..1bbafa33a1 100644
@@ -533,6 +533,14 @@ void putc(const char c)
 
 void puts(const char *s)
 {
+	if (gd->pr_prefix) {
+		const char *prefix = gd->pr_prefix;
+
+		gd->pr_prefix = NULL;
+		puts(prefix);
+		gd->pr_prefix = prefix;
+	}
+
 #ifdef CONFIG_DEBUG_UART
 	if (!gd || !(gd->flags & GD_FLG_SERIAL_READY)) {
 		while (*s) {

env/env.c +3 -1

index 5c0842ac07..56a105ea55 100644
@@ -194,12 +194,14 @@ int env_load(void)
 		if (!env_has_inited(drv->location))
 			continue;
 
-		printf("Loading Environment from %s... ", drv->name);
+		printf("Loading Environment from %s...\n", drv->name);
+		gd->pr_prefix = "   --> ";
 		ret = drv->load();
 		if (ret)
 			printf("Failed (%d)\n", ret);
 		else
 			printf("OK\n");
+		gd->pr_prefix = NULL;
 
 		if (!ret)
 			return 0;

include/asm-generic/global_data.h +1 -0

index 0fd4900392..32b80db96b 100644
@@ -44,6 +44,7 @@ typedef struct global_data {
 	unsigned long board_type;
 #endif
 	unsigned long have_console;	/* serial_init() was called */
+	const char *pr_prefix;		/* print prefix before message */
 #if CONFIG_IS_ENABLED(PRE_CONSOLE_BUFFER)
 	unsigned long precon_buf_idx;	/* Pre-Console buffer index */
 #endif

Signature · 1 line

2.18.0

Wolfgang Denk

NAK

2018-07-18 06:23 UTC #2 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

Dear Sam,

Quoted 9 lines · In message <[email protected]> you wrote:

This is just a draft to discuss ideas related to "Make U-Boot log great again" thread.
With this patch we will see something like:
    Loading Environment from FAT...
       --> MMC: no card present
       --> ** Bad device mmc 0 **
       --> Failed (-5)

This may be ok in the error case, but...

Quoted 2 lines

Loading Environment from MMC...
--> OK

it is definitely really ugly in the normal, non-error case.

NAK.

common/console.c void putc(const char c) · line 543

Quoted 10 lines

void puts(const char *s)
{

common/console.c +8 -0

+	if (gd->pr_prefix) {
+		const char *prefix = gd->pr_prefix;
+
+		gd->pr_prefix = NULL;
+		puts(prefix);
+		gd->pr_prefix = prefix;
+	}
+

Besides - global data is a precious resource on may systems, sometimes limited to very few kB of memory. It should be only used to the extend where it really cannot be avoided, but not for any such "beautifying" stuff.

Best regards,

Wolfgang Denk

Signature · 5 lines

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
There are no data that cannot be plotted on a straight  line  if  the
axis are chosen correctly.

Tom Rini

2018-07-18 12:53 UTC #3 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

In reply to Sam Protsenko

Quoted 12 lines · Sam Protsenko

This is just a draft to discuss ideas related to "Make U-Boot log great again" thread.
With this patch we will see something like:
    Loading Environment from FAT...
       --> MMC: no card present
       --> ** Bad device mmc 0 **
       --> Failed (-5)
    Loading Environment from MMC...
       --> OK

Quoted 6 lines

instead of:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK

So, I think maybe (and given Wolfgang's comments) we should think about how the output might want to look, and how to get there without GD changes. Perhaps:
Attempting to load Environment from FAT (do we have more easily available info at this point?):
MMC: no card present
** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC...
Attempting to load Environment from MMC:
Succeeded

Signature · 8 lines

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

Sam Protsenko

author

2018-07-18 13:04 UTC #4 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

Quoted 21 lines · Tom Rini

Quoted 12 lines · Sam Protsenko
This is just a draft to discuss ideas related to "Make U-Boot log great again" thread.
With this patch we will see something like:
    Loading Environment from FAT...
       --> MMC: no card present
       --> ** Bad device mmc 0 **
       --> Failed (-5)
    Loading Environment from MMC...
       --> OK
Quoted 6 lines
instead of:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK

Quoted 5 lines

So, I think maybe (and given Wolfgang's comments) we should think about how the output might want to look, and how to get there without GD changes. Perhaps:
Attempting to load Environment from FAT (do we have more easily available info at this point?):

Which exactly info do you mean?

Quoted 7 lines

MMC: no card present
** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC...
Attempting to load Environment from MMC:
Succeeded

What do you think if we add some prefix to first message, like:

---> Attempting to load Environment from FAT:
MMC: no card present
** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC...
---> Attempting to load Environment from MMC:
Succeeded

just to emphasize that possible errors are belong to prefixed line? Does it seem better or more ugly to you?

Overall, I agree that this is probably the only sane thing we can do right now, without meddling too much with drivers code.

Quoted 2 lines

--
Tom

Tom Rini

2018-07-18 14:28 UTC #5 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

Quoted 22 lines · Sam Protsenko

Quoted 21 lines · Tom Rini
Quoted 18 lines · Sam Protsenko
This is just a draft to discuss ideas related to "Make U-Boot log great again" thread.
With this patch we will see something like:
    Loading Environment from FAT...
       --> MMC: no card present
       --> ** Bad device mmc 0 **
       --> Failed (-5)
    Loading Environment from MMC...
       --> OK
instead of:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK

Quoted 7 lines

Quoted 5 lines
So, I think maybe (and given Wolfgang's comments) we should think about how the output might want to look, and how to get there without GD changes. Perhaps:
Attempting to load Environment from FAT (do we have more easily available info at this point?):
Which exactly info do you mean?

Do we easily know things like what device / partition we're trying? Or only "env type is $X" ?

Quoted 18 lines

Quoted 7 lines
MMC: no card present
** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC...
Attempting to load Environment from MMC:
Succeeded
What do you think if we add some prefix to first message, like:
---> Attempting to load Environment from FAT:
MMC: no card present
** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC...
---> Attempting to load Environment from MMC:
Succeeded

Quoted 2 lines

just to emphasize that possible errors are belong to prefixed line? Does it seem better or more ugly to you?

I don't know. I'm not a fan, but I don't always have the best taste.

Signature · 8 lines

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/20180718/1770e7be/attachment.sig>

Wolfgang Denk

2018-07-18 14:09 UTC #6 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

In reply to Tom Rini

Dear Tom,

Quoted 15 lines · In message <20180718125351.GE4609@bill-the-cat> you wrote:

Quoted 8 lines
    Loading Environment from FAT...
       --> MMC: no card present
       --> ** Bad device mmc 0 **
       --> Failed (-5)
    Loading Environment from MMC...
       --> OK
Quoted 6 lines
instead of:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK

Quoted 11 lines

So, I think maybe (and given Wolfgang's comments) we should think about how the output might want to look, and how to get there without GD changes. Perhaps:
Attempting to load Environment from FAT (do we have more easily available info at this point?):
MMC: no card present
** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC...
Attempting to load Environment from MMC:
Succeeded

Just my 0.02â‚¬:

In the non-error case, the output should be a single (ideally short) line.

Rationale: to many lines of ourput clutter your screen and make you miss context faster; to many/long lines take time to print so they make booting slower.

In the error case, the user should be able to understand what the problem was and decide if it was critical or can be ignored (like here when intentionally booting without SDCard).

Best regards,

Wolfgang Denk

Signature · 4 lines

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
"Wish not to seem, but to be, the best."                  - Aeschylus

Tom Rini

2018-07-19 12:52 UTC #7 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

Quoted 35 lines · Wolfgang Denk

Dear Tom,
Quoted 15 lines · In message <20180718125351.GE4609@bill-the-cat> you wrote:
Quoted 8 lines
    Loading Environment from FAT...
       --> MMC: no card present
       --> ** Bad device mmc 0 **
       --> Failed (-5)
    Loading Environment from MMC...
       --> OK
Quoted 6 lines
instead of:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK
Quoted 11 lines
So, I think maybe (and given Wolfgang's comments) we should think about how the output might want to look, and how to get there without GD changes. Perhaps:
Attempting to load Environment from FAT (do we have more easily available info at this point?):
MMC: no card present
** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC...
Attempting to load Environment from MMC:
Succeeded
Just my 0.02â‚¬:
In the non-error case, the output should be a single (ideally short) line.

Quoted 7 lines

Rationale: to many lines of ourput clutter your screen and make you miss context faster; to many/long lines take time to print so they make booting slower.
In the error case, the user should be able to understand what the problem was and decide if it was critical or can be ignored (like here when intentionally booting without SDCard).

I understand, but I don't know if we can get there still. The problem is we don't know if we've succeeded until we've done the relevant probing and that in turn is what's breaking the single line, and got us to where we are now.

Signature · 8 lines

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/20180719/5878a7ad/attachment.sig>

Sam Protsenko

author

2018-07-19 13:12 UTC #8 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

Quoted 40 lines · Tom Rini

Quoted 39 lines · Wolfgang Denk
Dear Tom,
Quoted 15 lines · In message <20180718125351.GE4609@bill-the-cat> you wrote:
Quoted 8 lines
    Loading Environment from FAT...
       --> MMC: no card present
       --> ** Bad device mmc 0 **
       --> Failed (-5)
    Loading Environment from MMC...
       --> OK
Quoted 6 lines
instead of:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK
Quoted 11 lines
So, I think maybe (and given Wolfgang's comments) we should think about how the output might want to look, and how to get there without GD changes. Perhaps:
Attempting to load Environment from FAT (do we have more easily available info at this point?):
MMC: no card present
** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC...
Attempting to load Environment from MMC:
Succeeded
Just my 0.02â‚¬:
In the non-error case, the output should be a single (ideally short) line.
Rationale: to many lines of ourput clutter your screen and make you miss context faster; to many/long lines take time to print so they make booting slower.

Quoted 9 lines

Quoted 3 lines
In the error case, the user should be able to understand what the problem was and decide if it was critical or can be ignored (like here when intentionally booting without SDCard).
I understand, but I don't know if we can get there still. The problem is we don't know if we've succeeded until we've done the relevant probing and that in turn is what's breaking the single line, and got us to where we are now.

Actually we can, please see my new RFC patch [1]. It's a bit hacky, but the only other way to do so is to rework drivers (MMC, etc).

Also, I figured how to do prefixing (to display MMC errors as nested w.r.t. "Loading environment), without adding new field to gd. We can just add some new GD_LG_ and print prefix when it's installed. I'm gonna send new RFC soon. Please let me know what you think about [1].

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

Quoted 2 lines

--
Tom

Wolfgang Denk

2018-07-19 19:52 UTC #9 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

Dear Sam,

Quoted 5 lines · In message <CAKaJLVuxoS0mz9pDDYqmqomsSfd83kSed=LZ261=wfUUC-Q-Xw@mail.gmail.com>…

Also, I figured how to do prefixing (to display MMC errors as nested w.r.t. "Loading environment), without adding new field to gd. We can just add some new GD_LG_ and print prefix when it's installed. I'm gonna send new RFC soon. Please let me know what you think about [1].

This is IMO a totally wrong approach. We don't want to add more code (and more output) jsut to paper over inconsistent error reporting. This problem should be fixed at the root cause, i. e. we should report errors where they are detected, and only once. If all callers can rely that, in the error path, their callees which return error codes, have already printed appropriate messages, there is no need to print even more lines.

What we need is not fancy code, but consistent (and consequent) error handling.

Best regards,

Wolfgang Denk

Signature · 7 lines

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
"One day," said a dull voice from down below, "I'm going to  be  back
in  form again and you're going to be very sorry you said that. For a
very long time. I might even go so far as to make even more Time just
for you to be sorry in."              - Terry Pratchett, _Small Gods_

Sam Protsenko

author

2018-07-19 20:16 UTC #10 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

Quoted 17 lines · Wolfgang Denk

Dear Sam,
Quoted 5 lines · In message <CAKaJLVuxoS0mz9pDDYqmqomsSfd83kSed=LZ261=wfUUC-Q-Xw@mail.gmail.com>…
Also, I figured how to do prefixing (to display MMC errors as nested w.r.t. "Loading environment), without adding new field to gd. We can just add some new GD_LG_ and print prefix when it's installed. I'm gonna send new RFC soon. Please let me know what you think about [1].
This is IMO a totally wrong approach. We don't want to add more code (and more output) jsut to paper over inconsistent error reporting. This problem should be fixed at the root cause, i. e. we should report errors where they are detected, and only once. If all callers can rely that, in the error path, their callees which return error codes, have already printed appropriate messages, there is no need to print even more lines.

Quoted 3 lines

What we need is not fancy code, but consistent (and consequent) error handling.

As a matter of fact, that was first thing that came into my mind when I started looking into this (and actually I mentioned that way in my first RFC, I guess). I will try to investigate it further and come back with more meaningful patch.

My concern w.r.t. this approach is next: if we suppress consequent warning messages, there might be some other use-case (other than loading environment), where there is another caller of that API, and we would just lose all error messages at all. I'll try to check if we have such cases as well.

Overall: agree with your review, thanks.

Quoted 12 lines

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 "One day," said a dull voice from down below, "I'm going to be back in form again and you're going to be very sorry you said that. For a very long time. I might even go so far as to make even more Time just
for you to be sorry in." - Terry Pratchett, _Small Gods_

Wolfgang Denk

2018-07-19 19:49 UTC #11 lore raw

RFC 2/2 env: Add prefix to error messages when loading env

In reply to Tom Rini

Dear Tom,

Quoted 5 lines · In message <20180719125230.GJ4609@bill-the-cat> you wrote:

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK

...

Quoted 7 lines

Quoted 7 lines
In the non-error case, the output should be a single (ideally short) line.
Rationale: to many lines of ourput clutter your screen and make you miss context faster; to many/long lines take time to print so they make booting slower.

Quoted 8 lines

Quoted 3 lines
In the error case, the user should be able to understand what the problem was and decide if it was critical or can be ignored (like here when intentionally booting without SDCard).
I understand, but I don't know if we can get there still. The problem is we don't know if we've succeeded until we've done the relevant probing and that in turn is what's breaking the single line, and got us to where we are now.

Well, IMO the approach should be the other way round - think about where we print errror messages, and when.

In the example above the "MMC: no card present" makes most sense.

When we come to printing the "** Bad device mmc 0 **" we should be in an error path, where all possible causes have already printed an appropriate message, so this line can be removed.

Ditto for the "Failed (-5)" which is pretty useless anyway - if error handling was consequent, this message should never be needed, as all error paths ending there would have printed proper messages long before.

So instead of adding prefixes or fancy postformatting we should clean up error handling and use a consistent style to report the errors where they are found and the causes for the errors are known.

Thanks.

Best regards,

Wolfgang Denk

Signature · 7 lines

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
Die meisten Menschen pflegen im Kindesalter vom Zeigen auf Gegenstän-
de (Mausbewegung) und "ga" sagen  (Mausklick)  abzukommen,  zugunsten
eines  mächtigeren  und langwierig zu erlernenden Tools (Sprache).
                             -- Achim Linder in de.comp.os.linux.misc

Wolfgang Denk

2018-07-18 06:19 UTC #12 lore raw

RFC 1/2 env: Drop error messages when loading environment

In reply to Sam Protsenko

Dear Sam,

Quoted 8 lines · In message <[email protected]> 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

Quoted 6 lines

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

Signature · 6 lines

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
Fools ignore complexity. Pragmatists suffer it. Some  can  avoid  it.
Geniuses remove it.
     - Perlis's Programming Proverb #58, SIGPLAN Notices, Sept.  1982

Sam Protsenko

author

2018-07-18 12:50 UTC #13 lore raw

RFC 1/2 env: Drop error messages when loading environment

Quoted 11 lines · Wolfgang Denk

Dear Sam,
Quoted 8 lines · In message <[email protected]> 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

Quoted 9 lines

Quoted 6 lines
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 :)

Quoted 17 lines

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 at denx.de Fools ignore complexity. Pragmatists suffer it. Some can avoid it. Geniuses remove it.
- Perlis's Programming Proverb #58, SIGPLAN Notices, Sept. 1982

Tom Rini

2018-07-18 12:51 UTC #14 lore raw

RFC 1/2 env: Drop error messages when loading environment

In reply to Wolfgang Denk

Quoted 18 lines · Wolfgang Denk

Dear Sam,
Quoted 8 lines · In message <[email protected]> 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
Quoted 6 lines
instead of this:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 **
Failed (-5)
Loading Environment from MMC... OK

Quoted 7 lines

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.

Signature · 8 lines

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/20180718/f87fd88e/attachment.sig>