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

[Andy Fleming]

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.


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

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.

Andy