[PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Simon Horman]

Hi,

this patch-set provides SDHI driver support for eMMC HS400.

Based on mmc/next

Dependencies for applying these patches: none

Dependencies to test eMMC HS400:
* [PATCH] clk: renesas: rcar-gen3: Fix SD divider setting
* [PATCH v2] arm64: dts: salvator-common: Enable HS400 of SDHI2

To assist testing and review this patch and the above mentioned
dependencies, which are necessary and sufficient to enable HS400 on H3 /
Salvator-X, M3-W 1.0 / Salvator-X and H3 ES2.0 Salvator-XS are available
at:

https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/hs400-v3

Changes since v2:
* Consolidate disable_scc and reset_hs400_mode into reset_hs400_tuning
  callback
* Reuse renesas_sdhi_reset_hs400_mode() in renesas_sdhi_hw_reset()
* Factor out renesas_sdhi_reset_scc()

Changes since v1:
* Use updated code from BSP v3.6.0
* Ironed out dependencies, eMMC HS400 is now working on
  H3 / Salvator-X, M3-W 1.0 / Salvator-X and H3 ES2.0 Salvator-XS.

Masaharu Hayakawa (2):
  mmc: tmio: add eMMC HS400 mode support
  mmc: renesas_sdhi: add eMMC HS400 mode support

 drivers/mmc/host/renesas_sdhi_core.c | 133 +++++++++++++++++++++++++++++------
 drivers/mmc/host/tmio_mmc.h          |   5 ++
 drivers/mmc/host/tmio_mmc_core.c     |  24 ++++++-
 3 files changed, 138 insertions(+), 24 deletions(-)

-- 
2.11.0

[PATCH v3 1/2] mmc: tmio: add eMMC HS400 mode support

[Simon Horman]

From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>

This patch adds processing for selecting HS400 mode.

Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v3 [Simon Horman]
* Consolidate disable_scc and reset_hs400_mode into reset_hs400_tuning
  callback

v2 [Simon Horman]
* Updated to new version of BSP patch from BSP v3.6.0
* Dropped 4 and 8 tap differentiation as all SoCs currently supported
  by the driver in upstream use 4 taps for HS400.
* Minor cleanup

v1 [Simon Horman]
* Combined patches by Ai Kyuse and Masaharu Hayakawa.
* Rebase
* Minor clean-up

v0 [Masaharu Hayakawa]
---
 drivers/mmc/host/tmio_mmc.h      |  5 +++++
 drivers/mmc/host/tmio_mmc_core.c | 24 ++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index e7d651352dc9..bac620109a90 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -46,6 +46,7 @@
 #define CTL_DMA_ENABLE 0xd8
 #define CTL_RESET_SD 0xe0
 #define CTL_VERSION 0xe2
+#define CTL_SDIF_MODE 0xe6
 #define CTL_SDIO_REGS 0x100
 #define CTL_CLK_AND_WAIT_CTL 0x138
 #define CTL_RESET_SDIO 0x1e0
@@ -191,6 +192,10 @@ struct tmio_mmc_host {
 	/* Tuning values: 1 for success, 0 for failure */
 	DECLARE_BITMAP(taps, BITS_PER_BYTE * sizeof(long));
 	unsigned int tap_num;
+	unsigned long tap_set;
+
+	void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios);
+	void (*reset_hs400_tuning)(struct mmc_host *mmc);
 
 	const struct tmio_mmc_dma_ops *dma_ops;
 };
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 33494241245a..889b4d43085b 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -199,6 +199,13 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
 		tmio_mmc_clk_stop(host);
 		return;
 	}
+	/*
+	 * Both HS400 and HS200/SD104 set 200MHz, but some devices need to
+	 * set 400MHz to distinguish the CPG settings in HS400.
+	 */
+	if (host->mmc->ios.timing == MMC_TIMING_MMC_HS400 &&
+	    new_clock == 200000000)
+		new_clock = 400000000;
 
 	if (host->clk_update)
 		clock = host->clk_update(host, new_clock) / 512;
