[PATCH 1/8] staging:iio:adis16200: Do not return a error in remove function

[PATCH 1/8] staging:iio:adis16200: Do not return a error in remove function

[Lars-Peter Clausen]

In the Linux device driver model the remove callback is not allowed to fail and
the device will be removed regardless of the return value of the remove
callback. So if we abort in the remove function and do not free all resources we
will create a resource leak. Also all kinds of undefined behaviour are expected
to happen since the IIO device is still there while its parent is already gone.

The error which the driver tries to handle in the remove function is
non-critical, so we can just ignore it and continue to free all resources and
remove the IIO device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/gyro/adis16260_core.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16260_core.c b/drivers/staging/iio/gyro/adis16260_core.c
index 1d58d0e..9571c03 100644
--- a/drivers/staging/iio/gyro/adis16260_core.c
+++ b/drivers/staging/iio/gyro/adis16260_core.c
@@ -702,22 +702,16 @@ error_ret:
 
 static int __devexit adis16260_remove(struct spi_device *spi)
 {
-	int ret;
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-
-	ret = adis16260_stop_device(indio_dev);
-	if (ret)
-		goto err_ret;
-
+	adis16260_stop_device(indio_dev);
 	adis16260_remove_trigger(indio_dev);
 	iio_buffer_unregister(indio_dev);
 	adis16260_unconfigure_ring(indio_dev);
 	iio_device_free(indio_dev);
 
-err_ret:
-	return ret;
+	return 0;
 }
 
 /*
-- 
1.7.10.4

[PATCH 2/8] staging:iio:adis16400: Do not return a error in remove function

[Lars-Peter Clausen]

In the Linux device driver model the remove callback is not allowed to fail and
the device will be removed regardless of the return value of the remove
callback. So if we abort in the remove function and do not free all resources we
will create a resource leak. Also all kinds of undefined behaviour are expected
to happen since the IIO device is still there while its parent is already gone.

The error which the driver tries to handle in the remove function is
non-critical, so we can just ignore it and continue to free all resources and
remove the IIO device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/imu/adis16400_core.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/staging/iio/imu/adis16400_core.c b/drivers/staging/iio/imu/adis16400_core.c
index b8c280c..b302c9b 100644
--- a/drivers/staging/iio/imu/adis16400_core.c
+++ b/drivers/staging/iio/imu/adis16400_core.c
@@ -1208,13 +1208,10 @@ error_ret:
 /* fixme, confirm ordering in this function */
 static int __devexit adis16400_remove(struct spi_device *spi)
 {
-	int ret;
 	struct iio_dev *indio_dev =  spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-	ret = adis16400_stop_device(indio_dev);
-	if (ret)
-		goto err_ret;
+	adis16400_stop_device(indio_dev);
 
 	adis16400_remove_trigger(indio_dev);
 	iio_buffer_unregister(indio_dev);
@@ -1222,9 +1219,6 @@ static int __devexit adis16400_remove(struct spi_device *spi)
 	iio_device_free(indio_dev);
 
 	return 0;
-
-err_ret:
-	return ret;
 }
 
 static const struct spi_device_id adis16400_id[] = {
-- 
1.7.10.4

[PATCH 3/8] staging:iio:ade7753: Do not return a error in remove function

[Lars-Peter Clausen]

In the Linux device driver model the remove callback is not allowed to fail and
the device will be removed regardless of the return value of the remove
callback. So if we abort in the remove function and do not free all resources we
will create a resource leak. Also all kinds of undefined behaviour are expected
to happen since the IIO device is still there while its parent is already gone.

The error which the driver tries to handle in the remove function is
non-critical, so we can just ignore it and continue to free all resources and
remove the IIO device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/meter/ade7753.c |   11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7753.c b/drivers/staging/iio/meter/ade7753.c
index 380c05e..8b9eceb 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -557,18 +557,13 @@ error_ret:
 /* fixme, confirm ordering in this function */
 static int __devexit ade7753_remove(struct spi_device *spi)
 {
-	int ret;
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-
-	ret = ade7753_stop_device(&(indio_dev->dev));
-	if (ret)
-		goto err_ret;
-
+	ade7753_stop_device(&indio_dev->dev);
 	iio_device_free(indio_dev);
-err_ret:
-	return ret;
+
+	return 0;
 }
 
 static struct spi_driver ade7753_driver = {
-- 
1.7.10.4

[PATCH 4/8] staging:iio:ade7754: Do not return a error in remove function

[Lars-Peter Clausen]

In the Linux device driver model the remove callback is not allowed to fail and
the device will be removed regardless of the return value of the remove
callback. So if we abort in the remove function and do not free all resources we
will create a resource leak. Also all kinds of undefined behaviour are expected
to happen since the IIO device is still there while its parent is already gone.

The error which the driver tries to handle in the remove function is
non-critical, so we can just ignore it and continue to free all resources and
remove the IIO device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/meter/ade7754.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
index 7dea7fd..76e0ade 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -579,19 +579,13 @@ error_ret:
 /* fixme, confirm ordering in this function */
 static int __devexit ade7754_remove(struct spi_device *spi)
 {
-	int ret;
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-	ret = ade7754_stop_device(&(indio_dev->dev));
-	if (ret)
-		goto err_ret;
-
+	ade7754_stop_device(&indio_dev->dev);
 	iio_device_free(indio_dev);
 
-err_ret:
-	return ret;
-
+	return 0;
 }
 
 static struct spi_driver ade7754_driver = {
-- 
1.7.10.4

[PATCH 5/8] staging:iio:ade7758: Do not return a error in remove function

[Lars-Peter Clausen]

In the Linux device driver model the remove callback is not allowed to fail and
the device will be removed regardless of the return value of the remove
callback. So if we abort in the remove function and do not free all resources we
will create a resource leak. Also all kinds of undefined behaviour are expected
to happen since the IIO device is still there while its parent is already gone.

The error which the driver tries to handle in the remove function is
non-critical, so we can just ignore it and continue to free all resources and
remove the IIO device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/meter/ade7758_core.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7758_core.c b/drivers/staging/iio/meter/ade7758_core.c
index 958f8f2..a0fef77 100644
--- a/drivers/staging/iio/meter/ade7758_core.c
+++ b/drivers/staging/iio/meter/ade7758_core.c
@@ -966,13 +966,9 @@ static int __devexit ade7758_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct ade7758_state *st = iio_priv(indio_dev);
-	int ret;
 
 	iio_device_unregister(indio_dev);
-	ret = ade7758_stop_device(&indio_dev->dev);
-	if (ret)
-		goto err_ret;
-
+	ade7758_stop_device(&indio_dev->dev);
 	ade7758_remove_trigger(indio_dev);
 	ade7758_uninitialize_ring(indio_dev);
 	ade7758_unconfigure_ring(indio_dev);
@@ -981,8 +977,7 @@ static int __devexit ade7758_remove(struct spi_device *spi)
 
 	iio_device_free(indio_dev);
 
-err_ret:
-	return ret;
+	return 0;
 }
 
 static const struct spi_device_id ade7758_id[] = {
-- 
1.7.10.4

[PATCH 6/8] staging:iio:ade7759: Do not return a error in remove function

[Lars-Peter Clausen]

In the Linux device driver model the remove callback is not allowed to fail and
the device will be removed regardless of the return value of the remove
callback. So if we abort in the remove function and do not free all resources we
will create a resource leak. Also all kinds of undefined behaviour are expected
to happen since the IIO device is still there while its parent is already gone.

The error which the driver tries to handle in the remove function is
non-critical, so we can just ignore it and continue to free all resources and
remove the IIO device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/meter/ade7759.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7759.c b/drivers/staging/iio/meter/ade7759.c
index 435e35b..cb0707c 100644
--- a/drivers/staging/iio/meter/ade7759.c
+++ b/drivers/staging/iio/meter/ade7759.c
@@ -501,18 +501,13 @@ error_ret:
 /* fixme, confirm ordering in this function */
 static int __devexit ade7759_remove(struct spi_device *spi)
 {
-	int ret;
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-	ret = ade7759_stop_device(&(indio_dev->dev));
-	if (ret)
-		goto err_ret;
-
+	ade7759_stop_device(&indio_dev->dev);
 	iio_device_free(indio_dev);
 
-err_ret:
-	return ret;
+	return 0;
 }
 
 static struct spi_driver ade7759_driver = {
-- 
1.7.10.4

[PATCH 7/8] staging:iio:lis3l02dq: Do not return a error in remove function

[Lars-Peter Clausen]

In the Linux device driver model the remove callback is not allowed to fail and
the device will be removed regardless of the return value of the remove
callback. So if we abort in the remove function and do not free all resources we
will create a resource leak. Also all kinds of undefined behaviour are expected
to happen since the IIO device is still there while its parent is already gone.

The errors which the driver tries to handle in the remove function are
non-critical, so we can just ignore them and continue to free all resources and
remove the IIO device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/accel/lis3l02dq_core.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/accel/lis3l02dq_core.c b/drivers/staging/iio/accel/lis3l02dq_core.c
index d900d63..21b0469 100644
--- a/drivers/staging/iio/accel/lis3l02dq_core.c
+++ b/drivers/staging/iio/accel/lis3l02dq_core.c
@@ -782,19 +782,13 @@ err_ret:
 /* fixme, confirm ordering in this function */
 static int __devexit lis3l02dq_remove(struct spi_device *spi)
 {
-	int ret;
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct lis3l02dq_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
 
-	ret = lis3l02dq_disable_all_events(indio_dev);
-	if (ret)
-		goto err_ret;
-
-	ret = lis3l02dq_stop_device(indio_dev);
-	if (ret)
-		goto err_ret;
+	lis3l02dq_disable_all_events(indio_dev);
+	lis3l02dq_stop_device(indio_dev);
 
 	if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0)
 		free_irq(st->us->irq, indio_dev);
@@ -804,8 +798,8 @@ static int __devexit lis3l02dq_remove(struct spi_device *spi)
 	lis3l02dq_unconfigure_buffer(indio_dev);
 
 	iio_device_free(indio_dev);
-err_ret:
-	return ret;
+
+	return 0;
 }
 
 static struct spi_driver lis3l02dq_driver = {
-- 
1.7.10.4

[PATCH 8/8] staging:iio:sca3000: Do not return a error in remove function

[Lars-Peter Clausen]

In the Linux device driver model the remove callback is not allowed to fail and
the device will be removed regardless of the return value of the remove
callback. So if we abort in the remove function and do not free all resources we
will create a resource leak. Also all kinds of undefined behaviour are expected
to happen since the IIO device is still there while its parent is already gone.

The errors which the driver tries to handle in the remove function are
non-critical, so we can just ignore them and continue to free all resources and
remove the IIO device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/accel/sca3000_core.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
index 6d72d97..ffd1697 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -1237,11 +1237,9 @@ static int __devexit sca3000_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct sca3000_state *st = iio_priv(indio_dev);
-	int ret;
+
 	/* Must ensure no interrupts can be generated after this!*/
-	ret = sca3000_stop_all_interrupts(st);
-	if (ret)
-		return ret;
+	sca3000_stop_all_interrupts(st);
 	if (spi->irq)
 		free_irq(spi->irq, indio_dev);
 	iio_device_unregister(indio_dev);
-- 
1.7.10.4

Re: [PATCH 1/8] staging:iio:adis16200: Do not return a error in remove function

[Jonathan Cameron]

On 09/22/2012 09:56 AM, Lars-Peter Clausen wrote:
> In the Linux device driver model the remove callback is not allowed to fail and
> the device will be removed regardless of the return value of the remove
> callback. So if we abort in the remove function and do not free all resources we
> will create a resource leak. Also all kinds of undefined behaviour are expected
> to happen since the IIO device is still there while its parent is already gone.
> 
> The error which the driver tries to handle in the remove function is
> non-critical, so we can just ignore it and continue to free all resources and
> remove the IIO device.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Added all 8 to togreg branch of iio.git.

Eariler in the cycle I'd have pushed these as fixes, but as they've been
there a long time, lets just leave it to the next cycle.

Good catch on these btw.

> ---
>  drivers/staging/iio/gyro/adis16260_core.c |   10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/gyro/adis16260_core.c b/drivers/staging/iio/gyro/adis16260_core.c
> index 1d58d0e..9571c03 100644
> --- a/drivers/staging/iio/gyro/adis16260_core.c
> +++ b/drivers/staging/iio/gyro/adis16260_core.c
> @@ -702,22 +702,16 @@ error_ret:
>  
>  static int __devexit adis16260_remove(struct spi_device *spi)
>  {
> -	int ret;
>  	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>  
>  	iio_device_unregister(indio_dev);
> -
> -	ret = adis16260_stop_device(indio_dev);
> -	if (ret)
> -		goto err_ret;
> -
> +	adis16260_stop_device(indio_dev);
>  	adis16260_remove_trigger(indio_dev);
>  	iio_buffer_unregister(indio_dev);
>  	adis16260_unconfigure_ring(indio_dev);
>  	iio_device_free(indio_dev);
>  
> -err_ret:
> -	return ret;
> +	return 0;
>  }
>  
>  /*
>