[PATCH 0/5] Tegra SDHCI UHS-I support

[PATCH 0/5] Tegra SDHCI UHS-I support

[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.

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 | 153 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 129 insertions(+), 24 deletions(-)

-- 
2.5.0

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

[Lucas Stach]

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 | 56 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index ad28b49..877daf5 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,53 @@ 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 +238,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 +254,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 +268,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,
 };
 
@@ -271,6 +314,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

[PATCH 2/5] mmc: tegra: disable SPI_MODE_CLKEN

[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 <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 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 877daf5..29c4753 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

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

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

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 29c4753..fd0529c 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;
@@ -213,6 +219,48 @@ 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 = 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,
@@ -220,6 +268,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,
 };
@@ -265,6 +314,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,
 };
@@ -330,6 +380,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

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

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

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index fd0529c..b5374d7 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);
 
@@ -302,8 +302,8 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
 static 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 = {
@@ -332,9 +332,9 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
 
 static 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 of_device_id sdhci_tegra_dt_match[] = {
@@ -380,7 +380,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

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

[Lucas Stach]

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@lynxeye.de>
---
 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 b5374d7..5df8166 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

Re: [PATCH 4/5] mmc: tegra: enable UHS-I modes

[Ulf Hansson]

On 19 December 2015 at 20:16, Lucas Stach <dev@lynxeye.de> wrote:
> 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 | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index fd0529c..b5374d7 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);
>
> @@ -302,8 +302,8 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
>  static 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 = {
> @@ -332,9 +332,9 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
>
>  static 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,
>  };

Pleas re-base the patchset, towards my next branch.

Tegra210 support has been queued for 4.5, which means there are
additional occurrences of  NVQUIRK_DISABLE_SDR50,
NVQUIRK_DISABLE_SDR104 etc.

>
>  static const struct of_device_id sdhci_tegra_dt_match[] = {
> @@ -380,7 +380,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
>

I would also appreciate if you checkpatch, before posting the new
version. Some of the patches has some warnings that I think you should
fix.

Besides the above minor things, the patchset looks good to me!

Kind regards
Uffe