diff mbox series

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

Message ID 20210103092633.36226-2-jernej.skrabec@siol.net
State Superseded
Delegated to: Andre Przywara
Headers show
Series sunxi: Introduce H616 support | expand

Commit Message

Jernej Škrabec Jan. 3, 2021, 9:26 a.m. UTC 
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>
---
 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

Comments

Jaehoon Chung Jan. 5, 2021, 10:36 p.m. UTC | #1 
Hi Jernej

On 1/3/21 6:26 PM, Jernej Skrabec wrote:
> This PMIC can be found on H616 boards and it's very similar to AXP805
> and AXP806.

Is there any plan to cleanup codes?

> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  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
>  
>  #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
> 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
> + * (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;

Initialized ret value to 0.

> +	u8 cfg;
> +
> +	if (mvolt >= 1600)
> +		cfg = 46 + axp305_mvolt_to_cfg(mvolt, 1600, 3300, 100);

Could you use macro instead of 46 (magic code)?

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

Ditto, use macro instead of 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
>
Andre Przywara Jan. 6, 2021, 10:11 a.m. UTC | #2 
On 05/01/2021 22:36, Jaehoon Chung wrote:

Hi,

thanks for having a look!

> Hi Jernej
> 
> On 1/3/21 6:26 PM, Jernej Skrabec wrote:
>> This PMIC can be found on H616 boards and it's very similar to AXP805
>> and AXP806.
> 
> Is there any plan to cleanup codes?

There is no support for either of these, we use the PMICs only in
Trusted Firmware or Linux. So nothing to consolidate, yet. This might
change in the future, and then of course we will use a common code base.

> 
>>
>> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>> ---
>>  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
>>  
>>  #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
>> 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
>> + * (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;
> 
> Initialized ret value to 0.

Why? We set it unconditionally below, so initialising it here would be
pointless and might actually be warned upon by astute compilers.

> 
>> +	u8 cfg;
>> +
>> +	if (mvolt >= 1600)
>> +		cfg = 46 + axp305_mvolt_to_cfg(mvolt, 1600, 3300, 100);
> 
> Could you use macro instead of 46 (magic code)?
> 
>> +	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)
> 
> Ditto, use macro instead of 0x40. 

Not sure a macro is too useful here. I think the statement is pretty
clear in that this must be the chip version, which is given as this
value in the manual.

Cheers,
Andre

>> +		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
>>
>
Jaehoon Chung Jan. 6, 2021, 11:33 p.m. UTC | #3 
On 1/6/21 7:11 PM, André Przywara wrote:
> On 05/01/2021 22:36, Jaehoon Chung wrote:
> 
> Hi,
> 
> thanks for having a look!
> 
>> Hi Jernej
>>
>> On 1/3/21 6:26 PM, Jernej Skrabec wrote:
>>> This PMIC can be found on H616 boards and it's very similar to AXP805
>>> and AXP806.
>>
>> Is there any plan to cleanup codes?
> 
> There is no support for either of these, we use the PMICs only in
> Trusted Firmware or Linux. So nothing to consolidate, yet. This might
> change in the future, and then of course we will use a common code base.
> 
>>
>>>
>>> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>>> ---
>>>  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
>>>  
>>>  #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
>>> 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
>>> + * (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;
>>
>> Initialized ret value to 0.
> 
> Why? We set it unconditionally below, so initialising it here would be
> pointless and might actually be warned upon by astute compilers.

Sorry. It's my misunderstanding. you're right.

> 
>>
>>> +	u8 cfg;
>>> +
>>> +	if (mvolt >= 1600)
>>> +		cfg = 46 + axp305_mvolt_to_cfg(mvolt, 1600, 3300, 100);
>>
>> Could you use macro instead of 46 (magic code)?
>>
>>> +	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)
>>
>> Ditto, use macro instead of 0x40. 
> 
> Not sure a macro is too useful here. I think the statement is pretty
> clear in that this must be the chip version, which is given as this
> value in the manual.

0x40 is chip version? When i saw 0x40, i thought it mean some bit values.
Thanks for kindly explanation. 

Best Regards,
Jaehoon Chung

> 
> Cheers,
> Andre
> 
>>> +		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
>>>
>>
> 
>
Andre Przywara Jan. 7, 2021, 9:49 a.m. UTC | #4 
On 06/01/2021 23:33, Jaehoon Chung wrote:
> On 1/6/21 7:11 PM, André Przywara wrote:
>> On 05/01/2021 22:36, Jaehoon Chung wrote:
>>
>> Hi,
>>
>> thanks for having a look!
>>
>>> Hi Jernej
>>>
>>> On 1/3/21 6:26 PM, Jernej Skrabec wrote:
>>>> This PMIC can be found on H616 boards and it's very similar to AXP805
>>>> and AXP806.
>>>
>>> Is there any plan to cleanup codes?
>>
>> There is no support for either of these, we use the PMICs only in
>> Trusted Firmware or Linux. So nothing to consolidate, yet. This might
>> change in the future, and then of course we will use a common code base.
>>
>>>
>>>>
>>>> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>>>> ---
>>>>  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
>>>>  
>>>>  #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
>>>> 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
>>>> + * (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;
>>>
>>> Initialized ret value to 0.
>>
>> Why? We set it unconditionally below, so initialising it here would be
>> pointless and might actually be warned upon by astute compilers.
> 
> Sorry. It's my misunderstanding. you're right.
> 
>>
>>>
>>>> +	u8 cfg;
>>>> +
>>>> +	if (mvolt >= 1600)
>>>> +		cfg = 46 + axp305_mvolt_to_cfg(mvolt, 1600, 3300, 100);
>>>
>>> Could you use macro instead of 46 (magic code)?
>>>
>>>> +	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)
>>>
>>> Ditto, use macro instead of 0x40. 
>>
>> Not sure a macro is too useful here. I think the statement is pretty
>> clear in that this must be the chip version, which is given as this
>> value in the manual.
> 
> 0x40 is chip version? When i saw 0x40, i thought it mean some bit values.
> Thanks for kindly explanation. 

Well, it's both, welcome to Allwinner :-D
This is what the manual says:
bits 7-6 & 3-0: IC Type NO.
		010000: IC is AXP305
		Others: Reserved

So it's a mess, and that number is not really unique (the AXP805 and
AXP806 also use the same bits), but it's just a safety measure to avoid
talking to the wrong device.

Cheers,
Andre

>>
>> Cheers,
>> Andre
>>
>>>> +		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
>>>>
>>>
>>
>>
>
Andre Przywara Jan. 11, 2021, 12:02 a.m. UTC | #5 
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.

> 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
>
Jaehoon Chung Jan. 11, 2021, 1:32 a.m. UTC | #6 
On 1/11/21 9:02 AM, André Przywara wrote:
> 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".

Agreed. it seems that it needs to make more time about refactoring whole codes. 


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

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> 
> 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.
> 
>> 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://protect2.fireeye.com/v1/url?k=d54639fd-8add0072-d547b2b2-0cc47a31307c-01fd70f5628c90cc&q=1&e=594dac17-d975-498f-b06b-149370c2c809&u=https%3A%2F%2Fgit.trustedfirmware.org%2FTF-A%2Ftrusted-firmware-a.git%2Ftree%2Fdrivers%2Fallwinner%2Faxp%2Faxp803.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
>>
> 
>
Jernej Škrabec Jan. 11, 2021, 7:48 p.m. UTC | #7 
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
> > 
> 
>
diff mbox series

Patch

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
 
 #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
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
+ * (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