Skip to contentSkip to conversation

Tegra SDHCI UHS-I support

PATCH Applied Lucas Stach 5 participants 12 messages Dec 22, 2015 – Mar 29, 2016 2 messages not in the archive

Subsystems (3)
MMC, SD and SDIO Tegra architecture support Secure digital host controller interface (SDHCI) driver
lore
Share & tools
lore

Lucas Stach

author
#0 lore raw

v2 cover Tegra SDHCI UHS-I support

Hi all,

this series implements UHS-I signaling for the Tegra SDHCI host, which mainly means putting a proper tuning sequence in place.

I've tested this on Jetson TK1 and got the following speed results, where mmcblk0 is the on-board eMMC and mmcblk1 is a micro SDXC card:

Without series applied:
hdparm -t /dev/mmcblk0
Timing buffered disk reads: 110 MB in 3.05 seconds = 36.02 MB/sec
hdparm -t /dev/mmcblk1
Timing buffered disk reads: 56 MB in 3.01 seconds = 18.63 MB/sec

With series applied:
hdparm -t /dev/mmcblk0
Timing buffered disk reads: 236 MB in 3.00 seconds = 78.58 MB/sec
hdparm -t /dev/mmcblk1
Timing buffered disk reads: 102 MB in 3.04 seconds = 33.51 MB/sec

Tegra 30 does support UHS-I speeds too, but currently has problems when lowering the card voltage, which is needed in order to switch to UHS-I signaling. I have some more patches to fix this, but they need a bit more cleanup, with them applied the gains on Tegra30 are similar to the results above.

For now the gains are limited to Tegra124+, with no regressions on Tegra30 and Tegra20.

V2 fixes some minor style problems and is rebased on top of mmc/next. This means it enables the same tuning logic on Tegra210 also. I don't have a way to test this myself, so any testing on Tegra210 much appreciated.

Regards,
Lucas

Lucas Stach (5):
mmc: tegra: implement module external clock change
mmc: tegra: disable SPI_MODE_CLKEN
mmc: tegra: implement UHS tuning
mmc: tegra: enable UHS-I modes
mmc: tegra: use correct accessor for misc ctrl register

 drivers/mmc/host/sdhci-tegra.c | 166 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 138 insertions(+), 28 deletions(-)
Signature · 1 line
2.5.0

A message that is not in the archive (sent off-list or not yet received): 1450809664-11360-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org

author
#2 lore raw

PATCH v2 1/5 mmc: tegra: implement module external clock change

Allow the the driver to change the clock supplied from the CAR directly, minimizing the need to divide the clock inside the SDMMC module itself.

This allows for higher clock speeds than the default 48MHz supplied to the module and is a prerequisite to support DDR signaling modes, where the Tegra host needs to be run with a fixed internal divider of 2 for data to be sampled correctly. (Tegra K1 TRM v03p chapter 29.7.1.1)

Also enable the broken preset value quirk as the preset values need to be adapted to the changed clocking. While Tegra114+ allows this through vendor registers, there is no such way for Tegra30. Takes the easy way out and keep things consistent between the different SoC generations by flagging the preset registers as unusable.

Signed-off-by Lucas Stach <[email protected]>
---
 drivers/mmc/host/sdhci-tegra.c | 61 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 54 insertions(+), 7 deletions(-)
drivers/mmc/host/sdhci-tegra.c +54 -7
index 368f1b7..f11db83 100644
@@ -49,6 +49,7 @@ struct sdhci_tegra_soc_data {
 struct sdhci_tegra {
 	const struct sdhci_tegra_soc_data *soc_data;
 	struct gpio_desc *power_gpio;
+	bool ddr_signaling;
 };
 
 static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)

@@ -143,6 +144,8 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 	if (soc_data->nvquirks & NVQUIRK_DISABLE_SDR104)
 		misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR104;
 	sdhci_writew(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+
+	tegra_host->ddr_signaling = false;
 }
 
 static void tegra_sdhci_set_bus_width(struct sdhci_host *host, int bus_width)

@@ -164,15 +167,54 @@ static void tegra_sdhci_set_bus_width(struct sdhci_host *host, int bus_width)
 	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 }
 
