Re: [PATCH 2/3] mfd: axp20x: Add support for backup battery charging

From: Lee Jones
Date: Tue Jan 02 2018 - 10:46:58 EST


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@xxxxxxxx>
>
> 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);

--
Lee Jones
Linaro Services Technical Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog