[PATCH 0/3] tmio: set max_busy_timeout

[PATCH 0/3] tmio: set max_busy_timeout

[Wolfram Sang]

This is a follow-up to the series "mmc: tmio: honor busy timeouts
properly" which I sent out a few days ago. One of the patches there
needs more discussion, so I regrouped the series with another one, and
this is the first outcome. It is solely about max_busy_timeout:

Patch 1 is from the previous series (with the comment from Shimoda-san
addressed) and sets max_busy_timeout with what TMIO always did. Patch 2
introduces a hook and a default fallback for extended timeout ranges.
Patch 3 uses the hook for the extended range of R-Car Gen3 SDHIs.

It has been tested that the applied values make sense. I have not
measured if the MMC core really sends R1 instead of R1B when the desired
timeout value is exceeded. All on a Salvator-XS with R-Car M3N.

The patches are based on mmc/next as of today. The branch is here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/sdhi/extop

Looking forward to comments!

Happy hacking,

   Wolfram


Wolfram Sang (3):
  mmc: tmio: set max_busy_timeout
  mmc: tmio: add hook for custom busy_wait calculation
  mmc: renesas_sdhi: populate hook for longer busy_wait

 drivers/mmc/host/renesas_sdhi_core.c | 23 +++++++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h          |  5 +++++
 drivers/mmc/host/tmio_mmc_core.c     | 22 ++++++++++++++++++++++
 drivers/mmc/host/uniphier-sd.c       |  1 +
 include/linux/mfd/tmio.h             |  7 ++++++-
 5 files changed, 57 insertions(+), 1 deletion(-)

-- 
2.28.0

[PATCH 1/3] mmc: tmio: set max_busy_timeout

[Wolfram Sang]

Set max_busy_timeouts for variants known to support the TOPxx bits in
the SD_OPTION register. The timeout mechanism was running in the
background but not yet properly handled in the driver. So, let the MMC
core know when to not use R1B to avoid unhandled timeouts.

My datasheets for older variants (tmio_mmc.c) suggest that they support
it, too. However, actual bit descriptions are lacking, so I chose an
opt-in approach.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/renesas_sdhi_core.c |  3 +++
 drivers/mmc/host/tmio_mmc.h          |  2 ++
 drivers/mmc/host/tmio_mmc_core.c     | 15 +++++++++++++++
 drivers/mmc/host/uniphier-sd.c       |  1 +
 include/linux/mfd/tmio.h             |  7 ++++++-
 5 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index bb937411c2ec..153767054c05 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -1041,6 +1041,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	/* All SDHI have SDIO status bits which must be 1 */
 	mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS;
 
+	/* All SDHI support HW busy detection */
+	mmc_data->flags |= TMIO_MMC_USE_BUSY_TIMEOUT;
+
 	dev_pm_domain_start(&pdev->dev);
 
 	ret = renesas_sdhi_clk_enable(host);
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 7ff41185896a..819198af17f4 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -80,6 +80,8 @@
 #define	CLK_CTL_SCLKEN		BIT(8)
 
 /* Definitions for values the CTL_SD_MEM_CARD_OPT register can take */
+#define CARD_OPT_TOP_MASK	0xf0
+#define CARD_OPT_TOP_SHIFT	4
 #define CARD_OPT_WIDTH8		BIT(13)
 #define CARD_OPT_WIDTH		BIT(15)
 
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 7f4a28125010..da9a6243f146 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -887,6 +887,18 @@ static void tmio_mmc_set_bus_width(struct tmio_mmc_host *host,
 	sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, reg);
 }
 
+static void tmio_mmc_max_busy_timeout(struct tmio_mmc_host *host)
+{
+	u16 val = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT);
+	unsigned int clk_rate = host->mmc->actual_clock ?: host->mmc->f_max;
+	unsigned int cycles;
+
+	val = (val & CARD_OPT_TOP_MASK) >> CARD_OPT_TOP_SHIFT;
+	cycles = 1 << (13 + val);
+
+	host->mmc->max_busy_timeout = cycles / (clk_rate / MSEC_PER_SEC);
+}
+
 /* Set MMC clock / power.
  * Note: This controller uses a simple divider scheme therefore it cannot
  * run a MMC card at full speed (20MHz). The max clock is 24MHz on SD, but as
@@ -945,6 +957,9 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		break;
 	}
 
+	if (host->pdata->flags & TMIO_MMC_USE_BUSY_TIMEOUT)
+		tmio_mmc_max_busy_timeout(host);
+
 	/* Let things settle. delay taken from winCE driver */
 	usleep_range(140, 200);
 	if (PTR_ERR(host->mrq) == -EINTR)
