Patchwork [U-Boot,4/7] mmc: rescan fails on empty slot

login
register
mail settings
Submitter Michael Jones
Date July 15, 2011, 9:09 a.m.
Message ID <1310720986-5474-5-git-send-email-michael.jones@matrix-vision.de>
Download mbox | patch
Permalink /patch/104769/
State Accepted
Commit 8fd01b8f6b5b0d8bceec443f0a62b52afa7452a1
Delegated to: Andy Fleming
Headers show

Comments

Michael Jones - July 15, 2011, 9:09 a.m.
Fail in 'mmc rescan' if mmc_init() returns error

Signed-off-by: Michael Jones <michael.jones@matrix-vision.de>
---
 common/cmd_mmc.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Jaehoon Chung - July 19, 2011, 2:06 a.m.
Hi Michael.

I have some question. there are some mmc_init().
But you are only checked there..
Did you have any special reason?

Regards,
Jaehoon Chung

Michael Jones wrote:
> Fail in 'mmc rescan' if mmc_init() returns error
> 
> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de>
> ---
>  common/cmd_mmc.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
> index 176646d..28918f6 100644
> --- a/common/cmd_mmc.c
> +++ b/common/cmd_mmc.c
> @@ -165,9 +165,11 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  		}
>  
>  		mmc->has_init = 0;
> -		mmc_init(mmc);
>  
> -		return 0;
> +		if (mmc_init(mmc))
> +			return 1;
> +		else
> +			return 0;
>  	} else if (strncmp(argv[1], "part", 4) == 0) {
>  		block_dev_desc_t *mmc_dev;
>  		struct mmc *mmc = find_mmc_device(curr_device);
Michael Jones - July 20, 2011, 8:51 a.m.
Hi Jaehoon Chung,

On 07/19/2011 04:06 AM, Jaehoon Chung wrote:
> 
> Hi Michael.
> 
> I have some question. there are some mmc_init().
> But you are only checked there..
> Did you have any special reason?
> 
> Regards,
> Jaehoon Chung

The purpose of my patch was to enable an if/else to detect whether there
was a card in the slot. Among the available mmc commands, I thought 'mmc
rescan' was the most logical to be able to use to detect whether a slot
was empty or not.

Below is a survey of other places where a similar check could be done. I
think that if a similar check is to be made in these cases, they could
be in separate patch(es) from this one.

- 'mmc read/write': These commands should fail anyway if the slot were
empty.
- 'mmc part' prints "## Unknown partition table" if the slot is empty,
although the command itself still returns successfully. I suppose the
check could make sense here.
- 'mmc dev': I thought there was perhaps a use case where it makes sense
to switch to a mmc device which is currently empty. If that is not true,
it would probably make sense to do the same check there.
- 'mmcinfo': Currently this basically prints '0' for all the fields when
the slot is empty. It probably would be more logical to print "device
init failed" or similar instead.

-Michael

> 
> Michael Jones wrote:
>> Fail in 'mmc rescan' if mmc_init() returns error
>>
>> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de>
>> ---
>>  common/cmd_mmc.c |    6 ++++--
>>  1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
>> index 176646d..28918f6 100644
>> --- a/common/cmd_mmc.c
>> +++ b/common/cmd_mmc.c
>> @@ -165,9 +165,11 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>  		}
>>  
>>  		mmc->has_init = 0;
>> -		mmc_init(mmc);
>>  
>> -		return 0;
>> +		if (mmc_init(mmc))
>> +			return 1;
>> +		else
>> +			return 0;
>>  	} else if (strncmp(argv[1], "part", 4) == 0) {
>>  		block_dev_desc_t *mmc_dev;
>>  		struct mmc *mmc = find_mmc_device(curr_device);
> 


MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
Jaehoon Chung - July 20, 2011, 9:52 a.m.
Hi Michael

Thanks for your explanation. :)

Regards,
Jaehoon Chung

Michael Jones wrote:
> Hi Jaehoon Chung,
> 
> On 07/19/2011 04:06 AM, Jaehoon Chung wrote:
>> Hi Michael.
>>
>> I have some question. there are some mmc_init().
>> But you are only checked there..
>> Did you have any special reason?
>>
>> Regards,
>> Jaehoon Chung
> 
> The purpose of my patch was to enable an if/else to detect whether there
> was a card in the slot. Among the available mmc commands, I thought 'mmc
> rescan' was the most logical to be able to use to detect whether a slot
> was empty or not.
> 
> Below is a survey of other places where a similar check could be done. I
> think that if a similar check is to be made in these cases, they could
> be in separate patch(es) from this one.
> 
> - 'mmc read/write': These commands should fail anyway if the slot were
> empty.
> - 'mmc part' prints "## Unknown partition table" if the slot is empty,
> although the command itself still returns successfully. I suppose the
> check could make sense here.
> - 'mmc dev': I thought there was perhaps a use case where it makes sense
> to switch to a mmc device which is currently empty. If that is not true,
> it would probably make sense to do the same check there.
> - 'mmcinfo': Currently this basically prints '0' for all the fields when
> the slot is empty. It probably would be more logical to print "device
> init failed" or similar instead.
> 
> -Michael
> 
>> Michael Jones wrote:
>>> Fail in 'mmc rescan' if mmc_init() returns error
>>>
>>> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de>
>>> ---
>>>  common/cmd_mmc.c |    6 ++++--
>>>  1 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
>>> index 176646d..28918f6 100644
>>> --- a/common/cmd_mmc.c
>>> +++ b/common/cmd_mmc.c
>>> @@ -165,9 +165,11 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>>  		}
>>>  
>>>  		mmc->has_init = 0;
>>> -		mmc_init(mmc);
>>>  
>>> -		return 0;
>>> +		if (mmc_init(mmc))
>>> +			return 1;
>>> +		else
>>> +			return 0;
>>>  	} else if (strncmp(argv[1], "part", 4) == 0) {
>>>  		block_dev_desc_t *mmc_dev;
>>>  		struct mmc *mmc = find_mmc_device(curr_device);
> 
> 
> MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
> Registergericht: Amtsgericht Stuttgart, HRB 271090
> Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

Patch

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 176646d..28918f6 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -165,9 +165,11 @@  int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		}
 
 		mmc->has_init = 0;
-		mmc_init(mmc);
 
-		return 0;
+		if (mmc_init(mmc))
+			return 1;
+		else
+			return 0;
 	} else if (strncmp(argv[1], "part", 4) == 0) {
 		block_dev_desc_t *mmc_dev;
 		struct mmc *mmc = find_mmc_device(curr_device);