| Message ID | 20211221123944.2683245-9-demonsingur@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1,01/10] hwmon: adt7x10: store bus_dev in private data | expand |
On 12/21/21 4:39 AM, Cosmin Tanislav wrote: > From: Cosmin Tanislav <cosmin.tanislav@analog.com> > > To simplify core driver remove function. > > Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com> > --- > drivers/hwmon/adt7x10.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c > index dd4901299590..c03805c72906 100644 > --- a/drivers/hwmon/adt7x10.c > +++ b/drivers/hwmon/adt7x10.c > @@ -54,7 +54,6 @@ > /* Each client has this additional data */ > struct adt7x10_data { > const struct adt7x10_ops *ops; > - struct device *hwmon_dev; > struct device *bus_dev; > struct mutex update_lock; > u8 config; > @@ -430,8 +429,8 @@ int adt7x10_probe(struct device *dev, const char *name, int irq, > if (ret) > goto exit_restore; > > - hdev = hwmon_device_register_with_info(dev, name, data, > - &adt7x10_chip_info, NULL); > + hdev = devm_hwmon_device_register_with_info(dev, name, data, > + &adt7x10_chip_info, NULL); > > if (IS_ERR(hdev)) { > ret = PTR_ERR(hdev); > @@ -445,15 +444,11 @@ int adt7x10_probe(struct device *dev, const char *name, int irq, > IRQF_ONESHOT, > dev_name(dev), hdev); > if (ret) > - goto exit_hwmon_device_unregister; > + goto exit_restore; > } > > - data->hwmon_dev = hdev; > - > return 0; > > -exit_hwmon_device_unregister: > - hwmon_device_unregister(hdev); > exit_restore: > adt7x10_write_byte(dev, ADT7X10_CONFIG, data->oldconfig); > return ret; > @@ -464,7 +459,6 @@ void adt7x10_remove(struct device *dev, int irq) > { > struct adt7x10_data *data = dev_get_drvdata(dev); > > - hwmon_device_unregister(data->hwmon_dev); > if (data->oldconfig != data->config) > adt7x10_write_byte(dev, ADT7X10_CONFIG, data->oldconfig); This doesn't work as-is because the hwmon device still exists at this point and at least in theory userspace could still write into the device after the old configuration was restored. To fix this, you'll need a preceding patch to introduce adt7x10_restore_config() or similar, and call it through devm_add_action_or_reset() after updating the chip configuration. You can then drop the restore code from here and from the exist_restore code in the probe function. After that, you can use devm_hwmon_device_register_with_info() and drop the remove function entirely. Guenter
diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c index dd4901299590..c03805c72906 100644 --- a/drivers/hwmon/adt7x10.c +++ b/drivers/hwmon/adt7x10.c @@ -54,7 +54,6 @@ /* Each client has this additional data */ struct adt7x10_data { const struct adt7x10_ops *ops; - struct device *hwmon_dev; struct device *bus_dev; struct mutex update_lock; u8 config; @@ -430,8 +429,8 @@ int adt7x10_probe(struct device *dev, const char *name, int irq, if (ret) goto exit_restore; - hdev = hwmon_device_register_with_info(dev, name, data, - &adt7x10_chip_info, NULL); + hdev = devm_hwmon_device_register_with_info(dev, name, data, + &adt7x10_chip_info, NULL); if (IS_ERR(hdev)) { ret = PTR_ERR(hdev); @@ -445,15 +444,11 @@ int adt7x10_probe(struct device *dev, const char *name, int irq, IRQF_ONESHOT, dev_name(dev), hdev); if (ret) - goto exit_hwmon_device_unregister; + goto exit_restore; } - data->hwmon_dev = hdev; - return 0; -exit_hwmon_device_unregister: - hwmon_device_unregister(hdev); exit_restore: adt7x10_write_byte(dev, ADT7X10_CONFIG, data->oldconfig); return ret; @@ -464,7 +459,6 @@ void adt7x10_remove(struct device *dev, int irq) { struct adt7x10_data *data = dev_get_drvdata(dev); - hwmon_device_unregister(data->hwmon_dev); if (data->oldconfig != data->config) adt7x10_write_byte(dev, ADT7X10_CONFIG, data->oldconfig); }