diff mbox

[v2,3/5] mmc: tegra: implement UHS tuning

Message ID 1450809664-11360-4-git-send-email-dev@lynxeye.de
State Accepted
Headers show

Commit Message

Lucas Stach Dec. 22, 2015, 6:41 p.m. UTC
This implements the UHS tuning sequence in a similar way to the one
contained in the TRM. It deviates in the way how to check if the tap
value is passing, by using the common Linux MMC function, which does
not only check for data CRC errors, but also if the received block
pattern is correct.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/mmc/host/sdhci-tegra.c | 55 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

Comments

Jon Hunter March 29, 2016, 4:37 p.m. UTC | #1
Hi Lucas,

On 22/12/15 18:41, Lucas Stach wrote:
> This implements the UHS tuning sequence in a similar way to the one
> contained in the TRM. It deviates in the way how to check if the tap
> value is passing, by using the common Linux MMC function, which does
> not only check for data CRC errors, but also if the received block
> pattern is correct.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>

I have been investigating a hang that occurs randomly during
system-suspend on Tegra124 Jetson TK1 (I have also reproduced the
problem on Tegra124 Nyan Big as well) with -next. The problem does not
occur on every suspend transition but if I execute 100 transitions I
will typically see it at some point. For example ...

count=0; while [ $count -lt 100 ]; do rtcwake -d rtc0 -m mem -s 3; echo
Suspend iteration $count; count=`expr $count + 1`; done

I was able to bisect this problem down to RMK's commit 71fcbda0fcdd
(“mmc: sdhci: fix command response CRC error handling”). However,
looking at RMK's change, I believe that this commit is not the problem
but has exposed a problem with the tegra SDHCI driver.

When the hang occurs I always see the following when starting the
suspend sequence and after which the system continues to suspend but
never exits suspend:

	mmc0: Timeout waiting for hardware interrupt.

The above message is a symptom of RMK's commit and I have found that
when I see this, it always occurs during tegra_sdhci_execute_tuning()
because sometimes we get a CRC error.

I don't fully understand why the hang occurs in suspend, but putting
this aside (as well as the CRC error and timeout), the more I look at
the tuning code, the more it appears incomplete. Unfortunately, the
Tegra TRM alone does not give the complete procedure for performing the
tuning because there can be multiple windows where the tap values may
pass and these windows will vary with voltage/frequency. Plus we need to
ensure that the window is greater than a minimum width of 4 valid tap
values. The 3.18 kernel that is used for chrome-os products with
Tegra has a more complete implementation that is a good reference
(although not that easy to follow without good documentation!) [0].

We would definitely like to get this working in the mainline and I am
not sure if this is something that you are keen to spend time on or not?
Let me know and we can decide what makes most sense here.

Cheers
Jon

[0]
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.10/drivers/mmc/host/sdhci-tegra.c


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter March 29, 2016, 5:12 p.m. UTC | #2
On 29/03/16 17:37, Jon Hunter wrote:
> Hi Lucas,
> 
> On 22/12/15 18:41, Lucas Stach wrote:
>> This implements the UHS tuning sequence in a similar way to the one
>> contained in the TRM. It deviates in the way how to check if the tap
>> value is passing, by using the common Linux MMC function, which does
>> not only check for data CRC errors, but also if the received block
>> pattern is correct.
>>
>> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> 
> I have been investigating a hang that occurs randomly during
> system-suspend on Tegra124 Jetson TK1 (I have also reproduced the
> problem on Tegra124 Nyan Big as well) with -next. The problem does not
> occur on every suspend transition but if I execute 100 transitions I
> will typically see it at some point. For example ...
> 
> count=0; while [ $count -lt 100 ]; do rtcwake -d rtc0 -m mem -s 3; echo
> Suspend iteration $count; count=`expr $count + 1`; done
> 
> I was able to bisect this problem down to RMK's commit 71fcbda0fcdd
> (“mmc: sdhci: fix command response CRC error handling”). However,
> looking at RMK's change, I believe that this commit is not the problem
> but has exposed a problem with the tegra SDHCI driver.
> 
> When the hang occurs I always see the following when starting the
> suspend sequence and after which the system continues to suspend but
> never exits suspend:
> 
> 	mmc0: Timeout waiting for hardware interrupt.
> 
> The above message is a symptom of RMK's commit and I have found that
> when I see this, it always occurs during tegra_sdhci_execute_tuning()
> because sometimes we get a CRC error.

