[RFC] shutdown machine when li-ion battery goes below 3V

[RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

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

Hi!

What about something like this? N900 will drain the battery down to
system crash, which is quite uncool.

									Pavel


diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 0fe278b..8eb2f8f 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -46,6 +46,7 @@
 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
+#include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/of.h>
 
@@ -740,6 +741,9 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
 }
 EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
 
+static int bq27xxx_battery_protect(struct bq27xxx_device_info *di);
+
+
 static void bq27xxx_battery_poll(struct work_struct *work)
 {
 	struct bq27xxx_device_info *di =
@@ -747,6 +751,7 @@ static void bq27xxx_battery_poll(struct work_struct *work)
 				     work.work);
 
 	bq27xxx_battery_update(di);
+	bq27xxx_battery_protect(di);
 
 	if (poll_interval > 0)
 		schedule_delayed_work(&di->work, poll_interval * HZ);
@@ -958,6 +963,41 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
 	return ret;
 }
 
+static int bq27xxx_battery_protect(struct bq27xxx_device_info *di)
+{
+	union power_supply_propval val;
+	int mV, mA, mOhm = 430, mVadj;
+	int res;
+
+	printk(KERN_INFO "Main battery check\n");
+
+	res = bq27xxx_battery_voltage(di, &val);
+	if (res)
+		return res;
+
+	mV = val.intval / 1000;
+	
+	if (mV < 2950) {
+		printk(KERN_ALERT "Main battery below 2.95V, forcing shutdown.\n");
+		orderly_poweroff(true);
+	}
+
+	res = bq27xxx_battery_current(di, &val);
+	if (res)
+		return res;
+	
+	mA = val.intval / 1000;
+	mVadj = mV + (mA * mOhm) / 1000;
+
+	if (mVadj < 3150) {
+		printk(KERN_ALERT "Main battery internal voltage below 3.15, shutdown.\n");
+		orderly_poweroff(true);
+	}
+	printk(KERN_INFO "Main battery %d mV, internal voltage %d mV\n",
+	       mV, mVadj);
+	return 0;
+}
+
 static void bq27xxx_external_power_changed(struct power_supply *psy)
 {
 	struct bq27xxx_device_info *di = power_supply_get_drvdata(psy);
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 226b0b4ac..bcdc1f8 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -444,7 +444,7 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
 
 	if (trip_type == THERMAL_TRIP_CRITICAL) {
 		dev_emerg(&tz->device,
-			  "critical temperature reached(%d C),shutting down\n",
+			  "critical temperature reached(%d C), shutting down\n",
 			  tz->temperature / 1000);
 		orderly_poweroff(true);
 	}

-- 
(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 --]

[RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

Hi!

What about something like this? N900 will drain the battery down to
system crash, which is quite uncool.

									Pavel


diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 0fe278b..8eb2f8f 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -46,6 +46,7 @@
 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
+#include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/of.h>
 
@@ -740,6 +741,9 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
 }
 EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
 
+static int bq27xxx_battery_protect(struct bq27xxx_device_info *di);
+
+
 static void bq27xxx_battery_poll(struct work_struct *work)
 {
 	struct bq27xxx_device_info *di =
@@ -747,6 +751,7 @@ static void bq27xxx_battery_poll(struct work_struct *work)
 				     work.work);
 
 	bq27xxx_battery_update(di);
+	bq27xxx_battery_protect(di);
 
 	if (poll_interval > 0)
 		schedule_delayed_work(&di->work, poll_interval * HZ);
@@ -958,6 +963,41 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
 	return ret;
 }
 
+static int bq27xxx_battery_protect(struct bq27xxx_device_info *di)
+{
+	union power_supply_propval val;
+	int mV, mA, mOhm = 430, mVadj;
+	int res;
+
+	printk(KERN_INFO "Main battery check\n");
+
+	res = bq27xxx_battery_voltage(di, &val);
+	if (res)
+		return res;
+
+	mV = val.intval / 1000;
+	
+	if (mV < 2950) {
+		printk(KERN_ALERT "Main battery below 2.95V, forcing shutdown.\n");
+		orderly_poweroff(true);
+	}
+
+	res = bq27xxx_battery_current(di, &val);
+	if (res)
+		return res;
+	
+	mA = val.intval / 1000;
+	mVadj = mV + (mA * mOhm) / 1000;
+
+	if (mVadj < 3150) {
+		printk(KERN_ALERT "Main battery internal voltage below 3.15, shutdown.\n");
+		orderly_poweroff(true);
+	}
+	printk(KERN_INFO "Main battery %d mV, internal voltage %d mV\n",
+	       mV, mVadj);
+	return 0;
+}
+
 static void bq27xxx_external_power_changed(struct power_supply *psy)
 {
 	struct bq27xxx_device_info *di = power_supply_get_drvdata(psy);
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 226b0b4ac..bcdc1f8 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -444,7 +444,7 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
 
 	if (trip_type == THERMAL_TRIP_CRITICAL) {
 		dev_emerg(&tz->device,
-			  "critical temperature reached(%d C),shutting down\n",
+			  "critical temperature reached(%d C), shutting down\n",
 			  tz->temperature / 1000);
 		orderly_poweroff(true);
 	}

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/5e57183f/attachment.sig>

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [161024 14:24]:
> Hi!
> 
> What about something like this? N900 will drain the battery down to
> system crash, which is quite uncool.

Can't we make that generic and configurable for the voltage somehow?

Also, the shutdown voltage can depend on external devices connected.
It could be for example 3.3V depending on eMMC on some devices while
devices with no eMMC could have it at 3.0V.

Regards,

Tony

[RFC] shutdown machine when li-ion battery goes below 3V

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [161024 14:24]:
> Hi!
> 
> What about something like this? N900 will drain the battery down to
> system crash, which is quite uncool.

Can't we make that generic and configurable for the voltage somehow?

Also, the shutdown voltage can depend on external devices connected.
It could be for example 3.3V depending on eMMC on some devices while
devices with no eMMC could have it at 3.0V.

Regards,

Tony

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

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

On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [161024 14:24]:
> > Hi!
> > 
> > What about something like this? N900 will drain the battery down to
> > system crash, which is quite uncool.
> 
> Can't we make that generic and configurable for the voltage somehow?

I was afraid someone would ask :-).

Yes, we probably need to create battery object in the device tree,
then add properties there.

> Also, the shutdown voltage can depend on external devices connected.
> It could be for example 3.3V depending on eMMC on some devices while
> devices with no eMMC could have it at 3.0V.

Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
going below that is pretty mean to the battery. But if I set threshold
too high, GSM activity will push it below that for a very short while,
and I'll shutdown too soon.

Ideas welcome...
									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 --]

[RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [161024 14:24]:
> > Hi!
> > 
> > What about something like this? N900 will drain the battery down to
> > system crash, which is quite uncool.
> 
> Can't we make that generic and configurable for the voltage somehow?

I was afraid someone would ask :-).

Yes, we probably need to create battery object in the device tree,
then add properties there.

> Also, the shutdown voltage can depend on external devices connected.
> It could be for example 3.3V depending on eMMC on some devices while
> devices with no eMMC could have it at 3.0V.

Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
going below that is pretty mean to the battery. But if I set threshold
too high, GSM activity will push it below that for a very short while,
and I'll shutdown too soon.

Ideas welcome...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/eff01459/attachment-0001.sig>

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 869 bytes --]

On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > Also, the shutdown voltage can depend on external devices
> > connected. It could be for example 3.3V depending on eMMC on some
> > devices while devices with no eMMC could have it at 3.0V.
> 
> Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> going below that is pretty mean to the battery. But if I set
> threshold too high, GSM activity will push it below that for a very
> short while, and I'll shutdown too soon.
> 
> Ideas welcome...

bq27x00 has EDVF flag which means that battery is empty. Maemo with 
bq27x00 driver is configured to issue system shutdown when EDVF is set.

Maybe kernel should issue emergency shutdown e.g. after minute or two 
after EDVF flag is set?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[RFC] shutdown machine when li-ion battery goes below 3V

[Pali Rohár]

On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > Also, the shutdown voltage can depend on external devices
> > connected. It could be for example 3.3V depending on eMMC on some
> > devices while devices with no eMMC could have it at 3.0V.
> 
> Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> going below that is pretty mean to the battery. But if I set
> threshold too high, GSM activity will push it below that for a very
> short while, and I'll shutdown too soon.
> 
> Ideas welcome...

bq27x00 has EDVF flag which means that battery is empty. Maemo with 
bq27x00 driver is configured to issue system shutdown when EDVF is set.

Maybe kernel should issue emergency shutdown e.g. after minute or two 
after EDVF flag is set?

-- 
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/80b0cce7/attachment.sig>

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

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

On Mon 2016-10-24 23:48:47, Pali Rohár wrote:
> On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> > On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > > Also, the shutdown voltage can depend on external devices
> > > connected. It could be for example 3.3V depending on eMMC on some
> > > devices while devices with no eMMC could have it at 3.0V.
> > 
> > Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> > going below that is pretty mean to the battery. But if I set
> > threshold too high, GSM activity will push it below that for a very
> > short while, and I'll shutdown too soon.
> > 
> > Ideas welcome...
> 
> bq27x00 has EDVF flag which means that battery is empty. Maemo with 
> bq27x00 driver is configured to issue system shutdown when EDVF is set.
> 
> Maybe kernel should issue emergency shutdown e.g. after minute or two 
> after EDVF flag is set?

Thanks for pointer.

EDVF seems to be exposed as health. ... but only if battery is
calibrated, AFAICT.

 if (has_ci_flag && (cache.flags & BQ27000_FLAG_CI)) {
      dev_info_once(di->dev, "battery is not calibrated! ignoring capacity values\n");
      ...
      cache.health = -ENODATA;

Plus, it prioritizes battery cold over battery dead. IMO we don't need
to shutdown on battery cold (we just may not charge the battery), but
we need to shutdown on battery dead.

So something like this?

								Pavel

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

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 8eb2f8f..5ddf6d7 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -680,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 	/* Unlikely but important to return first */
 	if (unlikely(bq27xxx_battery_overtemp(di, flags)))
 		return POWER_SUPPLY_HEALTH_OVERHEAT;
-	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
-		return POWER_SUPPLY_HEALTH_COLD;
 	if (unlikely(bq27xxx_battery_dead(di, flags)))
 		return POWER_SUPPLY_HEALTH_DEAD;
+	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
+		return POWER_SUPPLY_HEALTH_COLD;
 
 	return POWER_SUPPLY_HEALTH_GOOD;
 }


-- 
(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 --]

[RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

On Mon 2016-10-24 23:48:47, Pali Roh?r wrote:
> On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> > On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > > Also, the shutdown voltage can depend on external devices
> > > connected. It could be for example 3.3V depending on eMMC on some
> > > devices while devices with no eMMC could have it at 3.0V.
> > 
> > Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> > going below that is pretty mean to the battery. But if I set
> > threshold too high, GSM activity will push it below that for a very
> > short while, and I'll shutdown too soon.
> > 
> > Ideas welcome...
> 
> bq27x00 has EDVF flag which means that battery is empty. Maemo with 
> bq27x00 driver is configured to issue system shutdown when EDVF is set.
> 
> Maybe kernel should issue emergency shutdown e.g. after minute or two 
> after EDVF flag is set?

Thanks for pointer.

EDVF seems to be exposed as health. ... but only if battery is
calibrated, AFAICT.

 if (has_ci_flag && (cache.flags & BQ27000_FLAG_CI)) {
      dev_info_once(di->dev, "battery is not calibrated! ignoring capacity values\n");
      ...
      cache.health = -ENODATA;

Plus, it prioritizes battery cold over battery dead. IMO we don't need
to shutdown on battery cold (we just may not charge the battery), but
we need to shutdown on battery dead.

So something like this?

								Pavel

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

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 8eb2f8f..5ddf6d7 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -680,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 	/* Unlikely but important to return first */
 	if (unlikely(bq27xxx_battery_overtemp(di, flags)))
 		return POWER_SUPPLY_HEALTH_OVERHEAT;
-	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
-		return POWER_SUPPLY_HEALTH_COLD;
 	if (unlikely(bq27xxx_battery_dead(di, flags)))
 		return POWER_SUPPLY_HEALTH_DEAD;
+	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
+		return POWER_SUPPLY_HEALTH_COLD;
 
 	return POWER_SUPPLY_HEALTH_GOOD;
 }


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/199ed265/attachment.sig>

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Pali Rohár]

On Tuesday 25 October 2016 12:24:35 Pavel Machek wrote:
> On Mon 2016-10-24 23:48:47, Pali Rohár wrote:
> > On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> > > On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > > > Also, the shutdown voltage can depend on external devices
> > > > connected. It could be for example 3.3V depending on eMMC on some
> > > > devices while devices with no eMMC could have it at 3.0V.
> > > 
> > > Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> > > going below that is pretty mean to the battery. But if I set
> > > threshold too high, GSM activity will push it below that for a very
> > > short while, and I'll shutdown too soon.
> > > 
> > > Ideas welcome...
> > 
> > bq27x00 has EDVF flag which means that battery is empty. Maemo with 
> > bq27x00 driver is configured to issue system shutdown when EDVF is set.
> > 
> > Maybe kernel should issue emergency shutdown e.g. after minute or two 
> > after EDVF flag is set?
> 
> Thanks for pointer.
> 
> EDVF seems to be exposed as health. ... but only if battery is
> calibrated, AFAICT.

No, EDVF is available also for uncalibrated battery. There are EDV1 and
EDVF flags. Both are set based on battery voltage and some other
parameters from bq EEPROM.

>  if (has_ci_flag && (cache.flags & BQ27000_FLAG_CI)) {
>       dev_info_once(di->dev, "battery is not calibrated! ignoring capacity values\n");

Yes, it ignores only capacity values (which needs calibration), not
those raw flags which works also without calibration.

>       ...
>       cache.health = -ENODATA;
> 
> Plus, it prioritizes battery cold over battery dead. IMO we don't need
> to shutdown on battery cold (we just may not charge the battery), but
> we need to shutdown on battery dead.
> 
> So something like this?
> 
> 								Pavel
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 8eb2f8f..5ddf6d7 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -680,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
>  	/* Unlikely but important to return first */
>  	if (unlikely(bq27xxx_battery_overtemp(di, flags)))
>  		return POWER_SUPPLY_HEALTH_OVERHEAT;
> -	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> -		return POWER_SUPPLY_HEALTH_COLD;
>  	if (unlikely(bq27xxx_battery_dead(di, flags)))
>  		return POWER_SUPPLY_HEALTH_DEAD;
> +	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> +		return POWER_SUPPLY_HEALTH_COLD;
>  
>  	return POWER_SUPPLY_HEALTH_GOOD;
>  }
> 
> 

Looks like this is OK.

-- 
Pali Rohár
pali.rohar@gmail.com

[RFC] shutdown machine when li-ion battery goes below 3V

[Pali Rohár]

On Tuesday 25 October 2016 12:24:35 Pavel Machek wrote:
> On Mon 2016-10-24 23:48:47, Pali Roh?r wrote:
> > On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> > > On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > > > Also, the shutdown voltage can depend on external devices
> > > > connected. It could be for example 3.3V depending on eMMC on some
> > > > devices while devices with no eMMC could have it at 3.0V.
> > > 
> > > Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> > > going below that is pretty mean to the battery. But if I set
> > > threshold too high, GSM activity will push it below that for a very
> > > short while, and I'll shutdown too soon.
> > > 
> > > Ideas welcome...
> > 
> > bq27x00 has EDVF flag which means that battery is empty. Maemo with 
> > bq27x00 driver is configured to issue system shutdown when EDVF is set.
> > 
> > Maybe kernel should issue emergency shutdown e.g. after minute or two 
> > after EDVF flag is set?
> 
> Thanks for pointer.
> 
> EDVF seems to be exposed as health. ... but only if battery is
> calibrated, AFAICT.

No, EDVF is available also for uncalibrated battery. There are EDV1 and
EDVF flags. Both are set based on battery voltage and some other
parameters from bq EEPROM.

>  if (has_ci_flag && (cache.flags & BQ27000_FLAG_CI)) {
>       dev_info_once(di->dev, "battery is not calibrated! ignoring capacity values\n");

Yes, it ignores only capacity values (which needs calibration), not
those raw flags which works also without calibration.

>       ...
>       cache.health = -ENODATA;
> 
> Plus, it prioritizes battery cold over battery dead. IMO we don't need
> to shutdown on battery cold (we just may not charge the battery), but
> we need to shutdown on battery dead.
> 
> So something like this?
> 
> 								Pavel
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 8eb2f8f..5ddf6d7 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -680,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
>  	/* Unlikely but important to return first */
>  	if (unlikely(bq27xxx_battery_overtemp(di, flags)))
>  		return POWER_SUPPLY_HEALTH_OVERHEAT;
> -	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> -		return POWER_SUPPLY_HEALTH_COLD;
>  	if (unlikely(bq27xxx_battery_dead(di, flags)))
>  		return POWER_SUPPLY_HEALTH_DEAD;
> +	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> +		return POWER_SUPPLY_HEALTH_COLD;
>  
>  	return POWER_SUPPLY_HEALTH_GOOD;
>  }
> 
> 

Looks like this is OK.

-- 
Pali Roh?r
pali.rohar at gmail.com

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

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

On Tue 2016-10-25 12:53:20, Pali Rohár wrote:
> On Tuesday 25 October 2016 12:24:35 Pavel Machek wrote:
> > On Mon 2016-10-24 23:48:47, Pali Rohár wrote:
> > > On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> > > > On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > > > > Also, the shutdown voltage can depend on external devices
> > > > > connected. It could be for example 3.3V depending on eMMC on some
> > > > > devices while devices with no eMMC could have it at 3.0V.
> > > > 
> > > > Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> > > > going below that is pretty mean to the battery. But if I set
> > > > threshold too high, GSM activity will push it below that for a very
> > > > short while, and I'll shutdown too soon.
> > > > 
> > > > Ideas welcome...
> > > 
> > > bq27x00 has EDVF flag which means that battery is empty. Maemo with 
> > > bq27x00 driver is configured to issue system shutdown when EDVF is set.
> > > 
> > > Maybe kernel should issue emergency shutdown e.g. after minute or two 
> > > after EDVF flag is set?
> > 
> > Thanks for pointer.
> > 
> > EDVF seems to be exposed as health. ... but only if battery is
> > calibrated, AFAICT.
> 
> No, EDVF is available also for uncalibrated battery. There are EDV1 and
> EDVF flags. Both are set based on battery voltage and some other
> parameters from bq EEPROM.
> 
> >  if (has_ci_flag && (cache.flags & BQ27000_FLAG_CI)) {
> >       dev_info_once(di->dev, "battery is not calibrated! ignoring capacity values\n");
> 
> Yes, it ignores only capacity values (which needs calibration), not
> those raw flags which works also without calibration.
> 
> >       ...
> >       cache.health = -ENODATA;

Take a look at code. Health is not read from hardware unless battery
is calibrated.
								
								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 --]

[RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

On Tue 2016-10-25 12:53:20, Pali Roh?r wrote:
> On Tuesday 25 October 2016 12:24:35 Pavel Machek wrote:
> > On Mon 2016-10-24 23:48:47, Pali Roh?r wrote:
> > > On Monday 24 October 2016 23:41:52 Pavel Machek wrote:
> > > > On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > > > > Also, the shutdown voltage can depend on external devices
> > > > > connected. It could be for example 3.3V depending on eMMC on some
> > > > > devices while devices with no eMMC could have it at 3.0V.
> > > > 
> > > > Actually, I'd like to shutdown at 3.3V or more (like 3.5V), because
> > > > going below that is pretty mean to the battery. But if I set
> > > > threshold too high, GSM activity will push it below that for a very
> > > > short while, and I'll shutdown too soon.
> > > > 
> > > > Ideas welcome...
> > > 
> > > bq27x00 has EDVF flag which means that battery is empty. Maemo with 
> > > bq27x00 driver is configured to issue system shutdown when EDVF is set.
> > > 
> > > Maybe kernel should issue emergency shutdown e.g. after minute or two 
> > > after EDVF flag is set?
> > 
> > Thanks for pointer.
> > 
> > EDVF seems to be exposed as health. ... but only if battery is
> > calibrated, AFAICT.
> 
> No, EDVF is available also for uncalibrated battery. There are EDV1 and
> EDVF flags. Both are set based on battery voltage and some other
> parameters from bq EEPROM.
> 
> >  if (has_ci_flag && (cache.flags & BQ27000_FLAG_CI)) {
> >       dev_info_once(di->dev, "battery is not calibrated! ignoring capacity values\n");
> 
> Yes, it ignores only capacity values (which needs calibration), not
> those raw flags which works also without calibration.
> 
> >       ...
> >       cache.health = -ENODATA;

Take a look at code. Health is not read from hardware unless battery
is calibrated.
								
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/61eece66/attachment.sig>

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Pali Rohár]

On Tuesday 25 October 2016 12:56:04 Pavel Machek wrote:
> Take a look at code. Health is not read from hardware unless battery
> is calibrated.

Then it is bug. EDVF should be accepted also when battery is not
calibrated.

-- 
Pali Rohár
pali.rohar@gmail.com

[RFC] shutdown machine when li-ion battery goes below 3V

[Pali Rohár]

On Tuesday 25 October 2016 12:56:04 Pavel Machek wrote:
> Take a look at code. Health is not read from hardware unless battery
> is calibrated.

Then it is bug. EDVF should be accepted also when battery is not
calibrated.

-- 
Pali Roh?r
pali.rohar at gmail.com

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

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

On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [161024 14:24]:
> > Hi!
> > 
> > What about something like this? N900 will drain the battery down to
> > system crash, which is quite uncool.
> 
> Can't we make that generic and configurable for the voltage somehow?
> 
> Also, the shutdown voltage can depend on external devices connected.
> It could be for example 3.3V depending on eMMC on some devices while
> devices with no eMMC could have it at 3.0V.

Actually, do we need to make it configurable? It looks like we should
respect hardware telling us battery is dead, and only use (low)
hardcoded voltages as a fallback.

Currently patch looks like this. generic_protect() should work for
other batteries drivers, too.

								Pavel

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 0fe278b..04094ad 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -46,6 +46,7 @@
 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
+#include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/of.h>
 
@@ -679,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 	/* Unlikely but important to return first */
 	if (unlikely(bq27xxx_battery_overtemp(di, flags)))
 		return POWER_SUPPLY_HEALTH_OVERHEAT;
-	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
-		return POWER_SUPPLY_HEALTH_COLD;
 	if (unlikely(bq27xxx_battery_dead(di, flags)))
 		return POWER_SUPPLY_HEALTH_DEAD;
+	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
+		return POWER_SUPPLY_HEALTH_COLD;
 
 	return POWER_SUPPLY_HEALTH_GOOD;
 }
@@ -740,6 +741,49 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
 }
 EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
 
+static void shutdown(char *reason)
+{
+	pr_alert("%s Forcing shutdown\n", reason);
+	orderly_poweroff(true);
+}
+
+static int generic_protect(struct power_supply *psy)
+{
+	union power_supply_propval val;
+	int res;
+	int mV, mA, mOhm = 430, mVadj = 0;
+
+	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_HEALTH, &val);
+	if (res)
+		return res;
+
+	if (val.intval == POWER_SUPPLY_HEALTH_OVERHEAT)
+		shutdown("Battery overheat.");
+	if (val.intval == POWER_SUPPLY_HEALTH_DEAD)
+		shutdown("Battery dead.");
+
+	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_VOLTAGE_NOW, &val);
+	if (res)
+		return res;
+	mV = val.intval / 1000;
+
+	if (mV < 2950)
+		shutdown("Battery below 2.95V.");
+
+	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_CURRENT_NOW, &val);
+	if (res)
+		return res;
+	mA = val.intval / 1000;
+	mVadj = mV + (mA * mOhm) / 1000;
+
+	if (mVadj < 3150)
+		shutdown("Battery internal voltage below 3.15.");
+	
+	printk(KERN_INFO "Main battery %d mV, internal voltage %d mV\n",
+	       mV, mVadj);
+	return 0;
+}
+
 static void bq27xxx_battery_poll(struct work_struct *work)
 {
 	struct bq27xxx_device_info *di =
@@ -747,6 +791,7 @@ static void bq27xxx_battery_poll(struct work_struct *work)
 				     work.work);
 
 	bq27xxx_battery_update(di);
