| Message ID | 20211020085754.16654-1-lars@metafoo.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: imx7d_adc: Don't pass IIO device to imx7d_adc_{enable,disable}() | expand |
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; >
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;
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(-)