On Sat, Jul 27, 2019 at 12:57:49PM +0100, Jonathan Cameron wrote:
> On Fri, 26 Jul 2019 20:30:58 +0800
> Chuhong Yuan <hslester96@xxxxxxxxx> wrote:
>
> > Use devm_iio_device_register to simplify
> > the code.
> >
> > Signed-off-by: Chuhong Yuan <hslester96@xxxxxxxxx>
>
> Please try to pick up on likely reviewers in your cc list. In this case
> Brian did a lot of work cleaning these drivers up so I've added him.
> Not everyone keeps up with the linux-iio list for some reason ;)
>
> Immediate thought was that you had broken the ordering but turns out
> it is already buggy.
>
> As things stand, we remove the userspace interfaces (iio_device_unregister)
> after turning off the power. This is obviously a bad idea and also
> form a purely "obviously correct" stand point, we aren't doing the reverse
> of probe.
>
> So, I 'think' the right option is to reorder remove so that the power left
> on until after the iio_device_unregister call. At that point, we can't
> use devm_iio_device_register as we'll have the same incorrect ordering
> we currently have.
>
> Brian, you looked at this driver most recently. Thoughts?
devm_add_action() could be used in the probe function to schedule the call
to tsl2772_chip_off(). That would eliminate the need for
tsl2772_remove(). See tsl2772_disable_regulators_action() for an example in
that driver.
Chuhong: Another potential cleanup to shrink the size of this driver is
to move it over to the regulator_bulk_() API. I didn't realize that API
existed at the time I introduced the regulator support. If you're
interested in taking on that cleanup as well, I can test those changes
for you if you don't have the hardware.
Brian
> > ---
> > drivers/iio/light/tsl2772.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/light/tsl2772.c b/drivers/iio/light/tsl2772.c
> > index 83cece921843..aa5891d105e8 100644
> > --- a/drivers/iio/light/tsl2772.c
> > +++ b/drivers/iio/light/tsl2772.c
> > @@ -1877,7 +1877,7 @@ static int tsl2772_probe(struct i2c_client *clientp,
> > if (ret < 0)
> > return ret;
> >
> > - ret = iio_device_register(indio_dev);
> > + ret = devm_iio_device_register(&clientp->dev, indio_dev);
> > if (ret) {
> > tsl2772_chip_off(indio_dev);
> > dev_err(&clientp->dev,
> > @@ -1928,8 +1928,6 @@ static int tsl2772_remove(struct i2c_client *client)
> >
> > tsl2772_chip_off(indio_dev);
> >
> > - iio_device_unregister(indio_dev);
> > -
> > return 0;
> > }
> >
