[PATCH 0/3] cpcap charger and battery changes to deal with dropped voltage

[PATCH 0/3] cpcap charger and battery changes to deal with dropped voltage

[Tony Lindgren]

Hi,

With lowered charge voltage fixes in set "[PATCH 0/3] cpcap charger and
battery fixes", the first two patches deal with the lower voltage using
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE and make it configurable.

The third patch makes charging work for higher rates by lowering the
charge current basedon chrgcurr1 interrupts.

As far as I'm concerned, all these can easily wait for v5.5 merge window.

Regards,

Tony


Tony Lindgren (3):
  power: supply: cpcap-battery: Fix handling of lowered charger voltage
  power: supply: cpcap-charger: Allow changing constant charge voltage
  power: supply: cpcap-charger: Adjust current based on charger
    interrupts

 drivers/power/supply/cpcap-battery.c |  86 +++++++++++-
 drivers/power/supply/cpcap-charger.c | 188 ++++++++++++++++++++++++---
 2 files changed, 250 insertions(+), 24 deletions(-)

-- 
2.23.0

[PATCH 1/3] power: supply: cpcap-battery: Fix handling of lowered charger voltage

[Tony Lindgren]

With cpcap-charger now using 4.2V instead of 4.35V, we never reach
POWER_SUPPLY_CAPACITY_LEVEL_FULL unless we handle the lowered charge
voltage.

Let's do this by implementing POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
and assume anything at that level or higher is a full battery.

Let's also make it configurable for users who may still want to
reconfigure it, and notify the charger if supported by the charger.

Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/power/supply/cpcap-battery.c | 86 +++++++++++++++++++++++++---
 1 file changed, 79 insertions(+), 7 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -79,6 +79,7 @@ struct cpcap_battery_config {
 	int ccm;
 	int cd_factor;
 	struct power_supply_info info;
+	struct power_supply_battery_info bat;
 };
 
 struct cpcap_coulomb_counter_data {
@@ -369,8 +370,8 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
 {
 	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
 
-	/* Basically anything that measures above 4347000 is full */
-	if (state->voltage >= (ddata->config.info.voltage_max_design - 4000))
+	if (state->voltage >=
+	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000))
 		return true;
 
 	return false;
@@ -417,6 +418,7 @@ static enum power_supply_property cpcap_battery_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
 	POWER_SUPPLY_PROP_CURRENT_AVG,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
@@ -475,6 +477,9 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
 		val->intval = ddata->config.info.voltage_min_design;
 		break;
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		val->intval = ddata->config.bat.constant_charge_voltage_max_uv;
+		break;
 	case POWER_SUPPLY_PROP_CURRENT_AVG:
 		sample = latest->cc.sample - previous->cc.sample;
 		if (!sample) {
@@ -540,6 +545,70 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 	return 0;
 }
 
+static int cpcap_battery_update_charger(struct cpcap_battery_ddata *ddata,
+					int const_charge_voltage)
+{
+	union power_supply_propval prop;
+	union power_supply_propval val;
+	struct power_supply *charger;
+	int error;
+
+	charger = power_supply_get_by_name("usb");
+	if (!charger)
+		return -ENODEV;
+
+	error = power_supply_get_property(charger,
+				POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+				&prop);
+	if (error)
+		return error;
+
+	/* Allow charger const voltage lower than battery const voltage */
+	if (const_charge_voltage > prop.intval)
+		return 0;
+
+	val.intval = const_charge_voltage;
+
+	return power_supply_set_property(charger,
+			POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+			&val);
+}
+
+static int cpcap_battery_set_property(struct power_supply *psy,
+				      enum power_supply_property psp,
+				      const union power_supply_propval *val)
+{
+	struct cpcap_battery_ddata *ddata = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		if (val->intval < ddata->config.info.voltage_min_design)
+			return -EINVAL;
+		if (val->intval > ddata->config.info.voltage_max_design)
+			return -EINVAL;
+
+		ddata->config.bat.constant_charge_voltage_max_uv = val->intval;
+
+		return cpcap_battery_update_charger(ddata, val->intval);
+	break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int cpcap_battery_property_is_writeable(struct power_supply *psy,
+					       enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
 static irqreturn_t cpcap_battery_irq_thread(int irq, void *data)
 {
 	struct cpcap_battery_ddata *ddata = data;
@@ -696,6 +765,7 @@ static const struct cpcap_battery_config cpcap_battery_default_data = {
 	.info.voltage_max_design = 4351000,
 	.info.voltage_min_design = 3100000,
 	.info.charge_full_design = 1740000,
+	.bat.constant_charge_voltage_max_uv = 4200000,
 };
 
 #ifdef CONFIG_OF
@@ -763,11 +833,13 @@ static int cpcap_battery_probe(struct platform_device *pdev)
 	if (!psy_desc)
 		return -ENOMEM;
 
-	psy_desc->name = "battery",
-	psy_desc->type = POWER_SUPPLY_TYPE_BATTERY,
-	psy_desc->properties = cpcap_battery_props,
-	psy_desc->num_properties = ARRAY_SIZE(cpcap_battery_props),
-	psy_desc->get_property = cpcap_battery_get_property,
+	psy_desc->name = "battery";
+	psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
+	psy_desc->properties = cpcap_battery_props;
+	psy_desc->num_properties = ARRAY_SIZE(cpcap_battery_props);
+	psy_desc->get_property = cpcap_battery_get_property;
+	psy_desc->set_property = cpcap_battery_set_property;
+	psy_desc->property_is_writeable = cpcap_battery_property_is_writeable;
 
 	psy_cfg.of_node = pdev->dev.of_node;
 	psy_cfg.drv_data = ddata;
-- 
2.23.0

[PATCH 2/3] power: supply: cpcap-charger: Allow changing constant charge voltage

[Tony Lindgren]

Let's allow reconfiguring the cpcap-charger max charge voltage and
default to 4.2V that should be safe for the known users.

This allows the users to use 4.35V for the extra capacity if really
needed at a cost of probably shorter battery life. We check the
constant charge voltage limit set by the battery.

Some pieces of the property setting code is based on an earlier patch
from Pavel Machek <pavel@ucw.cz> but limited to configuring the charge
voltage for now.

Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/power/supply/cpcap-charger.c | 143 +++++++++++++++++++++++----
 1 file changed, 126 insertions(+), 17 deletions(-)

diff --git a/drivers/power/supply/cpcap-charger.c b/drivers/power/supply/cpcap-charger.c
--- a/drivers/power/supply/cpcap-charger.c
+++ b/drivers/power/supply/cpcap-charger.c
@@ -67,23 +67,48 @@
  * Note that these register bits don't match MC13783UG.pdf VCHRG
  * register bits.
  */
+enum {
+	CPCAP_REG_CRM_VCHRG_3V80,
+	CPCAP_REG_CRM_VCHRG_4V10,
+	CPCAP_REG_CRM_VCHRG_4V12,
+	CPCAP_REG_CRM_VCHRG_4V15,
+	CPCAP_REG_CRM_VCHRG_4V17,
+	CPCAP_REG_CRM_VCHRG_4V20,
+	CPCAP_REG_CRM_VCHRG_4V23,
+	CPCAP_REG_CRM_VCHRG_4V25,
+	CPCAP_REG_CRM_VCHRG_4V27,
+	CPCAP_REG_CRM_VCHRG_4V30,
+	CPCAP_REG_CRM_VCHRG_4V33,
+	CPCAP_REG_CRM_VCHRG_4V35,
+	CPCAP_REG_CRM_VCHRG_4V38,
+	CPCAP_REG_CRM_VCHRG_4V40,
+	CPCAP_REG_CRM_VCHRG_4V42,
+	CPCAP_REG_CRM_VCHRG_4V44,
+};
+
+const int cpcap_charge_voltage[] = {
+	[CPCAP_REG_CRM_VCHRG_3V80] = 3800000,
+	[CPCAP_REG_CRM_VCHRG_4V10] = 4100000,
+	[CPCAP_REG_CRM_VCHRG_4V12] = 4120000,
+	[CPCAP_REG_CRM_VCHRG_4V15] = 4150000,
+	[CPCAP_REG_CRM_VCHRG_4V17] = 4170000,
+	[CPCAP_REG_CRM_VCHRG_4V20] = 4200000,
+	[CPCAP_REG_CRM_VCHRG_4V23] = 4230000,
+	[CPCAP_REG_CRM_VCHRG_4V25] = 4250000,
+	[CPCAP_REG_CRM_VCHRG_4V27] = 4270000,
+	[CPCAP_REG_CRM_VCHRG_4V30] = 4300000,
+	[CPCAP_REG_CRM_VCHRG_4V33] = 4330000,
+	[CPCAP_REG_CRM_VCHRG_4V35] = 4350000,
+	[CPCAP_REG_CRM_VCHRG_4V38] = 4380000,
+	[CPCAP_REG_CRM_VCHRG_4V40] = 4400000,
+	[CPCAP_REG_CRM_VCHRG_4V42] = 4420000,
+	[CPCAP_REG_CRM_VCHRG_4V44] = 4440000,
+};
+
 #define CPCAP_REG_CRM_VCHRG(val)	(((val) & 0xf) << 4)
-#define CPCAP_REG_CRM_VCHRG_3V80	CPCAP_REG_CRM_VCHRG(0x0)
-#define CPCAP_REG_CRM_VCHRG_4V10	CPCAP_REG_CRM_VCHRG(0x1)
-#define CPCAP_REG_CRM_VCHRG_4V12	CPCAP_REG_CRM_VCHRG(0x2)
-#define CPCAP_REG_CRM_VCHRG_4V15	CPCAP_REG_CRM_VCHRG(0x3)
-#define CPCAP_REG_CRM_VCHRG_4V17	CPCAP_REG_CRM_VCHRG(0x4)
-#define CPCAP_REG_CRM_VCHRG_4V20	CPCAP_REG_CRM_VCHRG(0x5)
-#define CPCAP_REG_CRM_VCHRG_4V23	CPCAP_REG_CRM_VCHRG(0x6)
-#define CPCAP_REG_CRM_VCHRG_4V25	CPCAP_REG_CRM_VCHRG(0x7)
-#define CPCAP_REG_CRM_VCHRG_4V27	CPCAP_REG_CRM_VCHRG(0x8)
-#define CPCAP_REG_CRM_VCHRG_4V30	CPCAP_REG_CRM_VCHRG(0x9)
-#define CPCAP_REG_CRM_VCHRG_4V33	CPCAP_REG_CRM_VCHRG(0xa)
-#define CPCAP_REG_CRM_VCHRG_4V35	CPCAP_REG_CRM_VCHRG(0xb)
-#define CPCAP_REG_CRM_VCHRG_4V38	CPCAP_REG_CRM_VCHRG(0xc)
-#define CPCAP_REG_CRM_VCHRG_4V40	CPCAP_REG_CRM_VCHRG(0xd)
-#define CPCAP_REG_CRM_VCHRG_4V42	CPCAP_REG_CRM_VCHRG(0xe)
-#define CPCAP_REG_CRM_VCHRG_4V44	CPCAP_REG_CRM_VCHRG(0xf)
+#define CPCAP_REG_CRM_VCHRG_MAX		CPCAP_REG_CRM_VCHRG_4V44
+#define CPCAP_CHARGER_DEFAULT_VOLTAGE	\
+	cpcap_charge_voltage[CPCAP_REG_CRM_VCHRG_4V20]
 
 /*
  * CPCAP_REG_CRM charge currents. These seem to match MC13783UG.pdf
@@ -135,6 +160,7 @@ struct cpcap_charger_ddata {
 	struct phy_companion comparator;	/* For USB VBUS */
 	unsigned int vbus_enabled:1;
 	unsigned int feeding_vbus:1;
+	int const_charge_voltage;
 	atomic_t active;
 
 	int status;
@@ -162,6 +188,7 @@ struct cpcap_charger_ints_state {
 static enum power_supply_property cpcap_charger_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 };
@@ -225,6 +252,9 @@ static int cpcap_charger_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_STATUS:
 		val->intval = ddata->status;
 		break;
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		val->intval = ddata->const_charge_voltage;
+		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
 		if (ddata->status == POWER_SUPPLY_STATUS_CHARGING)
 			val->intval = cpcap_charger_get_charge_voltage(ddata) *
@@ -249,6 +279,79 @@ static int cpcap_charger_get_property(struct power_supply *psy,
 	return 0;
 }
 
+static int cpcap_charger_match_voltage(struct cpcap_charger_ddata *ddata,
+				       int voltage, int *regval)
+{
+	int i, v = 0;
+
+	for (i = CPCAP_REG_CRM_VCHRG_MAX; i >= 0; i--) {
+		v = cpcap_charge_voltage[i];
+		if (voltage >= v) {
+			*regval = i;
+			dev_dbg(ddata->dev, "%s: voltage: %i regval: %02x\n",
+				__func__, v, *regval);
+			break;
+		}
+	}
+
+	return v;
+}
+
+static int
+cpcap_charger_get_bat_const_charge_voltage(struct cpcap_charger_ddata *ddata)
+{
+	union power_supply_propval prop;
+	struct power_supply *battery;
+	int voltage = CPCAP_CHARGER_DEFAULT_VOLTAGE;
+	int error;
+
+	battery = power_supply_get_by_name("battery");
+	if (battery) {
+		error = power_supply_get_property(battery,
+				POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+				&prop);
+		if (!error)
+			voltage = prop.intval;
+	}
+
+	return voltage;
+}
+
+static int cpcap_charger_set_property(struct power_supply *psy,
+				      enum power_supply_property psp,
+				      const union power_supply_propval *val)
+{
+	struct cpcap_charger_ddata *ddata = dev_get_drvdata(psy->dev.parent);
+	int voltage, batvolt, unused = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		voltage = cpcap_charger_match_voltage(ddata, val->intval,
+						      &unused);
+		batvolt = cpcap_charger_get_bat_const_charge_voltage(ddata);
+		if (voltage > batvolt)
+			voltage = batvolt;
+		ddata->const_charge_voltage = voltage;
+		schedule_delayed_work(&ddata->detect_work, 0);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int cpcap_charger_property_is_writeable(struct power_supply *psy,
+					       enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
 static void cpcap_charger_set_cable_path(struct cpcap_charger_ddata *ddata,
 					 bool enabled)
 {
@@ -451,14 +554,17 @@ static void cpcap_usb_detect(struct work_struct *work)
 	if (!ddata->feeding_vbus && cpcap_charger_vbus_valid(ddata) &&
 	    s.chrgcurr1) {
 		int max_current;
+		int vchrg = 0;
 
 		if (cpcap_charger_battery_found(ddata))
 			max_current = CPCAP_REG_CRM_ICHRG_1A596;
 		else
 			max_current = CPCAP_REG_CRM_ICHRG_0A532;
 
+		cpcap_charger_match_voltage(ddata, ddata->const_charge_voltage,
+					    &vchrg);
 		error = cpcap_charger_set_state(ddata,
-						CPCAP_REG_CRM_VCHRG_4V20,
+						CPCAP_REG_CRM_VCHRG(vchrg),
 						max_current, 0);
 		if (error)
 			goto out_err;
@@ -597,6 +703,8 @@ static const struct power_supply_desc cpcap_charger_usb_desc = {
 	.properties	= cpcap_charger_props,
 	.num_properties	= ARRAY_SIZE(cpcap_charger_props),
 	.get_property	= cpcap_charger_get_property,
+	.set_property	= cpcap_charger_set_property,
+	.property_is_writeable = cpcap_charger_property_is_writeable,
 };
 
 #ifdef CONFIG_OF
@@ -626,6 +734,7 @@ static int cpcap_charger_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ddata->dev = &pdev->dev;
+	ddata->const_charge_voltage = CPCAP_CHARGER_DEFAULT_VOLTAGE;
 
 	ddata->reg = dev_get_regmap(ddata->dev->parent, NULL);
 	if (!ddata->reg)
-- 
2.23.0

[PATCH 3/3] power: supply: cpcap-charger: Adjust current based on charger interrupts

[Tony Lindgren]

When debugging why higher than 500 mA charge current does not work, I
noticed that we start getting lots of chrgcurr1 interrupts if we attempt
to charge at rates higher than the charger can provide.

We can take advantage of the chrgcurr1 interrupts for charger detection,
and retry charging at a lower rate if charging fails. When an acceptable
charge rate is found, the chrgcurr1 interrupts stop.

Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/power/supply/cpcap-charger.c | 45 ++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/power/supply/cpcap-charger.c b/drivers/power/supply/cpcap-charger.c
--- a/drivers/power/supply/cpcap-charger.c
+++ b/drivers/power/supply/cpcap-charger.c
@@ -145,6 +145,12 @@ enum {
 	CPCAP_CHARGER_IIO_NR,
 };
 
+enum {
+	CPCAP_CHARGER_DISCONNECTED,
+	CPCAP_CHARGER_DETECTING,
+	CPCAP_CHARGER_CONNECTED,
+};
+
 struct cpcap_charger_ddata {
 	struct device *dev;
 	struct regmap *reg;
@@ -161,6 +167,9 @@ struct cpcap_charger_ddata {
 	unsigned int vbus_enabled:1;
 	unsigned int feeding_vbus:1;
 	int const_charge_voltage;
+	int state;
+	int last_current;
+	int last_current_retries;
 	atomic_t active;
 
 	int status;
@@ -551,6 +560,15 @@ static void cpcap_usb_detect(struct work_struct *work)
 	if (error)
 		return;
 
+	/* Just init the state if a charger is connected with no chrg_det set */
+	if (!ddata->feeding_vbus && !s.chrg_det && s.chrgcurr1 && s.vbusvld) {
+		ddata->state = CPCAP_CHARGER_DETECTING;
+		ddata->last_current = 0;
+
+		return;
+	}
+
+	/* Start charger on chrgcurr1, stop chrger otherwise */
 	if (!ddata->feeding_vbus && cpcap_charger_vbus_valid(ddata) &&
 	    s.chrgcurr1) {
 		int max_current;
@@ -561,6 +579,32 @@ static void cpcap_usb_detect(struct work_struct *work)
 		else
 			max_current = CPCAP_REG_CRM_ICHRG_0A532;
 
+		switch (ddata->state) {
+		case CPCAP_CHARGER_DETECTING:
+			ddata->state = CPCAP_CHARGER_CONNECTED;
+			ddata->last_current_retries = 0;
+			break;
+		case CPCAP_CHARGER_DISCONNECTED:
+			if (ddata->last_current > CPCAP_REG_CRM_ICHRG_0A532) {
+				/* Attempt current 3 times before lowering */
+				if (ddata->last_current_retries++ >= 3) {
+					ddata->last_current--;
+					ddata->last_current_retries = 0;
+					/* Wait a bit for voltage to ramp up */
+					usleep_range(40000, 50000);
+				}
+				max_current = ddata->last_current;
+			}
+			ddata->state = CPCAP_CHARGER_CONNECTED;
+			dev_info(ddata->dev, "enabling charger with current %i\n",
+				 max_current);
+			break;
+		default:
+			ddata->last_current_retries = 0;
+			break;
+		}
+
+		ddata->last_current = max_current;
 		cpcap_charger_match_voltage(ddata, ddata->const_charge_voltage,
 					    &vchrg);
 		error = cpcap_charger_set_state(ddata,
@@ -569,6 +613,7 @@ static void cpcap_usb_detect(struct work_struct *work)
 		if (error)
 			goto out_err;
 	} else {
+		ddata->state = CPCAP_CHARGER_DISCONNECTED;
 		error = cpcap_charger_set_state(ddata, 0, 0, 0);
 		if (error)
 			goto out_err;
-- 
2.23.0

Re: [PATCH 2/3] power: supply: cpcap-charger: Allow changing constant charge voltage

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

Hi!

> Let's allow reconfiguring the cpcap-charger max charge voltage and
> default to 4.2V that should be safe for the known users.
> 
> This allows the users to use 4.35V for the extra capacity if really
> needed at a cost of probably shorter battery life. We check the
> constant charge voltage limit set by the battery.
> 
> Some pieces of the property setting code is based on an earlier patch
> from Pavel Machek <pavel@ucw.cz> but limited to configuring the charge
> voltage for now.

I'm sorry I'm a tiny bit busy at the moment.

> +const int cpcap_charge_voltage[] = {
> +	[CPCAP_REG_CRM_VCHRG_3V80] = 3800000,
> +	[CPCAP_REG_CRM_VCHRG_4V10] = 4100000,
> +	[CPCAP_REG_CRM_VCHRG_4V12] = 4120000,
> +	[CPCAP_REG_CRM_VCHRG_4V15] = 4150000,
> +	[CPCAP_REG_CRM_VCHRG_4V17] = 4170000,
> +	[CPCAP_REG_CRM_VCHRG_4V20] = 4200000,
> +	[CPCAP_REG_CRM_VCHRG_4V23] = 4230000,
> +	[CPCAP_REG_CRM_VCHRG_4V25] = 4250000,
> +	[CPCAP_REG_CRM_VCHRG_4V27] = 4270000,
> +	[CPCAP_REG_CRM_VCHRG_4V30] = 4300000,
> +	[CPCAP_REG_CRM_VCHRG_4V33] = 4330000,
> +	[CPCAP_REG_CRM_VCHRG_4V35] = 4350000,
> +	[CPCAP_REG_CRM_VCHRG_4V38] = 4380000,
> +	[CPCAP_REG_CRM_VCHRG_4V40] = 4400000,
> +	[CPCAP_REG_CRM_VCHRG_4V42] = 4420000,
> +	[CPCAP_REG_CRM_VCHRG_4V44] = 4440000,
> +};

We really don't need this kind of explicit table, as the values can be
simply computed. Can I offer this?

Best regards,
								Pavel

static int voltage_to_register(int microvolt)
{
	int milivolt = microvolt/1000;
	int res;

	if (milivolt < 4100)
		return CPCAP_REG_CRM_VCHRG_3V80;
	if (milivolt > 4350)
		return -EINVAL;

	milivolt = milivolt - (4100 - 250);
	res = milivolt / 250;
	BUG_ON(res < 1);
	BUG_ON(res > 0xb);
	return CPCAP_REG_CRM_VCHRG(res);
}

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 3/3] power: supply: cpcap-charger: Adjust current based on charger interrupts

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

On Tue 2019-09-17 14:52:53, Tony Lindgren wrote:
> When debugging why higher than 500 mA charge current does not work, I
> noticed that we start getting lots of chrgcurr1 interrupts if we attempt
> to charge at rates higher than the charger can provide.
> 
> We can take advantage of the chrgcurr1 interrupts for charger detection,
> and retry charging at a lower rate if charging fails. When an acceptable
> charge rate is found, the chrgcurr1 interrupts stop.

Do you still see these problems with "good" charger? (Wall one,
capable of providing 2A)?

Note that 1A charging will decrease battery lifetime, and that phone
definitely should not be charging with more than 500mA when charging
from computer. I actually prefer the way it charges slowly in mainline...

We'll eventually need a library or something; we don't want every
driver to reinvent charging code..

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 2/3] power: supply: cpcap-charger: Allow changing constant charge voltage

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [190919 09:27]:
> Hi!
> 
> > Let's allow reconfiguring the cpcap-charger max charge voltage and
> > default to 4.2V that should be safe for the known users.
> > 
> > This allows the users to use 4.35V for the extra capacity if really
> > needed at a cost of probably shorter battery life. We check the
> > constant charge voltage limit set by the battery.
> > 
> > Some pieces of the property setting code is based on an earlier patch
> > from Pavel Machek <pavel@ucw.cz> but limited to configuring the charge
> > voltage for now.
> 
> I'm sorry I'm a tiny bit busy at the moment.
> 
> > +const int cpcap_charge_voltage[] = {
> > +	[CPCAP_REG_CRM_VCHRG_3V80] = 3800000,
> > +	[CPCAP_REG_CRM_VCHRG_4V10] = 4100000,
> > +	[CPCAP_REG_CRM_VCHRG_4V12] = 4120000,
> > +	[CPCAP_REG_CRM_VCHRG_4V15] = 4150000,
> > +	[CPCAP_REG_CRM_VCHRG_4V17] = 4170000,
> > +	[CPCAP_REG_CRM_VCHRG_4V20] = 4200000,
> > +	[CPCAP_REG_CRM_VCHRG_4V23] = 4230000,
> > +	[CPCAP_REG_CRM_VCHRG_4V25] = 4250000,
> > +	[CPCAP_REG_CRM_VCHRG_4V27] = 4270000,
> > +	[CPCAP_REG_CRM_VCHRG_4V30] = 4300000,
> > +	[CPCAP_REG_CRM_VCHRG_4V33] = 4330000,
> > +	[CPCAP_REG_CRM_VCHRG_4V35] = 4350000,
> > +	[CPCAP_REG_CRM_VCHRG_4V38] = 4380000,
> > +	[CPCAP_REG_CRM_VCHRG_4V40] = 4400000,
> > +	[CPCAP_REG_CRM_VCHRG_4V42] = 4420000,
> > +	[CPCAP_REG_CRM_VCHRG_4V44] = 4440000,
> > +};
> 
> We really don't need this kind of explicit table, as the values can be
> simply computed. Can I offer this?
> 
> Best regards,
> 								Pavel
> 
> static int voltage_to_register(int microvolt)
> {
> 	int milivolt = microvolt/1000;
> 	int res;
> 
> 	if (milivolt < 4100)
> 		return CPCAP_REG_CRM_VCHRG_3V80;
> 	if (milivolt > 4350)
> 		return -EINVAL;
> 
> 	milivolt = milivolt - (4100 - 250);
> 	res = milivolt / 250;
> 	BUG_ON(res < 1);
> 	BUG_ON(res > 0xb);
> 	return CPCAP_REG_CRM_VCHRG(res);
> }

Well that does not help as we have four different ranges there.
I got something figured out for the new suggested fix I posted.

Regards,

TOny

Re: [PATCH 3/3] power: supply: cpcap-charger: Adjust current based on charger interrupts

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [190919 09:35]:
> On Tue 2019-09-17 14:52:53, Tony Lindgren wrote:
> > When debugging why higher than 500 mA charge current does not work, I
> > noticed that we start getting lots of chrgcurr1 interrupts if we attempt
> > to charge at rates higher than the charger can provide.
> > 
> > We can take advantage of the chrgcurr1 interrupts for charger detection,
> > and retry charging at a lower rate if charging fails. When an acceptable
> > charge rate is found, the chrgcurr1 interrupts stop.
> 
> Do you still see these problems with "good" charger? (Wall one,
> capable of providing 2A)?

Yes, need to recheck again with the updated fix I posted.

> Note that 1A charging will decrease battery lifetime, and that phone
> definitely should not be charging with more than 500mA when charging
> from computer. I actually prefer the way it charges slowly in mainline...

It should still charge at 500mA when connected to a computer
because of different charger detection bits. Needs to be checked
again .though

> We'll eventually need a library or something; we don't want every
> driver to reinvent charging code..

Yeah currently implementing a charger takes weeks of work :)

Regards,

Tony