[PATCH v2 3/4] regulator: spacemit-p1: Update supply names

[Guodong Xu]

Hi, Alex

On Wed, Jan 28, 2026 at 9:29 PM Alex Elder <elder at riscstar.com> wrote:
>
> On 1/23/26 6:20 PM, Guodong Xu wrote:
> > Update supply names to match the P1 PMIC's actual hardware pinout where
> > each buck has an individual VIN pin (vin1-vin6) and LDO groups have
> > dedicated input pins (aldoin, dldoin1, dldoin2).
> >
> > The supply is a board design decision and should not be hardcoded to any
> > existing power source. This allows boards to specify their actual power
> > tree topology in devicetree.
> >
> > Signed-off-by: Guodong Xu <guodong at riscstar.com>
>
> These are good changes but I have a suggestion on the way
> you define the DLDO descriptors.  I might be mistaken but
> I think you should make this change.
>
> Aside from that:
>
> Reviewed-by: Alex Elder <elder at riscstar.com>
>
> > ---
> > v2: No change.
> > ---
> >   drivers/regulator/spacemit-p1.c | 25 ++++++++++++++-----------
> >   1 file changed, 14 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/regulator/spacemit-p1.c b/drivers/regulator/spacemit-p1.c
> > index 2b585ba01a93..57e6e00a73fa 100644
> > --- a/drivers/regulator/spacemit-p1.c
> > +++ b/drivers/regulator/spacemit-p1.c
> > @@ -87,13 +87,16 @@ static const struct linear_range p1_ldo_ranges[] = {
> >       }
> >
> >   #define P1_BUCK_DESC(_n) \
> > -     P1_REG_DESC(BUCK, buck, _n, "vin", 0x47, BUCK_MASK, 255, p1_buck_ranges)
> > +     P1_REG_DESC(BUCK, buck, _n, "vin" #_n, 0x47, BUCK_MASK, 255, p1_buck_ranges)
>
> That was a simple change...
>
> >   #define P1_ALDO_DESC(_n) \
> > -     P1_REG_DESC(ALDO, aldo, _n, "vin", 0x5b, LDO_MASK, 128, p1_ldo_ranges)
> > +     P1_REG_DESC(ALDO, aldo, _n, "aldoin", 0x5b, LDO_MASK, 128, p1_ldo_ranges)
>
> As stated before, I believe the 128 should be 117 here.  (If

I will explain this in another email.

> you change the earlier patch, make sure the change to 128
> doesn't persist here.)  Same comment for the DLDO regulators.
>
> > -#define P1_DLDO_DESC(_n) \
> > -     P1_REG_DESC(DLDO, dldo, _n, "buck5", 0x67, LDO_MASK, 128, p1_ldo_ranges)
> > +#define P1_DLDO1_DESC(_n) \
> > +     P1_REG_DESC(DLDO, dldo, _n, "dldoin1", 0x67, LDO_MASK, 128, p1_ldo_ranges)
>
> Why can't you use _n here like you did for P1_BUCK_DESC() above?

The naming follows the P1 pinout definitions in the datasheet [1].

Unlike the BUCK regulators, which have individual input pins (e.g.,
VIN3 for BUCK3), the DLDOs share power inputs. For example, DLDOIN1 (pin 17)
powers DLDO1 through DLDO4. DLDOIN2 provides power to DLDO5, 6 and 7.

Since there are no physical pins named dldoin3, etc., I can't use the _n index
for the supply name argument like I did for the BUCKs.

Datasheet pin examples:
8 VIN3 PWR Buck3 power input (1:1 mapping)
17 DLDOIN1 PWR DLDO1~4 power input (1:Many mapping)

Link: https://developer.spacemit.com/documentation?token=T1Btw2BdiiSlSXkAdibcoMetnag
[1]

Best regards,
Guodong Xu

>
> > +
> > +#define P1_DLDO2_DESC(_n) \
> > +     P1_REG_DESC(DLDO, dldo, _n, "dldoin2", 0x67, LDO_MASK, 128, p1_ldo_ranges)
>
> So this is generalizing the input, which is good.  The use
> of "buck5" here was a Banana Pi BPI-F3 design and but it
> doesn't have to be that way.
>
> >   static const struct regulator_desc p1_regulator_desc[] = {
> >       P1_BUCK_DESC(1),
> > @@ -108,13 +111,13 @@ static const struct regulator_desc p1_regulator_desc[] = {
> >       P1_ALDO_DESC(3),
> >       P1_ALDO_DESC(4),
> >
> > -     P1_DLDO_DESC(1),
> > -     P1_DLDO_DESC(2),
> > -     P1_DLDO_DESC(3),
> > -     P1_DLDO_DESC(4),
> > -     P1_DLDO_DESC(5),
> > -     P1_DLDO_DESC(6),
> > -     P1_DLDO_DESC(7),
> > +     P1_DLDO1_DESC(1),
> > +     P1_DLDO1_DESC(2),
> > +     P1_DLDO1_DESC(3),
> > +     P1_DLDO1_DESC(4),
> > +     P1_DLDO2_DESC(5),
> > +     P1_DLDO2_DESC(6),
> > +     P1_DLDO2_DESC(7),
> >   };
> >
> >   static int p1_regulator_probe(struct platform_device *pdev)
> >
>