[PATCH V5 00/15] mmc: host: Add facility to support re-tuning

[PATCH V5 00/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

Hi

Here is V5 of some patches to move re-tuning support
out of sdhci and into the core, and add support for HS400
re-tuning.

Currently sdhci does re-tuning transparently by
calling sdhci_execute_tuning() from its ->request()
function.

The problem with HS400 re-tuning is that it must be
done in HS200 mode. That means using switch commands
and making ios changes. That means it potentially
conflicts with other command sequences. The new
re-tuning support accomodates that.


Changes in V5:

    mmc: host: Add facility to support re-tuning
	Make mmc_retune_enable() / mmc_retune_disable()
	only called from core.

    mmc: core: Enable / disable re-tuning
	Replaces mmc: core: Disable re-tuning when card is no longer initialized
	Enables re-tuning when tuning is executed

    mmc: sdhci: Change to new way of doing re-tuning
	Set host->retune_period instead of enabling re-tuning.

Changes in V4:

    These patches now depend on Ulf's patch:
	mmc: core: Enable runtime PM management of host devices

    mmc: host: Add facility to support re-tuning

	Assume mmc_claim_host() runtime resumes the host
	controller so there are no races with runtime pm.
	Consequently remove now un-needed re-tuning host
	operations.

    mmc: core: Add support for re-tuning before each request

	Call mmc_retune() prior to ->request()

    mmc: sdhci: Change to new way of doing re-tuning

	Updated to reflect the changes above.

Changes in V3:

    mmc: host: Add facility to support re-tuning

        Add host->retune_now flag so re-tuning can be
        started in the host drivers ->request()
        function to avoid racing with Runtime PM.

        Add host operations to let the host know when
        re-tuning is held, for eaxmple, to enable
        synchronization with runtime suspend / resume.

        Ensure functions are exported.

    mmc: core: Add support for re-tuning before each request
        Updated to reflect the change above.

    mmc: core: Check re-tuning before retrying
        Updated to reflect the change above.

    mmc: core: Hold re-tuning during switch commands
        Updated to reflect the change above.

    mmc: core: Hold re-tuning during erase commands
        Updated to reflect the change above.

    mmc: core: Hold re-tuning while bkops ongoing
        Updated to reflect the change above.

    mmc: core: Add support for HS400 re-tuning
        Updated and as already sent separately as V3:
        Remember to mmc_set_bus_speed(card) in mmc_hs400_to_hs200()

    mmc: sdhci: Change to new way of doing re-tuning
        Call mmc_retune() from ->request() function to
        avoid racing with Runtime PM. And implement
        hold_tuning / release_tuning operations to prevent
        runtime suspend while re-tuning is held.

    mmc: block: Retry errored data requests when re-tuning is needed
        Updated and as already sent separately as V3:
        Only retry when there is an error

Changes in V2:

    Added support to the block driver for re-tuning
    and retrying after a CRC error. The host driver
    is left to decide when an error indicates re-tuning
    is needed. The block driver will retry a data request
    once if re-tuning is flagged as needed.

    SDIO drivers need not be aware of re-tuning because
    retrying will anyway cause re-tuning when re-tuning
    is flagged as needed. Nevertheless SDIO drivers could
    use the need_retune flag to instigate a retry when
    otherwise they might not have.

    mmc: core: Simplify by adding mmc_execute_tuning()
        Dropped because it has been applied

    mmc: host: Add facility to support re-tuning
        Renamed mmc_retune_retry() to mmc_retune_recheck()
        to better reflect what it does.

    mmc: core: Move mmc_card_removed() into mmc_start_request()
        Dropped because it has been applied

    mmc: core: Add support for re-tuning before each request
        Fixed un-balanced re-tune hold / release

    mmc: sdhci: Always init buf_ready_int
        Dropped because it has been applied

    mmc: core: Separate out the mmc_switch status check so it can be re-used
        New patch

    mmc: core: Add support for HS400 re-tuning
        It was found that that the original code was not reliable
        after a CRC error. The problem was that the CMD13 after a
        switch was faiing. So the code was changed to check the
        switch status *after* changing the I/O state to match the
        switch i.e. the new I/O state is the correct one to use
        after a switch.

    mmc: sdhci: Flag re-tuning is needed on CRC or End-Bit errors
        New patch

    mmc: block: Check re-tuning in the recovery path
        New patch

    mmc: block: Retry data requests when re-tuning is needed
        New patch

    mmc: core: Don't print reset warning if reset is not supported
        New patch


Adrian Hunter (15):
      mmc: host: Add facility to support re-tuning
      mmc: core: Enable / disable re-tuning
      mmc: core: Add support for re-tuning before each request
      mmc: core: Check re-tuning before retrying
      mmc: core: Hold re-tuning during switch commands
      mmc: core: Hold re-tuning during erase commands
      mmc: core: Hold re-tuning while bkops ongoing
      mmc: mmc: Comment that callers need to hold re-tuning if the card is put to sleep
      mmc: core: Separate out the mmc_switch status check so it can be re-used
      mmc: core: Add support for HS400 re-tuning
      mmc: sdhci: Change to new way of doing re-tuning
      mmc: sdhci: Flag re-tuning is needed on CRC or End-Bit errors
      mmc: block: Check re-tuning in the recovery path
      mmc: block: Retry errored data requests when re-tuning is needed
      mmc: core: Don't print reset warning if reset is not supported

 drivers/mmc/card/block.c   |  14 ++++-
 drivers/mmc/card/queue.h   |   1 +
 drivers/mmc/core/core.c    |  51 +++++++++++++++---
 drivers/mmc/core/core.h    |   2 +
 drivers/mmc/core/host.c    |  88 +++++++++++++++++++++++++++++++
 drivers/mmc/core/mmc.c     |  89 ++++++++++++++++++++++++++++++++
 drivers/mmc/core/mmc_ops.c |  43 ++++++++++------
 drivers/mmc/core/mmc_ops.h |   1 +
 drivers/mmc/host/sdhci.c   | 126 ++++++++-------------------------------------
 drivers/mmc/host/sdhci.h   |   3 --
 include/linux/mmc/host.h   |  33 ++++++++++++
 11 files changed, 319 insertions(+), 132 deletions(-)


Regards
Adrian

[PATCH V5 01/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

Currently, there is core support for tuning during
initialization. There can also be a need to re-tune
periodically (e.g. sdhci) or to re-tune after the
host controller is powered off (e.g. after PM
runtime suspend / resume) or to re-tune in response
to CRC errors.

The main requirements for re-tuning are:
  - ability to enable / disable re-tuning
  - ability to flag that re-tuning is needed
  - ability to re-tune before any request
  - ability to hold off re-tuning if the card is busy
  - ability to hold off re-tuning if re-tuning is in
  progress
  - ability to run a re-tuning timer

To support those requirements 7 members are added to struct
mmc_host:

  unsigned int		can_retune:1;	/* re-tuning can be used */
  unsigned int		doing_retune:1;	/* re-tuning in progress */
  unsigned int		retune_now:1;   /* do re-tuning at next req */
  int			need_retune;	/* re-tuning is needed */
  int			hold_retune;	/* hold off re-tuning */
  unsigned int		retune_period;  /* re-tuning period in secs */
  struct timer_list	retune_timer;	/* for periodic re-tuning */

need_retune is an integer so it can be set without needing
synchronization. hold_retune is a integer to allow nesting.

Various simple functions are provided to set / clear those
variables.

Subsequent patches take those functions into use.

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

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 8be0df7..271986c 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,76 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
 
 #endif
 
+void mmc_retune_enable(struct mmc_host *host)
+{
+	host->can_retune = 1;
+	if (host->retune_period)
+		mod_timer(&host->retune_timer,
+			  jiffies + host->retune_period * HZ);
+}
+
+void mmc_retune_disable(struct mmc_host *host)
+{
+	host->can_retune = 0;
+	del_timer_sync(&host->retune_timer);
+	host->retune_now = 0;
+	host->need_retune = 0;
+}
+
+void mmc_retune_timer_stop(struct mmc_host *host)
+{
+	del_timer_sync(&host->retune_timer);
+}
+EXPORT_SYMBOL(mmc_retune_timer_stop);
+
+void mmc_retune_hold(struct mmc_host *host)
+{
+	if (!host->hold_retune)
+		host->retune_now = 1;
+	host->hold_retune += 1;
+}
+EXPORT_SYMBOL(mmc_retune_hold);
+
+void mmc_retune_release(struct mmc_host *host)
+{
+	if (host->hold_retune)
+		host->hold_retune -= 1;
+	else
+		WARN_ON(1);
+}
+EXPORT_SYMBOL(mmc_retune_release);
+
+int mmc_retune(struct mmc_host *host)
+{
+	int err;
+
+	if (host->retune_now)
+		host->retune_now = 0;
+	else
+		return 0;
+
+	if (!host->need_retune || host->doing_retune || !host->card)
+		return 0;
+
+	host->need_retune = 0;
+
+	host->doing_retune = 1;
+
+	err = mmc_execute_tuning(host->card);
+
+	host->doing_retune = 0;
+
+	return err;
+}
+EXPORT_SYMBOL(mmc_retune);
+
+static void mmc_retune_timer(unsigned long data)
+{
+	struct mmc_host *host = (struct mmc_host *)data;
+
+	mmc_retune_needed(host);
+}
+
 /**
  *	mmc_of_parse() - parse host's device-tree node
  *	@host: host whose node should be parsed.
@@ -504,6 +574,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 #ifdef CONFIG_PM
 	host->pm_notify.notifier_call = mmc_pm_notify;
 #endif
+	setup_timer(&host->retune_timer, mmc_retune_timer, (unsigned long)host);
 
 	/*
 	 * By default, hosts do not support SGIO or large requests.
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index b5bedae..e9f693e 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -12,6 +12,7 @@
 
 #include <linux/leds.h>
 #include <linux/mutex.h>
+#include <linux/timer.h>
 #include <linux/sched.h>
 #include <linux/device.h>
 #include <linux/fault-inject.h>
@@ -321,10 +322,18 @@ struct mmc_host {
 #ifdef CONFIG_MMC_DEBUG
 	unsigned int		removed:1;	/* host is being removed */
 #endif
+	unsigned int		can_retune:1;	/* re-tuning can be used */
+	unsigned int		doing_retune:1;	/* re-tuning in progress */
+	unsigned int		retune_now:1;	/* do re-tuning at next req */
 
 	int			rescan_disable;	/* disable card detection */
 	int			rescan_entered;	/* used with nonremovable devices */
 
+	int			need_retune;	/* re-tuning is needed */
+	int			hold_retune;	/* hold off re-tuning */
+	unsigned int		retune_period;	/* re-tuning period in secs */
+	struct timer_list	retune_timer;	/* for periodic re-tuning */
+
 	bool			trigger_card_event; /* card_event necessary */
 
 	struct mmc_card		*card;		/* device attached to this host */
@@ -513,4 +522,28 @@ static inline bool mmc_card_hs400(struct mmc_card *card)
 	return card->host->ios.timing == MMC_TIMING_MMC_HS400;
 }
 
+void mmc_retune_enable(struct mmc_host *host);
+void mmc_retune_disable(struct mmc_host *host);
+void mmc_retune_timer_stop(struct mmc_host *host);
+void mmc_retune_hold(struct mmc_host *host);
+void mmc_retune_release(struct mmc_host *host);
+int mmc_retune(struct mmc_host *host);
+
+static inline void mmc_retune_needed(struct mmc_host *host)
+{
+	if (host->can_retune)
+		host->need_retune = 1;
+}
+
+static inline void mmc_retune_not_needed(struct mmc_host *host)
+{
+	host->need_retune = 0;
+}
+
+static inline void mmc_retune_recheck(struct mmc_host *host)
+{
+	if (host->hold_retune <= 1)
+		host->retune_now = 1;
+}
+
 #endif /* LINUX_MMC_HOST_H */
-- 
1.9.1

[PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

Enable re-tuning when tuning is executed and
disable re-tuning when card is no longer initialized.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c296bc0..dd43f9b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
 
 	if (err)
 		pr_err("%s: tuning execution failed\n", mmc_hostname(host));
+	else
+		mmc_retune_enable(host);
 
 	return err;
 }
@@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
  */
 void mmc_set_initial_state(struct mmc_host *host)
 {
+	mmc_retune_disable(host);
+
 	if (mmc_host_is_spi(host))
 		host->ios.chip_select = MMC_CS_HIGH;
 	else
-- 
1.9.1

[PATCH V5 03/15] mmc: core: Add support for re-tuning before each request

[Adrian Hunter]

At the start of each request, re-tune if needed and
then hold off re-tuning again until the request is done.

Note that though there is one function that starts
requests (mmc_start_request) there are two that wait for
the request to be done (mmc_wait_for_req_done and
mmc_wait_for_data_req_done).  Also note that
mmc_wait_for_data_req_done can return even when the
request is not done (which allows the block driver
to prepare a newly arrived request while still
waiting for the previous request).

This patch ensures re-tuning is held for the duration
of a request.  Subsequent patches will also hold
re-tuning at other times when it might cause a
conflict.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/core.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dd43f9b..a9936cb 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -186,12 +186,29 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
 
 EXPORT_SYMBOL(mmc_request_done);
 
+static void __mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
+{
+	int err;
+
+	/* Assumes host controller has been runtime resumed by mmc_claim_host */
+	err = mmc_retune(host);
+	if (err) {
+		mrq->cmd->error = err;
+		mmc_request_done(host, mrq);
+		return;
+	}
+
+	host->ops->request(host, mrq);
+}
+
 static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 {
 #ifdef CONFIG_MMC_DEBUG
 	unsigned int i, sz;
 	struct scatterlist *sg;
 #endif
+	mmc_retune_hold(host);
+
 	if (mmc_card_removed(host->card))
 		return -ENOMEDIUM;
 
@@ -252,7 +269,7 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 	}
 	mmc_host_clk_hold(host);
 	led_trigger_event(host->led, LED_FULL);
-	host->ops->request(host, mrq);
+	__mmc_start_request(host, mrq);
 
 	return 0;
 }
@@ -422,17 +439,16 @@ static int mmc_wait_for_data_req_done(struct mmc_host *host,
 					cmd->opcode, cmd->error);
 				cmd->retries--;
 				cmd->error = 0;
-				host->ops->request(host, mrq);
+				__mmc_start_request(host, mrq);
 				continue; /* wait for done/new event again */
 			}
 		} else if (context_info->is_new_req) {
 			context_info->is_new_req = false;
-			if (!next_req) {
-				err = MMC_BLK_NEW_REQUEST;
-				break; /* return err */
-			}
+			if (!next_req)
+				return MMC_BLK_NEW_REQUEST;
 		}
 	}
