Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Safe Shutdown on Battery Empty Condition (N900) #171

Closed
sicelo opened this issue Sep 1, 2018 · 9 comments
Closed

Implement Safe Shutdown on Battery Empty Condition (N900) #171

sicelo opened this issue Sep 1, 2018 · 9 comments

Comments

@sicelo
Copy link
Collaborator

sicelo commented Sep 1, 2018

By accident today, I noticed that on the N900, we allow the battery to get too low (less than 3000mV), and soon thereafter the system dies without initiating a proper shutdown.

Perhaps @pali can suggest a good threshold for shutdown. But I seem to recall that we need the voltage to at least reach 3248mV and remain at that voltage or less for a couple of seconds (15 maybe?) for the battery learning procedure to succeed. This suggests that we probably should initiate shutdown at around 3200mV or slightly less than that.

@MerlijnWajer
Copy link
Member

I am not sure what tool should act on this. I believe it might be mce that should detect this and perhaps initiate a shutdown, but I am not sure if it is actually mce that perform/issues the actual shutdown.

@pavelmachek
Copy link

One possibility is to do shutdown from kernel. It protects battery, so it has some business being in kernel... and this way it will also work during fsck, init=/bin/sh, etc... cases. 3.2V seems like reasonable shutdown voltage.

I even have a patch that does something along those lines.

delme.txt

@pali
Copy link
Member

pali commented Jan 30, 2020

Battery drivers for power supply interface can already provide information that battery is fully empty and user should do immediate system shutdown. Maybe this "kernel shutdown" code can be implemented directly in power supply subsystem and so any machine can benefit from it?

@pavelmachek
Copy link

@pali See my patch attached above. I'm not sure we have suitable generic place where to implement that, but Droid 4 (cpcap-battery.c) already does shutdown, and likely N900 should do the same.

@pali
Copy link
Member

pali commented Jan 30, 2020

Logic for userspace shutdown for Maemo is implemented here:
https://github.com/community-ssu/hald-addon-bme/blob/master/hald-addon-bme.c

Important is #define POWER_SUPPLY_VOLTAGE_THRESHOLD_EMPTY 3248 and also when POWER_SUPPLY_CAPACITY_LEVEL is Low or Critical. IIRC when power charer is disconnected and one of that condition is true, then normal userspace shutdown procedure is executed.

So I think that kernel could look at POWER_SUPPLY_CAPACITY_LEVEL and when it is Critical and power supply is disconnected it should issue that kernel shutdown.

@pali
Copy link
Member

pali commented Jan 30, 2020

I'm not sure we have suitable generic place where to implement that

Somewhere in power_supply_core.c?

See my patch attached above

I saw. I think that generic_protect could be called from power_supply_changed.

@pavelmachek
Copy link

Dunno. I believe shutdown should be opt-in by the driver, so that we don't break someone's existing setup. OTOH, if you can push patch to Sebastian, I won't complain too much :-).

@spinal84
Copy link

spinal84 commented Feb 9, 2020

I'm absolutely fine to have something that protects our users' batteries in kernel level until we have some kind of protection in the user space.

Pavel, could you please prepare a commit(s) that look roughly like the linux kernel quality?
I mean clean code, nice commit message, enough comments in the code (if they are needed).

@IMbackK
Copy link
Collaborator

IMbackK commented Aug 7, 2021

mce has a feature called battery-guard now, it only needs to be configured for n900

@IMbackK IMbackK closed this as completed Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants