[PATCH v2 0/4] mmc: renesas_sdhi: upport improvements from BSP

[PATCH v2 0/4] mmc: renesas_sdhi: upport improvements from BSP

[Wolfram Sang]

This series upports two BSP improvements in a refactored way. BSP patch
de9461ece1fa ("mmc: renesas_sdhi: scc_ctl is always set in after R-Car
Gen3") made sure we have a pointer to always reset SCC. I refactored it
to include Gen2 as well, so we can optimize the following code a bit.
This is patch 1. BSP patch 15d7abbfc3f0 ("mmc: renesas_sdhi: reset
calibration register") added a new quirk for devices which always should
disable HS400 adjustment. This was only applicable for SoCs which either
had bad_taps or needed a calibration table. So, I simply used this logic
directly instead of a quirk. This is patch 2.

Two cleanups come afterwards. To increase readability, I finally did
something I wanted to do for some time now. Having a helper to check for
SDHI quirks. This is patch 3. And while working on this, I discovered a
new macro from the MMC core which we can make use of. This is patch 4.

The series has been tested on a Salvator-XS (R-Car M3-N) and a Lager
(R-Car H2) board. The patches are based on mmc/next as of today as they
depend on d901e2ba566 ("mmc: renesas_sdhi: take DMA end interrupts into
account"). A branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/sdhi/for-6.2

Let me know what you think!

Happy hacking,

   Wolfram

Changes since V1:
* added another improvement to the series
* reordered cleanups last so backporting fixes is easier


Wolfram Sang (4):
  mmc: renesas_sdhi: alway populate SCC pointer
  mmc: renesas_sdhi: better reset from HS400 mode
  mmc: renesas_sdhi: add helper to access quirks
  mmc: renesas_sdhi: use new convenience macro from MMC core

 drivers/mmc/host/renesas_sdhi.h               |  2 ++
 drivers/mmc/host/renesas_sdhi_core.c          | 29 ++++++++++---------
 drivers/mmc/host/renesas_sdhi_internal_dmac.c |  8 ++---
 3 files changed, 21 insertions(+), 18 deletions(-)

-- 
2.30.2

[PATCH v2 1/4] mmc: renesas_sdhi: alway populate SCC pointer

[Wolfram Sang]

We need the SCC pointer to reset the device, so populate it even when we
don't need it for tuning.

Fixes: 45bffc371fef ("mmc: renesas_sdhi: only reset SCC when its pointer is populated")
Signed-off-by: Takeshi Saito <takeshi.saito.xv@renesas.com>
Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/renesas_sdhi_core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index b93ab11f112a..4372197dc2a9 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -1067,11 +1067,14 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (ver >= SDHI_VER_GEN3_SD)
 		host->get_timeout_cycles = renesas_sdhi_gen3_get_cycles;
 
+	/* Check for SCC so we can reset it if needed */
+	if (of_data && of_data->scc_offset && ver >= SDHI_VER_GEN2_SDR104)
+		priv->scc_ctl = host->ctl + of_data->scc_offset;
+
 	/* Enable tuning iff we have an SCC and a supported mode */
-	if (of_data && of_data->scc_offset &&
-	    (host->mmc->caps & MMC_CAP_UHS_SDR104 ||
-	     host->mmc->caps2 & (MMC_CAP2_HS200_1_8V_SDR |
-				 MMC_CAP2_HS400_1_8V))) {
+	if (priv->scc_ctl && (host->mmc->caps & MMC_CAP_UHS_SDR104 ||
+	    host->mmc->caps2 & (MMC_CAP2_HS200_1_8V_SDR |
+				MMC_CAP2_HS400_1_8V))) {
 		const struct renesas_sdhi_scc *taps = of_data->taps;
 		bool use_4tap = quirks && quirks->hs400_4taps;
 		bool hit = false;
@@ -1091,7 +1094,6 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 		if (!hit)
 			dev_warn(&host->pdev->dev, "Unknown clock rate for tuning\n");
 
-		priv->scc_ctl = host->ctl + of_data->scc_offset;
 		host->check_retune = renesas_sdhi_check_scc_error;
 		host->ops.execute_tuning = renesas_sdhi_execute_tuning;
 		host->ops.prepare_hs400_tuning = renesas_sdhi_prepare_hs400_tuning;
-- 
2.30.2

[PATCH v2 2/4] mmc: renesas_sdhi: better reset from HS400 mode

[Wolfram Sang]

Up to now, HS400 adjustment mode was only disabled on soft reset when a
calibration table was in use. It is safer, though, to disable it as soon
as the instance has an adjustment related quirk set, i.e. bad taps or a
calibration table.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/renesas_sdhi_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 4372197dc2a9..d2521cf1411d 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -546,7 +546,7 @@ static void renesas_sdhi_reset_hs400_mode(struct tmio_mmc_host *host,
 			 SH_MOBILE_SDHI_SCC_TMPPORT2_HS400OSEL) &
 			sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2));
 
-	if (priv->adjust_hs400_calib_table)
+	if (priv->quirks && (priv->quirks->hs400_calib_table || priv->quirks->hs400_bad_taps))
 		renesas_sdhi_adjust_hs400_mode_disable(host);
 
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
-- 
2.30.2

[PATCH v2 3/4] mmc: renesas_sdhi: add helper to access quirks

[Wolfram Sang]

Add a macro to check for a quirk because it a) ensures that the check
for non-empty 'quirks' struct is not forgotten and b) is easier to read.
Convert existing quirk access as well.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/renesas_sdhi.h               |  2 ++
 drivers/mmc/host/renesas_sdhi_core.c          | 18 +++++++++---------
 drivers/mmc/host/renesas_sdhi_internal_dmac.c |  8 ++++----
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index 8f96457c9739..ea2a85174a09 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -38,6 +38,8 @@ struct renesas_sdhi_of_data {
 
 #define SDHI_CALIB_TABLE_MAX 32
 
+#define sdhi_has_quirk(p, q) ((p)->quirks && (p)->quirks->q)
+
 struct renesas_sdhi_quirks {
 	bool hs400_disabled;
 	bool hs400_4taps;
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index d2521cf1411d..ad8f79fe01b2 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -141,7 +141,7 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
 
 	if (priv->clkh) {
 		/* HS400 with 4TAP needs different clock settings */
-		bool use_4tap = priv->quirks && priv->quirks->hs400_4taps;
+		bool use_4tap = sdhi_has_quirk(priv, hs400_4taps);
 		bool need_slow_clkh = host->mmc->ios.timing == MMC_TIMING_MMC_HS400;
 		clkh_shift = use_4tap && need_slow_clkh ? 1 : 2;
 		ref_clk = priv->clkh;
@@ -383,7 +383,7 @@ static void renesas_sdhi_hs400_complete(struct mmc_host *mmc)
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 	struct renesas_sdhi *priv = host_to_priv(host);
 	u32 bad_taps = priv->quirks ? priv->quirks->hs400_bad_taps : 0;
-	bool use_4tap = priv->quirks && priv->quirks->hs400_4taps;
+	bool use_4tap = sdhi_has_quirk(priv, hs400_4taps);
 
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
 		sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
@@ -395,7 +395,7 @@ static void renesas_sdhi_hs400_complete(struct mmc_host *mmc)
 	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DT2FF,
 		       priv->scc_tappos_hs400);
 
-	if (priv->quirks && priv->quirks->manual_tap_correction)
+	if (sdhi_has_quirk(priv, manual_tap_correction))
 		sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
 			       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
 			       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
@@ -546,7 +546,7 @@ static void renesas_sdhi_reset_hs400_mode(struct tmio_mmc_host *host,
 			 SH_MOBILE_SDHI_SCC_TMPPORT2_HS400OSEL) &
 			sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2));
 
-	if (priv->quirks && (priv->quirks->hs400_calib_table || priv->quirks->hs400_bad_taps))
+	if (sdhi_has_quirk(priv, hs400_calib_table) || sdhi_has_quirk(priv, hs400_bad_taps))
 		renesas_sdhi_adjust_hs400_mode_disable(host);
 
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
@@ -732,7 +732,7 @@ static bool renesas_sdhi_manual_correction(struct tmio_mmc_host *host, bool use_
 	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
 
 	/* Change TAP position according to correction status */
-	if (priv->quirks && priv->quirks->manual_tap_correction &&
+	if (sdhi_has_quirk(priv, manual_tap_correction) &&
 	    host->mmc->ios.timing == MMC_TIMING_MMC_HS400) {
 		u32 bad_taps = priv->quirks ? priv->quirks->hs400_bad_taps : 0;
 		/*
@@ -796,7 +796,7 @@ static bool renesas_sdhi_check_scc_error(struct tmio_mmc_host *host,
 					 struct mmc_request *mrq)
 {
 	struct renesas_sdhi *priv = host_to_priv(host);
-	bool use_4tap = priv->quirks && priv->quirks->hs400_4taps;
+	bool use_4tap = sdhi_has_quirk(priv, hs400_4taps);
 	bool ret = false;
 
 	/*
@@ -990,7 +990,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	host->multi_io_quirk	= renesas_sdhi_multi_io_quirk;
 	host->dma_ops		= dma_ops;
 
-	if (quirks && quirks->hs400_disabled)
+	if (sdhi_has_quirk(priv, hs400_disabled))
 		host->mmc->caps2 &= ~(MMC_CAP2_HS400 | MMC_CAP2_HS400_ES);
 
 	/* For some SoC, we disable internal WP. GPIO may override this */
@@ -1055,7 +1055,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (ver == SDHI_VER_GEN2_SDR50)
 		mmc_data->flags &= ~TMIO_MMC_HAVE_CBSY;
 
-	if (ver == SDHI_VER_GEN3_SDMMC && quirks && quirks->hs400_calib_table) {
+	if (ver == SDHI_VER_GEN3_SDMMC && sdhi_has_quirk(priv, hs400_calib_table)) {
 		host->fixup_request = renesas_sdhi_fixup_request;
 		priv->adjust_hs400_calib_table = *(
 			res->start == SDHI_GEN3_MMC0_ADDR ?
@@ -1076,7 +1076,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	    host->mmc->caps2 & (MMC_CAP2_HS200_1_8V_SDR |
 				MMC_CAP2_HS400_1_8V))) {
 		const struct renesas_sdhi_scc *taps = of_data->taps;
-		bool use_4tap = quirks && quirks->hs400_4taps;
+		bool use_4tap = sdhi_has_quirk(priv, hs400_4taps);
 		bool hit = false;
 
 		for (i = 0; i < of_data->taps_num; i++) {
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index f6d1e04a627f..29f562115c66 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -285,7 +285,7 @@ renesas_sdhi_internal_dmac_enable_dma(struct tmio_mmc_host *host, bool enable)
 {
 	struct renesas_sdhi *priv = host_to_priv(host);
 	u32 dma_irqs = INFO1_DTRANEND0 |
-			(priv->quirks && priv->quirks->old_info1_layout ?
+			(sdhi_has_quirk(priv, old_info1_layout) ?
 			INFO1_DTRANEND1_OLD : INFO1_DTRANEND1);
 
 	if (!host->chan_tx || !host->chan_rx)
@@ -318,7 +318,7 @@ static bool renesas_sdhi_internal_dmac_dma_irq(struct tmio_mmc_host *host)
 	struct renesas_sdhi_dma *dma_priv = &priv->dma_priv;
 
 	u32 dma_irqs = INFO1_DTRANEND0 |
-			(priv->quirks && priv->quirks->old_info1_layout ?
+			(sdhi_has_quirk(priv, old_info1_layout) ?
 			INFO1_DTRANEND1_OLD : INFO1_DTRANEND1);
 	u32 status = readl(host->ctl + DM_CM_INFO1);
 
@@ -396,7 +396,7 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host,
 	struct scatterlist *sg = host->sg_ptr;
 	u32 dtran_mode = DTRAN_MODE_BUS_WIDTH;
 
-	if (!(priv->quirks && priv->quirks->fixed_addr_mode))
+	if (!sdhi_has_quirk(priv, fixed_addr_mode))
 		dtran_mode |= DTRAN_MODE_ADDR_MODE;
 
 	if (!renesas_sdhi_internal_dmac_map(host, data, COOKIE_MAPPED))
@@ -404,7 +404,7 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host,
 
 	if (data->flags & MMC_DATA_READ) {
 		dtran_mode |= DTRAN_MODE_CH_NUM_CH1;
-		if (priv->quirks && priv->quirks->dma_one_rx_only &&
+		if (sdhi_has_quirk(priv, dma_one_rx_only) &&
 		    test_and_set_bit(SDHI_INTERNAL_DMAC_RX_IN_USE, &global_flags))
 			goto force_pio_with_unmap;
 	} else {
-- 
2.30.2

[PATCH v2 4/4] mmc: renesas_sdhi: use new convenience macro from MMC core

[Wolfram Sang]

Makes the code more readable.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/renesas_sdhi_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index ad8f79fe01b2..345934e4f59e 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -1073,8 +1073,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 
 	/* Enable tuning iff we have an SCC and a supported mode */
 	if (priv->scc_ctl && (host->mmc->caps & MMC_CAP_UHS_SDR104 ||
-	    host->mmc->caps2 & (MMC_CAP2_HS200_1_8V_SDR |
-				MMC_CAP2_HS400_1_8V))) {
+	    host->mmc->caps2 & MMC_CAP2_HSX00_1_8V)) {
 		const struct renesas_sdhi_scc *taps = of_data->taps;
 		bool use_4tap = sdhi_has_quirk(priv, hs400_4taps);
 		bool hit = false;
-- 
2.30.2

RE: [PATCH v2 0/4] mmc: renesas_sdhi: upport improvements from BSP

[Yoshihiro Shimoda]

Hi Wolfram-san,

> From: Wolfram Sang, Sent: Sunday, November 20, 2022 8:35 PM
> 
> This series upports two BSP improvements in a refactored way. BSP patch
> de9461ece1fa ("mmc: renesas_sdhi: scc_ctl is always set in after R-Car
> Gen3") made sure we have a pointer to always reset SCC. I refactored it
> to include Gen2 as well, so we can optimize the following code a bit.
> This is patch 1. BSP patch 15d7abbfc3f0 ("mmc: renesas_sdhi: reset
> calibration register") added a new quirk for devices which always should
> disable HS400 adjustment. This was only applicable for SoCs which either
> had bad_taps or needed a calibration table. So, I simply used this logic
> directly instead of a quirk. This is patch 2.
> 
> Two cleanups come afterwards. To increase readability, I finally did
> something I wanted to do for some time now. Having a helper to check for
> SDHI quirks. This is patch 3. And while working on this, I discovered a
> new macro from the MMC core which we can make use of. This is patch 4.
> 
> The series has been tested on a Salvator-XS (R-Car M3-N) and a Lager
> (R-Car H2) board. The patches are based on mmc/next as of today as they
> depend on d901e2ba566 ("mmc: renesas_sdhi: take DMA end interrupts into
> account"). A branch can be found here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/sdhi/for-6.2
> 
> Let me know what you think!

Thank you for the patch!

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

I also tested on Salvator-XS (R-Car H3 ES3.0). So,

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

Best regards,
Yoshihiro Shimoda

> Happy hacking,
> 
>    Wolfram
> 
> Changes since V1:
> * added another improvement to the series
> * reordered cleanups last so backporting fixes is easier
> 
> 
> Wolfram Sang (4):
>   mmc: renesas_sdhi: alway populate SCC pointer
>   mmc: renesas_sdhi: better reset from HS400 mode
>   mmc: renesas_sdhi: add helper to access quirks
>   mmc: renesas_sdhi: use new convenience macro from MMC core
> 
>  drivers/mmc/host/renesas_sdhi.h               |  2 ++
>  drivers/mmc/host/renesas_sdhi_core.c          | 29 ++++++++++---------
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  8 ++---
>  3 files changed, 21 insertions(+), 18 deletions(-)
> 
> --
> 2.30.2

Re: [PATCH v2 3/4] mmc: renesas_sdhi: add helper to access quirks

[Geert Uytterhoeven]

On Sun, Nov 20, 2022 at 12:52 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Add a macro to check for a quirk because it a) ensures that the check
> for non-empty 'quirks' struct is not forgotten and b) is easier to read.
> Convert existing quirk access as well.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 4/4] mmc: renesas_sdhi: use new convenience macro from MMC core

[Geert Uytterhoeven]

On Sun, Nov 20, 2022 at 12:51 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Makes the code more readable.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 0/4] mmc: renesas_sdhi: upport improvements from BSP

[Ulf Hansson]

On Sun, 20 Nov 2022 at 12:35, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> This series upports two BSP improvements in a refactored way. BSP patch
> de9461ece1fa ("mmc: renesas_sdhi: scc_ctl is always set in after R-Car
> Gen3") made sure we have a pointer to always reset SCC. I refactored it
> to include Gen2 as well, so we can optimize the following code a bit.
> This is patch 1. BSP patch 15d7abbfc3f0 ("mmc: renesas_sdhi: reset
> calibration register") added a new quirk for devices which always should
> disable HS400 adjustment. This was only applicable for SoCs which either
> had bad_taps or needed a calibration table. So, I simply used this logic
> directly instead of a quirk. This is patch 2.
>
> Two cleanups come afterwards. To increase readability, I finally did
> something I wanted to do for some time now. Having a helper to check for
> SDHI quirks. This is patch 3. And while working on this, I discovered a
> new macro from the MMC core which we can make use of. This is patch 4.
>
> The series has been tested on a Salvator-XS (R-Car M3-N) and a Lager
> (R-Car H2) board. The patches are based on mmc/next as of today as they
> depend on d901e2ba566 ("mmc: renesas_sdhi: take DMA end interrupts into
> account"). A branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/sdhi/for-6.2
>
> Let me know what you think!
>
> Happy hacking,
>
>    Wolfram
>
> Changes since V1:
> * added another improvement to the series
> * reordered cleanups last so backporting fixes is easier
>
>
> Wolfram Sang (4):
>   mmc: renesas_sdhi: alway populate SCC pointer
>   mmc: renesas_sdhi: better reset from HS400 mode
>   mmc: renesas_sdhi: add helper to access quirks
>   mmc: renesas_sdhi: use new convenience macro from MMC core
>
>  drivers/mmc/host/renesas_sdhi.h               |  2 ++
>  drivers/mmc/host/renesas_sdhi_core.c          | 29 ++++++++++---------
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  8 ++---
>  3 files changed, 21 insertions(+), 18 deletions(-)
>

Applied for next, thanks!

Kind regards
Uffe