diff mbox series

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

Message ID 20240121-msm8976-dt-v2-6-7b186a02dc72@somainline.org (mailing list archive)
State Changes Requested
Headers show
Series arm64: dts: qcom: msm8956-loire: SDCard and USB support | expand

Commit Message

Marijn Suijten Jan. 21, 2024, 10:33 p.m. UTC
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(+)

Comments

AngeloGioacchino Del Regno Jan. 22, 2024, 11:48 a.m. UTC | #1
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 {
>
Marijn Suijten Jan. 22, 2024, 1:49 p.m. UTC | #2
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 {
> > 
> 
>
AngeloGioacchino Del Regno Jan. 22, 2024, 2:59 p.m. UTC | #3
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 {
>>>
>>
>>
Marijn Suijten Jan. 22, 2024, 3:15 p.m. UTC | #4
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
AngeloGioacchino Del Regno Jan. 29, 2024, 11:05 a.m. UTC | #5
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
diff mbox series

Patch

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 {