[U-Boot] [PATCH v3 0/2] env: Make environment loading log more clear

This patch series intended to make boot log better. Basically here we just remove unwanted error messages, relying on the message from most deep API to be printed (like mmc subsystem). At the moment this looks like most clean solution to cluttered log problem, as any other solution will be hackish.
With this patch set applied we will see something like this:
Loading Environment from FAT... MMC: no card present 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
Sam Protsenko (2): env: Don't show "Failed" error message disk: part: Don't show redundant error message
disk/part.c | 2 +- env/env.c | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-)

"Failed" error message from env_load() only clutters the log with unnecessary details, as we already have all needed warnings by that time. Example:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 ** Failed (-5)
Let's only print it in case when DEBUG is defined to keep log clear.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- Changes in v2: - Join two consecutive "if (!ret)" conditions - Add the comment with requirement for underlying API to print error message (as we don't print "Failed" message anymore) Changes in v3: - Print "Failed" message using debug() rather than removing it
env/env.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/env/env.c b/env/env.c index 5c0842ac07..dc2999625b 100644 --- a/env/env.c +++ b/env/env.c @@ -195,14 +195,18 @@ int env_load(void) continue;
printf("Loading Environment from %s... ", drv->name); + /* + * In error case, the error message must be printed during + * drv->load() in some underlying API, and it must be exactly + * one message. + */ ret = drv->load(); - if (ret) - printf("Failed (%d)\n", ret); - else + if (ret) { + debug("Failed (%d)\n", ret); + } else { printf("OK\n"); - - if (!ret) return 0; + } }
return -ENODEV;

On Mon, Jul 30, 2018 at 07:19:26PM +0300, Sam Protsenko wrote:
"Failed" error message from env_load() only clutters the log with unnecessary details, as we already have all needed warnings by that time. Example:
Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 ** Failed (-5)
Let's only print it in case when DEBUG is defined to keep log clear.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Applied to u-boot/master, thanks!

Underlying API should already print some meaningful error message, so this one is just brings more noise. E.g. we can see log like this:
MMC: no card present ** Bad device mmc 0 **
Obviously, second error message is unwanted. Let's only print it in case when DEBUG is defined to keep log short and clear.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- Changes in v2: - Instead of removing error message, print it with debug() Changes in v3: - None
disk/part.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/disk/part.c b/disk/part.c index 9266a09ec3..9e457a6e72 100644 --- a/disk/part.c +++ b/disk/part.c @@ -400,7 +400,7 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str,
*dev_desc = get_dev_hwpart(ifname, dev, hwpart); if (!(*dev_desc) || ((*dev_desc)->type == DEV_TYPE_UNKNOWN)) { - printf("** Bad device %s %s **\n", ifname, dev_hwpart_str); + debug("** Bad device %s %s **\n", ifname, dev_hwpart_str); dev = -ENOENT; goto cleanup; }

On Mon, Jul 30, 2018 at 07:19:27PM +0300, Sam Protsenko wrote:
Underlying API should already print some meaningful error message, so this one is just brings more noise. E.g. we can see log like this:
MMC: no card present ** Bad device mmc 0 **
Obviously, second error message is unwanted. Let's only print it in case when DEBUG is defined to keep log short and clear.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Applied to u-boot/master, thanks!
participants (2)
-
Sam Protsenko
-
Tom Rini