For completeness, when disabling the UHS modes for Tegra, I no longer
see the hang during suspend (because the execute_tuning() is not called).

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 20ce81b..0201549 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -22,6 +22,7 @@ 
 #include <linux/of_device.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
 #include <linux/mmc/slot-gpio.h>
 #include <linux/gpio/consumer.h>
 
@@ -29,6 +30,9 @@ 
 
 /* Tegra SDHOST controller vendor register definitions */
 #define SDHCI_TEGRA_VENDOR_CLOCK_CTRL			0x100
+#define SDHCI_CLOCK_CTRL_TAP_MASK			0x00ff0000
+#define SDHCI_CLOCK_CTRL_TAP_SHIFT			16
+#define SDHCI_CLOCK_CTRL_SDR50_TUNING_OVERRIDE		BIT(5)
 #define SDHCI_CLOCK_CTRL_PADPIPE_CLKEN_OVERRIDE		BIT(3)
 #define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE	BIT(2)
 
@@ -151,6 +155,8 @@  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 
 	clk_ctrl = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
 	clk_ctrl &= ~SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE;
+	if (!(soc_data->nvquirks & NVQUIRK_DISABLE_SDR50))
+		clk_ctrl |= SDHCI_CLOCK_CTRL_SDR50_TUNING_OVERRIDE;
 	sdhci_writel(host, clk_ctrl, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
 
 	tegra_host->ddr_signaling = false;
@@ -214,6 +220,50 @@  static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
 	return clk_round_rate(pltfm_host->clk, UINT_MAX) / 2;
 }
 
+static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
+{
+	u32 reg;
+
+	reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
+	reg &= ~SDHCI_CLOCK_CTRL_TAP_MASK;
+	reg |= tap << SDHCI_CLOCK_CTRL_TAP_SHIFT;
+	sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
+}
+
+static int tegra_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
+{
+	unsigned int min, max;
+
+	/*
+	 * Start search for minimum tap value at 10, as smaller values are
+	 * may wrongly be reported as working but fail at higher speeds,
+	 * according to the TRM.
+	 */
+	min = 10;
+	while (min < 255) {
+		tegra_sdhci_set_tap(host, min);
+		if (!mmc_send_tuning(host->mmc, opcode, NULL))
+			break;
+		min++;
+	}
+
+	/* Find the maximum tap value that still passes. */
+	max = min + 1;
+	while (max < 255) {
+		tegra_sdhci_set_tap(host, max);
+		if (mmc_send_tuning(host->mmc, opcode, NULL)) {
+			max--;
+			break;
+		}
+		max++;
+	}
+
+	/* The TRM states the ideal tap value is at 75% in the passing range. */
+	tegra_sdhci_set_tap(host, min + ((max - min) * 3 / 4));
+
+	return mmc_send_tuning(host->mmc, opcode, NULL);
+}
+
 static const struct sdhci_ops tegra_sdhci_ops = {
 	.get_ro     = tegra_sdhci_get_ro,
 	.read_w     = tegra_sdhci_readw,
@@ -221,6 +271,7 @@  static const struct sdhci_ops tegra_sdhci_ops = {
 	.set_clock  = tegra_sdhci_set_clock,
 	.set_bus_width = tegra_sdhci_set_bus_width,
 	.reset      = tegra_sdhci_reset,
+	.platform_execute_tuning = tegra_sdhci_execute_tuning,
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
 	.get_max_clock = tegra_sdhci_get_max_clock,
 };
@@ -266,6 +317,7 @@  static const struct sdhci_ops tegra114_sdhci_ops = {
 	.set_clock  = tegra_sdhci_set_clock,
 	.set_bus_width = tegra_sdhci_set_bus_width,
 	.reset      = tegra_sdhci_reset,
+	.platform_execute_tuning = tegra_sdhci_execute_tuning,
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
 	.get_max_clock = tegra_sdhci_get_max_clock,
 };
@@ -350,6 +402,9 @@  static int sdhci_tegra_probe(struct platform_device *pdev)
 	if (rc)
 		goto err_parse_dt;
 
+	if (!(tegra_host->soc_data->nvquirks & NVQUIRK_DISABLE_DDR50))
+		host->mmc->caps |= MMC_CAP_1_8V_DDR;
+
 	tegra_host->power_gpio = devm_gpiod_get_optional(&pdev->dev, "power",
 							 GPIOD_OUT_HIGH);
 	if (IS_ERR(tegra_host->power_gpio)) {