+	generic_protect(di->bat);
 
 	if (poll_interval > 0)
 		schedule_delayed_work(&di->work, poll_interval * HZ);

-- 
(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 --]

[RFC] shutdown machine when li-ion battery goes below 3V

[Pavel Machek]

On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [161024 14:24]:
> > Hi!
> > 
> > What about something like this? N900 will drain the battery down to
> > system crash, which is quite uncool.
> 
> Can't we make that generic and configurable for the voltage somehow?
> 
> Also, the shutdown voltage can depend on external devices connected.
> It could be for example 3.3V depending on eMMC on some devices while
> devices with no eMMC could have it at 3.0V.

Actually, do we need to make it configurable? It looks like we should
respect hardware telling us battery is dead, and only use (low)
hardcoded voltages as a fallback.

Currently patch looks like this. generic_protect() should work for
other batteries drivers, too.

								Pavel

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 0fe278b..04094ad 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -46,6 +46,7 @@
 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
+#include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/of.h>
 
@@ -679,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 	/* Unlikely but important to return first */
 	if (unlikely(bq27xxx_battery_overtemp(di, flags)))
 		return POWER_SUPPLY_HEALTH_OVERHEAT;
-	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
-		return POWER_SUPPLY_HEALTH_COLD;
 	if (unlikely(bq27xxx_battery_dead(di, flags)))
 		return POWER_SUPPLY_HEALTH_DEAD;
+	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
+		return POWER_SUPPLY_HEALTH_COLD;
 
 	return POWER_SUPPLY_HEALTH_GOOD;
 }
@@ -740,6 +741,49 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
 }
 EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
 
+static void shutdown(char *reason)
+{
+	pr_alert("%s Forcing shutdown\n", reason);
+	orderly_poweroff(true);
+}
+
+static int generic_protect(struct power_supply *psy)
+{
+	union power_supply_propval val;
+	int res;
+	int mV, mA, mOhm = 430, mVadj = 0;
+
+	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_HEALTH, &val);
+	if (res)
+		return res;
+
+	if (val.intval == POWER_SUPPLY_HEALTH_OVERHEAT)
+		shutdown("Battery overheat.");
+	if (val.intval == POWER_SUPPLY_HEALTH_DEAD)
+		shutdown("Battery dead.");
+
+	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_VOLTAGE_NOW, &val);
+	if (res)
+		return res;
+	mV = val.intval / 1000;
+
+	if (mV < 2950)
+		shutdown("Battery below 2.95V.");
+
+	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_CURRENT_NOW, &val);
+	if (res)
+		return res;
+	mA = val.intval / 1000;
+	mVadj = mV + (mA * mOhm) / 1000;
+
+	if (mVadj < 3150)
+		shutdown("Battery internal voltage below 3.15.");
+	
+	printk(KERN_INFO "Main battery %d mV, internal voltage %d mV\n",
+	       mV, mVadj);
+	return 0;
+}
+
 static void bq27xxx_battery_poll(struct work_struct *work)
 {
 	struct bq27xxx_device_info *di =
@@ -747,6 +791,7 @@ static void bq27xxx_battery_poll(struct work_struct *work)
 				     work.work);
 
 	bq27xxx_battery_update(di);
+	generic_protect(di->bat);
 
 	if (poll_interval > 0)
 		schedule_delayed_work(&di->work, poll_interval * HZ);

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/db1c8616/attachment.sig>

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Pali Rohár]

On Tuesday 25 October 2016 13:27:57 Pavel Machek wrote:
> On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > * Pavel Machek <pavel@ucw.cz> [161024 14:24]:
> > > Hi!
> > > 
> > > What about something like this? N900 will drain the battery down to
> > > system crash, which is quite uncool.
> > 
> > Can't we make that generic and configurable for the voltage somehow?
> > 
> > Also, the shutdown voltage can depend on external devices connected.
> > It could be for example 3.3V depending on eMMC on some devices while
> > devices with no eMMC could have it at 3.0V.
> 
> Actually, do we need to make it configurable? It looks like we should
> respect hardware telling us battery is dead, and only use (low)
> hardcoded voltages as a fallback.
> 
> Currently patch looks like this. generic_protect() should work for
> other batteries drivers, too.

Now I checked Maemo's BME replacement code and it shutdown device 10
seconds after it read that capacity level is LOW or CRITICAL. bq27xxx
driver reports LOW when EDV1 is set and CRITICAL when EDVF is set. And
bq27xxx reports those LOW/CRITICAL based on EDV1/EDVF flags even if
battery is not calibrated.

