| Message ID | 20190917213501.16907-3-tony@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cpcap charger and battery fixes | expand |
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
* 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
* 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
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;
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(-)