diff --git a/drivers/mmc/host/uniphier-sd.c b/drivers/mmc/host/uniphier-sd.c
index 3092466a99ab..a6cd16771d4e 100644
--- a/drivers/mmc/host/uniphier-sd.c
+++ b/drivers/mmc/host/uniphier-sd.c
@@ -586,6 +586,7 @@ static int uniphier_sd_probe(struct platform_device *pdev)
 
 	tmio_data = &priv->tmio_data;
 	tmio_data->flags |= TMIO_MMC_32BIT_DATA_PORT;
+	tmio_data->flags |= TMIO_MMC_USE_BUSY_TIMEOUT;
 
 	host = tmio_mmc_host_alloc(pdev, tmio_data);
 	if (IS_ERR(host))
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 8ba042430d8e..27264fe4b3b9 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -55,7 +55,12 @@
  */
 #define TMIO_MMC_HAS_IDLE_WAIT		BIT(4)
 
-/* BIT(5) is unused */
+/*
+ * Use the busy timeout feature. Probably all TMIO versions support it. Yet,
+ * we don't have documentation for old variants, so we enable only known good
+ * variants with this flag. Can be removed once all variants are known good.
+ */
+#define TMIO_MMC_USE_BUSY_TIMEOUT	BIT(5)
 
 /*
  * Some controllers have CMD12 automatically
-- 
2.28.0

[PATCH 2/3] mmc: tmio: add hook for custom busy_wait calculation

[Wolfram Sang]

From: Wolfram Sang <wsa@kernel.org>

Newer SDHI variants can 'wait while busy' longer than the generic TMIO.
Provide a hook to get the maximum cycle count to wait for. If the hook
is not populated, fall back to a generic version which works well with
all older TMIO/SDHI variants.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/tmio_mmc.h      |  1 +
 drivers/mmc/host/tmio_mmc_core.c | 17 ++++++++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 819198af17f4..f60559bc413a 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -181,6 +181,7 @@ struct tmio_mmc_host {
 	void (*reset)(struct tmio_mmc_host *host);
 	bool (*check_retune)(struct tmio_mmc_host *host);
 	void (*fixup_request)(struct tmio_mmc_host *host, struct mmc_request *mrq);
+	unsigned int (*get_timeout_cycles)(struct tmio_mmc_host *host);
 
 	void (*prepare_hs400_tuning)(struct tmio_mmc_host *host);
 	void (*hs400_downgrade)(struct tmio_mmc_host *host);
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index da9a6243f146..f0711f4c1e5b 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -887,16 +887,20 @@ static void tmio_mmc_set_bus_width(struct tmio_mmc_host *host,
 	sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, reg);
 }
 
-static void tmio_mmc_max_busy_timeout(struct tmio_mmc_host *host)
+static unsigned int tmio_mmc_get_timeout_cycles(struct tmio_mmc_host *host)
 {
 	u16 val = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT);
-	unsigned int clk_rate = host->mmc->actual_clock ?: host->mmc->f_max;
-	unsigned int cycles;
 
 	val = (val & CARD_OPT_TOP_MASK) >> CARD_OPT_TOP_SHIFT;
-	cycles = 1 << (13 + val);
+	return 1 << (13 + val);
+}
+
+static void tmio_mmc_max_busy_timeout(struct tmio_mmc_host *host)
+{
+	unsigned int clk_rate = host->mmc->actual_clock ?: host->mmc->f_max;
 
-	host->mmc->max_busy_timeout = cycles / (clk_rate / MSEC_PER_SEC);
+	host->mmc->max_busy_timeout = host->get_timeout_cycles(host) /
+				      (clk_rate / MSEC_PER_SEC);
 }
 
 /* Set MMC clock / power.
@@ -1116,6 +1120,9 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
 	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
 		_host->write16_hook = NULL;
 
+	if (pdata->flags & TMIO_MMC_USE_BUSY_TIMEOUT && !_host->get_timeout_cycles)
+		_host->get_timeout_cycles = tmio_mmc_get_timeout_cycles;
+
 	_host->set_pwr = pdata->set_pwr;
 
 	ret = tmio_mmc_init_ocr(_host);
-- 
2.28.0

[PATCH 3/3] mmc: renesas_sdhi: populate hook for longer busy_wait

[Wolfram Sang]

From: Wolfram Sang <wsa@kernel.org>

Make use of the EXTOP bit in R-Car Gen3 SoCs to have a twice as large
busy wait duration.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/renesas_sdhi_core.c | 20 ++++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h          |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 153767054c05..38f028e70633 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -561,6 +561,7 @@ static int renesas_sdhi_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_io
 static void renesas_sdhi_reset(struct tmio_mmc_host *host)
 {
 	struct renesas_sdhi *priv = host_to_priv(host);
+	u16 val;
 
 	if (priv->scc_ctl) {
 		renesas_sdhi_disable_scc(host->mmc);
@@ -573,6 +574,21 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host)
 	}
 
 	sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK, TMIO_MASK_INIT_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;
+		sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, val);
+	}
+}
+
+static unsigned int renesas_sdhi_gen3_get_cycles(struct tmio_mmc_host *host)
+{
+	u16 num, val = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT);
+
+	num = (val & CARD_OPT_TOP_MASK) >> CARD_OPT_TOP_SHIFT;
+	return 1 << ((val & CARD_OPT_EXTOP ? 14 : 13) + num);
+
 }
 
 #define SH_MOBILE_SDHI_MIN_TAP_ROW 3
@@ -1067,6 +1083,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 			quirks->hs400_calib_table + 1);
 	}
 
+	/* these have an EXTOP bit */
+	if (ver >= SDHI_VER_GEN3_SD)
+		host->get_timeout_cycles = renesas_sdhi_gen3_get_cycles;
+
 	/* 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 ||
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index f60559bc413a..784fa6ed5843 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -82,7 +82,9 @@
 /* Definitions for values the CTL_SD_MEM_CARD_OPT register can take */
 #define CARD_OPT_TOP_MASK	0xf0
 #define CARD_OPT_TOP_SHIFT	4
