[U-Boot] [PATCH V2] cmd_mmc.c: check mmc_init() during mmc dev

[U-Boot] [PATCH V2] cmd_mmc.c: check mmc_init() during mmc dev

[Stephen Warren]

From: Bryan Wu <cooloney@gmail.com>

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 <pengw@nvidia.com>
[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 <swarren@nvidia.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
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(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 572aa895fa1c..2e564572f74d 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -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",
-- 
1.8.1.5

[U-Boot] [PATCH V2] cmd_mmc.c: check mmc_init() during mmc dev

[Jaehoon Chung]

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

On 05/21/2014 01:57 AM, Stephen Warren wrote:
> From: Bryan Wu <cooloney@gmail.com>
> 
> 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 <pengw@nvidia.com>
> [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 <swarren@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> 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(-)
> 
> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
> index 572aa895fa1c..2e564572f74d 100644
> --- a/common/cmd_mmc.c
> +++ b/common/cmd_mmc.c
> @@ -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",
> 

[U-Boot] [PATCH V2] cmd_mmc.c: check mmc_init() during mmc dev

[Stephen Warren]

On 05/20/2014 11:40 PM, Jaehoon Chung wrote:
> 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.

> 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:

> On 05/21/2014 01:57 AM, Stephen Warren wrote:
>> From: Bryan Wu <cooloney@gmail.com>
>>
>> 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.

[U-Boot] [PATCH V2] cmd_mmc.c: check mmc_init() during mmc dev

[Jaehoon Chung]

On 05/22/2014 01:18 AM, Stephen Warren wrote:
> On 05/20/2014 11:40 PM, Jaehoon Chung wrote:
>> 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.

> 
>> 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
> 
>> On 05/21/2014 01:57 AM, Stephen Warren wrote:
>>> From: Bryan Wu <cooloney@gmail.com>
>>>
>>> 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.
> 
> 
> 

[U-Boot] [PATCH V2] cmd_mmc.c: check mmc_init() during mmc dev

[Stephen Warren]

On 05/21/2014 07:41 PM, Jaehoon Chung wrote:
> On 05/22/2014 01:18 AM, Stephen Warren wrote:
>> On 05/20/2014 11:40 PM, Jaehoon Chung wrote:
>>> 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.

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.

>>> 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.

[U-Boot] [PATCH V2] cmd_mmc.c: check mmc_init() during mmc dev

[Stephen Warren]

On 05/20/2014 10:57 AM, Stephen Warren wrote:
> From: Bryan Wu <cooloney@gmail.com>
> 
> 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)

[U-Boot] [PATCH V2] cmd_mmc.c: check mmc_init() during mmc dev

[Pantelis Antoniou]

Hi Stephen,

On May 23, 2014, at 9:59 PM, Stephen Warren wrote:

> On 05/20/2014 10:57 AM, Stephen Warren wrote:
>> From: Bryan Wu <cooloney@gmail.com>
>> 
>> 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)

Similar functionality is in master already. Dropping the patch.

-- Pantelis