Dne ponedeljek, 11. januar 2021 ob 01:02:07 CET je André Przywara napisal(a):
On 03/01/2021 09:26, Jernej Skrabec wrote:
This PMIC can be found on H616 boards and it's very similar to AXP805 and AXP806.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
The existing sunxi PMIC code is the typical U-Boot mess, but I don't want to block this series on a rework. I put some comments and ideas below how to improve the whole "framework".
For this actual patch I checked the bits against the AXP305 datasheet, it does what we need.
(reluctantly) Reviewed-by: Andre Przywara andre.przywara@arm.com
Thanks, Andre
arch/arm/mach-sunxi/pmic_bus.c | 6 +++ board/sunxi/board.c | 10 +++-- drivers/power/Kconfig | 13 +++++- drivers/power/Makefile | 1 + drivers/power/axp305.c | 80 ++++++++++++++++++++++++++++++++++ include/axp305.h | 17 ++++++++ include/axp_pmic.h | 3 ++ 7 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 drivers/power/axp305.c create mode 100644 include/axp305.h
diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/
pmic_bus.c
index dea42de833f1..0394ce856448 100644 --- a/arch/arm/mach-sunxi/pmic_bus.c +++ b/arch/arm/mach-sunxi/pmic_bus.c @@ -18,6 +18,8 @@
#define AXP209_I2C_ADDR 0x34
+#define AXP305_I2C_ADDR 0x36
#define AXP221_CHIP_ADDR 0x68 #define AXP221_CTRL_ADDR 0x3e #define AXP221_INIT_DATA 0x3e @@ -64,6 +66,8 @@ int pmic_bus_read(u8 reg, u8 *data) return i2c_read(AXP152_I2C_ADDR, reg, 1, data, 1); #elif defined CONFIG_AXP209_POWER return i2c_read(AXP209_I2C_ADDR, reg, 1, data, 1); +#elif defined CONFIG_AXP305_POWER
- return i2c_read(AXP305_I2C_ADDR, reg, 1, data, 1);
#elif defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || defined
CONFIG_AXP818_POWER
# ifdef CONFIG_MACH_SUN6I return p2wi_read(reg, data); @@ -81,6 +85,8 @@ int pmic_bus_write(u8 reg, u8 data) return i2c_write(AXP152_I2C_ADDR, reg, 1, &data, 1); #elif defined CONFIG_AXP209_POWER return i2c_write(AXP209_I2C_ADDR, reg, 1, &data, 1); +#elif defined CONFIG_AXP305_POWER
- return i2c_write(AXP305_I2C_ADDR, reg, 1, &data, 1);
#elif defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || defined
CONFIG_AXP818_POWER
# ifdef CONFIG_MACH_SUN6I return p2wi_write(reg, data); diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 708a27ed78e9..54ff9bc92396 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -634,16 +634,18 @@ void sunxi_board_init(void) #endif
This whole function is really a mess, but we can clean this up later. I wonder if we can guard with the actual CONFIG_AXP_*_VOLT symbols directly instead of specifying the list of PMICs requiring this rail (this list is already in Kconfig). Maybe with some macro magic to avoid the repetitions and move the ifdefs out of the function? power_failed |= axp_set_rail(dcdc1, DCDC1); and assemble the function name and CONFIG_ symbol in that macro, plus having the guard there?
#if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \
- defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
- defined CONFIG_AXP818_POWER
- defined CONFIG_AXP221_POWER || defined CONFIG_AXP305_POWER || \
- defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER power_failed = axp_init();
#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \ defined CONFIG_AXP818_POWER power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT); #endif +#if !defined(CONFIG_AXP305_POWER) power_failed |= axp_set_dcdc2(CONFIG_AXP_DCDC2_VOLT); power_failed |= axp_set_dcdc3(CONFIG_AXP_DCDC3_VOLT); +#endif #if !defined(CONFIG_AXP209_POWER) && !defined(CONFIG_AXP818_POWER) power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT); #endif @@ -656,8 +658,10 @@ void sunxi_board_init(void) defined CONFIG_AXP818_POWER power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT); #endif +#if !defined(CONFIG_AXP305_POWER) power_failed |= axp_set_aldo2(CONFIG_AXP_ALDO2_VOLT); -#if !defined(CONFIG_AXP152_POWER) +#endif +#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER) power_failed |= axp_set_aldo3(CONFIG_AXP_ALDO3_VOLT); #endif #ifdef CONFIG_AXP209_POWER
Verified this hunk by playing CPP and removing all non-applicable calls. This ended up in: power_failed = axp_init(); power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
Which looks like the idea behind this patch, but is totally non-obvious from the diff.
Yes, that's correct. I wonder if we need all these power rails to be really enabled at this time? IMO we need only those which powers DRAM and probably CPU, all others can be moved to U-Boot proper.
Every potential boot source must be already powered before SPL even begins execution, so first stage bootloader (BROM) can even load SPL from it.
Best regards, Jernej
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 02050f6f3569..d17cf2d9112a 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -48,6 +48,15 @@ config AXP221_POWER Select this to enable support for the axp221/axp223 pmic found on
most
A23 and A31 boards.
+config AXP305_POWER
- bool "axp305 pmic support"
- depends on MACH_SUN50I_H616
- select AXP_PMIC_BUS
- select CMD_POWEROFF
- ---help---
- Select this to enable support for the axp305 pmic found on most
- H616 boards.
config AXP809_POWER bool "axp809 pmic support" depends on MACH_SUN9I @@ -127,11 +136,12 @@ config AXP_DCDC3_VOLT
config AXP_DCDC4_VOLT int "axp pmic dcdc4 voltage"
- depends on AXP152_POWER || AXP221_POWER || AXP809_POWER ||
AXP818_POWER
- depends on AXP152_POWER || AXP221_POWER || AXP809_POWER ||
AXP818_POWER || AXP305_POWER
default 1250 if AXP152_POWER default 1200 if MACH_SUN6I default 0 if MACH_SUN8I default 900 if MACH_SUN9I
- default 1500 if AXP305_POWER ---help--- Set the voltage (mV) to program the axp pmic dcdc4 at, set to 0 to disable dcdc4.
@@ -140,6 +150,7 @@ config AXP_DCDC4_VOLT On A23 / A33 boards dcdc4 is unused and should be disabled. On A80 boards dcdc4 powers VDD-SYS, HDMI, USB OTG and should be
0.9V.
On A83T boards dcdc4 is used for VDD-GPU.
- On H616 boards dcdcd is used for VCC-DRAM.
config AXP_DCDC5_VOLT int "axp pmic dcdc5 voltage" diff --git a/drivers/power/Makefile b/drivers/power/Makefile index 2dcc7bb99d02..0bef06920a7d 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -6,6 +6,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_AXP809_POWER) += axp809.o obj-$(CONFIG_AXP818_POWER) += axp818.o obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o diff --git a/drivers/power/axp305.c b/drivers/power/axp305.c new file mode 100644 index 000000000000..f620798bb1d7 --- /dev/null +++ b/drivers/power/axp305.c @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- AXP305 driver
- (C) Copyright 2020 Jernej Skrabec jernej.skrabec@siol.net
- Based on axp221.c
Given that, I wonder if we can (later) simplify this, ideally we end up at something like this: const struct axp_regulator axp_regulators[] = { {"dcdc1", 1600, 3400, 100, NA, 0x20, 0x10, 0}, ... https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/drivers/
allwinner/axp/axp803.c
Not sure that can cover all AXP chips, but it's worth a try to remove all this boilerplate.
Cheers, Andre
- (C) Copyright 2014 Hans de Goede hdegoede@redhat.com
- (C) Copyright 2013 Oliver Schinagl oliver@schinagl.nl
- */
+#include <common.h> +#include <command.h> +#include <errno.h> +#include <asm/arch/pmic_bus.h> +#include <axp_pmic.h>
+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 = 46 + 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;
+}
+#ifndef CONFIG_PSCI_RESET +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/include/axp305.h b/include/axp305.h new file mode 100644 index 000000000000..225c5040a322 --- /dev/null +++ b/include/axp305.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- (C) Copyright 2020 Jernej Skrabec jernej.skrabec@siol.net
- */
+enum axp305_reg {
- AXP305_CHIP_VERSION = 0x3,
- AXP305_OUTPUT_CTRL1 = 0x10,
- AXP305_DCDCD_VOLTAGE = 0x15,
- AXP305_SHUTDOWN = 0x32,
+};
+#define AXP305_CHIP_VERSION_MASK 0xcf
+#define AXP305_OUTPUT_CTRL1_DCDCD_EN (1 << 3)
+#define AXP305_POWEROFF (1 << 7) diff --git a/include/axp_pmic.h b/include/axp_pmic.h index 10091d0bb278..405044c3a32f 100644 --- a/include/axp_pmic.h +++ b/include/axp_pmic.h @@ -15,6 +15,9 @@ #ifdef CONFIG_AXP221_POWER #include <axp221.h> #endif +#ifdef CONFIG_AXP305_POWER +#include <axp305.h> +#endif #ifdef CONFIG_AXP809_POWER #include <axp809.h> #endif