[PATCH V2 0/4] mmc: sdhci: Disable re-tuning for HS400

[PATCH V2 0/4] mmc: sdhci: Disable re-tuning for HS400

[Adrian Hunter]

Hi

Here is V2 of patches to disable re-tuning for HS400.

As described in patch 4, re-tuning for HS400 has to be done
in HS200 mode, but there is no support for that, so re-tuning
needs to be disabled until support is added.

Changes in V2:

	Added "mmc: sdhci: Simplify use of tuning timer"
	patch.

	Changed "mmc: sdhci: Disable re-tuning for HS400"
	so that the tuning timer is not started instead
	of stopping it when HS400 mode is selected.


Adrian Hunter (4):
      mmc: sdhci: Tuning should not change max_blk_count
      mmc: sdhci: Add out_unlock to sdhci_execute_tuning
      mmc: sdhci: Simplify use of tuning timer
      mmc: sdhci: Disable re-tuning for HS400

 drivers/mmc/host/sdhci.c  | 66 ++++++++++++++++++++++++++++++-----------------
 include/linux/mmc/sdhci.h |  1 +
 2 files changed, 43 insertions(+), 24 deletions(-)


Regards
Adrian

[PATCH V2 1/4] mmc: sdhci: Tuning should not change max_blk_count

[Adrian Hunter]

Re-tuning requires that the maximum data length
is limited to 4MiB. The code currently changes
max_blk_count in an attempt to achieve that.
This is wrong because max_blk_count is a different
limit, but it is also un-necessary because
max_req_size is 512KiB anyway. Consequently, the
changes to max_blk_count are removed and the
comment for max_req_size adjusted accordingly.
The comment is also tweaked to show that the 512KiB
limit is a SDMA limit not an ADMA limit.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 73de62a..b3d68e0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -259,8 +259,6 @@ static void sdhci_reinit(struct sdhci_host *host)
 
 		del_timer_sync(&host->tuning_timer);
 		host->flags &= ~SDHCI_NEEDS_RETUNING;
-		host->mmc->max_blk_count =
-			(host->quirks & SDHCI_QUIRK_NO_MULTIBLOCK) ? 1 : 65535;
 	}
 	sdhci_enable_card_detection(host);
 }
@@ -2048,8 +2046,6 @@ out:
 		host->flags |= SDHCI_USING_RETUNING_TIMER;
 		mod_timer(&host->tuning_timer, jiffies +
 			host->tuning_count * HZ);
