[PATCH 0/3] cpcap charger and battery fixes

[PATCH 0/3] cpcap charger and battery fixes

[Tony Lindgren]

Hi,

Here are few fixes for cpcap charger and battery.

The first fix lowers the battery charge voltage to 4.2V from 4.35V, and
should probably applied to the -rc series. The other two can wait if
preferred.

Regards,

Tony


Pavel Machek (1):
  power: supply: cpcap-charger: Limit voltage to 4.2V for battery

Tony Lindgren (2):
  power: supply: cpcap-battery: Check voltage before orderly_poweroff
  power: supply: cpcap-charger: Improve battery detection

 drivers/power/supply/cpcap-battery.c | 9 ++++++---
 drivers/power/supply/cpcap-charger.c | 9 +++++----
 2 files changed, 11 insertions(+), 7 deletions(-)

-- 
2.23.0

[PATCH 1/3] power: supply: cpcap-charger: Limit voltage to 4.2V for battery

[Tony Lindgren]

From: Pavel Machek <pavel@ucw.cz>

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.

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

Fixes: 3ae5f06681fc ("power: supply: cpcap-charger: Fix charge voltage configuration")
Reported-by: Merlijn Wajer <merlijn@wizzup.org>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
[tony@atomide.com: added description of the problem and fixes tag]
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/power/supply/cpcap-charger.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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
@@ -457,7 +457,7 @@ static void cpcap_usb_detect(struct work_struct *work)
 			max_current = CPCAP_REG_CRM_ICHRG_0A532;
 
 		error = cpcap_charger_set_state(ddata,
-						CPCAP_REG_CRM_VCHRG_4V35,
+						CPCAP_REG_CRM_VCHRG_4V20,
 						max_current, 0);
 		if (error)
 			goto out_err;
-- 
2.23.0

[PATCH 2/3] power: supply: cpcap-battery: Check voltage before orderly_poweroff

[Tony Lindgren]

We can get the low voltage interrupt trigger sometimes way too early,
maybe because of CPU load spikes. This causes orderly_poweroff() be
called too easily.

Let's check the voltage before orderly_poweroff in case it was not
yet a permanent condition. We will be getting more interrupts anyways
if the condition persists.

Let's also show the measured voltages for low battery and battery
empty warnings since we have them.

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 | 9 ++++++---
 1 file changed, 6 insertions(+), 3 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
@@ -562,12 +562,15 @@ static irqreturn_t cpcap_battery_irq_thread(int irq, void *data)
 	switch (d->action) {
 	case CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW:
 		if (latest->current_ua >= 0)
-			dev_warn(ddata->dev, "Battery low at 3.3V!\n");
+			dev_warn(ddata->dev, "Battery low at %i!\n",
+				latest->voltage);
 		break;
 	case CPCAP_BATTERY_IRQ_ACTION_POWEROFF:
-		if (latest->current_ua >= 0) {
+		if (latest->current_ua >= 0 && latest->voltage >= 0 &&
+		    latest->voltage <= 3100000) {
 			dev_emerg(ddata->dev,
-				  "Battery empty at 3.1V, powering off\n");
+				  "Battery empty at %i, powering off\n",
+				  latest->voltage);
 			orderly_poweroff(true);
 		}
 		break;
-- 
2.23.0

[PATCH 3/3] power: supply: cpcap-charger: Improve battery detection

[Tony Lindgren]

We are currently using a wrong ADC range for the battery detection.
The ADC returns the battery temperature if connected.

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 | 7 ++++---
 1 file changed, 4 insertions(+), 3 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
@@ -166,20 +166,21 @@ static enum power_supply_property cpcap_charger_props[] = {
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 };
 
+/* No battery always shows temperature of -40000 */
 static bool cpcap_charger_battery_found(struct cpcap_charger_ddata *ddata)
 {
 	struct iio_channel *channel;
-	int error, value;
+	int error, temperature;
 
 	channel = ddata->channels[CPCAP_CHARGER_IIO_BATTDET];
-	error = iio_read_channel_raw(channel, &value);
+	error = iio_read_channel_processed(channel, &temperature);
 	if (error < 0) {
 		dev_warn(ddata->dev, "%s failed: %i\n", __func__, error);
 
 		return false;
 	}
 
-	return value == 1;
+	return temperature > -20000 && temperature < 60000;
 }
 
 static int cpcap_charger_get_charge_voltage(struct cpcap_charger_ddata *ddata)
-- 
2.23.0

Re: [PATCH 1/3] power: supply: cpcap-charger: Limit voltage to 4.2V for battery

[Pavel Machek]

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

On Tue 2019-09-17 14:34:59, Tony Lindgren wrote:
> From: Pavel Machek <pavel@ucw.cz>
> 
> 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.

4.35V is known to make lifetime of battery shorter, but it provides
10% more capacity.

Disadvantage of this approach is that if droid is rebooted between
mainline and android, battery will go 4.35V->4.2V->4.35V... while on
charger.

I guess this patch still makes sense, I just wanted to make sure
disadvantages are mentioned.

Best regards,
								Pavel

> @@ -457,7 +457,7 @@ static void cpcap_usb_detect(struct work_struct *work)
>  			max_current = CPCAP_REG_CRM_ICHRG_0A532;
>  
>  		error = cpcap_charger_set_state(ddata,
> -						CPCAP_REG_CRM_VCHRG_4V35,
> +						CPCAP_REG_CRM_VCHRG_4V20,
>  						max_current, 0);
>  		if (error)
>  			goto out_err;

-- 
(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-battery: Check voltage before orderly_poweroff

[Pavel Machek]

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

On Tue 2019-09-17 14:35:00, Tony Lindgren wrote:
> We can get the low voltage interrupt trigger sometimes way too early,
> maybe because of CPU load spikes. This causes orderly_poweroff() be
> called too easily.
> 
> Let's check the voltage before orderly_poweroff in case it was not
> yet a permanent condition. We will be getting more interrupts anyways
> if the condition persists.
> 
> Let's also show the measured voltages for low battery and battery
> empty warnings since we have them.

Well, this is decision that will shorten battery lifetime. There's
very little capacity left when battery is down to 3.3V...

What kind of "way too early" do you see?

> @@ -562,12 +562,15 @@ static irqreturn_t cpcap_battery_irq_thread(int irq, void *data)
>  	switch (d->action) {
>  	case CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW:
>  		if (latest->current_ua >= 0)
> -			dev_warn(ddata->dev, "Battery low at 3.3V!\n");
> +			dev_warn(ddata->dev, "Battery low at %i!\n",
> +				latest->voltage);
>  		break;

I'd still leave unit ("uV"?) there. Or do /1000, as and display mV, as
our 
> -				  "Battery empty at 3.1V, powering off\n");
> +				  "Battery empty at %i, powering off\n",
> +				  latest->voltage);
>  			orderly_poweroff(true);

Same here.

Plus I see bigger problem: shutdown from mainline seems to leave
something powered in the phone (I believe I seen USB charge pump, for
example), so the battery will be completely empty next time I attempt
to use the phone. (I learned to reboot into stock android and shutdown
there).

Phone should last days when powered off, but it seems to only last
hours.

Unfortunately I don't know how to debug that :-(.

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 3/3] power: supply: cpcap-charger: Improve battery detection

