From: Thomas Renninger <trenn@suse.de>
To: Dave Jones <davej@redhat.com>
Cc: linux-acpi <linux-acpi@vger.kernel.org>,
"Brown, Len" <len.brown@intel.com>
Subject: Re: [PATCH] Check battery after resume
Date: Mon, 07 Aug 2006 17:22:51 +0200 [thread overview]
Message-ID: <1154964171.4302.643.camel@queen.suse.de> (raw)
In-Reply-To: <20060804170919.GL7265@redhat.com>
[-- 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;
prev parent reply other threads:[~2006-08-07 15:19 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1154964171.4302.643.camel@queen.suse.de \
--to=trenn@suse.de \
--cc=davej@redhat.com \
--cc=len.brown@intel.com \
--cc=linux-acpi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox