[PATCH v2 0/3] mmc: renesas_sdhi: add eMMC HS400 mode support

[PATCH v2 0/3] 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-v2

Changes since v2:
* 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 (3):
  mmc: tmio: correct treatment of errors during tuning
  mmc: tmio: add eMMC HS400 mode support
  mmc: renesas_sdhi: add eMMC HS400 mode support

 drivers/mmc/host/renesas_sdhi_core.c | 125 ++++++++++++++++++++++++++++++-----
 drivers/mmc/host/tmio_mmc.h          |   6 ++
 drivers/mmc/host/tmio_mmc_core.c     |  33 +++++++--
 3 files changed, 143 insertions(+), 21 deletions(-)

-- 
2.11.0

[PATCH v2 1/3] mmc: tmio: correct treatment of errors during tuning

[Simon Horman]

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

If the return value of mmc_send_tuning() is error other than -EILSEQ, the
tuning fails and process goes out of for_loop.  But the correct processing
is to judge their TAP as bad.

Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v2 [Simon Horman]
* Added to patchset targeted at upstream
* Minor revision of changelog

v0 [Masaharu Hayakawa]
---
 drivers/mmc/host/tmio_mmc_core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 6d8719be75a8..41767d33ef97 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -800,10 +800,7 @@ static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 		if (host->prepare_tuning)
 			host->prepare_tuning(host, i % host->tap_num);
 
-		ret = mmc_send_tuning(mmc, opcode, NULL);
-		if (ret && ret != -EILSEQ)
-			goto out;
-		if (ret == 0)
+		if (!mmc_send_tuning(mmc, opcode, NULL))
 			set_bit(i, host->taps);
 
 		usleep_range(1000, 1200);
-- 
2.11.0

Re: [PATCH v2 1/3] mmc: tmio: correct treatment of errors during tuning

[Wolfram Sang]

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

On Fri, Jan 19, 2018 at 02:39:04PM +0100, Simon Horman wrote:
> From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> 
> If the return value of mmc_send_tuning() is error other than -EILSEQ, the
> tuning fails and process goes out of for_loop.  But the correct processing
> is to judge their TAP as bad.

Ideally, we would have more specific reasons why this is correct processing.

What other codes could happen here?

> Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> v2 [Simon Horman]
> * Added to patchset targeted at upstream
> * Minor revision of changelog
> 
> v0 [Masaharu Hayakawa]
> ---
>  drivers/mmc/host/tmio_mmc_core.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 6d8719be75a8..41767d33ef97 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -800,10 +800,7 @@ static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  		if (host->prepare_tuning)
>  			host->prepare_tuning(host, i % host->tap_num);
>  
> -		ret = mmc_send_tuning(mmc, opcode, NULL);
> -		if (ret && ret != -EILSEQ)
> -			goto out;
> -		if (ret == 0)
> +		if (!mmc_send_tuning(mmc, opcode, NULL))

I'd prefer (mmc_send_tuning() == 0) here instead of '!mmc_send_tuning()'.
This reads as 'is ok' while the other reads more 'if not ok'.

>  			set_bit(i, host->taps);
>  
>  		usleep_range(1000, 1200);
> -- 
> 2.11.0
> 

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

Re: [PATCH v2 1/3] mmc: tmio: correct treatment of errors during tuning

[Simon Horman]

On Wed, Feb 07, 2018 at 10:52:52PM +0100, Wolfram Sang wrote:
> On Fri, Jan 19, 2018 at 02:39:04PM +0100, Simon Horman wrote:
> > From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> > 
> > If the return value of mmc_send_tuning() is error other than -EILSEQ, the
> > tuning fails and process goes out of for_loop.  But the correct processing
> > is to judge their TAP as bad.
> 
> Ideally, we would have more specific reasons why this is correct processing.
> 
> What other codes could happen here?

I mistakenly attached log below following to patch 2/3 rather than 1/3 (this
patch) which explains my reason for including this patch in the series.

However, I am no longer able to reproduce this problem - I tried to do so
in order to work out which errors were being handled differently with
and without the patch.

I think we can drop this patch for now.



* M3-W ES1.0 / Salvator-X

