[PATCH v1 0/3] arm64: dts: qcom: sdm845: Add SD nodes

[PATCH v1 0/3] arm64: dts: qcom: sdm845: Add SD nodes

[Evan Green]

This series adds the nodes for the SD host controller on SDM845, and wires it
up on the MTP.

Though I tested similar changes on another board, I was unable to actually run
this on an MTP. If someone felt like trying this out on an MTP I would be
grateful.

The original downstream nodes had sleep pinctrl states. It's not obvious to
me that all this messing with drive-strength saves non-negligible amounts of
power, so I didn't add them and figured we could add them later if needed.


Evan Green (3):
  dt-bindings: mmc: sdhci-msm: Clarify register requirements
  arm64: dts: qcom: sdm845: Add SD nodes
  arm64: dts: qcom: sdm845: Add SD nodes for sdm845-mtp

 .../devicetree/bindings/mmc/sdhci-msm.txt     |  2 +-
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts       | 53 +++++++++++++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 15 ++++++
 3 files changed, 69 insertions(+), 1 deletion(-)

-- 
2.18.1

[PATCH v1 1/3] dt-bindings: mmc: sdhci-msm: Clarify register requirements

[Evan Green]

In sdhci-msm-v5 and beyond, the MCI registers are removed, so there is only
one register region required.

Signed-off-by: Evan Green <evgreen@chromium.org>
---

 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
index b72871ac90cb2..da4edb146a983 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
@@ -25,7 +25,7 @@ Required properties:
 
 - reg: Base address and length of the register in the following order:
 	- Host controller register map (required)
-	- SD Core register map (required)
+	- SD Core register map (required for msm-v4 and below)
 - interrupts: Should contain an interrupt-specifiers for the interrupts:
 	- Host controller interrupt (required)
 - pinctrl-names: Should contain only one value - "default".
-- 
2.18.1

Re: [PATCH v1 1/3] dt-bindings: mmc: sdhci-msm: Clarify register requirements

[Ulf Hansson]

On Wed, 28 Nov 2018 at 23:34, Evan Green <evgreen@chromium.org> wrote:
>
> In sdhci-msm-v5 and beyond, the MCI registers are removed, so there is only
> one register region required.
>
> Signed-off-by: Evan Green <evgreen@chromium.org>

Applied for next, thanks!

Kind regards
Uffe

> ---
>
>  Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index b72871ac90cb2..da4edb146a983 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -25,7 +25,7 @@ Required properties:
>
>  - reg: Base address and length of the register in the following order:
>         - Host controller register map (required)
> -       - SD Core register map (required)
> +       - SD Core register map (required for msm-v4 and below)
>  - interrupts: Should contain an interrupt-specifiers for the interrupts:
>         - Host controller interrupt (required)
>  - pinctrl-names: Should contain only one value - "default".
> --
> 2.18.1
>

[PATCH v1 2/3] arm64: dts: qcom: sdm845: Add SD nodes

[Evan Green]

Add the SD controller to SDM845.

Signed-off-by: Evan Green <evgreen@chromium.org>
---

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 1419b0098cb38..bb8eacdf40910 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1078,6 +1078,21 @@
 			};
 		};
 
+		sdhc_2: sdhci@8804000 {
+			compatible = "qcom,sdm845-sdhci", "qcom,sdhci-msm-v5";
+			reg = <0x8804000 0x1000>;
+
+			interrupts = <GIC_SPI 204 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hc_irq", "pwr_irq";
+
+			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
+				 <&gcc GCC_SDCC2_APPS_CLK>;
+			clock-names = "iface", "core";
+
+			status = "disabled";
+		};
+
 		usb_1_hsphy: phy@88e2000 {
 			compatible = "qcom,sdm845-qusb2-phy";
 			reg = <0x88e2000 0x400>;
-- 
2.18.1

Re: [PATCH v1 2/3] arm64: dts: qcom: sdm845: Add SD nodes

[Doug Anderson]

Hi,

On Wed, Nov 28, 2018 at 2:34 PM Evan Green <evgreen@chromium.org> wrote:
>
> Add the SD controller to SDM845.
>
> Signed-off-by: Evan Green <evgreen@chromium.org>
> ---
>
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)

Looks right to me.  Possibly you might want to make it known in the
subject and description that this is only sdhc_2 since there are two
SD controllers in SDM845 (oddly numbered sdc2 and sdc4 in the
documentation).  sdc2 (the one you're supporting here) is the
dual-voltage one (supports higher speeds) and also the only one hooked
up on MTP I think.  ...so maybe nobody will ever use sdhc_4, but it
might be worth mentioning anyway.  ;-)

Other than that suggestion this looks good to me.

[note to Andy: please double-check that this ends up in the right sort
order when applying.  If you happen to apply the Quad SPI patch first
then "git am" might end up putting the SDHC_2 node _after_ the Quad
SPI but it should sort numerically before it]