[Pavel Machek]

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

Hi!

> We are currently using a wrong ADC range for the battery detection.
> The ADC returns the battery temperature if connected.

This one looks good.

> Cc: Merlijn Wajer <merlijn@wizzup.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

Would it also make sense to publish battery temperature somewhere? It
is somehow important for checking "what is going on" and it should
also be used to control charging. (Normal charging should only be
allowed in normal temperatures, like >10C and <30C or so..)

Thanks and best regards,
								Pavel


> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/power/supply/cpcap-charger.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 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
> @@ -166,20 +166,21 @@ static enum power_supply_property cpcap_charger_props[] = {
>  	POWER_SUPPLY_PROP_CURRENT_NOW,
>  };
>  
> +/* No battery always shows temperature of -40000 */
>  static bool cpcap_charger_battery_found(struct cpcap_charger_ddata *ddata)
>  {
>  	struct iio_channel *channel;
> -	int error, value;
> +	int error, temperature;
>  
>  	channel = ddata->channels[CPCAP_CHARGER_IIO_BATTDET];
> -	error = iio_read_channel_raw(channel, &value);
> +	error = iio_read_channel_processed(channel, &temperature);
>  	if (error < 0) {
>  		dev_warn(ddata->dev, "%s failed: %i\n", __func__, error);
>  
>  		return false;
>  	}
>  
> -	return value == 1;
> +	return temperature > -20000 && temperature < 60000;
>  }
>  
>  static int cpcap_charger_get_charge_voltage(struct cpcap_charger_ddata *ddata)

-- 
(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 1/3] power: supply: cpcap-charger: Limit voltage to 4.2V for battery

[Tony Lindgren]

Hi,

* Pavel Machek <pavel@ucw.cz> [190919 09:06]:
> On Tue 2019-09-17 14:34:59, Tony Lindgren wrote:
> > From: Pavel Machek <pavel@ucw.cz>
> > 
> > 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.
> 
> 4.35V is known to make lifetime of battery shorter, but it provides
> 10% more capacity.
> 
> Disadvantage of this approach is that if droid is rebooted between
> mainline and android, battery will go 4.35V->4.2V->4.35V... while on
> charger.

Yeah that's a valid concern, we can handle that in the driver
though, see below.

> I guess this patch still makes sense, I just wanted to make sure
> disadvantages are mentioned.

Well how about the following as a fix instead?

It's a bit intrusive for a fix, but I think it gets us where we
want to be with with charger state and voltage detection with
help of chrgcurr2 interrupt.

And it makes adding user configurable voltages trivial.

Regards,

Tony

8< -------------------
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,63 @@ 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;
+		cpcap_charger_disconnect(ddata, CPCAP_CHARGER_DONE,
+					 HZ * 5);
+		return;
+	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 +631,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 +732,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)
-- 
2.23.0

Re: [PATCH 2/3] power: supply: cpcap-battery: Check voltage before orderly_poweroff

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [190919 09:15]:
> On Tue 2019-09-17 14:35:00, Tony Lindgren wrote:
> > We can get the low voltage interrupt trigger sometimes way too early,
> > maybe because of CPU load spikes. This causes orderly_poweroff() be
> > called too easily.
> > 
> > Let's check the voltage before orderly_poweroff in case it was not
> > yet a permanent condition. We will be getting more interrupts anyways
> > if the condition persists.
> > 
> > Let's also show the measured voltages for low battery and battery
> > empty warnings since we have them.
> 
> Well, this is decision that will shorten battery lifetime. There's
> very little capacity left when battery is down to 3.3V...
> 
> What kind of "way too early" do you see?

I've seen it trigger spontaneously already around battery low
interrupt time.

> > @@ -562,12 +562,15 @@ static irqreturn_t cpcap_battery_irq_thread(int irq, void *data)
> >  	switch (d->action) {
> >  	case CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW:
> >  		if (latest->current_ua >= 0)
> > -			dev_warn(ddata->dev, "Battery low at 3.3V!\n");
> > +			dev_warn(ddata->dev, "Battery low at %i!\n",
> > +				latest->voltage);
> >  		break;
> 
> I'd still leave unit ("uV"?) there. Or do /1000, as and display mV, as
> our 
> > -				  "Battery empty at 3.1V, powering off\n");
> > +				  "Battery empty at %i, powering off\n",
> > +				  latest->voltage);
> >  			orderly_poweroff(true);
> 
> Same here.

Sure yeah I'll update it.

> Plus I see bigger problem: shutdown from mainline seems to leave
> something powered in the phone (I believe I seen USB charge pump, for
> example), so the battery will be completely empty next time I attempt
> to use the phone. (I learned to reboot into stock android and shutdown
> there).
> 
> Phone should last days when powered off, but it seems to only last
> hours.
> 
> Unfortunately I don't know how to debug that :-(.

Yes there's some issue with shutdown. I think it's somehow related
to mdm6600 being powered where the poweroff gpio does not allow
device to shut down with modem powered. We could try adding a
.power_off function to the modem code to see if it helps.

Additionally I've noticed that we leave some PMIC features powered
when device is powered off without a modem consuming about 2.5mW
while powering off from Android shows power consumption in uW
range probably with only RTC being powered.

Regards,

Tony

Re: [PATCH 3/3] power: supply: cpcap-charger: Improve battery detection

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [190919 09:20]:
> Would it also make sense to publish battery temperature somewhere? It
> is somehow important for checking "what is going on" and it should
> also be used to control charging. (Normal charging should only be
> allowed in normal temperatures, like >10C and <30C or so..)

We have it show up for battery already, not for usb charger
since it's battery value. The usb charger just uses it for
battery detection.

Regards,

Tony

Re: [PATCH 2/3] power: supply: cpcap-battery: Check voltage before orderly_poweroff

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [190920 14:13]:
> * Pavel Machek <pavel@ucw.cz> [190919 09:15]:
> > Plus I see bigger problem: shutdown from mainline seems to leave
> > something powered in the phone (I believe I seen USB charge pump, for
> > example), so the battery will be completely empty next time I attempt
> > to use the phone. (I learned to reboot into stock android and shutdown
> > there).
> > 
> > Phone should last days when powered off, but it seems to only last
> > hours.
> > 
> > Unfortunately I don't know how to debug that :-(.
> 
> Yes there's some issue with shutdown. I think it's somehow related
> to mdm6600 being powered where the poweroff gpio does not allow
> device to shut down with modem powered. We could try adding a
> .power_off function to the modem code to see if it helps.

Sorry I mean .shutdown function. But I doubt this is a modem
issue you're seeing, I already fixed that issue most likely with
commit 8ead7e817224 ("usb: core: Add PM runtime calls to
usb_hcd_platform_shutdown"). Well at least things are shutting
down for me now after checking few times. I recall the symptoms
of the shut down failing issue is that the also the LCD backlight
stays on.

There are some issues left with musb configured as a usb
gadget, but I have not been able to quite track those down
so far. It seems that there are some gadget framework kobject
warnings with the musb controlling device (omap2430) unloaded
and then doing a shutdown. The device shuts down though.

> Additionally I've noticed that we leave some PMIC features powered
> when device is powered off without a modem consuming about 2.5mW
> while powering off from Android shows power consumption in uW
> range probably with only RTC being powered.

AFAIK this issue still remains. I'll take a look at adding a
.shutdown somewhere for cpcap driver(s) so we get it cleared
for a low-power state.

Regards,

Tony