* [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
@ 2016-12-09 16:51 Wolfram Sang
2016-12-29 19:02 ` Ulf Hansson
2017-05-17 7:47 ` Yoshihiro Shimoda
0 siblings, 2 replies; 7+ messages in thread
From: Wolfram Sang @ 2016-12-09 16:51 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-renesas-soc, Wolfram Sang, Yasushi SHOJI
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);
}
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
2016-12-09 16:51 [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed Wolfram Sang
@ 2016-12-29 19:02 ` Ulf Hansson
2017-05-17 7:47 ` Yoshihiro Shimoda
1 sibling, 0 replies; 7+ messages in thread
From: Ulf Hansson @ 2016-12-29 19:02 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-mmc@vger.kernel.org, Linux-Renesas, Yasushi SHOJI
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
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
2016-12-09 16:51 [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed Wolfram Sang
2016-12-29 19:02 ` Ulf Hansson
@ 2017-05-17 7:47 ` Yoshihiro Shimoda
2017-05-18 5:09 ` Yasushi SHOJI
2017-05-18 6:17 ` Wolfram Sang
1 sibling, 2 replies; 7+ messages in thread
From: Yoshihiro Shimoda @ 2017-05-17 7:47 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-renesas-soc@vger.kernel.org, Yasushi SHOJI,
linux-mmc@vger.kernel.org
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
2017-05-17 7:47 ` Yoshihiro Shimoda
@ 2017-05-18 5:09 ` Yasushi SHOJI
2017-05-18 6:36 ` Yoshihiro Shimoda
2017-05-18 6:17 ` Wolfram Sang
1 sibling, 1 reply; 7+ messages in thread
From: Yasushi SHOJI @ 2017-05-18 5:09 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Wolfram Sang, linux-renesas-soc@vger.kernel.org,
linux-mmc@vger.kernel.org
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,
--
yashi
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
2017-05-18 5:09 ` Yasushi SHOJI
@ 2017-05-18 6:36 ` Yoshihiro Shimoda
0 siblings, 0 replies; 7+ messages in thread
From: Yoshihiro Shimoda @ 2017-05-18 6:36 UTC (permalink / raw)
To: Yasushi SHOJI
Cc: Wolfram Sang, linux-renesas-soc@vger.kernel.org,
linux-mmc@vger.kernel.org
Hi Shoji-san,
> From: Yasushi SHOJI
> Sent: Thursday, May 18, 2017 2:10 PM
>
> 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.
Thank you for the reply. I got it.
Wolfram-san will try to fix the issue.
Best regards,
Yoshihiro Shimoda
> Thanks,
> --
> yashi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
2017-05-17 7:47 ` Yoshihiro Shimoda
2017-05-18 5:09 ` Yasushi SHOJI
@ 2017-05-18 6:17 ` Wolfram Sang
2017-05-18 6:35 ` Yoshihiro Shimoda
1 sibling, 1 reply; 7+ messages in thread
From: Wolfram Sang @ 2017-05-18 6:17 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Wolfram Sang, linux-renesas-soc@vger.kernel.org, Yasushi SHOJI,
linux-mmc@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 340 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
2017-05-18 6:17 ` Wolfram Sang
@ 2017-05-18 6:35 ` Yoshihiro Shimoda
0 siblings, 0 replies; 7+ messages in thread
From: Yoshihiro Shimoda @ 2017-05-18 6:35 UTC (permalink / raw)
To: Wolfram Sang
Cc: Wolfram Sang, linux-renesas-soc@vger.kernel.org, Yasushi SHOJI,
linux-mmc@vger.kernel.org
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-05-18 6:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-09 16:51 [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed Wolfram Sang
2016-12-29 19:02 ` Ulf Hansson
2017-05-17 7:47 ` Yoshihiro Shimoda
2017-05-18 5:09 ` Yasushi SHOJI
2017-05-18 6:36 ` Yoshihiro Shimoda
2017-05-18 6:17 ` Wolfram Sang
2017-05-18 6:35 ` Yoshihiro Shimoda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox