| Message ID | 20210509113354.660190-24-jic23@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IIO: Runtime PM related cleanups. | expand |
Em Sun, 9 May 2021 12:33:49 +0100 Jonathan Cameron <jic23@kernel.org> escreveu: > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > This case illustrates why the new pm_runtime_sync_and_get() is good > in that it makes clear the correct way to handle errors. > > Calling pm_runtime_put() on failure of the _get() in pm_runtime_get_sync() > will potentially result in powering down an already powered down device > (as we never successfully powered it up. Unlikely to cause any problems > in reality. > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> This one seems a little odd on my eyes, although I don't know much about IIO, as it calls RPM get only at: zpa2326_init_runtime() on a balanced way, as the routine starts with RPM get() ends with RPM put(). Then it does a put at zpa2326_suspend() and a get at zpa2326_resume(). Can RPM usage_count be zero at suspend (or are there some other part of IIO core that increments it?). Because, if after resume, usage_count would be equal to 1, as I guess RPM core prevent negative values. Regards, Mauro > --- > drivers/iio/pressure/zpa2326.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c > index 89295c90f801..97ac3ba399f7 100644 > --- a/drivers/iio/pressure/zpa2326.c > +++ b/drivers/iio/pressure/zpa2326.c > @@ -664,11 +664,9 @@ static int zpa2326_resume(const struct iio_dev *indio_dev) > { > int err; > > - err = pm_runtime_get_sync(indio_dev->dev.parent); > - if (err < 0) { > - pm_runtime_put(indio_dev->dev.parent); > + err = pm_runtime_resume_and_get(indio_dev->dev.parent); > + if (err < 0) > return err; > - } > > if (err > 0) { > /* Thanks, Mauro
On Wed, 12 May 2021 16:49:58 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Em Sun, 9 May 2021 12:33:49 +0100 > Jonathan Cameron <jic23@kernel.org> escreveu: > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > This case illustrates why the new pm_runtime_sync_and_get() is good > > in that it makes clear the correct way to handle errors. > > > > Calling pm_runtime_put() on failure of the _get() in pm_runtime_get_sync() > > will potentially result in powering down an already powered down device > > (as we never successfully powered it up. Unlikely to cause any problems > > in reality. > > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > This one seems a little odd on my eyes, although I don't know much about > IIO, as it calls RPM get only at: > > zpa2326_init_runtime() > > on a balanced way, as the routine starts with RPM get() ends with RPM put(). > > Then it does a put at zpa2326_suspend() and a get at zpa2326_resume(). > > Can RPM usage_count be zero at suspend (or are there some other part > of IIO core that increments it?). Because, if after resume, usage_count > would be equal to 1, as I guess RPM core prevent negative values. I've stared at this for a few minutes and gotten nowhere in figuring out the intent. As such I'm going to drop the patch for now and revisit at a later date. Jonathan > > Regards, > Mauro > > > > > --- > > drivers/iio/pressure/zpa2326.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c > > index 89295c90f801..97ac3ba399f7 100644 > > --- a/drivers/iio/pressure/zpa2326.c > > +++ b/drivers/iio/pressure/zpa2326.c > > @@ -664,11 +664,9 @@ static int zpa2326_resume(const struct iio_dev *indio_dev) > > { > > int err; > > > > - err = pm_runtime_get_sync(indio_dev->dev.parent); > > - if (err < 0) { > > - pm_runtime_put(indio_dev->dev.parent); > > + err = pm_runtime_resume_and_get(indio_dev->dev.parent); > > + if (err < 0) > > return err; > > - } > > > > if (err > 0) { > > /* > > > > Thanks, > Mauro
diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c index 89295c90f801..97ac3ba399f7 100644 --- a/drivers/iio/pressure/zpa2326.c +++ b/drivers/iio/pressure/zpa2326.c @@ -664,11 +664,9 @@ static int zpa2326_resume(const struct iio_dev *indio_dev) { int err; - err = pm_runtime_get_sync(indio_dev->dev.parent); - if (err < 0) { - pm_runtime_put(indio_dev->dev.parent); + err = pm_runtime_resume_and_get(indio_dev->dev.parent); + if (err < 0) return err; - } if (err > 0) { /*