So I would be happy if kernel does not issue emergency shutdown before
Maemo's BME replacement issue own "correct" system shutdown. As Maemo is
doing it on EDV1 flag (not EDVF as I thought!) with 10 seconds delay and
check is done every 30 seconds it means that Maemo shutdown process in
worst case is started 40 seconds after EDV1 is set. Shutdown process is
about 60 seconds (probably max.), can we ensure that kernel does not do
its own emergency shutdown earlier then 2 minutes before first see EDV1
flag? Or can test that EDVF flag is set in most cases 2 minutes after
EDV1?

It is really bad idea to start emergency kernel shutdown before even
userspace do it!

> 								Pavel
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 0fe278b..04094ad 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -46,6 +46,7 @@
>  #include <linux/delay.h>
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
> +#include <linux/reboot.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  
> @@ -679,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
>  	/* Unlikely but important to return first */
>  	if (unlikely(bq27xxx_battery_overtemp(di, flags)))
>  		return POWER_SUPPLY_HEALTH_OVERHEAT;
> -	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> -		return POWER_SUPPLY_HEALTH_COLD;
>  	if (unlikely(bq27xxx_battery_dead(di, flags)))
>  		return POWER_SUPPLY_HEALTH_DEAD;
> +	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> +		return POWER_SUPPLY_HEALTH_COLD;
>  
>  	return POWER_SUPPLY_HEALTH_GOOD;
>  }
> @@ -740,6 +741,49 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
>  }
>  EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
>  
> +static void shutdown(char *reason)
> +{
> +	pr_alert("%s Forcing shutdown\n", reason);
> +	orderly_poweroff(true);
> +}
> +
> +static int generic_protect(struct power_supply *psy)
> +{
> +	union power_supply_propval val;
> +	int res;
> +	int mV, mA, mOhm = 430, mVadj = 0;
> +
> +	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_HEALTH, &val);
> +	if (res)
> +		return res;
> +
> +	if (val.intval == POWER_SUPPLY_HEALTH_OVERHEAT)
> +		shutdown("Battery overheat.");
> +	if (val.intval == POWER_SUPPLY_HEALTH_DEAD)
> +		shutdown("Battery dead.");

Generally this is not a good idea. On some boards with bq27xxx devices
you can have another battery device or connected power device. You could
have "dead" battery but device supplied e.g. from wallcharger.

N900 cannot be powered from wallcharger by default, but in specific
conditions (turned everything except display) you can do battery
hotswap (when wallcharger is connected; it can power system).

So this patch basically break lot of other self powered devices.

I would propose check for am_i_power_supplied() (function with such name
in power_supply interface exist) and do that only for negative response.

> +	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_VOLTAGE_NOW, &val);
> +	if (res)
> +		return res;
> +	mV = val.intval / 1000;
> +
> +	if (mV < 2950)
> +		shutdown("Battery below 2.95V.");
> +
> +	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_CURRENT_NOW, &val);
> +	if (res)
> +		return res;
> +	mA = val.intval / 1000;
> +	mVadj = mV + (mA * mOhm) / 1000;
> +
> +	if (mVadj < 3150)
> +		shutdown("Battery internal voltage below 3.15.");
> +	
> +	printk(KERN_INFO "Main battery %d mV, internal voltage %d mV\n",
> +	       mV, mVadj);

Please no printk. There is dev_info or how is that function called. And
spamming dmesg for every poll is not good idea. It should be probably
DEBUG not INFO.

> +	return 0;
> +}
> +
>  static void bq27xxx_battery_poll(struct work_struct *work)
>  {
>  	struct bq27xxx_device_info *di =
> @@ -747,6 +791,7 @@ static void bq27xxx_battery_poll(struct work_struct *work)
>  				     work.work);
>  
>  	bq27xxx_battery_update(di);
> +	generic_protect(di->bat);
>  
>  	if (poll_interval > 0)
>  		schedule_delayed_work(&di->work, poll_interval * HZ);
> 

-- 
Pali Rohár
pali.rohar@gmail.com

[RFC] shutdown machine when li-ion battery goes below 3V

[Pali Rohár]

On Tuesday 25 October 2016 13:27:57 Pavel Machek wrote:
> On Mon 2016-10-24 14:29:33, Tony Lindgren wrote:
> > * Pavel Machek <pavel@ucw.cz> [161024 14:24]:
> > > Hi!
> > > 
> > > What about something like this? N900 will drain the battery down to
> > > system crash, which is quite uncool.
> > 
> > Can't we make that generic and configurable for the voltage somehow?
> > 
> > Also, the shutdown voltage can depend on external devices connected.
> > It could be for example 3.3V depending on eMMC on some devices while
> > devices with no eMMC could have it at 3.0V.
> 
> Actually, do we need to make it configurable? It looks like we should
> respect hardware telling us battery is dead, and only use (low)
> hardcoded voltages as a fallback.
> 
> Currently patch looks like this. generic_protect() should work for
> other batteries drivers, too.

Now I checked Maemo's BME replacement code and it shutdown device 10
seconds after it read that capacity level is LOW or CRITICAL. bq27xxx
driver reports LOW when EDV1 is set and CRITICAL when EDVF is set. And
bq27xxx reports those LOW/CRITICAL based on EDV1/EDVF flags even if
battery is not calibrated.

So I would be happy if kernel does not issue emergency shutdown before
Maemo's BME replacement issue own "correct" system shutdown. As Maemo is
doing it on EDV1 flag (not EDVF as I thought!) with 10 seconds delay and
check is done every 30 seconds it means that Maemo shutdown process in
worst case is started 40 seconds after EDV1 is set. Shutdown process is
about 60 seconds (probably max.), can we ensure that kernel does not do
its own emergency shutdown earlier then 2 minutes before first see EDV1
flag? Or can test that EDVF flag is set in most cases 2 minutes after
EDV1?

It is really bad idea to start emergency kernel shutdown before even
userspace do it!

> 								Pavel
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 0fe278b..04094ad 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -46,6 +46,7 @@
>  #include <linux/delay.h>
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
> +#include <linux/reboot.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  
> @@ -679,10 +680,10 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
>  	/* Unlikely but important to return first */
>  	if (unlikely(bq27xxx_battery_overtemp(di, flags)))
>  		return POWER_SUPPLY_HEALTH_OVERHEAT;
> -	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> -		return POWER_SUPPLY_HEALTH_COLD;
>  	if (unlikely(bq27xxx_battery_dead(di, flags)))
>  		return POWER_SUPPLY_HEALTH_DEAD;
> +	if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> +		return POWER_SUPPLY_HEALTH_COLD;
>  
>  	return POWER_SUPPLY_HEALTH_GOOD;
>  }
> @@ -740,6 +741,49 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
>  }
>  EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
>  
> +static void shutdown(char *reason)
> +{
> +	pr_alert("%s Forcing shutdown\n", reason);
> +	orderly_poweroff(true);
> +}
> +
> +static int generic_protect(struct power_supply *psy)
> +{
> +	union power_supply_propval val;
> +	int res;
> +	int mV, mA, mOhm = 430, mVadj = 0;
> +
> +	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_HEALTH, &val);
> +	if (res)
> +		return res;
> +
> +	if (val.intval == POWER_SUPPLY_HEALTH_OVERHEAT)
> +		shutdown("Battery overheat.");
> +	if (val.intval == POWER_SUPPLY_HEALTH_DEAD)
> +		shutdown("Battery dead.");

Generally this is not a good idea. On some boards with bq27xxx devices
you can have another battery device or connected power device. You could
have "dead" battery but device supplied e.g. from wallcharger.

N900 cannot be powered from wallcharger by default, but in specific
conditions (turned everything except display) you can do battery
hotswap (when wallcharger is connected; it can power system).

So this patch basically break lot of other self powered devices.

I would propose check for am_i_power_supplied() (function with such name
in power_supply interface exist) and do that only for negative response.

> +	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_VOLTAGE_NOW, &val);
> +	if (res)
> +		return res;
> +	mV = val.intval / 1000;
> +
> +	if (mV < 2950)
> +		shutdown("Battery below 2.95V.");
> +
> +	res = psy->desc->get_property(psy, POWER_SUPPLY_PROP_CURRENT_NOW, &val);
> +	if (res)
> +		return res;
> +	mA = val.intval / 1000;
> +	mVadj = mV + (mA * mOhm) / 1000;
> +
> +	if (mVadj < 3150)
> +		shutdown("Battery internal voltage below 3.15.");
> +	
> +	printk(KERN_INFO "Main battery %d mV, internal voltage %d mV\n",
> +	       mV, mVadj);

Please no printk. There is dev_info or how is that function called. And
spamming dmesg for every poll is not good idea. It should be probably
DEBUG not INFO.

> +	return 0;
> +}
> +
>  static void bq27xxx_battery_poll(struct work_struct *work)
>  {
>  	struct bq27xxx_device_info *di =
> @@ -747,6 +791,7 @@ static void bq27xxx_battery_poll(struct work_struct *work)
>  				     work.work);
>  
>  	bq27xxx_battery_update(di);
> +	generic_protect(di->bat);
>  
>  	if (poll_interval > 0)
>  		schedule_delayed_work(&di->work, poll_interval * HZ);
> 

-- 
Pali Roh?r
pali.rohar at gmail.com

Re: [RFC] shutdown machine when li-ion battery goes below 3V

[Olaf Titz]

> +	res = bq27xxx_battery_voltage(di, &val);
> +	if (res)
> +		return res;
> +
> +	mV = val.intval / 1000;

Reading that code I stumbled over the comment in
bq27xxx_battery_voltage saying that it returns millivolts. The code
here, the code in bq27xxx_battery_voltage and power_supply.h all
indicate that it in fact returns microvolts. Please double-check and
fix, as it stands now the code looks inconsistent (but not knowing that
device at all I don't feel fit to submit a fix).

regards, Olaf