| 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: <CAGb2v649yQVcNn7uv1eKtnEDnb=D4X9yGYB1eOC3zeAe+encFg@mail.gmail.com>
Date: Sat, 28 Jan 2023 01:24:18 +0800
From: Chen-Yu Tsai <wens@...e.org>
To: Martin Botka <martin.botka@...ainline.org>
Cc: martin.botka1@...il.com,
Konrad Dybcio <konrad.dybcio@...ainline.org>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...ainline.org>,
Marijn Suijten <marijn.suijten@...ainline.org>,
Jami Kettunen <jamipkettunen@...ainline.org>,
Paul Bouchara <paul.bouchara@...ainline.org>,
Jan Trmal <jtrmal@...il.com>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Samuel Holland <samuel@...lland.org>,
Andre Przywara <andre.przywara@....com>,
Lee Jones <lee@...nel.org>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 3/3] regulator: axp20x: Add support for AXP313a variant
On Sat, Jan 21, 2023 at 2:45 AM Martin Botka
<martin.botka@...ainline.org> wrote:
>
> The AXP313a is your typical I2C controlled PMIC, although in a lighter
> fashion compared to the other X-Powers PMICs: it has only three DCDC
> rails, three LDOs, and no battery charging support.
>
> The AXP313a datasheet does not describe a register to change the DCDC
> switching frequency, and talks of it being fixed at 3 MHz. The BSP
> driver hints at a register being able to change that, but we haven't
> verified that, so leave that one out. It can be added later, if needed
> and/or required.
The datasheet released by MangoPi says this isn't configurable. The
thing that is configurable is spread-spectrum operation, and mode
switching between fixed PWM and hybrid PFM/PWM. So just drop the
DCDC frequency stuff and use the default code path.
> The third LDO, RTCLDO, is fixed, and cannot even be turned on or off,
> programmatically. On top of that, its voltage is customisable (either
> 1.8V or 3.3V), which we cannot describe easily using the existing
> regulator wrapper functions. This should be fixed properly, using
> regulator-{min,max}-microvolt in the DT, but this requires more changes
> to the code. As some other PMICs (AXP2xx, AXP803) seem to paper over the
> same problem as well, we follow suit here and pretend it's a fixed 1.8V
> regulator. A proper fix can follow later. The BSP code seems to ignore
> this regulator altogether.
>
> Describe the AXP313A's voltage settings and switch registers, how the
> voltages are encoded, and connect this to the MFD device via its
> regulator ID.
>
> Signed-off-by: Martin Botka <martin.botka@...ainline.org>
> Signed-off-by: Andre Przywara <andre.przywara@....com>
> ---
> drivers/regulator/axp20x-regulator.c | 60 ++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index d260c442b788..3087bc98694f 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -134,6 +134,11 @@
> #define AXP22X_PWR_OUT_DLDO4_MASK BIT_MASK(6)
> #define AXP22X_PWR_OUT_ALDO3_MASK BIT_MASK(7)
>
> +#define AXP313A_DCDC1_NUM_VOLTAGES 107
> +#define AXP313A_DCDC23_NUM_VOLTAGES 88
> +#define AXP313A_DCDC_V_OUT_MASK GENMASK(6, 0)
> +#define AXP313A_LDO_V_OUT_MASK GENMASK(4, 0)
> +
> #define AXP803_PWR_OUT_DCDC1_MASK BIT_MASK(0)
> #define AXP803_PWR_OUT_DCDC2_MASK BIT_MASK(1)
> #define AXP803_PWR_OUT_DCDC3_MASK BIT_MASK(2)
> @@ -638,6 +643,48 @@ static const struct regulator_desc axp22x_drivevbus_regulator = {
> .ops = &axp20x_ops_sw,
> };
>
> +static const struct linear_range axp313a_dcdc1_ranges[] = {
> + REGULATOR_LINEAR_RANGE(500000, 0, 70, 10000),
> + REGULATOR_LINEAR_RANGE(1220000, 71, 87, 20000),
> + REGULATOR_LINEAR_RANGE(1600000, 88, 106, 100000),
> +};
> +
> +static const struct linear_range axp313a_dcdc2_ranges[] = {
> + REGULATOR_LINEAR_RANGE(500000, 0, 70, 10000),
> + REGULATOR_LINEAR_RANGE(1220000, 71, 87, 20000),
> +};
> +
> +/*
> + * This is deviating from the datasheet. The values here are taken from the
> + * BSP driver and have been confirmed by measurements.
> + */
> +static const struct linear_range axp313a_dcdc3_ranges[] = {
> + REGULATOR_LINEAR_RANGE(500000, 0, 70, 10000),
> + REGULATOR_LINEAR_RANGE(1220000, 71, 102, 20000),
> +};
> +
> +static const struct regulator_desc axp313a_regulators[] = {
> + AXP_DESC_RANGES(AXP313A, DCDC1, "dcdc1", "vin1",
> + axp313a_dcdc1_ranges, AXP313A_DCDC1_NUM_VOLTAGES,
> + AXP313A_DCDC1_CONRTOL, AXP313A_DCDC_V_OUT_MASK,
> + AXP313A_OUTPUT_CONTROL, BIT(0)),
> + AXP_DESC_RANGES(AXP313A, DCDC2, "dcdc2", "vin2",
> + axp313a_dcdc2_ranges, AXP313A_DCDC23_NUM_VOLTAGES,
> + AXP313A_DCDC2_CONRTOL, AXP313A_DCDC_V_OUT_MASK,
> + AXP313A_OUTPUT_CONTROL, BIT(1)),
> + AXP_DESC_RANGES(AXP313A, DCDC3, "dcdc3", "vin3",
> + axp313a_dcdc3_ranges, AXP313A_DCDC23_NUM_VOLTAGES,
> + AXP313A_DCDC3_CONRTOL, AXP313A_DCDC_V_OUT_MASK,
> + AXP313A_OUTPUT_CONTROL, BIT(2)),
> + AXP_DESC(AXP313A, LDO1, "ldo1", "vin1", 500, 3500, 100,
> + AXP313A_ALDO1_CONRTOL, AXP313A_LDO_V_OUT_MASK,
> + AXP313A_OUTPUT_CONTROL, BIT(3)),
The datasheet says this one is called ALDO1 ...
> + AXP_DESC(AXP313A, LDO2, "ldo2", "vin1", 500, 3500, 100,
> + AXP313A_DLDO1_CONRTOL, AXP313A_LDO_V_OUT_MASK,
> + AXP313A_OUTPUT_CONTROL, BIT(4)),
... and this one DLDO1.
> + AXP_DESC_FIXED(AXP313A, RTC_LDO, "rtc-ldo", "vin1", 1800),
> +};
> +
> /* DCDC ranges shared with AXP813 */
> static const struct linear_range axp803_dcdc234_ranges[] = {
> REGULATOR_LINEAR_RANGE(500000,
> @@ -1040,6 +1087,15 @@ static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
> def = 3000;
> step = 150;
> break;
> + case AXP313A_ID:
> + /* The DCDC PWM frequency seems to be fixed to 3 MHz. */
> + if (dcdcfreq != 3000000 && dcdcfreq != 0) {
> + dev_err(&pdev->dev,
> + "DCDC frequency on AXP313a is fixed to 3 MHz.\n");
> + return -EINVAL;
> + }
> +
> + return 0;
As mentioned above, please drop this.
Besides the bits mentioned above, this looks OK.
> default:
> dev_err(&pdev->dev,
> "Setting DCDC frequency for unsupported AXP variant\n");
> @@ -1232,6 +1288,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
> "x-powers,drive-vbus-en");
> break;
> + case AXP313A_ID:
> + regulators = axp313a_regulators;
> + nregulators = AXP313A_REG_ID_MAX;
> + break;
> case AXP803_ID:
> regulators = axp803_regulators;
> nregulators = AXP803_REG_ID_MAX;
> --
> 2.39.0
>
Powered by blists - more mailing lists