| Message ID | 20220131161034.4.I79baad7f52351aafb470f8b21a9fa79d7031ad6a@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: sc7x80: A smattering of misc dts cleanups | expand |
Hi, On Mon, Jan 31, 2022 at 4:11 PM Douglas Anderson <dianders@chromium.org> wrote: > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 40cb414bc377..dc98a87e2871 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -616,6 +616,9 @@ qfprom: efuse@784000 { > > sdhc_1: sdhci@7c4000 { > compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; > + pinctrl-names = "default", "sleep"; > + pinctrl-0 = <&sdc1_clk>, <&sdc1_cmd>, <&sdc1_data>, <&sdc1_rclk>; > + pinctrl-1 = <&sdc1_clk_sleep>, <&sdc1_cmd_sleep>, <&sdc1_data_sleep>, <&sdc1_rclk_sleep>; > status = "disabled"; > > reg = <0 0x007c4000 0 0x1000>, > @@ -2425,6 +2428,9 @@ apss_merge_funnel_in: endpoint { > > sdhc_2: sdhci@8804000 { > compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; > + pinctrl-names = "default", "sleep"; > + pinctrl-0 = <&sdc2_clk>, <&sdc2_cmd>, <&sdc2_data>; > + pinctrl-1 = <&sdc2_clk_sleep>, <&sdc2_cmd_sleep>, <&sdc2_data_sleep>; > status = "disabled"; > > reg = <0 0x08804000 0 0x1000>; > @@ -3943,81 +3949,76 @@ qup_uart15_rx: qup-uart15-rx { > function = "qup17"; > }; > > - sdc1_on: sdc1-on { > - clk { > - pins = "sdc1_clk"; > - }; > > - cmd { > - pins = "sdc1_cmd"; > - }; > > - data { > - pins = "sdc1_data"; > - }; > + sdc1_clk: sdc1-clk { Ugh. I just noticed that there are way too many blank lines here in the output. Happy to have this fixed when applying or I can post a v2. -Doug
On Mon 31 Jan 18:25 CST 2022, Doug Anderson wrote: > Hi, > > On Mon, Jan 31, 2022 at 4:11 PM Douglas Anderson <dianders@chromium.org> wrote: > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > index 40cb414bc377..dc98a87e2871 100644 > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > @@ -616,6 +616,9 @@ qfprom: efuse@784000 { > > > > sdhc_1: sdhci@7c4000 { > > compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; > > + pinctrl-names = "default", "sleep"; > > + pinctrl-0 = <&sdc1_clk>, <&sdc1_cmd>, <&sdc1_data>, <&sdc1_rclk>; > > + pinctrl-1 = <&sdc1_clk_sleep>, <&sdc1_cmd_sleep>, <&sdc1_data_sleep>, <&sdc1_rclk_sleep>; > > status = "disabled"; > > > > reg = <0 0x007c4000 0 0x1000>, > > @@ -2425,6 +2428,9 @@ apss_merge_funnel_in: endpoint { > > > > sdhc_2: sdhci@8804000 { > > compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; > > + pinctrl-names = "default", "sleep"; > > + pinctrl-0 = <&sdc2_clk>, <&sdc2_cmd>, <&sdc2_data>; > > + pinctrl-1 = <&sdc2_clk_sleep>, <&sdc2_cmd_sleep>, <&sdc2_data_sleep>; > > status = "disabled"; > > > > reg = <0 0x08804000 0 0x1000>; > > @@ -3943,81 +3949,76 @@ qup_uart15_rx: qup-uart15-rx { > > function = "qup17"; > > }; > > > > - sdc1_on: sdc1-on { > > - clk { > > - pins = "sdc1_clk"; > > - }; > > > > - cmd { > > - pins = "sdc1_cmd"; > > - }; > > > > - data { > > - pins = "sdc1_data"; > > - }; > > + sdc1_clk: sdc1-clk { > > Ugh. I just noticed that there are way too many blank lines here in > the output. Happy to have this fixed when applying or I can post a v2. > I can fix that up as I apply it. Will let it sit for a few days to attract reviews first though. Regards, Bjorn
Hi, On Mon, Jan 31, 2022 at 4:33 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Mon 31 Jan 18:25 CST 2022, Doug Anderson wrote: > > > Hi, > > > > On Mon, Jan 31, 2022 at 4:11 PM Douglas Anderson <dianders@chromium.org> wrote: > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > index 40cb414bc377..dc98a87e2871 100644 > > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > @@ -616,6 +616,9 @@ qfprom: efuse@784000 { > > > > > > sdhc_1: sdhci@7c4000 { > > > compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; > > > + pinctrl-names = "default", "sleep"; > > > + pinctrl-0 = <&sdc1_clk>, <&sdc1_cmd>, <&sdc1_data>, <&sdc1_rclk>; > > > + pinctrl-1 = <&sdc1_clk_sleep>, <&sdc1_cmd_sleep>, <&sdc1_data_sleep>, <&sdc1_rclk_sleep>; > > > status = "disabled"; > > > > > > reg = <0 0x007c4000 0 0x1000>, > > > @@ -2425,6 +2428,9 @@ apss_merge_funnel_in: endpoint { > > > > > > sdhc_2: sdhci@8804000 { > > > compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; > > > + pinctrl-names = "default", "sleep"; > > > + pinctrl-0 = <&sdc2_clk>, <&sdc2_cmd>, <&sdc2_data>; > > > + pinctrl-1 = <&sdc2_clk_sleep>, <&sdc2_cmd_sleep>, <&sdc2_data_sleep>; > > > status = "disabled"; > > > > > > reg = <0 0x08804000 0 0x1000>; > > > @@ -3943,81 +3949,76 @@ qup_uart15_rx: qup-uart15-rx { > > > function = "qup17"; > > > }; > > > > > > - sdc1_on: sdc1-on { > > > - clk { > > > - pins = "sdc1_clk"; > > > - }; > > > > > > - cmd { > > > - pins = "sdc1_cmd"; > > > - }; > > > > > > - data { > > > - pins = "sdc1_data"; > > > - }; > > > + sdc1_clk: sdc1-clk { > > > > Ugh. I just noticed that there are way too many blank lines here in > > the output. Happy to have this fixed when applying or I can post a v2. > > > > I can fix that up as I apply it. Will let it sit for a few days to > attract reviews first though. Sounds good. Thanks! I might end up sending a v2 anyway since I found a few more fixups, but I'll at least wait a day or two so I don't spam people too hard. -Doug
Hi, On Mon, Jan 31, 2022 at 4:34 PM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Mon, Jan 31, 2022 at 4:33 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Mon 31 Jan 18:25 CST 2022, Doug Anderson wrote: > > > > > Hi, > > > > > > On Mon, Jan 31, 2022 at 4:11 PM Douglas Anderson <dianders@chromium.org> wrote: > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > > index 40cb414bc377..dc98a87e2871 100644 > > > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > > @@ -616,6 +616,9 @@ qfprom: efuse@784000 { > > > > > > > > sdhc_1: sdhci@7c4000 { > > > > compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; > > > > + pinctrl-names = "default", "sleep"; > > > > + pinctrl-0 = <&sdc1_clk>, <&sdc1_cmd>, <&sdc1_data>, <&sdc1_rclk>; > > > > + pinctrl-1 = <&sdc1_clk_sleep>, <&sdc1_cmd_sleep>, <&sdc1_data_sleep>, <&sdc1_rclk_sleep>; > > > > status = "disabled"; > > > > > > > > reg = <0 0x007c4000 0 0x1000>, > > > > @@ -2425,6 +2428,9 @@ apss_merge_funnel_in: endpoint { > > > > > > > > sdhc_2: sdhci@8804000 { > > > > compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; > > > > + pinctrl-names = "default", "sleep"; > > > > + pinctrl-0 = <&sdc2_clk>, <&sdc2_cmd>, <&sdc2_data>; > > > > + pinctrl-1 = <&sdc2_clk_sleep>, <&sdc2_cmd_sleep>, <&sdc2_data_sleep>; > > > > status = "disabled"; > > > > > > > > reg = <0 0x08804000 0 0x1000>; > > > > @@ -3943,81 +3949,76 @@ qup_uart15_rx: qup-uart15-rx { > > > > function = "qup17"; > > > > }; > > > > > > > > - sdc1_on: sdc1-on { > > > > - clk { > > > > - pins = "sdc1_clk"; > > > > - }; > > > > > > > > - cmd { > > > > - pins = "sdc1_cmd"; > > > > - }; > > > > > > > > - data { > > > > - pins = "sdc1_data"; > > > > - }; > > > > + sdc1_clk: sdc1-clk { > > > > > > Ugh. I just noticed that there are way too many blank lines here in > > > the output. Happy to have this fixed when applying or I can post a v2. > > > > > > > I can fix that up as I apply it. Will let it sit for a few days to > > attract reviews first though. > > Sounds good. Thanks! I might end up sending a v2 anyway since I found > a few more fixups, but I'll at least wait a day or two so I don't spam > people too hard. Breadcrumbs: I ended up sending out a v2 (tagged as v3 for other reasons) that fixes this. https://lore.kernel.org/r/20220202132301.v3.4.I79baad7f52351aafb470f8b21a9fa79d7031ad6a@changeid/ -Doug
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts index f159b5a6d7ef..918352c097bc 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts @@ -676,9 +676,6 @@ &qupv3_id_1 { &sdhc_1 { status = "okay"; - pinctrl-names = "default", "sleep"; - pinctrl-0 = <&sdc1_on>; - pinctrl-1 = <&sdc1_off>; vmmc-supply = <&pp2950_l7b>; vqmmc-supply = <&pp1800_l19b>; }; @@ -686,9 +683,8 @@ &sdhc_1 { &sdhc_2 { status = "okay"; - pinctrl-names = "default", "sleep"; - pinctrl-0 = <&sdc2_on>; - pinctrl-1 = <&sdc2_off>; + pinctrl-0 = <&sdc2_clk>, <&sdc2_cmd>, <&sdc2_data>, <&sd_cd>; + pinctrl-1 = <&sdc2_clk_sleep>, <&sdc2_cmd_sleep>, <&sdc2_data_sleep>, <&sd_cd>; vmmc-supply = <&pp2950_l9c>; vqmmc-supply = <&ppvar_l6c>; @@ -883,47 +879,38 @@ &qup_uart7_rx { bias-pull-up; }; -&sdc1_on { - clk { - bias-disable; - drive-strength = <16>; - }; - - cmd { - bias-pull-up; - drive-strength = <10>; - }; +&sdc1_clk { + bias-disable; + drive-strength = <16>; +}; - data { - bias-pull-up; - drive-strength = <10>; - }; +&sdc1_cmd { + bias-pull-up; + drive-strength = <10>; +}; - rclk { - bias-pull-down; - }; +&sdc1_data { + bias-pull-up; + drive-strength = <10>; }; -&sdc2_on { - clk { - bias-disable; - drive-strength = <16>; - }; +&sdc1_rclk { + bias-pull-down; +}; - cmd { - bias-pull-up; - drive-strength = <10>; - }; +&sdc2_clk { + bias-disable; + drive-strength = <16>; +}; - data { - bias-pull-up; - drive-strength = <10>; - }; +&sdc2_cmd { + bias-pull-up; + drive-strength = <10>; +}; - sd-cd { - pins = "gpio91"; - bias-pull-up; - }; +&sdc2_data { + bias-pull-up; + drive-strength = <10>; }; /* PINCTRL - board-specific pinctrl */ @@ -1311,6 +1298,12 @@ qup_uart7_sleep_tx: qup-uart7-sleep-tx { bias-pull-up; }; + sd_cd: sd-cd { + pins = "gpio91"; + function = "gpio"; + bias-pull-up; + }; + tp_int_odl: tp-int-odl { pins = "gpio102"; function = "gpio"; diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi index 78da9ac983db..7a987bc9b758 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi @@ -278,10 +278,6 @@ &qupv3_id_1 { &sdhc_1 { status = "okay"; - pinctrl-names = "default", "sleep"; - pinctrl-0 = <&sdc1_on>; - pinctrl-1 = <&sdc1_off>; - non-removable; no-sd; no-sdio; @@ -293,9 +289,8 @@ &sdhc_1 { &sdhc_2 { status = "okay"; - pinctrl-names = "default", "sleep"; - pinctrl-0 = <&sdc2_on>; - pinctrl-1 = <&sdc2_off>; + pinctrl-0 = <&sdc2_clk>, <&sdc2_cmd>, <&sdc2_data>, <&sd_cd>; + pinctrl-1 = <&sdc2_clk_sleep>, <&sdc2_cmd_sleep>, <&sdc2_data_sleep>, <&sd_cd>; vmmc-supply = <&vreg_l9c_2p9>; vqmmc-supply = <&vreg_l6c_2p9>; @@ -424,6 +419,40 @@ &qup_uart7_rx { bias-pull-up; }; +&sdc1_clk { + bias-disable; + drive-strength = <16>; +}; + +&sdc1_cmd { + bias-pull-up; + drive-strength = <10>; +}; + +&sdc1_data { + bias-pull-up; + drive-strength = <10>; +}; + +&sdc1_rclk { + bias-pull-down; +}; + +&sdc2_clk { + bias-disable; + drive-strength = <16>; +}; + +&sdc2_cmd { + bias-pull-up; + drive-strength = <10>; +}; + +&sdc2_data { + bias-pull-up; + drive-strength = <10>; +}; + &tlmm { bt_en: bt-en { pins = "gpio85"; @@ -496,53 +525,17 @@ qup_uart7_sleep_rx: qup-uart7-sleep-rx { bias-pull-up; }; - sw_ctrl: sw-ctrl { - pins = "gpio86"; + sd_cd: sd-cd { + pins = "gpio91"; function = "gpio"; - input-enable; - bias-pull-down; - }; -}; - -&sdc1_on { - clk { - bias-disable; - drive-strength = <16>; - }; - - cmd { bias-pull-up; - drive-strength = <10>; }; - data { - bias-pull-up; - drive-strength = <10>; - }; - - rclk { + sw_ctrl: sw-ctrl { + pins = "gpio86"; + function = "gpio"; + input-enable; bias-pull-down; }; }; -&sdc2_on { - clk { - bias-disable; - drive-strength = <16>; - }; - - cmd { - bias-pull-up; - drive-strength = <10>; - }; - - data { - bias-pull-up; - drive-strength = <10>; - }; - - sd-cd { - pins = "gpio91"; - bias-pull-up; - }; -}; diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index 40cb414bc377..dc98a87e2871 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -616,6 +616,9 @@ qfprom: efuse@784000 { sdhc_1: sdhci@7c4000 { compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&sdc1_clk>, <&sdc1_cmd>, <&sdc1_data>, <&sdc1_rclk>; + pinctrl-1 = <&sdc1_clk_sleep>, <&sdc1_cmd_sleep>, <&sdc1_data_sleep>, <&sdc1_rclk_sleep>; status = "disabled"; reg = <0 0x007c4000 0 0x1000>, @@ -2425,6 +2428,9 @@ apss_merge_funnel_in: endpoint { sdhc_2: sdhci@8804000 { compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5"; + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&sdc2_clk>, <&sdc2_cmd>, <&sdc2_data>; + pinctrl-1 = <&sdc2_clk_sleep>, <&sdc2_cmd_sleep>, <&sdc2_data_sleep>; status = "disabled"; reg = <0 0x08804000 0 0x1000>; @@ -3943,81 +3949,76 @@ qup_uart15_rx: qup-uart15-rx { function = "qup17"; }; - sdc1_on: sdc1-on { - clk { - pins = "sdc1_clk"; - }; - cmd { - pins = "sdc1_cmd"; - }; - data { - pins = "sdc1_data"; - }; + sdc1_clk: sdc1-clk { + pins = "sdc1_clk"; + }; - rclk { - pins = "sdc1_rclk"; - }; + sdc1_cmd: sdc1-cmd { + pins = "sdc1_cmd"; }; - sdc1_off: sdc1-off { - clk { - pins = "sdc1_clk"; - drive-strength = <2>; - bias-bus-hold; - }; + sdc1_data: sdc1-data { + pins = "sdc1_data"; + }; - cmd { - pins = "sdc1_cmd"; - drive-strength = <2>; - bias-bus-hold; - }; + sdc1_rclk: sdc1-rclk { + pins = "sdc1_rclk"; + }; - data { - pins = "sdc1_data"; - drive-strength = <2>; - bias-bus-hold; - }; + sdc1_clk_sleep: sdc1-clk-sleep { + pins = "sdc1_clk"; + drive-strength = <2>; + bias-bus-hold; + }; - rclk { - pins = "sdc1_rclk"; - bias-bus-hold; - }; + sdc1_cmd_sleep: sdc1-cmd-sleep { + pins = "sdc1_cmd"; + drive-strength = <2>; + bias-bus-hold; }; - sdc2_on: sdc2-on { - clk { - pins = "sdc2_clk"; - }; + sdc1_data_sleep: sdc1-data-sleep { + pins = "sdc1_data"; + drive-strength = <2>; + bias-bus-hold; + }; - cmd { - pins = "sdc2_cmd"; - }; + sdc1_rclk_sleep: sdc1-rclk-sleep { + pins = "sdc1_rclk"; + drive-strength = <2>; + bias-bus-hold; + }; - data { - pins = "sdc2_data"; - }; + sdc2_clk: sdc2-clk { + pins = "sdc2_clk"; }; - sdc2_off: sdc2-off { - clk { - pins = "sdc2_clk"; - drive-strength = <2>; - bias-bus-hold; - }; + sdc2_cmd: sdc2-cmd { + pins = "sdc2_cmd"; + }; - cmd { - pins ="sdc2_cmd"; - drive-strength = <2>; - bias-bus-hold; - }; + sdc2_data: sdc2-data { + pins = "sdc2_data"; + }; - data { - pins ="sdc2_data"; - drive-strength = <2>; - bias-bus-hold; - }; + sdc2_clk_sleep: sdc2-clk-sleep { + pins = "sdc2_clk"; + drive-strength = <2>; + bias-bus-hold; + }; + + sdc2_cmd_sleep: sdc2-cmd-sleep { + pins = "sdc2_cmd"; + drive-strength = <2>; + bias-bus-hold; + }; + + sdc2_data_sleep: sdc2-data-sleep { + pins = "sdc2_data"; + drive-strength = <2>; + bias-bus-hold; }; };
This patch makes a few improvements to the way that sdc1 / sdc2 pinctrl is specified on sc7280: 1. There's no reason to "group" the sdc pins into one overarching node and there's a downside: we have to replicate the hierarchy in the board device tree files. Let's clean this up. 2. There's really not a lot of reason not to list the "pinctrl" for sdc1 (eMMC) in the SoC dtsi file. These aren't GPIO pins and everyone's going to specify the same pins. 3. Even though it's likely that boards will need to override pinctrl for sdc2 (SD card) to add the card detect GPIO, we can be symmetric and add it to the SoC dsti file. 4. Let's get rid of the word "on" from the normal config and add a "sleep" suffix to the sleep config. This looks cleaner to me. This is intended to be a no-op change but it could plausibly change behavior depending on how the pinctrl code parses things. One thing to note is that "SD card detect" is explicitly listed now as keeping its pull enabled in sleep since we still want to detect card insertions even if the controller is suspended (because no card is inserted). The pinctrl framework likely did this anyway, but it's nice to see it explicit. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- .../qcom/sc7280-herobrine-herobrine-r0.dts | 73 +++++------ arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 91 +++++++------- arch/arm64/boot/dts/qcom/sc7280.dtsi | 119 +++++++++--------- 3 files changed, 135 insertions(+), 148 deletions(-)