All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Tegra SDHCI UHS-I support
@ 2015-12-22 18:40 Lucas Stach
       [not found] ` <1450809664-11360-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Lucas Stach @ 2015-12-22 18:40 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

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(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/5] mmc: tegra: implement module external clock change
       [not found] ` <1450809664-11360-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
@ 2015-12-22 18:41   ` Lucas Stach
  2015-12-22 18:41   ` [PATCH v2 5/5] mmc: tegra: use correct accessor for misc ctrl register Lucas Stach
  2016-02-17 13:19   ` [PATCH v2 0/5] Tegra SDHCI UHS-I support Jon Hunter
  2 siblings, 0 replies; 12+ messages in thread
From: Lucas Stach @ 2015-12-22 18:41 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

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 <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/mmc/host/sdhci-tegra.c | 61 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 54 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 368f1b7..f11db83 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -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;
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/5] mmc: tegra: disable SPI_MODE_CLKEN
  2015-12-22 18:40 [PATCH v2 0/5] Tegra SDHCI UHS-I support Lucas Stach
       [not found] ` <1450809664-11360-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
@ 2015-12-22 18:41 ` Lucas Stach
  2015-12-22 18:41 ` [PATCH v2 3/5] mmc: tegra: implement UHS tuning Lucas Stach
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Lucas Stach @ 2015-12-22 18:41 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, linux-mmc,
	linux-tegra

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

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

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index f11db83..20ce81b 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -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;
 }
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/5] mmc: tegra: implement UHS tuning
  2015-12-22 18:40 [PATCH v2 0/5] Tegra SDHCI UHS-I support Lucas Stach
       [not found] ` <1450809664-11360-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
  2015-12-22 18:41 ` [PATCH v2 2/5] mmc: tegra: disable SPI_MODE_CLKEN Lucas Stach
@ 2015-12-22 18:41 ` Lucas Stach
  2016-03-29 16:37   ` Jon Hunter
  2015-12-22 18:41 ` [PATCH v2 4/5] mmc: tegra: enable UHS-I modes Lucas Stach
  2015-12-28 13:18 ` [PATCH v2 0/5] Tegra SDHCI UHS-I support Ulf Hansson
  4 siblings, 1 reply; 12+ messages in thread
From: Lucas Stach @ 2015-12-22 18:41 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, linux-mmc,
	linux-tegra

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(+)

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)) {
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/5] mmc: tegra: enable UHS-I modes
  2015-12-22 18:40 [PATCH v2 0/5] Tegra SDHCI UHS-I support Lucas Stach
                   ` (2 preceding siblings ...)
  2015-12-22 18:41 ` [PATCH v2 3/5] mmc: tegra: implement UHS tuning Lucas Stach
@ 2015-12-22 18:41 ` Lucas Stach
  2015-12-28 13:18 ` [PATCH v2 0/5] Tegra SDHCI UHS-I support Ulf Hansson
  4 siblings, 0 replies; 12+ messages in thread
From: Lucas Stach @ 2015-12-22 18:41 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, linux-mmc,
	linux-tegra

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 <dev@lynxeye.de>
---
 drivers/mmc/host/sdhci-tegra.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 0201549..20084f8 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -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",
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/5] mmc: tegra: use correct accessor for misc ctrl register
       [not found] ` <1450809664-11360-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
  2015-12-22 18:41   ` [PATCH v2 1/5] mmc: tegra: implement module external clock change Lucas Stach
@ 2015-12-22 18:41   ` Lucas Stach
  2016-02-17 13:19   ` [PATCH v2 0/5] Tegra SDHCI UHS-I support Jon Hunter
  2 siblings, 0 replies; 12+ messages in thread
From: Lucas Stach @ 2015-12-22 18:41 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

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 <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/mmc/host/sdhci-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 20084f8..b5abbe2 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -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;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/5] Tegra SDHCI UHS-I support
  2015-12-22 18:40 [PATCH v2 0/5] Tegra SDHCI UHS-I support Lucas Stach
                   ` (3 preceding siblings ...)
  2015-12-22 18:41 ` [PATCH v2 4/5] mmc: tegra: enable UHS-I modes Lucas Stach
@ 2015-12-28 13:18 ` Ulf Hansson
  4 siblings, 0 replies; 12+ messages in thread
From: Ulf Hansson @ 2015-12-28 13:18 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, linux-mmc,
	linux-tegra

On 22 December 2015 at 19:40, Lucas Stach <dev@lynxeye.de> wrote:
> 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(-)
>

Thanks, applied for next!

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/5] Tegra SDHCI UHS-I support
       [not found] ` <1450809664-11360-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
  2015-12-22 18:41   ` [PATCH v2 1/5] mmc: tegra: implement module external clock change Lucas Stach
  2015-12-22 18:41   ` [PATCH v2 5/5] mmc: tegra: use correct accessor for misc ctrl register Lucas Stach
@ 2016-02-17 13:19   ` Jon Hunter
  2016-02-17 13:55     ` Jon Hunter
  2016-02-17 19:52     ` Lucas Stach
  2 siblings, 2 replies; 12+ messages in thread
From: Jon Hunter @ 2016-02-17 13:19 UTC (permalink / raw)
  To: Lucas Stach, Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

Hi Lucas,

On 22/12/15 18:40, Lucas Stach wrote:
> 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.

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

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 83c4bf7bc16c..bc7a0847e316 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -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,

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/5] Tegra SDHCI UHS-I support
  2016-02-17 13:19   ` [PATCH v2 0/5] Tegra SDHCI UHS-I support Jon Hunter
@ 2016-02-17 13:55     ` Jon Hunter
  2016-02-17 19:52     ` Lucas Stach
  1 sibling, 0 replies; 12+ messages in thread
From: Jon Hunter @ 2016-02-17 13:55 UTC (permalink / raw)
  To: Lucas Stach, Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, linux-mmc,
	linux-tegra


On 17/02/16 13:19, Jon Hunter wrote:
> Hi Lucas,
> 
> On 22/12/15 18:40, Lucas Stach wrote:
>> 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.
> 
> 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?

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

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 83c4bf7bc16c..bc7a0847e316 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -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 },


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/5] Tegra SDHCI UHS-I support
  2016-02-17 13:19   ` [PATCH v2 0/5] Tegra SDHCI UHS-I support Jon Hunter
  2016-02-17 13:55     ` Jon Hunter
@ 2016-02-17 19:52     ` Lucas Stach
  1 sibling, 0 replies; 12+ messages in thread
From: Lucas Stach @ 2016-02-17 19:52 UTC (permalink / raw)
  To: Jon Hunter, Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, linux-mmc,
	linux-tegra

Hi Jon,

Am Mittwoch, den 17.02.2016, 13:19 +0000 schrieb Jon Hunter:
> Hi Lucas,
> 
> On 22/12/15 18:40, Lucas Stach wrote:
> > 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.
> 
> 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
> 
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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/5] mmc: tegra: implement UHS tuning
  2015-12-22 18:41 ` [PATCH v2 3/5] mmc: tegra: implement UHS tuning Lucas Stach
@ 2016-03-29 16:37   ` Jon Hunter
       [not found]     ` <56FAAF59.8080501-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Jon Hunter @ 2016-03-29 16:37 UTC (permalink / raw)
  To: Lucas Stach, Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot, linux-mmc,
	linux-tegra

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/5] mmc: tegra: implement UHS tuning
       [not found]     ` <56FAAF59.8080501-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2016-03-29 17:12       ` Jon Hunter
  0 siblings, 0 replies; 12+ messages in thread
From: Jon Hunter @ 2016-03-29 17:12 UTC (permalink / raw)
  To: Lucas Stach, Ulf Hansson
  Cc: Stephen Warren, Thierry Reding, Alexandre Courbot,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA


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-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
> 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-03-29 17:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-22 18:40 [PATCH v2 0/5] Tegra SDHCI UHS-I support Lucas Stach
     [not found] ` <1450809664-11360-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
2015-12-22 18:41   ` [PATCH v2 1/5] mmc: tegra: implement module external clock change Lucas Stach
2015-12-22 18:41   ` [PATCH v2 5/5] mmc: tegra: use correct accessor for misc ctrl register Lucas Stach
2016-02-17 13:19   ` [PATCH v2 0/5] Tegra SDHCI UHS-I support Jon Hunter
2016-02-17 13:55     ` Jon Hunter
2016-02-17 19:52     ` Lucas Stach
2015-12-22 18:41 ` [PATCH v2 2/5] mmc: tegra: disable SPI_MODE_CLKEN Lucas Stach
2015-12-22 18:41 ` [PATCH v2 3/5] mmc: tegra: implement UHS tuning Lucas Stach
2016-03-29 16:37   ` Jon Hunter
     [not found]     ` <56FAAF59.8080501-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-03-29 17:12       ` Jon Hunter
2015-12-22 18:41 ` [PATCH v2 4/5] mmc: tegra: enable UHS-I modes Lucas Stach
2015-12-28 13:18 ` [PATCH v2 0/5] Tegra SDHCI UHS-I support Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.