| Message ID | 20170223173053.19701-7-thierry.reding@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 23/02/17 17:30, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > The P3110 processor module wires one of the SDHCI controllers to an on- > board eMMC and exposes another set of SD/MMC signals on the connector to > support an external SD/MMC card. A third controller is connected to the > SDIO pins of an M.2 KEY E connector. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi | 42 ++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi > index 1a9ac73b4ecb..b18e166527d8 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi > +++ b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi > @@ -7,6 +7,8 @@ > compatible = "nvidia,p3310", "nvidia,tegra186"; > > aliases { > + sdhci0 = "/sdhci@3460000"; > + sdhci1 = "/sdhci@3400000"; Any reason why you don't include all 3 sdhci controllers here? > serial0 = &uarta; > i2c0 = "/bpmp/i2c"; > i2c1 = "/i2c@3160000"; > @@ -72,6 +74,32 @@ > status = "okay"; > }; > > + /* SDMMC1 (SD/MMC) */ > + sdhci@3400000 { > + status = "okay"; > + > + cd-gpios = <&gpio TEGRA_MAIN_GPIO(P, 6) GPIO_ACTIVE_LOW>; I see the CD as P-5 and not P-6 on the schematic. > + wp-gpios = <&gpio TEGRA_MAIN_GPIO(P, 4) GPIO_ACTIVE_LOW>; > + > + vqmmc-supply = <&vddio_sdmmc1>; > + vmmc-supply = <&vdd_sd>; The card slot is on the carrier board and although the cd and wp pins are allocated on the cvm, I did not see a reference to the vmmc-supply on the cvm. So should the 'vmmc-supply' be in the carrier board file? > + }; > + > + /* SDMMC3 (SDIO) */ > + sdhci@3440000 { > + status = "okay"; > + }; > + > + /* SDMMC4 (eMMC) */ > + sdhci@3460000 { > + status = "okay"; > + bus-width = <8>; > + non-removable; > + > + vqmmc-supply = <&vdd_1v8_ap>; > + vmmc-supply = <&vdd_3v3_sys>; > + }; > + > hsp@3c00000 { > status = "okay"; > }; > @@ -333,5 +361,19 @@ > > vin-supply = <&vdd_1v8>; > }; > + > + vdd_sd: regulator@2 { > + compatible = "regulator-fixed"; > + reg = <2>; > + > + regulator-name = "SD_CARD_SW_PWR"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + > + gpio = <&gpio TEGRA_MAIN_GPIO(P, 5) GPIO_ACTIVE_HIGH>; Here I see the gpio as P-6 and not P-5. > + enable-active-high; > + > + vin-supply = <&vdd_3v3_sys>; > + }; > }; > }; I also see the above regulator on the carrier board and not on the cvm. Cheers Jon
On Wed, Mar 08, 2017 at 11:46:10AM +0000, Jon Hunter wrote: > > On 23/02/17 17:30, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > The P3110 processor module wires one of the SDHCI controllers to an on- > > board eMMC and exposes another set of SD/MMC signals on the connector to > > support an external SD/MMC card. A third controller is connected to the > > SDIO pins of an M.2 KEY E connector. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi | 42 ++++++++++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi > > index 1a9ac73b4ecb..b18e166527d8 100644 > > --- a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi > > +++ b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi > > @@ -7,6 +7,8 @@ > > compatible = "nvidia,p3310", "nvidia,tegra186"; > > > > aliases { > > + sdhci0 = "/sdhci@3460000"; > > + sdhci1 = "/sdhci@3400000"; > > Any reason why you don't include all 3 sdhci controllers here? It turns out this is completely useless. The intention had been to make sdhci@3460000 appear as mmc0 and sdhci@3400000 as mmc1 in order to give a more natural ordering (built-in eMMC first, then external SD/MMC) but there's no code in the kernel that will enforce this. This is also the reason why sdhci@3440000 is not included in this list because it connects to a WIFI/Bluetooth module. I'm slightly leaning towards leaving this in and possibly write a patch to enforce ordering as given by the aliases. Of course nobody in their right mind should be depending on the device names remaining the same and use partition UUIDs or labels instead. But having enforcing the order would still provide the least surprise. > > > serial0 = &uarta; > > i2c0 = "/bpmp/i2c"; > > i2c1 = "/i2c@3160000"; > > @@ -72,6 +74,32 @@ > > status = "okay"; > > }; > > > > + /* SDMMC1 (SD/MMC) */ > > + sdhci@3400000 { > > + status = "okay"; > > + > > + cd-gpios = <&gpio TEGRA_MAIN_GPIO(P, 6) GPIO_ACTIVE_LOW>; > > I see the CD as P-5 and not P-6 on the schematic. Urgh... I probably mixed these up by looking at the Parker datasheet (not sure if that's publicly available yet) and that says PP.05 actually has the SDMMC3_CD special function. But since these are used as plain GPIOs that's not where the function is defined. And indeed I see that the schematics has PP.05 as the CD for SDMMC1. Strangely I remember card-detect working with the above... > > + wp-gpios = <&gpio TEGRA_MAIN_GPIO(P, 4) GPIO_ACTIVE_LOW>; > > + > > + vqmmc-supply = <&vddio_sdmmc1>; > > + vmmc-supply = <&vdd_sd>; > > The card slot is on the carrier board and although the cd and wp pins > are allocated on the cvm, I did not see a reference to the vmmc-supply > on the cvm. So should the 'vmmc-supply' be in the carrier board file? Yes, I think it's best to move vmmc-supply to the P2771 board file... > > > + }; > > + > > + /* SDMMC3 (SDIO) */ > > + sdhci@3440000 { > > + status = "okay"; > > + }; > > + > > + /* SDMMC4 (eMMC) */ > > + sdhci@3460000 { > > + status = "okay"; > > + bus-width = <8>; > > + non-removable; > > + > > + vqmmc-supply = <&vdd_1v8_ap>; > > + vmmc-supply = <&vdd_3v3_sys>; > > + }; > > + > > hsp@3c00000 { > > status = "okay"; > > }; > > @@ -333,5 +361,19 @@ > > > > vin-supply = <&vdd_1v8>; > > }; > > + > > + vdd_sd: regulator@2 { > > + compatible = "regulator-fixed"; > > + reg = <2>; > > + > > + regulator-name = "SD_CARD_SW_PWR"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + > > + gpio = <&gpio TEGRA_MAIN_GPIO(P, 5) GPIO_ACTIVE_HIGH>; > > Here I see the gpio as P-6 and not P-5. Yes, that also matches the schematics that I have. Strange how this would still all work given the mixup... > > + enable-active-high; > > + > > + vin-supply = <&vdd_3v3_sys>; > > + }; > > }; > > }; > > I also see the above regulator on the carrier board and not on the cvm. ... and move this to the P2771 board file as well. Thanks, Thierry
diff --git a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi index 1a9ac73b4ecb..b18e166527d8 100644 --- a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi @@ -7,6 +7,8 @@ compatible = "nvidia,p3310", "nvidia,tegra186"; aliases { + sdhci0 = "/sdhci@3460000"; + sdhci1 = "/sdhci@3400000"; serial0 = &uarta; i2c0 = "/bpmp/i2c"; i2c1 = "/i2c@3160000"; @@ -72,6 +74,32 @@ status = "okay"; }; + /* SDMMC1 (SD/MMC) */ + sdhci@3400000 { + status = "okay"; + + cd-gpios = <&gpio TEGRA_MAIN_GPIO(P, 6) GPIO_ACTIVE_LOW>; + wp-gpios = <&gpio TEGRA_MAIN_GPIO(P, 4) GPIO_ACTIVE_LOW>; + + vqmmc-supply = <&vddio_sdmmc1>; + vmmc-supply = <&vdd_sd>; + }; + + /* SDMMC3 (SDIO) */ + sdhci@3440000 { + status = "okay"; + }; + + /* SDMMC4 (eMMC) */ + sdhci@3460000 { + status = "okay"; + bus-width = <8>; + non-removable; + + vqmmc-supply = <&vdd_1v8_ap>; + vmmc-supply = <&vdd_3v3_sys>; + }; + hsp@3c00000 { status = "okay"; }; @@ -333,5 +361,19 @@ vin-supply = <&vdd_1v8>; }; + + vdd_sd: regulator@2 { + compatible = "regulator-fixed"; + reg = <2>; + + regulator-name = "SD_CARD_SW_PWR"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + + gpio = <&gpio TEGRA_MAIN_GPIO(P, 5) GPIO_ACTIVE_HIGH>; + enable-active-high; + + vin-supply = <&vdd_3v3_sys>; + }; }; };