* [PATCH] Check battery after resume
@ 2006-08-03 17:17 Thomas Renninger
2006-08-03 19:02 ` Dave Jones
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Renninger @ 2006-08-03 17:17 UTC (permalink / raw)
To: linux-acpi; +Cc: Brown, Len
[-- Attachment #1: Type: text/plain, Size: 2518 bytes --]
Patched against latest test tree.
Compile and field tested.
Check battery after resume
Signed-off-by: Thomas Renninger <mail@renninger.de>
drivers/acpi/battery.c | 33 ++++++++++++++++++++++++++++++++-
1 files changed, 32 insertions(+), 1 deletion(-)
Index: linux-acpi-2.6.git_i386/drivers/acpi/battery.c
===================================================================
--- linux-acpi-2.6.git_i386.orig/drivers/acpi/battery.c
+++ linux-acpi-2.6.git_i386/drivers/acpi/battery.c
@@ -64,6 +64,7 @@ extern void *acpi_unlock_battery_dir(str
static int acpi_battery_add(struct acpi_device *device);
static int acpi_battery_remove(struct acpi_device *device, int type);
+static int acpi_battery_resume(struct acpi_device *device, int state);
static struct acpi_driver acpi_battery_driver = {
.name = ACPI_BATTERY_DRIVER_NAME,
@@ -72,6 +73,7 @@ static struct acpi_driver acpi_battery_d
.ops = {
.add = acpi_battery_add,
.remove = acpi_battery_remove,
+ .resume = acpi_battery_resume,
},
};
@@ -269,6 +271,14 @@ acpi_battery_set_alarm(struct acpi_batte
return 0;
}
+/*
+ * returns:
+ * 0 on success
+ * <0 on failure
+ * 1 if new battery found
+ * 2 if battery got removed
+ */
+
static int acpi_battery_check(struct acpi_battery *battery)
{
int result = 0;
@@ -311,12 +321,14 @@ static int acpi_battery_check(struct acp
battery->flags.alarm = 1;
acpi_battery_set_alarm(battery, battery->trips.warning);
}
+ result = 1;
}
/* Removal? */
else if (battery->flags.present && !device->status.battery_present) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Battery removed\n"));
+ result = 2;
}
battery->flags.present = device->status.battery_present;
@@ -703,7 +715,7 @@ static int acpi_battery_add(struct acpi_
acpi_driver_data(device) = battery;
result = acpi_battery_check(battery);
- if (result)
+ if (result < 0)
goto end;
result = acpi_battery_add_fs(device);
@@ -753,6 +765,25 @@ static int acpi_battery_remove(struct ac
return 0;
}
+static int acpi_battery_resume(struct acpi_device *device, int state){
+
+ int result = 0;
+ struct acpi_battery *battery = NULL;
+
+ if (!device || !acpi_driver_data(device))
+ return -EINVAL;
+
+ battery = (struct acpi_battery *)acpi_driver_data(device);
+
+ result = acpi_battery_check(battery);
+ if (result > 0){
+ acpi_bus_generate_event(device,
+ ACPI_NOTIFY_DEVICE_CHECK,
+ battery->flags.present);
+ }
+ return 0;
+}
+
static int __init acpi_battery_init(void)
{
int result;
[-- Attachment #2: battery_resume.patch --]
[-- Type: text/x-patch, Size: 2456 bytes --]
Check battery after resume
Signed-off-by: Thomas Renninger <mail@renninger.de>
drivers/acpi/battery.c | 33 ++++++++++++++++++++++++++++++++-
1 files changed, 32 insertions(+), 1 deletion(-)
Index: linux-acpi-2.6.git_i386/drivers/acpi/battery.c
===================================================================
--- linux-acpi-2.6.git_i386.orig/drivers/acpi/battery.c
+++ linux-acpi-2.6.git_i386/drivers/acpi/battery.c
@@ -64,6 +64,7 @@ extern void *acpi_unlock_battery_dir(str
static int acpi_battery_add(struct acpi_device *device);
static int acpi_battery_remove(struct acpi_device *device, int type);
+static int acpi_battery_resume(struct acpi_device *device, int state);
static struct acpi_driver acpi_battery_driver = {
.name = ACPI_BATTERY_DRIVER_NAME,
@@ -72,6 +73,7 @@ static struct acpi_driver acpi_battery_d
.ops = {
.add = acpi_battery_add,
.remove = acpi_battery_remove,
+ .resume = acpi_battery_resume,
},
};
@@ -269,6 +271,14 @@ acpi_battery_set_alarm(struct acpi_batte
return 0;
}
+/*
+ * returns:
+ * 0 on success
+ * <0 on failure
+ * 1 if new battery found
+ * 2 if battery got removed
+ */
+
static int acpi_battery_check(struct acpi_battery *battery)
{
int result = 0;
@@ -311,12 +321,14 @@ static int acpi_battery_check(struct acp
battery->flags.alarm = 1;
acpi_battery_set_alarm(battery, battery->trips.warning);
}
+ result = 1;
}
/* Removal? */
else if (battery->flags.present && !device->status.battery_present) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Battery removed\n"));
+ result = 2;
}
battery->flags.present = device->status.battery_present;
@@ -703,7 +715,7 @@ static int acpi_battery_add(struct acpi_
acpi_driver_data(device) = battery;
result = acpi_battery_check(battery);
- if (result)
+ if (result < 0)
goto end;
result = acpi_battery_add_fs(device);
@@ -753,6 +765,25 @@ static int acpi_battery_remove(struct ac
return 0;
}
+static int acpi_battery_resume(struct acpi_device *device, int state){
+
+ int result = 0;
+ struct acpi_battery *battery = NULL;
+
+ if (!device || !acpi_driver_data(device))
+ return -EINVAL;
+
+ battery = (struct acpi_battery *)acpi_driver_data(device);
+
+ result = acpi_battery_check(battery);
+ if (result > 0){
+ acpi_bus_generate_event(device,
+ ACPI_NOTIFY_DEVICE_CHECK,
+ battery->flags.present);
+ }
+ return 0;
+}
+
static int __init acpi_battery_init(void)
{
int result;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Check battery after resume 2006-08-03 17:17 [PATCH] Check battery after resume Thomas Renninger @ 2006-08-03 19:02 ` Dave Jones 2006-08-04 13:46 ` Thomas Renninger 0 siblings, 1 reply; 5+ messages in thread From: Dave Jones @ 2006-08-03 19:02 UTC (permalink / raw) To: Thomas Renninger; +Cc: linux-acpi, Brown, Len On Thu, Aug 03, 2006 at 07:17:37PM +0200, Thomas Renninger wrote: > +/* > + * returns: > + * 0 on success > + * <0 on failure > + * 1 if new battery found > + * 2 if battery got removed > + */ Why make this so complicated... > + result = acpi_battery_check(battery); > + if (result > 0){ > + acpi_bus_generate_event(device, > + ACPI_NOTIFY_DEVICE_CHECK, > + battery->flags.present); > + } > + return 0; > +} When we simply treat the result as a boolean ? Dave -- http://www.codemonkey.org.uk ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check battery after resume 2006-08-03 19:02 ` Dave Jones @ 2006-08-04 13:46 ` Thomas Renninger 2006-08-04 17:09 ` Dave Jones 0 siblings, 1 reply; 5+ messages in thread From: Thomas Renninger @ 2006-08-04 13:46 UTC (permalink / raw) To: Dave Jones; +Cc: linux-acpi, Brown, Len On Thu, 2006-08-03 at 15:02 -0400, Dave Jones wrote: > On Thu, Aug 03, 2006 at 07:17:37PM +0200, Thomas Renninger wrote: > > > +/* > > + * returns: > > + * 0 on success > > + * <0 on failure > > + * 1 if new battery found > > + * 2 if battery got removed > > + */ > > Why make this so complicated... > > > + result = acpi_battery_check(battery); > > + if (result > 0){ > > + acpi_bus_generate_event(device, > > + ACPI_NOTIFY_DEVICE_CHECK, > > + battery->flags.present); > > + } > > + return 0; > > +} > > When we simply treat the result as a boolean ? The return value is used to: check for error <0 success, no battery insertion/removal 0 battery insertion/removal >0 (1/2) The latter one is needed to inform userspace to reread complete battery information (possibly from other BATx dir) if battery has been inserted/removed. Thomas ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check battery after resume 2006-08-04 13:46 ` Thomas Renninger @ 2006-08-04 17:09 ` Dave Jones 2006-08-07 15:22 ` Thomas Renninger 0 siblings, 1 reply; 5+ messages in thread From: Dave Jones @ 2006-08-04 17:09 UTC (permalink / raw) To: Thomas Renninger; +Cc: linux-acpi, Brown, Len On Fri, Aug 04, 2006 at 03:46:31PM +0200, Thomas Renninger wrote: > On Thu, 2006-08-03 at 15:02 -0400, Dave Jones wrote: > > On Thu, Aug 03, 2006 at 07:17:37PM +0200, Thomas Renninger wrote: > > > > > +/* > > > + * returns: > > > + * 0 on success > > > + * <0 on failure > > > + * 1 if new battery found > > > + * 2 if battery got removed > > > + */ > > > > Why make this so complicated... > > > > > + result = acpi_battery_check(battery); > > > + if (result > 0){ > > > + acpi_bus_generate_event(device, > > > + ACPI_NOTIFY_DEVICE_CHECK, > > > + battery->flags.present); > > > + } > > > + return 0; > > > +} > > > > When we simply treat the result as a boolean ? > > The return value is used to: > check for error <0 > success, no battery insertion/removal 0 > battery insertion/removal >0 (1/2) > > The latter one is needed to inform userspace to reread complete battery > information (possibly from other BATx dir) if battery has been > inserted/removed. The code cares about 2 possible states 'is there a battery added/removed'. Yet there are 4 possible states for no obvious reason. acpi_battery_check code could just as easily return 0=nothing changed, 1=battery added/removed, as you don't distinguish between states '1' and '2' anyway. Dave -- http://www.codemonkey.org.uk ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check battery after resume 2006-08-04 17:09 ` Dave Jones @ 2006-08-07 15:22 ` Thomas Renninger 0 siblings, 0 replies; 5+ messages in thread From: Thomas Renninger @ 2006-08-07 15:22 UTC (permalink / raw) To: Dave Jones; +Cc: linux-acpi, Brown, Len [-- Attachment #1: Type: text/plain, Size: 4846 bytes --] On Fri, 2006-08-04 at 13:09 -0400, Dave Jones wrote: > On Fri, Aug 04, 2006 at 03:46:31PM +0200, Thomas Renninger wrote: > > On Thu, 2006-08-03 at 15:02 -0400, Dave Jones wrote: > > > On Thu, Aug 03, 2006 at 07:17:37PM +0200, Thomas Renninger wrote: > > > > > > > +/* > > > > + * returns: > > > > + * 0 on success > > > > + * <0 on failure > > > > + * 1 if new battery found > > > > + * 2 if battery got removed > > > > + */ > > > > > > Why make this so complicated... > > > > > > > + result = acpi_battery_check(battery); > > > > + if (result > 0){ > > > > + acpi_bus_generate_event(device, > > > > + ACPI_NOTIFY_DEVICE_CHECK, > > > > + battery->flags.present); > > > > + } > > > > + return 0; > > > > +} > > > > > > When we simply treat the result as a boolean ? > > > > The return value is used to: > > check for error <0 > > success, no battery insertion/removal 0 > > battery insertion/removal >0 (1/2) > > > > The latter one is needed to inform userspace to reread complete battery > > information (possibly from other BATx dir) if battery has been > > inserted/removed. > > The code cares about 2 possible states 'is there a battery added/removed'. > Yet there are 4 possible states for no obvious reason. > acpi_battery_check code could just as easily return > 0=nothing changed, 1=battery added/removed, as you don't distinguish > between states '1' and '2' anyway. But it is already checked for an error at the initial acpi_battery_add function which is mandatory: result = acpi_battery_check(battery); if (result < 0) goto end; -> I need three states here: error, success no hw change, success + hw change. Thinking about this again, maybe an error should also be checked in the resume case. It might be that an additional battery is added and the ACPI code is executed the first time. The user might want to know that something went wrong... On the other side ACPI code should throw an exception if something happens during ACPI func execution... not sure whether the additional printk(KERN_ERR..) is needed, it shouldn't hurt, though. Is this OK: Check battery after resume Signed-off-by: Thomas Renninger <mail@renninger.de> drivers/acpi/battery.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) Index: linux-acpi-2.6.git_i386/drivers/acpi/battery.c =================================================================== --- linux-acpi-2.6.git_i386.orig/drivers/acpi/battery.c +++ linux-acpi-2.6.git_i386/drivers/acpi/battery.c @@ -64,6 +64,7 @@ extern void *acpi_unlock_battery_dir(str static int acpi_battery_add(struct acpi_device *device); static int acpi_battery_remove(struct acpi_device *device, int type); +static int acpi_battery_resume(struct acpi_device *device, int state); static struct acpi_driver acpi_battery_driver = { .name = ACPI_BATTERY_DRIVER_NAME, @@ -72,6 +73,7 @@ static struct acpi_driver acpi_battery_d .ops = { .add = acpi_battery_add, .remove = acpi_battery_remove, + .resume = acpi_battery_resume, }, }; @@ -269,6 +271,13 @@ acpi_battery_set_alarm(struct acpi_batte return 0; } +/* + * returns: + * <0 on failure + * 0 on success, no hw change + * 1 on success and battery got inserted/removed + */ + static int acpi_battery_check(struct acpi_battery *battery) { int result = 0; @@ -311,12 +320,14 @@ static int acpi_battery_check(struct acp battery->flags.alarm = 1; acpi_battery_set_alarm(battery, battery->trips.warning); } + result = 1; } /* Removal? */ else if (battery->flags.present && !device->status.battery_present) { ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Battery removed\n")); + result = 1; } battery->flags.present = device->status.battery_present; @@ -703,7 +714,7 @@ static int acpi_battery_add(struct acpi_ acpi_driver_data(device) = battery; result = acpi_battery_check(battery); - if (result) + if (result < 0) goto end; result = acpi_battery_add_fs(device); @@ -753,6 +764,33 @@ static int acpi_battery_remove(struct ac return 0; } +static int acpi_battery_resume(struct acpi_device *device, int state){ + + int result = 0; + struct acpi_battery *battery = NULL; + + if (!device || !acpi_driver_data(device)) + return -EINVAL; + + battery = (struct acpi_battery *)acpi_driver_data(device); + + result = acpi_battery_check(battery); + if (!result){ + if (result > 0){ + acpi_bus_generate_event(device, + ACPI_NOTIFY_DEVICE_CHECK, + battery->flags.present); + } + else{ + printk(KERN_ERR "%s Slot [%s]" + " error while checking on resume\n", + ACPI_BATTERY_DEVICE_NAME, + acpi_device_bid(device)); + } + } + return 0; +} + static int __init acpi_battery_init(void) { int result; [-- Attachment #2: battery_resume.patch --] [-- Type: text/x-patch, Size: 2662 bytes --] Check battery after resume Signed-off-by: Thomas Renninger <mail@renninger.de> drivers/acpi/battery.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) Index: linux-acpi-2.6.git_i386/drivers/acpi/battery.c =================================================================== --- linux-acpi-2.6.git_i386.orig/drivers/acpi/battery.c +++ linux-acpi-2.6.git_i386/drivers/acpi/battery.c @@ -64,6 +64,7 @@ extern void *acpi_unlock_battery_dir(str static int acpi_battery_add(struct acpi_device *device); static int acpi_battery_remove(struct acpi_device *device, int type); +static int acpi_battery_resume(struct acpi_device *device, int state); static struct acpi_driver acpi_battery_driver = { .name = ACPI_BATTERY_DRIVER_NAME, @@ -72,6 +73,7 @@ static struct acpi_driver acpi_battery_d .ops = { .add = acpi_battery_add, .remove = acpi_battery_remove, + .resume = acpi_battery_resume, }, }; @@ -269,6 +271,13 @@ acpi_battery_set_alarm(struct acpi_batte return 0; } +/* + * returns: + * <0 on failure + * 0 on success, no hw change + * 1 on success and battery got inserted/removed + */ + static int acpi_battery_check(struct acpi_battery *battery) { int result = 0; @@ -311,12 +320,14 @@ static int acpi_battery_check(struct acp battery->flags.alarm = 1; acpi_battery_set_alarm(battery, battery->trips.warning); } + result = 1; } /* Removal? */ else if (battery->flags.present && !device->status.battery_present) { ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Battery removed\n")); + result = 1; } battery->flags.present = device->status.battery_present; @@ -703,7 +714,7 @@ static int acpi_battery_add(struct acpi_ acpi_driver_data(device) = battery; result = acpi_battery_check(battery); - if (result) + if (result < 0) goto end; result = acpi_battery_add_fs(device); @@ -753,6 +764,33 @@ static int acpi_battery_remove(struct ac return 0; } +static int acpi_battery_resume(struct acpi_device *device, int state){ + + int result = 0; + struct acpi_battery *battery = NULL; + + if (!device || !acpi_driver_data(device)) + return -EINVAL; + + battery = (struct acpi_battery *)acpi_driver_data(device); + + result = acpi_battery_check(battery); + if (!result){ + if (result > 0){ + acpi_bus_generate_event(device, + ACPI_NOTIFY_DEVICE_CHECK, + battery->flags.present); + } + else{ + printk(KERN_ERR "%s Slot [%s]" + " error while checking on resume\n", + ACPI_BATTERY_DEVICE_NAME, + acpi_device_bid(device)); + } + } + return 0; +} + static int __init acpi_battery_init(void) { int result; ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2006-08-07 15:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-08-03 17:17 [PATCH] Check battery after resume Thomas Renninger 2006-08-03 19:02 ` Dave Jones 2006-08-04 13:46 ` Thomas Renninger 2006-08-04 17:09 ` Dave Jones 2006-08-07 15:22 ` Thomas Renninger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox