diff mbox

mmc: tmio: use SDIO master interrupt bit only when allowed

Message ID 20161209165141.26599-1-wsa+renesas@sang-engineering.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wolfram Sang Dec. 9, 2016, 4:51 p.m. UTC 
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(-)

Comments

Ulf Hansson Dec. 29, 2016, 7:02 p.m. UTC | #1 
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
Yoshihiro Shimoda May 17, 2017, 7:47 a.m. UTC | #2 
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
Yasushi SHOJI May 18, 2017, 5:09 a.m. UTC | #3 
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,
Wolfram Sang May 18, 2017, 6:17 a.m. UTC | #4 
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
Yoshihiro Shimoda May 18, 2017, 6:35 a.m. UTC | #5 
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
Yoshihiro Shimoda May 18, 2017, 6:36 a.m. UTC | #6 
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 mbox

Patch

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);