| Message ID | 2FB34057-4156-4BCA-A896-733203F90DA3@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chris, Just tested the mod on brownstone. Works. Philip On May 24, 2011, at 6:14 PM, Philip Rakity wrote: > > resend > > Note: I cannot test test for a few days. Following Chris > suggestion posting to list. > > version 2 > --- > return -EINVAL when compare of ext_csd_fails > combine two compare routines into one per suggestion > from Chris. > > --- > version 1 > > CMD19 -- The offical way to validate bus widths from the > JEDEC spec does not work on all platforms. Some platforms > that use PCI/PCIe to connect their SD controllers are known > to fail. > > If the quirk MMC_BUS_WIDTH_TEST is not defined we try > to figure out the bus width by reading the ext_csd at different > bus widths and compare this against the ext_csd read in 1 bit > mode. If no ext_csd is available we default to 1 bit operations. > > Code has been tested on mmp2 against 8 bit eMMC and Transcend 2GB > card that is known to not work in 4 bit mode. The physical > pins on the card are not present to support 4 bit operation. > > Signed-off-by: Philip Rakity <prakity@marvell.com> > --- > drivers/mmc/core/mmc.c | 113 +++++++++++++++++++++++++++++++++++++++++++---- > 1 files changed, 103 insertions(+), 10 deletions(-) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index baab027..6db91d6 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card) > } > > /* > - * Read and decode extended CSD. > + * Read extended CSD. > */ > -static int mmc_read_ext_csd(struct mmc_card *card) > +static int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd) > { > int err; > u8 *ext_csd; > > BUG_ON(!card); > + BUG_ON(!new_ext_csd); > + > + *new_ext_csd = NULL; > > if (card->csd.mmca_vsn < CSD_SPEC_VER_4) > return 0; > @@ -199,12 +202,15 @@ static int mmc_read_ext_csd(struct mmc_card *card) > > err = mmc_send_ext_csd(card, ext_csd); > if (err) { > + kfree(ext_csd); > + *new_ext_csd = NULL; > + > /* If the host or the card can't do the switch, > * fail more gracefully. */ > if ((err != -EINVAL) > && (err != -ENOSYS) > && (err != -EFAULT)) > - goto out; > + return err; > > /* > * High capacity cards should have this "magic" size > @@ -222,9 +228,23 @@ static int mmc_read_ext_csd(struct mmc_card *card) > mmc_hostname(card->host)); > err = 0; > } > + } else > + *new_ext_csd = ext_csd; > > - goto out; > - } > + return err; > +} > + > +/* > + * Decode extended CSD. > + */ > +static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd) > +{ > + int err = 0; > + > + BUG_ON(!card); > + > + if (!ext_csd) > + return 0; > > /* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */ > if (card->csd.structure == 3) { > @@ -372,8 +392,70 @@ static int mmc_read_ext_csd(struct mmc_card *card) > card->erased_byte = 0x0; > > out: > + return err; > +} > + > +static inline void mmc_free_ext_csd(u8 *ext_csd) > +{ > kfree(ext_csd); > +} > + > > +static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd, > + unsigned bus_width) > +{ > + u8 *bw_ext_csd; > + int err; > + > + err = mmc_get_ext_csd(card, &bw_ext_csd); > + if (err) > + return err; > + > + if ((ext_csd == NULL || bw_ext_csd == NULL)) { > + if (bus_width != MMC_BUS_WIDTH_1) > + err = -EINVAL; > + goto out; > + } > + > + if (bus_width == MMC_BUS_WIDTH_1) > + goto out; > + > + /* only compare read only fields */ > + > + err = (!(ext_csd[EXT_CSD_PARTITION_SUPPORT] == > + bw_ext_csd[EXT_CSD_PARTITION_SUPPORT]) && > + (ext_csd[EXT_CSD_ERASED_MEM_CONT] == > + bw_ext_csd[EXT_CSD_ERASED_MEM_CONT]) && > + (ext_csd[EXT_CSD_REV] == > + bw_ext_csd[EXT_CSD_REV]) && > + (ext_csd[EXT_CSD_STRUCTURE] == > + bw_ext_csd[EXT_CSD_STRUCTURE]) && > + (ext_csd[EXT_CSD_CARD_TYPE] == > + bw_ext_csd[EXT_CSD_CARD_TYPE]) && > + (ext_csd[EXT_CSD_S_A_TIMEOUT] == > + bw_ext_csd[EXT_CSD_S_A_TIMEOUT]) && > + (ext_csd[EXT_CSD_HC_WP_GRP_SIZE] == > + bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE]) && > + (ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] == > + bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT]) && > + (ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] == > + bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]) && > + (ext_csd[EXT_CSD_SEC_TRIM_MULT] == > + bw_ext_csd[EXT_CSD_SEC_TRIM_MULT]) && > + (ext_csd[EXT_CSD_SEC_ERASE_MULT] == > + bw_ext_csd[EXT_CSD_SEC_ERASE_MULT]) && > + (ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] == > + bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT]) && > + (ext_csd[EXT_CSD_TRIM_MULT] == > + bw_ext_csd[EXT_CSD_TRIM_MULT]) && > + memcmp(&ext_csd[EXT_CSD_SEC_CNT], > + &bw_ext_csd[EXT_CSD_SEC_CNT], > + 4) != 0); > + if (err) > + err = -EINVAL; > + > +out: > + mmc_free_ext_csd(bw_ext_csd); > return err; > } > > @@ -438,6 +520,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > u32 cid[4]; > unsigned int max_dtr; > u32 rocr; > + u8 *ext_csd = NULL; > > BUG_ON(!host); > WARN_ON(!host->claimed); > @@ -536,7 +619,11 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > /* > * Fetch and process extended CSD. > */ > - err = mmc_read_ext_csd(card); > + > + err = mmc_get_ext_csd(card, &ext_csd); > + if (err) > + goto free_card; > + err = mmc_read_ext_csd(card, ext_csd); > if (err) > goto free_card; > > @@ -676,14 +763,18 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > 0); > if (!err) { > mmc_set_bus_width(card->host, bus_width); > + > /* > * If controller can't handle bus width test, > - * use the highest bus width to maintain > - * compatibility with previous MMC behavior. > + * compare ext_csd previously read in 1 bit mode > + * against ext_csd at new bus width > */ > if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST)) > - break; > - err = mmc_bus_test(card, bus_width); > + err = mmc_compare_ext_csds(card, > + ext_csd, > + bus_width); > + else > + err = mmc_bus_test(card, bus_width); > if (!err) > break; > } > @@ -730,12 +821,14 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > if (!oldcard) > host->card = card; > > + mmc_free_ext_csd(ext_csd); > return 0; > > free_card: > if (!oldcard) > mmc_remove_card(card); > err: > + mmc_free_ext_csd(ext_csd); > > return err; > } > -- > 1.7.0.4 > > > > On May 23, 2011, at 9:42 PM, Chris Ball wrote: > >> Hi Philip, >> >> On Sun, May 22 2011, Philip Rakity wrote: >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>> index baab027..6db91d6 100644 >>> --- a/drivers/mmc/core/mmc.c >>> +++ b/drivers/mmc/core/mmc.c >>> @@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card) >>> } >>> >>> /* >>> - * Read and decode extended CSD. >>> + * Read extended CSD. >>> */ >>> -static int mmc_read_ext_csd(struct mmc_card *card) >>> +static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd) >> >> This one doesn't work -- you're missing the half of the patch that hooks >> up the rest of mmc.c to the new code, which was in v1. As-is, we get: >> >> drivers/mmc/core/mmc.c: In function ‘mmc_init_card’: >> drivers/mmc/core/mmc.c:621:3: error: too few arguments to function ‘mmc_read_ext_csd’ >> drivers/mmc/core/mmc.c:240:12: note: declared here >> drivers/mmc/core/mmc.c: At top level: >> drivers/mmc/core/mmc.c:404:12: warning: ‘mmc_compare_ext_csds’ defined but not used [-Wunused-function] >> >> - Chris. >> -- >> Chris Ball <cjb@laptop.org> <http://printf.net/> >> One Laptop Per Child >> -- >> 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 > -- 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
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index baab027..6db91d6 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card) } /* - * Read and decode extended CSD. + * Read extended CSD. */ -static int mmc_read_ext_csd(struct mmc_card *card) +static int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd) { int err; u8 *ext_csd; BUG_ON(!card); + BUG_ON(!new_ext_csd); + + *new_ext_csd = NULL; if (card->csd.mmca_vsn < CSD_SPEC_VER_4) return 0; @@ -199,12 +202,15 @@ static int mmc_read_ext_csd(struct mmc_card *card) err = mmc_send_ext_csd(card, ext_csd); if (err) { + kfree(ext_csd); + *new_ext_csd = NULL; + /* If the host or the card can't do the switch, * fail more gracefully. */ if ((err != -EINVAL) && (err != -ENOSYS) && (err != -EFAULT)) - goto out; + return err; /* * High capacity cards should have this "magic" size @@ -222,9 +228,23 @@ static int mmc_read_ext_csd(struct mmc_card *card) mmc_hostname(card->host)); err = 0; } + } else + *new_ext_csd = ext_csd; - goto out; - } + return err; +} + +/* + * Decode extended CSD. + */ +static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd) +{ + int err = 0; + + BUG_ON(!card); + + if (!ext_csd) + return 0; /* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */ if (card->csd.structure == 3) { @@ -372,8 +392,70 @@ static int mmc_read_ext_csd(struct mmc_card *card) card->erased_byte = 0x0; out: + return err; +} + +static inline void mmc_free_ext_csd(u8 *ext_csd) +{ kfree(ext_csd); +} + +static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd, + unsigned bus_width) +{ + u8 *bw_ext_csd; + int err; + + err = mmc_get_ext_csd(card, &bw_ext_csd); + if (err) + return err; + + if ((ext_csd == NULL || bw_ext_csd == NULL)) { + if (bus_width != MMC_BUS_WIDTH_1) + err = -EINVAL; + goto out; + } + + if (bus_width == MMC_BUS_WIDTH_1) + goto out; + + /* only compare read only fields */ + + err = (!(ext_csd[EXT_CSD_PARTITION_SUPPORT] == + bw_ext_csd[EXT_CSD_PARTITION_SUPPORT]) && + (ext_csd[EXT_CSD_ERASED_MEM_CONT] == + bw_ext_csd[EXT_CSD_ERASED_MEM_CONT]) && + (ext_csd[EXT_CSD_REV] == + bw_ext_csd[EXT_CSD_REV]) && + (ext_csd[EXT_CSD_STRUCTURE] == + bw_ext_csd[EXT_CSD_STRUCTURE]) && + (ext_csd[EXT_CSD_CARD_TYPE] == + bw_ext_csd[EXT_CSD_CARD_TYPE]) && + (ext_csd[EXT_CSD_S_A_TIMEOUT] == + bw_ext_csd[EXT_CSD_S_A_TIMEOUT]) && + (ext_csd[EXT_CSD_HC_WP_GRP_SIZE] == + bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE]) && + (ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] == + bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT]) && + (ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] == + bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]) && + (ext_csd[EXT_CSD_SEC_TRIM_MULT] == + bw_ext_csd[EXT_CSD_SEC_TRIM_MULT]) && + (ext_csd[EXT_CSD_SEC_ERASE_MULT] == + bw_ext_csd[EXT_CSD_SEC_ERASE_MULT]) && + (ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] == + bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT]) && + (ext_csd[EXT_CSD_TRIM_MULT] == + bw_ext_csd[EXT_CSD_TRIM_MULT]) && + memcmp(&ext_csd[EXT_CSD_SEC_CNT], + &bw_ext_csd[EXT_CSD_SEC_CNT], + 4) != 0); + if (err) + err = -EINVAL; + +out: + mmc_free_ext_csd(bw_ext_csd); return err; } @@ -438,6 +520,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, u32 cid[4]; unsigned int max_dtr; u32 rocr; + u8 *ext_csd = NULL; BUG_ON(!host); WARN_ON(!host->claimed); @@ -536,7 +619,11 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, /* * Fetch and process extended CSD. */ - err = mmc_read_ext_csd(card); + + err = mmc_get_ext_csd(card, &ext_csd); + if (err) + goto free_card; + err = mmc_read_ext_csd(card, ext_csd); if (err) goto free_card; @@ -676,14 +763,18 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, 0); if (!err) { mmc_set_bus_width(card->host, bus_width); + /* * If controller can't handle bus width test, - * use the highest bus width to maintain - * compatibility with previous MMC behavior. + * compare ext_csd previously read in 1 bit mode + * against ext_csd at new bus width */ if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST)) - break; - err = mmc_bus_test(card, bus_width); + err = mmc_compare_ext_csds(card, + ext_csd, + bus_width); + else + err = mmc_bus_test(card, bus_width); if (!err) break; } @@ -730,12 +821,14 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, if (!oldcard) host->card = card; + mmc_free_ext_csd(ext_csd); return 0; free_card: if (!oldcard) mmc_remove_card(card); err: + mmc_free_ext_csd(ext_csd); return err; }
resend Note: I cannot test test for a few days. Following Chris suggestion posting to list. version 2 --- return -EINVAL when compare of ext_csd_fails combine two compare routines into one per suggestion from Chris. --- version 1 CMD19 -- The offical way to validate bus widths from the JEDEC spec does not work on all platforms. Some platforms that use PCI/PCIe to connect their SD controllers are known to fail. If the quirk MMC_BUS_WIDTH_TEST is not defined we try to figure out the bus width by reading the ext_csd at different bus widths and compare this against the ext_csd read in 1 bit mode. If no ext_csd is available we default to 1 bit operations. Code has been tested on mmp2 against 8 bit eMMC and Transcend 2GB card that is known to not work in 4 bit mode. The physical pins on the card are not present to support 4 bit operation. Signed-off-by: Philip Rakity <prakity@marvell.com> --- drivers/mmc/core/mmc.c | 113 +++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 103 insertions(+), 10 deletions(-)