| Message ID | 1519692565-110152-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, Gentle ping to see if possible this could be in -next for some days before 4.17 :) On 2018/2/27 8:49, Shawn Lin wrote: > Per SD specification physical layer v4.0, section 3.9.4, it > says "UHS-I supports only 4-bit mode. Host shall select 4-bit > mode by ACMD6. However mmc_sd_init_uhs_card() still go ahead > to initialize the cards anyway, whether card or host won't > support 4-bit mode. > > This breaks the platforms which could support UHS-I mode but on > some certain boards only support 1-bit mode with a UHS-I card inserted, > as all the tuning process is broken due to this. Alternatively, we > should check the return value from mmc_set_bus_width() to see if host > could finish the request to switch the bus width on its side. But that > needs more thing to do than this patch that just bails out early to try > high speed mode if 4-bit mode isn't available for whatever reason. And > this patch could also fix the same problem for sdio since R4_18V_PRESENT > won't be set for ocr when mmc_sdio_init_card() finds mmc_host_uhs() > is false. > > Note that this patch doesn't keep the checking of card->scr.sda_spec3 > and comparing card->scr.bus_widths with SD_SCR_BUS_WIDTH_4 within > mmc_sd_init_uhs_card() since if the sd cards response with SD_ROCR_S18A, > it definitely supports UHS-I mode, which implicitly means these checkings > are always true. > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > > --- > > Changes in v2: > - correct the mmc_host_uhs > > drivers/mmc/core/host.h | 3 ++- > drivers/mmc/core/sd.c | 16 +++++----------- > 2 files changed, 7 insertions(+), 12 deletions(-) > > diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h > index 06ec19b..4805438 100644 > --- a/drivers/mmc/core/host.h > +++ b/drivers/mmc/core/host.h > @@ -56,7 +56,8 @@ static inline int mmc_host_uhs(struct mmc_host *host) > return host->caps & > (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 | > MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | > - MMC_CAP_UHS_DDR50); > + MMC_CAP_UHS_DDR50) && > + host->caps & MMC_CAP_4_BIT_DATA; > } > > static inline bool mmc_card_hs200(struct mmc_card *card) > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index 62b84dd..d094497 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -582,9 +582,6 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) > int err; > u8 *status; > > - if (!card->scr.sda_spec3) > - return 0; > - > if (!(card->csd.cmdclass & CCC_SWITCH)) > return 0; > > @@ -593,14 +590,11 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) > return -ENOMEM; > > /* Set 4-bit bus width */ > - if ((card->host->caps & MMC_CAP_4_BIT_DATA) && > - (card->scr.bus_widths & SD_SCR_BUS_WIDTH_4)) { > - err = mmc_app_set_bus_width(card, MMC_BUS_WIDTH_4); > - if (err) > - goto out; > + err = mmc_app_set_bus_width(card, MMC_BUS_WIDTH_4); > + if (err) > + goto out; > > - mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4); > - } > + mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4); > > /* > * Select the bus speed mode depending on host > @@ -1033,7 +1027,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, > } > > /* Initialization sequence for UHS-I cards */ > - if (rocr & SD_ROCR_S18A) { > + if (rocr & SD_ROCR_S18A && mmc_host_uhs(host)) { > err = mmc_sd_init_uhs_card(card); > if (err) > goto free_card; > -- 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
On 27 February 2018 at 01:49, Shawn Lin <shawn.lin@rock-chips.com> wrote: > Per SD specification physical layer v4.0, section 3.9.4, it > says "UHS-I supports only 4-bit mode. Host shall select 4-bit > mode by ACMD6. However mmc_sd_init_uhs_card() still go ahead > to initialize the cards anyway, whether card or host won't > support 4-bit mode. > > This breaks the platforms which could support UHS-I mode but on > some certain boards only support 1-bit mode with a UHS-I card inserted, > as all the tuning process is broken due to this. Alternatively, we > should check the return value from mmc_set_bus_width() to see if host > could finish the request to switch the bus width on its side. But that > needs more thing to do than this patch that just bails out early to try > high speed mode if 4-bit mode isn't available for whatever reason. And > this patch could also fix the same problem for sdio since R4_18V_PRESENT > won't be set for ocr when mmc_sdio_init_card() finds mmc_host_uhs() > is false. > > Note that this patch doesn't keep the checking of card->scr.sda_spec3 > and comparing card->scr.bus_widths with SD_SCR_BUS_WIDTH_4 within > mmc_sd_init_uhs_card() since if the sd cards response with SD_ROCR_S18A, > it definitely supports UHS-I mode, which implicitly means these checkings > are always true. > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> Thanks, applied for next! Perhaps we should send this as fix and add a stable tag, but let's see how the test goes in next first. Kind regards Uffe > > --- > > Changes in v2: > - correct the mmc_host_uhs > > drivers/mmc/core/host.h | 3 ++- > drivers/mmc/core/sd.c | 16 +++++----------- > 2 files changed, 7 insertions(+), 12 deletions(-) > > diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h > index 06ec19b..4805438 100644 > --- a/drivers/mmc/core/host.h > +++ b/drivers/mmc/core/host.h > @@ -56,7 +56,8 @@ static inline int mmc_host_uhs(struct mmc_host *host) > return host->caps & > (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 | > MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | > - MMC_CAP_UHS_DDR50); > + MMC_CAP_UHS_DDR50) && > + host->caps & MMC_CAP_4_BIT_DATA; > } > > static inline bool mmc_card_hs200(struct mmc_card *card) > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index 62b84dd..d094497 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -582,9 +582,6 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) > int err; > u8 *status; > > - if (!card->scr.sda_spec3) > - return 0; > - > if (!(card->csd.cmdclass & CCC_SWITCH)) > return 0; > > @@ -593,14 +590,11 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) > return -ENOMEM; > > /* Set 4-bit bus width */ > - if ((card->host->caps & MMC_CAP_4_BIT_DATA) && > - (card->scr.bus_widths & SD_SCR_BUS_WIDTH_4)) { > - err = mmc_app_set_bus_width(card, MMC_BUS_WIDTH_4); > - if (err) > - goto out; > + err = mmc_app_set_bus_width(card, MMC_BUS_WIDTH_4); > + if (err) > + goto out; > > - mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4); > - } > + mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4); > > /* > * Select the bus speed mode depending on host > @@ -1033,7 +1027,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, > } > > /* Initialization sequence for UHS-I cards */ > - if (rocr & SD_ROCR_S18A) { > + if (rocr & SD_ROCR_S18A && mmc_host_uhs(host)) { > err = mmc_sd_init_uhs_card(card); > if (err) > goto free_card; > -- > 1.9.1 > > -- 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/host.h b/drivers/mmc/core/host.h index 06ec19b..4805438 100644 --- a/drivers/mmc/core/host.h +++ b/drivers/mmc/core/host.h @@ -56,7 +56,8 @@ static inline int mmc_host_uhs(struct mmc_host *host) return host->caps & (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 | MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | - MMC_CAP_UHS_DDR50); + MMC_CAP_UHS_DDR50) && + host->caps & MMC_CAP_4_BIT_DATA; } static inline bool mmc_card_hs200(struct mmc_card *card) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 62b84dd..d094497 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -582,9 +582,6 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) int err; u8 *status; - if (!card->scr.sda_spec3) - return 0; - if (!(card->csd.cmdclass & CCC_SWITCH)) return 0; @@ -593,14 +590,11 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) return -ENOMEM; /* Set 4-bit bus width */ - if ((card->host->caps & MMC_CAP_4_BIT_DATA) && - (card->scr.bus_widths & SD_SCR_BUS_WIDTH_4)) { - err = mmc_app_set_bus_width(card, MMC_BUS_WIDTH_4); - if (err) - goto out; + err = mmc_app_set_bus_width(card, MMC_BUS_WIDTH_4); + if (err) + goto out; - mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4); - } + mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4); /* * Select the bus speed mode depending on host @@ -1033,7 +1027,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, } /* Initialization sequence for UHS-I cards */ - if (rocr & SD_ROCR_S18A) { + if (rocr & SD_ROCR_S18A && mmc_host_uhs(host)) { err = mmc_sd_init_uhs_card(card); if (err) goto free_card;
Per SD specification physical layer v4.0, section 3.9.4, it says "UHS-I supports only 4-bit mode. Host shall select 4-bit mode by ACMD6. However mmc_sd_init_uhs_card() still go ahead to initialize the cards anyway, whether card or host won't support 4-bit mode. This breaks the platforms which could support UHS-I mode but on some certain boards only support 1-bit mode with a UHS-I card inserted, as all the tuning process is broken due to this. Alternatively, we should check the return value from mmc_set_bus_width() to see if host could finish the request to switch the bus width on its side. But that needs more thing to do than this patch that just bails out early to try high speed mode if 4-bit mode isn't available for whatever reason. And this patch could also fix the same problem for sdio since R4_18V_PRESENT won't be set for ocr when mmc_sdio_init_card() finds mmc_host_uhs() is false. Note that this patch doesn't keep the checking of card->scr.sda_spec3 and comparing card->scr.bus_widths with SD_SCR_BUS_WIDTH_4 within mmc_sd_init_uhs_card() since if the sd cards response with SD_ROCR_S18A, it definitely supports UHS-I mode, which implicitly means these checkings are always true. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- Changes in v2: - correct the mmc_host_uhs drivers/mmc/core/host.h | 3 ++- drivers/mmc/core/sd.c | 16 +++++----------- 2 files changed, 7 insertions(+), 12 deletions(-)