* [PATCH] arm64: dts: qcom: sm6115: Move SDHC node(s)'s 'pinctrl' properties to dts
@ 2022-12-20 11:36 Bhupesh Sharma
2022-12-21 22:10 ` Marijn Suijten
0 siblings, 1 reply; 3+ messages in thread
From: Bhupesh Sharma @ 2022-12-20 11:36 UTC (permalink / raw)
To: linux-arm-msm
Cc: devicetree, agross, bhupesh.sharma, bhupesh.linux, linux-kernel,
robh+dt, krzysztof.kozlowski, konrad.dybcio, andersson
Normally the 'pinctrl' properties of a SDHC controller and the
chip detect pin settings are dependent on the type of the slots
(for e.g uSD card slot), regulators and GPIO(s) available on the
board(s).
So, move the same from the sm6115 dtsi file to the respective
board file(s).
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
.../boot/dts/qcom/sm4250-oneplus-billie2.dts | 10 +++++++++
arch/arm64/boot/dts/qcom/sm6115.dtsi | 22 -------------------
2 files changed, 10 insertions(+), 22 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
index a3f1c7c41fd73..329eb496bbc5f 100644
--- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
+++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
@@ -202,12 +202,22 @@ &sdhc_2 {
vqmmc-supply = <&vreg_l5a>;
cd-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&sdc2_state_on &sdc2_card_det_n>;
+ pinctrl-1 = <&sdc2_state_off &sdc2_card_det_n>;
status = "okay";
};
&tlmm {
gpio-reserved-ranges = <14 4>;
+
+ sdc2_card_det_n: sd-card-det-n-state {
+ pins = "gpio88";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
};
&ufs_mem_hc {
diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index 572bf04adf906..6be763d39870d 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -518,13 +518,6 @@ data-pins {
bias-pull-up;
drive-strength = <10>;
};
-
- sd-cd-pins {
- pins = "gpio88";
- function = "gpio";
- bias-pull-up;
- drive-strength = <2>;
- };
};
sdc2_state_off: sdc2-off-state {
@@ -545,13 +538,6 @@ data-pins {
bias-pull-up;
drive-strength = <2>;
};
-
- sd-cd-pins {
- pins = "gpio88";
- function = "gpio";
- bias-disable;
- drive-strength = <2>;
- };
};
};
@@ -652,10 +638,6 @@ sdhc_1: mmc@4744000 {
<&gcc GCC_SDCC1_ICE_CORE_CLK>;
clock-names = "iface", "core", "xo", "ice";
- pinctrl-0 = <&sdc1_state_on>;
- pinctrl-1 = <&sdc1_state_off>;
- pinctrl-names = "default", "sleep";
-
bus-width = <8>;
status = "disabled";
};
@@ -672,10 +654,6 @@ sdhc_2: mmc@4784000 {
clocks = <&gcc GCC_SDCC2_AHB_CLK>, <&gcc GCC_SDCC2_APPS_CLK>, <&xo_board>;
clock-names = "iface", "core", "xo";
- pinctrl-0 = <&sdc2_state_on>;
- pinctrl-1 = <&sdc2_state_off>;
- pinctrl-names = "default", "sleep";
-
power-domains = <&rpmpd SM6115_VDDCX>;
operating-points-v2 = <&sdhc2_opp_table>;
iommus = <&apps_smmu 0x00a0 0x0>;
--
2.38.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] arm64: dts: qcom: sm6115: Move SDHC node(s)'s 'pinctrl' properties to dts
2022-12-20 11:36 [PATCH] arm64: dts: qcom: sm6115: Move SDHC node(s)'s 'pinctrl' properties to dts Bhupesh Sharma
@ 2022-12-21 22:10 ` Marijn Suijten
2023-02-08 9:51 ` Bhupesh Sharma
0 siblings, 1 reply; 3+ messages in thread
From: Marijn Suijten @ 2022-12-21 22:10 UTC (permalink / raw)
To: Bhupesh Sharma
Cc: linux-arm-msm, devicetree, agross, bhupesh.linux, linux-kernel,
robh+dt, krzysztof.kozlowski, konrad.dybcio, andersson
On 2022-12-20 17:06:16, Bhupesh Sharma wrote:
> Normally the 'pinctrl' properties of a SDHC controller and the
> chip detect pin settings are dependent on the type of the slots
> (for e.g uSD card slot), regulators and GPIO(s) available on the
> board(s).
I always thought it was okay to give the `sdcX_*` pins to the sdhc_X
node unconditionally in SoC DTSI, and leave board-dependent card-detect
(if this SDHCI slot is even used for removable cards) pins to the board
DTS.
> So, move the same from the sm6115 dtsi file to the respective
> board file(s).
>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
Thanks for cleaning this up, we're already using this pattern in quite a
few SoCs now.
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Though...
> ---
> .../boot/dts/qcom/sm4250-oneplus-billie2.dts | 10 +++++++++
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 22 -------------------
> 2 files changed, 10 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> index a3f1c7c41fd73..329eb496bbc5f 100644
> --- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> @@ -202,12 +202,22 @@ &sdhc_2 {
> vqmmc-supply = <&vreg_l5a>;
>
> cd-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&sdc2_state_on &sdc2_card_det_n>;
> + pinctrl-1 = <&sdc2_state_off &sdc2_card_det_n>;
>
> status = "okay";
> };
>
> &tlmm {
> gpio-reserved-ranges = <14 4>;
> +
> + sdc2_card_det_n: sd-card-det-n-state {
> + pins = "gpio88";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-up;
Note that SoC DTSI uses bias-disable in the off state.
> + };
> };
>
> &ufs_mem_hc {
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 572bf04adf906..6be763d39870d 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -518,13 +518,6 @@ data-pins {
> bias-pull-up;
> drive-strength = <10>;
> };
> -
> - sd-cd-pins {
> - pins = "gpio88";
> - function = "gpio";
> - bias-pull-up;
> - drive-strength = <2>;
> - };
> };
>
> sdc2_state_off: sdc2-off-state {
> @@ -545,13 +538,6 @@ data-pins {
> bias-pull-up;
> drive-strength = <2>;
> };
> -
> - sd-cd-pins {
> - pins = "gpio88";
> - function = "gpio";
> - bias-disable;
> - drive-strength = <2>;
> - };
> };
> };
>
> @@ -652,10 +638,6 @@ sdhc_1: mmc@4744000 {
> <&gcc GCC_SDCC1_ICE_CORE_CLK>;
> clock-names = "iface", "core", "xo", "ice";
>
> - pinctrl-0 = <&sdc1_state_on>;
> - pinctrl-1 = <&sdc1_state_off>;
> - pinctrl-names = "default", "sleep";
> -
You can probably leave these and below? Only boards needing to extend
pinctrl with board-specific SDCard pins would have to update/override
these properties that way?
- Marijn
> bus-width = <8>;
> status = "disabled";
> };
> @@ -672,10 +654,6 @@ sdhc_2: mmc@4784000 {
> clocks = <&gcc GCC_SDCC2_AHB_CLK>, <&gcc GCC_SDCC2_APPS_CLK>, <&xo_board>;
> clock-names = "iface", "core", "xo";
>
> - pinctrl-0 = <&sdc2_state_on>;
> - pinctrl-1 = <&sdc2_state_off>;
> - pinctrl-names = "default", "sleep";
> -
> power-domains = <&rpmpd SM6115_VDDCX>;
> operating-points-v2 = <&sdhc2_opp_table>;
> iommus = <&apps_smmu 0x00a0 0x0>;
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] arm64: dts: qcom: sm6115: Move SDHC node(s)'s 'pinctrl' properties to dts
2022-12-21 22:10 ` Marijn Suijten
@ 2023-02-08 9:51 ` Bhupesh Sharma
0 siblings, 0 replies; 3+ messages in thread
From: Bhupesh Sharma @ 2023-02-08 9:51 UTC (permalink / raw)
To: Marijn Suijten, andersson
Cc: linux-arm-msm, devicetree, agross, bhupesh.linux, linux-kernel,
robh+dt, krzysztof.kozlowski, konrad.dybcio
Thanks Marjin for the review. Sorry for the delay in replying to your
comments.
On 12/22/22 3:40 AM, Marijn Suijten wrote:
> On 2022-12-20 17:06:16, Bhupesh Sharma wrote:
>> Normally the 'pinctrl' properties of a SDHC controller and the
>> chip detect pin settings are dependent on the type of the slots
>> (for e.g uSD card slot), regulators and GPIO(s) available on the
>> board(s).
>
> I always thought it was okay to give the `sdcX_*` pins to the sdhc_X
> node unconditionally in SoC DTSI, and leave board-dependent card-detect
> (if this SDHCI slot is even used for removable cards) pins to the board
> DTS.
>
>> So, move the same from the sm6115 dtsi file to the respective
>> board file(s).
>>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>
> Thanks for cleaning this up, we're already using this pattern in quite a
> few SoCs now.
Sure. But since they are being used in other SoC DTSIs doesn't guarantee
the correct demarcation between SoC DTSI and Board DTS :)
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> Though...
>
>> ---
>> .../boot/dts/qcom/sm4250-oneplus-billie2.dts | 10 +++++++++
>> arch/arm64/boot/dts/qcom/sm6115.dtsi | 22 -------------------
>> 2 files changed, 10 insertions(+), 22 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
>> index a3f1c7c41fd73..329eb496bbc5f 100644
>> --- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
>> +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
>> @@ -202,12 +202,22 @@ &sdhc_2 {
>> vqmmc-supply = <&vreg_l5a>;
>>
>> cd-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>;
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <&sdc2_state_on &sdc2_card_det_n>;
>> + pinctrl-1 = <&sdc2_state_off &sdc2_card_det_n>;
>>
>> status = "okay";
>> };
>>
>> &tlmm {
>> gpio-reserved-ranges = <14 4>;
>> +
>> + sdc2_card_det_n: sd-card-det-n-state {
>> + pins = "gpio88";
>> + function = "gpio";
>> + drive-strength = <2>;
>> + bias-pull-up;
>
> Note that SoC DTSI uses bias-disable in the off state.
The downstream uses bias-pull-up instead, as is the case with other
upstream qcom dtsi (see [1] and [2]).
[1].
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts#L1242
[2].
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/sdm845-mtp.dts#L783
>> + };
>> };
>>
>> &ufs_mem_hc {
>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>> index 572bf04adf906..6be763d39870d 100644
>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>> @@ -518,13 +518,6 @@ data-pins {
>> bias-pull-up;
>> drive-strength = <10>;
>> };
>> -
>> - sd-cd-pins {
>> - pins = "gpio88";
>> - function = "gpio";
>> - bias-pull-up;
>> - drive-strength = <2>;
>> - };
>> };
>>
>> sdc2_state_off: sdc2-off-state {
>> @@ -545,13 +538,6 @@ data-pins {
>> bias-pull-up;
>> drive-strength = <2>;
>> };
>> -
>> - sd-cd-pins {
>> - pins = "gpio88";
>> - function = "gpio";
>> - bias-disable;
>> - drive-strength = <2>;
>> - };
>> };
>> };
>>
>> @@ -652,10 +638,6 @@ sdhc_1: mmc@4744000 {
>> <&gcc GCC_SDCC1_ICE_CORE_CLK>;
>> clock-names = "iface", "core", "xo", "ice";
>>
>> - pinctrl-0 = <&sdc1_state_on>;
>> - pinctrl-1 = <&sdc1_state_off>;
>> - pinctrl-names = "default", "sleep";
>> -
>
> You can probably leave these and below? Only boards needing to extend
> pinctrl with board-specific SDCard pins would have to update/override
> these properties that way?
I disagree on this. But let's defer to @Bjorn for further inputs on the
same.
Thanks,
Bhupesh
>> bus-width = <8>;
>> status = "disabled";
>> };
>> @@ -672,10 +654,6 @@ sdhc_2: mmc@4784000 {
>> clocks = <&gcc GCC_SDCC2_AHB_CLK>, <&gcc GCC_SDCC2_APPS_CLK>, <&xo_board>;
>> clock-names = "iface", "core", "xo";
>>
>> - pinctrl-0 = <&sdc2_state_on>;
>> - pinctrl-1 = <&sdc2_state_off>;
>> - pinctrl-names = "default", "sleep";
>> -
>> power-domains = <&rpmpd SM6115_VDDCX>;
>> operating-points-v2 = <&sdhc2_opp_table>;
>> iommus = <&apps_smmu 0x00a0 0x0>;
>> --
>> 2.38.1
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-08 9:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-20 11:36 [PATCH] arm64: dts: qcom: sm6115: Move SDHC node(s)'s 'pinctrl' properties to dts Bhupesh Sharma
2022-12-21 22:10 ` Marijn Suijten
2023-02-08 9:51 ` Bhupesh Sharma
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).