Remove option having i2c client contain raw gpio number instead of proper
IRQ number. There are no users of this facility in mainline and it will
allow cleaning up the driver code with regard to wakeup handling, etc.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/keyboard/tca6416-keypad.c | 27 +++++++++----------------
include/linux/tca6416_keypad.h | 1 -
2 files changed, 10 insertions(+), 18 deletions(-)
diff --git a/drivers/input/keyboard/tca6416-keypad.c b/drivers/input/keyboard/tca6416-keypad.c
index 2f745cabf4f2..01bc0b881188 100644
--- a/drivers/input/keyboard/tca6416-keypad.c
+++ b/drivers/input/keyboard/tca6416-keypad.c
@@ -148,7 +148,7 @@ static int tca6416_keys_open(struct input_dev *dev)
if (chip->use_polling)
schedule_delayed_work(&chip->dwork, msecs_to_jiffies(100));
else
- enable_irq(chip->irqnum);
+ enable_irq(chip->client->irq);
return 0;
}
@@ -160,7 +160,7 @@ static void tca6416_keys_close(struct input_dev *dev)
if (chip->use_polling)
cancel_delayed_work_sync(&chip->dwork);
else
- disable_irq(chip->irqnum);
+ disable_irq(chip->client->irq);
}
static int tca6416_setup_registers(struct tca6416_keypad_chip *chip)
@@ -266,12 +266,7 @@ static int tca6416_keypad_probe(struct i2c_client *client)
goto fail1;
if (!chip->use_polling) {
- if (pdata->irq_is_gpio)
- chip->irqnum = gpio_to_irq(client->irq);
- else
- chip->irqnum = client->irq;
-
- error = request_threaded_irq(chip->irqnum, NULL,
+ error = request_threaded_irq(client->irq, NULL,
tca6416_keys_isr,
IRQF_TRIGGER_FALLING |
IRQF_ONESHOT | IRQF_NO_AUTOEN,
@@ -279,7 +274,7 @@ static int tca6416_keypad_probe(struct i2c_client *client)
if (error) {
dev_dbg(&client->dev,
"Unable to claim irq %d; error %d\n",
- chip->irqnum, error);
+ client->irq, error);
goto fail1;
}
}
@@ -298,8 +293,8 @@ static int tca6416_keypad_probe(struct i2c_client *client)
fail2:
if (!chip->use_polling) {
- free_irq(chip->irqnum, chip);
- enable_irq(chip->irqnum);
+ free_irq(client->irq, chip);
+ enable_irq(client->irq);
}
fail1:
input_free_device(input);
@@ -312,8 +307,8 @@ static void tca6416_keypad_remove(struct i2c_client *client)
struct tca6416_keypad_chip *chip = i2c_get_clientdata(client);
if (!chip->use_polling) {
- free_irq(chip->irqnum, chip);
- enable_irq(chip->irqnum);
+ free_irq(client->irq, chip);
+ enable_irq(client->irq);
}
input_unregister_device(chip->input);
@@ -323,10 +318,9 @@ static void tca6416_keypad_remove(struct i2c_client *client)
static int tca6416_keypad_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
- struct tca6416_keypad_chip *chip = i2c_get_clientdata(client);
if (device_may_wakeup(dev))
- enable_irq_wake(chip->irqnum);
+ enable_irq_wake(client->irq);
return 0;
}
@@ -334,10 +328,9 @@ static int tca6416_keypad_suspend(struct device *dev)
static int tca6416_keypad_resume(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
- struct tca6416_keypad_chip *chip = i2c_get_clientdata(client);
if (device_may_wakeup(dev))
- disable_irq_wake(chip->irqnum);
+ disable_irq_wake(client->irq);
return 0;
}
diff --git a/include/linux/tca6416_keypad.h b/include/linux/tca6416_keypad.h
index b0d36a9934cc..5cf6f6f82aa7 100644
--- a/include/linux/tca6416_keypad.h
+++ b/include/linux/tca6416_keypad.h
@@ -25,7 +25,6 @@ struct tca6416_keys_platform_data {
unsigned int rep:1; /* enable input subsystem auto repeat */
uint16_t pinmask;
uint16_t invert;
- int irq_is_gpio;
int use_polling; /* use polling if Interrupt is not connected*/
};
#endif
--
2.41.0.487.g6d72f3e995-goog
The driver has been switched to use IRQF_NO_AUTOEN, but in the error
unwinding and remove paths calls to enable_irq() were left in place, which
will lead to an incorrect enable counter value.
Fixes: bcd9730a04a1 ("Input: move to use request_irq by IRQF_NO_AUTOEN flag")
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/keyboard/tca6416-keypad.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/tca6416-keypad.c b/drivers/input/keyboard/tca6416-keypad.c
index 906dffbf171c..21a2f2de4345 100644
--- a/drivers/input/keyboard/tca6416-keypad.c
+++ b/drivers/input/keyboard/tca6416-keypad.c
@@ -291,10 +291,8 @@ static int tca6416_keypad_probe(struct i2c_client *client)
return 0;
fail2:
- if (!chip->use_polling) {
+ if (!chip->use_polling)
free_irq(client->irq, chip);
- enable_irq(client->irq);
- }
fail1:
input_free_device(input);
kfree(chip);
@@ -305,10 +303,8 @@ static void tca6416_keypad_remove(struct i2c_client *client)
{
struct tca6416_keypad_chip *chip = i2c_get_clientdata(client);
- if (!chip->use_polling) {
+ if (!chip->use_polling)
free_irq(client->irq, chip);
- enable_irq(client->irq);
- }
input_unregister_device(chip->input);
kfree(chip);
--
2.41.0.487.g6d72f3e995-goog
From: Yangtao Li <[email protected]>
Use devm_* api to simplify code, this makes it unnecessary to explicitly
release resources.
Signed-off-by: Yangtao Li <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/keyboard/tca6416-keypad.c | 53 +++++++++----------------
1 file changed, 18 insertions(+), 35 deletions(-)
diff --git a/drivers/input/keyboard/tca6416-keypad.c b/drivers/input/keyboard/tca6416-keypad.c
index 21a2f2de4345..ff665319791e 100644
--- a/drivers/input/keyboard/tca6416-keypad.c
+++ b/drivers/input/keyboard/tca6416-keypad.c
@@ -216,12 +216,15 @@ static int tca6416_keypad_probe(struct i2c_client *client)
return -EINVAL;
}
- chip = kzalloc(struct_size(chip, buttons, pdata->nbuttons), GFP_KERNEL);
- input = input_allocate_device();
- if (!chip || !input) {
- error = -ENOMEM;
- goto fail1;
- }
+ chip = devm_kzalloc(&client->dev,
+ struct_size(chip, buttons, pdata->nbuttons),
+ GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ input = devm_input_allocate_device(&client->dev);
+ if (!input)
+ return -ENOMEM;
chip->client = client;
chip->input = input;
@@ -233,7 +236,6 @@ static int tca6416_keypad_probe(struct i2c_client *client)
input->phys = "tca6416-keys/input0";
input->name = client->name;
- input->dev.parent = &client->dev;
input->open = tca6416_keys_open;
input->close = tca6416_keys_close;
@@ -263,19 +265,20 @@ static int tca6416_keypad_probe(struct i2c_client *client)
*/
error = tca6416_setup_registers(chip);
if (error)
- goto fail1;
+ return error;
if (!chip->use_polling) {
- error = request_threaded_irq(client->irq, NULL,
- tca6416_keys_isr,
- IRQF_TRIGGER_FALLING |
- IRQF_ONESHOT | IRQF_NO_AUTOEN,
- "tca6416-keypad", chip);
+ error = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, tca6416_keys_isr,
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT |
+ IRQF_NO_AUTOEN,
+ "tca6416-keypad", chip);
if (error) {
dev_dbg(&client->dev,
"Unable to claim irq %d; error %d\n",
client->irq, error);
- goto fail1;
+ return error;
}
}
@@ -283,31 +286,12 @@ static int tca6416_keypad_probe(struct i2c_client *client)
if (error) {
dev_dbg(&client->dev,
"Unable to register input device, error: %d\n", error);
- goto fail2;
+ return error;
}
i2c_set_clientdata(client, chip);
return 0;
-
-fail2:
- if (!chip->use_polling)
- free_irq(client->irq, chip);
-fail1:
- input_free_device(input);
- kfree(chip);
- return error;
-}
-
-static void tca6416_keypad_remove(struct i2c_client *client)
-{
- struct tca6416_keypad_chip *chip = i2c_get_clientdata(client);
-
- if (!chip->use_polling)
- free_irq(client->irq, chip);
-
- input_unregister_device(chip->input);
- kfree(chip);
}
static struct i2c_driver tca6416_keypad_driver = {
@@ -315,7 +299,6 @@ static struct i2c_driver tca6416_keypad_driver = {
.name = "tca6416-keypad",
},
.probe = tca6416_keypad_probe,
- .remove = tca6416_keypad_remove,
.id_table = tca6416_id,
};
--
2.41.0.487.g6d72f3e995-goog
Instead of rolling custom polling implementation use input core
facilities.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
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) {
+ error = input_setup_polling(input, tca6416_keys_scan);
+ if (error) {
+ dev_err(&client->dev, "Failed to setup polling\n");
+ return error;
+ }
+
+ input_set_poll_interval(input, TCA6416_POLL_INTERVAL);
+ } else {
error = devm_request_threaded_irq(&client->dev, client->irq,
NULL, tca6416_keys_isr,
IRQF_TRIGGER_FALLING |
IRQF_ONESHOT |
IRQF_NO_AUTOEN,
- "tca6416-keypad", chip);
+ "tca6416-keypad", input);
if (error) {
dev_dbg(&client->dev,
"Unable to claim irq %d; error %d\n",
--
2.41.0.487.g6d72f3e995-goog
Hi
Just one question below.
Dmitry Torokhov <[email protected]> wrote:
> Instead of rolling custom polling implementation use input core
> facilities.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> 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?
Cheers,
Silvan
> + error = input_setup_polling(input, tca6416_keys_scan);
> + if (error) {
> + dev_err(&client->dev, "Failed to setup polling\n");
> + return error;
> + }
> +
> + input_set_poll_interval(input, TCA6416_POLL_INTERVAL);
> + } else {
> error = devm_request_threaded_irq(&client->dev, client->irq,
> NULL, tca6416_keys_isr,
> IRQF_TRIGGER_FALLING |
> IRQF_ONESHOT |
> IRQF_NO_AUTOEN,
> - "tca6416-keypad", chip);
> + "tca6416-keypad", input);
> if (error) {
> dev_dbg(&client->dev,
> "Unable to claim irq %d; error %d\n",
On Tue, Jul 25, 2023 at 09:43:12PM +0200, Silvan Jegen wrote:
> Hi
>
> Just one question below.
>
> Dmitry Torokhov <[email protected]> wrote:
> > Instead of rolling custom polling implementation use input core
> > facilities.
> >
> > Signed-off-by: Dmitry Torokhov <[email protected]>
> > ---
> > 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