[PATCH v2 0/4] mmc: Convert host drivers to mmc_send_tuning()

[PATCH v2 0/4] mmc: Convert host drivers to mmc_send_tuning()

[Ulf Hansson]

Changes in v2:
	Rebased on latest next branch, which means the mmc_send_tuning() has
	changed to take the struct mmc_host as parameter instead of the struct
	mmc_card.


Due to commit "mmc: core: add core-level function for sending tuning commands",
the mmc core provides an API to send the tuning command and as well compare the
response pattern.

Convert those host drivers which implemented their own version for how to deal
with this, into using the new API.

Note, this patchset has currently only been compile tested. Any help in testing
on HW is higly appreciated.


Ulf Hansson (4):
  mmc: sdhci-esdhc-imx: Convert to mmc_send_tuning()
  mmc: sdhci-msm: Convert to mmc_send_tuning()
  mmc: dw_mmc: Convert to mmc_send_tuning()
  mmc: core: Make tuning block patterns static

 drivers/mmc/core/mmc.c             | 32 ------------------
 drivers/mmc/core/mmc_ops.c         | 30 +++++++++++++++++
 drivers/mmc/host/dw_mmc-exynos.c   | 49 +++------------------------
 drivers/mmc/host/dw_mmc.c          | 22 +-----------
 drivers/mmc/host/dw_mmc.h          |  8 +----
 drivers/mmc/host/sdhci-esdhc-imx.c | 68 ++------------------------------------
 drivers/mmc/host/sdhci-msm.c       | 50 ++++------------------------
 include/linux/mmc/mmc.h            |  5 ---
 8 files changed, 46 insertions(+), 218 deletions(-)

-- 
1.9.1

[PATCH v2 1/4] mmc: sdhci-esdhc-imx: Convert to mmc_send_tuning()

[Ulf Hansson]

Instead of having a local function taking care of sending the tuning
command, let's use the common mmc_send_tuning() API provided by the mmc
core. In this way the request will be handled as any other request by
sdhci core.

As an effect of this change, the pm_runtime_get_sync() call at
esdhc_prepare_tuning() isn't needed any more.

This patch will also introduce another change in behavior, since before
the response pattern to the tuning command wasn't verified by
sdhci-esdhc-imx. The mmc_send_tuning() does that.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased, to get latest version of the mmc_send_tuning() API.

---
 drivers/mmc/host/sdhci-esdhc-imx.c | 68 ++------------------------------------
 1 file changed, 3 insertions(+), 65 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 0135f00..12711ab 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -65,8 +65,6 @@
 /* NOTE: the minimum valid tuning start tap for mx6sl is 1 */
 #define ESDHC_TUNING_START_TAP		0x1
 
-#define ESDHC_TUNING_BLOCK_PATTERN_LEN	64
-
 /* pinctrl state */
 #define ESDHC_PINCTRL_STATE_100MHZ	"state_100mhz"
 #define ESDHC_PINCTRL_STATE_200MHZ	"state_200mhz"
@@ -692,8 +690,6 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
 	/* FIXME: delay a bit for card to be ready for next tuning due to errors */
 	mdelay(1);
 
-	/* This is balanced by the runtime put in sdhci_tasklet_finish */
-	pm_runtime_get_sync(host->mmc->parent);
 	reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
 	reg |= ESDHC_MIX_CTRL_EXE_TUNE | ESDHC_MIX_CTRL_SMPCLK_SEL |
 			ESDHC_MIX_CTRL_FBCLK_SEL;
@@ -704,54 +700,6 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
 			val, readl(host->ioaddr + ESDHC_TUNE_CTRL_STATUS));
 }
 
-static void esdhc_request_done(struct mmc_request *mrq)
-{
-	complete(&mrq->completion);
-}
-
-static int esdhc_send_tuning_cmd(struct sdhci_host *host, u32 opcode,
-				 struct scatterlist *sg)
-{
-	struct mmc_command cmd = {0};
-	struct mmc_request mrq = {NULL};
-	struct mmc_data data = {0};
-
-	cmd.opcode = opcode;
-	cmd.arg = 0;
-	cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
-
-	data.blksz = ESDHC_TUNING_BLOCK_PATTERN_LEN;
-	data.blocks = 1;
-	data.flags = MMC_DATA_READ;
-	data.sg = sg;
-	data.sg_len = 1;
-
-	mrq.cmd = &cmd;
-	mrq.cmd->mrq = &mrq;
-	mrq.data = &data;
-	mrq.data->mrq = &mrq;
-	mrq.cmd->data = mrq.data;
-
-	mrq.done = esdhc_request_done;
-	init_completion(&(mrq.completion));
-
-	spin_lock_irq(&host->lock);
-	host->mrq = &mrq;
-
-	sdhci_send_command(host, mrq.cmd);
-
-	spin_unlock_irq(&host->lock);
-
-	wait_for_completion(&mrq.completion);
-
-	if (cmd.error)
-		return cmd.error;
-	if (data.error)
-		return data.error;
-
-	return 0;
-}
-
 static void esdhc_post_tuning(struct sdhci_host *host)
 {
 	u32 reg;
@@ -763,21 +711,13 @@ static void esdhc_post_tuning(struct sdhci_host *host)
 
 static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
 {
-	struct scatterlist sg;
-	char *tuning_pattern;
 	int min, max, avg, ret;
 
-	tuning_pattern = kmalloc(ESDHC_TUNING_BLOCK_PATTERN_LEN, GFP_KERNEL);
-	if (!tuning_pattern)
-		return -ENOMEM;
-
-	sg_init_one(&sg, tuning_pattern, ESDHC_TUNING_BLOCK_PATTERN_LEN);
-
 	/* find the mininum delay first which can pass tuning */
 	min = ESDHC_TUNE_CTRL_MIN;
 	while (min < ESDHC_TUNE_CTRL_MAX) {
 		esdhc_prepare_tuning(host, min);
-		if (!esdhc_send_tuning_cmd(host, opcode, &sg))
+		if (!mmc_send_tuning(host->mmc))
 			break;
 		min += ESDHC_TUNE_CTRL_STEP;
 	}
@@ -786,7 +726,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
 	max = min + ESDHC_TUNE_CTRL_STEP;
 	while (max < ESDHC_TUNE_CTRL_MAX) {
 		esdhc_prepare_tuning(host, max);
-		if (esdhc_send_tuning_cmd(host, opcode, &sg)) {
+		if (mmc_send_tuning(host->mmc)) {
 			max -= ESDHC_TUNE_CTRL_STEP;
 			break;
 		}
@@ -796,11 +736,9 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
 	/* use average delay to get the best timing */
 	avg = (min + max) / 2;
 	esdhc_prepare_tuning(host, avg);
-	ret = esdhc_send_tuning_cmd(host, opcode, &sg);
+	ret = mmc_send_tuning(host->mmc);
 	esdhc_post_tuning(host);
 
-	kfree(tuning_pattern);
-
 	dev_dbg(mmc_dev(host->mmc), "tunning %s at 0x%x ret %d\n",
 		ret ? "failed" : "passed", avg, ret);
 
-- 
1.9.1

[PATCH v2 2/4] mmc: sdhci-msm: Convert to mmc_send_tuning()

[Ulf Hansson]

Instead of having a local hack taking care of sending the tuning
command and as well to verify the response pattern, let's convert to
the common mmc_send_tuning() API instead.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased, to get latest version of the mmc_send_tuning() API.

---
 drivers/mmc/host/sdhci-msm.c | 50 +++++++-------------------------------------
 1 file changed, 7 insertions(+), 43 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 3080438..3d32ce8 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -339,9 +339,7 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
 {
 	int tuning_seq_cnt = 3;
-	u8 phase, *data_buf, tuned_phases[16], tuned_phase_cnt = 0;
-	const u8 *tuning_block_pattern = tuning_blk_pattern_4bit;
-	int size = sizeof(tuning_blk_pattern_4bit);
+	u8 phase, tuned_phases[16], tuned_phase_cnt = 0;
 	int rc;
 	struct mmc_host *mmc = host->mmc;
 	struct mmc_ios ios = host->mmc->ios;
@@ -355,53 +353,21 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
 	      (ios.timing == MMC_TIMING_UHS_SDR104)))
 		return 0;
 
-	if ((opcode == MMC_SEND_TUNING_BLOCK_HS200) &&
-	    (mmc->ios.bus_width == MMC_BUS_WIDTH_8)) {
-		tuning_block_pattern = tuning_blk_pattern_8bit;
-		size = sizeof(tuning_blk_pattern_8bit);
-	}
-
-	data_buf = kmalloc(size, GFP_KERNEL);
-	if (!data_buf)
-		return -ENOMEM;
-
 retry:
 	/* First of all reset the tuning block */
 	rc = msm_init_cm_dll(host);
 	if (rc)
-		goto out;
+		return rc;
 
 	phase = 0;
 	do {
-		struct mmc_command cmd = { 0 };
-		struct mmc_data data = { 0 };
-		struct mmc_request mrq = {
-			.cmd = &cmd,
-			.data = &data
-		};
-		struct scatterlist sg;
-
 		/* Set the phase in delay line hw block */
 		rc = msm_config_cm_dll_phase(host, phase);
 		if (rc)
-			goto out;
+			return rc;
 
-		cmd.opcode = opcode;
-		cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
-
-		data.blksz = size;
-		data.blocks = 1;
-		data.flags = MMC_DATA_READ;
-		data.timeout_ns = NSEC_PER_SEC;	/* 1 second */
-
-		data.sg = &sg;
-		data.sg_len = 1;
-		sg_init_one(&sg, data_buf, size);
-		memset(data_buf, 0, size);
-		mmc_wait_for_req(mmc, &mrq);
-
-		if (!cmd.error && !data.error &&
-		    !memcmp(data_buf, tuning_block_pattern, size)) {
+		rc = mmc_send_tuning(mmc);
+		if (!rc) {
 			/* Tuning is successful at this tuning point */
 			tuned_phases[tuned_phase_cnt++] = phase;
 			dev_dbg(mmc_dev(mmc), "%s: Found good phase = %d\n",
@@ -413,7 +379,7 @@ retry:
 		rc = msm_find_most_appropriate_phase(host, tuned_phases,
 						     tuned_phase_cnt);
 		if (rc < 0)
-			goto out;
+			return rc;
 		else
 			phase = rc;
 
@@ -423,7 +389,7 @@ retry:
 		 */
 		rc = msm_config_cm_dll_phase(host, phase);
 		if (rc)
-			goto out;
+			return rc;
 		dev_dbg(mmc_dev(mmc), "%s: Setting the tuning phase to %d\n",
 			 mmc_hostname(mmc), phase);
 	} else {
@@ -435,8 +401,6 @@ retry:
 		rc = -EIO;
 	}
 
-out:
-	kfree(data_buf);
 	return rc;
 }
 
-- 
1.9.1

[PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Ulf Hansson]

Instead of having a local hack taking care of sending the tuning
command and as well to verify the response pattern, let's convert to
the common mmc_send_tuning() API.

This change affects the Exynos variant, since it's the only one which
support the dw_mmc's ->execute_tuning() callback.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased, to get latest version of the mmc_send_tuning() API.

---
 drivers/mmc/host/dw_mmc-exynos.c | 49 ++++------------------------------------
 drivers/mmc/host/dw_mmc.c        | 22 +-----------------
 drivers/mmc/host/dw_mmc.h        |  8 +------
 3 files changed, 6 insertions(+), 73 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 509365c..d0d7aec 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -375,64 +375,24 @@ out:
 	return loc;
 }
 
-static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot, u32 opcode,
-					struct dw_mci_tuning_data *tuning_data)
+static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
 {
 	struct dw_mci *host = slot->host;
 	struct mmc_host *mmc = slot->mmc;
-	const u8 *blk_pattern = tuning_data->blk_pattern;
-	u8 *blk_test;
-	unsigned int blksz = tuning_data->blksz;
 	u8 start_smpl, smpl, candiates = 0;
 	s8 found = -1;
 	int ret = 0;
 
-	blk_test = kmalloc(blksz, GFP_KERNEL);
-	if (!blk_test)
-		return -ENOMEM;
-
 	start_smpl = dw_mci_exynos_get_clksmpl(host);
 
 	do {
-		struct mmc_request mrq = {NULL};
-		struct mmc_command cmd = {0};
-		struct mmc_command stop = {0};
-		struct mmc_data data = {0};
-		struct scatterlist sg;
-
-		cmd.opcode = opcode;
-		cmd.arg = 0;
-		cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
-
-		stop.opcode = MMC_STOP_TRANSMISSION;
-		stop.arg = 0;
-		stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
-
-		data.blksz = blksz;
-		data.blocks = 1;
-		data.flags = MMC_DATA_READ;
-		data.sg = &sg;
-		data.sg_len = 1;
-
-		sg_init_one(&sg, blk_test, blksz);
-		mrq.cmd = &cmd;
-		mrq.stop = &stop;
-		mrq.data = &data;
-		host->mrq = &mrq;
-
 		mci_writel(host, TMOUT, ~0);
 		smpl = dw_mci_exynos_move_next_clksmpl(host);
 
-		mmc_wait_for_req(mmc, &mrq);
+		ret = mmc_send_tuning(mmc);
+		if (!ret)
+			candiates |= (1 << smpl);
 
-		if (!cmd.error && !data.error) {
-			if (!memcmp(blk_pattern, blk_test, blksz))
-				candiates |= (1 << smpl);
-		} else {
-			dev_dbg(host->dev,
-				"Tuning error: cmd.error:%d, data.error:%d\n",
-				cmd.error, data.error);
-		}
 	} while (start_smpl != smpl);
 
 	found = dw_mci_exynos_get_best_clksmpl(candiates);
@@ -441,7 +401,6 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot, u32 opcode,
 	else
 		ret = -EIO;
 
-	kfree(blk_test);
 	return ret;
 }
 
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 67c0451..265bb28 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1299,30 +1299,10 @@ static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	struct dw_mci_slot *slot = mmc_priv(mmc);
 	struct dw_mci *host = slot->host;
 	const struct dw_mci_drv_data *drv_data = host->drv_data;
-	struct dw_mci_tuning_data tuning_data;
 	int err = -ENOSYS;
 
-	if (opcode == MMC_SEND_TUNING_BLOCK_HS200) {
-		if (mmc->ios.bus_width == MMC_BUS_WIDTH_8) {
-			tuning_data.blk_pattern = tuning_blk_pattern_8bit;
-			tuning_data.blksz = sizeof(tuning_blk_pattern_8bit);
-		} else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
-			tuning_data.blk_pattern = tuning_blk_pattern_4bit;
-			tuning_data.blksz = sizeof(tuning_blk_pattern_4bit);
-		} else {
-			return -EINVAL;
-		}
-	} else if (opcode == MMC_SEND_TUNING_BLOCK) {
-		tuning_data.blk_pattern = tuning_blk_pattern_4bit;
-		tuning_data.blksz = sizeof(tuning_blk_pattern_4bit);
-	} else {
-		dev_err(host->dev,
-			"Undefined command(%d) for tuning\n", opcode);
-		return -EINVAL;
-	}
-
 	if (drv_data && drv_data->execute_tuning)
-		err = drv_data->execute_tuning(slot, opcode, &tuning_data);
+		err = drv_data->execute_tuning(slot);
 	return err;
 }
 
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 0d0f7a2..c1f4557 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -248,11 +248,6 @@ struct dw_mci_slot {
 	int			sdio_id;
 };
 
-struct dw_mci_tuning_data {
-	const u8 *blk_pattern;
-	unsigned int blksz;
-};
-
 /**
  * dw_mci driver data - dw-mshc implementation specific driver data.
  * @caps: mmc subsystem specified capabilities of the controller(s).
@@ -274,7 +269,6 @@ struct dw_mci_drv_data {
 	void		(*prepare_command)(struct dw_mci *host, u32 *cmdr);
 	void		(*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
 	int		(*parse_dt)(struct dw_mci *host);
-	int		(*execute_tuning)(struct dw_mci_slot *slot, u32 opcode,
-					struct dw_mci_tuning_data *tuning_data);
+	int		(*execute_tuning)(struct dw_mci_slot *slot);
 };
 #endif /* _DW_MMC_H_ */
-- 
1.9.1

[PATCH v2 4/4] mmc: core: Make tuning block patterns static

[Ulf Hansson]

Since previous patches removed the need for the tuning block patterns
to be exported, let's move them close to the mmc_send_tuning() API.

Those are now intended to be used only by the mmc core.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	None.

---
 drivers/mmc/core/mmc.c     | 32 --------------------------------
 drivers/mmc/core/mmc_ops.c | 30 ++++++++++++++++++++++++++++++
 include/linux/mmc/mmc.h    |  5 -----
 3 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 02ad792..ea96705 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1155,38 +1155,6 @@ bus_speed:
 	return err;
 }
 