+static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = pltfm_host->priv;
+	unsigned long host_clk;
+
+	if (!clock)
+		return;
+
+	host_clk = tegra_host->ddr_signaling ? clock * 2 : clock;
+	clk_set_rate(pltfm_host->clk, host_clk);
+	host->max_clk = clk_get_rate(pltfm_host->clk);
+
+	return sdhci_set_clock(host, clock);
+}
+
+static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
+					  unsigned timing)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = pltfm_host->priv;
+
+	if (timing == MMC_TIMING_UHS_DDR50)
+		tegra_host->ddr_signaling = true;
+
+	return sdhci_set_uhs_signaling(host, timing);
+}
+
+static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	/*
+	 * DDR modes require the host to run at double the card frequency, so
+	 * the maximum rate we can support is half of the module input clock.
+	 */
+	return clk_round_rate(pltfm_host->clk, UINT_MAX) / 2;
+}
+
 static const struct sdhci_ops tegra_sdhci_ops = {
 	.get_ro     = tegra_sdhci_get_ro,
 	.read_w     = tegra_sdhci_readw,
 	.write_l    = tegra_sdhci_writel,
-	.set_clock  = sdhci_set_clock,
+	.set_clock  = tegra_sdhci_set_clock,
 	.set_bus_width = tegra_sdhci_set_bus_width,
 	.reset      = tegra_sdhci_reset,
-	.set_uhs_signaling = sdhci_set_uhs_signaling,
-	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
+	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
+	.get_max_clock = tegra_sdhci_get_max_clock,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra20_pdata = {

@@ -197,6 +239,7 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
 		  SDHCI_QUIRK_NO_HISPD_BIT |
 		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
 		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
 	.ops  = &tegra_sdhci_ops,
 };
 

@@ -212,11 +255,11 @@ static const struct sdhci_ops tegra114_sdhci_ops = {
 	.read_w     = tegra_sdhci_readw,
 	.write_w    = tegra_sdhci_writew,
 	.write_l    = tegra_sdhci_writel,
-	.set_clock  = sdhci_set_clock,
+	.set_clock  = tegra_sdhci_set_clock,
 	.set_bus_width = tegra_sdhci_set_bus_width,
 	.reset      = tegra_sdhci_reset,
-	.set_uhs_signaling = sdhci_set_uhs_signaling,
-	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
+	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
+	.get_max_clock = tegra_sdhci_get_max_clock,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {

@@ -226,6 +269,7 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
 		  SDHCI_QUIRK_NO_HISPD_BIT |
 		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
 		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
 	.ops  = &tegra114_sdhci_ops,
 };
 

@@ -241,7 +285,9 @@ static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
 		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
 		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
 		  SDHCI_QUIRK_NO_HISPD_BIT |
-		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
 	.ops  = &tegra114_sdhci_ops,
 };
 

@@ -288,6 +334,7 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 		rc = -ENOMEM;
 		goto err_alloc_tegra_host;
 	}
+	tegra_host->ddr_signaling = false;
 	tegra_host->soc_data = soc_data;
 	pltfm_host->priv = tegra_host;
Signature · 1 line
2.5.0
author
#3 lore raw

PATCH v2 5/5 mmc: tegra: use correct accessor for misc ctrl register

In reply to a message not in the archive

The misc control register is 32bit wide, the used readw/writew accessors only mainipulate the low 16bit of this register. It currently doesn't matter as all the bit changed are located in the lower half, but together with the u32 variable used to hold the contents of the register it is seriously confusing.

Switch to 32bit accessors to avoid any future breakage.

Signed-off-by Lucas Stach <[email protected]>
---
 drivers/mmc/host/sdhci-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci-tegra.c +2 -2
index 20084f8..b5abbe2 100644
@@ -140,7 +140,7 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 	if (!(mask & SDHCI_RESET_ALL))
 		return;
 
-	misc_ctrl = sdhci_readw(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+	misc_ctrl = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
 	/* Erratum: Enable SDHCI spec v3.00 support */
 	if (soc_data->nvquirks & NVQUIRK_ENABLE_SDHCI_SPEC_300)
 		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300;

@@ -151,7 +151,7 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_DDR50;
 	if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR104)
 		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR104;
-	sdhci_writew(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+	sdhci_writel(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
 
 	clk_ctrl = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
 	clk_ctrl &= ~SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE;
Signature · 1 line
2.5.0
#4 lore raw
In reply to a message not in the archive

Hi Lucas,

Quoted 26 lines · Lucas Stach

Hi all,

this series implements UHS-I signaling for the Tegra SDHCI host, which mainly means putting a proper tuning sequence in place.

I've tested this on Jetson TK1 and got the following speed results, where mmcblk0 is the on-board eMMC and mmcblk1 is a micro SDXC card:

Without series applied:
hdparm -t /dev/mmcblk0
Timing buffered disk reads: 110 MB in 3.05 seconds = 36.02 MB/sec
hdparm -t /dev/mmcblk1
Timing buffered disk reads: 56 MB in 3.01 seconds = 18.63 MB/sec

With series applied:
hdparm -t /dev/mmcblk0
Timing buffered disk reads: 236 MB in 3.00 seconds = 78.58 MB/sec
hdparm -t /dev/mmcblk1
Timing buffered disk reads: 102 MB in 3.04 seconds = 33.51 MB/sec

Tegra 30 does support UHS-I speeds too, but currently has problems when lowering the card voltage, which is needed in order to switch to UHS-I signaling. I have some more patches to fix this, but they need a bit more cleanup, with them applied the gains on Tegra30 are similar to the results above.

Quoted 7 lines

For now the gains are limited to Tegra124+, with no regressions on Tegra30 and Tegra20.

V2 fixes some minor style problems and is rebased on top of mmc/next. This means it enables the same tuning logic on Tegra210 also. I don't have a way to test this myself, so any testing on Tegra210 much appreciated.

I have recently noticed that on my tegra114-dalmore-a04, SD cards are no longer recognised on boot. It appears that the problem started after this series was merged.

On tegra114 I see:

[    1.777006] mmc0: error -110 whilst initialising SD card

If I disable the UHS-I changes then it works again (see below). I am guessing you don't have a tegra114 board to test on? I have not had time to look into this further, but I am not sure if we should disable UHS-I on tegra114 for now?

Cheers
Jon

drivers/mmc/host/sdhci-tegra.c +0 -0
index 83c4bf7bc16c..bc7a0847e316 100644
@@ -147,10 +147,16 @@ static void tegra_sdhci_reset(struct sdhci_host

*host, u8 mask)
/* Advertise UHS modes as supported by host */
if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR50)
misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR50;
+ else
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR50;
if (soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_DDR50;
+ else
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_DDR50;
if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR104)
misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR104;
+ else
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR104;
sdhci_writel(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);

clk_ctrl = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
@@ -315,6 +321,32 @@ static const struct sdhci_ops tegra114_sdhci_ops = {

 	.write_w    = tegra_sdhci_writew,
 	.write_l    = tegra_sdhci_writel,
 	.set_clock  = tegra_sdhci_set_clock,

+ .set_bus_width = sdhci_set_bus_width,
+ .reset = tegra_sdhci_reset,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+ .get_max_clock = sdhci_pltfm_clk_get_max_clock,
+};
+
+static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
+ .quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |

+		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
+		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
+		  SDHCI_QUIRK_NO_HISPD_BIT |
+		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,

+ .ops = &tegra114_sdhci_ops,
+};
+
+static const struct sdhci_tegra_soc_data soc_data_tegra114 = {
+ .pdata = &sdhci_tegra114_pdata,
+};
+
+static const struct sdhci_ops tegra124_sdhci_ops = {

+	.get_ro     = tegra_sdhci_get_ro,
+	.read_w     = tegra_sdhci_readw,
+	.write_w    = tegra_sdhci_writew,
+	.write_l    = tegra_sdhci_writel,
+	.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,
@@ -322,7 +354,7 @@ static const struct sdhci_ops tegra114_sdhci_ops = {
.get_max_clock = tegra_sdhci_get_max_clock,
};

-static const struct sdhci_pltfm_data sdhci_tegra114_pdata = { +static const struct sdhci_pltfm_data sdhci_tegra124_pdata = {
.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | SDHCI_QUIRK_SINGLE_POWER_WRITE |
@@ -330,11 +362,11 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC | SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
- .ops = &tegra114_sdhci_ops,
+ .ops = &tegra124_sdhci_ops,

maintainer
#5 lore raw
Quoted 41 lines · Jon Hunter

Hi Lucas,

Quoted 26 lines · Lucas Stach

Hi all,

this series implements UHS-I signaling for the Tegra SDHCI host, which mainly means putting a proper tuning sequence in place.

I've tested this on Jetson TK1 and got the following speed results, where mmcblk0 is the on-board eMMC and mmcblk1 is a micro SDXC card:

Without series applied:
hdparm -t /dev/mmcblk0
Timing buffered disk reads: 110 MB in 3.05 seconds = 36.02 MB/sec
hdparm -t /dev/mmcblk1
Timing buffered disk reads: 56 MB in 3.01 seconds = 18.63 MB/sec

With series applied:
hdparm -t /dev/mmcblk0
Timing buffered disk reads: 236 MB in 3.00 seconds = 78.58 MB/sec
hdparm -t /dev/mmcblk1
Timing buffered disk reads: 102 MB in 3.04 seconds = 33.51 MB/sec

Tegra 30 does support UHS-I speeds too, but currently has problems when lowering the card voltage, which is needed in order to switch to UHS-I signaling. I have some more patches to fix this, but they need a bit more cleanup, with them applied the gains on Tegra30 are similar to the results above.

Quoted 7 lines

For now the gains are limited to Tegra124+, with no regressions on Tegra30 and Tegra20.

V2 fixes some minor style problems and is rebased on top of mmc/next. This means it enables the same tuning logic on Tegra210 also. I don't have a way to test this myself, so any testing on Tegra210 much appreciated.

I have recently noticed that on my tegra114-dalmore-a04, SD cards are no longer recognised on boot. It appears that the problem started after this series was merged.

Quoted 8 lines

On tegra114 I see:

[    1.777006] mmc0: error -110 whilst initialising SD card

If I disable the UHS-I changes then it works again (see below). I am guessing you don't have a tegra114 board to test on? I have not had time to look into this further, but I am not sure if we should disable UHS-I on tegra114 for now?

The diff got truncated somewhere along the lines, it should have been:

drivers/mmc/host/sdhci-tegra.c +37 -5
index 83c4bf7bc16c..bc7a0847e316 100644
@@ -147,10 +147,16 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 	/* Advertise UHS modes as supported by host */
 	if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR50)
 		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR50;
+	else
+		misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR50;
 	if (soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
 		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_DDR50;
+	else
+		misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_DDR50;
 	if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR104)
 		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR104;
+	else
+		misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR104;
 	sdhci_writel(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
 
 	clk_ctrl = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);

@@ -315,6 +321,32 @@ static const struct sdhci_ops tegra114_sdhci_ops = {
 	.write_w    = tegra_sdhci_writew,
 	.write_l    = tegra_sdhci_writel,
 	.set_clock  = tegra_sdhci_set_clock,
+	.set_bus_width = sdhci_set_bus_width,
+	.reset      = tegra_sdhci_reset,
+	.set_uhs_signaling = sdhci_set_uhs_signaling,
+	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
+};
+
+static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
+	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
+		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
+		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
+		  SDHCI_QUIRK_NO_HISPD_BIT |
+		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.ops  = &tegra114_sdhci_ops,
+};
+
+static const struct sdhci_tegra_soc_data soc_data_tegra114 = {
+	.pdata = &sdhci_tegra114_pdata,
+};
+
+static const struct sdhci_ops tegra124_sdhci_ops = {
+	.get_ro     = tegra_sdhci_get_ro,
+	.read_w     = tegra_sdhci_readw,
+	.write_w    = tegra_sdhci_writew,
+	.write_l    = tegra_sdhci_writel,
+	.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,

@@ -322,7 +354,7 @@ static const struct sdhci_ops tegra114_sdhci_ops = {
 	.get_max_clock = tegra_sdhci_get_max_clock,
 };
 
-static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
+static const struct sdhci_pltfm_data sdhci_tegra124_pdata = {
 	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
 		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
 		  SDHCI_QUIRK_SINGLE_POWER_WRITE |

@@ -330,11 +362,11 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
 		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
 		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
 	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
-	.ops  = &tegra114_sdhci_ops,
+	.ops  = &tegra124_sdhci_ops,
 };
 
-static const struct sdhci_tegra_soc_data soc_data_tegra114 = {
-	.pdata = &sdhci_tegra114_pdata,
+static const struct sdhci_tegra_soc_data soc_data_tegra124 = {
+	.pdata = &sdhci_tegra124_pdata,
 	.nvquirks = NVQUIRK_ENABLE_SDR50 |
 		    NVQUIRK_ENABLE_DDR50 |
 		    NVQUIRK_ENABLE_SDR104,

@@ -357,7 +389,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
 
 static const struct of_device_id sdhci_tegra_dt_match[] = {
 	{ .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
-	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra114 },
+	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra124 },
 	{ .compatible = "nvidia,tegra114-sdhci", .data = &soc_data_tegra114 },
 	{ .compatible = "nvidia,tegra30-sdhci", .data = &soc_data_tegra30 },
 	{ .compatible = "nvidia,tegra20-sdhci", .data = &soc_data_tegra20 },
#6 lore raw
In reply to Jon Hunter

Hi Jon,

Am Mittwoch, den 17.02.2016, 13:19 +0000 schrieb Jon Hunter:

Quoted 60 lines

Hi Lucas,

Quoted 34 lines · Lucas Stach

Hi all,

this series implements UHS-I signaling for the Tegra SDHCI host, which mainly means putting a proper tuning sequence in place.

I've tested this on Jetson TK1 and got the following speed results, where mmcblk0 is the on-board eMMC and mmcblk1 is a micro SDXC card:

Without series applied:
hdparm -t /dev/mmcblk0
 Timing buffered disk reads:  110 MB in  3.05 seconds =  36.02 MB/sec
hdparm -t /dev/mmcblk1
 Timing buffered disk reads:   56 MB in  3.01 seconds =  18.63 MB/sec

With series applied:
hdparm -t /dev/mmcblk0
 Timing buffered disk reads:  236 MB in  3.00 seconds =  78.58 MB/sec
hdparm -t /dev/mmcblk1
 Timing buffered disk reads:  102 MB in  3.04 seconds =  33.51 MB/sec

Tegra 30 does support UHS-I speeds too, but currently has problems when lowering the card voltage, which is needed in order to switch to UHS-I signaling. I have some more patches to fix this, but they need a bit more cleanup, with them applied the gains on Tegra30 are similar to the results above.

For now the gains are limited to Tegra124+, with no regressions on Tegra30 and Tegra20.

Quoted 6 lines

V2 fixes some minor style problems and is rebased on top of mmc/next.
This means it enables the same tuning logic on Tegra210 also. I don't have a way to test this myself, so any testing on Tegra210 much
appreciated.

I have recently noticed that on my tegra114-dalmore-a04, SD cards are no
longer recognised on boot. It appears that the problem started after this series was merged.

On tegra114 I see:

[    1.777006] mmc0: error -110 whilst initialising SD card

If I disable the UHS-I changes then it works again (see below). I am guessing you don't have a tegra114 board to test on? I have not had time
to look into this further, but I am not sure if we should disable UHS-I
on tegra114 for now?

Quoted 3 lines

Cheers
Jon

Right I don't have a Tegra114 board to test with.

I already know that Tegra30 needs some more fixes to get UHS-I to work and I have patches for that, but they are not yet ready for sending out. I would guess that Tegra114 may need the same fix as Tegra30. I can Cc you on those patches if you prefer.

Please submit a formal patch to disable UHS-I on Tegra114, I think it's the right thing to do at this point and will happily ACK it.

Regards,
Lucas

#7 lore raw

PATCH v2 2/5 mmc: tegra: disable SPI_MODE_CLKEN

In reply to Lucas Stach

The Tegra30 and up TRM states that this bit should always be programmed to 0 by driver software.

Signed-off-by Lucas Stach <[email protected]>
---
 drivers/mmc/host/sdhci-tegra.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/mmc/host/sdhci-tegra.c +9 -1
index f11db83..20ce81b 100644
@@ -28,6 +28,10 @@
 #include "sdhci-pltfm.h"
 
 /* Tegra SDHOST controller vendor register definitions */
+#define SDHCI_TEGRA_VENDOR_CLOCK_CTRL			0x100
+#define SDHCI_CLOCK_CTRL_PADPIPE_CLKEN_OVERRIDE		BIT(3)
+#define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE	BIT(2)
+
 #define SDHCI_TEGRA_VENDOR_MISC_CTRL		0x120
 #define SDHCI_MISC_CTRL_ENABLE_SDR104		0x8
 #define SDHCI_MISC_CTRL_ENABLE_SDR50		0x10

@@ -125,7 +129,7 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_tegra *tegra_host = pltfm_host->priv;
 	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
-	u32 misc_ctrl;
+	u32 misc_ctrl, clk_ctrl;
 
 	sdhci_reset(host, mask);
 

@@ -145,6 +149,10 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 		misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR104;
 	sdhci_writew(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
 
+	clk_ctrl = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
+	clk_ctrl &= ~SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE;
+	sdhci_writel(host, clk_ctrl, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
+
 	tegra_host->ddr_signaling = false;
 }
Signature · 1 line
2.5.0
#8 lore raw

PATCH v2 3/5 mmc: tegra: implement UHS tuning

In reply to Lucas Stach

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 <[email protected]>
---
 drivers/mmc/host/sdhci-tegra.c | 55 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
drivers/mmc/host/sdhci-tegra.c +55 -0
index 20ce81b..0201549 100644
@@ -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)) {
Signature · 1 line
2.5.0
maintainer
#9 lore raw

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

Hi Lucas,

Quoted 7 lines · Lucas Stach

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 <[email protected]>

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#11 lore raw

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

Quoted 29 lines · Jon Hunter

Hi Lucas,

Quoted 7 lines · Lucas Stach

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 <[email protected]>

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:

Quoted 5 lines

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

#12 lore raw

PATCH v2 4/5 mmc: tegra: enable UHS-I modes

In reply to Lucas Stach

Keep the quirk bits, as Tegra30 and Tegra114 host have different levels of support for UHS-I modes and so need different spare bits to be set, but change the logic to be positive.

Signed-off-by Lucas Stach <[email protected]>
---
 drivers/mmc/host/sdhci-tegra.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)
drivers/mmc/host/sdhci-tegra.c +20 -20
index 0201549..20084f8 100644
@@ -45,9 +45,9 @@
 #define NVQUIRK_FORCE_SDHCI_SPEC_200	BIT(0)
 #define NVQUIRK_ENABLE_BLOCK_GAP_DET	BIT(1)
 #define NVQUIRK_ENABLE_SDHCI_SPEC_300	BIT(2)
-#define NVQUIRK_DISABLE_SDR50		BIT(3)
-#define NVQUIRK_DISABLE_SDR104		BIT(4)
-#define NVQUIRK_DISABLE_DDR50		BIT(5)
+#define NVQUIRK_ENABLE_SDR50		BIT(3)
+#define NVQUIRK_ENABLE_SDR104		BIT(4)
+#define NVQUIRK_ENABLE_DDR50		BIT(5)
 
 struct sdhci_tegra_soc_data {
 	const struct sdhci_pltfm_data *pdata;

@@ -144,18 +144,18 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 	/* Erratum: Enable SDHCI spec v3.00 support */
 	if (soc_data->nvquirks & NVQUIRK_ENABLE_SDHCI_SPEC_300)
 		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300;
-	/* Don't advertise UHS modes which aren't supported yet */
-	if (soc_data->nvquirks & NVQUIRK_DISABLE_SDR50)
-		misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR50;
-	if (soc_data->nvquirks & NVQUIRK_DISABLE_DDR50)
-		misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_DDR50;
-	if (soc_data->nvquirks & NVQUIRK_DISABLE_SDR104)
-		misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR104;
+	/* Advertise UHS modes as supported by host */
+	if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR50)
+		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR50;
+	if (soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
+		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_DDR50;
+	if (soc_data->nvquirks & NVQUIRK_ENABLE_SDR104)
+		misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDR104;
 	sdhci_writew(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
 
 	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))
+	if (soc_data->nvquirks & SDHCI_MISC_CTRL_ENABLE_SDR50)
 		clk_ctrl |= SDHCI_CLOCK_CTRL_SDR50_TUNING_OVERRIDE;
 	sdhci_writel(host, clk_ctrl, SDHCI_TEGRA_VENDOR_CLOCK_CTRL);
 

@@ -305,8 +305,8 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
 static const struct sdhci_tegra_soc_data soc_data_tegra30 = {
 	.pdata = &sdhci_tegra30_pdata,
 	.nvquirks = NVQUIRK_ENABLE_SDHCI_SPEC_300 |
-		    NVQUIRK_DISABLE_SDR50 |
-		    NVQUIRK_DISABLE_SDR104,
+		    NVQUIRK_ENABLE_SDR50 |
+		    NVQUIRK_ENABLE_SDR104,
 };
 
 static const struct sdhci_ops tegra114_sdhci_ops = {

@@ -335,9 +335,9 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
 
 static const struct sdhci_tegra_soc_data soc_data_tegra114 = {
 	.pdata = &sdhci_tegra114_pdata,
-	.nvquirks = NVQUIRK_DISABLE_SDR50 |
-		    NVQUIRK_DISABLE_DDR50 |
-		    NVQUIRK_DISABLE_SDR104,
+	.nvquirks = NVQUIRK_ENABLE_SDR50 |
+		    NVQUIRK_ENABLE_DDR50 |
+		    NVQUIRK_ENABLE_SDR104,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {

@@ -353,9 +353,9 @@ static const struct sdhci_pltfm_data sdhci_tegra210_pdata = {
 
 static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
 	.pdata = &sdhci_tegra210_pdata,
-	.nvquirks = NVQUIRK_DISABLE_SDR50 |
-		    NVQUIRK_DISABLE_DDR50 |
-		    NVQUIRK_DISABLE_SDR104,
+	.nvquirks = NVQUIRK_ENABLE_SDR50 |
+		    NVQUIRK_ENABLE_DDR50 |
+		    NVQUIRK_ENABLE_SDR104,
 };
 
 static const struct of_device_id sdhci_tegra_dt_match[] = {

@@ -402,7 +402,7 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	if (rc)
 		goto err_parse_dt;
 
-	if (!(tegra_host->soc_data->nvquirks & NVQUIRK_DISABLE_DDR50))
+	if (tegra_host->soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
 		host->mmc->caps |= MMC_CAP_1_8V_DDR;
 
 	tegra_host->power_gpio = devm_gpiod_get_optional(&pdev->dev, "power",
Signature · 1 line
2.5.0
Applied
#13 lore raw
In reply to Lucas Stach
Quoted 44 lines · Lucas Stach

Hi all,

this series implements UHS-I signaling for the Tegra SDHCI host, which mainly means putting a proper tuning sequence in place.

I've tested this on Jetson TK1 and got the following speed results, where mmcblk0 is the on-board eMMC and mmcblk1 is a micro SDXC card:

Without series applied:
hdparm -t /dev/mmcblk0
Timing buffered disk reads: 110 MB in 3.05 seconds = 36.02 MB/sec
hdparm -t /dev/mmcblk1
Timing buffered disk reads: 56 MB in 3.01 seconds = 18.63 MB/sec

With series applied:
hdparm -t /dev/mmcblk0
Timing buffered disk reads: 236 MB in 3.00 seconds = 78.58 MB/sec
hdparm -t /dev/mmcblk1
Timing buffered disk reads: 102 MB in 3.04 seconds = 33.51 MB/sec

Tegra 30 does support UHS-I speeds too, but currently has problems when lowering the card voltage, which is needed in order to switch to UHS-I signaling. I have some more patches to fix this, but they need a bit more cleanup, with them applied the gains on Tegra30 are similar to the results above.

For now the gains are limited to Tegra124+, with no regressions on Tegra30 and Tegra20.

V2 fixes some minor style problems and is rebased on top of mmc/next. This means it enables the same tuning logic on Tegra210 also. I don't have a way to test this myself, so any testing on Tegra210 much appreciated.

Regards,
Lucas

Lucas Stach (5):
mmc: tegra: implement module external clock change
mmc: tegra: disable SPI_MODE_CLKEN
mmc: tegra: implement UHS tuning
mmc: tegra: enable UHS-I modes
mmc: tegra: use correct accessor for misc ctrl register

Quoted 3 lines
 drivers/mmc/host/sdhci-tegra.c | 166 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 138 insertions(+), 28 deletions(-)

Thanks, applied for next!

Kind regards
Uffe