[PATCH] iio: imx7d_adc: Don't pass IIO device to imx7d_adc_{enable,disable}()

[PATCH] iio: imx7d_adc: Don't pass IIO device to imx7d_adc_{enable,disable}()

[Lars-Peter Clausen]

The `imx7d_adc_enable()` and `imx7d_adc_disable()` functions are used as
the suspend and resume callbacks for the device. When called as
suspend/resume functions they are called with the platform_device's device
as their parameter.

In addition the functions are called on device probe and remove. In this
case they are passed the struct device of the IIO device that the driver
registers.

This works because in the `imx7d_adc_{enable,disable}()` functions the
passed struct device is only ever used as a parameter to `dev_get_drvdata()`
and `dev_get_drvdata()` returns the same value for the platform device and
the IIO device.

But for consistency we should pass the same struct device to the
`imx7d_adc_{enable,disable}()` in all cases. This will avoid accidental
breakage if the device is ever used for something more than
`dev_get_drvdata()`.

Another motivation is that `dev_get_drvdata()` on the IIO device relies on
the IIO core calling `dev_set_drvdata()`. Something we want to remove.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/adc/imx7d_adc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/imx7d_adc.c b/drivers/iio/adc/imx7d_adc.c
index 4969a5f941e3..7abf0ad526a3 100644
--- a/drivers/iio/adc/imx7d_adc.c
+++ b/drivers/iio/adc/imx7d_adc.c
@@ -528,12 +528,11 @@ static int imx7d_adc_probe(struct platform_device *pdev)
 
 	imx7d_adc_feature_config(info);
 
-	ret = imx7d_adc_enable(&indio_dev->dev);
+	ret = imx7d_adc_enable(dev);
 	if (ret)
 		return ret;
 
-	ret = devm_add_action_or_reset(dev, __imx7d_adc_disable,
-				       &indio_dev->dev);
+	ret = devm_add_action_or_reset(dev, __imx7d_adc_disable, dev);
 	if (ret)
 		return ret;
 
-- 
2.20.1

Re: [PATCH] iio: imx7d_adc: Don't pass IIO device to imx7d_adc_{enable,disable}()

[Jonathan Cameron]

On Wed, 20 Oct 2021 10:57:54 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> The `imx7d_adc_enable()` and `imx7d_adc_disable()` functions are used as
> the suspend and resume callbacks for the device. When called as
> suspend/resume functions they are called with the platform_device's device
> as their parameter.
> 
> In addition the functions are called on device probe and remove. In this
> case they are passed the struct device of the IIO device that the driver
> registers.
> 
> This works because in the `imx7d_adc_{enable,disable}()` functions the
> passed struct device is only ever used as a parameter to `dev_get_drvdata()`
> and `dev_get_drvdata()` returns the same value for the platform device and
> the IIO device.
> 
> But for consistency we should pass the same struct device to the
> `imx7d_adc_{enable,disable}()` in all cases. This will avoid accidental
> breakage if the device is ever used for something more than
> `dev_get_drvdata()`.
> 
> Another motivation is that `dev_get_drvdata()` on the IIO device relies on
> the IIO core calling `dev_set_drvdata()`. Something we want to remove.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Applied to the togreg branch of iio.git and pushed out as testing to let 0-day
work its magic.

Note, plenty of time for reviews before I push this out as non rebasing if
anyone else has time to take a look.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/imx7d_adc.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/imx7d_adc.c b/drivers/iio/adc/imx7d_adc.c
> index 4969a5f941e3..7abf0ad526a3 100644
> --- a/drivers/iio/adc/imx7d_adc.c
> +++ b/drivers/iio/adc/imx7d_adc.c
> @@ -528,12 +528,11 @@ static int imx7d_adc_probe(struct platform_device *pdev)
>  
>  	imx7d_adc_feature_config(info);
>  
> -	ret = imx7d_adc_enable(&indio_dev->dev);
> +	ret = imx7d_adc_enable(dev);
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_add_action_or_reset(dev, __imx7d_adc_disable,
> -				       &indio_dev->dev);
> +	ret = devm_add_action_or_reset(dev, __imx7d_adc_disable, dev);
>  	if (ret)
>  		return ret;
>