[PATCH 0/3] power: add AXP313 PMIC support

[PATCH 0/3] power: add AXP313 PMIC support

[Andre Przywara]

The X-Powers AXP313 is a small PMIC that is controlled via I2C and
provides just three buck converters and three LDOs, plus a power button.
It is used on many newer boards using the Allwinner H616 or H618 SoCs.

Mostly all rails need to be always on, since each of them supplies an
essential part of the system, consequentially the reset default is to have
all of them enabled.
However the voltages need to be adjusted, especially the DRAM rail is
typically at 900mV, for instance, which is too low.

This series adds support for the proper DM AXP PMIC driver (patch 3/3),
but also adds a small driver to be used in (our non-DM) SPL, to adjust
the DRAM voltage before the DRAM initialisation starts (patch 2/3).

This also uses the opportunity to clean up an #ifdef nightmare in our
board.c (patch 1/3), which was actually duplicating some dependencies
already described in Kconfig.

This is one part of the enablement of many newer boards with the
H616/H618 SoC: without the DRAM voltage increased to 1.1V they won't boot.

Cheers,
Andre

Andre Przywara (3):
  sunxi: board: simplify early PMIC setup conditions
  power: pmic: sunxi: add AXP313 SPL driver
  power: regulator: add AXP313 support

 arch/arm/mach-sunxi/pmic_bus.c          |   3 +
 board/sunxi/board.c                     |  35 +++----
 drivers/power/Kconfig                   |  19 +++-
 drivers/power/Makefile                  |   1 +
 drivers/power/axp313.c                  | 134 ++++++++++++++++++++++++
 drivers/power/pmic/axp.c                |   1 +
 drivers/power/regulator/axp_regulator.c |  17 +++
 include/axp_pmic.h                      |   1 +
 8 files changed, 189 insertions(+), 22 deletions(-)
 create mode 100644 drivers/power/axp313.c

-- 
2.25.1

[PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Andre Przywara]

So far we have a convoluted #ifdef mesh that guards the early AXP PMIC
setup in board.c. That combination of &&, || and negations is very hard
to read, maintain and especially to extend.

Fortunately we have those same conditions already modelled in the
Kconfig file, so they are actually redundant. On top of that the real
reason we have those preprocessor guards in the first place is about the
symbols that are *conditionally* defined: without #ifdefs the build
would break because of them being undefined for many boards.

To simplify this, just change the guards to actually look at the symbols
needed, so CONFIG_AXP_xxx_VOLT instead of CONFIG_AXPyyy_POWER.
This drastically improves the readability of this code, and makes adding
PMIC support a pure Kconfig matter.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 board/sunxi/board.c   | 32 ++++++++++++++------------------
 drivers/power/Kconfig |  2 +-
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index ebaa9431984..65d79a02c25 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -597,50 +597,46 @@ void sunxi_board_init(void)
 		}
 	}
 
-#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
-	defined CONFIG_AXP818_POWER
+#ifdef CONFIG_AXP_DCDC1_VOLT
 	power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT);
+	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
 #endif
-#if !defined(CONFIG_AXP305_POWER)
+#ifdef CONFIG_AXP_DCDC2_VOLT
 	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)
+#ifdef CONFIG_AXP_DCDC4_VOLT
 	power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
 #endif
-#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
-	defined CONFIG_AXP818_POWER
-	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
-#endif
 
-#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
-	defined CONFIG_AXP818_POWER
+#ifdef CONFIG_AXP_ALDO1_VOLT
 	power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT);
 #endif
-#if !defined(CONFIG_AXP305_POWER)
+#ifdef CONFIG_AXP_ALDO2_VOLT
 	power_failed |= axp_set_aldo2(CONFIG_AXP_ALDO2_VOLT);
 #endif
-#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER)
+#ifdef CONFIG_AXP_ALDO3_VOLT
 	power_failed |= axp_set_aldo3(CONFIG_AXP_ALDO3_VOLT);
 #endif
-#ifdef CONFIG_AXP209_POWER
+#ifdef CONFIG_AXP_ALDO4_VOLT
 	power_failed |= axp_set_aldo4(CONFIG_AXP_ALDO4_VOLT);
 #endif
 
-#if defined(CONFIG_AXP221_POWER) || defined(CONFIG_AXP809_POWER) || \
-	defined(CONFIG_AXP818_POWER)
+#ifdef CONFIG_AXP_DLDO1_VOLT
 	power_failed |= axp_set_dldo(1, CONFIG_AXP_DLDO1_VOLT);
 	power_failed |= axp_set_dldo(2, CONFIG_AXP_DLDO2_VOLT);
-#if !defined CONFIG_AXP809_POWER
+#endif
+#ifdef CONFIG_AXP_DLDO3_VOLT
 	power_failed |= axp_set_dldo(3, CONFIG_AXP_DLDO3_VOLT);
 	power_failed |= axp_set_dldo(4, CONFIG_AXP_DLDO4_VOLT);
 #endif
+#ifdef CONFIG_AXP_ELDO1_VOLT
 	power_failed |= axp_set_eldo(1, CONFIG_AXP_ELDO1_VOLT);
 	power_failed |= axp_set_eldo(2, CONFIG_AXP_ELDO2_VOLT);
 	power_failed |= axp_set_eldo(3, CONFIG_AXP_ELDO3_VOLT);
 #endif
 
-#ifdef CONFIG_AXP818_POWER
+#ifdef CONFIG_AXP_FLDO1_VOLT
 	power_failed |= axp_set_fldo(1, CONFIG_AXP_FLDO1_VOLT);
 	power_failed |= axp_set_fldo(2, CONFIG_AXP_FLDO2_VOLT);
 	power_failed |= axp_set_fldo(3, CONFIG_AXP_FLDO3_VOLT);
@@ -649,7 +645,7 @@ void sunxi_board_init(void)
 #if defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER
 	power_failed |= axp_set_sw(IS_ENABLED(CONFIG_AXP_SW_ON));
 #endif
-#endif
+#endif	/* CONFIG_AXPxxx_POWER */
 	printf("DRAM:");
 	gd->ram_size = sunxi_dram_init();
 	printf(" %d MiB\n", (int)(gd->ram_size >> 20));
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 7f3b990d231..83cb31c937a 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -180,7 +180,7 @@ config AXP_DCDC3_VOLT
 
 config AXP_DCDC4_VOLT
 	int "axp pmic dcdc4 voltage"
-	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER || AXP305_POWER
+	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER || AXP305_POWER
 	default 1250 if AXP152_POWER
 	default 1200 if MACH_SUN6I
 	default 0 if MACH_SUN8I
-- 
2.25.1

[PATCH 2/3] power: pmic: sunxi: add AXP313 SPL driver

[Andre Przywara]

On boards using the AXP313 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.

Add a simple AXP313 PMIC driver that knows about DCDC2(CPU) and
DCDC3(DRAM), so that we can bump up the voltage before the DRAM init.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/pmic_bus.c |   3 +
 board/sunxi/board.c            |   3 +-
 drivers/power/Kconfig          |  17 ++++-
 drivers/power/Makefile         |   1 +
 drivers/power/axp313.c         | 134 +++++++++++++++++++++++++++++++++
 5 files changed, 155 insertions(+), 3 deletions(-)
 create mode 100644 drivers/power/axp313.c

diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/pmic_bus.c
index c0908406370..8e7625fe057 100644
--- a/arch/arm/mach-sunxi/pmic_bus.c
+++ b/arch/arm/mach-sunxi/pmic_bus.c
@@ -22,6 +22,7 @@
 #define AXP209_I2C_ADDR			0x34
 
 #define AXP305_I2C_ADDR			0x36
+#define AXP313_I2C_ADDR			0x36
 
 #define AXP221_CHIP_ADDR		0x68
 
@@ -34,6 +35,8 @@ static int pmic_i2c_address(void)
 		return AXP152_I2C_ADDR;
 	if (IS_ENABLED(CONFIG_AXP305_POWER))
 		return AXP305_I2C_ADDR;
+	if (IS_ENABLED(CONFIG_AXP313_POWER))
+		return AXP313_I2C_ADDR;
 
 	/* Other AXP2xx and AXP8xx variants */
 	return AXP209_I2C_ADDR;
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 65d79a02c25..39b0ad73a9c 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -584,7 +584,8 @@ 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_AXP809_POWER || defined CONFIG_AXP818_POWER || \
+	defined CONFIG_AXP313_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 83cb31c937a..a9117d215eb 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -101,6 +101,15 @@ config AXP305_POWER
 	Select this to enable support for the axp305 pmic found on most
 	H616 boards.
 
+config AXP313_POWER
+	bool "axp311 pmic support"
+	depends on MACH_SUN50I_H616
+	select AXP_PMIC_BUS
+	select CMD_POWEROFF
+	---help---
+	Select this to enable support for the AXP313 PMIC found on some
+	H616 boards.
+
 config AXP809_POWER
 	bool "axp809 pmic support"
 	depends on MACH_SUN9I
@@ -143,9 +152,10 @@ 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
+	depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER || AXP313_POWER
 	default 900 if AXP818_POWER
 	default 1400 if AXP152_POWER || AXP209_POWER
+	default 1000 if AXP313_POWER
 	default 1200 if MACH_SUN6I
 	default 1100 if MACH_SUN8I
 	default 0 if MACH_SUN9I
@@ -158,13 +168,15 @@ 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.
 
 config AXP_DCDC3_VOLT
 	int "axp pmic dcdc3 voltage"
-	depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER
+	depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER || AXP313_POWER
 	default 900 if AXP809_POWER || AXP818_POWER
 	default 1500 if AXP152_POWER
 	default 1250 if AXP209_POWER
+	default 1100 if AXP313_POWER
 	default 1100 if MACH_SUN8I_R40
 	default 1200 if MACH_SUN6I || MACH_SUN8I
 	---help---
@@ -177,6 +189,7 @@ 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.
 
 config AXP_DCDC4_VOLT
 	int "axp pmic dcdc4 voltage"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index ba64b2c5938..c7ee4595fc8 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -12,6 +12,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_AXP809_POWER)	+= axp809.o
 obj-$(CONFIG_AXP818_POWER)	+= axp818.o
 obj-$(CONFIG_EXYNOS_TMU)	+= exynos-tmu.o
diff --git a/drivers/power/axp313.c b/drivers/power/axp313.c
new file mode 100644
index 00000000000..bbc9e911115
--- /dev/null
+++ b/drivers/power/axp313.c
@@ -0,0 +1,134 @@
+// 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 <common.h>
+#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
-- 
2.25.1

[PATCH 3/3] power: regulator: add AXP313 support

[Andre Przywara]

The X-Powers AXP313a is a small PMIC with just three buck converters and
three 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>
---
 drivers/power/pmic/axp.c                |  1 +
 drivers/power/regulator/axp_regulator.c | 17 +++++++++++++++++
 include/axp_pmic.h                      |  1 +
 3 files changed, 19 insertions(+)

diff --git a/drivers/power/pmic/axp.c b/drivers/power/pmic/axp.c
index 025dac24f28..0e1e45fba74 100644
--- a/drivers/power/pmic/axp.c
+++ b/drivers/power/pmic/axp.c
@@ -87,6 +87,7 @@ static const struct udevice_id axp_pmic_ids[] = {
 	{ .compatible = "x-powers,axp209", .data = AXP209_ID },
 	{ .compatible = "x-powers,axp221", .data = AXP221_ID },
 	{ .compatible = "x-powers,axp223", .data = AXP223_ID },
+	{ .compatible = "x-powers,axp313a", .data = AXP313_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 02f320eac1e..d27e09538e0 100644
--- a/drivers/power/regulator/axp_regulator.c
+++ b/drivers/power/regulator/axp_regulator.c
@@ -173,6 +173,22 @@ static const struct axp_regulator_plat axp22x_regulators[] = {
 	{ }
 };
 
+/*
+ * The "dcdc1" 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 ~1.1V anyway, so just ignore it.
+ * Also the DCDC3 regulator is described wrongly in the (available) manual,
+ * experiments show that the split point is at 1200mV, as for DCDC1/2.
+ */
+static const struct axp_regulator_plat axp313_regulators[] = {
+	{ "dcdc1", 0x10, BIT(0), 0x13, 0x7f,  500, 1540,  10, 70 },
+	{ "dcdc2", 0x10, BIT(1), 0x14, 0x7f,  500, 1540,  10, 70 },
+	{ "dcdc3", 0x10, BIT(2), 0x15, 0x7f,  500, 1840,  10, 70 },
+	{ "aldo1", 0x10, BIT(3), 0x16, 0x1f,  500, 3500, 100, NA },
+	{ "dldo1", 0x10, BIT(4), 0x17, 0x1f,  500, 3500, 100, 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 },
@@ -274,6 +290,7 @@ static const struct axp_regulator_plat *const axp_regulators[] = {
 	[AXP209_ID]	= axp20x_regulators,
 	[AXP221_ID]	= axp22x_regulators,
 	[AXP223_ID]	= axp22x_regulators,
+	[AXP313_ID]	= axp313_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 4ac64865831..aabafc8501b 100644
--- a/include/axp_pmic.h
+++ b/include/axp_pmic.h
@@ -32,6 +32,7 @@ enum {
 	AXP209_ID,
 	AXP221_ID,
 	AXP223_ID,
+	AXP313_ID,
 	AXP803_ID,
 	AXP806_ID,
 	AXP809_ID,
-- 
2.25.1

Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Jernej Škrabec]

On Wednesday, October 18, 2023 5:50:12 PM CEST Andre Przywara wrote:
> So far we have a convoluted #ifdef mesh that guards the early AXP PMIC
> setup in board.c. That combination of &&, || and negations is very hard
> to read, maintain and especially to extend.
> 
> Fortunately we have those same conditions already modelled in the
> Kconfig file, so they are actually redundant. On top of that the real
> reason we have those preprocessor guards in the first place is about the
> symbols that are *conditionally* defined: without #ifdefs the build
> would break because of them being undefined for many boards.
> 
> To simplify this, just change the guards to actually look at the symbols
> needed, so CONFIG_AXP_xxx_VOLT instead of CONFIG_AXPyyy_POWER.
> This drastically improves the readability of this code, and makes adding
> PMIC support a pure Kconfig matter.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  board/sunxi/board.c   | 32 ++++++++++++++------------------
>  drivers/power/Kconfig |  2 +-
>  2 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index ebaa9431984..65d79a02c25 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -597,50 +597,46 @@ void sunxi_board_init(void)
>  		}
>  	}
> 
> -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> -	defined CONFIG_AXP818_POWER
> +#ifdef CONFIG_AXP_DCDC1_VOLT
>  	power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT);
> +	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
>  #endif
> -#if !defined(CONFIG_AXP305_POWER)
> +#ifdef CONFIG_AXP_DCDC2_VOLT
>  	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)
> +#ifdef CONFIG_AXP_DCDC4_VOLT
>  	power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
>  #endif
> -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> -	defined CONFIG_AXP818_POWER
> -	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
> -#endif
> 
> -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> -	defined CONFIG_AXP818_POWER
> +#ifdef CONFIG_AXP_ALDO1_VOLT
>  	power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT);
>  #endif
> -#if !defined(CONFIG_AXP305_POWER)
> +#ifdef CONFIG_AXP_ALDO2_VOLT
>  	power_failed |= axp_set_aldo2(CONFIG_AXP_ALDO2_VOLT);
>  #endif
> -#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER)
> +#ifdef CONFIG_AXP_ALDO3_VOLT
>  	power_failed |= axp_set_aldo3(CONFIG_AXP_ALDO3_VOLT);
>  #endif
> -#ifdef CONFIG_AXP209_POWER
> +#ifdef CONFIG_AXP_ALDO4_VOLT
>  	power_failed |= axp_set_aldo4(CONFIG_AXP_ALDO4_VOLT);
>  #endif
> 
> -#if defined(CONFIG_AXP221_POWER) || defined(CONFIG_AXP809_POWER) || \
> -	defined(CONFIG_AXP818_POWER)
> +#ifdef CONFIG_AXP_DLDO1_VOLT
>  	power_failed |= axp_set_dldo(1, CONFIG_AXP_DLDO1_VOLT);
>  	power_failed |= axp_set_dldo(2, CONFIG_AXP_DLDO2_VOLT);
> -#if !defined CONFIG_AXP809_POWER
> +#endif
> +#ifdef CONFIG_AXP_DLDO3_VOLT
>  	power_failed |= axp_set_dldo(3, CONFIG_AXP_DLDO3_VOLT);
>  	power_failed |= axp_set_dldo(4, CONFIG_AXP_DLDO4_VOLT);
>  #endif
> +#ifdef CONFIG_AXP_ELDO1_VOLT
>  	power_failed |= axp_set_eldo(1, CONFIG_AXP_ELDO1_VOLT);
>  	power_failed |= axp_set_eldo(2, CONFIG_AXP_ELDO2_VOLT);
>  	power_failed |= axp_set_eldo(3, CONFIG_AXP_ELDO3_VOLT);
>  #endif
> 
> -#ifdef CONFIG_AXP818_POWER
> +#ifdef CONFIG_AXP_FLDO1_VOLT
>  	power_failed |= axp_set_fldo(1, CONFIG_AXP_FLDO1_VOLT);
>  	power_failed |= axp_set_fldo(2, CONFIG_AXP_FLDO2_VOLT);
>  	power_failed |= axp_set_fldo(3, CONFIG_AXP_FLDO3_VOLT);
> @@ -649,7 +645,7 @@ void sunxi_board_init(void)
>  #if defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER
>  	power_failed |= axp_set_sw(IS_ENABLED(CONFIG_AXP_SW_ON));
>  #endif
> -#endif
> +#endif	/* CONFIG_AXPxxx_POWER */
>  	printf("DRAM:");
>  	gd->ram_size = sunxi_dram_init();
>  	printf(" %d MiB\n", (int)(gd->ram_size >> 20));
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 7f3b990d231..83cb31c937a 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -180,7 +180,7 @@ config AXP_DCDC3_VOLT
> 
>  config AXP_DCDC4_VOLT
>  	int "axp pmic dcdc4 voltage"
> -	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER || 
AXP818_POWER ||
> AXP305_POWER +	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER 
||
> AXP305_POWER default 1250 if AXP152_POWER
>  	default 1200 if MACH_SUN6I
>  	default 0 if MACH_SUN8I

This patch is great, but this last change doesn't seem to be directly 
connected?

Best regards,
Jernej

Re: [PATCH 2/3] power: pmic: sunxi: add AXP313 SPL driver

[Jernej Škrabec]

On Wednesday, October 18, 2023 5:50:13 PM CEST Andre Przywara wrote:
> On boards using the AXP313 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.
> 
> Add a simple AXP313 PMIC driver that knows about DCDC2(CPU) and
> DCDC3(DRAM), so that we can bump up the voltage before the DRAM init.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/mach-sunxi/pmic_bus.c |   3 +
>  board/sunxi/board.c            |   3 +-
>  drivers/power/Kconfig          |  17 ++++-
>  drivers/power/Makefile         |   1 +
>  drivers/power/axp313.c         | 134 +++++++++++++++++++++++++++++++++
>  5 files changed, 155 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/power/axp313.c
> 
> diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/pmic_bus.c
> index c0908406370..8e7625fe057 100644
> --- a/arch/arm/mach-sunxi/pmic_bus.c
> +++ b/arch/arm/mach-sunxi/pmic_bus.c
> @@ -22,6 +22,7 @@
>  #define AXP209_I2C_ADDR			0x34
> 
>  #define AXP305_I2C_ADDR			0x36
> +#define AXP313_I2C_ADDR			0x36
> 
>  #define AXP221_CHIP_ADDR		0x68
> 
> @@ -34,6 +35,8 @@ static int pmic_i2c_address(void)
>  		return AXP152_I2C_ADDR;
>  	if (IS_ENABLED(CONFIG_AXP305_POWER))
>  		return AXP305_I2C_ADDR;
> +	if (IS_ENABLED(CONFIG_AXP313_POWER))
> +		return AXP313_I2C_ADDR;
> 
>  	/* Other AXP2xx and AXP8xx variants */
>  	return AXP209_I2C_ADDR;
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 65d79a02c25..39b0ad73a9c 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -584,7 +584,8 @@ 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_AXP809_POWER || defined CONFIG_AXP818_POWER || \
> +	defined CONFIG_AXP313_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 83cb31c937a..a9117d215eb 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -101,6 +101,15 @@ config AXP305_POWER
>  	Select this to enable support for the axp305 pmic found on most
>  	H616 boards.
> 
> +config AXP313_POWER
> +	bool "axp311 pmic support"

Typo: axp311 -> axp313

Other than that, it looks good. I can't test it since I don't have any 
hardware with such PMIC.

So FWIW:
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> +	depends on MACH_SUN50I_H616
> +	select AXP_PMIC_BUS
> +	select CMD_POWEROFF
> +	---help---
> +	Select this to enable support for the AXP313 PMIC found on some
> +	H616 boards.
> +
>  config AXP809_POWER
>  	bool "axp809 pmic support"
>  	depends on MACH_SUN9I
> @@ -143,9 +152,10 @@ 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 +	depends on AXP152_POWER || AXP209_POWER || AXP221_POWER 
||
> AXP809_POWER || AXP818_POWER || AXP313_POWER default 900 if AXP818_POWER
>  	default 1400 if AXP152_POWER || AXP209_POWER
> +	default 1000 if AXP313_POWER
>  	default 1200 if MACH_SUN6I
>  	default 1100 if MACH_SUN8I
>  	default 0 if MACH_SUN9I
> @@ -158,13 +168,15 @@ 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.
> 
>  config AXP_DCDC3_VOLT
>  	int "axp pmic dcdc3 voltage"
> -	depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || 
AXP809_POWER ||
> AXP818_POWER +	depends on AXP152_POWER || AXP209_POWER || AXP221_POWER 
||
> AXP809_POWER || AXP818_POWER || AXP313_POWER default 900 if AXP809_POWER ||
> AXP818_POWER
>  	default 1500 if AXP152_POWER
>  	default 1250 if AXP209_POWER
> +	default 1100 if AXP313_POWER
>  	default 1100 if MACH_SUN8I_R40
>  	default 1200 if MACH_SUN6I || MACH_SUN8I
>  	---help---
> @@ -177,6 +189,7 @@ 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.
> 
>  config AXP_DCDC4_VOLT
>  	int "axp pmic dcdc4 voltage"
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index ba64b2c5938..c7ee4595fc8 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -12,6 +12,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_AXP809_POWER)	+= axp809.o
>  obj-$(CONFIG_AXP818_POWER)	+= axp818.o
>  obj-$(CONFIG_EXYNOS_TMU)	+= exynos-tmu.o
> diff --git a/drivers/power/axp313.c b/drivers/power/axp313.c
> new file mode 100644
> index 00000000000..bbc9e911115
> --- /dev/null
> +++ b/drivers/power/axp313.c
> @@ -0,0 +1,134 @@
> +// 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 <common.h>
> +#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

Re: [PATCH 3/3] power: regulator: add AXP313 support

[Jernej Škrabec]

On Wednesday, October 18, 2023 5:50:14 PM CEST Andre Przywara wrote:
> The X-Powers AXP313a is a small PMIC with just three buck converters and
> three 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>
> ---
>  drivers/power/pmic/axp.c                |  1 +
>  drivers/power/regulator/axp_regulator.c | 17 +++++++++++++++++
>  include/axp_pmic.h                      |  1 +
>  3 files changed, 19 insertions(+)
> 
> diff --git a/drivers/power/pmic/axp.c b/drivers/power/pmic/axp.c
> index 025dac24f28..0e1e45fba74 100644
> --- a/drivers/power/pmic/axp.c
> +++ b/drivers/power/pmic/axp.c
> @@ -87,6 +87,7 @@ static const struct udevice_id axp_pmic_ids[] = {
>  	{ .compatible = "x-powers,axp209", .data = AXP209_ID },
>  	{ .compatible = "x-powers,axp221", .data = AXP221_ID },
>  	{ .compatible = "x-powers,axp223", .data = AXP223_ID },
> +	{ .compatible = "x-powers,axp313a", .data = AXP313_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 02f320eac1e..d27e09538e0
> 100644
> --- a/drivers/power/regulator/axp_regulator.c
> +++ b/drivers/power/regulator/axp_regulator.c
> @@ -173,6 +173,22 @@ static const struct axp_regulator_plat
> axp22x_regulators[] = { { }
>  };
> 
> +/*
> + * The "dcdc1" 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 ~1.1V anyway, so just ignore it.

It can be modelled with look up table, as it's already done for some other 
regulators?

Best regards,
Jernej

> + * Also the DCDC3 regulator is described wrongly in the (available) manual,
> + * experiments show that the split point is at 1200mV, as for DCDC1/2. +
> */
> +static const struct axp_regulator_plat axp313_regulators[] = {
> +	{ "dcdc1", 0x10, BIT(0), 0x13, 0x7f,  500, 1540,  10, 70 },
> +	{ "dcdc2", 0x10, BIT(1), 0x14, 0x7f,  500, 1540,  10, 70 },
> +	{ "dcdc3", 0x10, BIT(2), 0x15, 0x7f,  500, 1840,  10, 70 },
> +	{ "aldo1", 0x10, BIT(3), 0x16, 0x1f,  500, 3500, 100, NA },
> +	{ "dldo1", 0x10, BIT(4), 0x17, 0x1f,  500, 3500, 100, 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 },
> @@ -274,6 +290,7 @@ static const struct axp_regulator_plat *const
> axp_regulators[] = { [AXP209_ID]	= axp20x_regulators,
>  	[AXP221_ID]	= axp22x_regulators,
>  	[AXP223_ID]	= axp22x_regulators,
> +	[AXP313_ID]	= axp313_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 4ac64865831..aabafc8501b 100644
> --- a/include/axp_pmic.h
> +++ b/include/axp_pmic.h
> @@ -32,6 +32,7 @@ enum {
>  	AXP209_ID,
>  	AXP221_ID,
>  	AXP223_ID,
> +	AXP313_ID,
>  	AXP803_ID,
>  	AXP806_ID,
>  	AXP809_ID,

Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Andre Przywara]

On Sat, 21 Oct 2023 08:34:06 +0200
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

> On Wednesday, October 18, 2023 5:50:12 PM CEST Andre Przywara wrote:
> > So far we have a convoluted #ifdef mesh that guards the early AXP PMIC
> > setup in board.c. That combination of &&, || and negations is very hard
> > to read, maintain and especially to extend.
> > 
> > Fortunately we have those same conditions already modelled in the
> > Kconfig file, so they are actually redundant. On top of that the real
> > reason we have those preprocessor guards in the first place is about the
> > symbols that are *conditionally* defined: without #ifdefs the build
> > would break because of them being undefined for many boards.
> > 
> > To simplify this, just change the guards to actually look at the symbols
> > needed, so CONFIG_AXP_xxx_VOLT instead of CONFIG_AXPyyy_POWER.
> > This drastically improves the readability of this code, and makes adding
> > PMIC support a pure Kconfig matter.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  board/sunxi/board.c   | 32 ++++++++++++++------------------
> >  drivers/power/Kconfig |  2 +-
> >  2 files changed, 15 insertions(+), 19 deletions(-)
> > 
> > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > index ebaa9431984..65d79a02c25 100644
> > --- a/board/sunxi/board.c
> > +++ b/board/sunxi/board.c
> > @@ -597,50 +597,46 @@ void sunxi_board_init(void)
> >  		}
> >  	}
> > 
> > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > -	defined CONFIG_AXP818_POWER
> > +#ifdef CONFIG_AXP_DCDC1_VOLT
> >  	power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT);
> > +	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
> >  #endif
> > -#if !defined(CONFIG_AXP305_POWER)
> > +#ifdef CONFIG_AXP_DCDC2_VOLT
> >  	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)
> > +#ifdef CONFIG_AXP_DCDC4_VOLT
> >  	power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
> >  #endif
> > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > -	defined CONFIG_AXP818_POWER
> > -	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
> > -#endif
> > 
> > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > -	defined CONFIG_AXP818_POWER
> > +#ifdef CONFIG_AXP_ALDO1_VOLT
> >  	power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT);
> >  #endif
> > -#if !defined(CONFIG_AXP305_POWER)
> > +#ifdef CONFIG_AXP_ALDO2_VOLT
> >  	power_failed |= axp_set_aldo2(CONFIG_AXP_ALDO2_VOLT);
> >  #endif
> > -#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER)
> > +#ifdef CONFIG_AXP_ALDO3_VOLT
> >  	power_failed |= axp_set_aldo3(CONFIG_AXP_ALDO3_VOLT);
> >  #endif
> > -#ifdef CONFIG_AXP209_POWER
> > +#ifdef CONFIG_AXP_ALDO4_VOLT
> >  	power_failed |= axp_set_aldo4(CONFIG_AXP_ALDO4_VOLT);
> >  #endif
> > 
> > -#if defined(CONFIG_AXP221_POWER) || defined(CONFIG_AXP809_POWER) || \
> > -	defined(CONFIG_AXP818_POWER)
> > +#ifdef CONFIG_AXP_DLDO1_VOLT
> >  	power_failed |= axp_set_dldo(1, CONFIG_AXP_DLDO1_VOLT);
> >  	power_failed |= axp_set_dldo(2, CONFIG_AXP_DLDO2_VOLT);
> > -#if !defined CONFIG_AXP809_POWER
> > +#endif
> > +#ifdef CONFIG_AXP_DLDO3_VOLT
> >  	power_failed |= axp_set_dldo(3, CONFIG_AXP_DLDO3_VOLT);
> >  	power_failed |= axp_set_dldo(4, CONFIG_AXP_DLDO4_VOLT);
> >  #endif
> > +#ifdef CONFIG_AXP_ELDO1_VOLT
> >  	power_failed |= axp_set_eldo(1, CONFIG_AXP_ELDO1_VOLT);
> >  	power_failed |= axp_set_eldo(2, CONFIG_AXP_ELDO2_VOLT);
> >  	power_failed |= axp_set_eldo(3, CONFIG_AXP_ELDO3_VOLT);
> >  #endif
> > 
> > -#ifdef CONFIG_AXP818_POWER
> > +#ifdef CONFIG_AXP_FLDO1_VOLT
> >  	power_failed |= axp_set_fldo(1, CONFIG_AXP_FLDO1_VOLT);
> >  	power_failed |= axp_set_fldo(2, CONFIG_AXP_FLDO2_VOLT);
> >  	power_failed |= axp_set_fldo(3, CONFIG_AXP_FLDO3_VOLT);
> > @@ -649,7 +645,7 @@ void sunxi_board_init(void)
> >  #if defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER
> >  	power_failed |= axp_set_sw(IS_ENABLED(CONFIG_AXP_SW_ON));
> >  #endif
> > -#endif
> > +#endif	/* CONFIG_AXPxxx_POWER */
> >  	printf("DRAM:");
> >  	gd->ram_size = sunxi_dram_init();
> >  	printf(" %d MiB\n", (int)(gd->ram_size >> 20));
> > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> > index 7f3b990d231..83cb31c937a 100644
> > --- a/drivers/power/Kconfig
> > +++ b/drivers/power/Kconfig
> > @@ -180,7 +180,7 @@ config AXP_DCDC3_VOLT
> > 
> >  config AXP_DCDC4_VOLT
> >  	int "axp pmic dcdc4 voltage"
> > -	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER ||   
> AXP818_POWER ||
> > AXP305_POWER +	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER   
> ||
> > AXP305_POWER default 1250 if AXP152_POWER
> >  	default 1200 if MACH_SUN6I
> >  	default 0 if MACH_SUN8I  
> 
> This patch is great, but this last change doesn't seem to be directly 
> connected?

It is an existing bug: there is no axp_set_dcdc4() for the AXP818, but
since the AXP818 is not included in the guards at the calling site
in board.c above, we didn't notice.
So without this change, all A83T boards (the only ones using AXP818)
would fail compilation.
I just forgot to mention it in the commit message, will add it.

Cheers,
Andre

Re: [PATCH 3/3] power: regulator: add AXP313 support

[Andre Przywara]

On Sat, 21 Oct 2023 08:51:09 +0200
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

> On Wednesday, October 18, 2023 5:50:14 PM CEST Andre Przywara wrote:
> > The X-Powers AXP313a is a small PMIC with just three buck converters and
> > three 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>
> > ---
> >  drivers/power/pmic/axp.c                |  1 +
> >  drivers/power/regulator/axp_regulator.c | 17 +++++++++++++++++
> >  include/axp_pmic.h                      |  1 +
> >  3 files changed, 19 insertions(+)
> > 
> > diff --git a/drivers/power/pmic/axp.c b/drivers/power/pmic/axp.c
> > index 025dac24f28..0e1e45fba74 100644
> > --- a/drivers/power/pmic/axp.c
> > +++ b/drivers/power/pmic/axp.c
> > @@ -87,6 +87,7 @@ static const struct udevice_id axp_pmic_ids[] = {
> >  	{ .compatible = "x-powers,axp209", .data = AXP209_ID },
> >  	{ .compatible = "x-powers,axp221", .data = AXP221_ID },
> >  	{ .compatible = "x-powers,axp223", .data = AXP223_ID },
> > +	{ .compatible = "x-powers,axp313a", .data = AXP313_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 02f320eac1e..d27e09538e0
> > 100644
> > --- a/drivers/power/regulator/axp_regulator.c
> > +++ b/drivers/power/regulator/axp_regulator.c
> > @@ -173,6 +173,22 @@ static const struct axp_regulator_plat
> > axp22x_regulators[] = { { }
> >  };
> > 
> > +/*
> > + * The "dcdc1" 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 ~1.1V anyway, so just ignore it.  
> 
> It can be modelled with look up table, as it's already done for some other 
> regulators?

Theoretically yes, but it would require about 105 entries, that's quite
a lot for something we don't need. As mentioned, it's
typically VDD_GPU/VDD_SYS, at 0.9V at reset, and no device used in
U-Boot directly references this regulator, so I think it wouldn't be
used anyways. I doubt that even with a user we ever would go beyond
1.54V. So for the sake of simplicity, I'd like to keep this cut to
those two ranges.

Thanks for having a look!

Cheers,
Andre

> 
> Best regards,
> Jernej
> 
> > + * Also the DCDC3 regulator is described wrongly in the (available) manual,
> > + * experiments show that the split point is at 1200mV, as for DCDC1/2. +
> > */
> > +static const struct axp_regulator_plat axp313_regulators[] = {
> > +	{ "dcdc1", 0x10, BIT(0), 0x13, 0x7f,  500, 1540,  10, 70 },
> > +	{ "dcdc2", 0x10, BIT(1), 0x14, 0x7f,  500, 1540,  10, 70 },
> > +	{ "dcdc3", 0x10, BIT(2), 0x15, 0x7f,  500, 1840,  10, 70 },
> > +	{ "aldo1", 0x10, BIT(3), 0x16, 0x1f,  500, 3500, 100, NA },
> > +	{ "dldo1", 0x10, BIT(4), 0x17, 0x1f,  500, 3500, 100, 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 },
> > @@ -274,6 +290,7 @@ static const struct axp_regulator_plat *const
> > axp_regulators[] = { [AXP209_ID]	= axp20x_regulators,
> >  	[AXP221_ID]	= axp22x_regulators,
> >  	[AXP223_ID]	= axp22x_regulators,
> > +	[AXP313_ID]	= axp313_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 4ac64865831..aabafc8501b 100644
> > --- a/include/axp_pmic.h
> > +++ b/include/axp_pmic.h
> > @@ -32,6 +32,7 @@ enum {
> >  	AXP209_ID,
> >  	AXP221_ID,
> >  	AXP223_ID,
> > +	AXP313_ID,
> >  	AXP803_ID,
> >  	AXP806_ID,
> >  	AXP809_ID,  
> 
> 
> 
> 
> 

Re: [PATCH 3/3] power: regulator: add AXP313 support

[Jaehoon Chung]

On 10/19/23 00:50, Andre Przywara wrote:
> The X-Powers AXP313a is a small PMIC with just three buck converters and
> three 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: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/power/pmic/axp.c                |  1 +
>  drivers/power/regulator/axp_regulator.c | 17 +++++++++++++++++
>  include/axp_pmic.h                      |  1 +
>  3 files changed, 19 insertions(+)
> 
> diff --git a/drivers/power/pmic/axp.c b/drivers/power/pmic/axp.c
> index 025dac24f28..0e1e45fba74 100644
> --- a/drivers/power/pmic/axp.c
> +++ b/drivers/power/pmic/axp.c
> @@ -87,6 +87,7 @@ static const struct udevice_id axp_pmic_ids[] = {
>  	{ .compatible = "x-powers,axp209", .data = AXP209_ID },
>  	{ .compatible = "x-powers,axp221", .data = AXP221_ID },
>  	{ .compatible = "x-powers,axp223", .data = AXP223_ID },
> +	{ .compatible = "x-powers,axp313a", .data = AXP313_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 02f320eac1e..d27e09538e0 100644
> --- a/drivers/power/regulator/axp_regulator.c
> +++ b/drivers/power/regulator/axp_regulator.c
> @@ -173,6 +173,22 @@ static const struct axp_regulator_plat axp22x_regulators[] = {
>  	{ }
>  };
>  
> +/*
> + * The "dcdc1" 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 ~1.1V anyway, so just ignore it.
> + * Also the DCDC3 regulator is described wrongly in the (available) manual,
> + * experiments show that the split point is at 1200mV, as for DCDC1/2.
> + */
> +static const struct axp_regulator_plat axp313_regulators[] = {
> +	{ "dcdc1", 0x10, BIT(0), 0x13, 0x7f,  500, 1540,  10, 70 },
> +	{ "dcdc2", 0x10, BIT(1), 0x14, 0x7f,  500, 1540,  10, 70 },
> +	{ "dcdc3", 0x10, BIT(2), 0x15, 0x7f,  500, 1840,  10, 70 },
> +	{ "aldo1", 0x10, BIT(3), 0x16, 0x1f,  500, 3500, 100, NA },
> +	{ "dldo1", 0x10, BIT(4), 0x17, 0x1f,  500, 3500, 100, 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 },
> @@ -274,6 +290,7 @@ static const struct axp_regulator_plat *const axp_regulators[] = {
>  	[AXP209_ID]	= axp20x_regulators,
>  	[AXP221_ID]	= axp22x_regulators,
>  	[AXP223_ID]	= axp22x_regulators,
> +	[AXP313_ID]	= axp313_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 4ac64865831..aabafc8501b 100644
> --- a/include/axp_pmic.h
> +++ b/include/axp_pmic.h
> @@ -32,6 +32,7 @@ enum {
>  	AXP209_ID,
>  	AXP221_ID,
>  	AXP223_ID,
> +	AXP313_ID,
>  	AXP803_ID,
>  	AXP806_ID,
>  	AXP809_ID,

RE: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Jaehoon Chung]

Hi,

> -----Original Message-----
> From: Andre Przywara <andre.przywara@arm.com>
> Sent: Sunday, October 22, 2023 6:19 AM
> To: Jernej Škrabec <jernej.skrabec@gmail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>; Jaehoon Chung <jh80.chung@samsung.com>; Samuel Holland
> <samuel@sholland.org>; SASANO Takayoshi <uaa@mx5.nisiq.net>; Mikhail Kalashnikov <iuncuim@gmail.com>;
> Piotr Oniszczuk <piotr.oniszczuk@gmail.com>; u-boot@lists.denx.de; linux-sunxi@lists.linux.dev
> Subject: Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions
> 
> On Sat, 21 Oct 2023 08:34:06 +0200
> Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> 
> > On Wednesday, October 18, 2023 5:50:12 PM CEST Andre Przywara wrote:
> > > So far we have a convoluted #ifdef mesh that guards the early AXP PMIC
> > > setup in board.c. That combination of &&, || and negations is very hard
> > > to read, maintain and especially to extend.
> > >
> > > Fortunately we have those same conditions already modelled in the
> > > Kconfig file, so they are actually redundant. On top of that the real
> > > reason we have those preprocessor guards in the first place is about the
> > > symbols that are *conditionally* defined: without #ifdefs the build
> > > would break because of them being undefined for many boards.
> > >
> > > To simplify this, just change the guards to actually look at the symbols
> > > needed, so CONFIG_AXP_xxx_VOLT instead of CONFIG_AXPyyy_POWER.
> > > This drastically improves the readability of this code, and makes adding
> > > PMIC support a pure Kconfig matter.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > ---
> > >  board/sunxi/board.c   | 32 ++++++++++++++------------------
> > >  drivers/power/Kconfig |  2 +-
> > >  2 files changed, 15 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index ebaa9431984..65d79a02c25 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -597,50 +597,46 @@ void sunxi_board_init(void)
> > >  		}
> > >  	}
> > >
> > > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > > -	defined CONFIG_AXP818_POWER
> > > +#ifdef CONFIG_AXP_DCDC1_VOLT
> > >  	power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT);
> > > +	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
> > >  #endif
> > > -#if !defined(CONFIG_AXP305_POWER)
> > > +#ifdef CONFIG_AXP_DCDC2_VOLT
> > >  	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)
> > > +#ifdef CONFIG_AXP_DCDC4_VOLT
> > >  	power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
> > >  #endif
> > > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > > -	defined CONFIG_AXP818_POWER
> > > -	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
> > > -#endif
> > >
> > > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > > -	defined CONFIG_AXP818_POWER
> > > +#ifdef CONFIG_AXP_ALDO1_VOLT
> > >  	power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT);
> > >  #endif
> > > -#if !defined(CONFIG_AXP305_POWER)
> > > +#ifdef CONFIG_AXP_ALDO2_VOLT
> > >  	power_failed |= axp_set_aldo2(CONFIG_AXP_ALDO2_VOLT);
> > >  #endif
> > > -#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER)
> > > +#ifdef CONFIG_AXP_ALDO3_VOLT
> > >  	power_failed |= axp_set_aldo3(CONFIG_AXP_ALDO3_VOLT);
> > >  #endif
> > > -#ifdef CONFIG_AXP209_POWER
> > > +#ifdef CONFIG_AXP_ALDO4_VOLT
> > >  	power_failed |= axp_set_aldo4(CONFIG_AXP_ALDO4_VOLT);
> > >  #endif
> > >
> > > -#if defined(CONFIG_AXP221_POWER) || defined(CONFIG_AXP809_POWER) || \
> > > -	defined(CONFIG_AXP818_POWER)
> > > +#ifdef CONFIG_AXP_DLDO1_VOLT
> > >  	power_failed |= axp_set_dldo(1, CONFIG_AXP_DLDO1_VOLT);
> > >  	power_failed |= axp_set_dldo(2, CONFIG_AXP_DLDO2_VOLT);
> > > -#if !defined CONFIG_AXP809_POWER
> > > +#endif
> > > +#ifdef CONFIG_AXP_DLDO3_VOLT
> > >  	power_failed |= axp_set_dldo(3, CONFIG_AXP_DLDO3_VOLT);
> > >  	power_failed |= axp_set_dldo(4, CONFIG_AXP_DLDO4_VOLT);
> > >  #endif
> > > +#ifdef CONFIG_AXP_ELDO1_VOLT
> > >  	power_failed |= axp_set_eldo(1, CONFIG_AXP_ELDO1_VOLT);
> > >  	power_failed |= axp_set_eldo(2, CONFIG_AXP_ELDO2_VOLT);
> > >  	power_failed |= axp_set_eldo(3, CONFIG_AXP_ELDO3_VOLT);
> > >  #endif
> > >
> > > -#ifdef CONFIG_AXP818_POWER
> > > +#ifdef CONFIG_AXP_FLDO1_VOLT
> > >  	power_failed |= axp_set_fldo(1, CONFIG_AXP_FLDO1_VOLT);
> > >  	power_failed |= axp_set_fldo(2, CONFIG_AXP_FLDO2_VOLT);
> > >  	power_failed |= axp_set_fldo(3, CONFIG_AXP_FLDO3_VOLT);
> > > @@ -649,7 +645,7 @@ void sunxi_board_init(void)
> > >  #if defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER
> > >  	power_failed |= axp_set_sw(IS_ENABLED(CONFIG_AXP_SW_ON));
> > >  #endif
> > > -#endif
> > > +#endif	/* CONFIG_AXPxxx_POWER */
> > >  	printf("DRAM:");
> > >  	gd->ram_size = sunxi_dram_init();
> > >  	printf(" %d MiB\n", (int)(gd->ram_size >> 20));
> > > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> > > index 7f3b990d231..83cb31c937a 100644
> > > --- a/drivers/power/Kconfig
> > > +++ b/drivers/power/Kconfig
> > > @@ -180,7 +180,7 @@ config AXP_DCDC3_VOLT
> > >
> > >  config AXP_DCDC4_VOLT
> > >  	int "axp pmic dcdc4 voltage"
> > > -	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER ||
> > AXP818_POWER ||
> > > AXP305_POWER +	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER
> > ||
> > > AXP305_POWER default 1250 if AXP152_POWER
> > >  	default 1200 if MACH_SUN6I
> > >  	default 0 if MACH_SUN8I
> >
> > This patch is great, but this last change doesn't seem to be directly
> > connected?
> 
> It is an existing bug: there is no axp_set_dcdc4() for the AXP818, but
> since the AXP818 is not included in the guards at the calling site
> in board.c above, we didn't notice.
> So without this change, all A83T boards (the only ones using AXP818)
> would fail compilation.
> I just forgot to mention it in the commit message, will add it.

Will you resend this patch after updating the commit message?

Best Regards,
Jaehoon Chung

> 
> Cheers,
> Andre

Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Andre Przywara]

On Tue, 31 Oct 2023 15:42:54 +0900
"Jaehoon Chung" <jh80.chung@samsung.com> wrote:

Hi Jaehoon,

> Hi,
> 
> > -----Original Message-----
> > From: Andre Przywara <andre.przywara@arm.com>
> > Sent: Sunday, October 22, 2023 6:19 AM
> > To: Jernej Škrabec <jernej.skrabec@gmail.com>
> > Cc: Jagan Teki <jagan@amarulasolutions.com>; Jaehoon Chung <jh80.chung@samsung.com>; Samuel Holland
> > <samuel@sholland.org>; SASANO Takayoshi <uaa@mx5.nisiq.net>; Mikhail Kalashnikov <iuncuim@gmail.com>;
> > Piotr Oniszczuk <piotr.oniszczuk@gmail.com>; u-boot@lists.denx.de; linux-sunxi@lists.linux.dev
> > Subject: Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions
> > 
> > On Sat, 21 Oct 2023 08:34:06 +0200
> > Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> >   
> > > On Wednesday, October 18, 2023 5:50:12 PM CEST Andre Przywara wrote:  
> > > > So far we have a convoluted #ifdef mesh that guards the early AXP PMIC
> > > > setup in board.c. That combination of &&, || and negations is very hard
> > > > to read, maintain and especially to extend.
> > > >
> > > > Fortunately we have those same conditions already modelled in the
> > > > Kconfig file, so they are actually redundant. On top of that the real
> > > > reason we have those preprocessor guards in the first place is about the
> > > > symbols that are *conditionally* defined: without #ifdefs the build
> > > > would break because of them being undefined for many boards.
> > > >
> > > > To simplify this, just change the guards to actually look at the symbols
> > > > needed, so CONFIG_AXP_xxx_VOLT instead of CONFIG_AXPyyy_POWER.
> > > > This drastically improves the readability of this code, and makes adding
> > > > PMIC support a pure Kconfig matter.
> > > >
> > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > > ---
> > > >  board/sunxi/board.c   | 32 ++++++++++++++------------------
> > > >  drivers/power/Kconfig |  2 +-
> > > >  2 files changed, 15 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > > index ebaa9431984..65d79a02c25 100644
> > > > --- a/board/sunxi/board.c
> > > > +++ b/board/sunxi/board.c
> > > > @@ -597,50 +597,46 @@ void sunxi_board_init(void)
> > > >  		}
> > > >  	}
> > > >
> > > > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > > > -	defined CONFIG_AXP818_POWER
> > > > +#ifdef CONFIG_AXP_DCDC1_VOLT
> > > >  	power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT);
> > > > +	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
> > > >  #endif
> > > > -#if !defined(CONFIG_AXP305_POWER)
> > > > +#ifdef CONFIG_AXP_DCDC2_VOLT
> > > >  	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)
> > > > +#ifdef CONFIG_AXP_DCDC4_VOLT
> > > >  	power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
> > > >  #endif
> > > > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > > > -	defined CONFIG_AXP818_POWER
> > > > -	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
> > > > -#endif
> > > >
> > > > -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
> > > > -	defined CONFIG_AXP818_POWER
> > > > +#ifdef CONFIG_AXP_ALDO1_VOLT
> > > >  	power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT);
> > > >  #endif
> > > > -#if !defined(CONFIG_AXP305_POWER)
> > > > +#ifdef CONFIG_AXP_ALDO2_VOLT
> > > >  	power_failed |= axp_set_aldo2(CONFIG_AXP_ALDO2_VOLT);
> > > >  #endif
> > > > -#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER)
> > > > +#ifdef CONFIG_AXP_ALDO3_VOLT
> > > >  	power_failed |= axp_set_aldo3(CONFIG_AXP_ALDO3_VOLT);
> > > >  #endif
> > > > -#ifdef CONFIG_AXP209_POWER
> > > > +#ifdef CONFIG_AXP_ALDO4_VOLT
> > > >  	power_failed |= axp_set_aldo4(CONFIG_AXP_ALDO4_VOLT);
> > > >  #endif
> > > >
> > > > -#if defined(CONFIG_AXP221_POWER) || defined(CONFIG_AXP809_POWER) || \
> > > > -	defined(CONFIG_AXP818_POWER)
> > > > +#ifdef CONFIG_AXP_DLDO1_VOLT
> > > >  	power_failed |= axp_set_dldo(1, CONFIG_AXP_DLDO1_VOLT);
> > > >  	power_failed |= axp_set_dldo(2, CONFIG_AXP_DLDO2_VOLT);
> > > > -#if !defined CONFIG_AXP809_POWER
> > > > +#endif
> > > > +#ifdef CONFIG_AXP_DLDO3_VOLT
> > > >  	power_failed |= axp_set_dldo(3, CONFIG_AXP_DLDO3_VOLT);
> > > >  	power_failed |= axp_set_dldo(4, CONFIG_AXP_DLDO4_VOLT);
> > > >  #endif
> > > > +#ifdef CONFIG_AXP_ELDO1_VOLT
> > > >  	power_failed |= axp_set_eldo(1, CONFIG_AXP_ELDO1_VOLT);
> > > >  	power_failed |= axp_set_eldo(2, CONFIG_AXP_ELDO2_VOLT);
> > > >  	power_failed |= axp_set_eldo(3, CONFIG_AXP_ELDO3_VOLT);
> > > >  #endif
> > > >
> > > > -#ifdef CONFIG_AXP818_POWER
> > > > +#ifdef CONFIG_AXP_FLDO1_VOLT
> > > >  	power_failed |= axp_set_fldo(1, CONFIG_AXP_FLDO1_VOLT);
> > > >  	power_failed |= axp_set_fldo(2, CONFIG_AXP_FLDO2_VOLT);
> > > >  	power_failed |= axp_set_fldo(3, CONFIG_AXP_FLDO3_VOLT);
> > > > @@ -649,7 +645,7 @@ void sunxi_board_init(void)
> > > >  #if defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER
> > > >  	power_failed |= axp_set_sw(IS_ENABLED(CONFIG_AXP_SW_ON));
> > > >  #endif
> > > > -#endif
> > > > +#endif	/* CONFIG_AXPxxx_POWER */
> > > >  	printf("DRAM:");
> > > >  	gd->ram_size = sunxi_dram_init();
> > > >  	printf(" %d MiB\n", (int)(gd->ram_size >> 20));
> > > > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> > > > index 7f3b990d231..83cb31c937a 100644
> > > > --- a/drivers/power/Kconfig
> > > > +++ b/drivers/power/Kconfig
> > > > @@ -180,7 +180,7 @@ config AXP_DCDC3_VOLT
> > > >
> > > >  config AXP_DCDC4_VOLT
> > > >  	int "axp pmic dcdc4 voltage"
> > > > -	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER ||  
> > > AXP818_POWER ||  
> > > > AXP305_POWER +	depends on AXP152_POWER || AXP221_POWER || AXP809_POWER  
> > > ||  
> > > > AXP305_POWER default 1250 if AXP152_POWER
> > > >  	default 1200 if MACH_SUN6I
> > > >  	default 0 if MACH_SUN8I  
> > >
> > > This patch is great, but this last change doesn't seem to be directly
> > > connected?  
> > 
> > It is an existing bug: there is no axp_set_dcdc4() for the AXP818, but
> > since the AXP818 is not included in the guards at the calling site
> > in board.c above, we didn't notice.
> > So without this change, all A83T boards (the only ones using AXP818)
> > would fail compilation.
> > I just forgot to mention it in the commit message, will add it.  
> 
> Will you resend this patch after updating the commit message?

Well, I was actually thinking of merging it straight away (with the
updated commit message), since I think that plus a tiny typo were the only
changes requested in that series.
Unless there is more from your side?

Cheers,
Andre

Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Leon Anavi]

Hi,

Commit ffb0294 from 12 November 2023 that simplifies early PMIC setup
conditions causes issues on Cubieboard 4 and Merrii A80 Optimus with
Allwinner A80 SoC (sun9i). The commit was introduced with U-Boot 2024.01
(rc3) and remains as of today. Because of it both of these boards hang at:

Starting kernel ...

Older U-Boot versions without this commit work fine. As a temporary
solution I reverted commit ffb0294 and this way the boards boot
successfully. I tested this work around on Merrii A80 Optimus with several
U-Boot versions, including with U-Boot 2024.10.

Lazar, a friend who owns Cubieboard 4, also tested and confirmed his board
boots with U-Boot 2024.10 if this commit has been reverted.

How to fix this? Is there a known configuration that can be added to
Merrii_A80_Optimus_defconfig and Cubieboard4_defconfig to avoid hanging
with the existing source code from commit ffb0294 ?

Best regards,
Leon

Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Andre Przywara]

On Mon, 9 Dec 2024 23:08:19 +0200
Leon Anavi <leon.anavi@konsulko.com> wrote:

Hi Leon,

thanks for the report!

> Commit ffb0294 from 12 November 2023 that simplifies early PMIC setup
> conditions causes issues on Cubieboard 4 and Merrii A80 Optimus with
> Allwinner A80 SoC (sun9i). The commit was introduced with U-Boot 2024.01
> (rc3) and remains as of today. Because of it both of these boards hang at:
> 
> Starting kernel ...

That's odd, how do you boot the kernel, exactly?
I just tried mainline U-Boot (via FEL), with:
=> setenv bootargs "console=ttyS0,115200n8 earlycon"
=> bootz $kernel_addr_r $ramdisk_addr_r:300000 $fdtcontroladdr

and it booted fine to the prompt, on a Cubieboard 4 (CC-A80 v1.2).
Kernel was some 6.11-rc6 I just had lying around.

I also compared the code before and after that patch, the only
difference is the order at which DCDC5 gets programmed: before it's
after DCDC4, with the patch it's right after DCDC1.
The rest looked the same.
Booting ffb0294~1 and ffb0294~0 also worked for me, without issues.
So can you please describe how you test that, exactly?

Please also note we fixed d75fa8c80dcfa in U-Boot (DCDC4/5 typo), and
dd36ad71ad6 in the kernel (DCDC5 constraints in the DT).

Cheers,
Andre

> Older U-Boot versions without this commit work fine. As a temporary
> solution I reverted commit ffb0294 and this way the boards boot
> successfully. I tested this work around on Merrii A80 Optimus with several
> U-Boot versions, including with U-Boot 2024.10.


> 
> Lazar, a friend who owns Cubieboard 4, also tested and confirmed his board
> boots with U-Boot 2024.10 if this commit has been reverted.
> 
> How to fix this? Is there a known configuration that can be added to
> Merrii_A80_Optimus_defconfig and Cubieboard4_defconfig to avoid hanging
> with the existing source code from commit ffb0294 ?
> 
> Best regards,
> Leon

Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Leon Anavi]

Hi Andre,

On 11.12.24 г. 23:53 ч., Andre Przywara wrote:
> On Mon, 9 Dec 2024 23:08:19 +0200
> Leon Anavi<leon.anavi@konsulko.com> wrote:
>
> Hi Leon,
>
> thanks for the report!
>
>> Commit ffb0294 from 12 November 2023 that simplifies early PMIC setup
>> conditions causes issues on Cubieboard 4 and Merrii A80 Optimus with
>> Allwinner A80 SoC (sun9i). The commit was introduced with U-Boot 2024.01
>> (rc3) and remains as of today. Because of it both of these boards hang at:
>>
>> Starting kernel ...
> That's odd, how do you boot the kernel, exactly?
> I just tried mainline U-Boot (via FEL), with:
> => setenv bootargs "console=ttyS0,115200n8 earlycon"
> => bootz $kernel_addr_r $ramdisk_addr_r:300000 $fdtcontroladdr
>
> and it booted fine to the prompt, on a Cubieboard 4 (CC-A80 v1.2).
> Kernel was some 6.11-rc6 I just had lying around.
>
> I also compared the code before and after that patch, the only
> difference is the order at which DCDC5 gets programmed: before it's
> after DCDC4, with the patch it's right after DCDC1.
> The rest looked the same.
> Booting ffb0294~1 and ffb0294~0 also worked for me, without issues.
> So can you please describe how you test that, exactly?
I built core-image-base using the Yocto Project and OpenEmbedded as well 
as the meta-sunxi BSP layer. The U-Boot version depends on the Yocto 
release. For Scarthgap it is with U-Boot 2024.01 and for the ongoing 
development in the master branches the U-Boot version is 2024.10. The 
Linux kernel version is 6.6.28. I experienced the same issue with both 
U-Boot versions on Merrii A80 Optimus and Lazar (his email is CC) 
confirmed the same results on Cubieboard 4. The boot sequence for the 
boards in meta-sunxi is through boot.scr generated from the following 
boot.cmd:
https://github.com/linux-sunxi/meta-sunxi/blob/master/recipes-bsp/u-boot/files/arm/boot.cmd

In a nutshell, the issue can be reproduced with the following scenario 
in U-Boot:

setenv bootargs console=ttyS0,115200 console=tty1 root=/dev/mmcblk0p2 
rootwait panic=10
load mmc 0:1 ${fdt_addr_r} ${fdtfile}
load mmc 0:1 ${kernel_addr_r} zImage
bootz ${kernel_addr_r} - ${fdt_addr_r}

Merrii A80 Optimus board boots fine if I revert commit ffb0294. However, 
if commit ffb0294 is present in U-Boot the board hangs. Here is the log: 
U-Boot 2024.10-dirty (Oct 07 2024 - 14:54:35 +0000) Allwinner Technology 
CPU: Allwinner A80 (SUN9I) Model: Merrii A80 Optimus Board DRAM: 2 GiB 
Core: 62 devices, 18 uclasses, devicetree: separate WDT: Not starting 
watchdog@6000ca0 WDT: Not starting watchdog@8001000 MMC: 
sunxi_set_reset: (RST#0) unhandled sunxi_set_reset: (RST#2) unhandled 
sunxi_set_reset: (RST#1) unhandled mmc@1c0f000: 0, mmc@1c10000: 2, 
mmc@1c11000: 1 Loading Environment from FAT... Unable to read 
"uboot.env" from mmc0:1... In: serial@7000000 Out: serial@7000000 Err: 
serial@7000000 Net: No ethernet found. Hit any key to stop autoboot: 0 
=> setenv bootargs console=ttyS0,115200 console=tty1 root=/dev/mmcblk0p2 
rootwait panic=10 => load mmc 0:1 ${fdt_addr_r} ${fdtfile} 24443 bytes 
read in 3 ms (7.8 MiB/s) => load mmc 0:1 ${kernel_addr_r} zImage 5397992 
bytes read in 236 ms (21.8 MiB/s) => bootz ${kernel_addr_r} - 
${fdt_addr_r} Kernel image @ 0x22000000 [ 0x000000 - 0x525de8 ] ## 
Flattened Device Tree blob at 23000000 Booting using the fdt blob at 
0x23000000 Working FDT set to 23000000 Loading Device Tree to 29ff7000, 
end 29ffff7a ... OK Working FDT set to 29ff7000 Starting kernel ... 
Please note that U-Boot is marked with suffix "-dirty" because of the 
patches applied by u-boot_%.bbappend from Yocto/OE layer meta-sunxi but 
these patches are for other boards and don't touch the file 
board/sunxi/board.c. Am I missing something as a configuration that can 
make the board boot the same way without hanging when commit ffb0294 is 
present?Best regards,
Leon

>
> Please also note we fixed d75fa8c80dcfa in U-Boot (DCDC4/5 typo), and
> dd36ad71ad6 in the kernel (DCDC5 constraints in the DT).
>
> Cheers,
> Andre
>
>> Older U-Boot versions without this commit work fine. As a temporary
>> solution I reverted commit ffb0294 and this way the boards boot
>> successfully. I tested this work around on Merrii A80 Optimus with several
>> U-Boot versions, including with U-Boot 2024.10.
>
>> Lazar, a friend who owns Cubieboard 4, also tested and confirmed his board
>> boots with U-Boot 2024.10 if this commit has been reverted.
>>
>> How to fix this? Is there a known configuration that can be added to
>> Merrii_A80_Optimus_defconfig and Cubieboard4_defconfig to avoid hanging
>> with the existing source code from commit ffb0294 ?
>>
>> Best regards,
>> Leon

Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Andre Przywara]

On Thu, 12 Dec 2024 11:19:06 +0200
Leon Anavi <leon.anavi@konsulko.com> wrote:

Hi Leon,

> On 11.12.24 г. 23:53 ч., Andre Przywara wrote:
> > On Mon, 9 Dec 2024 23:08:19 +0200
> > Leon Anavi<leon.anavi@konsulko.com> wrote:
> >
> > Hi Leon,
> >
> > thanks for the report!
> >  
> >> Commit ffb0294 from 12 November 2023 that simplifies early PMIC setup
> >> conditions causes issues on Cubieboard 4 and Merrii A80 Optimus with
> >> Allwinner A80 SoC (sun9i). The commit was introduced with U-Boot 2024.01
> >> (rc3) and remains as of today. Because of it both of these boards hang at:
> >>
> >> Starting kernel ...  
> > That's odd, how do you boot the kernel, exactly?
> > I just tried mainline U-Boot (via FEL), with:  
> > => setenv bootargs "console=ttyS0,115200n8 earlycon"
> > => bootz $kernel_addr_r $ramdisk_addr_r:300000 $fdtcontroladdr  
> >
> > and it booted fine to the prompt, on a Cubieboard 4 (CC-A80 v1.2).
> > Kernel was some 6.11-rc6 I just had lying around.
> >
> > I also compared the code before and after that patch, the only
> > difference is the order at which DCDC5 gets programmed: before it's
> > after DCDC4, with the patch it's right after DCDC1.
> > The rest looked the same.
> > Booting ffb0294~1 and ffb0294~0 also worked for me, without issues.
> > So can you please describe how you test that, exactly?  
> I built core-image-base using the Yocto Project and OpenEmbedded as well Starting kernel ...
> as the meta-sunxi BSP layer. The U-Boot version depends on the Yocto 
> release. For Scarthgap it is with U-Boot 2024.01 and for the ongoing 
> development in the master branches the U-Boot version is 2024.10. The 
> Linux kernel version is 6.6.28. I experienced the same issue with both 
> U-Boot versions on Merrii A80 Optimus and Lazar (his email is CC) 
> confirmed the same results on Cubieboard 4. The boot sequence for the 
> boards in meta-sunxi is through boot.scr generated from the following 
> boot.cmd:
> https://github.com/linux-sunxi/meta-sunxi/blob/master/recipes-bsp/u-boot/files/arm/boot.cmd
> 
> In a nutshell, the issue can be reproduced with the following scenario 
> in U-Boot:
> 
> setenv bootargs console=ttyS0,115200 console=tty1 root=/dev/mmcblk0p2 
> rootwait panic=10

So I tried with an SD card now, and I think I see what you mean now.
I was a bit confused first why it would hang for you after "Starting
kernel ...", but I guess it actually doesn't: can you add "earlycon" to your
command line and confirm that the kernel boots, but hangs while waiting
for the secondary cores?
I am still scratching my head how this commit would affect this
behaviour, but this change here seems to fix it for me:

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 29a329f41a2..d7c46eec615 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -582,7 +582,6 @@ void sunxi_board_init(void)
 
 #ifdef CONFIG_AXP_DCDC1_VOLT
 	power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT);
-	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
 #endif
 #ifdef CONFIG_AXP_DCDC2_VOLT
 	power_failed |= axp_set_dcdc2(CONFIG_AXP_DCDC2_VOLT);
@@ -591,6 +590,9 @@ void sunxi_board_init(void)
 #ifdef CONFIG_AXP_DCDC4_VOLT
 	power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
 #endif
+#ifdef CONFIG_AXP_DCDC5_VOLT
+	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
+#endif
 
 #ifdef CONFIG_AXP_ALDO1_VOLT
 	power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT);

Can you confirm this?
This fix looks easily mainline-able, as it just restores the old
initialisation order, but I still would like to know why the other
order hangs. Will try to do some experiments.

Cheers,
Andre


> load mmc 0:1 ${fdt_addr_r} ${fdtfile}
> load mmc 0:1 ${kernel_addr_r} zImage
> bootz ${kernel_addr_r} - ${fdt_addr_r}
> 
> Merrii A80 Optimus board boots fine if I revert commit ffb0294.
> However, if commit ffb0294 is present in U-Boot the board hangs. Here
> is the log: U-Boot 2024.10-dirty (Oct 07 2024 - 14:54:35 +0000)
> Allwinner Technology CPU: Allwinner A80 (SUN9I) Model: Merrii A80
> Optimus Board DRAM: 2 GiB Core: 62 devices, 18 uclasses, devicetree:
> separate WDT: Not starting watchdog@6000ca0 WDT: Not starting
> watchdog@8001000 MMC: sunxi_set_reset: (RST#0) unhandled
> sunxi_set_reset: (RST#2) unhandled sunxi_set_reset: (RST#1) unhandled
> mmc@1c0f000: 0, mmc@1c10000: 2, mmc@1c11000: 1 Loading Environment
> from FAT... Unable to read "uboot.env" from mmc0:1... In:
> serial@7000000 Out: serial@7000000 Err: serial@7000000 Net: No
> ethernet found. Hit any key to stop autoboot: 0 => setenv bootargs
> console=ttyS0,115200 console=tty1 root=/dev/mmcblk0p2 rootwait
> panic=10 => load mmc 0:1 ${fdt_addr_r} ${fdtfile} 24443 bytes read in
> 3 ms (7.8 MiB/s) => load mmc 0:1 ${kernel_addr_r} zImage 5397992
> bytes read in 236 ms (21.8 MiB/s) => bootz ${kernel_addr_r} -
> ${fdt_addr_r} Kernel image @ 0x22000000 [ 0x000000 - 0x525de8 ] ##
> Flattened Device Tree blob at 23000000 Booting using the fdt blob at
> 0x23000000 Working FDT set to 23000000 Loading Device Tree to
> 29ff7000, end 29ffff7a ... OK Working FDT set to 29ff7000 Starting
> kernel ... Please note that U-Boot is marked with suffix "-dirty"
> because of the patches applied by u-boot_%.bbappend from Yocto/OE
> layer meta-sunxi but these patches are for other boards and don't
> touch the file board/sunxi/board.c. Am I missing something as a
> configuration that can make the board boot the same way without
> hanging when commit ffb0294 is present?Best regards, Leon
> 
> >
> > Please also note we fixed d75fa8c80dcfa in U-Boot (DCDC4/5 typo),
> > and dd36ad71ad6 in the kernel (DCDC5 constraints in the DT).
> >
> > Cheers,
> > Andre
> >  
> >> Older U-Boot versions without this commit work fine. As a temporary
> >> solution I reverted commit ffb0294 and this way the boards boot
> >> successfully. I tested this work around on Merrii A80 Optimus with
> >> several U-Boot versions, including with U-Boot 2024.10.  
> >  
> >> Lazar, a friend who owns Cubieboard 4, also tested and confirmed
> >> his board boots with U-Boot 2024.10 if this commit has been
> >> reverted.
> >>
> >> How to fix this? Is there a known configuration that can be added
> >> to Merrii_A80_Optimus_defconfig and Cubieboard4_defconfig to avoid
> >> hanging with the existing source code from commit ffb0294 ?
> >>
> >> Best regards,
> >> Leon  

Re: [PATCH 1/3] sunxi: board: simplify early PMIC setup conditions

[Leon Anavi]

Hi Andre,

On 14.12.24 г. 4:19 ч., Andre Przywara wrote:
> On Thu, 12 Dec 2024 11:19:06 +0200
> Leon Anavi<leon.anavi@konsulko.com> wrote:
>
> Hi Leon,
>
>> On 11.12.24 г. 23:53 ч., Andre Przywara wrote:
>>> On Mon, 9 Dec 2024 23:08:19 +0200
>>> Leon Anavi<leon.anavi@konsulko.com> wrote:
>>>
>>> Hi Leon,
>>>
>>> thanks for the report!
>>>   
>>>> Commit ffb0294 from 12 November 2023 that simplifies early PMIC setup
>>>> conditions causes issues on Cubieboard 4 and Merrii A80 Optimus with
>>>> Allwinner A80 SoC (sun9i). The commit was introduced with U-Boot 2024.01
>>>> (rc3) and remains as of today. Because of it both of these boards hang at:
>>>>
>>>> Starting kernel ...
>>> That's odd, how do you boot the kernel, exactly?
>>> I just tried mainline U-Boot (via FEL), with:
>>> => setenv bootargs "console=ttyS0,115200n8 earlycon"
>>> => bootz $kernel_addr_r $ramdisk_addr_r:300000 $fdtcontroladdr
>>>
>>> and it booted fine to the prompt, on a Cubieboard 4 (CC-A80 v1.2).
>>> Kernel was some 6.11-rc6 I just had lying around.
>>>
>>> I also compared the code before and after that patch, the only
>>> difference is the order at which DCDC5 gets programmed: before it's
>>> after DCDC4, with the patch it's right after DCDC1.
>>> The rest looked the same.
>>> Booting ffb0294~1 and ffb0294~0 also worked for me, without issues.
>>> So can you please describe how you test that, exactly?
>> I built core-image-base using the Yocto Project and OpenEmbedded as well Starting kernel ...
>> as the meta-sunxi BSP layer. The U-Boot version depends on the Yocto
>> release. For Scarthgap it is with U-Boot 2024.01 and for the ongoing
>> development in the master branches the U-Boot version is 2024.10. The
>> Linux kernel version is 6.6.28. I experienced the same issue with both
>> U-Boot versions on Merrii A80 Optimus and Lazar (his email is CC)
>> confirmed the same results on Cubieboard 4. The boot sequence for the
>> boards in meta-sunxi is through boot.scr generated from the following
>> boot.cmd:
>> https://github.com/linux-sunxi/meta-sunxi/blob/master/recipes-bsp/u-boot/files/arm/boot.cmd
>>
>> In a nutshell, the issue can be reproduced with the following scenario
>> in U-Boot:
>>
>> setenv bootargs console=ttyS0,115200 console=tty1 root=/dev/mmcblk0p2
>> rootwait panic=10
> So I tried with an SD card now, and I think I see what you mean now.
> I was a bit confused first why it would hang for you after "Starting
> kernel ...", but I guess it actually doesn't: can you add "earlycon" to your
> command line and confirm that the kernel boots, but hangs while waiting
> for the secondary cores?
> I am still scratching my head how this commit would affect this
> behaviour, but this change here seems to fix it for me:
>
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 29a329f41a2..d7c46eec615 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -582,7 +582,6 @@ void sunxi_board_init(void)
>   
>   #ifdef CONFIG_AXP_DCDC1_VOLT
>   	power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT);
> -	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
>   #endif
>   #ifdef CONFIG_AXP_DCDC2_VOLT
>   	power_failed |= axp_set_dcdc2(CONFIG_AXP_DCDC2_VOLT);
> @@ -591,6 +590,9 @@ void sunxi_board_init(void)
>   #ifdef CONFIG_AXP_DCDC4_VOLT
>   	power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
>   #endif
> +#ifdef CONFIG_AXP_DCDC5_VOLT
> +	power_failed |= axp_set_dcdc5(CONFIG_AXP_DCDC5_VOLT);
> +#endif
>   
>   #ifdef CONFIG_AXP_ALDO1_VOLT
>   	power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT);
>
> Can you confirm this?


Yes, thank you. Excellent work! I confirm this solution works. I have 
just tested it on Merrii A80 Optimus board. I applied your patch on top 
of U-Boot 2024.10.

> This fix looks easily mainline-able, as it just restores the old
> initialisation order, but I still would like to know why the other
> order hangs. Will try to do some experiments.

OK. In the mean time I will contribute your patch to meta-sunxi BSP 
layer so that it can be used for the existing U-Boot versions in images 
for sun9i machines built using the Yocto project and OpenEmbedded. This 
is obviously a far better solution than reverting commit ffb0294.
Thanks,
Leon

>
> Cheers,
> Andre
>
>
>> load mmc 0:1 ${fdt_addr_r} ${fdtfile}
>> load mmc 0:1 ${kernel_addr_r} zImage
>> bootz ${kernel_addr_r} - ${fdt_addr_r}
>>
>> Merrii A80 Optimus board boots fine if I revert commit ffb0294.
>> However, if commit ffb0294 is present in U-Boot the board hangs. Here
>> is the log: U-Boot 2024.10-dirty (Oct 07 2024 - 14:54:35 +0000)
>> Allwinner Technology CPU: Allwinner A80 (SUN9I) Model: Merrii A80
>> Optimus Board DRAM: 2 GiB Core: 62 devices, 18 uclasses, devicetree:
>> separate WDT: Not starting watchdog@6000ca0 WDT: Not starting
>> watchdog@8001000 MMC: sunxi_set_reset: (RST#0) unhandled
>> sunxi_set_reset: (RST#2) unhandled sunxi_set_reset: (RST#1) unhandled
>> mmc@1c0f000: 0, mmc@1c10000: 2, mmc@1c11000: 1 Loading Environment
>> from FAT... Unable to read "uboot.env" frommmc0:1... In:
>> serial@7000000 Out: serial@7000000 Err: serial@7000000 Net: No
>> ethernet found. Hit any key to stop autoboot: 0 => setenv bootargs
>> console=ttyS0,115200 console=tty1 root=/dev/mmcblk0p2 rootwait
>> panic=10 => load mmc 0:1 ${fdt_addr_r} ${fdtfile} 24443 bytes read in
>> 3 ms (7.8 MiB/s) => load mmc 0:1 ${kernel_addr_r} zImage 5397992
>> bytes read in 236 ms (21.8 MiB/s) => bootz ${kernel_addr_r} -
>> ${fdt_addr_r} Kernel image @ 0x22000000 [ 0x000000 - 0x525de8 ] ##
>> Flattened Device Tree blob at 23000000 Booting using the fdt blob at
>> 0x23000000 Working FDT set to 23000000 Loading Device Tree to
>> 29ff7000, end 29ffff7a ... OK Working FDT set to 29ff7000 Starting
>> kernel ... Please note that U-Boot is marked with suffix "-dirty"
>> because of the patches applied by u-boot_%.bbappend from Yocto/OE
>> layer meta-sunxi but these patches are for other boards and don't
>> touch the file board/sunxi/board.c. Am I missing something as a
>> configuration that can make the board boot the same way without
>> hanging when commit ffb0294 is present?Best regards, Leon
>>
>>> Please also note we fixed d75fa8c80dcfa in U-Boot (DCDC4/5 typo),
>>> and dd36ad71ad6 in the kernel (DCDC5 constraints in the DT).
>>>
>>> Cheers,
>>> Andre
>>>   
>>>> Older U-Boot versions without this commit work fine. As a temporary
>>>> solution I reverted commit ffb0294 and this way the boards boot
>>>> successfully. I tested this work around on Merrii A80 Optimus with
>>>> several U-Boot versions, including with U-Boot 2024.10.
>>>   
>>>> Lazar, a friend who owns Cubieboard 4, also tested and confirmed
>>>> his board boots with U-Boot 2024.10 if this commit has been
>>>> reverted.
>>>>
>>>> How to fix this? Is there a known configuration that can be added
>>>> to Merrii_A80_Optimus_defconfig and Cubieboard4_defconfig to avoid
>>>> hanging with the existing source code from commit ffb0294 ?
>>>>
>>>> Best regards,
>>>> Leon