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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4395bea9-44d7-4f0f-81d6-c1e05976a308@riscstar.com>
Date: Wed, 28 Jan 2026 07:28:58 -0600
From: Alex Elder <elder@...cstar.com>
To: Guodong Xu <guodong@...cstar.com>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>, Yixun Lan <dlan@...too.org>,
 Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Troy Mitchell <troy.mitchell@...ux.spacemit.com>,
 Paul Walmsley <pjw@...nel.org>, Palmer Dabbelt <palmer@...belt.com>,
 Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>
Cc: linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
 spacemit@...ts.linux.dev, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 3/4] regulator: spacemit-p1: Update supply names

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@...cstar.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@...cstar.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
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?

> +
> +#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)
>