| Message ID | 20161209165141.26599-1-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 9 December 2016 at 17:51, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > The master bit to enable SDIO interrupts can only be accessed if > SCLKDIVEN bit allows that. However, the core uses the SDIO enable > callback at times when SCLKDIVEN forbids the change. This leads to > "timeout waiting for SD bus idle" messages. > > We now activate the master bit in probe once if SDIO is supported. IRQ > en-/disabling will be done now by the individual IRQ enablement bits > only. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > Reviewed-by: Yasushi SHOJI <yashi@atmark-techno.com> Thanks, applied for next! Kind regards Uffe > --- > > No change from RFC, only Rev-by added (which included testing). > > drivers/mmc/host/tmio_mmc_pio.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c > index 7ef24ec620b542..526e52719f81b9 100644 > --- a/drivers/mmc/host/tmio_mmc_pio.c > +++ b/drivers/mmc/host/tmio_mmc_pio.c > @@ -140,12 +140,10 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) > > host->sdio_irq_mask = TMIO_SDIO_MASK_ALL & > ~TMIO_SDIO_STAT_IOIRQ; > - sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001); > sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); > } else if (!enable && host->sdio_irq_enabled) { > host->sdio_irq_mask = TMIO_SDIO_MASK_ALL; > sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); > - sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000); > > host->sdio_irq_enabled = false; > pm_runtime_mark_last_busy(mmc_dev(mmc)); > @@ -1203,7 +1201,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host, > if (pdata->flags & TMIO_MMC_SDIO_IRQ) { > _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL; > sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, _host->sdio_irq_mask); > - sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0000); > + sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0001); > } > > spin_lock_init(&_host->lock); > @@ -1251,6 +1249,9 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host) > struct platform_device *pdev = host->pdev; > struct mmc_host *mmc = host->mmc; > > + if (host->pdata->flags & TMIO_MMC_SDIO_IRQ) > + sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000); > + > if (!host->native_hotplug) > pm_runtime_get_sync(&pdev->dev); > > -- > 2.10.2 > > -- > 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 Wolfram-san, > -----Original Message----- > From: Wolfram Sang > Sent: Saturday, December 10, 2016 1:52 AM > > The master bit to enable SDIO interrupts can only be accessed if > SCLKDIVEN bit allows that. However, the core uses the SDIO enable > callback at times when SCLKDIVEN forbids the change. This leads to > "timeout waiting for SD bus idle" messages. > > We now activate the master bit in probe once if SDIO is supported. IRQ > en-/disabling will be done now by the individual IRQ enablement bits > only. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > Reviewed-by: Yasushi SHOJI <yashi@atmark-techno.com> > --- > > No change from RFC, only Rev-by added (which included testing). > > drivers/mmc/host/tmio_mmc_pio.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c > index 7ef24ec620b542..526e52719f81b9 100644 > --- a/drivers/mmc/host/tmio_mmc_pio.c > +++ b/drivers/mmc/host/tmio_mmc_pio.c > @@ -140,12 +140,10 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) > > host->sdio_irq_mask = TMIO_SDIO_MASK_ALL & > ~TMIO_SDIO_STAT_IOIRQ; > - sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001); > sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); > } else if (!enable && host->sdio_irq_enabled) { > host->sdio_irq_mask = TMIO_SDIO_MASK_ALL; > sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); > - sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000); > > host->sdio_irq_enabled = false; > pm_runtime_mark_last_busy(mmc_dev(mmc)); > @@ -1203,7 +1201,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host, > if (pdata->flags & TMIO_MMC_SDIO_IRQ) { > _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL; > sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, _host->sdio_irq_mask); > - sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0000); > + sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0001); > } I'm afraid but I would like to confirm about this 6 month ago's patch :) This patch enables CTL_TRANSACTION_CTL to 0x0001 in tmio_mmc_host_probe(). But, I have a concern we have to disable/enable the register in suspend/resume() because registers setting is possible to be cleared after resume. What do you think? Best regards, Yoshihiro Shimoda -- 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, On Wed, May 17, 2017 at 4:47 PM, Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: >> >> drivers/mmc/host/tmio_mmc_pio.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c >> index 7ef24ec620b542..526e52719f81b9 100644 >> --- a/drivers/mmc/host/tmio_mmc_pio.c >> +++ b/drivers/mmc/host/tmio_mmc_pio.c >> @@ -140,12 +140,10 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) >> >> host->sdio_irq_mask = TMIO_SDIO_MASK_ALL & >> ~TMIO_SDIO_STAT_IOIRQ; >> - sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001); >> sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); >> } else if (!enable && host->sdio_irq_enabled) { >> host->sdio_irq_mask = TMIO_SDIO_MASK_ALL; >> sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); >> - sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000); >> >> host->sdio_irq_enabled = false; >> pm_runtime_mark_last_busy(mmc_dev(mmc)); >> @@ -1203,7 +1201,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host, >> if (pdata->flags & TMIO_MMC_SDIO_IRQ) { >> _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL; >> sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, _host->sdio_irq_mask); >> - sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0000); >> + sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0001); >> } > > I'm afraid but I would like to confirm about this 6 month ago's patch :) > > This patch enables CTL_TRANSACTION_CTL to 0x0001 in tmio_mmc_host_probe(). > But, I have a concern we have to disable/enable the register in suspend/resume() > because registers setting is possible to be cleared after resume. > What do you think? That's a good catch. I suppose we need that. I didn't check it for suspend/resume at thattime. My bad. Thanks,
Shimoda-san, > This patch enables CTL_TRANSACTION_CTL to 0x0001 in tmio_mmc_host_probe(). > But, I have a concern we have to disable/enable the register in suspend/resume() > because registers setting is possible to be cleared after resume. > What do you think? This is very likely true. I'll cook up a patch today. Thanks, Wolfram
Hi Wolfram-san, > From: Wolfram Sang > Sent: Thursday, May 18, 2017 3:18 PM > > Shimoda-san, > > > This patch enables CTL_TRANSACTION_CTL to 0x0001 in tmio_mmc_host_probe(). > > But, I have a concern we have to disable/enable the register in suspend/resume() > > because registers setting is possible to be cleared after resume. > > What do you think? > > This is very likely true. I'll cook up a patch today. Thank you very much! Best regards, Yoshihiro Shimoda > Thanks, > > Wolfram -- 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
SGkgU2hvamktc2FuLA0KDQo+IEZyb206IFlhc3VzaGkgU0hPSkkNCj4gU2VudDogVGh1cnNkYXks IE1heSAxOCwgMjAxNyAyOjEwIFBNDQo+IA0KPiBIaSwNCj4gDQo+IE9uIFdlZCwgTWF5IDE3LCAy MDE3IGF0IDQ6NDcgUE0sIFlvc2hpaGlybyBTaGltb2RhDQo+IDx5b3NoaWhpcm8uc2hpbW9kYS51 aEByZW5lc2FzLmNvbT4gd3JvdGU6DQo+ID4+DQo+ID4+ICBkcml2ZXJzL21tYy9ob3N0L3RtaW9f bW1jX3Bpby5jIHwgNyArKysrLS0tDQo+ID4+ICAxIGZpbGUgY2hhbmdlZCwgNCBpbnNlcnRpb25z KCspLCAzIGRlbGV0aW9ucygtKQ0KPiA+Pg0KPiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9tbWMv aG9zdC90bWlvX21tY19waW8uYyBiL2RyaXZlcnMvbW1jL2hvc3QvdG1pb19tbWNfcGlvLmMNCj4g Pj4gaW5kZXggN2VmMjRlYzYyMGI1NDIuLjUyNmU1MjcxOWY4MWI5IDEwMDY0NA0KPiA+PiAtLS0g YS9kcml2ZXJzL21tYy9ob3N0L3RtaW9fbW1jX3Bpby5jDQo+ID4+ICsrKyBiL2RyaXZlcnMvbW1j L2hvc3QvdG1pb19tbWNfcGlvLmMNCj4gPj4gQEAgLTE0MCwxMiArMTQwLDEwIEBAIHN0YXRpYyB2 b2lkIHRtaW9fbW1jX2VuYWJsZV9zZGlvX2lycShzdHJ1Y3QgbW1jX2hvc3QgKm1tYywgaW50IGVu YWJsZSkNCj4gPj4NCj4gPj4gICAgICAgICAgICAgICBob3N0LT5zZGlvX2lycV9tYXNrID0gVE1J T19TRElPX01BU0tfQUxMICYNCj4gPj4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICB+VE1JT19TRElPX1NUQVRfSU9JUlE7DQo+ID4+IC0gICAgICAgICAgICAgc2RfY3RybF93 cml0ZTE2KGhvc3QsIENUTF9UUkFOU0FDVElPTl9DVEwsIDB4MDAwMSk7DQo+ID4+ICAgICAgICAg ICAgICAgc2RfY3RybF93cml0ZTE2KGhvc3QsIENUTF9TRElPX0lSUV9NQVNLLCBob3N0LT5zZGlv X2lycV9tYXNrKTsNCj4gPj4gICAgICAgfSBlbHNlIGlmICghZW5hYmxlICYmIGhvc3QtPnNkaW9f aXJxX2VuYWJsZWQpIHsNCj4gPj4gICAgICAgICAgICAgICBob3N0LT5zZGlvX2lycV9tYXNrID0g VE1JT19TRElPX01BU0tfQUxMOw0KPiA+PiAgICAgICAgICAgICAgIHNkX2N0cmxfd3JpdGUxNiho b3N0LCBDVExfU0RJT19JUlFfTUFTSywgaG9zdC0+c2Rpb19pcnFfbWFzayk7DQo+ID4+IC0gICAg ICAgICAgICAgc2RfY3RybF93cml0ZTE2KGhvc3QsIENUTF9UUkFOU0FDVElPTl9DVEwsIDB4MDAw MCk7DQo+ID4+DQo+ID4+ICAgICAgICAgICAgICAgaG9zdC0+c2Rpb19pcnFfZW5hYmxlZCA9IGZh bHNlOw0KPiA+PiAgICAgICAgICAgICAgIHBtX3J1bnRpbWVfbWFya19sYXN0X2J1c3kobW1jX2Rl dihtbWMpKTsNCj4gPj4gQEAgLTEyMDMsNyArMTIwMSw3IEBAIGludCB0bWlvX21tY19ob3N0X3By b2JlKHN0cnVjdCB0bWlvX21tY19ob3N0ICpfaG9zdCwNCj4gPj4gICAgICAgaWYgKHBkYXRhLT5m bGFncyAmIFRNSU9fTU1DX1NESU9fSVJRKSB7DQo+ID4+ICAgICAgICAgICAgICAgX2hvc3QtPnNk aW9faXJxX21hc2sgPSBUTUlPX1NESU9fTUFTS19BTEw7DQo+ID4+ICAgICAgICAgICAgICAgc2Rf Y3RybF93cml0ZTE2KF9ob3N0LCBDVExfU0RJT19JUlFfTUFTSywgX2hvc3QtPnNkaW9faXJxX21h c2spOw0KPiA+PiAtICAgICAgICAgICAgIHNkX2N0cmxfd3JpdGUxNihfaG9zdCwgQ1RMX1RSQU5T QUNUSU9OX0NUTCwgMHgwMDAwKTsNCj4gPj4gKyAgICAgICAgICAgICBzZF9jdHJsX3dyaXRlMTYo X2hvc3QsIENUTF9UUkFOU0FDVElPTl9DVEwsIDB4MDAwMSk7DQo+ID4+ICAgICAgIH0NCj4gPg0K PiA+IEknbSBhZnJhaWQgYnV0IEkgd291bGQgbGlrZSB0byBjb25maXJtIGFib3V0IHRoaXMgNiBt b250aCBhZ28ncyBwYXRjaCA6KQ0KPiA+DQo+ID4gVGhpcyBwYXRjaCBlbmFibGVzIENUTF9UUkFO U0FDVElPTl9DVEwgdG8gMHgwMDAxIGluIHRtaW9fbW1jX2hvc3RfcHJvYmUoKS4NCj4gPiBCdXQs IEkgaGF2ZSBhIGNvbmNlcm4gd2UgaGF2ZSB0byBkaXNhYmxlL2VuYWJsZSB0aGUgcmVnaXN0ZXIg aW4gc3VzcGVuZC9yZXN1bWUoKQ0KPiA+IGJlY2F1c2UgcmVnaXN0ZXJzIHNldHRpbmcgaXMgcG9z c2libGUgdG8gYmUgY2xlYXJlZCBhZnRlciByZXN1bWUuDQo+ID4gV2hhdCBkbyB5b3UgdGhpbms/ DQo+IA0KPiBUaGF0J3MgYSBnb29kIGNhdGNoLg0KPiBJIHN1cHBvc2Ugd2UgbmVlZCB0aGF0LiAg SSBkaWRuJ3QgY2hlY2sgaXQgZm9yIHN1c3BlbmQvcmVzdW1lIGF0DQo+IHRoYXR0aW1lLiBNeSBi YWQuDQoNClRoYW5rIHlvdSBmb3IgdGhlIHJlcGx5LiBJIGdvdCBpdC4NCldvbGZyYW0tc2FuIHdp bGwgdHJ5IHRvIGZpeCB0aGUgaXNzdWUuDQoNCkJlc3QgcmVnYXJkcywNCllvc2hpaGlybyBTaGlt b2RhDQoNCj4gVGhhbmtzLA0KPiAtLQ0KPiAgICAgICAgICAgICB5YXNoaQ0K -- 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/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c index 7ef24ec620b542..526e52719f81b9 100644 --- a/drivers/mmc/host/tmio_mmc_pio.c +++ b/drivers/mmc/host/tmio_mmc_pio.c @@ -140,12 +140,10 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable) host->sdio_irq_mask = TMIO_SDIO_MASK_ALL & ~TMIO_SDIO_STAT_IOIRQ; - sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001); sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); } else if (!enable && host->sdio_irq_enabled) { host->sdio_irq_mask = TMIO_SDIO_MASK_ALL; sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); - sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000); host->sdio_irq_enabled = false; pm_runtime_mark_last_busy(mmc_dev(mmc)); @@ -1203,7 +1201,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host, if (pdata->flags & TMIO_MMC_SDIO_IRQ) { _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL; sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, _host->sdio_irq_mask); - sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0000); + sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0001); } spin_lock_init(&_host->lock); @@ -1251,6 +1249,9 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host) struct platform_device *pdev = host->pdev; struct mmc_host *mmc = host->mmc; + if (host->pdata->flags & TMIO_MMC_SDIO_IRQ) + sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000); + if (!host->native_hotplug) pm_runtime_get_sync(&pdev->dev);