[    1.812758] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    1.818778] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    1.874951] renesas_sdhi_internal_dmac ee140000.sd: mmc0 base at 0xee140000
+max clock rate 200 MHz
[    1.884950] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    1.891088] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO
[    2.083508] mmc0: new HS400 MMC card at address 0001
[    2.084827] mmcblk0: mmc0:0001 eMMC   28.8 GiB
[    2.085234] mmcblk0boot0: mmc0:0001 eMMC   partition 1 4.00 MiB
[    2.085727] mmcblk0boot1: mmc0:0001 eMMC   partition 2 4.00 MiB
[    2.086398] mmcblk0rpmb: mmc0:0001 eMMC   partition 3 4.00 MiB, chardev
+(243:0)
[    2.097926]  mmcblk0: p1
[    2.360533] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    2.367633] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    2.424700] renesas_sdhi_internal_dmac ee100000.sd: mmc1 base at 0xee100000
+max clock rate 200 MHz
[    2.436021] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    2.443100] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO

* On H3 ES2.0 / Salvator-XS:

[    2.452354] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    2.458344] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    2.513917] renesas_sdhi_internal_dmac ee140000.sd: mmc0 base at 0xee140000
+max clock rate 200 MHz
[    2.523564] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    2.529559] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO
[    2.636678] renesas_sdhi_internal_dmac ee140000.sd: Tuning procedure failed
[    2.643739] mmc0: tuning execution failed: -5
[    2.648211] mmc0: error -5 whilst initialising MMC card
[    2.730078] renesas_sdhi_internal_dmac ee140000.sd: Tuning procedure failed
[    2.730085] mmc0: tuning execution failed: -5
[    2.730093] mmc0: error -5 whilst initialising MMC card
[    2.858718] renesas_sdhi_internal_dmac ee140000.sd: Tuning procedure failed
[    2.858725] mmc0: tuning execution failed: -5
[    2.858733] mmc0: error -5 whilst initialising MMC card
[    2.991258] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    2.998333] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    3.063121] renesas_sdhi_internal_dmac ee140000.sd: Tuning procedure failed
[    3.063128] mmc0: tuning execution failed: -5
[    3.063135] mmc0: error -5 whilst initialising MMC card
[    3.085170] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    3.092222] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO

> 
> > Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> > v2 [Simon Horman]
> > * Added to patchset targeted at upstream
> > * Minor revision of changelog
> > 
> > v0 [Masaharu Hayakawa]
> > ---
> >  drivers/mmc/host/tmio_mmc_core.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> > index 6d8719be75a8..41767d33ef97 100644
> > --- a/drivers/mmc/host/tmio_mmc_core.c
> > +++ b/drivers/mmc/host/tmio_mmc_core.c
> > @@ -800,10 +800,7 @@ static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> >  		if (host->prepare_tuning)
> >  			host->prepare_tuning(host, i % host->tap_num);
> >  
> > -		ret = mmc_send_tuning(mmc, opcode, NULL);
> > -		if (ret && ret != -EILSEQ)
> > -			goto out;
> > -		if (ret == 0)
> > +		if (!mmc_send_tuning(mmc, opcode, NULL))
> 
> I'd prefer (mmc_send_tuning() == 0) here instead of '!mmc_send_tuning()'.
> This reads as 'is ok' while the other reads more 'if not ok'.
> 
> >  			set_bit(i, host->taps);
> >  
> >  		usleep_range(1000, 1200);
> > -- 
> > 2.11.0
> > 

Re: [PATCH v2 1/3] mmc: tmio: correct treatment of errors during tuning

[Wolfram Sang]

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



> I think we can drop this patch for now.

Nice! But we should keep it in mind and recall it, if issues pop up
later.


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

[PATCH v2 2/3] 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>
---
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]
---
I believe that this is a pre-requisite for enabling HS400 as with HS400
enabled but without this patch I see:

* M3-W ES1.0 / Salvator-X

