Re: [PATCH v3] iio: light: ltr390: Add device powerdown functionality via devm api

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Aug 13, 2025 at 07:27:09PM +0530, Akshay Jindal wrote:
> On Wed, Aug 13, 2025 at 7:20 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Wed, Aug 13, 2025 at 07:00:14PM +0530, Akshay Jindal wrote:


...

> > > Set client data with i2c_set_clientdata().
> >
> > This is not used anymore, correct?

...

> > > -     data = iio_priv(indio_dev);
> > > +     i2c_set_clientdata(client, indio_dev);
> > >
> > > +     data = iio_priv(indio_dev);
> > >       data->regmap = devm_regmap_init_i2c(client, &ltr390_regmap_config);
> > >       if (IS_ERR(data->regmap))
> > >               return dev_err_probe(dev, PTR_ERR(data->regmap),
> >
> > So this hunk needs to be removed from the patch.
> I thought so, but removing i2c_set_clientdata would mean that
> dev->driver_data will NOT contain a pointer to indio_dev.
> Irrespective of usage, ideally dev->driver_data should contain legit value.

NULL is legit value for driver_data. The rule of thumb is that we don't assign
something that we know will never be used. Also think about debugging issues,
with the "legit" value in some pointer it may lead to not noticing a real
problem or to a problem that never exists if driver_data left untouched.

> Hence I kept it.
> If you feel otherwise, I can remove it, but I feel this should be kept.

I feel definitely otherwise.

-- 
With Best Regards,
Andy Shevchenko