[PATCH RFC 0/5] mmc: sdhci: Tidy tuning

[PATCH RFC 0/5] mmc: sdhci: Tidy tuning

[Adrian Hunter]

Hi

Here are some tidy-ups wrt SDHCI tuning.  Un-tested so far.

Any other suggestions?  Otherwise I will test and submit the whole series.

Regards
Adrian

[PATCH RFC 1/5] mmc: mmc: Introduce mmc_abort_tuning()

[Adrian Hunter]

If a tuning command times out, the card could still be processing it, which
will cause problems for recovery. The eMMC specification says that CMD12
can be used to stop CMD21, so add a function that does that.

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

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 9b2617cfff67..c0cfaaeb7637 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -688,6 +688,31 @@ int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error)
 }
 EXPORT_SYMBOL_GPL(mmc_send_tuning);
 
+int mmc_abort_tuning(struct mmc_host *host, u32 opcode)
+{
+	struct mmc_command cmd = {0};
+
+	/*
+	 * eMMC specification specifies that CMD12 can be used to stop a tuning
+	 * command, but SD specification does not, so do nothing unless it is
+	 * eMMC.
+	 */
+	if (opcode != MMC_SEND_TUNING_BLOCK_HS200)
+		return 0;
+
+	cmd.opcode = MMC_STOP_TRANSMISSION;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
+
+	/*
+	 * For drivers that override R1 to R1b, set an arbitrary timeout based
+	 * on the tuning timeout i.e. 150ms.
+	 */
+	cmd.busy_timeout = 150;
+
+	return mmc_wait_for_cmd(host, &cmd, 0);
+}
+EXPORT_SYMBOL_GPL(mmc_abort_tuning);
+
 static int
 mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
 		  u8 len)
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 0ce928b3ce90..e33cc748dcfe 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -176,6 +176,7 @@ extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
 extern void mmc_start_bkops(struct mmc_card *card, bool from_exception);
 extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
 extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
+extern int mmc_abort_tuning(struct mmc_host *host, u32 opcode);
 extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
 
 #define MMC_ERASE_ARG		0x00000000
-- 
1.9.1

[PATCH RFC 2/5] mmc: sdhci: Use mmc_abort_tuning()

[Adrian Hunter]

Use mmc_abort_tuning() instead of open-coding the stop command.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a23887799f43..b841d0a57af1 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2098,20 +2098,11 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 			sdhci_do_reset(host, SDHCI_RESET_CMD);
 			sdhci_do_reset(host, SDHCI_RESET_DATA);
 
-			if (cmd.opcode != MMC_SEND_TUNING_BLOCK_HS200)
-				goto out;
-
 			sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
 			sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 
 			spin_unlock_irqrestore(&host->lock, flags);
-
-			memset(&cmd, 0, sizeof(cmd));
-			cmd.opcode = MMC_STOP_TRANSMISSION;
-			cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
-			cmd.busy_timeout = 50;
-			mmc_wait_for_cmd(mmc, &cmd, 0);
-
+			mmc_abort_tuning(mmc, opcode);
 			spin_lock_irqsave(&host->lock, flags);
 
 			goto out;
-- 
1.9.1

[PATCH RFC 3/5] mmc: sdhci: Factor out tuning helper functions

[Adrian Hunter]

Factor out some functions to tidy up the code in sdhci_execute_tuning.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b841d0a57af1..fa89870a690b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1952,6 +1952,115 @@ static int sdhci_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios)
 	return 0;
 }
 
+static void sdhci_start_tuning(struct sdhci_host *host)
+{
+	u16 ctrl;
+
+	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+	ctrl |= SDHCI_CTRL_EXEC_TUNING;
+	if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
+		ctrl |= SDHCI_CTRL_TUNED_CLK;
+	sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+
+	/*
+	 * As per the Host Controller spec v3.00, tuning command
+	 * generates Buffer Read Ready interrupt, so enable that.
+	 *
+	 * Note: The spec clearly says that when tuning sequence
+	 * is being performed, the controller does not generate
+	 * interrupts other than Buffer Read Ready interrupt. But
+	 * to make sure we don't hit a controller bug, we _only_
+	 * enable Buffer Read Ready interrupt here.
+	 */
+	sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
+	sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE);
+}
+
+static void sdhci_end_tuning(struct sdhci_host *host)
+{
+	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+}
+
+static void sdhci_reset_tuning(struct sdhci_host *host)
+{
+	u16 ctrl;
+
+	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+	ctrl &= ~SDHCI_CTRL_TUNED_CLK;
+	ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
+	sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+}
+
+static void sdhci_abort_tuning(struct sdhci_host *host, u32 opcode,
+			       unsigned long flags)
+{
+	sdhci_reset_tuning(host);
+
+	sdhci_do_reset(host, SDHCI_RESET_CMD);
+	sdhci_do_reset(host, SDHCI_RESET_DATA);
+
+	sdhci_end_tuning(host);
+
+	spin_unlock_irqrestore(&host->lock, flags);
+	mmc_abort_tuning(host->mmc, opcode);
+	spin_lock_irqsave(&host->lock, flags);
+}
+
+static void sdhci_send_tuning(struct sdhci_host *host, u32 opcode,
+			      unsigned long flags)
+{
+	struct mmc_host *mmc = host->mmc;
+	struct mmc_command cmd = {0};
+	struct mmc_request mrq = {NULL};
+
+	cmd.opcode = opcode;
+	cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+	cmd.mrq = &mrq;
+
+	mrq.cmd = &cmd;
+	/*
+	 * In response to CMD19, the card sends 64 bytes of tuning
+	 * block to the Host Controller. So we set the block size
+	 * to 64 here.
+	 */
+	if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
+		if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
+			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
+				     SDHCI_BLOCK_SIZE);
+		else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
+			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
+				     SDHCI_BLOCK_SIZE);
+	} else {
+		sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
+			     SDHCI_BLOCK_SIZE);
+	}
+
+	/*
+	 * The tuning block is sent by the card to the host controller.
+	 * So we set the TRNS_READ bit in the Transfer Mode register.
+	 * This also takes care of setting DMA Enable and Multi Block
+	 * Select in the same register to 0.
+	 */
+	sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
+
+	sdhci_send_command(host, &cmd);
+
+	host->cmd = NULL;
+
+	sdhci_del_timer(host, &mrq);
+
+	host->tuning_done = 0;
+
+	spin_unlock_irqrestore(&host->lock, flags);
+
+	/* Wait for Buffer Read Ready interrupt */
+	wait_event_timeout(host->buf_ready_int, (host->tuning_done == 1),
+			   msecs_to_jiffies(50));
+
+	spin_lock_irqsave(&host->lock, flags);
+}
+
 static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
@@ -2011,105 +2120,24 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 		return err;
 	}
 
-	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
-	ctrl |= SDHCI_CTRL_EXEC_TUNING;
-	if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
-		ctrl |= SDHCI_CTRL_TUNED_CLK;
-	sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
-
-	/*
-	 * As per the Host Controller spec v3.00, tuning command
-	 * generates Buffer Read Ready interrupt, so enable that.
-	 *
-	 * Note: The spec clearly says that when tuning sequence
-	 * is being performed, the controller does not generate
-	 * interrupts other than Buffer Read Ready interrupt. But
-	 * to make sure we don't hit a controller bug, we _only_
-	 * enable Buffer Read Ready interrupt here.
-	 */
-	sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
-	sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE);
+	sdhci_start_tuning(host);
 
 	/*
 	 * Issue CMD19 repeatedly till Execute Tuning is set to 0 or the number
 	 * of loops reaches 40 times.
 	 */
 	do {
-		struct mmc_command cmd = {0};
-		struct mmc_request mrq = {NULL};
-
-		cmd.opcode = opcode;
-		cmd.arg = 0;
-		cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
-		cmd.retries = 0;
-		cmd.data = NULL;
-		cmd.mrq = &mrq;
-		cmd.error = 0;
-
 		if (tuning_loop_counter-- == 0)
 			break;
 
-		mrq.cmd = &cmd;
-
-		/*
-		 * In response to CMD19, the card sends 64 bytes of tuning
-		 * block to the Host Controller. So we set the block size
-		 * to 64 here.
-		 */
-		if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
-			if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
-				sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
-					     SDHCI_BLOCK_SIZE);
-			else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
-				sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
-					     SDHCI_BLOCK_SIZE);
-		} else {
-			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
-				     SDHCI_BLOCK_SIZE);
-		}
-
-		/*
-		 * The tuning block is sent by the card to the host controller.
-		 * So we set the TRNS_READ bit in the Transfer Mode register.
-		 * This also takes care of setting DMA Enable and Multi Block
-		 * Select in the same register to 0.
-		 */
-		sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
-
-		sdhci_send_command(host, &cmd);
-
-		host->cmd = NULL;
-		sdhci_del_timer(host, &mrq);
-
-		spin_unlock_irqrestore(&host->lock, flags);
-		/* Wait for Buffer Read Ready interrupt */
-		wait_event_timeout(host->buf_ready_int,
-					(host->tuning_done == 1),
-					msecs_to_jiffies(50));
-		spin_lock_irqsave(&host->lock, flags);
+		sdhci_send_tuning(host, opcode, flags);
 
 		if (!host->tuning_done) {
 			pr_info(DRIVER_NAME ": Timeout waiting for Buffer Read Ready interrupt during tuning procedure, falling back to fixed sampling clock\n");
-			ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
-			ctrl &= ~SDHCI_CTRL_TUNED_CLK;
-			ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
-			sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
-
-			sdhci_do_reset(host, SDHCI_RESET_CMD);
-			sdhci_do_reset(host, SDHCI_RESET_DATA);
-
-			sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
-			sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
-
-			spin_unlock_irqrestore(&host->lock, flags);
-			mmc_abort_tuning(mmc, opcode);
-			spin_lock_irqsave(&host->lock, flags);
-
+			sdhci_abort_tuning(host, opcode, flags);
 			goto out;
 		}
 
-		host->tuning_done = 0;
-
 		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 
 		/* eMMC spec does not require a delay between tuning cycles */
@@ -2121,18 +2149,15 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	 * The Host Driver has exhausted the maximum number of loops allowed,
 	 * so use fixed sampling frequency.
 	 */
-	if (tuning_loop_counter < 0) {
-		ctrl &= ~SDHCI_CTRL_TUNED_CLK;
-		ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
-		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
-	}
-	if (!(ctrl & SDHCI_CTRL_TUNED_CLK))
+	if (tuning_loop_counter < 0)
+		sdhci_reset_tuning(host);
+
+	if (tuning_loop_counter < 0 || !(ctrl & SDHCI_CTRL_TUNED_CLK))
 		pr_info(DRIVER_NAME ": Tuning procedure failed, falling back to fixed sampling clock\n");
 out:
 	host->mmc->retune_period = tuning_count;
 
-	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
-	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+	sdhci_end_tuning(host);
 out_unlock:
 	spin_unlock_irqrestore(&host->lock, flags);
 	return err;
-- 
1.9.1

[PATCH RFC 4/5] mmc: sdhci: Simplify tuning block size logic

[Adrian Hunter]

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fa89870a690b..e9ff7d005851 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2024,17 +2024,11 @@ static void sdhci_send_tuning(struct sdhci_host *host, u32 opcode,
 	 * block to the Host Controller. So we set the block size
 	 * to 64 here.
 	 */
-	if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
-		if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
-			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
-				     SDHCI_BLOCK_SIZE);
-		else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
-			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
-				     SDHCI_BLOCK_SIZE);
-	} else {
-		sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
-			     SDHCI_BLOCK_SIZE);
-	}
+	if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200 &&
+	    mmc->ios.bus_width == MMC_BUS_WIDTH_8)
+		sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128), SDHCI_BLOCK_SIZE);
+	else
+		sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64), SDHCI_BLOCK_SIZE);
 
 	/*
 	 * The tuning block is sent by the card to the host controller.
-- 
1.9.1

[PATCH RFC 5/5] mmc: sdhci: Tidy tuning loop

[Adrian Hunter]

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e9ff7d005851..1ac08e1a6211 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2055,11 +2055,47 @@ static void sdhci_send_tuning(struct sdhci_host *host, u32 opcode,
 	spin_lock_irqsave(&host->lock, flags);
 }
 
+static void __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode,
+				   unsigned long flags)
+{
+	int i;
+
+	/*
+	 * Issue opcode repeatedly till Execute Tuning is set to 0 or the number
+	 * of loops reaches 40 times.
+	 */
+	for (i = 0; i < MAX_TUNING_LOOP; i++) {
+		u16 ctrl;
+
+		sdhci_send_tuning(host, opcode, flags);
+
+		if (!host->tuning_done) {
+			pr_info("%s: Tuning timeout, falling back to fixed sampling clock\n",
+				mmc_hostname(host->mmc));
+			sdhci_abort_tuning(host, opcode, flags);
+			return;
+		}
+
+		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) {
+			if (ctrl & SDHCI_CTRL_TUNED_CLK)
+				return; /* Success! */
+			break;
+		}
+
+		/* eMMC spec does not require a delay between tuning cycles */
+		if (opcode == MMC_SEND_TUNING_BLOCK)
+			mdelay(1);
+	}
+
+	pr_info("%s: Tuning failed, falling back to fixed sampling clock\n",
+		mmc_hostname(host->mmc));
+	sdhci_reset_tuning(host);
+}
+
 static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
-	u16 ctrl;
-	int tuning_loop_counter = MAX_TUNING_LOOP;
 	int err = 0;
 	unsigned long flags;
 	unsigned int tuning_count = 0;
@@ -2110,50 +2146,19 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
 	if (host->ops->platform_execute_tuning) {
 		spin_unlock_irqrestore(&host->lock, flags);
-		err = host->ops->platform_execute_tuning(host, opcode);
-		return err;
+		return host->ops->platform_execute_tuning(host, opcode);
 	}
 
-	sdhci_start_tuning(host);
-
-	/*
-	 * Issue CMD19 repeatedly till Execute Tuning is set to 0 or the number
-	 * of loops reaches 40 times.
-	 */
-	do {
-		if (tuning_loop_counter-- == 0)
-			break;
-
-		sdhci_send_tuning(host, opcode, flags);
-
-		if (!host->tuning_done) {
-			pr_info(DRIVER_NAME ": Timeout waiting for Buffer Read Ready interrupt during tuning procedure, falling back to fixed sampling clock\n");
-			sdhci_abort_tuning(host, opcode, flags);
-			goto out;
-		}
-
-		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
-
-		/* eMMC spec does not require a delay between tuning cycles */
-		if (opcode == MMC_SEND_TUNING_BLOCK)
-			mdelay(1);
-	} while (ctrl & SDHCI_CTRL_EXEC_TUNING);
+	host->mmc->retune_period = tuning_count;
 
-	/*
-	 * The Host Driver has exhausted the maximum number of loops allowed,
-	 * so use fixed sampling frequency.
-	 */
-	if (tuning_loop_counter < 0)
-		sdhci_reset_tuning(host);
+	sdhci_start_tuning(host);
 
-	if (tuning_loop_counter < 0 || !(ctrl & SDHCI_CTRL_TUNED_CLK))
-		pr_info(DRIVER_NAME ": Tuning procedure failed, falling back to fixed sampling clock\n");
-out:
-	host->mmc->retune_period = tuning_count;
+	__sdhci_execute_tuning(host, opcode, flags);
 
 	sdhci_end_tuning(host);
 out_unlock:
 	spin_unlock_irqrestore(&host->lock, flags);
+
 	return err;
 }
 
-- 
1.9.1

Re: [PATCH RFC 0/5] mmc: sdhci: Tidy tuning

[Ulf Hansson]

On 1 December 2016 at 15:04, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> Here are some tidy-ups wrt SDHCI tuning.  Un-tested so far.

Nice. Thanks!

>
> Any other suggestions?  Otherwise I will test and submit the whole series.

As an additional step, could you try to make use of mmc_send_tuning(),
instead of sending the command internally from sdhci_send_tuning()?

Kind regards
Uffe

Re: [PATCH RFC 0/5] mmc: sdhci: Tidy tuning

[Adrian Hunter]

On 01/12/16 16:57, Ulf Hansson wrote:
> On 1 December 2016 at 15:04, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> Hi
>>
>> Here are some tidy-ups wrt SDHCI tuning.  Un-tested so far.
> 
> Nice. Thanks!
> 
>>
>> Any other suggestions?  Otherwise I will test and submit the whole series.
> 
> As an additional step, could you try to make use of mmc_send_tuning(),
> instead of sending the command internally from sdhci_send_tuning()?

mmc_send_tuning() is not a good fit.  SDHCI tuning command does not have a
data payload (or rather the hardware does it automatically) so unless
mmc_send_tuning() is changed it will return -EIO.  If that is changed then,
SDHCI request function needs to be changed to special-case the tuning
command because the interrupt setup is different.  Then the problem is that
there is no timeout interrupt so another timer is needed, so a special case
would have to be created for that too.

The current code is much simpler.

Re: [PATCH RFC 0/5] mmc: sdhci: Tidy tuning

[Ulf Hansson]

On 2 December 2016 at 09:28, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 01/12/16 16:57, Ulf Hansson wrote:
>> On 1 December 2016 at 15:04, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> Hi
>>>
>>> Here are some tidy-ups wrt SDHCI tuning.  Un-tested so far.
>>
>> Nice. Thanks!
>>
>>>
>>> Any other suggestions?  Otherwise I will test and submit the whole series.
>>
>> As an additional step, could you try to make use of mmc_send_tuning(),
>> instead of sending the command internally from sdhci_send_tuning()?
>
> mmc_send_tuning() is not a good fit.  SDHCI tuning command does not have a
> data payload (or rather the hardware does it automatically) so unless
> mmc_send_tuning() is changed it will return -EIO.  If that is changed then,
> SDHCI request function needs to be changed to special-case the tuning
> command because the interrupt setup is different.  Then the problem is that
> there is no timeout interrupt so another timer is needed, so a special case
> would have to be created for that too.
>
> The current code is much simpler.

Ah, I see. How about adding something like the above as comment in the
code to justify it?

Kind regards
Uffe