[PATCH V1 01/11] mmc: tegra: fix ddr signaling for non-ddr modes

[PATCH V1 01/11] mmc: tegra: fix ddr signaling for non-ddr modes

[Sowjanya Komatineni]

ddr_signaling is set to true for DDR50 and DDR52 modes but is
not set back to false for other modes. This programs incorrect
host clock when mode change happens from DDR52/DDR50 to other
SDR or HS modes like incase of mmc_retune where it switches
from HS400 to HS DDR and then from HS DDR to HS mode and then
to HS200.

This patch fixes the ddr_signaling to set properly for non DDR
modes.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 32e62904c0d3..46086dd43bfb 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -779,6 +779,7 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
 	bool set_dqs_trim = false;
 	bool do_hs400_dll_cal = false;
 
+	tegra_host->ddr_signaling = false;
 	switch (timing) {
 	case MMC_TIMING_UHS_SDR50:
 	case MMC_TIMING_UHS_SDR104:
-- 
2.7.4

[PATCH V1 01/11] mmc: tegra: fix ddr signaling for non-ddr modes

[Sowjanya Komatineni]

ddr_signaling is set to true for DDR50 and DDR52 modes but is
not set back to false for other modes. This programs incorrect
host clock when mode change happens from DDR52/DDR50 to other
SDR or HS modes like incase of mmc_retune where it switches
from HS400 to HS DDR and then from HS DDR to HS mode and then
to HS200.

This patch fixes the ddr_signaling to set properly for non DDR
modes.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 32e62904c0d3..46086dd43bfb 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -779,6 +779,7 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
 	bool set_dqs_trim = false;
 	bool do_hs400_dll_cal = false;
 
+	tegra_host->ddr_signaling = false;
 	switch (timing) {
 	case MMC_TIMING_UHS_SDR50:
 	case MMC_TIMING_UHS_SDR104:
-- 
2.7.4

[PATCH V1 02/11] mmc: sdhci: allow host to specify maximum tuning loops

[Sowjanya Komatineni]

As per the Host Controller Standard Specification Version 4.20,
limitation of tuning iteration count is removed as PLL locking
time can be longer than UHS-1 tuning due to larger PVT fluctuation
and it will result in increase of tuning iteration to complete the
tuning.

This patch creates a hook get_max_tuning_loop_count to allow hosts
to specify maximum tuning iterations and updates execute_tuning
to use the specified maximum tuning iteration count.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci.c | 7 +++++--
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a8141ff9be03..e9e919218006 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2366,12 +2366,15 @@ EXPORT_SYMBOL_GPL(sdhci_send_tuning);
 static int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 {
 	int i;
+	int tuning_loop_count = MAX_TUNING_LOOP;
 
+	if (host->ops->get_max_tuning_loop_count)
+		tuning_loop_count = host->ops->get_max_tuning_loop_count(host);
 	/*
 	 * Issue opcode repeatedly till Execute Tuning is set to 0 or the number
-	 * of loops reaches 40 times.
+	 * of loops reaches tuning loop count.
 	 */
-	for (i = 0; i < MAX_TUNING_LOOP; i++) {
+	for (i = 0; i < tuning_loop_count; i++) {
 		u16 ctrl;
 
 		sdhci_send_tuning(host, opcode);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 01002cba1359..c80e0d6f9b10 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -638,6 +638,7 @@ struct sdhci_ops {
 	unsigned int    (*get_ro)(struct sdhci_host *host);
 	void		(*reset)(struct sdhci_host *host, u8 mask);
 	int	(*platform_execute_tuning)(struct sdhci_host *host, u32 opcode);
+	int	(*get_max_tuning_loop_count)(struct sdhci_host *host);
 	void	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
 	void	(*hw_reset)(struct sdhci_host *host);
 	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
-- 
2.7.4

[PATCH V1 02/11] mmc: sdhci: allow host to specify maximum tuning loops

[Sowjanya Komatineni]

As per the Host Controller Standard Specification Version 4.20,
limitation of tuning iteration count is removed as PLL locking
time can be longer than UHS-1 tuning due to larger PVT fluctuation
and it will result in increase of tuning iteration to complete the
tuning.

This patch creates a hook get_max_tuning_loop_count to allow hosts
to specify maximum tuning iterations and updates execute_tuning
to use the specified maximum tuning iteration count.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci.c | 7 +++++--
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a8141ff9be03..e9e919218006 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2366,12 +2366,15 @@ EXPORT_SYMBOL_GPL(sdhci_send_tuning);
 static int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 {
 	int i;
+	int tuning_loop_count = MAX_TUNING_LOOP;
 
+	if (host->ops->get_max_tuning_loop_count)
+		tuning_loop_count = host->ops->get_max_tuning_loop_count(host);
 	/*
 	 * Issue opcode repeatedly till Execute Tuning is set to 0 or the number
-	 * of loops reaches 40 times.
+	 * of loops reaches tuning loop count.
 	 */
-	for (i = 0; i < MAX_TUNING_LOOP; i++) {
+	for (i = 0; i < tuning_loop_count; i++) {
 		u16 ctrl;
 
 		sdhci_send_tuning(host, opcode);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 01002cba1359..c80e0d6f9b10 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -638,6 +638,7 @@ struct sdhci_ops {
 	unsigned int    (*get_ro)(struct sdhci_host *host);
 	void		(*reset)(struct sdhci_host *host, u8 mask);
 	int	(*platform_execute_tuning)(struct sdhci_host *host, u32 opcode);
+	int	(*get_max_tuning_loop_count)(struct sdhci_host *host);
 	void	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
 	void	(*hw_reset)(struct sdhci_host *host);
 	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
-- 
2.7.4

[PATCH V1 03/11] mmc: sdhci: add support for post tuning process

[Sowjanya Komatineni]

This patch adds support for post tuning process needed for some hosts
to perform after successful completion of HW tuning.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci.c | 6 +++++-
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e9e919218006..976d4d1e2400 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2392,8 +2392,12 @@ static int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 
 		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 		if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) {
-			if (ctrl & SDHCI_CTRL_TUNED_CLK)
+			if (ctrl & SDHCI_CTRL_TUNED_CLK) {
+				if (host->ops->post_tuning)
+					host->ops->post_tuning(host);
 				return 0; /* Success! */
+			}
+
 			break;
 		}
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c80e0d6f9b10..236d67778645 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -639,6 +639,7 @@ struct sdhci_ops {
 	void		(*reset)(struct sdhci_host *host, u8 mask);
 	int	(*platform_execute_tuning)(struct sdhci_host *host, u32 opcode);
 	int	(*get_max_tuning_loop_count)(struct sdhci_host *host);
+	void	(*post_tuning)(struct sdhci_host *host);
 	void	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
 	void	(*hw_reset)(struct sdhci_host *host);
 	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
-- 
2.7.4

[PATCH V1 03/11] mmc: sdhci: add support for post tuning process

[Sowjanya Komatineni]

This patch adds support for post tuning process needed for some hosts
to perform after successful completion of HW tuning.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci.c | 6 +++++-
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e9e919218006..976d4d1e2400 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2392,8 +2392,12 @@ static int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 
 		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 		if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) {
-			if (ctrl & SDHCI_CTRL_TUNED_CLK)
+			if (ctrl & SDHCI_CTRL_TUNED_CLK) {
+				if (host->ops->post_tuning)
+					host->ops->post_tuning(host);
 				return 0; /* Success! */
+			}
+
 			break;
 		}
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c80e0d6f9b10..236d67778645 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -639,6 +639,7 @@ struct sdhci_ops {
 	void		(*reset)(struct sdhci_host *host, u8 mask);
 	int	(*platform_execute_tuning)(struct sdhci_host *host, u32 opcode);
 	int	(*get_max_tuning_loop_count)(struct sdhci_host *host);
+	void	(*post_tuning)(struct sdhci_host *host);
 	void	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
 	void	(*hw_reset)(struct sdhci_host *host);
 	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
-- 
2.7.4

[PATCH V1 04/11] mmc: tegra: update hw tuning process

[Sowjanya Komatineni]

This patch includes below HW tuning related fixes.
- configures tuning parameters as per Tegra TRM
- WAR fix for manual tap change
- HW auto-tuning post process

As per Tegra TRM, SDR50 mode tuning execution takes upto maximum
of 256 tuning iterations and SDR104/HS200/HS400 modes tuning
execution takes upto maximum of 128 tuning iterations.

This patch programs tuning control register with maximum tuning
iterations needed based on the timing along with the start tap,
multiplier, and step size used by the HW tuning.

Tegra210 has a known issue of glitch on trimmer output when the
tap value is changed with the trimmer input clock running and the
WAR is to disable card clock before sending tuning command and
after sending tuning command wait for 1usec and issue SW reset
followed by enabling card clock.

This WAR is applicable when changing tap value manually as well.
Tegra SDHCI driver has this implemented correctly for manual tap
change but missing SW reset before enabling card clock during
sending tuning command.

Issuing SW reset during tuning command as a part of WAR and is
applicable in cases where tuning is performed with single step size
for more iterations. This patch includes this fix.

HW auto-tuning finds the best largest passing window and sets the
tap at the middle of the window. With some devices like sandisk
eMMC driving fast edges and due to high tap to tap delay in the
Tegra chipset, auto-tuning does not detect falling tap between the
valid windows resulting in a parital window or a merged window and
the best tap is set at the signal transition which is actually the
worst tap location.

Recommended SW solution is to detect if the best passing window
picked by the HW tuning is a partial or a merged window based on
min and max tap delays found from chip characterization across
PVT and perform tuning correction to pick the best tap.

This patch has this implemention of post tuning and uses tuned tap
delay.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 218 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 217 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 46086dd43bfb..2086e0eced88 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -66,6 +66,23 @@
 
 #define SDHCI_VNDR_TUN_CTRL0_0				0x1c0
 #define SDHCI_VNDR_TUN_CTRL0_TUN_HW_TAP			0x20000
+#define SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_MASK		0x03fc0000
+#define SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_SHIFT	18
+#define SDHCI_VNDR_TUN_CTRL0_MUL_M_MASK			0x00001fc0
+#define SDHCI_VNDR_TUN_CTRL0_MUL_M_SHIFT		6
+#define SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK		0x000e000
+#define SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT		13
+#define TRIES_40					0
+#define TRIES_128					2
+#define TRIES_256					4
+#define SDHCI_VNDR_TUN_CTRL0_TUN_WORD_SEL_MASK		0x7
+
+#define SDHCI_TEGRA_VNDR_TUN_CTRL1_0			0x1c4
+#define SDHCI_TEGRA_VNDR_TUN_STATUS0			0x1C8
+#define SDHCI_TEGRA_VNDR_TUN_STATUS1			0x1CC
+#define SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK		0xFF
+#define SDHCI_TEGRA_VNDR_TUN_STATUS1_END_TAP_SHIFT	0x8
+#define TUNING_WORD_BIT_SIZE				32
 
 #define SDHCI_TEGRA_AUTO_CAL_CONFIG			0x1e4
 #define SDHCI_AUTO_CAL_START				BIT(31)
@@ -97,6 +114,8 @@
 struct sdhci_tegra_soc_data {
 	const struct sdhci_pltfm_data *pdata;
 	u32 nvquirks;
+	u8 min_tap_delay;
+	u8 max_tap_delay;
 };
 
 /* Magic pull up and pull down pad calibration offsets */
@@ -136,6 +155,8 @@ struct sdhci_tegra {
 	u32 default_trim;
 	u32 dqs_trim;
 	bool enable_hwcq;
+	unsigned long curr_clk_rate;
+	u8 tuned_tap_delay;
 };
 
 static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
@@ -241,6 +262,7 @@ static void tegra210_sdhci_writew(struct sdhci_host *host, u16 val, int reg)
 
 	if (is_tuning_cmd) {
 		udelay(1);
+		sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
 		tegra_sdhci_configure_card_clk(host, clk_enabled);
 	}
 }
@@ -722,6 +744,7 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	 */
 	host_clk = tegra_host->ddr_signaling ? clock * 2 : clock;
 	clk_set_rate(pltfm_host->clk, host_clk);
+	tegra_host->curr_clk_rate = host_clk;
 	if (tegra_host->ddr_signaling)
 		host->max_clk = host_clk;
 	else
@@ -770,6 +793,162 @@ static void tegra_sdhci_hs400_dll_cal(struct sdhci_host *host)
 			"HS400 delay line calibration timed out\n");
 }
 
+static int tegra_sdhci_get_max_tuning_loop(struct sdhci_host *host)
+{
+	u32 val;
+
+	val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
+	val &= SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK;
+	val = val >> SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT;
+	if (val == TRIES_128)
+		return 128;
+	else if (val == TRIES_256)
+		return 256;
+	else
+		return 40;
+}
+
+static void tegra_sdhci_tap_correction(struct sdhci_host *host, u8 num_iter,
+				       u8 thd_up, u8 thd_low, u8 fixed_tap)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+	u32 val, tun_status;
+	u8 word, bit, edge1, tap, window;
+	bool tap_result;
+	bool start_fail = false;
+	bool start_pass = false;
+	bool end_pass = false;
+	bool first_fail = false;
+	bool first_pass = false;
+	u8 start_pass_tap = 0;
+	u8 end_pass_tap = 0;
+	u8 first_fail_tap = 0;
+	u8 first_pass_tap = 0;
+	u8 total_tuning_words = num_iter / TUNING_WORD_BIT_SIZE;
+
+	/*
+	 * Read auto-tuned results and extract good valid passing window by
+	 * filtering out un-wanted bubble/partial/merged windows.
+	 */
+	for (word = 0; word < total_tuning_words; word++) {
+		val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
+		val &= ~SDHCI_VNDR_TUN_CTRL0_TUN_WORD_SEL_MASK;
+		val |= word;
+		sdhci_writel(host, val, SDHCI_VNDR_TUN_CTRL0_0);
+		tun_status = sdhci_readl(host, SDHCI_TEGRA_VNDR_TUN_STATUS0);
+		bit = 0;
+		while (bit < TUNING_WORD_BIT_SIZE) {
+			tap = word * TUNING_WORD_BIT_SIZE + bit;
+			tap_result = tun_status & (1 << bit);
+			if (!tap_result && !start_fail) {
+				start_fail = true;
+				if (!first_fail) {
+					first_fail_tap = tap;
+					first_fail = true;
+				}
+
+			} else if (tap_result && start_fail && !start_pass) {
+				start_pass_tap = tap;
+				start_pass = true;
+				if (!first_pass) {
+					first_pass_tap = tap;
+					first_pass = true;
+				}
+
+			} else if (!tap_result && start_fail && start_pass &&
+				   !end_pass) {
+				end_pass_tap = tap - 1;
+				end_pass = true;
+			} else if (tap_result && start_pass && start_fail &&
+				   end_pass) {
+				window = end_pass_tap - start_pass_tap;
+				/* discard merged window and bubble window */
+				if (window >= thd_up || window < thd_low) {
+					start_pass_tap = tap;
+					end_pass = false;
+				} else {
+					/* set tap at middle of valid window */
+					tap = start_pass_tap + window / 2;
+					tegra_host->tuned_tap_delay = tap;
+					return;
+				}
+			}
+
+			bit++;
+		}
+	}
+
+	if (!first_fail) {
+		WARN_ON("no edge detected, continue with hw tuned delay.\n");
+	} else if (first_pass) {
+		/* set tap location at fixed tap relative to the first edge */
+		edge1 = first_fail_tap + (first_pass_tap - first_fail_tap) / 2;
+		if (edge1 - 1 > fixed_tap)
+			tegra_host->tuned_tap_delay = edge1 - fixed_tap;
+		else
+			tegra_host->tuned_tap_delay = edge1 + fixed_tap;
+	}
+}
+
+static void tegra_sdhci_post_tuning(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
+	u32 avg_tap_dly, val, min_tap_dly, max_tap_dly;
+	u8 fixed_tap, start_tap, end_tap, window_width;
+	u8 thdupper, thdlower;
+	u8 num_iter;
+	u32 clk_rate_mhz, period_ps, bestcase, worstcase;
+
+	/* retain HW tuned tap to use incase if no correction is needed */
+	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
+	tegra_host->tuned_tap_delay = (val & SDHCI_CLOCK_CTRL_TAP_MASK) >>
+				      SDHCI_CLOCK_CTRL_TAP_SHIFT;
+	if (soc_data->min_tap_delay && soc_data->max_tap_delay) {
+		min_tap_dly = soc_data->min_tap_delay;
+		max_tap_dly = soc_data->max_tap_delay;
+		clk_rate_mhz = tegra_host->curr_clk_rate / USEC_PER_SEC;
+		period_ps = USEC_PER_SEC / clk_rate_mhz;
+		bestcase = period_ps / min_tap_dly;
+		worstcase = period_ps / max_tap_dly;
+		/*
+		 * Upper and Lower bound thresholds used to detect merged and
+		 * bubble windows
+		 */
+		thdupper = (2 * worstcase + bestcase) / 2;
+		thdlower = worstcase / 4;
+		/*
+		 * fixed tap is used when HW tuning result contains single edge
+		 * and tap is set at fixed tap delay relative to the first edge
+		 */
+		avg_tap_dly = (period_ps * 2) / (min_tap_dly + max_tap_dly);
+		fixed_tap = avg_tap_dly / 2;
+
+		val = sdhci_readl(host, SDHCI_TEGRA_VNDR_TUN_STATUS1);
+		start_tap = val & SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK;
+		end_tap = (val >> SDHCI_TEGRA_VNDR_TUN_STATUS1_END_TAP_SHIFT) &
+			  SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK;
+		window_width = end_tap - start_tap;
+		num_iter = tegra_sdhci_get_max_tuning_loop(host);
+		/*
+		 * partial window includes edges of the tuning range.
+		 * merged window includes more taps so window width is higher
+		 * than upper threshold.
+		 */
+		if (start_tap == 0 || (end_tap == (num_iter - 1)) ||
+		    (end_tap == num_iter - 2) || window_width >= thdupper) {
+			pr_debug("%s: Apply tuning correction\n",
+				 mmc_hostname(host->mmc));
+			tegra_sdhci_tap_correction(host, num_iter, thdupper,
+						   thdlower, fixed_tap);
+		}
+	}
+
+	tegra_sdhci_set_tap(host, tegra_host->tuned_tap_delay);
+}
+
 static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
 					  unsigned timing)
 {
@@ -778,17 +957,22 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
 	bool set_default_tap = false;
 	bool set_dqs_trim = false;
 	bool do_hs400_dll_cal = false;
+	u8 iter = TRIES_256;
+	u32 val;
 
 	tegra_host->ddr_signaling = false;
 	switch (timing) {
 	case MMC_TIMING_UHS_SDR50:
+		break;
 	case MMC_TIMING_UHS_SDR104:
 	case MMC_TIMING_MMC_HS200:
 		/* Don't set default tap on tunable modes. */
+		iter = TRIES_128;
 		break;
 	case MMC_TIMING_MMC_HS400:
 		set_dqs_trim = true;
 		do_hs400_dll_cal = true;
+		iter = TRIES_128;
 		break;
 	case MMC_TIMING_MMC_DDR52:
 	case MMC_TIMING_UHS_DDR50:
@@ -800,11 +984,23 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
 		break;
 	}
 
+	val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
+	val &= ~(SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK |
+		 SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_MASK |
+		 SDHCI_VNDR_TUN_CTRL0_MUL_M_MASK);
+	val |= (iter << SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT |
+		0 << SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_SHIFT |
+		1 << SDHCI_VNDR_TUN_CTRL0_MUL_M_SHIFT);
+	sdhci_writel(host, val, SDHCI_VNDR_TUN_CTRL0_0);
+	sdhci_writel(host, 0, SDHCI_TEGRA_VNDR_TUN_CTRL1_0);
+
 	sdhci_set_uhs_signaling(host, timing);
 
 	tegra_sdhci_pad_autocalib(host);
 
-	if (set_default_tap)
+	if (tegra_host->tuned_tap_delay && !set_default_tap)
+		tegra_sdhci_set_tap(host, tegra_host->tuned_tap_delay);
+	else
 		tegra_sdhci_set_tap(host, tegra_host->default_tap);
 
 	if (set_dqs_trim)
@@ -1090,6 +1286,8 @@ static const struct sdhci_ops tegra210_sdhci_ops = {
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
 	.voltage_switch = tegra_sdhci_voltage_switch,
 	.get_max_clock = tegra_sdhci_get_max_clock,
+	.get_max_tuning_loop_count = tegra_sdhci_get_max_tuning_loop,
+	.post_tuning = tegra_sdhci_post_tuning,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
@@ -1110,6 +1308,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
 		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
 		    NVQUIRK_ENABLE_SDR50 |
 		    NVQUIRK_ENABLE_SDR104,
+	.min_tap_delay = 106,
+	.max_tap_delay = 185,
 };
 
 static const struct sdhci_ops tegra186_sdhci_ops = {
@@ -1121,6 +1321,8 @@ static const struct sdhci_ops tegra186_sdhci_ops = {
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
 	.voltage_switch = tegra_sdhci_voltage_switch,
 	.get_max_clock = tegra_sdhci_get_max_clock,
+	.get_max_tuning_loop_count = tegra_sdhci_get_max_tuning_loop,
+	.post_tuning = tegra_sdhci_post_tuning,
 	.irq = sdhci_tegra_cqhci_irq,
 };
 
@@ -1150,9 +1352,23 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
 		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
 		    NVQUIRK_ENABLE_SDR50 |
 		    NVQUIRK_ENABLE_SDR104,
+	.min_tap_delay = 84,
+	.max_tap_delay = 136,
+};
+
+static const struct sdhci_tegra_soc_data soc_data_tegra194 = {
+	.pdata = &sdhci_tegra186_pdata,
+	.nvquirks = NVQUIRK_NEEDS_PAD_CONTROL |
+		    NVQUIRK_HAS_PADCALIB |
+		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
+		    NVQUIRK_ENABLE_SDR50 |
+		    NVQUIRK_ENABLE_SDR104,
+	.min_tap_delay = 96,
+	.max_tap_delay = 139,
 };
 
 static const struct of_device_id sdhci_tegra_dt_match[] = {
+	{ .compatible = "nvidia,tegra194-sdhci", .data = &soc_data_tegra194 },
 	{ .compatible = "nvidia,tegra186-sdhci", .data = &soc_data_tegra186 },
 	{ .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
 	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra124 },
-- 
2.7.4

[PATCH V1 04/11] mmc: tegra: update hw tuning process

[Sowjanya Komatineni]

This patch includes below HW tuning related fixes.
- configures tuning parameters as per Tegra TRM
- WAR fix for manual tap change
- HW auto-tuning post process

As per Tegra TRM, SDR50 mode tuning execution takes upto maximum
of 256 tuning iterations and SDR104/HS200/HS400 modes tuning
execution takes upto maximum of 128 tuning iterations.

This patch programs tuning control register with maximum tuning
iterations needed based on the timing along with the start tap,
multiplier, and step size used by the HW tuning.

Tegra210 has a known issue of glitch on trimmer output when the
tap value is changed with the trimmer input clock running and the
WAR is to disable card clock before sending tuning command and
after sending tuning command wait for 1usec and issue SW reset
followed by enabling card clock.

This WAR is applicable when changing tap value manually as well.
Tegra SDHCI driver has this implemented correctly for manual tap
change but missing SW reset before enabling card clock during
sending tuning command.

Issuing SW reset during tuning command as a part of WAR and is
applicable in cases where tuning is performed with single step size
for more iterations. This patch includes this fix.

HW auto-tuning finds the best largest passing window and sets the
tap at the middle of the window. With some devices like sandisk
eMMC driving fast edges and due to high tap to tap delay in the
Tegra chipset, auto-tuning does not detect falling tap between the
valid windows resulting in a parital window or a merged window and
the best tap is set at the signal transition which is actually the
worst tap location.

Recommended SW solution is to detect if the best passing window
picked by the HW tuning is a partial or a merged window based on
min and max tap delays found from chip characterization across
PVT and perform tuning correction to pick the best tap.

This patch has this implemention of post tuning and uses tuned tap
delay.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 218 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 217 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 46086dd43bfb..2086e0eced88 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -66,6 +66,23 @@
 
 #define SDHCI_VNDR_TUN_CTRL0_0				0x1c0
 #define SDHCI_VNDR_TUN_CTRL0_TUN_HW_TAP			0x20000
+#define SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_MASK		0x03fc0000
+#define SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_SHIFT	18
+#define SDHCI_VNDR_TUN_CTRL0_MUL_M_MASK			0x00001fc0
+#define SDHCI_VNDR_TUN_CTRL0_MUL_M_SHIFT		6
+#define SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK		0x000e000
+#define SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT		13
+#define TRIES_40					0
+#define TRIES_128					2
+#define TRIES_256					4
+#define SDHCI_VNDR_TUN_CTRL0_TUN_WORD_SEL_MASK		0x7
+
+#define SDHCI_TEGRA_VNDR_TUN_CTRL1_0			0x1c4
+#define SDHCI_TEGRA_VNDR_TUN_STATUS0			0x1C8
+#define SDHCI_TEGRA_VNDR_TUN_STATUS1			0x1CC
+#define SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK		0xFF
+#define SDHCI_TEGRA_VNDR_TUN_STATUS1_END_TAP_SHIFT	0x8
+#define TUNING_WORD_BIT_SIZE				32
 
 #define SDHCI_TEGRA_AUTO_CAL_CONFIG			0x1e4
 #define SDHCI_AUTO_CAL_START				BIT(31)
@@ -97,6 +114,8 @@
 struct sdhci_tegra_soc_data {
 	const struct sdhci_pltfm_data *pdata;
 	u32 nvquirks;
+	u8 min_tap_delay;
+	u8 max_tap_delay;
 };
 
 /* Magic pull up and pull down pad calibration offsets */
@@ -136,6 +155,8 @@ struct sdhci_tegra {
 	u32 default_trim;
 	u32 dqs_trim;
 	bool enable_hwcq;
+	unsigned long curr_clk_rate;
+	u8 tuned_tap_delay;
 };
 
 static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
@@ -241,6 +262,7 @@ static void tegra210_sdhci_writew(struct sdhci_host *host, u16 val, int reg)
 
 	if (is_tuning_cmd) {
 		udelay(1);
+		sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
 		tegra_sdhci_configure_card_clk(host, clk_enabled);
 	}
 }
@@ -722,6 +744,7 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	 */
 	host_clk = tegra_host->ddr_signaling ? clock * 2 : clock;
 	clk_set_rate(pltfm_host->clk, host_clk);
+	tegra_host->curr_clk_rate = host_clk;
 	if (tegra_host->ddr_signaling)
 		host->max_clk = host_clk;
 	else
@@ -770,6 +793,162 @@ static void tegra_sdhci_hs400_dll_cal(struct sdhci_host *host)
 			"HS400 delay line calibration timed out\n");
 }
 
+static int tegra_sdhci_get_max_tuning_loop(struct sdhci_host *host)
+{
+	u32 val;
+
+	val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
+	val &= SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK;
+	val = val >> SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT;
+	if (val == TRIES_128)
+		return 128;
+	else if (val == TRIES_256)
+		return 256;
+	else
+		return 40;
+}
+
+static void tegra_sdhci_tap_correction(struct sdhci_host *host, u8 num_iter,
+				       u8 thd_up, u8 thd_low, u8 fixed_tap)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+	u32 val, tun_status;
+	u8 word, bit, edge1, tap, window;
+	bool tap_result;
+	bool start_fail = false;
+	bool start_pass = false;
+	bool end_pass = false;
+	bool first_fail = false;
+	bool first_pass = false;
+	u8 start_pass_tap = 0;
+	u8 end_pass_tap = 0;
+	u8 first_fail_tap = 0;
+	u8 first_pass_tap = 0;
+	u8 total_tuning_words = num_iter / TUNING_WORD_BIT_SIZE;
+
+	/*
+	 * Read auto-tuned results and extract good valid passing window by
+	 * filtering out un-wanted bubble/partial/merged windows.
+	 */
+	for (word = 0; word < total_tuning_words; word++) {
+		val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
+		val &= ~SDHCI_VNDR_TUN_CTRL0_TUN_WORD_SEL_MASK;
+		val |= word;
+		sdhci_writel(host, val, SDHCI_VNDR_TUN_CTRL0_0);
+		tun_status = sdhci_readl(host, SDHCI_TEGRA_VNDR_TUN_STATUS0);
+		bit = 0;
+		while (bit < TUNING_WORD_BIT_SIZE) {
+			tap = word * TUNING_WORD_BIT_SIZE + bit;
+			tap_result = tun_status & (1 << bit);
+			if (!tap_result && !start_fail) {
+				start_fail = true;
+				if (!first_fail) {
+					first_fail_tap = tap;
+					first_fail = true;
+				}
+
+			} else if (tap_result && start_fail && !start_pass) {
+				start_pass_tap = tap;
+				start_pass = true;
+				if (!first_pass) {
+					first_pass_tap = tap;
+					first_pass = true;
+				}
+
+			} else if (!tap_result && start_fail && start_pass &&
+				   !end_pass) {
+				end_pass_tap = tap - 1;
+				end_pass = true;
+			} else if (tap_result && start_pass && start_fail &&
+				   end_pass) {
+				window = end_pass_tap - start_pass_tap;
+				/* discard merged window and bubble window */
+				if (window >= thd_up || window < thd_low) {
+					start_pass_tap = tap;
+					end_pass = false;
+				} else {
+					/* set tap at middle of valid window */
+					tap = start_pass_tap + window / 2;
+					tegra_host->tuned_tap_delay = tap;
+					return;
+				}
+			}
+
+			bit++;
+		}
+	}
+
+	if (!first_fail) {
+		WARN_ON("no edge detected, continue with hw tuned delay.\n");
+	} else if (first_pass) {
+		/* set tap location at fixed tap relative to the first edge */
+		edge1 = first_fail_tap + (first_pass_tap - first_fail_tap) / 2;
+		if (edge1 - 1 > fixed_tap)
+			tegra_host->tuned_tap_delay = edge1 - fixed_tap;
+		else
+			tegra_host->tuned_tap_delay = edge1 + fixed_tap;
+	}
+}
+
+static void tegra_sdhci_post_tuning(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
+	u32 avg_tap_dly, val, min_tap_dly, max_tap_dly;
+	u8 fixed_tap, start_tap, end_tap, window_width;
+	u8 thdupper, thdlower;
+	u8 num_iter;
+	u32 clk_rate_mhz, period_ps, bestcase, worstcase;
+
+	/* retain HW tuned tap to use incase if no correction is needed */
+	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
+	tegra_host->tuned_tap_delay = (val & SDHCI_CLOCK_CTRL_TAP_MASK) >>
+				      SDHCI_CLOCK_CTRL_TAP_SHIFT;
+	if (soc_data->min_tap_delay && soc_data->max_tap_delay) {
+		min_tap_dly = soc_data->min_tap_delay;
+		max_tap_dly = soc_data->max_tap_delay;
+		clk_rate_mhz = tegra_host->curr_clk_rate / USEC_PER_SEC;
+		period_ps = USEC_PER_SEC / clk_rate_mhz;
+		bestcase = period_ps / min_tap_dly;
+		worstcase = period_ps / max_tap_dly;
+		/*
+		 * Upper and Lower bound thresholds used to detect merged and
+		 * bubble windows
+		 */
+		thdupper = (2 * worstcase + bestcase) / 2;
+		thdlower = worstcase / 4;
+		/*
+		 * fixed tap is used when HW tuning result contains single edge
+		 * and tap is set at fixed tap delay relative to the first edge
+		 */
+		avg_tap_dly = (period_ps * 2) / (min_tap_dly + max_tap_dly);
+		fixed_tap = avg_tap_dly / 2;
+
+		val = sdhci_readl(host, SDHCI_TEGRA_VNDR_TUN_STATUS1);
+		start_tap = val & SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK;
+		end_tap = (val >> SDHCI_TEGRA_VNDR_TUN_STATUS1_END_TAP_SHIFT) &
+			  SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK;
+		window_width = end_tap - start_tap;
+		num_iter = tegra_sdhci_get_max_tuning_loop(host);
+		/*
+		 * partial window includes edges of the tuning range.
+		 * merged window includes more taps so window width is higher
+		 * than upper threshold.
+		 */
+		if (start_tap == 0 || (end_tap == (num_iter - 1)) ||
+		    (end_tap == num_iter - 2) || window_width >= thdupper) {
+			pr_debug("%s: Apply tuning correction\n",
+				 mmc_hostname(host->mmc));
+			tegra_sdhci_tap_correction(host, num_iter, thdupper,
+						   thdlower, fixed_tap);
+		}
+	}
+
+	tegra_sdhci_set_tap(host, tegra_host->tuned_tap_delay);
+}
+
 static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
 					  unsigned timing)
 {
@@ -778,17 +957,22 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
 	bool set_default_tap = false;
 	bool set_dqs_trim = false;
 	bool do_hs400_dll_cal = false;
+	u8 iter = TRIES_256;
+	u32 val;
 
 	tegra_host->ddr_signaling = false;
 	switch (timing) {
 	case MMC_TIMING_UHS_SDR50:
+		break;
 	case MMC_TIMING_UHS_SDR104:
 	case MMC_TIMING_MMC_HS200:
 		/* Don't set default tap on tunable modes. */
+		iter = TRIES_128;
 		break;
 	case MMC_TIMING_MMC_HS400:
 		set_dqs_trim = true;
 		do_hs400_dll_cal = true;
+		iter = TRIES_128;
 		break;
 	case MMC_TIMING_MMC_DDR52:
 	case MMC_TIMING_UHS_DDR50:
@@ -800,11 +984,23 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
 		break;
 	}
 
+	val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
+	val &= ~(SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK |
+		 SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_MASK |
+		 SDHCI_VNDR_TUN_CTRL0_MUL_M_MASK);
+	val |= (iter << SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT |
+		0 << SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_SHIFT |
+		1 << SDHCI_VNDR_TUN_CTRL0_MUL_M_SHIFT);
+	sdhci_writel(host, val, SDHCI_VNDR_TUN_CTRL0_0);
+	sdhci_writel(host, 0, SDHCI_TEGRA_VNDR_TUN_CTRL1_0);
+
 	sdhci_set_uhs_signaling(host, timing);
 
 	tegra_sdhci_pad_autocalib(host);
 
-	if (set_default_tap)
+	if (tegra_host->tuned_tap_delay && !set_default_tap)
+		tegra_sdhci_set_tap(host, tegra_host->tuned_tap_delay);
+	else
 		tegra_sdhci_set_tap(host, tegra_host->default_tap);
 
 	if (set_dqs_trim)
@@ -1090,6 +1286,8 @@ static const struct sdhci_ops tegra210_sdhci_ops = {
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
 	.voltage_switch = tegra_sdhci_voltage_switch,
 	.get_max_clock = tegra_sdhci_get_max_clock,
+	.get_max_tuning_loop_count = tegra_sdhci_get_max_tuning_loop,
+	.post_tuning = tegra_sdhci_post_tuning,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
@@ -1110,6 +1308,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
 		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
 		    NVQUIRK_ENABLE_SDR50 |
 		    NVQUIRK_ENABLE_SDR104,
+	.min_tap_delay = 106,
+	.max_tap_delay = 185,
 };
 
 static const struct sdhci_ops tegra186_sdhci_ops = {
@@ -1121,6 +1321,8 @@ static const struct sdhci_ops tegra186_sdhci_ops = {
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
 	.voltage_switch = tegra_sdhci_voltage_switch,
 	.get_max_clock = tegra_sdhci_get_max_clock,
+	.get_max_tuning_loop_count = tegra_sdhci_get_max_tuning_loop,
+	.post_tuning = tegra_sdhci_post_tuning,
 	.irq = sdhci_tegra_cqhci_irq,
 };
 
@@ -1150,9 +1352,23 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
 		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
 		    NVQUIRK_ENABLE_SDR50 |
 		    NVQUIRK_ENABLE_SDR104,
+	.min_tap_delay = 84,
+	.max_tap_delay = 136,
+};
+
+static const struct sdhci_tegra_soc_data soc_data_tegra194 = {
+	.pdata = &sdhci_tegra186_pdata,
+	.nvquirks = NVQUIRK_NEEDS_PAD_CONTROL |
+		    NVQUIRK_HAS_PADCALIB |
+		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
+		    NVQUIRK_ENABLE_SDR50 |
+		    NVQUIRK_ENABLE_SDR104,
+	.min_tap_delay = 96,
+	.max_tap_delay = 139,
 };
 
 static const struct of_device_id sdhci_tegra_dt_match[] = {
+	{ .compatible = "nvidia,tegra194-sdhci", .data = &soc_data_tegra194 },
 	{ .compatible = "nvidia,tegra186-sdhci", .data = &soc_data_tegra186 },
 	{ .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
 	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra124 },
-- 
2.7.4

[PATCH V1 05/11] dt-bindings: mmc: tegra: document Tegra194 compatible string

[Sowjanya Komatineni]

SDHCI controller of Tegra194 is similar to SDHCI controller in Tegra186.
This patch documents Tegra194 sdhci compatible string.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
index 2cecdc71d94c..2cf3affa1be7 100644
--- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
@@ -14,6 +14,7 @@ Required properties:
   - "nvidia,tegra124-sdhci": for Tegra124 and Tegra132
   - "nvidia,tegra210-sdhci": for Tegra210
   - "nvidia,tegra186-sdhci": for Tegra186
+  - "nvidia,tegra194-sdhci": for Tegra194
 - clocks : Must contain one entry, for the module clock.
   See ../clocks/clock-bindings.txt for details.
 - resets : Must contain an entry for each entry in reset-names.
-- 
2.7.4

[PATCH V1 05/11] dt-bindings: mmc: tegra: document Tegra194 compatible string

[Sowjanya Komatineni]

SDHCI controller of Tegra194 is similar to SDHCI controller in Tegra186.
This patch documents Tegra194 sdhci compatible string.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
index 2cecdc71d94c..2cf3affa1be7 100644
--- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
@@ -14,6 +14,7 @@ Required properties:
   - "nvidia,tegra124-sdhci": for Tegra124 and Tegra132
   - "nvidia,tegra210-sdhci": for Tegra210
   - "nvidia,tegra186-sdhci": for Tegra186
+  - "nvidia,tegra194-sdhci": for Tegra194
 - clocks : Must contain one entry, for the module clock.
   See ../clocks/clock-bindings.txt for details.
 - resets : Must contain an entry for each entry in reset-names.
-- 
2.7.4

[PATCH V1 06/11] arm64: tegra: fix default tap and trim values

[Sowjanya Komatineni]

Default tap and trim values are incorrect for Tegra186 SDMMC4.
This patch fixes it.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 97aeb946ed5e..472f55fe9488 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -317,8 +317,8 @@
 		nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
 		nvidia,pad-autocal-pull-up-offset-3v3-timeout = <0x0a>;
 		nvidia,pad-autocal-pull-down-offset-3v3-timeout = <0x0a>;
-		nvidia,default-tap = <0x5>;
-		nvidia,default-trim = <0x9>;
+		nvidia,default-tap = <0x9>;
+		nvidia,default-trim = <0x5>;
 		nvidia,dqs-trim = <63>;
 		mmc-hs400-1_8v;
 		status = "disabled";
-- 
2.7.4

[PATCH V1 06/11] arm64: tegra: fix default tap and trim values

[Sowjanya Komatineni]

Default tap and trim values are incorrect for Tegra186 SDMMC4.
This patch fixes it.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 97aeb946ed5e..472f55fe9488 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -317,8 +317,8 @@
 		nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
 		nvidia,pad-autocal-pull-up-offset-3v3-timeout = <0x0a>;
 		nvidia,pad-autocal-pull-down-offset-3v3-timeout = <0x0a>;
-		nvidia,default-tap = <0x5>;
-		nvidia,default-trim = <0x9>;
+		nvidia,default-tap = <0x9>;
+		nvidia,default-trim = <0x5>;
 		nvidia,dqs-trim = <63>;
 		mmc-hs400-1_8v;
 		status = "disabled";
-- 
2.7.4

[PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Sowjanya Komatineni]

This patch adds a quirk for setting CMD_TIMING to 1 in descriptor
for DCMD with R1B response type to allow the command to be sent to
device during data activity or busy time.

Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT
to 1 by CQHCI controller for DCMDs with R1B response type and
since DCMD does not trigger any data transfer, DCMD task complete
happens leaving the DATA FSM of host controller in wait state for
data.

This effects the data transfer task issued after R1B DCMD task
and no interrupt is generated for the data transfer task.

SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task
descriptor and as DCMD task descriptor preparation is done by
cqhci driver, this patch adds cqequirk to handle this.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/cqhci.c | 5 ++++-
 drivers/mmc/host/cqhci.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
index a8af682a9182..b34c07125f32 100644
--- a/drivers/mmc/host/cqhci.c
+++ b/drivers/mmc/host/cqhci.c
@@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
 	} else {
 		if (mrq->cmd->flags & MMC_RSP_R1B) {
 			resp_type = 0x3;
-			timing = 0x0;
+			if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
+				timing = 0x1;
+			else
+				timing = 0x0;
 		} else {
 			resp_type = 0x2;
 			timing = 0x1;
diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index 9e68286a07b4..f96d8565cc07 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -170,6 +170,7 @@ struct cqhci_host {
 
 	u32 quirks;
 #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
+#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
 
 	bool enabled;
 	bool halted;
-- 
2.7.4

[PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Sowjanya Komatineni]

This patch adds a quirk for setting CMD_TIMING to 1 in descriptor
for DCMD with R1B response type to allow the command to be sent to
device during data activity or busy time.

Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT
to 1 by CQHCI controller for DCMDs with R1B response type and
since DCMD does not trigger any data transfer, DCMD task complete
happens leaving the DATA FSM of host controller in wait state for
data.

This effects the data transfer task issued after R1B DCMD task
and no interrupt is generated for the data transfer task.

SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task
descriptor and as DCMD task descriptor preparation is done by
cqhci driver, this patch adds cqequirk to handle this.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/cqhci.c | 5 ++++-
 drivers/mmc/host/cqhci.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
index a8af682a9182..b34c07125f32 100644
--- a/drivers/mmc/host/cqhci.c
+++ b/drivers/mmc/host/cqhci.c
@@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
 	} else {
 		if (mrq->cmd->flags & MMC_RSP_R1B) {
 			resp_type = 0x3;
-			timing = 0x0;
+			if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
+				timing = 0x1;
+			else
+				timing = 0x0;
 		} else {
 			resp_type = 0x2;
 			timing = 0x1;
diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index 9e68286a07b4..f96d8565cc07 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -170,6 +170,7 @@ struct cqhci_host {
 
 	u32 quirks;
 #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
+#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
 
 	bool enabled;
 	bool halted;
-- 
2.7.4

[PATCH V1 08/11] mmc: tegra: add Tegra186 WAR for CQE

[Sowjanya Komatineni]

Tegra186 design has a known bug where CQE does not generated task
complete interrupt for data transfer tasks issued after DCMD task
with R1b response type and results in timeout.

SW WAR is to set CMD_TIMING to 1 in task descriptor for DCMDs with
R1b response type. This bug and SW WAR is applicable only for
Tegra186 and not for Tegra194.

This patch adds this WAR to Tegra186.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 2086e0eced88..2b63626dc2fa 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -116,6 +116,7 @@ struct sdhci_tegra_soc_data {
 	u32 nvquirks;
 	u8 min_tap_delay;
 	u8 max_tap_delay;
+	u32 cqequirks;
 };
 
 /* Magic pull up and pull down pad calibration offsets */
@@ -1354,6 +1355,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
 		    NVQUIRK_ENABLE_SDR104,
 	.min_tap_delay = 84,
 	.max_tap_delay = 136,
+	.cqequirks = CQHCI_QUIRK_CMD_TIMING_R1B_DCMD,
 };
 
 static const struct sdhci_tegra_soc_data soc_data_tegra194 = {
@@ -1383,6 +1385,7 @@ static int sdhci_tegra_add_host(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
 	struct cqhci_host *cq_host;
 	bool dma64;
 	int ret;
@@ -1407,6 +1410,7 @@ static int sdhci_tegra_add_host(struct sdhci_host *host)
 
 	cq_host->mmio = host->ioaddr + SDHCI_TEGRA_CQE_BASE_ADDR;
 	cq_host->ops = &sdhci_tegra_cqhci_ops;
+	cq_host->quirks = soc_data->cqequirks;
 
 	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
 	if (dma64)
-- 
2.7.4

[PATCH V1 08/11] mmc: tegra: add Tegra186 WAR for CQE

[Sowjanya Komatineni]

Tegra186 design has a known bug where CQE does not generated task
complete interrupt for data transfer tasks issued after DCMD task
with R1b response type and results in timeout.

SW WAR is to set CMD_TIMING to 1 in task descriptor for DCMDs with
R1b response type. This bug and SW WAR is applicable only for
Tegra186 and not for Tegra194.

This patch adds this WAR to Tegra186.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 2086e0eced88..2b63626dc2fa 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -116,6 +116,7 @@ struct sdhci_tegra_soc_data {
 	u32 nvquirks;
 	u8 min_tap_delay;
 	u8 max_tap_delay;
+	u32 cqequirks;
 };
 
 /* Magic pull up and pull down pad calibration offsets */
@@ -1354,6 +1355,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
 		    NVQUIRK_ENABLE_SDR104,
 	.min_tap_delay = 84,
 	.max_tap_delay = 136,
+	.cqequirks = CQHCI_QUIRK_CMD_TIMING_R1B_DCMD,
 };
 
 static const struct sdhci_tegra_soc_data soc_data_tegra194 = {
@@ -1383,6 +1385,7 @@ static int sdhci_tegra_add_host(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
 	struct cqhci_host *cq_host;
 	bool dma64;
 	int ret;
@@ -1407,6 +1410,7 @@ static int sdhci_tegra_add_host(struct sdhci_host *host)
 
 	cq_host->mmio = host->ioaddr + SDHCI_TEGRA_CQE_BASE_ADDR;
 	cq_host->ops = &sdhci_tegra_cqhci_ops;
+	cq_host->quirks = soc_data->cqequirks;
 
 	dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
 	if (dma64)
-- 
2.7.4

[PATCH V1 09/11] mmc: cqhci: add CQHCI_SSC1 register CBC field mask

[Sowjanya Komatineni]

This patch adds define for CBC field mask of the register
CQHCI_SSC1.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/cqhci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index f96d8565cc07..f1dc48c7436f 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -88,6 +88,7 @@
 
 /* send status config 1 */
 #define CQHCI_SSC1			0x40
+#define CQHCI_SSC1_CBC_MASK		GENMASK(19, 16)
 
 /* send status config 2 */
 #define CQHCI_SSC2			0x44
-- 
2.7.4

[PATCH V1 09/11] mmc: cqhci: add CQHCI_SSC1 register CBC field mask

[Sowjanya Komatineni]

This patch adds define for CBC field mask of the register
CQHCI_SSC1.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/cqhci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index f96d8565cc07..f1dc48c7436f 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -88,6 +88,7 @@
 
 /* send status config 1 */
 #define CQHCI_SSC1			0x40
+#define CQHCI_SSC1_CBC_MASK		GENMASK(19, 16)
 
 /* send status config 2 */
 #define CQHCI_SSC2			0x44
-- 
2.7.4

[PATCH V1 10/11] mmc: tegra: fix CQE resume sequence

[Sowjanya Komatineni]

Tegra CQHCI/SDHCI design prevents write access to SDHCI block size
register when CQE is enabled and unhalted.

CQHCI driver enabled CQE prior to invoking sdhci_cqe_enable which
violates this Tegra specific host requirement.

This patch fixes this by configuring sdhci block registers prior
to CQE unhalt.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 71 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 2b63626dc2fa..7063cfcdc590 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -1126,23 +1126,75 @@ static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
 		tegra_host->pad_calib_required = true;
 }
 
+static void tegra_cqhci_writel(struct cqhci_host *cq_host, u32 val, int reg)
+{
+	struct mmc_host *mmc = cq_host->mmc;
+	u8 ctrl;
+	ktime_t timeout;
+	bool timed_out;
+
+	/*
+	 * During CQE resume/unhalt, CQHCI driver unhalts CQE prior to
+	 * cqhci_host_ops enable where SDHCI DMA and BLOCK_SIZE registers need
+	 * to be re-configured.
+	 * Tegra CQHCI/SDHCI prevents write access to block size register when
+	 * CQE is unhalted. So handling CQE resume sequence here to configure
+	 * SDHCI block registers prior to exiting CQE halt state.
+	 */
+	if (reg == CQHCI_CTL && !(val & CQHCI_HALT) &&
+	    cqhci_readl(cq_host, CQHCI_CTL) & CQHCI_HALT) {
+		sdhci_cqe_enable(mmc);
+		writel(val, cq_host->mmio + reg);
+		timeout = ktime_add_us(ktime_get(), 50);
+		while (1) {
+			timed_out = ktime_compare(ktime_get(), timeout) > 0;
+			ctrl = cqhci_readl(cq_host, CQHCI_CTL);
+			if (!(ctrl & CQHCI_HALT) || timed_out)
+				break;
+		}
+		/*
+		 * CQE usually resumes very quick, but incase if Tegra CQE
+		 * doesn't resume retry unhalt.
+		 */
+		if (timed_out)
+			writel(val, cq_host->mmio + reg);
+	} else {
+		writel(val, cq_host->mmio + reg);
+	}
+}
+
 static void sdhci_tegra_cqe_enable(struct mmc_host *mmc)
 {
 	struct cqhci_host *cq_host = mmc->cqe_private;
-	u32 cqcfg = 0;
+	u32 val;
 
 	/*
-	 * Tegra SDMMC Controller design prevents write access to BLOCK_COUNT
-	 * registers when CQE is enabled.
+	 * Tegra CQHCI/SDMMC design prevents write access to sdhci block size
+	 * register when CQE is enabled and unhalted.
+	 * CQHCI driver enables CQE prior to activation, so disable CQE before
+	 * programming block size in sdhci controller and enable it back.
 	 */
-	cqcfg = cqhci_readl(cq_host, CQHCI_CFG);
-	if (cqcfg & CQHCI_ENABLE)
-		cqhci_writel(cq_host, (cqcfg & ~CQHCI_ENABLE), CQHCI_CFG);
+	if (!cq_host->activated) {
+		val = cqhci_readl(cq_host, CQHCI_CFG);
+		if (val & CQHCI_ENABLE)
+			cqhci_writel(cq_host, (val & ~CQHCI_ENABLE),
+				     CQHCI_CFG);
+		sdhci_cqe_enable(mmc);
+		if (val & CQHCI_ENABLE)
+			cqhci_writel(cq_host, val, CQHCI_CFG);
+	}
 
-	sdhci_cqe_enable(mmc);
+	/*
+	 * CMD CRC errors are seen sometimes with some eMMC devices when status
+	 * command is sent during transfer of last data block which is the
+	 * default case as send status command block counter (CBC) is 1.
+	 * So changing send status command block counter (CBC) to 0 to allow
+	 * send status command only when the data lines are idle.
+	 */
+	val = cqhci_readl(cq_host, CQHCI_SSC1);
+	val &= ~CQHCI_SSC1_CBC_MASK;
+	cqhci_writel(cq_host, val, CQHCI_SSC1);
 
-	if (cqcfg & CQHCI_ENABLE)
-		cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
 }
 
 static void sdhci_tegra_dumpregs(struct mmc_host *mmc)
@@ -1167,6 +1219,7 @@ static const struct cqhci_host_ops sdhci_tegra_cqhci_ops = {
 	.enable	= sdhci_tegra_cqe_enable,
 	.disable = sdhci_cqe_disable,
 	.dumpregs = sdhci_tegra_dumpregs,
+	.write_l    = tegra_cqhci_writel,
 };
 
 static const struct sdhci_ops tegra_sdhci_ops = {
-- 
2.7.4

[PATCH V1 10/11] mmc: tegra: fix CQE resume sequence

[Sowjanya Komatineni]

Tegra CQHCI/SDHCI design prevents write access to SDHCI block size
register when CQE is enabled and unhalted.

CQHCI driver enabled CQE prior to invoking sdhci_cqe_enable which
violates this Tegra specific host requirement.

This patch fixes this by configuring sdhci block registers prior
to CQE unhalt.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 71 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 2b63626dc2fa..7063cfcdc590 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -1126,23 +1126,75 @@ static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
 		tegra_host->pad_calib_required = true;
 }
 
+static void tegra_cqhci_writel(struct cqhci_host *cq_host, u32 val, int reg)
+{
+	struct mmc_host *mmc = cq_host->mmc;
+	u8 ctrl;
+	ktime_t timeout;
+	bool timed_out;
+
+	/*
+	 * During CQE resume/unhalt, CQHCI driver unhalts CQE prior to
+	 * cqhci_host_ops enable where SDHCI DMA and BLOCK_SIZE registers need
+	 * to be re-configured.
+	 * Tegra CQHCI/SDHCI prevents write access to block size register when
+	 * CQE is unhalted. So handling CQE resume sequence here to configure
+	 * SDHCI block registers prior to exiting CQE halt state.
+	 */
+	if (reg == CQHCI_CTL && !(val & CQHCI_HALT) &&
+	    cqhci_readl(cq_host, CQHCI_CTL) & CQHCI_HALT) {
+		sdhci_cqe_enable(mmc);
+		writel(val, cq_host->mmio + reg);
+		timeout = ktime_add_us(ktime_get(), 50);
+		while (1) {
+			timed_out = ktime_compare(ktime_get(), timeout) > 0;
+			ctrl = cqhci_readl(cq_host, CQHCI_CTL);
+			if (!(ctrl & CQHCI_HALT) || timed_out)
+				break;
+		}
+		/*
+		 * CQE usually resumes very quick, but incase if Tegra CQE
+		 * doesn't resume retry unhalt.
+		 */
+		if (timed_out)
+			writel(val, cq_host->mmio + reg);
+	} else {
+		writel(val, cq_host->mmio + reg);
+	}
+}
+
 static void sdhci_tegra_cqe_enable(struct mmc_host *mmc)
 {
 	struct cqhci_host *cq_host = mmc->cqe_private;
-	u32 cqcfg = 0;
+	u32 val;
 
 	/*
-	 * Tegra SDMMC Controller design prevents write access to BLOCK_COUNT
-	 * registers when CQE is enabled.
+	 * Tegra CQHCI/SDMMC design prevents write access to sdhci block size
+	 * register when CQE is enabled and unhalted.
+	 * CQHCI driver enables CQE prior to activation, so disable CQE before
+	 * programming block size in sdhci controller and enable it back.
 	 */
-	cqcfg = cqhci_readl(cq_host, CQHCI_CFG);
-	if (cqcfg & CQHCI_ENABLE)
-		cqhci_writel(cq_host, (cqcfg & ~CQHCI_ENABLE), CQHCI_CFG);
+	if (!cq_host->activated) {
+		val = cqhci_readl(cq_host, CQHCI_CFG);
+		if (val & CQHCI_ENABLE)
+			cqhci_writel(cq_host, (val & ~CQHCI_ENABLE),
+				     CQHCI_CFG);
+		sdhci_cqe_enable(mmc);
+		if (val & CQHCI_ENABLE)
+			cqhci_writel(cq_host, val, CQHCI_CFG);
+	}
 
-	sdhci_cqe_enable(mmc);
+	/*
+	 * CMD CRC errors are seen sometimes with some eMMC devices when status
+	 * command is sent during transfer of last data block which is the
+	 * default case as send status command block counter (CBC) is 1.
+	 * So changing send status command block counter (CBC) to 0 to allow
+	 * send status command only when the data lines are idle.
+	 */
+	val = cqhci_readl(cq_host, CQHCI_SSC1);
+	val &= ~CQHCI_SSC1_CBC_MASK;
+	cqhci_writel(cq_host, val, CQHCI_SSC1);
 
-	if (cqcfg & CQHCI_ENABLE)
-		cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
 }
 
 static void sdhci_tegra_dumpregs(struct mmc_host *mmc)
@@ -1167,6 +1219,7 @@ static const struct cqhci_host_ops sdhci_tegra_cqhci_ops = {
 	.enable	= sdhci_tegra_cqe_enable,
 	.disable = sdhci_cqe_disable,
 	.dumpregs = sdhci_tegra_dumpregs,
+	.write_l    = tegra_cqhci_writel,
 };
 
 static const struct sdhci_ops tegra_sdhci_ops = {
-- 
2.7.4

[PATCH V1 11/11] arm64: tegra: enable command queue for tegra186 sdmmc4

[Sowjanya Komatineni]

This patch enables command queue support for Tegra186 SDMMC4.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 472f55fe9488..6e2b6ce99df2 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -321,6 +321,7 @@
 		nvidia,default-trim = <0x5>;
 		nvidia,dqs-trim = <63>;
 		mmc-hs400-1_8v;
+		supports-cqe;
 		status = "disabled";
 	};
 
-- 
2.7.4

[PATCH V1 11/11] arm64: tegra: enable command queue for tegra186 sdmmc4

[Sowjanya Komatineni]

This patch enables command queue support for Tegra186 SDMMC4.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 472f55fe9488..6e2b6ce99df2 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -321,6 +321,7 @@
 		nvidia,default-trim = <0x5>;
 		nvidia,dqs-trim = <63>;
 		mmc-hs400-1_8v;
+		supports-cqe;
 		status = "disabled";
 	};
 
-- 
2.7.4

Re: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Adrian Hunter]

On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor
> for DCMD with R1B response type to allow the command to be sent to
> device during data activity or busy time.
> 
> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT
> to 1 by CQHCI controller for DCMDs with R1B response type and
> since DCMD does not trigger any data transfer, DCMD task complete
> happens leaving the DATA FSM of host controller in wait state for
> data.
> 
> This effects the data transfer task issued after R1B DCMD task
> and no interrupt is generated for the data transfer task.
> 
> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task
> descriptor and as DCMD task descriptor preparation is done by
> cqhci driver, this patch adds cqequirk to handle this.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/mmc/host/cqhci.c | 5 ++++-
>  drivers/mmc/host/cqhci.h | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
> index a8af682a9182..b34c07125f32 100644
> --- a/drivers/mmc/host/cqhci.c
> +++ b/drivers/mmc/host/cqhci.c
> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
>  	} else {
>  		if (mrq->cmd->flags & MMC_RSP_R1B) {
>  			resp_type = 0x3;
> -			timing = 0x0;
> +			if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
> +				timing = 0x1;
> +			else
> +				timing = 0x0;

I was thinking it would be nice if there was a generic way for drivers to
make changes to descriptors before a task is started.  Currently there is
host->ops->write_l() which would make it possible by checking for CQHCI_TDBR
register and, in this case, the DCMD tag.  We would need to export
get_desc(), perhaps rename it cqhci_get_desc() and put it in cqhci.h since
it is an inline function.

Alternatively we could add host->ops for descriptor preparation.

What do people think?

>  		} else {
>  			resp_type = 0x2;
>  			timing = 0x1;
> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> index 9e68286a07b4..f96d8565cc07 100644
> --- a/drivers/mmc/host/cqhci.h
> +++ b/drivers/mmc/host/cqhci.h
> @@ -170,6 +170,7 @@ struct cqhci_host {
>  
>  	u32 quirks;
>  #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
>  
>  	bool enabled;
>  	bool halted;
> 

Re: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Ritesh Harjani]

On 3/6/2019 6:30 PM, Adrian Hunter wrote:
> On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
>> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor
>> for DCMD with R1B response type to allow the command to be sent to
>> device during data activity or busy time.
>>
>> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT
>> to 1 by CQHCI controller for DCMDs with R1B response type and
>> since DCMD does not trigger any data transfer, DCMD task complete
>> happens leaving the DATA FSM of host controller in wait state for
>> data.
>>
>> This effects the data transfer task issued after R1B DCMD task
>> and no interrupt is generated for the data transfer task.
>>
>> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task
>> descriptor and as DCMD task descriptor preparation is done by
>> cqhci driver, this patch adds cqequirk to handle this.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>>   drivers/mmc/host/cqhci.c | 5 ++++-
>>   drivers/mmc/host/cqhci.h | 1 +
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
>> index a8af682a9182..b34c07125f32 100644
>> --- a/drivers/mmc/host/cqhci.c
>> +++ b/drivers/mmc/host/cqhci.c
>> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
>>   	} else {
>>   		if (mrq->cmd->flags & MMC_RSP_R1B) {
>>   			resp_type = 0x3;
>> -			timing = 0x0;
>> +			if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
>> +				timing = 0x1;
>> +			else
>> +				timing = 0x0;
> I was thinking it would be nice if there was a generic way for drivers to
> make changes to descriptors before a task is started.  Currently there is
> host->ops->write_l() which would make it possible by checking for CQHCI_TDBR
> register and, in this case, the DCMD tag.  We would need to export
> get_desc(), perhaps rename it cqhci_get_desc() and put it in cqhci.h since
> it is an inline function.

We take spin_lock_irqsave after the descriptor is prepared and before 
writing to TDBR.
Not sure but tomorrow this may become a limitation for drivers to make 
changes to
descriptors if they edit descriptors in host->ops->write_l() call.
Though in this case it is not required here.

>
> Alternatively we could add host->ops for descriptor preparation.

Both ways sounds good to me.
But maybe adding a host->ops for descriptor preparation is better way to go,
since that will be the right interface exposed to make changes to 
descriptors.

>
> What do people think?
>
>>   		} else {
>>   			resp_type = 0x2;
>>   			timing = 0x1;
>> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
>> index 9e68286a07b4..f96d8565cc07 100644
>> --- a/drivers/mmc/host/cqhci.h
>> +++ b/drivers/mmc/host/cqhci.h
>> @@ -170,6 +170,7 @@ struct cqhci_host {
>>   
>>   	u32 quirks;
>>   #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
>> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
>>   
>>   	bool enabled;
>>   	bool halted;
>>

RE: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Sowjanya Komatineni]

> On 3/6/2019 6:30 PM, Adrian Hunter wrote:
>> On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
>>> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor for 
>>> DCMD with R1B response type to allow the command to be sent to device 
>>> during data activity or busy time.
>>>
>>> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT to 1 
>>> by CQHCI controller for DCMDs with R1B response type and since DCMD 
>>> does not trigger any data transfer, DCMD task complete happens 
>>> leaving the DATA FSM of host controller in wait state for data.
>>>
>>> This effects the data transfer task issued after R1B DCMD task and no 
>>> interrupt is generated for the data transfer task.
>>>
>>> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task 
>>> descriptor and as DCMD task descriptor preparation is done by cqhci 
>>> driver, this patch adds cqequirk to handle this.
>>>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>>   drivers/mmc/host/cqhci.c | 5 ++++-
>>>   drivers/mmc/host/cqhci.h | 1 +
>>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c 
>>> index a8af682a9182..b34c07125f32 100644
>>> --- a/drivers/mmc/host/cqhci.c
>>> +++ b/drivers/mmc/host/cqhci.c
>>> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
>>>   	} else {
>>>   		if (mrq->cmd->flags & MMC_RSP_R1B) {
>>>   			resp_type = 0x3;
>>> -			timing = 0x0;
>>> +			if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
>>> +				timing = 0x1;
>>> +			else
>>> +				timing = 0x0;
>> I was thinking it would be nice if there was a generic way for drivers 
>> to make changes to descriptors before a task is started.  Currently 
>> there is
>> host->ops->write_l() which would make it possible by checking for 
>> host->ops->CQHCI_TDBR
>> register and, in this case, the DCMD tag.  We would need to export 
>> get_desc(), perhaps rename it cqhci_get_desc() and put it in cqhci.h 
>> since it is an inline function.
>
>We take spin_lock_irqsave after the descriptor is prepared and before writing to TDBR.
>Not sure but tomorrow this may become a limitation for drivers to make changes to descriptors if they edit descriptors in host->ops->write_l() call.
>Though in this case it is not required here.
>
>>
>> Alternatively we could add host->ops for descriptor preparation.
>
>Both ways sounds good to me.
>But maybe adding a host->ops for descriptor preparation is better way to go,
>since that will be the right interface exposed to make changes to 
>descriptors.
>

DCMD descriptor attributes remain same for any host and also task parameters as QBR need to be enabled with DCMD.
So I believe it should be ok if we just add callback to allow hosts to update command parameters of DCMD descriptor only thru cqhci_host_ops.

Also, don’t see any requirement for host specific Task parameter updates in Task descriptor so not sure if there is any need to provide callback for task descriptor data preparation to hosts.
Please confirm.

>>
>> What do people think?
>>
>>>   		} else {
>>>   			resp_type = 0x2;
>>>   			timing = 0x1;
>>> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
>>> index 9e68286a07b4..f96d8565cc07 100644
>>> --- a/drivers/mmc/host/cqhci.h
>>> +++ b/drivers/mmc/host/cqhci.h
>>> @@ -170,6 +170,7 @@ struct cqhci_host {
>>>   
>>>   	u32 quirks;
>>>   #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
>>> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
>>>   
>>>   	bool enabled;
>>>   	bool halted;
>>>

Re: [PATCH V1 01/11] mmc: tegra: fix ddr signaling for non-ddr modes

[Jon Hunter]

Hi Sowjanya,

On 02/03/2019 05:20, Sowjanya Komatineni wrote:
> ddr_signaling is set to true for DDR50 and DDR52 modes but is
> not set back to false for other modes. This programs incorrect
> host clock when mode change happens from DDR52/DDR50 to other
> SDR or HS modes like incase of mmc_retune where it switches
> from HS400 to HS DDR and then from HS DDR to HS mode and then
> to HS200.
> 
> This patch fixes the ddr_signaling to set properly for non DDR
> modes.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 32e62904c0d3..46086dd43bfb 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -779,6 +779,7 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
>  	bool set_dqs_trim = false;
>  	bool do_hs400_dll_cal = false;
>  
> +	tegra_host->ddr_signaling = false;
>  	switch (timing) {
>  	case MMC_TIMING_UHS_SDR50:
>  	case MMC_TIMING_UHS_SDR104:

I have tested this series on Tegra210, Tegra186 and Tegra194 and see no
further issues. Thanks for getting this out! Feel free to add my ...

Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH V1 01/11] mmc: tegra: fix ddr signaling for non-ddr modes

[Jon Hunter]

Hi Sowjanya,

On 02/03/2019 05:20, Sowjanya Komatineni wrote:
> ddr_signaling is set to true for DDR50 and DDR52 modes but is
> not set back to false for other modes. This programs incorrect
> host clock when mode change happens from DDR52/DDR50 to other
> SDR or HS modes like incase of mmc_retune where it switches
> from HS400 to HS DDR and then from HS DDR to HS mode and then
> to HS200.
> 
> This patch fixes the ddr_signaling to set properly for non DDR
> modes.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 32e62904c0d3..46086dd43bfb 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -779,6 +779,7 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
>  	bool set_dqs_trim = false;
>  	bool do_hs400_dll_cal = false;
>  
> +	tegra_host->ddr_signaling = false;
>  	switch (timing) {
>  	case MMC_TIMING_UHS_SDR50:
>  	case MMC_TIMING_UHS_SDR104:

I have tested this series on Tegra210, Tegra186 and Tegra194 and see no
further issues. Thanks for getting this out! Feel free to add my ...

Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH V1 01/11] mmc: tegra: fix ddr signaling for non-ddr modes

[Adrian Hunter]

On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> ddr_signaling is set to true for DDR50 and DDR52 modes but is
> not set back to false for other modes. This programs incorrect
> host clock when mode change happens from DDR52/DDR50 to other
> SDR or HS modes like incase of mmc_retune where it switches
> from HS400 to HS DDR and then from HS DDR to HS mode and then
> to HS200.
> 
> This patch fixes the ddr_signaling to set properly for non DDR
> modes.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>

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

> ---
>  drivers/mmc/host/sdhci-tegra.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 32e62904c0d3..46086dd43bfb 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -779,6 +779,7 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
>  	bool set_dqs_trim = false;
>  	bool do_hs400_dll_cal = false;
>  
> +	tegra_host->ddr_signaling = false;
>  	switch (timing) {
>  	case MMC_TIMING_UHS_SDR50:
>  	case MMC_TIMING_UHS_SDR104:
> 

Re: [PATCH V1 02/11] mmc: sdhci: allow host to specify maximum tuning loops

[Adrian Hunter]

On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> As per the Host Controller Standard Specification Version 4.20,
> limitation of tuning iteration count is removed as PLL locking
> time can be longer than UHS-1 tuning due to larger PVT fluctuation
> and it will result in increase of tuning iteration to complete the
> tuning.
> 
> This patch creates a hook get_max_tuning_loop_count to allow hosts
> to specify maximum tuning iterations and updates execute_tuning
> to use the specified maximum tuning iteration count.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/mmc/host/sdhci.c | 7 +++++--
>  drivers/mmc/host/sdhci.h | 1 +
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index a8141ff9be03..e9e919218006 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2366,12 +2366,15 @@ EXPORT_SYMBOL_GPL(sdhci_send_tuning);
>  static int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
>  {
>  	int i;
> +	int tuning_loop_count = MAX_TUNING_LOOP;
>  
> +	if (host->ops->get_max_tuning_loop_count)
> +		tuning_loop_count = host->ops->get_max_tuning_loop_count(host);

I would prefer to make tuning_loop_count a sdhci_host member initialized to
MAX_TUNING_LOOP in sdhci_alloc_host().

Then hook .execute_tuning e.g.

	host->mmc_host_ops.execute_tuning = tegra_sdhci_execute_tuning;

and set up host->tuning_loop_count


>  	/*
>  	 * Issue opcode repeatedly till Execute Tuning is set to 0 or the number
> -	 * of loops reaches 40 times.
> +	 * of loops reaches tuning loop count.
>  	 */
> -	for (i = 0; i < MAX_TUNING_LOOP; i++) {
> +	for (i = 0; i < tuning_loop_count; i++) {
>  		u16 ctrl;
>  
>  		sdhci_send_tuning(host, opcode);
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 01002cba1359..c80e0d6f9b10 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -638,6 +638,7 @@ struct sdhci_ops {
>  	unsigned int    (*get_ro)(struct sdhci_host *host);
>  	void		(*reset)(struct sdhci_host *host, u8 mask);
>  	int	(*platform_execute_tuning)(struct sdhci_host *host, u32 opcode);
> +	int	(*get_max_tuning_loop_count)(struct sdhci_host *host);
>  	void	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
>  	void	(*hw_reset)(struct sdhci_host *host);
>  	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
> 

Re: [PATCH V1 03/11] mmc: sdhci: add support for post tuning process

[Adrian Hunter]

On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> This patch adds support for post tuning process needed for some hosts
> to perform after successful completion of HW tuning.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/mmc/host/sdhci.c | 6 +++++-
>  drivers/mmc/host/sdhci.h | 1 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index e9e919218006..976d4d1e2400 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2392,8 +2392,12 @@ static int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
>  
>  		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>  		if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) {
> -			if (ctrl & SDHCI_CTRL_TUNED_CLK)
> +			if (ctrl & SDHCI_CTRL_TUNED_CLK) {
> +				if (host->ops->post_tuning)
> +					host->ops->post_tuning(host);
>  				return 0; /* Success! */

I think you can hook .execute_tuning and just check if the return value is
zero before doing post tuning. i.e. something like:

	host->mmc_host_ops.execute_tuning = tegra_sdhci_execute_tuning;

int tegra_sdhci_execute_tuning(blah)
{
	int err;

	err = sdhci_execute_tuning(blah);
	if (!err)
		tegra_sdhci_post_tuning(host);
}

> +			}
> +
>  			break;
>  		}
>  
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index c80e0d6f9b10..236d67778645 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -639,6 +639,7 @@ struct sdhci_ops {
>  	void		(*reset)(struct sdhci_host *host, u8 mask);
>  	int	(*platform_execute_tuning)(struct sdhci_host *host, u32 opcode);
>  	int	(*get_max_tuning_loop_count)(struct sdhci_host *host);
> +	void	(*post_tuning)(struct sdhci_host *host);
>  	void	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
>  	void	(*hw_reset)(struct sdhci_host *host);
>  	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
> 

Re: [PATCH V1 04/11] mmc: tegra: update hw tuning process

[Adrian Hunter]

On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> This patch includes below HW tuning related fixes.
> - configures tuning parameters as per Tegra TRM
> - WAR fix for manual tap change
> - HW auto-tuning post process
> 
> As per Tegra TRM, SDR50 mode tuning execution takes upto maximum
> of 256 tuning iterations and SDR104/HS200/HS400 modes tuning
> execution takes upto maximum of 128 tuning iterations.
> 
> This patch programs tuning control register with maximum tuning
> iterations needed based on the timing along with the start tap,
> multiplier, and step size used by the HW tuning.
> 
> Tegra210 has a known issue of glitch on trimmer output when the
> tap value is changed with the trimmer input clock running and the
> WAR is to disable card clock before sending tuning command and
> after sending tuning command wait for 1usec and issue SW reset
> followed by enabling card clock.
> 
> This WAR is applicable when changing tap value manually as well.
> Tegra SDHCI driver has this implemented correctly for manual tap
> change but missing SW reset before enabling card clock during
> sending tuning command.
> 
> Issuing SW reset during tuning command as a part of WAR and is
> applicable in cases where tuning is performed with single step size
> for more iterations. This patch includes this fix.
> 
> HW auto-tuning finds the best largest passing window and sets the
> tap at the middle of the window. With some devices like sandisk
> eMMC driving fast edges and due to high tap to tap delay in the
> Tegra chipset, auto-tuning does not detect falling tap between the
> valid windows resulting in a parital window or a merged window and
> the best tap is set at the signal transition which is actually the
> worst tap location.
> 
> Recommended SW solution is to detect if the best passing window
> picked by the HW tuning is a partial or a merged window based on
> min and max tap delays found from chip characterization across
> PVT and perform tuning correction to pick the best tap.
> 
> This patch has this implemention of post tuning and uses tuned tap
> delay.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---

This looks fine, but will need to be adjusted for changes to patches 2 and 3.

>  drivers/mmc/host/sdhci-tegra.c | 218 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 217 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 46086dd43bfb..2086e0eced88 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -66,6 +66,23 @@
>  
>  #define SDHCI_VNDR_TUN_CTRL0_0				0x1c0
>  #define SDHCI_VNDR_TUN_CTRL0_TUN_HW_TAP			0x20000
> +#define SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_MASK		0x03fc0000
> +#define SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_SHIFT	18
> +#define SDHCI_VNDR_TUN_CTRL0_MUL_M_MASK			0x00001fc0
> +#define SDHCI_VNDR_TUN_CTRL0_MUL_M_SHIFT		6
> +#define SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK		0x000e000
> +#define SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT		13
> +#define TRIES_40					0
> +#define TRIES_128					2
> +#define TRIES_256					4
> +#define SDHCI_VNDR_TUN_CTRL0_TUN_WORD_SEL_MASK		0x7
> +
> +#define SDHCI_TEGRA_VNDR_TUN_CTRL1_0			0x1c4
> +#define SDHCI_TEGRA_VNDR_TUN_STATUS0			0x1C8
> +#define SDHCI_TEGRA_VNDR_TUN_STATUS1			0x1CC
> +#define SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK		0xFF
> +#define SDHCI_TEGRA_VNDR_TUN_STATUS1_END_TAP_SHIFT	0x8
> +#define TUNING_WORD_BIT_SIZE				32
>  
>  #define SDHCI_TEGRA_AUTO_CAL_CONFIG			0x1e4
>  #define SDHCI_AUTO_CAL_START				BIT(31)
> @@ -97,6 +114,8 @@
>  struct sdhci_tegra_soc_data {
>  	const struct sdhci_pltfm_data *pdata;
>  	u32 nvquirks;
> +	u8 min_tap_delay;
> +	u8 max_tap_delay;
>  };
>  
>  /* Magic pull up and pull down pad calibration offsets */
> @@ -136,6 +155,8 @@ struct sdhci_tegra {
>  	u32 default_trim;
>  	u32 dqs_trim;
>  	bool enable_hwcq;
> +	unsigned long curr_clk_rate;
> +	u8 tuned_tap_delay;
>  };
>  
>  static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
> @@ -241,6 +262,7 @@ static void tegra210_sdhci_writew(struct sdhci_host *host, u16 val, int reg)
>  
>  	if (is_tuning_cmd) {
>  		udelay(1);
> +		sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
>  		tegra_sdhci_configure_card_clk(host, clk_enabled);
>  	}
>  }
> @@ -722,6 +744,7 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  	 */
>  	host_clk = tegra_host->ddr_signaling ? clock * 2 : clock;
>  	clk_set_rate(pltfm_host->clk, host_clk);
> +	tegra_host->curr_clk_rate = host_clk;
>  	if (tegra_host->ddr_signaling)
>  		host->max_clk = host_clk;
>  	else
> @@ -770,6 +793,162 @@ static void tegra_sdhci_hs400_dll_cal(struct sdhci_host *host)
>  			"HS400 delay line calibration timed out\n");
>  }
>  
> +static int tegra_sdhci_get_max_tuning_loop(struct sdhci_host *host)
> +{
> +	u32 val;
> +
> +	val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
> +	val &= SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK;
> +	val = val >> SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT;
> +	if (val == TRIES_128)
> +		return 128;
> +	else if (val == TRIES_256)
> +		return 256;
> +	else
> +		return 40;
> +}
> +
> +static void tegra_sdhci_tap_correction(struct sdhci_host *host, u8 num_iter,
> +				       u8 thd_up, u8 thd_low, u8 fixed_tap)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
> +	u32 val, tun_status;
> +	u8 word, bit, edge1, tap, window;
> +	bool tap_result;
> +	bool start_fail = false;
> +	bool start_pass = false;
> +	bool end_pass = false;
> +	bool first_fail = false;
> +	bool first_pass = false;
> +	u8 start_pass_tap = 0;
> +	u8 end_pass_tap = 0;
> +	u8 first_fail_tap = 0;
> +	u8 first_pass_tap = 0;
> +	u8 total_tuning_words = num_iter / TUNING_WORD_BIT_SIZE;
> +
> +	/*
> +	 * Read auto-tuned results and extract good valid passing window by
> +	 * filtering out un-wanted bubble/partial/merged windows.
> +	 */
> +	for (word = 0; word < total_tuning_words; word++) {
> +		val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
> +		val &= ~SDHCI_VNDR_TUN_CTRL0_TUN_WORD_SEL_MASK;
> +		val |= word;
> +		sdhci_writel(host, val, SDHCI_VNDR_TUN_CTRL0_0);
> +		tun_status = sdhci_readl(host, SDHCI_TEGRA_VNDR_TUN_STATUS0);
> +		bit = 0;
> +		while (bit < TUNING_WORD_BIT_SIZE) {
> +			tap = word * TUNING_WORD_BIT_SIZE + bit;
> +			tap_result = tun_status & (1 << bit);
> +			if (!tap_result && !start_fail) {
> +				start_fail = true;
> +				if (!first_fail) {
> +					first_fail_tap = tap;
> +					first_fail = true;
> +				}
> +
> +			} else if (tap_result && start_fail && !start_pass) {
> +				start_pass_tap = tap;
> +				start_pass = true;
> +				if (!first_pass) {
> +					first_pass_tap = tap;
> +					first_pass = true;
> +				}
> +
> +			} else if (!tap_result && start_fail && start_pass &&
> +				   !end_pass) {
> +				end_pass_tap = tap - 1;
> +				end_pass = true;
> +			} else if (tap_result && start_pass && start_fail &&
> +				   end_pass) {
> +				window = end_pass_tap - start_pass_tap;
> +				/* discard merged window and bubble window */
> +				if (window >= thd_up || window < thd_low) {
> +					start_pass_tap = tap;
> +					end_pass = false;
> +				} else {
> +					/* set tap at middle of valid window */
> +					tap = start_pass_tap + window / 2;
> +					tegra_host->tuned_tap_delay = tap;
> +					return;
> +				}
> +			}
> +
> +			bit++;
> +		}
> +	}
> +
> +	if (!first_fail) {
> +		WARN_ON("no edge detected, continue with hw tuned delay.\n");
> +	} else if (first_pass) {
> +		/* set tap location at fixed tap relative to the first edge */
> +		edge1 = first_fail_tap + (first_pass_tap - first_fail_tap) / 2;
> +		if (edge1 - 1 > fixed_tap)
> +			tegra_host->tuned_tap_delay = edge1 - fixed_tap;
> +		else
> +			tegra_host->tuned_tap_delay = edge1 + fixed_tap;
> +	}
> +}
> +
> +static void tegra_sdhci_post_tuning(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
> +	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
> +	u32 avg_tap_dly, val, min_tap_dly, max_tap_dly;
> +	u8 fixed_tap, start_tap, end_tap, window_width;
> +	u8 thdupper, thdlower;
> +	u8 num_iter;
> +	u32 clk_rate_mhz, period_ps, bestcase, worstcase;
> +
> +	/* retain HW tuned tap to use incase if no correction is needed */
> +	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
> +	tegra_host->tuned_tap_delay = (val & SDHCI_CLOCK_CTRL_TAP_MASK) >>
> +				      SDHCI_CLOCK_CTRL_TAP_SHIFT;
> +	if (soc_data->min_tap_delay && soc_data->max_tap_delay) {
> +		min_tap_dly = soc_data->min_tap_delay;
> +		max_tap_dly = soc_data->max_tap_delay;
> +		clk_rate_mhz = tegra_host->curr_clk_rate / USEC_PER_SEC;
> +		period_ps = USEC_PER_SEC / clk_rate_mhz;
> +		bestcase = period_ps / min_tap_dly;
> +		worstcase = period_ps / max_tap_dly;
> +		/*
> +		 * Upper and Lower bound thresholds used to detect merged and
> +		 * bubble windows
> +		 */
> +		thdupper = (2 * worstcase + bestcase) / 2;
> +		thdlower = worstcase / 4;
> +		/*
> +		 * fixed tap is used when HW tuning result contains single edge
> +		 * and tap is set at fixed tap delay relative to the first edge
> +		 */
> +		avg_tap_dly = (period_ps * 2) / (min_tap_dly + max_tap_dly);
> +		fixed_tap = avg_tap_dly / 2;
> +
> +		val = sdhci_readl(host, SDHCI_TEGRA_VNDR_TUN_STATUS1);
> +		start_tap = val & SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK;
> +		end_tap = (val >> SDHCI_TEGRA_VNDR_TUN_STATUS1_END_TAP_SHIFT) &
> +			  SDHCI_TEGRA_VNDR_TUN_STATUS1_TAP_MASK;
> +		window_width = end_tap - start_tap;
> +		num_iter = tegra_sdhci_get_max_tuning_loop(host);
> +		/*
> +		 * partial window includes edges of the tuning range.
> +		 * merged window includes more taps so window width is higher
> +		 * than upper threshold.
> +		 */
> +		if (start_tap == 0 || (end_tap == (num_iter - 1)) ||
> +		    (end_tap == num_iter - 2) || window_width >= thdupper) {
> +			pr_debug("%s: Apply tuning correction\n",
> +				 mmc_hostname(host->mmc));
> +			tegra_sdhci_tap_correction(host, num_iter, thdupper,
> +						   thdlower, fixed_tap);
> +		}
> +	}
> +
> +	tegra_sdhci_set_tap(host, tegra_host->tuned_tap_delay);
> +}
> +
>  static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
>  					  unsigned timing)
>  {
> @@ -778,17 +957,22 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
>  	bool set_default_tap = false;
>  	bool set_dqs_trim = false;
>  	bool do_hs400_dll_cal = false;
> +	u8 iter = TRIES_256;
> +	u32 val;
>  
>  	tegra_host->ddr_signaling = false;
>  	switch (timing) {
>  	case MMC_TIMING_UHS_SDR50:
> +		break;
>  	case MMC_TIMING_UHS_SDR104:
>  	case MMC_TIMING_MMC_HS200:
>  		/* Don't set default tap on tunable modes. */
> +		iter = TRIES_128;
>  		break;
>  	case MMC_TIMING_MMC_HS400:
>  		set_dqs_trim = true;
>  		do_hs400_dll_cal = true;
> +		iter = TRIES_128;
>  		break;
>  	case MMC_TIMING_MMC_DDR52:
>  	case MMC_TIMING_UHS_DDR50:
> @@ -800,11 +984,23 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
>  		break;
>  	}
>  
> +	val = sdhci_readl(host, SDHCI_VNDR_TUN_CTRL0_0);
> +	val &= ~(SDHCI_VNDR_TUN_CTRL0_TUN_ITER_MASK |
> +		 SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_MASK |
> +		 SDHCI_VNDR_TUN_CTRL0_MUL_M_MASK);
> +	val |= (iter << SDHCI_VNDR_TUN_CTRL0_TUN_ITER_SHIFT |
> +		0 << SDHCI_VNDR_TUN_CTRL0_START_TAP_VAL_SHIFT |
> +		1 << SDHCI_VNDR_TUN_CTRL0_MUL_M_SHIFT);
> +	sdhci_writel(host, val, SDHCI_VNDR_TUN_CTRL0_0);
> +	sdhci_writel(host, 0, SDHCI_TEGRA_VNDR_TUN_CTRL1_0);
> +
>  	sdhci_set_uhs_signaling(host, timing);
>  
>  	tegra_sdhci_pad_autocalib(host);
>  
> -	if (set_default_tap)
> +	if (tegra_host->tuned_tap_delay && !set_default_tap)
> +		tegra_sdhci_set_tap(host, tegra_host->tuned_tap_delay);
> +	else
>  		tegra_sdhci_set_tap(host, tegra_host->default_tap);
>  
>  	if (set_dqs_trim)
> @@ -1090,6 +1286,8 @@ static const struct sdhci_ops tegra210_sdhci_ops = {
>  	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
>  	.voltage_switch = tegra_sdhci_voltage_switch,
>  	.get_max_clock = tegra_sdhci_get_max_clock,
> +	.get_max_tuning_loop_count = tegra_sdhci_get_max_tuning_loop,
> +	.post_tuning = tegra_sdhci_post_tuning,
>  };
>  
>  static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
> @@ -1110,6 +1308,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
>  		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
>  		    NVQUIRK_ENABLE_SDR50 |
>  		    NVQUIRK_ENABLE_SDR104,
> +	.min_tap_delay = 106,
> +	.max_tap_delay = 185,
>  };
>  
>  static const struct sdhci_ops tegra186_sdhci_ops = {
> @@ -1121,6 +1321,8 @@ static const struct sdhci_ops tegra186_sdhci_ops = {
>  	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
>  	.voltage_switch = tegra_sdhci_voltage_switch,
>  	.get_max_clock = tegra_sdhci_get_max_clock,
> +	.get_max_tuning_loop_count = tegra_sdhci_get_max_tuning_loop,
> +	.post_tuning = tegra_sdhci_post_tuning,
>  	.irq = sdhci_tegra_cqhci_irq,
>  };
>  
> @@ -1150,9 +1352,23 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
>  		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
>  		    NVQUIRK_ENABLE_SDR50 |
>  		    NVQUIRK_ENABLE_SDR104,
> +	.min_tap_delay = 84,
> +	.max_tap_delay = 136,
> +};
> +
> +static const struct sdhci_tegra_soc_data soc_data_tegra194 = {
> +	.pdata = &sdhci_tegra186_pdata,
> +	.nvquirks = NVQUIRK_NEEDS_PAD_CONTROL |
> +		    NVQUIRK_HAS_PADCALIB |
> +		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
> +		    NVQUIRK_ENABLE_SDR50 |
> +		    NVQUIRK_ENABLE_SDR104,
> +	.min_tap_delay = 96,
> +	.max_tap_delay = 139,
>  };
>  
>  static const struct of_device_id sdhci_tegra_dt_match[] = {
> +	{ .compatible = "nvidia,tegra194-sdhci", .data = &soc_data_tegra194 },
>  	{ .compatible = "nvidia,tegra186-sdhci", .data = &soc_data_tegra186 },
>  	{ .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
>  	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra124 },
> 

Re: [PATCH V1 09/11] mmc: cqhci: add CQHCI_SSC1 register CBC field mask

[Adrian Hunter]

On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> This patch adds define for CBC field mask of the register
> CQHCI_SSC1.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>

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

> ---
>  drivers/mmc/host/cqhci.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> index f96d8565cc07..f1dc48c7436f 100644
> --- a/drivers/mmc/host/cqhci.h
> +++ b/drivers/mmc/host/cqhci.h
> @@ -88,6 +88,7 @@
>  
>  /* send status config 1 */
>  #define CQHCI_SSC1			0x40
> +#define CQHCI_SSC1_CBC_MASK		GENMASK(19, 16)
>  
>  /* send status config 2 */
>  #define CQHCI_SSC2			0x44
> 

Re: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Adrian Hunter]

On 7/03/19 8:16 PM, Sowjanya Komatineni wrote:
> 
>> On 3/6/2019 6:30 PM, Adrian Hunter wrote:
>>> On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
>>>> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor for 
>>>> DCMD with R1B response type to allow the command to be sent to device 
>>>> during data activity or busy time.
>>>>
>>>> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT to 1 
>>>> by CQHCI controller for DCMDs with R1B response type and since DCMD 
>>>> does not trigger any data transfer, DCMD task complete happens 
>>>> leaving the DATA FSM of host controller in wait state for data.
>>>>
>>>> This effects the data transfer task issued after R1B DCMD task and no 
>>>> interrupt is generated for the data transfer task.
>>>>
>>>> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task 
>>>> descriptor and as DCMD task descriptor preparation is done by cqhci 
>>>> driver, this patch adds cqequirk to handle this.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>>   drivers/mmc/host/cqhci.c | 5 ++++-
>>>>   drivers/mmc/host/cqhci.h | 1 +
>>>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c 
>>>> index a8af682a9182..b34c07125f32 100644
>>>> --- a/drivers/mmc/host/cqhci.c
>>>> +++ b/drivers/mmc/host/cqhci.c
>>>> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
>>>>   	} else {
>>>>   		if (mrq->cmd->flags & MMC_RSP_R1B) {
>>>>   			resp_type = 0x3;
>>>> -			timing = 0x0;
>>>> +			if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
>>>> +				timing = 0x1;
>>>> +			else
>>>> +				timing = 0x0;
>>> I was thinking it would be nice if there was a generic way for drivers 
>>> to make changes to descriptors before a task is started.  Currently 
>>> there is
>>> host->ops->write_l() which would make it possible by checking for 
>>> host->ops->CQHCI_TDBR
>>> register and, in this case, the DCMD tag.  We would need to export 
>>> get_desc(), perhaps rename it cqhci_get_desc() and put it in cqhci.h 
>>> since it is an inline function.
>>
>> We take spin_lock_irqsave after the descriptor is prepared and before writing to TDBR.
>> Not sure but tomorrow this may become a limitation for drivers to make changes to descriptors if they edit descriptors in host->ops->write_l() call.
>> Though in this case it is not required here.
>>
>>>
>>> Alternatively we could add host->ops for descriptor preparation.
>>
>> Both ways sounds good to me.
>> But maybe adding a host->ops for descriptor preparation is better way to go,
>> since that will be the right interface exposed to make changes to 
>> descriptors.
>>
> 
> DCMD descriptor attributes remain same for any host and also task parameters as QBR need to be enabled with DCMD.
> So I believe it should be ok if we just add callback to allow hosts to update command parameters of DCMD descriptor only thru cqhci_host_ops.

That sounds fine to me.  Maybe do that for the next version of this patchset.

> 
> Also, don’t see any requirement for host specific Task parameter updates in Task descriptor so not sure if there is any need to provide callback for task descriptor data preparation to hosts.
> Please confirm.
> 
>>>
>>> What do people think?
>>>
>>>>   		} else {
>>>>   			resp_type = 0x2;
>>>>   			timing = 0x1;
>>>> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
>>>> index 9e68286a07b4..f96d8565cc07 100644
>>>> --- a/drivers/mmc/host/cqhci.h
>>>> +++ b/drivers/mmc/host/cqhci.h
>>>> @@ -170,6 +170,7 @@ struct cqhci_host {
>>>>   
>>>>   	u32 quirks;
>>>>   #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
>>>> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
>>>>   
>>>>   	bool enabled;
>>>>   	bool halted;
>>>>

Re: [PATCH V1 10/11] mmc: tegra: fix CQE resume sequence

[Adrian Hunter]

On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> Tegra CQHCI/SDHCI design prevents write access to SDHCI block size
> register when CQE is enabled and unhalted.
> 
> CQHCI driver enabled CQE prior to invoking sdhci_cqe_enable which
> violates this Tegra specific host requirement.
> 
> This patch fixes this by configuring sdhci block registers prior
> to CQE unhalt.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>

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

> ---
>  drivers/mmc/host/sdhci-tegra.c | 71 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 62 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 2b63626dc2fa..7063cfcdc590 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -1126,23 +1126,75 @@ static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
>  		tegra_host->pad_calib_required = true;
>  }
>  
> +static void tegra_cqhci_writel(struct cqhci_host *cq_host, u32 val, int reg)
> +{
> +	struct mmc_host *mmc = cq_host->mmc;
> +	u8 ctrl;
> +	ktime_t timeout;
> +	bool timed_out;
> +
> +	/*
> +	 * During CQE resume/unhalt, CQHCI driver unhalts CQE prior to
> +	 * cqhci_host_ops enable where SDHCI DMA and BLOCK_SIZE registers need
> +	 * to be re-configured.
> +	 * Tegra CQHCI/SDHCI prevents write access to block size register when
> +	 * CQE is unhalted. So handling CQE resume sequence here to configure
> +	 * SDHCI block registers prior to exiting CQE halt state.
> +	 */
> +	if (reg == CQHCI_CTL && !(val & CQHCI_HALT) &&
> +	    cqhci_readl(cq_host, CQHCI_CTL) & CQHCI_HALT) {
> +		sdhci_cqe_enable(mmc);
> +		writel(val, cq_host->mmio + reg);
> +		timeout = ktime_add_us(ktime_get(), 50);
> +		while (1) {
> +			timed_out = ktime_compare(ktime_get(), timeout) > 0;
> +			ctrl = cqhci_readl(cq_host, CQHCI_CTL);
> +			if (!(ctrl & CQHCI_HALT) || timed_out)
> +				break;
> +		}
> +		/*
> +		 * CQE usually resumes very quick, but incase if Tegra CQE
> +		 * doesn't resume retry unhalt.
> +		 */
> +		if (timed_out)
> +			writel(val, cq_host->mmio + reg);
> +	} else {
> +		writel(val, cq_host->mmio + reg);
> +	}
> +}
> +
>  static void sdhci_tegra_cqe_enable(struct mmc_host *mmc)
>  {
>  	struct cqhci_host *cq_host = mmc->cqe_private;
> -	u32 cqcfg = 0;
> +	u32 val;
>  
>  	/*
> -	 * Tegra SDMMC Controller design prevents write access to BLOCK_COUNT
> -	 * registers when CQE is enabled.
> +	 * Tegra CQHCI/SDMMC design prevents write access to sdhci block size
> +	 * register when CQE is enabled and unhalted.
> +	 * CQHCI driver enables CQE prior to activation, so disable CQE before
> +	 * programming block size in sdhci controller and enable it back.
>  	 */
> -	cqcfg = cqhci_readl(cq_host, CQHCI_CFG);
> -	if (cqcfg & CQHCI_ENABLE)
> -		cqhci_writel(cq_host, (cqcfg & ~CQHCI_ENABLE), CQHCI_CFG);
> +	if (!cq_host->activated) {
> +		val = cqhci_readl(cq_host, CQHCI_CFG);
> +		if (val & CQHCI_ENABLE)
> +			cqhci_writel(cq_host, (val & ~CQHCI_ENABLE),
> +				     CQHCI_CFG);
> +		sdhci_cqe_enable(mmc);
> +		if (val & CQHCI_ENABLE)
> +			cqhci_writel(cq_host, val, CQHCI_CFG);
> +	}
>  
> -	sdhci_cqe_enable(mmc);
> +	/*
> +	 * CMD CRC errors are seen sometimes with some eMMC devices when status
> +	 * command is sent during transfer of last data block which is the
> +	 * default case as send status command block counter (CBC) is 1.
> +	 * So changing send status command block counter (CBC) to 0 to allow
> +	 * send status command only when the data lines are idle.
> +	 */
> +	val = cqhci_readl(cq_host, CQHCI_SSC1);
> +	val &= ~CQHCI_SSC1_CBC_MASK;
> +	cqhci_writel(cq_host, val, CQHCI_SSC1);
>  
> -	if (cqcfg & CQHCI_ENABLE)
> -		cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
>  }
>  
>  static void sdhci_tegra_dumpregs(struct mmc_host *mmc)
> @@ -1167,6 +1219,7 @@ static const struct cqhci_host_ops sdhci_tegra_cqhci_ops = {
>  	.enable	= sdhci_tegra_cqe_enable,
>  	.disable = sdhci_cqe_disable,
>  	.dumpregs = sdhci_tegra_dumpregs,
> +	.write_l    = tegra_cqhci_writel,
>  };
>  
>  static const struct sdhci_ops tegra_sdhci_ops = {
> 

RE: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Sowjanya Komatineni]

> On 7/03/19 8:16 PM, Sowjanya Komatineni wrote:
>> 
>>> On 3/6/2019 6:30 PM, Adrian Hunter wrote:
>>>> On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
>>>>> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor 
>>>>> for DCMD with R1B response type to allow the command to be sent to 
>>>>> device during data activity or busy time.
>>>>>
>>>>> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT to 
>>>>> 1 by CQHCI controller for DCMDs with R1B response type and since 
>>>>> DCMD does not trigger any data transfer, DCMD task complete happens 
>>>>> leaving the DATA FSM of host controller in wait state for data.
>>>>>
>>>>> This effects the data transfer task issued after R1B DCMD task and 
>>>>> no interrupt is generated for the data transfer task.
>>>>>
>>>>> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task 
>>>>> descriptor and as DCMD task descriptor preparation is done by cqhci 
>>>>> driver, this patch adds cqequirk to handle this.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>>   drivers/mmc/host/cqhci.c | 5 ++++-
>>>>>   drivers/mmc/host/cqhci.h | 1 +
>>>>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c 
>>>>> index a8af682a9182..b34c07125f32 100644
>>>>> --- a/drivers/mmc/host/cqhci.c
>>>>> +++ b/drivers/mmc/host/cqhci.c
>>>>> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
>>>>>   	} else {
>>>>>   		if (mrq->cmd->flags & MMC_RSP_R1B) {
>>>>>   			resp_type = 0x3;
>>>>> -			timing = 0x0;
>>>>> +			if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
>>>>> +				timing = 0x1;
>>>>> +			else
>>>>> +				timing = 0x0;
>>>> I was thinking it would be nice if there was a generic way for 
>>>> drivers to make changes to descriptors before a task is started.  
>>>> Currently there is
>>>> host->ops->write_l() which would make it possible by checking for 
>>>> host->ops->CQHCI_TDBR
>>>> register and, in this case, the DCMD tag.  We would need to export 
>>>> get_desc(), perhaps rename it cqhci_get_desc() and put it in cqhci.h 
>>>> since it is an inline function.
>>>
>>> We take spin_lock_irqsave after the descriptor is prepared and before writing to TDBR.
>>> Not sure but tomorrow this may become a limitation for drivers to make changes to descriptors if they edit descriptors in host->ops->write_l() call.
>>> Though in this case it is not required here.
>>>
>>>>
>>>> Alternatively we could add host->ops for descriptor preparation.
>>>
>>> Both ways sounds good to me.
>>> But maybe adding a host->ops for descriptor preparation is better way 
>>> to go, since that will be the right interface exposed to make changes 
>>> to descriptors.
>>>
>> 
>> DCMD descriptor attributes remain same for any host and also task parameters as QBR need to be enabled with DCMD.
>> So I believe it should be ok if we just add callback to allow hosts to update command parameters of DCMD descriptor only thru cqhci_host_ops.
>
>That sounds fine to me.  Maybe do that for the next version of this patchset.

Thanks Adrian. Actually, DCMD command parameters include opcode, timing, and response type where opcode and response type stays same.
So I only see need for hosts to choose cmd timing to control if hardware should wait for busy time or not basically to send command during busy or idle time.

Will add cqhci_host_ops interface to allow host driver to choose cmd timing and will send along with the other tuning related feedback fixes of this series.

>> 
>> Also, don’t see any requirement for host specific Task parameter updates in Task descriptor so not sure if there is any need to provide callback for task descriptor data preparation to hosts.
>> Please confirm.
>> 
>>>>
>>>> What do people think?
>>>>
>>>>>   		} else {
>>>>>   			resp_type = 0x2;
>>>>>   			timing = 0x1;
>>>>> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h 
>>>>> index 9e68286a07b4..f96d8565cc07 100644
>>>>> --- a/drivers/mmc/host/cqhci.h
>>>>> +++ b/drivers/mmc/host/cqhci.h
>>>>> @@ -170,6 +170,7 @@ struct cqhci_host {
>>>>>   
>>>>>   	u32 quirks;
>>>>>   #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
>>>>> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
>>>>>   
>>>>>   	bool enabled;
>>>>>   	bool halted;
>>>>>

Re: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Ritesh Harjani]

On 3/7/2019 11:46 PM, Sowjanya Komatineni wrote:
>> On 3/6/2019 6:30 PM, Adrian Hunter wrote:
>>> On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
>>>> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor for
>>>> DCMD with R1B response type to allow the command to be sent to device
>>>> during data activity or busy time.
>>>>
>>>> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT to 1
>>>> by CQHCI controller for DCMDs with R1B response type and since DCMD
>>>> does not trigger any data transfer, DCMD task complete happens
>>>> leaving the DATA FSM of host controller in wait state for data.
>>>>
>>>> This effects the data transfer task issued after R1B DCMD task and no
>>>> interrupt is generated for the data transfer task.
>>>>
>>>> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task
>>>> descriptor and as DCMD task descriptor preparation is done by cqhci
>>>> driver, this patch adds cqequirk to handle this.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>>    drivers/mmc/host/cqhci.c | 5 ++++-
>>>>    drivers/mmc/host/cqhci.h | 1 +
>>>>    2 files changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
>>>> index a8af682a9182..b34c07125f32 100644
>>>> --- a/drivers/mmc/host/cqhci.c
>>>> +++ b/drivers/mmc/host/cqhci.c
>>>> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
>>>>    	} else {
>>>>    		if (mrq->cmd->flags & MMC_RSP_R1B) {
>>>>    			resp_type = 0x3;
>>>> -			timing = 0x0;
>>>> +			if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
>>>> +				timing = 0x1;
>>>> +			else
>>>> +				timing = 0x0;
>>> I was thinking it would be nice if there was a generic way for drivers
>>> to make changes to descriptors before a task is started.  Currently
>>> there is
>>> host->ops->write_l() which would make it possible by checking for
>>> host->ops->CQHCI_TDBR
>>> register and, in this case, the DCMD tag.  We would need to export
>>> get_desc(), perhaps rename it cqhci_get_desc() and put it in cqhci.h
>>> since it is an inline function.
>> We take spin_lock_irqsave after the descriptor is prepared and before writing to TDBR.
>> Not sure but tomorrow this may become a limitation for drivers to make changes to descriptors if they edit descriptors in host->ops->write_l() call.
>> Though in this case it is not required here.
>>
>>> Alternatively we could add host->ops for descriptor preparation.
>> Both ways sounds good to me.
>> But maybe adding a host->ops for descriptor preparation is better way to go,
>> since that will be the right interface exposed to make changes to
>> descriptors.
>>
> DCMD descriptor attributes remain same for any host and also task parameters as QBR need to be enabled with DCMD.
> So I believe it should be ok if we just add callback to allow hosts to update command parameters of DCMD descriptor only thru cqhci_host_ops.

For now we can add host->ops as update_dcmd_desc and pass the task_desc 
of DCMD for updating any params which host may want to update.

>
> Also, don’t see any requirement for host specific Task parameter updates in Task descriptor so not sure if there is any need to provide callback for task descriptor data preparation to hosts.
> Please confirm.

Sure, for now the requirement has come up only for DCMD desc update. 
Sure we can add task descriptor ops in the similar way later when required.

Adrian, please confirm if you are fine with both of above?

>
>>> What do people think?
>>>
>>>>    		} else {
>>>>    			resp_type = 0x2;
>>>>    			timing = 0x1;
>>>> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
>>>> index 9e68286a07b4..f96d8565cc07 100644
>>>> --- a/drivers/mmc/host/cqhci.h
>>>> +++ b/drivers/mmc/host/cqhci.h
>>>> @@ -170,6 +170,7 @@ struct cqhci_host {
>>>>    
>>>>    	u32 quirks;
>>>>    #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
>>>> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
>>>>    
>>>>    	bool enabled;
>>>>    	bool halted;
>>>>

RE: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Hunter, Adrian]

> -----Original Message-----
> From: Ritesh Harjani [mailto:riteshh@codeaurora.org]
> Sent: Wednesday, March 13, 2019 4:31 AM
> To: Sowjanya Komatineni <skomatineni@nvidia.com>; Hunter, Adrian
> <adrian.hunter@intel.com>; ulf.hansson@linaro.org; robh+dt@kernel.org;
> mark.rutland@arm.com; Asutosh Das <asutoshd@codeaurora.org>
> Cc: thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> Aniruddha Tvs Rao <anrao@nvidia.com>; linux-tegra@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-mmc@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD
> CMD_TIMING
> 
> 
> On 3/7/2019 11:46 PM, Sowjanya Komatineni wrote:
> >> On 3/6/2019 6:30 PM, Adrian Hunter wrote:
> >>> On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> >>>> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor
> >>>> for DCMD with R1B response type to allow the command to be sent to
> >>>> device during data activity or busy time.
> >>>>
> >>>> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT
> to
> >>>> 1 by CQHCI controller for DCMDs with R1B response type and since
> >>>> DCMD does not trigger any data transfer, DCMD task complete
> happens
> >>>> leaving the DATA FSM of host controller in wait state for data.
> >>>>
> >>>> This effects the data transfer task issued after R1B DCMD task and
> >>>> no interrupt is generated for the data transfer task.
> >>>>
> >>>> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task
> >>>> descriptor and as DCMD task descriptor preparation is done by cqhci
> >>>> driver, this patch adds cqequirk to handle this.
> >>>>
> >>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> >>>> ---
> >>>>    drivers/mmc/host/cqhci.c | 5 ++++-
> >>>>    drivers/mmc/host/cqhci.h | 1 +
> >>>>    2 files changed, 5 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
> >>>> index a8af682a9182..b34c07125f32 100644
> >>>> --- a/drivers/mmc/host/cqhci.c
> >>>> +++ b/drivers/mmc/host/cqhci.c
> >>>> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct
> mmc_host *mmc,
> >>>>    	} else {
> >>>>    		if (mrq->cmd->flags & MMC_RSP_R1B) {
> >>>>    			resp_type = 0x3;
> >>>> -			timing = 0x0;
> >>>> +			if (cq_host->quirks &
> CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
> >>>> +				timing = 0x1;
> >>>> +			else
> >>>> +				timing = 0x0;
> >>> I was thinking it would be nice if there was a generic way for
> >>> drivers to make changes to descriptors before a task is started.
> >>> Currently there is
> >>> host->ops->write_l() which would make it possible by checking for
> >>> host->ops->CQHCI_TDBR
> >>> register and, in this case, the DCMD tag.  We would need to export
> >>> get_desc(), perhaps rename it cqhci_get_desc() and put it in cqhci.h
> >>> since it is an inline function.
> >> We take spin_lock_irqsave after the descriptor is prepared and before
> writing to TDBR.
> >> Not sure but tomorrow this may become a limitation for drivers to make
> changes to descriptors if they edit descriptors in host->ops->write_l() call.
> >> Though in this case it is not required here.
> >>
> >>> Alternatively we could add host->ops for descriptor preparation.
> >> Both ways sounds good to me.
> >> But maybe adding a host->ops for descriptor preparation is better way
> >> to go, since that will be the right interface exposed to make changes
> >> to descriptors.
> >>
> > DCMD descriptor attributes remain same for any host and also task
> parameters as QBR need to be enabled with DCMD.
> > So I believe it should be ok if we just add callback to allow hosts to update
> command parameters of DCMD descriptor only thru cqhci_host_ops.
> 
> For now we can add host->ops as update_dcmd_desc and pass the
> task_desc of DCMD for updating any params which host may want to update.
> 
> >
> > Also, don’t see any requirement for host specific Task parameter updates
> in Task descriptor so not sure if there is any need to provide callback for task
> descriptor data preparation to hosts.
> > Please confirm.
> 
> Sure, for now the requirement has come up only for DCMD desc update.
> Sure we can add task descriptor ops in the similar way later when required.
> 
> Adrian, please confirm if you are fine with both of above?

Yes, I agree.  Sowjanya's V2 06/10 patch is quite narrowly focused, whereas
update_dcmd_desc as you described seems just as easy to implement, but
Is more flexible.

> 
> >
> >>> What do people think?
> >>>
> >>>>    		} else {
> >>>>    			resp_type = 0x2;
> >>>>    			timing = 0x1;
> >>>> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> >>>> index 9e68286a07b4..f96d8565cc07 100644
> >>>> --- a/drivers/mmc/host/cqhci.h
> >>>> +++ b/drivers/mmc/host/cqhci.h
> >>>> @@ -170,6 +170,7 @@ struct cqhci_host {
> >>>>
> >>>>    	u32 quirks;
> >>>>    #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
> >>>> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
> >>>>
> >>>>    	bool enabled;
> >>>>    	bool halted;
> >>>>

RE: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING

[Sowjanya Komatineni]

> -----Original Message-----
> From: Hunter, Adrian <adrian.hunter@intel.com> 
> Sent: Wednesday, March 13, 2019 2:57 AM
> To: Ritesh Harjani <riteshh@codeaurora.org>; Sowjanya Komatineni <skomatineni@nvidia.com>; ulf.hansson@linaro.org; robh+dt@kernel.org; mark.rutland@arm.com; Asutosh Das <asutoshd@codeaurora.org>
> Cc: thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>; Aniruddha Tvs Rao <anrao@nvidia.com>; linux-tegra@vger.kernel.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org
> Subject: RE: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING
>
> > -----Original Message-----
> > From: Ritesh Harjani [mailto:riteshh@codeaurora.org]
> > Sent: Wednesday, March 13, 2019 4:31 AM
> > To: Sowjanya Komatineni <skomatineni@nvidia.com>; Hunter, Adrian 
> > <adrian.hunter@intel.com>; ulf.hansson@linaro.org; robh+dt@kernel.org; 
> > mark.rutland@arm.com; Asutosh Das <asutoshd@codeaurora.org>
> > Cc: thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>; 
> > Aniruddha Tvs Rao <anrao@nvidia.com>; linux-tegra@vger.kernel.org; 
> > linux- kernel@vger.kernel.org; linux-mmc@vger.kernel.org; 
> > devicetree@vger.kernel.org
> > Subject: Re: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD 
> > CMD_TIMING
> > 
> > 
> > On 3/7/2019 11:46 PM, Sowjanya Komatineni wrote:
> > >> On 3/6/2019 6:30 PM, Adrian Hunter wrote:
> > >>> On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
> > >>>> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor 
> > >>>> for DCMD with R1B response type to allow the command to be sent 
> > >>>> to device during data activity or busy time.
> > >>>>
> > >>>> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT
> > to
> > >>>> 1 by CQHCI controller for DCMDs with R1B response type and since 
> > >>>> DCMD does not trigger any data transfer, DCMD task complete
> > happens
> > >>>> leaving the DATA FSM of host controller in wait state for data.
> > >>>>
> > >>>> This effects the data transfer task issued after R1B DCMD task 
> > >>>> and no interrupt is generated for the data transfer task.
> > >>>>
> > >>>> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task 
> > >>>> descriptor and as DCMD task descriptor preparation is done by 
> > >>>> cqhci driver, this patch adds cqequirk to handle this.
> > >>>>
> > >>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> > >>>> ---
> > >>>>    drivers/mmc/host/cqhci.c | 5 ++++-
> > >>>>    drivers/mmc/host/cqhci.h | 1 +
> > >>>>    2 files changed, 5 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c 
> > >>>> index a8af682a9182..b34c07125f32 100644
> > >>>> --- a/drivers/mmc/host/cqhci.c
> > >>>> +++ b/drivers/mmc/host/cqhci.c
> > >>>> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct
> > mmc_host *mmc,
> > >>>>    	} else {
> > >>>>    		if (mrq->cmd->flags & MMC_RSP_R1B) {
> > >>>>    			resp_type = 0x3;
> > >>>> -			timing = 0x0;
> > >>>> +			if (cq_host->quirks &
> > CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
> > >>>> +				timing = 0x1;
> > >>>> +			else
> > >>>> +				timing = 0x0;
> > >>> I was thinking it would be nice if there was a generic way for 
> > >>> drivers to make changes to descriptors before a task is started.
> > >>> Currently there is
> > >>> host->ops->write_l() which would make it possible by checking for 
> > >>> host->ops->CQHCI_TDBR
> > >>> register and, in this case, the DCMD tag.  We would need to export 
> > >>> get_desc(), perhaps rename it cqhci_get_desc() and put it in 
> > >>> cqhci.h since it is an inline function.
> > >> We take spin_lock_irqsave after the descriptor is prepared and 
> > >> before
> > writing to TDBR.
> > >> Not sure but tomorrow this may become a limitation for drivers to 
> > >> make
> > changes to descriptors if they edit descriptors in host->ops->write_l() call.
> > >> Though in this case it is not required here.
> > >>
> > >>> Alternatively we could add host->ops for descriptor preparation.
> > >> Both ways sounds good to me.
> > >> But maybe adding a host->ops for descriptor preparation is better 
> > >> way to go, since that will be the right interface exposed to make 
> > >> changes to descriptors.
> > >>
> > > DCMD descriptor attributes remain same for any host and also task
> > parameters as QBR need to be enabled with DCMD.
> > > So I believe it should be ok if we just add callback to allow hosts 
> > > to update
> > command parameters of DCMD descriptor only thru cqhci_host_ops.
> > 
> > For now we can add host->ops as update_dcmd_desc and pass the 
> > task_desc of DCMD for updating any params which host may want to update.
> > 
> > >
> > > Also, don’t see any requirement for host specific Task parameter 
> > > updates
> > in Task descriptor so not sure if there is any need to provide 
> > callback for task descriptor data preparation to hosts.
> > > Please confirm.
> > 
> > Sure, for now the requirement has come up only for DCMD desc update.
> > Sure we can add task descriptor ops in the similar way later when required.
> > 
> > Adrian, please confirm if you are fine with both of above?
>
> Yes, I agree.  Sowjanya's V2 06/10 patch is quite narrowly focused, whereas update_dcmd_desc as you described seems just as easy to implement, but Is more flexible.
>
In case if my response to feedback got missed, DCMD descriptor attributes and task parameters remain same for any host including QBR as its must to enable with DCMD.
DCMD command parameters include opcode, timing, and response type where opcode and response type stays same.
So added interface only to allow command timing parameter which host can set either 1 or 0 in V2.

Irrespective of DCMD descriptor update possibilities needed for hosts, we can expose complete DCMD descriptor data to hosts.

Based on your both feedbacks, Will go ahead and update to allow complete DCMD descriptor data update for hosts for more flexibility.

> > 
> > >
> > >>> What do people think?
> > >>>
> > >>>>    		} else {
> > >>>>    			resp_type = 0x2;
> > >>>>    			timing = 0x1;
> > >>>> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h 
> > >>>> index 9e68286a07b4..f96d8565cc07 100644
> > >>>> --- a/drivers/mmc/host/cqhci.h
> > >>>> +++ b/drivers/mmc/host/cqhci.h
> > >>>> @@ -170,6 +170,7 @@ struct cqhci_host {
> > >>>>
> > >>>>    	u32 quirks;
> > >>>>    #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ	0x1
> > >>>> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD	0x2
> > >>>>
> > >>>>    	bool enabled;
> > >>>>    	bool halted;
> > > >>>>