Quoting Doug Anderson (2022-05-03 08:09:11) > On Mon, May 2, 2022 at 9:22 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > > > diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c > > index eef909e52e23..04c550aaf897 100644 > > --- a/drivers/input/keyboard/cros_ec_keyb.c > > +++ b/drivers/input/keyboard/cros_ec_keyb.c > > @@ -536,14 +536,11 @@ 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; > > > > - /* > > - * No rows and columns? There isn't a matrix but maybe there are > > - * switches to register in cros_ec_keyb_register_bs() because > > - * this is a detachable device. > > - */ > > - if (!device_property_present(dev, "keypad,num-rows") && > > - !device_property_present(dev, "keypad,num-cols")) > > + /* Skip matrix registration if no keyboard */ > > + register_keyboard = device_get_match_data(dev); > > + if (!register_keyboard) > > return 0; > > I'm a little on the fence about the local variable. It could have been > shorter as: > > /* Skip matrix registration if no keyboard */ > if (!device_get_match_data(dev)) > > ...but I guess the "register_keyboard" maybe makes it more a little > more obvious? Yes, the idea is to make it more obvious to the point that the comment isn't needed. I'll change it to 'has_keyboard' and then drop the comment! > > In any case, I'm happy either way: > > Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
