cmd_mmc.c: check mmc_init() during mmc dev

PATCH Superseded Stephen Warren 3 participants 7 messages May 20, 2014 – Jun 12, 2014

lore

Share & tools

lore

Stephen Warren

author

2014-05-20 16:57 UTC #0 lore raw

PATCH v2 1/1 cmd_mmc.c: check mmc_init() during mmc dev

From: Bryan Wu <[email protected]>

mmc dev ${devnum} will return 0 as success even if there is no card inserted. Booting script like tegra-common-post.h will call mmc dev ${devnum} to check the mmc device status, it always return 0 even if mmc_init() fails.

Check mmc_init() return value let mmc dev command return failure when mmc_init() fails.

Signed-off-by Bryan Wu <[email protected]>

[swarren: Always return 1 on error; hush doesn't appear to support arbitrary return values. Fixed compilation error due to multiple declaration of ret variable.]

Tested-by Stephen Warren <[email protected]>

Signed-off-by Stephen Warren <[email protected]>

---
v2:
* Always return 1 on error; hush doesn't appear to support arbitrary return values.
* Fixed compilation error due to multiple declaration of ret variable.

---
 common/cmd_mmc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

common/cmd_mmc.c +4 -1

index 572aa895fa1c..2e564572f74d 100644
@@ -217,7 +217,10 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			return 1;
 		}
 
