diff mbox series

[v2] mmc: tmio: always restore irq register

Message ID 20210413083137.11171-1-wsa+renesas@sang-engineering.com (mailing list archive)
State New, archived
Headers show
Series [v2] mmc: tmio: always restore irq register | expand

Commit Message

Wolfram Sang April 13, 2021, 8:31 a.m. UTC
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.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes since v1:

* use direct register write to initialize irq register instead of
  masking bits. Also initialize the cache variable directly.

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

Comments

Niklas Söderlund April 13, 2021, 3:37 p.m. UTC | #1
Hi Wolfram,

Thanks for your work.

On 2021-04-13 10:31:37 +0200, Wolfram Sang wrote:
> 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.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

I like patches like this.

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> 
> Changes since v1:
> 
> * use direct register write to initialize irq register instead of
>   masking bits. Also initialize the cache variable directly.
> 
>  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..7dfc26f48c18 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);
>  
> +	sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK, host->sdcard_irq_mask_all);
> +	host->sdcard_irq_mask = 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
>
Yoshihiro Shimoda April 15, 2021, 11:54 a.m. UTC | #2
Hi Wolfram-san,

> From: Wolfram Sang, Sent: Tuesday, April 13, 2021 5:32 PM
> 
> 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.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

I tested on my environment (R-Car H3 ES3.0) and I didn't observe any regression.
So,

Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda
Ulf Hansson April 15, 2021, 12:02 p.m. UTC | #3
On Tue, 13 Apr 2021 at 10:31, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> 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.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
>
> Changes since v1:
>
> * use direct register write to initialize irq register instead of
>   masking bits. Also initialize the cache variable directly.
>
>  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..7dfc26f48c18 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);
>
> +       sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK, host->sdcard_irq_mask_all);
> +       host->sdcard_irq_mask = 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
>
Wolfram Sang April 15, 2021, 7:58 p.m. UTC | #4
> Applied for next, thanks!

Thanks for being so super-quick, Ulf!
diff mbox series

Patch

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..7dfc26f48c18 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);
 
+	sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK, host->sdcard_irq_mask_all);
+	host->sdcard_irq_mask = 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,