[PATCH 0/6] arm64: dts: qcom: msm8956-loire: SDCard

[PATCH 0/6] arm64: dts: qcom: msm8956-loire: SDCard

[Marijn Suijten]

Add pinctrl nodes to enable SD Cards to work on the Sony Loire platform,
and define extcon nodes in PMI8950 to feed into the ci-hdrc driver as it
cannot figure out the presence of a USB cable (nor the desired role
based on the ID pin) on its own.  While at it, extend PMI8950 with some
more channels with now-available VADC_ register constants.

Depends on:
- Fix pm8941-misc extcon interrupt dependency assumptions:
  https://lore.kernel.org/linux-arm-msm/20220926113143.40768-1-bryan.odonoghue@linaro.org/
- dt-bindings: iio: qcom-spmi-vadc: Add definitions for USB DP/DM VADCs:
  https://lore.kernel.org/linux-arm-msm/20221111120156.48040-2-angelogioacchino.delregno@collabora.com/

Marijn Suijten (6):
  arm64: dts: qcom: pmi8950: Add USB vbus and id sensing nodes
  arm64: dts: qcom: msm8956-loire: Add usb vbus and id extcons to
    ci-hdrc
  arm64: dts: qcom: pmi8950: Add missing ADC channels
  arm64: dts: qcom: msm8976: Declare and use SDC1 pins
  arm64: dts: qcom: msm8976: Declare and use SDC2 pins
  arm64: dts: qcom: msm8956-loire: Add SD Card Detect to SDC2 pin states

 .../qcom/msm8956-sony-xperia-loire-kugo.dts   |   6 ++
 .../dts/qcom/msm8956-sony-xperia-loire.dtsi   |  25 +++++
 arch/arm64/boot/dts/qcom/msm8976.dtsi         | 100 ++++++++++++++++++
 arch/arm64/boot/dts/qcom/pmi8950.dtsi         |  38 +++++++
 4 files changed, 169 insertions(+)

--
2.39.0

[PATCH 1/6] arm64: dts: qcom: pmi8950: Add USB vbus and id sensing nodes

[Marijn Suijten]

