* [PATCH] Recognize CSD structure version 3
@ 2010-05-31 6:40 Kyungmin Park
2010-05-31 7:06 ` Ben Dooks
2010-06-01 9:51 ` Adrian Hunter
0 siblings, 2 replies; 6+ messages in thread
From: Kyungmin Park @ 2010-05-31 6:40 UTC (permalink / raw)
To: linux-mmc, akpm
The eMMC spec 4.4 and 4.3 + additional feature chips has CSD structure version 3
To probe these chip properly and make it simple.
it doesn't check CSD structure.
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 89f7a25..9e42bc6 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -118,15 +118,12 @@ static int mmc_decode_csd(struct mmc_card *card)
u32 *resp = card->raw_csd;
/*
- * We only understand CSD structure v1.1 and v1.2.
+ * We understand all CSD structure v1.1, v1.2 and v1.3.
* v1.2 has extra information in bits 15, 11 and 10.
*/
csd_struct = UNSTUFF_BITS(resp, 126, 2);
- if (csd_struct != 1 && csd_struct != 2) {
- printk(KERN_ERR "%s: unrecognised CSD structure version %d\n",
+ printk(KERN_DEBUG "%s: recognised CSD structure version %d\n",
mmc_hostname(card->host), csd_struct);
- return -EINVAL;
- }
csd->mmca_vsn = UNSTUFF_BITS(resp, 122, 4);
m = UNSTUFF_BITS(resp, 115, 4);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] Recognize CSD structure version 3
2010-05-31 6:40 [PATCH] Recognize CSD structure version 3 Kyungmin Park
@ 2010-05-31 7:06 ` Ben Dooks
2010-05-31 7:35 ` Kyungmin Park
2010-06-01 9:51 ` Adrian Hunter
1 sibling, 1 reply; 6+ messages in thread
From: Ben Dooks @ 2010-05-31 7:06 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mmc, akpm
On Mon, May 31, 2010 at 03:40:35PM +0900, Kyungmin Park wrote:
> The eMMC spec 4.4 and 4.3 + additional feature chips has CSD structure version 3
> To probe these chip properly and make it simple.
> it doesn't check CSD structure.
>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 89f7a25..9e42bc6 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -118,15 +118,12 @@ static int mmc_decode_csd(struct mmc_card *card)
> u32 *resp = card->raw_csd;
>
> /*
> - * We only understand CSD structure v1.1 and v1.2.
> + * We understand all CSD structure v1.1, v1.2 and v1.3.
> * v1.2 has extra information in bits 15, 11 and 10.
> */
> csd_struct = UNSTUFF_BITS(resp, 126, 2);
> - if (csd_struct != 1 && csd_struct != 2) {
Hmm, it isn't easy to see what was going on here, this version field is 2bits?
I'd still check for 0 in case someone repurposes the 0 bit to mean some
future version we don't know.
> - printk(KERN_ERR "%s: unrecognised CSD structure version %d\n",
> + printk(KERN_DEBUG "%s: recognised CSD structure version %d\n",
> mmc_hostname(card->host), csd_struct);
> - return -EINVAL;
> - }
>
> csd->mmca_vsn = UNSTUFF_BITS(resp, 122, 4);
> m = UNSTUFF_BITS(resp, 115, 4);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Recognize CSD structure version 3
2010-05-31 7:06 ` Ben Dooks
@ 2010-05-31 7:35 ` Kyungmin Park
0 siblings, 0 replies; 6+ messages in thread
From: Kyungmin Park @ 2010-05-31 7:35 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-mmc, akpm
On Mon, May 31, 2010 at 4:06 PM, Ben Dooks <ben-linux@fluff.org> wrote:
> On Mon, May 31, 2010 at 03:40:35PM +0900, Kyungmin Park wrote:
>> The eMMC spec 4.4 and 4.3 + additional feature chips has CSD structure version 3
>> To probe these chip properly and make it simple.
>> it doesn't check CSD structure.
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 89f7a25..9e42bc6 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -118,15 +118,12 @@ static int mmc_decode_csd(struct mmc_card *card)
>> u32 *resp = card->raw_csd;
>>
>> /*
>> - * We only understand CSD structure v1.1 and v1.2.
>> + * We understand all CSD structure v1.1, v1.2 and v1.3.
>> * v1.2 has extra information in bits 15, 11 and 10.
>> */
>> csd_struct = UNSTUFF_BITS(resp, 126, 2);
>> - if (csd_struct != 1 && csd_struct != 2) {
>
> Hmm, it isn't easy to see what was going on here, this version field is 2bits?
>
> I'd still check for 0 in case someone repurposes the 0 bit to mean some
> future version we don't know.
I also consider it. but even though we don't check the 0 at here.
later time it can't pass the EXT_CSD version check. so it will return
error.
That's reason I remain it printk at here. and even though '0' csd
version has some meaning at future. no need to modify for this.
I want to wait others comments.
Thank you,
Kyungmin Park
>
>> - printk(KERN_ERR "%s: unrecognised CSD structure version %d\n",
>> + printk(KERN_DEBUG "%s: recognised CSD structure version %d\n",
>> mmc_hostname(card->host), csd_struct);
>> - return -EINVAL;
>> - }
>>
>> csd->mmca_vsn = UNSTUFF_BITS(resp, 122, 4);
>> m = UNSTUFF_BITS(resp, 115, 4);
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> --
> Ben
>
> Q: What's a light-year?
> A: One-third less calories than a regular year.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Recognize CSD structure version 3
2010-05-31 6:40 [PATCH] Recognize CSD structure version 3 Kyungmin Park
2010-05-31 7:06 ` Ben Dooks
@ 2010-06-01 9:51 ` Adrian Hunter
2010-06-02 8:12 ` Kyungmin Park
1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2010-06-01 9:51 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org
Kyungmin Park wrote:
> The eMMC spec 4.4 and 4.3 + additional feature chips has CSD structure version 3
That is not exactly correct. There is no CSD structure version 1.3. Instead
there are two fields for CSD structure version. One in CSD and one in Extended CSD.
In CSD there is:
CSD_STRUCTURE [127:126]
Describes the version of the CSD structure.
Table 44 — CSD register structure
CSD_STRUCTURE CSD Structure Version Valid for System Specification Version
0 CSD version No. 1.0 Allocated by MMCA
1 CSD version No. 1.1 Allocated by MMCA
2 CSD version No. 1.2 Version 4.1–4.2–4.3
3 Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register
So the 3 does not mean version 1.3 it just means look at Extended CSD instead.
But the Extended CSD field does not define CSD structure version 3 at all.
CSD_STRUCTURE [194]
This field is a continuation of the CSD_STRUCTURE field in the CSD register
Table 78 — CSD register structure
CSD_STRUCTURE CSD structure version Valid for System Specification Version
0 CSD version No. 1.0 Allocated by MMCA
1 CSD version No. 1.1 Allocated by MMCA
2 CSD version No. 1.2 Version 4.1–4.2–4.3-4.4
3–255 Reserved for future use
> To probe these chip properly and make it simple.
> it doesn't check CSD structure.
>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 89f7a25..9e42bc6 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -118,15 +118,12 @@ static int mmc_decode_csd(struct mmc_card *card)
> u32 *resp = card->raw_csd;
>
> /*
> - * We only understand CSD structure v1.1 and v1.2.
> + * We understand all CSD structure v1.1, v1.2 and v1.3.
This comment is not correct. v1.3 does not exist.
> * v1.2 has extra information in bits 15, 11 and 10.
> */
> csd_struct = UNSTUFF_BITS(resp, 126, 2);
> - if (csd_struct != 1 && csd_struct != 2) {
> - printk(KERN_ERR "%s: unrecognised CSD structure version %d\n",
I don't agree with removing this validation without a reason. Instead
extend it to 3.
To support the '3' value properly, the Extended CSD CSD_STRUCTURE
field needs to be validated also i.e. in mmc_read_ext_csd() do
something like:
u32 *resp = card->raw_csd;
if (UNSTUFF_BITS(resp, 126, 2) == 3) {
check ext_csd[194]
}
> + printk(KERN_DEBUG "%s: recognised CSD structure version %d\n",
> mmc_hostname(card->host), csd_struct);
> - return -EINVAL;
> - }
>
> csd->mmca_vsn = UNSTUFF_BITS(resp, 122, 4);
> m = UNSTUFF_BITS(resp, 115, 4);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Recognize CSD structure version 3
2010-06-01 9:51 ` Adrian Hunter
@ 2010-06-02 8:12 ` Kyungmin Park
2010-06-02 19:31 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Kyungmin Park @ 2010-06-02 8:12 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org
Hi
On Tue, Jun 1, 2010 at 6:51 PM, Adrian Hunter <adrian.hunter@nokia.com> wrote:
> Kyungmin Park wrote:
>>
>> The eMMC spec 4.4 and 4.3 + additional feature chips has CSD structure
>> version 3
>
> That is not exactly correct. There is no CSD structure version 1.3.
> Instead
> there are two fields for CSD structure version. One in CSD and one in
> Extended CSD.
>
> In CSD there is:
>
> CSD_STRUCTURE [127:126]
> Describes the version of the CSD structure.
> Table 44 — CSD register structure
> CSD_STRUCTURE CSD Structure Version Valid for System Specification
> Version
> 0 CSD version No. 1.0 Allocated by MMCA
> 1 CSD version No. 1.1 Allocated by MMCA
> 2 CSD version No. 1.2 Version 4.1–4.2–4.3
> 3 Version is coded in the CSD_STRUCTURE byte in the EXT_CSD
> register
>
> So the 3 does not mean version 1.3 it just means look at Extended CSD
> instead.
> But the Extended CSD field does not define CSD structure version 3 at all.
>
> CSD_STRUCTURE [194]
> This field is a continuation of the CSD_STRUCTURE field in the CSD register
> Table 78 — CSD register structure
> CSD_STRUCTURE CSD structure version Valid for System Specification
> Version
> 0 CSD version No. 1.0 Allocated by MMCA
> 1 CSD version No. 1.1 Allocated by MMCA
> 2 CSD version No. 1.2 Version 4.1–4.2–4.3-4.4
> 3–255 Reserved for future use
Thank you for Info. I got the eMMC v4.4 spec yesterday.
>
>> To probe these chip properly and make it simple.
>> it doesn't check CSD structure.
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 89f7a25..9e42bc6 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -118,15 +118,12 @@ static int mmc_decode_csd(struct mmc_card *card)
>> u32 *resp = card->raw_csd;
>> /*
>> - * We only understand CSD structure v1.1 and v1.2.
>> + * We understand all CSD structure v1.1, v1.2 and v1.3.
>
> This comment is not correct. v1.3 does not exist.
I will fix it correctly.
>
>> * v1.2 has extra information in bits 15, 11 and 10.
>> */
>> csd_struct = UNSTUFF_BITS(resp, 126, 2);
>> - if (csd_struct != 1 && csd_struct != 2) {
>> - printk(KERN_ERR "%s: unrecognised CSD structure version
>> %d\n",
>
> I don't agree with removing this validation without a reason. Instead
> extend it to 3.
With above description. it has all meaning. even though we don't use
the '0' value.
Do you want to check if csd_struct == 0 case then fail?
>
> To support the '3' value properly, the Extended CSD CSD_STRUCTURE
> field needs to be validated also i.e. in mmc_read_ext_csd() do
> something like:
>
>
> u32 *resp = card->raw_csd;
>
> if (UNSTUFF_BITS(resp, 126, 2) == 3) {
> check ext_csd[194]
> }
I wonder are there any use case of csd version? it's just check
routine for proper operation.
basically no problem to display csd version and ext_csd version.
Thank you,
Kyungmin Park
>
>
>
>
>> + printk(KERN_DEBUG "%s: recognised CSD structure version %d\n",
>> mmc_hostname(card->host), csd_struct);
>> - return -EINVAL;
>> - }
>> csd->mmca_vsn = UNSTUFF_BITS(resp, 122, 4);
>> m = UNSTUFF_BITS(resp, 115, 4);
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Recognize CSD structure version 3
2010-06-02 8:12 ` Kyungmin Park
@ 2010-06-02 19:31 ` Adrian Hunter
0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2010-06-02 19:31 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org
Kyungmin Park wrote:
> Hi
>
> On Tue, Jun 1, 2010 at 6:51 PM, Adrian Hunter <adrian.hunter@nokia.com> wrote:
>> Kyungmin Park wrote:
>>> The eMMC spec 4.4 and 4.3 + additional feature chips has CSD structure
>>> version 3
>> That is not exactly correct. There is no CSD structure version 1.3.
>> Instead
>> there are two fields for CSD structure version. One in CSD and one in
>> Extended CSD.
>>
>> In CSD there is:
>>
>> CSD_STRUCTURE [127:126]
>> Describes the version of the CSD structure.
>> Table 44 — CSD register structure
>> CSD_STRUCTURE CSD Structure Version Valid for System Specification
>> Version
>> 0 CSD version No. 1.0 Allocated by MMCA
>> 1 CSD version No. 1.1 Allocated by MMCA
>> 2 CSD version No. 1.2 Version 4.1–4.2–4.3
>> 3 Version is coded in the CSD_STRUCTURE byte in the EXT_CSD
>> register
>>
>> So the 3 does not mean version 1.3 it just means look at Extended CSD
>> instead.
>> But the Extended CSD field does not define CSD structure version 3 at all.
>>
>> CSD_STRUCTURE [194]
>> This field is a continuation of the CSD_STRUCTURE field in the CSD register
>> Table 78 — CSD register structure
>> CSD_STRUCTURE CSD structure version Valid for System Specification
>> Version
>> 0 CSD version No. 1.0 Allocated by MMCA
>> 1 CSD version No. 1.1 Allocated by MMCA
>> 2 CSD version No. 1.2 Version 4.1–4.2–4.3-4.4
>> 3–255 Reserved for future use
>
> Thank you for Info. I got the eMMC v4.4 spec yesterday.
>
>>> To probe these chip properly and make it simple.
>>> it doesn't check CSD structure.
>>>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index 89f7a25..9e42bc6 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -118,15 +118,12 @@ static int mmc_decode_csd(struct mmc_card *card)
>>> u32 *resp = card->raw_csd;
>>> /*
>>> - * We only understand CSD structure v1.1 and v1.2.
>>> + * We understand all CSD structure v1.1, v1.2 and v1.3.
>> This comment is not correct. v1.3 does not exist.
>
> I will fix it correctly.
>
>>> * v1.2 has extra information in bits 15, 11 and 10.
>>> */
>>> csd_struct = UNSTUFF_BITS(resp, 126, 2);
>>> - if (csd_struct != 1 && csd_struct != 2) {
>>> - printk(KERN_ERR "%s: unrecognised CSD structure version
>>> %d\n",
>> I don't agree with removing this validation without a reason. Instead
>> extend it to 3.
>
> With above description. it has all meaning. even though we don't use
> the '0' value.
> Do you want to check if csd_struct == 0 case then fail?
Yes because that is how is presently works. We should not change that
without a reason.
>
>> To support the '3' value properly, the Extended CSD CSD_STRUCTURE
>> field needs to be validated also i.e. in mmc_read_ext_csd() do
>> something like:
>>
>>
>> u32 *resp = card->raw_csd;
>>
>> if (UNSTUFF_BITS(resp, 126, 2) == 3) {
>> check ext_csd[194]
>> }
>
> I wonder are there any use case of csd version? it's just check
> routine for proper operation.
I presume it is being checked to prevent unsupported or broken
cards from causing unpredictable behaviour. I think you should
do the check I proposed above.
> basically no problem to display csd version and ext_csd version.
>
> Thank you,
> Kyungmin Park
>>
>>
>>
>>> + printk(KERN_DEBUG "%s: recognised CSD structure version %d\n",
>>> mmc_hostname(card->host), csd_struct);
>>> - return -EINVAL;
>>> - }
>>> csd->mmca_vsn = UNSTUFF_BITS(resp, 122, 4);
>>> m = UNSTUFF_BITS(resp, 115, 4);
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-06-02 19:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-31 6:40 [PATCH] Recognize CSD structure version 3 Kyungmin Park
2010-05-31 7:06 ` Ben Dooks
2010-05-31 7:35 ` Kyungmin Park
2010-06-01 9:51 ` Adrian Hunter
2010-06-02 8:12 ` Kyungmin Park
2010-06-02 19:31 ` Adrian Hunter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.