[PATCH 0/2] arm64: dts: qcom: sc7180-trogdor: Simplify!

[PATCH 0/2] arm64: dts: qcom: sc7180-trogdor: Simplify!

[Stephen Boyd]

Here's a couple patches to simplify sc7180-trogdor dtsi files further.

Stephen Boyd (2):
  arm64: dts: qcom: sc7180-trogdor: Simplify trackpad enabling
  arm64: dts: qcom: sc7180-trogdor: Simplify spi0/spi6 labeling

 arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi   | 9 ---------
 arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 7 -------
 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi    | 7 ++++---
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi          | 6 +++---
 4 files changed, 7 insertions(+), 22 deletions(-)

Cc: Joseph Barrera <joebar@google.com>
Cc: Douglas Anderson <dianders@chromium.org>

base-commit: 52deda9551a01879b3562e7b41748e85c591f14c
-- 
https://chromeos.dev

[PATCH 1/2] arm64: dts: qcom: sc7180-trogdor: Simplify trackpad enabling

[Stephen Boyd]

Trogdor boards with a detachable keyboard don't have a trackpad over
i2c. Instead the trackpad is on the detachable keyboard base. Let's move
the enabling of the trackpad i2c bus out of the base sc7180-trogdor.dtsi
file so that each trogdor board that is detachable, of which there are
many, doesn't have to disable the trackpad bus.

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   | 4 ----
 arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 4 ----
 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi    | 4 ++++
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi          | 1 -
 4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
index c81805ef2250..8da61a52f150 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
@@ -111,10 +111,6 @@ ap_ts: touchscreen@5d {
 	};
 };
 
-&i2c7 {
-	status = "disabled";
-};
-
 &i2c9 {
 	status = "disabled";
 };
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
index bff2b556cc75..532c7dcc3f73 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
@@ -88,10 +88,6 @@ map1 {
 	};
 };
 
-&ap_tp_i2c {
-	status = "disabled";
-};
-
 ap_ts_pen_1v8: &i2c4 {
 	status = "okay";
 	clock-frequency = <400000>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
index 69666f92176a..75df5d1633b2 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
@@ -19,6 +19,10 @@ &ap_sar_sensor {
 	semtech,avg-pos-strength = <64>;
 };
 
+&ap_tp_i2c {
+	status = "okay";
+};
+
 /*
  * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
  * not supported by the PM6150 ADC driver. Disable the charger thermal zone
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 732e1181af48..3bf40b6abcba 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -728,7 +728,6 @@ ap_sar_sensor: proximity@28 {
 };
 
 ap_tp_i2c: &i2c7 {
-	status = "okay";
 	clock-frequency = <400000>;
 
 	trackpad: trackpad@15 {
-- 
https://chromeos.dev

[PATCH 2/2] arm64: dts: qcom: sc7180-trogdor: Simplify spi0/spi6 labeling

[Stephen Boyd]

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(-)

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";
-- 
https://chromeos.dev

Re: [PATCH 1/2] arm64: dts: qcom: sc7180-trogdor: Simplify trackpad enabling

[Doug Anderson]

Hi,

On Fri, Mar 25, 2022 at 4:43 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Trogdor boards with a detachable keyboard don't have a trackpad over
> i2c. Instead the trackpad is on the detachable keyboard base. Let's move
> the enabling of the trackpad i2c bus out of the base sc7180-trogdor.dtsi
> file so that each trogdor board that is detachable, of which there are
> many, doesn't have to disable the trackpad bus.
>
> 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   | 4 ----
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 4 ----
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi    | 4 ++++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi          | 1 -
>  4 files changed, 4 insertions(+), 9 deletions(-)

What about pompom?
What about trogdor-r1?

Re: [PATCH 2/2] arm64: dts: qcom: sc7180-trogdor: Simplify spi0/spi6 labeling

[Doug Anderson]

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.

Re: [PATCH 1/2] arm64: dts: qcom: sc7180-trogdor: Simplify trackpad enabling

[Stephen Boyd]

Quoting Doug Anderson (2022-03-28 09:04:57)
> Hi,
>
> On Fri, Mar 25, 2022 at 4:43 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Trogdor boards with a detachable keyboard don't have a trackpad over
> > i2c. Instead the trackpad is on the detachable keyboard base. Let's move
> > the enabling of the trackpad i2c bus out of the base sc7180-trogdor.dtsi
> > file so that each trogdor board that is detachable, of which there are
> > many, doesn't have to disable the trackpad bus.
> >
> > 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   | 4 ----
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 4 ----
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi    | 4 ++++
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi          | 1 -
> >  4 files changed, 4 insertions(+), 9 deletions(-)
>
> What about pompom?
> What about trogdor-r1?

Oh I missed those ones. Will fix it!

(no subject)

[Stephen Boyd]

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.