| 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
| ||
|
Message-ID: <20251022014706.2475539d@minigeek.lan>
Date: Wed, 22 Oct 2025 01:47:06 +0100
From: Andre Przywara <andre.przywara@....com>
To: Yixun Lan <dlan@...too.org>
Cc: Lee Jones <lee@...nel.org>, Chen-Yu Tsai <wens@...nel.org>, Liam
Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Samuel
Holland <samuel@...lland.org>, Jernej Skrabec <jernej.skrabec@...il.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] regulator: axp20x: add support for the AXP318W
On Wed, 22 Oct 2025 08:14:20 +0800
Yixun Lan <dlan@...too.org> wrote:
Hi,
thanks for having a look!
> Hi Andre,
>
> On 12:20 Tue 21 Oct , Andre Przywara wrote:
> > The X-Powers AXP318W is a typical PMIC from X-Powers, featuring nine
> > DC/DC converters and 28 LDOs, on the regulator side.
> >
> > Describe the chip's voltage settings and switch registers, how the
> > voltages are encoded, and connect this to the MFD device via its
> > regulator ID.
> > We use just "318" for the internal identifiers, for easier typing and
> > less churn. If something else other than the "AXP318W" shows up, that's
> > an easy change, externally visible strings carry the additional letter
> > already.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@....com>
> > ---
> > drivers/regulator/axp20x-regulator.c | 170 ++++++++++++++++++++++++++-
> > include/linux/mfd/axp20x.h | 43 +++++++
> > 2 files changed, 211 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> > index da891415efc0b..1576bf4178f8f 100644
> > --- a/drivers/regulator/axp20x-regulator.c
> > +++ b/drivers/regulator/axp20x-regulator.c
> > @@ -138,6 +138,15 @@
> > #define AXP313A_DCDC_V_OUT_MASK GENMASK(6, 0)
> > #define AXP313A_LDO_V_OUT_MASK GENMASK(4, 0)
> >
> > +#define AXP318_DCDC1_V_OUT_MASK GENMASK(4, 0)
> > +#define AXP318_DCDC2_V_OUT_MASK GENMASK(6, 0)
> > +#define AXP318_LDO_V_OUT_MASK GENMASK(4, 0)
> > +#define AXP318_ELDO_V_OUT_MASK GENMASK(5, 0)
> > +#define AXP318_DCDC2_NUM_VOLTAGES 88
> > +#define AXP318_DCDC6_NUM_VOLTAGES 128
> > +#define AXP318_DCDC7_NUM_VOLTAGES 103
> > +#define AXP318_DCDC8_NUM_VOLTAGES 119
> > +
> > #define AXP717_DCDC1_NUM_VOLTAGES 88
> > #define AXP717_DCDC2_NUM_VOLTAGES 107
> > #define AXP717_DCDC3_NUM_VOLTAGES 103
> > @@ -765,6 +774,155 @@ static const struct regulator_desc axp313a_regulators[] = {
> > AXP_DESC_FIXED(AXP313A, RTC_LDO, "rtc-ldo", "vin1", 1800),
> > };
> >
> > +static const struct linear_range axp318_dcdc2_ranges[] = {
> > + REGULATOR_LINEAR_RANGE(500000, 0, 70, 10000),
> > + REGULATOR_LINEAR_RANGE(1220000, 71, 87, 20000),
> > +};
> > +
> ..
> > +static const struct linear_range axp318_dcdc6_ranges[] = {
> > + REGULATOR_LINEAR_RANGE(500000, 0, 70, 10000),
> > + REGULATOR_LINEAR_RANGE(1220000, 71, 87, 20000),
> > + REGULATOR_LINEAR_RANGE(1800000, 88, 118, 20000),
> > + REGULATOR_LINEAR_RANGE(2440000, 119, 127, 40000),
> > +};
> > +
> > +static const struct linear_range axp318_dcdc7_ranges[] = {
> > + REGULATOR_LINEAR_RANGE(500000, 0, 70, 10000),
> > + REGULATOR_LINEAR_RANGE(1220000, 71, 102, 20000),
> > +};
> > +
> > +static const struct linear_range axp318_dcdc8_ranges[] = {
> > + REGULATOR_LINEAR_RANGE(500000, 0, 70, 10000),
> > + REGULATOR_LINEAR_RANGE(1220000, 71, 102, 20000),
> > + REGULATOR_LINEAR_RANGE(1900000, 103, 118, 100000),
> > +};
>
> In the AXP318W datasheet, it says:
> section 7.1 DCDC/LCO desgin
> 8. DCDC6/7/8/9 only able to tune at two voltage ranges which are
> <1.54v and >1.54v, the tuning voltage should not step cross 1.54v
> (I translate the original doc into english)
Thanks, I now read something similar in my Google translated copy of the
datasheet. But I don't understand what this is supposed to mean? That
exactly 1.54V does not work, so the value of 87 is invalid? But any
other value can be set?
>
> so, with this restricition, should we split the range into two?
> one is dcdc6_lo_range, another dcdc6_hi_range
>
> or what do you think?
>
> > +
> > +static const struct regulator_desc axp318_regulators[] = {
> > + AXP_DESC(AXP318, DCDC1, "dcdc1", "vin19", 1000, 3400, 100,
> > + AXP318_DCDC1_CONTROL, AXP318_DCDC1_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL1, BIT(0)),
> > + AXP_DESC_RANGES(AXP318, DCDC2, "dcdc2", "vin23",
> > + axp318_dcdc2_ranges, AXP318_DCDC2_NUM_VOLTAGES,
> > + AXP318_DCDC2_CONTROL, AXP318_DCDC2_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL1, BIT(1)),
> > + AXP_DESC_RANGES(AXP318, DCDC3, "dcdc3", "vin23",
> > + axp318_dcdc2_ranges, AXP318_DCDC2_NUM_VOLTAGES,
> > + AXP318_DCDC3_CONTROL, AXP318_DCDC2_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL1, BIT(2)),
> > + AXP_DESC_RANGES(AXP318, DCDC4, "dcdc4", "vin45",
> > + axp318_dcdc2_ranges, AXP318_DCDC2_NUM_VOLTAGES,
> > + AXP318_DCDC4_CONTROL, AXP318_DCDC2_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL1, BIT(3)),
> > + AXP_DESC_RANGES(AXP318, DCDC5, "dcdc5", "vin45",
> > + axp318_dcdc2_ranges, AXP318_DCDC2_NUM_VOLTAGES,
> > + AXP318_DCDC5_CONTROL, AXP318_DCDC2_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL1, BIT(4)),
> > + AXP_DESC_RANGES(AXP318, DCDC6, "dcdc6", "vin678",
> > + axp318_dcdc6_ranges, AXP318_DCDC6_NUM_VOLTAGES,
> > + AXP318_DCDC6_CONTROL, AXP318_DCDC2_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL1, BIT(5)),
> > + AXP_DESC_RANGES(AXP318, DCDC7, "dcdc7", "vin678",
> > + axp318_dcdc7_ranges, AXP318_DCDC7_NUM_VOLTAGES,
> > + AXP318_DCDC7_CONTROL, AXP318_DCDC2_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL1, BIT(6)),
> > + AXP_DESC_RANGES(AXP318, DCDC8, "dcdc8", "vin678",
> > + axp318_dcdc8_ranges, AXP318_DCDC8_NUM_VOLTAGES,
> > + AXP318_DCDC8_CONTROL, AXP318_DCDC2_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL1, BIT(7)),
> > + AXP_DESC_RANGES(AXP318, DCDC9, "dcdc9", "vin19",
> > + axp318_dcdc8_ranges, AXP318_DCDC8_NUM_VOLTAGES,
> > + AXP318_DCDC9_CONTROL, AXP318_DCDC2_V_OUT_MASK,
> > + AXP318_DCDC_OUTPUT_CONTROL2, BIT(0)),
> > + AXP_DESC_SW(AXP318, SWOUT1, "swout1", NULL,
> > + AXP318_DCDC_OUTPUT_CONTROL2, BIT(3)),
> > + AXP_DESC_SW(AXP318, SWOUT2, "swout2", NULL,
> > + AXP318_DCDC_OUTPUT_CONTROL2, BIT(4)),
> > + AXP_DESC(AXP318, ALDO1, "aldo1", "aldo156in", 500, 3400, 100,
> > + AXP318_ALDO1_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL1, BIT(0)),
> > + AXP_DESC(AXP318, ALDO2, "aldo2", "aldo234in", 500, 3400, 100,
> > + AXP318_ALDO2_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL1, BIT(1)),
> > + AXP_DESC(AXP318, ALDO3, "aldo3", "aldo234in", 500, 3400, 100,
> > + AXP318_ALDO3_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL1, BIT(2)),
> > + AXP_DESC(AXP318, ALDO4, "aldo4", "aldo234in", 500, 3400, 100,
> > + AXP318_ALDO4_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL1, BIT(3)),
> > + AXP_DESC(AXP318, ALDO5, "aldo5", "aldo156in", 500, 3400, 100,
> > + AXP318_ALDO5_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL1, BIT(4)),
> > + AXP_DESC(AXP318, ALDO6, "aldo6", "aldo156in", 500, 3400, 100,
> > + AXP318_ALDO6_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL1, BIT(5)),
> > + AXP_DESC(AXP318, BLDO1, "bldo1", "bldoin", 500, 3400, 100,
> > + AXP318_BLDO1_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL1, BIT(6)),
> > + AXP_DESC(AXP318, BLDO2, "bldo2", "bldoin", 500, 3400, 100,
> > + AXP318_BLDO2_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL1, BIT(7)),
> > + AXP_DESC(AXP318, BLDO3, "bldo3", "bldoin", 500, 3400, 100,
> > + AXP318_BLDO3_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL2, BIT(0)),
> > + AXP_DESC(AXP318, BLDO4, "bldo4", "bldoin", 500, 3400, 100,
> > + AXP318_BLDO4_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL2, BIT(1)),
> > + AXP_DESC(AXP318, BLDO5, "bldo5", "bldoin", 500, 3400, 100,
> > + AXP318_BLDO5_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL2, BIT(2)),
> > + AXP_DESC(AXP318, CLDO1, "cldo1", "cldoin", 500, 3400, 100,
> > + AXP318_CLDO1_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL2, BIT(3)),
> > + AXP_DESC(AXP318, CLDO2, "cldo2", "cldoin", 500, 3400, 100,
> > + AXP318_CLDO2_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL2, BIT(4)),
> > + AXP_DESC(AXP318, CLDO3, "cldo3", "cldoin", 500, 3400, 100,
> > + AXP318_CLDO3_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL2, BIT(5)),
> > + AXP_DESC(AXP318, CLDO4, "cldo4", "cldoin", 500, 3400, 100,
> > + AXP318_CLDO4_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL2, BIT(6)),
> > + AXP_DESC(AXP318, CLDO5, "cldo5", "cldoin", 500, 3400, 100,
> > + AXP318_CLDO5_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL2, BIT(7)),
> > + AXP_DESC(AXP318, DLDO1, "dldo1", "dldoin", 500, 3400, 100,
> > + AXP318_DLDO1_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL3, BIT(0)),
> > + AXP_DESC(AXP318, DLDO2, "dldo2", "dldoin", 500, 3400, 100,
> > + AXP318_DLDO2_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL3, BIT(1)),
> > + AXP_DESC(AXP318, DLDO3, "dldo3", "dldoin", 500, 3400, 100,
> > + AXP318_DLDO3_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL3, BIT(2)),
> > + AXP_DESC(AXP318, DLDO4, "dldo4", "dldoin", 500, 3400, 100,
> > + AXP318_DLDO4_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL3, BIT(3)),
> > + AXP_DESC(AXP318, DLDO5, "dldo5", "dldoin", 500, 3400, 100,
> > + AXP318_DLDO5_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL3, BIT(4)),
> > + AXP_DESC(AXP318, DLDO6, "dldo6", "dldoin", 500, 3400, 100,
> > + AXP318_DLDO6_CONTROL, AXP318_LDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL3, BIT(5)),
> ..
> > + AXP_DESC(AXP318, ELDO1, "eldo1", "eldoin", 500, 1500, 25,
> > + AXP318_ELDO1_CONTROL, AXP318_ELDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL3, BIT(6)),
> > + AXP_DESC(AXP318, ELDO2, "eldo2", "eldoin", 500, 1500, 25,
> > + AXP318_ELDO2_CONTROL, AXP318_ELDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL3, BIT(7)),
> > + AXP_DESC(AXP318, ELDO3, "eldo3", "eldoin", 500, 1500, 25,
> > + AXP318_ELDO3_CONTROL, AXP318_ELDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL4, BIT(0)),
> > + AXP_DESC(AXP318, ELDO4, "eldo4", "eldoin", 500, 1500, 25,
> > + AXP318_ELDO4_CONTROL, AXP318_ELDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL4, BIT(1)),
> > + AXP_DESC(AXP318, ELDO5, "eldo5", "eldoin", 500, 1500, 25,
> > + AXP318_ELDO5_CONTROL, AXP318_ELDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL4, BIT(2)),
> > + AXP_DESC(AXP318, ELDO6, "eldo6", "eldoin", 500, 1500, 25,
> > + AXP318_ELDO6_CONTROL, AXP318_ELDO_V_OUT_MASK,
> > + AXP318_LDO_OUTPUT_CONTROL4, BIT(3)),
>
> also, in section 7.1 DCDC/LCO desgin
> 3. ELDOIN can use DCDC's output as the voltage input, once in this case,
> the LDO (output?) config voltage should lower than DCDC input voltage.
>
> Note: ELDOIN can use PS(Power Supply, should be equal to DCIN) or DCDC as input
>
> in case of Radxa A7A (A733) board, it use DCDC9 as ELDOIN,
> Should we do something in the driver level? or leave up to user
"User" really means board vendor here, right? As the actual board
user is not meant to adjust those voltages anyway, and any range
limitations should be considered during the board design phase.
So yes, DCDC9 is at 1.24V on the Radxa, and ELDOIN is connected to
that, but only ELDO1 and ELDO6 are used, and they are fixed to 900mV
and 800mV, respectively, and connected to SoC pins that require exactly
those voltages. So there is no room for change or to adjust things
here, and the requirements are met.
If board designers/people ignore that, that it just won't work, and
they get to keep the pieces. Nothing the driver can do here.
Cheers,
Andre
>
> > +};
> > +
> > static const struct linear_range axp717_dcdc1_ranges[] = {
> > REGULATOR_LINEAR_RANGE(500000, 0, 70, 10000),
> > REGULATOR_LINEAR_RANGE(1220000, 71, 87, 20000),
> > @@ -1347,6 +1505,7 @@ static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
> > step = 150;
> > break;
> > case AXP313A_ID:
> > + case AXP318_ID:
> > case AXP323_ID:
> > case AXP717_ID:
> > case AXP15060_ID:
> > @@ -1585,6 +1744,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> > regulators = axp313a_regulators;
> > nregulators = AXP313A_REG_ID_MAX;
> > break;
> > + case AXP318_ID:
> > + regulators = axp318_regulators;
> > + nregulators = AXP318_REG_ID_MAX;
> > + break;
> > case AXP717_ID:
> > regulators = axp717_regulators;
> > nregulators = AXP717_REG_ID_MAX;
> > @@ -1651,7 +1814,9 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> > if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
> > (regulators == axp803_regulators && i == AXP803_DC1SW) ||
> > (regulators == axp809_regulators && i == AXP809_DC1SW) ||
> > - (regulators == axp15060_regulators && i == AXP15060_SW)) {
> > + (regulators == axp15060_regulators && i == AXP15060_SW) ||
> > + (regulators == axp318_regulators && i == AXP318_SWOUT1) ||
> > + (regulators == axp318_regulators && i == AXP318_SWOUT2)) {
> > new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
> > GFP_KERNEL);
> > if (!new_desc)
> > @@ -1709,7 +1874,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> > */
> > if ((regulators == axp22x_regulators && i == AXP22X_DCDC1) ||
> > (regulators == axp809_regulators && i == AXP809_DCDC1) ||
> > - (regulators == axp15060_regulators && i == AXP15060_DCDC1))
> > + (regulators == axp15060_regulators && i == AXP15060_DCDC1) ||
> > + (regulators == axp318_regulators && i == AXP318_DCDC1))
> > of_property_read_string(rdev->dev.of_node,
> > "regulator-name",
> > &dcdc1_name);
> > diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> > index a871789f6cfa9..9957185458d63 100644
> > --- a/include/linux/mfd/axp20x.h
> > +++ b/include/linux/mfd/axp20x.h
> > @@ -559,6 +559,49 @@ enum {
> > AXP313A_REG_ID_MAX,
> > };
> >
> > +enum {
> > + AXP318_DCDC1 = 0,
> > + AXP318_DCDC2,
> > + AXP318_DCDC3,
> > + AXP318_DCDC4,
> > + AXP318_DCDC5,
> > + AXP318_DCDC6,
> > + AXP318_DCDC7,
> > + AXP318_DCDC8,
> > + AXP318_DCDC9,
> > + AXP318_ALDO1,
> > + AXP318_ALDO2,
> > + AXP318_ALDO3,
> > + AXP318_ALDO4,
> > + AXP318_ALDO5,
> > + AXP318_ALDO6,
> > + AXP318_BLDO1,
> > + AXP318_BLDO2,
> > + AXP318_BLDO3,
> > + AXP318_BLDO4,
> > + AXP318_BLDO5,
> > + AXP318_CLDO1,
> > + AXP318_CLDO2,
> > + AXP318_CLDO3,
> > + AXP318_CLDO4,
> > + AXP318_CLDO5,
> > + AXP318_DLDO1,
> > + AXP318_DLDO2,
> > + AXP318_DLDO3,
> > + AXP318_DLDO4,
> > + AXP318_DLDO5,
> > + AXP318_DLDO6,
> > + AXP318_ELDO1,
> > + AXP318_ELDO2,
> > + AXP318_ELDO3,
> > + AXP318_ELDO4,
> > + AXP318_ELDO5,
> > + AXP318_ELDO6,
> > + AXP318_SWOUT1,
> > + AXP318_SWOUT2,
> > + AXP318_REG_ID_MAX,
> > +};
> > +
> > enum {
> > AXP717_DCDC1 = 0,
> > AXP717_DCDC2,
> > --
> > 2.25.1
> >
>
Powered by blists - more mailing lists