-		mmc_init(mmc);
+		ret = mmc_init(mmc);
+		if (ret)
+			return 1;
+
 		if (part != -1) {
 			ret = mmc_select_hwpart(dev, part);
 			printf("switch to partitions #%d, %s\n",

Signature · 1 line

1.8.1.5

Jaehoon Chung

2014-05-21 05:40 UTC #1 lore raw

PATCH v2 1/1 cmd_mmc.c: check mmc_init() during mmc dev

Hi, Stephen.

i didn't apply your patch. Which repository do you use? Well, if you want to check, can be used "if (mmc_init(mmc))".

And i'm not sure whether this code is really need or not.

Best Regards,
Jaehoon Chung

Quoted 42 lines · Stephen Warren

From: Bryan Wu <[email protected]>
mmc dev ${devnum} will return 0 as success even if there is no card inserted. Booting script like tegra-common-post.h will call mmc dev ${devnum} to check the mmc device status, it always return 0 even if mmc_init() fails.
Check mmc_init() return value let mmc dev command return failure when mmc_init() fails.
Signed-off-by Bryan Wu <[email protected]>
[swarren: Always return 1 on error; hush doesn't appear to support arbitrary return values. Fixed compilation error due to multiple declaration of ret variable.]
Tested-by Stephen Warren <[email protected]>
Signed-off-by Stephen Warren <[email protected]>
---
v2:
* Always return 1 on error; hush doesn't appear to support arbitrary return values.
* Fixed compilation error due to multiple declaration of ret variable.
---
 common/cmd_mmc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
common/cmd_mmc.c +4 -1
index 572aa895fa1c..2e564572f74d 100644
@@ -217,7 +217,10 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			return 1;
 		}
 
-		mmc_init(mmc);
+		ret = mmc_init(mmc);
+		if (ret)
+			return 1;
+
 		if (part != -1) {
 			ret = mmc_select_hwpart(dev, part);
 			printf("switch to partitions #%d, %s\n",

Stephen Warren

author

2014-05-21 16:18 UTC #2 lore raw

PATCH v2 1/1 cmd_mmc.c: check mmc_init() during mmc dev

Quoted 3 lines · Jaehoon Chung

Hi, Stephen.
i didn't apply your patch. Which repository do you use?

It's based on u-boot.git master branch. The latest u-boot-mmc.git master branch is already included in that branch, and it looks like some changes have been applied to cmd_mmc.c in u-boot/master that aren't in u-boot-mmc/master.

Quoted 3 lines

Well, if you want to check, can be used "if (mmc_init(mmc))".
And i'm not sure whether this code is really need or not.

Why not? This code is required to solve the problem described in the commit description:

Quoted 7 lines · Stephen Warren

Quoted 6 lines
From: Bryan Wu <[email protected]>
mmc dev ${devnum} will return 0 as success even if there is no card inserted. Booting script like tegra-common-post.h will call mmc dev ${devnum} to check the mmc device status, it always return 0 even if mmc_init() fails.

Jaehoon Chung

2014-05-22 01:41 UTC #3 lore raw

PATCH v2 1/1 cmd_mmc.c: check mmc_init() during mmc dev

Quoted 9 lines · Stephen Warren

Quoted 3 lines · Jaehoon Chung
Hi, Stephen.
i didn't apply your patch. Which repository do you use?
It's based on u-boot.git master branch. The latest u-boot-mmc.git master branch is already included in that branch, and it looks like some changes have been applied to cmd_mmc.c in u-boot/master that aren't in u-boot-mmc/master.

I have pulled the latest u-boot.git, but it didn't apply this patch. If i missed something, let me know plz.

Quoted 7 lines

Quoted 4 lines
Well, if you want to check, can be used "if (mmc_init(mmc))".
And i'm not sure whether this code is really need or not.
Why not? This code is required to solve the problem described in the commit description:

I will try to reproduce the problem described in the commit-msg. Because, i didn't reproduce it, so i'm not sure.
But to control the return value, it's reasonable, right?

Best Regards,
Jaehoon Chung

Quoted 11 lines

Quoted 8 lines · Stephen Warren
Quoted 6 lines
From: Bryan Wu <[email protected]>
mmc dev ${devnum} will return 0 as success even if there is no card inserted. Booting script like tegra-common-post.h will call mmc dev ${devnum} to check the mmc device status, it always return 0 even if mmc_init() fails.

Stephen Warren

author

2014-05-22 16:11 UTC #4 lore raw

PATCH v2 1/1 cmd_mmc.c: check mmc_init() during mmc dev

Quoted 6 lines · Jaehoon Chung

Quoted 5 lines · Stephen Warren
Quoted 3 lines · Jaehoon Chung
Hi, Stephen.
i didn't apply your patch. Which repository do you use?

Quoted 7 lines

Quoted 4 lines
It's based on u-boot.git master branch. The latest u-boot-mmc.git master branch is already included in that branch, and it looks like some changes have been applied to cmd_mmc.c in u-boot/master that aren't in u-boot-mmc/master.
I have pulled the latest u-boot.git, but it didn't apply this patch. If i missed something, let me know plz.

Ah, I guess I hadn't noticed there's an interaction (context changes) with some other MMC-related patches that I sent:

http://patchwork.ozlabs.org/patch/346771/
[U-Boot,1/4] cmd_part: fix type in part command help text

http://patchwork.ozlabs.org/patch/346770/
[U-Boot,2/4] disk: support devices with HW partitions

http://patchwork.ozlabs.org/patch/346768/
[U-Boot,3/4] mmc: provide a select_hwpart implementation for get_device()

http://patchwork.ozlabs.org/patch/346769/
[U-Boot,4/4] cmd_mmc: use new mmc_select_hwpart() function

So, you can either apply those first, or use "git am -3" rather than "git am", plus declare "int ret"; patch 4/4 above does that.

Quoted 11 lines

Quoted 6 lines
Quoted 3 lines
Well, if you want to check, can be used "if (mmc_init(mmc))".
And i'm not sure whether this code is really need or not.
Why not? This code is required to solve the problem described in the commit description:
I will try to reproduce the problem described in the commit-msg. Because, i didn't reproduce it, so i'm not sure.
But to control the return value, it's reasonable, right?

Yes, I think so.

Stephen Warren

author

2014-05-23 18:59 UTC #5 lore raw

PATCH v2 1/1 cmd_mmc.c: check mmc_init() during mmc dev

In reply to Stephen Warren

Quoted 9 lines · Stephen Warren

From: Bryan Wu <[email protected]>
mmc dev ${devnum} will return 0 as success even if there is no card inserted. Booting script like tegra-common-post.h will call mmc dev ${devnum} to check the mmc device status, it always return 0 even if mmc_init() fails.
Check mmc_init() return value let mmc dev command return failure when mmc_init() fails.

For the email archive record, it looks like this same logic has already been implemented as part of:

1fd93c6e7d8a eMMC: cmd_mmc.c adds the 'rpmb' sub-command for the 'mmc' command

... which was just recently committed into u-boot-mmc/master.

(See the code in init_mmc_device)

Pantelis Antoniou

2014-06-12 11:13 UTC #6 lore raw

PATCH v2 1/1 cmd_mmc.c: check mmc_init() during mmc dev

Hi Stephen,

Quoted 14 lines · Stephen Warren

Quoted 9 lines · Stephen Warren
From: Bryan Wu <[email protected]>
mmc dev ${devnum} will return 0 as success even if there is no card inserted. Booting script like tegra-common-post.h will call mmc dev ${devnum} to check the mmc device status, it always return 0 even if mmc_init() fails.
Check mmc_init() return value let mmc dev command return failure when mmc_init() fails.
For the email archive record, it looks like this same logic has already been implemented as part of:

Quoted 6 lines

1fd93c6e7d8a eMMC: cmd_mmc.c adds the 'rpmb' sub-command for the 'mmc' command
... which was just recently committed into u-boot-mmc/master.
(See the code in init_mmc_device)

Similar functionality is in master already. Dropping the patch.

-- Pantelis