| Message ID | 1354620980-23764-3-git-send-email-subhashj@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, > From: Sujit Reddy Thumma <sthumma@codeaurora.org> > > According to UHS-I initialization sequence for SDIO 3.0 cards, > the host must set bit[24] (S18R) of OCR register during OCR > handshake to know whether the SDIO card is capable of doing > 1.8V I/O. In SDIO 2.0 spec, bit[24] is reserved, "shall be set to 0". Setting it to 1 might cause side effect to 2.0 cards. Perhaps using a quirk for S18R is more suitable for all combinations. Regards, Bing > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> > --- > drivers/mmc/core/sdio.c | 22 +++++++++++----------- > include/linux/mmc/host.h | 8 ++++++++ > 2 files changed, 19 insertions(+), 11 deletions(-) -- 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
Hi Bing, The function mmc_host_uhs() will return false and never set bit[24] (S18R) for SDIO 2.0 cards. Thanks, Jackey -----Original Message----- From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Bing Zhao Sent: Wednesday, December 05, 2012 5:14 AM To: Subhash Jadavani; linux-mmc@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org; Sujit Reddy Thumma Subject: RE: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I initialization sequence Hi, > From: Sujit Reddy Thumma <sthumma@codeaurora.org> > > According to UHS-I initialization sequence for SDIO 3.0 cards, the > host must set bit[24] (S18R) of OCR register during OCR handshake to > know whether the SDIO card is capable of doing 1.8V I/O. In SDIO 2.0 spec, bit[24] is reserved, "shall be set to 0". Setting it to 1 might cause side effect to 2.0 cards. Perhaps using a quirk for S18R is more suitable for all combinations. Regards, Bing > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> > --- > drivers/mmc/core/sdio.c | 22 +++++++++++----------- > include/linux/mmc/host.h | 8 ++++++++ > 2 files changed, 19 insertions(+), 11 deletions(-) -- 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
Hi Jackey, > Hi Bing, > > The function mmc_host_uhs() will return false and never set bit[24] (S18R) for SDIO 2.0 cards. I thought mmc_host_uhs() will return host controller's capabilities, no? Thanks, Bing > > Thanks, > Jackey > > -----Original Message----- > From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Bing Zhao > Sent: Wednesday, December 05, 2012 5:14 AM > To: Subhash Jadavani; linux-mmc@vger.kernel.org > Cc: linux-arm-msm@vger.kernel.org; Sujit Reddy Thumma > Subject: RE: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I initialization sequence > > Hi, > > > From: Sujit Reddy Thumma <sthumma@codeaurora.org> > > > > According to UHS-I initialization sequence for SDIO 3.0 cards, the > > host must set bit[24] (S18R) of OCR register during OCR handshake to > > know whether the SDIO card is capable of doing 1.8V I/O. > > In SDIO 2.0 spec, bit[24] is reserved, "shall be set to 0". > Setting it to 1 might cause side effect to 2.0 cards. > Perhaps using a quirk for S18R is more suitable for all combinations. > > Regards, > Bing > > > > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> > > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> > > --- > > drivers/mmc/core/sdio.c | 22 +++++++++++----------- > > include/linux/mmc/host.h | 8 ++++++++ > > 2 files changed, 19 insertions(+), 11 deletions(-) > > -- > 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
Hi Bing, sdhci_add_host() reads host controller's capabilities and initialize host->caps of function mmc_host_uhs(). So mmc_host_uhs() only return this cached values. Thanks, Jackey -----Original Message----- From: Bing Zhao [mailto:bzhao@marvell.com] Sent: Thursday, December 06, 2012 3:14 AM To: Shen, Jackey; Subhash Jadavani; linux-mmc@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org; Sujit Reddy Thumma Subject: RE: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I initialization sequence Hi Jackey, > Hi Bing, > > The function mmc_host_uhs() will return false and never set bit[24] (S18R) for SDIO 2.0 cards. I thought mmc_host_uhs() will return host controller's capabilities, no? Thanks, Bing > > Thanks, > Jackey > > -----Original Message----- > From: linux-mmc-owner@vger.kernel.org > [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Bing Zhao > Sent: Wednesday, December 05, 2012 5:14 AM > To: Subhash Jadavani; linux-mmc@vger.kernel.org > Cc: linux-arm-msm@vger.kernel.org; Sujit Reddy Thumma > Subject: RE: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I > initialization sequence > > Hi, > > > From: Sujit Reddy Thumma <sthumma@codeaurora.org> > > > > According to UHS-I initialization sequence for SDIO 3.0 cards, the > > host must set bit[24] (S18R) of OCR register during OCR handshake to > > know whether the SDIO card is capable of doing 1.8V I/O. > > In SDIO 2.0 spec, bit[24] is reserved, "shall be set to 0". > Setting it to 1 might cause side effect to 2.0 cards. > Perhaps using a quirk for S18R is more suitable for all combinations. > > Regards, > Bing > > > > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> > > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> > > --- > > drivers/mmc/core/sdio.c | 22 +++++++++++----------- > > include/linux/mmc/host.h | 8 ++++++++ > > 2 files changed, 19 insertions(+), 11 deletions(-) > > -- > 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
Hi Jackey, > Hi Bing, > > sdhci_add_host() reads host controller's capabilities and initialize host->caps of function > mmc_host_uhs(). So mmc_host_uhs() only return this cached values. So, it does represent host controller's capabilities, not the capabilities of the card. What happens if I have a UHS capable controller and a 2.0 card? The S18R bit will be set in this case, which might cause problem for the card? Thanks, Bing > > Thanks, > Jackey > > -----Original Message----- > From: Bing Zhao [mailto:bzhao@marvell.com] > Sent: Thursday, December 06, 2012 3:14 AM > To: Shen, Jackey; Subhash Jadavani; linux-mmc@vger.kernel.org > Cc: linux-arm-msm@vger.kernel.org; Sujit Reddy Thumma > Subject: RE: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I initialization sequence > > Hi Jackey, > > > Hi Bing, > > > > The function mmc_host_uhs() will return false and never set bit[24] (S18R) for SDIO 2.0 cards. > > I thought mmc_host_uhs() will return host controller's capabilities, no? > > Thanks, > Bing > > > > > Thanks, > > Jackey > > > > -----Original Message----- > > From: linux-mmc-owner@vger.kernel.org > > [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Bing Zhao > > Sent: Wednesday, December 05, 2012 5:14 AM > > To: Subhash Jadavani; linux-mmc@vger.kernel.org > > Cc: linux-arm-msm@vger.kernel.org; Sujit Reddy Thumma > > Subject: RE: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I > > initialization sequence > > > > Hi, > > > > > From: Sujit Reddy Thumma <sthumma@codeaurora.org> > > > > > > According to UHS-I initialization sequence for SDIO 3.0 cards, the > > > host must set bit[24] (S18R) of OCR register during OCR handshake to > > > know whether the SDIO card is capable of doing 1.8V I/O. > > > > In SDIO 2.0 spec, bit[24] is reserved, "shall be set to 0". > > Setting it to 1 might cause side effect to 2.0 cards. > > Perhaps using a quirk for S18R is more suitable for all combinations. > > > > Regards, > > Bing > > > > > > > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> > > > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> > > > --- > > > drivers/mmc/core/sdio.c | 22 +++++++++++----------- > > > include/linux/mmc/host.h | 8 ++++++++ > > > 2 files changed, 19 insertions(+), 11 deletions(-) > > > > -- > > 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
On 12/6/2012 9:03 AM, Bing Zhao wrote: > Hi Jackey, > >> Hi Bing, >> >> sdhci_add_host() reads host controller's capabilities and initialize host->caps of function >> mmc_host_uhs(). So mmc_host_uhs() only return this cached values. > So, it does represent host controller's capabilities, not the capabilities of the card. > What happens if I have a UHS capable controller and a 2.0 card? > The S18R bit will be set in this case, which might cause problem for the card? Hi Bing, Why do you see an issue with setting S18R bit when sending CMD5 to SDIO2.0 compliant card? If SDIO2.0 card don't support it, return OCR won't have the S18A bit set. We are already doing same thing for SD cards as well (check mmc_sd_get_cid function). There also even if we set the S18R bit which sending OCR to non-SD3.0 cards, they respond back with S18A bit cleared. If non-SDIO3.0 card misbehaves after receiving the OCR with S18R bit set then it means card is not compliant to specification properly and in that case those cards can be handled with QUIRKs. But there is no need to add any QUIRKs as of now. I hope this make sense. Regards, Subhash > > Thanks, > Bing > >> Thanks, >> Jackey >> >> -----Original Message----- >> From: Bing Zhao [mailto:bzhao@marvell.com] >> Sent: Thursday, December 06, 2012 3:14 AM >> To: Shen, Jackey; Subhash Jadavani; linux-mmc@vger.kernel.org >> Cc: linux-arm-msm@vger.kernel.org; Sujit Reddy Thumma >> Subject: RE: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I initialization sequence >> >> Hi Jackey, >> >>> Hi Bing, >>> >>> The function mmc_host_uhs() will return false and never set bit[24] (S18R) for SDIO 2.0 cards. >> I thought mmc_host_uhs() will return host controller's capabilities, no? >> >> Thanks, >> Bing >> >>> Thanks, >>> Jackey >>> >>> -----Original Message----- >>> From: linux-mmc-owner@vger.kernel.org >>> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Bing Zhao >>> Sent: Wednesday, December 05, 2012 5:14 AM >>> To: Subhash Jadavani; linux-mmc@vger.kernel.org >>> Cc: linux-arm-msm@vger.kernel.org; Sujit Reddy Thumma >>> Subject: RE: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I >>> initialization sequence >>> >>> Hi, >>> >>>> From: Sujit Reddy Thumma <sthumma@codeaurora.org> >>>> >>>> According to UHS-I initialization sequence for SDIO 3.0 cards, the >>>> host must set bit[24] (S18R) of OCR register during OCR handshake to >>>> know whether the SDIO card is capable of doing 1.8V I/O. >>> In SDIO 2.0 spec, bit[24] is reserved, "shall be set to 0". >>> Setting it to 1 might cause side effect to 2.0 cards. >>> Perhaps using a quirk for S18R is more suitable for all combinations. >>> >>> Regards, >>> Bing >>> >>>> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> >>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >>>> --- >>>> drivers/mmc/core/sdio.c | 22 +++++++++++----------- >>>> include/linux/mmc/host.h | 8 ++++++++ >>>> 2 files changed, 19 insertions(+), 11 deletions(-) >>> -- >>> 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
Hi Subhash, > >> sdhci_add_host() reads host controller's capabilities and initialize host->caps of function > >> mmc_host_uhs(). So mmc_host_uhs() only return this cached values. > > So, it does represent host controller's capabilities, not the capabilities of the card. > > What happens if I have a UHS capable controller and a 2.0 card? > > The S18R bit will be set in this case, which might cause problem for the card? > Hi Bing, > > Why do you see an issue with setting S18R bit when sending CMD5 to > SDIO2.0 compliant card? > If SDIO2.0 card don't support it, return OCR won't have the S18A bit set. Some times ago I ever encountered a problem with CMD52 when I played with an SDIO driver and accidentally set one of the reserved bits to 1. The issue was resolved immediately after I realized it and cleared that bit to 0. This experience makes me thinking that CMD5 S18R might confuse some 2.0 cards, because bit24 is also a reserved bit in 2.0 spec. > We are already doing same thing for SD cards as well (check > mmc_sd_get_cid function). There also even if we set the S18R bit which > sending OCR to non-SD3.0 cards, they respond back with S18A bit cleared. Then it's fair enough to handle SDIO the same way. Thanks for the elaboration. > > If non-SDIO3.0 card misbehaves after receiving the OCR with S18R bit set > then it means card is not compliant to specification properly and in > that case those cards can be handled with QUIRKs. But there is no need > to add any QUIRKs as of now. That sounds reasonable. The quirk for non-compliant cards can be done later when they come up. Regards, Bing > > I hope this make sense. > > Regards, > Subhash > -- 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
Hi, 2012/12/4 Subhash Jadavani <subhashj@codeaurora.org>: > From: Sujit Reddy Thumma <sthumma@codeaurora.org> > > According to UHS-I initialization sequence for SDIO 3.0 cards, > the host must set bit[24] (S18R) of OCR register during OCR > handshake to know whether the SDIO card is capable of doing > 1.8V I/O. > Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com> -- 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 7 December 2012 09:42, Johan Rudholm <jrudholm@gmail.com> wrote: > Hi, > > 2012/12/4 Subhash Jadavani <subhashj@codeaurora.org>: >> From: Sujit Reddy Thumma <sthumma@codeaurora.org> >> >> According to UHS-I initialization sequence for SDIO 3.0 cards, >> the host must set bit[24] (S18R) of OCR register during OCR >> handshake to know whether the SDIO card is capable of doing >> 1.8V I/O. >> > > Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com> > -- > 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 Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> -- 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/sdio.c b/drivers/mmc/core/sdio.c index 34ad4c8..9565d38 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -157,10 +157,7 @@ static int sdio_read_cccr(struct mmc_card *card, u32 ocr) if (ret) goto out; - if (card->host->caps & - (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 | - MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | - MMC_CAP_UHS_DDR50)) { + if (mmc_host_uhs(card->host)) { if (data & SDIO_UHS_DDR50) card->sw_caps.sd3_bus_mode |= SD_MODE_UHS_DDR50; @@ -478,8 +475,7 @@ static int sdio_set_bus_speed_mode(struct mmc_card *card) * If the host doesn't support any of the UHS-I modes, fallback on * default speed. */ - if (!(card->host->caps & (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 | - MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | MMC_CAP_UHS_DDR50))) + if (!mmc_host_uhs(card->host)) return 0; bus_speed = SDIO_SPEED_SDR12; @@ -645,11 +641,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr, * systems that claim 1.8v signalling in fact do not support * it. */ - if ((ocr & R4_18V_PRESENT) && - (host->caps & - (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 | - MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | - MMC_CAP_UHS_DDR50))) { + if ((ocr & R4_18V_PRESENT) && mmc_host_uhs(host)) { err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, true); if (err) { @@ -1022,6 +1014,10 @@ static int mmc_sdio_power_restore(struct mmc_host *host) goto out; } + if (mmc_host_uhs(host)) + /* to query card if 1.8V signalling is supported */ + host->ocr |= R4_18V_PRESENT; + ret = mmc_sdio_init_card(host, host->ocr, host->card, mmc_card_keep_power(host)); if (!ret && host->sdio_irqs) @@ -1087,6 +1083,10 @@ int mmc_attach_sdio(struct mmc_host *host) /* * Detect and init the card. */ + if (mmc_host_uhs(host)) + /* to query card if 1.8V signalling is supported */ + host->ocr |= R4_18V_PRESENT; + err = mmc_sdio_init_card(host, host->ocr, NULL, 0); if (err) { if (err == -EAGAIN) { diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 23df21e..9be0440 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -433,6 +433,14 @@ static inline int mmc_boot_partition_access(struct mmc_host *host) return !(host->caps2 & MMC_CAP2_BOOTPART_NOACC); } +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); +} + #ifdef CONFIG_MMC_CLKGATE void mmc_host_clk_hold(struct mmc_host *host); void mmc_host_clk_release(struct mmc_host *host);