diff mbox

[U-Boot] mmc: show error message when "mmc dev" command fails

Message ID 1455108396-31937-1-git-send-email-yamada.masahiro@socionext.com
State Deferred
Delegated to: Tom Rini
Headers show

Commit Message

Masahiro Yamada Feb. 10, 2016, 12:46 p.m. UTC
Currently, "mmc dev" can silently fail.  In case of failure, tell
the user about it.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 cmd/mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Stefan Roese Feb. 10, 2016, 12:49 p.m. UTC | #1
On 10.02.2016 13:46, Masahiro Yamada wrote:
> Currently, "mmc dev" can silently fail.  In case of failure, tell
> the user about it.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>   cmd/mmc.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 1c7156f..87a9fb6 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -463,8 +463,10 @@ static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag,
>   	}
>
>   	mmc = init_mmc_device(dev, true);
> -	if (!mmc)
> +	if (!mmc) {
> +		printf("could not switch to mmc%d\n", dev);
>   		return CMD_RET_FAILURE;
> +	}
>
>   	ret = mmc_select_hwpart(dev, part);
>   	printf("switch to partitions #%d, %s\n",
>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan
Stephen Warren Feb. 10, 2016, 4:27 p.m. UTC | #2
On 02/10/2016 05:46 AM, Masahiro Yamada wrote:
> Currently, "mmc dev" can silently fail.  In case of failure, tell
> the user about it.

Does it make sense to print this error inside init_mmc_device() instead, 
so that we don't have to add printf()s to all call sites?

I'm not sure the answer to my question is yes; just something to ponder.

Also note that one disadvantage of printing error messages all the time 
is that the distro boot scripts (by default) attempt to probe all 
defined storage devices, so printing errors will make that noisy. 
Perhaps that should be solved by adding a -q option (for "quiet") so 
those scripts can be non-noisy?
Masahiro Yamada Feb. 12, 2016, 2:55 p.m. UTC | #3
2016-02-11 1:27 GMT+09:00 Stephen Warren <swarren@wwwdotorg.org>:
> On 02/10/2016 05:46 AM, Masahiro Yamada wrote:
>>
>> Currently, "mmc dev" can silently fail.  In case of failure, tell
>> the user about it.
>
>
> Does it make sense to print this error inside init_mmc_device() instead, so
> that we don't have to add printf()s to all call sites?
>
> I'm not sure the answer to my question is yes; just something to ponder.

I guess it makes sense.
(I have not taken a close look, though)


> Also note that one disadvantage of printing error messages all the time is
> that the distro boot scripts (by default) attempt to probe all defined
> storage devices, so printing errors will make that noisy. Perhaps that
> should be solved by adding a -q option (for "quiet") so those scripts can be
> non-noisy?

Sounds reasonable, too.
diff mbox

Patch

diff --git a/cmd/mmc.c b/cmd/mmc.c
index 1c7156f..87a9fb6 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -463,8 +463,10 @@  static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag,
 	}
 
 	mmc = init_mmc_device(dev, true);
-	if (!mmc)
+	if (!mmc) {
+		printf("could not switch to mmc%d\n", dev);
 		return CMD_RET_FAILURE;
+	}
 
 	ret = mmc_select_hwpart(dev, part);
 	printf("switch to partitions #%d, %s\n",