[PATCH v2 00/10] Tegra SDHCI enable 1.8 V signaling on Tegar210 and Tegra186

[PATCH v2 00/10] Tegra SDHCI enable 1.8 V signaling on Tegar210 and Tegra186

[Aapo Vienamo]

Hi all,

Reconfigure pad voltages as part of mmc voltage switching on
controllers with adjustable voltages. Allow for reconfiguration of the
signaling voltage of SDMMC1 on Tegra210 P2597 and fix SDMMC4 signaling
voltage regulator configuration on Tegra210 P2180.

This series depends on the "Tegra PMC pinctrl pad configuration" series posted
earlier.

Changelog:
v2:
	- Change the pinctrl bindings commit title
	- Use IS_ERR in tegra_sdhci_init_pinctrl_info()
	- Add nvidia,only-1-8-v property
	- Disable UHS modes in case the pad and regulator configuration
	  is invalid

Aapo Vienamo (10):
  dt-bindings: mmc: tegra: Add pad voltage control properties
  dt-bindings: mmc: tegra: Add nvidia,only-1-8-v property
  mmc: tegra: Reconfigure pad voltages during voltage switching
  arm64: dts: Add Tegra210 sdmmc pinctrl voltage states
  arm64: dts: Add Tegra186 sdmmc pinctrl voltage states
  arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V
  arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply
  arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1
  arm64: dts: tegra210: Add nvidia,only-1-8-v to sdmmc4
  arm64: dts: tegra186: Add nvidia,only-1-8-v to sdmmc4

 .../bindings/mmc/nvidia,tegra20-sdhci.txt          |  24 ++++
 arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  41 ++++++
 arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi     |  12 +-
 arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |   1 -
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  28 +++++
 drivers/mmc/host/sdhci-tegra.c                     | 140 +++++++++++++++++++--
 6 files changed, 222 insertions(+), 24 deletions(-)

-- 
2.7.4

[PATCH v2 01/10] dt-bindings: mmc: tegra: Add pad voltage control properties

[Aapo Vienamo]

Document the pinctrl bindings used by the SDHCI driver to reconfigure
pad voltages on controllers supporting multiple voltage levels.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 .../bindings/mmc/nvidia,tegra20-sdhci.txt          | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
index 9bce578..90c214d 100644
--- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
@@ -38,3 +38,25 @@ sdhci@c8000200 {
 	power-gpios = <&gpio 155 0>; /* gpio PT3 */
 	bus-width = <8>;
 };
+
+Optional properties for Tegra210 and Tegra186:
+- pinctrl-names, pinctrl-0, pinctrl-1 : Specify pad voltage
+  configurations. Valid pinctrl-names are "sdmmc-3v3" and "sdmmc-1v8"
+  for controllers supporting multiple voltage levels. The order of names
+  should correspond to the pin configuration states in pinctrl-0 and
+  pinctrl-1.
+
+Example:
+sdhci@700b0000 {
+	compatible = "nvidia,tegra210-sdhci", "nvidia,tegra124-sdhci";
+	reg = <0x0 0x700b0000 0x0 0x200>;
+	interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&tegra_car TEGRA210_CLK_SDMMC1>;
+	clock-names = "sdhci";
+	resets = <&tegra_car 14>;
+	reset-names = "sdhci";
+	pinctrl-names = "sdmmc-3v3", "sdmmc-1v8";
+	pinctrl-0 = <&sdmmc1_3v3>;
+	pinctrl-1 = <&sdmmc1_1v8>;
+	status = "disabled";
+};
-- 
2.7.4

[PATCH v2 02/10] dt-bindings: mmc: tegra: Add nvidia,only-1-8-v property

[Aapo Vienamo]

Add a property to mark controllers which operate at a 1.8 V fixed I/O
voltage.

This feature of the hardware needs to be signaled this way because it
cannot be probed at runtime or reliably derived from other properties.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
---
 Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
index 90c214d..95010cf 100644
--- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
@@ -45,6 +45,8 @@ Optional properties for Tegra210 and Tegra186:
   for controllers supporting multiple voltage levels. The order of names
   should correspond to the pin configuration states in pinctrl-0 and
   pinctrl-1.
+- nvidia,only-1-8-v : The presence of this property indicates that the
+  controller operates at a 1.8 V fixed I/O voltage.
 
 Example:
 sdhci@700b0000 {
-- 
2.7.4

[PATCH v2 03/10] mmc: tegra: Reconfigure pad voltages during voltage switching

[Aapo Vienamo]

Parse the pinctrl state and nvidia,only-1-8-v properties from the device
tree and implement pad voltage state reconfiguration in the mmc
start_signal_voltage_switch() callback. Validate the pinctrl and
regulator configuration before unmasking UHS modes. Add
NVQUIRK_NEEDS_PAD_CONTROL and add set it for Tegra210 and Tegra186.

The pad configuration is done in the mmc callback because the order of
pad reconfiguration and sdhci voltage switch depend on the voltage to
which the transition occurs.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 140 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 127 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index ddf00166..9587365 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/reset.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
@@ -55,6 +56,7 @@
 #define NVQUIRK_ENABLE_SDR104		BIT(4)
 #define NVQUIRK_ENABLE_DDR50		BIT(5)
 #define NVQUIRK_HAS_PADCALIB		BIT(6)
+#define NVQUIRK_NEEDS_PAD_CONTROL	BIT(7)
 
 struct sdhci_tegra_soc_data {
 	const struct sdhci_pltfm_data *pdata;
@@ -66,8 +68,13 @@ struct sdhci_tegra {
 	struct gpio_desc *power_gpio;
 	bool ddr_signaling;
 	bool pad_calib_required;
+	bool pad_control_available;
+	bool only_1v8;
 
 	struct reset_control *rst;
+	struct pinctrl *pinctrl_sdmmc;
+	struct pinctrl_state *pinctrl_state_3v3;
+	struct pinctrl_state *pinctrl_state_1v8;
 };
 
 static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
@@ -138,12 +145,47 @@ static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host)
 	return mmc_gpio_get_ro(host->mmc);
 }
 
