[v5,0/6] add support for AXP813 ADC and battery power supply

The AXP813 PMIC is relatively close to the already supported AXP20X and
AXP22X. It provides three different power outputs: battery, AC and USB, and
measures a few different things: temperature, power supply status, current
current and voltage supplied, maximum current limit, battery capacity, min
and max voltage limits.

One of its two GPIOs can be used as an ADC.

There are a few differences with AXP20X/AXP22X PMICs though:
  - a different constant charge current formula,
  - battery temperature, GPIO0 and battery voltages are the only voltages
  measurable,
  - all data are stored on 12 bits (AXP20X/AXP22X had one type of data that
  was stored on 13 bits),
  - different scales and offsets,
  - a different ADC rate formula and register,

This patch series adds support for the PMIC's ADC and battery power supply
in the existing drivers.

Make the axp20x MFD automatically probe the ADC driver, add the battery
power supply node in axp81x node and enable it for the TBS A711 since it
has a soldered battery.

I suggest patches:
  - 1,2,6 go through Lee's tree,
  - 3,4,5 go through Sebastian's tree,

v5:
  - added static in front of the axp_data struct in the battery driver to
  make sparse happy,
  - removed merged patches,

v4:
  - shortened one commit title as a workaround to Chen-Yu note,
  - added const to data structures as proposed by Chen-Yu,
  - added last patch for making sparse happy, on a proposal from Jonathan,
  - removed already applied patches (IIO ones),

v3:
  - merging dt-bindings patches for axp_adc as requested by Rob,
  - re-ordered constant in IIO driver as requested by Julian,
  - compatibles for ADC are now named after the first design that
  introduced the IP instead of wildcard as requested by Maxime,
  - renamed DT node name from axp-adc to adc as requested by Rob,
  - replaced enumeration of supported PMICs in battery power supply DT
  bindings documentation by "supported devices" as requested by Jonathan,
  - added a new patch for removing "axp-" from axp81x's pinctrl DT node,

v2:
  - introduce data structure instead of ID for variant specific code in
  battery driver,
  - add DT binding for ADC driver,
  - make mfd probe the ADC driver via DT as well so that its IIO channels
  can be consumed by other drivers via DT mapping,

Thanks,
Quentin

Quentin Schulz (6):
  mfd: axp20x: make AXP209/22x cells probe their ADC via DT
  mfd: axp20x: probe axp20x_adc driver for AXP813
  power: supply: axp20x_battery: use data struct for variant specific code
  dt-bindings: power: supply: axp20x: add AXP813 battery DT binding
  power: supply: axp20x_battery: add support for AXP813
  mfd: axp20x: add battery power supply cell for AXP813

 Documentation/devicetree/bindings/power/supply/axp20x_battery.txt |   8 ++--
 drivers/mfd/axp20x.c                                              |  13 +++++--
 drivers/power/supply/axp20x_battery.c                             | 134 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 119 insertions(+), 36 deletions(-)

base-commit: 827ad482fda17d0de5df5116fda827cd3671e62e

On Wed, 28 Feb 2018, Quentin Schulz wrote:

> This makes AXP209 and AXP22x ADCs probe first via DT and then by
> fallback via platform.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> Acked-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/mfd/axp20x.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Applied, thanks.

On Wed, 28 Feb 2018, Quentin Schulz wrote:

> This makes the axp20x_adc driver probe with platform device id
> "axp813-adc".
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> Acked-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/mfd/axp20x.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Applied, thanks.

On Wed, 28 Feb 2018, Quentin Schulz wrote:

> As axp20x-battery-power-supply now supports AXP813, add a cell for it.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/mfd/axp20x.c | 3 +++
>  1 file changed, 3 insertions(+)

Applied, thanks.

Hi Quentin,

On Wed, Feb 28, 2018 at 11:35:58AM +0100, Quentin Schulz wrote:
> We used to use IDs to select a function or a feature depending on the
> variant. It's easier to maintain the code by adding data structure
> storing the few differences between variants so that we don't add a pile
> of if conditions.
> 
> Let's use this data structure and update the code to use it.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> ---

I applied this, but modified the patch slightly, so that
it actually calls your new set_max_voltage() callback in
axp20x_battery_set_prop(). Please check that the resulting
patch looks correct to you:

