2021-03-21 23:48:16

by Dmitry Torokhov

Subject: [PATCH 1/2] Input: wacom_i2c - do not force interrupt trigger

Instead of forcing interrupt trigger to "level low" rely on the
platform to set it up according to how it is wired on the given
board.

Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/touchscreen/wacom_i2c.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index 1afc6bde2891..609ff84e7693 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -195,8 +195,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
input_set_drvdata(input, wac_i2c);

error = request_threaded_irq(client->irq, NULL, wacom_i2c_irq,
- IRQF_TRIGGER_LOW | IRQF_ONESHOT,
- "wacom_i2c", wac_i2c);
+ IRQF_ONESHOT, "wacom_i2c", wac_i2c);
if (error) {
dev_err(&client->dev,
"Failed to enable IRQ, error: %d\n", error);
--
2.31.0.rc2.261.g7f71774620-goog

2021-03-21 23:51:03

by Dmitry Torokhov

Subject: [PATCH 2/2] Input: wacom_i2c - switch to using managed resources

This simplifies error unwinding path and allows us to get rid of
remove() method.

Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/touchscreen/wacom_i2c.c | 55 +++++++++------------------
1 file changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index 609ff84e7693..22826c387da5 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -145,15 +145,16 @@ static void wacom_i2c_close(struct input_dev *dev)
}

static int wacom_i2c_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+ const struct i2c_device_id *id)
{
+ struct device *dev = &client->dev;
struct wacom_i2c *wac_i2c;
struct input_dev *input;
struct wacom_features features = { 0 };
int error;

if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
- dev_err(&client->dev, "i2c_check_functionality error\n");
+ dev_err(dev, "i2c_check_functionality error\n");
return -EIO;
}

@@ -161,21 +162,22 @@ static int wacom_i2c_probe(struct i2c_client *client,
if (error)
return error;

- wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
- input = input_allocate_device();
- if (!wac_i2c || !input) {
- error = -ENOMEM;
- goto err_free_mem;
- }
+ wac_i2c = devm_kzalloc(dev, sizeof(*wac_i2c), GFP_KERNEL);
+ if (!wac_i2c)
+ return -ENOMEM;

wac_i2c->client = client;
+
+ input = devm_input_allocate_device(dev);
+ if (!input)
+ return -ENOMEM;
+
wac_i2c->input = input;

input->name = "Wacom I2C Digitizer";
input->id.bustype = BUS_I2C;
input->id.vendor = 0x56a;
input->id.version = features.fw_version;
- input->dev.parent = &client->dev;
input->open = wacom_i2c_open;
input->close = wacom_i2c_close;

@@ -194,12 +196,11 @@ static int wacom_i2c_probe(struct i2c_client *client,

input_set_drvdata(input, wac_i2c);

- error = request_threaded_irq(client->irq, NULL, wacom_i2c_irq,
- IRQF_ONESHOT, "wacom_i2c", wac_i2c);
+ error = devm_request_threaded_irq(dev, client->irq, NULL, wacom_i2c_irq,
+ IRQF_ONESHOT, "wacom_i2c", wac_i2c);
if (error) {
- dev_err(&client->dev,
- "Failed to enable IRQ, error: %d\n", error);
- goto err_free_mem;
+ dev_err(dev, "Failed to request IRQ: %d\n", error);
+ return error;
}

/* Disable the IRQ, we'll enable it in wac_i2c_open() */
@@ -207,31 +208,10 @@ static int wacom_i2c_probe(struct i2c_client *client,

error = input_register_device(wac_i2c->input);
if (error) {
- dev_err(&client->dev,
- "Failed to register input device, error: %d\n", error);
- goto err_free_irq;
+ dev_err(dev, "Failed to register input device: %d\n", error);
+ return error;
}

- i2c_set_clientdata(client, wac_i2c);
- return 0;
-
-err_free_irq:
- free_irq(client->irq, wac_i2c);
-err_free_mem:
- input_free_device(input);
- kfree(wac_i2c);
-
- return error;
-}
-
-static int wacom_i2c_remove(struct i2c_client *client)
-{
- struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
-
- free_irq(client->irq, wac_i2c);
- input_unregister_device(wac_i2c->input);
- kfree(wac_i2c);
-
return 0;
}

@@ -268,7 +248,6 @@ static struct i2c_driver wacom_i2c_driver = {
},

.probe = wacom_i2c_probe,
- .remove = wacom_i2c_remove,
.id_table = wacom_i2c_id,
};
module_i2c_driver(wacom_i2c_driver);
--
2.31.0.rc2.261.g7f71774620-goog

2021-03-22 13:21:05

by Alistair Francis

Subject: Re: [PATCH 1/2] Input: wacom_i2c - do not force interrupt trigger

On Sun, Mar 21, 2021, at 6:00 PM, Dmitry Torokhov wrote:
> Instead of forcing interrupt trigger to "level low" rely on the
> platform to set it up according to how it is wired on the given
> board.
>
> Signed-off-by: Dmitry Torokhov <[email protected] <mailto:dmitry.torokhov%40gmail.com>>

Reviewed-by: Alistair Francis <[email protected]>

Alistair

> ---
> drivers/input/touchscreen/wacom_i2c.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> index 1afc6bde2891..609ff84e7693 100644
> --- a/drivers/input/touchscreen/wacom_i2c.c
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -195,8 +195,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
> input_set_drvdata(input, wac_i2c);
>
> error = request_threaded_irq(client->irq, NULL, wacom_i2c_irq,
> - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> - "wacom_i2c", wac_i2c);
> + IRQF_ONESHOT, "wacom_i2c", wac_i2c);
> if (error) {
> dev_err(&client->dev,
> "Failed to enable IRQ, error: %d\n", error);
> --
> 2.31.0.rc2.261.g7f71774620-goog
>
>

2021-03-22 13:22:34

by Alistair Francis

Subject: Re: [PATCH 2/2] Input: wacom_i2c - switch to using managed resources

On Sun, Mar 21, 2021, at 6:00 PM, Dmitry Torokhov wrote:
> This simplifies error unwinding path and allows us to get rid of
> remove() method.
>
> Signed-off-by: Dmitry Torokhov <[email protected] <mailto:dmitry.torokhov%40gmail.com>>

Reviewed-by: Alistair Francis <[email protected]>

Alistair

> ---
> drivers/input/touchscreen/wacom_i2c.c | 55 +++++++++------------------
> 1 file changed, 17 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> index 609ff84e7693..22826c387da5 100644
> --- a/drivers/input/touchscreen/wacom_i2c.c
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -145,15 +145,16 @@ static void wacom_i2c_close(struct input_dev *dev)
> }
>
> static int wacom_i2c_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> + const struct i2c_device_id *id)
> {
> + struct device *dev = &client->dev;
> struct wacom_i2c *wac_i2c;
> struct input_dev *input;
> struct wacom_features features = { 0 };
> int error;
>
> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> - dev_err(&client->dev, "i2c_check_functionality error\n");
> + dev_err(dev, "i2c_check_functionality error\n");
> return -EIO;
> }
>
> @@ -161,21 +162,22 @@ static int wacom_i2c_probe(struct i2c_client *client,
> if (error)
> return error;
>
> - wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
> - input = input_allocate_device();
> - if (!wac_i2c || !input) {
> - error = -ENOMEM;
> - goto err_free_mem;
> - }
> + wac_i2c = devm_kzalloc(dev, sizeof(*wac_i2c), GFP_KERNEL);
> + if (!wac_i2c)
> + return -ENOMEM;
>
> wac_i2c->client = client;
> +
> + input = devm_input_allocate_device(dev);
> + if (!input)
> + return -ENOMEM;
> +
> wac_i2c->input = input;
>
> input->name = "Wacom I2C Digitizer";
> input->id.bustype = BUS_I2C;
> input->id.vendor = 0x56a;
> input->id.version = features.fw_version;
> - input->dev.parent = &client->dev;
> input->open = wacom_i2c_open;
> input->close = wacom_i2c_close;
>
> @@ -194,12 +196,11 @@ static int wacom_i2c_probe(struct i2c_client *client,
>
> input_set_drvdata(input, wac_i2c);
>
> - error = request_threaded_irq(client->irq, NULL, wacom_i2c_irq,
> - IRQF_ONESHOT, "wacom_i2c", wac_i2c);
> + error = devm_request_threaded_irq(dev, client->irq, NULL, wacom_i2c_irq,
> + IRQF_ONESHOT, "wacom_i2c", wac_i2c);
> if (error) {
> - dev_err(&client->dev,
> - "Failed to enable IRQ, error: %d\n", error);
> - goto err_free_mem;
> + dev_err(dev, "Failed to request IRQ: %d\n", error);
> + return error;
> }
>
> /* Disable the IRQ, we'll enable it in wac_i2c_open() */
> @@ -207,31 +208,10 @@ static int wacom_i2c_probe(struct i2c_client *client,
>
> error = input_register_device(wac_i2c->input);
> if (error) {
> - dev_err(&client->dev,
> - "Failed to register input device, error: %d\n", error);
> - goto err_free_irq;
> + dev_err(dev, "Failed to register input device: %d\n", error);
> + return error;
> }
>
> - i2c_set_clientdata(client, wac_i2c);
> - return 0;
> -
> -err_free_irq:
> - free_irq(client->irq, wac_i2c);
> -err_free_mem:
> - input_free_device(input);
> - kfree(wac_i2c);
> -
> - return error;
> -}
> -
> -static int wacom_i2c_remove(struct i2c_client *client)
> -{
> - struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> -
> - free_irq(client->irq, wac_i2c);
> - input_unregister_device(wac_i2c->input);
> - kfree(wac_i2c);
> -
> return 0;
> }
>
> @@ -268,7 +248,6 @@ static struct i2c_driver wacom_i2c_driver = {
> },
>
> .probe = wacom_i2c_probe,
> - .remove = wacom_i2c_remove,
> .id_table = wacom_i2c_id,
> };
> module_i2c_driver(wacom_i2c_driver);
> --
> 2.31.0.rc2.261.g7f71774620-goog
>
>