-const u8 tuning_blk_pattern_4bit[MMC_TUNING_BLK_PATTERN_4BIT_SIZE] = {
-	0xff, 0x0f, 0xff, 0x00, 0xff, 0xcc, 0xc3, 0xcc,
-	0xc3, 0x3c, 0xcc, 0xff, 0xfe, 0xff, 0xfe, 0xef,
-	0xff, 0xdf, 0xff, 0xdd, 0xff, 0xfb, 0xff, 0xfb,
-	0xbf, 0xff, 0x7f, 0xff, 0x77, 0xf7, 0xbd, 0xef,
-	0xff, 0xf0, 0xff, 0xf0, 0x0f, 0xfc, 0xcc, 0x3c,
-	0xcc, 0x33, 0xcc, 0xcf, 0xff, 0xef, 0xff, 0xee,
-	0xff, 0xfd, 0xff, 0xfd, 0xdf, 0xff, 0xbf, 0xff,
-	0xbb, 0xff, 0xf7, 0xff, 0xf7, 0x7f, 0x7b, 0xde,
-};
-EXPORT_SYMBOL(tuning_blk_pattern_4bit);
-
-const u8 tuning_blk_pattern_8bit[MMC_TUNING_BLK_PATTERN_8BIT_SIZE] = {
-	0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00, 0x00,
-	0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc, 0xcc,
-	0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff, 0xff,
-	0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee, 0xff,
-	0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd, 0xdd,
-	0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff, 0xbb,
-	0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff, 0xff,
-	0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee, 0xff,
-	0xff, 0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00,
-	0x00, 0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc,
-	0xcc, 0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff,
-	0xff, 0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee,
-	0xff, 0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd,
-	0xdd, 0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff,
-	0xbb, 0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff,
-	0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
-};
-EXPORT_SYMBOL(tuning_blk_pattern_8bit);
-
 /*
  * Execute tuning sequence to seek the proper bus operating
  * conditions for HS200 and HS400, which sends CMD21 to the device.
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 3b044c5..0ea042d 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -23,6 +23,36 @@
 
 #define MMC_OPS_TIMEOUT_MS	(10 * 60 * 1000) /* 10 minute timeout */
 