+	mmc_retune_release(host);
 	return err;
 }
 
@@ -471,8 +487,10 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
 			 mmc_hostname(host), cmd->opcode, cmd->error);
 		cmd->retries--;
 		cmd->error = 0;
-		host->ops->request(host, mrq);
+		__mmc_start_request(host, mrq);
 	}
+
+	mmc_retune_release(host);
 }
 
 /**
-- 
1.9.1

[PATCH V5 04/15] mmc: core: Check re-tuning before retrying

[Adrian Hunter]

Possibly a command is failing because re-tuning is needed.
Use mmc_retune_recheck() to check re-tuning. At that point
re-tuning is held, at least by the request, so
mmc_retune_recheck() flags host->retune_now if the hold
count is 1.

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

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a9936cb..08bf7e3 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -434,6 +434,7 @@ static int mmc_wait_for_data_req_done(struct mmc_host *host,
 							    host->areq);
 				break; /* return err */
 			} else {
+				mmc_retune_recheck(host);
 				pr_info("%s: req failed (CMD%u): %d, retrying...\n",
 					mmc_hostname(host),
 					cmd->opcode, cmd->error);
@@ -483,6 +484,8 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
 		    mmc_card_removed(host->card))
 			break;
 
+		mmc_retune_recheck(host);
+
 		pr_debug("%s: req failed (CMD%u): %d, retrying...\n",
 			 mmc_hostname(host), cmd->opcode, cmd->error);
 		cmd->retries--;
-- 
1.9.1

[PATCH V5 05/15] mmc: core: Hold re-tuning during switch commands

[Adrian Hunter]

Hold re-tuning during switch commands to prevent
it from conflicting with the busy state or the CMD13
verification.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/mmc_ops.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0ea042d..f67b0fd 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -474,6 +474,8 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 	u32 status = 0;
 	bool use_r1b_resp = use_busy_signal;
 
+	mmc_retune_hold(host);
+
 	/*
 	 * If the cmd timeout and the max_busy_timeout of the host are both
 	 * specified, let's validate them. A failure means we need to prevent
@@ -506,11 +508,11 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 
 	err = mmc_wait_for_cmd(host, &cmd, MMC_CMD_RETRIES);
 	if (err)
-		return err;
+		goto out;
 
 	/* No need to check card status in case of unblocking command */
 	if (!use_busy_signal)
-		return 0;
+		goto out;
 
 	/*
 	 * CRC errors shall only be ignored in cases were CMD13 is used to poll
@@ -529,7 +531,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		if (send_status) {
 			err = __mmc_send_status(card, &status, ignore_crc);
 			if (err)
-				return err;
+				goto out;
 		}
 		if ((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
 			break;
@@ -543,29 +545,36 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		 */
 		if (!send_status) {
 			mmc_delay(timeout_ms);
-			return 0;
+			goto out;
 		}
 
 		/* Timeout if the device never leaves the program state. */
 		if (time_after(jiffies, timeout)) {
 			pr_err("%s: Card stuck in programming state! %s\n",
 				mmc_hostname(host), __func__);
-			return -ETIMEDOUT;
+			err = -ETIMEDOUT;
+			goto out;
 		}
 	} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
 
 	if (mmc_host_is_spi(host)) {
-		if (status & R1_SPI_ILLEGAL_COMMAND)
-			return -EBADMSG;
+		if (status & R1_SPI_ILLEGAL_COMMAND) {
+			err = -EBADMSG;
+			goto out;
+		}
 	} else {
 		if (status & 0xFDFFA000)
 			pr_warn("%s: unexpected status %#x after switch\n",
 				mmc_hostname(host), status);
-		if (status & R1_SWITCH_ERROR)
-			return -EBADMSG;
+		if (status & R1_SWITCH_ERROR) {
+			err = -EBADMSG;
+			goto out;
+		}
 	}
+out:
+	mmc_retune_release(host);
 
-	return 0;
+	return err;
 }
 EXPORT_SYMBOL_GPL(__mmc_switch);
 
-- 
1.9.1

[PATCH V5 06/15] mmc: core: Hold re-tuning during erase commands

[Adrian Hunter]

Hold re-tuning during erase commands to prevent
it from conflicting with the sequence of commands.

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

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 08bf7e3..dbd7a77 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1995,6 +1995,8 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from,
 	unsigned long timeout;
 	int err;
 
+	mmc_retune_hold(card->host);
+
 	/*
 	 * qty is used to calculate the erase timeout which depends on how many
 	 * erase groups (or allocation units in SD terminology) are affected.
@@ -2098,6 +2100,7 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from,
 	} while (!(cmd.resp[0] & R1_READY_FOR_DATA) ||
 		 (R1_CURRENT_STATE(cmd.resp[0]) == R1_STATE_PRG));
 out:
+	mmc_retune_release(card->host);
 	return err;
 }
 
-- 
1.9.1

[PATCH V5 07/15] mmc: core: Hold re-tuning while bkops ongoing

[Adrian Hunter]

Hold re-tuning during bkops to prevent
it from conflicting with the busy state.

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

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dbd7a77..4a42174 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -316,6 +316,8 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
 	} else {
 		timeout = 0;
 		use_busy_signal = false;
+		/* Hold re-tuning for ongoing bkops */
+		mmc_retune_hold(card->host);
 	}
 
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
@@ -324,6 +326,9 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
 	if (err) {
 		pr_warn("%s: Error %d starting bkops\n",
 			mmc_hostname(card->host), err);
+		/* bkops not ongoing, so release re-tuning */
+		if (!use_busy_signal)
+			mmc_retune_release(card->host);
 		goto out;
 	}
 
@@ -749,6 +754,7 @@ int mmc_stop_bkops(struct mmc_card *card)
 	 */
 	if (!err || (err == -EINVAL)) {
 		mmc_card_clr_doing_bkops(card);
+		mmc_retune_release(card->host);
 		err = 0;
 	}
 
-- 
1.9.1

[PATCH V5 08/15] mmc: mmc: Comment that callers need to hold re-tuning if the card is put to sleep

[Adrian Hunter]

Currently "mmc sleep" is only used before power off and
is not paired with waking up.  If that ever changed,
re-tuning might need to be held, so add a comment for that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index c84131e..53a9cb3 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1504,6 +1504,7 @@ static int mmc_can_sleep(struct mmc_card *card)
 	return (card && card->ext_csd.rev >= 3);
 }
 
+/* If necessary, callers must hold re-tuning */
 static int mmc_sleep(struct mmc_host *host)
 {
 	struct mmc_command cmd = {0};
-- 
1.9.1

[PATCH V5 09/15] mmc: core: Separate out the mmc_switch status check so it can be re-used

[Adrian Hunter]

Make a separate function to do the mmc_switch status check
so it can be re-used. This is preparation for adding support
for HS400 re-tuning.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/mmc_ops.c | 30 ++++++++++++++++--------------
 drivers/mmc/core/mmc_ops.h |  1 +
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index f67b0fd..02be77e 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -449,6 +449,21 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
 	return err;
 }
 
+int mmc_switch_status_error(struct mmc_host *host, u32 status)
+{
+	if (mmc_host_is_spi(host)) {
+		if (status & R1_SPI_ILLEGAL_COMMAND)
+			return -EBADMSG;
+	} else {
+		if (status & 0xFDFFA000)
+			pr_warn("%s: unexpected status %#x after switch\n",
+				mmc_hostname(host), status);
+		if (status & R1_SWITCH_ERROR)
+			return -EBADMSG;
+	}
+	return 0;
+}
+
 /**
  *	__mmc_switch - modify EXT_CSD register
  *	@card: the MMC card associated with the data transfer
@@ -557,20 +572,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		}
 	} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
 
-	if (mmc_host_is_spi(host)) {
-		if (status & R1_SPI_ILLEGAL_COMMAND) {
-			err = -EBADMSG;
-			goto out;
-		}
-	} else {
-		if (status & 0xFDFFA000)
-			pr_warn("%s: unexpected status %#x after switch\n",
-				mmc_hostname(host), status);
-		if (status & R1_SWITCH_ERROR) {
-			err = -EBADMSG;
-			goto out;
-		}
-	}
+	err = mmc_switch_status_error(host, status);
 out:
 	mmc_retune_release(host);
 
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 6f4b00e..f498f9a 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -27,6 +27,7 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc);
 int mmc_bus_test(struct mmc_card *card, u8 bus_width);
 int mmc_send_hpi_cmd(struct mmc_card *card, u32 *status);
 int mmc_can_ext_csd(struct mmc_card *card);
+int mmc_switch_status_error(struct mmc_host *host, u32 status);
 
 #endif
 
-- 
1.9.1

[PATCH V5 10/15] mmc: core: Add support for HS400 re-tuning

[Adrian Hunter]

HS400 re-tuning must be done in HS200 mode. Add
the ability to switch from HS400 mode to HS200
mode before re-tuning and switch back to HS400
after re-tuning.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/core.h |  2 ++
 drivers/mmc/core/host.c | 17 ++++++++++
 drivers/mmc/core/mmc.c  | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+)

diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index cfba3c0..e6f2de7 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -88,6 +88,8 @@ void mmc_remove_card_debugfs(struct mmc_card *card);
 void mmc_init_context_info(struct mmc_host *host);
 
 int mmc_execute_tuning(struct mmc_card *card);
+int mmc_hs200_to_hs400(struct mmc_card *card);
+int mmc_hs400_to_hs200(struct mmc_card *card);
 
 #endif
 
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 271986c..f480cb7 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -342,6 +342,7 @@ EXPORT_SYMBOL(mmc_retune_release);
 
 int mmc_retune(struct mmc_host *host)
 {
+	bool return_to_hs400 = false;
 	int err;
 
 	if (host->retune_now)
@@ -356,8 +357,24 @@ int mmc_retune(struct mmc_host *host)
 
 	host->doing_retune = 1;
 
+	if (host->ios.timing == MMC_TIMING_MMC_HS400) {
+		err = mmc_hs400_to_hs200(host->card);
+		if (err)
+			goto out;
+
+		return_to_hs400 = true;
+
+		if (host->ops->prepare_hs400_tuning)
+			host->ops->prepare_hs400_tuning(host, &host->ios);
+	}
+
 	err = mmc_execute_tuning(host->card);
+	if (err)
+		goto out;
 
+	if (return_to_hs400)
+		err = mmc_hs200_to_hs400(host->card);
+out:
 	host->doing_retune = 0;
 
 	return err;
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 53a9cb3..5a52b26 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1091,6 +1091,94 @@ static int mmc_select_hs400(struct mmc_card *card)
 	return 0;
 }
 
+int mmc_hs200_to_hs400(struct mmc_card *card)
+{
+	return mmc_select_hs400(card);
+}
+
+/* Caller must hold re-tuning */
+static int mmc_switch_status(struct mmc_card *card)
+{
+	u32 status;
+	int err;
+
+	err = mmc_send_status(card, &status);
+	if (err)
+		return err;
+
+	return mmc_switch_status_error(card->host, status);
+}
+
+int mmc_hs400_to_hs200(struct mmc_card *card)
+{
+	struct mmc_host *host = card->host;
+	bool send_status = true;
+	unsigned int max_dtr;
+	int err;
+
+	if (host->caps & MMC_CAP_WAIT_WHILE_BUSY)
+		send_status = false;
+
+	/* Reduce frequency to HS */
+	max_dtr = card->ext_csd.hs_max_dtr;
+	mmc_set_clock(host, max_dtr);
+
+	/* Switch HS400 to HS DDR */
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
+			   EXT_CSD_TIMING_HS, card->ext_csd.generic_cmd6_time,
+			   true, send_status, true);
+	if (err)
+		goto out_err;
+
+	mmc_set_timing(host, MMC_TIMING_MMC_DDR52);
+
+	if (!send_status) {
+		err = mmc_switch_status(card);
+		if (err)
+			goto out_err;
+	}
+
+	/* Switch HS DDR to HS */
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
+			   EXT_CSD_BUS_WIDTH_8, card->ext_csd.generic_cmd6_time,
+			   true, send_status, true);
+	if (err)
+		goto out_err;
+
+	mmc_set_timing(host, MMC_TIMING_MMC_HS);
+
+	if (!send_status) {
+		err = mmc_switch_status(card);
+		if (err)
+			goto out_err;
+	}
+
+	/* Switch HS to HS200 */
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
+			   EXT_CSD_TIMING_HS200,
+			   card->ext_csd.generic_cmd6_time, true, send_status,
+			   true);
+	if (err)
+		goto out_err;
+
+	mmc_set_timing(host, MMC_TIMING_MMC_HS200);
+
+	if (!send_status) {
+		err = mmc_switch_status(card);
+		if (err)
+			goto out_err;
+	}
+
+	mmc_set_bus_speed(card);
+
+	return 0;
+
+out_err:
+	pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
+	       __func__, err);
+	return err;
+}
+
 /*
  * For device supporting HS200 mode, the following sequence
  * should be done before executing the tuning process.
-- 
1.9.1

[PATCH V5 11/15] mmc: sdhci: Change to new way of doing re-tuning

[Adrian Hunter]

Make use of mmc core support for re-tuning instead
of doing it all in the sdhci driver.

This patch also changes to flag the need for re-tuning
always after runtime suspend when tuning has been used
at initialization. Previously it was only done if
the re-tuning timer was in use.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 112 ++++++-----------------------------------------
 drivers/mmc/host/sdhci.h |   3 --
 2 files changed, 13 insertions(+), 102 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c80287a..b345844 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -52,7 +52,6 @@ static void sdhci_finish_data(struct sdhci_host *);
 
 static void sdhci_finish_command(struct sdhci_host *);
 static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode);
-static void sdhci_tuning_timer(unsigned long data);
 static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable);
 static int sdhci_pre_dma_transfer(struct sdhci_host *host,
 					struct mmc_data *data,
@@ -254,17 +253,6 @@ static void sdhci_init(struct sdhci_host *host, int soft)
 static void sdhci_reinit(struct sdhci_host *host)
 {
 	sdhci_init(host, 0);
-	/*
-	 * Retuning stuffs are affected by different cards inserted and only
-	 * applicable to UHS-I cards. So reset these fields to their initial
-	 * value when card is removed.
-	 */
-	if (host->flags & SDHCI_USING_RETUNING_TIMER) {
-		host->flags &= ~SDHCI_USING_RETUNING_TIMER;
-
-		del_timer_sync(&host->tuning_timer);
-		host->flags &= ~SDHCI_NEEDS_RETUNING;
-	}
 	sdhci_enable_card_detection(host);
 }
 
