Re: [PATCH v2 10/12] sunxi: pmic_bus: Use the DM PMIC interface when possible

From: Andre Przywara <andre.przywara@arm.com>
To: Samuel Holland <samuel@sholland.org>
Cc: u-boot@lists.denx.de, Jagan Teki <jagan@amarulasolutions.com>,
	Jaehoon Chung <jh80.chung@samsung.com>,
	Heiko Schocher <hs@denx.de>, Anatolij Gustschin <agust@denx.de>,
	Arnaud Ferraris <arnaud.ferraris@gmail.com>,
	Bharat Gooty <bharat.gooty@broadcom.com>,
	Igor Opaniuk <igor.opaniuk@gmail.com>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Minkyu Kang <mk7.kang@samsung.com>,
	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>,
	Simon Glass <sjg@chromium.org>,
	Stephan Gerhold <stephan@gerhold.net>,
	Tim Harvey <tharvey@gateworks.com>, Tom Rini <trini@konsulko.com>
Subject: Re: [PATCH v2 10/12] sunxi: pmic_bus: Use the DM PMIC interface when possible
Date: Tue, 12 Oct 2021 11:49:49 +0100	[thread overview]
Message-ID: <20211012114949.0e100fc7@donnerap.cambridge.arm.com> (raw)
In-Reply-To: <20211008051725.29401-11-samuel@sholland.org>

On Fri,  8 Oct 2021 00:17:23 -0500
Samuel Holland <samuel@sholland.org> wrote:

Hi,

> The pmic_bus functions are used in both SPL (for regulator setup) and
> U-Boot proper (for regulator setup, SID access, GPIO, and poweroff).
> 
> Currently, pmic_bus conflicts with DM_I2C because it uses the legacy I2C
> interface. This commit makes pmic_bus dual-compatible with either the
> legacy I2C functions or the newly-added PMIC_AXP driver (which uses
> DM_I2C). In turn, this allows platforms to start transitioning to DM_I2C
> in U-Boot proper, without breaking boards that still depend on the
> legacy I2C interface for other reasons.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
> 
> Changes in v2:
> - No changes
> 
>  arch/arm/mach-sunxi/Kconfig    |  2 ++
>  arch/arm/mach-sunxi/pmic_bus.c | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index df160c9a775..cfd95b09498 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -96,6 +96,8 @@ config SUN6I_PRCM
>  
>  config AXP_PMIC_BUS
>  	bool
> +	select DM_PMIC if DM_I2C
> +	select PMIC_AXP if DM_I2C
>  	help
>  	  Select this PMIC bus access helpers for Sunxi platform PRCM or other
>  	  AXP family PMIC devices.
> diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/pmic_bus.c
> index 20ded436cd2..c0908406370 100644
> --- a/arch/arm/mach-sunxi/pmic_bus.c
> +++ b/arch/arm/mach-sunxi/pmic_bus.c
> @@ -10,9 +10,11 @@
>  
>  #include <axp_pmic.h>
>  #include <common.h>
> +#include <dm.h>
>  #include <asm/arch/p2wi.h>
>  #include <asm/arch/rsb.h>
>  #include <i2c.h>
> +#include <power/pmic.h>
>  #include <asm/arch/pmic_bus.h>
>  
>  #define AXP152_I2C_ADDR			0x30
> @@ -23,6 +25,9 @@
>  
>  #define AXP221_CHIP_ADDR		0x68
>  
> +#if CONFIG_IS_ENABLED(PMIC_AXP)
> +static struct udevice *pmic;
> +#else
>  static int pmic_i2c_address(void)
>  {
>  	if (IS_ENABLED(CONFIG_AXP152_POWER))
> @@ -33,6 +38,7 @@ static int pmic_i2c_address(void)
>  	/* Other AXP2xx and AXP8xx variants */
>  	return AXP209_I2C_ADDR;
>  }
> +#endif
>  
>  int pmic_bus_init(void)
>  {
> @@ -43,6 +49,10 @@ int pmic_bus_init(void)
>  	if (!needs_init)
>  		return 0;
>  
> +#if CONFIG_IS_ENABLED(PMIC_AXP)
> +	ret = uclass_get_device_by_driver(UCLASS_PMIC, DM_DRIVER_GET(axp_pmic),
> +					  &pmic);
> +#else
>  	if (IS_ENABLED(CONFIG_SYS_I2C_SUN6I_P2WI)) {
>  		p2wi_init();
>  		ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR,
> @@ -56,6 +66,7 @@ int pmic_bus_init(void)
>  		ret = rsb_set_device_address(AXP_PMIC_PRI_DEVICE_ADDR,
>  					     AXP_PMIC_PRI_RUNTIME_ADDR);
>  	}
> +#endif
>  
>  	needs_init = ret;
>  
> @@ -64,22 +75,30 @@ int pmic_bus_init(void)
>  
>  int pmic_bus_read(u8 reg, u8 *data)
>  {
> +#if CONFIG_IS_ENABLED(PMIC_AXP)
> +	return pmic_read(pmic, reg, data, 1);
> +#else
>  	if (IS_ENABLED(CONFIG_SYS_I2C_SUN6I_P2WI))
>  		return p2wi_read(reg, data);
>  	if (IS_ENABLED(CONFIG_SYS_I2C_SUN8I_RSB))
>  		return rsb_read(AXP_PMIC_PRI_RUNTIME_ADDR, reg, data);
>  
>  	return i2c_read(pmic_i2c_address(), reg, 1, data, 1);
> +#endif
>  }
>  
>  int pmic_bus_write(u8 reg, u8 data)
>  {
> +#if CONFIG_IS_ENABLED(PMIC_AXP)
> +	return pmic_write(pmic, reg, &data, 1);
> +#else
>  	if (IS_ENABLED(CONFIG_SYS_I2C_SUN6I_P2WI))
>  		return p2wi_write(reg, data);
>  	if (IS_ENABLED(CONFIG_SYS_I2C_SUN8I_RSB))
>  		return rsb_write(AXP_PMIC_PRI_RUNTIME_ADDR, reg, data);
>  
>  	return i2c_write(pmic_i2c_address(), reg, 1, &data, 1);
> +#endif
>  }
>  
>  int pmic_bus_setbits(u8 reg, u8 bits)