RE: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
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 > Reviewed-by: Yasushi SHOJI > --- > > 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, 0x); > > 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, 0x); > + 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
Re: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
Hi, On Wed, May 17, 2017 at 4:47 PM, Yoshihiro Shimoda 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, 0x); >> >> 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, 0x); >> + 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
Re: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
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 signature.asc Description: PGP signature
RE: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
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
RE: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
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 > 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, 0x); > >> > >> 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, 0x); > >> + 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
Re: [PATCH] mmc: tmio: use SDIO master interrupt bit only when allowed
On 9 December 2016 at 17:51, Wolfram Sang 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 > Reviewed-by: Yasushi SHOJI 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, 0x); > > 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, 0x); > + 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, 0x); > + > 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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html