mmc: Fix handling of bus widths and DDR card capabilities

If the MMC_MODE_DDR_52MHz flag is set in card capabilities bitmask,
it is never cleared, even if switching to DDR mode fails, and if
the controller driver uses this flag to check the DDR mode, it can
take incorrect actions.

Also, DDR related checks in mmc_startup() incorrectly handle the case
when the host controller does not support some bus widths (e.g. can't
support 8 bits), since the host_caps is checked for DDR bit, but not
bus width bits.

This fix clearly separates using of card_caps bitmask, having there
the flags for the capabilities, that the card can support, and actual
operation mode, described outside of card_caps (i.e. bus_width and
ddr_mode fields in mmc structure). Separate host controller drivers
may need to be updated to use the actual flags. Respectively,
the capabilities checks in mmc_startup are made more correct and clear.

Also, some clean up is made with errors handling and code syntax layout.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 4286e2696363cab44b44772251a4f52c488395a2..96478e45c14039cd88a1a59427d0ec7f1a44d07e 100644 (file)
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -90,7 +90,8 @@ static void print_mmcinfo(struct mmc *mmc)
        puts("Capacity: ");
        print_size(mmc->capacity, "\n");
 
-       printf("Bus Width: %d-bit\n", mmc->bus_width);
+       printf("Bus Width: %d-bit%s\n", mmc->bus_width,
+                       mmc->ddr_mode ? " DDR" : "");
 }
 static struct mmc *init_mmc_device(int dev, bool force_init)
 {

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 99f03c9101ff970a59c162ed6bbf89688532f97d..a290031d171c528e04a5ae7d2e1561ede5ec5450 100644 (file)
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -159,7 +159,7 @@ int mmc_set_blocklen(struct mmc *mmc, int len)
 {
        struct mmc_cmd cmd;
 
-       if (mmc->card_caps & MMC_MODE_DDR_52MHz)
+       if (mmc->ddr_mode)
                return 0;
 
        cmd.cmdidx = MMC_CMD_SET_BLOCKLEN;
@@ -486,7 +486,7 @@ static int mmc_change_freq(struct mmc *mmc)
        char cardtype;
        int err;
 
-       mmc->card_caps = 0;
+       mmc->card_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
 
        if (mmc_host_is_spi(mmc))
                return 0;
@@ -1121,8 +1121,10 @@ static int mmc_startup(struct mmc *mmc)
 
                /* An array to map CSD bus widths to host cap bits */
                static unsigned ext_to_hostcaps[] = {
-                       [EXT_CSD_DDR_BUS_WIDTH_4] = MMC_MODE_DDR_52MHz,
-                       [EXT_CSD_DDR_BUS_WIDTH_8] = MMC_MODE_DDR_52MHz,
+                       [EXT_CSD_DDR_BUS_WIDTH_4] =
+                               MMC_MODE_DDR_52MHz | MMC_MODE_4BIT,
+                       [EXT_CSD_DDR_BUS_WIDTH_8] =
+                               MMC_MODE_DDR_52MHz | MMC_MODE_8BIT,
                        [EXT_CSD_BUS_WIDTH_4] = MMC_MODE_4BIT,
                        [EXT_CSD_BUS_WIDTH_8] = MMC_MODE_8BIT,
                };
@@ -1134,13 +1136,13 @@ static int mmc_startup(struct mmc *mmc)
 
                for (idx=0; idx < ARRAY_SIZE(ext_csd_bits); idx++) {
                        unsigned int extw = ext_csd_bits[idx];
+                       unsigned int caps = ext_to_hostcaps[extw];
 
                        /*
-                        * Check to make sure the controller supports
-                        * this bus width, if it's more than 1
+                        * Check to make sure the card and controller support
+                        * these capabilities
                         */
-                       if (extw != EXT_CSD_BUS_WIDTH_1 &&
-                                       !(mmc->cfg->host_caps & ext_to_hostcaps[extw]))
+                       if ((mmc->card_caps & caps) != caps)
                                continue;
 
                        err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
@@ -1149,26 +1151,33 @@ static int mmc_startup(struct mmc *mmc)
                        if (err)
                                continue;
 
+                       mmc->ddr_mode = (caps & MMC_MODE_DDR_52MHz) ? 1 : 0;
                        mmc_set_bus_width(mmc, widths[idx]);
 
                        err = mmc_send_ext_csd(mmc, test_csd);
+
+                       if (err)
+                               continue;
+
                        /* Only compare read only fields */
-                       if (!err && ext_csd[EXT_CSD_PARTITIONING_SUPPORT] \
-                                   == test_csd[EXT_CSD_PARTITIONING_SUPPORT]
-                                && ext_csd[EXT_CSD_HC_WP_GRP_SIZE] \
-                                   == test_csd[EXT_CSD_HC_WP_GRP_SIZE] \
-                                && ext_csd[EXT_CSD_REV] \
-                                   == test_csd[EXT_CSD_REV]
-                                && ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] \
-                                   == test_csd[EXT_CSD_HC_ERASE_GRP_SIZE]
-                                && memcmp(&ext_csd[EXT_CSD_SEC_CNT], \
-                                       &test_csd[EXT_CSD_SEC_CNT], 4) == 0) {
-
-                               mmc->card_caps |= ext_to_hostcaps[extw];
+                       if (ext_csd[EXT_CSD_PARTITIONING_SUPPORT]
+                               == test_csd[EXT_CSD_PARTITIONING_SUPPORT] &&
+                           ext_csd[EXT_CSD_HC_WP_GRP_SIZE]
+                               == test_csd[EXT_CSD_HC_WP_GRP_SIZE] &&
+                           ext_csd[EXT_CSD_REV]
+                               == test_csd[EXT_CSD_REV] &&
+                           ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]
+                               == test_csd[EXT_CSD_HC_ERASE_GRP_SIZE] &&
+                           memcmp(&ext_csd[EXT_CSD_SEC_CNT],
+                                  &test_csd[EXT_CSD_SEC_CNT], 4) == 0)
                                break;
-                       }
+                       else
+                               err = SWITCH_ERR;
                }
 
+               if (err)
+                       return err;
+
                if (mmc->card_caps & MMC_MODE_HS) {
                        if (mmc->card_caps & MMC_MODE_HS_52MHz)
                                mmc->tran_speed = 52000000;
@@ -1324,6 +1333,7 @@ int mmc_start_init(struct mmc *mmc)
        if (err)
                return err;
 
+       mmc->ddr_mode = 0;
        mmc_set_bus_width(mmc, 1);
        mmc_set_clock(mmc, 1);
 

diff --git a/include/mmc.h b/include/mmc.h
index fcea195ea5673ce0e0bbe40e96f0706d44b48190..7ec255d882199c88dc7bf79f52857c87a796a46c 100644 (file)
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -318,6 +318,7 @@ struct mmc {
        char init_in_progress;  /* 1 if we have done mmc_start_init() */
        char preinit;           /* start init as early as possible */
        uint op_cond_response;  /* the response byte from the last op_cond */
+       int ddr_mode;
 };
 
 int mmc_register(struct mmc *mmc);