Skip to contentSkip to conversation

mmc: sdhci: Disable re-tuning for HS400

PATCH Applied Adrian Hunter 3 participants 9 messages Dec 5, 2014 – Dec 30, 2014

Subsystems (2)
lore
Share & tools
lore

Adrian Hunter

author maintainer
#0 lore raw

v2 cover mmc: sdhci: Disable re-tuning for HS400

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

author maintainer
#1 lore raw

PATCH v2 1/4 mmc: sdhci: Tuning should not change max_blk_count

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.

---
 drivers/mmc/host/sdhci.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/mmc/host/sdhci.c +3 -6
index 73de62a..b3d68e0 100644
@@ -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;
 
Signature · 1 line
1.9.1
author maintainer
#2 lore raw

PATCH v2 2/4 mmc: sdhci: Add out_unlock to sdhci_execute_tuning

In reply to Adrian Hunter

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

---
 drivers/mmc/host/sdhci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/mmc/host/sdhci.c +2 -3
index b3d68e0..2efa7fe 100644
@@ -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);
 
Signature · 1 line
1.9.1
author maintainer
#3 lore raw

PATCH v2 3/4 mmc: sdhci: Simplify use of tuning timer

In reply to 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.

---
 drivers/mmc/host/sdhci.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)
drivers/mmc/host/sdhci.c +8 -15
index 2efa7fe..ff0fe90 100644
@@ -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);
 	}
 
 	/*
Signature · 1 line
1.9.1
author maintainer
#4 lore raw

PATCH v2 4/4 mmc: sdhci: Disable re-tuning for HS400

In reply to 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.

---
 drivers/mmc/host/sdhci.c  | 29 +++++++++++++++++++++++++++++
 include/linux/mmc/sdhci.h |  1 +
 2 files changed, 30 insertions(+)
drivers/mmc/host/sdhci.c +29 -0
index ff0fe90..bfde165 100644
@@ -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,
include/linux/mmc/sdhci.h +1 -0
index 375af80..f767a0d 100644
@@ -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 */
 
Signature · 1 line
1.9.1
#5 lore raw
In reply to Adrian Hunter
Quoted 18 lines · 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.

Quoted 5 lines

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 <[email protected]>

Thanks,
Aaron

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

Regards
Adrian

author maintainer
#6 lore raw
In reply to Adrian Hunter
Quoted 24 lines · 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

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

Ulf, will you take these?

author maintainer
#7 lore raw
Quoted 25 lines · Adrian Hunter
Quoted 24 lines · 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

Quoted 5 lines
Quoted 3 lines
 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.

Applied
#8 lore raw
Quoted 30 lines · Adrian Hunter
Quoted 29 lines · Adrian Hunter
Quoted 27 lines · 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(-)
Quoted 7 lines
Quoted 1 line

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