Re: [PATCH] Recognize CSD structure version 3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kyungmin Park wrote:
Hi

On Tue, Jun 1, 2010 at 6:51 PM, Adrian Hunter <adrian.hunter@xxxxxxxxx> 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@xxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux