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

["Maxwell Doose" <maxwell@maxwelld.cc>]

On Wed Aug 5, 2026 at 9:21 PM CDT
Linmao Li <lilinmao@kylinos.cn> wrote:

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

I see. This ought to give me an excuse to look into PM stuff :)

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

Since the PM core already logs it let's go no dev_warn().

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

Perhaps something that would be good to test on actual hardware to see
if it matters. Maybe Wadim can check? However in all of the (very very
basic) drivers I've written I've tried my best to account for hardware
delays so I would do some more research into this.

thanks,
max