All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nuno Sá" <noname.nuno@gmail.com>
To: Linmao Li <lilinmao@kylinos.cn>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
	"Wadim Mueller" <wafgo01@gmail.com>,
	"Maxwell Doose" <maxwell@maxwelld.cc>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails
Date: Thu, 6 Aug 2026 10:39:56 +0100	[thread overview]
Message-ID: <anRUF3ucjK7n4iN-@nsa> (raw)
In-Reply-To: <20260805110255.504576-1-lilinmao@kylinos.cn>

On Wed, Aug 05, 2026 at 07:02:55PM +0800, Linmao Li wrote:
> slf3s_suspend() stops continuous measurement before disabling VDD. If
> regulator_disable() fails while the supply remains enabled, the system
> sleep transition is aborted. Since the PM core does not call the
> corresponding resume callback for a device whose suspend callback failed,
> the sensor remains idle after the system returns to the running state and
> subsequent reads fail.
> 
> Attempt to restart continuous measurement on this error path. Preserve the
> regulator error and warn if restarting the measurement also fails.
> 
> Fixes: d240b0b8a1ce ("iio: flow: add Sensirion SLF3S liquid flow sensor driver")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
>  drivers/iio/flow/slf3s.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/flow/slf3s.c b/drivers/iio/flow/slf3s.c
> index dfa7c14090454..75ee82fbd3295 100644
> --- a/drivers/iio/flow/slf3s.c
> +++ b/drivers/iio/flow/slf3s.c
> @@ -462,6 +462,7 @@ static int slf3s_suspend(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct slf3s_data *sf = iio_priv(indio_dev);
> +	int restart_ret;
>  	int ret;
>  
>  	guard(mutex)(&sf->lock);

Side note and not related to this patch but, AFAIK, there's no point in the
locking the mutex on the PM callbacks.

> @@ -470,7 +471,16 @@ static int slf3s_suspend(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> -	return regulator_disable(sf->vdd);
> +	ret = regulator_disable(sf->vdd);
> +	if (!ret)
> +		return 0;
> +
> +	restart_ret = slf3s_start_meas(sf, sf->medium);
> +	if (restart_ret)
> +		dev_warn(dev, "failed to restart measurement after suspend failure: %d\n",
> +			 restart_ret);
> +
> +	return ret;

I'm also not sure about the above. If the regulator fails to disable I
would say things are already in a bad state anyways. Is there any strong
reason to do `slf3s_send_cmd(sf->client, slf3s_cmd_stop_meas)` before
disabling vdd? I would assume that without vdd things will terminate
anyways. Asking because if we just disable it then the above stops
being a question. Though I do understand it's better to gracefully
terminate things. Just not sure if there's any added value for that in
this path.

Just my 2 cents. No strong feelings so if the driver author is fine with
this, also looks like a sensible change.

- Nuno Sá

>  }
>  
>  static int slf3s_resume(struct device *dev)
> 
> base-commit: 0efaefce4e95a3331550329c0078b2fb38b3ff1f
> -- 
> 2.25.1
> 

  parent reply	other threads:[~2026-08-06  9:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 11:02 [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails Linmao Li
2026-08-05 12:59 ` David Lechner
2026-08-06  2:18   ` Linmao Li
2026-08-05 16:45 ` Maxwell Doose
2026-08-06  2:21   ` Linmao Li
2026-08-11  0:24     ` Maxwell Doose
2026-08-11  9:48       ` Wadim Mueller
2026-08-06  9:39 ` Nuno Sá [this message]
2026-08-06 12:55   ` Wadim Mueller
2026-08-06 13:05     ` Nuno Sá
2026-08-15 21:05 ` Jonathan Cameron
2026-08-17  1:34   ` Linmao Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=anRUF3ucjK7n4iN-@nsa \
    --to=noname.nuno@gmail.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=lilinmao@kylinos.cn \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxwell@maxwelld.cc \
    --cc=nuno.sa@analog.com \
    --cc=wafgo01@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.