[U-Boot] [PATCH] sunxi: power: add support for sy8106a driver

Siarhei Siamashka siarhei.siamashka at gmail.com
Wed Feb 10 11:31:43 CET 2016


Hello,

Thanks for the patch. I can confirm that this patch sets the
voltage correctly.

On Tue,  9 Feb 2016 22:42:27 +0100
Hans de Goede <hdegoede at redhat.com> wrote:

> From: Jelle van der Waa <jelle at vdwaa.nl>
> 
> SY8106A is a PMIC which is used on the Allwinner
> H3 Orange Pi Pc board. The VOUT1_SEL register is
> implemented to set thea default V-CPU voltage to 1200 mV.

"thea" -> "the" typo

Also maybe explain in the commit message that the SY8106A
setup is retained after software reset and that's why this
patch is needed. On cold boot the default SY8106A output
voltage is selected by a pair of resistors and happens
to be 1.2V on the Orange Pi PC and Orange Pi Plus boards.

> 
> Signed-off-by: Jelle van der Waa <jelle at vdwaa.nl>
> [hdegoede at redhat.com: Polish Kconfig parts a bit]
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  board/sunxi/Kconfig            |  6 +++++-
>  board/sunxi/board.c            |  5 +++++
>  configs/orangepi_pc_defconfig  |  1 -
>  drivers/power/Kconfig          | 18 +++++++++++++++++-
>  drivers/power/Makefile         |  1 +
>  drivers/power/sy8106a.c        | 23 +++++++++++++++++++++++
>  include/configs/sunxi-common.h |  6 ++++--
>  include/sy8106a.h              |  5 +++++
>  8 files changed, 60 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/power/sy8106a.c
>  create mode 100644 include/sy8106a.h
> 
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index a334aa3..449d9dd 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -372,11 +372,15 @@ config I2C3_ENABLE
>  	See I2C0_ENABLE help text.
>  endif
>  
> +if SUNXI_GEN_SUN6I
>  config R_I2C_ENABLE
>  	bool "Enable the PRCM I2C/TWI controller"
> -	default n
> +	# This is used for the pmic on H3
> +	default y if MACH_SUN8I_H3
> +	default n if !MACH_SUN8I_H3

Maybe depend on SY8106A_POWER ?

>  	---help---
>  	Set this to y to enable the I2C controller which is part of the PRCM.
> +endif
>  
>  if MACH_SUN7I
>  config I2C4_ENABLE
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 420481a..15b7af6 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -25,6 +25,7 @@
>  #include <asm/io.h>
>  #include <nand.h>
>  #include <net.h>
> +#include <sy8106a.h>
>  
>  #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
>  /* So that we can use pin names in Kconfig and sunxi_name_to_gpio() */
> @@ -436,6 +437,10 @@ void sunxi_board_init(void)
>  	int power_failed = 0;
>  	unsigned long ramsize;
>  
> +#ifdef CONFIG_SY8106A_POWER
> +	power_failed = sy8106a_set_vout1(CONFIG_SY8106A_VOUT1_VOLT);
> +#endif
> +
>  #if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \
>  	defined CONFIG_AXP221_POWER || defined CONFIG_AXP818_POWER
>  	power_failed = axp_init();
> diff --git a/configs/orangepi_pc_defconfig b/configs/orangepi_pc_defconfig
> index ea9ed87..358caa5 100644
> --- a/configs/orangepi_pc_defconfig
> +++ b/configs/orangepi_pc_defconfig
> @@ -12,4 +12,3 @@ CONFIG_SPL=y
>  # CONFIG_CMD_FLASH is not set
>  # CONFIG_CMD_FPGA is not set
>  CONFIG_CMD_GPIO=y
> -CONFIG_R_I2C_ENABLE=y
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 10683a2..7fcf78e 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -10,7 +10,7 @@ choice
>  	default AXP209_POWER if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
>  	default AXP221_POWER if MACH_SUN6I || MACH_SUN8I_A23 || MACH_SUN8I_A33
>  	default AXP818_POWER if MACH_SUN8I_A83T
> -	default SUNXI_NO_PMIC if MACH_SUN8I_H3
> +	default SY8106A_POWER if MACH_SUN8I_H3
>  
>  config SUNXI_NO_PMIC
>  	boolean "board without a pmic"
> @@ -48,6 +48,13 @@ config AXP818_POWER
>  	Say y here to enable support for the axp818 pmic found on
>  	A83T dev board.
>  
> +config SY8106A_POWER
> +	boolean "SY8106A pmic support"
> +	depends on MACH_SUN8I_H3
> +	---help---
> +	Select this to enable support for the SY8106A pmic found on most
> +	H3 boards.

I'm assuming that this "most H3 boards" statement is based on having a
sample set of two sibling Orange Pi boards (PC and Plus flavours)?

But there is also the H3-OLinuXino-NANO board being developed by Olimex
and also the Orange Pi One board already on sale. These boards do not
use SY8106A.

So it comes to either having SY8106A_POWER option in the current
Orange Pi PC/Plus defconfigs or having SUNXI_NO_PMIC in the
future H3-OLinuXino-NANO defconfig. Having configuration options
with a "negative" meaning is generally considered to be a bad
style in many software projects.

> +
>  endchoice
>  
>  config AXP_DCDC1_VOLT
> @@ -232,4 +239,13 @@ config AXP_ELDO3_VOLT
>  	1.2V for the SSD2828 chip (converter of parallel LCD interface
>  	into MIPI DSI).
>  
> +config SY8106A_VOUT1_VOLT
> +	int "SY8106A pmic VOUT1 voltage"
> +	depends on SY8106A_POWER
> +	default 1200
> +	---help---
> +	Set the voltage (mV) to program the SY8106A pmic VOUT1. This
> +	is typically used to power the VDD-CPU and should be 1200mV.
> +	Values can range from 680mV till 1950mV.
> +

In general, I'm not a big fan of having implicit defaults for the
dangerous things such as voltages somewhere outside of defconfig
files. This makes it difficult to review the correctness of board
setup and compare it with the settings from the Allwinner's FEX file.

If the implicit defaults change, then a bunch of defconfigs have
to change too, resulting in big and error prone patches. The problem
here is that when such big patch is submitted, other patches for
defconfigs of some new boards may be also submitted concurrently.
These concurrent patches need to be merged carefully or we end up
with the wrong configs. IIRC this has already resulted in troubles
in the past.

>  endmenu
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 0fdbca3..690faa0 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_AXP221_POWER)	+= axp221.o
>  obj-$(CONFIG_AXP818_POWER)	+= axp818.o
>  obj-$(CONFIG_EXYNOS_TMU)	+= exynos-tmu.o
>  obj-$(CONFIG_FTPMU010_POWER)	+= ftpmu010.o
> +obj-$(CONFIG_SY8106A_POWER)	+= sy8106a.o
>  obj-$(CONFIG_TPS6586X_POWER)	+= tps6586x.o
>  obj-$(CONFIG_TWL4030_POWER)	+= twl4030.o
>  obj-$(CONFIG_TWL6030_POWER)	+= twl6030.o
> diff --git a/drivers/power/sy8106a.c b/drivers/power/sy8106a.c
> new file mode 100644
> index 0000000..6492ddd
> --- /dev/null
> +++ b/drivers/power/sy8106a.c
> @@ -0,0 +1,23 @@

The license/copyright notice is missing here.

> +#include <common.h>
> +#include <i2c.h>
> +#include <sy8106a.h>
> +
> +#define SY8106A_I2C_ADDR 0x65
> +#define SY8106A_VOUT1_SEL 1
> +#define SY8106A_VOUT1_SEL_ENABLE (1 << 7)
> +
> +static u8 sy8106a_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 sy8106a_set_vout1(unsigned int mvolt)
> +{
> +	u8 data = sy8106a_mvolt_to_cfg(mvolt, 680, 1950, 10) | SY8106A_VOUT1_SEL_ENABLE;
> +	return i2c_write(SY8106A_I2C_ADDR, SY8106A_VOUT1_SEL, 1, &data, 1);
> +}
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index b4dfb3c..40850e5 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -206,7 +206,8 @@
>  #define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
>  
>  /* I2C */
> -#if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER
> +#if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \
> +    defined CONFIG_SY8106A_POWER
>  #define CONFIG_SPL_I2C_SUPPORT
>  #endif
>  
> @@ -240,7 +241,8 @@ extern int soft_i2c_gpio_scl;
>  
>  /* PMU */
>  #if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \
> -    defined CONFIG_AXP221_POWER || defined CONFIG_AXP818_POWER
> +    defined CONFIG_AXP221_POWER || defined CONFIG_AXP818_POWER || \
> +    defined CONFIG_SY8106A_POWER
>  #define CONFIG_SPL_POWER_SUPPORT
>  #endif
>  
> diff --git a/include/sy8106a.h b/include/sy8106a.h
> new file mode 100644
> index 0000000..714c314
> --- /dev/null
> +++ b/include/sy8106a.h
> @@ -0,0 +1,5 @@
> +#ifndef _SY8106A_PMIC_H_
> +
> +int sy8106a_set_vout1(unsigned int mvolt);
> +
> +#endif

Tested-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>
Acked-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>

-- 
Best regards,
Siarhei Siamashka


More information about the U-Boot mailing list