2022-04-27 10:55:57

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH] arm64: dts: qcom: sc7180-trogdor: Remove cros-ec keyboard from detachables

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. Let's delete the rows/columns properties of the
device node to indicate that there isn't a matrix keyboard on these
boards.

Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Hsin-Yi Wang <[email protected]>
Cc: "Joseph S. Barrera III" <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 +++++
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 5 +++++
2 files changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
index c81805ef2250..4173623cc241 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
@@ -119,6 +119,11 @@ &i2c9 {
status = "disabled";
};

+&keyboard_controller {
+ /delete-property/keypad,num-rows;
+ /delete-property/keypad,num-columns;
+};
+
&panel {
compatible = "boe,nv110wtm-n61";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
index bff2b556cc75..7205062e88b4 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
@@ -121,6 +121,11 @@ &camcc {
status = "okay";
};

+&keyboard_controller {
+ /delete-property/keypad,num-rows;
+ /delete-property/keypad,num-columns;
+};
+
&panel {
compatible = "samsung,atna33xc20";
enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
--
https://chromeos.dev


2022-05-09 02:49:59

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sc7180-trogdor: Remove cros-ec keyboard from detachables

On Tue 26 Apr 17:57 CDT 2022, Stephen Boyd 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. Let's delete the rows/columns properties of the
> device node to indicate that there isn't a matrix keyboard on these
> boards.
>

As this seems to directly relate to the final design of each device,
would it make sense to push out the &keyboard_controller from
trogdor.dtsi? Or do you think it would be too much duplication for it to
be worth it?

Regards,
Bjorn

> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Cc: Hsin-Yi Wang <[email protected]>
> Cc: "Joseph S. Barrera III" <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 +++++
> arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 5 +++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
> index c81805ef2250..4173623cc241 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
> @@ -119,6 +119,11 @@ &i2c9 {
> status = "disabled";
> };
>
> +&keyboard_controller {
> + /delete-property/keypad,num-rows;
> + /delete-property/keypad,num-columns;
> +};
> +
> &panel {
> compatible = "boe,nv110wtm-n61";
> };
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
> index bff2b556cc75..7205062e88b4 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
> @@ -121,6 +121,11 @@ &camcc {
> status = "okay";
> };
>
> +&keyboard_controller {
> + /delete-property/keypad,num-rows;
> + /delete-property/keypad,num-columns;
> +};
> +
> &panel {
> compatible = "samsung,atna33xc20";
> enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
> --
> https://chromeos.dev
>

2022-05-09 07:06:22

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sc7180-trogdor: Remove cros-ec keyboard from detachables

Quoting Bjorn Andersson (2022-05-05 20:40:22)
> On Tue 26 Apr 17:57 CDT 2022, Stephen Boyd 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. Let's delete the rows/columns properties of the
> > device node to indicate that there isn't a matrix keyboard on these
> > boards.
> >
>
> As this seems to directly relate to the final design of each device,
> would it make sense to push out the &keyboard_controller from
> trogdor.dtsi? Or do you think it would be too much duplication for it to
> be worth it?

I tried it out a few days ago but I'm waiting to see how the driver
patch goes before sending the dts bits. See the WIP patches up to the
end of the chain on chromium gerrit[1].

[1] https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3609017