[21/22] arm64: dts: qcom: sc7180-trogdor: Make clamshell/detachable fragments
Commit Message
At a high-level, detachable Trogdors (sometimes known as Strongbads)
don't have a cros_ec keyboard, while all clamshell Trogdors (only known
as Trogdors) always have a cros_ec keyboard. Looking closer though, all
clamshells have a USB type-A connector and a hardwired USB camera. And
all detachables replace the USB camera with a MIPI based one and swap
the USB type-a connector for the detachable keyboard pogo pins.
Split the detachable and clamshell bits into different files so we can
describe these differences in one place instead of in each board that
includes sc7180-trogdor.dtsi. For now this is just the keyboard part,
but eventually this will include the type-a port and the pogo pins.
Cc: <cros-qcom-dts-watchers@chromium.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: <linux-arm-msm@vger.kernel.org>
Cc: <devicetree@vger.kernel.org>
Cc: Pin-yen Lin <treapking@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
.../boot/dts/qcom/sc7180-trogdor-clamshell.dtsi | 9 +++++++++
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 +----
.../boot/dts/qcom/sc7180-trogdor-detachable.dtsi | 12 ++++++++++++
.../arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 7 +------
.../boot/dts/qcom/sc7180-trogdor-kingoftown.dts | 2 +-
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 3 +--
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 2 +-
.../boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 7 +------
arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts | 2 +-
.../boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi | 5 +----
11 files changed, 30 insertions(+), 26 deletions(-)
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
Comments
On Sat, 10 Feb 2024 at 09:17, Stephen Boyd <swboyd@chromium.org> wrote:
>
> At a high-level, detachable Trogdors (sometimes known as Strongbads)
> don't have a cros_ec keyboard, while all clamshell Trogdors (only known
> as Trogdors) always have a cros_ec keyboard. Looking closer though, all
> clamshells have a USB type-A connector and a hardwired USB camera. And
> all detachables replace the USB camera with a MIPI based one and swap
> the USB type-a connector for the detachable keyboard pogo pins.
>
> Split the detachable and clamshell bits into different files so we can
> describe these differences in one place instead of in each board that
> includes sc7180-trogdor.dtsi. For now this is just the keyboard part,
> but eventually this will include the type-a port and the pogo pins.
>
> Cc: <cros-qcom-dts-watchers@chromium.org>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: <linux-arm-msm@vger.kernel.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: Pin-yen Lin <treapking@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> .../boot/dts/qcom/sc7180-trogdor-clamshell.dtsi | 9 +++++++++
> arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 +----
> .../boot/dts/qcom/sc7180-trogdor-detachable.dtsi | 12 ++++++++++++
> .../arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 7 +------
> .../boot/dts/qcom/sc7180-trogdor-kingoftown.dts | 2 +-
> arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 3 +--
> arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 2 +-
> .../boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 7 +------
> arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts | 2 +-
> .../boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi | 5 +----
> 11 files changed, 30 insertions(+), 26 deletions(-)
> create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
> create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Hi,
On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
> new file mode 100644
> index 000000000000..bcf3df463f80
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Trogdor dts framgent for clamshells
s/framgent/fragment
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
> new file mode 100644
> index 000000000000..ab0f30288871
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Trogdor dts framgent for detachables
s/framgent/fragment
> + * Copyright 2024 Google LLC.
> + */
> +
Tiny nit: should this file have a comment like "/* This file must be
included after sc7180-trogdor.dtsi */" like the clamshell file?
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> index e9f213d27711..c3fd6760de7a 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> @@ -5,8 +5,7 @@
> * Copyright 2020 Google LLC.
> */
>
> -/* This file must be included after sc7180-trogdor.dtsi */
> -#include <arm/cros-ec-keyboard.dtsi>
> +#include "sc7180-trogdor-clamshell.dtsi"
nit: Not that it was terribly consistent before, but in lazor you
remove the "This file must be included after sc7180-trogdor.dtsi"
because (I guess) it moved to the clamshell file. However, in other
dts files you don't remove it. pazquel has the exact same comment and
it's not removed. Pompom has a slight variant of the comment where it
explains the reason (to modify cros_ec) and it's not removed. Could
make it more consistent...
Everything above is either tiny typos or nits, so happy enough with:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Quoting Doug Anderson (2024-02-13 15:34:38)
> Hi,
>
> On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > + * Copyright 2024 Google LLC.
> > + */
> > +
>
> Tiny nit: should this file have a comment like "/* This file must be
> included after sc7180-trogdor.dtsi */" like the clamshell file?
I was copying the comment for the keyboard include file, but the
detachable doesn't have that include. I can add it here though, that's
fine.
>
>
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > index e9f213d27711..c3fd6760de7a 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > @@ -5,8 +5,7 @@
> > * Copyright 2020 Google LLC.
> > */
> >
> > -/* This file must be included after sc7180-trogdor.dtsi */
> > -#include <arm/cros-ec-keyboard.dtsi>
> > +#include "sc7180-trogdor-clamshell.dtsi"
>
> nit: Not that it was terribly consistent before, but in lazor you
> remove the "This file must be included after sc7180-trogdor.dtsi"
> because (I guess) it moved to the clamshell file. However, in other
> dts files you don't remove it. pazquel has the exact same comment and
> it's not removed. Pompom has a slight variant of the comment where it
> explains the reason (to modify cros_ec) and it's not removed. Could
> make it more consistent...
Sure I can make it more consistent with the explanation. Which way to go
though? Remove it from the boards and put it into the fragment files?
Hi,
On Wed, Feb 14, 2024 at 4:35 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> > > -/* This file must be included after sc7180-trogdor.dtsi */
> > > -#include <arm/cros-ec-keyboard.dtsi>
> > > +#include "sc7180-trogdor-clamshell.dtsi"
> >
> > nit: Not that it was terribly consistent before, but in lazor you
> > remove the "This file must be included after sc7180-trogdor.dtsi"
> > because (I guess) it moved to the clamshell file. However, in other
> > dts files you don't remove it. pazquel has the exact same comment and
> > it's not removed. Pompom has a slight variant of the comment where it
> > explains the reason (to modify cros_ec) and it's not removed. Could
> > make it more consistent...
>
> Sure I can make it more consistent with the explanation. Which way to go
> though? Remove it from the boards and put it into the fragment files?
I don't care too much. I guess I'd lean toward just putting it in the
fragment files just to copy it fewer times.
-Doug
new file mode 100644
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Trogdor dts framgent for clamshells
+ *
+ * Copyright 2024 Google LLC.
+ */
+
+/* This file must be included after sc7180-trogdor.dtsi */
+#include <arm/cros-ec-keyboard.dtsi>
@@ -7,6 +7,7 @@
#include "sc7180-trogdor.dtsi"
#include "sc7180-trogdor-ti-sn65dsi86.dtsi"
+#include "sc7180-trogdor-detachable.dtsi"
/* Deleted nodes from sc7180-trogdor.dtsi */
@@ -80,10 +81,6 @@ &camcc {
};
&cros_ec {
- keyboard-controller {
- compatible = "google,cros-ec-keyb-switches";
- };
-
cros_ec_proximity: proximity {
compatible = "google,cros-ec-mkbp-proximity";
label = "proximity-wifi";
new file mode 100644
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Trogdor dts framgent for detachables
+ *
+ * Copyright 2024 Google LLC.
+ */
+
+&cros_ec {
+ keyboard-controller {
+ compatible = "google,cros-ec-keyb-switches";
+ };
+};
@@ -8,6 +8,7 @@
/* This file must be included after sc7180-trogdor.dtsi */
#include "sc7180-trogdor-rt5682i-sku.dtsi"
+#include "sc7180-trogdor-detachable.dtsi"
/ {
/* BOARD-SPECIFIC TOP LEVEL NODES */
@@ -135,12 +136,6 @@ &camcc {
status = "okay";
};
-&cros_ec {
- keyboard-controller {
- compatible = "google,cros-ec-keyb-switches";
- };
-};
-
&panel {
compatible = "samsung,atna33xc20";
enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
@@ -9,7 +9,7 @@
#include "sc7180-trogdor.dtsi"
#include "sc7180-trogdor-parade-ps8640.dtsi"
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
#include "sc7180-trogdor-lte-sku.dtsi"
#include "sc7180-trogdor-rt5682s-sku.dtsi"
@@ -5,8 +5,7 @@
* Copyright 2020 Google LLC.
*/
-/* This file must be included after sc7180-trogdor.dtsi */
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
&ap_sar_sensor {
semtech,cs0-ground;
@@ -6,7 +6,7 @@
*/
/* This file must be included after sc7180-trogdor.dtsi */
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
&ap_sar_sensor {
compatible = "semtech,sx9324";
@@ -7,7 +7,7 @@
#include "sc7180-trogdor.dtsi"
/* Must come after sc7180-trogdor.dtsi to modify cros_ec */
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
#include "sc7180-trogdor-rt5682i-sku.dtsi"
#include "sc7180-trogdor-ti-sn65dsi86.dtsi"
@@ -9,6 +9,7 @@
#include "sc7180-trogdor.dtsi"
#include "sc7180-trogdor-rt5682i-sku.dtsi"
+#include "sc7180-trogdor-detachable.dtsi"
/ {
ppvar_lcd: ppvar-lcd-regulator {
@@ -44,12 +45,6 @@ &camcc {
status = "okay";
};
-&cros_ec {
- keyboard-controller {
- compatible = "google,cros-ec-keyb-switches";
- };
-};
-
&gpio_keys {
status = "okay";
};
@@ -9,7 +9,7 @@
#include "sc7180-trogdor.dtsi"
/* Must come after sc7180-trogdor.dtsi to modify cros_ec */
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
#include "sc7180-trogdor-rt5682i-sku.dtsi"
#include "sc7180-trogdor-ti-sn65dsi86.dtsi"
@@ -8,6 +8,7 @@
/dts-v1/;
#include "sc7180-trogdor.dtsi"
+#include "sc7180-trogdor-detachable.dtsi"
/ {
avdd_lcd: avdd-lcd-regulator {
@@ -104,10 +105,6 @@ &cros_ec {
base_detection: cbas {
compatible = "google,cros-cbas";
};
-
- keyboard-controller {
- compatible = "google,cros-ec-keyb-switches";
- };
};
&i2c4 {