@@ -1353,7 +1341,6 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	struct sdhci_host *host;
 	int present;
 	unsigned long flags;
-	u32 tuning_opcode;
 
 	host = mmc_priv(mmc);
 
@@ -1387,39 +1374,6 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		host->mrq->cmd->error = -ENOMEDIUM;
 		tasklet_schedule(&host->finish_tasklet);
 	} else {
-		u32 present_state;
-
-		present_state = sdhci_readl(host, SDHCI_PRESENT_STATE);
-		/*
-		 * Check if the re-tuning timer has already expired and there
-		 * is no on-going data transfer and DAT0 is not busy. If so,
-		 * we need to execute tuning procedure before sending command.
-		 */
-		if ((host->flags & SDHCI_NEEDS_RETUNING) &&
-		    !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ)) &&
-		    (present_state & SDHCI_DATA_0_LVL_MASK)) {
-			if (mmc->card) {
-				/* eMMC uses cmd21 but sd and sdio use cmd19 */
-				tuning_opcode =
-					mmc->card->type == MMC_TYPE_MMC ?
-					MMC_SEND_TUNING_BLOCK_HS200 :
-					MMC_SEND_TUNING_BLOCK;
-
-				/* Here we need to set the host->mrq to NULL,
-				 * in case the pending finish_tasklet
-				 * finishes it incorrectly.
-				 */
-				host->mrq = NULL;
-
-				spin_unlock_irqrestore(&host->lock, flags);
-				sdhci_execute_tuning(mmc, tuning_opcode);
-				spin_lock_irqsave(&host->lock, flags);
-
-				/* Restore original mmc_request structure */
-				host->mrq = mrq;
-			}
-		}
-
 		if (mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23))
 			sdhci_send_command(host, mrq->sbc);
 		else
@@ -2065,23 +2019,18 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	}
 
 out:
-	host->flags &= ~SDHCI_NEEDS_RETUNING;
-
 	if (tuning_count) {
-		host->flags |= SDHCI_USING_RETUNING_TIMER;
-		mod_timer(&host->tuning_timer, jiffies + tuning_count * HZ);
+		/*
+		 * In case tuning fails, host controllers which support
+		 * re-tuning can try tuning again at a later time, when the
+		 * re-tuning timer expires.  So for these controllers, we
+		 * return 0. Since there might be other controllers who do not
+		 * have this capability, we return error for them.
+		 */
+		err = 0;
 	}
 
-	/*
-	 * In case tuning fails, host controllers which support re-tuning can
-	 * try tuning again at a later time, when the re-tuning timer expires.
-	 * So for these controllers, we return 0. Since there might be other
-	 * controllers who do not have this capability, we return error for
-	 * them. SDHCI_USING_RETUNING_TIMER means the host is currently using
-	 * a retuning timer to do the retuning for the card.
-	 */
-	if (err && (host->flags & SDHCI_USING_RETUNING_TIMER))
-		err = 0;
+	host->mmc->retune_period = err ? 0 : tuning_count;
 
 	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
@@ -2337,20 +2286,6 @@ static void sdhci_timeout_timer(unsigned long data)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
-static void sdhci_tuning_timer(unsigned long data)
-{
-	struct sdhci_host *host;
-	unsigned long flags;
-
-	host = (struct sdhci_host *)data;
-
-	spin_lock_irqsave(&host->lock, flags);
-
-	host->flags |= SDHCI_NEEDS_RETUNING;
-
-	spin_unlock_irqrestore(&host->lock, flags);
-}
-
 /*****************************************************************************\
  *                                                                           *
  * Interrupt handling                                                        *
@@ -2728,11 +2663,8 @@ int sdhci_suspend_host(struct sdhci_host *host)
 {
 	sdhci_disable_card_detection(host);
 
-	/* Disable tuning since we are suspending */
-	if (host->flags & SDHCI_USING_RETUNING_TIMER) {
-		del_timer_sync(&host->tuning_timer);
-		host->flags &= ~SDHCI_NEEDS_RETUNING;
-	}
+	mmc_retune_timer_stop(host->mmc);
+	mmc_retune_needed(host->mmc);
 
 	if (!device_may_wakeup(mmc_dev(host->mmc))) {
 		host->ier = 0;
@@ -2782,10 +2714,6 @@ int sdhci_resume_host(struct sdhci_host *host)
 
 	sdhci_enable_card_detection(host);
 
-	/* Set the re-tuning expiration flag */
-	if (host->flags & SDHCI_USING_RETUNING_TIMER)
-		host->flags |= SDHCI_NEEDS_RETUNING;
-
 	return ret;
 }
 
@@ -2822,11 +2750,8 @@ int sdhci_runtime_suspend_host(struct sdhci_host *host)
 {
 	unsigned long flags;
 
-	/* Disable tuning since we are suspending */
-	if (host->flags & SDHCI_USING_RETUNING_TIMER) {
-		del_timer_sync(&host->tuning_timer);
-		host->flags &= ~SDHCI_NEEDS_RETUNING;
-	}
+	mmc_retune_timer_stop(host->mmc);
+	mmc_retune_needed(host->mmc);
 
 	spin_lock_irqsave(&host->lock, flags);
 	host->ier &= SDHCI_INT_CARD_INT;
@@ -2869,10 +2794,6 @@ int sdhci_runtime_resume_host(struct sdhci_host *host)
 		spin_unlock_irqrestore(&host->lock, flags);
 	}
 
-	/* Set the re-tuning expiration flag */
-	if (host->flags & SDHCI_USING_RETUNING_TIMER)
-		host->flags |= SDHCI_NEEDS_RETUNING;
-
 	spin_lock_irqsave(&host->lock, flags);
 
 	host->runtime_suspended = false;
@@ -3408,13 +3329,6 @@ int sdhci_add_host(struct sdhci_host *host)
 
 	init_waitqueue_head(&host->buf_ready_int);
 
