All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: qcom: sc7180-trogdor: Split out keyboard node and describe detachables
@ 2022-05-18 17:25 Stephen Boyd
  2022-05-20 22:23 ` Doug Anderson
  0 siblings, 1 reply; 2+ messages in thread
From: Stephen Boyd @ 2022-05-18 17:25 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson
  Cc: linux-kernel, patches, linux-arm-msm, Dmitry Torokhov,
	Benson Leung, Guenter Roeck, Douglas Anderson, Hsin-Yi Wang,
	Joseph S. Barrera III

Trogdor devices that have a detachable keyboard still have a
non-detachable keyboard input device present because we include the
cros-ec-keyboard.dtsi snippet in the top-level sc7180-trogdor.dtsi file
that every variant board includes. We do this because the
keyboard-controller node also provides some buttons like the power
button and volume buttons. Unfortunately, this means we register a
keyboard input device that doesn't do anything on boards with a
detachable keyboard.

Change the node's compatible on detachables to the newly introduced
"google,cros-ec-keyb-switches" compatible to indicate that there are
only switches and no keyboard to register. Similarly, move the keyboard
include that defines the keyboard-controller node out of
sc7180-trogdor.dtsi to boards that actually have a keyboard so that the
matrix properties are not defined on boards with the switches
compatible. Future boards can either use the include approach or the
node definition approach to describe a keyboard with possible switches
or just some switches.

Cc: Benson Leung <bleung@chromium.org>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: "Joseph S. Barrera III" <joebar@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

This is based on an immutable branch from Dmitry (on Cc)

	git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git ib/5.17-cros-ec-keyb

where this patch should be applied on top of so that a bisect doesn't
break. The driver side change only supports one compatible or the other,
so we can't apply this patch unless the driver side change is present.

 arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi   | 4 ++++
 arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 6 ++++++
 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi    | 2 ++
 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi   | 2 ++
 arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts        | 2 ++
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi          | 1 -
 6 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
index 14ed09f30a73..a131dba4bf97 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
@@ -86,6 +86,10 @@ &camcc {
 };
 
 &cros_ec {
+	keyboard-controller {
+		compatible = "google,cros-ec-keyb-switches";
+	};
+
 	cros_ec_proximity: proximity {
 		compatible = "google,cros-ec-mkbp-proximity";
 		label = "proximity-wifi";
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
index f32369af1351..5d7ca89f7862 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
@@ -121,6 +121,12 @@ &camcc {
 	status = "okay";
 };
 
+&cros_ec {
+	keyboard-controller {
+		compatible = "google,cros-ec-keyb-switches";
+	};
+};
+
 &panel {
 	compatible = "samsung,atna33xc20";
 	enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
index 69666f92176a..4601f5afe255 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
@@ -9,6 +9,8 @@
 ap_h1_spi: &spi0 {};
 
 #include "sc7180-trogdor.dtsi"
+/* Must come after sc7180-trogdor.dtsi to modify cros_ec */
+#include <arm/cros-ec-keyboard.dtsi>
 
 &ap_sar_sensor {
 	semtech,cs0-ground;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
index e90f99ef5323..64535bd03db9 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
@@ -11,6 +11,8 @@
 ap_h1_spi: &spi0 {};
 
 #include "sc7180-trogdor.dtsi"
+/* Must come after sc7180-trogdor.dtsi to modify cros_ec */
+#include <arm/cros-ec-keyboard.dtsi>
 #include "sc7180-trogdor-ti-sn65dsi86.dtsi"
 
 / {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
index 457c25499863..9a424a96c003 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
@@ -13,6 +13,8 @@
 ap_h1_spi: &spi0 {};
 
 #include "sc7180-trogdor.dtsi"
+/* Must come after sc7180-trogdor.dtsi to modify cros_ec */
+#include <arm/cros-ec-keyboard.dtsi>
 #include "sc7180-trogdor-ti-sn65dsi86.dtsi"
 
 / {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index bd5909ffb3dc..a09b2e452712 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -810,7 +810,6 @@ cros_ec_fp: ec@0 {
 	};
 };
 
-#include <arm/cros-ec-keyboard.dtsi>
 #include <arm/cros-ec-sbs.dtsi>
 
 &uart3 {

base-commit: ca1eadbfcd36bec73f2a2111c28e8c7e9e8ae6c0
-- 
https://chromeos.dev


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] arm64: dts: qcom: sc7180-trogdor: Split out keyboard node and describe detachables
  2022-05-18 17:25 [PATCH] arm64: dts: qcom: sc7180-trogdor: Split out keyboard node and describe detachables Stephen Boyd
@ 2022-05-20 22:23 ` Doug Anderson
  0 siblings, 0 replies; 2+ messages in thread
