[PATCH v2 0/6] arm64: dts: qcom: msm8956-loire: SDCard and USB support

[PATCH v2 0/6] arm64: dts: qcom: msm8956-loire: SDCard and USB support

[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:
- 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/

Changes since v1:
- Moved pinctrl-names before pinctrl-N (Konrad);
- Keep status=okay last in OTG node (Konrad);
- Rename `adc-chan@` node names to `channel@` (my own patch from a long
  time ago).

v1: https://lore.kernel.org/linux-arm-msm/60a40ace-d4e9-df74-88f9-4354d80efaac@linaro.org/

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

---
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

 .../dts/qcom/msm8956-sony-xperia-loire-kugo.dts    |   6 ++
 .../boot/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(+)
---
base-commit: ad5c60d66016e544c51ed98635a74073f761f45d
change-id: 20240121-msm8976-dt-73f42eeef1bc

Best regards,
-- 
Marijn Suijten <marijn.suijten@somainline.org>

[PATCH v2 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 1029f3b1bb9a..49e97ebdbb3c 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.43.0

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

[Konrad Dybcio]

On 21.01.2024 23:33, Marijn Suijten wrote:
> 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>
> ---

I hope the USB_ID thing won't mess with a possible future charger impl

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

[PATCH v2 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 085d79542e1b..a82b58cdd99c 100644
--- a/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8956-sony-xperia-loire.dtsi
@@ -76,6 +76,8 @@ &gcc {
 };
 
 &otg {
+	extcon = <&pmi8950_usb_vbus>, <&pmi8950_usb_id>;
+
 	status = "okay";
 };
 

-- 
2.43.0

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

[Konrad Dybcio]

On 21.01.2024 23:33, 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>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

[PATCH v2 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.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
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 49e97ebdbb3c..bc341849b9fe 100644
--- a/arch/arm64/boot/dts/qcom/pmi8950.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmi8950.dtsi
@@ -69,6 +69,30 @@ channel@d {
 				qcom,pre-scaling = <1 1>;
 				label = "chg_temp";
 			};
+
+			channel@e {
+				reg = <VADC_GND_REF>;
+				qcom,pre-scaling = <1 1>;
+				label = "ref_gnd";
+			};
+
+			channel@f {
+				reg = <VADC_VDD_VADC>;
+				qcom,pre-scaling = <1 1>;
+				label = "ref_vdd";
+			};
+
+			channel@43 {
+				reg = <VADC_USB_DP>;
+				qcom,pre-scaling = <1 1>;
+				label = "usb_dp";
+			};
+
+			channel@44 {
+				reg = <VADC_USB_DM>;
+				qcom,pre-scaling = <1 1>;
+				label = "usb_dm";
+			};
 		};
 
 		pmi8950_mpps: mpps@a000 {

-- 
2.43.0

[PATCH v2 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 d2bb1ada361a..1d06f9b8a0f1 100644
--- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
@@ -669,6 +669,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";
@@ -840,6 +890,11 @@ sdhc_1: mmc@7824900 {
 				 <&gcc GCC_SDCC1_APPS_CLK>,
 				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
 			clock-names = "iface", "core", "xo";
+
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&sdc1_on_state>;
+			pinctrl-1 = <&sdc1_off_state>;
+
 			status = "disabled";
 		};
 

-- 
2.43.0

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

[Konrad Dybcio]

On 21.01.2024 23:33, Marijn Suijten wrote:
> Add the pinctrl states for SDC1 and use them on sdhc_1.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---

[...]


> @@ -840,6 +890,11 @@ sdhc_1: mmc@7824900 {
>  				 <&gcc GCC_SDCC1_APPS_CLK>,
>  				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
>  			clock-names = "iface", "core", "xo";
> +
> +			pinctrl-names = "default", "sleep";
> +			pinctrl-0 = <&sdc1_on_state>;
> +			pinctrl-1 = <&sdc1_off_state>;

-names should go last

Konrad

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

[Marijn Suijten]

On 2024-02-06 19:58:50, Konrad Dybcio wrote:
> On 21.01.2024 23:33, Marijn Suijten wrote:
> > Add the pinctrl states for SDC1 and use them on sdhc_1.
> > 
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> 
> [...]
> 
> 
> > @@ -840,6 +890,11 @@ sdhc_1: mmc@7824900 {
> >  				 <&gcc GCC_SDCC1_APPS_CLK>,
> >  				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
> >  			clock-names = "iface", "core", "xo";
> > +
> > +			pinctrl-names = "default", "sleep";
> > +			pinctrl-0 = <&sdc1_on_state>;
> > +			pinctrl-1 = <&sdc1_off_state>;
> 
> -names should go last

Per the cover letter:

    - Moved pinctrl-names before pinctrl-N (Konrad);

You explicitly requested this in https://lore.kernel.org/linux-arm-msm/60a40ace-d4e9-df74-88f9-4354d80efaac@linaro.org/#t
but we also backtracked on it somewhat for consistency with other -names, and the general state of this file.

Maybe we should apply v1 of this specific patch?

- Marijn

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

[AngeloGioacchino Del Regno]

Il 06/02/24 23:36, Marijn Suijten ha scritto:
> On 2024-02-06 19:58:50, Konrad Dybcio wrote:
>> On 21.01.2024 23:33, Marijn Suijten wrote:
>>> Add the pinctrl states for SDC1 and use them on sdhc_1.
>>>
>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>> ---
>>
>> [...]
>>
>>
>>> @@ -840,6 +890,11 @@ sdhc_1: mmc@7824900 {
>>>   				 <&gcc GCC_SDCC1_APPS_CLK>,
>>>   				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
>>>   			clock-names = "iface", "core", "xo";
>>> +
>>> +			pinctrl-names = "default", "sleep";
>>> +			pinctrl-0 = <&sdc1_on_state>;
>>> +			pinctrl-1 = <&sdc1_off_state>;
>>
>> -names should go last
> 

Konrad is technically correct as per [1] but, for consistency, since all of the
other nodes in msm8976.dtsi are following that order for pinctrl, this version of
the patch is correct.

That, unless anyone wants to reorder everything in this file as per [1], which
may or may not be worth the noise.

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst

Regards,
Angelo

> Per the cover letter:
> 
>      - Moved pinctrl-names before pinctrl-N (Konrad);
> 
> You explicitly requested this in https://lore.kernel.org/linux-arm-msm/60a40ace-d4e9-df74-88f9-4354d80efaac@linaro.org/#t
> but we also backtracked on it somewhat for consistency with other -names, and the general state of this file.
> 
> Maybe we should apply v1 of this specific patch?
> 
> - Marijn

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

[Konrad Dybcio]

On 6.02.2024 23:36, Marijn Suijten wrote:
> On 2024-02-06 19:58:50, Konrad Dybcio wrote:
>> On 21.01.2024 23:33, Marijn Suijten wrote:
>>> Add the pinctrl states for SDC1 and use them on sdhc_1.
>>>
>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>> ---
>>
>> [...]
>>
>>
>>> @@ -840,6 +890,11 @@ sdhc_1: mmc@7824900 {
>>>  				 <&gcc GCC_SDCC1_APPS_CLK>,
>>>  				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
>>>  			clock-names = "iface", "core", "xo";
>>> +
>>> +			pinctrl-names = "default", "sleep";
>>> +			pinctrl-0 = <&sdc1_on_state>;
>>> +			pinctrl-1 = <&sdc1_off_state>;
>>
>> -names should go last
> 
> Per the cover letter:
> 
>     - Moved pinctrl-names before pinctrl-N (Konrad);
> 
> You explicitly requested this in https://lore.kernel.org/linux-arm-msm/60a40ace-d4e9-df74-88f9-4354d80efaac@linaro.org/#t
> but we also backtracked on it somewhat for consistency with other -names, and the general state of this file.

Agreed, but we made up our minds since 2022-12-15 ;)

Konrad

[PATCH v2 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>
---
 .../dts/qcom/msm8956-sony-xperia-loire-kugo.dts    |  6 +++
 .../boot/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 a82b58cdd99c..b0b83edd3627 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 1d06f9b8a0f1..c100f63b1883 100644
--- a/arch/arm64/boot/dts/qcom/msm8976.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
@@ -719,6 +719,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";
@@ -911,6 +951,11 @@ sdhc_2: mmc@7864900 {
 				 <&gcc GCC_SDCC2_APPS_CLK>,
 				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
 			clock-names = "iface", "core", "xo";
+
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&sdc2_on_state>;
+			pinctrl-1 = <&sdc2_off_state>;
+
 			status = "disabled";
 		};
 

-- 
2.43.0

[PATCH v2 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>
---
 arch/arm64/boot/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 b0b83edd3627..75412e37334c 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.43.0

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

[AngeloGioacchino Del Regno]

Il 21/01/24 23:33, Marijn Suijten ha scritto:
> 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>
> ---
>   arch/arm64/boot/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 b0b83edd3627..75412e37334c 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;
> +	};

Are you sure that you really don't want card detect during system suspend?

You could simply add a sdc2-cd-pins out of sdc2_{on,off}_state and add use it for
both default and sleep.

pinctrl-0 = <&sdc2_on_state>, <&sdc2_card_det_n>;
pinctrl-1 = <&sdc2_off_state>;

Cheers,
Angelo

> +};
> +
>   &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 {
> 

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

[Marijn Suijten]

On 2024-01-22 12:48:27, AngeloGioacchino Del Regno wrote:
> Il 21/01/24 23:33, Marijn Suijten ha scritto:
> > 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>
> > ---
> >   arch/arm64/boot/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 b0b83edd3627..75412e37334c 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;
> > +	};
> 
> Are you sure that you really don't want card detect during system suspend?

Does it make a difference if the rest of pinctrl and the SDHCI controller are
also turned off?

> You could simply add a sdc2-cd-pins out of sdc2_{on,off}_state and add use it for
> both default and sleep.

This sounds close to what Konrad suggested by using a new block wit its own
label rather than extending the existing state.

> pinctrl-0 = <&sdc2_on_state>, <&sdc2_card_det_n>;
> pinctrl-1 = <&sdc2_off_state>;

You said both, but it's not in pinctrl-1 here?  (And might unselect bias-pull-up
implicitly instead of explicitly selecting bias-disable via an off node?)

- Marijn

> Cheers,
> Angelo
> 
> > +};
> > +
> >   &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 {
> > 
> 
> 

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

[AngeloGioacchino Del Regno]

Il 22/01/24 14:49, Marijn Suijten ha scritto:
> On 2024-01-22 12:48:27, AngeloGioacchino Del Regno wrote:
>> Il 21/01/24 23:33, Marijn Suijten ha scritto:
>>> 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>
>>> ---
>>>    arch/arm64/boot/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 b0b83edd3627..75412e37334c 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;
>>> +	};
>>
>> Are you sure that you really don't want card detect during system suspend?
> 
> Does it make a difference if the rest of pinctrl and the SDHCI controller are
> also turned off?
> 
>> You could simply add a sdc2-cd-pins out of sdc2_{on,off}_state and add use it for
>> both default and sleep.
> 
> This sounds close to what Konrad suggested by using a new block wit its own
> label rather than extending the existing state.
> 
>> pinctrl-0 = <&sdc2_on_state>, <&sdc2_card_det_n>;
>> pinctrl-1 = <&sdc2_off_state>;
> 
> You said both, but it's not in pinctrl-1 here?  (And might unselect bias-pull-up
> implicitly instead of explicitly selecting bias-disable via an off node?)
> 

I meant to add it to both, sorry.

In any case, take the typo'ed example as a simplification of your first version :-)


> - Marijn
> 
>> Cheers,
>> Angelo
>>
>>> +};
>>> +
>>>    &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 {
>>>
>>
>>

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

[Marijn Suijten]

On 2024-01-22 15:59:37, AngeloGioacchino Del Regno wrote:
> Il 22/01/24 14:49, Marijn Suijten ha scritto:
> > On 2024-01-22 12:48:27, AngeloGioacchino Del Regno wrote:
> >> Il 21/01/24 23:33, Marijn Suijten ha scritto:
> >>> 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>
> >>> ---
> >>>    arch/arm64/boot/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 b0b83edd3627..75412e37334c 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;
> >>> +	};
> >>
> >> Are you sure that you really don't want card detect during system suspend?
> > 
> > Does it make a difference if the rest of pinctrl and the SDHCI controller are
> > also turned off?
> > 
> >> You could simply add a sdc2-cd-pins out of sdc2_{on,off}_state and add use it for
> >> both default and sleep.
> > 
> > This sounds close to what Konrad suggested by using a new block wit its own
> > label rather than extending the existing state.
> > 
> >> pinctrl-0 = <&sdc2_on_state>, <&sdc2_card_det_n>;
> >> pinctrl-1 = <&sdc2_off_state>;
> > 
> > You said both, but it's not in pinctrl-1 here?  (And might unselect bias-pull-up
> > implicitly instead of explicitly selecting bias-disable via an off node?)
> > 
> 
> I meant to add it to both, sorry.
> 
> In any case, take the typo'ed example as a simplification of your first version :-)

Okay, I'll resend a version that creates a new pinctrl node and applies it to both cases.

Unfortunately I can no longer test and confirm that it makes a difference
to have the card-detect IRQ always biased, even while the SDHCI controller
is "asleep" or off, so I'll trust your word for it.  If I remember correctly
downstream turns it off as well?

- Marijn

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

[AngeloGioacchino Del Regno]

Il 22/01/24 16:15, Marijn Suijten ha scritto:
> On 2024-01-22 15:59:37, AngeloGioacchino Del Regno wrote:
>> Il 22/01/24 14:49, Marijn Suijten ha scritto:
>>> On 2024-01-22 12:48:27, AngeloGioacchino Del Regno wrote:
>>>> Il 21/01/24 23:33, Marijn Suijten ha scritto:
>>>>> 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>
>>>>> ---
>>>>>     arch/arm64/boot/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 b0b83edd3627..75412e37334c 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;
>>>>> +	};
>>>>
>>>> Are you sure that you really don't want card detect during system suspend?
>>>
>>> Does it make a difference if the rest of pinctrl and the SDHCI controller are
>>> also turned off?
>>>
>>>> You could simply add a sdc2-cd-pins out of sdc2_{on,off}_state and add use it for
>>>> both default and sleep.
>>>
>>> This sounds close to what Konrad suggested by using a new block wit its own
>>> label rather than extending the existing state.
>>>
>>>> pinctrl-0 = <&sdc2_on_state>, <&sdc2_card_det_n>;
>>>> pinctrl-1 = <&sdc2_off_state>;
>>>
>>> You said both, but it's not in pinctrl-1 here?  (And might unselect bias-pull-up
>>> implicitly instead of explicitly selecting bias-disable via an off node?)
>>>
>>
>> I meant to add it to both, sorry.
>>
>> In any case, take the typo'ed example as a simplification of your first version :-)
> 
> Okay, I'll resend a version that creates a new pinctrl node and applies it to both cases.
> 
> Unfortunately I can no longer test and confirm that it makes a difference
> to have the card-detect IRQ always biased, even while the SDHCI controller
> is "asleep" or off, so I'll trust your word for it.  If I remember correctly
> downstream turns it off as well?

Marijn, I don't remember anything about a downstream kernel from 10 years ago..!

But yes, it's okay to do so - the worst case is that it won't wake up, but that
won't happen, as it's going to generate an interrupt and wake up the entire system
which includes the SDHCI controller.

Go on!

Cheers,
Angelo