[    1.812758] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    1.818778] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    1.874951] renesas_sdhi_internal_dmac ee140000.sd: mmc0 base at 0xee140000 max clock rate 200 MHz
[    1.884950] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    1.891088] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO
[    2.083508] mmc0: new HS400 MMC card at address 0001
[    2.084827] mmcblk0: mmc0:0001 eMMC   28.8 GiB
[    2.085234] mmcblk0boot0: mmc0:0001 eMMC   partition 1 4.00 MiB
[    2.085727] mmcblk0boot1: mmc0:0001 eMMC   partition 2 4.00 MiB
[    2.086398] mmcblk0rpmb: mmc0:0001 eMMC   partition 3 4.00 MiB, chardev (243:0)
[    2.097926]  mmcblk0: p1
[    2.360533] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    2.367633] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    2.424700] renesas_sdhi_internal_dmac ee100000.sd: mmc1 base at 0xee100000 max clock rate 200 MHz
[    2.436021] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    2.443100] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO

* On H3 ES2.0 / Salvator-XS:

[    2.452354] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    2.458344] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    2.513917] renesas_sdhi_internal_dmac ee140000.sd: mmc0 base at 0xee140000 max clock rate 200 MHz
[    2.523564] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    2.529559] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO
[    2.636678] renesas_sdhi_internal_dmac ee140000.sd: Tuning procedure failed
[    2.643739] mmc0: tuning execution failed: -5
[    2.648211] mmc0: error -5 whilst initialising MMC card
[    2.730078] renesas_sdhi_internal_dmac ee140000.sd: Tuning procedure failed
[    2.730085] mmc0: tuning execution failed: -5
[    2.730093] mmc0: error -5 whilst initialising MMC card
[    2.858718] renesas_sdhi_internal_dmac ee140000.sd: Tuning procedure failed
[    2.858725] mmc0: tuning execution failed: -5
[    2.858733] mmc0: error -5 whilst initialising MMC card
[    2.991258] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    2.998333] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    3.063121] renesas_sdhi_internal_dmac ee140000.sd: Tuning procedure failed
[    3.063128] mmc0: tuning execution failed: -5
[    3.063135] mmc0: error -5 whilst initialising MMC card
[    3.085170] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    3.092222] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO

On H3 ES1.0 / Salvator-X I do not see any problems either with or without
this patch.

To assist testing and review I will make patches, including this one, to
enable HS400 on H3 / Salvator-X, M3-W 1.0 / Salvator-X and
H3 ES2.0 Salvator-XS available at:

https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/hs400-v2
---
 drivers/mmc/host/tmio_mmc.h      |  6 ++++++
 drivers/mmc/host/tmio_mmc_core.c | 28 ++++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index e7d651352dc9..8f163e7f71e0 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,11 @@ 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 (*disable_scc)(struct mmc_host *mmc);
+	void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios);
+	void (*reset_hs400_mode)(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 41767d33ef97..2a84da51b08d 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);
@@ -1012,6 +1024,13 @@ 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->disable_scc)
+		host->disable_scc(mmc);
+
+	/* reset HS400 mode */
+	if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode)
+		host->reset_hs400_mode(mmc);
+
 	mutex_lock(&host->ios_lock);
 
 	spin_lock_irqsave(&host->lock, flags);
@@ -1062,6 +1081,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

Re: [PATCH v2 2/3] mmc: tmio: add eMMC HS400 mode support

[Wolfram Sang]

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


Hi Simon,

> +	void (*disable_scc)(struct mmc_host *mmc);

Do we really need this callback? I'd think it can be folded into
reset_hs400_mode() because it is called only once?

> +	void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios);

Can't we use the host->ops->prepare_hs400_tuning() callback invoked by
the core?

> +	void (*reset_hs400_mode)(struct mmc_host *mmc);

Maybe we can get rid of this, too? See later...

> +	if (host->disable_scc)
> +		host->disable_scc(mmc);

(Here, this can be folded into the next callback)

> +
> +	/* reset HS400 mode */
> +	if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode)
> +		host->reset_hs400_mode(mmc);

I wonder: If for any ios which is != MMC_TIMING_MMC_HS400, the
hs400_mode needs to be reset. Couldn't we as well then disable the mode
always after the MMC_TIMING_MMC_HS400 tuning was selected? Just
brainstorming here...

Regards,

   Wolfram


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

Re: [PATCH v2 2/3] mmc: tmio: add eMMC HS400 mode support

[Simon Horman]

On Wed, Feb 07, 2018 at 11:20:12PM +0100, Wolfram Sang wrote:
> 
> Hi Simon,
> 
> > +	void (*disable_scc)(struct mmc_host *mmc);
> 
> Do we really need this callback? I'd think it can be folded into
> reset_hs400_mode() because it is called only once?
> 
> > +	void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios);
> 
> Can't we use the host->ops->prepare_hs400_tuning() callback invoked by
> the core?

Empirically that does not seem to work.

> > +	void (*reset_hs400_mode)(struct mmc_host *mmc);
> 
> Maybe we can get rid of this, too? See later...
> 
> > +	if (host->disable_scc)
> > +		host->disable_scc(mmc);
> 
> (Here, this can be folded into the next callback)

Yes, agreed. I've folded the callbacks as you suggest.

> > +
> > +	/* reset HS400 mode */
> > +	if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode)
> > +		host->reset_hs400_mode(mmc);
> 
> I wonder: If for any ios which is != MMC_TIMING_MMC_HS400, the
> hs400_mode needs to be reset. Couldn't we as well then disable the mode
> always after the MMC_TIMING_MMC_HS400 tuning was selected? Just
> brainstorming here...

Perhaps but I'm unsure where we would hook in this change, any ideas?

Re: [PATCH v2 2/3] mmc: tmio: add eMMC HS400 mode support

[Wolfram Sang]

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

On Tue, Feb 13, 2018 at 12:33:53PM +0100, Simon Horman wrote:
> On Wed, Feb 07, 2018 at 11:20:12PM +0100, Wolfram Sang wrote:
> > 
> > Hi Simon,
> > 
> > > +	void (*disable_scc)(struct mmc_host *mmc);
> > 
> > Do we really need this callback? I'd think it can be folded into
> > reset_hs400_mode() because it is called only once?
> > 
> > > +	void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios);
> > 
> > Can't we use the host->ops->prepare_hs400_tuning() callback invoked by
> > the core?
> 
> Empirically that does not seem to work.

:( Pity.

> > > +	void (*reset_hs400_mode)(struct mmc_host *mmc);
> > 
> > Maybe we can get rid of this, too? See later...
> > 
> > > +	if (host->disable_scc)
> > > +		host->disable_scc(mmc);
> > 
> > (Here, this can be folded into the next callback)
> 
> Yes, agreed. I've folded the callbacks as you suggest.

Cool, thanks!

> > > +
> > > +	/* reset HS400 mode */
> > > +	if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode)
> > > +		host->reset_hs400_mode(mmc);
> > 
> > I wonder: If for any ios which is != MMC_TIMING_MMC_HS400, the
> > hs400_mode needs to be reset. Couldn't we as well then disable the mode
> > always after the MMC_TIMING_MMC_HS400 tuning was selected? Just
> > brainstorming here...
> 
> Perhaps but I'm unsure where we would hook in this change, any ideas?

I think this becomes moot if we can't get the prepare-callback of the
core to work. I'd think it makes sense to have both callbacks from here
moved to the core or none to keep at least one kind of symmetry.


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

[PATCH v2 3/3] 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>
---
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 | 125 ++++++++++++++++++++++++++++++-----
 1 file changed, 110 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 80943fa07db6..e3f00d051bc6 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,95 @@ 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_disable_scc(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)
+		return;
+
+	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));
+
+	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_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_hs400_mode(struct mmc_host *mmc)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct renesas_sdhi *priv = host_to_priv(host);
+
+	if (!priv->scc_ctl)
+		return;
+
+	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));
+}
+
 #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 +412,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,
@@ -375,6 +458,14 @@ static void renesas_sdhi_hw_reset(struct tmio_mmc_host *host)
 		       ~SH_MOBILE_SDHI_SCC_CKSEL_DTSEL &
 		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
 
