diff mbox series

power: supply: cpcap-charger: Limit voltage to 4.2V for battery

Message ID 20191009203355.5622-1-tony@atomide.com (mailing list archive)
State New, archived
Headers show
Series power: supply: cpcap-charger: Limit voltage to 4.2V for battery | expand

Commit Message

Tony Lindgren Oct. 9, 2019, 8:33 p.m. UTC
There have been some cases of droid4 battery bulging that seem to be
related to being left connected to the charger for several weeks.

It is suspected that the 4.35V charge voltage configured for the battery
is too much in the long run, so lets limit the charge voltage to 4.2V.
It could also be that the batteries are just getting old.

We don't really want to just change the charge voltage to 4.2V as Android
may have charged the battery to 3.51.V as pointed out by Pavel Machek.

To add checks for battery voltage, the driver needs to understand the
voltage it's charging at, and also needs to better understand it's
charger state. Right now it only understands connect and disconnect,
while now we need to know also a connected state but not charging.

So let's add better charger state handling with help of chrgcurr2 interrupt
for detecting charge full and retry, and add a check for battery voltage
before we start charging. And then we finally can lower the charge voltage
to 4.2V.

Note that we've been using the same register values as the Android distros
on droid4, so it is suspected that the same problem also exists in Android.

Cc: Pavel Machek <pavel@ucw.cz>
Cc: Rob Herring <robh+dt@kernel.org>
Reported-by: Merlijn Wajer <merlijn@wizzup.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

Sorry for dragging my feet on this, here's what I think we should apply
as a fix. It's a bit intrusive but gets us where we want to be for using
4.2V charge voltage.

This is against v5.3 so it can be easily picked for earlier kernels as
needed. If no changes needed, this is best applied on v5.3 and then merged
into v5.4-rc based branch as this will cause a minor merge conflict with
v5.4-rc because of 7f7378618b41 ("power: supply: cpcap-charger: Enable vbus
boost voltage").

---
 .../bindings/power/supply/cpcap-charger.txt   |   9 +-
 .../arm/boot/dts/motorola-cpcap-mapphone.dtsi |   6 +-
 drivers/power/supply/cpcap-charger.c          | 132 +++++++++++++++++-
 3 files changed, 140 insertions(+), 7 deletions(-)

Comments

Tony Lindgren Oct. 9, 2019, 8:56 p.m. UTC | #1
* Tony Lindgren <tony@atomide.com> [191009 13:34]:
> This is against v5.3 so it can be easily picked for earlier kernels as
> needed. If no changes needed, this is best applied on v5.3 and then merged
> into v5.4-rc based branch as this will cause a minor merge conflict with
> v5.4-rc because of 7f7378618b41 ("power: supply: cpcap-charger: Enable vbus
> boost voltage").

And below is the merge resolution I used for reference.

Regards,

Tony

8< ---------------------
diff --cc drivers/power/supply/cpcap-charger.c
--- a/drivers/power/supply/cpcap-charger.c
+++ b/drivers/power/supply/cpcap-charger.c
@@@ -447,9 -515,48 +531,49 @@@ static void cpcap_usb_detect(struct wor
  	if (error)
  		return;
  
+ 	/* Just init the state if a charger is connected with no chrg_det set */
+ 	if (!s.chrg_det && s.chrgcurr1 && s.vbusvld) {
+ 		cpcap_charger_update_state(ddata, CPCAP_CHARGER_DETECTING);
+ 
+ 		return;
+ 	}
+ 
+ 	/*
+ 	 * If battery voltage is higher than charge voltage, it may have been
+ 	 * charged to 3.51V by Android. Try again in 10 minutes.
+ 	 */
+ 	if (cpcap_charger_get_charge_voltage(ddata) > ddata->voltage) {
+ 		cpcap_charger_disconnect(ddata, CPCAP_CHARGER_DETECTING,
+ 					 HZ * 60 * 10);
+ 
+ 		return;
+ 	}
+ 
+ 	/* Throttle chrgcurr2 interrupt for charger done and retry */
+ 	switch (ddata->state) {
+ 	case CPCAP_CHARGER_CHARGING:
+ 		if (s.chrgcurr2)
+ 			break;
+ 		if (s.chrgcurr1 && s.vbusvld) {
+ 			cpcap_charger_disconnect(ddata, CPCAP_CHARGER_DONE,
+ 						 HZ * 5);
+ 			return;
+ 		}
+ 		break;
+ 	case CPCAP_CHARGER_DONE:
+ 		if (!s.chrgcurr2)
+ 			break;
+ 		cpcap_charger_disconnect(ddata, CPCAP_CHARGER_DETECTING,
+ 					 HZ * 5);
+ 		return;
+ 	default:
+ 		break;
+ 	}
+ 
 -	if (cpcap_charger_vbus_valid(ddata) && s.chrgcurr1) {
 +	if (!ddata->feeding_vbus && cpcap_charger_vbus_valid(ddata) &&
 +	    s.chrgcurr1) {
  		int max_current;
+ 		int vchrg;
  
  		if (cpcap_charger_battery_found(ddata))
  			max_current = CPCAP_REG_CRM_ICHRG_1A596;
Pavel Machek Oct. 13, 2019, 11:27 a.m. UTC | #2
Hi!

> There have been some cases of droid4 battery bulging that seem to be
> related to being left connected to the charger for several weeks.
> 
> It is suspected that the 4.35V charge voltage configured for the battery
> is too much in the long run, so lets limit the charge voltage to 4.2V.
> It could also be that the batteries are just getting old.
> 
> We don't really want to just change the charge voltage to 4.2V as Android
> may have charged the battery to 3.51.V as pointed out by Pavel
> Machek.

Now I'm confused. What is 3.51V? Is it typo for 4.35V?

> +	/*
> +	 * If battery voltage is higher than charge voltage, it may have been
> +	 * charged to 3.51V by Android. Try again in 10 minutes.
> +	 */

Ok, so maybe it is not a typo. I'm confused.

Note that I'd prefer not to see this in -stable.

Best regards,
									Pavel
Tony Lindgren Oct. 15, 2019, 5:22 p.m. UTC | #3
* Pavel Machek <pavel@ucw.cz> [191013 11:27]:
> Hi!
> 
> > There have been some cases of droid4 battery bulging that seem to be
> > related to being left connected to the charger for several weeks.
> > 
> > It is suspected that the 4.35V charge voltage configured for the battery
> > is too much in the long run, so lets limit the charge voltage to 4.2V.
> > It could also be that the batteries are just getting old.
> > 
> > We don't really want to just change the charge voltage to 4.2V as Android
> > may have charged the battery to 3.51.V as pointed out by Pavel
> > Machek.
> 
> Now I'm confused. What is 3.51V? Is it typo for 4.35V?
> 
> > +	/*
> > +	 * If battery voltage is higher than charge voltage, it may have been
> > +	 * charged to 3.51V by Android. Try again in 10 minutes.
> > +	 */
> 
> Ok, so maybe it is not a typo. I'm confused.

Hmm a long mistake.. No idea what I was thinking with 3.51..
Yeah both should be 4.35V.

> Note that I'd prefer not to see this in -stable.

OK. In that case I'll send a new version with the typos
fixed against v5.4-rc cycle instead of v5.3.

Regards,

Tony
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/cpcap-charger.txt b/Documentation/devicetree/bindings/power/supply/cpcap-charger.txt
--- a/Documentation/devicetree/bindings/power/supply/cpcap-charger.txt
+++ b/Documentation/devicetree/bindings/power/supply/cpcap-charger.txt
@@ -5,7 +5,8 @@  Required properties:
 - interrupts: Interrupt specifier for each name in interrupt-names
 - interrupt-names: Should contain the following entries:
 		   "chrg_det", "rvrs_chrg", "chrg_se1b", "se0conn",
-		   "rvrs_mode", "chrgcurr1", "vbusvld", "battdetb"
+		   "rvrs_mode", "chrgcurr2", "chrgcurr1", "vbusvld",
+		   "battdetb"
 - io-channels: IIO ADC channel specifier for each name in io-channel-names
 - io-channel-names: Should contain the following entries:
 		    "battdetb", "battp", "vbus", "chg_isense", "batti"
@@ -21,11 +22,13 @@  cpcap_charger: charger {
 	compatible = "motorola,mapphone-cpcap-charger";
 	interrupts-extended = <
 		&cpcap 13 0 &cpcap 12 0 &cpcap 29 0 &cpcap 28 0
-		&cpcap 22 0 &cpcap 20 0 &cpcap 19 0 &cpcap 54 0
+		&cpcap 22 0 &cpcap 21 0 &cpcap 20 0 &cpcap 19 0
+		&cpcap 54 0
 	>;
 	interrupt-names =
 		"chrg_det", "rvrs_chrg", "chrg_se1b", "se0conn",
-		"rvrs_mode", "chrgcurr1", "vbusvld", "battdetb";
+		"rvrs_mode", "chrgcurr2", "chrgcurr1", "vbusvld",
+		"battdetb";
 	mode-gpios = <&gpio3 29 GPIO_ACTIVE_LOW
 		      &gpio3 23 GPIO_ACTIVE_LOW>;
 	io-channels = <&cpcap_adc 0 &cpcap_adc 1
diff --git a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
--- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
+++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
@@ -43,11 +43,13 @@ 
 			compatible = "motorola,mapphone-cpcap-charger";
 			interrupts-extended = <
 				&cpcap 13 0 &cpcap 12 0 &cpcap 29 0 &cpcap 28 0
-				&cpcap 22 0 &cpcap 20 0 &cpcap 19 0 &cpcap 54 0
+				&cpcap 22 0 &cpcap 21 0 &cpcap 20 0 &cpcap 19 0
+				&cpcap 54 0
 			>;
 			interrupt-names =
 				"chrg_det", "rvrs_chrg", "chrg_se1b", "se0conn",
-				"rvrs_mode", "chrgcurr1", "vbusvld", "battdetb";
+				"rvrs_mode", "chrgcurr2", "chrgcurr1", "vbusvld",
+				"battdetb";
 			mode-gpios = <&gpio3 29 GPIO_ACTIVE_LOW
 				      &gpio3 23 GPIO_ACTIVE_LOW>;
 			io-channels = <&cpcap_adc 0 &cpcap_adc 1
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
@@ -117,6 +117,13 @@  enum {
 	CPCAP_CHARGER_IIO_NR,
 };
 
+enum {
+	CPCAP_CHARGER_DISCONNECTED,
+	CPCAP_CHARGER_DETECTING,
+	CPCAP_CHARGER_CHARGING,
+	CPCAP_CHARGER_DONE,
+};
+
 struct cpcap_charger_ddata {
 	struct device *dev;
 	struct regmap *reg;
@@ -134,6 +141,8 @@  struct cpcap_charger_ddata {
 	atomic_t active;
 
 	int status;
+	int state;
+	int voltage;
 };
 
 struct cpcap_interrupt_desc {
@@ -149,6 +158,7 @@  struct cpcap_charger_ints_state {
 
 	bool chrg_se1b;
 	bool rvrs_mode;
+	bool chrgcurr2;
 	bool chrgcurr1;
 	bool vbusvld;
 
@@ -406,6 +416,7 @@  static int cpcap_charger_get_ints_state(struct cpcap_charger_ddata *ddata,
 
 	s->chrg_se1b = val & BIT(13);
 	s->rvrs_mode = val & BIT(6);
+	s->chrgcurr2 = val & BIT(5);
 	s->chrgcurr1 = val & BIT(4);
 	s->vbusvld = val & BIT(3);
 
@@ -418,6 +429,79 @@  static int cpcap_charger_get_ints_state(struct cpcap_charger_ddata *ddata,
 	return 0;
 }
 
+static void cpcap_charger_update_state(struct cpcap_charger_ddata *ddata,
+				       int state)
+{
+	const char *status;
+
+	if (state > CPCAP_CHARGER_DONE) {
+		dev_warn(ddata->dev, "unknown state: %i\n", state);
+
+		return;
+	}
+
+	ddata->state = state;
+
+	switch (state) {
+	case CPCAP_CHARGER_DISCONNECTED:
+		status = "DISCONNECTED";
+		break;
+	case CPCAP_CHARGER_DETECTING:
+		status = "DETECTING";
+		break;
+	case CPCAP_CHARGER_CHARGING:
+		status = "CHARGING";
+		break;
+	case CPCAP_CHARGER_DONE:
+		status = "DONE";
+		break;
+	default:
+		return;
+	}
+
+	dev_dbg(ddata->dev, "state: %s\n", status);
+}
+
+int cpcap_charger_voltage_to_regval(int voltage)
+{
+	int offset;
+
+	switch (voltage) {
+	case 0 ... 4100000 - 1:
+		return 0;
+	case 4100000 ... 4200000 - 1:
+		offset = 1;
+		break;
+	case 4200000 ... 4300000 - 1:
+		offset = 0;
+		break;
+	case 4300000 ... 4380000 - 1:
+		offset = -1;
+		break;
+	case 4380000 ... 4440000:
+		offset = -2;
+		break;
+	default:
+		return 0;
+	}
+
+	return ((voltage - 4100000) / 20000) + offset;
+}
+
+static void cpcap_charger_disconnect(struct cpcap_charger_ddata *ddata,
+				     int state, unsigned long delay)
+{
+	int error;
+
+	error = cpcap_charger_set_state(ddata, 0, 0, 0);
+	if (error)
+		return;
+
+	cpcap_charger_update_state(ddata, state);
+	power_supply_changed(ddata->usb);
+	schedule_delayed_work(&ddata->detect_work, delay);
+}
+
 static void cpcap_usb_detect(struct work_struct *work)
 {
 	struct cpcap_charger_ddata *ddata;
@@ -431,23 +515,66 @@  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 (!s.chrg_det && s.chrgcurr1 && s.vbusvld) {
+		cpcap_charger_update_state(ddata, CPCAP_CHARGER_DETECTING);
+
+		return;
+	}
+
+	/*
+	 * If battery voltage is higher than charge voltage, it may have been
+	 * charged to 3.51V by Android. Try again in 10 minutes.
+	 */
+	if (cpcap_charger_get_charge_voltage(ddata) > ddata->voltage) {
+		cpcap_charger_disconnect(ddata, CPCAP_CHARGER_DETECTING,
+					 HZ * 60 * 10);
+
+		return;
+	}
+
+	/* Throttle chrgcurr2 interrupt for charger done and retry */
+	switch (ddata->state) {
+	case CPCAP_CHARGER_CHARGING:
+		if (s.chrgcurr2)
+			break;
+		if (s.chrgcurr1 && s.vbusvld) {
+			cpcap_charger_disconnect(ddata, CPCAP_CHARGER_DONE,
+						 HZ * 5);
+			return;
+		}
+		break;
+	case CPCAP_CHARGER_DONE:
+		if (!s.chrgcurr2)
+			break;
+		cpcap_charger_disconnect(ddata, CPCAP_CHARGER_DETECTING,
+					 HZ * 5);
+		return;
+	default:
+		break;
+	}
+
 	if (cpcap_charger_vbus_valid(ddata) && s.chrgcurr1) {
 		int max_current;
+		int vchrg;
 
 		if (cpcap_charger_battery_found(ddata))
 			max_current = CPCAP_REG_CRM_ICHRG_1A596;
 		else
 			max_current = CPCAP_REG_CRM_ICHRG_0A532;
 
+		vchrg = cpcap_charger_voltage_to_regval(ddata->voltage);
 		error = cpcap_charger_set_state(ddata,
-						CPCAP_REG_CRM_VCHRG_4V35,
+						CPCAP_REG_CRM_VCHRG(vchrg),
 						max_current, 0);
 		if (error)
 			goto out_err;
+		cpcap_charger_update_state(ddata, CPCAP_CHARGER_CHARGING);
 	} else {
 		error = cpcap_charger_set_state(ddata, 0, 0, 0);
 		if (error)
 			goto out_err;
+		cpcap_charger_update_state(ddata, CPCAP_CHARGER_DISCONNECTED);
 	}
 
 	power_supply_changed(ddata->usb);
@@ -507,7 +634,7 @@  static const char * const cpcap_charger_irqs[] = {
 	"chrg_det", "rvrs_chrg",
 
 	/* REG_INT1 */
-	"chrg_se1b", "se0conn", "rvrs_mode", "chrgcurr1", "vbusvld",
+	"chrg_se1b", "se0conn", "rvrs_mode", "chrgcurr2", "chrgcurr1", "vbusvld",
 
 	/* REG_INT_3 */
 	"battdetb",
@@ -608,6 +735,7 @@  static int cpcap_charger_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ddata->dev = &pdev->dev;
+	ddata->voltage = 4200000;
 
 	ddata->reg = dev_get_regmap(ddata->dev->parent, NULL);
 	if (!ddata->reg)