+static const u8 tuning_blk_pattern_4bit[] = {
+	0xff, 0x0f, 0xff, 0x00, 0xff, 0xcc, 0xc3, 0xcc,
+	0xc3, 0x3c, 0xcc, 0xff, 0xfe, 0xff, 0xfe, 0xef,
+	0xff, 0xdf, 0xff, 0xdd, 0xff, 0xfb, 0xff, 0xfb,
+	0xbf, 0xff, 0x7f, 0xff, 0x77, 0xf7, 0xbd, 0xef,
+	0xff, 0xf0, 0xff, 0xf0, 0x0f, 0xfc, 0xcc, 0x3c,
+	0xcc, 0x33, 0xcc, 0xcf, 0xff, 0xef, 0xff, 0xee,
+	0xff, 0xfd, 0xff, 0xfd, 0xdf, 0xff, 0xbf, 0xff,
+	0xbb, 0xff, 0xf7, 0xff, 0xf7, 0x7f, 0x7b, 0xde,
+};
+
+static const u8 tuning_blk_pattern_8bit[] = {
+	0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00, 0x00,
+	0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc, 0xcc,
+	0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff, 0xff,
+	0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee, 0xff,
+	0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd, 0xdd,
+	0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff, 0xbb,
+	0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff, 0xff,
+	0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee, 0xff,
+	0xff, 0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00,
+	0x00, 0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc,
+	0xcc, 0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff,
+	0xff, 0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee,
+	0xff, 0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd,
+	0xdd, 0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff,
+	0xbb, 0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff,
+	0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
+};
+
 static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
 				    bool ignore_crc)
 {
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 49ad7a9..fb97b5c 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -53,11 +53,6 @@
 #define MMC_SEND_TUNING_BLOCK    19   /* adtc                    R1  */
 #define MMC_SEND_TUNING_BLOCK_HS200	21	/* adtc R1  */
 
-#define MMC_TUNING_BLK_PATTERN_4BIT_SIZE	 64
-#define MMC_TUNING_BLK_PATTERN_8BIT_SIZE	128
-extern const u8 tuning_blk_pattern_4bit[MMC_TUNING_BLK_PATTERN_4BIT_SIZE];
-extern const u8 tuning_blk_pattern_8bit[MMC_TUNING_BLK_PATTERN_8BIT_SIZE];
-
   /* class 3 */
 #define MMC_WRITE_DAT_UNTIL_STOP 20   /* adtc [31:0] data addr   R1  */
 
-- 
1.9.1

Re: [PATCH v2 2/4] mmc: sdhci-msm: Convert to mmc_send_tuning()

[Georgi Djakov]

On 12/05/2014 01:59 PM, Ulf Hansson wrote:
> Instead of having a local hack taking care of sending the tuning
> command and as well to verify the response pattern, let's convert to
> the common mmc_send_tuning() API instead.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Hi Ulf,
Tuning works for eMMC on apq8074-dragonboard and ifc6540 boards, so:
Tested-by: Georgi Djakov <gdjakov@mm-sol.com>
Acked-by: Georgi Djakov <gdjakov@mm-sol.com>

Thanks,
Georgi

> Changes in v2:
> 	Rebased, to get latest version of the mmc_send_tuning() API.
> 
> ---
>  drivers/mmc/host/sdhci-msm.c | 50 +++++++-------------------------------------
>  1 file changed, 7 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 3080438..3d32ce8 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -339,9 +339,7 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>  {
>  	int tuning_seq_cnt = 3;
> -	u8 phase, *data_buf, tuned_phases[16], tuned_phase_cnt = 0;
> -	const u8 *tuning_block_pattern = tuning_blk_pattern_4bit;
> -	int size = sizeof(tuning_blk_pattern_4bit);
> +	u8 phase, tuned_phases[16], tuned_phase_cnt = 0;
>  	int rc;
>  	struct mmc_host *mmc = host->mmc;
>  	struct mmc_ios ios = host->mmc->ios;
> @@ -355,53 +353,21 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>  	      (ios.timing == MMC_TIMING_UHS_SDR104)))
>  		return 0;
>  
> -	if ((opcode == MMC_SEND_TUNING_BLOCK_HS200) &&
> -	    (mmc->ios.bus_width == MMC_BUS_WIDTH_8)) {
> -		tuning_block_pattern = tuning_blk_pattern_8bit;
> -		size = sizeof(tuning_blk_pattern_8bit);
> -	}
> -
> -	data_buf = kmalloc(size, GFP_KERNEL);
> -	if (!data_buf)
> -		return -ENOMEM;
> -
>  retry:
>  	/* First of all reset the tuning block */
>  	rc = msm_init_cm_dll(host);
>  	if (rc)
> -		goto out;
> +		return rc;
>  
>  	phase = 0;
>  	do {
> -		struct mmc_command cmd = { 0 };
> -		struct mmc_data data = { 0 };
> -		struct mmc_request mrq = {
> -			.cmd = &cmd,
> -			.data = &data
> -		};
> -		struct scatterlist sg;
> -
>  		/* Set the phase in delay line hw block */
>  		rc = msm_config_cm_dll_phase(host, phase);
>  		if (rc)
> -			goto out;
> +			return rc;
>  
> -		cmd.opcode = opcode;
> -		cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
> -
> -		data.blksz = size;
> -		data.blocks = 1;
> -		data.flags = MMC_DATA_READ;
> -		data.timeout_ns = NSEC_PER_SEC;	/* 1 second */
> -
> -		data.sg = &sg;
> -		data.sg_len = 1;
> -		sg_init_one(&sg, data_buf, size);
> -		memset(data_buf, 0, size);
> -		mmc_wait_for_req(mmc, &mrq);
> -
> -		if (!cmd.error && !data.error &&
> -		    !memcmp(data_buf, tuning_block_pattern, size)) {
> +		rc = mmc_send_tuning(mmc);
> +		if (!rc) {
>  			/* Tuning is successful at this tuning point */
>  			tuned_phases[tuned_phase_cnt++] = phase;
>  			dev_dbg(mmc_dev(mmc), "%s: Found good phase = %d\n",
> @@ -413,7 +379,7 @@ retry:
>  		rc = msm_find_most_appropriate_phase(host, tuned_phases,
>  						     tuned_phase_cnt);
>  		if (rc < 0)
> -			goto out;
> +			return rc;
>  		else
>  			phase = rc;
>  
> @@ -423,7 +389,7 @@ retry:
>  		 */
>  		rc = msm_config_cm_dll_phase(host, phase);
>  		if (rc)
> -			goto out;
> +			return rc;
>  		dev_dbg(mmc_dev(mmc), "%s: Setting the tuning phase to %d\n",
>  			 mmc_hostname(mmc), phase);
>  	} else {
> @@ -435,8 +401,6 @@ retry:
>  		rc = -EIO;
>  	}
>  
> -out:
> -	kfree(data_buf);
>  	return rc;
>  }
>  
> 

Re: [PATCH v2 4/4] mmc: core: Make tuning block patterns static

[Stephen Boyd]

On 12/05/2014 03:59 AM, Ulf Hansson wrote:
> Since previous patches removed the need for the tuning block patterns
> to be exported, let's move them close to the mmc_send_tuning() API.
>
> Those are now intended to be used only by the mmc core.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 2/4] mmc: sdhci-msm: Convert to mmc_send_tuning()

[Stephen Boyd]

On 12/05/2014 03:59 AM, Ulf Hansson wrote:
> Instead of having a local hack taking care of sending the tuning
> command and as well to verify the response pattern, let's convert to
> the common mmc_send_tuning() API instead.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Alim Akhtar]

Hi Ulf,

On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Instead of having a local hack taking care of sending the tuning
> command and as well to verify the response pattern, let's convert to
> the common mmc_send_tuning() API.
>
> This change affects the Exynos variant, since it's the only one which
> support the dw_mmc's ->execute_tuning() callback.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>
With this change HS200 mode does not work on exynos5800 peach-pi board.
I got below error while testing this series:

mmc0: tuning execution failed
mmc0: error -5 whilst initialising MMC card

Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.


> Changes in v2:
>         Rebased, to get latest version of the mmc_send_tuning() API.
>
> ---
>  drivers/mmc/host/dw_mmc-exynos.c | 49 ++++------------------------------------
>  drivers/mmc/host/dw_mmc.c        | 22 +-----------------
>  drivers/mmc/host/dw_mmc.h        |  8 +------
>  3 files changed, 6 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index 509365c..d0d7aec 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -375,64 +375,24 @@ out:
>         return loc;
>  }
>
> -static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot, u32 opcode,
> -                                       struct dw_mci_tuning_data *tuning_data)
> +static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
>  {
>         struct dw_mci *host = slot->host;
>         struct mmc_host *mmc = slot->mmc;
> -       const u8 *blk_pattern = tuning_data->blk_pattern;
> -       u8 *blk_test;
> -       unsigned int blksz = tuning_data->blksz;
>         u8 start_smpl, smpl, candiates = 0;
>         s8 found = -1;
>         int ret = 0;
>
> -       blk_test = kmalloc(blksz, GFP_KERNEL);
> -       if (!blk_test)
> -               return -ENOMEM;
> -
>         start_smpl = dw_mci_exynos_get_clksmpl(host);
>
>         do {
> -               struct mmc_request mrq = {NULL};
> -               struct mmc_command cmd = {0};
> -               struct mmc_command stop = {0};
> -               struct mmc_data data = {0};
> -               struct scatterlist sg;
> -
> -               cmd.opcode = opcode;
> -               cmd.arg = 0;
> -               cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
> -
> -               stop.opcode = MMC_STOP_TRANSMISSION;
> -               stop.arg = 0;
> -               stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
> -
> -               data.blksz = blksz;
> -               data.blocks = 1;
> -               data.flags = MMC_DATA_READ;
> -               data.sg = &sg;
> -               data.sg_len = 1;
> -
> -               sg_init_one(&sg, blk_test, blksz);
> -               mrq.cmd = &cmd;
> -               mrq.stop = &stop;
> -               mrq.data = &data;
> -               host->mrq = &mrq;
> -
>                 mci_writel(host, TMOUT, ~0);
>                 smpl = dw_mci_exynos_move_next_clksmpl(host);
>
> -               mmc_wait_for_req(mmc, &mrq);
> +               ret = mmc_send_tuning(mmc);
> +               if (!ret)
> +                       candiates |= (1 << smpl);
>
> -               if (!cmd.error && !data.error) {
> -                       if (!memcmp(blk_pattern, blk_test, blksz))
> -                               candiates |= (1 << smpl);
> -               } else {
> -                       dev_dbg(host->dev,
> -                               "Tuning error: cmd.error:%d, data.error:%d\n",
> -                               cmd.error, data.error);
> -               }
>         } while (start_smpl != smpl);
>
>         found = dw_mci_exynos_get_best_clksmpl(candiates);
> @@ -441,7 +401,6 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot, u32 opcode,
>         else
>                 ret = -EIO;
>
> -       kfree(blk_test);
>         return ret;
>  }
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 67c0451..265bb28 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1299,30 +1299,10 @@ static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>         struct dw_mci_slot *slot = mmc_priv(mmc);
>         struct dw_mci *host = slot->host;
>         const struct dw_mci_drv_data *drv_data = host->drv_data;
> -       struct dw_mci_tuning_data tuning_data;
>         int err = -ENOSYS;
>
> -       if (opcode == MMC_SEND_TUNING_BLOCK_HS200) {
> -               if (mmc->ios.bus_width == MMC_BUS_WIDTH_8) {
> -                       tuning_data.blk_pattern = tuning_blk_pattern_8bit;
> -                       tuning_data.blksz = sizeof(tuning_blk_pattern_8bit);
> -               } else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
> -                       tuning_data.blk_pattern = tuning_blk_pattern_4bit;
> -                       tuning_data.blksz = sizeof(tuning_blk_pattern_4bit);
> -               } else {
> -                       return -EINVAL;
> -               }
> -       } else if (opcode == MMC_SEND_TUNING_BLOCK) {
> -               tuning_data.blk_pattern = tuning_blk_pattern_4bit;
> -               tuning_data.blksz = sizeof(tuning_blk_pattern_4bit);
> -       } else {
> -               dev_err(host->dev,
> -                       "Undefined command(%d) for tuning\n", opcode);
> -               return -EINVAL;
> -       }
> -
>         if (drv_data && drv_data->execute_tuning)
> -               err = drv_data->execute_tuning(slot, opcode, &tuning_data);
> +               err = drv_data->execute_tuning(slot);
>         return err;
>  }
>
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 0d0f7a2..c1f4557 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -248,11 +248,6 @@ struct dw_mci_slot {
>         int                     sdio_id;
>  };
>
> -struct dw_mci_tuning_data {
> -       const u8 *blk_pattern;
> -       unsigned int blksz;
> -};
> -
>  /**
>   * dw_mci driver data - dw-mshc implementation specific driver data.
>   * @caps: mmc subsystem specified capabilities of the controller(s).
> @@ -274,7 +269,6 @@ struct dw_mci_drv_data {
>         void            (*prepare_command)(struct dw_mci *host, u32 *cmdr);
>         void            (*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
>         int             (*parse_dt)(struct dw_mci *host);
> -       int             (*execute_tuning)(struct dw_mci_slot *slot, u32 opcode,
> -                                       struct dw_mci_tuning_data *tuning_data);
> +       int             (*execute_tuning)(struct dw_mci_slot *slot);
>  };
>  #endif /* _DW_MMC_H_ */
> --
> 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



-- 
Regards,
Alim

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Ulf Hansson]

On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
> Hi Ulf,
>
> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Instead of having a local hack taking care of sending the tuning
>> command and as well to verify the response pattern, let's convert to
>> the common mmc_send_tuning() API.
>>
>> This change affects the Exynos variant, since it's the only one which
>> support the dw_mmc's ->execute_tuning() callback.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>

Alim, thanks for helping out testing!

> With this change HS200 mode does not work on exynos5800 peach-pi board.
> I got below error while testing this series:
>
> mmc0: tuning execution failed
> mmc0: error -5 whilst initialising MMC card
>
> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.

I was looking into the details of what change my patchset introduces
for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
specifications states.

Do you have any idea of why dw_mmc-exynos was using
MMC_STOP_TRANSMISSION for CMD19/21?

To see if my theory is correct, could you try out the following patch
on top of $subject patch?
BTW, I have queued patch 1 and 2, from this patchset available on my
next branch.


>From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <ulf.hansson@linaro.org>
Date: Mon, 8 Dec 2014 10:58:48 +0100
Subject: [PATCH] mmc: core: End tuning request with stop command

Not to be merged!

This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
request. For some reason dw_mmc seems to need this to complete the
tuning request without errors.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/mmc_ops.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 3b044c5..aa79e0c 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
 {
        struct mmc_request mrq = {NULL};
        struct mmc_command cmd = {0};
+       struct mmc_command stop = {0};
        struct mmc_data data = {0};
        struct scatterlist sg;
        struct mmc_ios *ios = &host->ios;
@@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)

        mrq.cmd = &cmd;
        mrq.data = &data;
+       mrq.stop = &stop;

        cmd.opcode = opcode;
        cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;

+       stop.opcode = MMC_STOP_TRANSMISSION;
+       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
+
        data.blksz = size;
        data.blocks = 1;
        data.flags = MMC_DATA_READ;
-- 
1.9.1

Kind regards
Uffe

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Alim Akhtar]

Hi Ulf

On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>> Hi Ulf,
>>
>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> Instead of having a local hack taking care of sending the tuning
>>> command and as well to verify the response pattern, let's convert to
>>> the common mmc_send_tuning() API.
>>>
>>> This change affects the Exynos variant, since it's the only one which
>>> support the dw_mmc's ->execute_tuning() callback.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>
>
> Alim, thanks for helping out testing!
>
Sorry for the delay but currently I don't have access to my work station.
And thanks for quick suggested patch, I will test this as soon asI go
back to my work.

Hi Jaehoon, can you test this and the original patch for hs200 on
exynos? Just to confirm my board is not the special one.
Thanks.

>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>> I got below error while testing this series:
>>
>> mmc0: tuning execution failed
>> mmc0: error -5 whilst initialising MMC card
>>
>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>
> I was looking into the details of what change my patchset introduces
> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
> specifications states.
>
> Do you have any idea of why dw_mmc-exynos was using
> MMC_STOP_TRANSMISSION for CMD19/21?
>
> To see if my theory is correct, could you try out the following patch
> on top of $subject patch?
> BTW, I have queued patch 1 and 2, from this patchset available on my
> next branch.
>
>
> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Mon, 8 Dec 2014 10:58:48 +0100
> Subject: [PATCH] mmc: core: End tuning request with stop command
>
> Not to be merged!
>
> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
> request. For some reason dw_mmc seems to need this to complete the
> tuning request without errors.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/core/mmc_ops.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 3b044c5..aa79e0c 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>  {
>         struct mmc_request mrq = {NULL};
>         struct mmc_command cmd = {0};
> +       struct mmc_command stop = {0};
>         struct mmc_data data = {0};
>         struct scatterlist sg;
>         struct mmc_ios *ios = &host->ios;
> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>
>         mrq.cmd = &cmd;
>         mrq.data = &data;
> +       mrq.stop = &stop;
>
>         cmd.opcode = opcode;
>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>
> +       stop.opcode = MMC_STOP_TRANSMISSION;
> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
> +
>         data.blksz = size;
>         data.blocks = 1;
>         data.flags = MMC_DATA_READ;
> --
> 1.9.1
>
> Kind regards
> Uffe



-- 
Regards,
Alim

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Jaehoon Chung]

Hi.

On 12/10/2014 06:30 AM, Alim Akhtar wrote:
> Hi Ulf
> 
> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>> Hi Ulf,
>>>
>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>> Instead of having a local hack taking care of sending the tuning
>>>> command and as well to verify the response pattern, let's convert to
>>>> the common mmc_send_tuning() API.
>>>>
>>>> This change affects the Exynos variant, since it's the only one which
>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>
>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>> ---
>>>>
>>
>> Alim, thanks for helping out testing!
>>
> Sorry for the delay but currently I don't have access to my work station.
> And thanks for quick suggested patch, I will test this as soon asI go
> back to my work.
> 
> Hi Jaehoon, can you test this and the original patch for hs200 on
> exynos? Just to confirm my board is not the special one.
> Thanks.

Does it need to send "stop command" at exynos5800?
Well, most SoC doesn't need to send stop command, also exynos.
But i will check other exynos boards..then i will share result.

Best Regards,
Jaehoon Chung

> 
>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>> I got below error while testing this series:
>>>
>>> mmc0: tuning execution failed
>>> mmc0: error -5 whilst initialising MMC card
>>>
>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>
>> I was looking into the details of what change my patchset introduces
>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>> specifications states.
>>
>> Do you have any idea of why dw_mmc-exynos was using
>> MMC_STOP_TRANSMISSION for CMD19/21?
>>
>> To see if my theory is correct, could you try out the following patch
>> on top of $subject patch?
>> BTW, I have queued patch 1 and 2, from this patchset available on my
>> next branch.
>>
>>
>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>
>> Not to be merged!
>>
>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>> request. For some reason dw_mmc seems to need this to complete the
>> tuning request without errors.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>> index 3b044c5..aa79e0c 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>  {
>>         struct mmc_request mrq = {NULL};
>>         struct mmc_command cmd = {0};
>> +       struct mmc_command stop = {0};
>>         struct mmc_data data = {0};
>>         struct scatterlist sg;
>>         struct mmc_ios *ios = &host->ios;
>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>
>>         mrq.cmd = &cmd;
>>         mrq.data = &data;
>> +       mrq.stop = &stop;
>>
>>         cmd.opcode = opcode;
>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>
>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>> +
>>         data.blksz = size;
>>         data.blocks = 1;
>>         data.flags = MMC_DATA_READ;
>> --
>> 1.9.1
>>
>> Kind regards
>> Uffe
> 
> 
> 

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Jaehoon Chung]

Hi, Alim.

On 12/13/2014 12:13 AM, Jaehoon Chung wrote:
> Hi.
> 
> On 12/10/2014 06:30 AM, Alim Akhtar wrote:
>> Hi Ulf
>>
>> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>>> Hi Ulf,
>>>>
>>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>> Instead of having a local hack taking care of sending the tuning
>>>>> command and as well to verify the response pattern, let's convert to
>>>>> the common mmc_send_tuning() API.
>>>>>
>>>>> This change affects the Exynos variant, since it's the only one which
>>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> ---
>>>>>
>>>
>>> Alim, thanks for helping out testing!
>>>
>> Sorry for the delay but currently I don't have access to my work station.
>> And thanks for quick suggested patch, I will test this as soon asI go
>> back to my work.
>>
>> Hi Jaehoon, can you test this and the original patch for hs200 on
>> exynos? Just to confirm my board is not the special one.
>> Thanks.
> 
> Does it need to send "stop command" at exynos5800?
> Well, most SoC doesn't need to send stop command, also exynos.
> But i will check other exynos boards..then i will share result.
> 
> Best Regards,
> Jaehoon Chung
> 
>>
>>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>>> I got below error while testing this series:
>>>>
>>>> mmc0: tuning execution failed
>>>> mmc0: error -5 whilst initialising MMC card