From: Doug Anderson @ 2022-05-20 22:23 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andy Gross, Bjorn Andersson, LKML, patches, linux-arm-msm,
	Dmitry Torokhov, Benson Leung, Guenter Roeck, Hsin-Yi Wang,
	Joseph S. Barrera III

Hi,

On Wed, May 18, 2022 at 10:25 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Trogdor devices that have a detachable keyboard still have a
> non-detachable keyboard input device present because we include the
> cros-ec-keyboard.dtsi snippet in the top-level sc7180-trogdor.dtsi file
> that every variant board includes. We do this because the
> keyboard-controller node also provides some buttons like the power
> button and volume buttons. Unfortunately, this means we register a
> keyboard input device that doesn't do anything on boards with a
> detachable keyboard.
>
> Change the node's compatible on detachables to the newly introduced
> "google,cros-ec-keyb-switches" compatible to indicate that there are
> only switches and no keyboard to register. Similarly, move the keyboard
> include that defines the keyboard-controller node out of
> sc7180-trogdor.dtsi to boards that actually have a keyboard so that the
> matrix properties are not defined on boards with the switches
> compatible. Future boards can either use the include approach or the
> node definition approach to describe a keyboard with possible switches
> or just some switches.
>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Cc: "Joseph S. Barrera III" <joebar@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> This is based on an immutable branch from Dmitry (on Cc)
>
>         git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git ib/5.17-cros-ec-keyb
>
> where this patch should be applied on top of so that a bisect doesn't
> break. The driver side change only supports one compatible or the other,
> so we can't apply this patch unless the driver side change is present.
>
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi   | 4 ++++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 6 ++++++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi    | 2 ++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi   | 2 ++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts        | 2 ++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi          | 1 -
>  6 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
> index 14ed09f30a73..a131dba4bf97 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
> @@ -86,6 +86,10 @@ &camcc {
>  };
>
>  &cros_ec {
> +       keyboard-controller {
> +               compatible = "google,cros-ec-keyb-switches";
> +       };
> +
>         cros_ec_proximity: proximity {
>                 compatible = "google,cros-ec-mkbp-proximity";
>                 label = "proximity-wifi";
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
> index f32369af1351..5d7ca89f7862 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
> @@ -121,6 +121,12 @@ &camcc {
>         status = "okay";
>  };
>
> +&cros_ec {
> +       keyboard-controller {
> +               compatible = "google,cros-ec-keyb-switches";
> +       };
> +};
> +
>  &panel {
>         compatible = "samsung,atna33xc20";
>         enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> index 69666f92176a..4601f5afe255 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> @@ -9,6 +9,8 @@
>  ap_h1_spi: &spi0 {};
>
>  #include "sc7180-trogdor.dtsi"
> +/* Must come after sc7180-trogdor.dtsi to modify cros_ec */
> +#include <arm/cros-ec-keyboard.dtsi>

As pointed out in a separate email [1], I think the include of
sc7180-trogdor.dtsi needs to be removed from this file. I think it's
still OK to include the keyboard.dtsi file here, though, since the
trogdor.dtsi file will always be included before the lazor.dtsi. You
just might need to update the comment.

https://lore.kernel.org/r/CAD=FV=VSMyJrs9bTE2XEC=q7VtZi6YKFaKJQJyLTZhVaYa9-Jg@mail.gmail.com/

Other than that, this looks good to me.

This will, of course, need to be coordinated with Joe's patches adding
the new boards.

-Doug

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-20 22:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-18 17:25 [PATCH] arm64: dts: qcom: sc7180-trogdor: Split out keyboard node and describe detachables Stephen Boyd
2022-05-20 22:23 ` Doug Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.