Reviewed-by: Douglas Anderson <dianders@chromium.org>

[PATCH v1 3/3] arm64: dts: qcom: sdm845: Add SD nodes for sdm845-mtp

[Evan Green]

Enable support for the micro SD slot on the MTP.

Signed-off-by: Evan Green <evgreen@chromium.org>
---

 arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 53 +++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index d667eee4e6d03..b85e9fed14845 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -7,6 +7,7 @@
 
 /dts-v1/;
 
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
 #include "sdm845.dtsi"
 
@@ -348,6 +349,18 @@
 	clock-frequency = <400000>;
 };
 
+&sdhc_2 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data &sd_cd_odl>;
+
+	vmmc-supply = <&vreg_l21a_2p95>;
+	vqmmc-supply = <&vddpx_2>;
+
+	cd-gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
+};
+
 &qupv3_id_1 {
 	status = "okay";
 };
@@ -444,3 +457,43 @@
 		bias-pull-up;
 	};
 };
+
+&tlmm {
+	/* It seems that mmc_test reports errors if drive strength is not 16. */
+	sdc2_clk: sdc2-clk {
+		pinconf {
+			pins = "sdc2_clk";
+			bias-disable;
+			drive-strength = <16>;
+		};
+	};
+
+	sdc2_cmd: sdc2-cmd {
+		pinconf {
+			pins = "sdc2_cmd";
+			bias-pull-up;
+			drive-strength = <16>;
+		};
+	};
+
+	sdc2_data: sdc2-data {
+		pinconf {
+			pins = "sdc2_data";
+			bias-pull-up;
+			drive-strength = <16>;
+		};
+	};
+
+	sd_cd_odl: sd-cd-odl {
+		pinmux {
+			pins = "gpio126";
+			function = "gpio";
+		};
+
+		pinconf {
+			pins = "gpio126";
+			bias-pull-up;
+			drive-strength = <2>;
+		};
+	};
+};
-- 
2.18.1

Re: [PATCH v1 3/3] arm64: dts: qcom: sdm845: Add SD nodes for sdm845-mtp

[Doug Anderson]

Hi,

On Wed, Nov 28, 2018 at 2:34 PM Evan Green <evgreen@chromium.org> wrote:
> +&sdhc_2 {
> +       status = "okay";
> +
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data &sd_cd_odl>;
> +
> +       vmmc-supply = <&vreg_l21a_2p95>;
> +       vqmmc-supply = <&vddpx_2>;
> +
> +       cd-gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
> +};
> +
>  &qupv3_id_1 {

Please sort alphabetically.  "s" comes after "q".


> +       /* It seems that mmc_test reports errors if drive strength is not 16. */

Possibly put this comment right next to one of the the "drive-strength" lines?


> +       sd_cd_odl: sd-cd-odl {

"sd_cd_odl" is the name on a different board.  On sdm845 this is
'sd_card_det_n", so it'd be better to use that name.


> +               pinmux {
> +                       pins = "gpio126";
> +                       function = "gpio";
> +               };
> +
> +               pinconf {
> +                       pins = "gpio126";
> +                       bias-pull-up;
> +                       drive-strength = <2>;

Maybe leave this off?  According to Bjorn's research "drive-strength"
has no affect on inputs and this is most certainly an input.

-Doug

Re: [PATCH v1 3/3] arm64: dts: qcom: sdm845: Add SD nodes for sdm845-mtp

[Evan Green]

On Wed, Dec 5, 2018 at 1:19 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Nov 28, 2018 at 2:34 PM Evan Green <evgreen@chromium.org> wrote:
> > +&sdhc_2 {
> > +       status = "okay";
> > +
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data &sd_cd_odl>;
> > +
> > +       vmmc-supply = <&vreg_l21a_2p95>;
> > +       vqmmc-supply = <&vddpx_2>;
> > +
> > +       cd-gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
> > +};
> > +
> >  &qupv3_id_1 {
>
> Please sort alphabetically.  "s" comes after "q".

I've got A through F down cold, but those later letters are spotty :) Will fix.

>
>
> > +       /* It seems that mmc_test reports errors if drive strength is not 16. */
>
> Possibly put this comment right next to one of the the "drive-strength" lines?

Ok.

>
>
> > +       sd_cd_odl: sd-cd-odl {
>
> "sd_cd_odl" is the name on a different board.  On sdm845 this is
> 'sd_card_det_n", so it'd be better to use that name.
>
>
> > +               pinmux {
> > +                       pins = "gpio126";
> > +                       function = "gpio";
> > +               };
> > +
> > +               pinconf {
> > +                       pins = "gpio126";
> > +                       bias-pull-up;
> > +                       drive-strength = <2>;
>
> Maybe leave this off?  According to Bjorn's research "drive-strength"
> has no affect on inputs and this is most certainly an input.

Sure. Spin coming up.

>
> -Doug