Re: [PATCH 1/3] iio: accel: kxsd9: validate input before acquiring runtime PM in write_raw runtime PM in write_raw

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Tue, 14 Jul 2026 17:55:21 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> Currently, kxsd9_write_raw() acquires a runtime PM reference at the very
> top of the function, before checking if the requested mask or value are
> actually supported. This causes unnecessary hardware wakeups for invalid
> inputs.
> 
> Refactor write_raw to validate inputs first. Additionally, modernize the
> PM runtime call by replacing pm_runtime_get_sync() with
> pm_runtime_resume_and_get() and adding the missing error check to
> prevent proceeding if power-on fails.
> 
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>

Hi Biren,

For a series there should always be a cover letter.

This patch looks fine but you may want to take the approaches
I'll suggest in review of patch 2 and apply them here for consistency.

Thanks,

Jonathan


> ---
>  drivers/iio/accel/kxsd9.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 7ac885d94d7f4..b3acb20441d86 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -139,18 +139,21 @@ static int kxsd9_write_raw(struct iio_dev *indio_dev,
>  			   int val2,
>  			   long mask)
>  {
> -	int ret = -EINVAL;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> +	int ret;
>  
> -	pm_runtime_get_sync(st->dev);
> +	if (mask != IIO_CHAN_INFO_SCALE)
> +		return -EINVAL;
>  
> -	if (mask == IIO_CHAN_INFO_SCALE) {
> -		/* Check no integer component */
> -		if (val)
> -			ret = -EINVAL;
> -		else
> -			ret = kxsd9_write_scale(indio_dev, val2);
> -	}
> +	/* Check no integer component */
> +	if (val)
> +		return -EINVAL;
> +
> +	ret = pm_runtime_resume_and_get(st->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = kxsd9_write_scale(indio_dev, val2);
>  
>  	pm_runtime_put_autosuspend(st->dev);
>