diff mbox

[RFC,4/7] power: supply: axp20x-battery: support AXP803

Message ID 20170920151814.22461-5-icenowy@aosc.io (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Icenowy Zheng Sept. 20, 2017, 3:18 p.m. UTC
The AXP803 PMIC has battery support like other AXP PMICs, but with
different definition of max target charging voltage and constant
charging current.

Add support for AXP803 battery in axp20x-battery driver.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/power/supply/axp20x_battery.c | 88 +++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 10 deletions(-)

Comments

Jonathan Cameron Sept. 24, 2017, 2:34 p.m. UTC | #1
On Wed, 20 Sep 2017 23:18:11 +0800
Icenowy Zheng <icenowy@aosc.io> wrote:

> The AXP803 PMIC has battery support like other AXP PMICs, but with
> different definition of max target charging voltage and constant
> charging current.
> 
> Add support for AXP803 battery in axp20x-battery driver.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>

This looks fine to me, but I haven't dived into the precise values
etc.

Jonathan

> ---
>  drivers/power/supply/axp20x_battery.c | 88 +++++++++++++++++++++++++++++++----
>  1 file changed, 78 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> index 7494f0f0eadb..c9a9fb320c92 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 AXP803_CHRG_CTRL1_TGT_4_35V	(3 << 5)
> +
>  #define AXP20X_CHRG_CTRL1_TGT_CURR	GENMASK(3, 0)
>  
>  #define AXP20X_V_OFF_MASK		GENMASK(2, 0)
> @@ -123,20 +125,71 @@ static int axp22x_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
>  	return 0;
>  }
>  
> +static int axp803_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 AXP803_CHRG_CTRL1_TGT_4_35V:
> +		*val = 4350000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static void raw_to_constant_charge_current(struct axp20x_batt_ps *axp, int *val)
>  {
> -	if (axp->axp_id == AXP209_ID)
> +	switch (axp->axp_id) {
> +	case AXP209_ID:
>  		*val = *val * 100000 + 300000;
> -	else
> +		break;
> +	case AXP221_ID:
>  		*val = *val * 150000 + 300000;
> +		break;
> +	case AXP803_ID:
> +		*val = *val * 200000 + 200000;
> +		break;
> +	}
>  }
>  
>  static void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val)
>  {
> -	if (axp->axp_id == AXP209_ID)
> +	switch (axp->axp_id) {
> +	case AXP209_ID:
>  		*val = (*val - 300000) / 100000;
> -	else
> +		break;
> +	case AXP221_ID:
>  		*val = (*val - 300000) / 150000;
> +		break;
> +	case AXP803_ID:
> +		*val = (*val - 200000) / 200000;
> +		/*
> +		 * The maximum charge current on AXP803 is 2.8A, and the
> +		 * datasheet says "1110-1111 reserved" in this part.
> +		 * So we return an invalid value -1 in this situation,
> +		 * which will be dealed by the caller of this function,
> +		 */
> +		if (*val > 13)
> +			*val = -1;
> +		break;
> +	}
>  }
>  
>  static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
> @@ -269,9 +322,13 @@ 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))
> -			return -EINVAL;
> +		switch (axp20x_batt->axp_id) {
> +		case AXP221_ID:
> +		case AXP803_ID:
> +			if (!(reg & AXP22X_FG_VALID))
> +				return -EINVAL;
> +			break;
> +		};
>  
>  		/*
>  		 * Fuel Gauge data takes 7 bits but the stored value seems to be
> @@ -281,11 +338,19 @@ 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)
> +		switch (axp20x_batt->axp_id) {
> +		case AXP209_ID:
>  			return axp20x_battery_get_max_voltage(axp20x_batt,
>  							      &val->intval);
> -		return axp22x_battery_get_max_voltage(axp20x_batt,
> -						      &val->intval);
> +		case AXP221_ID:
> +			return axp22x_battery_get_max_voltage(axp20x_batt,
> +							      &val->intval);
> +		case AXP803_ID:
> +			return axp803_battery_get_max_voltage(axp20x_batt,
> +							      &val->intval);
> +		default:
> +			return -EINVAL;
> +		}
>  
>  	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
>  		ret = regmap_read(axp20x_batt->regmap, AXP20X_V_OFF, &reg);
> @@ -467,6 +532,9 @@ static const struct of_device_id axp20x_battery_ps_id[] = {
>  	}, {
>  		.compatible = "x-powers,axp221-battery-power-supply",
>  		.data = (void *)AXP221_ID,
> +	}, {
> +		.compatible = "x-powers,axp803-battery-power-supply",
> +		.data = (void *)AXP803_ID,
>  	}, { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);
Quentin Schulz Sept. 25, 2017, 8:58 a.m. UTC | #2
Hi Icenowy,

On 20/09/2017 17:18, Icenowy Zheng wrote:
> The AXP803 PMIC has battery support like other AXP PMICs, but with
> different definition of max target charging voltage and constant
> charging current.
> 
> Add support for AXP803 battery in axp20x-battery driver.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  drivers/power/supply/axp20x_battery.c | 88 +++++++++++++++++++++++++++++++----
>  1 file changed, 78 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> index 7494f0f0eadb..c9a9fb320c92 100644
> --- a/drivers/power/supply/axp20x_battery.c
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -49,6 +49,8 @@
[...]
>  static void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val)
>  {
> -	if (axp->axp_id == AXP209_ID)
> +	switch (axp->axp_id) {
> +	case AXP209_ID:
>  		*val = (*val - 300000) / 100000;
> -	else
> +		break;
> +	case AXP221_ID:
>  		*val = (*val - 300000) / 150000;
> +		break;
> +	case AXP803_ID:
> +		*val = (*val - 200000) / 200000;
> +		/*
> +		 * The maximum charge current on AXP803 is 2.8A, and the
> +		 * datasheet says "1110-1111 reserved" in this part.
> +		 * So we return an invalid value -1 in this situation,
> +		 * which will be dealed by the caller of this function,
> +		 */

Good, we could do that for the two others compatible as well. They are
not explicitly marked as reserved but it stops at 1100 for AXP223/AXP221
for example.

> +		if (*val > 13)
> +			*val = -1;
> +		break;
> +	}
>  }
>  
>  static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
> @@ -269,9 +322,13 @@ 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))
> -			return -EINVAL;
> +		switch (axp20x_batt->axp_id) {
> +		case AXP221_ID:
> +		case AXP803_ID:
> +			if (!(reg & AXP22X_FG_VALID))
> +				return -EINVAL;
> +			break;
> +		};

Looks weird to me.

if (axp20x_batt->axp_id != AXP209_ID && !(reg & AXP22X_FG_VALID))

would be a better match?
[...]

Thanks,
Quentin
diff mbox

Patch

diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
index 7494f0f0eadb..c9a9fb320c92 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 AXP803_CHRG_CTRL1_TGT_4_35V	(3 << 5)
+
 #define AXP20X_CHRG_CTRL1_TGT_CURR	GENMASK(3, 0)
 
 #define AXP20X_V_OFF_MASK		GENMASK(2, 0)
@@ -123,20 +125,71 @@  static int axp22x_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
 	return 0;
 }
 
+static int axp803_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 AXP803_CHRG_CTRL1_TGT_4_35V:
+		*val = 4350000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static void raw_to_constant_charge_current(struct axp20x_batt_ps *axp, int *val)
 {
-	if (axp->axp_id == AXP209_ID)
+	switch (axp->axp_id) {
+	case AXP209_ID:
 		*val = *val * 100000 + 300000;
-	else
+		break;
+	case AXP221_ID:
 		*val = *val * 150000 + 300000;
+		break;
+	case AXP803_ID:
+		*val = *val * 200000 + 200000;
+		break;
+	}
 }
 
 static void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val)
 {
-	if (axp->axp_id == AXP209_ID)
+	switch (axp->axp_id) {
+	case AXP209_ID:
 		*val = (*val - 300000) / 100000;
-	else
+		break;
+	case AXP221_ID:
 		*val = (*val - 300000) / 150000;
+		break;
+	case AXP803_ID:
+		*val = (*val - 200000) / 200000;
+		/*
+		 * The maximum charge current on AXP803 is 2.8A, and the
+		 * datasheet says "1110-1111 reserved" in this part.
+		 * So we return an invalid value -1 in this situation,
+		 * which will be dealed by the caller of this function,
+		 */
+		if (*val > 13)
+			*val = -1;
+		break;
+	}
 }
 
 static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
@@ -269,9 +322,13 @@  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))
-			return -EINVAL;
+		switch (axp20x_batt->axp_id) {
+		case AXP221_ID:
+		case AXP803_ID:
+			if (!(reg & AXP22X_FG_VALID))
+				return -EINVAL;
+			break;
+		};
 
 		/*
 		 * Fuel Gauge data takes 7 bits but the stored value seems to be
@@ -281,11 +338,19 @@  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)
+		switch (axp20x_batt->axp_id) {
+		case AXP209_ID:
 			return axp20x_battery_get_max_voltage(axp20x_batt,
 							      &val->intval);
-		return axp22x_battery_get_max_voltage(axp20x_batt,
-						      &val->intval);
+		case AXP221_ID:
+			return axp22x_battery_get_max_voltage(axp20x_batt,
+							      &val->intval);
+		case AXP803_ID:
+			return axp803_battery_get_max_voltage(axp20x_batt,
+							      &val->intval);
+		default:
+			return -EINVAL;
+		}
 
 	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
 		ret = regmap_read(axp20x_batt->regmap, AXP20X_V_OFF, &reg);
@@ -467,6 +532,9 @@  static const struct of_device_id axp20x_battery_ps_id[] = {
 	}, {
 		.compatible = "x-powers,axp221-battery-power-supply",
 		.data = (void *)AXP221_ID,
+	}, {
+		.compatible = "x-powers,axp803-battery-power-supply",
+		.data = (void *)AXP803_ID,
 	}, { /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);