[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <24904c919d52e40bbe6089af54e013d2351296d3.camel@gmail.com>
Date: Mon, 09 Feb 2026 14:27:15 +0000
From: Nuno Sá <noname.nuno@...il.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>, Linus Walleij
<linusw@...nel.org>, Jonathan Cameron <jic23@...nel.org>, David Lechner
<dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>,
Andy Shevchenko
<andy@...nel.org>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] iio: gyro: mpu3050-core: fix pm_runtime error
handling
On Mon, 2026-02-09 at 14:02 +0200, Antoniu Miclaus wrote:
> pm_runtime_get_sync() does not check its return value, allowing the
I don't want to be too picky but the above is a bit of bad wording :).
pm_runtime_get_sync() indeed returns 'int' so the caller is the one not checking
it. So we have two things happening:
1) We don't want to ignore the return value from the PM API;
2) We don't want to, unconditionally, increment the device usage count and hence pivot to
use pm_runtime_resume_and_get()
- Nuno Sá
> driver to access hardware that may fail to resume. Use
> pm_runtime_resume_and_get() which propagates errors and avoids
> incrementing the usage count on failure.
>
> In preenable, add pm_runtime_put_autosuspend() on set_8khz_samplerate()
> failure since postdisable does not run when preenable fails.
>
> Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Reviewed-by: Linus Walleij <linusw@...nel.org>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
> Changes in v3:
> - Rename patch to focus on fix rather than API change
> - Reword commit message
> - Add Fixes: tag
>
> drivers/iio/gyro/mpu3050-core.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index ee2fcd20545d..1cc421eb4782 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -322,7 +322,9 @@ static int mpu3050_read_raw(struct iio_dev *indio_dev,
> }
> case IIO_CHAN_INFO_RAW:
> /* Resume device */
> - pm_runtime_get_sync(mpu3050->dev);
> + ret = pm_runtime_resume_and_get(mpu3050->dev);
> + if (ret)
> + return ret;
> mutex_lock(&mpu3050->lock);
>
> ret = mpu3050_set_8khz_samplerate(mpu3050);
> @@ -647,12 +649,19 @@ static irqreturn_t mpu3050_trigger_handler(int irq, void *p)
> static int mpu3050_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct mpu3050 *mpu3050 = iio_priv(indio_dev);
> + int ret;
>
> - pm_runtime_get_sync(mpu3050->dev);
> + ret = pm_runtime_resume_and_get(mpu3050->dev);
> + if (ret)
> + return ret;
>
> /* Unless we have OUR trigger active, run at full speed */
> - if (!mpu3050->hw_irq_trigger)
> - return mpu3050_set_8khz_samplerate(mpu3050);
> + if (!mpu3050->hw_irq_trigger) {
> + ret = mpu3050_set_8khz_samplerate(mpu3050);
> + if (ret)
> + pm_runtime_put_autosuspend(mpu3050->dev);
> + return ret;
> + }
>
> return 0;
> }
Powered by blists - more mailing lists