| Message ID | 1401313610-14252-3-git-send-email-nm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 29, 2014 at 12:46 AM, Nishanth Menon <nm@ti.com> wrote: > Attempt to power off in case of critical events such as battery removal, > over voltage events. > > There is no guarentee that we'd be in a safe scenario here, but the very > least we can try to do is to power off the device to prevent damage to > the system instead of just printing a message and hoping for the best. At least "battery temperature out of range" does seem to happen quite often while charging on hot summer day. I'd prefer my pandora to not shutdown in such case, it could just stop charging instead. > > NOTE: twl4030 should attempt some form of recovery, but just depending > on that is never a safe alternative. > > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > new patch. original attempt was: https://patchwork.kernel.org/patch/4002371/ > > NOTE: we dont have poweroff support yet enabled on LDP, but it just needs > relevant dts entry. > > drivers/power/twl4030_charger.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c > index 2598c58..ed0dbd2 100644 > --- a/drivers/power/twl4030_charger.c > +++ b/drivers/power/twl4030_charger.c > @@ -22,6 +22,7 @@ > #include <linux/power_supply.h> > #include <linux/notifier.h> > #include <linux/usb/otg.h> > +#include <linux/reboot.h> > #include <linux/regulator/machine.h> > > #define TWL4030_BCIMSTATEC 0x02 > @@ -332,6 +333,7 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg) > struct twl4030_bci *bci = arg; > u8 irqs1, irqs2; > int ret; > + bool power_off = false; > > ret = twl_i2c_read_u8(TWL4030_MODULE_INTERRUPTS, &irqs1, > TWL4030_INTERRUPTS_BCIISR1A); > @@ -352,20 +354,34 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg) > } > > /* various monitoring events, for now we just log them here */ > - if (irqs1 & (TWL4030_TBATOR2 | TWL4030_TBATOR1)) > + if (irqs1 & (TWL4030_TBATOR2 | TWL4030_TBATOR1)) { > dev_warn(bci->dev, "battery temperature out of range\n"); > + power_off = true; > + } > > - if (irqs1 & TWL4030_BATSTS) > + if (irqs1 & TWL4030_BATSTS) { > dev_crit(bci->dev, "battery disconnected\n"); > + power_off = true; > + } > > - if (irqs2 & TWL4030_VBATOV) > + if (irqs2 & TWL4030_VBATOV) { > dev_crit(bci->dev, "VBAT overvoltage\n"); > + power_off = true; > + } > > - if (irqs2 & TWL4030_VBUSOV) > + if (irqs2 & TWL4030_VBUSOV) { > dev_crit(bci->dev, "VBUS overvoltage\n"); > + power_off = true; > + } > > - if (irqs2 & TWL4030_ACCHGOV) > + if (irqs2 & TWL4030_ACCHGOV) { > dev_crit(bci->dev, "Ac charger overvoltage\n"); > + power_off = true; > + } > + > + /* Try to shutdown the system */ > + if (power_off) > + orderly_poweroff(true); > > return IRQ_HANDLED; > } > -- > 1.7.9.5 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 06/04/2014 05:04 AM, Grazvydas Ignotas wrote: > On Thu, May 29, 2014 at 12:46 AM, Nishanth Menon <nm@ti.com> wrote: >> Attempt to power off in case of critical events such as battery removal, >> over voltage events. >> >> There is no guarentee that we'd be in a safe scenario here, but the very >> least we can try to do is to power off the device to prevent damage to >> the system instead of just printing a message and hoping for the best. > > At least "battery temperature out of range" does seem to happen quite > often while charging on hot summer day. I'd prefer my pandora to not > shutdown in such case, it could just stop charging instead. Yeah, We could call twl4030_charger_enable_ac(false); twl4030_charger_enable_usb(bci, false); But then, is that sufficient? From the TRM: 7.5.8 Battery Temperature Out-of-Range Detection Battery temperature out-of-range detection detects whether the battery temperature is within a specific range. Detection is possible for two temperature ranges. When the battery temperature is not in the 2–50°C range or is in the 3–43°C range, the TBATOR1 and TBATOR2 status bits rise and an interrupt is generated. This MADC monitoring function can be enabled by writing to the TBATOR1EN (BCIMFEN2[3]) and TBATOR2EN (BCIMFEN2[1]) fields. Battery pack at high temperature is a risk, no? and it may not be just charger that might be causing such a condition. Is'nt it safer to shut the device down in such a case?
On Wed, Jun 4, 2014 at 4:01 PM, Nishanth Menon <nm@ti.com> wrote: > On 06/04/2014 05:04 AM, Grazvydas Ignotas wrote: >> On Thu, May 29, 2014 at 12:46 AM, Nishanth Menon <nm@ti.com> wrote: >>> Attempt to power off in case of critical events such as battery removal, >>> over voltage events. >>> >>> There is no guarentee that we'd be in a safe scenario here, but the very >>> least we can try to do is to power off the device to prevent damage to >>> the system instead of just printing a message and hoping for the best. >> >> At least "battery temperature out of range" does seem to happen quite >> often while charging on hot summer day. I'd prefer my pandora to not >> shutdown in such case, it could just stop charging instead. > Yeah, We could call > twl4030_charger_enable_ac(false); > twl4030_charger_enable_usb(bci, false); > > But then, is that sufficient? > From the TRM: > 7.5.8 Battery Temperature Out-of-Range Detection > Battery temperature out-of-range detection detects whether the battery > temperature is within a specific > range. Detection is possible for two temperature ranges. When the > battery temperature is not in the > 2–50°C range or is in the 3–43°C range, the TBATOR1 and TBATOR2 status > bits rise and an interrupt is > generated. > This MADC monitoring function can be enabled by writing to the > TBATOR1EN (BCIMFEN2[3]) and > TBATOR2EN (BCIMFEN2[1]) fields. > > Battery pack at high temperature is a risk, no? and it may not be just > charger that might be causing such a condition. Is'nt it safer to shut > the device down in such a case? I don't know, so far nobody has complained about the battery exploding and anybody getting hurt, but it would make the device unusable for people in hot climates. From what I remember the automatic charge is stopped automatically on this condition, as some people complained they couldn't charge their device and saw these messages in dmesg. I guess mainline could choose the safer option and shutdown, no strong opinion about this.
Hi! > + if (irqs2 & TWL4030_ACCHGOV) { > dev_crit(bci->dev, "Ac charger overvoltage\n"); > + power_off = true; > + } > + > + /* Try to shutdown the system */ > + if (power_off) > + orderly_poweroff(true); > > return IRQ_HANDLED; > } Userland can make orderly_poweroff take long time, or even forever. (Think disconnected network with nfsroot). Should we do something more forceful here? Userland has to handle sudden poweroffs, anyway... (You could invent new function "give userland 5 seconds to shut system down" if you really wanted...) Pavel
On Thu 2014-06-05 01:30:40, Grazvydas Ignotas wrote: > On Wed, Jun 4, 2014 at 4:01 PM, Nishanth Menon <nm@ti.com> wrote: > > On 06/04/2014 05:04 AM, Grazvydas Ignotas wrote: > >> On Thu, May 29, 2014 at 12:46 AM, Nishanth Menon <nm@ti.com> wrote: > >> often while charging on hot summer day. I'd prefer my pandora to not > >> shutdown in such case, it could just stop charging instead. > > Yeah, We could call > > twl4030_charger_enable_ac(false); > > twl4030_charger_enable_usb(bci, false); > > > > But then, is that sufficient? > > From the TRM: > > 7.5.8 Battery Temperature Out-of-Range Detection > > Battery temperature out-of-range detection detects whether the battery > > temperature is within a specific > > range. Detection is possible for two temperature ranges. When the > > battery temperature is not in the > > 2???50°C range or is in the 3???43°C range, the TBATOR1 and TBATOR2 status > > bits rise and an interrupt is > > generated. > > This MADC monitoring function can be enabled by writing to the > > TBATOR1EN (BCIMFEN2[3]) and > > TBATOR2EN (BCIMFEN2[1]) fields. > > > > Battery pack at high temperature is a risk, no? and it may not be just > > charger that might be causing such a condition. Is'nt it safer to shut > > the device down in such a case? > > I don't know, so far nobody has complained about the battery exploding > and anybody getting hurt, but it would make the device unusable for > people in hot climates. From what I remember the automatic charge is > stopped automatically on this condition, as some people complained > they couldn't charge their device and saw these messages in dmesg. I > guess mainline could choose the safer option and shutdown, no strong > opinion about this. Li-ions have strong casing -- battery will not explode, just will grow a bit. And 50C will probably not make it explode immediately, just shorten its life a lot. If your battery is close enough to CPU (for example), it should be something else that heats it up. I think that shutdown by default is a good idea. If particular hardware triggers this a lot _and_ there are no components (CPU, hdd) heating the battery, you may want to just stop charging... but do it per-machine ... Actually, you probably want to stop charging at 37C or so, to stop triggering this... Pavel
On Sat, Jun 14, 2014 at 5:21 PM, Pavel Machek <pavel@ucw.cz> wrote: > > > Userland can make orderly_poweroff take long time, or even forever. (Think disconnected > network with nfsroot). > > Should we do something more forceful here? Userland has to handle sudden poweroffs, anyway... > > (You could invent new function "give userland 5 seconds to shut system down" if you > really wanted...) Or how about: /* If we have Power OFF ability, use it, else try restarting */ if (pm_power_off) { kernel_power_off(); } else { WARN(1, "FIXME: NO pm_power_off!!! trying restart\n"); kernel_restart("Charger triggered emergency restart"); }
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c index 2598c58..ed0dbd2 100644 --- a/drivers/power/twl4030_charger.c +++ b/drivers/power/twl4030_charger.c @@ -22,6 +22,7 @@ #include <linux/power_supply.h> #include <linux/notifier.h> #include <linux/usb/otg.h> +#include <linux/reboot.h> #include <linux/regulator/machine.h> #define TWL4030_BCIMSTATEC 0x02 @@ -332,6 +333,7 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg) struct twl4030_bci *bci = arg; u8 irqs1, irqs2; int ret; + bool power_off = false; ret = twl_i2c_read_u8(TWL4030_MODULE_INTERRUPTS, &irqs1, TWL4030_INTERRUPTS_BCIISR1A); @@ -352,20 +354,34 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg) } /* various monitoring events, for now we just log them here */ - if (irqs1 & (TWL4030_TBATOR2 | TWL4030_TBATOR1)) + if (irqs1 & (TWL4030_TBATOR2 | TWL4030_TBATOR1)) { dev_warn(bci->dev, "battery temperature out of range\n"); + power_off = true; + } - if (irqs1 & TWL4030_BATSTS) + if (irqs1 & TWL4030_BATSTS) { dev_crit(bci->dev, "battery disconnected\n"); + power_off = true; + } - if (irqs2 & TWL4030_VBATOV) + if (irqs2 & TWL4030_VBATOV) { dev_crit(bci->dev, "VBAT overvoltage\n"); + power_off = true; + } - if (irqs2 & TWL4030_VBUSOV) + if (irqs2 & TWL4030_VBUSOV) { dev_crit(bci->dev, "VBUS overvoltage\n"); + power_off = true; + } - if (irqs2 & TWL4030_ACCHGOV) + if (irqs2 & TWL4030_ACCHGOV) { dev_crit(bci->dev, "Ac charger overvoltage\n"); + power_off = true; + } + + /* Try to shutdown the system */ + if (power_off) + orderly_poweroff(true); return IRQ_HANDLED; }
Attempt to power off in case of critical events such as battery removal, over voltage events. There is no guarentee that we'd be in a safe scenario here, but the very least we can try to do is to power off the device to prevent damage to the system instead of just printing a message and hoping for the best. NOTE: twl4030 should attempt some form of recovery, but just depending on that is never a safe alternative. Signed-off-by: Nishanth Menon <nm@ti.com> --- new patch. original attempt was: https://patchwork.kernel.org/patch/4002371/ NOTE: we dont have poweroff support yet enabled on LDP, but it just needs relevant dts entry. drivers/power/twl4030_charger.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-)