Re: [PATCH] acpi: Update battery information on notification 0x81

Rafael J. Wysocki
2010-12-23T22:24:21Z

On Thursday, December 23, 2010, Matthew Garrett wrote:
> A notification event 0x81 from an ACPI battery device requires us to
> re-read the battery information structure. Do so, and if the battery's
> reporting units have changed (as is the case on some Thinkpads) destroy
> and recreate the battery in order to populate the fields correctly.
>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>
> Respun against Linus master

As I've already said on IRC, I'd simply add a new function like:

void acpi_battery_refresh(struct acpi_battery *battery)
{
if (!battery->bat.dev)
return;
acpi_battery_get_info(battery);
/* The battery may have changed its reporting units */
sysfs_remove_battery(battery);
sysfs_add_battery(battery);
}

and call it before acpi_battery_update()
(a) in acpi_battery_resume()
(b) in acpi_battery_notify() if event == ACPI_BATTERY_NOTIFY_INFO

That would be more straightforward IMHO.

Thanks,
Rafael

> drivers/acpi/battery.c | 24 +++++++++++++++++-------
> 1 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 9fb9d5a..8da9c88 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -130,7 +130,7 @@ struct acpi_battery {
> unsigned long flags;
> };
>
> -static int acpi_battery_update(struct acpi_battery *battery);
> +static int acpi_battery_update(struct acpi_battery *battery, bool get_info);
>
> #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
>
> @@ -186,7 +186,7 @@ static int acpi_battery_get_property(struct power_supply *psy,
> int ret = 0;
> struct acpi_battery *battery = to_acpi_battery(psy);
>
> - if (acpi_battery_update(battery))
> + if (acpi_battery_update(battery, false))
> return -ENODEV;
>
> if (acpi_battery_present(battery)) {
> @@ -610,9 +610,11 @@ static void acpi_battery_quirks2(struct acpi_battery *battery)
> }
> }
>
> -static int acpi_battery_update(struct acpi_battery *battery)
> +static int acpi_battery_update(struct acpi_battery *battery, bool get_info)
> {
> int result, old_present = acpi_battery_present(battery);
> + int old_power_unit = battery->power_unit;
> +
> result = acpi_battery_get_status(battery);
> if (result)
> return result;
> @@ -631,6 +633,14 @@ static int acpi_battery_update(struct acpi_battery *battery)
> }
> if (!battery->bat.dev)
> sysfs_add_battery(battery);
> + if (get_info) {
> + acpi_battery_get_info(battery);
> + if (old_power_unit != battery->power_unit) {
> + /* The battery has changed its reporting units */
> + sysfs_remove_battery(battery);
> + sysfs_add_battery(battery);
> + }
> + }
> result = acpi_battery_get_state(battery);
> acpi_battery_quirks2(battery);
> return result;
> @@ -808,7 +818,7 @@ static print_func acpi_print_funcs[ACPI_BATTERY_NUMFILES] = {
> static int acpi_battery_read(int fid, struct seq_file *seq)
> {
> struct acpi_battery *battery = seq->private;
> - int result = acpi_battery_update(battery);
> + int result = acpi_battery_update(battery, false);
> return acpi_print_funcs[fid](seq, result);
> }
>
> @@ -919,7 +929,7 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
> if (!battery)
> return;
> old = battery->bat.dev;
> - acpi_battery_update(battery);
> + acpi_battery_update(battery, (event == ACPI_BATTERY_NOTIFY_INFO));
> acpi_bus_generate_proc_event(device, event,
> acpi_battery_present(battery));
> acpi_bus_generate_netlink_event(device->pnp.device_class,
> @@ -948,7 +958,7 @@ static int acpi_battery_add(struct acpi_device *device)
> if (ACPI_SUCCESS(acpi_get_handle(battery->device->handle,
> "_BIX", &handle)))
> set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags);
> - acpi_battery_update(battery);
> + acpi_battery_update(battery, false);
> #ifdef CONFIG_ACPI_PROCFS_POWER
> result = acpi_battery_add_fs(device);
> #endif
> @@ -989,7 +999,7 @@ static int acpi_battery_resume(struct acpi_device *device)
> return -EINVAL;
> battery = acpi_driver_data(device);
> battery->update_time = 0;
> - acpi_battery_update(battery);
> + acpi_battery_update(battery, true);
> return 0;
> }
>
>