[0/3] axp20x backup battery charging

This series introduces support for axp20x backup battery charging, with
a dedicated device-tree property.

I wondered whether to include this in a power-supply driver or not.
Since it does not, in fact, supply power to the whole system and
because no status changes over time, I thought it would be inappropriate
to craft a power supply driver only for this.
I also wondered whether to stick this into an existing power-supply
driver, as is done by e.g. twl4030, but we have two distinct supply
drivers for the axp20x (ac and usb), that may be used together or not.
Also, the backup battery isn't tied to the power supply anyway.

This is why I thought it would make more sense to put this in the mfd
driver directly. What do you think?

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, 30 Dec 2017, Paul Kocialkowski wrote:

> This adds support for backup battery charging for axp20x PMICs, that is
> configured through a dedicated device-tree property.
> 
> It supports 4 different charging voltages and as many charging currents.
> This is especially useful to allow the on-chip RTC (on the SoC side) to
> be powered when the rest of the system is off.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 2468b431bb22..7847f5d0b979 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -34,6 +34,16 @@
>  #define AXP806_REG_ADDR_EXT_ADDR_MASTER_MODE	0
>  #define AXP806_REG_ADDR_EXT_ADDR_SLAVE_MODE	BIT(4)
>  
> +#define AXP20X_CHRG_BAK_CTRL_ENABLE		BIT(7)
> +#define AXP20X_CHRG_BAK_VOLTAGE_3100_MV		(0 << 5)
> +#define AXP20X_CHRG_BAK_VOLTAGE_3000_MV		(1 << 5)
> +#define AXP20X_CHRG_BAK_VOLTAGE_3600_MV		(2 << 5)
> +#define AXP20X_CHRG_BAK_VOLTAGE_2500_MV		(3 << 5)
> +#define AXP20X_CHRG_BAK_CURRENT_50_UA		(0 << 0)
> +#define AXP20X_CHRG_BAK_CURRENT_100_UA		(1 << 0)
> +#define AXP20X_CHRG_BAK_CURRENT_200_UA		(2 << 0)
> +#define AXP20X_CHRG_BAK_CURRENT_400_UA		(3 << 0)
> +
>  static const char * const axp20x_model_names[] = {
>  	"AXP152",
>  	"AXP202",
> @@ -894,6 +904,63 @@ static void axp20x_power_off(void)
>  	msleep(500);
>  }
>  
> +static void axp20x_backup_setup(struct axp20x_dev *axp20x)
> +{
> +	u32 backup[2];
> +	int reg;
> +	int ret;
> +
> +	ret = of_property_read_u32_array(axp20x->dev->of_node, "backup", backup,
> +					 2);
> +	if (ret != 0)

Nit:
        if (ret)

> +		return;
> +
> +	switch (axp20x->variant) {
> +	case AXP202_ID:
> +	case AXP209_ID:

Nested switch statements, hmm ...

Instead, what if you either only invoked this function for supported
devices, or at least returned early for non-supported ones?

if (axp20x->variant != AXP202_ID && axp20x->variant != AXP209_ID)
        return;

> +		reg = AXP20X_CHRG_BAK_CTRL_ENABLE;
> +
> +		/* Charging voltage. */
> +		switch (backup[0]) {
> +		case 2500:
> +			reg |= AXP20X_CHRG_BAK_VOLTAGE_2500_MV;
> +			break;
> +		case 3000:
> +			reg |= AXP20X_CHRG_BAK_VOLTAGE_3000_MV;
> +			break;
> +		case 3100:
> +			reg |= AXP20X_CHRG_BAK_VOLTAGE_3100_MV;
> +			break;
> +		case 3600:
> +			reg |= AXP20X_CHRG_BAK_VOLTAGE_3600_MV;
> +			break;
> +		default:
> +			return;
> +		}
> +
> +		/* Charging current. */
> +		switch (backup[1]) {
> +		case 50:
> +			reg |= AXP20X_CHRG_BAK_CURRENT_50_UA;
> +			break;
> +		case 100:
> +			reg |= AXP20X_CHRG_BAK_CURRENT_100_UA;
> +			break;
> +		case 200:
> +			reg |= AXP20X_CHRG_BAK_CURRENT_200_UA;
> +			break;
> +		case 400:
> +			reg |= AXP20X_CHRG_BAK_CURRENT_400_UA;
> +			break;
> +		default:
> +			return;
> +		}
> +
> +		regmap_write(axp20x->regmap, AXP20X_CHRG_BAK_CTRL, reg);
> +		break;
> +	}
> +}
> +
>  int axp20x_match_device(struct axp20x_dev *axp20x)
>  {
>  	struct device *dev = axp20x->dev;
> @@ -1023,6 +1090,9 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
>  				     AXP806_REG_ADDR_EXT_ADDR_SLAVE_MODE);
>  	}
>  
> +	/* Backup RTC battery. */
> +	axp20x_backup_setup(axp20x);
> +
>  	ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
>  			  IRQF_ONESHOT | IRQF_SHARED | axp20x->irq_flags,
>  			   -1, axp20x->regmap_irq_chip, &axp20x->regmap_irqc);