On Wed, 12 May 2021 15:44:30 +0200
Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:
> Em Sun, 9 May 2021 12:33:34 +0100
> Jonathan Cameron <jic23@xxxxxxxxxx> escreveu:
>
> > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> >
> > The call to pm_runtime_put_noidle() in remove() callback is not
> > balanced by any gets
> >
> > Note this doesn't cause any problems beyond reader confusion as the runtime
> > pm core protects against the reference counter going negative.
> >
> > Whilst here, use pm_runtiem_resume_and_get() to simplify code a little.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > Cc: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
>
> RPM get/put logic LGTM.
>
> Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
After our side discussion on this one I wanted to take another look.
I'm not sure I follow the logic behind the runtime_enable() path
and why we don't want to enable runtime_pm until first use. However
it's not directly related to this patch and as far as I can tell
it's just unsuual rather than broken. As such, applied this one to the
togreg branch of iio.git and pushed out as testing.
Thanks,
Jonathan
>
> > ---
> > drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > index 5a7b3e253e58..c06537e106e9 100644
> > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > @@ -163,18 +163,15 @@ int hid_sensor_power_state(struct hid_sensor_common *st, bool state)
> >
> > if (state) {
> > atomic_inc(&st->user_requested_state);
> > - ret = pm_runtime_get_sync(&st->pdev->dev);
> > + ret = pm_runtime_resume_and_get(&st->pdev->dev);
> > } else {
> > atomic_dec(&st->user_requested_state);
> > pm_runtime_mark_last_busy(&st->pdev->dev);
> > pm_runtime_use_autosuspend(&st->pdev->dev);
> > ret = pm_runtime_put_autosuspend(&st->pdev->dev);
> > }
> > - if (ret < 0) {
> > - if (state)
> > - pm_runtime_put_noidle(&st->pdev->dev);
> > + if (ret < 0)
> > return ret;
> > - }
> >
> > return 0;
> > #else
> > @@ -222,7 +219,6 @@ void hid_sensor_remove_trigger(struct iio_dev *indio_dev,
> > pm_runtime_disable(&attrb->pdev->dev);
> >
> > pm_runtime_set_suspended(&attrb->pdev->dev);
> > - pm_runtime_put_noidle(&attrb->pdev->dev);
> >
> > cancel_work_sync(&attrb->work);
> > iio_trigger_unregister(attrb->trigger);
>
>
>
> Thanks,
> Mauro
