[PATCH 01/17] sunxi: Add support for AXP305 PMIC

["Jernej Škrabec" <jernej.skrabec@siol.net>]

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
> > 
> 
>