diff mbox series

[07/10] platform/x86: toshiba_acpi: use device-managed functions for accelerometer

Message ID 20210324125548.45983-8-aardelean@deviqon.com (mailing list archive)
State New
Headers show
Series platform/x86: toshiba_acpi: move acpi add/remove to device-managed routines | expand

Commit Message

Alexandru Ardelean March 24, 2021, 12:55 p.m. UTC
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(-)

Comments

Jonathan Cameron March 29, 2021, 2:54 p.m. UTC | #1
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 mbox series

Patch

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: