[PATCH 0/9] treewide: simplify getting .driver_data

[PATCH 0/9] treewide: simplify getting .driver_data

[Wolfram Sang]

I got tired of fixing this in Renesas drivers manually, so I took the big
hammer. Remove this cumbersome code pattern which got copy-pasted too much
already:

-	struct platform_device *pdev = to_platform_device(dev);
-	struct ep93xx_keypad *keypad = platform_get_drvdata(pdev);
+	struct ep93xx_keypad *keypad = dev_get_drvdata(dev);

A branch, tested by buildbot, can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git coccinelle/get_drvdata

I am open for other comments, suggestions, too, of course.

Here is the cocci-script I created:

@@
struct device* d;
identifier pdev;
expression *ptr;
@@
(
-	struct platform_device *pdev = to_platform_device(d);
|
-	struct platform_device *pdev;
	...
-	pdev = to_platform_device(d);
)
	<... when != pdev
-	&pdev->dev
+	d
	...>

	ptr =
-	platform_get_drvdata(pdev)
+	dev_get_drvdata(d)

	<... when != pdev
-	&pdev->dev
+	d
	...>

Kind regards,

   Wolfram


Wolfram Sang (9):
  dmaengine: stm32-dmamux: simplify getting .driver_data
  firmware: meson: simplify getting .driver_data
  gpio: xilinx: simplify getting .driver_data
  drm/msm: simplify getting .driver_data
  drm/panfrost: simplify getting .driver_data
  iio: common: cros_ec_sensors: simplify getting .driver_data
  net: mdio: mdio-bcm-iproc: simplify getting .driver_data
  platform: chrome: cros_ec_sensorhub: simplify getting .driver_data
  remoteproc: omap_remoteproc: simplify getting .driver_data

 drivers/dma/stm32-dmamux.c                         | 14 +++++---------
 drivers/firmware/meson/meson_sm.c                  |  3 +--
 drivers/gpio/gpio-xilinx.c                         |  6 ++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            | 13 +++++--------
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c           |  6 ++----
 drivers/gpu/drm/msm/dp/dp_display.c                |  6 ++----
 drivers/gpu/drm/msm/dsi/dsi_host.c                 |  6 ++----
 drivers/gpu/drm/msm/msm_drv.c                      |  3 +--
 drivers/gpu/drm/panfrost/panfrost_device.c         |  6 ++----
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  |  3 +--
 drivers/net/mdio/mdio-bcm-iproc.c                  |  3 +--
 drivers/platform/chrome/cros_ec_sensorhub.c        |  6 ++----
 drivers/remoteproc/omap_remoteproc.c               |  6 ++----
 13 files changed, 28 insertions(+), 53 deletions(-)

-- 
2.30.2

[PATCH 6/9] iio: common: cros_ec_sensors: simplify getting .driver_data

[Wolfram Sang]

We should get 'driver_data' from 'struct device' directly. Going via
platform_device is an unneeded step back and forth.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Build tested only. buildbot is happy.

 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 28bde13003b7..b2725c6adc7f 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -831,8 +831,7 @@ EXPORT_SYMBOL_GPL(cros_ec_sensors_core_write);
 
 static int __maybe_unused cros_ec_sensors_resume(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
 	int ret = 0;
 
-- 
2.30.2

Re: [PATCH 6/9] iio: common: cros_ec_sensors: simplify getting .driver_data

[Enric Balletbo i Serra]

Hi Wolfram,

On 20/9/21 11:05, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

I'm fine to pick this patch through chrome-platform tree if Jonathan is fine, or
can go through his tree.

I plan also to pick patch  "[PATCH 8/9] platform: chrome: cros_ec_sensorhub:
simplify getting .driver_data"

Thanks,
  Enric

> ---
> 
> Build tested only. buildbot is happy.
> 
>  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 28bde13003b7..b2725c6adc7f 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -831,8 +831,7 @@ EXPORT_SYMBOL_GPL(cros_ec_sensors_core_write);
>  
>  static int __maybe_unused cros_ec_sensors_resume(struct device *dev)
>  {
> -	struct platform_device *pdev = to_platform_device(dev);
> -	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
>  	int ret = 0;
>  
> 

Re: [PATCH 6/9] iio: common: cros_ec_sensors: simplify getting .driver_data

[Jonathan Cameron]

On Thu, 23 Sep 2021 11:16:47 +0200
Enric Balletbo i Serra <enric.balletbo@collabora.com> wrote:

> Hi Wolfram,
> 
> On 20/9/21 11:05, Wolfram Sang wrote:
> > We should get 'driver_data' from 'struct device' directly. Going via
> > platform_device is an unneeded step back and forth.
> > 
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>  
> 
> Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
> I'm fine to pick this patch through chrome-platform tree if Jonathan is fine, or
> can go through his tree.

Fine by me, though a suggestion follows to take this a little further than done here.

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

It's not something that ever bothered me that much, but we have had debates in
the past about whether there are semantic issues around this sort of cleanup
as it mixes

platform_set_drvdata() with device_get_drvdata()

Whilst they access the same pointer today, in theory that isn't necessarily
always going to be the case in future and it isn't necessarily apparent
to the casual reader of the code.

In this particular case you could tidy that up by using device_set_drvdata() in
the first place, but then to keep things consistent there is one other place
where platform_get_drvdata is used in a devm_add_action_or_reset() callback.
That one is also easily fixed though if we want to be consistent throughout.

Jonathan

> 
> I plan also to pick patch  "[PATCH 8/9] platform: chrome: cros_ec_sensorhub:
> simplify getting .driver_data"
> 
> Thanks,
>   Enric
> 
> > ---
> > 
> > Build tested only. buildbot is happy.
> > 
> >  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > index 28bde13003b7..b2725c6adc7f 100644
> > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > @@ -831,8 +831,7 @@ EXPORT_SYMBOL_GPL(cros_ec_sensors_core_write);
> >  
> >  static int __maybe_unused cros_ec_sensors_resume(struct device *dev)
> >  {
> > -	struct platform_device *pdev = to_platform_device(dev);
> > -	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> >  	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> >  	int ret = 0;
> >  
> >   

Re: [PATCH 6/9] iio: common: cros_ec_sensors: simplify getting .driver_data

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

Hi Jonathan,

> It's not something that ever bothered me that much, but we have had debates in
> the past about whether there are semantic issues around this sort of cleanup
> as it mixes
> 
> platform_set_drvdata() with device_get_drvdata()

Yeah, I see this concern. Mixing the two makes reading the code a bit
more difficult. As I said, it wasn't so easy to convert set_drvdata, but
I will have another go at this.

> Whilst they access the same pointer today, in theory that isn't necessarily
> always going to be the case in future and it isn't necessarily apparent
> to the casual reader of the code.

That one I don't really see. *_get_drvdata() should always get
'dev->driver_data' and the prefix just tells from what namespace we
come. If you want to change that, a lot of things will break loose, I'd
think. Even in the unlikely case of platform_device gaining a seperate
driver_data(?), it probably should be named *_get_pdrvdata(), or?

Thanks and happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 6/9] iio: common: cros_ec_sensors: simplify getting .driver_data

[Jonathan Cameron]

On Tue, 12 Oct 2021 09:31:11 +0200
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:

> Hi Jonathan,
> 
> > It's not something that ever bothered me that much, but we have had debates in
> > the past about whether there are semantic issues around this sort of cleanup
> > as it mixes
> > 
> > platform_set_drvdata() with device_get_drvdata()  
> 
> Yeah, I see this concern. Mixing the two makes reading the code a bit
> more difficult. As I said, it wasn't so easy to convert set_drvdata, but
> I will have another go at this.
> 
> > Whilst they access the same pointer today, in theory that isn't necessarily
> > always going to be the case in future and it isn't necessarily apparent
> > to the casual reader of the code.  
> 
> That one I don't really see. *_get_drvdata() should always get
> 'dev->driver_data' and the prefix just tells from what namespace we
> come. If you want to change that, a lot of things will break loose, I'd
> think. Even in the unlikely case of platform_device gaining a seperate
> driver_data(?), it probably should be named *_get_pdrvdata(), or?

Agreed. Does indeed seem like any change to this would be a mess so would
require different naming etc.

Thanks,

Jonathan

> 
> Thanks and happy hacking,
> 
>    Wolfram
> 
> 

Re: [PATCH 0/9] treewide: simplify getting .driver_data

[Bjorn Andersson]

On Mon, 20 Sep 2021 11:05:12 +0200, Wolfram Sang wrote:
> I got tired of fixing this in Renesas drivers manually, so I took the big
> hammer. Remove this cumbersome code pattern which got copy-pasted too much
> already:
> 
> -	struct platform_device *pdev = to_platform_device(dev);
> -	struct ep93xx_keypad *keypad = platform_get_drvdata(pdev);
> +	struct ep93xx_keypad *keypad = dev_get_drvdata(dev);
> 
> [...]

Applied, thanks!

[1/9] dmaengine: stm32-dmamux: simplify getting .driver_data
      (no commit info)
[2/9] firmware: meson: simplify getting .driver_data
      (no commit info)
[3/9] gpio: xilinx: simplify getting .driver_data
      (no commit info)
[4/9] drm/msm: simplify getting .driver_data
      (no commit info)
[5/9] drm/panfrost: simplify getting .driver_data
      (no commit info)
[6/9] iio: common: cros_ec_sensors: simplify getting .driver_data
      (no commit info)
[7/9] net: mdio: mdio-bcm-iproc: simplify getting .driver_data
      (no commit info)
[8/9] platform: chrome: cros_ec_sensorhub: simplify getting .driver_data
      (no commit info)
[9/9] remoteproc: omap_remoteproc: simplify getting .driver_data
      commit: c34bfafd7c6ce8bdb5205aa990973b6ec7a6557c

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>