[U-Boot] [PATCH] mmc: support the revision check for eMMC4.5

[Jaehoon Chung]

Hi Andy.

On 05/08/2012 07:16 AM, Andy Fleming wrote:

> On Tue, Mar 27, 2012 at 1:55 AM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>> eMMC card is introduced the eMMC4.5.
>> But now eMMC card is checked up to eMMC4.0.
>> This patch is supported until eMMC4.5
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
>> ---
>>  common/cmd_mmc.c  |    5 ++++-
>>  drivers/mmc/mmc.c |   25 +++++++++++++++++++++++++
>>  include/mmc.h     |    8 ++++++++
>>  3 files changed, 37 insertions(+), 1 deletions(-)
>>
>> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
>> index 8f13c22..ff150ca 100644
>> --- a/common/cmd_mmc.c
>> +++ b/common/cmd_mmc.c
>> @@ -106,7 +106,10 @@ static void print_mmcinfo(struct mmc *mmc)
>>        printf("Rd Block Len: %d\n", mmc->read_bl_len);
>>
>>        printf("%s version %d.%d\n", IS_SD(mmc) ? "SD" : "MMC",
>> -                       (mmc->version >> 4) & 0xf, mmc->version & 0xf);
>> +                       (mmc->version >> 4) & 0xf,
>> +                       (mmc->version & 0xf) == EXT_CSD_REV_1_5 ?
>> +                       41 :((mmc->version & 0xf) > EXT_CSD_REV_1_5 ?
>> +                       (mmc->version & 0xf) - 1 : (mmc->version & 0xf)));
> 
> 
> This is very confusing. Let's pull some of that math and control code
> out of the printf invocation.

Ok..i will do it.

> 
> 
>>
>>        printf("High Capacity: %s\n", mmc->high_capacity ? "Yes" : "No");
>>        puts("Capacity: ");
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index 49c3349..e035012 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -651,6 +651,31 @@ int mmc_change_freq(struct mmc *mmc)
>>        if (err)
>>                return err;
>>
>> +       switch (ext_csd[EXT_CSD_REV]) {
>> +       case EXT_CSD_REV_1_0:
>> +               mmc->version |= EXT_CSD_REV_1_0;
>> +               break;
>> +       case EXT_CSD_REV_1_1:
>> +               mmc->version |= EXT_CSD_REV_1_1;
>> +               break;
>> +       case EXT_CSD_REV_1_2:
>> +               mmc->version |= EXT_CSD_REV_1_2;
>> +               break;
>> +       case EXT_CSD_REV_1_3:
>> +               mmc->version |= EXT_CSD_REV_1_3;
>> +               break;
>> +       case EXT_CSD_REV_1_5:
>> +               mmc->version |= EXT_CSD_REV_1_5;
>> +               break;
>> +       case EXT_CSD_REV_1_6:
>> +               mmc->version |= EXT_CSD_REV_1_6;
>> +               break;
>> +       case EXT_CSD_REV_1_4:
>> +       default:
>> +               printf("Unknown revision - %x\n", ext_csd[EXT_CSD_REV]);
>> +               return 0;
>> +       }
>> +
> 
> 
> This seems broken, and the wrong place to do this. the mmc->version
> field will already be set based on the CSD, to values that overlap
> with the ones you are ORing, here.

Yes..it's already set based on the CSD. but we can't check whether version4.1 or 4.2 or 4.3..
To check More exactly, we need to check CSD structure version in ext_csd register.
And also check EXT_CSD_REV[192] in ext_csd register.

> 
> Also, the case statement appears to serve no function, other than to
> paradoxically identify that version 1.4 is unknown.
> 
> I think that, if we want to do any version checking, we should do it
> in mmc_startup(). I suspect that the version information obtained here
> is only appropriate for certain versions of the spec, so that should
> be checked before adding in the information.

I will resend the patch.

Best Regards,
Jaehoon Chung

> 
> Andy
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>