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.przyw...@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
