On Sun, 16 May 2021 19:19:40 -0700
Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
> On Sun, 2021-05-16 at 16:19 +0100, Jonathan Cameron wrote:
> > 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.
> >
>
> This was enabled in init path but caused issue with USB sensor hub in
> Thinkpasd Yoga S1, where we can never power off sensor and it wakes up
> device immediately after suspend.
>
> commit
> ad7532cefd11d11a0814a75fb814c205ee3d9d4c
Ah. Thanks for the history. Thanks makes a horrible kind of sense ;)
Jonathan
>
>
> Thanks,
> Srinivas
>
> > 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
> >
>
>
