| Message ID | 20210324125548.45983-8-aardelean@deviqon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | platform/x86: toshiba_acpi: move acpi add/remove to device-managed routines | expand |
On Wed, 24 Mar 2021 14:55:45 +0200 Alexandru Ardelean <aardelean@deviqon.com> wrote: > This change converts the IIO registration to use devm_iio_device_alloc() > and devm_iio_device_register(). > With this change we can remove the manual deregistrations an freeing of the > IIO data. > > This also makes the deregistration symmetrical with the registration. > > One side-effect (that is undesired), is that if devm_iio_device_register() > fails, then the IIO object will not be free'd and will stick around until > the parent object is free'd. This is because there is no > devm_iio_device_free() function anymore in IIO. > However, this is a pretty bad corner-case that should not happen under > normal operation. Hmm. The way this driver papers over failed elements is rather irritating, though I'm sure there are reasons for it. Indeed, I'd not worry about the left over iio_dev structures > > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/platform/x86/toshiba_acpi.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c > index e787c140eec2..12860ef60e4d 100644 > --- a/drivers/platform/x86/toshiba_acpi.c > +++ b/drivers/platform/x86/toshiba_acpi.c > @@ -2992,11 +2992,6 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev) > > remove_toshiba_proc_entries(dev); > > - if (dev->accelerometer_supported && dev->indio_dev) { > - iio_device_unregister(dev->indio_dev); > - iio_device_free(dev->indio_dev); > - } > - > if (dev->sysfs_created) > sysfs_remove_group(&dev->acpi_dev->dev.kobj, > &toshiba_attr_group); > @@ -3149,7 +3144,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev) > > toshiba_accelerometer_available(dev); > if (dev->accelerometer_supported) { > - dev->indio_dev = iio_device_alloc(&acpi_dev->dev, sizeof(*dev)); > + dev->indio_dev = devm_iio_device_alloc(parent, sizeof(*dev)); > if (!dev->indio_dev) { > pr_err("Unable to allocate iio device\n"); > goto iio_error; > @@ -3164,10 +3159,10 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev) > dev->indio_dev->num_channels = > ARRAY_SIZE(toshiba_iio_accel_channels); > > - ret = iio_device_register(dev->indio_dev); > + ret = devm_iio_device_register(parent, dev->indio_dev); > if (ret < 0) { > pr_err("Unable to register iio device\n"); > - iio_device_free(dev->indio_dev); > + dev->indio_dev = NULL; > } > } > iio_error:
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c index e787c140eec2..12860ef60e4d 100644 --- a/drivers/platform/x86/toshiba_acpi.c +++ b/drivers/platform/x86/toshiba_acpi.c @@ -2992,11 +2992,6 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev) remove_toshiba_proc_entries(dev); - if (dev->accelerometer_supported && dev->indio_dev) { - iio_device_unregister(dev->indio_dev); - iio_device_free(dev->indio_dev); - } - if (dev->sysfs_created) sysfs_remove_group(&dev->acpi_dev->dev.kobj, &toshiba_attr_group); @@ -3149,7 +3144,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev) toshiba_accelerometer_available(dev); if (dev->accelerometer_supported) { - dev->indio_dev = iio_device_alloc(&acpi_dev->dev, sizeof(*dev)); + dev->indio_dev = devm_iio_device_alloc(parent, sizeof(*dev)); if (!dev->indio_dev) { pr_err("Unable to allocate iio device\n"); goto iio_error; @@ -3164,10 +3159,10 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev) dev->indio_dev->num_channels = ARRAY_SIZE(toshiba_iio_accel_channels); - ret = iio_device_register(dev->indio_dev); + ret = devm_iio_device_register(parent, dev->indio_dev); if (ret < 0) { pr_err("Unable to register iio device\n"); - iio_device_free(dev->indio_dev); + dev->indio_dev = NULL; } } iio_error:
This change converts the IIO registration to use devm_iio_device_alloc() and devm_iio_device_register(). With this change we can remove the manual deregistrations an freeing of the IIO data. This also makes the deregistration symmetrical with the registration. One side-effect (that is undesired), is that if devm_iio_device_register() fails, then the IIO object will not be free'd and will stick around until the parent object is free'd. This is because there is no devm_iio_device_free() function anymore in IIO. However, this is a pretty bad corner-case that should not happen under normal operation. Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com> --- drivers/platform/x86/toshiba_acpi.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)