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

[sashiko-bot@kernel.org]

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