@@ -209,8 +216,13 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
 		clock <<= 1;
 
 	/* 1/1 clock is option */
-	if ((host->pdata->flags & TMIO_MMC_CLK_ACTUAL) && ((clk >> 22) & 0x1))
-		clk |= 0xff;
+	if ((host->pdata->flags & TMIO_MMC_CLK_ACTUAL) &&
+	    ((clk >> 22) & 0x1)) {
+		if (!(host->mmc->ios.timing == MMC_TIMING_MMC_HS400))
+			clk |= 0xff;
+		else
+			clk &= ~0xff;
+	}
 
 	if (host->set_clk_div)
 		host->set_clk_div(host->pdev, (clk >> 22) & 1);
@@ -1001,6 +1013,9 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	struct device *dev = &host->pdev->dev;
 	unsigned long flags;
 
+	if (host->reset_hs400_tuning)
+		host->reset_hs400_tuning(mmc);
+
 	mutex_lock(&host->ios_lock);
 
 	spin_lock_irqsave(&host->lock, flags);
@@ -1051,6 +1066,11 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			"%s.%d: IOS interrupted: clk %u, mode %u",
 			current->comm, task_pid_nr(current),
 			ios->clock, ios->power_mode);
+
+	/* HS400 Register setting */
+	if (ios->timing == MMC_TIMING_MMC_HS400 && host->prepare_hs400_tuning)
+		host->prepare_hs400_tuning(mmc, ios);
+
 	host->mrq = NULL;
 
 	host->clk_cache = ios->clock;
-- 
2.11.0

[PATCH v3 2/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Simon Horman]

From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>

This patch adds processing for selecting HS400 mode.

Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v3 [Simon Horman]
* Consolidate disable_scc and reset_hs400_mode into reset_hs400_tuning
  callback
* Reuse renesas_sdhi_reset_hs400_mode() in renesas_sdhi_hw_reset()
* Factor out renesas_sdhi_reset_scc()

v2 [Simon Horman]
* Updated to new version from BSP v3.6.0
* Dropped 4 and 8 tap differentiation as all SoCs currently supported
  by the driver in upstream use 4 taps for HS400.

v1 [Simon Horman]
* Combined patched by Ai Kyuse and Masaharu Hayakawa
* Rebase

v0 [Masaharu Hayakawa]
---
 drivers/mmc/host/renesas_sdhi_core.c | 133 +++++++++++++++++++++++++++++------
 1 file changed, 111 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 80943fa07db6..f4c202019cd0 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -211,6 +211,7 @@ static int renesas_sdhi_start_signal_voltage_switch(struct mmc_host *mmc,
 #define SH_MOBILE_SDHI_SCC_CKSEL	0x006
 #define SH_MOBILE_SDHI_SCC_RVSCNTL	0x008
 #define SH_MOBILE_SDHI_SCC_RVSREQ	0x00A
+#define SH_MOBILE_SDHI_SCC_TMPPORT2	0x00E
 
 /* Definitions for values the SH_MOBILE_SDHI_SCC_DTCNTL register */
 #define SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN		BIT(0)
@@ -223,6 +224,9 @@ static int renesas_sdhi_start_signal_voltage_switch(struct mmc_host *mmc,
 #define SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN	BIT(0)
 /* Definitions for values the SH_MOBILE_SDHI_SCC_RVSREQ register */
 #define SH_MOBILE_SDHI_SCC_RVSREQ_RVSERR	BIT(2)
+/* Definitions for values the SH_MOBILE_SDHI_SCC_TMPPORT2 register */
+#define SH_MOBILE_SDHI_SCC_TMPPORT2_HS400OSEL	BIT(4)
+#define SH_MOBILE_SDHI_SCC_TMPPORT2_HS400EN	BIT(31)
 
 static inline u32 sd_scc_read32(struct tmio_mmc_host *host,
 				struct renesas_sdhi *priv, int addr)
@@ -243,33 +247,30 @@ static unsigned int renesas_sdhi_init_tuning(struct tmio_mmc_host *host)
 
 	priv = host_to_priv(host);
 
-	/* set sampling clock selection range */
-	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL,
-		       0x8 << SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT);
-
 	/* Initialize SCC */
 	sd_ctrl_write32_as_16_and_16(host, CTL_STATUS, 0x0);
 
-	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL,
-		       SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN |
-		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL));
-
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
 			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
 
