[PATCH v2 0/6] power: pmic: sunxi: consolidate AXP SPL drivers
Hi,
v2 of this series merges the AXP717 support patches in, as they didn't make it into the tree yet. As there is little point in creating a separate AXP717 SPL driver file, just to delete it again two patches later, this series skips the separate file altogether. I put in the related AXP717 U-Boot proper driver here. The code itself is mostly unchanged from v1, it's just a rebase and re-arrangement of the patches.
========================= This is the first series in an attempt to clean up the X-Powers AXP PMIC drivers used by the SPL for sunxi boards. So far we have a separate driver file for each AXP variant, but the code was largely the same, just differing by the regulator ranges.
This adds a new generic driver, which reads the regulator description from an array of structs. This is similar to how the DM AXP driver used for U-Boot proper works, but is simplified, since we won't need the full feature set for the SPL, and we want to keep the code size small.
To help bisect-ability, and to simplify review, each of the simpler AXP drivers covered by this approach is handled in a separate patch.
We just add the AXP717 support first, then convert over the AXP313 and AXP305. The other AXP SPL drivers are more complex, and support more regulators, but my hunch is that we really just need the DC/DC converters in the SPL. However I need to prove and test this, so I will convert the other AXP chips later.
Please have a look and comment whether the approach in general is a good idea.
Cheers, Andre
Changelog v1 .. v2: - rebase against latest master - merge originally separate AXP717 SPL driver into patch 4/6 - include AXP717 U-Boot proper support patch - drop AXP707 support for now
Andre Przywara (6): power: regulator: add AXP717 support power: pmic: sunxi: only build AXP drivers for SPL power: pmic: sunxi: introduce generic SPL AXP DC/DC driver power: pmic: sunxi: add AXP717 SPL support power: pmic: sunxi: use generic AXP SPL driver for AXP313 power: pmic: sunxi: use generic AXP SPL driver for AXP305
arch/arm/mach-sunxi/pmic_bus.c | 3 + board/sunxi/board.c | 2 +- drivers/power/Kconfig | 17 ++- drivers/power/Makefile | 7 +- drivers/power/axp305.c | 82 ----------- drivers/power/axp313.c | 133 ------------------ drivers/power/axp_spl.c | 173 ++++++++++++++++++++++++ drivers/power/pmic/axp.c | 1 + drivers/power/regulator/axp_regulator.c | 28 ++++ include/axp_pmic.h | 1 + 10 files changed, 225 insertions(+), 222 deletions(-) delete mode 100644 drivers/power/axp305.c delete mode 100644 drivers/power/axp313.c create mode 100644 drivers/power/axp_spl.c
The X-Powers AXP717 is a PMIC with four buck converters and a number of LDOs, one of which is actually fixed (so not modelled here).
Add the compatible string and the respective regulator ranges to allow drivers to adjust voltages.
Signed-off-by: Andre Przywara andre.przywara@arm.com Reviewed-by: Ryan Walklin ryan@testtoast.com --- drivers/power/pmic/axp.c | 1 + drivers/power/regulator/axp_regulator.c | 28 +++++++++++++++++++++++++ include/axp_pmic.h | 1 + 3 files changed, 30 insertions(+)
diff --git a/drivers/power/pmic/axp.c b/drivers/power/pmic/axp.c index 0e1e45fba74..521a39dd566 100644 --- a/drivers/power/pmic/axp.c +++ b/drivers/power/pmic/axp.c @@ -88,6 +88,7 @@ static const struct udevice_id axp_pmic_ids[] = { { .compatible = "x-powers,axp221", .data = AXP221_ID }, { .compatible = "x-powers,axp223", .data = AXP223_ID }, { .compatible = "x-powers,axp313a", .data = AXP313_ID }, + { .compatible = "x-powers,axp717", .data = AXP717_ID }, { .compatible = "x-powers,axp803", .data = AXP803_ID }, { .compatible = "x-powers,axp806", .data = AXP806_ID }, { .compatible = "x-powers,axp809", .data = AXP809_ID }, diff --git a/drivers/power/regulator/axp_regulator.c b/drivers/power/regulator/axp_regulator.c index d27e09538e0..75cdbca30f6 100644 --- a/drivers/power/regulator/axp_regulator.c +++ b/drivers/power/regulator/axp_regulator.c @@ -189,6 +189,33 @@ static const struct axp_regulator_plat axp313_regulators[] = { { } };
+/* + * The "dcdc2" regulator has another range, beyond 1.54V up to 3.4V, in + * steps of 100mV. We cannot model this easily, but also don't need that, + * since it's typically only used for lower voltages anyway, so just ignore it. + */ +static const struct axp_regulator_plat axp717_regulators[] = { + { "dcdc1", 0x80, BIT(0), 0x83, 0x7f, 500, 1540, 10, 70 }, + { "dcdc2", 0x80, BIT(1), 0x84, 0x7f, 500, 1540, 10, 70 }, + { "dcdc3", 0x80, BIT(2), 0x85, 0x7f, 500, 1840, 10, 70 }, + { "dcdc4", 0x80, BIT(3), 0x86, 0x7f, 1000, 3700, 100, NA }, + { "aldo1", 0x90, BIT(0), 0x93, 0x1f, 500, 3500, 100, NA }, + { "aldo2", 0x90, BIT(1), 0x94, 0x1f, 500, 3500, 100, NA }, + { "aldo3", 0x90, BIT(2), 0x95, 0x1f, 500, 3500, 100, NA }, + { "aldo4", 0x90, BIT(3), 0x96, 0x1f, 500, 3500, 100, NA }, + { "bldo1", 0x90, BIT(4), 0x97, 0x1f, 500, 3500, 100, NA }, + { "bldo2", 0x90, BIT(5), 0x98, 0x1f, 500, 3500, 100, NA }, + { "bldo3", 0x90, BIT(6), 0x99, 0x1f, 500, 3500, 100, NA }, + { "bldo4", 0x90, BIT(7), 0x9a, 0x1f, 500, 3500, 100, NA }, + { "cldo1", 0x91, BIT(0), 0x9b, 0x1f, 500, 3500, 100, NA }, + { "cldo2", 0x91, BIT(1), 0x9c, 0x1f, 500, 3500, 100, NA }, + { "cldo3", 0x91, BIT(2), 0x9d, 0x1f, 500, 3500, 100, NA }, + { "cldo4", 0x91, BIT(3), 0x9e, 0x1f, 500, 3500, 100, NA }, + {"cpusldo",0x91, BIT(4), 0x9f, 0x1f, 500, 1400, 50, NA }, + {" boost", 0x19, BIT(4), 0x1e, 0xf0, 4550, 5510, 64, NA }, + { } +}; + static const struct axp_regulator_plat axp803_regulators[] = { { "dcdc1", 0x10, BIT(0), 0x20, 0x1f, 1600, 3400, 100, NA }, { "dcdc2", 0x10, BIT(1), 0x21, 0x7f, 500, 1300, 10, 70 }, @@ -291,6 +318,7 @@ static const struct axp_regulator_plat *const axp_regulators[] = { [AXP221_ID] = axp22x_regulators, [AXP223_ID] = axp22x_regulators, [AXP313_ID] = axp313_regulators, + [AXP717_ID] = axp717_regulators, [AXP803_ID] = axp803_regulators, [AXP806_ID] = axp806_regulators, [AXP809_ID] = axp809_regulators, diff --git a/include/axp_pmic.h b/include/axp_pmic.h index aabafc8501b..ae62ef0d76d 100644 --- a/include/axp_pmic.h +++ b/include/axp_pmic.h @@ -33,6 +33,7 @@ enum { AXP221_ID, AXP223_ID, AXP313_ID, + AXP717_ID, AXP803_ID, AXP806_ID, AXP809_ID,
The axp<xxx>.c drivers are only used for the SPL, for U-Boot proper we have a separate, DM compliant driver. Mask the build instructions with CONFIG_SPL_BUILD, to avoid them being build for U-Boot proper as well.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- drivers/power/Makefile | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index c7ee4595fc8..0f39459dec4 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_$(SPL_TPL_)POWER_DOMAIN) += domain/ obj-y += pmic/ obj-y += regulator/
+ifdef CONFIG_SPL_BUILD obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_AXP221_POWER) += axp221.o @@ -15,6 +16,7 @@ obj-$(CONFIG_AXP305_POWER) += axp305.o obj-$(CONFIG_AXP313_POWER) += axp313.o obj-$(CONFIG_AXP809_POWER) += axp809.o obj-$(CONFIG_AXP818_POWER) += axp818.o +endif obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o obj-$(CONFIG_SY8106A_POWER) += sy8106a.o obj-$(CONFIG_TPS6586X_POWER) += tps6586x.o
On Fri, 12 Jul 2024 17:53:48 +0100 Andre Przywara andre.przywara@arm.com wrote:
The axp<xxx>.c drivers are only used for the SPL, for U-Boot proper we have a separate, DM compliant driver. Mask the build instructions with CONFIG_SPL_BUILD, to avoid them being build for U-Boot proper as well.
For the records: this breaks builds of boards with the AXP221, since they need axp_get_sid() for the cpuinfo() function in U-Boot proper, and axp_set_eldo() for some hideous code in sunxi_display.c. I have a decent fix for the former, but the latter requires some refactoring. So I will keep the axp221.o line out of this SPL-only guard for now.
Cheers, Andre
Signed-off-by: Andre Przywara andre.przywara@arm.com
drivers/power/Makefile | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index c7ee4595fc8..0f39459dec4 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_$(SPL_TPL_)POWER_DOMAIN) += domain/ obj-y += pmic/ obj-y += regulator/
+ifdef CONFIG_SPL_BUILD obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_AXP221_POWER) += axp221.o @@ -15,6 +16,7 @@ obj-$(CONFIG_AXP305_POWER) += axp305.o obj-$(CONFIG_AXP313_POWER) += axp313.o obj-$(CONFIG_AXP809_POWER) += axp809.o obj-$(CONFIG_AXP818_POWER) += axp818.o +endif obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o obj-$(CONFIG_SY8106A_POWER) += sy8106a.o obj-$(CONFIG_TPS6586X_POWER) += tps6586x.o
So far we had a separate driver file for each AXP PMIC chip that we need to support in the SPL. The code in there was largely similar, but differed in many details.
Based on the idea of the DM AXP driver, introduce a data structure to describe each regulator in a compact way. This is a simplified version of the struct used in the DM driver, as we don't need to support the full voltage range and not every regulator in the SPL. For now we only support the DC/DC buck converters, since that's what we need the SPL to configure, mostly. Also we get rid of the regulator name, and hardcode the regulator number by its position in the array (first is DCDC1, second is DCDC2, etc). We also drop support for the value table, we ideally won't need that for the subset of regulators required. At the end each regulator is described by a 10 bytes struct, so we avoid blowing up the SPL footprint, but still can use generic code.
Each chip is supposed to be described separately, and protected by ifdef's, to only build in the regulators needed for a particular board. We also describe the bits to help identifying the AXP chip, and the shutdown details in that section.
Add a generic driver, that exports axp_set_dcdc<x>() functions to set up the buck converters. For now this just contains the bits for the (new) AXP717, but it's not wired up anywhere yet.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- drivers/power/axp_spl.c | 143 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 drivers/power/axp_spl.c
diff --git a/drivers/power/axp_spl.c b/drivers/power/axp_spl.c new file mode 100644 index 00000000000..0382d91cb4c --- /dev/null +++ b/drivers/power/axp_spl.c @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * AXP PMIC SPL driver + * (C) Copyright 2024 Arm Ltd. + */ + +#include <errno.h> +#include <linux/types.h> +#include <asm/arch/pmic_bus.h> +#include <axp_pmic.h> + +struct axp_reg_desc_spl { + u8 enable_reg; + u8 enable_mask; + u8 volt_reg; + u8 volt_mask; + u16 min_mV; + u16 max_mV; + u8 step_mV; + u8 split; +}; + +#define NA 0xff + +#if defined(CONFIG_AXP717_POWER) /* AXP717 */ + +static const struct axp_reg_desc_spl axp_spl_dcdc_regulators[] = { + { 0x80, BIT(0), 0x83, 0x7f, 500, 1540, 10, 70 }, + { 0x80, BIT(1), 0x84, 0x7f, 500, 1540, 10, 70 }, + { 0x80, BIT(2), 0x85, 0x7f, 500, 1840, 10, 70 }, +}; + +#define AXP_CHIP_VERSION 0x0 +#define AXP_CHIP_VERSION_MASK 0x0 +#define AXP_CHIP_ID 0x0 +#define AXP_SHUTDOWN_REG 0x27 +#define AXP_SHUTDOWN_MASK BIT(0) + +#else + + #error "Please define the regulator registers in axp_spl_regulators[]." + +#endif + +static u8 axp_mvolt_to_cfg(int mvolt, const struct axp_reg_desc_spl *reg) +{ + if (mvolt < reg->min_mV) + mvolt = reg->min_mV; + else if (mvolt > reg->max_mV) + mvolt = reg->max_mV; + + mvolt -= reg->min_mV; + + /* voltage in the first range ? */ + if (mvolt <= reg->split * reg->step_mV) + return mvolt / reg->step_mV; + + mvolt -= reg->split * reg->step_mV; + + return reg->split + mvolt / (reg->step_mV * 2); +} + +static int axp_set_dcdc(int dcdc_num, unsigned int mvolt) +{ + const struct axp_reg_desc_spl *reg; + int ret; + + if (dcdc_num < 1 || dcdc_num > ARRAY_SIZE(axp_spl_dcdc_regulators)) + return -EINVAL; + + reg = &axp_spl_dcdc_regulators[dcdc_num - 1]; + + if (mvolt == 0) + return pmic_bus_clrbits(reg->enable_reg, reg->enable_mask); + + ret = pmic_bus_write(reg->volt_reg, axp_mvolt_to_cfg(mvolt, reg)); + if (ret) + return ret; + + return pmic_bus_setbits(reg->enable_reg, reg->enable_mask); +} + +int axp_set_dcdc1(unsigned int mvolt) +{ + return axp_set_dcdc(1, mvolt); +} + +int axp_set_dcdc2(unsigned int mvolt) +{ + return axp_set_dcdc(2, mvolt); +} + +int axp_set_dcdc3(unsigned int mvolt) +{ + return axp_set_dcdc(3, mvolt); +} + +int axp_set_dcdc4(unsigned int mvolt) +{ + return axp_set_dcdc(4, mvolt); +} + +int axp_set_dcdc5(unsigned int mvolt) +{ + return axp_set_dcdc(5, mvolt); +} + +int axp_init(void) +{ + int ret = pmic_bus_init(); + + if (ret) + return ret; + + if (AXP_CHIP_VERSION_MASK) { + u8 axp_chip_id; + + ret = pmic_bus_read(AXP_CHIP_VERSION, &axp_chip_id); + if (ret) + return ret; + + if ((axp_chip_id & AXP_CHIP_VERSION_MASK) != AXP_CHIP_ID) { + debug("unknown PMIC: 0x%x\n", axp_chip_id); + return -EINVAL; + } + } + + return 0; +} + +#if !CONFIG_IS_ENABLED(ARM_PSCI_FW) && !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) +int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{ + pmic_bus_setbits(AXP_SHUTDOWN_REG, AXP_SHUTDOWN_MASK); + + /* infinite loop during shutdown */ + while (1) + ; + + /* not reached */ + return 0; +} +#endif
On boards using the AXP717 PMIC, the DRAM rail is often not setup correctly at reset time, so we have to program the PMIC very early in the SPL, before running the DRAM initialisation.
Using the new generic AXP SPL driver, add the Kconfig options and platform bits needed to support an AXP717 PMIC chip in I2C mode. This allows to set up the correct voltage for the DRAM chips and the CPU cores.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- arch/arm/mach-sunxi/pmic_bus.c | 3 +++ board/sunxi/board.c | 2 +- drivers/power/Kconfig | 17 +++++++++++++---- drivers/power/Makefile | 1 + 4 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/pmic_bus.c index 87df312725c..8e19324c8ac 100644 --- a/arch/arm/mach-sunxi/pmic_bus.c +++ b/arch/arm/mach-sunxi/pmic_bus.c @@ -19,6 +19,7 @@ #define AXP152_I2C_ADDR 0x30
#define AXP209_I2C_ADDR 0x34 +#define AXP717_I2C_ADDR 0x34
#define AXP305_I2C_ADDR 0x36 #define AXP313_I2C_ADDR 0x36 @@ -36,6 +37,8 @@ static int pmic_i2c_address(void) return AXP305_I2C_ADDR; if (IS_ENABLED(CONFIG_AXP313_POWER)) return AXP313_I2C_ADDR; + if (IS_ENABLED(CONFIG_AXP717_POWER)) + return AXP717_I2C_ADDR;
/* Other AXP2xx and AXP8xx variants */ return AXP209_I2C_ADDR; diff --git a/board/sunxi/board.c b/board/sunxi/board.c index ed86f1df5dc..9928069c0eb 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -562,7 +562,7 @@ void sunxi_board_init(void) #if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \ defined CONFIG_AXP221_POWER || defined CONFIG_AXP305_POWER || \ defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER || \ - defined CONFIG_AXP313_POWER + defined CONFIG_AXP313_POWER || defined CONFIG_AXP717_POWER power_failed = axp_init();
if (IS_ENABLED(CONFIG_AXP_DISABLE_BOOT_ON_POWERON) && !power_failed) { diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 33b8bc1214d..5556a22cf69 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -109,6 +109,13 @@ config AXP313_POWER Select this to enable support for the AXP313 PMIC found on some H616 boards.
+config AXP717_POWER + bool "axp717 pmic support" + select AXP_PMIC_BUS + select CMD_POWEROFF + ---help--- + Select this to enable support for the AXP717 PMIC found on some boards. + config AXP809_POWER bool "axp809 pmic support" depends on MACH_SUN9I @@ -151,10 +158,11 @@ config AXP_DCDC1_VOLT
config AXP_DCDC2_VOLT int "axp pmic dcdc2 voltage" - depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER || AXP313_POWER + depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER || AXP313_POWER || AXP717_POWER default 900 if AXP818_POWER default 1400 if AXP152_POWER || AXP209_POWER default 1000 if AXP313_POWER + default 1000 if AXP717_POWER default 1200 if MACH_SUN6I default 1100 if MACH_SUN8I default 0 if MACH_SUN9I @@ -167,11 +175,11 @@ config AXP_DCDC2_VOLT On A80 boards dcdc2 powers the GPU and can be left off. On A83T boards dcdc2 is used for VDD-CPUA(cluster 0) and should be 0.9V. On R40 boards dcdc2 is VDD-CPU and should be 1.1V - On boards using the AXP313 it's often VDD-CPU. + On boards using the AXP313 or AXP717 it's often VDD-CPU.
config AXP_DCDC3_VOLT int "axp pmic dcdc3 voltage" - depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER || AXP313_POWER + depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER || AXP313_POWER || AXP717_POWER default 900 if AXP809_POWER || AXP818_POWER default 1500 if AXP152_POWER default 1250 if AXP209_POWER @@ -188,7 +196,8 @@ config AXP_DCDC3_VOLT On A80 boards dcdc3 is used for VDD-CPUA(cluster 0) and should be 0.9V. On A83T boards dcdc3 is used for VDD-CPUB(cluster 1) and should be 0.9V. On R40 boards dcdc3 is VDD-SYS and VDD-GPU and should be 1.1V. - On boards using the AXP313 it's often VDD-DRAM and should be 1.1V for LPDDR4. + On boards using the AXP313 or AXP717 it's often VDD-DRAM and should + be 1.1V for LPDDR4.
config AXP_DCDC4_VOLT int "axp pmic dcdc4 voltage" diff --git a/drivers/power/Makefile b/drivers/power/Makefile index 0f39459dec4..6df23a81c29 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_AXP221_POWER) += axp221.o obj-$(CONFIG_AXP305_POWER) += axp305.o obj-$(CONFIG_AXP313_POWER) += axp313.o +obj-$(CONFIG_AXP717_POWER) += axp_spl.o obj-$(CONFIG_AXP809_POWER) += axp809.o obj-$(CONFIG_AXP818_POWER) += axp818.o endif
Hi Andre,
On Sat, 13 Jul 2024, at 4:53 AM, Andre Przywara wrote:
#define AXP209_I2C_ADDR 0x34 +#define AXP717_I2C_ADDR 0x34
#define AXP305_I2C_ADDR 0x36 #define AXP313_I2C_ADDR 0x36 @@ -36,6 +37,8 @@ static int pmic_i2c_address(void) return AXP305_I2C_ADDR; if (IS_ENABLED(CONFIG_AXP313_POWER)) return AXP313_I2C_ADDR;
if (IS_ENABLED(CONFIG_AXP717_POWER))
return AXP717_I2C_ADDR;/* Other AXP2xx and AXP8xx variants */ return AXP209_I2C_ADDR;
Small point, but is this check strictly necessary, given that the AXP717 uses the same I2C address as the AXP209? If CONFIG_AXP717_POWER is not checked here, this logic will still fall through to the default AXP209 address, which will also be correct for the 717.
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index ed86f1df5dc..9928069c0eb 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -562,7 +562,7 @@ void sunxi_board_init(void) #if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \ defined CONFIG_AXP221_POWER || defined CONFIG_AXP305_POWER || \ defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER || \
- defined CONFIG_AXP313_POWER
defined CONFIG_AXP313_POWER || defined CONFIG_AXP717_POWER power_failed = axp_init();
if (IS_ENABLED(CONFIG_AXP_DISABLE_BOOT_ON_POWERON) && !power_failed) {
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 33b8bc1214d..5556a22cf69 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -109,6 +109,13 @@ config AXP313_POWER Select this to enable support for the AXP313 PMIC found on some H616 boards.
+config AXP717_POWER
- bool "axp717 pmic support"
- select AXP_PMIC_BUS
- select CMD_POWEROFF
- ---help---
- Select this to enable support for the AXP717 PMIC found on some boards.
config AXP809_POWER bool "axp809 pmic support" depends on MACH_SUN9I @@ -151,10 +158,11 @@ config AXP_DCDC1_VOLT
config AXP_DCDC2_VOLT int "axp pmic dcdc2 voltage"
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER ||
AXP809_POWER || AXP818_POWER || AXP313_POWER
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER ||
AXP809_POWER || AXP818_POWER || AXP313_POWER || AXP717_POWER default 900 if AXP818_POWER default 1400 if AXP152_POWER || AXP209_POWER default 1000 if AXP313_POWER
- default 1000 if AXP717_POWER default 1200 if MACH_SUN6I default 1100 if MACH_SUN8I default 0 if MACH_SUN9I
@@ -167,11 +175,11 @@ config AXP_DCDC2_VOLT On A80 boards dcdc2 powers the GPU and can be left off. On A83T boards dcdc2 is used for VDD-CPUA(cluster 0) and should be 0.9V. On R40 boards dcdc2 is VDD-CPU and should be 1.1V
- On boards using the AXP313 it's often VDD-CPU.
- On boards using the AXP313 or AXP717 it's often VDD-CPU.
config AXP_DCDC3_VOLT int "axp pmic dcdc3 voltage"
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER ||
AXP809_POWER || AXP818_POWER || AXP313_POWER
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER ||
AXP809_POWER || AXP818_POWER || AXP313_POWER || AXP717_POWER default 900 if AXP809_POWER || AXP818_POWER default 1500 if AXP152_POWER default 1250 if AXP209_POWER @@ -188,7 +196,8 @@ config AXP_DCDC3_VOLT On A80 boards dcdc3 is used for VDD-CPUA(cluster 0) and should be 0.9V. On A83T boards dcdc3 is used for VDD-CPUB(cluster 1) and should be 0.9V. On R40 boards dcdc3 is VDD-SYS and VDD-GPU and should be 1.1V.
- On boards using the AXP313 it's often VDD-DRAM and should be 1.1V for
LPDDR4.
- On boards using the AXP313 or AXP717 it's often VDD-DRAM and should
- be 1.1V for LPDDR4.
config AXP_DCDC4_VOLT int "axp pmic dcdc4 voltage" diff --git a/drivers/power/Makefile b/drivers/power/Makefile index 0f39459dec4..6df23a81c29 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_AXP221_POWER) += axp221.o obj-$(CONFIG_AXP305_POWER) += axp305.o obj-$(CONFIG_AXP313_POWER) += axp313.o +obj-$(CONFIG_AXP717_POWER) += axp_spl.o obj-$(CONFIG_AXP809_POWER) += axp809.o obj-$(CONFIG_AXP818_POWER) += axp818.o endif -- 2.25.1
Tested with RG35XX-H and RG35XX-Plus boards (Allwinner H700 with AXP717). Confirmed both DCDC2 and DCDC3 are set correctly in SPL, allowing successful DRAM init and boot.
Tested-by: Ryan Walklin ryan@testtoast.com
Regards,
Ryan
On Sun, 14 Jul 2024 20:20:44 +1200 "Ryan Walklin" ryan@testtoast.com wrote:
Hi Ryan,
On Sat, 13 Jul 2024, at 4:53 AM, Andre Przywara wrote:
#define AXP209_I2C_ADDR 0x34 +#define AXP717_I2C_ADDR 0x34
#define AXP305_I2C_ADDR 0x36 #define AXP313_I2C_ADDR 0x36 @@ -36,6 +37,8 @@ static int pmic_i2c_address(void) return AXP305_I2C_ADDR; if (IS_ENABLED(CONFIG_AXP313_POWER)) return AXP313_I2C_ADDR;
if (IS_ENABLED(CONFIG_AXP717_POWER))
return AXP717_I2C_ADDR;/* Other AXP2xx and AXP8xx variants */ return AXP209_I2C_ADDR;
Small point, but is this check strictly necessary, given that the AXP717 uses the same I2C address as the AXP209? If CONFIG_AXP717_POWER is not checked here, this logic will still fall through to the default AXP209 address, which will also be correct for the 717.
I consider the fact that the AXP209 and the AXP717 use the same I2C address a sheer coincidence, so would like to keep the code readable and maintainable. This "IS_ENABLED(AXPyyy) return AXPyyy_I2C_ADDR" patterns seems to be easy to understand and copy, IMHO. If you are concerned about generated code size: this whole function is optimised away, and is replaced with a single "mov w0, #0x34" assembly instruction, just before the respective i2c function calls. This is due to the fact that those "if" statements have compile-time constant input, so the compiler knows the final result already, and just replaces the function call with this constant load. That's a neat feature of IS_ENABLED(), and a pattern we are exercising in U-Boot for years, to keep the code both readable, but also minimal.
Thanks for the Tested-by: tag!
Cheers, Andre
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index ed86f1df5dc..9928069c0eb 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -562,7 +562,7 @@ void sunxi_board_init(void) #if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \ defined CONFIG_AXP221_POWER || defined CONFIG_AXP305_POWER || \ defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER || \
- defined CONFIG_AXP313_POWER
defined CONFIG_AXP313_POWER || defined CONFIG_AXP717_POWER power_failed = axp_init();
if (IS_ENABLED(CONFIG_AXP_DISABLE_BOOT_ON_POWERON) && !power_failed) {
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 33b8bc1214d..5556a22cf69 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -109,6 +109,13 @@ config AXP313_POWER Select this to enable support for the AXP313 PMIC found on some H616 boards.
+config AXP717_POWER
- bool "axp717 pmic support"
- select AXP_PMIC_BUS
- select CMD_POWEROFF
- ---help---
- Select this to enable support for the AXP717 PMIC found on some boards.
config AXP809_POWER bool "axp809 pmic support" depends on MACH_SUN9I @@ -151,10 +158,11 @@ config AXP_DCDC1_VOLT
config AXP_DCDC2_VOLT int "axp pmic dcdc2 voltage"
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER ||
AXP809_POWER || AXP818_POWER || AXP313_POWER
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER ||
AXP809_POWER || AXP818_POWER || AXP313_POWER || AXP717_POWER default 900 if AXP818_POWER default 1400 if AXP152_POWER || AXP209_POWER default 1000 if AXP313_POWER
- default 1000 if AXP717_POWER default 1200 if MACH_SUN6I default 1100 if MACH_SUN8I default 0 if MACH_SUN9I
@@ -167,11 +175,11 @@ config AXP_DCDC2_VOLT On A80 boards dcdc2 powers the GPU and can be left off. On A83T boards dcdc2 is used for VDD-CPUA(cluster 0) and should be 0.9V. On R40 boards dcdc2 is VDD-CPU and should be 1.1V
- On boards using the AXP313 it's often VDD-CPU.
- On boards using the AXP313 or AXP717 it's often VDD-CPU.
config AXP_DCDC3_VOLT int "axp pmic dcdc3 voltage"
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER ||
AXP809_POWER || AXP818_POWER || AXP313_POWER
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER ||
AXP809_POWER || AXP818_POWER || AXP313_POWER || AXP717_POWER default 900 if AXP809_POWER || AXP818_POWER default 1500 if AXP152_POWER default 1250 if AXP209_POWER @@ -188,7 +196,8 @@ config AXP_DCDC3_VOLT On A80 boards dcdc3 is used for VDD-CPUA(cluster 0) and should be 0.9V. On A83T boards dcdc3 is used for VDD-CPUB(cluster 1) and should be 0.9V. On R40 boards dcdc3 is VDD-SYS and VDD-GPU and should be 1.1V.
- On boards using the AXP313 it's often VDD-DRAM and should be 1.1V for
LPDDR4.
- On boards using the AXP313 or AXP717 it's often VDD-DRAM and should
- be 1.1V for LPDDR4.
config AXP_DCDC4_VOLT int "axp pmic dcdc4 voltage" diff --git a/drivers/power/Makefile b/drivers/power/Makefile index 0f39459dec4..6df23a81c29 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_AXP221_POWER) += axp221.o obj-$(CONFIG_AXP305_POWER) += axp305.o obj-$(CONFIG_AXP313_POWER) += axp313.o +obj-$(CONFIG_AXP717_POWER) += axp_spl.o obj-$(CONFIG_AXP809_POWER) += axp809.o obj-$(CONFIG_AXP818_POWER) += axp818.o endif -- 2.25.1
Tested with RG35XX-H and RG35XX-Plus boards (Allwinner H700 with AXP717). Confirmed both DCDC2 and DCDC3 are set correctly in SPL, allowing successful DRAM init and boot.
Tested-by: Ryan Walklin ryan@testtoast.com
Regards,
Ryan
On Mon, 15 Jul 2024, at 11:38 AM, Andre Przywara wrote:
On Sun, 14 Jul 2024 20:20:44 +1200 "Ryan Walklin" ryan@testtoast.com wrote:
Hi Ryan,
I consider the fact that the AXP209 and the AXP717 use the same I2C address a sheer coincidence, so would like to keep the code readable and maintainable. This "IS_ENABLED(AXPyyy) return AXPyyy_I2C_ADDR" patterns seems to be easy to understand and copy, IMHO. If you are concerned about generated code size: this whole function is optimised away, and is replaced with a single "mov w0, #0x34" assembly instruction, just before the respective i2c function calls. This is due to the fact that those "if" statements have compile-time constant input, so the compiler knows the final result already, and just replaces the function call with this constant load. That's a neat feature of IS_ENABLED(), and a pattern we are exercising in U-Boot for years, to keep the code both readable, but also minimal.
I did wonder if the compiler might optimise this down to a single instruction, makes perfect sense in that case and agree regarding code clarity.
In that case, and having also checked the DCDC voltage defaults are reasonable for known AXP717-integrating devices in the updated Kconfig:
Reviewed-by: Ryan Walklin ryan@testtoast.com
Regards,
Ryan
The generic AXP SPL driver implementation can cover all regulators we need for the AXP313.
Add the descriptions for the three DC/DC regulators of the AXP313, and enable that when CONFIG_AXP313_POWER is enabled. Also remove the old driver, and switch the Makefile to include the new, generic version.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- drivers/power/Makefile | 2 +- drivers/power/axp313.c | 133 ---------------------------------------- drivers/power/axp_spl.c | 14 +++++ 3 files changed, 15 insertions(+), 134 deletions(-) delete mode 100644 drivers/power/axp313.c
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index 6df23a81c29..30e0daf0621 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -13,7 +13,7 @@ obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_AXP221_POWER) += axp221.o obj-$(CONFIG_AXP305_POWER) += axp305.o -obj-$(CONFIG_AXP313_POWER) += axp313.o +obj-$(CONFIG_AXP313_POWER) += axp_spl.o obj-$(CONFIG_AXP717_POWER) += axp_spl.o obj-$(CONFIG_AXP809_POWER) += axp809.o obj-$(CONFIG_AXP818_POWER) += axp818.o diff --git a/drivers/power/axp313.c b/drivers/power/axp313.c deleted file mode 100644 index 09ecb5b1ec2..00000000000 --- a/drivers/power/axp313.c +++ /dev/null @@ -1,133 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * AXP313(a) driver - * - * (C) Copyright 2023 Arm Ltd. - * - * Based on axp305.c - * (C) Copyright 2020 Jernej Skrabec jernej.skrabec@siol.net - * (C) Copyright 2014 Hans de Goede hdegoede@redhat.com - * (C) Copyright 2013 Oliver Schinagl oliver@schinagl.nl - */ - -#include <command.h> -#include <errno.h> -#include <asm/arch/pmic_bus.h> -#include <axp_pmic.h> - -enum axp313_reg { - AXP313_CHIP_VERSION = 0x03, - AXP313_OUTPUT_CTRL = 0x10, - AXP313_DCDC1_CTRL = 0x13, - AXP313_SHUTDOWN = 0x1a, -}; - -#define AXP313_CHIP_VERSION_MASK 0xcf -#define AXP313_CHIP_VERSION_AXP1530 0x48 -#define AXP313_CHIP_VERSION_AXP313A 0x4b -#define AXP313_CHIP_VERSION_AXP313B 0x4c - -#define AXP313_DCDC_SPLIT_OFFSET 71 -#define AXP313_DCDC_SPLIT_MVOLT 1200 - -#define AXP313_POWEROFF BIT(7) - -static u8 mvolt_to_cfg(int mvolt, int min, int max, int div) -{ - if (mvolt < min) - mvolt = min; - else if (mvolt > max) - mvolt = max; - - return (mvolt - min) / div; -} - -static int axp_set_dcdc(int dcdc_num, unsigned int mvolt) -{ - int ret; - u8 cfg, enable_mask = 1U << (dcdc_num - 1); - int volt_reg = AXP313_DCDC1_CTRL + dcdc_num - 1; - int max_mV; - - switch (dcdc_num) { - case 1: - case 2: - max_mV = 1540; - break; - case 3: - /* - * The manual defines a different split point, but tests - * show that it's the same 1200mV as for DCDC1/2. - */ - max_mV = 1840; - break; - default: - return -EINVAL; - } - - if (mvolt > AXP313_DCDC_SPLIT_MVOLT) - cfg = AXP313_DCDC_SPLIT_OFFSET + mvolt_to_cfg(mvolt, - AXP313_DCDC_SPLIT_MVOLT + 20, max_mV, 20); - else - cfg = mvolt_to_cfg(mvolt, 500, AXP313_DCDC_SPLIT_MVOLT, 10); - - if (mvolt == 0) - return pmic_bus_clrbits(AXP313_OUTPUT_CTRL, enable_mask); - - debug("DCDC%d: writing 0x%x to reg 0x%x\n", dcdc_num, cfg, volt_reg); - ret = pmic_bus_write(volt_reg, cfg); - if (ret) - return ret; - - return pmic_bus_setbits(AXP313_OUTPUT_CTRL, enable_mask); -} - -int axp_set_dcdc2(unsigned int mvolt) -{ - return axp_set_dcdc(2, mvolt); -} - -int axp_set_dcdc3(unsigned int mvolt) -{ - return axp_set_dcdc(3, mvolt); -} - -int axp_init(void) -{ - u8 axp_chip_id; - int ret; - - ret = pmic_bus_init(); - if (ret) - return ret; - - ret = pmic_bus_read(AXP313_CHIP_VERSION, &axp_chip_id); - if (ret) - return ret; - - axp_chip_id &= AXP313_CHIP_VERSION_MASK; - switch (axp_chip_id) { - case AXP313_CHIP_VERSION_AXP1530: - case AXP313_CHIP_VERSION_AXP313A: - case AXP313_CHIP_VERSION_AXP313B: - break; - default: - debug("unknown PMIC: 0x%x\n", axp_chip_id); - return -EINVAL; - } - - return ret; -} - -#if !CONFIG_IS_ENABLED(ARM_PSCI_FW) && !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) -int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) -{ - pmic_bus_write(AXP313_SHUTDOWN, AXP313_POWEROFF); - - /* infinite loop during shutdown */ - while (1) {} - - /* not reached */ - return 0; -} -#endif diff --git a/drivers/power/axp_spl.c b/drivers/power/axp_spl.c index 0382d91cb4c..68cbf60eedf 100644 --- a/drivers/power/axp_spl.c +++ b/drivers/power/axp_spl.c @@ -36,6 +36,20 @@ static const struct axp_reg_desc_spl axp_spl_dcdc_regulators[] = { #define AXP_SHUTDOWN_REG 0x27 #define AXP_SHUTDOWN_MASK BIT(0)
+#elif defined(CONFIG_AXP313_POWER) /* AXP313 */ + +static const struct axp_reg_desc_spl axp_spl_dcdc_regulators[] = { + { 0x10, BIT(0), 0x13, 0x7f, 500, 1540, 10, 70 }, + { 0x10, BIT(1), 0x14, 0x7f, 500, 1540, 10, 70 }, + { 0x10, BIT(2), 0x15, 0x7f, 500, 1840, 10, 70 }, +}; + +#define AXP_CHIP_VERSION 0x3 +#define AXP_CHIP_VERSION_MASK 0xc8 +#define AXP_CHIP_ID 0x48 +#define AXP_SHUTDOWN_REG 0x1a +#define AXP_SHUTDOWN_MASK BIT(7) + #else
#error "Please define the regulator registers in axp_spl_regulators[]."
The generic AXP SPL driver implementation can cover all regulators we need for the AXP305.
Add the descriptions for four of the six DC/DC regulators of the AXP305, and enable that when CONFIG_AXP305_POWER is enabled. We won't need DCDC2 and DCDC3, but by using the position in the array for the index we keep the code cleaner. Also remove the old driver, and switch the Makefile to include the new, generic driver.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- drivers/power/Makefile | 2 +- drivers/power/axp305.c | 82 ----------------------------------------- drivers/power/axp_spl.c | 16 ++++++++ 3 files changed, 17 insertions(+), 83 deletions(-) delete mode 100644 drivers/power/axp305.c
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index 30e0daf0621..8d36f87ed32 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -12,7 +12,7 @@ ifdef CONFIG_SPL_BUILD obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_AXP221_POWER) += axp221.o -obj-$(CONFIG_AXP305_POWER) += axp305.o +obj-$(CONFIG_AXP305_POWER) += axp_spl.o obj-$(CONFIG_AXP313_POWER) += axp_spl.o obj-$(CONFIG_AXP717_POWER) += axp_spl.o obj-$(CONFIG_AXP809_POWER) += axp809.o diff --git a/drivers/power/axp305.c b/drivers/power/axp305.c deleted file mode 100644 index 0312ad9af76..00000000000 --- a/drivers/power/axp305.c +++ /dev/null @@ -1,82 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * AXP305 driver - * - * (C) Copyright 2020 Jernej Skrabec jernej.skrabec@siol.net - * - * Based on axp221.c - * (C) Copyright 2014 Hans de Goede hdegoede@redhat.com - * (C) Copyright 2013 Oliver Schinagl oliver@schinagl.nl - */ - -#include <command.h> -#include <errno.h> -#include <asm/arch/pmic_bus.h> -#include <axp_pmic.h> - -#define AXP305_DCDC4_1600MV_OFFSET 46 - -static u8 axp305_mvolt_to_cfg(int mvolt, int min, int max, int div) -{ - if (mvolt < min) - mvolt = min; - else if (mvolt > max) - mvolt = max; - - return (mvolt - min) / div; -} - -int axp_set_dcdc4(unsigned int mvolt) -{ - int ret; - u8 cfg; - - if (mvolt >= 1600) - cfg = AXP305_DCDC4_1600MV_OFFSET + - axp305_mvolt_to_cfg(mvolt, 1600, 3300, 100); - else - cfg = axp305_mvolt_to_cfg(mvolt, 600, 1500, 20); - - if (mvolt == 0) - return pmic_bus_clrbits(AXP305_OUTPUT_CTRL1, - AXP305_OUTPUT_CTRL1_DCDCD_EN); - - ret = pmic_bus_write(AXP305_DCDCD_VOLTAGE, cfg); - if (ret) - return ret; - - return pmic_bus_setbits(AXP305_OUTPUT_CTRL1, - AXP305_OUTPUT_CTRL1_DCDCD_EN); -} - -int axp_init(void) -{ - u8 axp_chip_id; - int ret; - - ret = pmic_bus_init(); - if (ret) - return ret; - - ret = pmic_bus_read(AXP305_CHIP_VERSION, &axp_chip_id); - if (ret) - return ret; - - if ((axp_chip_id & AXP305_CHIP_VERSION_MASK) != 0x40) - return -ENODEV; - - return ret; -} - -#if !CONFIG_IS_ENABLED(ARM_PSCI_FW) && !IS_ENABLED(CONFIG_SYSRESET_CMD_POWEROFF) -int do_poweroff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) -{ - pmic_bus_write(AXP305_SHUTDOWN, AXP305_POWEROFF); - - /* infinite loop during shutdown */ - while (1) {} - - /* not reached */ - return 0; -} -#endif diff --git a/drivers/power/axp_spl.c b/drivers/power/axp_spl.c index 68cbf60eedf..3c86eb20ab4 100644 --- a/drivers/power/axp_spl.c +++ b/drivers/power/axp_spl.c @@ -50,6 +50,22 @@ static const struct axp_reg_desc_spl axp_spl_dcdc_regulators[] = { #define AXP_SHUTDOWN_REG 0x1a #define AXP_SHUTDOWN_MASK BIT(7)
+#elif defined(CONFIG_AXP305_POWER) /* AXP305 */ + +static const struct axp_reg_desc_spl axp_spl_dcdc_regulators[] = { + { 0x10, BIT(0), 0x12, 0x7f, 600, 1520, 10, 50 }, + { 0x10, BIT(1), 0x13, 0x1f, 1000, 2550, 50, NA }, + { 0x10, BIT(2), 0x14, 0x7f, 600, 1520, 10, 50 }, + { 0x10, BIT(3), 0x15, 0x3f, 600, 1500, 20, NA }, + { 0x10, BIT(4), 0x16, 0x1f, 1100, 3400, 100, NA }, +}; + +#define AXP_CHIP_VERSION 0x3 +#define AXP_CHIP_VERSION_MASK 0xcf +#define AXP_CHIP_ID 0x40 +#define AXP_SHUTDOWN_REG 0x32 +#define AXP_SHUTDOWN_MASK BIT(7) + #else
#error "Please define the regulator registers in axp_spl_regulators[]."
participants (2)
-
Andre Przywara -
Ryan Walklin