Did you share the CLKSEL register value?
I think exynos SoC didn't need to send stop command.
(I didn't find the STOP command is completed when tuning sequence.)
When i have tested, according to SELCLK_DRV value, tuning results were changed.

if you got the above error, i recommend you can change the SELCLK_DRV value.

Best Regards,
Jaehoon Chung

>>>>
>>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>>
>>> I was looking into the details of what change my patchset introduces
>>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>>> specifications states.
>>>
>>> Do you have any idea of why dw_mmc-exynos was using
>>> MMC_STOP_TRANSMISSION for CMD19/21?
>>>
>>> To see if my theory is correct, could you try out the following patch
>>> on top of $subject patch?
>>> BTW, I have queued patch 1 and 2, from this patchset available on my
>>> next branch.
>>>
>>>
>>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>>
>>> Not to be merged!
>>>
>>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>>> request. For some reason dw_mmc seems to need this to complete the
>>> tuning request without errors.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>> index 3b044c5..aa79e0c 100644
>>> --- a/drivers/mmc/core/mmc_ops.c
>>> +++ b/drivers/mmc/core/mmc_ops.c
>>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>>  {
>>>         struct mmc_request mrq = {NULL};
>>>         struct mmc_command cmd = {0};
>>> +       struct mmc_command stop = {0};
>>>         struct mmc_data data = {0};
>>>         struct scatterlist sg;
>>>         struct mmc_ios *ios = &host->ios;
>>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>>
>>>         mrq.cmd = &cmd;
>>>         mrq.data = &data;
>>> +       mrq.stop = &stop;
>>>
>>>         cmd.opcode = opcode;
>>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>>
>>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>>> +
>>>         data.blksz = size;
>>>         data.blocks = 1;
>>>         data.flags = MMC_DATA_READ;
>>> --
>>> 1.9.1
>>>
>>> Kind regards
>>> Uffe
>>
>>
>>
> 
> --
> 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
> 

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Alim Akhtar]

Hi Ulf,

On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>> Hi Ulf,
>>
>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> Instead of having a local hack taking care of sending the tuning
>>> command and as well to verify the response pattern, let's convert to
>>> the common mmc_send_tuning() API.
>>>
>>> This change affects the Exynos variant, since it's the only one which
>>> support the dw_mmc's ->execute_tuning() callback.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>
>
> Alim, thanks for helping out testing!
>
>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>> I got below error while testing this series:
>>
>> mmc0: tuning execution failed
>> mmc0: error -5 whilst initialising MMC card
>>
>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>
> I was looking into the details of what change my patchset introduces
> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
> specifications states.
>
> Do you have any idea of why dw_mmc-exynos was using
> MMC_STOP_TRANSMISSION for CMD19/21?
>
> To see if my theory is correct, could you try out the following patch
> on top of $subject patch?
> BTW, I have queued patch 1 and 2, from this patchset available on my
> next branch.
>
>
> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Mon, 8 Dec 2014 10:58:48 +0100
> Subject: [PATCH] mmc: core: End tuning request with stop command
>
> Not to be merged!
>
> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
> request. For some reason dw_mmc seems to need this to complete the
> tuning request without errors.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/core/mmc_ops.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 3b044c5..aa79e0c 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>  {
>         struct mmc_request mrq = {NULL};
>         struct mmc_command cmd = {0};
> +       struct mmc_command stop = {0};
>         struct mmc_data data = {0};
>         struct scatterlist sg;
>         struct mmc_ios *ios = &host->ios;
> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>
>         mrq.cmd = &cmd;
>         mrq.data = &data;
> +       mrq.stop = &stop;
>
>         cmd.opcode = opcode;
>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>
> +       stop.opcode = MMC_STOP_TRANSMISSION;
> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
> +
>         data.blksz = size;
>         data.blocks = 1;
>         data.flags = MMC_DATA_READ;
> --
> 1.9.1
>
Sorry for delay in testing this suggested patch, I would say this
certainly helps, but still I need to change sample phase to make it
work with generic tuning function.
So with your's adding STOP_TRANSMISSION command and below change,
HS200 works well on exynos5800

diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts
b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index e8fdda8..e0f0337 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -555,7 +555,7 @@
        card-detect-delay = <200>;
        clock-frequency = <400000000>;
        samsung,dw-mshc-ciu-div = <3>;
-       samsung,dw-mshc-sdr-timing = <0 4>;
+       samsung,dw-mshc-sdr-timing = <2 4>;

This basically change the clock-sample phase with which the tuning
process starts.

I didn't find anything is exynos documentations which suggest
STOP_TRANSMISSION is needed, may be Seungwon might be know as he wrote
this.

Are you ok to add STOP_TRANSMISSION in generic tuning code (probably
with a quirk, so that other hosts are still happy)?
If so, I can send the above DT change as a fix.

> Kind regards
> Uffe



-- 
Regards,
Alim

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Alim Akhtar]

Hi Jaehoon

On Mon, Dec 15, 2014 at 10:18 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi, Alim.
>
> On 12/13/2014 12:13 AM, Jaehoon Chung wrote:
>> Hi.
>>
>> On 12/10/2014 06:30 AM, Alim Akhtar wrote:
>>> Hi Ulf
>>>
>>> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>>>> Hi Ulf,
>>>>>
>>>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>>> Instead of having a local hack taking care of sending the tuning
>>>>>> command and as well to verify the response pattern, let's convert to
>>>>>> the common mmc_send_tuning() API.
>>>>>>
>>>>>> This change affects the Exynos variant, since it's the only one which
>>>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>>>
>>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>> ---
>>>>>>
>>>>
>>>> Alim, thanks for helping out testing!
>>>>
>>> Sorry for the delay but currently I don't have access to my work station.
>>> And thanks for quick suggested patch, I will test this as soon asI go
>>> back to my work.
>>>
>>> Hi Jaehoon, can you test this and the original patch for hs200 on
>>> exynos? Just to confirm my board is not the special one.
>>> Thanks.
>>
>> Does it need to send "stop command" at exynos5800?
>> Well, most SoC doesn't need to send stop command, also exynos.
>> But i will check other exynos boards..then i will share result.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>>>> I got below error while testing this series:
>>>>>
>>>>> mmc0: tuning execution failed
>>>>> mmc0: error -5 whilst initialising MMC card
>
> Did you share the CLKSEL register value?

CLKSEL value remain same in failed and pass case (in my case it is
0x0304000x, x is for sample phases)

> I think exynos SoC didn't need to send stop command.
> (I didn't find the STOP command is completed when tuning sequence.)
> When i have tested, according to SELCLK_DRV value, tuning results were changed.
>
Did you tested on exynos5?
> if you got the above error, i recommend you can change the SELCLK_DRV value.
>
I did tried changing that but didn't helped, but not tried all
combinations though, I can recheck this again.

> Best Regards,
> Jaehoon Chung
>
>>>>>
>>>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>>>
>>>> I was looking into the details of what change my patchset introduces
>>>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>>>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>>>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>>>> specifications states.
>>>>
>>>> Do you have any idea of why dw_mmc-exynos was using
>>>> MMC_STOP_TRANSMISSION for CMD19/21?
>>>>
>>>> To see if my theory is correct, could you try out the following patch
>>>> on top of $subject patch?
>>>> BTW, I have queued patch 1 and 2, from this patchset available on my
>>>> next branch.
>>>>
>>>>
>>>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>>>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>>>
>>>> Not to be merged!
>>>>
>>>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>>>> request. For some reason dw_mmc seems to need this to complete the
>>>> tuning request without errors.
>>>>
>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>> ---
>>>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>>> index 3b044c5..aa79e0c 100644
>>>> --- a/drivers/mmc/core/mmc_ops.c
>>>> +++ b/drivers/mmc/core/mmc_ops.c
>>>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>>>  {
>>>>         struct mmc_request mrq = {NULL};
>>>>         struct mmc_command cmd = {0};
>>>> +       struct mmc_command stop = {0};
>>>>         struct mmc_data data = {0};
>>>>         struct scatterlist sg;
>>>>         struct mmc_ios *ios = &host->ios;
>>>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>>>
>>>>         mrq.cmd = &cmd;
>>>>         mrq.data = &data;
>>>> +       mrq.stop = &stop;
>>>>
>>>>         cmd.opcode = opcode;
>>>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>>>
>>>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>>>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>>>> +
>>>>         data.blksz = size;
>>>>         data.blocks = 1;
>>>>         data.flags = MMC_DATA_READ;
>>>> --
>>>> 1.9.1
>>>>
>>>> Kind regards
>>>> Uffe
>>>
>>>
>>>
>>
>> --
>> 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
>>
>



-- 
Regards,
Alim

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Jaehoon Chung]

Hi, Alim.

On 12/20/2014 10:34 PM, Alim Akhtar wrote:
> Hi Jaehoon
> 
> On Mon, Dec 15, 2014 at 10:18 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi, Alim.
>>
>> On 12/13/2014 12:13 AM, Jaehoon Chung wrote:
>>> Hi.
>>>
>>> On 12/10/2014 06:30 AM, Alim Akhtar wrote:
>>>> Hi Ulf
>>>>
>>>> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>>>>> Hi Ulf,
>>>>>>
>>>>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>>>> Instead of having a local hack taking care of sending the tuning
>>>>>>> command and as well to verify the response pattern, let's convert to
>>>>>>> the common mmc_send_tuning() API.
>>>>>>>
>>>>>>> This change affects the Exynos variant, since it's the only one which
>>>>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>>>>
>>>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>>> ---
>>>>>>>
>>>>>
>>>>> Alim, thanks for helping out testing!
>>>>>
>>>> Sorry for the delay but currently I don't have access to my work station.
>>>> And thanks for quick suggested patch, I will test this as soon asI go
>>>> back to my work.
>>>>
>>>> Hi Jaehoon, can you test this and the original patch for hs200 on
>>>> exynos? Just to confirm my board is not the special one.
>>>> Thanks.
>>>
>>> Does it need to send "stop command" at exynos5800?
>>> Well, most SoC doesn't need to send stop command, also exynos.
>>> But i will check other exynos boards..then i will share result.
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>
>>>>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>>>>> I got below error while testing this series:
>>>>>>
>>>>>> mmc0: tuning execution failed
>>>>>> mmc0: error -5 whilst initialising MMC card
>>
>> Did you share the CLKSEL register value?
> 
> CLKSEL value remain same in failed and pass case (in my case it is
> 0x0304000x, x is for sample phases)

Thanks for share the value. i will check it.

> 
>> I think exynos SoC didn't need to send stop command.
>> (I didn't find the STOP command is completed when tuning sequence.)
>> When i have tested, according to SELCLK_DRV value, tuning results were changed.
>>
> Did you tested on exynos5?

Sure, I have tested on exynos5. It's working well without stop command.

>> if you got the above error, i recommend you can change the SELCLK_DRV value.
>>
> I did tried changing that but didn't helped, but not tried all
> combinations though, I can recheck this again.

Right. there is too many combinations..so i also didn't check all combinations.
Did you enable the Debugging message for mmc?

Best Regards,
Jaehoon Chung

> 
>> Best Regards,
>> Jaehoon Chung
>>
>>>>>>
>>>>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>>>>
>>>>> I was looking into the details of what change my patchset introduces
>>>>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>>>>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>>>>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>>>>> specifications states.
>>>>>
>>>>> Do you have any idea of why dw_mmc-exynos was using
>>>>> MMC_STOP_TRANSMISSION for CMD19/21?
>>>>>
>>>>> To see if my theory is correct, could you try out the following patch
>>>>> on top of $subject patch?
>>>>> BTW, I have queued patch 1 and 2, from this patchset available on my
>>>>> next branch.
>>>>>
>>>>>
>>>>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>>>>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>>>>
>>>>> Not to be merged!
>>>>>
>>>>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>>>>> request. For some reason dw_mmc seems to need this to complete the
>>>>> tuning request without errors.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> ---
>>>>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>>>> index 3b044c5..aa79e0c 100644
>>>>> --- a/drivers/mmc/core/mmc_ops.c
>>>>> +++ b/drivers/mmc/core/mmc_ops.c
>>>>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>>>>  {
>>>>>         struct mmc_request mrq = {NULL};
>>>>>         struct mmc_command cmd = {0};
>>>>> +       struct mmc_command stop = {0};
>>>>>         struct mmc_data data = {0};
>>>>>         struct scatterlist sg;
>>>>>         struct mmc_ios *ios = &host->ios;
>>>>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>>>>
>>>>>         mrq.cmd = &cmd;
>>>>>         mrq.data = &data;
>>>>> +       mrq.stop = &stop;
>>>>>
>>>>>         cmd.opcode = opcode;
>>>>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>>>>
>>>>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>>>>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>>>>> +
>>>>>         data.blksz = size;
>>>>>         data.blocks = 1;
>>>>>         data.flags = MMC_DATA_READ;
>>>>> --
>>>>> 1.9.1
>>>>>
>>>>> Kind regards
>>>>> Uffe
>>>>
>>>>
>>>>
>>>
>>> --
>>> 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
>>>
>>
> 
> 
> 

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Ulf Hansson]

On 20 December 2014 at 14:18, Alim Akhtar <alim.akhtar@gmail.com> wrote:
> Hi Ulf,
>
> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>> Hi Ulf,
>>>
>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>> Instead of having a local hack taking care of sending the tuning
>>>> command and as well to verify the response pattern, let's convert to
>>>> the common mmc_send_tuning() API.
>>>>
>>>> This change affects the Exynos variant, since it's the only one which
>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>
>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>> ---
>>>>
>>
>> Alim, thanks for helping out testing!
>>
>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>> I got below error while testing this series:
>>>
>>> mmc0: tuning execution failed
>>> mmc0: error -5 whilst initialising MMC card
>>>
>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>
>> I was looking into the details of what change my patchset introduces
>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>> specifications states.
>>
>> Do you have any idea of why dw_mmc-exynos was using
>> MMC_STOP_TRANSMISSION for CMD19/21?
>>
>> To see if my theory is correct, could you try out the following patch
>> on top of $subject patch?
>> BTW, I have queued patch 1 and 2, from this patchset available on my
>> next branch.
>>
>>
>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>
>> Not to be merged!
>>
>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>> request. For some reason dw_mmc seems to need this to complete the
>> tuning request without errors.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>> index 3b044c5..aa79e0c 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>  {
>>         struct mmc_request mrq = {NULL};
>>         struct mmc_command cmd = {0};
>> +       struct mmc_command stop = {0};
>>         struct mmc_data data = {0};
>>         struct scatterlist sg;
>>         struct mmc_ios *ios = &host->ios;
>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>
>>         mrq.cmd = &cmd;
>>         mrq.data = &data;
>> +       mrq.stop = &stop;
>>
>>         cmd.opcode = opcode;
>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>
>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>> +
>>         data.blksz = size;
>>         data.blocks = 1;
>>         data.flags = MMC_DATA_READ;
>> --
>> 1.9.1
>>
> Sorry for delay in testing this suggested patch, I would say this
> certainly helps, but still I need to change sample phase to make it
> work with generic tuning function.
> So with your's adding STOP_TRANSMISSION command and below change,
> HS200 works well on exynos5800
>
> diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts
> b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> index e8fdda8..e0f0337 100644
> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> @@ -555,7 +555,7 @@
>         card-detect-delay = <200>;
>         clock-frequency = <400000000>;
>         samsung,dw-mshc-ciu-div = <3>;
> -       samsung,dw-mshc-sdr-timing = <0 4>;
> +       samsung,dw-mshc-sdr-timing = <2 4>;
>
> This basically change the clock-sample phase with which the tuning
> process starts.
>
> I didn't find anything is exynos documentations which suggest
> STOP_TRANSMISSION is needed, may be Seungwon might be know as he wrote
> this.
>
> Are you ok to add STOP_TRANSMISSION in generic tuning code (probably
> with a quirk, so that other hosts are still happy)?

No, I don't want that.

If special treatment are needed for dw_mmc to handle a tuning request,
dw_mmc should be able to take care by itself. How about below patch,
do you think that could work?

>From c2e5783beaa322178f2336b4bff4bdf13da76f56 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <ulf.hansson@linaro.org>
Date: Mon, 22 Dec 2014 15:24:14 +0100
Subject: [PATCH] mmc: dw_mmc: Generate stop commands for failed tuning
 requests

It's seems like dw_mmc internal logic expects failed data transfers to
be ended using a stop command. Let the tuning requests also fall into
this category, since there are data transfer involved.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/dw_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index a1b80e5..976382db 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -314,7 +314,9 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci
*host, struct mmc_command *cmd)
        if (cmdr == MMC_READ_SINGLE_BLOCK ||
            cmdr == MMC_READ_MULTIPLE_BLOCK ||
            cmdr == MMC_WRITE_BLOCK ||
-           cmdr == MMC_WRITE_MULTIPLE_BLOCK) {
+           cmdr == MMC_WRITE_MULTIPLE_BLOCK ||
+           cmdr == MMC_SEND_TUNING_BLOCK ||
+           cmdr == MMC_SEND_TUNING_BLOCK_HS200) {
                stop->opcode = MMC_STOP_TRANSMISSION;
                stop->arg = 0;
                stop->flags = MMC_RSP_R1B | MMC_CMD_AC;
-- 
1.9.1

> If so, I can send the above DT change as a fix.

Please do!

Again, I appreciate all your efforts in helping out!

Kind regards
Uffe

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Jaehoon Chung]

Hi. Ulf.

Your patch has bug..
If last sample value is failed, then it's returned with error.

I will recommend this. how about?

 drivers/mmc/host/dw_mmc-exynos.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index d0d7aec..d6b8846 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -381,7 +381,7 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
        struct mmc_host *mmc = slot->mmc;
        u8 start_smpl, smpl, candiates = 0;
        s8 found = -1;
-       int ret = 0;
+       int ret = 0, success;

        start_smpl = dw_mci_exynos_get_clksmpl(host);

@@ -389,8 +389,8 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
                mci_writel(host, TMOUT, ~0);
                smpl = dw_mci_exynos_move_next_clksmpl(host);

-               ret = mmc_send_tuning(mmc);
-               if (!ret)
+               success = mmc_send_tuning(mmc);
+               if (!success)
                        candiates |= (1 << smpl);

        } while (start_smpl != smpl);
--
1.9.1


On 12/22/2014 11:41 PM, Ulf Hansson wrote:
> On 20 December 2014 at 14:18, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>> Hi Ulf,
>>
>> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>>> Hi Ulf,
>>>>
>>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>> Instead of having a local hack taking care of sending the tuning
>>>>> command and as well to verify the response pattern, let's convert to
>>>>> the common mmc_send_tuning() API.
>>>>>
>>>>> This change affects the Exynos variant, since it's the only one which
>>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> ---
>>>>>
>>>
>>> Alim, thanks for helping out testing!
>>>
>>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>>> I got below error while testing this series:
>>>>
>>>> mmc0: tuning execution failed
>>>> mmc0: error -5 whilst initialising MMC card
>>>>
>>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>>
>>> I was looking into the details of what change my patchset introduces
>>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>>> specifications states.
>>>
>>> Do you have any idea of why dw_mmc-exynos was using
>>> MMC_STOP_TRANSMISSION for CMD19/21?
>>>
>>> To see if my theory is correct, could you try out the following patch
>>> on top of $subject patch?
>>> BTW, I have queued patch 1 and 2, from this patchset available on my
>>> next branch.
>>>
>>>
>>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>>
>>> Not to be merged!
>>>
>>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>>> request. For some reason dw_mmc seems to need this to complete the
>>> tuning request without errors.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>> index 3b044c5..aa79e0c 100644
>>> --- a/drivers/mmc/core/mmc_ops.c
>>> +++ b/drivers/mmc/core/mmc_ops.c
>>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>>  {
>>>         struct mmc_request mrq = {NULL};
>>>         struct mmc_command cmd = {0};
>>> +       struct mmc_command stop = {0};
>>>         struct mmc_data data = {0};
>>>         struct scatterlist sg;
>>>         struct mmc_ios *ios = &host->ios;
>>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>>
>>>         mrq.cmd = &cmd;
>>>         mrq.data = &data;
>>> +       mrq.stop = &stop;
>>>
>>>         cmd.opcode = opcode;
>>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>>
>>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>>> +
>>>         data.blksz = size;
>>>         data.blocks = 1;
>>>         data.flags = MMC_DATA_READ;
>>> --
>>> 1.9.1
>>>
>> Sorry for delay in testing this suggested patch, I would say this
>> certainly helps, but still I need to change sample phase to make it
>> work with generic tuning function.
>> So with your's adding STOP_TRANSMISSION command and below change,
>> HS200 works well on exynos5800
>>
>> diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> index e8fdda8..e0f0337 100644
>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> @@ -555,7 +555,7 @@
>>         card-detect-delay = <200>;
>>         clock-frequency = <400000000>;
>>         samsung,dw-mshc-ciu-div = <3>;
>> -       samsung,dw-mshc-sdr-timing = <0 4>;
>> +       samsung,dw-mshc-sdr-timing = <2 4>;
>>
>> This basically change the clock-sample phase with which the tuning
>> process starts.
>>
>> I didn't find anything is exynos documentations which suggest
>> STOP_TRANSMISSION is needed, may be Seungwon might be know as he wrote
>> this.
>>
>> Are you ok to add STOP_TRANSMISSION in generic tuning code (probably
>> with a quirk, so that other hosts are still happy)?
> 
> No, I don't want that.
> 
> If special treatment are needed for dw_mmc to handle a tuning request,
> dw_mmc should be able to take care by itself. How about below patch,
> do you think that could work?
> 
>>From c2e5783beaa322178f2336b4bff4bdf13da76f56 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Mon, 22 Dec 2014 15:24:14 +0100
> Subject: [PATCH] mmc: dw_mmc: Generate stop commands for failed tuning
>  requests
> 
> It's seems like dw_mmc internal logic expects failed data transfers to
> be ended using a stop command. Let the tuning requests also fall into
> this category, since there are data transfer involved.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/host/dw_mmc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index a1b80e5..976382db 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -314,7 +314,9 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci
> *host, struct mmc_command *cmd)
>         if (cmdr == MMC_READ_SINGLE_BLOCK ||
>             cmdr == MMC_READ_MULTIPLE_BLOCK ||
>             cmdr == MMC_WRITE_BLOCK ||
> -           cmdr == MMC_WRITE_MULTIPLE_BLOCK) {
> +           cmdr == MMC_WRITE_MULTIPLE_BLOCK ||
> +           cmdr == MMC_SEND_TUNING_BLOCK ||
> +           cmdr == MMC_SEND_TUNING_BLOCK_HS200) {
>                 stop->opcode = MMC_STOP_TRANSMISSION;
>                 stop->arg = 0;
>                 stop->flags = MMC_RSP_R1B | MMC_CMD_AC;
> 

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Jaehoon Chung]

Hi. Ulf. 

change from success to error or other..:)
sorry.

And dw-mmc controller doesn't need to send stop command. it's not spec.
I believe this changing is fixed tuning sequence's problem for dw-mmc.

Best Regards,
Jaehoon Chung

On 12/23/2014 06:49 PM, Jaehoon Chung wrote:
> Hi. Ulf.
> 
> Your patch has bug..
> If last sample value is failed, then it's returned with error.
> 
> I will recommend this. how about?
> 
>  drivers/mmc/host/dw_mmc-exynos.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index d0d7aec..d6b8846 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -381,7 +381,7 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
>         struct mmc_host *mmc = slot->mmc;
>         u8 start_smpl, smpl, candiates = 0;
>         s8 found = -1;
> -       int ret = 0;
> +       int ret = 0, success;
> 
>         start_smpl = dw_mci_exynos_get_clksmpl(host);
> 
> @@ -389,8 +389,8 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
>                 mci_writel(host, TMOUT, ~0);
>                 smpl = dw_mci_exynos_move_next_clksmpl(host);
> 
> -               ret = mmc_send_tuning(mmc);
> -               if (!ret)
> +               success = mmc_send_tuning(mmc);
> +               if (!success)
>                         candiates |= (1 << smpl);
> 
>         } while (start_smpl != smpl);
> --
> 1.9.1
> 
> 
> On 12/22/2014 11:41 PM, Ulf Hansson wrote:
>> On 20 December 2014 at 14:18, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>> Hi Ulf,
>>>
>>> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>>>> Hi Ulf,
>>>>>
>>>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>>> Instead of having a local hack taking care of sending the tuning
>>>>>> command and as well to verify the response pattern, let's convert to
>>>>>> the common mmc_send_tuning() API.
>>>>>>
>>>>>> This change affects the Exynos variant, since it's the only one which
>>>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>>>
>>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>> ---
>>>>>>
>>>>
>>>> Alim, thanks for helping out testing!
>>>>
>>>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>>>> I got below error while testing this series:
>>>>>
>>>>> mmc0: tuning execution failed
>>>>> mmc0: error -5 whilst initialising MMC card
>>>>>
>>>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>>>
>>>> I was looking into the details of what change my patchset introduces
>>>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>>>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>>>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>>>> specifications states.
>>>>
>>>> Do you have any idea of why dw_mmc-exynos was using
>>>> MMC_STOP_TRANSMISSION for CMD19/21?
>>>>
>>>> To see if my theory is correct, could you try out the following patch
>>>> on top of $subject patch?
>>>> BTW, I have queued patch 1 and 2, from this patchset available on my
>>>> next branch.
>>>>
>>>>
>>>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>>>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>>>
>>>> Not to be merged!
>>>>
>>>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>>>> request. For some reason dw_mmc seems to need this to complete the
>>>> tuning request without errors.
>>>>
>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>> ---
>>>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>>> index 3b044c5..aa79e0c 100644
>>>> --- a/drivers/mmc/core/mmc_ops.c
>>>> +++ b/drivers/mmc/core/mmc_ops.c
>>>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>>>  {
>>>>         struct mmc_request mrq = {NULL};
>>>>         struct mmc_command cmd = {0};
>>>> +       struct mmc_command stop = {0};
>>>>         struct mmc_data data = {0};
>>>>         struct scatterlist sg;
>>>>         struct mmc_ios *ios = &host->ios;
>>>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>>>
>>>>         mrq.cmd = &cmd;
>>>>         mrq.data = &data;
>>>> +       mrq.stop = &stop;
>>>>
>>>>         cmd.opcode = opcode;
>>>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>>>
>>>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>>>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>>>> +
>>>>         data.blksz = size;
>>>>         data.blocks = 1;
>>>>         data.flags = MMC_DATA_READ;
>>>> --
>>>> 1.9.1
>>>>
>>> Sorry for delay in testing this suggested patch, I would say this
>>> certainly helps, but still I need to change sample phase to make it
>>> work with generic tuning function.
>>> So with your's adding STOP_TRANSMISSION command and below change,
>>> HS200 works well on exynos5800
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts
>>> b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>>> index e8fdda8..e0f0337 100644
>>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
>>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>>> @@ -555,7 +555,7 @@
>>>         card-detect-delay = <200>;
>>>         clock-frequency = <400000000>;
>>>         samsung,dw-mshc-ciu-div = <3>;
>>> -       samsung,dw-mshc-sdr-timing = <0 4>;
>>> +       samsung,dw-mshc-sdr-timing = <2 4>;
>>>
>>> This basically change the clock-sample phase with which the tuning
>>> process starts.
>>>
>>> I didn't find anything is exynos documentations which suggest
>>> STOP_TRANSMISSION is needed, may be Seungwon might be know as he wrote
>>> this.
>>>
>>> Are you ok to add STOP_TRANSMISSION in generic tuning code (probably
>>> with a quirk, so that other hosts are still happy)?
>>
>> No, I don't want that.
>>
>> If special treatment are needed for dw_mmc to handle a tuning request,
>> dw_mmc should be able to take care by itself. How about below patch,
>> do you think that could work?
>>
>> >From c2e5783beaa322178f2336b4bff4bdf13da76f56 Mon Sep 17 00:00:00 2001
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>> Date: Mon, 22 Dec 2014 15:24:14 +0100
>> Subject: [PATCH] mmc: dw_mmc: Generate stop commands for failed tuning
>>  requests
>>
>> It's seems like dw_mmc internal logic expects failed data transfers to
>> be ended using a stop command. Let the tuning requests also fall into
>> this category, since there are data transfer involved.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index a1b80e5..976382db 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -314,7 +314,9 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci
>> *host, struct mmc_command *cmd)
>>         if (cmdr == MMC_READ_SINGLE_BLOCK ||
>>             cmdr == MMC_READ_MULTIPLE_BLOCK ||
>>             cmdr == MMC_WRITE_BLOCK ||
>> -           cmdr == MMC_WRITE_MULTIPLE_BLOCK) {
>> +           cmdr == MMC_WRITE_MULTIPLE_BLOCK ||
>> +           cmdr == MMC_SEND_TUNING_BLOCK ||
>> +           cmdr == MMC_SEND_TUNING_BLOCK_HS200) {
>>                 stop->opcode = MMC_STOP_TRANSMISSION;
>>                 stop->arg = 0;
>>                 stop->flags = MMC_RSP_R1B | MMC_CMD_AC;
>>
> 
> --
> 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
> 

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Ulf Hansson]

