lkml.org 
[lkml]   [2022]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 2/2] Input: cros-ec-keyb - skip keyboard registration for switches compatible
    Quoting Doug Anderson (2022-05-02 18:06:39)
    > Hi,
    >
    > On Mon, May 2, 2022 at 3:02 PM Stephen Boyd <swboyd@chromium.org> wrote:
    > >
    > > Quoting Doug Anderson (2022-05-02 10:02:54)
    > > > On Fri, Apr 29, 2022 at 4:31 PM Stephen Boyd <swboyd@chromium.org> wrote:
    > > > >
    > > > >
    > > > > diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
    > > > > index eef909e52e23..1bbe2987bf52 100644
    > > > > --- a/drivers/input/keyboard/cros_ec_keyb.c
    > > > > +++ b/drivers/input/keyboard/cros_ec_keyb.c
    > > > > @@ -536,6 +536,12 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
    > > > > u32 *physmap;
    > > > > u32 key_pos;
    > > > > unsigned int row, col, scancode, n_physmap;
    > > > > + bool register_keyboard;
    > > > > +
    > > > > + /* Skip matrix registration if no keyboard */
    > > > > + register_keyboard = device_get_match_data(dev);
    > > > > + if (!register_keyboard)
    > > > > + return 0;
    > > > >
    > > > > /*
    > > > > * No rows and columns? There isn't a matrix but maybe there are
    > > >
    > > > As per my comments in patch #1, I wonder if it makes sense to delete
    > > > the "No rows and columns?" logic and settle on the compatible as the
    > > > one true way to specify this.
    > > >
    > >
    > > Ok. My only concern is that means we have to check for both compatibles
    > > which is not really how DT compatible strings work. The compatible
    > > string usually finds the more specific compatible that is first in the
    > > list of compatibles in DT. You're essentially proposing that the
    > > switches compatible could be first or last, the order doesn't matter.
    >
    > It's not quite what I was proposing. I think my summary really sums it up:

    Alright, I'm glad I misunderstood.

    >
    > 1. If you have a matrix keyboard and maybe also some buttons/switches
    > then use the compatible: google,cros-ec-keyb
    >
    > 2. If you only have buttons/switches but you want to be compatible
    > with the old driver in Linux that looked for the compatible
    > "google,cros-ec-keyb" and required the matrix properties, use the
    > compatible: "google,cros-ec-keyb-switches", "google,cros-ec-keyb"
    >
    > 3. If you have only buttons/switches and don't need compatibility with
    > old Linux drivers, use the compatible: "google,cros-ec-keyb-switches"
    >
    > ...but just to say it another way:
    >
    > * If you have the compatible "google,cros-ec-keyb-switches" I mean to
    > say that you _only_ have switches and buttons. You'd _never_ have this
    > compatible string if you truly have a matrix keyboard. If you have
    > this, it will always be first.
    >
    > * If you only have switches and buttons but you care about backward
    > compatibility then you can add a fallback compatible second:
    > "google,cros-ec-keyb"
    >
    > * In order for the fallback compatible to be at all useful as a
    > fallback (it's only useful at all if you're on an old driver), if you
    > specify it you should pretend that you have matrix properties even
    > though you don't really have them, just like we used to do.
    >

    Another important point is that the matrix properties are willfully
    ignored by the new driver if the "google,cros-ec-keyb-switches"
    compatible is present. Maybe it should be "google,cros-ec-no-keyb" to
    describe the true intent, i.e. ignore the keyboard properties. Or
    "google,cros-ec-keyboardless". I think it's confusing that I put
    "switches" in the compatible. It really should be about not registering
    the keyboard input device.

    Anyway, I agree that we don't need to use the matrix keyboard properties
    to figure out what to do. In fact, it isn't possible to remove the
    properties if "google,cros-ec-keyb" is present, so checking for them is
    redundant.

    \
     
     \ /
      Last update: 2022-05-03 04:20    [W:8.348 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site