[PATCH v3 0/2] mmc: {tmio,renesas_sdhi}: retune if SCC error detected

[PATCH v3 0/2] mmc: {tmio,renesas_sdhi}: retune if SCC error detected

[Niklas Söderlund]

Hi,

These patches triggers a retune if a SCC error is detected. They where
ported from the renesas BSP. Masaharu-san did all the real work I just
ported them to upstream and did small fixups.

These patches where broken out of my retuning series as more work where
needed to adapt them to the recent HS400 series picked-up. It's tested
on M3-N using both HS200 and HS400 and on H3 ES2.0 using HS200.

Masaharu Hayakawa (2):
  mmc: renesas_sdhi: skip SCC error check when retuning
  mmc: tmio: Fix SCC error detection

 drivers/mmc/host/renesas_sdhi_core.c | 13 +++++++++++++
 drivers/mmc/host/tmio_mmc_core.c     |  4 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

-- 
2.18.0

[PATCH v3 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Niklas Söderlund]

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

Checking for SCC error during retuning is unnecessary.

Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
[Niklas: fix small style issue]
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

---

* Changes since v2
- Add comment to describe why checking SCC errors when using 4 taps is
  not needed.

* Changes since v1
- Use intermediate variable use_4tap to simplify check.
---
 drivers/mmc/host/renesas_sdhi_core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 777e32b0e410e850..4427f0e7058f3ee5 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -443,6 +443,19 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host *host)
 static bool renesas_sdhi_check_scc_error(struct tmio_mmc_host *host)
 {
 	struct renesas_sdhi *priv = host_to_priv(host);
+	bool use_4tap = host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400;
+
+	/*
+	 * Skip checking SCC errors when running on 4 taps in HS400 mode as
+	 * any retuning would still result in the same 4 taps being used.
+	 */
+	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 && !use_4tap))
+		return false;
+
+	if (host->mmc->doing_retune)
+		return false;
 
 	/* Check SCC error */
 	if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) &
-- 
2.18.0

[PATCH v3 2/2] mmc: tmio: Fix SCC error detection

[Niklas Söderlund]

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

SDR104 and HS200 need to check for SCC error. If SCC error is detected,
retuning is necessary.

Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
[Niklas: update commit message]
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 261b4d62d2b1061f..268c4a3b728c775f 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -919,8 +919,8 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
 	if (mrq->cmd->error || (mrq->data && mrq->data->error))
 		tmio_mmc_abort_dma(host);
 
-	if (host->check_scc_error)
-		host->check_scc_error(host);
+	if (host->check_scc_error && host->check_scc_error(host))
+		mrq->cmd->error = -EILSEQ;
 
 	/* If SET_BLOCK_COUNT, continue with main command */
 	if (host->mrq && !mrq->cmd->error) {
-- 
2.18.0

Re: [PATCH v3 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Ulf Hansson]

On 27 August 2018 at 17:13, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
>
> Checking for SCC error during retuning is unnecessary.
>
> Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> [Niklas: fix small style issue]
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> ---
>
> * Changes since v2
> - Add comment to describe why checking SCC errors when using 4 taps is
>   not needed.
>
> * Changes since v1
> - Use intermediate variable use_4tap to simplify check.
> ---
>  drivers/mmc/host/renesas_sdhi_core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 777e32b0e410e850..4427f0e7058f3ee5 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -443,6 +443,19 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host *host)
>  static bool renesas_sdhi_check_scc_error(struct tmio_mmc_host *host)
>  {
>         struct renesas_sdhi *priv = host_to_priv(host);
> +       bool use_4tap = host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400;
> +
> +       /*
> +        * Skip checking SCC errors when running on 4 taps in HS400 mode as
> +        * any retuning would still result in the same 4 taps being used.
> +        */
> +       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 && !use_4tap))
> +               return false;
> +
> +       if (host->mmc->doing_retune)

To keep the tuning internals close to the mmc core, please add a new
inline function in include/linux/mmc/host.h to give the value of the
"doing_retune", something along the lines of this:

/* Allows hosts to distinguish between re-tuning and first time tuning. */
static inline bool mmc_doing_retune(struct mmc_host *host)
{
        return host->doing_retune == 1;
}

And of course in separate patch.

[...]

Otherwise this looks okay to me.

Kind regards
Uffe

Re: [PATCH v3 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Wolfram Sang]

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


> +	/*
> +	 * Skip checking SCC errors when running on 4 taps in HS400 mode as
> +	 * any retuning would still result in the same 4 taps being used.
> +	 */

Did you verify this? I didn't have the time to do so :(


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

Re: [PATCH v3 2/2] mmc: tmio: Fix SCC error detection

[Wolfram Sang]

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

On Mon, Aug 27, 2018 at 05:13:21PM +0200, Niklas Söderlund wrote:
> From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> 
> SDR104 and HS200 need to check for SCC error. If SCC error is detected,
> retuning is necessary.

HS400, 8 taps, too, or am I missing something?


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

Re: [PATCH v3 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Niklas Söderlund]

Hi Wolfram,

Thanks for your feedback,

On 2018-08-29 09:16:04 +0200, Wolfram Sang wrote:
> 
> > +	/*
> > +	 * Skip checking SCC errors when running on 4 taps in HS400 mode as
> > +	 * any retuning would still result in the same 4 taps being used.
> > +	 */
> 
> Did you verify this? I didn't have the time to do so :(
> 

I tested it on M3-W ES1.0 and it H3 ES2.0 and it works as I expect it to 
do, but forcing a retune due to SCC error is beyond my skill-set. I 
checked the documentation and can't find anything contradicting 
Shimoda-sans information from the HW team. So to the best of my 
knowledge this is correct. On the other hand I can't find anything in 
the documentation which supports the claim from the HW team.

If you have any tips on how to better test this I'm more then happy to 
do the work. Also would you like me to post a v4 addressing Ulf's 
comment or wait until we are convinced this is the right path forward?

-- 
Regards,
Niklas S�derlund

Re: [PATCH v3 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Wolfram Sang]

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

Hi Niklas,

> I tested it on M3-W ES1.0 and it H3 ES2.0 and it works as I expect it to 
> do, but forcing a retune due to SCC error is beyond my skill-set. I 
> checked the documentation and can't find anything contradicting 
> Shimoda-sans information from the HW team. So to the best of my 
> knowledge this is correct. On the other hand I can't find anything in 
> the documentation which supports the claim from the HW team.

Sorry for not being precise. With "verifying" I didn't mean at runtime,
but checking if documentation supports this information from the HW
team.

> If you have any tips on how to better test this I'm more then happy to 
> do the work. Also would you like me to post a v4 addressing Ulf's 
> comment or wait until we are convinced this is the right path forward?

As you say it is in accordance with the docs, please send v4 with Ulf's
comments addressed.

Thanks,

   Wolfram


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