On 23 December 2014 at 10:49, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi. Ulf.
>
> Your patch has bug..
> If last sample value is failed, then it's returned with error.
>
> I will recommend this. how about?
>
>  drivers/mmc/host/dw_mmc-exynos.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index d0d7aec..d6b8846 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -381,7 +381,7 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
>         struct mmc_host *mmc = slot->mmc;
>         u8 start_smpl, smpl, candiates = 0;
>         s8 found = -1;
> -       int ret = 0;
> +       int ret = 0, success;
>
>         start_smpl = dw_mci_exynos_get_clksmpl(host);
>
> @@ -389,8 +389,8 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
>                 mci_writel(host, TMOUT, ~0);
>                 smpl = dw_mci_exynos_move_next_clksmpl(host);
>
> -               ret = mmc_send_tuning(mmc);
> -               if (!ret)
> +               success = mmc_send_tuning(mmc);
> +               if (!success)
>                         candiates |= (1 << smpl);
>
>         } while (start_smpl != smpl);
> --
> 1.9.1
>

Urgh! Thanks a lot for spotting this Jaehoon! I will update my patch
according to your proposal. Sorry for all the noise I caused around
this.

Kind regards
Uffe

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Jaehoon Chung]

Hi, Ulf.

On 12/22/2014 11:41 PM, Ulf Hansson wrote:
> On 20 December 2014 at 14:18, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>> Hi Ulf,
>>
>> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>>> Hi Ulf,
>>>>
>>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>> Instead of having a local hack taking care of sending the tuning
>>>>> command and as well to verify the response pattern, let's convert to
>>>>> the common mmc_send_tuning() API.
>>>>>
>>>>> This change affects the Exynos variant, since it's the only one which
>>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> ---
>>>>>
>>>
>>> Alim, thanks for helping out testing!
>>>
>>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>>> I got below error while testing this series:
>>>>
>>>> mmc0: tuning execution failed
>>>> mmc0: error -5 whilst initialising MMC card
>>>>
>>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>>
>>> I was looking into the details of what change my patchset introduces
>>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>>> specifications states.
>>>
>>> Do you have any idea of why dw_mmc-exynos was using
>>> MMC_STOP_TRANSMISSION for CMD19/21?
>>>
>>> To see if my theory is correct, could you try out the following patch
>>> on top of $subject patch?
>>> BTW, I have queued patch 1 and 2, from this patchset available on my
>>> next branch.
>>>
>>>
>>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>>
>>> Not to be merged!
>>>
>>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>>> request. For some reason dw_mmc seems to need this to complete the
>>> tuning request without errors.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>> index 3b044c5..aa79e0c 100644
>>> --- a/drivers/mmc/core/mmc_ops.c
>>> +++ b/drivers/mmc/core/mmc_ops.c
>>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>>  {
>>>         struct mmc_request mrq = {NULL};
>>>         struct mmc_command cmd = {0};
>>> +       struct mmc_command stop = {0};
>>>         struct mmc_data data = {0};
>>>         struct scatterlist sg;
>>>         struct mmc_ios *ios = &host->ios;
>>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>>
>>>         mrq.cmd = &cmd;
>>>         mrq.data = &data;
>>> +       mrq.stop = &stop;
>>>
>>>         cmd.opcode = opcode;
>>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>>
>>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>>> +
>>>         data.blksz = size;
>>>         data.blocks = 1;
>>>         data.flags = MMC_DATA_READ;
>>> --
>>> 1.9.1
>>>
>> Sorry for delay in testing this suggested patch, I would say this
>> certainly helps, but still I need to change sample phase to make it
>> work with generic tuning function.
>> So with your's adding STOP_TRANSMISSION command and below change,
>> HS200 works well on exynos5800
>>
>> diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> index e8fdda8..e0f0337 100644
>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> @@ -555,7 +555,7 @@
>>         card-detect-delay = <200>;
>>         clock-frequency = <400000000>;
>>         samsung,dw-mshc-ciu-div = <3>;
>> -       samsung,dw-mshc-sdr-timing = <0 4>;
>> +       samsung,dw-mshc-sdr-timing = <2 4>;
>>
>> This basically change the clock-sample phase with which the tuning
>> process starts.
>>
>> I didn't find anything is exynos documentations which suggest
>> STOP_TRANSMISSION is needed, may be Seungwon might be know as he wrote
>> this.
>>
>> Are you ok to add STOP_TRANSMISSION in generic tuning code (probably
>> with a quirk, so that other hosts are still happy)?
> 
> No, I don't want that.
> 
> If special treatment are needed for dw_mmc to handle a tuning request,
> dw_mmc should be able to take care by itself. How about below patch,
> do you think that could work?

According to my analysis, s/w logic of dwmmc controller needs to add the below your patch.
If cmd->error is occurred, dw-mmc controller needs to send stop command.
I will rework for this..But now this patch is good solution for tuning sequence.

Best Regards,
Jaehoon Chung
> 
>>From c2e5783beaa322178f2336b4bff4bdf13da76f56 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Mon, 22 Dec 2014 15:24:14 +0100
> Subject: [PATCH] mmc: dw_mmc: Generate stop commands for failed tuning
>  requests
> 
> It's seems like dw_mmc internal logic expects failed data transfers to
> be ended using a stop command. Let the tuning requests also fall into
> this category, since there are data transfer involved.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/host/dw_mmc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index a1b80e5..976382db 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -314,7 +314,9 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci
> *host, struct mmc_command *cmd)
>         if (cmdr == MMC_READ_SINGLE_BLOCK ||
>             cmdr == MMC_READ_MULTIPLE_BLOCK ||
>             cmdr == MMC_WRITE_BLOCK ||
> -           cmdr == MMC_WRITE_MULTIPLE_BLOCK) {
> +           cmdr == MMC_WRITE_MULTIPLE_BLOCK ||
> +           cmdr == MMC_SEND_TUNING_BLOCK ||
> +           cmdr == MMC_SEND_TUNING_BLOCK_HS200) {
>                 stop->opcode = MMC_STOP_TRANSMISSION;
>                 stop->arg = 0;
>                 stop->flags = MMC_RSP_R1B | MMC_CMD_AC;
> 

Re: [PATCH v2 3/4] mmc: dw_mmc: Convert to mmc_send_tuning()

[Alim Akhtar]

Hi Ulf,

On Mon, Dec 22, 2014 at 8:11 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 20 December 2014 at 14:18, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>> Hi Ulf,
>>
>> On Mon, Dec 8, 2014 at 3:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> On 6 December 2014 at 13:43, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>>> Hi Ulf,
>>>>
>>>> On Fri, Dec 5, 2014 at 5:29 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>> Instead of having a local hack taking care of sending the tuning
>>>>> command and as well to verify the response pattern, let's convert to
>>>>> the common mmc_send_tuning() API.
>>>>>
>>>>> This change affects the Exynos variant, since it's the only one which
>>>>> support the dw_mmc's ->execute_tuning() callback.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> ---
>>>>>
>>>
>>> Alim, thanks for helping out testing!
>>>
>>>> With this change HS200 mode does not work on exynos5800 peach-pi board.
>>>> I got below error while testing this series:
>>>>
>>>> mmc0: tuning execution failed
>>>> mmc0: error -5 whilst initialising MMC card
>>>>
>>>> Though, your's next branch with commit _a1d06b4_ works fine in HS200 mode.
>>>
>>> I was looking into the details of what change my patchset introduces
>>> for dw_mmc-exynos. Apparently, dw_mmc-exynos was using the
>>> MMC_STOP_TRANSMISSION to end the tuning reqeust (CMD21/19). The new
>>> mmc_send_tuning() API doesn't, which also conforms to what the eMMC/SD
>>> specifications states.
>>>
>>> Do you have any idea of why dw_mmc-exynos was using
>>> MMC_STOP_TRANSMISSION for CMD19/21?
>>>
>>> To see if my theory is correct, could you try out the following patch
>>> on top of $subject patch?
>>> BTW, I have queued patch 1 and 2, from this patchset available on my
>>> next branch.
>>>
>>>
>>> From e1ac35bb0e90254275ec7088f41e6e2d59e48367 Mon Sep 17 00:00:00 2001
>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>> Date: Mon, 8 Dec 2014 10:58:48 +0100
>>> Subject: [PATCH] mmc: core: End tuning request with stop command
>>>
>>> Not to be merged!
>>>
>>> This patch adds the MMC_STOP_TRANSMISSION command to end a tuning
>>> request. For some reason dw_mmc seems to need this to complete the
>>> tuning request without errors.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>  drivers/mmc/core/mmc_ops.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>> index 3b044c5..aa79e0c 100644
>>> --- a/drivers/mmc/core/mmc_ops.c
>>> +++ b/drivers/mmc/core/mmc_ops.c
>>> @@ -551,6 +551,7 @@ int mmc_send_tuning(struct mmc_host *host)
>>>  {
>>>         struct mmc_request mrq = {NULL};
>>>         struct mmc_command cmd = {0};
>>> +       struct mmc_command stop = {0};
>>>         struct mmc_data data = {0};
>>>         struct scatterlist sg;
>>>         struct mmc_ios *ios = &host->ios;
>>> @@ -576,10 +577,14 @@ int mmc_send_tuning(struct mmc_host *host)
>>>
>>>         mrq.cmd = &cmd;
>>>         mrq.data = &data;
>>> +       mrq.stop = &stop;
>>>
>>>         cmd.opcode = opcode;
>>>         cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>>
>>> +       stop.opcode = MMC_STOP_TRANSMISSION;
>>> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
>>> +
>>>         data.blksz = size;
>>>         data.blocks = 1;
>>>         data.flags = MMC_DATA_READ;
>>> --
>>> 1.9.1
>>>
>> Sorry for delay in testing this suggested patch, I would say this
>> certainly helps, but still I need to change sample phase to make it
>> work with generic tuning function.
>> So with your's adding STOP_TRANSMISSION command and below change,
>> HS200 works well on exynos5800
>>
>> diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> index e8fdda8..e0f0337 100644
>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> @@ -555,7 +555,7 @@
>>         card-detect-delay = <200>;
>>         clock-frequency = <400000000>;
>>         samsung,dw-mshc-ciu-div = <3>;
>> -       samsung,dw-mshc-sdr-timing = <0 4>;
>> +       samsung,dw-mshc-sdr-timing = <2 4>;
>>
>> This basically change the clock-sample phase with which the tuning
>> process starts.
>>
>> I didn't find anything is exynos documentations which suggest
>> STOP_TRANSMISSION is needed, may be Seungwon might be know as he wrote
>> this.
>>
>> Are you ok to add STOP_TRANSMISSION in generic tuning code (probably
>> with a quirk, so that other hosts are still happy)?
>
> No, I don't want that.
>
> If special treatment are needed for dw_mmc to handle a tuning request,
> dw_mmc should be able to take care by itself. How about below patch,
> do you think that could work?
>
> From c2e5783beaa322178f2336b4bff4bdf13da76f56 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Mon, 22 Dec 2014 15:24:14 +0100
> Subject: [PATCH] mmc: dw_mmc: Generate stop commands for failed tuning
>  requests
>
> It's seems like dw_mmc internal logic expects failed data transfers to
> be ended using a stop command. Let the tuning requests also fall into
> this category, since there are data transfer involved.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
Yes, this works for me. So,
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
Tested-by: Alim Akhtar <alim.akhtar@samsung.com>

>  drivers/mmc/host/dw_mmc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index a1b80e5..976382db 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -314,7 +314,9 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci
> *host, struct mmc_command *cmd)
>         if (cmdr == MMC_READ_SINGLE_BLOCK ||
>             cmdr == MMC_READ_MULTIPLE_BLOCK ||
>             cmdr == MMC_WRITE_BLOCK ||
> -           cmdr == MMC_WRITE_MULTIPLE_BLOCK) {
> +           cmdr == MMC_WRITE_MULTIPLE_BLOCK ||
> +           cmdr == MMC_SEND_TUNING_BLOCK ||
> +           cmdr == MMC_SEND_TUNING_BLOCK_HS200) {
>                 stop->opcode = MMC_STOP_TRANSMISSION;
>                 stop->arg = 0;
>                 stop->flags = MMC_RSP_R1B | MMC_CMD_AC;
> --
> 1.9.1
>
>> If so, I can send the above DT change as a fix.
>
> Please do!
>
> Again, I appreciate all your efforts in helping out!
>
> Kind regards
> Uffe



-- 
Regards,
Alim