| Message ID | 20221214232049.703484-5-marijn.suijten@somainline.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arm64: dts: qcom: msm8956-loire: SDCard | expand |
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"; > }; >
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
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
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
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"; };
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(+)