Message ID | 20250208162210.3929473-3-superm1@kernel.org |
---|---|
State | New |
Headers | show |
Series | Improvements to ACPI battery handling over s2idle | expand |
Hi, On Sat, Feb 08, 2025 at 10:22:08AM -0600, Mario Limonciello wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > To find out if a device is malfunctioning over suspend it's often > interesting to know how much battery was lost. > > At the start of the suspend cycle cache the amount of battery. > During resume, read the battery level again and if the battery > has decreased report the difference to the suspend stats using > pm_report_sleep_energy() > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- This code assumes, that there is only a single battery, but there can be more than one battery supplying the system. For example Thinkpads used to have an internal battery and a user swappable one. Also it seems in almost all cases debugging this from userspace by dropping a script in /usr/lib/systemd/system-sleep is good enough, so I wonder if extending the kernel ABI makes sense at all. Greetings, -- Sebastian > drivers/acpi/battery.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c > index 6760330a8af55..f21bfd02a26d1 100644 > --- a/drivers/acpi/battery.c > +++ b/drivers/acpi/battery.c > @@ -124,6 +124,7 @@ struct acpi_battery { > char oem_info[MAX_STRING_LENGTH]; > int state; > int power_unit; > + int capacity_suspend; > unsigned long flags; > }; > > @@ -1011,9 +1012,6 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) > return 0; > } > > - if (resume) > - return 0; > - > if (!battery->update_time) { > result = acpi_battery_get_info(battery); > if (result) > @@ -1032,6 +1030,14 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) > return result; > } > > + if (resume) { > + if (battery->capacity_suspend > battery->capacity_now) > + pm_report_sleep_energy(battery->capacity_suspend - battery->capacity_now); > + else > + pm_report_sleep_energy(0); > + return 0; > + } > + > /* > * Wakeup the system if battery is critical low > * or lower than the alarm level > @@ -1285,6 +1291,22 @@ static void acpi_battery_remove(struct acpi_device *device) > } > > /* this is needed to learn about changes made in suspended state */ > +static int acpi_battery_suspend(struct device *dev) > +{ > + struct acpi_battery *battery; > + > + if (!dev) > + return -EINVAL; > + > + battery = acpi_driver_data(to_acpi_device(dev)); > + if (!battery) > + return -EINVAL; > + > + battery->capacity_suspend = battery->capacity_now; > + > + return 0; > +} > + > static int acpi_battery_resume(struct device *dev) > { > struct acpi_battery *battery; > @@ -1301,7 +1323,7 @@ static int acpi_battery_resume(struct device *dev) > return 0; > } > > -static DEFINE_SIMPLE_DEV_PM_OPS(acpi_battery_pm, NULL, acpi_battery_resume); > +static DEFINE_SIMPLE_DEV_PM_OPS(acpi_battery_pm, acpi_battery_suspend, acpi_battery_resume); > > static struct acpi_driver acpi_battery_driver = { > .name = "battery", > -- > 2.43.0 > >
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6760330a8af55..f21bfd02a26d1 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -124,6 +124,7 @@ struct acpi_battery { char oem_info[MAX_STRING_LENGTH]; int state; int power_unit; + int capacity_suspend; unsigned long flags; }; @@ -1011,9 +1012,6 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return 0; } - if (resume) - return 0; - if (!battery->update_time) { result = acpi_battery_get_info(battery); if (result) @@ -1032,6 +1030,14 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; } + if (resume) { + if (battery->capacity_suspend > battery->capacity_now) + pm_report_sleep_energy(battery->capacity_suspend - battery->capacity_now); + else + pm_report_sleep_energy(0); + return 0; + } + /* * Wakeup the system if battery is critical low * or lower than the alarm level @@ -1285,6 +1291,22 @@ static void acpi_battery_remove(struct acpi_device *device) } /* this is needed to learn about changes made in suspended state */ +static int acpi_battery_suspend(struct device *dev) +{ + struct acpi_battery *battery; + + if (!dev) + return -EINVAL; + + battery = acpi_driver_data(to_acpi_device(dev)); + if (!battery) + return -EINVAL; + + battery->capacity_suspend = battery->capacity_now; + + return 0; +} + static int acpi_battery_resume(struct device *dev) { struct acpi_battery *battery; @@ -1301,7 +1323,7 @@ static int acpi_battery_resume(struct device *dev) return 0; } -static DEFINE_SIMPLE_DEV_PM_OPS(acpi_battery_pm, NULL, acpi_battery_resume); +static DEFINE_SIMPLE_DEV_PM_OPS(acpi_battery_pm, acpi_battery_suspend, acpi_battery_resume); static struct acpi_driver acpi_battery_driver = { .name = "battery",