| Message ID | 1450809664-11360-4-git-send-email-dev@lynxeye.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)) {
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(+)