[PATCH 0/2] mfd: axp20x: Fix AXP806 access errors on cold boot

[PATCH 0/2] mfd: axp20x: Fix AXP806 access errors on cold boot

[Chen-Yu Tsai]

Hi Lee,

Recently we've added full SPL support for A80 to mainline U-boot. This
means we no longer depend on Allwinner's bootloader. It also means that
some of system configuration the bootloader set up no longer applies.

The bootloader was correctly configuring the multi-device addressing
support in the AXP806 PMIC. If the PMIC is not correctly addressed, it
just ignores all reads and writes to the other registers. As mainline
U-boot does not support the AXP806, and since we can't always count on
a good bootloader, we should re-configure this in the kernel regardless.

Patch 1 adds the registers for the multi-device addressing scheme to
the AXP806 regmap.

Patch 2 configures the register at probe time, and reinitializes the
regmap cache.

Since support for this PMIC was just added in 4.9-rc1, I hope you can
merge these 2 patches as fixes for 4.9.

Thank you!


Regards
ChenYu


Chen-Yu Tsai (2):
  mfd: axp20x: Add address extension registers for AXP806 regmap
  mfd: axp20x: Fix AXP806 access errors on cold boot

 drivers/mfd/axp20x.c       | 38 +++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/axp20x.h |  2 ++
 2 files changed, 39 insertions(+), 1 deletion(-)

-- 
2.10.2

[PATCH 1/2] mfd: axp20x: Add address extension registers for AXP806 regmap

[Chen-Yu Tsai]

The AXP806 supports either master/standalone or slave mode.
Slave mode allows sharing the serial bus, even with multiple
AXP806 which all have the same hardware address.

This is done with extra "serial interface address extension",
or AXP806_BUS_ADDR_EXT, and "register address extension", or
AXP806_REG_ADDR_EXT, registers. The former is read-only, with
1 bit customizable at the factory, and 1 bit depending on the
state of an external pin. The latter is writable. Only when
the these device addressing bits (in the upper 4 bits of the
registers) match, will the device respond to operations on
its other registers.

Add these 2 registers to the regmap so we can access them.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/mfd/axp20x.c       | 3 ++-
 include/linux/mfd/axp20x.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index ba130be32e61..cdaeb34a9a38 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -135,6 +135,7 @@ static const struct regmap_range axp806_writeable_ranges[] = {
 	regmap_reg_range(AXP806_PWR_OUT_CTRL1, AXP806_CLDO3_V_CTRL),
 	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ2_EN),
 	regmap_reg_range(AXP20X_IRQ1_STATE, AXP20X_IRQ2_STATE),
+	regmap_reg_range(AXP806_REG_ADDR_EXT, AXP806_REG_ADDR_EXT),
 };
 
 static const struct regmap_range axp806_volatile_ranges[] = {
@@ -305,7 +306,7 @@ static const struct regmap_config axp806_regmap_config = {
 	.val_bits	= 8,
 	.wr_table	= &axp806_writeable_table,
 	.volatile_table	= &axp806_volatile_table,
-	.max_register	= AXP806_VREF_TEMP_WARN_L,
+	.max_register	= AXP806_REG_ADDR_EXT,
 	.cache_type	= REGCACHE_RBTREE,
 };
 
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index fec597fb34cb..7e85ececcedf 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -115,6 +115,8 @@ enum {
 #define AXP806_CLDO2_V_CTRL		0x25
 #define AXP806_CLDO3_V_CTRL		0x26
 #define AXP806_VREF_TEMP_WARN_L		0xf3
+#define AXP806_BUS_ADDR_EXT		0xfe
+#define AXP806_REG_ADDR_EXT		0xff
 
 /* Interrupt */
 #define AXP152_IRQ1_EN			0x40
-- 
2.10.2

[PATCH 2/2] mfd: axp20x: Fix AXP806 access errors on cold boot

[Chen-Yu Tsai]

The AXP806 supports either master/standalone or slave mode.
Slave mode allows sharing the serial bus, even with multiple
AXP806 which all have the same hardware address.

This is done with extra "serial interface address extension",
or AXP806_BUS_ADDR_EXT, and "register address extension", or
AXP806_REG_ADDR_EXT, registers. The former is read-only, with
1 bit customizable at the factory, and 1 bit depending on the
state of an external pin. The latter is writable. Only when
the these device addressing bits (in the upper 4 bits of the
registers) match, will the device respond to operations on
its other registers.

The AXP806_REG_ADDR_EXT was previously configured by Allwinner's
bootloader. Work on U-boot SPL support now allows us to switch
to mainline U-boot, which doesn't do this for us. There might
be other bare minimum bootloaders out there which don't to this
either. It's best to handle this in the kernel.

This patch sets AXP806_REG_ADDR_EXT to 0x10, which is what we
know to be the proper value for a standard AXP806 in slave mode.
Afterwards it will reinitialize the regmap cache, to purge any
invalid stale values.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/mfd/axp20x.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index cdaeb34a9a38..2f5d46f28511 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -829,6 +829,41 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
 {
 	int ret;
 
+	/*
+	 * The AXP806 supports either master/standalone or slave mode.
+	 * Slave mode allows sharing the serial bus, even with multiple
+	 * AXP806 which all have the same hardware address.
+	 *
+	 * This is done with extra "serial interface address extension",
+	 * or AXP806_BUS_ADDR_EXT, and "register address extension", or
+	 * AXP806_REG_ADDR_EXT, registers. The former is read-only, with
+	 * 1 bit customizable at the factory, and 1 bit depending on the
+	 * state of an external pin. The latter is writable. Only when
+	 * the these device addressing bits (in the upper 4 bits of the
+	 * registers) match, will the device respond to operations on its
+	 * other registers.
+	 *
+	 * Since we only support an AXP806 chained to an AXP809 in slave
+	 * mode, and there isn't any existing hardware which uses AXP806
+	 * in master mode, or has 2 AXP806s in the same system, we can
+	 * just program the register address extension to the slave mode
+	 * address.
+	 */
+	if (axp20x->variant == AXP806_ID) {
+		/* Write to the register address extension register */
+		regmap_write(axp20x->regmap, AXP806_REG_ADDR_EXT, 0x10);
+
+		/* Make sure the write hits the device */
+		regcache_sync_region(axp20x->regmap, AXP806_REG_ADDR_EXT,
+				     AXP806_REG_ADDR_EXT);
+
+		/*
+		 * Reinitialize the regmap cache in case the device didn't
+		 * properly respond to our reads before.
+		 */
+		regmap_reinit_cache(axp20x->regmap, axp20x->regmap_cfg);
+	}
+
 	ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
 				  IRQF_ONESHOT | IRQF_SHARED, -1,
 				  axp20x->regmap_irq_chip,
-- 
2.10.2

Re: [PATCH 1/2] mfd: axp20x: Add address extension registers for AXP806 regmap

[Lee Jones]

On Fri, 11 Nov 2016, Chen-Yu Tsai wrote:

> The AXP806 supports either master/standalone or slave mode.
> Slave mode allows sharing the serial bus, even with multiple
> AXP806 which all have the same hardware address.
> 
> This is done with extra "serial interface address extension",
> or AXP806_BUS_ADDR_EXT, and "register address extension", or
> AXP806_REG_ADDR_EXT, registers. The former is read-only, with
> 1 bit customizable at the factory, and 1 bit depending on the
> state of an external pin. The latter is writable. Only when
> the these device addressing bits (in the upper 4 bits of the
> registers) match, will the device respond to operations on
> its other registers.
> 
> Add these 2 registers to the regmap so we can access them.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/mfd/axp20x.c       | 3 ++-
>  include/linux/mfd/axp20x.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)

Applied, thanks.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index ba130be32e61..cdaeb34a9a38 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -135,6 +135,7 @@ static const struct regmap_range axp806_writeable_ranges[] = {
>  	regmap_reg_range(AXP806_PWR_OUT_CTRL1, AXP806_CLDO3_V_CTRL),
>  	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ2_EN),
>  	regmap_reg_range(AXP20X_IRQ1_STATE, AXP20X_IRQ2_STATE),
> +	regmap_reg_range(AXP806_REG_ADDR_EXT, AXP806_REG_ADDR_EXT),
>  };
>  
>  static const struct regmap_range axp806_volatile_ranges[] = {
> @@ -305,7 +306,7 @@ static const struct regmap_config axp806_regmap_config = {
>  	.val_bits	= 8,
>  	.wr_table	= &axp806_writeable_table,
>  	.volatile_table	= &axp806_volatile_table,
> -	.max_register	= AXP806_VREF_TEMP_WARN_L,
> +	.max_register	= AXP806_REG_ADDR_EXT,
>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index fec597fb34cb..7e85ececcedf 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -115,6 +115,8 @@ enum {
>  #define AXP806_CLDO2_V_CTRL		0x25
>  #define AXP806_CLDO3_V_CTRL		0x26
>  #define AXP806_VREF_TEMP_WARN_L		0xf3
> +#define AXP806_BUS_ADDR_EXT		0xfe
> +#define AXP806_REG_ADDR_EXT		0xff
>  
>  /* Interrupt */
>  #define AXP152_IRQ1_EN			0x40

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/2] mfd: axp20x: Fix AXP806 access errors on cold boot

[Lee Jones]

On Fri, 11 Nov 2016, Chen-Yu Tsai wrote:

> The AXP806 supports either master/standalone or slave mode.
> Slave mode allows sharing the serial bus, even with multiple
> AXP806 which all have the same hardware address.
> 
> This is done with extra "serial interface address extension",
> or AXP806_BUS_ADDR_EXT, and "register address extension", or
> AXP806_REG_ADDR_EXT, registers. The former is read-only, with
> 1 bit customizable at the factory, and 1 bit depending on the
> state of an external pin. The latter is writable. Only when
> the these device addressing bits (in the upper 4 bits of the
> registers) match, will the device respond to operations on
> its other registers.
> 
> The AXP806_REG_ADDR_EXT was previously configured by Allwinner's
> bootloader. Work on U-boot SPL support now allows us to switch
> to mainline U-boot, which doesn't do this for us. There might
> be other bare minimum bootloaders out there which don't to this
> either. It's best to handle this in the kernel.
> 
> This patch sets AXP806_REG_ADDR_EXT to 0x10, which is what we
> know to be the proper value for a standard AXP806 in slave mode.
> Afterwards it will reinitialize the regmap cache, to purge any
> invalid stale values.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/mfd/axp20x.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index cdaeb34a9a38..2f5d46f28511 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -829,6 +829,41 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
>  {
>  	int ret;
>  
> +	/*
> +	 * The AXP806 supports either master/standalone or slave mode.
> +	 * Slave mode allows sharing the serial bus, even with multiple
> +	 * AXP806 which all have the same hardware address.
> +	 *
> +	 * This is done with extra "serial interface address extension",
> +	 * or AXP806_BUS_ADDR_EXT, and "register address extension", or
> +	 * AXP806_REG_ADDR_EXT, registers. The former is read-only, with
> +	 * 1 bit customizable at the factory, and 1 bit depending on the
> +	 * state of an external pin. The latter is writable. Only when
> +	 * the these device addressing bits (in the upper 4 bits of the
> +	 * registers) match, will the device respond to operations on its
> +	 * other registers.
> +	 *
> +	 * Since we only support an AXP806 chained to an AXP809 in slave
> +	 * mode, and there isn't any existing hardware which uses AXP806
> +	 * in master mode, or has 2 AXP806s in the same system, we can
> +	 * just program the register address extension to the slave mode
> +	 * address.
> +	 */
> +	if (axp20x->variant == AXP806_ID) {
> +		/* Write to the register address extension register */
> +		regmap_write(axp20x->regmap, AXP806_REG_ADDR_EXT, 0x10);

Please define 0x10.

> +		/* Make sure the write hits the device */
> +		regcache_sync_region(axp20x->regmap, AXP806_REG_ADDR_EXT,
> +				     AXP806_REG_ADDR_EXT);
> +
> +		/*
> +		 * Reinitialize the regmap cache in case the device didn't
> +		 * properly respond to our reads before.
> +		 */
> +		regmap_reinit_cache(axp20x->regmap, axp20x->regmap_cfg);
> +	}
> +
>  	ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
>  				  IRQF_ONESHOT | IRQF_SHARED, -1,
>  				  axp20x->regmap_irq_chip,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog