[PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[Linmao Li]

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);
@@ -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;
 }
 
 static int slf3s_resume(struct device *dev)

base-commit: 0efaefce4e95a3331550329c0078b2fb38b3ff1f
-- 
2.25.1

Re: [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[David Lechner]

On 8/5/26 6:02 AM, 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.
> 
Was this observed to happen on real hardware or was this just deduced
through code analysis?

Re: [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[Maxwell Doose]

On Wed, Aug 5, 2026 at 6:07 AM Linmao Li <lilinmao@kylinos.cn> 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);
> @@ -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;
>  }
>

In addition to David's question, why reenable vs keep it sleeping (and
why don't we dev_warn() after regulator disable failure)?

-- 
best regards,
max

Re: [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[Linmao Li]

在 2026/8/5 20:59, David Lechner 写道:
> On 8/5/26 6:02 AM, 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.
>>
> Was this observed to happen on real hardware or was this just deduced
> through code analysis?
Code inspection only.  I have no SLF3S hardware and no way to inject a

regulator_disable() failure, so this has not been reproduced.


Thanks,
Linmao

Re: [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[Linmao Li]

在 2026/8/6 0:45, Maxwell Doose 写道:
> On Wed, Aug 5, 2026 at 6:07 AM Linmao Li <lilinmao@kylinos.cn> 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);
>> @@ -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;
>>   }
>>
> In addition to David's question, why reenable vs keep it sleeping (and
> why don't we dev_warn() after regulator disable failure)?
Because a failed suspend callback means the device never gets its resume
callback, so nothing would restart the measurement.  And as the comment
in probe() says, a stop command sent to an already idle sensor errors
out - slf3s_suspend() returns that error - so every later system suspend
would fail as well.

No dev_warn() on the regulator error because it is the return value and
the PM core already logs it.  Happy to add one if you prefer.

One thing I noticed while doing this, not specific to my patch: the
datasheets for all three supported variants say that after the stop
command the sensor "needs up to 0.5 ms to power down the heater, enter
idle mode and be receptive for a new command".  probe() and
slf3s_set_medium() both send the next command right away, and my
rollback can too.  Is that something you have had to care about in

practice?


Thanks,
Linmao

>

Re: [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[Nuno Sá]

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
> 

Re: [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[Wadim Mueller]

On Thu Aug 6, 2026 at 11:39 AM CEST, Nuno Sá wrote:
> On Wed, Aug 05, 2026 at 07:02:55PM +0800, Linmao Li wrote:
>
> 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.

on quite a few boards vdd is tied directly to the always-on rail, so there's
no real supply node in the DT and the regulator core falls back to a dummy regulator there
regulator_disable() becomes a pure no-op that always returns success without  actually removing power.
so we can't rely on cutting vdd to stop the sensor

Wadim

>
> 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
> > 

Re: [PATCH] iio: flow: slf3s: restart measurement if VDD disable fails

[Nuno Sá]

On Thu, Aug 06, 2026 at 02:55:25PM +0200, Wadim Mueller wrote:
> On Thu Aug 6, 2026 at 11:39 AM CEST, Nuno Sá wrote:
> > On Wed, Aug 05, 2026 at 07:02:55PM +0800, Linmao Li wrote:
> >
> > 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.
> 
> on quite a few boards vdd is tied directly to the always-on rail, so there's
> no real supply node in the DT and the regulator core falls back to a dummy regulator there
> regulator_disable() becomes a pure no-op that always returns success without  actually removing power.
> so we can't rely on cutting vdd to stop the sensor

Oh yeah! That make total sense so dummy me :)

- Nuno Sá

> 
> Wadim
> 
> >
> > 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
> > > 
>