| Message ID | 20220325234344.199841-3-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: sc7180-trogdor: Simplify! | expand |
Hi, On Fri, Mar 25, 2022 at 4:43 PM Stephen Boyd <swboyd@chromium.org> wrote: > > We had to do this spi0/spi6 flip-flop on trogdor-r0 because the spi > buses got swizzled between r0 and r1. The swizzle stopped after r1, but > we kept this around to support either hardware possibility and to keep > trogdor-r0 working. > > trogdor-r0 isn't supported upstream, so this swizzle is not doing > anything besides making a pattern that others try to copy for the EC and > H1 nodes. Let's remove it and simplify the dts files. > > Cc: Joseph Barrera <joebar@google.com> > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 ----- > arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 3 --- > arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 3 --- > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 5 +++-- > 4 files changed, 3 insertions(+), 13 deletions(-) What about pompom? What about trogdor-r1? > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi > index 75df5d1633b2..fe2369c29aad 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi > @@ -5,9 +5,6 @@ > * Copyright 2020 Google LLC. > */ > > -ap_ec_spi: &spi6 {}; > -ap_h1_spi: &spi0 {}; > - > #include "sc7180-trogdor.dtsi" > > &ap_sar_sensor { > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > index 3bf40b6abcba..3123665f6c3c 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > @@ -11,6 +11,7 @@ > #include <dt-bindings/regulator/qcom,rpmh-regulator.h> > #include <dt-bindings/sound/sc7180-lpass.h> > > +#include "sc7180.dtsi" If we're going to move the "sc7180.dtsi" to trogdor then we should move it for everyone, right? Right now you're _only_ removing it from coachz. I think that means that for every other board the "sc7180.dtsi" file is included more than once. These aren't like C header files where there's a convention to have header guards. I _think_ that could actually cause problems too, right? So if you include it once and then you override a status to "okay" and then you included it again it might get changed back to "disabled" ? The easiest solution would be to just leave the sc7180.dtsi include where it was. If you want to try to do better, I _think_ (but haven't tried) that you can change all of the existing includes of "sc7180.dtsi" to be includes of "sc7180-trogdor.dtsi" and then remove the existing includes of "sc7180-trogdor.dtsi". IIRC the reason that the sc7180.dtsi include is scattered everywhere is that we needed the i2c bus before we could define the parade / ti bridge but then we needed the parade / ti bridge before we included the board specific bits so we could define the panel.
Quoting Doug Anderson (2022-03-28 09:05:05) > Hi, > > On Fri, Mar 25, 2022 at 4:43 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > We had to do this spi0/spi6 flip-flop on trogdor-r0 because the spi > > buses got swizzled between r0 and r1. The swizzle stopped after r1, but > > we kept this around to support either hardware possibility and to keep > > trogdor-r0 working. > > > > trogdor-r0 isn't supported upstream, so this swizzle is not doing > > anything besides making a pattern that others try to copy for the EC and > > H1 nodes. Let's remove it and simplify the dts files. > > > > Cc: Joseph Barrera <joebar@google.com> > > Cc: Douglas Anderson <dianders@chromium.org> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > --- > > arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 ----- > > arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 3 --- > > arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 3 --- > > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 5 +++-- > > 4 files changed, 3 insertions(+), 13 deletions(-) > > What about pompom? > What about trogdor-r1? > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi > > index 75df5d1633b2..fe2369c29aad 100644 > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi > > @@ -5,9 +5,6 @@ > > * Copyright 2020 Google LLC. > > */ > > > > -ap_ec_spi: &spi6 {}; > > -ap_h1_spi: &spi0 {}; > > - > > #include "sc7180-trogdor.dtsi" > > > > &ap_sar_sensor { > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > index 3bf40b6abcba..3123665f6c3c 100644 > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > @@ -11,6 +11,7 @@ > > #include <dt-bindings/regulator/qcom,rpmh-regulator.h> > > #include <dt-bindings/sound/sc7180-lpass.h> > > > > +#include "sc7180.dtsi" > > If we're going to move the "sc7180.dtsi" to trogdor then we should > move it for everyone, right? Right now you're _only_ removing it from > coachz. I think that means that for every other board the > "sc7180.dtsi" file is included more than once. These aren't like C > header files where there's a convention to have header guards. Yes that was the intention. I didn't notice that there were so many other includes of sc7180.dtsi though. > > I _think_ that could actually cause problems too, right? So if you > include it once and then you override a status to "okay" and then you > included it again it might get changed back to "disabled" ? > > The easiest solution would be to just leave the sc7180.dtsi include > where it was. If you want to try to do better, I _think_ (but haven't > tried) that you can change all of the existing includes of > "sc7180.dtsi" to be includes of "sc7180-trogdor.dtsi" and then remove > the existing includes of "sc7180-trogdor.dtsi". IIRC the reason that > the sc7180.dtsi include is scattered everywhere is that we needed the > i2c bus before we could define the parade / ti bridge but then we > needed the parade / ti bridge before we included the board specific > bits so we could define the panel. Yeah let me try that.
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi index 8da61a52f150..186be3f419d7 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi @@ -5,11 +5,6 @@ * Copyright 2020 Google LLC. */ -#include "sc7180.dtsi" - -ap_ec_spi: &spi6 {}; -ap_h1_spi: &spi0 {}; - #include "sc7180-trogdor.dtsi" #include "sc7180-trogdor-ti-sn65dsi86.dtsi" diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi index 532c7dcc3f73..9b3e3d13c165 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi @@ -5,9 +5,6 @@ * Copyright 2021 Google LLC. */ -ap_ec_spi: &spi6 {}; -ap_h1_spi: &spi0 {}; - #include "sc7180-trogdor.dtsi" / { diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi index 75df5d1633b2..fe2369c29aad 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi @@ -5,9 +5,6 @@ * Copyright 2020 Google LLC. */ -ap_ec_spi: &spi6 {}; -ap_h1_spi: &spi0 {}; - #include "sc7180-trogdor.dtsi" &ap_sar_sensor { diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi index 3bf40b6abcba..3123665f6c3c 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi @@ -11,6 +11,7 @@ #include <dt-bindings/regulator/qcom,rpmh-regulator.h> #include <dt-bindings/sound/sc7180-lpass.h> +#include "sc7180.dtsi" /* PMICs depend on spmi_bus label and so must come after SoC */ #include "pm6150.dtsi" #include "pm6150l.dtsi" @@ -626,7 +627,7 @@ src_vreg_bob: bob { }; }; -&ap_ec_spi { +ap_ec_spi: &spi6 { status = "okay"; cros_ec: ec@0 { compatible = "google,cros-ec-spi"; @@ -675,7 +676,7 @@ usb_c1: connector@1 { }; }; -&ap_h1_spi { +ap_h1_spi: &spi0 { status = "okay"; cr50: tpm@0 { compatible = "google,cr50";
We had to do this spi0/spi6 flip-flop on trogdor-r0 because the spi buses got swizzled between r0 and r1. The swizzle stopped after r1, but we kept this around to support either hardware possibility and to keep trogdor-r0 working. trogdor-r0 isn't supported upstream, so this swizzle is not doing anything besides making a pattern that others try to copy for the EC and H1 nodes. Let's remove it and simplify the dts files. Cc: Joseph Barrera <joebar@google.com> Cc: Douglas Anderson <dianders@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 ----- arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 3 --- arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 3 --- arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 5 +++-- 4 files changed, 3 insertions(+), 13 deletions(-)