USB sensing is performed on the PMIC, exposed as extcon nodes for use in
the relevant USB (otg) driver nodes as the hardware itself is not able
to sense USB presence (5V vbus) nor the role (ID pin).

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 arch/arm64/boot/dts/qcom/pmi8950.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pmi8950.dtsi b/arch/arm64/boot/dts/qcom/pmi8950.dtsi
index 32d27e2187e3..7a857b2f3a5a 100644
--- a/arch/arm64/boot/dts/qcom/pmi8950.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmi8950.dtsi
@@ -12,6 +12,20 @@ pmic@2 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		pmi8950_usb_id: usb-id@1100 {
+			compatible = "qcom,pm8941-misc";
+			reg = <0x1100>;
+			interrupts = <0x2 0x11 0x3 IRQ_TYPE_NONE>;
+			interrupt-names = "usb_id";
+		};
+
+		pmi8950_usb_vbus: usb-detect@1300 {
+			compatible = "qcom,pm8941-misc";
+			reg = <0x1300>;
+			interrupts = <0x2 0x13 0x2 IRQ_TYPE_NONE>;
+			interrupt-names = "usb_vbus";
+		};
+
 		pmi8950_vadc: adc@3100 {
 			compatible = "qcom,spmi-vadc";
 			reg = <0x3100>;
-- 
2.39.0

[PATCH 2/6] arm64: dts: qcom: msm8956-loire: Add usb vbus and id extcons to ci-hdrc

[Marijn Suijten]

ci-hdrc does not have the ability to detect voltage presence (5V vbus)
on the USB connector nor the role (via an ID sensing pin), and relies on
the PMIC-side charger to provide such information through an extcon
driver.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
index 67baced639c9..700583a56a0e 100644
--- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
@@ -77,6 +77,8 @@ &gcc {
 
 &otg {
 	status = "okay";
+
+	extcon = <&pmi8950_usb_vbus>, <&pmi8950_usb_id>;
 };
 
 &pm8004_spmi_regulators {
-- 
2.39.0

[PATCH 3/6] arm64: dts: qcom: pmi8950: Add missing ADC channels

[Marijn Suijten]

These seem to have previously been excluded due to either not residing
on the test board, and/or lacking VADC_USB_DP/VADC_USB_DM definitions.
Now that the channel constants are included in dt-bindings, add the
channels to DT as well.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 arch/arm64/boot/dts/qcom/pmi8950.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pmi8950.dtsi b/arch/arm64/boot/dts/qcom/pmi8950.dtsi
index 7a857b2f3a5a..42a867685275 100644
--- a/arch/arm64/boot/dts/qcom/pmi8950.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmi8950.dtsi
@@ -69,6 +69,30 @@ adc-chan@d {
 				qcom,pre-scaling = <1 1>;
 				label = "chg_temp";
 			};
+
+			adc-chan@e {
+				reg = <VADC_GND_REF>;
+				qcom,pre-scaling = <1 1>;
+				label = "ref_gnd";
+			};
+
+			adc-chan@f {
+				reg = <VADC_VDD_VADC>;
+				qcom,pre-scaling = <1 1>;
+				label = "ref_vdd";
+			};
+
+			adc-chan@43 {
+				reg = <VADC_USB_DP>;
+				qcom,pre-scaling = <1 1>;
+				label = "usb_dp";
+			};
+
+			adc-chan@44 {
+				reg = <VADC_USB_DM>;
+				qcom,pre-scaling = <1 1>;
+				label = "usb_dm";
+			};
 		};
 
 		pmi8950_mpps: mpps@a000 {
-- 
2.39.0

[PATCH 4/6] arm64: dts: qcom: msm8976: Declare and use SDC1 pins

[Marijn Suijten]

Add the pinctrl states for SDC1 and use them on sdhc_1.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 arch/arm64/boot/dts/qcom/msm8976.dtsi | 55 +++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
index 05dcb30b0779..7d4c7548882c 100644
--- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
@@ -508,6 +508,56 @@ tlmm: pinctrl@1000000 {
 			interrupt-controller;
 			#interrupt-cells = <2>;
 
+			sdc1_off_state: sdc1-off-state {
+				clk-pins {
+					pins = "sdc1_clk";
+					drive-strength = <2>;
+					bias-disable;
+				};
+
+				cmd-pins {
+					pins = "sdc1_cmd";
+					drive-strength = <2>;
+					bias-pull-up;
+				};
+
+				data-pins {
+					pins = "sdc1_data";
+					drive-strength = <2>;
+					bias-pull-up;
+				};
+
+				rclk-pins {
+					pins = "sdc1_rclk";
+					bias-pull-down;
+				};
+			};
+
+			sdc1_on_state: sdc1-on-state {
+				clk-pins {
+					pins = "sdc1_clk";
+					drive-strength = <16>;
+					bias-disable;
+				};
+
+				cmd-pins {
+					pins = "sdc1_cmd";
+					drive-strength = <10>;
+					bias-pull-up;
+				};
+
+				data-pins {
+					pins = "sdc1_data";
+					drive-strength = <10>;
+					bias-pull-up;
+				};
+
+				rclk-pins {
+					pins = "sdc1_rclk";
+					bias-pull-down;
+				};
+			};
+
 			spi1_default: spi0-default-state {
 				spi-pins {
 					pins = "gpio0", "gpio1", "gpio3";
@@ -680,6 +730,11 @@ sdhc_1: mmc@7824000 {
 				 <&gcc GCC_SDCC1_APPS_CLK>,
 				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
 			clock-names = "iface", "core", "xo";
+
+			pinctrl-0 = <&sdc1_on_state>;
+			pinctrl-1 = <&sdc1_off_state>;
+			pinctrl-names = "default", "sleep";
+
 			status = "disabled";
 		};
 
-- 
2.39.0

[PATCH 5/6] arm64: dts: qcom: msm8976: Declare and use SDC2 pins

[Marijn Suijten]

Add the pinctrl states for SDC2 and use them on sdhc_2 to support SD
Cards on the currently mainlined Sony Loire platform.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 .../qcom/msm8956-sony-xperia-loire-kugo.dts   |  6 +++
 .../dts/qcom/msm8956-sony-xperia-loire.dtsi   |  6 +++
 arch/arm64/boot/dts/qcom/msm8976.dtsi         | 45 +++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
index 3fb8e23e4330..9178943e2ee1 100644
--- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
+++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
@@ -33,3 +33,9 @@ &pm8950_l1 {
 	regulator-min-microvolt = <1100000>;
 	regulator-max-microvolt = <1300000>;
 };
+
+&sdc2_on_state {
+	data-pins {
+		drive-strength = <8>;
+	};
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
index 700583a56a0e..2253fb05f1c9 100644
--- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
@@ -264,6 +264,12 @@ &sdhc_1 {
 	status = "okay";
 };
 
+&sdc2_on_state {
+	clk-pins {
+		drive-strength = <10>;
+	};
+};
+
 &sdhc_2 {
 	bus-width = <4>;
 	cd-gpios = <&tlmm 100 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
index 7d4c7548882c..f3371eaa2940 100644
--- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
@@ -558,6 +558,46 @@ rclk-pins {
 				};
 			};
 
+			sdc2_off_state: sdc2-off-state {
+				clk-pins {
+					pins = "sdc2_clk";
+					drive-strength = <2>;
+					bias-disable;
+				};
+
+				cmd-pins {
+					pins = "sdc2_cmd";
+					drive-strength = <2>;
+					bias-pull-up;
+				};
+
+				data-pins {
+					pins = "sdc2_data";
+					drive-strength = <2>;
+					bias-pull-up;
+				};
+			};
+
+			sdc2_on_state: sdc2-on-state {
+				clk-pins {
+					pins = "sdc2_clk";
+					drive-strength = <16>;
+					bias-disable;
+				};
+
+				cmd-pins {
+					pins = "sdc2_cmd";
+					drive-strength = <10>;
+					bias-pull-up;
+				};
+
+				data-pins {
+					pins = "sdc2_data";
+					drive-strength = <10>;
+					bias-pull-up;
+				};
+			};
+
 			spi1_default: spi0-default-state {
 				spi-pins {
 					pins = "gpio0", "gpio1", "gpio3";
@@ -751,6 +791,11 @@ sdhc_2: mmc@7864000 {
 				 <&gcc GCC_SDCC2_APPS_CLK>,
 				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
 			clock-names = "iface", "core", "xo";
+
+			pinctrl-0 = <&sdc2_on_state>;
+			pinctrl-1 = <&sdc2_off_state>;
+			pinctrl-names = "default", "sleep";
+
 			status = "disabled";
 		};
 
-- 
2.39.0

[PATCH 6/6] arm64: dts: qcom: msm8956-loire: Add SD Card Detect to SDC2 pin states

[Marijn Suijten]

In addition to the SDC2 pins, set the SD Card Detect pin in a sane state
to be used as an interrupt when an SD Card is slotted in or removed.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 .../dts/qcom/msm8956-sony-xperia-loire.dtsi     | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
index 2253fb05f1c9..a91b0af3e8c6 100644
--- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
@@ -264,10 +264,27 @@ &sdhc_1 {
 	status = "okay";
 };
 
+&sdc2_off_state {
+	sd-cd-pins {
+		pins = "gpio100";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+};
+
 &sdc2_on_state {
 	clk-pins {
 		drive-strength = <10>;
 	};
+
+	sd-cd-pins {
+		pins = "gpio100";
+		function = "gpio";
+		drive-strength = <2>;
+		input-enable;
+		bias-pull-up;
+	};
 };
 
 &sdhc_2 {
-- 
2.39.0

Re: [PATCH 0/6] arm64: dts: qcom: msm8956-loire: SDCard

[Marijn Suijten]

... and USB support

On 2022-12-15 00:20:43, Marijn Suijten wrote:
> Add pinctrl nodes to enable SD Cards to work on the Sony Loire platform,
> and define extcon nodes in PMI8950 to feed into the ci-hdrc driver as it
> cannot figure out the presence of a USB cable (nor the desired role
> based on the ID pin) on its own.  While at it, extend PMI8950 with some
> more channels with now-available VADC_ register constants.
> 
> Depends on:
> - Fix pm8941-misc extcon interrupt dependency assumptions:
>   https://lore.kernel.org/linux-arm-msm/20220926113143.40768-1-bryan.odonoghue@linaro.org/
> - dt-bindings: iio: qcom-spmi-vadc: Add definitions for USB DP/DM VADCs:
>   https://lore.kernel.org/linux-arm-msm/20221111120156.48040-2-angelogioacchino.delregno@collabora.com/
> 
> Marijn Suijten (6):
>   arm64: dts: qcom: pmi8950: Add USB vbus and id sensing nodes
>   arm64: dts: qcom: msm8956-loire: Add usb vbus and id extcons to
>     ci-hdrc
>   arm64: dts: qcom: pmi8950: Add missing ADC channels
>   arm64: dts: qcom: msm8976: Declare and use SDC1 pins
>   arm64: dts: qcom: msm8976: Declare and use SDC2 pins
>   arm64: dts: qcom: msm8956-loire: Add SD Card Detect to SDC2 pin states
> 
>  .../qcom/msm8956-sony-xperia-loire-kugo.dts   |   6 ++
>  .../dts/qcom/msm8956-sony-xperia-loire.dtsi   |  25 +++++
>  arch/arm64/boot/dts/qcom/msm8976.dtsi         | 100 ++++++++++++++++++
>  arch/arm64/boot/dts/qcom/pmi8950.dtsi         |  38 +++++++
>  4 files changed, 169 insertions(+)
> 
> --
> 2.39.0
> 

Re: [PATCH 2/6] arm64: dts: qcom: msm8956-loire: Add usb vbus and id extcons to ci-hdrc

[Konrad Dybcio]

On 15.12.2022 00:20, Marijn Suijten wrote:
> ci-hdrc does not have the ability to detect voltage presence (5V vbus)
> on the USB connector nor the role (via an ID sensing pin), and relies on
> the PMIC-side charger to provide such information through an extcon
> driver.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
> index 67baced639c9..700583a56a0e 100644
> --- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
> @@ -77,6 +77,8 @@ &gcc {
>  
>  &otg {
>  	status = "okay";
> +
> +	extcon = <&pmi8950_usb_vbus>, <&pmi8950_usb_id>;
Please add it above to keep status as the last property.

Konrad
>  };
>  
>  &pm8004_spmi_regulators {

Re: [PATCH 3/6] arm64: dts: qcom: pmi8950: Add missing ADC channels

[Konrad Dybcio]

On 15.12.2022 00:20, Marijn Suijten wrote:
> These seem to have previously been excluded due to either not residing
> on the test board, and/or lacking VADC_USB_DP/VADC_USB_DM definitions.
> Now that the channel constants are included in dt-bindings, add the
> channels to DT as well.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/pmi8950.dtsi | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pmi8950.dtsi b/arch/arm64/boot/dts/qcom/pmi8950.dtsi
> index 7a857b2f3a5a..42a867685275 100644
> --- a/arch/arm64/boot/dts/qcom/pmi8950.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pmi8950.dtsi
> @@ -69,6 +69,30 @@ adc-chan@d {
>  				qcom,pre-scaling = <1 1>;
>  				label = "chg_temp";
>  			};
> +
> +			adc-chan@e {
> +				reg = <VADC_GND_REF>;
> +				qcom,pre-scaling = <1 1>;
> +				label = "ref_gnd";
> +			};
> +
> +			adc-chan@f {
> +				reg = <VADC_VDD_VADC>;
> +				qcom,pre-scaling = <1 1>;
> +				label = "ref_vdd";
> +			};
> +
> +			adc-chan@43 {
> +				reg = <VADC_USB_DP>;
> +				qcom,pre-scaling = <1 1>;
> +				label = "usb_dp";
> +			};
> +
> +			adc-chan@44 {
> +				reg = <VADC_USB_DM>;
> +				qcom,pre-scaling = <1 1>;
> +				label = "usb_dm";
> +			};
>  		};
>  
>  		pmi8950_mpps: mpps@a000 {

Re: [PATCH 4/6] arm64: dts: qcom: msm8976: Declare and use SDC1 pins

[Konrad Dybcio]

On 15.12.2022 00:20, Marijn Suijten wrote:
> Add the pinctrl states for SDC1 and use them on sdhc_1.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8976.dtsi | 55 +++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> index 05dcb30b0779..7d4c7548882c 100644
> --- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> @@ -508,6 +508,56 @@ tlmm: pinctrl@1000000 {
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
>  
> +			sdc1_off_state: sdc1-off-state {
> +				clk-pins {
> +					pins = "sdc1_clk";
> +					drive-strength = <2>;
> +					bias-disable;
> +				};
> +
> +				cmd-pins {
> +					pins = "sdc1_cmd";
> +					drive-strength = <2>;
> +					bias-pull-up;
> +				};
> +
> +				data-pins {
> +					pins = "sdc1_data";
> +					drive-strength = <2>;
> +					bias-pull-up;
> +				};
> +
> +				rclk-pins {
> +					pins = "sdc1_rclk";
> +					bias-pull-down;
> +				};
> +			};
> +
> +			sdc1_on_state: sdc1-on-state {
> +				clk-pins {
> +					pins = "sdc1_clk";
> +					drive-strength = <16>;
> +					bias-disable;
> +				};
> +
> +				cmd-pins {
> +					pins = "sdc1_cmd";
> +					drive-strength = <10>;
> +					bias-pull-up;
> +				};
> +
> +				data-pins {
> +					pins = "sdc1_data";
> +					drive-strength = <10>;
> +					bias-pull-up;
> +				};
> +
> +				rclk-pins {
> +					pins = "sdc1_rclk";
> +					bias-pull-down;
> +				};
> +			};
> +
>  			spi1_default: spi0-default-state {
>  				spi-pins {
>  					pins = "gpio0", "gpio1", "gpio3";
> @@ -680,6 +730,11 @@ sdhc_1: mmc@7824000 {
>  				 <&gcc GCC_SDCC1_APPS_CLK>,
>  				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
>  			clock-names = "iface", "core", "xo";
> +
> +			pinctrl-0 = <&sdc1_on_state>;
> +			pinctrl-1 = <&sdc1_off_state>;
> +			pinctrl-names = "default", "sleep";
pinctrl-names usually goes before pinctrl-N

Konrad
> +
>  			status = "disabled";
>  		};
>  

Re: [PATCH 5/6] arm64: dts: qcom: msm8976: Declare and use SDC2 pins

[Konrad Dybcio]

On 15.12.2022 00:20, Marijn Suijten wrote:
> Add the pinctrl states for SDC2 and use them on sdhc_2 to support SD
> Cards on the currently mainlined Sony Loire platform.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  .../qcom/msm8956-sony-xperia-loire-kugo.dts   |  6 +++
>  .../dts/qcom/msm8956-sony-xperia-loire.dtsi   |  6 +++
>  arch/arm64/boot/dts/qcom/msm8976.dtsi         | 45 +++++++++++++++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
> index 3fb8e23e4330..9178943e2ee1 100644
> --- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
> @@ -33,3 +33,9 @@ &pm8950_l1 {
>  	regulator-min-microvolt = <1100000>;
>  	regulator-max-microvolt = <1300000>;
>  };
> +
> +&sdc2_on_state {
> +	data-pins {
> +		drive-strength = <8>;
> +	};
> +};
You can add a label to the data-pins subnode.


> diff --git a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
> index 700583a56a0e..2253fb05f1c9 100644
> --- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
> @@ -264,6 +264,12 @@ &sdhc_1 {
>  	status = "okay";
>  };
>  
> +&sdc2_on_state {
> +	clk-pins {
> +		drive-strength = <10>;
> +	};
> +};
> +
>  &sdhc_2 {
>  	bus-width = <4>;
>  	cd-gpios = <&tlmm 100 GPIO_ACTIVE_HIGH>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> index 7d4c7548882c..f3371eaa2940 100644
> --- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> @@ -558,6 +558,46 @@ rclk-pins {
>  				};
>  			};
>  
> +			sdc2_off_state: sdc2-off-state {
> +				clk-pins {
> +					pins = "sdc2_clk";
> +					drive-strength = <2>;
> +					bias-disable;
> +				};
> +
> +				cmd-pins {
> +					pins = "sdc2_cmd";
> +					drive-strength = <2>;
> +					bias-pull-up;
> +				};
> +
> +				data-pins {
> +					pins = "sdc2_data";
> +					drive-strength = <2>;
> +					bias-pull-up;
> +				};
> +			};
> +
> +			sdc2_on_state: sdc2-on-state {
> +				clk-pins {
> +					pins = "sdc2_clk";
> +					drive-strength = <16>;
> +					bias-disable;
> +				};
> +
> +				cmd-pins {
> +					pins = "sdc2_cmd";
> +					drive-strength = <10>;
> +					bias-pull-up;
> +				};
> +
> +				data-pins {
> +					pins = "sdc2_data";
> +					drive-strength = <10>;
> +					bias-pull-up;
> +				};
> +			};
> +
>  			spi1_default: spi0-default-state {
>  				spi-pins {
>  					pins = "gpio0", "gpio1", "gpio3";
> @@ -751,6 +791,11 @@ sdhc_2: mmc@7864000 {
>  				 <&gcc GCC_SDCC2_APPS_CLK>,
>  				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
>  			clock-names = "iface", "core", "xo";
> +
> +			pinctrl-0 = <&sdc2_on_state>;
> +			pinctrl-1 = <&sdc2_off_state>;
> +			pinctrl-names = "default", "sleep";
Ditto (prev patch)

Konrad
> +
>  			status = "disabled";
>  		};
>  

Re: [PATCH 4/6] arm64: dts: qcom: msm8976: Declare and use SDC1 pins

[Marijn Suijten]

On 2022-12-15 14:19:41, Konrad Dybcio wrote:
> 
> 
> On 15.12.2022 00:20, Marijn Suijten wrote:
> > Add the pinctrl states for SDC1 and use them on sdhc_1.
> > 
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> >  arch/arm64/boot/dts/qcom/msm8976.dtsi | 55 +++++++++++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> > index 05dcb30b0779..7d4c7548882c 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> > @@ -508,6 +508,56 @@ tlmm: pinctrl@1000000 {
> >  			interrupt-controller;
> >  			#interrupt-cells = <2>;
> >  
> > +			sdc1_off_state: sdc1-off-state {
> > +				clk-pins {
> > +					pins = "sdc1_clk";
> > +					drive-strength = <2>;
> > +					bias-disable;
> > +				};
> > +
> > +				cmd-pins {
> > +					pins = "sdc1_cmd";
> > +					drive-strength = <2>;
> > +					bias-pull-up;
> > +				};
> > +
> > +				data-pins {
> > +					pins = "sdc1_data";
> > +					drive-strength = <2>;
> > +					bias-pull-up;
> > +				};
> > +
> > +				rclk-pins {
> > +					pins = "sdc1_rclk";
> > +					bias-pull-down;
> > +				};
> > +			};
> > +
> > +			sdc1_on_state: sdc1-on-state {
> > +				clk-pins {
> > +					pins = "sdc1_clk";
> > +					drive-strength = <16>;
> > +					bias-disable;
> > +				};
> > +
> > +				cmd-pins {
> > +					pins = "sdc1_cmd";
> > +					drive-strength = <10>;
> > +					bias-pull-up;
> > +				};
> > +
> > +				data-pins {
> > +					pins = "sdc1_data";
> > +					drive-strength = <10>;
> > +					bias-pull-up;
> > +				};
> > +
> > +				rclk-pins {
> > +					pins = "sdc1_rclk";
> > +					bias-pull-down;
> > +				};
> > +			};
> > +
> >  			spi1_default: spi0-default-state {
> >  				spi-pins {
> >  					pins = "gpio0", "gpio1", "gpio3";
> > @@ -680,6 +730,11 @@ sdhc_1: mmc@7824000 {
> >  				 <&gcc GCC_SDCC1_APPS_CLK>,
> >  				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
> >  			clock-names = "iface", "core", "xo";
> > +
> > +			pinctrl-0 = <&sdc1_on_state>;
> > +			pinctrl-1 = <&sdc1_off_state>;
> > +			pinctrl-names = "default", "sleep";
> pinctrl-names usually goes before pinctrl-N

I thought I had seen them _after_ nowadays, same for reg-names,
phy-names, interrupt-names and clock-names.  What is it?

Regardless, I'd rather keep this consistent across this file (sdc2 also
has it after, same for other *-names) and correct it at once in a
separate patch, if someone really cares.

But really, we should have a checker/autoformatter for these "rules",
instead of all this manual back-and-forth (is this order already set in
stone under Documentation/ or something?).

- Marijn

Re: [PATCH 4/6] arm64: dts: qcom: msm8976: Declare and use SDC1 pins

[Marijn Suijten]

On 2022-12-15 22:00:12, Marijn Suijten wrote:
> On 2022-12-15 14:19:41, Konrad Dybcio wrote:
> > 
> > 
> > On 15.12.2022 00:20, Marijn Suijten wrote:
> > > Add the pinctrl states for SDC1 and use them on sdhc_1.
> > > 
> > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/msm8976.dtsi | 55 +++++++++++++++++++++++++++
> > >  1 file changed, 55 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> > > index 05dcb30b0779..7d4c7548882c 100644
> > > --- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> > > @@ -508,6 +508,56 @@ tlmm: pinctrl@1000000 {
> > >  			interrupt-controller;
> > >  			#interrupt-cells = <2>;
> > >  
> > > +			sdc1_off_state: sdc1-off-state {
> > > +				clk-pins {
> > > +					pins = "sdc1_clk";
> > > +					drive-strength = <2>;
> > > +					bias-disable;
> > > +				};
> > > +
> > > +				cmd-pins {
> > > +					pins = "sdc1_cmd";
> > > +					drive-strength = <2>;
> > > +					bias-pull-up;
> > > +				};
> > > +
> > > +				data-pins {
> > > +					pins = "sdc1_data";
> > > +					drive-strength = <2>;
> > > +					bias-pull-up;
> > > +				};
> > > +
> > > +				rclk-pins {
> > > +					pins = "sdc1_rclk";
> > > +					bias-pull-down;
> > > +				};
> > > +			};
> > > +
> > > +			sdc1_on_state: sdc1-on-state {
> > > +				clk-pins {
> > > +					pins = "sdc1_clk";
> > > +					drive-strength = <16>;
> > > +					bias-disable;
> > > +				};
> > > +
> > > +				cmd-pins {
> > > +					pins = "sdc1_cmd";
> > > +					drive-strength = <10>;
> > > +					bias-pull-up;
> > > +				};
> > > +
> > > +				data-pins {
> > > +					pins = "sdc1_data";
> > > +					drive-strength = <10>;
> > > +					bias-pull-up;
> > > +				};
> > > +
> > > +				rclk-pins {
> > > +					pins = "sdc1_rclk";
> > > +					bias-pull-down;
> > > +				};
> > > +			};
> > > +
> > >  			spi1_default: spi0-default-state {
> > >  				spi-pins {
> > >  					pins = "gpio0", "gpio1", "gpio3";
> > > @@ -680,6 +730,11 @@ sdhc_1: mmc@7824000 {
> > >  				 <&gcc GCC_SDCC1_APPS_CLK>,
> > >  				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
> > >  			clock-names = "iface", "core", "xo";
> > > +
> > > +			pinctrl-0 = <&sdc1_on_state>;
> > > +			pinctrl-1 = <&sdc1_off_state>;
> > > +			pinctrl-names = "default", "sleep";
> > pinctrl-names usually goes before pinctrl-N
> 
> I thought I had seen them _after_ nowadays, same for reg-names,
> phy-names, interrupt-names and clock-names.  What is it?
> 
> Regardless, I'd rather keep this consistent across this file (sdc2 also
> has it after, same for other *-names)

Excuse me, I was looking at sm6125 DT while writing this, sdc2 for
msm8976 is introduced _and used by sdhc_2 in a followup patch.

The other points still stand though, everything has -names last.

- Marijn

> and correct it at once in a
> separate patch, if someone really cares.
> 
> But really, we should have a checker/autoformatter for these "rules",
> instead of all this manual back-and-forth (is this order already set in
> stone under Documentation/ or something?).
> 
> - Marijn

Re: [PATCH 5/6] arm64: dts: qcom: msm8976: Declare and use SDC2 pins

[Marijn Suijten]

On 2022-12-15 14:20:30, Konrad Dybcio wrote:
> 
> 
> On 15.12.2022 00:20, Marijn Suijten wrote:
> > Add the pinctrl states for SDC2 and use them on sdhc_2 to support SD
> > Cards on the currently mainlined Sony Loire platform.
> > 
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> >  .../qcom/msm8956-sony-xperia-loire-kugo.dts   |  6 +++
> >  .../dts/qcom/msm8956-sony-xperia-loire.dtsi   |  6 +++
> >  arch/arm64/boot/dts/qcom/msm8976.dtsi         | 45 +++++++++++++++++++
> >  3 files changed, 57 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
> > index 3fb8e23e4330..9178943e2ee1 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
> > +++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire-kugo.dts
> > @@ -33,3 +33,9 @@ &pm8950_l1 {
> >  	regulator-min-microvolt = <1100000>;
> >  	regulator-max-microvolt = <1300000>;
> >  };
> > +
> > +&sdc2_on_state {
> > +	data-pins {
> > +		drive-strength = <8>;
> > +	};
> > +};
> You can add a label to the data-pins subnode.

I prefer this simplicity as the sdc2_*_state labels are already
available and will be extended later with sd-cd-pins regardless. I'll
change this if others share the same opinion.

However, you're also right this way of writing makes it unclear to the
reader whether data-pins already exists or has to exist; that would not
be the case when a label is explicitly assigned to data-pins.

- Marijn

Re: [PATCH 4/6] arm64: dts: qcom: msm8976: Declare and use SDC1 pins

[Konrad Dybcio]

On 15.12.2022 22:02, Marijn Suijten wrote:
> On 2022-12-15 22:00:12, Marijn Suijten wrote:
>> On 2022-12-15 14:19:41, Konrad Dybcio wrote:
>>>
>>>
>>> On 15.12.2022 00:20, Marijn Suijten wrote:
>>>> Add the pinctrl states for SDC1 and use them on sdhc_1.
>>>>
>>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/msm8976.dtsi | 55 +++++++++++++++++++++++++++
>>>>  1 file changed, 55 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
>>>> index 05dcb30b0779..7d4c7548882c 100644
>>>> --- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
>>>> @@ -508,6 +508,56 @@ tlmm: pinctrl@1000000 {
>>>>  			interrupt-controller;
>>>>  			#interrupt-cells = <2>;
>>>>  
>>>> +			sdc1_off_state: sdc1-off-state {
>>>> +				clk-pins {
>>>> +					pins = "sdc1_clk";
>>>> +					drive-strength = <2>;
>>>> +					bias-disable;
>>>> +				};
>>>> +
>>>> +				cmd-pins {
>>>> +					pins = "sdc1_cmd";
>>>> +					drive-strength = <2>;
>>>> +					bias-pull-up;
>>>> +				};
>>>> +
>>>> +				data-pins {
>>>> +					pins = "sdc1_data";
>>>> +					drive-strength = <2>;
>>>> +					bias-pull-up;
>>>> +				};
>>>> +
>>>> +				rclk-pins {
>>>> +					pins = "sdc1_rclk";
>>>> +					bias-pull-down;
>>>> +				};
>>>> +			};
>>>> +
>>>> +			sdc1_on_state: sdc1-on-state {
>>>> +				clk-pins {
>>>> +					pins = "sdc1_clk";
>>>> +					drive-strength = <16>;
>>>> +					bias-disable;
>>>> +				};
>>>> +
>>>> +				cmd-pins {
>>>> +					pins = "sdc1_cmd";
>>>> +					drive-strength = <10>;
>>>> +					bias-pull-up;
>>>> +				};
>>>> +
>>>> +				data-pins {
>>>> +					pins = "sdc1_data";
>>>> +					drive-strength = <10>;
>>>> +					bias-pull-up;
>>>> +				};
>>>> +
>>>> +				rclk-pins {
>>>> +					pins = "sdc1_rclk";
>>>> +					bias-pull-down;
>>>> +				};
>>>> +			};
>>>> +
>>>>  			spi1_default: spi0-default-state {
>>>>  				spi-pins {
>>>>  					pins = "gpio0", "gpio1", "gpio3";
>>>> @@ -680,6 +730,11 @@ sdhc_1: mmc@7824000 {
>>>>  				 <&gcc GCC_SDCC1_APPS_CLK>,
>>>>  				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
>>>>  			clock-names = "iface", "core", "xo";
>>>> +
>>>> +			pinctrl-0 = <&sdc1_on_state>;
>>>> +			pinctrl-1 = <&sdc1_off_state>;
>>>> +			pinctrl-names = "default", "sleep";
>>> pinctrl-names usually goes before pinctrl-N
>>
>> I thought I had seen them _after_ nowadays, same for reg-names,
>> phy-names, interrupt-names and clock-names.  What is it?
>>
>> Regardless, I'd rather keep this consistent across this file (sdc2 also
>> has it after, same for other *-names)
> 
> Excuse me, I was looking at sm6125 DT while writing this, sdc2 for
> msm8976 is introduced _and used by sdhc_2 in a followup patch.
> 
> The other points still stand though, everything has -names last.
Hm, that's a good point, perhaps we should apply it to pinctrl-
too then. I like this.

Konrad
> 
> - Marijn
> 
>> and correct it at once in a
>> separate patch, if someone really cares.
>>
>> But really, we should have a checker/autoformatter for these "rules",
>> instead of all this manual back-and-forth (is this order already set in
>> stone under Documentation/ or something?).
>>
>> - Marijn