-	if (host->version >= SDHCI_SPEC_300) {
-		/* Initialize re-tuning timer */
-		init_timer(&host->tuning_timer);
-		host->tuning_timer.data = (unsigned long)host;
-		host->tuning_timer.function = sdhci_tuning_timer;
-	}
-
 	sdhci_init(host, 0);
 
 	ret = request_threaded_irq(host->irq, sdhci_irq, sdhci_thread_irq,
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index e639b7f..923ff22 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -432,13 +432,11 @@ struct sdhci_host {
 #define SDHCI_REQ_USE_DMA	(1<<2)	/* Use DMA for this req. */
 #define SDHCI_DEVICE_DEAD	(1<<3)	/* Device unresponsive */
 #define SDHCI_SDR50_NEEDS_TUNING (1<<4)	/* SDR50 needs tuning */
-#define SDHCI_NEEDS_RETUNING	(1<<5)	/* Host needs retuning */
 #define SDHCI_AUTO_CMD12	(1<<6)	/* Auto CMD12 support */
 #define SDHCI_AUTO_CMD23	(1<<7)	/* Auto CMD23 support */
 #define SDHCI_PV_ENABLED	(1<<8)	/* Preset value enabled */
 #define SDHCI_SDIO_IRQ_ENABLED	(1<<9)	/* SDIO irq enabled */
 #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 */
 
@@ -504,7 +502,6 @@ struct sdhci_host {
 	unsigned int		tuning_count;	/* Timer count for re-tuning */
 	unsigned int		tuning_mode;	/* Re-tuning mode supported by host */
 #define SDHCI_TUNING_MODE_1	0
-	struct timer_list	tuning_timer;	/* Timer for tuning */
 
 	struct sdhci_host_next	next_data;
 	unsigned long private[0] ____cacheline_aligned;
-- 
1.9.1

[PATCH V5 12/15] mmc: sdhci: Flag re-tuning is needed on CRC or End-Bit errors

[Adrian Hunter]

CRC or End-Bit errors could possibly be alleviated by
re-tuning so flag re-tuning needed in those cases.
Note this has no effect if re-tuning has not been
enabled.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b345844..d11fae7 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2307,8 +2307,10 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *mask)
 	if (intmask & SDHCI_INT_TIMEOUT)
 		host->cmd->error = -ETIMEDOUT;
 	else if (intmask & (SDHCI_INT_CRC | SDHCI_INT_END_BIT |
-			SDHCI_INT_INDEX))
+			SDHCI_INT_INDEX)) {
 		host->cmd->error = -EILSEQ;
+		mmc_retune_needed(host->mmc);
+	}
 
 	if (host->cmd->error) {
 		tasklet_schedule(&host->finish_tasklet);
@@ -2433,13 +2435,15 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 
 	if (intmask & SDHCI_INT_DATA_TIMEOUT)
 		host->data->error = -ETIMEDOUT;
-	else if (intmask & SDHCI_INT_DATA_END_BIT)
+	else if (intmask & SDHCI_INT_DATA_END_BIT) {
 		host->data->error = -EILSEQ;
-	else if ((intmask & SDHCI_INT_DATA_CRC) &&
+		mmc_retune_needed(host->mmc);
+	} else if ((intmask & SDHCI_INT_DATA_CRC) &&
 		SDHCI_GET_CMD(sdhci_readw(host, SDHCI_COMMAND))
-			!= MMC_BUS_TEST_R)
+			!= MMC_BUS_TEST_R) {
 		host->data->error = -EILSEQ;
-	else if (intmask & SDHCI_INT_ADMA_ERROR) {
+		mmc_retune_needed(host->mmc);
+	} else if (intmask & SDHCI_INT_ADMA_ERROR) {
 		pr_err("%s: ADMA error\n", mmc_hostname(host->mmc));
 		sdhci_adma_show_error(host);
 		host->data->error = -EIO;
-- 
1.9.1

[PATCH V5 13/15] mmc: block: Check re-tuning in the recovery path

[Adrian Hunter]

If re-tuning is needed, do it in the recovery path to
give recovery commands a better chance of success.

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

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c69afb5..293e938 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -913,6 +913,9 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, struct request *req,
 		if (!err)
 			break;
 
+		/* Re-tune if needed */
+		mmc_retune_recheck(card->host);
+
 		prev_cmd_status_valid = false;
 		pr_err("%s: error %d sending status command, %sing\n",
 		       req->rq_disk->disk_name, err, retry ? "retry" : "abort");
-- 
1.9.1

[PATCH V5 14/15] mmc: block: Retry errored data requests when re-tuning is needed

[Adrian Hunter]

Retry errored data requests when re-tuning is needed and
add a flag to struct mmc_blk_request so that the retry
is only done once.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/card/block.c | 11 ++++++++++-
 drivers/mmc/card/queue.h |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 293e938..3baf7ea 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1195,6 +1195,7 @@ static int mmc_blk_err_check(struct mmc_card *card,
 						    mmc_active);
 	struct mmc_blk_request *brq = &mq_mrq->brq;
 	struct request *req = mq_mrq->req;
+	int need_retune = card->host->need_retune;
 	int ecc_err = 0, gen_err = 0;
 
 	/*
@@ -1262,6 +1263,12 @@ static int mmc_blk_err_check(struct mmc_card *card,
 	}
 
 	if (brq->data.error) {
+		if (need_retune && !brq->retune_retry_done) {
+			pr_info("%s: retrying because a re-tune was needed\n",
+				req->rq_disk->disk_name);
+			brq->retune_retry_done = 1;
+			return MMC_BLK_RETRY;
+		}
 		pr_err("%s: error %d transferring data, sector %u, nr %u, cmd response %#x, card status %#x\n",
 		       req->rq_disk->disk_name, brq->data.error,
 		       (unsigned)blk_rq_pos(req),
@@ -1821,7 +1828,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 	struct mmc_blk_data *md = mq->data;
 	struct mmc_card *card = md->queue.card;
 	struct mmc_blk_request *brq = &mq->mqrq_cur->brq;
-	int ret = 1, disable_multi = 0, retry = 0, type;
+	int ret = 1, disable_multi = 0, retry = 0, type, retune_retry_done = 0;
 	enum mmc_blk_status status;
 	struct mmc_queue_req *mq_rq;
 	struct request *req = rqc;
@@ -1905,6 +1912,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 				break;
 			goto cmd_abort;
 		case MMC_BLK_RETRY:
+			retune_retry_done = brq->retune_retry_done;
 			if (retry++ < 5)
 				break;
 			/* Fall through */
@@ -1967,6 +1975,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 				mmc_start_req(card->host,
 						&mq_rq->mmc_active, NULL);
 			}
+			mq_rq->brq.retune_retry_done = retune_retry_done;
 		}
 	} while (ret);
 
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index 5752d50..7e27915 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -12,6 +12,7 @@ struct mmc_blk_request {
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
+	int			retune_retry_done;
 };
 
 enum mmc_packed_type {
-- 
1.9.1

[PATCH V5 15/15] mmc: core: Don't print reset warning if reset is not supported

[Adrian Hunter]

Check the error code for EOPNOTSUPP and do not print
reset warning in that case.

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

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 4a42174..e27ae49 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2365,7 +2365,8 @@ int mmc_hw_reset(struct mmc_host *host)
 	ret = host->bus_ops->reset(host);
 	mmc_bus_put(host);
 
-	pr_warn("%s: tried to reset card\n", mmc_hostname(host));
+	if (ret != -EOPNOTSUPP)
+		pr_warn("%s: tried to reset card\n", mmc_hostname(host));
 
 	return ret;
 }
-- 
1.9.1

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Ulf Hansson]

On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Enable re-tuning when tuning is executed and
> disable re-tuning when card is no longer initialized.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/mmc/core/core.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index c296bc0..dd43f9b 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
>
>         if (err)
>                 pr_err("%s: tuning execution failed\n", mmc_hostname(host));
> +       else
> +               mmc_retune_enable(host);
>
>         return err;
>  }
> @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
>   */
>  void mmc_set_initial_state(struct mmc_host *host)
>  {
> +       mmc_retune_disable(host);
> +
>         if (mmc_host_is_spi(host))
>                 host->ios.chip_select = MMC_CS_HIGH;
>         else
> --
> 1.9.1

I don't think you have fully considered these cases for the mmc/sd/sdio cards.

1) Card removal/detect (hold/release?)
2) system PM (disable?)
3) runtime PM (disable?)
4) reset (?)

Kind regards
Uffe

Re: [PATCH V5 08/15] mmc: mmc: Comment that callers need to hold re-tuning if the card is put to sleep

[Ulf Hansson]

On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Currently "mmc sleep" is only used before power off and
> is not paired with waking up.  If that ever changed,
> re-tuning might need to be held, so add a comment for that.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/mmc/core/mmc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index c84131e..53a9cb3 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1504,6 +1504,7 @@ static int mmc_can_sleep(struct mmc_card *card)
>         return (card && card->ext_csd.rev >= 3);
>  }
>
> +/* If necessary, callers must hold re-tuning */

Instead of adding a comment, let's fix what needs to be fixed.

I believe the proper thing would be to disable re-tuning when the card
is about to enter system PM sleep state. So somewhere early in
_mmc_suspend() we should disable re-tune. Likewise for SD and SDIO.

>  static int mmc_sleep(struct mmc_host *host)
>  {
>         struct mmc_command cmd = {0};
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kind regards
Uffe

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 11:57, Ulf Hansson wrote:
> On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> Enable re-tuning when tuning is executed and
>> disable re-tuning when card is no longer initialized.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  drivers/mmc/core/core.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index c296bc0..dd43f9b 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
>>
>>         if (err)
>>                 pr_err("%s: tuning execution failed\n", mmc_hostname(host));
>> +       else
>> +               mmc_retune_enable(host);
>>
>>         return err;
>>  }
>> @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
>>   */
>>  void mmc_set_initial_state(struct mmc_host *host)
>>  {
>> +       mmc_retune_disable(host);
>> +
>>         if (mmc_host_is_spi(host))
>>                 host->ios.chip_select = MMC_CS_HIGH;
>>         else
>> --
>> 1.9.1
> 
> I don't think you have fully considered these cases for the mmc/sd/sdio cards.

Thanks for look at this, but I don't see a problem - see below.

> 
> 1) Card removal/detect (hold/release?)

Re-tuning will be done if needed before detect (because it is done before a
request), which is necessary because detect can fail if tuning is needed.

Re-tuning is done before a request. Requests aren't started if the card has
been removed i.e. mmc_start_request() calls mmc_card_removed()

If tuning is executed while a card is being removed, then the tuning will
fail and the request will be errored out.

> 2) system PM (disable?)

System pm does mmc_power_off() which calls mmc_set_initial_state()

> 3) runtime PM (disable?)

If the card powers off then mmc_set_initial_state() will called.

For anything else the card might be doing, the mmc_retune_hold() /
mmc_retune_release() functions are used.

> 4) reset (?)

Reset goes through mmc_set_initial_state()

Re: [PATCH V5 07/15] mmc: core: Hold re-tuning while bkops ongoing

[Ulf Hansson]

On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hold re-tuning during bkops to prevent
> it from conflicting with the busy state.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/mmc/core/core.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index dbd7a77..4a42174 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -316,6 +316,8 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
>         } else {
>                 timeout = 0;
>                 use_busy_signal = false;
> +               /* Hold re-tuning for ongoing bkops */
> +               mmc_retune_hold(card->host);
>         }

I would rather change the behaviour to always hold re-tune, no matter
of "use_busy_signal".
Then don't release re-tune if it's an ongoing BKOPS.

I do understand the hold/release you added in __mmc_switch() will
cover this, but I think this code becomes more clear if you change to
my proposal.