+static bool tegra_sdhci_is_uhs_valid(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
+
+	/*
+	 * The 1.8 V only host controllers don't need to have configurable
+	 * regulators and pad voltages. In this case the UHS modes can be
+	 * enabled regardless.
+	 */
+	if (tegra_host->only_1v8)
+		return true;
+
+	/*
+	 * If the board does not define a regulator for the SDHCI IO voltage,
+	 * then don't advertise support for UHS modes even if the device
+	 * supports it because the IO voltage cannot be configured.
+	 */
+	if (IS_ERR(host->mmc->supply.vqmmc))
+		return false;
+
+	/*
+	 * Later SoC generations require software pad voltage configuration.
+	 * The UHS modes should only be enabled if the pad configuration states
+	 * are available on platforms where they are required in order to switch
+	 * the signaling voltage.
+	 */
+	if (soc_data->nvquirks & NVQUIRK_NEEDS_PAD_CONTROL)
+		return tegra_host->pad_control_available;
+
+	return false;
+}
+
 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 = sdhci_pltfm_priv(pltfm_host);
 	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
 	u32 misc_ctrl, clk_ctrl;
+	bool uhs_valid;
 
 	sdhci_reset(host, mask);
 
@@ -160,13 +202,8 @@ static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 
 	clk_ctrl &= ~SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE;
 
