[PATCH 0/2] regulator: fp9931: fix voltage mapping and runtime PM issues

[PATCH 0/2] regulator: fp9931: fix voltage mapping and runtime PM issues

[robby.cai]

From: Robby Cai <robby.cai@nxp.com>

Hi,

This series fixes two issues in the FP9931/JD9930 regulator driver.

Patch 1 fixes an off-by-one error in the VPOS/VNEG voltage selector
table and aligns it with the datasheet-defined voltage mapping [1].

Patch 2 fixes a Runtime PM reference counting bug in the V3P3 regulator
callbacks that can lead to "Runtime PM usage count underflow!" warnings.

Thanks,
Robby

[1] https://www.fitipower.com/dl/file/flXa6hIchVeu0W3K (Datasheet, p.13)


Robby Cai (2):
  regulator: fp9931: Fix VPOS/VNEG voltage selector table
  regulator: fp9931: Fix Runtime PM usage count underflow in v3p3 ops

 drivers/regulator/fp9931.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
2.50.1

[PATCH 1/2] regulator: fp9931: Fix VPOS/VNEG voltage selector table

[robby.cai]

From: Robby Cai <robby.cai@nxp.com>

The VPOSNEG_table[] mapping does not match the FP9931/JD9930 datasheet.

The datasheet defines the VPOS/VNEG voltage mapping as:

  00h-04h ->  7.04V (-7.04V)
  05h     ->  7.26V (-7.26V)
  06h     ->  7.49V (-7.49V)
  ...
  28h-3Fh -> 15.06V (-15.06V)

However, the current table maps selector 0x05 to 7.04V instead of
7.26V, causing all subsequent entries to be shifted by one position.

Update VPOSNEG_table[] to match the documented register encoding.

Fixes: 12d821bd13d4 ("regulator: Add FP9931/JD9930 driver")
Signed-off-by: Robby Cai <robby.cai@nxp.com>
---
 drivers/regulator/fp9931.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/regulator/fp9931.c b/drivers/regulator/fp9931.c
index 002b41f53eff..5eee21034f6b 100644
--- a/drivers/regulator/fp9931.c
+++ b/drivers/regulator/fp9931.c
@@ -42,7 +42,6 @@ static const unsigned int VPOSNEG_table[] = {
 	7040000,
 	7040000,
 	7040000,
-	7040000,
 	7260000,
 	7490000,
 	7710000,
-- 
2.50.1

Re: [PATCH 1/2] regulator: fp9931: Fix VPOS/VNEG voltage selector table

[sashiko-bot]

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

New issues:
- [High] The VPOSNEG_table lacks padding for selectors 0x29-0x3F, causing valid hardware states to be rejected, and shrinking the array introduces a regression for selector 0x29.

Pre-existing issues:
- [High] The vcom regulator specifies an off-by-one n_voltages limit, incorrectly rejecting the highest valid selector (0xFF).
--

--- Patch [1]: [PATCH 1/2] regulator: fp9931: Fix VPOS/VNEG voltage selector table ---
commit 69720a9d40de43e41550a108ea7fe0d153732240
Author: Robby Cai <robby.cai@nxp.com>

regulator: fp9931: Fix VPOS/VNEG voltage selector table

This commit updates the VPOS/VNEG voltage selector table to match the
datasheet mapping by removing a duplicate 7.04V entry, ensuring the
selector 0x05 correctly aligns with 7.26V.

> diff --git a/drivers/regulator/fp9931.c b/drivers/regulator/fp9931.c
> index 002b41f53eff8..5eee21034f6b7 100644
> --- a/drivers/regulator/fp9931.c
> +++ b/drivers/regulator/fp9931.c
> @@ -42,7 +42,6 @@ static const unsigned int VPOSNEG_table[] = {
>  	7040000,
>  	7040000,
>  	7040000,
> -	7040000,

[Severity: High]
Does shrinking VPOSNEG_table[] introduce a regression for selector 0x29?

By removing this entry, the array size decreases from 42 to 41. However,
the datasheet and commit message indicate that selectors 0x28 through 0x3F
are all valid and map to 15.06V.

Because the vposneg regulator configures n_voltages based on the array size:

drivers/regulator/fp9931.c:regulators[] {
	...
	.name = "vposneg",
	...
	.n_voltages = ARRAY_SIZE(VPOSNEG_table),
	.vsel_reg = FP9931_REG_VPOSNEG_SETTING,
	.vsel_mask = 0x3F,
	...
}

the core will now reject selector 0x29 which was previously accepted.
Furthermore, since vsel_mask is 0x3F, if the hardware boots with a selector
between 0x29 and 0x3F, it will be rejected because it is >= n_voltages,
causing a failure during probe.

Should the array be explicitly padded up to index 63 to match the vsel_mask?

>  	7260000,
>  	7490000,
>  	7710000,

[ ... ]

Looking further down in the file at the vcom regulator configuration:

drivers/regulator/fp9931.c:regulators[] {
	...
	{
		.name = "vcom",
		...
		.n_voltages = 255,
		.min_uV = 0,
		.uV_step = 5000000 / 255,
		.vsel_reg = FP9931_REG_VCOM_SETTING,
		.vsel_mask = 0xFF,
	},
	...
}

[Severity: High]
This is a pre-existing issue, but does the vcom regulator have an off-by-one
error in its n_voltages limit?

Since vsel_mask is 0xFF, the hardware supports 256 states (0 to 255).
However, n_voltages is set to 255. If the core reads a hardware initialization
state of 0xFF, or a consumer requests the maximum voltage, it will be
incorrectly rejected because the maximum selector is equal to n_voltages
instead of strictly less than it.

Should n_voltages be 256?

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

RE: [PATCH 1/2] regulator: fp9931: Fix VPOS/VNEG voltage selector table

[Robby Cai (OSS)]

>-----Original Message-----
>From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
>Sent: Tuesday, July 21, 2026 6:08 PM
>To: Robby Cai (OSS) <robby.cai@oss.nxp.com>
>Cc: Frank.Li@kernel.org; imx@lists.linux.dev
>Subject: Re: [PATCH 1/2] regulator: fp9931: Fix VPOS/VNEG voltage selector
>table
>
>[You don't often get email from sashiko-bot@kernel.org. Learn why this is
>important at https://aka.ms/LearnAboutSenderIdentification ]
>
>Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
>to consider:
>
>New issues:
>- [High] The VPOSNEG_table lacks padding for selectors 0x29-0x3F, causing
>valid hardware states to be rejected, and shrinking the array introduces a
>regression for selector 0x29.
>
>Pre-existing issues:
>- [High] The vcom regulator specifies an off-by-one n_voltages limit,
>incorrectly rejecting the highest valid selector (0xFF).
>--
>
>--- Patch [1]: [PATCH 1/2] regulator: fp9931: Fix VPOS/VNEG voltage selector
>table --- commit 69720a9d40de43e41550a108ea7fe0d153732240
>Author: Robby Cai <robby.cai@nxp.com>
>
>regulator: fp9931: Fix VPOS/VNEG voltage selector table
>
>This commit updates the VPOS/VNEG voltage selector table to match the
>datasheet mapping by removing a duplicate 7.04V entry, ensuring the selector
>0x05 correctly aligns with 7.26V.
>
>> diff --git a/drivers/regulator/fp9931.c b/drivers/regulator/fp9931.c
>> index 002b41f53eff8..5eee21034f6b7 100644
>> --- a/drivers/regulator/fp9931.c
>> +++ b/drivers/regulator/fp9931.c
>> @@ -42,7 +42,6 @@ static const unsigned int VPOSNEG_table[] = {
>>       7040000,
>>       7040000,
>>       7040000,
>> -     7040000,
>
>[Severity: High]
>Does shrinking VPOSNEG_table[] introduce a regression for selector 0x29?
>
>By removing this entry, the array size decreases from 42 to 41. However, the
>datasheet and commit message indicate that selectors 0x28 through 0x3F are
>all valid and map to 15.06V.
>
>Because the vposneg regulator configures n_voltages based on the array size:
>
>drivers/regulator/fp9931.c:regulators[] {
>        ...
>        .name = "vposneg",
>        ...
>        .n_voltages = ARRAY_SIZE(VPOSNEG_table),
>        .vsel_reg = FP9931_REG_VPOSNEG_SETTING,
>        .vsel_mask = 0x3F,
>        ...
>}
>
>the core will now reject selector 0x29 which was previously accepted.
>Furthermore, since vsel_mask is 0x3F, if the hardware boots with a selector
>between 0x29 and 0x3F, it will be rejected because it is >= n_voltages, causing
>a failure during probe.
>
>Should the array be explicitly padded up to index 63 to match the vsel_mask?

Good point. The datasheet defines selectors 0x29-0x3f as valid values
mapping to 15.06V (-15.06V), so the table should cover the full selector
range allowed by the 6-bit field. I'll pad the table accordingly in v2.

>
>>       7260000,
>>       7490000,
>>       7710000,
>
>[ ... ]
>
>Looking further down in the file at the vcom regulator configuration:
>
>drivers/regulator/fp9931.c:regulators[] {
>        ...
>        {
>                .name = "vcom",
>                ...
>                .n_voltages = 255,
>                .min_uV = 0,
>                .uV_step = 5000000 / 255,
>                .vsel_reg = FP9931_REG_VCOM_SETTING,
>                .vsel_mask = 0xFF,
>        },
>        ...
>}
>
>[Severity: High]
>This is a pre-existing issue, but does the vcom regulator have an off-by-one
>error in its n_voltages limit?
>
>Since vsel_mask is 0xFF, the hardware supports 256 states (0 to 255).
>However, n_voltages is set to 255. If the core reads a hardware initialization
>state of 0xFF, or a consumer requests the maximum voltage, it will be
>incorrectly rejected because the maximum selector is equal to n_voltages
>instead of strictly less than it.
>
>Should n_voltages be 256?
>
>--
>Sashiko AI review · https://sashiko.dev/#/patchset/20260721095959.1855436-
>1-robby.cai@oss.nxp.com?part=1

Re: [PATCH 1/2] regulator: fp9931: Fix VPOS/VNEG voltage selector table

[Andreas Kemnade]

On Tue, 21 Jul 2026 17:59:58 +0800
robby.cai@oss.nxp.com wrote:

> From: Robby Cai <robby.cai@nxp.com>
> 
> The VPOSNEG_table[] mapping does not match the FP9931/JD9930
> datasheet.
> 
> The datasheet defines the VPOS/VNEG voltage mapping as:
> 
>   00h-04h ->  7.04V (-7.04V)
>   05h     ->  7.26V (-7.26V)
>   06h     ->  7.49V (-7.49V)
>   ...
>   28h-3Fh -> 15.06V (-15.06V)
> 
> However, the current table maps selector 0x05 to 7.04V instead of
> 7.26V, causing all subsequent entries to be shifted by one position.
> 
> Update VPOSNEG_table[] to match the documented register encoding.
> 
> Fixes: 12d821bd13d4 ("regulator: Add FP9931/JD9930 driver")
> Signed-off-by: Robby Cai <robby.cai@nxp.com>

Reviewed-by: Andreas Kemnade <andreas@kemnade.info>

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

[robby.cai]

From: Robby Cai <robby.cai@nxp.com>

The fp9931_v3p3_enable() and fp9931_v3p3_disable() callbacks currently
acquire a Runtime PM reference when V3P3 is enabled and release it when
V3P3 is disabled.

This assumes that Runtime PM references remain associated with the V3P3
regulator state. However, FP9931 regulators share common device-level
Runtime PM state and references may be released through other regulator
paths, such as the VCOM control path. In addition, the regulator
framework may invoke disable callbacks during cleanup even when no
corresponding enable callback was executed.

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!

Fix this by scoping the Runtime PM reference lifetime to the register
access itself, pairing pm_runtime_resume_and_get() and
pm_runtime_put_autosuspend() within each callback.

Fixes: 12d821bd13d4 ("regulator: Add FP9931/JD9930 driver")
Signed-off-by: Robby Cai <robby.cai@nxp.com>
---
 drivers/regulator/fp9931.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/fp9931.c b/drivers/regulator/fp9931.c
index 5eee21034f6b..3ffba6fb75ff 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);
 
 	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);
+	if (ret < 0)
+		return ret;
+
 	ret = regulator_disable_regmap(rdev);
 	pm_runtime_put_autosuspend(data->dev);
 
-- 
2.50.1

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

[sashiko-bot]

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

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

[Robby Cai (OSS)]

>-----Original Message-----
>From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
>Sent: Tuesday, July 21, 2026 6:07 PM
>To: Robby Cai (OSS) <robby.cai@oss.nxp.com>
>Cc: imx@lists.linux.dev; Frank.Li@kernel.org
>Subject: Re: [PATCH 2/2] regulator: fp9931: Fix Runtime PM usage count
>underflow in v3p3 ops
>
>[You don't often get email from sashiko-bot@kernel.org. Learn why this is
>important at https://aka.ms/LearnAboutSenderIdentification ]
>
>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.

This should not be an issue for FP9931.

The V3P3 regulator is not independently controlling the device power
state. FP9931 regulators share a common enable mechanism through the EN
control path, implemented by fp9931_set_enable() /
fp9931_clear_enable().

A runtime PM reference held by fp9931_v3p3_enable() does not represent
the lifetime of the V3P3 regulator itself. In fact, keeping the
reference until fp9931_v3p3_disable() causes reference accounting
problems because Runtime PM state is shared across multiple regulators.

The purpose of the Runtime PM reference in fp9931_v3p3_enable() and
fp9931_v3p3_disable() is only to guarantee that the device is active
while the register access is performed. Once the register update
completes, the reference can be released safely.

Therefore the change scopes Runtime PM references to the regmap access
itself rather than the logical regulator enable state.

Regards,
Robby
>
>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