Re: [PATCH 2/2] regulator: fp9931: Fix Runtime PM usage count underflow in v3p3 ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Dropping the Runtime PM reference in the regulator enable path allows the device to auto-suspend while the regulator is logically active, causing silent power failure.

Pre-existing issues:
- [High] The VCOM regulator's disable callback unconditionally drops a runtime PM reference, causing a usage count underflow if disabled during cleanup.
- [High] Calling `regulator_enable()` from within a `runtime_resume` callback triggers a WW Mutex nesting deadlock and lockdep BUG.
--

--- Patch [2]: [PATCH 2/2] regulator: fp9931: Fix Runtime PM usage count underflow in v3p3 ops ---
commit e0c7f5e5591f4d063d494917c2e53d290ddca4e4
Author: Robby Cai <robby.cai@nxp.com>

regulator: fp9931: Fix Runtime PM usage count underflow in v3p3 ops

This patch attempts to fix a runtime PM usage count underflow by scoping
the PM reference lifetime strictly to the register accesses in the v3p3
enable and disable callbacks.

> As a result, fp9931_v3p3_disable() may call pm_runtime_put_autosuspend()
> when the device runtime PM usage count has already reached 0, triggering:
> 
>   fp9931 1-0018: Runtime PM usage count underflow!

[Severity: High]
This is a pre-existing issue, but doesn't this warning actually stem from
the VCOM regulator's fp9931_clear_enable() unconditionally dropping a
runtime PM reference?

If the regulator framework disables VCOM during late init cleanup (for
example, if it is marked boot-on but has no active consumers),
fp9931_clear_enable() calls pm_runtime_put_autosuspend() without a
corresponding get. This unbalanced put drops the shared usage count, which
seems to be the actual root cause of the underflow warning mentioned here.

> diff --git a/drivers/regulator/fp9931.c b/drivers/regulator/fp9931.c
> index 5eee21034f6b7..3ffba6fb75ff2 100644
> --- a/drivers/regulator/fp9931.c
> +++ b/drivers/regulator/fp9931.c
> @@ -243,8 +243,7 @@ static int fp9931_v3p3_enable(struct regulator_dev *rdev)
>  		return ret;
>  
>  	ret = regulator_enable_regmap(rdev);
> -	if (ret < 0)
> -		pm_runtime_put_autosuspend(data->dev);
> +	pm_runtime_put_autosuspend(data->dev);

[Severity: High]
Does dropping the runtime PM reference immediately after enabling the
regulator allow the device to auto-suspend while the regulator is logically
still active?

If the usage count reaches 0, the device will suspend. The auto-suspend
triggers fp9931_runtime_suspend(), which calls
regulator_disable(data->vin_reg). This would cut physical power to the IC,
causing silent power failure for any consumers using the v3p3 regulator.

>  
>  	return ret;
>  }
> @@ -254,6 +253,10 @@ static int fp9931_v3p3_disable(struct regulator_dev *rdev)
>  	struct fp9931_data *data = rdev_get_drvdata(rdev);
>  	int ret;
>  
> +	ret = pm_runtime_resume_and_get(data->dev);

[Severity: High]
This is a pre-existing issue, but does calling pm_runtime_resume_and_get()
from within a regulator callback trigger a WW mutex nesting deadlock?

The regulator framework holds the v3p3 regulator's WW mutex when calling
this disable callback. The synchronous resume triggers
fp9931_runtime_resume(), which executes:

fp9931_runtime_resume()
    regulator_enable(data->vin_reg);
    
This attempts to acquire the vin regulator's WW mutex using a new context
while already holding another WW mutex of the same class
(regulator_ww_class).

> +	if (ret < 0)
> +		return ret;
> +
>  	ret = regulator_disable_regmap(rdev);
>  	pm_runtime_put_autosuspend(data->dev);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721095959.1855436-1-robby.cai@oss.nxp.com?part=2