+#define CARD_OPT_EXTOP		BIT(9) /* first appeared on R-Car Gen3 SDHI */
 #define CARD_OPT_WIDTH8		BIT(13)
+#define CARD_OPT_ALWAYS1	BIT(14)
 #define CARD_OPT_WIDTH		BIT(15)
 
 /* Definitions for values the CTL_SDIO_STATUS register can take */
-- 
2.28.0

Re: [PATCH 3/3] mmc: renesas_sdhi: populate hook for longer busy_wait

[Wolfram Sang]

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

On Wed, Nov 25, 2020 at 10:30:01PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa@kernel.org>

Eeks, this should have been:

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Please let me know if I should resend.


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

Re: [PATCH 2/3] mmc: tmio: add hook for custom busy_wait calculation

[Wolfram Sang]

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


> From: Wolfram Sang <wsa@kernel.org>

Same comment as for patch 3 :(


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

RE: [PATCH 0/3] tmio: set max_busy_timeout

[Yoshihiro Shimoda]

Hi Wolfram-san,

> From: Wolfram Sang, Sent: Thursday, November 26, 2020 6:30 AM
> 
> This is a follow-up to the series "mmc: tmio: honor busy timeouts
> properly" which I sent out a few days ago. One of the patches there
> needs more discussion, so I regrouped the series with another one, and
> this is the first outcome. It is solely about max_busy_timeout:
> 
> Patch 1 is from the previous series (with the comment from Shimoda-san
> addressed) and sets max_busy_timeout with what TMIO always did. Patch 2
> introduces a hook and a default fallback for extended timeout ranges.
> Patch 3 uses the hook for the extended range of R-Car Gen3 SDHIs.
> 
> It has been tested that the applied values make sense. I have not
> measured if the MMC core really sends R1 instead of R1B when the desired
> timeout value is exceeded. All on a Salvator-XS with R-Car M3N.

Thank you for the patch! I tested on Salvator-XS with R-Car H3 and
I checked the MMC core use R1 instead of R1B by using an additional
printk on mmc_do_erase().

So,

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

Best regards,
Yoshihiro Shimoda

Re: [PATCH 0/3] tmio: set max_busy_timeout

[Ulf Hansson]

On Wed, 25 Nov 2020 at 22:30, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> This is a follow-up to the series "mmc: tmio: honor busy timeouts
> properly" which I sent out a few days ago. One of the patches there
> needs more discussion, so I regrouped the series with another one, and
> this is the first outcome. It is solely about max_busy_timeout:
>
> Patch 1 is from the previous series (with the comment from Shimoda-san
> addressed) and sets max_busy_timeout with what TMIO always did. Patch 2
> introduces a hook and a default fallback for extended timeout ranges.
> Patch 3 uses the hook for the extended range of R-Car Gen3 SDHIs.
>
> It has been tested that the applied values make sense. I have not
> measured if the MMC core really sends R1 instead of R1B when the desired
> timeout value is exceeded. All on a Salvator-XS with R-Car M3N.
>
> The patches are based on mmc/next as of today. The branch is here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/sdhi/extop
>
> Looking forward to comments!
>
> Happy hacking,
>
>    Wolfram
>
>
> Wolfram Sang (3):
>   mmc: tmio: set max_busy_timeout
>   mmc: tmio: add hook for custom busy_wait calculation
>   mmc: renesas_sdhi: populate hook for longer busy_wait
>
>  drivers/mmc/host/renesas_sdhi_core.c | 23 +++++++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h          |  5 +++++
>  drivers/mmc/host/tmio_mmc_core.c     | 22 ++++++++++++++++++++++
>  drivers/mmc/host/uniphier-sd.c       |  1 +
>  include/linux/mfd/tmio.h             |  7 ++++++-
>  5 files changed, 57 insertions(+), 1 deletion(-)
>
> --
> 2.28.0
>

Applied for next, by amending "from" to "Wolfram Sang
<wsa+renesas@sang-engineering.com>", thanks!

Kind regards
Uffe