On Tue, Jul 25, 2023 at 09:43:12PM +0200, Silvan Jegen wrote:
> Hi
>
> Just one question below.
>
> Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
> > Instead of rolling custom polling implementation use input core
> > facilities.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > ---
> > drivers/input/keyboard/tca6416-keypad.c | 46 ++++++++++---------------
> > 1 file changed, 19 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/tca6416-keypad.c b/drivers/input/keyboard/tca6416-keypad.c
> > index ff665319791e..ebc8b9561266 100644
> > --- a/drivers/input/keyboard/tca6416-keypad.c
> > +++ b/drivers/input/keyboard/tca6416-keypad.c
> > @@ -24,6 +24,8 @@
> > #define TCA6416_INVERT 2
> > #define TCA6416_DIRECTION 3
> >
> > +#define TCA6416_POLL_INTERVAL 100 /* msec */
> > +
> > static const struct i2c_device_id tca6416_id[] = {
> > { "tca6416-keys", 16, },
> > { "tca6408-keys", 8, },
> > @@ -43,7 +45,6 @@ struct tca6416_keypad_chip {
> >
> > struct i2c_client *client;
> > struct input_dev *input;
> > - struct delayed_work dwork;
> > int io_size;
> > int irqnum;
> > u16 pinmask;
> > @@ -85,9 +86,9 @@ static int tca6416_read_reg(struct tca6416_keypad_chip *chip, int reg, u16 *val)
> > return 0;
> > }
> >
> > -static void tca6416_keys_scan(struct tca6416_keypad_chip *chip)
> > +static void tca6416_keys_scan(struct input_dev *input)
> > {
> > - struct input_dev *input = chip->input;
> > + struct tca6416_keypad_chip *chip = input_get_drvdata(input);
> > u16 reg_val, val;
> > int error, i, pin_index;
> >
> > @@ -122,33 +123,20 @@ static void tca6416_keys_scan(struct tca6416_keypad_chip *chip)
> > */
> > static irqreturn_t tca6416_keys_isr(int irq, void *dev_id)
> > {
> > - struct tca6416_keypad_chip *chip = dev_id;
> > -
> > - tca6416_keys_scan(chip);
> > + tca6416_keys_scan(dev_id);
> >
> > return IRQ_HANDLED;
> > }
> >
> > -static void tca6416_keys_work_func(struct work_struct *work)
> > -{
> > - struct tca6416_keypad_chip *chip =
> > - container_of(work, struct tca6416_keypad_chip, dwork.work);
> > -
> > - tca6416_keys_scan(chip);
> > - schedule_delayed_work(&chip->dwork, msecs_to_jiffies(100));
> > -}
> > -
> > static int tca6416_keys_open(struct input_dev *dev)
> > {
> > struct tca6416_keypad_chip *chip = input_get_drvdata(dev);
> >
> > - /* Get initial device state in case it has switches */
> > - tca6416_keys_scan(chip);
> > -
> > - if (chip->use_polling)
> > - schedule_delayed_work(&chip->dwork, msecs_to_jiffies(100));
> > - else
> > + if (!chip->use_polling) {
> > + /* Get initial device state in case it has switches */
> > + tca6416_keys_scan(dev);
> > enable_irq(chip->client->irq);
> > + }
> >
> > return 0;
> > }
> > @@ -157,9 +145,7 @@ static void tca6416_keys_close(struct input_dev *dev)
> > {
> > struct tca6416_keypad_chip *chip = input_get_drvdata(dev);
> >
> > - if (chip->use_polling)
> > - cancel_delayed_work_sync(&chip->dwork);
> > - else
> > + if (!chip->use_polling)
> > disable_irq(chip->client->irq);
> > }
> >
> > @@ -232,8 +218,6 @@ static int tca6416_keypad_probe(struct i2c_client *client)
> > chip->pinmask = pdata->pinmask;
> > chip->use_polling = pdata->use_polling;
> >
> > - INIT_DELAYED_WORK(&chip->dwork, tca6416_keys_work_func);
> > -
> > input->phys = "tca6416-keys/input0";
> > input->name = client->name;
> >
> > @@ -268,12 +252,20 @@ static int tca6416_keypad_probe(struct i2c_client *client)
> > return error;
> >
> > if (!chip->use_polling) {
>
> Sorry for my ignorant question but it seems counterituitive that we set
> up polling when chip->use_polling is false. Is this intended?
Nope, this is my brain fart. Thanks for noticing! I'll fix up the
condition before committing...
Thanks.
--
Dmitry
