[1/2] power: supply: cpcap-battery: Check voltage before orderly_poweroff
diff mbox series

Message ID 20191009205252.9510-2-tony@atomide.com
State New
Headers show
Series
  • cpcap charger and battery fixes
Related show

Commit Message

Tony Lindgren Oct. 9, 2019, 8:52 p.m. UTC
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(-)

Comments

Pavel Machek Oct. 13, 2019, 11:28 a.m. UTC | #1
On Wed 2019-10-09 13:52:51, 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.

> +++ 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 %imV!\n",
> +				latest->voltage / 1000);
>  		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 %imV, powering off\n",
> +				  latest->voltage / 1000);
>  			orderly_poweroff(true);
>  		}

Hmm.

So if latest->voltage is < 0, I'd preffer to shut down the machine,
too.

Actually, if we got POWEROFF irq, and voltage is close to 3.1V (like
maybe < 3.2V), maybe it would be good to shutdown anyway?

Best regards,
									Pavel
Tony Lindgren Oct. 15, 2019, 5:26 p.m. UTC | #2
* Pavel Machek <pavel@ucw.cz> [191013 11:28]:
> On Wed 2019-10-09 13:52:51, 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.
> 
> > +++ 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 %imV!\n",
> > +				latest->voltage / 1000);
> >  		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 %imV, powering off\n",
> > +				  latest->voltage / 1000);
> >  			orderly_poweroff(true);
> >  		}
> 
> Hmm.
> 
> So if latest->voltage is < 0, I'd preffer to shut down the machine,
> too.

Hmm I need to recheck if that is needed or not when booting without
a battery on a power supply.

> Actually, if we got POWEROFF irq, and voltage is close to 3.1V (like
> maybe < 3.2V), maybe it would be good to shutdown anyway?

No, this is some spurious interrupt issue that I've seen triggering
at way higher voltages than it should be happening at.

Regards,

Tony

Patch
diff mbox series

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 %imV!\n",
+				latest->voltage / 1000);
 		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 %imV, powering off\n",
+				  latest->voltage / 1000);
 			orderly_poweroff(true);
 		}
 		break;