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

[Linmao Li]

在 2026/8/16 5:05, Jonathan Cameron 写道:
> On Wed,  5 Aug 2026 19:02:55 +0800
> Linmao Li <lilinmao@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >   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);
> Similar to previous thread I replied to, I'm not convinced spending much
> effort to recover from this sort of condition is useful.
Fair enough - this was found by code inspection only, so I cannot say
how likely the regulator failure is in practice.

One consequence that was not in the commit message is that leaving the
sensor idle makes the failure sticky.  The comment in slf3s_probe()
anticipates the stop command returning an error when the sensor is
already idle, and both slf3s_suspend() and slf3s_set_medium() propagate
that error.  So subsequent suspend attempts are expected to fail at the
stop command, and selecting a medium cannot restart the measurement
either; recovery then requires the device to be reprobed or otherwise
reset.

The restart is only a best-effort rollback for cases where VDD remains
enabled; the driver cannot determine the supply state from the return
value alone.  It does not address whatever made regulator_disable()
fail, so a later suspend may fail there again if that condition persists.
Its purpose is to avoid additionally leaving the sensor idle.

If that still does not justify the extra branch, I will drop the patch.
Otherwise I will respin it with the error handling out of line as you
suggested.
> If we have a regulator that doesn't respond reliably to instruction chances
> are we very unlikely to recover, and adding the recovery logic complicates
> the driver somewhat.
>
> I we do decide this is worth doing then not this style.  Keep the error handling
> as the out of line path
> 	if (ret) {
> 		int restart_ret;
>
> 		restart_ret = ....
>
> Makes it easier to spot that it is an error path.
>
> > +	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