-	/*
-	 * If the board does not define a regulator for the SDHCI
-	 * IO voltage, then don't advertise support for UHS modes
-	 * even if the device supports it because the IO voltage
-	 * cannot be configured.
-	 */
-	if (!IS_ERR(host->mmc->supply.vqmmc)) {
+	uhs_valid = tegra_sdhci_is_uhs_valid(host);
+	if (uhs_valid) {
 		/* 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;
@@ -286,14 +323,80 @@ static int tegra_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 	return mmc_send_tuning(host->mmc, opcode, NULL);
 }
 
-static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
+static int tegra_sdhci_set_padctrl(struct sdhci_host *host, int voltage)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
-	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
+	int ret;
+
+	if (!tegra_host->pad_control_available)
+		return 0;
+
+	if (voltage == MMC_SIGNAL_VOLTAGE_180) {
+		ret = pinctrl_select_state(tegra_host->pinctrl_sdmmc,
+					   tegra_host->pinctrl_state_1v8);
+		if (ret < 0)
+			dev_err(mmc_dev(host->mmc),
+				"setting 1.8V failed, ret: %d\n", ret);
+	} else {
+		ret = pinctrl_select_state(tegra_host->pinctrl_sdmmc,
+					   tegra_host->pinctrl_state_3v3);
+		if (ret < 0)
+			dev_err(mmc_dev(host->mmc),
+				"setting 3.3V failed, ret: %d\n", ret);
+	}
 
-	if (soc_data->nvquirks & NVQUIRK_HAS_PADCALIB)
-		tegra_host->pad_calib_required = true;
+	return ret;
+}
+
+static int sdhci_tegra_start_signal_voltage_switch(struct mmc_host *mmc,
+						   struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	int ret = 0;
+
+	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
+		ret = tegra_sdhci_set_padctrl(host, ios->signal_voltage);
+		if (ret < 0)
+			return ret;
+		ret = sdhci_start_signal_voltage_switch(mmc, ios);
+	} else if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
+		ret = sdhci_start_signal_voltage_switch(mmc, ios);
+		if (ret < 0)
+			return ret;
+		ret = tegra_sdhci_set_padctrl(host, ios->signal_voltage);
+	}
+
+	return ret;
+}
+
+static void tegra_sdhci_init_pinctrl_info(struct device *dev,
+					  struct sdhci_tegra *tegra_host)
+{
+	tegra_host->pinctrl_sdmmc = devm_pinctrl_get(dev);
+	if (IS_ERR(tegra_host->pinctrl_sdmmc)) {
+		dev_dbg(dev, "No pinctrl info, err: %ld\n",
+			PTR_ERR(tegra_host->pinctrl_sdmmc));
+		return;
+	}
+
+	tegra_host->pinctrl_state_3v3 =
+		pinctrl_lookup_state(tegra_host->pinctrl_sdmmc, "sdmmc-3v3");
+	if (IS_ERR(tegra_host->pinctrl_state_3v3)) {
+		dev_err(dev, "Missing 3.3V pad state, err: %ld\n",
+			PTR_ERR(tegra_host->pinctrl_state_3v3));
+		return;
+	}
+
+	tegra_host->pinctrl_state_1v8 =
+		pinctrl_lookup_state(tegra_host->pinctrl_sdmmc, "sdmmc-1v8");
+	if (IS_ERR(tegra_host->pinctrl_state_1v8)) {
+		dev_err(dev, "Missing 1.8V pad state, err: %ld\n",
+			  PTR_ERR(tegra_host->pinctrl_state_3v3));
+		return;
+	}
+
+	tegra_host->pad_control_available = true;
 }
 
 static const struct sdhci_ops tegra_sdhci_ops = {
@@ -305,7 +408,6 @@ static const struct sdhci_ops tegra_sdhci_ops = {
 	.reset      = tegra_sdhci_reset,
 	.platform_execute_tuning = tegra_sdhci_execute_tuning,
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
-	.voltage_switch = tegra_sdhci_voltage_switch,
 	.get_max_clock = tegra_sdhci_get_max_clock,
 };
 
@@ -362,7 +464,6 @@ static const struct sdhci_ops tegra114_sdhci_ops = {
 	.reset      = tegra_sdhci_reset,
 	.platform_execute_tuning = tegra_sdhci_execute_tuning,
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
-	.voltage_switch = tegra_sdhci_voltage_switch,
 	.get_max_clock = tegra_sdhci_get_max_clock,
 };
 
@@ -419,6 +520,7 @@ 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_NEEDS_PAD_CONTROL,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra186_pdata = {
@@ -442,6 +544,7 @@ static const struct sdhci_pltfm_data sdhci_tegra186_pdata = {
 
 static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
 	.pdata = &sdhci_tegra186_pdata,
+	.nvquirks = NVQUIRK_NEEDS_PAD_CONTROL,
 };
 
 static const struct of_device_id sdhci_tegra_dt_match[] = {
@@ -478,12 +581,23 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	tegra_host = sdhci_pltfm_priv(pltfm_host);
 	tegra_host->ddr_signaling = false;
 	tegra_host->pad_calib_required = false;
+	tegra_host->pad_control_available = false;
+	tegra_host->only_1v8 = false;
 	tegra_host->soc_data = soc_data;
 
+	if (soc_data->nvquirks & NVQUIRK_NEEDS_PAD_CONTROL) {
+		host->mmc_host_ops.start_signal_voltage_switch =
+			sdhci_tegra_start_signal_voltage_switch;
+		tegra_sdhci_init_pinctrl_info(&pdev->dev, tegra_host);
+	}
+
 	rc = mmc_of_parse(host->mmc);
 	if (rc)
 		goto err_parse_dt;
 
+	if (of_get_property(pdev->dev.of_node, "nvidia,only-1-8-v", NULL))
+		tegra_host->only_1v8 = true;
+
 	if (tegra_host->soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
 		host->mmc->caps |= MMC_CAP_1_8V_DDR;
 
-- 
2.7.4

[PATCH v2 04/10] arm64: dts: Add Tegra210 sdmmc pinctrl voltage states

[Aapo Vienamo]

Add pad voltage configuration nodes for sdmmc pads with configurable
voltages on Tegra210.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 3be920e..bc1918e 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -3,6 +3,7 @@
 #include <dt-bindings/gpio/tegra-gpio.h>
 #include <dt-bindings/memory/tegra210-mc.h>
 #include <dt-bindings/pinctrl/pinctrl-tegra.h>
+#include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/thermal/tegra124-soctherm.h>
 
@@ -776,6 +777,26 @@
 				#power-domain-cells = <0>;
 			};
 		};
+
+		sdmmc1_3v3: sdmmc1-3v3 {
+			pins = "sdmmc1";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_3V3>;
+		};
+
+		sdmmc1_1v8: sdmmc1-1v8 {
+			pins = "sdmmc1";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_1V8>;
+		};
+
+		sdmmc3_3v3: sdmmc3-3v3 {
+			pins = "sdmmc3";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_3V3>;
+		};
+
+		sdmmc3_1v8: sdmmc3-1v8 {
+			pins = "sdmmc3";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_1V8>;
+		};
 	};
 
 	fuse@7000f800 {
@@ -1027,6 +1048,9 @@
 		clock-names = "sdhci";
 		resets = <&tegra_car 14>;
 		reset-names = "sdhci";
+		pinctrl-names = "sdmmc-3v3", "sdmmc-1v8";
+		pinctrl-0 = <&sdmmc1_3v3>;
+		pinctrl-1 = <&sdmmc1_1v8>;
 		status = "disabled";
 	};
 
@@ -1049,6 +1073,9 @@
 		clock-names = "sdhci";
 		resets = <&tegra_car 69>;
 		reset-names = "sdhci";
+		pinctrl-names = "sdmmc-3v3", "sdmmc-1v8";
+		pinctrl-0 = <&sdmmc3_3v3>;
+		pinctrl-1 = <&sdmmc3_1v8>;
 		status = "disabled";
 	};
 
-- 
2.7.4

[PATCH v2 05/10] arm64: dts: Add Tegra186 sdmmc pinctrl voltage states

[Aapo Vienamo]

Add pad voltage configuration nodes for sdmmc pads with configurable
voltages on Tegra186.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 40 ++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index b762227..7669756 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -4,6 +4,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/mailbox/tegra186-hsp.h>
 #include <dt-bindings/memory/tegra186-mc.h>
+#include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
 #include <dt-bindings/power/tegra186-powergate.h>
 #include <dt-bindings/reset/tegra186-reset.h>
 #include <dt-bindings/thermal/tegra186-bpmp-thermal.h>
@@ -236,6 +237,9 @@
 		clock-names = "sdhci";
 		resets = <&bpmp TEGRA186_RESET_SDMMC1>;
 		reset-names = "sdhci";
+		pinctrl-names = "sdmmc-3v3", "sdmmc-1v8";
+		pinctrl-0 = <&sdmmc1_3v3>;
+		pinctrl-1 = <&sdmmc1_1v8>;
 		status = "disabled";
 	};
 
@@ -247,6 +251,9 @@
 		clock-names = "sdhci";
 		resets = <&bpmp TEGRA186_RESET_SDMMC2>;
 		reset-names = "sdhci";
+		pinctrl-names = "sdmmc-3v3", "sdmmc-1v8";
+		pinctrl-0 = <&sdmmc2_3v3>;
+		pinctrl-1 = <&sdmmc2_1v8>;
 		status = "disabled";
 	};
 
@@ -258,6 +265,9 @@
 		clock-names = "sdhci";
 		resets = <&bpmp TEGRA186_RESET_SDMMC3>;
 		reset-names = "sdhci";
+		pinctrl-names = "sdmmc-3v3", "sdmmc-1v8";
+		pinctrl-0 = <&sdmmc3_3v3>;
+		pinctrl-1 = <&sdmmc3_1v8>;
 		status = "disabled";
 	};
 
@@ -368,6 +378,36 @@
 		      <0 0x0c380000 0 0x10000>,
 		      <0 0x0c390000 0 0x10000>;
 		reg-names = "pmc", "wake", "aotag", "scratch";
+
+		sdmmc1_3v3: sdmmc1-3v3 {
+			pins = "sdmmc1-hv";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_3V3>;
+		};
+
+		sdmmc1_1v8: sdmmc1-1v8 {
+			pins = "sdmmc1-hv";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_1V8>;
+		};
+
+		sdmmc2_3v3: sdmmc2-3v3 {
+			pins = "sdmmc2-hv";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_3V3>;
+		};
+
+		sdmmc2_1v8: sdmmc2-1v8 {
+			pins = "sdmmc2-hv";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_1V8>;
+		};
+
+		sdmmc3_3v3: sdmmc3-3v3 {
+			pins = "sdmmc3-hv";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_3V3>;
+		};
+
+		sdmmc3_1v8: sdmmc3-1v8 {
+			pins = "sdmmc3-hv";
+			power-source = <TEGRA_IO_PAD_VOLTAGE_1V8>;
+		};
 	};
 
 	ccplex@e000000 {
-- 
2.7.4

[PATCH v2 06/10] arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V

[Aapo Vienamo]

Set regulator-min-microvolt property of ldo2 to 1.8 V in
tegra210-p2180.dtsi. ldo2 is used by the sdmmc1 SDHCI controller and its
voltage needs to be adjusted down to 1.8 V to support faster signaling
modes. It appears that the comment about the SDHCI driver requesting
invalid voltages no longer applies.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
index 212e663..8496101 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
@@ -178,16 +178,7 @@
 
 				vddio_sdmmc: ldo2 {
 					regulator-name = "VDDIO_SDMMC";
-					/*
-					 * Technically this supply should have
-					 * a supported range from 1.8 - 3.3 V.
-					 * However, that would cause the SDHCI
-					 * driver to request 2.7 V upon access
-					 * and that in turn will cause traffic
-					 * to be broken. Leave it at 3.3 V for
-					 * now.
-					 */
-					regulator-min-microvolt = <3300000>;
+					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <3300000>;
 					regulator-always-on;
 					regulator-boot-on;
-- 
2.7.4

[PATCH v2 07/10] arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply

[Aapo Vienamo]

On p2180 sdmmc4 is powered from a fixed 1.8 V regulator.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
index 8496101..053458a 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
@@ -273,6 +273,7 @@
 		status = "okay";
 		bus-width = <8>;
 		non-removable;
+		vqmmc-supply = <&vdd_1v8>;
 	};
 
 	clocks {
-- 
2.7.4

[PATCH v2 08/10] arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1

[Aapo Vienamo]

Allow sdmmc1 to set the signaling voltage to 1.8 V in order to support
faster signaling modes.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
index 9d5a0e6..365726d 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
@@ -1452,7 +1452,6 @@
 	sdhci@700b0000 {
 		status = "okay";
 		bus-width = <4>;
-		no-1-8-v;
 
 		cd-gpios = <&gpio TEGRA_GPIO(Z, 1) GPIO_ACTIVE_LOW>;
 
-- 
2.7.4

[PATCH v2 09/10] arm64: dts: tegra210: Add nvidia,only-1-8-v to sdmmc4

[Aapo Vienamo]

Mark sdmmc4 as 1.8 V only.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index bc1918e..c449566 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -1087,6 +1087,7 @@
 		clock-names = "sdhci";
 		resets = <&tegra_car 15>;
 		reset-names = "sdhci";
+		nvidia,only-1-8-v;
 		status = "disabled";
 	};
 
-- 
2.7.4

[PATCH v2 10/10] arm64: dts: tegra186: Add nvidia,only-1-8-v to sdmmc4

[Aapo Vienamo]

Mark sdmmc4 as 1.8 V only.

Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 7669756..fc14dd3 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -279,6 +279,7 @@
 		clock-names = "sdhci";
 		resets = <&bpmp TEGRA186_RESET_SDMMC4>;
 		reset-names = "sdhci";
+		nvidia,only-1-8-v;
 		status = "disabled";
 	};
 
-- 
2.7.4

Re: [PATCH v2 02/10] dt-bindings: mmc: tegra: Add nvidia,only-1-8-v property

[Stefan Agner]

On 26.07.2018 14:19, Aapo Vienamo wrote:
> Add a property to mark controllers which operate at a 1.8 V fixed I/O
> voltage.
> 
> This feature of the hardware needs to be signaled this way because it
> cannot be probed at runtime or reliably derived from other properties.

Is this really needed? Can we not use vqmmc to determine which voltage
the controller runs on?

There is already some precedence in the SDHCI core to determine which
voltage levels are supported:
https://lkml.org/lkml/2018/7/5/342

--
Stefan

> 
> Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
> ---
>  Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> index 90c214d..95010cf 100644
> --- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> @@ -45,6 +45,8 @@ Optional properties for Tegra210 and Tegra186:
>    for controllers supporting multiple voltage levels. The order of names
>    should correspond to the pin configuration states in pinctrl-0 and
>    pinctrl-1.
> +- nvidia,only-1-8-v : The presence of this property indicates that the
> +  controller operates at a 1.8 V fixed I/O voltage.
>  
>  Example:
>  sdhci@700b0000 {

Re: [PATCH v2 03/10] mmc: tegra: Reconfigure pad voltages during voltage switching

[Stefan Agner]

On 26.07.2018 14:19, Aapo Vienamo wrote:
> Parse the pinctrl state and nvidia,only-1-8-v properties from the device
> tree and implement pad voltage state reconfiguration in the mmc
> start_signal_voltage_switch() callback. Validate the pinctrl and
> regulator configuration before unmasking UHS modes. Add
> NVQUIRK_NEEDS_PAD_CONTROL and add set it for Tegra210 and Tegra186.
> 
> The pad configuration is done in the mmc callback because the order of
> pad reconfiguration and sdhci voltage switch depend on the voltage to
> which the transition occurs.
> 
> Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 140 +++++++++++++++++++++++++++++++++++++----
>  1 file changed, 127 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index ddf00166..9587365 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -21,6 +21,7 @@
>  #include <linux/io.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/reset.h>
>  #include <linux/mmc/card.h>
>  #include <linux/mmc/host.h>
> @@ -55,6 +56,7 @@
>  #define NVQUIRK_ENABLE_SDR104		BIT(4)
>  #define NVQUIRK_ENABLE_DDR50		BIT(5)
>  #define NVQUIRK_HAS_PADCALIB		BIT(6)
> +#define NVQUIRK_NEEDS_PAD_CONTROL	BIT(7)
>  
>  struct sdhci_tegra_soc_data {
>  	const struct sdhci_pltfm_data *pdata;
> @@ -66,8 +68,13 @@ struct sdhci_tegra {
>  	struct gpio_desc *power_gpio;
>  	bool ddr_signaling;
>  	bool pad_calib_required;
> +	bool pad_control_available;
> +	bool only_1v8;
>  
>  	struct reset_control *rst;
> +	struct pinctrl *pinctrl_sdmmc;
> +	struct pinctrl_state *pinctrl_state_3v3;
> +	struct pinctrl_state *pinctrl_state_1v8;
>  };
>  
>  static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
> @@ -138,12 +145,47 @@ static unsigned int tegra_sdhci_get_ro(struct
> sdhci_host *host)
>  	return mmc_gpio_get_ro(host->mmc);
>  }
>  
> +static bool tegra_sdhci_is_uhs_valid(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
> +	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
> +
> +	/*
> +	 * The 1.8 V only host controllers don't need to have configurable
> +	 * regulators and pad voltages. In this case the UHS modes can be
> +	 * enabled regardless.
> +	 */
> +	if (tegra_host->only_1v8)
> +		return true;

Hm, SD always needs 3.3V capabilities, not?

So if the controller is 1.8V only, then only eMMC modes are allowed.

You can use MMC_CAP2_HSX00_1_8V in caps2.

So far the Tegra SDHCI driver did not use device tree to indicate modes,
but maybe we should start doing that. In this case you can use
mmc-hs200-1_8v/mmc-hs400-1_8v to indicate higher eMMC speed modes.

> +
> +	/*
> +	 * If the board does not define a regulator for the SDHCI IO voltage,
> +	 * then don't advertise support for UHS modes even if the device
> +	 * supports it because the IO voltage cannot be configured.
> +	 */
> +	if (IS_ERR(host->mmc->supply.vqmmc))
> +		return false;

The stack should already check that and mask caps1 accordingly (see
sdhci_setup_host).

> +
> +	/*
> +	 * Later SoC generations require software pad voltage configuration.
> +	 * The UHS modes should only be enabled if the pad configuration states
> +	 * are available on platforms where they are required in order to switch
> +	 * the signaling voltage.
> +	 */
> +	if (soc_data->nvquirks & NVQUIRK_NEEDS_PAD_CONTROL)
> +		return tegra_host->pad_control_available;
> +
> +	return false;
> +}
> +
>  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 = sdhci_pltfm_priv(pltfm_host);
>  	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
>  	u32 misc_ctrl, clk_ctrl;
> +	bool uhs_valid;
>  
>  	sdhci_reset(host, mask);
>  
> @@ -160,13 +202,8 @@ static void tegra_sdhci_reset(struct sdhci_host
> *host, u8 mask)
>  
>  	clk_ctrl &= ~SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE;
>  
> -	/*
> -	 * If the board does not define a regulator for the SDHCI
> -	 * IO voltage, then don't advertise support for UHS modes
> -	 * even if the device supports it because the IO voltage
> -	 * cannot be configured.
> -	 */
> -	if (!IS_ERR(host->mmc->supply.vqmmc)) {
> +	uhs_valid = tegra_sdhci_is_uhs_valid(host);
> +	if (uhs_valid) {
>  		/* 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;
> @@ -286,14 +323,80 @@ static int tegra_sdhci_execute_tuning(struct
> sdhci_host *host, u32 opcode)
>  	return mmc_send_tuning(host->mmc, opcode, NULL);
>  }
>  
> -static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
> +static int tegra_sdhci_set_padctrl(struct sdhci_host *host, int voltage)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
> -	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
> +	int ret;
> +
> +	if (!tegra_host->pad_control_available)
> +		return 0;
> +
> +	if (voltage == MMC_SIGNAL_VOLTAGE_180) {
> +		ret = pinctrl_select_state(tegra_host->pinctrl_sdmmc,
> +					   tegra_host->pinctrl_state_1v8);
> +		if (ret < 0)
> +			dev_err(mmc_dev(host->mmc),
> +				"setting 1.8V failed, ret: %d\n", ret);
> +	} else {
> +		ret = pinctrl_select_state(tegra_host->pinctrl_sdmmc,
> +					   tegra_host->pinctrl_state_3v3);
> +		if (ret < 0)
> +			dev_err(mmc_dev(host->mmc),
> +				"setting 3.3V failed, ret: %d\n", ret);
> +	}
>  
> -	if (soc_data->nvquirks & NVQUIRK_HAS_PADCALIB)
> -		tegra_host->pad_calib_required = true;
> +	return ret;
> +}
> +
> +static int sdhci_tegra_start_signal_voltage_switch(struct mmc_host *mmc,
> +						   struct mmc_ios *ios)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	int ret = 0;
> +
> +	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> +		ret = tegra_sdhci_set_padctrl(host, ios->signal_voltage);
> +		if (ret < 0)
> +			return ret;
> +		ret = sdhci_start_signal_voltage_switch(mmc, ios);
> +	} else if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
> +		ret = sdhci_start_signal_voltage_switch(mmc, ios);
> +		if (ret < 0)
> +			return ret;
> +		ret = tegra_sdhci_set_padctrl(host, ios->signal_voltage);

So depending on voltage you first set the pads setting and then do the
signaling voltage switch. Is this necessary? Why?

> +	}
> +
> +	return ret;
> +}
> +
> +static void tegra_sdhci_init_pinctrl_info(struct device *dev,
> +					  struct sdhci_tegra *tegra_host)
> +{
> +	tegra_host->pinctrl_sdmmc = devm_pinctrl_get(dev);
> +	if (IS_ERR(tegra_host->pinctrl_sdmmc)) {
> +		dev_dbg(dev, "No pinctrl info, err: %ld\n",
> +			PTR_ERR(tegra_host->pinctrl_sdmmc));
> +		return;
> +	}
> +
> +	tegra_host->pinctrl_state_3v3 =
> +		pinctrl_lookup_state(tegra_host->pinctrl_sdmmc, "sdmmc-3v3");
> +	if (IS_ERR(tegra_host->pinctrl_state_3v3)) {
> +		dev_err(dev, "Missing 3.3V pad state, err: %ld\n",
> +			PTR_ERR(tegra_host->pinctrl_state_3v3));
> +		return;
> +	}
> +
> +	tegra_host->pinctrl_state_1v8 =
> +		pinctrl_lookup_state(tegra_host->pinctrl_sdmmc, "sdmmc-1v8");
> +	if (IS_ERR(tegra_host->pinctrl_state_1v8)) {
> +		dev_err(dev, "Missing 1.8V pad state, err: %ld\n",
> +			  PTR_ERR(tegra_host->pinctrl_state_3v3));

Is missing pad states considered as error? If yes, we should return a
error code and make the probe function fail.

If it is ok to run it without the states, then this should be dev_warn
or dev_info.

--
Stefan

> +		return;
> +	}
> +
> +	tegra_host->pad_control_available = true;
>  }
>  
>  static const struct sdhci_ops tegra_sdhci_ops = {
> @@ -305,7 +408,6 @@ static const struct sdhci_ops tegra_sdhci_ops = {
>  	.reset      = tegra_sdhci_reset,
>  	.platform_execute_tuning = tegra_sdhci_execute_tuning,
>  	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
> -	.voltage_switch = tegra_sdhci_voltage_switch,
>  	.get_max_clock = tegra_sdhci_get_max_clock,
>  };
>  
> @@ -362,7 +464,6 @@ static const struct sdhci_ops tegra114_sdhci_ops = {
>  	.reset      = tegra_sdhci_reset,
>  	.platform_execute_tuning = tegra_sdhci_execute_tuning,
>  	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
> -	.voltage_switch = tegra_sdhci_voltage_switch,
>  	.get_max_clock = tegra_sdhci_get_max_clock,
>  };
>  
> @@ -419,6 +520,7 @@ 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_NEEDS_PAD_CONTROL,
>  };
>  
>  static const struct sdhci_pltfm_data sdhci_tegra186_pdata = {
> @@ -442,6 +544,7 @@ static const struct sdhci_pltfm_data
> sdhci_tegra186_pdata = {
>  
>  static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
>  	.pdata = &sdhci_tegra186_pdata,
> +	.nvquirks = NVQUIRK_NEEDS_PAD_CONTROL,
>  };
>  
>  static const struct of_device_id sdhci_tegra_dt_match[] = {
> @@ -478,12 +581,23 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
>  	tegra_host = sdhci_pltfm_priv(pltfm_host);
>  	tegra_host->ddr_signaling = false;
>  	tegra_host->pad_calib_required = false;
> +	tegra_host->pad_control_available = false;
> +	tegra_host->only_1v8 = false;
>  	tegra_host->soc_data = soc_data;
>  
> +	if (soc_data->nvquirks & NVQUIRK_NEEDS_PAD_CONTROL) {
> +		host->mmc_host_ops.start_signal_voltage_switch =
> +			sdhci_tegra_start_signal_voltage_switch;
> +		tegra_sdhci_init_pinctrl_info(&pdev->dev, tegra_host);
> +	}
> +
>  	rc = mmc_of_parse(host->mmc);
>  	if (rc)
>  		goto err_parse_dt;
>  
> +	if (of_get_property(pdev->dev.of_node, "nvidia,only-1-8-v", NULL))
> +		tegra_host->only_1v8 = true;
> +
>  	if (tegra_host->soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
>  		host->mmc->caps |= MMC_CAP_1_8V_DDR;

Re: [PATCH v2 02/10] dt-bindings: mmc: tegra: Add nvidia,only-1-8-v property

[Aapo Vienamo]

On Thu, 26 Jul 2018 15:05:55 +0200
Stefan Agner <stefan@agner.ch> wrote:

> On 26.07.2018 14:19, Aapo Vienamo wrote:
> > Add a property to mark controllers which operate at a 1.8 V fixed I/O
> > voltage.
> > 
> > This feature of the hardware needs to be signaled this way because it
> > cannot be probed at runtime or reliably derived from other properties.  
> 
> Is this really needed? Can we not use vqmmc to determine which voltage
> the controller runs on?
> 
> There is already some precedence in the SDHCI core to determine which
> voltage levels are supported:
> https://lkml.org/lkml/2018/7/5/342

This property is introduced to solve a slightly different issue. The
thing is that supplying a fixed voltage SDHCI controller from a variable
regulator is still a valid configuration. Which means that testing the
capabilities of the regulator doesn't actually describe the SDHCI
controller itself.

In practice this property is used to communicate whether pad
reconfiguration and voltage switching needs to be performed or not. This
cannot be determined from the absence or presence of the pinctrl
properties either because they naturally won't be there on older dtbs.

The logic behind this goes like this: if this property is present,
there's no need to perform pad or regulator reconfiguration and UHS
modes can be enabled. If this property is missing then valid pinctrl and
regulator properties are required to enable UHS signaling. This is
implemented in tegra_sdhci_is_uhs_valid() in "[PATCH v2 03/10] mmc:
tegra: Reconfigure pad voltages during voltage switching"

 -Aapo

Re: [PATCH v2 03/10] mmc: tegra: Reconfigure pad voltages during voltage switching

[Aapo Vienamo]

On Thu, 26 Jul 2018 15:33:11 +0200
Stefan Agner <stefan@agner.ch> wrote:

> On 26.07.2018 14:19, Aapo Vienamo wrote:
> > Parse the pinctrl state and nvidia,only-1-8-v properties from the device
> > tree and implement pad voltage state reconfiguration in the mmc
> > start_signal_voltage_switch() callback. Validate the pinctrl and
> > regulator configuration before unmasking UHS modes. Add
> > NVQUIRK_NEEDS_PAD_CONTROL and add set it for Tegra210 and Tegra186.
> > 
> > The pad configuration is done in the mmc callback because the order of
> > pad reconfiguration and sdhci voltage switch depend on the voltage to
> > which the transition occurs.
> > 
> > Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
> > ---
> >  drivers/mmc/host/sdhci-tegra.c | 140 +++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 127 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> > index ddf00166..9587365 100644
> > --- a/drivers/mmc/host/sdhci-tegra.c
> > +++ b/drivers/mmc/host/sdhci-tegra.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> > +#include <linux/pinctrl/consumer.h>
> >  #include <linux/reset.h>
> >  #include <linux/mmc/card.h>
> >  #include <linux/mmc/host.h>
> > @@ -55,6 +56,7 @@
> >  #define NVQUIRK_ENABLE_SDR104		BIT(4)
> >  #define NVQUIRK_ENABLE_DDR50		BIT(5)
> >  #define NVQUIRK_HAS_PADCALIB		BIT(6)
> > +#define NVQUIRK_NEEDS_PAD_CONTROL	BIT(7)
> >  
> >  struct sdhci_tegra_soc_data {
> >  	const struct sdhci_pltfm_data *pdata;
> > @@ -66,8 +68,13 @@ struct sdhci_tegra {
> >  	struct gpio_desc *power_gpio;
> >  	bool ddr_signaling;
> >  	bool pad_calib_required;
> > +	bool pad_control_available;
> > +	bool only_1v8;
> >  
> >  	struct reset_control *rst;
> > +	struct pinctrl *pinctrl_sdmmc;
> > +	struct pinctrl_state *pinctrl_state_3v3;
> > +	struct pinctrl_state *pinctrl_state_1v8;
> >  };
> >  
> >  static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
> > @@ -138,12 +145,47 @@ static unsigned int tegra_sdhci_get_ro(struct
> > sdhci_host *host)
> >  	return mmc_gpio_get_ro(host->mmc);
> >  }
> >  
> > +static bool tegra_sdhci_is_uhs_valid(struct sdhci_host *host)
> > +{
> > +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > +	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
> > +	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
> > +
> > +	/*
> > +	 * The 1.8 V only host controllers don't need to have configurable
> > +	 * regulators and pad voltages. In this case the UHS modes can be
> > +	 * enabled regardless.
> > +	 */
> > +	if (tegra_host->only_1v8)
> > +		return true;  
> 
> Hm, SD always needs 3.3V capabilities, not?

Yes, the controllers used for eMMCs should have the only_1v8 property
set and they exit the function here. With controllers used for SD cards,
the regulator and pad configurations are verified later in the function.

> So if the controller is 1.8V only, then only eMMC modes are allowed.
> 
> You can use MMC_CAP2_HSX00_1_8V in caps2.

I can't quite follow you, how this should be used here?

> So far the Tegra SDHCI driver did not use device tree to indicate modes,
> but maybe we should start doing that. In this case you can use
> mmc-hs200-1_8v/mmc-hs400-1_8v to indicate higher eMMC speed modes.
> 
> > +
> > +	/*
> > +	 * If the board does not define a regulator for the SDHCI IO voltage,
> > +	 * then don't advertise support for UHS modes even if the device
> > +	 * supports it because the IO voltage cannot be configured.
> > +	 */
> > +	if (IS_ERR(host->mmc->supply.vqmmc))
> > +		return false;  
> 
> The stack should already check that and mask caps1 accordingly (see
> sdhci_setup_host).

True, the logic seems to be duplicated here. Although also
tegra_sdhci_reset() needs to be change so that the bits masked off by
sdhci_setup_host() aren't set if this check is removed.

> > +
> > +	/*
> > +	 * Later SoC generations require software pad voltage configuration.
> > +	 * The UHS modes should only be enabled if the pad configuration states
> > +	 * are available on platforms where they are required in order to switch
> > +	 * the signaling voltage.
> > +	 */
> > +	if (soc_data->nvquirks & NVQUIRK_NEEDS_PAD_CONTROL)
> > +		return tegra_host->pad_control_available;
> > +
> > +	return false;
> > +}
> > +
> >  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 = sdhci_pltfm_priv(pltfm_host);
> >  	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
> >  	u32 misc_ctrl, clk_ctrl;
> > +	bool uhs_valid;
> >  
> >  	sdhci_reset(host, mask);
> >  
> > @@ -160,13 +202,8 @@ static void tegra_sdhci_reset(struct sdhci_host
> > *host, u8 mask)
> >  
> >  	clk_ctrl &= ~SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE;
> >  
> > -	/*
> > -	 * If the board does not define a regulator for the SDHCI
> > -	 * IO voltage, then don't advertise support for UHS modes
> > -	 * even if the device supports it because the IO voltage
> > -	 * cannot be configured.
> > -	 */
> > -	if (!IS_ERR(host->mmc->supply.vqmmc)) {
> > +	uhs_valid = tegra_sdhci_is_uhs_valid(host);
> > +	if (uhs_valid) {
> >  		/* 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;
> > @@ -286,14 +323,80 @@ static int tegra_sdhci_execute_tuning(struct
> > sdhci_host *host, u32 opcode)
> >  	return mmc_send_tuning(host->mmc, opcode, NULL);
> >  }
> >  
> > -static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
> > +static int tegra_sdhci_set_padctrl(struct sdhci_host *host, int voltage)
> >  {
> >  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >  	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
> > -	const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
> > +	int ret;
> > +
> > +	if (!tegra_host->pad_control_available)
> > +		return 0;
> > +
> > +	if (voltage == MMC_SIGNAL_VOLTAGE_180) {
> > +		ret = pinctrl_select_state(tegra_host->pinctrl_sdmmc,
> > +					   tegra_host->pinctrl_state_1v8);
> > +		if (ret < 0)
> > +			dev_err(mmc_dev(host->mmc),
> > +				"setting 1.8V failed, ret: %d\n", ret);
> > +	} else {
> > +		ret = pinctrl_select_state(tegra_host->pinctrl_sdmmc,
> > +					   tegra_host->pinctrl_state_3v3);
> > +		if (ret < 0)
> > +			dev_err(mmc_dev(host->mmc),
> > +				"setting 3.3V failed, ret: %d\n", ret);
> > +	}
> >  
> > -	if (soc_data->nvquirks & NVQUIRK_HAS_PADCALIB)
> > -		tegra_host->pad_calib_required = true;
> > +	return ret;
> > +}
> > +
> > +static int sdhci_tegra_start_signal_voltage_switch(struct mmc_host *mmc,
> > +						   struct mmc_ios *ios)
> > +{
> > +	struct sdhci_host *host = mmc_priv(mmc);
> > +	int ret = 0;
> > +
> > +	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> > +		ret = tegra_sdhci_set_padctrl(host, ios->signal_voltage);
> > +		if (ret < 0)
> > +			return ret;
> > +		ret = sdhci_start_signal_voltage_switch(mmc, ios);
> > +	} else if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
> > +		ret = sdhci_start_signal_voltage_switch(mmc, ios);
> > +		if (ret < 0)
> > +			return ret;
> > +		ret = tegra_sdhci_set_padctrl(host, ios->signal_voltage);  
> 
> So depending on voltage you first set the pads setting and then do the
> signaling voltage switch. Is this necessary? Why?

This is done to enforce the constraint of keeping the signaling voltage
always less or equal to the pad voltage. Driving 3.3 V into a pad that
has been configured to 1.8 V will damage it. This is stated in the
Tegra210 and Tegra186 TRMs in SDMMC Initialization Sequece sections of
controllers supporting voltage switching.

Here when switching from 3.3 V to 1.8 V the regulator is first
configured to 1.8 V by sdhci_start_signal_voltage_switch() and after
that the pad is configured to 1.8 V. When going in the other direction
the pad needs to be first configured to 3.3 V before the regulator
voltage can be raised.

> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static void tegra_sdhci_init_pinctrl_info(struct device *dev,
> > +					  struct sdhci_tegra *tegra_host)
> > +{
> > +	tegra_host->pinctrl_sdmmc = devm_pinctrl_get(dev);
> > +	if (IS_ERR(tegra_host->pinctrl_sdmmc)) {
> > +		dev_dbg(dev, "No pinctrl info, err: %ld\n",
> > +			PTR_ERR(tegra_host->pinctrl_sdmmc));
> > +		return;
> > +	}
> > +
> > +	tegra_host->pinctrl_state_3v3 =
> > +		pinctrl_lookup_state(tegra_host->pinctrl_sdmmc, "sdmmc-3v3");
> > +	if (IS_ERR(tegra_host->pinctrl_state_3v3)) {
> > +		dev_err(dev, "Missing 3.3V pad state, err: %ld\n",
> > +			PTR_ERR(tegra_host->pinctrl_state_3v3));
> > +		return;
> > +	}
> > +
> > +	tegra_host->pinctrl_state_1v8 =
> > +		pinctrl_lookup_state(tegra_host->pinctrl_sdmmc, "sdmmc-1v8");
> > +	if (IS_ERR(tegra_host->pinctrl_state_1v8)) {
> > +		dev_err(dev, "Missing 1.8V pad state, err: %ld\n",
> > +			  PTR_ERR(tegra_host->pinctrl_state_3v3));  
> 
> Is missing pad states considered as error? If yes, we should return a
> error code and make the probe function fail.
> 
> If it is ok to run it without the states, then this should be dev_warn
> or dev_info.

I don't think failing probe due to missing pinctrl entries can be done
because it would break backwards compatibility with pre-existing dtbs.
Also SDHCI controllers with fixed signaling voltage don't support pad
voltage reconfiguration and therefore the pinctrl properties won't be
there either. In the case of fixed voltage SDHCI controllers the
"nvidia,only-1-8-v" property is used to signal that pad reconfiguration
doesn't need to be performed.

 -Aapo

Re: [PATCH v2 01/10] dt-bindings: mmc: tegra: Add pad voltage control properties

[Rob Herring]

On Thu, Jul 26, 2018 at 03:19:11PM +0300, Aapo Vienamo wrote:
> Document the pinctrl bindings used by the SDHCI driver to reconfigure
> pad voltages on controllers supporting multiple voltage levels.
> 
> Signed-off-by: Aapo Vienamo <avienamo@nvidia.com>
> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  .../bindings/mmc/nvidia,tegra20-sdhci.txt          | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 02/10] dt-bindings: mmc: tegra: Add nvidia,only-1-8-v property

[Rob Herring]

On Fri, Jul 27, 2018 at 11:05:55AM +0300, Aapo Vienamo wrote:
> On Thu, 26 Jul 2018 15:05:55 +0200
> Stefan Agner <stefan@agner.ch> wrote:
> 
> > On 26.07.2018 14:19, Aapo Vienamo wrote:
> > > Add a property to mark controllers which operate at a 1.8 V fixed I/O
> > > voltage.
> > > 
> > > This feature of the hardware needs to be signaled this way because it
> > > cannot be probed at runtime or reliably derived from other properties.  
> > 
> > Is this really needed? Can we not use vqmmc to determine which voltage
> > the controller runs on?
> > 
> > There is already some precedence in the SDHCI core to determine which
> > voltage levels are supported:
> > https://lkml.org/lkml/2018/7/5/342
> 
> This property is introduced to solve a slightly different issue. The
> thing is that supplying a fixed voltage SDHCI controller from a variable
> regulator is still a valid configuration. Which means that testing the
> capabilities of the regulator doesn't actually describe the SDHCI
> controller itself.

The regulator constraints should reflect this. The constraints aren't 
the capabilities of the regulator, but the limits on what it is 
supplying.

> 
> In practice this property is used to communicate whether pad
> reconfiguration and voltage switching needs to be performed or not. This
> cannot be determined from the absence or presence of the pinctrl
> properties either because they naturally won't be there on older dtbs.
> 
> The logic behind this goes like this: if this property is present,
> there's no need to perform pad or regulator reconfiguration and UHS
> modes can be enabled. If this property is missing then valid pinctrl and
> regulator properties are required to enable UHS signaling. This is
> implemented in tegra_sdhci_is_uhs_valid() in "[PATCH v2 03/10] mmc:
> tegra: Reconfigure pad voltages during voltage switching"
> 
>  -Aapo