On 1/15/19 1:33 AM, Dmitry Torokhov wrote:
> 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.
So shall I just drop this patch ?
--
Best regards,
Marek Vasut
