2018-07-20 12:48:18

by Aapo Vienamo

[permalink] [raw]
Subject: [PATCH 0/7] Tegra SDHCI enable 1.8 V signaling on Tegar210 and Tegra186

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.

Aapo Vienamo (7):
dt-bindings: Document Tegra SDHCI pinctrl bindings
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

.../bindings/mmc/nvidia,tegra20-sdhci.txt | 22 ++++++
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 40 ++++++++++
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 | 27 +++++++
drivers/mmc/host/sdhci-tegra.c | 91 ++++++++++++++++++++--
6 files changed, 176 insertions(+), 17 deletions(-)

--
2.7.4



2018-07-20 12:47:24

by Aapo Vienamo

[permalink] [raw]
Subject: [PATCH 2/7] mmc: tegra: Reconfigure pad voltages during voltage switching

Parse the pinctrl states from the device tree and implement pad voltage
state reconfiguration in the mmc start_signal_voltage_switch() callback.
This 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.

Add NVQUIRK_NEEDS_PAD_CONTROL and add set it for Tegra210 and Tegra186.

Signed-off-by: Aapo Vienamo <[email protected]>
---
drivers/mmc/host/sdhci-tegra.c | 91 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 85 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index ddf00166..f108c48 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,12 @@ struct sdhci_tegra {
struct gpio_desc *power_gpio;
bool ddr_signaling;
bool pad_calib_required;
+ bool pad_control_required;

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)
@@ -286,14 +292,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_required)
+ 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_OR_NULL(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_OR_NULL(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_OR_NULL(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_required = true;
}

static const struct sdhci_ops tegra_sdhci_ops = {
@@ -305,7 +377,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 +433,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 +489,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 +513,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,8 +550,15 @@ 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_required = 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;
--
2.7.4


2018-07-20 12:47:33

by Aapo Vienamo

[permalink] [raw]
Subject: [PATCH 3/7] arm64: dts: Add Tegra210 sdmmc pinctrl voltage states

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

Signed-off-by: Aapo Vienamo <[email protected]>
---
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


2018-07-20 12:47:38

by Aapo Vienamo

[permalink] [raw]
Subject: [PATCH 6/7] arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply

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

Signed-off-by: Aapo Vienamo <[email protected]>
---
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


2018-07-20 12:47:43

by Aapo Vienamo

[permalink] [raw]
Subject: [PATCH 7/7] arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1

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

Signed-off-by: Aapo Vienamo <[email protected]>
---
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


2018-07-20 12:47:52

by Aapo Vienamo

[permalink] [raw]
Subject: [PATCH 5/7] arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V

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


2018-07-20 12:48:12

by Aapo Vienamo

[permalink] [raw]
Subject: [PATCH 1/7] dt-bindings: Document Tegra SDHCI pinctrl bindings

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

Signed-off-by: Aapo Vienamo <[email protected]>
---
.../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


2018-07-20 12:48:22

by Aapo Vienamo

[permalink] [raw]
Subject: [PATCH 4/7] arm64: dts: Add Tegra186 sdmmc pinctrl voltage states

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

Signed-off-by: Aapo Vienamo <[email protected]>
---
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


2018-07-21 00:16:31

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 3/7] arm64: dts: Add Tegra210 sdmmc pinctrl voltage states

Hi Aapo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.18-rc5 next-20180720]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Aapo-Vienamo/Tegra-SDHCI-enable-1-8-V-signaling-on-Tegar210-and-Tegra186/20180721-043249
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm64

All errors (new ones prefixed by >>):

In file included from arch/arm64/boot/dts/nvidia/tegra210-p2530.dtsi:2:0,
from arch/arm64/boot/dts/nvidia/tegra210-p2371-0000.dts:4:
>> arch/arm64/boot/dts/nvidia/tegra210.dtsi:6:10: fatal error: dt-bindings/pinctrl/pinctrl-tegra-io-pad.h: No such file or directory
#include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

vim +6 arch/arm64/boot/dts/nvidia/tegra210.dtsi

> 6 #include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
7 #include <dt-bindings/interrupt-controller/arm-gic.h>
8 #include <dt-bindings/thermal/tegra124-soctherm.h>
9

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.70 kB)
.config.gz (37.85 kB)
Download all attachments

2018-07-21 01:31:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 4/7] arm64: dts: Add Tegra186 sdmmc pinctrl voltage states

Hi Aapo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.18-rc5 next-20180720]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Aapo-Vienamo/Tegra-SDHCI-enable-1-8-V-signaling-on-Tegar210-and-Tegra186/20180721-043249
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm64

All errors (new ones prefixed by >>):

In file included from arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi:2:0,
from arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts:6:
>> arch/arm64/boot/dts/nvidia/tegra186.dtsi:7:10: fatal error: dt-bindings/pinctrl/pinctrl-tegra-io-pad.h: No such file or directory
#include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

vim +7 arch/arm64/boot/dts/nvidia/tegra186.dtsi

> 7 #include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
8 #include <dt-bindings/power/tegra186-powergate.h>
9 #include <dt-bindings/reset/tegra186-reset.h>
10 #include <dt-bindings/thermal/tegra186-bpmp-thermal.h>
11

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.75 kB)
.config.gz (37.85 kB)
Download all attachments

2018-07-25 06:43:01

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH 2/7] mmc: tegra: Reconfigure pad voltages during voltage switching

On 20.07.2018 15:45, Aapo Vienamo wrote:
> Parse the pinctrl states from the device tree and implement pad voltage
> state reconfiguration in the mmc start_signal_voltage_switch() callback.
> This 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.
>
> Add NVQUIRK_NEEDS_PAD_CONTROL and add set it for Tegra210 and Tegra186.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> drivers/mmc/host/sdhci-tegra.c | 91 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 85 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index ddf00166..f108c48 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,12 @@ struct sdhci_tegra {
> struct gpio_desc *power_gpio;
> bool ddr_signaling;
> bool pad_calib_required;
> + bool pad_control_required;
>
> 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)
> @@ -286,14 +292,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_required)
> + 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_OR_NULL(tegra_host->pinctrl_sdmmc)) {

Can this ever return NULL, considering ARCH_TEGRA selects PINCTRL?
IS_ERR is probably better. Same for the two other checks in this function.

> + 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_OR_NULL(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_OR_NULL(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_required = true; > }
>
> static const struct sdhci_ops tegra_sdhci_ops = {
> @@ -305,7 +377,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 +433,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 +489,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 +513,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,8 +550,15 @@ 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_required = 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);
> + }
> +

Do we know here if the controller is for eMMC or SD? If it's for SD we
should probably print a warning if the pinctrl info is not available.
Later we would also need to disable 1.8V modes if we fail to get the
pinctrl info.

Cheers,
Mikko

> rc = mmc_of_parse(host->mmc);
> if (rc)
> goto err_parse_dt;
>

2018-07-25 06:48:20

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH 1/7] dt-bindings: Document Tegra SDHCI pinctrl bindings

I would maybe say "dt-bindings: mmc: tegra: Add pad voltage control
properties" or similar for the subject - the current kind of looks like
the SDHCI controller is a pinctrl device :)

Reviewed-by: Mikko Perttunen <[email protected]>

On 20.07.2018 15:45, 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 <[email protected]>
> ---
> .../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";
> +};
>

2018-07-25 06:50:40

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH 3/7] arm64: dts: Add Tegra210 sdmmc pinctrl voltage states

Reviewed-by: Mikko Perttunen <[email protected]>

On 20.07.2018 15:45, Aapo Vienamo wrote:
> Add pad voltage configuration nodes for sdmmc pads with configurable
> voltages on Tegra210.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> 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";
> };
>
>

2018-07-25 06:51:46

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH 4/7] arm64: dts: Add Tegra186 sdmmc pinctrl voltage states

Reviewed-by: Mikko Perttunen <[email protected]>

On 20.07.2018 15:45, Aapo Vienamo wrote:
> Add pad voltage configuration nodes for sdmmc pads with configurable
> voltages on Tegra186.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> 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 {
>

2018-07-25 06:54:50

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH 6/7] arm64: dts: tegra210-p2180: Correct sdmmc4 vqmmc-supply

Technically this shouldn't be required since VDD_1V8 is always on
anyway, but I think it's nicer to specify regulators anyway, so +1!

Reviewed-by: Mikko Perttunen <[email protected]>

On 20.07.2018 15:45, Aapo Vienamo wrote:
> On p2180 sdmmc4 is powered from a fixed 1.8 V regulator.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> 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 {
>

2018-07-25 06:55:15

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH 5/7] arm64: dts: tegra210-p2180: Allow ldo2 to go down to 1.8 V

