* [PATCH] acpi: Update battery information on notification 0x81 @ 2010-08-16 20:32 Matthew Garrett 2010-08-31 9:06 ` Richard Hughes 2010-12-15 17:19 ` Matthew Garrett 0 siblings, 2 replies; 7+ messages in thread From: Matthew Garrett @ 2010-08-16 20:32 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, Matthew Garrett 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> --- drivers/acpi/battery.c | 22 +++++++++++++++++----- 1 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index dc58402..69638c4 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -562,9 +562,10 @@ static void acpi_battery_quirks(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; @@ -587,6 +588,16 @@ static int acpi_battery_update(struct acpi_battery *battery) if (!battery->bat.dev) sysfs_add_battery(battery); #endif + if (get_info) { + acpi_battery_get_info(battery); +#ifdef CONFIG_ACPI_SYSFS_POWER + if (old_power_unit != battery->power_unit) { + /* The battery has changed its reporting units */ + sysfs_remove_battery(battery); + sysfs_add_battery(battery); + } +#endif + } return acpi_battery_get_state(battery); } @@ -762,7 +773,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); } @@ -877,7 +888,8 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event) #ifdef CONFIG_ACPI_SYSFS_POWER old = battery->bat.dev; #endif - acpi_battery_update(battery); + acpi_battery_update(battery, (event == ACPI_BATTERY_NOTIFY_INFO ? true + : false)); acpi_bus_generate_proc_event(device, event, acpi_battery_present(battery)); acpi_bus_generate_netlink_event(device->pnp.device_class, @@ -908,7 +920,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 @@ -951,7 +963,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; } -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Update battery information on notification 0x81 2010-08-16 20:32 [PATCH] acpi: Update battery information on notification 0x81 Matthew Garrett @ 2010-08-31 9:06 ` Richard Hughes 2010-12-15 17:19 ` Matthew Garrett 1 sibling, 0 replies; 7+ messages in thread From: Richard Hughes @ 2010-08-31 9:06 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-acpi, linux-kernel On 16 August 2010 21:32, Matthew Garrett <mjg@redhat.com> 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. I've had three reports in upower of this bug (the units changing) so users are really hitting this in real world usage. As soon as this kernel fix gets released in a new kernel version I'll remove the upower workaround. I can't reproduce this myself, so I've not tested the patch, although the logic is solid in my opinion. Richard. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Update battery information on notification 0x81 2010-08-16 20:32 [PATCH] acpi: Update battery information on notification 0x81 Matthew Garrett 2010-08-31 9:06 ` Richard Hughes @ 2010-12-15 17:19 ` Matthew Garrett 2010-12-23 21:16 ` Matthew Garrett 1 sibling, 1 reply; 7+ messages in thread From: Matthew Garrett @ 2010-12-15 17:19 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb Len, any chance you could pick this up? -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] acpi: Update battery information on notification 0x81 2010-12-15 17:19 ` Matthew Garrett @ 2010-12-23 21:16 ` Matthew Garrett 2010-12-23 22:23 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Matthew Garrett @ 2010-12-23 21:16 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb 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 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; } -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Update battery information on notification 0x81 2010-12-23 21:16 ` Matthew Garrett @ 2010-12-23 22:23 ` Rafael J. Wysocki 2011-01-03 17:59 ` Matthew Garrett 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2010-12-23 22:23 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-acpi, linux-kernel, lenb 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; > } > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Update battery information on notification 0x81 2010-12-23 22:23 ` Rafael J. Wysocki @ 2011-01-03 17:59 ` Matthew Garrett 2011-01-03 20:56 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Matthew Garrett @ 2011-01-03 17:59 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: linux-acpi, linux-kernel, lenb On Thu, Dec 23, 2010 at 11:23:42PM +0100, Rafael J. Wysocki wrote: > 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. That might be more reasonable. Want to send that, or shall I? -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] acpi: Update battery information on notification 0x81 2011-01-03 17:59 ` Matthew Garrett @ 2011-01-03 20:56 ` Rafael J. Wysocki 0 siblings, 0 replies; 7+ messages in thread From: Rafael J. Wysocki @ 2011-01-03 20:56 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-acpi, linux-kernel, lenb On Monday, January 03, 2011, Matthew Garrett wrote: > On Thu, Dec 23, 2010 at 11:23:42PM +0100, Rafael J. Wysocki wrote: > > > 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. > > That might be more reasonable. Want to send that, or shall I? I can do that. Thanks, Rafael ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-01-03 20:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-08-16 20:32 [PATCH] acpi: Update battery information on notification 0x81 Matthew Garrett 2010-08-31 9:06 ` Richard Hughes 2010-12-15 17:19 ` Matthew Garrett 2010-12-23 21:16 ` Matthew Garrett 2010-12-23 22:23 ` Rafael J. Wysocki 2011-01-03 17:59 ` Matthew Garrett 2011-01-03 20:56 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox