linux-kernel - Re: [PATCH 6/9] iio: common: cros_ec_sensors: simplify getting .driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 25 Sep 2021 15:54:45 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc:     Wolfram Sang <wsa+renesas@...g-engineering.com>,
        linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
        Lars-Peter Clausen <lars@...afoo.de>,
        Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>, linux-iio@...r.kernel.org
Subject: Re: [PATCH 6/9] iio: common: cros_ec_sensors: simplify getting
 .driver_data

On Thu, 23 Sep 2021 11:16:47 +0200
Enric Balletbo i Serra <enric.balletbo@...labora.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@...g-engineering.com>  
> 
> Acked-by: Enric Balletbo i Serra <enric.balletbo@...labora.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@...wei.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;
> >  
> >