>
>         err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> @@ -324,6 +326,9 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
>         if (err) {
>                 pr_warn("%s: Error %d starting bkops\n",
>                         mmc_hostname(card->host), err);
> +               /* bkops not ongoing, so release re-tuning */
> +               if (!use_busy_signal)
> +                       mmc_retune_release(card->host);
>                 goto out;
>         }
>
> @@ -749,6 +754,7 @@ int mmc_stop_bkops(struct mmc_card *card)
>          */
>         if (!err || (err == -EINVAL)) {
>                 mmc_card_clr_doing_bkops(card);
> +               mmc_retune_release(card->host);
>                 err = 0;
>         }
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Kind regards
Uffe

Re: [PATCH V5 08/15] mmc: mmc: Comment that callers need to hold re-tuning if the card is put to sleep

[Adrian Hunter]

On 16/04/15 12:01, Ulf Hansson wrote:
> On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> Currently "mmc sleep" is only used before power off and
>> is not paired with waking up.  If that ever changed,
>> re-tuning might need to be held, so add a comment for that.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  drivers/mmc/core/mmc.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index c84131e..53a9cb3 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1504,6 +1504,7 @@ static int mmc_can_sleep(struct mmc_card *card)
>>         return (card && card->ext_csd.rev >= 3);
>>  }
>>
>> +/* If necessary, callers must hold re-tuning */
> 
> Instead of adding a comment, let's fix what needs to be fixed.
> 
> I believe the proper thing would be to disable re-tuning when the card
> is about to enter system PM sleep state. So somewhere early in
> _mmc_suspend() we should disable re-tune. Likewise for SD and SDIO.

_mmc_suspend() calls mmc_power_off() which calls  mmc_set_initial_state()
which disables re-tuning. Likewise for SD and SDIO.

So it is already done?

> 
>>  static int mmc_sleep(struct mmc_host *host)
>>  {
>>         struct mmc_command cmd = {0};
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Kind regards
> Uffe
> 
> 

[PATCH V6 07/15] mmc: core: Hold re-tuning while bkops ongoing

[Adrian Hunter]

Hold re-tuning during bkops to prevent
it from conflicting with the busy state.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---



Changes in V6:

	Hold re-tuning always and release only if
	bkops is not ongoing.



 drivers/mmc/core/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dbd7a77..7aae0ff 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -318,12 +318,15 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
 		use_busy_signal = false;
 	}
 
+	mmc_retune_hold(card->host);
+
 	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 			EXT_CSD_BKOPS_START, 1, timeout,
 			use_busy_signal, true, false);
 	if (err) {
 		pr_warn("%s: Error %d starting bkops\n",
 			mmc_hostname(card->host), err);
+		mmc_retune_release(card->host);
 		goto out;
 	}
 
@@ -334,6 +337,8 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
 	 */
 	if (!use_busy_signal)
 		mmc_card_set_doing_bkops(card);
+	else
+		mmc_retune_release(card->host);
 out:
 	mmc_release_host(card->host);
 }
@@ -749,6 +754,7 @@ int mmc_stop_bkops(struct mmc_card *card)
 	 */
 	if (!err || (err == -EINVAL)) {
 		mmc_card_clr_doing_bkops(card);
+		mmc_retune_release(card->host);
 		err = 0;
 	}
 
-- 
1.9.1

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Ulf Hansson]

On 16 April 2015 at 11:26, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 16/04/15 11:57, Ulf Hansson wrote:
>> On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> Enable re-tuning when tuning is executed and
>>> disable re-tuning when card is no longer initialized.
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>> ---
>>>  drivers/mmc/core/core.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index c296bc0..dd43f9b 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
>>>
>>>         if (err)
>>>                 pr_err("%s: tuning execution failed\n", mmc_hostname(host));
>>> +       else
>>> +               mmc_retune_enable(host);
>>>
>>>         return err;
>>>  }
>>> @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
>>>   */
>>>  void mmc_set_initial_state(struct mmc_host *host)
>>>  {
>>> +       mmc_retune_disable(host);
>>> +
>>>         if (mmc_host_is_spi(host))
>>>                 host->ios.chip_select = MMC_CS_HIGH;
>>>         else
>>> --
>>> 1.9.1
>>
>> I don't think you have fully considered these cases for the mmc/sd/sdio cards.
>
> Thanks for look at this, but I don't see a problem - see below.
>
>>
>> 1) Card removal/detect (hold/release?)
>
> Re-tuning will be done if needed before detect (because it is done before a
> request), which is necessary because detect can fail if tuning is needed.
>
> Re-tuning is done before a request. Requests aren't started if the card has
> been removed i.e. mmc_start_request() calls mmc_card_removed()
>
> If tuning is executed while a card is being removed, then the tuning will
> fail and the request will be errored out.

So you are saying that we instead of relying on the CMD13 (for SD/MMC)
to check whether the card is still alive, it's fine to trigger a
re-tuning instead.

I don't think that is an effective way to remove a card.

Moreover, I find it quite unreasonable to say the check for an alive
card, would fail because of that a re-tuning is needed. That would in
principle mean that we never should be able to hold re-tune for any
commands sequences.

>
>> 2) system PM (disable?)
>
> System pm does mmc_power_off() which calls mmc_set_initial_state()

At System PM other commands will be sent to put the card into sleep
state. For example CMD5. These commands are invoked prior
mmc_power_off() is called.

In the SDIO case, mmc_power_off() might not even be called at all,
since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.

>
>> 3) runtime PM (disable?)
>
> If the card powers off then mmc_set_initial_state() will called.

Again that's too late, since for example the CMD5 might have been sent
before this.

>
> For anything else the card might be doing, the mmc_retune_hold() /
> mmc_retune_release() functions are used.
>
>> 4) reset (?)
>
> Reset goes through mmc_set_initial_state()

In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
during that period?

>
>

Kind regards
Uffe

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 15:00, Ulf Hansson wrote:
> On 16 April 2015 at 11:26, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 16/04/15 11:57, Ulf Hansson wrote:
>>> On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> Enable re-tuning when tuning is executed and
>>>> disable re-tuning when card is no longer initialized.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>>  drivers/mmc/core/core.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index c296bc0..dd43f9b 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
>>>>
>>>>         if (err)
>>>>                 pr_err("%s: tuning execution failed\n", mmc_hostname(host));
>>>> +       else
>>>> +               mmc_retune_enable(host);
>>>>
>>>>         return err;
>>>>  }
>>>> @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
>>>>   */
>>>>  void mmc_set_initial_state(struct mmc_host *host)
>>>>  {
>>>> +       mmc_retune_disable(host);
>>>> +
>>>>         if (mmc_host_is_spi(host))
>>>>                 host->ios.chip_select = MMC_CS_HIGH;
>>>>         else
>>>> --
>>>> 1.9.1
>>>
>>> I don't think you have fully considered these cases for the mmc/sd/sdio cards.
>>
>> Thanks for look at this, but I don't see a problem - see below.
>>
>>>
>>> 1) Card removal/detect (hold/release?)
>>
>> Re-tuning will be done if needed before detect (because it is done before a
>> request), which is necessary because detect can fail if tuning is needed.
>>
>> Re-tuning is done before a request. Requests aren't started if the card has
>> been removed i.e. mmc_start_request() calls mmc_card_removed()
>>
>> If tuning is executed while a card is being removed, then the tuning will
>> fail and the request will be errored out.
> 
> So you are saying that we instead of relying on the CMD13 (for SD/MMC)
> to check whether the card is still alive, it's fine to trigger a
> re-tuning instead.
> 
> I don't think that is an effective way to remove a card.
> 
> Moreover, I find it quite unreasonable to say the check for an alive
> card, would fail because of that a re-tuning is needed. That would in
> principle mean that we never should be able to hold re-tune for any
> commands sequences.
> 
>>
>>> 2) system PM (disable?)
>>
>> System pm does mmc_power_off() which calls mmc_set_initial_state()
> 
> At System PM other commands will be sent to put the card into sleep
> state. For example CMD5. These commands are invoked prior
> mmc_power_off() is called.

You can't do *any* commands after CMD5 except CMD0 or another CMD5 to wake up.

So if you want to wake-up from sleep, then you need to hold re-tuning, but
that is not what is happening at the moment.

> 
> In the SDIO case, mmc_power_off() might not even be called at all,
> since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.

If the card is not going to be re-initialized then disabling is not necessary.

> 
>>
>>> 3) runtime PM (disable?)
>>
>> If the card powers off then mmc_set_initial_state() will called.
> 
> Again that's too late, since for example the CMD5 might have been sent
> before this.

CMD5 is only used by _mmc_suspend()

Yes if it were used elsewhere then re-tuning would have to be held, which is
why I added a comment before mmc_sleep()

	/* If necessary, callers must hold re-tuning */
	static int mmc_sleep(struct mmc_host *host)

> 
>>
>> For anything else the card might be doing, the mmc_retune_hold() /
>> mmc_retune_release() functions are used.
>>
>>> 4) reset (?)
>>
>> Reset goes through mmc_set_initial_state()
> 
> In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
> during that period?

If reset happens, then the next command will fail, whether it is re-tuning
or CMD13, so no different.

If reset doesn't happen, then it is no different to normal operations.

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 15:00, Ulf Hansson wrote:
> On 16 April 2015 at 11:26, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 16/04/15 11:57, Ulf Hansson wrote:
>>> On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> Enable re-tuning when tuning is executed and
>>>> disable re-tuning when card is no longer initialized.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>>  drivers/mmc/core/core.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index c296bc0..dd43f9b 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
>>>>
>>>>         if (err)
>>>>                 pr_err("%s: tuning execution failed\n", mmc_hostname(host));
>>>> +       else
>>>> +               mmc_retune_enable(host);
>>>>
>>>>         return err;
>>>>  }
>>>> @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
>>>>   */
>>>>  void mmc_set_initial_state(struct mmc_host *host)
>>>>  {
>>>> +       mmc_retune_disable(host);
>>>> +
>>>>         if (mmc_host_is_spi(host))
>>>>                 host->ios.chip_select = MMC_CS_HIGH;
>>>>         else
>>>> --
>>>> 1.9.1
>>>
>>> I don't think you have fully considered these cases for the mmc/sd/sdio cards.
>>
>> Thanks for look at this, but I don't see a problem - see below.
>>
>>>
>>> 1) Card removal/detect (hold/release?)
>>
>> Re-tuning will be done if needed before detect (because it is done before a
>> request), which is necessary because detect can fail if tuning is needed.
>>
>> Re-tuning is done before a request. Requests aren't started if the card has
>> been removed i.e. mmc_start_request() calls mmc_card_removed()
>>
>> If tuning is executed while a card is being removed, then the tuning will
>> fail and the request will be errored out.
> 
> So you are saying that we instead of relying on the CMD13 (for SD/MMC)
> to check whether the card is still alive, it's fine to trigger a
> re-tuning instead.

(Oops forgot to answer this one, sorry)

Yes, why not?

> 
> I don't think that is an effective way to remove a card.

Generally we know the card is removed from card-detect so no commands are
sent in either case.

> 
> Moreover, I find it quite unreasonable to say the check for an alive
> card, would fail because of that a re-tuning is needed. That would in
> principle mean that we never should be able to hold re-tune for any
> commands sequences.

Re-tuning must work if the card is alive. CMD13 might get a CRC error if
re-tuning is needed.

> 
>>
>>> 2) system PM (disable?)
>>
>> System pm does mmc_power_off() which calls mmc_set_initial_state()
> 
> At System PM other commands will be sent to put the card into sleep
> state. For example CMD5. These commands are invoked prior
> mmc_power_off() is called.
> 
> In the SDIO case, mmc_power_off() might not even be called at all,
> since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.
> 
>>
>>> 3) runtime PM (disable?)
>>
>> If the card powers off then mmc_set_initial_state() will called.
> 
> Again that's too late, since for example the CMD5 might have been sent
> before this.
> 
>>
>> For anything else the card might be doing, the mmc_retune_hold() /
>> mmc_retune_release() functions are used.
>>
>>> 4) reset (?)
>>
>> Reset goes through mmc_set_initial_state()
> 
> In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
> during that period?
> 
>>
>>
> 
> Kind regards
> Uffe
> 
> 

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Ulf Hansson]

[...]

>>>> 2) system PM (disable?)
>>>
>>> System pm does mmc_power_off() which calls mmc_set_initial_state()
>>
>> At System PM other commands will be sent to put the card into sleep
>> state. For example CMD5. These commands are invoked prior
>> mmc_power_off() is called.
>
> You can't do *any* commands after CMD5 except CMD0 or another CMD5 to wake up.
>
> So if you want to wake-up from sleep, then you need to hold re-tuning, but
> that is not what is happening at the moment.

So then we need to fix this.

Also, it seems like disabling the re-tuning timer would also be the
proper thing to do, thus we should rather do disable instead of
hold/release.

>
>>
>> In the SDIO case, mmc_power_off() might not even be called at all,
>> since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.
>
> If the card is not going to be re-initialized then disabling is not necessary.

I don't want the re-tuning timer to be running, thus it seems like we
should do disable. Right?

>
>>
>>>
>>>> 3) runtime PM (disable?)
>>>
>>> If the card powers off then mmc_set_initial_state() will called.
>>
>> Again that's too late, since for example the CMD5 might have been sent
>> before this.
>
> CMD5 is only used by _mmc_suspend()
>
> Yes if it were used elsewhere then re-tuning would have to be held, which is
> why I added a comment before mmc_sleep()
>
>         /* If necessary, callers must hold re-tuning */
>         static int mmc_sleep(struct mmc_host *host)
>

I don't follow here. Why would we like to allow a re-tuning to be done
in this part of the system PM phase? It doesn't make sense to me.

>>
>>>
>>> For anything else the card might be doing, the mmc_retune_hold() /
>>> mmc_retune_release() functions are used.
>>>
>>>> 4) reset (?)
>>>
>>> Reset goes through mmc_set_initial_state()
>>
>> In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
>> during that period?
>
> If reset happens, then the next command will fail, whether it is re-tuning
> or CMD13, so no different.

That depends on how each an every host has implemented their tuning mechanism.

CMD13 is more light weight, so I believe we should hold re-tune to
make sure we do the CMD13 and fail quickly.

>
> If reset doesn't happen, then it is no different to normal operations.
>

Kind regards
Uffe

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Ulf Hansson]

[...]

>>>> 1) Card removal/detect (hold/release?)
>>>
>>> Re-tuning will be done if needed before detect (because it is done before a
>>> request), which is necessary because detect can fail if tuning is needed.
>>>
>>> Re-tuning is done before a request. Requests aren't started if the card has
>>> been removed i.e. mmc_start_request() calls mmc_card_removed()
>>>
>>> If tuning is executed while a card is being removed, then the tuning will
>>> fail and the request will be errored out.
>>
>> So you are saying that we instead of relying on the CMD13 (for SD/MMC)
>> to check whether the card is still alive, it's fine to trigger a
>> re-tuning instead.
>
> (Oops forgot to answer this one, sorry)
>
> Yes, why not?
>
>>
>> I don't think that is an effective way to remove a card.
>
> Generally we know the card is removed from card-detect so no commands are
> sent in either case.

Not sure what you mean here.

In case when the card is "idle" and the host sees a GPIO CD irq, it
will trigger a detect work to run mmc_rescan().

In this case, it's the responsibility of mmc_rescan() to find out if
the card is being removed. It does so by invoking the
bus_ops->detect() callback, which eventually will send a CMD13 for
mmc/sd.

In this scenario, I can't see why we want to allow re-tuning to happen
in front of the CMD13 command.

>
>>
>> Moreover, I find it quite unreasonable to say the check for an alive
>> card, would fail because of that a re-tuning is needed. That would in
>> principle mean that we never should be able to hold re-tune for any
>> commands sequences.
>
> Re-tuning must work if the card is alive. CMD13 might get a CRC error if
> re-tuning is needed.

And that then applies to all commands which we hold re-tuning for. So
then we can't _ever_  hold re-tuning for any command, since we might
get CRC errors.

Kind regards
Uffe

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 16:57, Ulf Hansson wrote:
> [...]
> 
>>>>> 2) system PM (disable?)
>>>>
>>>> System pm does mmc_power_off() which calls mmc_set_initial_state()
>>>
>>> At System PM other commands will be sent to put the card into sleep
>>> state. For example CMD5. These commands are invoked prior
>>> mmc_power_off() is called.
>>
>> You can't do *any* commands after CMD5 except CMD0 or another CMD5 to wake up.
>>
>> So if you want to wake-up from sleep, then you need to hold re-tuning, but
>> that is not what is happening at the moment.
> 
> So then we need to fix this.
> 
> Also, it seems like disabling the re-tuning timer would also be the
> proper thing to do, thus we should rather do disable instead of
> hold/release.

I can add hold/release. The hold should be done before sleeping
and the release after waking up. In this case there is no wake-up, instead
there is power-off, so the release can be done then.

Disabling before CMD5 is not the right thing to do because we might want to
re-tune before issuing CMD5.

> 
>>
>>>
>>> In the SDIO case, mmc_power_off() might not even be called at all,
>>> since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.
>>
>> If the card is not going to be re-initialized then disabling is not necessary.
> 
> I don't want the re-tuning timer to be running, thus it seems like we
> should do disable. Right?

Re-tuning remains enabled while the card has a transfer mode that requires
it. Re-tuning is only done before a request so it doesn't need to be
disabled during suspend i.e. no requests implies no-retuning

I can add disabling the re-tuning timer, although the host does that too.

> 
>>
>>>
>>>>
>>>>> 3) runtime PM (disable?)
>>>>
>>>> If the card powers off then mmc_set_initial_state() will called.
>>>
>>> Again that's too late, since for example the CMD5 might have been sent
>>> before this.
>>
>> CMD5 is only used by _mmc_suspend()
>>
>> Yes if it were used elsewhere then re-tuning would have to be held, which is
>> why I added a comment before mmc_sleep()
>>
>>         /* If necessary, callers must hold re-tuning */
>>         static int mmc_sleep(struct mmc_host *host)
>>
> 
> I don't follow here. Why would we like to allow a re-tuning to be done
> in this part of the system PM phase? It doesn't make sense to me.

Because it is needed.

If re-tuning is needed and can be done, it is done. It doesn't need to be
more complicated than that.

> 
>>>
>>>>
>>>> For anything else the card might be doing, the mmc_retune_hold() /
>>>> mmc_retune_release() functions are used.
>>>>
>>>>> 4) reset (?)
>>>>
>>>> Reset goes through mmc_set_initial_state()
>>>
>>> In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
>>> during that period?
>>
>> If reset happens, then the next command will fail, whether it is re-tuning
>> or CMD13, so no different.
> 
> That depends on how each an every host has implemented their tuning mechanism.

