| Message ID | 20220427203026.828183-2-swboyd@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: google,cros-ec-keyb: Introduce switches only compatible | expand |
On 28/04/2022 18:01, Stephen Boyd wrote: >> >>> Given that none of the properties are >>> required for google,cros-ec-keyb it didn't seem necessary to make having >>> the google,cros-ec-keyb-switches compatible deny the existence of the >>> matrix-keymap properties. >> >> Maybe I misunderstood the commit msg. Are the >> "google,cros-ec-keyb-switches" devices coming with matrix keyboard or >> not? I mean physically. >> > > The answer is "sometimes, physically". Sometimes there are switches like > volume buttons and power buttons and also a matrix keyboard (convertible > and clamshells). Other times there are volume buttons and power buttons > and no matrix keyboard (detachable). This device node represents both > the keyboard and the switches. > > Unfortunately the EC firmware on older Chromebooks that don't have a > matrix keyboard still report that they have some number of columns and > rows. I was hoping to make this fully dynamic by querying the EC but > that isn't possible. OK, then it's indeed slightly different case. Let's skip my comment. Best regards, Krzysztof
On 29/04/2022 18:31, Doug Anderson wrote: >> - $ref: "/schemas/input/matrix-keymap.yaml#" >> >> properties: >> compatible: >> - const: google,cros-ec-keyb >> + oneOf: >> + - items: >> + - const: google,cros-ec-keyb-switches >> + - const: google,cros-ec-keyb >> + - items: >> + - const: google,cros-ec-keyb > > nit: if I come back and read this binding later I'm not sure it would > be obvious which compatible I should pick. Can we give any description > here that indicates that the first choice is for devices that _only_ > have buttons and switches (the google,cros-ec-keyb is just for > backward compatibility) and the second choice is for devices that have > a physical keyboard and _also_ possibly some buttons/switches? > > I could also imagine people in the future being confused about whether > it's allowed to specify matrix properties even for devices that don't > have a matrix keyboard. It might be worth noting that it's allowed (to > support old drivers that might still be matching against the > google,cros-ec-keyb compatible) but not required. +1 > > >> google,needs-ghost-filter: >> description: >> @@ -50,7 +56,7 @@ examples: >> - | >> #include <dt-bindings/input/input.h> >> cros-ec-keyb { >> - compatible = "google,cros-ec-keyb"; >> + compatible = "google,cros-ec-keyb-switches", "google,cros-ec-keyb"; > > Feels like we should create a second example? +1 as well, because it really would confuse what's the difference between them. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml index e8f137abb03c..edc1194d558d 100644 --- a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml +++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml @@ -15,14 +15,20 @@ description: | Google's ChromeOS EC Keyboard is a simple matrix keyboard implemented on a separate EC (Embedded Controller) device. It provides a message for reading key scans from the EC. These are then converted - into keycodes for processing by the kernel. + into keycodes for processing by the kernel. This device also supports + switches/buttons like power and volume buttons. allOf: - $ref: "/schemas/input/matrix-keymap.yaml#" properties: compatible: - const: google,cros-ec-keyb + oneOf: + - items: + - const: google,cros-ec-keyb-switches + - const: google,cros-ec-keyb + - items: + - const: google,cros-ec-keyb google,needs-ghost-filter: description: @@ -50,7 +56,7 @@ examples: - | #include <dt-bindings/input/input.h> cros-ec-keyb { - compatible = "google,cros-ec-keyb"; + compatible = "google,cros-ec-keyb-switches", "google,cros-ec-keyb"; keypad,num-rows = <8>; keypad,num-columns = <13>; google,needs-ghost-filter;
If the device is a detachable, this device won't have a matrix keyboard but it may have some button switches, e.g. volume buttons and power buttons. Let's add a more specific compatible for this type of device that indicates to the OS that there are only switches and no matrix keyboard present. Cc: Krzysztof Kozlowski <krzk@kernel.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: <devicetree@vger.kernel.org> 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> --- .../bindings/input/google,cros-ec-keyb.yaml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)