+	/* set sampling clock selection range */
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL,
+		       SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN |
+		       0x8 << SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT);
+
 	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
 		       SH_MOBILE_SDHI_SCC_CKSEL_DTSEL |
 		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
 
-	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
-			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
-
 	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));
 
 	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DT2FF, priv->scc_tappos);
 
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
 	/* Read TAPNUM */
 	return (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL) >>
 		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
@@ -285,13 +286,103 @@ static void renesas_sdhi_prepare_tuning(struct tmio_mmc_host *host,
 	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, tap);
 }
 
+static void renesas_sdhi_prepare_hs400_tuning(struct mmc_host *mmc,
+					      struct mmc_ios *ios)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct renesas_sdhi *priv = host_to_priv(host);
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
+		sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	/* Set HS400 mode */
+	sd_ctrl_write16(host, CTL_SDIF_MODE, 0x0001 |
+			sd_ctrl_read16(host, CTL_SDIF_MODE));
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2,
+		       (SH_MOBILE_SDHI_SCC_TMPPORT2_HS400EN |
+			SH_MOBILE_SDHI_SCC_TMPPORT2_HS400OSEL) |
+			sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2));
+
+	/* Set the sampling clock selection range of HS400 mode */
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL,
+		       SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN |
+		       0x4 << SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT);
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET,
+		       host->tap_set / 2);
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
+		       SH_MOBILE_SDHI_SCC_CKSEL_DTSEL |
+		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+}
+
+static void renesas_sdhi_reset_scc(struct tmio_mmc_host *host,
+				   struct renesas_sdhi *priv)
+{
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
+		       ~SH_MOBILE_SDHI_SCC_CKSEL_DTSEL &
+		       sd_scc_read32(host, priv,
+				     SH_MOBILE_SDHI_SCC_CKSEL));
+}
+
+static void renesas_sdhi_disable_scc(struct tmio_mmc_host *host,
+				     struct renesas_sdhi *priv)
+{
+
+	renesas_sdhi_reset_scc(host, priv);
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL,
+		       ~SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN &
+		       sd_scc_read32(host, priv,
+				     SH_MOBILE_SDHI_SCC_DTCNTL));
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+}
+
+static void renesas_sdhi_reset_hs400_mode(struct tmio_mmc_host *host,
+					  struct renesas_sdhi *priv)
+{
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	/* Reset HS400 mode */
+	sd_ctrl_write16(host, CTL_SDIF_MODE, ~0x0001 &
+			sd_ctrl_read16(host, CTL_SDIF_MODE));
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2,
+		       ~(SH_MOBILE_SDHI_SCC_TMPPORT2_HS400EN |
+			 SH_MOBILE_SDHI_SCC_TMPPORT2_HS400OSEL) &
+			sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2));
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+}
+
+static void renesas_sdhi_reset_hs400_tuning(struct mmc_host *mmc)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct renesas_sdhi *priv = host_to_priv(host);
+
+	if (host->mmc->ios.timing != MMC_TIMING_UHS_SDR104 &&
+	    host->mmc->ios.timing != MMC_TIMING_MMC_HS200 &&
+	    host->mmc->ios.timing != MMC_TIMING_MMC_HS400)
+		renesas_sdhi_disable_scc(host, priv);
+	if (host->mmc->ios.timing == MMC_TIMING_MMC_HS400 || !priv->scc_ctl)
+		renesas_sdhi_reset_hs400_mode(host, priv);
+}
+
 #define SH_MOBILE_SDHI_MAX_TAP 3
 
 static int renesas_sdhi_select_tuning(struct tmio_mmc_host *host)
 {
 	struct renesas_sdhi *priv = host_to_priv(host);
 	unsigned long tap_cnt;  /* counter of tuning success */
-	unsigned long tap_set;  /* tap position */
 	unsigned long tap_start;/* start position of tuning success */
 	unsigned long tap_end;  /* end position of tuning success */
 	unsigned long ntap;     /* temporary counter of tuning success */
@@ -329,12 +420,12 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host *host)
 	}
 
 	if (tap_cnt >= SH_MOBILE_SDHI_MAX_TAP)
