Re: [PATCH v2 09/12] sunxi: pmic_bus: Clean up preprocessor conditions

2021-10-12 Thread Andre Przywara
On Fri,  8 Oct 2021 00:17:22 -0500
Samuel Holland  wrote:

> Instead of using the SoC symbols to decide the bus type, use whichever
> bus driver is actually enabled. This allows collapsing all of the AXP2xx
> and AXP8xx variants into one "else" case. It also has the advantage of
> falling back to I2C when the other bus drivers are disabled; this works
> because all of the PMICs support I2C in addition to other interfaces.

Many many thanks for cleaning up this #ifdef mess! I couldn't be bothered
to double check all variants (did I mention that it's a mess?), but I
glanced over it, and it looks alright.

> Signed-off-by: Samuel Holland 

Reviewed-by: Andre Przywara 

Cheers,
Andre

> ---
> 
> Changes in v2:
> - Update IS_ENABLEDs in pmic_bus.c to match chages to previous patches
> 
>  arch/arm/mach-sunxi/pmic_bus.c | 90 +++---
>  1 file changed, 39 insertions(+), 51 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/pmic_bus.c
> b/arch/arm/mach-sunxi/pmic_bus.c index 827797249ea..20ded436cd2 100644
> --- a/arch/arm/mach-sunxi/pmic_bus.c
> +++ b/arch/arm/mach-sunxi/pmic_bus.c
> @@ -23,75 +23,63 @@
>  
>  #define AXP221_CHIP_ADDR 0x68
>  
> +static int pmic_i2c_address(void)
> +{
> + if (IS_ENABLED(CONFIG_AXP152_POWER))
> + return AXP152_I2C_ADDR;
> + if (IS_ENABLED(CONFIG_AXP305_POWER))
> + return AXP305_I2C_ADDR;
> +
> + /* Other AXP2xx and AXP8xx variants */
> + return AXP209_I2C_ADDR;
> +}
> +
>  int pmic_bus_init(void)
>  {
>   /* This cannot be 0 because it is used in SPL before BSS is
> ready */ static int needs_init = 1;
> - __maybe_unused int ret;
> + int ret = 0;
>  
>   if (!needs_init)
>   return 0;
>  
> -#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER ||
> defined CONFIG_AXP818_POWER -# ifdef CONFIG_MACH_SUN6I
> - p2wi_init();
> - ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR,
> AXP_PMIC_MODE_REG,
> -AXP_PMIC_MODE_P2WI);
> -# elif defined CONFIG_MACH_SUN8I_R40
> - /* Nothing. R40 uses the AXP221s in I2C mode */
> - ret = 0;
> -# else
> - ret = rsb_init();
> - if (ret)
> - return ret;
> + if (IS_ENABLED(CONFIG_SYS_I2C_SUN6I_P2WI)) {
> + p2wi_init();
> + ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR,
> +AXP_PMIC_MODE_REG,
> +AXP_PMIC_MODE_P2WI);
> + } else if (IS_ENABLED(CONFIG_SYS_I2C_SUN8I_RSB)) {
> + ret = rsb_init();
> + if (ret)
> + return ret;
>  
> - ret = rsb_set_device_address(AXP_PMIC_PRI_DEVICE_ADDR,
> -  AXP_PMIC_PRI_RUNTIME_ADDR);
> -# endif
> - if (ret)
> - return ret;
> -#endif
> + ret = rsb_set_device_address(AXP_PMIC_PRI_DEVICE_ADDR,
> +  AXP_PMIC_PRI_RUNTIME_ADDR);
> + }
> +
> + needs_init = ret;
>  
> - needs_init = 0;
> - return 0;
> + return ret;
>  }
>  
>  int pmic_bus_read(u8 reg, u8 *data)
>  {
> -#ifdef CONFIG_AXP152_POWER
> - 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);
> -# elif defined CONFIG_MACH_SUN8I_R40
> - return i2c_read(AXP209_I2C_ADDR, reg, 1, data, 1);
> -# else
> - return rsb_read(AXP_PMIC_PRI_RUNTIME_ADDR, reg, data);
> -# endif
> -#endif
> + 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);
>  }
>  
>  int pmic_bus_write(u8 reg, u8 data)
>  {
> -#ifdef CONFIG_AXP152_POWER
> - 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);
> -# elif defined CONFIG_MACH_SUN8I_R40
> - return i2c_write(AXP209_I2C_ADDR, reg, 1, &data, 1);
> -# else
> - return rsb_write(AXP_PMIC_PRI_RUNTIME_ADDR, reg, data);
> -# endif
> -#endif
> + 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, 

[PATCH v2 09/12] sunxi: pmic_bus: Clean up preprocessor conditions

2021-10-07 Thread Samuel Holland
Instead of using the SoC symbols to decide the bus type, use whichever
bus driver is actually enabled. This allows collapsing all of the AXP2xx
and AXP8xx variants into one "else" case. It also has the advantage of
falling back to I2C when the other bus drivers are disabled; this works
because all of the PMICs support I2C in addition to other interfaces.

Signed-off-by: Samuel Holland 
---

Changes in v2:
- Update IS_ENABLEDs in pmic_bus.c to match chages to previous patches

 arch/arm/mach-sunxi/pmic_bus.c | 90 +++---
 1 file changed, 39 insertions(+), 51 deletions(-)

diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/pmic_bus.c
index 827797249ea..20ded436cd2 100644
--- a/arch/arm/mach-sunxi/pmic_bus.c
+++ b/arch/arm/mach-sunxi/pmic_bus.c
@@ -23,75 +23,63 @@
 
 #define AXP221_CHIP_ADDR   0x68
 
+static int pmic_i2c_address(void)
+{
+   if (IS_ENABLED(CONFIG_AXP152_POWER))
+   return AXP152_I2C_ADDR;
+   if (IS_ENABLED(CONFIG_AXP305_POWER))
+   return AXP305_I2C_ADDR;
+
+   /* Other AXP2xx and AXP8xx variants */
+   return AXP209_I2C_ADDR;
+}
+
 int pmic_bus_init(void)
 {
/* This cannot be 0 because it is used in SPL before BSS is ready */
static int needs_init = 1;
-   __maybe_unused int ret;
+   int ret = 0;
 
if (!needs_init)
return 0;
 
-#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || defined 
CONFIG_AXP818_POWER
-# ifdef CONFIG_MACH_SUN6I
-   p2wi_init();
-   ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR, AXP_PMIC_MODE_REG,
-  AXP_PMIC_MODE_P2WI);
-# elif defined CONFIG_MACH_SUN8I_R40
-   /* Nothing. R40 uses the AXP221s in I2C mode */
-   ret = 0;
-# else
-   ret = rsb_init();
-   if (ret)
-   return ret;
+   if (IS_ENABLED(CONFIG_SYS_I2C_SUN6I_P2WI)) {
+   p2wi_init();
+   ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR,
+  AXP_PMIC_MODE_REG,
+  AXP_PMIC_MODE_P2WI);
+   } else if (IS_ENABLED(CONFIG_SYS_I2C_SUN8I_RSB)) {
+   ret = rsb_init();
+   if (ret)
+   return ret;
 
-   ret = rsb_set_device_address(AXP_PMIC_PRI_DEVICE_ADDR,
-AXP_PMIC_PRI_RUNTIME_ADDR);
-# endif
-   if (ret)
-   return ret;
-#endif
+   ret = rsb_set_device_address(AXP_PMIC_PRI_DEVICE_ADDR,
+AXP_PMIC_PRI_RUNTIME_ADDR);
+   }
+
+   needs_init = ret;
 
-   needs_init = 0;
-   return 0;
+   return ret;
 }
 
 int pmic_bus_read(u8 reg, u8 *data)
 {
-#ifdef CONFIG_AXP152_POWER
-   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);
-# elif defined CONFIG_MACH_SUN8I_R40
-   return i2c_read(AXP209_I2C_ADDR, reg, 1, data, 1);
-# else
-   return rsb_read(AXP_PMIC_PRI_RUNTIME_ADDR, reg, data);
-# endif
-#endif
+   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);
 }
 
 int pmic_bus_write(u8 reg, u8 data)
 {
-#ifdef CONFIG_AXP152_POWER
-   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);
-# elif defined CONFIG_MACH_SUN8I_R40
-   return i2c_write(AXP209_I2C_ADDR, reg, 1, &data, 1);
-# else
-   return rsb_write(AXP_PMIC_PRI_RUNTIME_ADDR, reg, data);
-# endif
-#endif
+   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);
 }
 
 int pmic_bus_setbits(u8 reg, u8 bits)
-- 
2.32.0