-		/* Tuning mode 1 limits the maximum data length to 4MB */
-		mmc->max_blk_count = (4 * 1024 * 1024) / mmc->max_blk_size;
 	} else if (host->flags & SDHCI_USING_RETUNING_TIMER) {
 		host->flags &= ~SDHCI_NEEDS_RETUNING;
 		/* Reload the new initial value for timer */
@@ -3263,8 +3259,9 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc->max_segs = SDHCI_MAX_SEGS;
 
 	/*
-	 * Maximum number of sectors in one transfer. Limited by DMA boundary
-	 * size (512KiB).
+	 * Maximum number of sectors in one transfer. Limited by SDMA boundary
+	 * size (512KiB). Note some tuning modes impose a 4MiB limit, but this
+	 * is less anyway.
 	 */
 	mmc->max_req_size = 524288;
 
-- 
1.9.1

[PATCH V2 2/4] mmc: sdhci: Add out_unlock to sdhci_execute_tuning

[Adrian Hunter]

A 'goto' can be used to save duplicating unlocking
and returning.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b3d68e0..2efa7fe 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1909,9 +1909,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 		/* FALLTHROUGH */
 
 	default:
-		spin_unlock_irqrestore(&host->lock, flags);
-		sdhci_runtime_pm_put(host);
-		return 0;
+		goto out_unlock;
 	}
 
 	if (host->ops->platform_execute_tuning) {
@@ -2066,6 +2064,7 @@ out:
 
 	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+out_unlock:
 	spin_unlock_irqrestore(&host->lock, flags);
 	sdhci_runtime_pm_put(host);
 
-- 
1.9.1

[PATCH V2 3/4] mmc: sdhci: Simplify use of tuning timer

[Adrian Hunter]

The tuning timer is always used if the tuning mode
is 1 and there is a tuning count, irrespective of
whether this is the first call, or any subsequent
call. Consequently the logic to start the timer
can be simplified.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2efa7fe..ff0fe90 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1885,10 +1885,14 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	int tuning_loop_counter = MAX_TUNING_LOOP;
 	int err = 0;
 	unsigned long flags;
+	unsigned int tuning_count = 0;
 
 	sdhci_runtime_pm_get(host);
 	spin_lock_irqsave(&host->lock, flags);
 
+	if (host->tuning_mode == SDHCI_TUNING_MODE_1)
+		tuning_count = host->tuning_count;
+
 	/*
 	 * The Host Controller needs tuning only in case of SDR104 mode
 	 * and for SDR50 mode when Use Tuning for SDR50 is set in the
@@ -2033,22 +2037,11 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	}
 
 out:
-	/*
-	 * If this is the very first time we are here, we start the retuning
-	 * timer. Since only during the first time, SDHCI_NEEDS_RETUNING
-	 * flag won't be set, we check this condition before actually starting
-	 * the timer.
-	 */
-	if (!(host->flags & SDHCI_NEEDS_RETUNING) && host->tuning_count &&
-	    (host->tuning_mode == SDHCI_TUNING_MODE_1)) {
+	host->flags &= ~SDHCI_NEEDS_RETUNING;
+
+	if (tuning_count) {
 		host->flags |= SDHCI_USING_RETUNING_TIMER;
-		mod_timer(&host->tuning_timer, jiffies +
-			host->tuning_count * HZ);
-	} else if (host->flags & SDHCI_USING_RETUNING_TIMER) {
-		host->flags &= ~SDHCI_NEEDS_RETUNING;
-		/* Reload the new initial value for timer */
-		mod_timer(&host->tuning_timer, jiffies +
-			  host->tuning_count * HZ);
+		mod_timer(&host->tuning_timer, jiffies + tuning_count * HZ);
 	}
 
 	/*
-- 
1.9.1

[PATCH V2 4/4] mmc: sdhci: Disable re-tuning for HS400

[Adrian Hunter]

Re-tuning for HS400 mode must be done in HS200
mode. Currently there is no support for that.
That needs to be reflected in the code.
Specifically, if tuning is executed in HS400 mode
then return an error, and do not start the
tuning timer if HS200 tuning is being done prior
to switching to HS400.

Note that periodic re-tuning is not expected
to be needed for HS400 but re-tuning is still
needed after the host controller has lost power.
In the case of suspend/resume that is not necessary
because the card is fully re-initialised. That
just leaves runtime suspend/resume with no support
for HS400 re-tuning.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c  | 29 +++++++++++++++++++++++++++++
 include/linux/mmc/sdhci.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ff0fe90..bfde165 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1878,6 +1878,18 @@ static int sdhci_card_busy(struct mmc_host *mmc)
 	return !(present_state & SDHCI_DATA_LVL_MASK);
 }
 
+static int sdhci_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&host->lock, flags);
+	host->flags |= SDHCI_HS400_TUNING;
+	spin_unlock_irqrestore(&host->lock, flags);
+
+	return 0;
+}
+
 static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
@@ -1886,10 +1898,14 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	int err = 0;
 	unsigned long flags;
 	unsigned int tuning_count = 0;
+	bool hs400_tuning;
 
 	sdhci_runtime_pm_get(host);
 	spin_lock_irqsave(&host->lock, flags);
 
+	hs400_tuning = host->flags & SDHCI_HS400_TUNING;
+	host->flags &= ~SDHCI_HS400_TUNING;
+
 	if (host->tuning_mode == SDHCI_TUNING_MODE_1)
 		tuning_count = host->tuning_count;
 
@@ -1901,8 +1917,20 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	 * tuning function has to be executed.
 	 */
 	switch (host->timing) {
+	/* HS400 tuning is done in HS200 mode */
 	case MMC_TIMING_MMC_HS400:
+		err = -EINVAL;
+		goto out_unlock;
+
 	case MMC_TIMING_MMC_HS200:
+		/*
+		 * Periodic re-tuning for HS400 is not expected to be needed, so
+		 * disable it here.
+		 */
+		if (hs400_tuning)
+			tuning_count = 0;
+		break;
+
 	case MMC_TIMING_UHS_SDR104:
 		break;
 
@@ -2130,6 +2158,7 @@ static const struct mmc_host_ops sdhci_ops = {
 	.hw_reset	= sdhci_hw_reset,
 	.enable_sdio_irq = sdhci_enable_sdio_irq,
 	.start_signal_voltage_switch	= sdhci_start_signal_voltage_switch,
+	.prepare_hs400_tuning		= sdhci_prepare_hs400_tuning,
 	.execute_tuning			= sdhci_execute_tuning,
 	.card_event			= sdhci_card_event,
 	.card_busy	= sdhci_card_busy,
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 375af80..f767a0d 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -137,6 +137,7 @@ struct sdhci_host {
 #define SDHCI_SDR104_NEEDS_TUNING (1<<10)	/* SDR104/HS200 needs tuning */
 #define SDHCI_USING_RETUNING_TIMER (1<<11)	/* Host is using a retuning timer for the card */
 #define SDHCI_USE_64_BIT_DMA	(1<<12)	/* Use 64-bit DMA */
+#define SDHCI_HS400_TUNING	(1<<13)	/* Tuning for HS400 */
 
 	unsigned int version;	/* SDHCI spec. version */
 
-- 
1.9.1

Re: [PATCH V2 0/4] mmc: sdhci: Disable re-tuning for HS400

[Aaron Lu]

On 12/06/2014 01:25 AM, Adrian Hunter wrote:
> Hi
> 
> Here is V2 of patches to disable re-tuning for HS400.
> 
> As described in patch 4, re-tuning for HS400 has to be done
> in HS200 mode, but there is no support for that, so re-tuning
> needs to be disabled until support is added.
> 
> Changes in V2:
> 
> 	Added "mmc: sdhci: Simplify use of tuning timer"
> 	patch.
> 
> 	Changed "mmc: sdhci: Disable re-tuning for HS400"
> 	so that the tuning timer is not started instead
> 	of stopping it when HS400 mode is selected.
> 
> 
> Adrian Hunter (4):
>       mmc: sdhci: Tuning should not change max_blk_count
>       mmc: sdhci: Add out_unlock to sdhci_execute_tuning
>       mmc: sdhci: Simplify use of tuning timer
>       mmc: sdhci: Disable re-tuning for HS400

For patch 1-3: Reviewed-by: Aaron Lu <aaron.lu@intel.com>

Thanks,
Aaron

> 
>  drivers/mmc/host/sdhci.c  | 66 ++++++++++++++++++++++++++++++-----------------
>  include/linux/mmc/sdhci.h |  1 +
>  2 files changed, 43 insertions(+), 24 deletions(-)
> 
> 
> Regards
> Adrian
> 

Re: [PATCH V2 0/4] mmc: sdhci: Disable re-tuning for HS400

[Adrian Hunter]

On 05/12/14 19:25, Adrian Hunter wrote:
> Hi
> 
> Here is V2 of patches to disable re-tuning for HS400.
> 
> As described in patch 4, re-tuning for HS400 has to be done
> in HS200 mode, but there is no support for that, so re-tuning
> needs to be disabled until support is added.
> 
> Changes in V2:
> 
> 	Added "mmc: sdhci: Simplify use of tuning timer"
> 	patch.
> 
> 	Changed "mmc: sdhci: Disable re-tuning for HS400"
> 	so that the tuning timer is not started instead
> 	of stopping it when HS400 mode is selected.
> 
> 
> Adrian Hunter (4):
>       mmc: sdhci: Tuning should not change max_blk_count
>       mmc: sdhci: Add out_unlock to sdhci_execute_tuning
>       mmc: sdhci: Simplify use of tuning timer
>       mmc: sdhci: Disable re-tuning for HS400
> 
>  drivers/mmc/host/sdhci.c  | 66 ++++++++++++++++++++++++++++++-----------------
>  include/linux/mmc/sdhci.h |  1 +
>  2 files changed, 43 insertions(+), 24 deletions(-)

Ulf, will you take these?

Re: [PATCH V2 0/4] mmc: sdhci: Disable re-tuning for HS400

[Adrian Hunter]

On 19/12/14 16:06, Adrian Hunter wrote:
> On 05/12/14 19:25, Adrian Hunter wrote:
>> Hi
>>
>> Here is V2 of patches to disable re-tuning for HS400.
>>
>> As described in patch 4, re-tuning for HS400 has to be done
>> in HS200 mode, but there is no support for that, so re-tuning
>> needs to be disabled until support is added.
>>
>> Changes in V2:
>>
>> 	Added "mmc: sdhci: Simplify use of tuning timer"
>> 	patch.
>>
>> 	Changed "mmc: sdhci: Disable re-tuning for HS400"
>> 	so that the tuning timer is not started instead
>> 	of stopping it when HS400 mode is selected.
>>
>>
>> Adrian Hunter (4):
>>       mmc: sdhci: Tuning should not change max_blk_count
>>       mmc: sdhci: Add out_unlock to sdhci_execute_tuning
>>       mmc: sdhci: Simplify use of tuning timer
>>       mmc: sdhci: Disable re-tuning for HS400
>>
>>  drivers/mmc/host/sdhci.c  | 66 ++++++++++++++++++++++++++++++-----------------
>>  include/linux/mmc/sdhci.h |  1 +
>>  2 files changed, 43 insertions(+), 24 deletions(-)
> 
> Ulf, will you take these?

Ping?

Given that HS400 was added to sdhci in 3.19 they could go in now as fixes?

Note Aaron reviewed patches 1-3.

Re: [PATCH V2 0/4] mmc: sdhci: Disable re-tuning for HS400

[Ulf Hansson]

On 30 December 2014 at 10:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 19/12/14 16:06, Adrian Hunter wrote:
>> On 05/12/14 19:25, Adrian Hunter wrote:
>>> Hi
>>>
>>> Here is V2 of patches to disable re-tuning for HS400.
>>>
>>> As described in patch 4, re-tuning for HS400 has to be done
>>> in HS200 mode, but there is no support for that, so re-tuning
>>> needs to be disabled until support is added.
>>>
>>> Changes in V2:
>>>
>>>      Added "mmc: sdhci: Simplify use of tuning timer"
>>>      patch.
>>>
>>>      Changed "mmc: sdhci: Disable re-tuning for HS400"
>>>      so that the tuning timer is not started instead
>>>      of stopping it when HS400 mode is selected.
>>>
>>>
>>> Adrian Hunter (4):
>>>       mmc: sdhci: Tuning should not change max_blk_count
>>>       mmc: sdhci: Add out_unlock to sdhci_execute_tuning
>>>       mmc: sdhci: Simplify use of tuning timer
>>>       mmc: sdhci: Disable re-tuning for HS400
>>>
>>>  drivers/mmc/host/sdhci.c  | 66 ++++++++++++++++++++++++++++++-----------------
>>>  include/linux/mmc/sdhci.h |  1 +
>>>  2 files changed, 43 insertions(+), 24 deletions(-)
>>
>> Ulf, will you take these?
>
> Ping?
>
> Given that HS400 was added to sdhci in 3.19 they could go in now as fixes?
>
> Note Aaron reviewed patches 1-3.

Thanks! Applied for fixes with Aaron's reviewed by tag. And sorry for the delay.

Kind regards
Uffe