-		tap_set = (tap_start + tap_end) / 2 % host->tap_num;
+		host->tap_set = (tap_start + tap_end) / 2 % host->tap_num;
 	else
 		return -EIO;
 
 	/* Set SCC */
-	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, tap_set);
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, host->tap_set);
 
 	/* Enable auto re-tuning */
 	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
@@ -367,13 +458,8 @@ static void renesas_sdhi_hw_reset(struct tmio_mmc_host *host)
 
 	priv = host_to_priv(host);
 
-	/* Reset SCC */
-	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
-			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
-
-	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
-		       ~SH_MOBILE_SDHI_SCC_CKSEL_DTSEL &
-		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
+	renesas_sdhi_reset_scc(host, priv);
+	renesas_sdhi_reset_hs400_mode(host, priv);
 
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
 			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
@@ -587,7 +673,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	/* 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)) {
+	     host->mmc->caps2 & (MMC_CAP2_HS200_1_8V_SDR |
+				 MMC_CAP2_HS400_1_8V))) {
 		const struct renesas_sdhi_scc *taps = of_data->taps;
 		bool hit = false;
 
@@ -611,6 +698,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 		host->select_tuning = renesas_sdhi_select_tuning;
 		host->check_scc_error = renesas_sdhi_check_scc_error;
 		host->hw_reset = renesas_sdhi_hw_reset;
+		host->prepare_hs400_tuning = renesas_sdhi_prepare_hs400_tuning;
+		host->reset_hs400_tuning = renesas_sdhi_reset_hs400_tuning;
 	}
 
 	i = 0;
-- 
2.11.0

Re: [PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Ulf Hansson]

On 13 February 2018 at 13:33, Simon Horman <horms+renesas@verge.net.au> wrote:
> Hi,
>
> this patch-set provides SDHI driver support for eMMC HS400.
>
> Based on mmc/next
>
> Dependencies for applying these patches: none
>
> Dependencies to test eMMC HS400:
> * [PATCH] clk: renesas: rcar-gen3: Fix SD divider setting
> * [PATCH v2] arm64: dts: salvator-common: Enable HS400 of SDHI2
>
> To assist testing and review this patch and the above mentioned
> dependencies, which are necessary and sufficient to enable HS400 on H3 /
> Salvator-X, M3-W 1.0 / Salvator-X and H3 ES2.0 Salvator-XS are available
> at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/hs400-v3
>
> Changes since v2:
> * Consolidate disable_scc and reset_hs400_mode into reset_hs400_tuning
>   callback
> * Reuse renesas_sdhi_reset_hs400_mode() in renesas_sdhi_hw_reset()
> * Factor out renesas_sdhi_reset_scc()
>
> Changes since v1:
> * Use updated code from BSP v3.6.0
> * Ironed out dependencies, eMMC HS400 is now working on
>   H3 / Salvator-X, M3-W 1.0 / Salvator-X and H3 ES2.0 Salvator-XS.
>
> Masaharu Hayakawa (2):
>   mmc: tmio: add eMMC HS400 mode support
>   mmc: renesas_sdhi: add eMMC HS400 mode support
>
>  drivers/mmc/host/renesas_sdhi_core.c | 133 +++++++++++++++++++++++++++++------
>  drivers/mmc/host/tmio_mmc.h          |   5 ++
>  drivers/mmc/host/tmio_mmc_core.c     |  24 ++++++-
>  3 files changed, 138 insertions(+), 24 deletions(-)
>
> --
> 2.11.0
>

Simon, Wolfram,

Is this ready to be queued up for next? It looks good to me.

Kind regards
Uffe

Re: [PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Simon Horman]

On Tue, Feb 27, 2018 at 08:53:17AM +0100, Ulf Hansson wrote:
> On 13 February 2018 at 13:33, Simon Horman <horms+renesas@verge.net.au> wrote:
> > Hi,
> >
> > this patch-set provides SDHI driver support for eMMC HS400.
> >
> > Based on mmc/next
> >
> > Dependencies for applying these patches: none
> >
> > Dependencies to test eMMC HS400:
> > * [PATCH] clk: renesas: rcar-gen3: Fix SD divider setting
> > * [PATCH v2] arm64: dts: salvator-common: Enable HS400 of SDHI2
> >
> > To assist testing and review this patch and the above mentioned
> > dependencies, which are necessary and sufficient to enable HS400 on H3 /
> > Salvator-X, M3-W 1.0 / Salvator-X and H3 ES2.0 Salvator-XS are available
> > at:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/hs400-v3
> >
> > Changes since v2:
> > * Consolidate disable_scc and reset_hs400_mode into reset_hs400_tuning
> >   callback
> > * Reuse renesas_sdhi_reset_hs400_mode() in renesas_sdhi_hw_reset()
> > * Factor out renesas_sdhi_reset_scc()
> >
> > Changes since v1:
> > * Use updated code from BSP v3.6.0
> > * Ironed out dependencies, eMMC HS400 is now working on
> >   H3 / Salvator-X, M3-W 1.0 / Salvator-X and H3 ES2.0 Salvator-XS.
> >
> > Masaharu Hayakawa (2):
> >   mmc: tmio: add eMMC HS400 mode support
> >   mmc: renesas_sdhi: add eMMC HS400 mode support
> >
> >  drivers/mmc/host/renesas_sdhi_core.c | 133 +++++++++++++++++++++++++++++------
> >  drivers/mmc/host/tmio_mmc.h          |   5 ++
> >  drivers/mmc/host/tmio_mmc_core.c     |  24 ++++++-
> >  3 files changed, 138 insertions(+), 24 deletions(-)
> >
> > --
> > 2.11.0
> >
> 
> Simon, Wolfram,
> 
> Is this ready to be queued up for next? It looks good to me.

Hi Ulf, I expect Wolfram would like some more time for review and testing.

Re: [PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Wolfram Sang]

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


> Is this ready to be queued up for next? It looks good to me.

Ulf, out of your head, do you have an idea why preparing HS400 during
set_ios works but populating host->ops->prepare_hs400_tuning() does not
(according to Simon's tests)?


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

Re: [PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Ulf Hansson]

On 27 February 2018 at 10:07, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> Is this ready to be queued up for next? It looks good to me.
>
> Ulf, out of your head, do you have an idea why preparing HS400 during
> set_ios works but populating host->ops->prepare_hs400_tuning() does not
> (according to Simon's tests)?

Oh, that sounds weird. Obviously I didn't read the code carefully enough.

Preparing tuning (if needed, as this is optional) needs to be done
prior mmc->ios.timing is set to MMC_TIMING_MMC_HS400, because the
tuning has already been done when we reach that point.

There seems to be something fishy going on here, as there seemed to be
some kind of reset sequence being added and executed as well.

Perhaps someone need to explain in more detail what the HW controller
needs to manage tuning for HS400? I don't like that we may end up
getting it magically to work, then it's we better understand the
details and if the current sequence provided by the mmc core can't
fulfill the need for this case, we may instead consider adding an
additional pre/post host ops.

Hope this helps!

Kind regards
Uffe

Re: [PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Wolfram Sang]

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


> Perhaps someone need to explain in more detail what the HW controller
> needs to manage tuning for HS400? I don't like that we may end up
> getting it magically to work, then it's we better understand the
> details and if the current sequence provided by the mmc core can't
> fulfill the need for this case, we may instead consider adding an
> additional pre/post host ops.

Thanks. Exactly this thinking prevented me from saying "It works, let's
ship it" ;)


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

Re: [PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Simon Horman]

On Tue, Feb 27, 2018 at 02:02:23PM +0100, Wolfram Sang wrote:
> 
> > Perhaps someone need to explain in more detail what the HW controller
> > needs to manage tuning for HS400? I don't like that we may end up
> > getting it magically to work, then it's we better understand the
> > details and if the current sequence provided by the mmc core can't
> > fulfill the need for this case, we may instead consider adding an
> > additional pre/post host ops.
> 
> Thanks. Exactly this thinking prevented me from saying "It works, let's
> ship it" ;)

Thanks, I've dug a little deeper into the history of this code and
the associated hardware documentation and it seems that the desired
sequence is as follows:

1. Reset HS400 mode (currently executed near beginning of tmio_mmc_set_ios())
2. Set clock for HS200 mode (tmio_mmc_set_ios())
3. Tune in HS200 mode
4. Set clock for HS200 mode (tmio_mmc_set_ios())
5. Set to HS400 mode (currently renesas_sdhi_prepare_hs400_tuning() which
                      is executed near the end of tmio_mmc_set_ios())

X. It is also necessary to disable the SCC after setting HS400 mode
   has occurred before setting to High Speed mode.
   This is currently executed near the beginning of tmio_mmc_set_ios().

The current implementation differs a little from the above and its
hard to tell if that is because of hardware requirements or as
an artefact of the implementation of steps 1 and 5 being hooked into
tmio_mmc_set_ios().

It seems to me that step 1 could use the MMC core prepare_hs400_tuning
hook. Its less clear to me where steps 5 and X should hook in but perhaps
tmio_mmc_set_ios() is the right place.

Re: [PATCH v3 0/2] mmc: renesas_sdhi: add eMMC HS400 mode support

[Ulf Hansson]

On 27 March 2018 at 13:12, Simon Horman <horms@verge.net.au> wrote:
> On Tue, Feb 27, 2018 at 02:02:23PM +0100, Wolfram Sang wrote:
>>
>> > Perhaps someone need to explain in more detail what the HW controller
>> > needs to manage tuning for HS400? I don't like that we may end up
>> > getting it magically to work, then it's we better understand the
>> > details and if the current sequence provided by the mmc core can't
>> > fulfill the need for this case, we may instead consider adding an
>> > additional pre/post host ops.
>>
>> Thanks. Exactly this thinking prevented me from saying "It works, let's
>> ship it" ;)
>
> Thanks, I've dug a little deeper into the history of this code and
> the associated hardware documentation and it seems that the desired
> sequence is as follows:
>
> 1. Reset HS400 mode (currently executed near beginning of tmio_mmc_set_ios())
> 2. Set clock for HS200 mode (tmio_mmc_set_ios())
> 3. Tune in HS200 mode
> 4. Set clock for HS200 mode (tmio_mmc_set_ios())
> 5. Set to HS400 mode (currently renesas_sdhi_prepare_hs400_tuning() which
>                       is executed near the end of tmio_mmc_set_ios())

This seems to also refer to how the eMMC spec describes the sequence,
and thus also implemented by the mmc core.

>
> X. It is also necessary to disable the SCC after setting HS400 mode
>    has occurred before setting to High Speed mode.
>    This is currently executed near the beginning of tmio_mmc_set_ios().
>
> The current implementation differs a little from the above and its
> hard to tell if that is because of hardware requirements or as
> an artefact of the implementation of steps 1 and 5 being hooked into
> tmio_mmc_set_ios().
>
> It seems to me that step 1 could use the MMC core prepare_hs400_tuning
> hook. Its less clear to me where steps 5 and X should hook in but perhaps
> tmio_mmc_set_ios() is the right place.

In ->set_ios() you are relying on checking the value of ->ios.timing
(MMC_TIMING_MMC_HS200 and MMC_TIMING_MMC_HS400) to understand what
actions to take.

For tuning, we found this was not sufficient when trying to cope with
all different MMC controller constraints, which is why we added the
->prepare_hs400_tuning() ops. I am guessing it should work for your
case as well.

However, if that isn't the case and you find reasons to add another
->[xyz]_hs400_tuning() callback, then I am open to discuss that.

Kind regards
Uffe