| Message ID | CAGsJ_4x5Nd3Cts1BHsUyB_mCejxbHPPjznUVWUcFoZhxKy+Tcg@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15 September 2015 at 09:13, Barry Song <21cnbao@gmail.com> wrote: > 2015-08-25 20:05 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>: >> On 20 August 2015 at 02:16, Barry Song <21cnbao@gmail.com> wrote: >>> 2015-08-18 1:11 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>: >>>> On 11 August 2015 at 10:41, Barry Song <21cnbao@gmail.com> wrote: >>>>> From: Weijun Yang <york.yang@csr.com> >>>>> >>>>> As SD Specifications Part1 Physical Layer Specification Version >>>>> 3.01 says, CMD19 tuning is available for unlocked cards in transfer >>>>> state of 1.8V signaling mode. The small difference between v3.00 >>>>> and 3.01 spec means that CMD19 tuning is also available for DDR50 >>>>> mode. >>>> >>>> So what happens with cards following the 3.0 spec version, those >>>> doesn't need to support the tuning CMD right? Perhaps that needs to be >>>> addressed in this patch well!? >>> >>> from HW registers of the card, we cann't know whether the HW needs >>> tuning. it is said 3.0.x need tuning, but 3.0 doesn't need. @weijun, >>> pls fix me if i am wrong. >>> if so, it seems we need a static flag somewhere to indicate whether >>> the tuning is needed. we can't detect to find the tuning requirement. >> >> Another way is to always try doing the tuning for DDR50, but in case >> of errors just ignore them and print a debug/info message!? > > Uffe, do you mean something like below: > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index 4e7366a..d4df9f0 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -629,8 +629,23 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) > */ > if (!mmc_host_is_spi(card->host) && > (card->sd_bus_speed == UHS_SDR50_BUS_SPEED || > - card->sd_bus_speed == UHS_SDR104_BUS_SPEED)) > + card->sd_bus_speed == UHS_DDR50_BUS_SPEED || > + card->sd_bus_speed == UHS_SDR104_BUS_SPEED)) { > err = mmc_execute_tuning(card); > + > + /* > + * As SD Specifications Part1 Physical Layer Specification Version > + * 3.01 says, CMD19 tuning is available for unlocked cards in transfer > + * state of 1.8V signaling mode. The small difference between v3.00 > + * and 3.01 spec means that CMD19 tuning is also available for DDR50 > + * mode. > + */ > + if (err && (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) > + pr_err("%s: ddr50 tuning failed\n", mmc_hostname(card->host)); Perhaps pr_warn() instead. Moreover, you may just assign err = 0 and leave the return/kfree to the "out" label. > + kfree(status); > + return 0; > + } > + } > out: > kfree(status); > [...] Also, I was giving this a second thought... What happens with the SD 3.0 card when it *doesn't* support CMD19. Will it still be functional afterwards or will it move from "transfer state" to another not known state? So maybe we should just do the tuning and return the error if it fails, as you suggested in the original patch. :-) Kind regards Uffe -- 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
2015-09-17 20:27 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>: > On 15 September 2015 at 09:13, Barry Song <21cnbao@gmail.com> wrote: >> 2015-08-25 20:05 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>: >>> On 20 August 2015 at 02:16, Barry Song <21cnbao@gmail.com> wrote: >>>> 2015-08-18 1:11 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>: >>>>> On 11 August 2015 at 10:41, Barry Song <21cnbao@gmail.com> wrote: >>>>>> From: Weijun Yang <york.yang@csr.com> >>>>>> >>>>>> As SD Specifications Part1 Physical Layer Specification Version >>>>>> 3.01 says, CMD19 tuning is available for unlocked cards in transfer >>>>>> state of 1.8V signaling mode. The small difference between v3.00 >>>>>> and 3.01 spec means that CMD19 tuning is also available for DDR50 >>>>>> mode. >>>>> >>>>> So what happens with cards following the 3.0 spec version, those >>>>> doesn't need to support the tuning CMD right? Perhaps that needs to be >>>>> addressed in this patch well!? >>>> >>>> from HW registers of the card, we cann't know whether the HW needs >>>> tuning. it is said 3.0.x need tuning, but 3.0 doesn't need. @weijun, >>>> pls fix me if i am wrong. >>>> if so, it seems we need a static flag somewhere to indicate whether >>>> the tuning is needed. we can't detect to find the tuning requirement. >>> >>> Another way is to always try doing the tuning for DDR50, but in case >>> of errors just ignore them and print a debug/info message!? >> >> Uffe, do you mean something like below: >> >> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c >> index 4e7366a..d4df9f0 100644 >> --- a/drivers/mmc/core/sd.c >> +++ b/drivers/mmc/core/sd.c >> @@ -629,8 +629,23 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) >> */ >> if (!mmc_host_is_spi(card->host) && >> (card->sd_bus_speed == UHS_SDR50_BUS_SPEED || >> - card->sd_bus_speed == UHS_SDR104_BUS_SPEED)) >> + card->sd_bus_speed == UHS_DDR50_BUS_SPEED || >> + card->sd_bus_speed == UHS_SDR104_BUS_SPEED)) { >> err = mmc_execute_tuning(card); >> + >> + /* >> + * As SD Specifications Part1 Physical Layer Specification Version >> + * 3.01 says, CMD19 tuning is available for unlocked cards in transfer >> + * state of 1.8V signaling mode. The small difference between v3.00 >> + * and 3.01 spec means that CMD19 tuning is also available for DDR50 >> + * mode. >> + */ >> + if (err && (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) >> + pr_err("%s: ddr50 tuning failed\n", mmc_hostname(card->host)); > > Perhaps pr_warn() instead. > > Moreover, you may just assign err = 0 and leave the return/kfree to > the "out" label. > >> + kfree(status); >> + return 0; >> + } >> + } >> out: >> kfree(status); >> > > [...] > > Also, I was giving this a second thought... > > What happens with the SD 3.0 card when it *doesn't* support CMD19. > Will it still be functional afterwards or will it move from "transfer > state" to another not known state? uffe, i am not the expert of this issue but weijun is. weijun told that for the cards who don't support cmd19, it will take it as illegal command and not response it. and it also sets illegal command error in registers. but the card status is still functional and doesn't cause unknown state. so maybe we can just ignore the err of tuning, and use the second patch with the refine you mentioned? > > So maybe we should just do the tuning and return the error if it > fails, as you suggested in the original patch. :-) > > Kind regards > Uffe -barry -- 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
[...] >> >> Also, I was giving this a second thought... >> >> What happens with the SD 3.0 card when it *doesn't* support CMD19. >> Will it still be functional afterwards or will it move from "transfer >> state" to another not known state? > > uffe, i am not the expert of this issue but weijun is. weijun told that > for the cards who don't support cmd19, it will take it as illegal > command and not response it. and it also sets illegal command error in > registers. > but the card status is still functional and doesn't cause unknown state. > > so maybe we can just ignore the err of tuning, and use the second > patch with the refine you mentioned? Sure, let's do that. Please send a v2 of the patchset then. Kind regards Uffe -- 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 Wed, Sep 23, 2015 at 9:25 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > [...] > >>> >>> Also, I was giving this a second thought... >>> >>> What happens with the SD 3.0 card when it *doesn't* support CMD19. >>> Will it still be functional afterwards or will it move from "transfer >>> state" to another not known state? >> >> uffe, i am not the expert of this issue but weijun is. weijun told that >> for the cards who don't support cmd19, it will take it as illegal >> command and not response it. and it also sets illegal command error in >> registers. >> but the card status is still functional and doesn't cause unknown state. >> >> so maybe we can just ignore the err of tuning, and use the second >> patch with the refine you mentioned? > > Sure, let's do that. Please send a v2 of the patchset then. Replying here since I'm not subscribed to linux-mmc. Commit 9faac7b95 "mmc: sdhci: enable tuning for DDR50" actually breaks WiFi on SDIO at least for the ASUS X205TA. I tried to ignore the error from the tuning also for the SDIO card patching mmc_sdio_init_uhs_card() in drivers/mmc/core/sdio.c but I end up with the error "Controller never released inhibit bit(s)". At least in my case CMD19 shouldn't be issued at all to have the SDIO card working. Any suggestion how to approach this problem?
SGksIENhcmxvDQoJSW4gOWZhYWM3Yjk1LCB0dW5pbmcgZm9yIEREUjUwIGlzIGVuYWJsZWQgaW4g bW1jX3NkX2luaXRfdWhzX2NhcmQoKSwgc28gb25seSBTRCBjYXJkIGlzIGFmZmVjdGVkLg0KCVRo aXMgaXMgYmFzZWQgb24gdGhlIFNEIHNwZWMgdmVyc2lvbiAzLjAxLg0KCW1tY19zZGlvX2luaXRf dWhzX2NhcmQoKSBpcyB1bnRvdWNoZWQuIEkgZG9uJ3Qga25vdyB3aG8gYWRkIHRoZSBjb2RlIGlu dG8gbW1jX3NkaW9faW5pdF91aHNfY2FyZCgpIGFuZCB5b3UgbWF5IGhhdmUgYSBjaGVjayBvbiB5 b3VyIHNpZGUuDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBjYXJsby5jYWlv bmVAZ21haWwuY29tIFttYWlsdG86Y2FybG8uY2Fpb25lQGdtYWlsLmNvbV0gT24gQmVoYWxmIE9m IENhcmxvIENhaW9uZQ0KU2VudDogMjAxNeW5tDEy5pyIMTPml6UgNzo1Mg0KVG86IFVsZiBIYW5z c29uIDx1bGYuaGFuc3NvbkBsaW5hcm8ub3JnPg0KQ2M6IEJhcnJ5IFNvbmcgPDIxY25iYW9AZ21h aWwuY29tPjsgWWFuZywgWW9yayA8d2VpanVueUBxdGkucXVhbGNvbW0uY29tPjsgbGludXgtbW1j IDxsaW51eC1tbWNAdmdlci5rZXJuZWwub3JnPjsgREwtU0hBLVdvcmtHcm91cExpbnV4IDx3b3Jr Z3JvdXAubGludXhAY3NyLmNvbT47IGxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9y ZzsgU29uZywgQmFycnkgPGJhb2h1YXNAcXRpLnF1YWxjb21tLmNvbT4NClN1YmplY3Q6IFJlOiBb UEFUQ0ggMS8yXSBtbWM6IGNvcmU6IGVuYWJsZSBDTUQxOSB0dW5pbmcgZm9yIEREUjUwIG1vZGUN Cg0KT24gV2VkLCBTZXAgMjMsIDIwMTUgYXQgOToyNSBQTSwgVWxmIEhhbnNzb24gPHVsZi5oYW5z c29uQGxpbmFyby5vcmc+IHdyb3RlOg0KPiBbLi4uXQ0KPg0KPj4+DQo+Pj4gQWxzbywgSSB3YXMg Z2l2aW5nIHRoaXMgYSBzZWNvbmQgdGhvdWdodC4uLg0KPj4+DQo+Pj4gV2hhdCBoYXBwZW5zIHdp dGggdGhlIFNEIDMuMCBjYXJkIHdoZW4gaXQgKmRvZXNuJ3QqIHN1cHBvcnQgQ01EMTkuDQo+Pj4g V2lsbCBpdCBzdGlsbCBiZSBmdW5jdGlvbmFsIGFmdGVyd2FyZHMgb3Igd2lsbCBpdCBtb3ZlIGZy b20gDQo+Pj4gInRyYW5zZmVyIHN0YXRlIiB0byBhbm90aGVyIG5vdCBrbm93biBzdGF0ZT8NCj4+ DQo+PiB1ZmZlLCBpIGFtIG5vdCB0aGUgZXhwZXJ0IG9mIHRoaXMgaXNzdWUgYnV0IHdlaWp1biBp cy4gd2VpanVuIHRvbGQgDQo+PiB0aGF0IGZvciB0aGUgY2FyZHMgd2hvIGRvbid0IHN1cHBvcnQg Y21kMTksIGl0IHdpbGwgdGFrZSBpdCBhcyANCj4+IGlsbGVnYWwgY29tbWFuZCBhbmQgbm90IHJl c3BvbnNlIGl0LiBhbmQgaXQgYWxzbyBzZXRzIGlsbGVnYWwgY29tbWFuZCANCj4+IGVycm9yIGlu IHJlZ2lzdGVycy4NCj4+IGJ1dCB0aGUgY2FyZCBzdGF0dXMgaXMgc3RpbGwgZnVuY3Rpb25hbCBh bmQgZG9lc24ndCBjYXVzZSB1bmtub3duIHN0YXRlLg0KPj4NCj4+IHNvIG1heWJlIHdlIGNhbiBq dXN0IGlnbm9yZSB0aGUgZXJyIG9mIHR1bmluZywgYW5kIHVzZSB0aGUgc2Vjb25kIA0KPj4gcGF0 Y2ggd2l0aCB0aGUgcmVmaW5lIHlvdSBtZW50aW9uZWQ/DQo+DQo+IFN1cmUsIGxldCdzIGRvIHRo YXQuIFBsZWFzZSBzZW5kIGEgdjIgb2YgdGhlIHBhdGNoc2V0IHRoZW4uDQoNClJlcGx5aW5nIGhl cmUgc2luY2UgSSdtIG5vdCBzdWJzY3JpYmVkIHRvIGxpbnV4LW1tYy4NCg0KQ29tbWl0IDlmYWFj N2I5NSAibW1jOiBzZGhjaTogZW5hYmxlIHR1bmluZyBmb3IgRERSNTAiIGFjdHVhbGx5IGJyZWFr cyBXaUZpIG9uIFNESU8gYXQgbGVhc3QgZm9yIHRoZSBBU1VTIFgyMDVUQS4NCg0KSSB0cmllZCB0 byBpZ25vcmUgdGhlIGVycm9yIGZyb20gdGhlIHR1bmluZyBhbHNvIGZvciB0aGUgU0RJTyBjYXJk IHBhdGNoaW5nIG1tY19zZGlvX2luaXRfdWhzX2NhcmQoKSBpbiBkcml2ZXJzL21tYy9jb3JlL3Nk aW8uYyBidXQgSSBlbmQgdXAgd2l0aCB0aGUgZXJyb3IgIkNvbnRyb2xsZXIgbmV2ZXIgcmVsZWFz ZWQgaW5oaWJpdCBiaXQocykiLiBBdCBsZWFzdCBpbiBteSBjYXNlIENNRDE5IHNob3VsZG4ndCBi ZSBpc3N1ZWQgYXQgYWxsIHRvIGhhdmUgdGhlIFNESU8gY2FyZCB3b3JraW5nLg0KDQpBbnkgc3Vn Z2VzdGlvbiBob3cgdG8gYXBwcm9hY2ggdGhpcyBwcm9ibGVtPw0KDQotLQ0KQ2FybG8gQ2Fpb25l DQoNCg0KIFRvIHJlcG9ydCB0aGlzIGVtYWlsIGFzIHNwYW0gY2xpY2sgaHR0cHM6Ly93d3cubWFp bGNvbnRyb2wuY29tL3NyL3QhRU8wNU1xZlE3R1gyUFFQT212VWtXTTg1c0VLRDQralpRQ1FNV2NF RitLdHU3NUduSmpLSXVJTEMxM1lwRjZMZDkwcjBZWE1VYU5jIXJKaVZHYUpnPT0gLg0K -- 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 Mon, Dec 14, 2015 at 9:46 AM, Yang, York <weijuny@qti.qualcomm.com> wrote: > Hi, Carlo > In 9faac7b95, tuning for DDR50 is enabled in mmc_sd_init_uhs_card(), so only SD card is affected. > This is based on the SD spec version 3.01. > mmc_sdio_init_uhs_card() is untouched. I don't know who add the code into mmc_sdio_init_uhs_card() and you may have a check on your side. Hi Yang, sorry for not being clear. Modifying mmc_sdio_init_uhs_card() was only a test I did to check whether that could fix the issue, but it didn't. Right now the situation is: - In the latest master with commit 9faac7b95 WiFi is broken on the ASUS X205TA (bcm43341) - Reverting 9faac7b95 makes it working fine again My conclusion is that issuing the CMD19 is basically breaking WiFi on this platform.
Hi, Carlo I don't think so. You may check the 9faac7b95, there are two changes in sd.c and sdhc.c. Surely the change in sdhc.c will not affect ASUS X205TA (bcm43341) since it is a SDIO device. Actually the calling sequence is this: mmc_sd_init_uhs_card() calls sdhci_execute_tuning(). In 9faac7b95, I enabled tuning for DDR50 in these two functions. If your function calling sequence is mmc_sdio_init_uhs_card()->sdhci_execute_tuning(), the tuning will not carried out, since you didn't enable tuning for DDR50 in mmc_sdio_init_uhs_card(). Unless you call sdhci_execute_tuning directly, the tuning command may go through be sent out. -----Original Message----- From: carlo.caione@gmail.com [mailto:carlo.caione@gmail.com] On Behalf Of Carlo Caione Sent: 2015?12?14? 17:14 To: Yang, York <weijuny@qti.qualcomm.com> Cc: Carlo Caione <carlo@caione.org>; Ulf Hansson <ulf.hansson@linaro.org>; Barry Song <21cnbao@gmail.com>; linux-mmc <linux-mmc@vger.kernel.org>; DL-SHA-WorkGroupLinux <workgroup.linux@csr.com>; linux-arm-kernel@lists.infradead.org; Song, Barry <baohuas@qti.qualcomm.com> Subject: Re: [PATCH 1/2] mmc: core: enable CMD19 tuning for DDR50 mode On Mon, Dec 14, 2015 at 9:46 AM, Yang, York <weijuny@qti.qualcomm.com> wrote: > Hi, Carlo > In 9faac7b95, tuning for DDR50 is enabled in mmc_sd_init_uhs_card(), so only SD card is affected. > This is based on the SD spec version 3.01. > mmc_sdio_init_uhs_card() is untouched. I don't know who add the code into mmc_sdio_init_uhs_card() and you may have a check on your side. Hi Yang, sorry for not being clear. Modifying mmc_sdio_init_uhs_card() was only a test I did to check whether that could fix the issue, but it didn't. Right now the situation is: - In the latest master with commit 9faac7b95 WiFi is broken on the ASUS X205TA (bcm43341) - Reverting 9faac7b95 makes it working fine again My conclusion is that issuing the CMD19 is basically breaking WiFi on this platform. -- Carlo Caione
On Mon, Dec 14, 2015 at 10:24 AM, Yang, York <weijuny@qti.qualcomm.com> wrote: > Hi, Carlo > > I don't think so. You may check the 9faac7b95, there are two changes in sd.c and sdhc.c. > Surely the change in sdhc.c will not affect ASUS X205TA (bcm43341) since it is a SDIO device. > Actually the calling sequence is this: mmc_sd_init_uhs_card() calls sdhci_execute_tuning(). In 9faac7b95, I enabled tuning for DDR50 in these two functions. > If your function calling sequence is mmc_sdio_init_uhs_card()->sdhci_execute_tuning(), the tuning will not carried out, since you didn't enable tuning for DDR50 in mmc_sdio_init_uhs_card(). > Unless you call sdhci_execute_tuning directly, the tuning command may go through be sent out. Hi Yang, On the unmodified master what I see is: - mmc_sdio_init_uhs_card() is called - card->sw_caps.sd3_bus_mode is 0x14 - card->sw_caps.sd3_bus_mode & SD_MODE_UHS_SDR50 is actually verified (even though the tuning for DDR50 is not enabled) - mmc_execute_tuning() is actually called
Hi, On 12/14/2015 07:56 PM, Carlo Caione wrote: > On Mon, Dec 14, 2015 at 10:24 AM, Yang, York <weijuny@qti.qualcomm.com> wrote: >> Hi, Carlo >> >> I don't think so. You may check the 9faac7b95, there are two changes in sd.c and sdhc.c. >> Surely the change in sdhc.c will not affect ASUS X205TA (bcm43341) since it is a SDIO device. >> Actually the calling sequence is this: mmc_sd_init_uhs_card() calls sdhci_execute_tuning(). In 9faac7b95, I enabled tuning for DDR50 in these two functions. >> If your function calling sequence is mmc_sdio_init_uhs_card()->sdhci_execute_tuning(), the tuning will not carried out, since you didn't enable tuning for DDR50 in mmc_sdio_init_uhs_card(). >> Unless you call sdhci_execute_tuning directly, the tuning command may go through be sent out. > > Hi Yang, > > On the unmodified master what I see is: > > - mmc_sdio_init_uhs_card() is called > - card->sw_caps.sd3_bus_mode is 0x14 > - card->sw_caps.sd3_bus_mode & SD_MODE_UHS_SDR50 is actually verified > (even though the tuning for DDR50 is not enabled) > - mmc_execute_tuning() is actually called As i know, When mode is UHS_SDR50, tuning is not mandatory. Even though i need to check the spec file, i think that condition check is strange. Best Regards, Jaehoon Chung > -- 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, Carlo I don't understand. We can see that 9faac7b95 is all about DDR50. It should not affect the behavior of SDR50. Since the commit is very simple, you may remove the change one by one by yourself. And see if any difference occurs. Besides, please check if the card->sw_caps.sd3_bus_mode changes with or without DDR50 tuning enabled. -----Original Message----- From: carlo.caione@gmail.com [mailto:carlo.caione@gmail.com] On Behalf Of Carlo Caione Sent: 2015?12?14? 18:57 To: Yang, York <weijuny@qti.qualcomm.com> Cc: Carlo Caione <carlo@caione.org>; Ulf Hansson <ulf.hansson@linaro.org>; Barry Song <21cnbao@gmail.com>; linux-mmc <linux-mmc@vger.kernel.org>; DL-SHA-WorkGroupLinux <workgroup.linux@csr.com>; linux-arm-kernel@lists.infradead.org; Song, Barry <baohuas@qti.qualcomm.com> Subject: Re: [PATCH 1/2] mmc: core: enable CMD19 tuning for DDR50 mode On Mon, Dec 14, 2015 at 10:24 AM, Yang, York <weijuny@qti.qualcomm.com> wrote: > Hi, Carlo > > I don't think so. You may check the 9faac7b95, there are two changes in sd.c and sdhc.c. > Surely the change in sdhc.c will not affect ASUS X205TA (bcm43341) since it is a SDIO device. > Actually the calling sequence is this: mmc_sd_init_uhs_card() calls sdhci_execute_tuning(). In 9faac7b95, I enabled tuning for DDR50 in these two functions. > If your function calling sequence is mmc_sdio_init_uhs_card()->sdhci_execute_tuning(), the tuning will not carried out, since you didn't enable tuning for DDR50 in mmc_sdio_init_uhs_card(). > Unless you call sdhci_execute_tuning directly, the tuning command may go through be sent out. Hi Yang, On the unmodified master what I see is: - mmc_sdio_init_uhs_card() is called - card->sw_caps.sd3_bus_mode is 0x14 - card->sw_caps.sd3_bus_mode & SD_MODE_UHS_SDR50 is actually verified (even though the tuning for DDR50 is not enabled) - mmc_execute_tuning() is actually called -- Carlo Caione
On gio, dic 17, 2015 at 02:00:17 +0000, Yang, York wrote: > Hi, Carlo Hi Yang, > I don't understand. We can see that 9faac7b95 is all about > DDR50. It should not affect the behavior of SDR50. What I'm saying is that this commit is negatively affecting some DDR50 SDIO card as in my case. Ok, let's try to go through the code with and without 9faac7b95. Without 9faac7b95: - mmc_sdio_init_uhs_card() is called - sw_caps.sd3_bus_mode is 0x14 (SD_MODE_UHS_SDR50 | SD_MODE_UHS_DDR50) - card->sw_caps.sd3_bus_mode & SD_MODE_UHS_SDR50 is verified and mmc_execute_tuning() is called - sdhci_execute_tuning() is called - host->timing is MMC_TIMING_UHS_DDR50 - since there is no case for that we end up in 'default', goto out_unlock and CMD19 is never issued With 9faac7b95: - mmc_sdio_init_uhs_card() is called - sw_caps.sd3_bus_mode is 0x14 (SD_MODE_UHS_SDR50 | SD_MODE_UHS_DDR50) - card->sw_caps.sd3_bus_mode & SD_MODE_UHS_SDR50 is verified and mmc_execute_tuning() is called - sdhci_execute_tuning() is called - host->timing is MMC_TIMING_UHS_DDR50 - this time we have the the 'case MMC_TIMING_UHS_DDR50' and we break on that - CMD19 is issued and the card stops working > Since the commit is very simple, you may remove the change one by one > by yourself. And see if any difference occurs. As stated above the problem is in the new 'case MMC_TIMING_UHS_DDR50' we have in sdhci_execute_tuning() > Besides, please check > if the card->sw_caps.sd3_bus_mode changes with or without DDR50 tuning > enabled. It doesn't.
SGksIENhcmxvDQoJSSB1bmRlcnN0YW5kIG5vdy4gU28gdGhlIHByb2JsZW0gY29tZXMgb3V0IGJl Y2F1c2UgdGhlIHN3X2NhcHMuc2QzX2J1c19tb2RlIG1hcmtzIHRoZSBjYXBhYmlsaXR5IG9mIGEg Y2FyZCwgd2hpbGUgdGhlIGhvc3QtPnRpbWluZyBpcyB0aGUgbW9kZSB3ZSBhY3R1YWxseSBkbyB3 aXRoLg0KCUZvciB5b3VyIGNhc2UsIHlvdSBhcmUgcnVubmluZyBERFI1MCBtb2RlLCB3aGF0IGlm IHdlIGFkZCBtb3JlIGNvbmRpdGlvbiBmb3IgY29taW5nIGludG8gdGhlIHR1bmluZyBsb2dpYz8N CglzdHJ1Y3Qgc2RoY2lfaG9zdCAqaG9zdCA9IG1tY19wcml2KGNhcmQtPmhvc3QpOw0KCWlmICgh bW1jX2hvc3RfaXNfc3BpKGNhcmQtPmhvc3QpICYmIGNhcmQtPmhvc3QtPm9wcy0+ZXhlY3V0ZV90 dW5pbmcgJiYNCgkJCSgoY2FyZC0+c3dfY2Fwcy5zZDNfYnVzX21vZGUgJiBTRF9NT0RFX1VIU19T RFI1MCkgfHwNCgkJCSAoY2FyZC0+c3dfY2Fwcy5zZDNfYnVzX21vZGUgJiBTRF9NT0RFX1VIU19T RFIxMDQpKSYmDQoJCQkoKGhvc3QtPnRpbWluZyAmIE1NQ19USU1JTkdfVUhTX1NEUjUwKSB8fA0K CQkJCShob3N0LT50aW1pbmcgJiBNTUNfVElNSU5HX1VIU19TRFIxMDQpKSkgew0KCQltbWNfaG9z dF9jbGtfaG9sZChjYXJkLT5ob3N0KTsNCgkJZXJyID0gY2FyZC0+aG9zdC0+b3BzLT5leGVjdXRl X3R1bmluZyhjYXJkLT5ob3N0LA0KCQkJCQkJICAgICAgTU1DX1NFTkRfVFVOSU5HX0JMT0NLKTsN CgkJbW1jX2hvc3RfY2xrX3JlbGVhc2UoY2FyZC0+aG9zdCk7DQoJfQ0KDQotLS0tLU9yaWdpbmFs IE1lc3NhZ2UtLS0tLQ0KRnJvbTogY2FybG8uY2Fpb25lQGdtYWlsLmNvbSBbbWFpbHRvOmNhcmxv LmNhaW9uZUBnbWFpbC5jb21dIE9uIEJlaGFsZiBPZiBDYXJsbyBDYWlvbmUNClNlbnQ6IDIwMTXl ubQxMuaciDE45pelIDM6NTQNClRvOiBZYW5nLCBZb3JrIDx3ZWlqdW55QHF0aS5xdWFsY29tbS5j b20+DQpDYzogVWxmIEhhbnNzb24gPHVsZi5oYW5zc29uQGxpbmFyby5vcmc+OyBCYXJyeSBTb25n IDwyMWNuYmFvQGdtYWlsLmNvbT47IGxpbnV4LW1tYyA8bGludXgtbW1jQHZnZXIua2VybmVsLm9y Zz47IERMLVNIQS1Xb3JrR3JvdXBMaW51eCA8d29ya2dyb3VwLmxpbnV4QGNzci5jb20+OyBsaW51 eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmc7IFNvbmcsIEJhcnJ5IDxiYW9odWFzQHF0 aS5xdWFsY29tbS5jb20+DQpTdWJqZWN0OiBSZTogW1BBVENIIDEvMl0gbW1jOiBjb3JlOiBlbmFi bGUgQ01EMTkgdHVuaW5nIGZvciBERFI1MCBtb2RlDQoNCk9uIGdpbywgZGljIDE3LCAyMDE1IGF0 IDAyOjAwOjE3ICswMDAwLCBZYW5nLCBZb3JrIHdyb3RlOg0KPiBIaSwgQ2FybG8NCg0KSGkgWWFu ZywNCg0KPiBJIGRvbid0IHVuZGVyc3RhbmQuIFdlIGNhbiBzZWUgdGhhdCA5ZmFhYzdiOTUgaXMg YWxsIGFib3V0IEREUjUwLiBJdCANCj4gc2hvdWxkIG5vdCBhZmZlY3QgdGhlIGJlaGF2aW9yIG9m IFNEUjUwLg0KDQpXaGF0IEknbSBzYXlpbmcgaXMgdGhhdCB0aGlzIGNvbW1pdCBpcyBuZWdhdGl2 ZWx5IGFmZmVjdGluZyBzb21lIEREUjUwIFNESU8gY2FyZCBhcyBpbiBteSBjYXNlLg0KDQpPaywg bGV0J3MgdHJ5IHRvIGdvIHRocm91Z2ggdGhlIGNvZGUgd2l0aCBhbmQgd2l0aG91dCA5ZmFhYzdi OTUuDQoNCldpdGhvdXQgOWZhYWM3Yjk1Og0KLSBtbWNfc2Rpb19pbml0X3Voc19jYXJkKCkgaXMg Y2FsbGVkDQotIHN3X2NhcHMuc2QzX2J1c19tb2RlIGlzIDB4MTQgKFNEX01PREVfVUhTX1NEUjUw IHwgU0RfTU9ERV9VSFNfRERSNTApDQotIGNhcmQtPnN3X2NhcHMuc2QzX2J1c19tb2RlICYgU0Rf TU9ERV9VSFNfU0RSNTAgaXMgdmVyaWZpZWQgYW5kDQptbWNfZXhlY3V0ZV90dW5pbmcoKSBpcyBj YWxsZWQNCi0gc2RoY2lfZXhlY3V0ZV90dW5pbmcoKSBpcyBjYWxsZWQNCi0gaG9zdC0+dGltaW5n IGlzIE1NQ19USU1JTkdfVUhTX0REUjUwDQotIHNpbmNlIHRoZXJlIGlzIG5vIGNhc2UgZm9yIHRo YXQgd2UgZW5kIHVwIGluICdkZWZhdWx0JywgZ290byBvdXRfdW5sb2NrIGFuZCBDTUQxOSBpcyBu ZXZlciBpc3N1ZWQNCg0KV2l0aCA5ZmFhYzdiOTU6DQotIG1tY19zZGlvX2luaXRfdWhzX2NhcmQo KSBpcyBjYWxsZWQNCi0gc3dfY2Fwcy5zZDNfYnVzX21vZGUgaXMgMHgxNCAoU0RfTU9ERV9VSFNf U0RSNTAgfCBTRF9NT0RFX1VIU19ERFI1MCkNCi0gY2FyZC0+c3dfY2Fwcy5zZDNfYnVzX21vZGUg JiBTRF9NT0RFX1VIU19TRFI1MCBpcyB2ZXJpZmllZCBhbmQNCiAgbW1jX2V4ZWN1dGVfdHVuaW5n KCkgaXMgY2FsbGVkDQotIHNkaGNpX2V4ZWN1dGVfdHVuaW5nKCkgaXMgY2FsbGVkDQotIGhvc3Qt PnRpbWluZyBpcyBNTUNfVElNSU5HX1VIU19ERFI1MA0KLSB0aGlzIHRpbWUgd2UgaGF2ZSB0aGUg dGhlICdjYXNlIE1NQ19USU1JTkdfVUhTX0REUjUwJyBhbmQgd2UgYnJlYWsgb24gdGhhdA0KLSBD TUQxOSBpcyBpc3N1ZWQgYW5kIHRoZSBjYXJkIHN0b3BzIHdvcmtpbmcNCg0KPiBTaW5jZSB0aGUg Y29tbWl0IGlzIHZlcnkgc2ltcGxlLCB5b3UgbWF5IHJlbW92ZSB0aGUgY2hhbmdlIG9uZSBieSBv bmUgDQo+IGJ5IHlvdXJzZWxmLiBBbmQgc2VlIGlmIGFueSBkaWZmZXJlbmNlIG9jY3Vycy4NCg0K QXMgc3RhdGVkIGFib3ZlIHRoZSBwcm9ibGVtIGlzIGluIHRoZSBuZXcgJ2Nhc2UgTU1DX1RJTUlO R19VSFNfRERSNTAnDQp3ZSBoYXZlIGluIHNkaGNpX2V4ZWN1dGVfdHVuaW5nKCkNCg0KPiBCZXNp ZGVzLCBwbGVhc2UgY2hlY2sNCj4gaWYgdGhlIGNhcmQtPnN3X2NhcHMuc2QzX2J1c19tb2RlIGNo YW5nZXMgd2l0aCBvciB3aXRob3V0IEREUjUwIHR1bmluZyANCj4gZW5hYmxlZC4NCg0KSXQgZG9l c24ndC4NCg0KLS0NCkNhcmxvIENhaW9uZQ0K -- 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/sd.c b/drivers/mmc/core/sd.c index 4e7366a..d4df9f0 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -629,8 +629,23 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) */ if (!mmc_host_is_spi(card->host) && (card->sd_bus_speed == UHS_SDR50_BUS_SPEED || - card->sd_bus_speed == UHS_SDR104_BUS_SPEED)) + card->sd_bus_speed == UHS_DDR50_BUS_SPEED || + card->sd_bus_speed == UHS_SDR104_BUS_SPEED)) { err = mmc_execute_tuning(card); + + /* + * As SD Specifications Part1 Physical Layer Specification Version + * 3.01 says, CMD19 tuning is available for unlocked cards in transfer + * state of 1.8V signaling mode. The small difference between v3.00 + * and 3.01 spec means that CMD19 tuning is also available for DDR50 + * mode. + */ + if (err && (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) + pr_err("%s: ddr50 tuning failed\n", mmc_hostname(card->host)); + kfree(status); + return 0; + } + } out: kfree(status); diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 64b7fdb..382810d 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1915,6 +1915,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) break; case MMC_TIMING_UHS_SDR104: + case MMC_TIMING_UHS_DDR50: break;