Reviewed-by: Mikko Perttunen <[email protected]>

On 20.07.2018 15:45, Aapo Vienamo wrote:
> 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 <[email protected]>
> ---
> 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;
>

2018-07-25 06:59:49

by Mikko Perttunen

[permalink] [raw]
Subject: Re: [PATCH 7/7] arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1

Looks like patch 6 will probably cause tegra-sdhci to start advertising
faster modes (see " if (!IS_ERR(host->mmc->supply.vqmmc))" in
sdhci-tegra.c). With that patch and this, will the SDHCI core start to
try putting us into these higher modes? Clearly that won't work yet
before the upcoming patches.

Mikko

On 20.07.2018 15:45, Aapo Vienamo wrote:
> Allow sdmmc1 to set the signaling voltage to 1.8 V in order to support
> faster signaling modes.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> 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>;
>
>

2018-07-25 08:14:23

by Aapo Vienamo

[permalink] [raw]
Subject: Re: [PATCH 7/7] arm64: dts: tegra210-p2597: Remove no-1-8-v from sdmmc1

On Wed, 25 Jul 2018 09:58:37 +0300
Mikko Perttunen <[email protected]> wrote:

> Looks like patch 6 will probably cause tegra-sdhci to start advertising
> faster modes (see " if (!IS_ERR(host->mmc->supply.vqmmc))" in
> sdhci-tegra.c). With that patch and this, will the SDHCI core start to
> try putting us into these higher modes? Clearly that won't work yet
> before the upcoming patches.

The SDHCI core won't do that yet at this point. The host capability bits
for the higher speed modes are currently masked by tegra_sdhci_reset()
in sdhci-tegra. The modes are unmasked in another patch after everything
that's required to support the modes has been implemented.

-Aapo

2018-07-25 08:36:41

by Aapo Vienamo

[permalink] [raw]
Subject: Re: [PATCH 2/7] mmc: tegra: Reconfigure pad voltages during voltage switching

On Wed, 25 Jul 2018 09:41:05 +0300
Mikko Perttunen <[email protected]> wrote:

> On 20.07.2018 15:45, Aapo Vienamo wrote:
> > Parse the pinctrl states from the device tree and implement pad voltage
> > state reconfiguration in the mmc start_signal_voltage_switch() callback.
> > This 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.
> >
> > Add NVQUIRK_NEEDS_PAD_CONTROL and add set it for Tegra210 and Tegra186.
> >
> > Signed-off-by: Aapo Vienamo <[email protected]>
> > ---
> > drivers/mmc/host/sdhci-tegra.c | 91 +++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 85 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> > index ddf00166..f108c48 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,12 @@ struct sdhci_tegra {
> > struct gpio_desc *power_gpio;
> > bool ddr_signaling;
> > bool pad_calib_required;
> > + bool pad_control_required;
> >
> > 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)
> > @@ -286,14 +292,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_required)
> > + 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_OR_NULL(tegra_host->pinctrl_sdmmc)) {
>
> Can this ever return NULL, considering ARCH_TEGRA selects PINCTRL?
> IS_ERR is probably better. Same for the two other checks in this function.
>
> > + 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_OR_NULL(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_OR_NULL(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_required = true; > }
> >
> > static const struct sdhci_ops tegra_sdhci_ops = {
> > @@ -305,7 +377,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 +433,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 +489,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 +513,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,8 +550,15 @@ 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_required = 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);
> > + }
> > +
>
> Do we know here if the controller is for eMMC or SD? If it's for SD we
> should probably print a warning if the pinctrl info is not available.
> Later we would also need to disable 1.8V modes if we fail to get the
> pinctrl info.

As far as I can tell the controllers themselves are the same from the
software interface standpoint regardless whether they are connected to
an SD or eMMC device. The differences among the controllers arise from
the way they are attached to rest of the SoC. The controllers which are
hooked to an SD card slot are usually supplied from an adjustable
regulator, whereas the regulator for eMMC controllers is fixed. Also
the SD controllers have configurable pad voltages and the eMMC ones
don't.

The driver probably should check whether the regulator capabilities
align with the available pinctrl configurations and act accordingly to
avoid invalid configurations.

-Aapo