lkml.org 
[lkml]   [2021]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v1 03/10] hwmon: adt7x10: use devm_request_threaded_irq
From
Date
On 12/21/21 4:39 AM, Cosmin Tanislav wrote:
> From: Cosmin Tanislav <cosmin.tanislav@analog.com>
>
> To simplify the core driver remove function.
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> ---
> drivers/hwmon/adt7x10.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
> index dbe9f1ad7db0..48adc0344e88 100644
> --- a/drivers/hwmon/adt7x10.c
> +++ b/drivers/hwmon/adt7x10.c
> @@ -402,9 +402,11 @@ int adt7x10_probe(struct device *dev, const char *name, int irq,
> }
>
> if (irq > 0) {
> - ret = request_threaded_irq(irq, NULL, adt7x10_irq_handler,
> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> - dev_name(dev), dev);
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + adt7x10_irq_handler,
> + IRQF_TRIGGER_FALLING |
> + IRQF_ONESHOT,
> + dev_name(dev), dev);
> if (ret)
> goto exit_hwmon_device_unregister;
> }
> @@ -425,9 +427,6 @@ void adt7x10_remove(struct device *dev, int irq)
> {
> struct adt7x10_data *data = dev_get_drvdata(dev);
>
> - if (irq > 0)
> - free_irq(irq, dev);
> -
> hwmon_device_unregister(data->hwmon_dev);
> sysfs_remove_group(&dev->kobj, &adt7x10_group);
> if (data->oldconfig != data->config)
>

This will keep the interrupt running after the hwmon device was removed,
and after the configuration was restored. If an interrupt occurs at that time,
the handler may potentially access released data. I don't mind making those
changes, but the patches will have to be well sequenced to ensure that each
patch on its own doesn't leave a crippled driver behind. Again, "oh, this will
be ok after the entire series was applied" is not acceptable.

Thanks,
Guenter

\
 
 \ /
  Last update: 2021-12-21 17:09    [W:0.085 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site