[PATCH v2 1/4] regulator: spacemit-p1: Fix n_voltages for BUCK and LDO regulators

[Guodong Xu]

Hi, Alex

On Wed, Jan 28, 2026 at 9:28 PM Alex Elder <elder at riscstar.com> wrote:
>
> On 1/23/26 6:20 PM, Guodong Xu wrote:
> > Higher voltage settings were unusable due to incorrect n_voltages values
> > causing registration failures. For example, setting aldo4 to 3.3V failed
> > with -EINVAL because the required selector (123) exceeded the allowed
> > range (n_voltages=117).
> >
> > Fix by aligning n_voltages with the hardware register widths per the P1
> > datasheet [1]:
> > - BUCK: 255 (was 254), allows selectors 0-254, selector 255 is reserved
> > - LDO: 128 (was 117), allows selectors 0-127, selectors 0-10 are for
> >    suspend mode, valid operational range is 11-127
> >
> > This enables the full voltage range supported by the hardware.
> >
> > Fixes: 8b84d712ad84 ("regulator: spacemit: support SpacemiT P1 regulators")
> > Link: https://developer.spacemit.com/documentation [1]
> > Signed-off-by: Guodong Xu <guodong at riscstar.com>
> > ---
> > v2: No change.
> > ---
> >   drivers/regulator/spacemit-p1.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/regulator/spacemit-p1.c b/drivers/regulator/spacemit-p1.c
> > index 2bf9137e12b1..2b585ba01a93 100644
> > --- a/drivers/regulator/spacemit-p1.c
> > +++ b/drivers/regulator/spacemit-p1.c
> > @@ -87,13 +87,13 @@ static const struct linear_range p1_ldo_ranges[] = {
> >       }
> >
> >   #define P1_BUCK_DESC(_n) \
> > -     P1_REG_DESC(BUCK, buck, _n, "vin", 0x47, BUCK_MASK, 254, p1_buck_ranges)
> > +     P1_REG_DESC(BUCK, buck, _n, "vin", 0x47, BUCK_MASK, 255, p1_buck_ranges)
>
> This is correct.  There are 255 possible ranges, 0..254, and
> 255 is an illegal value.
>
> I think this bug is an artifact of a change I made while
> chasing an issue during development, and I neglected to
> change it back.
>
> Technically this is a bug fix but it doesn't matter because
> this voltage value (255 represents 3.450 volts) was not
> required.
>
> >   #define P1_ALDO_DESC(_n) \
> > -     P1_REG_DESC(ALDO, aldo, _n, "vin", 0x5b, LDO_MASK, 117, p1_ldo_ranges)
> > +     P1_REG_DESC(ALDO, aldo, _n, "vin", 0x5b, LDO_MASK, 128, p1_ldo_ranges)
>
> I would say this is not correct.
>
> The valid range of values in this register is 0xd-0x1f (11-127),
> which is 117 values; 0xd represents 0.500V and 0x1f represents
> 3.400V.
>
> Technically, all other values represent 0.5v (and could therefore
> be considered valid), but I believe those should never be used
> and intentionally considered them invalid.  If 0.5V is desired,
> 0xd should be used.
>
> Do you disagree with this?

I understand your concern about selectors 0-10. However, maybe you missed
this part:

Code snippet from the c file, Line 53:
(The p1_buck_ranges and p1_ldo_ranges are defined correctly.)

/* Selector value 255 can be used to disable the buck converter on sleep */
static const struct linear_range p1_buck_ranges[] = {
REGULATOR_LINEAR_RANGE(500000, 0, 170, 5000),
REGULATOR_LINEAR_RANGE(1375000, 171, 254, 25000),
};

/* Selector value 0 can be used for suspend */
static const struct linear_range p1_ldo_ranges[] = {
REGULATOR_LINEAR_RANGE(500000, 11, 127, 25000),
};

.linear_range, the number of valid voltage steps (selectors 11-127)
.n_voltages field, which defines the selector namespace (0 to 170, then to 254)

With n_voltages = 117, the maximum accessible selector is 116. This makes
selectors 117-127 unreachable, even though they're defined in the linear_range.

n_voltages = 128 doesn't enable those for operational use, it just allows
the full valid range (11-127) to be accessible.

This is why in my test for the K3 pico board, setting ALDO to 3.3V
(selector 123) or 3.4V (selector 127) fails with the current code.
I mean that leads me to this bug fix.

Best regards,
Guodong Xu


> >   #define P1_DLDO_DESC(_n) \
> > -     P1_REG_DESC(DLDO, dldo, _n, "buck5", 0x67, LDO_MASK, 117, p1_ldo_ranges)
> > +     P1_REG_DESC(DLDO, dldo, _n, "buck5", 0x67, LDO_MASK, 128, p1_ldo_ranges)
> >
> >   static const struct regulator_desc p1_regulator_desc[] = {
> >       P1_BUCK_DESC(1),
> >
>
> I have exactly the same comment about this change to the
> number of supported values.
>
>                                         -Alex