No it doesn't. Tuning either succeeds or fails. When there is no card, if it
fails then the CMD13 is not needed, and if it succeeds then CMD13 will fail.

> 
> CMD13 is more light weight, so I believe we should hold re-tune to
> make sure we do the CMD13 and fail quickly.

If re-tuning is needed and can be done, it is done. It doesn't need to be
more complicated than that.

> 
>>
>> If reset doesn't happen, then it is no different to normal operations.
>>

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 18:19, Ulf Hansson wrote:
> [...]
> 
>>>>> 1) Card removal/detect (hold/release?)
>>>>
>>>> Re-tuning will be done if needed before detect (because it is done before a
>>>> request), which is necessary because detect can fail if tuning is needed.
>>>>
>>>> Re-tuning is done before a request. Requests aren't started if the card has
>>>> been removed i.e. mmc_start_request() calls mmc_card_removed()
>>>>
>>>> If tuning is executed while a card is being removed, then the tuning will
>>>> fail and the request will be errored out.
>>>
>>> So you are saying that we instead of relying on the CMD13 (for SD/MMC)
>>> to check whether the card is still alive, it's fine to trigger a
>>> re-tuning instead.
>>
>> (Oops forgot to answer this one, sorry)
>>
>> Yes, why not?
>>
>>>
>>> I don't think that is an effective way to remove a card.
>>
>> Generally we know the card is removed from card-detect so no commands are
>> sent in either case.
> 
> Not sure what you mean here.

The sdhci driver won't issue commands if card-detect shows no card. It just
sets the error to -ENOMEDIUM.

> 
> In case when the card is "idle" and the host sees a GPIO CD irq, it
> will trigger a detect work to run mmc_rescan().
> 
> In this case, it's the responsibility of mmc_rescan() to find out if
> the card is being removed. It does so by invoking the
> bus_ops->detect() callback, which eventually will send a CMD13 for
> mmc/sd.
> 
> In this scenario, I can't see why we want to allow re-tuning to happen
> in front of the CMD13 command.

If re-tuning is needed and can be done, it is done. It doesn't need to be
more complicated than that.

> 
>>
>>>
>>> Moreover, I find it quite unreasonable to say the check for an alive
>>> card, would fail because of that a re-tuning is needed. That would in
>>> principle mean that we never should be able to hold re-tune for any
>>> commands sequences.
>>
>> Re-tuning must work if the card is alive. CMD13 might get a CRC error if
>> re-tuning is needed.
> 
> And that then applies to all commands which we hold re-tuning for. So
> then we can't _ever_  hold re-tuning for any command, since we might
> get CRC errors.

Periodic re-tuning and re-tuning after pm-runtime-resume should ensure no
CRC errors while re-tuning is held.

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Ulf Hansson]

On 17 April 2015 at 09:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 16/04/15 18:19, Ulf Hansson wrote:
>> [...]
>>
>>>>>> 1) Card removal/detect (hold/release?)
>>>>>
>>>>> Re-tuning will be done if needed before detect (because it is done before a
>>>>> request), which is necessary because detect can fail if tuning is needed.
>>>>>
>>>>> Re-tuning is done before a request. Requests aren't started if the card has
>>>>> been removed i.e. mmc_start_request() calls mmc_card_removed()
>>>>>
>>>>> If tuning is executed while a card is being removed, then the tuning will
>>>>> fail and the request will be errored out.
>>>>
>>>> So you are saying that we instead of relying on the CMD13 (for SD/MMC)
>>>> to check whether the card is still alive, it's fine to trigger a
>>>> re-tuning instead.
>>>
>>> (Oops forgot to answer this one, sorry)
>>>
>>> Yes, why not?
>>>
>>>>
>>>> I don't think that is an effective way to remove a card.
>>>
>>> Generally we know the card is removed from card-detect so no commands are
>>> sent in either case.
>>
>> Not sure what you mean here.
>
> The sdhci driver won't issue commands if card-detect shows no card. It just
> sets the error to -ENOMEDIUM.

That's sdhci, but we have have a lot more drivers than sdhci to care about.

>
>>
>> In case when the card is "idle" and the host sees a GPIO CD irq, it
>> will trigger a detect work to run mmc_rescan().
>>
>> In this case, it's the responsibility of mmc_rescan() to find out if
>> the card is being removed. It does so by invoking the
>> bus_ops->detect() callback, which eventually will send a CMD13 for
>> mmc/sd.
>>
>> In this scenario, I can't see why we want to allow re-tuning to happen
>> in front of the CMD13 command.
>
> If re-tuning is needed and can be done, it is done. It doesn't need to be
> more complicated than that.
>
>>
>>>
>>>>
>>>> Moreover, I find it quite unreasonable to say the check for an alive
>>>> card, would fail because of that a re-tuning is needed. That would in
>>>> principle mean that we never should be able to hold re-tune for any
>>>> commands sequences.
>>>
>>> Re-tuning must work if the card is alive. CMD13 might get a CRC error if
>>> re-tuning is needed.
>>
>> And that then applies to all commands which we hold re-tuning for. So
>> then we can't _ever_  hold re-tuning for any command, since we might
>> get CRC errors.
>
> Periodic re-tuning and re-tuning after pm-runtime-resume should ensure no
> CRC errors while re-tuning is held.
>

When re-tune is being held, there's no guarantee the re-tune timer
will not timeout. So that means every time you decide to hold re-tune
for a command (or sequence of commands) you will risk getting CRC
errors.

The hole idea with the periodic re-tuning timer is to _minimize_ the
probability of getting CRC errors, it's not to remove them completely,
right!?

So from that reasoning, I don't see why the mmc core shouldn't be
holding/disabling re-tuning under those circumstances when it make
sense. As for those I suggested.

Kind regards
Uffe

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 17/04/15 11:56, Ulf Hansson wrote:
> On 17 April 2015 at 09:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 16/04/15 18:19, Ulf Hansson wrote:
>>> [...]
>>>
>>>>>>> 1) Card removal/detect (hold/release?)
>>>>>>
>>>>>> Re-tuning will be done if needed before detect (because it is done before a
>>>>>> request), which is necessary because detect can fail if tuning is needed.
>>>>>>
>>>>>> Re-tuning is done before a request. Requests aren't started if the card has
>>>>>> been removed i.e. mmc_start_request() calls mmc_card_removed()
>>>>>>
>>>>>> If tuning is executed while a card is being removed, then the tuning will
>>>>>> fail and the request will be errored out.
>>>>>
>>>>> So you are saying that we instead of relying on the CMD13 (for SD/MMC)
>>>>> to check whether the card is still alive, it's fine to trigger a
>>>>> re-tuning instead.
>>>>
>>>> (Oops forgot to answer this one, sorry)
>>>>
>>>> Yes, why not?
>>>>
>>>>>
>>>>> I don't think that is an effective way to remove a card.
>>>>
>>>> Generally we know the card is removed from card-detect so no commands are
>>>> sent in either case.
>>>
>>> Not sure what you mean here.
>>
>> The sdhci driver won't issue commands if card-detect shows no card. It just
>> sets the error to -ENOMEDIUM.
> 
> That's sdhci, but we have have a lot more drivers than sdhci to care about.
> 
>>
>>>
>>> In case when the card is "idle" and the host sees a GPIO CD irq, it
>>> will trigger a detect work to run mmc_rescan().
>>>
>>> In this case, it's the responsibility of mmc_rescan() to find out if
>>> the card is being removed. It does so by invoking the
>>> bus_ops->detect() callback, which eventually will send a CMD13 for
>>> mmc/sd.
>>>
>>> In this scenario, I can't see why we want to allow re-tuning to happen
>>> in front of the CMD13 command.
>>
>> If re-tuning is needed and can be done, it is done. It doesn't need to be
>> more complicated than that.
>>
>>>
>>>>
>>>>>
>>>>> Moreover, I find it quite unreasonable to say the check for an alive
>>>>> card, would fail because of that a re-tuning is needed. That would in
>>>>> principle mean that we never should be able to hold re-tune for any
>>>>> commands sequences.
>>>>
>>>> Re-tuning must work if the card is alive. CMD13 might get a CRC error if
>>>> re-tuning is needed.
>>>
>>> And that then applies to all commands which we hold re-tuning for. So
>>> then we can't _ever_  hold re-tuning for any command, since we might
>>> get CRC errors.
>>
>> Periodic re-tuning and re-tuning after pm-runtime-resume should ensure no
>> CRC errors while re-tuning is held.
>>
> 
> When re-tune is being held, there's no guarantee the re-tune timer
> will not timeout. So that means every time you decide to hold re-tune
> for a command (or sequence of commands) you will risk getting CRC
> errors.
> 
> The hole idea with the periodic re-tuning timer is to _minimize_ the
> probability of getting CRC errors, it's not to remove them completely,
> right!?

Yes I shouldn't have asserted there will be no CRC errors. As you say, there
could be. Currently there is no attempt to recover from such errors and
AFAIK there is also no indication that that has been a problem yet. In my
patches I have added recovery for mmc block data transfers. So that is at
least a step forward.

> 
> So from that reasoning, I don't see why the mmc core shouldn't be
> holding/disabling re-tuning under those circumstances when it make
> sense. As for those I suggested.

To reduce the chance of CRC errors when re-tuning is held, we should hold
re-tuning only when absolutely necessary. Wouldn't that mean not in the
extra places you suggested?