+	/* 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));
 
@@ -587,7 +678,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 +703,9 @@ 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->disable_scc = renesas_sdhi_disable_scc;
+		host->prepare_hs400_tuning = renesas_sdhi_prepare_hs400_tuning;
+		host->reset_hs400_mode = renesas_sdhi_reset_hs400_mode;
 	}
 
 	i = 0;
-- 
2.11.0

Re: [PATCH v2 3/3] mmc: renesas_sdhi: add eMMC HS400 mode support

[Wolfram Sang]

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


> +	/* 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));
> +

This looks like code duplication. Can't we simply call
renesas_sdhi_reset_hs400_mode()?


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

Re: [PATCH v2 3/3] mmc: renesas_sdhi: add eMMC HS400 mode support

[Simon Horman]

On Wed, Feb 07, 2018 at 11:21:44PM +0100, Wolfram Sang wrote:
> 
> > +	/* 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));
> > +
> 
> This looks like code duplication. Can't we simply call
> renesas_sdhi_reset_hs400_mode()?

Yes, thanks for noticing. I will make that so.

I think that the previous few lines, not added by this patch,
can also be consolidated into a renesas_sdhi_reset_scc() function
and re-used in renesas_sdhi_reset_scc().

Re: [PATCH v2 3/3] mmc: renesas_sdhi: add eMMC HS400 mode support

[Wolfram Sang]

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

On Tue, Feb 13, 2018 at 12:38:44PM +0100, Simon Horman wrote:
> On Wed, Feb 07, 2018 at 11:21:44PM +0100, Wolfram Sang wrote:
> > 
> > > +	/* 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));
> > > +
> > 
> > This looks like code duplication. Can't we simply call
> > renesas_sdhi_reset_hs400_mode()?
> 
> Yes, thanks for noticing. I will make that so.
> 
> I think that the previous few lines, not added by this patch,
> can also be consolidated into a renesas_sdhi_reset_scc() function
> and re-used in renesas_sdhi_reset_scc().

Even better!


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

Re: [PATCH v2 0/3] mmc: renesas_sdhi: add eMMC HS400 mode support

[Wolfram Sang]

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


> * [PATCH] clk: renesas: rcar-gen3: Fix SD divider setting

I tried to address this one before:

[PATCH 0/3] clk: renesas: rcar-gen3-cpg: updates to SD divider table

but this had problems with H3 ES1.0 :(

I haven't yet compared the above dependency patch with what I did back
then. Will need to do that hopefully tomorrow. But as I read it, it
worked fine for you on H3 ES1.0?


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

Re: [PATCH v2 0/3] mmc: renesas_sdhi: add eMMC HS400 mode support

[Simon Horman]

On Wed, Feb 07, 2018 at 11:26:30PM +0100, Wolfram Sang wrote:
> 
> > * [PATCH] clk: renesas: rcar-gen3: Fix SD divider setting
> 
> I tried to address this one before:
> 
> [PATCH 0/3] clk: renesas: rcar-gen3-cpg: updates to SD divider table
> 
> but this had problems with H3 ES1.0 :(
> 
> I haven't yet compared the above dependency patch with what I did back
> then. Will need to do that hopefully tomorrow. But as I read it, it
> worked fine for you on H3 ES1.0?

Yes, it seemed to, though my memory of the testing is not fresh at this point.

Re: [PATCH v2 0/3] mmc: renesas_sdhi: add eMMC HS400 mode support

[Simon Horman]

On Thu, Feb 08, 2018 at 12:37:03PM +0100, Simon Horman wrote:
> On Wed, Feb 07, 2018 at 11:26:30PM +0100, Wolfram Sang wrote:
> > 
> > > * [PATCH] clk: renesas: rcar-gen3: Fix SD divider setting
> > 
> > I tried to address this one before:
> > 
> > [PATCH 0/3] clk: renesas: rcar-gen3-cpg: updates to SD divider table
> > 
> > but this had problems with H3 ES1.0 :(
> > 
> > I haven't yet compared the above dependency patch with what I did back
> > then. Will need to do that hopefully tomorrow. But as I read it, it
> > worked fine for you on H3 ES1.0?
> 
> Yes, it seemed to, though my memory of the testing is not fresh at this point.

I've done some more testing. This seems to work on:

* H3 ES1.0 / Salvator-X
* M3W ES1.0 / Salvator-X
* H3 ES2.0 / Salvator-XS