Hi Marek,
On Thu, Jan 03, 2019 at 02:29:33AM +0100, Marek Vasut wrote:
> Convert the driver to devm_request_threaded_irq() and drop the related
> unmanaged deregistration code.
>
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> Cc: Henrik Rydberg <rydberg@xxxxxxxxxxx>
> Cc: Olivier Sobrie <olivier@xxxxxxxxx>
> Cc: Philipp Puschmann <pp@xxxxxxxxx>
> To: linux-input@xxxxxxxxxxxxxxx
> ---
> V2: Retain workqueue
> V3: Reword the commit message
> ---
> drivers/input/touchscreen/ili210x.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index 788f4140e302..436bfedb6769 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -5,7 +5,6 @@
> #include <linux/input.h>
> #include <linux/input/mt.h>
> #include <linux/delay.h>
> -#include <linux/workqueue.h>
>
> #define MAX_TOUCHES 2
> #define DEFAULT_POLL_PERIOD 20
> @@ -240,19 +239,19 @@ static int ili210x_i2c_probe(struct i2c_client *client,
>
> i2c_set_clientdata(client, priv);
>
> - error = request_irq(client->irq, ili210x_irq, 0,
> - client->name, priv);
> + error = devm_request_threaded_irq(dev, client->irq, NULL, ili210x_irq,
> + IRQF_ONESHOT, client->name, priv);
What is the point of making it threaded given that the only thing you do
in now threaded interrupt handler is scheduling a work?
> if (error) {
> dev_err(dev, "Unable to request touchscreen IRQ, err: %d\n",
> error);
> - goto err_free_mem;
> + return error;
> }
>
> error = sysfs_create_group(&dev->kobj, &ili210x_attr_group);
> if (error) {
> dev_err(dev, "Unable to create sysfs attributes, err: %d\n",
> error);
> - goto err_free_irq;
> + return error;
> }
>
> error = input_register_device(priv->input);
> @@ -271,9 +270,6 @@ static int ili210x_i2c_probe(struct i2c_client *client,
>
> err_remove_sysfs:
> sysfs_remove_group(&dev->kobj, &ili210x_attr_group);
> -err_free_irq:
> - free_irq(client->irq, priv);
> -err_free_mem:
> return error;
> }
>
> @@ -282,7 +278,6 @@ static int ili210x_i2c_remove(struct i2c_client *client)
> struct ili210x *priv = i2c_get_clientdata(client);
>
> sysfs_remove_group(&client->dev.kobj, &ili210x_attr_group);
> - free_irq(priv->client->irq, priv);
> cancel_delayed_work_sync(&priv->dwork);
This is wrong as now can end up with work being scheduled again and
you'll end up with use after free.
Thanks.
--
Dmitry
