On Wed, 12 May 2021 15:48:57 +0200
Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:
> Em Sun, 9 May 2021 12:33:37 +0100
> Jonathan Cameron <jic23@xxxxxxxxxx> escreveu:
>
> > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> >
> > probe() error paths after runtime pm is enabled, should disable it.
> > remove() should not call pm_runtime_put_noidle() as there is no
> > matching get() to have raised the reference count. This case
> > has no affect a the runtime pm core protects against going negative.
> >
> > Whilst here use pm_runtime_resume_and_get() to tidy things up a little.
> > coccicheck script didn't get this one due to complex code structure so
> > found by inspection.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
> LGTM.
>
> Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders
to take a poke at it.
Still time for additional reviews as this won't go out in a non rebasing
tree for a few days yet.
Thanks,
Jonathan
>
> > ---
> > drivers/iio/magnetometer/bmc150_magn.c | 10 ++++------
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
> > index d75b437a43f2..39920cb764bf 100644
> > --- a/drivers/iio/magnetometer/bmc150_magn.c
> > +++ b/drivers/iio/magnetometer/bmc150_magn.c
> > @@ -262,7 +262,7 @@ static int bmc150_magn_set_power_state(struct bmc150_magn_data *data, bool on)
> > int ret;
> >
> > if (on) {
> > - ret = pm_runtime_get_sync(data->dev);
> > + ret = pm_runtime_resume_and_get(data->dev);
> > } else {
> > pm_runtime_mark_last_busy(data->dev);
> > ret = pm_runtime_put_autosuspend(data->dev);
> > @@ -271,9 +271,6 @@ static int bmc150_magn_set_power_state(struct bmc150_magn_data *data, bool on)
> > if (ret < 0) {
> > dev_err(data->dev,
> > "failed to change power state to %d\n", on);
> > - if (on)
> > - pm_runtime_put_noidle(data->dev);
> > -
> > return ret;
> > }
> > #endif
> > @@ -963,12 +960,14 @@ int bmc150_magn_probe(struct device *dev, struct regmap *regmap,
> > ret = iio_device_register(indio_dev);
> > if (ret < 0) {
> > dev_err(dev, "unable to register iio device\n");
> > - goto err_buffer_cleanup;
> > + goto err_disable_runtime_pm;
> > }
> >
> > dev_dbg(dev, "Registered device %s\n", name);
> > return 0;
> >
> > +err_disable_runtime_pm:
> > + pm_runtime_disable(dev);
> > err_buffer_cleanup:
> > iio_triggered_buffer_cleanup(indio_dev);
> > err_free_irq:
> > @@ -992,7 +991,6 @@ int bmc150_magn_remove(struct device *dev)
> >
> > pm_runtime_disable(dev);
> > pm_runtime_set_suspended(dev);
> > - pm_runtime_put_noidle(dev);
> >
> > iio_triggered_buffer_cleanup(indio_dev);
> >
>
>
>
> Thanks,
> Mauro