https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/commit/?h=for-next&id=648badd797c762a713b48afc3b67d56abdd0073b

-- Sebastian

>  drivers/power/supply/axp20x_battery.c | 100 +++++++++++++++++----------
>  1 file changed, 66 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> index 7494f0f..04a0d91 100644
> --- a/drivers/power/supply/axp20x_battery.c
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -53,6 +53,16 @@
>  
>  #define AXP20X_V_OFF_MASK		GENMASK(2, 0)
>  
> +struct axp20x_batt_ps;
> +
> +struct axp_data {
> +	int	ccc_scale;
> +	int	ccc_offset;
> +	bool	has_fg_valid;
> +	int	(*get_max_voltage)(struct axp20x_batt_ps *batt, int *val);
> +	int	(*set_max_voltage)(struct axp20x_batt_ps *batt, int val);
> +};
> +
>  struct axp20x_batt_ps {
>  	struct regmap *regmap;
>  	struct power_supply *batt;
> @@ -62,7 +72,7 @@ struct axp20x_batt_ps {
>  	struct iio_channel *batt_v;
>  	/* Maximum constant charge current */
>  	unsigned int max_ccc;
> -	u8 axp_id;
> +	const struct axp_data	*data;
>  };
>  
>  static int axp20x_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
> @@ -123,22 +133,6 @@ static int axp22x_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
>  	return 0;
>  }
>  
> -static void raw_to_constant_charge_current(struct axp20x_batt_ps *axp, int *val)
> -{
> -	if (axp->axp_id == AXP209_ID)
> -		*val = *val * 100000 + 300000;
> -	else
> -		*val = *val * 150000 + 300000;
> -}
> -
> -static void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val)
> -{
> -	if (axp->axp_id == AXP209_ID)
> -		*val = (*val - 300000) / 100000;
> -	else
> -		*val = (*val - 300000) / 150000;
> -}
> -
>  static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
>  					      int *val)
>  {
> @@ -150,7 +144,7 @@ static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
>  
>  	*val &= AXP20X_CHRG_CTRL1_TGT_CURR;
>  
> -	raw_to_constant_charge_current(axp, val);
> +	*val = *val * axp->data->ccc_scale + axp->data->ccc_offset;
>  
>  	return 0;
>  }
> @@ -269,8 +263,7 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
>  		if (ret)
>  			return ret;
>  
> -		if (axp20x_batt->axp_id == AXP221_ID &&
> -		    !(reg & AXP22X_FG_VALID))
> +		if (axp20x_batt->data->has_fg_valid && !(reg & AXP22X_FG_VALID))
>  			return -EINVAL;
>  
>  		/*
> @@ -281,11 +274,8 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
>  		break;
>  
>  	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> -		if (axp20x_batt->axp_id == AXP209_ID)
> -			return axp20x_battery_get_max_voltage(axp20x_batt,
> -							      &val->intval);
> -		return axp22x_battery_get_max_voltage(axp20x_batt,
> -						      &val->intval);
> +		return axp20x_batt->data->get_max_voltage(axp20x_batt,
> +							  &val->intval);
>
>  	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
>  		ret = regmap_read(axp20x_batt->regmap, AXP20X_V_OFF, &reg);
> @@ -312,6 +302,32 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
>  	return 0;
>  }
>  
> +static int axp22x_battery_set_max_voltage(struct axp20x_batt_ps *axp20x_batt,
> +					  int val)
> +{
> +	switch (val) {
> +	case 4100000:
> +		val = AXP20X_CHRG_CTRL1_TGT_4_1V;
> +		break;
> +
> +	case 4200000:
> +		val = AXP20X_CHRG_CTRL1_TGT_4_2V;
> +		break;
> +
> +	default:
> +		/*
> +		 * AXP20x max voltage can be set to 4.36V and AXP22X max voltage
> +		 * can be set to 4.22V and 4.24V, but these voltages are too
> +		 * high for Lithium based batteries (AXP PMICs are supposed to
> +		 * be used with these kinds of battery).
> +		 */
> +		return -EINVAL;
> +	}
> +
> +	return regmap_update_bits(axp20x_batt->regmap, AXP20X_CHRG_CTRL1,
> +				  AXP20X_CHRG_CTRL1_TGT_VOLT, val);
> +}
> +
>  static int axp20x_battery_set_max_voltage(struct axp20x_batt_ps *axp20x_batt,
>  					  int val)
>  {
> @@ -321,9 +337,6 @@ static int axp20x_battery_set_max_voltage(struct axp20x_batt_ps *axp20x_batt,
>  		break;
>  
>  	case 4150000:
> -		if (axp20x_batt->axp_id == AXP221_ID)
> -			return -EINVAL;
> -
>  		val = AXP20X_CHRG_CTRL1_TGT_4_15V;
>  		break;
>  
> @@ -351,7 +364,8 @@ static int axp20x_set_constant_charge_current(struct axp20x_batt_ps *axp_batt,
>  	if (charge_current > axp_batt->max_ccc)
>  		return -EINVAL;
>  
> -	constant_charge_current_to_raw(axp_batt, &charge_current);
> +	charge_current = (charge_current - axp_batt->data->ccc_offset) /
> +		axp_batt->data->ccc_scale;
>  
>  	if (charge_current > AXP20X_CHRG_CTRL1_TGT_CURR || charge_current < 0)
>  		return -EINVAL;
> @@ -365,12 +379,14 @@ static int axp20x_set_max_constant_charge_current(struct axp20x_batt_ps *axp,
>  {
>  	bool lower_max = false;
>  
> -	constant_charge_current_to_raw(axp, &charge_current);
> +	charge_current = (charge_current - axp->data->ccc_offset) /
> +		axp->data->ccc_scale;
>  
>  	if (charge_current > AXP20X_CHRG_CTRL1_TGT_CURR || charge_current < 0)
>  		return -EINVAL;
>  
> -	raw_to_constant_charge_current(axp, &charge_current);
> +	charge_current = charge_current * axp->data->ccc_scale +
> +		axp->data->ccc_offset;
>  
>  	if (charge_current > axp->max_ccc)
>  		dev_warn(axp->dev,
> @@ -460,13 +476,28 @@ static const struct power_supply_desc axp20x_batt_ps_desc = {
>  	.set_property = axp20x_battery_set_prop,
>  };
>  
> +static const struct axp_data axp209_data = {
> +	.ccc_scale = 100000,
> +	.ccc_offset = 300000,
> +	.get_max_voltage = axp20x_battery_get_max_voltage,
> +	.set_max_voltage = axp20x_battery_set_max_voltage,
> +};
> +
> +static const struct axp_data axp221_data = {
> +	.ccc_scale = 150000,
> +	.ccc_offset = 300000,
> +	.has_fg_valid = true,
> +	.get_max_voltage = axp22x_battery_get_max_voltage,
> +	.set_max_voltage = axp22x_battery_set_max_voltage,
> +};
> +
>  static const struct of_device_id axp20x_battery_ps_id[] = {
>  	{
>  		.compatible = "x-powers,axp209-battery-power-supply",
> -		.data = (void *)AXP209_ID,
> +		.data = (void *)&axp209_data,
>  	}, {
>  		.compatible = "x-powers,axp221-battery-power-supply",
> -		.data = (void *)AXP221_ID,
> +		.data = (void *)&axp221_data,
>  	}, { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);
> @@ -476,6 +507,7 @@ static int axp20x_power_probe(struct platform_device *pdev)
>  	struct axp20x_batt_ps *axp20x_batt;
>  	struct power_supply_config psy_cfg = {};
>  	struct power_supply_battery_info info;
> +	struct device *dev = &pdev->dev;
>  
>  	if (!of_device_is_available(pdev->dev.of_node))
>  		return -ENODEV;
> @@ -516,7 +548,7 @@ static int axp20x_power_probe(struct platform_device *pdev)
>  	psy_cfg.drv_data = axp20x_batt;
>  	psy_cfg.of_node = pdev->dev.of_node;
>  
> -	axp20x_batt->axp_id = (uintptr_t)of_device_get_match_data(&pdev->dev);
> +	axp20x_batt->data = (struct axp_data *)of_device_get_match_data(dev);
>  
>  	axp20x_batt->batt = devm_power_supply_register(&pdev->dev,
>  						       &axp20x_batt_ps_desc,
> -- 
> git-series 0.9.1

Hi,

On Wed, Feb 28, 2018 at 11:36:00AM +0100, Quentin Schulz wrote:
> The X-Powers AXP813 PMIC has got some slight differences from
> AXP20X/AXP22X PMICs:
>  - the maximum voltage supplied by the PMIC is 4.35 instead of 4.36/4.24
>  for AXP20X/AXP22X,
>  - the constant charge current formula is different,
> 
> It also has a bit to tell whether the battery percentage returned by the
> PMIC is valid.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/axp20x_battery.c | 42 ++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> index 04a0d91..4f69c0e 100644
> --- a/drivers/power/supply/axp20x_battery.c
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -49,6 +49,8 @@
>  #define AXP22X_CHRG_CTRL1_TGT_4_22V	(1 << 5)
>  #define AXP22X_CHRG_CTRL1_TGT_4_24V	(3 << 5)
>  
> +#define AXP813_CHRG_CTRL1_TGT_4_35V	(3 << 5)
> +
>  #define AXP20X_CHRG_CTRL1_TGT_CURR	GENMASK(3, 0)
>  
>  #define AXP20X_V_OFF_MASK		GENMASK(2, 0)
> @@ -133,6 +135,35 @@ static int axp22x_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
>  	return 0;
>  }
>  
> +static int axp813_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
> +					  int *val)
> +{
> +	int ret, reg;
> +
> +	ret = regmap_read(axp20x_batt->regmap, AXP20X_CHRG_CTRL1, &reg);
> +	if (ret)
> +		return ret;
> +
> +	switch (reg & AXP20X_CHRG_CTRL1_TGT_VOLT) {
> +	case AXP20X_CHRG_CTRL1_TGT_4_1V:
> +		*val = 4100000;
> +		break;
> +	case AXP20X_CHRG_CTRL1_TGT_4_15V:
> +		*val = 4150000;
> +		break;
> +	case AXP20X_CHRG_CTRL1_TGT_4_2V:
> +		*val = 4200000;
> +		break;
> +	case AXP813_CHRG_CTRL1_TGT_4_35V:
> +		*val = 4350000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
>  					      int *val)
>  {
> @@ -491,6 +522,14 @@ static const struct axp_data axp221_data = {
>  	.set_max_voltage = axp22x_battery_set_max_voltage,
>  };
>  
> +static const struct axp_data axp813_data = {
> +	.ccc_scale = 200000,
> +	.ccc_offset = 200000,
> +	.has_fg_valid = true,
> +	.get_max_voltage = axp813_battery_get_max_voltage,
> +	.set_max_voltage = axp20x_battery_set_max_voltage,
> +};
> +
>  static const struct of_device_id axp20x_battery_ps_id[] = {
>  	{
>  		.compatible = "x-powers,axp209-battery-power-supply",
> @@ -498,6 +537,9 @@ static const struct of_device_id axp20x_battery_ps_id[] = {
>  	}, {
>  		.compatible = "x-powers,axp221-battery-power-supply",
>  		.data = (void *)&axp221_data,
> +	}, {
> +		.compatible = "x-powers,axp813-battery-power-supply",
> +		.data = (void *)&axp813_data,
>  	}, { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);
> -- 
> git-series 0.9.1

Hi Sebastian,

On Fri, Mar 09, 2018 at 05:08:25PM +0100, Sebastian Reichel wrote:
> Hi Quentin,
> 
> On Wed, Feb 28, 2018 at 11:35:58AM +0100, Quentin Schulz wrote:
> > We used to use IDs to select a function or a feature depending on the
> > variant. It's easier to maintain the code by adding data structure
> > storing the few differences between variants so that we don't add a pile
> > of if conditions.
> > 
> > Let's use this data structure and update the code to use it.
> > 
> > Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> 
> I applied this, but modified the patch slightly, so that
> it actually calls your new set_max_voltage() callback in
> axp20x_battery_set_prop(). Please check that the resulting
> patch looks correct to you:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/commit/?h=for-next&id=648badd797c762a713b48afc3b67d56abdd0073b
> 

You actually fixed an error in this patch while doing so, so thank you :)

Looks good to me.

Quentin