All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC/RFT] mmc: tmio: always restore irq register
@ 2021-04-07 15:31 Wolfram Sang
  2021-04-09  2:41 ` Yoshihiro Shimoda
  0 siblings, 1 reply; 3+ messages in thread
From: Wolfram Sang @ 2021-04-07 15:31 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Yoshihiro Shimoda, Wolfram Sang

Currently, only SDHI on R-Car Gen2+ reinitializes the irq register
during reset but it should be done on all instances. We can move it from
the SDHI driver to the TMIO core, because we now have the
'sd_irq_mask_all' variable which carries the proper value to use. That
also means we can remove the initialization from tmio_mmc_probe()
because it calls tmio_mmc_reset(), too. We only move that
tmio_mmc_reset() call there a little to ensure 'sd_irq_mask_all' is
properly set.

Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Shimoda-san, I think this is the implementation of what we discussed. It
passes my tests on a Renesas H3 ES2.0. I'd be happy if you or the BSP
team could run their additional checks with this patch. Thank you and
kind regards!

 drivers/mmc/host/renesas_sdhi_core.c |  2 --
 drivers/mmc/host/tmio_mmc_core.c     | 11 ++++++-----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index d36181b6f687..635bf31a6735 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -588,8 +588,6 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host)
 		renesas_sdhi_scc_reset(host, priv);
 	}
 
-	sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK, TMIO_MASK_ALL_RCAR2);
-
 	if (sd_ctrl_read16(host, CTL_VERSION) >= SDHI_VER_GEN3_SD) {
 		val = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT);
 		val |= CARD_OPT_EXTOP;
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 0c474d78b186..bcd26056d47a 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -192,6 +192,9 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
 	if (host->reset)
 		host->reset(host);
 
+	host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(host, CTL_IRQ_MASK);
+	tmio_mmc_disable_mmc_irqs(host, host->sdcard_irq_mask_all);
+
 	tmio_mmc_set_bus_width(host, host->mmc->ios.bus_width);
 
 	if (host->pdata->flags & TMIO_MMC_SDIO_IRQ) {
@@ -1176,13 +1179,11 @@ 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;
 
-	_host->set_clock(_host, 0);
-	tmio_mmc_reset(_host);
-
-	_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, CTL_IRQ_MASK);
 	if (!_host->sdcard_irq_mask_all)
 		_host->sdcard_irq_mask_all = TMIO_MASK_ALL;
-	tmio_mmc_disable_mmc_irqs(_host, _host->sdcard_irq_mask_all);
+
+	_host->set_clock(_host, 0);
+	tmio_mmc_reset(_host);
 
 	if (_host->native_hotplug)
 		tmio_mmc_enable_mmc_irqs(_host,
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [PATCH RFC/RFT] mmc: tmio: always restore irq register
  2021-04-07 15:31 [PATCH RFC/RFT] mmc: tmio: always restore irq register Wolfram Sang
@ 2021-04-09  2:41 ` Yoshihiro Shimoda
  2021-04-13  8:07   ` Wolfram Sang
  0 siblings, 1 reply; 3+ messages in thread
From: Yoshihiro Shimoda @ 2021-04-09  2:41 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-renesas-soc, linux-mmc

Hi Wolfram-san,

Thank you for the patch!

> From: Wolfram Sang, Sent: Thursday, April 8, 2021 12:31 AM
> 
> Currently, only SDHI on R-Car Gen2+ reinitializes the irq register
> during reset but it should be done on all instances. We can move it from
> the SDHI driver to the TMIO core, because we now have the
> 'sd_irq_mask_all' variable which carries the proper value to use. That
> also means we can remove the initialization from tmio_mmc_probe()
> because it calls tmio_mmc_reset(), too. We only move that
> tmio_mmc_reset() call there a little to ensure 'sd_irq_mask_all' is
> properly set.

Yes, this is my expectation. However...

> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Shimoda-san, I think this is the implementation of what we discussed. It
> passes my tests on a Renesas H3 ES2.0. I'd be happy if you or the BSP
> team could run their additional checks with this patch. Thank you and
> kind regards!
> 
>  drivers/mmc/host/renesas_sdhi_core.c |  2 --
>  drivers/mmc/host/tmio_mmc_core.c     | 11 ++++++-----
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index d36181b6f687..635bf31a6735 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -588,8 +588,6 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host)
>  		renesas_sdhi_scc_reset(host, priv);
>  	}
> 
> -	sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK, TMIO_MASK_ALL_RCAR2);
> -
>  	if (sd_ctrl_read16(host, CTL_VERSION) >= SDHI_VER_GEN3_SD) {
>  		val = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT);
>  		val |= CARD_OPT_EXTOP;
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 0c474d78b186..bcd26056d47a 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -192,6 +192,9 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
>  	if (host->reset)
>  		host->reset(host);
> 
> +	host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(host, CTL_IRQ_MASK);
> +	tmio_mmc_disable_mmc_irqs(host, host->sdcard_irq_mask_all);
> +

This code could not resolve my concern. This code still read
CTL_IRQ_MASK at first. So, if the register value is incorrect
(when "host->reset" didn't exist), the sdcard_irq_mask value
will be not expected value.

So, I'm thinking we should write CTL_IRQ_MASK with sdcard_irq_mask_all
by using sd_ctrl_write32_as_16_and_16() instead of using
tmio_mmc_disable_mmc_irqs() at first.

Best regards,
Yoshihiro Shimoda


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH RFC/RFT] mmc: tmio: always restore irq register
  2021-04-09  2:41 ` Yoshihiro Shimoda
@ 2021-04-13  8:07   ` Wolfram Sang
  0 siblings, 0 replies; 3+ messages in thread
From: Wolfram Sang @ 2021-04-13  8:07 UTC (permalink / raw)
  To: Yoshihiro Shimoda; +Cc: linux-renesas-soc, linux-mmc

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

Hi Shimoda-san,

> This code could not resolve my concern. This code still read
> CTL_IRQ_MASK at first. So, if the register value is incorrect
> (when "host->reset" didn't exist), the sdcard_irq_mask value
> will be not expected value.

Geez, I forgot (again) that SD_RESET will not reset the irq register :(
I will initialize the register and the cache variable in reset() and
will resend in a few minutes after the tests.

Thanks!

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-13  8:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 15:31 [PATCH RFC/RFT] mmc: tmio: always restore irq register Wolfram Sang
2021-04-09  2:41 ` Yoshihiro Shimoda
2021-04-13  8:07   ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.