slf3s_resume() enables vdd and then restarts the measurement. When the restart fails it returns an error with the supply still on, and slf3s_suspend() bails out before regulator_disable() when the stop command fails. Both paths leave the driver's idea of the power state and the regulator use count out of sync.
The PM core only runs the resume callback when the preceding suspend callback returned 0 (device_suspend() sets power.is_suspended on success only, and device_resume() bails out when it is clear), so the enable count cannot climb above one. The damage is different: - After a failed restart the sensor stays powered but idle. Every following suspend then fails too, because the idle part NACKs the stop command and slf3s_suspend() returns early - the system can no longer suspend at all until the driver is rebound. - If regulator_enable() itself fails during resume, the next suspend is free to run. Should the stop command succeed there, the driver calls regulator_disable() on a supply it never enabled, which trips "unbalanced disables" in the regulator core and aborts the system suspend with -EIO. The devm cleanup has the same problem on unbind. Track the supply state in the driver and drive regulator_enable() and regulator_disable() from that state only. A stop command that fails no longer keeps the supply on: it is cut right afterwards anyway. Reported-by: Li Youhong <[email protected]> Closes: https://lore.kernel.org/linux-iio/[email protected]/ Fixes: d240b0b8a1ce ("iio: flow: add Sensirion SLF3S liquid flow sensor driver") Signed-off-by: Wadim Mueller <[email protected]> --- drivers/iio/flow/slf3s.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/drivers/iio/flow/slf3s.c b/drivers/iio/flow/slf3s.c index dfa7c1409045..37f40685e9d9 100644 --- a/drivers/iio/flow/slf3s.c +++ b/drivers/iio/flow/slf3s.c @@ -110,6 +110,7 @@ static const struct slf3s_variant slf3s_variants[] = { * @vdd: supply regulator, disabled while suspended * @variant: pointer into @slf3s_variants for the detected device * @medium: currently active calibration medium + * @vdd_on: tracks whether @vdd is currently enabled by this driver * @lock: serialises the multi-step command/response exchanges * @crc_table: pre-computed CRC-8 lookup table for SLF3S_CRC8_POLY */ @@ -118,6 +119,7 @@ struct slf3s_data { struct regulator *vdd; const struct slf3s_variant *variant; enum slf3s_medium medium; + bool vdd_on; struct mutex lock; u8 crc_table[CRC8_TABLE_SIZE]; }; @@ -382,6 +384,12 @@ static void slf3s_disable_vdd(void *data) { struct slf3s_data *sf = data; + guard(mutex)(&sf->lock); + + if (!sf->vdd_on) + return; + + sf->vdd_on = false; regulator_disable(sf->vdd); } @@ -416,6 +424,8 @@ static int slf3s_probe(struct i2c_client *client) if (ret) return dev_err_probe(dev, ret, "failed to enable vdd supply\n"); + sf->vdd_on = true; + ret = devm_add_action_or_reset(dev, slf3s_disable_vdd, sf); if (ret) return ret; @@ -453,10 +463,8 @@ static int slf3s_probe(struct i2c_client *client) } /* - * The sensor has no low-power state of its own, so stop the measurement - * and cut the supply while suspended. Resume powers it back up, waits - * out the power-up time and restarts with the medium that was active - * before. + * The sensor has no low-power state, so stop measuring and cut the supply. + * Resume powers it up again and restarts the previous medium. */ static int slf3s_suspend(struct device *dev) { @@ -466,9 +474,16 @@ static int slf3s_suspend(struct device *dev) guard(mutex)(&sf->lock); + /* A failed resume may have left the supply off, nothing to do then. */ + if (!sf->vdd_on) + return 0; + + /* The supply goes away below anyway, so a failed stop is not fatal. */ ret = slf3s_send_cmd(sf->client, slf3s_cmd_stop_meas); if (ret) - return ret; + dev_warn(dev, "failed to stop measurement: %d\n", ret); + + sf->vdd_on = false; return regulator_disable(sf->vdd); } @@ -485,9 +500,17 @@ static int slf3s_resume(struct device *dev) if (ret) return ret; + sf->vdd_on = true; + fsleep(SLF3S_POWER_UP_DELAY_US); - return slf3s_start_meas(sf, sf->medium); + ret = slf3s_start_meas(sf, sf->medium); + if (ret) { + sf->vdd_on = false; + regulator_disable(sf->vdd); + } + + return ret; } static DEFINE_SIMPLE_DEV_PM_OPS(slf3s_pm_ops, slf3s_suspend, slf3s_resume); -- 2.55.0
