Hi,
On Sat, Feb 08, 2025 at 10:22:08AM -0600, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@xxxxxxx>
>
> 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@xxxxxxx>
> ---
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
>
>
Attachment:
signature.asc
Description: PGP signature
