Re: new regression in 2.6.25-rc3: no keyboard/lid acpi events on thinkpad T61p
Michael S. Tsirkin wrote:
On Mon, Feb 25, 2008 at 10:50 PM, Alexey Starikovskiy
<[EMAIL PROTECTED]> wrote:
Michael S. Tsirkin wrote:
> Did you guys stop accepting reports by mail?
> I hope not.
It is easier to track bug information in bugzilla.
If you for some reason do not wish to create a bug report,
I can do it for you. You only need to provide acpidump.
Great.
>> and attach acpidump?
>
> I'll see if I can get acpidump output - in which state do you want it?
> Right after boot on the broken kernel?
acpidump output does not change over time, you could get it even with some
other kernel.
Attached is the acpidump output run under 2.6.23-rc3 + with reverted
37f9b4c7c612fcbeb8fb6faddaef4ccdb5350145
(IOW - this is a working configuration).
Thanks, you've got round 10100 bug number.
Please check if the following patch on top of Linus git tree helps.
Regards,
Alex
HTH,
MST
ACPI: EC: fix regression
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/ec.c | 18 +++---
1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index caf873c..d6f9956 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -129,7 +129,8 @@ static struct acpi_ec {
struct mutex lock;
wait_queue_head_t wait;
struct list_head list;
- u8 handlers_installed;
+ u8 handlers_installed:1;
+ u8 from_ecdt:1;
} *boot_ec, *first_ec;
/* --
@@ -772,16 +773,18 @@ static int acpi_ec_add(struct acpi_device *device)
/* Check for boot EC */
if (boot_ec) {
- if (boot_ec->handle == device->handle) {
- /* Pre-loaded EC from DSDT, just move pointer */
- ec = boot_ec;
- boot_ec = NULL;
- goto end;
- } else if (boot_ec->handle == ACPI_ROOT_OBJECT) {
+ if (boot_ec->from_ecdt &&
+ (boot_ec->handle == device->handle ||
+ boot_ec->handle == ACPI_ROOT_OBJECT)) {
/* ECDT-based EC, time to shut it down */
ec_remove_handlers(boot_ec);
kfree(boot_ec);
first_ec = boot_ec = NULL;
+ } else if (boot_ec->handle == device->handle) {
+ /* Pre-loaded EC from DSDT, just move pointer */
+ ec = boot_ec;
+ boot_ec = NULL;
+ goto end;
}
}
@@ -943,6 +946,7 @@ int __init acpi_ec_ecdt_probe(void)
boot_ec->command_addr = ecdt_ptr->control.address;
boot_ec->data_addr = ecdt_ptr->data.address;
boot_ec->gpe = ecdt_ptr->gpe;
+ boot_ec->from_ecdt = 1;
if (ACPI_FAILURE(acpi_get_handle(NULL, ecdt_ptr->id,
_ec->handle))) {
pr_info("Failed to locate handle for boot EC\n");
Re: new regression in 2.6.25-rc3: no keyboard/lid acpi events on thinkpad T61p
Michael S. Tsirkin wrote: Did you guys stop accepting reports by mail? I hope not. It is easier to track bug information in bugzilla. If you for some reason do not wish to create a bug report, I can do it for you. You only need to provide acpidump. and attach acpidump? I'll see if I can get acpidump output - in which state do you want it? Right after boot on the broken kernel? acpidump output does not change over time, you could get it even with some other kernel. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: new regression in 2.6.25-rc3: no keyboard/lid acpi events on thinkpad T61p
Michael S. Tsirkin wrote: On my T61p, 2.6.25-rc2 seems to get acpi events from keypresses such as Fn-F4 and lid open/close, prints them in /var/log/acpid and reacts accordingly (my acpi scripts suspend on lid close and Fn-F4). This no longer happens in 2.6.25-rc3: I see nothing in /var/log/acpid after power on: [Mon Feb 25 20:47:28 2008] completed event "button/power PWRF 0080 0001" [Mon Feb 25 20:48:44 2008] starting up [Mon Feb 25 20:48:44 2008] 57 rules loaded [Mon Feb 25 20:48:48 2008] client connected from 5927[0:0] [Mon Feb 25 20:48:48 2008] 1 client rule loaded and pressing buttons or closing lid produces no output in /var/log/acpid and seems to have no effect. git bisect pointed at commit 208c70a45624400fafd7511b96bc426bf01f8f5e : ACPI: EC: Use proper handle for boot EC If I do git revert 208c70a45624400fafd7511b96bc426bf01f8f5e on top of 2.6.25-rc3, I start getting keyboard acpi events, again. Could you please create a bug report and attach acpidump? Thanks, Alex. Thanks, MST PS: I see a different problem on resume, it seems to be unrelated to ACPI and I will bisect and report separately. PPS: Pls Cc me directly, I am not on the list. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: new regression in 2.6.25-rc3: no keyboard/lid acpi events on thinkpad T61p
Michael S. Tsirkin wrote: On my T61p, 2.6.25-rc2 seems to get acpi events from keypresses such as Fn-F4 and lid open/close, prints them in /var/log/acpid and reacts accordingly (my acpi scripts suspend on lid close and Fn-F4). This no longer happens in 2.6.25-rc3: I see nothing in /var/log/acpid after power on: [Mon Feb 25 20:47:28 2008] completed event button/power PWRF 0080 0001 [Mon Feb 25 20:48:44 2008] starting up [Mon Feb 25 20:48:44 2008] 57 rules loaded [Mon Feb 25 20:48:48 2008] client connected from 5927[0:0] [Mon Feb 25 20:48:48 2008] 1 client rule loaded and pressing buttons or closing lid produces no output in /var/log/acpid and seems to have no effect. git bisect pointed at commit 208c70a45624400fafd7511b96bc426bf01f8f5e : ACPI: EC: Use proper handle for boot EC If I do git revert 208c70a45624400fafd7511b96bc426bf01f8f5e on top of 2.6.25-rc3, I start getting keyboard acpi events, again. Could you please create a bug report and attach acpidump? Thanks, Alex. Thanks, MST PS: I see a different problem on resume, it seems to be unrelated to ACPI and I will bisect and report separately. PPS: Pls Cc me directly, I am not on the list. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: new regression in 2.6.25-rc3: no keyboard/lid acpi events on thinkpad T61p
Michael S. Tsirkin wrote: Did you guys stop accepting reports by mail? I hope not. It is easier to track bug information in bugzilla. If you for some reason do not wish to create a bug report, I can do it for you. You only need to provide acpidump. and attach acpidump? I'll see if I can get acpidump output - in which state do you want it? Right after boot on the broken kernel? acpidump output does not change over time, you could get it even with some other kernel. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: new regression in 2.6.25-rc3: no keyboard/lid acpi events on thinkpad T61p
Michael S. Tsirkin wrote:
On Mon, Feb 25, 2008 at 10:50 PM, Alexey Starikovskiy
[EMAIL PROTECTED] wrote:
Michael S. Tsirkin wrote:
Did you guys stop accepting reports by mail?
I hope not.
It is easier to track bug information in bugzilla.
If you for some reason do not wish to create a bug report,
I can do it for you. You only need to provide acpidump.
Great.
and attach acpidump?
I'll see if I can get acpidump output - in which state do you want it?
Right after boot on the broken kernel?
acpidump output does not change over time, you could get it even with some
other kernel.
Attached is the acpidump output run under 2.6.23-rc3 + with reverted
37f9b4c7c612fcbeb8fb6faddaef4ccdb5350145
(IOW - this is a working configuration).
Thanks, you've got round 10100 bug number.
Please check if the following patch on top of Linus git tree helps.
Regards,
Alex
HTH,
MST
ACPI: EC: fix regression
From: Alexey Starikovskiy [EMAIL PROTECTED]
---
drivers/acpi/ec.c | 18 +++---
1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index caf873c..d6f9956 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -129,7 +129,8 @@ static struct acpi_ec {
struct mutex lock;
wait_queue_head_t wait;
struct list_head list;
- u8 handlers_installed;
+ u8 handlers_installed:1;
+ u8 from_ecdt:1;
} *boot_ec, *first_ec;
/* --
@@ -772,16 +773,18 @@ static int acpi_ec_add(struct acpi_device *device)
/* Check for boot EC */
if (boot_ec) {
- if (boot_ec-handle == device-handle) {
- /* Pre-loaded EC from DSDT, just move pointer */
- ec = boot_ec;
- boot_ec = NULL;
- goto end;
- } else if (boot_ec-handle == ACPI_ROOT_OBJECT) {
+ if (boot_ec-from_ecdt
+ (boot_ec-handle == device-handle ||
+ boot_ec-handle == ACPI_ROOT_OBJECT)) {
/* ECDT-based EC, time to shut it down */
ec_remove_handlers(boot_ec);
kfree(boot_ec);
first_ec = boot_ec = NULL;
+ } else if (boot_ec-handle == device-handle) {
+ /* Pre-loaded EC from DSDT, just move pointer */
+ ec = boot_ec;
+ boot_ec = NULL;
+ goto end;
}
}
@@ -943,6 +946,7 @@ int __init acpi_ec_ecdt_probe(void)
boot_ec-command_addr = ecdt_ptr-control.address;
boot_ec-data_addr = ecdt_ptr-data.address;
boot_ec-gpe = ecdt_ptr-gpe;
+ boot_ec-from_ecdt = 1;
if (ACPI_FAILURE(acpi_get_handle(NULL, ecdt_ptr-id,
boot_ec-handle))) {
pr_info(Failed to locate handle for boot EC\n);
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off after suspend-to-disk. Screen becomes green.
Rafael J. Wysocki wrote: On Wednesday, 20 of February 2008, Linus Torvalds wrote: On Wed, 20 Feb 2008, Rafael J. Wysocki wrote: I think we should export the target sleep state somehow. Yeah. By *not* using "->suspend()" for freezing or hibernate. Please, Rafael - just make the f*cking suspend-to-disk use other routines already. Okay, I think I'll just start sending patches for that, but rather not earlier than in the 2.6.27 time frame. No one else works on that and I've been busy with other things recently. Besides, I'm not even a full time kernel developer ... Rafael, If I can help, please say so. Regards, Alex. 99% of all hardware needs to do exactly *nothing* on suspend-to-disk, and the ones that really do need things tend to need to not do a whole lot. For example, the "freeze" action for USB (which is one of the hardest things to suspend) should literally be something like just setting the controller STOP bit, and waiting for it to have stopped. The "unfreeze" should be to just clear the stop bit, while the "restart" should be just a controller reset to use the current memory image. NONE OF THIS HAS ABSOLUTELY ANYTHING TO DO WITH SUSPEND. It never did. I've told people so for years. Maybe actually seeing the problems will make people realize. I think so. So please, we shouldn't call "->suspend[_late]" or "->resume[_early]" at all. Not with PMSG_FREEZE, not with PMSG_*anything*. Can we please get this fixed some day? Yes, we can (hopefully). Thanks, Rafael - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off after suspend-to-disk. Screen becomes green.
Rafael J. Wysocki wrote: On Wednesday, 20 of February 2008, Linus Torvalds wrote: On Wed, 20 Feb 2008, Rafael J. Wysocki wrote: I think we should export the target sleep state somehow. Yeah. By *not* using -suspend() for freezing or hibernate. Please, Rafael - just make the f*cking suspend-to-disk use other routines already. Okay, I think I'll just start sending patches for that, but rather not earlier than in the 2.6.27 time frame. No one else works on that and I've been busy with other things recently. Besides, I'm not even a full time kernel developer ... Rafael, If I can help, please say so. Regards, Alex. 99% of all hardware needs to do exactly *nothing* on suspend-to-disk, and the ones that really do need things tend to need to not do a whole lot. For example, the freeze action for USB (which is one of the hardest things to suspend) should literally be something like just setting the controller STOP bit, and waiting for it to have stopped. The unfreeze should be to just clear the stop bit, while the restart should be just a controller reset to use the current memory image. NONE OF THIS HAS ABSOLUTELY ANYTHING TO DO WITH SUSPEND. It never did. I've told people so for years. Maybe actually seeing the problems will make people realize. I think so. So please, we shouldn't call -suspend[_late] or -resume[_early] at all. Not with PMSG_FREEZE, not with PMSG_*anything*. Can we please get this fixed some day? Yes, we can (hopefully). Thanks, Rafael - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI: EC: add leading zeros to debug messages
Marton,
Could you please attach these patches to bug report? It is easier to track them
there.
Thanks,
Alex.
Németh Márton wrote:
From: Márton Németh <[EMAIL PROTECTED]>
Add leading zeros to pr_debug() calls. For example if x=0x0a, the format
"0x%2x" will result the string "0x a", the format "0x%2.2x" will result "0x0a".
Signed-off-by: Márton Németh <[EMAIL PROTECTED]>
---
--- linux-2.6.24-rc8/drivers/acpi/ec.c.orig 2008-01-16 07:25:33.0
+0100
+++ linux-2.6.24-rc8/drivers/acpi/ec.c 2008-01-17 07:15:10.0 +0100
@@ -138,26 +138,26 @@ static struct acpi_ec {
static inline u8 acpi_ec_read_status(struct acpi_ec *ec)
{
u8 x = inb(ec->command_addr);
- pr_debug(PREFIX "---> status = 0x%2x\n", x);
+ pr_debug(PREFIX "---> status = 0x%2.2x\n", x);
return x;
}
static inline u8 acpi_ec_read_data(struct acpi_ec *ec)
{
u8 x = inb(ec->data_addr);
- pr_debug(PREFIX "---> data = 0x%2x\n", x);
+ pr_debug(PREFIX "---> data = 0x%2.2x\n", x);
return inb(ec->data_addr);
}
static inline void acpi_ec_write_cmd(struct acpi_ec *ec, u8 command)
{
- pr_debug(PREFIX "<--- command = 0x%2x\n", command);
+ pr_debug(PREFIX "<--- command = 0x%2.2x\n", command);
outb(command, ec->command_addr);
}
static inline void acpi_ec_write_data(struct acpi_ec *ec, u8 data)
{
- pr_debug(PREFIX "<--- data = 0x%2x\n", data);
+ pr_debug(PREFIX "<--- data = 0x%2.2x\n", data);
outb(data, ec->data_addr);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI: EC: add leading zeros to debug messages
Marton,
Could you please attach these patches to bug report? It is easier to track them
there.
Thanks,
Alex.
Németh Márton wrote:
From: Márton Németh [EMAIL PROTECTED]
Add leading zeros to pr_debug() calls. For example if x=0x0a, the format
0x%2x will result the string 0x a, the format 0x%2.2x will result 0x0a.
Signed-off-by: Márton Németh [EMAIL PROTECTED]
---
--- linux-2.6.24-rc8/drivers/acpi/ec.c.orig 2008-01-16 07:25:33.0
+0100
+++ linux-2.6.24-rc8/drivers/acpi/ec.c 2008-01-17 07:15:10.0 +0100
@@ -138,26 +138,26 @@ static struct acpi_ec {
static inline u8 acpi_ec_read_status(struct acpi_ec *ec)
{
u8 x = inb(ec-command_addr);
- pr_debug(PREFIX --- status = 0x%2x\n, x);
+ pr_debug(PREFIX --- status = 0x%2.2x\n, x);
return x;
}
static inline u8 acpi_ec_read_data(struct acpi_ec *ec)
{
u8 x = inb(ec-data_addr);
- pr_debug(PREFIX --- data = 0x%2x\n, x);
+ pr_debug(PREFIX --- data = 0x%2.2x\n, x);
return inb(ec-data_addr);
}
static inline void acpi_ec_write_cmd(struct acpi_ec *ec, u8 command)
{
- pr_debug(PREFIX --- command = 0x%2x\n, command);
+ pr_debug(PREFIX --- command = 0x%2.2x\n, command);
outb(command, ec-command_addr);
}
static inline void acpi_ec_write_data(struct acpi_ec *ec, u8 data)
{
- pr_debug(PREFIX --- data = 0x%2x\n, data);
+ pr_debug(PREFIX --- data = 0x%2.2x\n, data);
outb(data, ec-data_addr);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: kpowersave stuck at battery charging
Andrey Borzenkov wrote: On Wednesday 02 January 2008, Alexey Starikovskiy wrote: Andrey Borzenkov wrote: This is did not happen before; I am not sure right now what caused this (i.e. battery aging or some software change) nor whether this is kernel/HAL/kpowersave issue. kpowersave is stuck at assuming battery is loading and at 94%. Sysfs displays battery state as Full: Frequent battery charging shortens lifetime of the battery, so some (may be all now) notebook manufacturers do not start charging battery until it discharge to some degree (~90%). I thought Li-Ion batteries do not have memory effect. Actually I remember to have read recommendation to avoid deep discharges of Li-Ion battery, it was adviced to charge it as often as possible. IBM has advice to not re-charge battery until it discharged to 95%. It was implemented in their batteries/notebooks. There is no memory effect in Li-Ion batteries, there is only limited number of charge cycles -- about 1000 times, thus "as often as possible" is advice of the battery seller :) It could be your case. Please try to discharge battery to, say, 89% and then check if it charges to the 100%. That is exactly the question - how do you compute 100%? As far as I can tell the only possibility is - when battery stops charging. At this point you have to assume battery is fully charged. Last charge would be good reference. Think of the battery as constantly degrading resource. I tried to discharge battery (it was around 78%) and plug AC in again. It went on Charging until the same limit after that state changed to Full (well, in case of ACPI battery we really only can state - not (dis-)charging, there is no special Full state flag); kpowersave still believes battery is not fully charged. Main interface shows 84% (no Charging) - tooltip states it is being charged. So, that is the state of your battery. If you buy new one, it will go high again. Regards, Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kpowersave stuck at battery charging
Andrey Borzenkov wrote: This is did not happen before; I am not sure right now what caused this (i.e. battery aging or some software change) nor whether this is kernel/HAL/kpowersave issue. kpowersave is stuck at assuming battery is loading and at 94%. Sysfs displays battery state as Full: Frequent battery charging shortens lifetime of the battery, so some (may be all now) notebook manufacturers do not start charging battery until it discharge to some degree (~90%). It could be your case. Please try to discharge battery to, say, 89% and then check if it charges to the 100%. - does ACPI battery code misuse POWER_SUPPLY_PROP_ENERGY_FULL? - does HAL misuse .../energy_full? - does kpowersave misuse battery.charge_level.last_full? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kpowersave stuck at battery charging
Andrey Borzenkov wrote: This is did not happen before; I am not sure right now what caused this (i.e. battery aging or some software change) nor whether this is kernel/HAL/kpowersave issue. kpowersave is stuck at assuming battery is loading and at 94%. Sysfs displays battery state as Full: Frequent battery charging shortens lifetime of the battery, so some (may be all now) notebook manufacturers do not start charging battery until it discharge to some degree (~90%). It could be your case. Please try to discharge battery to, say, 89% and then check if it charges to the 100%. - does ACPI battery code misuse POWER_SUPPLY_PROP_ENERGY_FULL? - does HAL misuse .../energy_full? - does kpowersave misuse battery.charge_level.last_full? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kpowersave stuck at battery charging
Andrey Borzenkov wrote: On Wednesday 02 January 2008, Alexey Starikovskiy wrote: Andrey Borzenkov wrote: This is did not happen before; I am not sure right now what caused this (i.e. battery aging or some software change) nor whether this is kernel/HAL/kpowersave issue. kpowersave is stuck at assuming battery is loading and at 94%. Sysfs displays battery state as Full: Frequent battery charging shortens lifetime of the battery, so some (may be all now) notebook manufacturers do not start charging battery until it discharge to some degree (~90%). I thought Li-Ion batteries do not have memory effect. Actually I remember to have read recommendation to avoid deep discharges of Li-Ion battery, it was adviced to charge it as often as possible. IBM has advice to not re-charge battery until it discharged to 95%. It was implemented in their batteries/notebooks. There is no memory effect in Li-Ion batteries, there is only limited number of charge cycles -- about 1000 times, thus as often as possible is advice of the battery seller :) It could be your case. Please try to discharge battery to, say, 89% and then check if it charges to the 100%. That is exactly the question - how do you compute 100%? As far as I can tell the only possibility is - when battery stops charging. At this point you have to assume battery is fully charged. Last charge would be good reference. Think of the battery as constantly degrading resource. I tried to discharge battery (it was around 78%) and plug AC in again. It went on Charging until the same limit after that state changed to Full (well, in case of ACPI battery we really only can state - not (dis-)charging, there is no special Full state flag); kpowersave still believes battery is not fully charged. Main interface shows 84% (no Charging) - tooltip states it is being charged. So, that is the state of your battery. If you buy new one, it will go high again. Regards, Alex. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Suspend code ordering (again)
Rafael J. Wysocki wrote: On Wednesday, 26 of December 2007, Linus Torvalds wrote: On Tue, 25 Dec 2007, Rafael J. Wysocki wrote: the ACPI specification between versions 1.0x and 2.0. Namely, while ACPI 2.0 and later wants us to put devices into low power states before calling _PTS, ACPI 1.0x wants us to do that after calling _PTS. Since we're following the 2.0 and later specifications right now, we're not doing the right thing for the (strictly) ACPI 1.0x-compliant systems. We ought to be able to fix things on the high level, by calling _PTS earlier on systems that claim to be ACPI 1.0x-compliant. That will require us to modify the generic susped code quite a bit and will need to be tested for some time. That's insane. Are you really saying that ACPI wants totally different orderings for different versions of the spec? Yes, I am. And does Windows really do that? I don't know. Windows was compliant only with 1.x spec until Vista. With Vista claims are 3.x compliance. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Suspend code ordering (again)
Rafael J. Wysocki wrote: On Wednesday, 26 of December 2007, Linus Torvalds wrote: On Tue, 25 Dec 2007, Rafael J. Wysocki wrote: the ACPI specification between versions 1.0x and 2.0. Namely, while ACPI 2.0 and later wants us to put devices into low power states before calling _PTS, ACPI 1.0x wants us to do that after calling _PTS. Since we're following the 2.0 and later specifications right now, we're not doing the right thing for the (strictly) ACPI 1.0x-compliant systems. We ought to be able to fix things on the high level, by calling _PTS earlier on systems that claim to be ACPI 1.0x-compliant. That will require us to modify the generic susped code quite a bit and will need to be tested for some time. That's insane. Are you really saying that ACPI wants totally different orderings for different versions of the spec? Yes, I am. And does Windows really do that? I don't know. Windows was compliant only with 1.x spec until Vista. With Vista claims are 3.x compliance. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc4-mm1: acpi reboots machine... solved
Borislav Petkov wrote: On Tue, Dec 11, 2007 at 05:08:59PM -0700, Bjorn Helgaas wrote: On Tuesday 11 December 2007 01:52:55 pm Borislav Petkov wrote: From what i can roughly tell so far it seems like an resource conflict between acpi and the pnp requested regions in your patch which result in the acpi_thermal code to read the wrong (0xff) temperature value and halt the machine, but i might be wrong on the details since acpi is such a big code chunk to swallow. I don't see any obvious conflict from the log you posted. For the sake of comparison, can you post the corresponding dmesg log after you removed the patch? The only difference i see is that ACPI finds EC in DSDT in the working kernel and in the broken case something silently fails. Please find attached the 2 bootlogs and a disassembled DSDT. This seems to be the start of trouble... PCI: Cannot allocate resource region 4 of device :00:1f.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc4-mm1: acpi reboots machine... solved
Borislav Petkov wrote: On Tue, Dec 11, 2007 at 05:08:59PM -0700, Bjorn Helgaas wrote: On Tuesday 11 December 2007 01:52:55 pm Borislav Petkov wrote: From what i can roughly tell so far it seems like an resource conflict between acpi and the pnp requested regions in your patch which result in the acpi_thermal code to read the wrong (0xff) temperature value and halt the machine, but i might be wrong on the details since acpi is such a big code chunk to swallow. I don't see any obvious conflict from the log you posted. For the sake of comparison, can you post the corresponding dmesg log after you removed the patch? The only difference i see is that ACPI finds EC in DSDT in the working kernel and in the broken case something silently fails. Please find attached the 2 bootlogs and a disassembled DSDT. This seems to be the start of trouble... PCI: Cannot allocate resource region 4 of device :00:1f.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix segfault when printing battery status
Rolf Eike Beer wrote: Alexey Starikovskiy wrote: Rolf Eike Beer wrote: cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/PNP0C0A:00/power_ supply/BAT1/status This leads to a stacktrace as acpi_battery_get_property() returns 0 for a case where it does not set val->intval. These value is used as an array index in drivers/power/power_supply_sysfs.c::power_supply_show_property(). I had a situation where the value was 4096 which caused a problem as the array only has 5 entries. Signed-off-by: Rolf Eike Beer <[EMAIL PROTECTED]> Rolf, thanks for remainding. Huh? This one is unrelated to the problem I reported two weeks ago... Eike You are second to send the same patch, first one I already acked. But it seems that Len did not pick it up yet. Look for "ACPI: Always return valid 'status' from acpi_battery_get_property()" if interested... Regards, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix segfault when printing battery status
Rolf Eike Beer wrote: cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/PNP0C0A:00/power_supply/BAT1/status This leads to a stacktrace as acpi_battery_get_property() returns 0 for a case where it does not set val->intval. These value is used as an array index in drivers/power/power_supply_sysfs.c::power_supply_show_property(). I had a situation where the value was 4096 which caused a problem as the array only has 5 entries. Signed-off-by: Rolf Eike Beer <[EMAIL PROTECTED]> Rolf, thanks for remainding. Acked again, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix segfault when printing battery status
Rolf Eike Beer wrote: cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/PNP0C0A:00/power_supply/BAT1/status This leads to a stacktrace as acpi_battery_get_property() returns 0 for a case where it does not set val-intval. These value is used as an array index in drivers/power/power_supply_sysfs.c::power_supply_show_property(). I had a situation where the value was 4096 which caused a problem as the array only has 5 entries. Signed-off-by: Rolf Eike Beer [EMAIL PROTECTED] Rolf, thanks for remainding. Acked again, Alex. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix segfault when printing battery status
Rolf Eike Beer wrote: Alexey Starikovskiy wrote: Rolf Eike Beer wrote: cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/PNP0C0A:00/power_ supply/BAT1/status This leads to a stacktrace as acpi_battery_get_property() returns 0 for a case where it does not set val-intval. These value is used as an array index in drivers/power/power_supply_sysfs.c::power_supply_show_property(). I had a situation where the value was 4096 which caused a problem as the array only has 5 entries. Signed-off-by: Rolf Eike Beer [EMAIL PROTECTED] Rolf, thanks for remainding. Huh? This one is unrelated to the problem I reported two weeks ago... Eike You are second to send the same patch, first one I already acked. But it seems that Len did not pick it up yet. Look for ACPI: Always return valid 'status' from acpi_battery_get_property() if interested... Regards, Alex. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc1: OOPS at acpi_battery_update
Andrew,
I can not contact with Len for several days, while the oops on battery
seems quite important.
It also seem to behave well in -mm tree (as part of Len's acpi-test).
Will you send this patch to Linus without approval from Len or should I?
Thanks,
Alex.
Andrew Morton wrote:
On Fri, 09 Nov 2007 12:36:43 +0300 Alexey Starikovskiy <[EMAIL PROTECTED]>
wrote:
Andrew Morton wrote:
A> On Thu, 08 Nov 2007 19:35:23 +0300 Alexey Starikovskiy <[EMAIL PROTECTED]>
wrote:
[remove_cycle_at_battery_removal.patch text/x-patch (1.7KB)]
ACPI: Battery: remove cycle from battery removal.
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
Tested-by: Rolf Eike Beer <[EMAIL PROTECTED]>
A patch similar to this one but with an identical changelog was merged into
Len's tree on November 2.
If it had been promptly merged into mainline then quite a lot of people's
time would not have been wasted.
Andrew,
should I send such patches directly to you/Linus?
Well if Len doesn't object and you're confident in the changes, why not?
Any time we leave bugs unfixed we drive away testers and that impacts all
parts of the kernel.
ACPI: Battery: remove cycle from battery removal.
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
Tested-by: Rolf Eike Beer <[EMAIL PROTECTED]>
---
drivers/acpi/battery.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..192c244 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -132,7 +132,7 @@ static int acpi_battery_technology(struct acpi_battery *battery)
return POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
}
-static int acpi_battery_update(struct acpi_battery *battery);
+static int acpi_battery_get_state(struct acpi_battery *battery);
static int acpi_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery)) &&
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_get_state(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery->state & 0x01)
@@ -457,6 +458,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery)
return;
device_remove_file(battery->bat.dev, _attr);
power_supply_unregister(>bat);
+ battery->bat.dev = NULL;
}
static int acpi_battery_update(struct acpi_battery *battery)
Re: 2.6.24-rc1: OOPS at acpi_battery_update
Andrew,
I can not contact with Len for several days, while the oops on battery
seems quite important.
It also seem to behave well in -mm tree (as part of Len's acpi-test).
Will you send this patch to Linus without approval from Len or should I?
Thanks,
Alex.
Andrew Morton wrote:
On Fri, 09 Nov 2007 12:36:43 +0300 Alexey Starikovskiy [EMAIL PROTECTED]
wrote:
Andrew Morton wrote:
A On Thu, 08 Nov 2007 19:35:23 +0300 Alexey Starikovskiy [EMAIL PROTECTED]
wrote:
[remove_cycle_at_battery_removal.patch text/x-patch (1.7KB)]
ACPI: Battery: remove cycle from battery removal.
From: Alexey Starikovskiy [EMAIL PROTECTED]
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
Tested-by: Rolf Eike Beer [EMAIL PROTECTED]
A patch similar to this one but with an identical changelog was merged into
Len's tree on November 2.
If it had been promptly merged into mainline then quite a lot of people's
time would not have been wasted.
Andrew,
should I send such patches directly to you/Linus?
Well if Len doesn't object and you're confident in the changes, why not?
Any time we leave bugs unfixed we drive away testers and that impacts all
parts of the kernel.
ACPI: Battery: remove cycle from battery removal.
From: Alexey Starikovskiy [EMAIL PROTECTED]
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
Tested-by: Rolf Eike Beer [EMAIL PROTECTED]
---
drivers/acpi/battery.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..192c244 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -132,7 +132,7 @@ static int acpi_battery_technology(struct acpi_battery *battery)
return POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
}
-static int acpi_battery_update(struct acpi_battery *battery);
+static int acpi_battery_get_state(struct acpi_battery *battery);
static int acpi_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery))
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_get_state(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery-state 0x01)
@@ -457,6 +458,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery)
return;
device_remove_file(battery-bat.dev, alarm_attr);
power_supply_unregister(battery-bat);
+ battery-bat.dev = NULL;
}
static int acpi_battery_update(struct acpi_battery *battery)
Re: [PATCH] 2.6.24-rc2: fix oops in ACPI battery removal
Andrey,
Fix already exists. It already went up to Len's tree.
Regards,
Alex.
Andrey Borzenkov wrote:
Alexey, this fixes my patch that went into -rc2. Sorry for Oops.
Subject: [PATCH] 2.6.24-rc2: do not unregister power_supply in sysfs ->show
method
From: Andrey Borzenkov <[EMAIL PROTECTED]>
Currently unplugging battery oopses with call stack:
[ 2245.375393] [] show_trace_log_lvl+0x1a/0x30
[ 2245.375570] [] show_stack_log_lvl+0xa9/0xd0
[ 2245.375732] [] show_registers+0xc6/0x1c0
[ 2245.375884] [] die+0xf1/0x200
[ 2245.376014] [] do_page_fault+0x198/0x690
[ 2245.376207] [] error_code+0x6a/0x70
[ 2245.376366] [] device_del+0x20/0x280
[ 2245.376530] [] device_unregister+0xb/0x20
[ 2245.376686] [] power_supply_unregister+0x1a/0x20 [power_supply]
[ 2245.376911] [] sysfs_remove_battery+0x1f/0x22 [battery]
[ 2245.382969] [] acpi_battery_update+0x42/0x293 [battery]
[ 2245.388862] [] acpi_battery_get_property+0x34/0x16e [battery]
[ 2245.394770] [] power_supply_show_property+0x38/0x130
[power_supply]
[ 2245.400733] [] power_supply_uevent+0x111/0x1c0 [power_supply]
[ 2245.406745] [] dev_uevent+0x99/0x100
[ 2245.412744] [] kobject_uevent_env+0x17f/0x3c0
[ 2245.418768] [] kobject_uevent+0xa/0x10
[ 2245.424732] [] device_del+0x1a3/0x280
[ 2245.430683] [] device_unregister+0xb/0x20
[ 2245.436632] [] power_supply_unregister+0x1a/0x20 [power_supply]
[ 2245.442670] [] sysfs_remove_battery+0x1f/0x22 [battery]
[ 2245.448790] [] acpi_battery_update+0x42/0x293 [battery]
[ 2245.454874] [] acpi_battery_notify+0x1e/0x69 [battery]
[ 2245.460957] [] acpi_ev_notify_dispatch+0x4c/0x57
[ 2245.467077] [] acpi_os_execute_notify+0x24/0x2f
[ 2245.473167] [] run_workqueue+0x161/0x1e0
[ 2245.479289] [] worker_thread+0x8b/0xf0
[ 2245.485279] [] kthread+0x42/0x70
[ 2245.491068] [] kernel_thread_helper+0x7/0x14
Do not try to (un-)register sysfs in acpi_battery_update - instead, do
it only on battery add or in notification handler.
On the way fix battery removal (reset device to NULL) and use proper
interface for sending change event (power_supply_changed) instead of
doing it directly.
Signed-off-by: Andrey Borzenkov <[EMAIL PROTECTED]>
---
drivers/acpi/battery.c | 18 +-
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 1905b88..2810a9b 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -457,6 +457,7 @@ static void sysfs_remove_battery(struct acpi_battery
*battery)
return;
device_remove_file(battery->bat.dev, _attr);
power_supply_unregister(>bat);
+ battery->bat.dev = NULL;
}
static int acpi_battery_update(struct acpi_battery *battery)
@@ -464,12 +465,6 @@ static int acpi_battery_update(struct acpi_battery
*battery)
int result = acpi_battery_get_status(battery);
if (result)
return result;
- if (!acpi_battery_present(battery)) {
- sysfs_remove_battery(battery);
- return 0;
- }
- if (!battery->bat.dev)
- sysfs_add_battery(battery);
return acpi_battery_get_state(battery);
}
@@ -758,14 +753,17 @@ static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
return;
device = battery->device;
acpi_battery_update(battery);
+ if (!acpi_battery_present(battery))
+ sysfs_remove_battery(battery);
+ else if (!battery->bat.dev)
+ sysfs_add_battery(battery);
+ else
+ power_supply_changed(>bat);
acpi_bus_generate_proc_event(device, event,
acpi_battery_present(battery));
acpi_bus_generate_netlink_event(device->pnp.device_class,
device->dev.bus_id, event,
acpi_battery_present(battery));
- /* acpi_batter_update could remove power_supply object */
- if (battery->bat.dev)
- kobject_uevent(>bat.dev->kobj, KOBJ_CHANGE);
}
static int acpi_battery_add(struct acpi_device *device)
@@ -784,6 +782,8 @@ static int acpi_battery_add(struct acpi_device *device)
acpi_driver_data(device) = battery;
mutex_init(>lock);
acpi_battery_update(battery);
+ if (acpi_battery_present(battery))
+ sysfs_add_battery(battery);
#ifdef CONFIG_ACPI_PROCFS
result = acpi_battery_add_fs(device);
if (result)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] 2.6.24-rc2: fix oops in ACPI battery removal
Andrey,
Fix already exists. It already went up to Len's tree.
Regards,
Alex.
Andrey Borzenkov wrote:
Alexey, this fixes my patch that went into -rc2. Sorry for Oops.
Subject: [PATCH] 2.6.24-rc2: do not unregister power_supply in sysfs -show
method
From: Andrey Borzenkov [EMAIL PROTECTED]
Currently unplugging battery oopses with call stack:
[ 2245.375393] [c010488a] show_trace_log_lvl+0x1a/0x30
[ 2245.375570] [c0104949] show_stack_log_lvl+0xa9/0xd0
[ 2245.375732] [c0104a36] show_registers+0xc6/0x1c0
[ 2245.375884] [c0104c21] die+0xf1/0x200
[ 2245.376014] [c02d2a58] do_page_fault+0x198/0x690
[ 2245.376207] [c02d1522] error_code+0x6a/0x70
[ 2245.376366] [c0243c20] device_del+0x20/0x280
[ 2245.376530] [c0243e8b] device_unregister+0xb/0x20
[ 2245.376686] [dfd7401a] power_supply_unregister+0x1a/0x20 [power_supply]
[ 2245.376911] [dfd5f035] sysfs_remove_battery+0x1f/0x22 [battery]
[ 2245.382969] [dfd5f1e6] acpi_battery_update+0x42/0x293 [battery]
[ 2245.388862] [dfd5f5f5] acpi_battery_get_property+0x34/0x16e [battery]
[ 2245.394770] [dfd74368] power_supply_show_property+0x38/0x130
[power_supply]
[ 2245.400733] [dfd746f1] power_supply_uevent+0x111/0x1c0 [power_supply]
[ 2245.406745] [c0244559] dev_uevent+0x99/0x100
[ 2245.412744] [c01d37ff] kobject_uevent_env+0x17f/0x3c0
[ 2245.418768] [c01d3a4a] kobject_uevent+0xa/0x10
[ 2245.424732] [c0243da3] device_del+0x1a3/0x280
[ 2245.430683] [c0243e8b] device_unregister+0xb/0x20
[ 2245.436632] [dfd7401a] power_supply_unregister+0x1a/0x20 [power_supply]
[ 2245.442670] [dfd5f035] sysfs_remove_battery+0x1f/0x22 [battery]
[ 2245.448790] [dfd5f1e6] acpi_battery_update+0x42/0x293 [battery]
[ 2245.454874] [dfd5f7ad] acpi_battery_notify+0x1e/0x69 [battery]
[ 2245.460957] [c02086cd] acpi_ev_notify_dispatch+0x4c/0x57
[ 2245.467077] [c0200d54] acpi_os_execute_notify+0x24/0x2f
[ 2245.473167] [c012a431] run_workqueue+0x161/0x1e0
[ 2245.479289] [c012ae5b] worker_thread+0x8b/0xf0
[ 2245.485279] [c012e0b2] kthread+0x42/0x70
[ 2245.491068] [c01044e3] kernel_thread_helper+0x7/0x14
Do not try to (un-)register sysfs in acpi_battery_update - instead, do
it only on battery add or in notification handler.
On the way fix battery removal (reset device to NULL) and use proper
interface for sending change event (power_supply_changed) instead of
doing it directly.
Signed-off-by: Andrey Borzenkov [EMAIL PROTECTED]
---
drivers/acpi/battery.c | 18 +-
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 1905b88..2810a9b 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -457,6 +457,7 @@ static void sysfs_remove_battery(struct acpi_battery
*battery)
return;
device_remove_file(battery-bat.dev, alarm_attr);
power_supply_unregister(battery-bat);
+ battery-bat.dev = NULL;
}
static int acpi_battery_update(struct acpi_battery *battery)
@@ -464,12 +465,6 @@ static int acpi_battery_update(struct acpi_battery
*battery)
int result = acpi_battery_get_status(battery);
if (result)
return result;
- if (!acpi_battery_present(battery)) {
- sysfs_remove_battery(battery);
- return 0;
- }
- if (!battery-bat.dev)
- sysfs_add_battery(battery);
return acpi_battery_get_state(battery);
}
@@ -758,14 +753,17 @@ static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
return;
device = battery-device;
acpi_battery_update(battery);
+ if (!acpi_battery_present(battery))
+ sysfs_remove_battery(battery);
+ else if (!battery-bat.dev)
+ sysfs_add_battery(battery);
+ else
+ power_supply_changed(battery-bat);
acpi_bus_generate_proc_event(device, event,
acpi_battery_present(battery));
acpi_bus_generate_netlink_event(device-pnp.device_class,
device-dev.bus_id, event,
acpi_battery_present(battery));
- /* acpi_batter_update could remove power_supply object */
- if (battery-bat.dev)
- kobject_uevent(battery-bat.dev-kobj, KOBJ_CHANGE);
}
static int acpi_battery_add(struct acpi_device *device)
@@ -784,6 +782,8 @@ static int acpi_battery_add(struct acpi_device *device)
acpi_driver_data(device) = battery;
mutex_init(battery-lock);
acpi_battery_update(battery);
+ if (acpi_battery_present(battery))
+ sysfs_add_battery(battery);
#ifdef CONFIG_ACPI_PROCFS
result = acpi_battery_add_fs(device);
if (result)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc1: OOPS at acpi_battery_update
Andrew Morton wrote: A> On Thu, 08 Nov 2007 19:35:23 +0300 Alexey Starikovskiy <[EMAIL PROTECTED]> wrote: [remove_cycle_at_battery_removal.patch text/x-patch (1.7KB)] ACPI: Battery: remove cycle from battery removal. From: Alexey Starikovskiy <[EMAIL PROTECTED]> get_property() should not call battery_update() on absent battery to avoid cycle and oops. Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> Tested-by: Rolf Eike Beer <[EMAIL PROTECTED]> A patch similar to this one but with an identical changelog was merged into Len's tree on November 2. If it had been promptly merged into mainline then quite a lot of people's time would not have been wasted. Andrew, should I send such patches directly to you/Linus? Regards, Alex - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc1: OOPS at acpi_battery_update
Andrew Morton wrote: A On Thu, 08 Nov 2007 19:35:23 +0300 Alexey Starikovskiy [EMAIL PROTECTED] wrote: [remove_cycle_at_battery_removal.patch text/x-patch (1.7KB)] ACPI: Battery: remove cycle from battery removal. From: Alexey Starikovskiy [EMAIL PROTECTED] get_property() should not call battery_update() on absent battery to avoid cycle and oops. Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED] Tested-by: Rolf Eike Beer [EMAIL PROTECTED] A patch similar to this one but with an identical changelog was merged into Len's tree on November 2. If it had been promptly merged into mainline then quite a lot of people's time would not have been wasted. Andrew, should I send such patches directly to you/Linus? Regards, Alex - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc1: OOPS at acpi_battery_update
Rafael J. Wysocki wrote:
On Thursday, 8 of November 2007, Johannes Weiner wrote:
Hi,
is there any reason, why acpi_battery_get_property() should call
acpi_battery_update() at all?
Alex?
Do you mean "why should it call _whole_ battery update?" ?
get_state should be sufficient in order to not get stale data.
Regards,
Alex.
ACPI: Battery: remove cycle from battery removal.
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
Tested-by: Rolf Eike Beer <[EMAIL PROTECTED]>
---
drivers/acpi/battery.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..192c244 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -132,7 +132,7 @@ static int acpi_battery_technology(struct acpi_battery *battery)
return POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
}
-static int acpi_battery_update(struct acpi_battery *battery);
+static int acpi_battery_get_state(struct acpi_battery *battery);
static int acpi_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery)) &&
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_get_state(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery->state & 0x01)
@@ -457,6 +458,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery)
return;
device_remove_file(battery->bat.dev, _attr);
power_supply_unregister(>bat);
+ battery->bat.dev = NULL;
}
static int acpi_battery_update(struct acpi_battery *battery)
Re: 2.6.24-rc1: OOPS at acpi_battery_update
Rafael J. Wysocki wrote: On Thursday, 8 of November 2007, Johannes Weiner wrote: Hi, is there any reason, why acpi_battery_get_property() should call acpi_battery_update() at all? Alex? If someone wants to read stale values, he could comment out acpi_battery_update. Regards, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc2 (esthetic?) regression: auto select interrupt spams my logs
Hi Romano, EC was changed to automatically choose it's working mode (poll vs. interrupt driven). You see it oscillating between modes because it receives interrupts just after it stops waiting for them. Please open new bug entry at bugzilla.kernel.org. Your .config might be usefull. Thanks, Alex. Romano Giannetti wrote: Hi, After the ACPI changes between 2.6.24-rc1 and -rc2 I have my logs "spammed" (every 2-3 seconds) by: [ 423.112903] ACPI: EC: missing IBF_1 confirmations,switch off interrupt mode. [ 423.113020] ACPI: EC: non-query interrupt received, switching to interrupt mode [ 426.078972] ACPI: EC: missing IBF_1 confirmations,switch off interrupt mode. [ 426.079645] ACPI: EC: non-query interrupt received, switching to interrupt mode [ 426.622773] ACPI: EC: missing IBF_1 confirmations,switch off interrupt mode. [ 426.622889] ACPI: EC: non-query interrupt received, switching to interrupt mode It seems that no harm is done, apart (but it could be another thing) that gnome-panel is much slower on startup. Romano - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc2 (esthetic?) regression: auto select interrupt spams my logs
Hi Romano, EC was changed to automatically choose it's working mode (poll vs. interrupt driven). You see it oscillating between modes because it receives interrupts just after it stops waiting for them. Please open new bug entry at bugzilla.kernel.org. Your .config might be usefull. Thanks, Alex. Romano Giannetti wrote: Hi, After the ACPI changes between 2.6.24-rc1 and -rc2 I have my logs spammed (every 2-3 seconds) by: [ 423.112903] ACPI: EC: missing IBF_1 confirmations,switch off interrupt mode. [ 423.113020] ACPI: EC: non-query interrupt received, switching to interrupt mode [ 426.078972] ACPI: EC: missing IBF_1 confirmations,switch off interrupt mode. [ 426.079645] ACPI: EC: non-query interrupt received, switching to interrupt mode [ 426.622773] ACPI: EC: missing IBF_1 confirmations,switch off interrupt mode. [ 426.622889] ACPI: EC: non-query interrupt received, switching to interrupt mode It seems that no harm is done, apart (but it could be another thing) that gnome-panel is much slower on startup. Romano - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc1: OOPS at acpi_battery_update
Rafael J. Wysocki wrote: On Thursday, 8 of November 2007, Johannes Weiner wrote: Hi, is there any reason, why acpi_battery_get_property() should call acpi_battery_update() at all? Alex? If someone wants to read stale values, he could comment out acpi_battery_update. Regards, Alex. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc1: OOPS at acpi_battery_update
Rafael J. Wysocki wrote:
On Thursday, 8 of November 2007, Johannes Weiner wrote:
Hi,
is there any reason, why acpi_battery_get_property() should call
acpi_battery_update() at all?
Alex?
Do you mean why should it call _whole_ battery update? ?
get_state should be sufficient in order to not get stale data.
Regards,
Alex.
ACPI: Battery: remove cycle from battery removal.
From: Alexey Starikovskiy [EMAIL PROTECTED]
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
Tested-by: Rolf Eike Beer [EMAIL PROTECTED]
---
drivers/acpi/battery.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..192c244 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -132,7 +132,7 @@ static int acpi_battery_technology(struct acpi_battery *battery)
return POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
}
-static int acpi_battery_update(struct acpi_battery *battery);
+static int acpi_battery_get_state(struct acpi_battery *battery);
static int acpi_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery))
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_get_state(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery-state 0x01)
@@ -457,6 +458,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery)
return;
device_remove_file(battery-bat.dev, alarm_attr);
power_supply_unregister(battery-bat);
+ battery-bat.dev = NULL;
}
static int acpi_battery_update(struct acpi_battery *battery)
Re: [PATCH] 2.6.24: make /proc/acpi/ac_adapter dependent on ACPI_PROCFS
Andrey Borzenkov wrote: Let's make it consistent with battery code; also it fixes HAL case of duplicated adapters. Somebody will have to sort out HAL with ACPI_PROCFS case ... -andrey Andrey, Please excuse me, I mis-understood your comment, and thought that you are trying to do AC adapter register/unregister at on/off. Acked-by: Alexey Starikovskiy <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] 2.6.24: make /proc/acpi/ac_adapter dependent on ACPI_PROCFS
Andrey Borzenkov wrote: Let's make it consistent with battery code; also it fixes HAL case of duplicated adapters. Somebody will have to sort out HAL with ACPI_PROCFS case ... -andrey Andrey, Please excuse me, I mis-understood your comment, and thought that you are trying to do AC adapter register/unregister at on/off. Acked-by: Alexey Starikovskiy [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6.24-rc1][BUG] Oops on battery removal
Rolf Eike Beer wrote:
Alexey Starikovskiy wrote:
Rolf Eike Beer wrote:
Alexey Starikovskiy wrote:
Rolf Eike Beer wrote:
Rolf Eike Beer wrote:
Hi,
this happened while I removed my battery on bootup. Complete dmesg is
attached. Kernel is 2.6.24-rc1-git of yesterday (last commit was
d919fd433b5823d1cf9d0688eb2eec183de9b74c).
Ok, I found out that it has nothing to do with the actual removal as it
seems. When I start the laptop with battery already detached I get the
same error.
Please check if this patch helps.
It made things worse:
How about this patch?
Ehm, that is the same patch as before!
Yes, sorry... Two patches in parallel -- not good.
Btw: it has an empty From: line
I know, thanks.
Eike
Patch updated.
ACPI: Battery: remove cycle from battery removal.
From: <>
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/battery.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..ca37db4 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery)) &&
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_update(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery->state & 0x01)
@@ -457,6 +458,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery)
return;
device_remove_file(battery->bat.dev, _attr);
power_supply_unregister(>bat);
+ battery->bat.dev = NULL;
}
static int acpi_battery_update(struct acpi_battery *battery)
Re: [2.6.24-rc1][BUG] Oops on battery removal
Rolf Eike Beer wrote:
Alexey Starikovskiy wrote:
Rolf Eike Beer wrote:
Rolf Eike Beer wrote:
Hi,
this happened while I removed my battery on bootup. Complete dmesg is
attached. Kernel is 2.6.24-rc1-git of yesterday (last commit was
d919fd433b5823d1cf9d0688eb2eec183de9b74c).
Ok, I found out that it has nothing to do with the actual removal as it
seems. When I start the laptop with battery already detached I get the
same error.
Please check if this patch helps.
It made things worse:
How about this patch?
ACPI: Battery: remove cycle from battery removal.
From: <>
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/battery.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..50cdf6f 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery)) &&
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_update(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery->state & 0x01)
Re: [2.6.24-rc1][BUG] Oops on battery removal
Rolf Eike Beer wrote:
Rolf Eike Beer wrote:
Hi,
this happened while I removed my battery on bootup. Complete dmesg is
attached. Kernel is 2.6.24-rc1-git of yesterday (last commit was
d919fd433b5823d1cf9d0688eb2eec183de9b74c).
Ok, I found out that it has nothing to do with the actual removal as it seems.
When I start the laptop with battery already detached I get the same error.
Eike
Please check if this patch helps.
Thanks,
Alex.
ACPI: Battery: remove cycle from battery removal.
From: <>
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/battery.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..50cdf6f 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery)) &&
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_update(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery->state & 0x01)
Re: [2.6.24-rc1][BUG] Oops on battery removal
Rolf Eike Beer wrote:
Alexey Starikovskiy wrote:
Rolf Eike Beer wrote:
Alexey Starikovskiy wrote:
Rolf Eike Beer wrote:
Rolf Eike Beer wrote:
Hi,
this happened while I removed my battery on bootup. Complete dmesg is
attached. Kernel is 2.6.24-rc1-git of yesterday (last commit was
d919fd433b5823d1cf9d0688eb2eec183de9b74c).
Ok, I found out that it has nothing to do with the actual removal as it
seems. When I start the laptop with battery already detached I get the
same error.
Please check if this patch helps.
It made things worse:
How about this patch?
Ehm, that is the same patch as before!
Yes, sorry... Two patches in parallel -- not good.
Btw: it has an empty From: line
I know, thanks.
Eike
Patch updated.
ACPI: Battery: remove cycle from battery removal.
From:
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
---
drivers/acpi/battery.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..ca37db4 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery))
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_update(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery-state 0x01)
@@ -457,6 +458,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery)
return;
device_remove_file(battery-bat.dev, alarm_attr);
power_supply_unregister(battery-bat);
+ battery-bat.dev = NULL;
}
static int acpi_battery_update(struct acpi_battery *battery)
Re: [2.6.24-rc1][BUG] Oops on battery removal
Rolf Eike Beer wrote:
Alexey Starikovskiy wrote:
Rolf Eike Beer wrote:
Rolf Eike Beer wrote:
Hi,
this happened while I removed my battery on bootup. Complete dmesg is
attached. Kernel is 2.6.24-rc1-git of yesterday (last commit was
d919fd433b5823d1cf9d0688eb2eec183de9b74c).
Ok, I found out that it has nothing to do with the actual removal as it
seems. When I start the laptop with battery already detached I get the
same error.
Please check if this patch helps.
It made things worse:
How about this patch?
ACPI: Battery: remove cycle from battery removal.
From:
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
---
drivers/acpi/battery.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..50cdf6f 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery))
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_update(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery-state 0x01)
Re: [2.6.24-rc1][BUG] Oops on battery removal
Rolf Eike Beer wrote:
Rolf Eike Beer wrote:
Hi,
this happened while I removed my battery on bootup. Complete dmesg is
attached. Kernel is 2.6.24-rc1-git of yesterday (last commit was
d919fd433b5823d1cf9d0688eb2eec183de9b74c).
Ok, I found out that it has nothing to do with the actual removal as it seems.
When I start the laptop with battery already detached I get the same error.
Eike
Please check if this patch helps.
Thanks,
Alex.
ACPI: Battery: remove cycle from battery removal.
From:
get_property() should not call battery_update() on absent battery to
avoid cycle and oops.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
---
drivers/acpi/battery.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..50cdf6f 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
{
struct acpi_battery *battery = to_acpi_battery(psy);
- if ((!acpi_battery_present(battery))
- psp != POWER_SUPPLY_PROP_PRESENT)
+ if (acpi_battery_present(battery)) {
+ /* run battery update only if it is present */
+ acpi_battery_update(battery);
+ } else if (psp != POWER_SUPPLY_PROP_PRESENT)
return -ENODEV;
- acpi_battery_update(battery);
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (battery-state 0x01)
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
Andrey Borzenkov wrote:
> On Wednesday 31 October 2007, Alexey Starikovskiy wrote:
>> Andrey Borzenkov wrote:
>>> I suspect new ACPI AC adapter code but have to add some printk's to be
>>> sure.
>>>
>>> To reproduce - plug in AC cord, suspend, unplug, resume - kpowersave and
>>> sysfs still show AC adapter online. Or other way round.
>>>
>>> -andrey
>> Please check if this patch helps.
>>
>
> It does not even compile :)
>
> On serious note (after adding forward declaration) - patch half works. It
> does
> make "online" return true value (which confirms that underlying ACPI works
> correctly) but HAL still does not notice it.
>
> The problem is, with power_supply interface HAL does not use polling, it
> relies on CHANGE event. This event is apparently never generated if AC
> adapter state changes on resume. So the obvious fix is to compare AC state
> before and after suspend in ->resume function and generate synthetic CHANGE
> event if something has changed.
>
> This will also make superfluous polling redundant.
Ok, here. This one compiles :)
Regards,
Alex.
ACPI: AC: Update AC state on resume
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
Check if AC state has changed across resume and notify userspace if so.
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/ac.c | 17 +
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index e03de37..06308ff 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -54,6 +54,7 @@ extern void *acpi_unlock_ac_dir(struct proc_dir_entry *acpi_ac_dir);
static int acpi_ac_add(struct acpi_device *device);
static int acpi_ac_remove(struct acpi_device *device, int type);
+static int acpi_ac_resume(struct acpi_device *device);
static int acpi_ac_open_fs(struct inode *inode, struct file *file);
const static struct acpi_device_id ac_device_ids[] = {
@@ -69,6 +70,7 @@ static struct acpi_driver acpi_ac_driver = {
.ops = {
.add = acpi_ac_add,
.remove = acpi_ac_remove,
+ .resume = acpi_ac_resume,
},
};
@@ -294,6 +296,21 @@ static int acpi_ac_add(struct acpi_device *device)
return result;
}
+static int acpi_ac_resume(struct acpi_device *device)
+{
+ struct acpi_ac *ac;
+ unsigned old_state;
+ if (!device || !acpi_driver_data(device))
+ return -EINVAL;
+ ac = acpi_driver_data(device);
+ old_state = ac->state;
+ if (acpi_ac_get_state(ac))
+ return 0;
+ if (old_state != ac->state)
+ kobject_uevent(>charger.dev->kobj, KOBJ_CHANGE);
+ return 0;
+}
+
static int acpi_ac_remove(struct acpi_device *device, int type)
{
acpi_status status = AE_OK;
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
Andrey Borzenkov wrote:
> I suspect new ACPI AC adapter code but have to add some printk's to be sure.
>
> To reproduce - plug in AC cord, suspend, unplug, resume - kpowersave and
> sysfs
> still show AC adapter online. Or other way round.
>
> -andrey
Please check if this patch helps.
Regards,
Alex.
ACPI: AC: Update AC state on sysfs read
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/ac.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index e03de37..bb618c8 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -91,6 +91,9 @@ static int get_ac_property(struct power_supply *psy,
union power_supply_propval *val)
{
struct acpi_ac *ac = to_acpi_ac(psy);
+
+ if (acpi_ac_get_state(ac))
+ return 0;
switch (psp) {
case POWER_SUPPLY_PROP_ONLINE:
val->intval = ac->state;
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
Andrey Borzenkov wrote: > I suspect new ACPI AC adapter code but have to add some printk's to be sure. > > To reproduce - plug in AC cord, suspend, unplug, resume - kpowersave and > sysfs > still show AC adapter online. Or other way round. > > -andrey The only change to drivers/acpi/ac.c after .23 was d5b4a3d0efa36de31b86d5677dad6c36cb8735d7. Don't see how it could lead to less messages from AC. Regards, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
Andrey Borzenkov wrote: I suspect new ACPI AC adapter code but have to add some printk's to be sure. To reproduce - plug in AC cord, suspend, unplug, resume - kpowersave and sysfs still show AC adapter online. Or other way round. -andrey The only change to drivers/acpi/ac.c after .23 was d5b4a3d0efa36de31b86d5677dad6c36cb8735d7. Don't see how it could lead to less messages from AC. Regards, Alex. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
Andrey Borzenkov wrote:
I suspect new ACPI AC adapter code but have to add some printk's to be sure.
To reproduce - plug in AC cord, suspend, unplug, resume - kpowersave and
sysfs
still show AC adapter online. Or other way round.
-andrey
Please check if this patch helps.
Regards,
Alex.
ACPI: AC: Update AC state on sysfs read
From: Alexey Starikovskiy [EMAIL PROTECTED]
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
---
drivers/acpi/ac.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index e03de37..bb618c8 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -91,6 +91,9 @@ static int get_ac_property(struct power_supply *psy,
union power_supply_propval *val)
{
struct acpi_ac *ac = to_acpi_ac(psy);
+
+ if (acpi_ac_get_state(ac))
+ return 0;
switch (psp) {
case POWER_SUPPLY_PROP_ONLINE:
val-intval = ac-state;
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
Andrey Borzenkov wrote:
On Wednesday 31 October 2007, Alexey Starikovskiy wrote:
Andrey Borzenkov wrote:
I suspect new ACPI AC adapter code but have to add some printk's to be
sure.
To reproduce - plug in AC cord, suspend, unplug, resume - kpowersave and
sysfs still show AC adapter online. Or other way round.
-andrey
Please check if this patch helps.
It does not even compile :)
On serious note (after adding forward declaration) - patch half works. It
does
make online return true value (which confirms that underlying ACPI works
correctly) but HAL still does not notice it.
The problem is, with power_supply interface HAL does not use polling, it
relies on CHANGE event. This event is apparently never generated if AC
adapter state changes on resume. So the obvious fix is to compare AC state
before and after suspend in -resume function and generate synthetic CHANGE
event if something has changed.
This will also make superfluous polling redundant.
Ok, here. This one compiles :)
Regards,
Alex.
ACPI: AC: Update AC state on resume
From: Alexey Starikovskiy [EMAIL PROTECTED]
Check if AC state has changed across resume and notify userspace if so.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
---
drivers/acpi/ac.c | 17 +
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index e03de37..06308ff 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -54,6 +54,7 @@ extern void *acpi_unlock_ac_dir(struct proc_dir_entry *acpi_ac_dir);
static int acpi_ac_add(struct acpi_device *device);
static int acpi_ac_remove(struct acpi_device *device, int type);
+static int acpi_ac_resume(struct acpi_device *device);
static int acpi_ac_open_fs(struct inode *inode, struct file *file);
const static struct acpi_device_id ac_device_ids[] = {
@@ -69,6 +70,7 @@ static struct acpi_driver acpi_ac_driver = {
.ops = {
.add = acpi_ac_add,
.remove = acpi_ac_remove,
+ .resume = acpi_ac_resume,
},
};
@@ -294,6 +296,21 @@ static int acpi_ac_add(struct acpi_device *device)
return result;
}
+static int acpi_ac_resume(struct acpi_device *device)
+{
+ struct acpi_ac *ac;
+ unsigned old_state;
+ if (!device || !acpi_driver_data(device))
+ return -EINVAL;
+ ac = acpi_driver_data(device);
+ old_state = ac-state;
+ if (acpi_ac_get_state(ac))
+ return 0;
+ if (old_state != ac-state)
+ kobject_uevent(ac-charger.dev-kobj, KOBJ_CHANGE);
+ return 0;
+}
+
static int acpi_ac_remove(struct acpi_device *device, int type)
{
acpi_status status = AE_OK;
Re: [PATCH] 2.6.24-rc1: ensure "present" sysfs attribute even if battery is absent
Andrey Borzenkov wrote: > On Saturday 27 October 2007, Alexey Starikovskiy wrote: >> Andrey Borzenkov wrote: >>> I am not exactly sure about this one ... what other power_supply class >>> drivers do? Should I fix HAL instead (but then, I do not know whether HAL >>> is the only application that is using this interface). >> Hm, do you need separate set of properties for that? You could register >> either of existing two, and read function will not allow read of anything >> but "present". IMHO, this is what other modules do (/drivers/power) > > Do they have different set of properties depending on underlying hardware > that > you can't query unless hardware is present? I'd rather avoid adding fake > attributes; but I do not actually care so which one do you prefer? :) I prefer less code :) > >> One remaining trick here, you need to call unregister/register for >> power_supply if you change attributes -- so please check if your patched >> driver survives insertion of the battery. >> > > > Neither does your code (nor kpowersave :) ) Remove battery and set of > attributes is "stuck" instead of being reset to only fixed set of power > device attributes (basically "info"). The only call to power_supply_register > is in acpi_battery_add and as far as I can tell this is executed on adding > *slot* not when content of this slot changes. Point is -- it does not break :) If you start to play with shorter length of attributes table and don't call unregister/register, power_supply may go past the end of table. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] 2.6.24-rc1: ensure "present" sysfs attribute even if battery is absent
Andrey Borzenkov wrote: > I am not exactly sure about this one ... what other power_supply class > drivers > do? Should I fix HAL instead (but then, I do not know whether HAL is the only > application that is using this interface). > > Hm, do you need separate set of properties for that? You could register either of existing two, and read function will not allow read of anything but "present". IMHO, this is what other modules do (/drivers/power) One remaining trick here, you need to call unregister/register for power_supply if you change attributes -- so please check if your patched driver survives insertion of the battery. Regards, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.624-rc1 regression] lost battery information
Andrey Borzenkov wrote: > On Saturday 27 October 2007, Alexey Starikovskiy wrote: >> As you wish... :) Please check the attached patch. >> > > Not sure why you need to reimplement acpi_extract_package, but ... Take a look on memory allocations around it... :) > > Tested-by: Andrey Borzenkov <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.624-rc1 regression] lost battery information
Andrey Borzenkov wrote:
> On Saturday 27 October 2007, Alexey Starikovskiy wrote:
>> Andrey,
>> Please try the attached patch. I choose to do snprintf() instead of direct
>> copy, as your previous message showed empty OEM type.
>>
>
> Not quite. Now I get
>
> OEM info:0
Ok, I was hoping to see some number starting with 0, which would be printed
as empty string...
>
> while before I got empty string. If I read acpi_extract_package correctly, it
> actually interpreted integer as string without any conversion. Which in this
> case obviously gave us empty string (integer being 0). I'd prefer to remain
> compatible.
As you wish... :) Please check the attached patch.
>
> also
>
> {pts/1}% cat /sys/class/power_supply/BAT1/manufacturer
> 0
>
> which is rather weird manufacturer name :)
>
>> Thanks,
>> Alex.
>>
>> Andrey Borzenkov wrote:
>>> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>>>> Your cat's "Bad address" means -EFAULT, according to "man errno".
>>>> Please apply this patch to see what exactly failed...
>>> [ 1191.471572] ACPI: element[12]->type = 1, expected string
>>> [ 1196.640065] ACPI: element[12]->type = 1, expected string
>>> [ 1199.479773] ACPI: element[12]->type = 1, expected string
>>> [ 1199.745435] ACPI: element[12]->type = 1, expected string
>>>
>>> it is "OEM type". For reference here is _BIF from my DSDT:
>>>
>>> Method (_BIF, 0, NotSerialized)
>>> {
>>> Name (BUFF, Package (0x0D) {})
>>> Store (0x00, Index (BUFF, 0x00))
>>> Store (\_SB.MEM.BDV2, Local2)
>>> Multiply (\_SB.MEM.BDC2, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x01))
>>> Multiply (\_SB.MEM.BLF2, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x02))
>>> Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
>>> Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
>>> Multiply (\_SB.MEM.BCW2, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x05))
>>> Multiply (\_SB.MEM.BCL2, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x06))
>>> Multiply (\_SB.MEM.BG12, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x07))
>>> Multiply (\_SB.MEM.BG22, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x08))
>>> Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
>>> Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
>>> Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
>>> Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
>>> Return (BUFF)
>>> }
>>>
>>> This is behaviour change. Previous battery.c used generic
>>> acpi_extract_package which allowed (allows) for object of type integer
>>> when string is requested:
>>>
>>> case ACPI_TYPE_INTEGER:
>>> switch (format_string[i]) {
>>> case 'N':
>>> size_required += sizeof(acpi_integer);
>>> tail_offset += sizeof(acpi_integer);
>>> break;
>>> case 'S':
>>> size_required +=
>>> sizeof(char *) + sizeof(acpi_integer)
>>> + sizeof(char);
>>> tail_offset += sizeof(char *);
>>> break;
>>>
>>> while current battery.c:extract_package fails:
>>>
>>> if (offsets[i].mode) {
>>> if (element->type != ACPI_TYPE_STRING &&
>>> element->type != ACPI_TYPE_BUFFER) {
>>> printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i,
>>> element->type);
Re: [2.624-rc1 regression] lost battery information
Andrey,
Please try the attached patch. I choose to do snprintf() instead of direct copy,
as your previous message showed empty OEM type.
Thanks,
Alex.
Andrey Borzenkov wrote:
> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>> Your cat's "Bad address" means -EFAULT, according to "man errno".
>> Please apply this patch to see what exactly failed...
>
>
>
> [ 1191.471572] ACPI: element[12]->type = 1, expected string
> [ 1196.640065] ACPI: element[12]->type = 1, expected string
> [ 1199.479773] ACPI: element[12]->type = 1, expected string
> [ 1199.745435] ACPI: element[12]->type = 1, expected string
>
> it is "OEM type". For reference here is _BIF from my DSDT:
>
> Method (_BIF, 0, NotSerialized)
> {
> Name (BUFF, Package (0x0D) {})
> Store (0x00, Index (BUFF, 0x00))
> Store (\_SB.MEM.BDV2, Local2)
> Multiply (\_SB.MEM.BDC2, Local2, Local0)
> Divide (Local0, 0x03E8, Local1, Local0)
> Store (Local0, Index (BUFF, 0x01))
> Multiply (\_SB.MEM.BLF2, Local2, Local0)
> Divide (Local0, 0x03E8, Local1, Local0)
> Store (Local0, Index (BUFF, 0x02))
> Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
> Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
> Multiply (\_SB.MEM.BCW2, Local2, Local0)
> Divide (Local0, 0x03E8, Local1, Local0)
> Store (Local0, Index (BUFF, 0x05))
> Multiply (\_SB.MEM.BCL2, Local2, Local0)
> Divide (Local0, 0x03E8, Local1, Local0)
> Store (Local0, Index (BUFF, 0x06))
> Multiply (\_SB.MEM.BG12, Local2, Local0)
> Divide (Local0, 0x03E8, Local1, Local0)
> Store (Local0, Index (BUFF, 0x07))
> Multiply (\_SB.MEM.BG22, Local2, Local0)
> Divide (Local0, 0x03E8, Local1, Local0)
> Store (Local0, Index (BUFF, 0x08))
> Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
> Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
> Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
> Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
> Return (BUFF)
> }
>
> This is behaviour change. Previous battery.c used generic
> acpi_extract_package
> which allowed (allows) for object of type integer when string is requested:
>
> case ACPI_TYPE_INTEGER:
> switch (format_string[i]) {
> case 'N':
> size_required += sizeof(acpi_integer);
> tail_offset += sizeof(acpi_integer);
> break;
> case 'S':
> size_required +=
> sizeof(char *) + sizeof(acpi_integer) +
> sizeof(char);
> tail_offset += sizeof(char *);
> break;
>
> while current battery.c:extract_package fails:
>
> if (offsets[i].mode) {
> if (element->type != ACPI_TYPE_STRING &&
> element->type != ACPI_TYPE_BUFFER) {
> printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i,
> element->type);
> return -EFAULT;
> }
>
> well, while it could be BIOS fault this happily worked before ... This is
> obviously also the reason why I do not have anything in /sys
>
> Fans, could you check whether you have the same issue using test patch?
ACPI: Battery: Allow extract string from integer
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
Some machines return integer instead of expected string.
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/battery.c | 24 ++--
1 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..6841358 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -260,7 +260,7 @@ static int extract_package(struct acpi_battery *battery,
union acpi_object *package,
struct acpi_offsets *offsets, int num)
{
- int i, *x;
+ int i;
union acpi_object *element;
if (package->type != ACPI_TYPE_PACKAGE)
return -EFAULT;
@@ -269,16 +269,20 @@ static int extract_package(struct acpi_battery *battery,
return -EFAULT;
element = >package.elements[i];
if (offsets[i].mode) {
-
Re: [2.624-rc1 regression] lost battery information
Andrey,
Please try the attached patch. I choose to do snprintf() instead of direct copy,
as your previous message showed empty OEM type.
Thanks,
Alex.
Andrey Borzenkov wrote:
On Friday 26 October 2007, Alexey Starikovskiy wrote:
Your cat's Bad address means -EFAULT, according to man errno.
Please apply this patch to see what exactly failed...
[ 1191.471572] ACPI: element[12]-type = 1, expected string
[ 1196.640065] ACPI: element[12]-type = 1, expected string
[ 1199.479773] ACPI: element[12]-type = 1, expected string
[ 1199.745435] ACPI: element[12]-type = 1, expected string
it is OEM type. For reference here is _BIF from my DSDT:
Method (_BIF, 0, NotSerialized)
{
Name (BUFF, Package (0x0D) {})
Store (0x00, Index (BUFF, 0x00))
Store (\_SB.MEM.BDV2, Local2)
Multiply (\_SB.MEM.BDC2, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x01))
Multiply (\_SB.MEM.BLF2, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x02))
Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
Multiply (\_SB.MEM.BCW2, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x05))
Multiply (\_SB.MEM.BCL2, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x06))
Multiply (\_SB.MEM.BG12, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x07))
Multiply (\_SB.MEM.BG22, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x08))
Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
Return (BUFF)
}
This is behaviour change. Previous battery.c used generic
acpi_extract_package
which allowed (allows) for object of type integer when string is requested:
case ACPI_TYPE_INTEGER:
switch (format_string[i]) {
case 'N':
size_required += sizeof(acpi_integer);
tail_offset += sizeof(acpi_integer);
break;
case 'S':
size_required +=
sizeof(char *) + sizeof(acpi_integer) +
sizeof(char);
tail_offset += sizeof(char *);
break;
while current battery.c:extract_package fails:
if (offsets[i].mode) {
if (element-type != ACPI_TYPE_STRING
element-type != ACPI_TYPE_BUFFER) {
printk (KERN_ERR PREFIX element[%d]-type = %x, expected string\n, i,
element-type);
return -EFAULT;
}
well, while it could be BIOS fault this happily worked before ... This is
obviously also the reason why I do not have anything in /sys
Fans, could you check whether you have the same issue using test patch?
ACPI: Battery: Allow extract string from integer
From: Alexey Starikovskiy [EMAIL PROTECTED]
Some machines return integer instead of expected string.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
---
drivers/acpi/battery.c | 24 ++--
1 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..6841358 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -260,7 +260,7 @@ static int extract_package(struct acpi_battery *battery,
union acpi_object *package,
struct acpi_offsets *offsets, int num)
{
- int i, *x;
+ int i;
union acpi_object *element;
if (package-type != ACPI_TYPE_PACKAGE)
return -EFAULT;
@@ -269,16 +269,20 @@ static int extract_package(struct acpi_battery *battery,
return -EFAULT;
element = package-package.elements[i];
if (offsets[i].mode) {
- if (element-type != ACPI_TYPE_STRING
- element-type != ACPI_TYPE_BUFFER)
-return -EFAULT;
- strncpy((u8 *)battery + offsets[i].offset,
-element-string.pointer, 32);
+ u8 *ptr = (u8 *)battery + offsets[i].offset;
+ if (element-type == ACPI_TYPE_STRING ||
+ element-type == ACPI_TYPE_BUFFER)
+strncpy(ptr, element-string.pointer, 32);
+ else if (element-type == ACPI_TYPE_INTEGER) {
+snprintf
Re: [2.624-rc1 regression] lost battery information
Andrey Borzenkov wrote:
On Saturday 27 October 2007, Alexey Starikovskiy wrote:
Andrey,
Please try the attached patch. I choose to do snprintf() instead of direct
copy, as your previous message showed empty OEM type.
Not quite. Now I get
OEM info:0
Ok, I was hoping to see some number starting with 0, which would be printed
as empty string...
while before I got empty string. If I read acpi_extract_package correctly, it
actually interpreted integer as string without any conversion. Which in this
case obviously gave us empty string (integer being 0). I'd prefer to remain
compatible.
As you wish... :) Please check the attached patch.
also
{pts/1}% cat /sys/class/power_supply/BAT1/manufacturer
0
which is rather weird manufacturer name :)
Thanks,
Alex.
Andrey Borzenkov wrote:
On Friday 26 October 2007, Alexey Starikovskiy wrote:
Your cat's Bad address means -EFAULT, according to man errno.
Please apply this patch to see what exactly failed...
[ 1191.471572] ACPI: element[12]-type = 1, expected string
[ 1196.640065] ACPI: element[12]-type = 1, expected string
[ 1199.479773] ACPI: element[12]-type = 1, expected string
[ 1199.745435] ACPI: element[12]-type = 1, expected string
it is OEM type. For reference here is _BIF from my DSDT:
Method (_BIF, 0, NotSerialized)
{
Name (BUFF, Package (0x0D) {})
Store (0x00, Index (BUFF, 0x00))
Store (\_SB.MEM.BDV2, Local2)
Multiply (\_SB.MEM.BDC2, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x01))
Multiply (\_SB.MEM.BLF2, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x02))
Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
Multiply (\_SB.MEM.BCW2, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x05))
Multiply (\_SB.MEM.BCL2, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x06))
Multiply (\_SB.MEM.BG12, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x07))
Multiply (\_SB.MEM.BG22, Local2, Local0)
Divide (Local0, 0x03E8, Local1, Local0)
Store (Local0, Index (BUFF, 0x08))
Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
Return (BUFF)
}
This is behaviour change. Previous battery.c used generic
acpi_extract_package which allowed (allows) for object of type integer
when string is requested:
case ACPI_TYPE_INTEGER:
switch (format_string[i]) {
case 'N':
size_required += sizeof(acpi_integer);
tail_offset += sizeof(acpi_integer);
break;
case 'S':
size_required +=
sizeof(char *) + sizeof(acpi_integer)
+ sizeof(char);
tail_offset += sizeof(char *);
break;
while current battery.c:extract_package fails:
if (offsets[i].mode) {
if (element-type != ACPI_TYPE_STRING
element-type != ACPI_TYPE_BUFFER) {
printk (KERN_ERR PREFIX element[%d]-type = %x, expected string\n, i,
element-type);
return -EFAULT;
}
well, while it could be BIOS fault this happily worked before ... This is
obviously also the reason why I do not have anything in /sys
Fans, could you check whether you have the same issue using test patch?
ACPI: Battery: Allow extract string from integer
From: Alexey Starikovskiy [EMAIL PROTECTED]
Some machines return integer instead of expected string.
Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED]
---
drivers/acpi/battery.c | 25 +++--
1 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..6c06879 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -260,7 +260,7 @@ static int extract_package(struct acpi_battery *battery,
union acpi_object *package,
struct acpi_offsets *offsets, int num)
{
- int i, *x;
+ int i;
union acpi_object *element;
if (package-type != ACPI_TYPE_PACKAGE)
return
Re: [2.624-rc1 regression] lost battery information
Andrey Borzenkov wrote: On Saturday 27 October 2007, Alexey Starikovskiy wrote: As you wish... :) Please check the attached patch. Not sure why you need to reimplement acpi_extract_package, but ... Take a look on memory allocations around it... :) Tested-by: Andrey Borzenkov [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] 2.6.24-rc1: ensure present sysfs attribute even if battery is absent
Andrey Borzenkov wrote: I am not exactly sure about this one ... what other power_supply class drivers do? Should I fix HAL instead (but then, I do not know whether HAL is the only application that is using this interface). Hm, do you need separate set of properties for that? You could register either of existing two, and read function will not allow read of anything but present. IMHO, this is what other modules do (/drivers/power) One remaining trick here, you need to call unregister/register for power_supply if you change attributes -- so please check if your patched driver survives insertion of the battery. Regards, Alex. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] 2.6.24-rc1: ensure present sysfs attribute even if battery is absent
Andrey Borzenkov wrote: On Saturday 27 October 2007, Alexey Starikovskiy wrote: Andrey Borzenkov wrote: I am not exactly sure about this one ... what other power_supply class drivers do? Should I fix HAL instead (but then, I do not know whether HAL is the only application that is using this interface). Hm, do you need separate set of properties for that? You could register either of existing two, and read function will not allow read of anything but present. IMHO, this is what other modules do (/drivers/power) Do they have different set of properties depending on underlying hardware that you can't query unless hardware is present? I'd rather avoid adding fake attributes; but I do not actually care so which one do you prefer? :) I prefer less code :) One remaining trick here, you need to call unregister/register for power_supply if you change attributes -- so please check if your patched driver survives insertion of the battery. Neither does your code (nor kpowersave :) ) Remove battery and set of attributes is stuck instead of being reset to only fixed set of power device attributes (basically info). The only call to power_supply_register is in acpi_battery_add and as far as I can tell this is executed on adding *slot* not when content of this slot changes. Point is -- it does not break :) If you start to play with shorter length of attributes table and don't call unregister/register, power_supply may go past the end of table. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.624-rc1 regression] lost battery information
Frans Pop wrote: > Alexey Starikovskiy wrote: >> Andrey Borzenkov wrote: >>> is it in -rc1 or can you point me to the patch (I'd rather avoid having >>> to pull from different git trees). Thank you. >> No, it should be rc1. >>> And what about ACPI_PROCFS case? It still needs attention I believe. >> As you can see, there is info in /proc too: > > I'm missing the info in /sys too, so it looks like the error in proc and > the missing info in /sys are related. > > Alexey: do you have POWER_SUPPLY and/or AC/BATTERY compiled in or modular? > I wonder if that could make the difference. > Currently everything is module. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.624-rc1 regression] lost battery information
Andrey Borzenkov wrote:
> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>> Andrey Borzenkov wrote:
>>> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>>>> Andrey Borzenkov wrote:
>>>>> I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have
>>>>> no /proc/acpi/battery and cannot test netlink interface because right
>>>>> now there is no consumer of this.
>>>> for /sysfs interface you need to enable power_supply driver.
>>>> or you could apply this patch and power_supply will be selected by
>>>> battery itself.
>>> I already have power_supply module, battery depends on it and it is
>>> autloaded. But I fail to see where I can get battery info in /sys
>> Intent was to put into /sysfs same information:
>> [EMAIL PROTECTED]:~/client_conf$ ls /sys/class/power_supply/BAT1/
>> alarm charge_full charge_full_design charge_now current_now device
>> manufacturer model_name present status subsystem technology type
>> uevent voltage_min_design voltage_now
>>
>
> is it in -rc1 or can you point me to the patch (I'd rather avoid having to
> pull from different git trees). Thank you.
No, it should be rc1.
>
> And what about ACPI_PROCFS case? It still needs attention I believe.
As you can see, there is info in /proc too:
>> [EMAIL PROTECTED]:~/client_conf$ cat /proc/acpi/battery/BAT1/*
>> alarm: unsupported
>> present: yes
>> design capacity: 4300 mAh
>> last full capacity: 4172 mAh
>> battery technology: rechargeable
>> design voltage: 14800 mV
>> design capacity warning: 208 mAh
>> design capacity low: 41 mAh
>> capacity granularity 1: 41 mAh
>> capacity granularity 2: 41 mAh
>> model number:Bat1
>> serial number: 001
>> battery type:LION
>> OEM info:Pacifi
>> present: yes
>> capacity state: ok
>> charging state: charged
>> present rate:0 mA
>> remaining capacity: 4172 mAh
>> present voltage: 16518 mV
Your cat's "Bad address" means -EFAULT, according to "man errno".
Please apply this patch to see what exactly failed...
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..55e9a8e 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -262,21 +262,29 @@ static int extract_package(struct acpi_battery *battery,
{
int i, *x;
union acpi_object *element;
- if (package->type != ACPI_TYPE_PACKAGE)
+ if (package->type != ACPI_TYPE_PACKAGE) {
+printk (KERN_ERR PREFIX "package->type = %x\n", package->type);
return -EFAULT;
+ }
for (i = 0; i < num; ++i) {
- if (package->package.count <= i)
+ if (package->package.count <= i) {
+printk (KERN_ERR PREFIX "package.count = %d, i = %d\n", package->package.count, i);
return -EFAULT;
+ }
element = >package.elements[i];
if (offsets[i].mode) {
if (element->type != ACPI_TYPE_STRING &&
- element->type != ACPI_TYPE_BUFFER)
+ element->type != ACPI_TYPE_BUFFER) {
+printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i, element->type);
return -EFAULT;
+ }
strncpy((u8 *)battery + offsets[i].offset,
element->string.pointer, 32);
} else {
- if (element->type != ACPI_TYPE_INTEGER)
+ if (element->type != ACPI_TYPE_INTEGER) {
+printk (KERN_ERR PREFIX "element[%d]->type = %x, expected integer\n", i, element->type);
return -EFAULT;
+ }
x = (int *)((u8 *)battery + offsets[i].offset);
*x = element->integer.value;
}
Re: [2.624-rc1 regression] lost battery information
Andrey Borzenkov wrote: > On Friday 26 October 2007, Alexey Starikovskiy wrote: >> Andrey Borzenkov wrote: >>> I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have >>> no /proc/acpi/battery and cannot test netlink interface because right now >>> there is no consumer of this. >> for /sysfs interface you need to enable power_supply driver. >> or you could apply this patch and power_supply will be selected by battery >> itself. >> > > I already have power_supply module, battery depends on it and it is > autloaded. > But I fail to see where I can get battery info in /sys Intent was to put into /sysfs same information: [EMAIL PROTECTED]:~/client_conf$ ls /sys/class/power_supply/BAT1/ alarm charge_full charge_full_design charge_now current_now device manufacturer model_name present status subsystem technology type uevent voltage_min_design voltage_now [EMAIL PROTECTED]:~/client_conf$ cat /sys/class/power_supply/BAT1/* 0 4172000 430 4172000 0 cat: /sys/class/power_supply/BAT1/device: Is a directory Pacifi Bat1 1 Full cat: /sys/class/power_supply/BAT1/subsystem: Is a directory Li-ion Battery PHYSDEVPATH=/devices/LNXSYSTM:00/device:00/PNP0C0A:00 PHYSDEVBUS=acpi PHYSDEVDRIVER=battery POWER_SUPPLY_NAME=BAT1 POWER_SUPPLY_TYPE=Battery POWER_SUPPLY_STATUS=Full POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_VOLTAGE_MIN_DESIGN=1480 POWER_SUPPLY_VOLTAGE_NOW=16522000 POWER_SUPPLY_CURRENT_NOW=0 POWER_SUPPLY_CHARGE_FULL_DESIGN=430 POWER_SUPPLY_CHARGE_FULL=4172000 POWER_SUPPLY_CHARGE_NOW=4172000 POWER_SUPPLY_MODEL_NAME=Bat1 POWER_SUPPLY_MANUFACTURER=Pacifi 1480 16522000 [EMAIL PROTECTED]:~/client_conf$ cat /proc/acpi/battery/BAT1/* alarm: unsupported present: yes design capacity: 4300 mAh last full capacity: 4172 mAh battery technology: rechargeable design voltage: 14800 mV design capacity warning: 208 mAh design capacity low: 41 mAh capacity granularity 1: 41 mAh capacity granularity 2: 41 mAh model number:Bat1 serial number: 001 battery type:LION OEM info:Pacifi present: yes capacity state: ok charging state: charged present rate:0 mA remaining capacity: 4172 mAh present voltage: 16518 mV - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.624-rc1 regression] lost battery information
:06.0[A] -> Link [LNKH] -> GSI 11 > (level, low) -> IRQ 11 > [ 927.120073] ACPI: AC Adapter [ADP1] (on-line) > [ 927.195942] ACPI: Battery Slot [BAT1] (battery present) > [ 927.200475] ACPI: Battery Slot [BAT2] (battery absent) > [ 927.277564] ACPI: Power Button (FF) [PWRF] > [ 927.290786] ACPI: Lid Switch [LID] > [ 927.324850] ACPI: Transitioning device [FAN] to D3 > [ 927.324867] ACPI: Transitioning device [FAN] to D3 > [ 927.324891] ACPI: Fan [FAN] (off) > [ 927.535960] ACPI: CPU0 (power states: C1[C1] C2[C2]) > [ 927.638487] ACPI: Thermal Zone [THRM] (55 C) > [ 927.770100] toshiba_acpi: Toshiba Laptop ACPI Extras version 0.18 > [ 927.920519] ACPI: Video Device [VGA] (multi-head: yes rom: yes post: no) > [ 1055.552624] ACPI: PCI interrupt for device :00:0a.0 disabled > [ 1055.554812] ACPI: PCI interrupt for device :00:06.0 disabled > [ 1055.555479] ACPI: PCI interrupt for device :00:02.0 disabled > [0.901020] ACPI: PCI Interrupt :00:02.0[A] -> Link [LNKG] -> GSI 11 > (level, low) -> IRQ 11 > [0.901271] ACPI: Unable to derive IRQ for device :00:04.0 > [0.901278] ACPI: PCI Interrupt :00:04.0[A]: no GSI > [0.904594] ACPI: PCI Interrupt :00:06.0[A] -> Link [LNKH] -> GSI 11 > (level, low) -> IRQ 11 > > > As for the case without ACPI_PROCFS ... well, I do not have it in /proc - > which is expected - but neither I do have it in /sys. And Kconfig help reads > > The deprecated files (and their replacements) include: > > /proc/acpi/sleep (/sys/power/state) > /proc/acpi/info (/sys/modules/acpi/parameters/acpica_version) > /proc/acpi/dsdt (/sys/firmware/acpi/tables/DSDT) > /proc/acpi/fadt (/sys/firmware/acpi/tables/FACP) > /proc/acpi/debug_layer (/sys/module/acpi/parameters/debug_layer) > /proc/acpi/debug_level (/sys/module/acpi/parameters/debug_level) > > neither does it mention /proc/acpi/battery not do I actually have any battery > information in /sys. > > Personally I do not like it (if it is intentional). Leaving only netlink > interface means user has no way to query for actual state. We need something > running all the time and hope, it never loses any event and thus reflects > actual state. But it also means we are not allowed to restart it (whatever it > is) as it will have no way to query for actual state on restart ... > > -andrey ACPI: use select POWER_SUPPLY for AC, BATTERY and SBS From: Alexey Starikovskiy <[EMAIL PROTECTED]> POWER_SUPPLY is needed for AC, battery, and SBS sysfs support. Use 'select' instead of 'depends on', as it is will not be selected by anything else, leading to confusion. Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> --- drivers/acpi/Kconfig |8 +--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 5d0e26a..ecd87d7 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -88,7 +88,8 @@ config ACPI_PROC_EVENT config ACPI_AC tristate "AC Adapter" - depends on X86 && POWER_SUPPLY + depends on X86 + select POWER_SUPPLY default y help This driver adds support for the AC Adapter object, which indicates @@ -97,7 +98,8 @@ config ACPI_AC config ACPI_BATTERY tristate "Battery" - depends on X86 && POWER_SUPPLY + depends on X86 + select POWER_SUPPLY default y help This driver adds support for battery information through @@ -352,7 +354,7 @@ config ACPI_HOTPLUG_MEMORY config ACPI_SBS tristate "Smart Battery System" depends on X86 - depends on POWER_SUPPLY + select POWER_SUPPLY help This driver adds support for the Smart Battery System, another type of access to battery information, found on some laptops.
Re: [2.624-rc1 regression] lost battery information
] ACPI: Fan [FAN] (off) [ 927.535960] ACPI: CPU0 (power states: C1[C1] C2[C2]) [ 927.638487] ACPI: Thermal Zone [THRM] (55 C) [ 927.770100] toshiba_acpi: Toshiba Laptop ACPI Extras version 0.18 [ 927.920519] ACPI: Video Device [VGA] (multi-head: yes rom: yes post: no) [ 1055.552624] ACPI: PCI interrupt for device :00:0a.0 disabled [ 1055.554812] ACPI: PCI interrupt for device :00:06.0 disabled [ 1055.555479] ACPI: PCI interrupt for device :00:02.0 disabled [0.901020] ACPI: PCI Interrupt :00:02.0[A] - Link [LNKG] - GSI 11 (level, low) - IRQ 11 [0.901271] ACPI: Unable to derive IRQ for device :00:04.0 [0.901278] ACPI: PCI Interrupt :00:04.0[A]: no GSI [0.904594] ACPI: PCI Interrupt :00:06.0[A] - Link [LNKH] - GSI 11 (level, low) - IRQ 11 As for the case without ACPI_PROCFS ... well, I do not have it in /proc - which is expected - but neither I do have it in /sys. And Kconfig help reads The deprecated files (and their replacements) include: /proc/acpi/sleep (/sys/power/state) /proc/acpi/info (/sys/modules/acpi/parameters/acpica_version) /proc/acpi/dsdt (/sys/firmware/acpi/tables/DSDT) /proc/acpi/fadt (/sys/firmware/acpi/tables/FACP) /proc/acpi/debug_layer (/sys/module/acpi/parameters/debug_layer) /proc/acpi/debug_level (/sys/module/acpi/parameters/debug_level) neither does it mention /proc/acpi/battery not do I actually have any battery information in /sys. Personally I do not like it (if it is intentional). Leaving only netlink interface means user has no way to query for actual state. We need something running all the time and hope, it never loses any event and thus reflects actual state. But it also means we are not allowed to restart it (whatever it is) as it will have no way to query for actual state on restart ... -andrey ACPI: use select POWER_SUPPLY for AC, BATTERY and SBS From: Alexey Starikovskiy [EMAIL PROTECTED] POWER_SUPPLY is needed for AC, battery, and SBS sysfs support. Use 'select' instead of 'depends on', as it is will not be selected by anything else, leading to confusion. Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED] --- drivers/acpi/Kconfig |8 +--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 5d0e26a..ecd87d7 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -88,7 +88,8 @@ config ACPI_PROC_EVENT config ACPI_AC tristate AC Adapter - depends on X86 POWER_SUPPLY + depends on X86 + select POWER_SUPPLY default y help This driver adds support for the AC Adapter object, which indicates @@ -97,7 +98,8 @@ config ACPI_AC config ACPI_BATTERY tristate Battery - depends on X86 POWER_SUPPLY + depends on X86 + select POWER_SUPPLY default y help This driver adds support for battery information through @@ -352,7 +354,7 @@ config ACPI_HOTPLUG_MEMORY config ACPI_SBS tristate Smart Battery System depends on X86 - depends on POWER_SUPPLY + select POWER_SUPPLY help This driver adds support for the Smart Battery System, another type of access to battery information, found on some laptops.
Re: [2.624-rc1 regression] lost battery information
Andrey Borzenkov wrote: On Friday 26 October 2007, Alexey Starikovskiy wrote: Andrey Borzenkov wrote: I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have no /proc/acpi/battery and cannot test netlink interface because right now there is no consumer of this. for /sysfs interface you need to enable power_supply driver. or you could apply this patch and power_supply will be selected by battery itself. I already have power_supply module, battery depends on it and it is autloaded. But I fail to see where I can get battery info in /sys Intent was to put into /sysfs same information: [EMAIL PROTECTED]:~/client_conf$ ls /sys/class/power_supply/BAT1/ alarm charge_full charge_full_design charge_now current_now device manufacturer model_name present status subsystem technology type uevent voltage_min_design voltage_now [EMAIL PROTECTED]:~/client_conf$ cat /sys/class/power_supply/BAT1/* 0 4172000 430 4172000 0 cat: /sys/class/power_supply/BAT1/device: Is a directory Pacifi Bat1 1 Full cat: /sys/class/power_supply/BAT1/subsystem: Is a directory Li-ion Battery PHYSDEVPATH=/devices/LNXSYSTM:00/device:00/PNP0C0A:00 PHYSDEVBUS=acpi PHYSDEVDRIVER=battery POWER_SUPPLY_NAME=BAT1 POWER_SUPPLY_TYPE=Battery POWER_SUPPLY_STATUS=Full POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_VOLTAGE_MIN_DESIGN=1480 POWER_SUPPLY_VOLTAGE_NOW=16522000 POWER_SUPPLY_CURRENT_NOW=0 POWER_SUPPLY_CHARGE_FULL_DESIGN=430 POWER_SUPPLY_CHARGE_FULL=4172000 POWER_SUPPLY_CHARGE_NOW=4172000 POWER_SUPPLY_MODEL_NAME=Bat1 POWER_SUPPLY_MANUFACTURER=Pacifi 1480 16522000 [EMAIL PROTECTED]:~/client_conf$ cat /proc/acpi/battery/BAT1/* alarm: unsupported present: yes design capacity: 4300 mAh last full capacity: 4172 mAh battery technology: rechargeable design voltage: 14800 mV design capacity warning: 208 mAh design capacity low: 41 mAh capacity granularity 1: 41 mAh capacity granularity 2: 41 mAh model number:Bat1 serial number: 001 battery type:LION OEM info:Pacifi present: yes capacity state: ok charging state: charged present rate:0 mA remaining capacity: 4172 mAh present voltage: 16518 mV - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.624-rc1 regression] lost battery information
Andrey Borzenkov wrote:
On Friday 26 October 2007, Alexey Starikovskiy wrote:
Andrey Borzenkov wrote:
On Friday 26 October 2007, Alexey Starikovskiy wrote:
Andrey Borzenkov wrote:
I have lost battery in 2.6.24-rc1. Without CONFIG_ACPI_PROCFS I have
no /proc/acpi/battery and cannot test netlink interface because right
now there is no consumer of this.
for /sysfs interface you need to enable power_supply driver.
or you could apply this patch and power_supply will be selected by
battery itself.
I already have power_supply module, battery depends on it and it is
autloaded. But I fail to see where I can get battery info in /sys
Intent was to put into /sysfs same information:
[EMAIL PROTECTED]:~/client_conf$ ls /sys/class/power_supply/BAT1/
alarm charge_full charge_full_design charge_now current_now device
manufacturer model_name present status subsystem technology type
uevent voltage_min_design voltage_now
is it in -rc1 or can you point me to the patch (I'd rather avoid having to
pull from different git trees). Thank you.
No, it should be rc1.
And what about ACPI_PROCFS case? It still needs attention I believe.
As you can see, there is info in /proc too:
[EMAIL PROTECTED]:~/client_conf$ cat /proc/acpi/battery/BAT1/*
alarm: unsupported
present: yes
design capacity: 4300 mAh
last full capacity: 4172 mAh
battery technology: rechargeable
design voltage: 14800 mV
design capacity warning: 208 mAh
design capacity low: 41 mAh
capacity granularity 1: 41 mAh
capacity granularity 2: 41 mAh
model number:Bat1
serial number: 001
battery type:LION
OEM info:Pacifi
present: yes
capacity state: ok
charging state: charged
present rate:0 mA
remaining capacity: 4172 mAh
present voltage: 16518 mV
Your cat's Bad address means -EFAULT, according to man errno.
Please apply this patch to see what exactly failed...
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..55e9a8e 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -262,21 +262,29 @@ static int extract_package(struct acpi_battery *battery,
{
int i, *x;
union acpi_object *element;
- if (package-type != ACPI_TYPE_PACKAGE)
+ if (package-type != ACPI_TYPE_PACKAGE) {
+printk (KERN_ERR PREFIX package-type = %x\n, package-type);
return -EFAULT;
+ }
for (i = 0; i num; ++i) {
- if (package-package.count = i)
+ if (package-package.count = i) {
+printk (KERN_ERR PREFIX package.count = %d, i = %d\n, package-package.count, i);
return -EFAULT;
+ }
element = package-package.elements[i];
if (offsets[i].mode) {
if (element-type != ACPI_TYPE_STRING
- element-type != ACPI_TYPE_BUFFER)
+ element-type != ACPI_TYPE_BUFFER) {
+printk (KERN_ERR PREFIX element[%d]-type = %x, expected string\n, i, element-type);
return -EFAULT;
+ }
strncpy((u8 *)battery + offsets[i].offset,
element-string.pointer, 32);
} else {
- if (element-type != ACPI_TYPE_INTEGER)
+ if (element-type != ACPI_TYPE_INTEGER) {
+printk (KERN_ERR PREFIX element[%d]-type = %x, expected integer\n, i, element-type);
return -EFAULT;
+ }
x = (int *)((u8 *)battery + offsets[i].offset);
*x = element-integer.value;
}
Re: [2.624-rc1 regression] lost battery information
Frans Pop wrote: Alexey Starikovskiy wrote: Andrey Borzenkov wrote: is it in -rc1 or can you point me to the patch (I'd rather avoid having to pull from different git trees). Thank you. No, it should be rc1. And what about ACPI_PROCFS case? It still needs attention I believe. As you can see, there is info in /proc too: I'm missing the info in /sys too, so it looks like the error in proc and the missing info in /sys are related. Alexey: do you have POWER_SUPPLY and/or AC/BATTERY compiled in or modular? I wonder if that could make the difference. Currently everything is module. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] acpi/ec.c: fix use-after-free
Adrian Bunk wrote:
> On Wed, Oct 24, 2007 at 09:15:18PM +0400, Alexey Starikovskiy wrote:
>> Adrian,
>>
>> commit 30c08574da0ead1a47797ce028218ce5b2de61c7 can not introduce
>> use-after-free.
>>
>> Please check...
>
>
> Commit 30c08574da0ead1a47797ce028218ce5b2de61c7 did:
>
> <-- snip -->
>
> list_for_each_entry(handler, >list, node) {
> if (query_bit == handler->query_bit) {
> list_del(>node);
> kfree(handler);
> - break;
> }
> }
>
> <-- snip -->
>
>
> If you look at the definition of list_for_each_entry() in
> include/linux/list.h:
>
> <-- snip -->
>
> #define list_for_each_entry(pos, head, member) \
> for (pos = list_entry((head)->next, typeof(*pos), member); \
> prefetch(pos->member.next), >member != (head);\
> pos = list_entry(pos->member.next, typeof(*pos), member))
>
>
> <-- snip -->
>
>
> Without the "break", "handler" is being dereferenced after it was freed.
Yes, found it minute before :(
Acked, thanks.
>
>
>> Regards,
>> Alex.
>> Adrian Bunk wrote:
>>> This patch fixes a use-after-free introduced by
>>> commit 30c08574da0ead1a47797ce028218ce5b2de61c7.
>>>
>>> Spotted by the Coverity checker.
>>>
>>> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
>>>
>>> ---
>>> --- linux-2.6/drivers/acpi/ec.c.old 2007-10-23 19:39:47.0 +0200
>>> +++ linux-2.6/drivers/acpi/ec.c 2007-10-23 19:34:55.0 +0200
>>> @@ -434,11 +442,11 @@
>>> EXPORT_SYMBOL_GPL(acpi_ec_add_query_handler);
>>>
>>> void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit)
>>> {
>>> - struct acpi_ec_query_handler *handler;
>>> + struct acpi_ec_query_handler *handler, *tmp;
>>> mutex_lock(>lock);
>>> - list_for_each_entry(handler, >list, node) {
>>> + list_for_each_entry_safe(handler, tmp, >list, node) {
>>> if (query_bit == handler->query_bit) {
>>> list_del(>node);
>>> kfree(handler);
>>> }
>>>
>
>
> cu
> Adrian
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] acpi/battery.c: remove dead code
Adrian Bunk wrote:
> After commit f1d4661abe05d0a2c014166042d15ed8b69ae8f2 this was dead
> code.
>
> Spotted by the Coverity checker.
>
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
Acked. Thanks.
>
> ---
> --- linux-2.6/drivers/acpi/battery.c.old 2007-10-23 18:44:13.0
> +0200
> +++ linux-2.6/drivers/acpi/battery.c 2007-10-23 18:44:43.0 +0200
> @@ -550,18 +550,14 @@ static ssize_t acpi_battery_write_alarm(
> int result = 0;
> char alarm_string[12] = { '\0' };
> struct seq_file *m = file->private_data;
> struct acpi_battery *battery = m->private;
>
> if (!battery || (count > sizeof(alarm_string) - 1))
> return -EINVAL;
> - if (result) {
> - result = -ENODEV;
> - goto end;
> - }
> if (!acpi_battery_present(battery)) {
> result = -ENODEV;
> goto end;
> }
> if (copy_from_user(alarm_string, buffer, count)) {
> result = -EFAULT;
> goto end;
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] acpi/ec.c: fix use-after-free
Adrian,
commit 30c08574da0ead1a47797ce028218ce5b2de61c7 can not introduce
use-after-free.
Please check...
Regards,
Alex.
Adrian Bunk wrote:
> This patch fixes a use-after-free introduced by
> commit 30c08574da0ead1a47797ce028218ce5b2de61c7.
>
> Spotted by the Coverity checker.
>
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
>
> ---
> --- linux-2.6/drivers/acpi/ec.c.old 2007-10-23 19:39:47.0 +0200
> +++ linux-2.6/drivers/acpi/ec.c 2007-10-23 19:34:55.0 +0200
> @@ -434,11 +442,11 @@
> EXPORT_SYMBOL_GPL(acpi_ec_add_query_handler);
>
> void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit)
> {
> - struct acpi_ec_query_handler *handler;
> + struct acpi_ec_query_handler *handler, *tmp;
> mutex_lock(>lock);
> - list_for_each_entry(handler, >list, node) {
> + list_for_each_entry_safe(handler, tmp, >list, node) {
> if (query_bit == handler->query_bit) {
> list_del(>node);
> kfree(handler);
> }
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] acpi/ec.c: fix use-after-free
Adrian,
commit 30c08574da0ead1a47797ce028218ce5b2de61c7 can not introduce
use-after-free.
Please check...
Regards,
Alex.
Adrian Bunk wrote:
This patch fixes a use-after-free introduced by
commit 30c08574da0ead1a47797ce028218ce5b2de61c7.
Spotted by the Coverity checker.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
--- linux-2.6/drivers/acpi/ec.c.old 2007-10-23 19:39:47.0 +0200
+++ linux-2.6/drivers/acpi/ec.c 2007-10-23 19:34:55.0 +0200
@@ -434,11 +442,11 @@
EXPORT_SYMBOL_GPL(acpi_ec_add_query_handler);
void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit)
{
- struct acpi_ec_query_handler *handler;
+ struct acpi_ec_query_handler *handler, *tmp;
mutex_lock(ec-lock);
- list_for_each_entry(handler, ec-list, node) {
+ list_for_each_entry_safe(handler, tmp, ec-list, node) {
if (query_bit == handler-query_bit) {
list_del(handler-node);
kfree(handler);
}
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] acpi/battery.c: remove dead code
Adrian Bunk wrote:
After commit f1d4661abe05d0a2c014166042d15ed8b69ae8f2 this was dead
code.
Spotted by the Coverity checker.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
Acked. Thanks.
---
--- linux-2.6/drivers/acpi/battery.c.old 2007-10-23 18:44:13.0
+0200
+++ linux-2.6/drivers/acpi/battery.c 2007-10-23 18:44:43.0 +0200
@@ -550,18 +550,14 @@ static ssize_t acpi_battery_write_alarm(
int result = 0;
char alarm_string[12] = { '\0' };
struct seq_file *m = file-private_data;
struct acpi_battery *battery = m-private;
if (!battery || (count sizeof(alarm_string) - 1))
return -EINVAL;
- if (result) {
- result = -ENODEV;
- goto end;
- }
if (!acpi_battery_present(battery)) {
result = -ENODEV;
goto end;
}
if (copy_from_user(alarm_string, buffer, count)) {
result = -EFAULT;
goto end;
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] acpi/ec.c: fix use-after-free
Adrian Bunk wrote:
On Wed, Oct 24, 2007 at 09:15:18PM +0400, Alexey Starikovskiy wrote:
Adrian,
commit 30c08574da0ead1a47797ce028218ce5b2de61c7 can not introduce
use-after-free.
Please check...
Commit 30c08574da0ead1a47797ce028218ce5b2de61c7 did:
-- snip --
list_for_each_entry(handler, ec-list, node) {
if (query_bit == handler-query_bit) {
list_del(handler-node);
kfree(handler);
- break;
}
}
-- snip --
If you look at the definition of list_for_each_entry() in
include/linux/list.h:
-- snip --
#define list_for_each_entry(pos, head, member) \
for (pos = list_entry((head)-next, typeof(*pos), member); \
prefetch(pos-member.next), pos-member != (head);\
pos = list_entry(pos-member.next, typeof(*pos), member))
-- snip --
Without the break, handler is being dereferenced after it was freed.
Yes, found it minute before :(
Acked, thanks.
Regards,
Alex.
Adrian Bunk wrote:
This patch fixes a use-after-free introduced by
commit 30c08574da0ead1a47797ce028218ce5b2de61c7.
Spotted by the Coverity checker.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
--- linux-2.6/drivers/acpi/ec.c.old 2007-10-23 19:39:47.0 +0200
+++ linux-2.6/drivers/acpi/ec.c 2007-10-23 19:34:55.0 +0200
@@ -434,11 +442,11 @@
EXPORT_SYMBOL_GPL(acpi_ec_add_query_handler);
void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit)
{
- struct acpi_ec_query_handler *handler;
+ struct acpi_ec_query_handler *handler, *tmp;
mutex_lock(ec-lock);
- list_for_each_entry(handler, ec-list, node) {
+ list_for_each_entry_safe(handler, tmp, ec-list, node) {
if (query_bit == handler-query_bit) {
list_del(handler-node);
kfree(handler);
}
cu
Adrian
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: latest 2.6.23 git missing ACPI POWER_SUPPLY
Frans Pop wrote: > Jeff Chua wrote: >> Just pulled latest linux-2.6, and couldn't get ACPI to detect >> ACPI_BATTERY and ACPI_AC. >> >> It seems ACPI POWER_SUPPLY is still missing. > > I had the same problem. It turns out you need to enable >drivers -> Power supply class support > (either built in or as module) to get ACPI AC/Battery support. > > I must say that having these relatively top-level ACPI settings depending on > something that is relatively buried away is not very intuitive! > Especially not since at first glance you don't really seem to need that > option except for some weird hardware. > > CC'ing ACPI mailing list for other opinions. I was thinking that 'select' might be more appropriate here... Please take a look on attached patch. > - > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > ACPI: use select POWER_SUPPLY for AC, BATTERY and SBS From: Alexey Starikovskiy <[EMAIL PROTECTED]> POWER_SUPPLY is needed for AC, battery, and SBS sysfs support. Use 'select' instead of 'depends on', as it is will not be selected by anything else, leading to confusion. Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> --- drivers/acpi/Kconfig |8 +--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 5d0e26a..ecd87d7 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -88,7 +88,8 @@ config ACPI_PROC_EVENT config ACPI_AC tristate "AC Adapter" - depends on X86 && POWER_SUPPLY + depends on X86 + select POWER_SUPPLY default y help This driver adds support for the AC Adapter object, which indicates @@ -97,7 +98,8 @@ config ACPI_AC config ACPI_BATTERY tristate "Battery" - depends on X86 && POWER_SUPPLY + depends on X86 + select POWER_SUPPLY default y help This driver adds support for battery information through @@ -352,7 +354,7 @@ config ACPI_HOTPLUG_MEMORY config ACPI_SBS tristate "Smart Battery System" depends on X86 - depends on POWER_SUPPLY + select POWER_SUPPLY help This driver adds support for the Smart Battery System, another type of access to battery information, found on some laptops.
Re: latest 2.6.23 git missing ACPI POWER_SUPPLY
Frans Pop wrote: Jeff Chua wrote: Just pulled latest linux-2.6, and couldn't get ACPI to detect ACPI_BATTERY and ACPI_AC. It seems ACPI POWER_SUPPLY is still missing. I had the same problem. It turns out you need to enable drivers - Power supply class support (either built in or as module) to get ACPI AC/Battery support. I must say that having these relatively top-level ACPI settings depending on something that is relatively buried away is not very intuitive! Especially not since at first glance you don't really seem to need that option except for some weird hardware. CC'ing ACPI mailing list for other opinions. I was thinking that 'select' might be more appropriate here... Please take a look on attached patch. - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html ACPI: use select POWER_SUPPLY for AC, BATTERY and SBS From: Alexey Starikovskiy [EMAIL PROTECTED] POWER_SUPPLY is needed for AC, battery, and SBS sysfs support. Use 'select' instead of 'depends on', as it is will not be selected by anything else, leading to confusion. Signed-off-by: Alexey Starikovskiy [EMAIL PROTECTED] --- drivers/acpi/Kconfig |8 +--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 5d0e26a..ecd87d7 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -88,7 +88,8 @@ config ACPI_PROC_EVENT config ACPI_AC tristate AC Adapter - depends on X86 POWER_SUPPLY + depends on X86 + select POWER_SUPPLY default y help This driver adds support for the AC Adapter object, which indicates @@ -97,7 +98,8 @@ config ACPI_AC config ACPI_BATTERY tristate Battery - depends on X86 POWER_SUPPLY + depends on X86 + select POWER_SUPPLY default y help This driver adds support for battery information through @@ -352,7 +354,7 @@ config ACPI_HOTPLUG_MEMORY config ACPI_SBS tristate Smart Battery System depends on X86 - depends on POWER_SUPPLY + select POWER_SUPPLY help This driver adds support for the Smart Battery System, another type of access to battery information, found on some laptops.
Re: halt does not shut the system down
John Sigler wrote: > Alexey Starikovskiy wrote: > >> John Sigler wrote: >> >>> +===+ >>> | Soft-Off by PWR-BTTN | >>> |---| >>> | Instant-Off . [v]| >>> | Delay 4 Sec. . [ ]| >>> | | >>> |---| >>> | ^V:Move ENTER:Accept ESC:Abort | >>> +===+ >>> >>> 'Instant-Off' is the appropriate setting, right? >> >> Actually, default should be 4 sec delay. OS should have a chance to >> shut down the system... > > I don't see why this setting would have an impact on the outcome > of the 'halt' and 'poweroff' commands. > Well, it is not possible to tell, what BIOS writer have connected to this flag... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
John Sigler wrote: Alexey Starikovskiy wrote: John Sigler wrote: +===+ | Soft-Off by PWR-BTTN | |---| | Instant-Off . [v]| | Delay 4 Sec. . [ ]| | | |---| | ^V:Move ENTER:Accept ESC:Abort | +===+ 'Instant-Off' is the appropriate setting, right? Actually, default should be 4 sec delay. OS should have a chance to shut down the system... I don't see why this setting would have an impact on the outcome of the 'halt' and 'poweroff' commands. Well, it is not possible to tell, what BIOS writer have connected to this flag... - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
John Sigler wrote: > +===+ > | Soft-Off by PWR-BTTN | > |---| > | Instant-Off . [v]| > | Delay 4 Sec. . [ ]| > | | > |---| > | ^V:Move ENTER:Accept ESC:Abort | > +===+ > > 'Instant-Off' is the appropriate setting, right? Actually, default should be 4 sec delay. OS should have a chance to shut down the system... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
John Sigler wrote: +===+ | Soft-Off by PWR-BTTN | |---| | Instant-Off . [v]| | Delay 4 Sec. . [ ]| | | |---| | ^V:Move ENTER:Accept ESC:Abort | +===+ 'Instant-Off' is the appropriate setting, right? Actually, default should be 4 sec delay. OS should have a chance to shut down the system... - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
John Sigler wrote: > Alexey Starikovskiy wrote: > >> John Sigler wrote: >> >>> Alexey Starikovskiy wrote: >>> >>>> Could you please open bug at bugzilla.kernel.org and put all >>>> these files there? >>> >>> http://bugzilla.kernel.org/show_bug.cgi?id=9148 >>> >>> Writing 15361 (i.e. 0x3C01) to ACPI_REGISTER_PM1A_CONTROL appears >>> to hang my system in acpi_os_write_port(). What can I do about that? >> >> That is supposed to turn your machine off. At least we now know that >> ACPI did try to turn it off. You could also try different kernel or >> defconfig. > > What's defconfig? .config? Which option(s) might have an impact? This is an option to make. It creates .config file with some default settings, appropriate to most computers. >>> Is it a BIOS issue? a kernel issue? a hardware issue? >> >> I think _other_ OS could turn your machine off just fine, so the >> issue is not HW, not BIOS. Probably, first thing to try is 2.6.23.1 >> as it was just released and has some changes in power management >> section... > > I have the same problem in 2.6.23.1 (cf. my bug report in the database) > > I'll ask the manufacturer whether they could get poweroff to work. > > Regards. > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
John Sigler wrote: > John Sigler wrote: > >> Alexey Starikovskiy wrote: >> >>> Could you please open bug at bugzilla.kernel.org and put all these >>> files there? >> >> http://bugzilla.kernel.org/show_bug.cgi?id=9148 > > Writing 15361 (i.e. 0x3C01) to ACPI_REGISTER_PM1A_CONTROL appears to > hang my system in acpi_os_write_port(). What can I do about that? That is supposed to turn your machine off. At least we now know that ACPI did try to turn it off. You could also try different kernel or defconfig. > > Is it a BIOS issue? a kernel issue? a hardware issue? I think _other_ OS could turn your machine off just fine, so the issue is not HW, not BIOS. Probably, first thing to try is 2.6.23.1 as it was just released and has some changes in power management section... Regards, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
John Sigler wrote: John Sigler wrote: Alexey Starikovskiy wrote: Could you please open bug at bugzilla.kernel.org and put all these files there? http://bugzilla.kernel.org/show_bug.cgi?id=9148 Writing 15361 (i.e. 0x3C01) to ACPI_REGISTER_PM1A_CONTROL appears to hang my system in acpi_os_write_port(). What can I do about that? That is supposed to turn your machine off. At least we now know that ACPI did try to turn it off. You could also try different kernel or defconfig. Is it a BIOS issue? a kernel issue? a hardware issue? I think _other_ OS could turn your machine off just fine, so the issue is not HW, not BIOS. Probably, first thing to try is 2.6.23.1 as it was just released and has some changes in power management section... Regards, Alex. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
John Sigler wrote: Alexey Starikovskiy wrote: John Sigler wrote: Alexey Starikovskiy wrote: Could you please open bug at bugzilla.kernel.org and put all these files there? http://bugzilla.kernel.org/show_bug.cgi?id=9148 Writing 15361 (i.e. 0x3C01) to ACPI_REGISTER_PM1A_CONTROL appears to hang my system in acpi_os_write_port(). What can I do about that? That is supposed to turn your machine off. At least we now know that ACPI did try to turn it off. You could also try different kernel or defconfig. What's defconfig? .config? Which option(s) might have an impact? This is an option to make. It creates .config file with some default settings, appropriate to most computers. Is it a BIOS issue? a kernel issue? a hardware issue? I think _other_ OS could turn your machine off just fine, so the issue is not HW, not BIOS. Probably, first thing to try is 2.6.23.1 as it was just released and has some changes in power management section... I have the same problem in 2.6.23.1 (cf. my bug report in the database) I'll ask the manufacturer whether they could get poweroff to work. Regards. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
Could you please open bug at bugzilla.kernel.org and put all these files there? Thanks, Alex. John Sigler wrote: John Sigler wrote: When I run 'halt' the kernel prints: Halting. Shutdown: hdc ACPI: PCI interrupt for device :01:05.0 disabled ACPI: PCI interrupt for device :01:04.0 disabled ACPI: PCI interrupt for device :01:03.0 disabled ACPI: PCI interrupt for device :01:02.0 disabled Power down. acpi_power_off called But the system does not shut down. (The fans keep spinning, the LEDs keep shining, the LCD keeps displaying.) Basically, the motherboard is still providing power to every component, as if the power supply had refused to stop. http://linux.kernel.free.fr/halt/config-2.6.22.1-rt9 http://linux.kernel.free.fr/halt/acpidump.txt http://linux.kernel.free.fr/halt/dmesg.txt http://linux.kernel.free.fr/halt/halt.txt http://linux.kernel.free.fr/halt/lspci.txt Is there something else I can provide that might help in identifying the problem? Regards. - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: halt does not shut the system down
Could you please open bug at bugzilla.kernel.org and put all these files there? Thanks, Alex. John Sigler wrote: John Sigler wrote: When I run 'halt' the kernel prints: Halting. Shutdown: hdc ACPI: PCI interrupt for device :01:05.0 disabled ACPI: PCI interrupt for device :01:04.0 disabled ACPI: PCI interrupt for device :01:03.0 disabled ACPI: PCI interrupt for device :01:02.0 disabled Power down. acpi_power_off called But the system does not shut down. (The fans keep spinning, the LEDs keep shining, the LCD keeps displaying.) Basically, the motherboard is still providing power to every component, as if the power supply had refused to stop. http://linux.kernel.free.fr/halt/config-2.6.22.1-rt9 http://linux.kernel.free.fr/halt/acpidump.txt http://linux.kernel.free.fr/halt/dmesg.txt http://linux.kernel.free.fr/halt/halt.txt http://linux.kernel.free.fr/halt/lspci.txt Is there something else I can provide that might help in identifying the problem? Regards. - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: regression in 2.6.23-rc8 - power off failed
Rafael J. Wysocki wrote:
On Saturday, 29 September 2007 22:47, Bill Davidsen wrote:
Alexey Starikovskiy wrote:
-static void
-acpi_power_off (void)
-{
- printk("%s called\n",__FUNCTION__);
- /* Some SMP machines only can poweroff in boot CPU */
- set_cpus_allowed(current, cpumask_of_cpu(0));
ACPI in kernel 2.6.12 did disable non-boot cpus too in powe_off.
Later only comment was left for some reason...
Am I midreading that code, or does it really assume that the boot cpu is
always zero? Or just that zero will be able to do the power off?
In any case I have had an SMP machine which did not have a CPU zero, and
it was discussed here, I believe. Wonder what happens if you set
affinity to a CPU you don't have...
Good question, but it also caused other problems to appear, IIRC.
IMHO, it's better to call disable_nonboot_cpus() in an appropriate place
anyway.
Greetings,
Rafael
Ok, here is commit which removed the code in question from acpi_power_off:
commit 6660316cb7a1a2c59a73a52870490c0f782f45c1
Author: Eric W. Biederman <[EMAIL PROTECTED]>
Date: Tue Jul 26 12:16:00 2005 -0600
[PATCH] acpi_power_off: Don't switch to the boot cpu
machine_power_off on i386 and x86_64 now switch to the
boot cpu out of paranoia and because the MP Specification indicates it
is a good idea on reboot, so for those architectures it is a noop.
I can't see anything in the acpi spec that requires you to be on
the boot cpu to power off the system, so this should not be an issue
for ia64. In addition ia64 has the altix a massive multi-node
system where switching to the boot cpu sounds insane as we may
hot removed the boot cpu.
Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>
Signed-off-by: Linus Torvalds <[EMAIL PROTECTED]>
Regards,
Alex.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: regression in 2.6.23-rc8 - power off failed
Rafael J. Wysocki wrote:
On Saturday, 29 September 2007 22:47, Bill Davidsen wrote:
Alexey Starikovskiy wrote:
-static void
-acpi_power_off (void)
-{
- printk(%s called\n,__FUNCTION__);
- /* Some SMP machines only can poweroff in boot CPU */
- set_cpus_allowed(current, cpumask_of_cpu(0));
ACPI in kernel 2.6.12 did disable non-boot cpus too in powe_off.
Later only comment was left for some reason...
Am I midreading that code, or does it really assume that the boot cpu is
always zero? Or just that zero will be able to do the power off?
In any case I have had an SMP machine which did not have a CPU zero, and
it was discussed here, I believe. Wonder what happens if you set
affinity to a CPU you don't have...
Good question, but it also caused other problems to appear, IIRC.
IMHO, it's better to call disable_nonboot_cpus() in an appropriate place
anyway.
Greetings,
Rafael
Ok, here is commit which removed the code in question from acpi_power_off:
commit 6660316cb7a1a2c59a73a52870490c0f782f45c1
Author: Eric W. Biederman [EMAIL PROTECTED]
Date: Tue Jul 26 12:16:00 2005 -0600
[PATCH] acpi_power_off: Don't switch to the boot cpu
machine_power_off on i386 and x86_64 now switch to the
boot cpu out of paranoia and because the MP Specification indicates it
is a good idea on reboot, so for those architectures it is a noop.
I can't see anything in the acpi spec that requires you to be on
the boot cpu to power off the system, so this should not be an issue
for ia64. In addition ia64 has the altix a massive multi-node
system where switching to the boot cpu sounds insane as we may
hot removed the boot cpu.
Signed-off-by: Eric W. Biederman [EMAIL PROTECTED]
Signed-off-by: Linus Torvalds [EMAIL PROTECTED]
Regards,
Alex.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: regression in 2.6.23-rc8 - power off failed
Mark Lord wrote:
Wolfgang Erig wrote:
On Sat, Sep 29, 2007 at 01:30:33AM -0700, H. Peter Anvin wrote:
Wolfgang Erig wrote:
Both are bad.
Two different systems and two different bisections.
I sent the last step of each.
$ git bisect good Bisecting: 0 revisions left to test after this
[626073132b381684c4983e0d911e9aceb32e2cbc] Assembly header and
main routine for new x86 setup code
OK, so which one is the bad one?
This problem (no power off) persists after pull some minutes ago.
Sorry for the confusion.
I believe there must have been something wrong here (possibly
inconsistent experiments?) This checkin has *zero code changes* from
the previous one (and next one) -- the kernel should have been binarily
identical to the previous one. The code introduced in this checkin
doesn't even get compiled until two checkins later,
4fd06960f120e02e9abc802a09f9511c400042a5.
I have done two bisections simultanously and it was late at night.
I start again with a fresh tree and better controlled experiments.
If this is an SMP system, then you could just be getting random results,
depending upon which CPU is attempting the poweroff.
I have a newish patch in Andrew's tree now to fix SMP poweroff
(has been broken forever), reproduced here below in case you missed it.
* * *
We need to disable all CPUs other than the boot CPU (usually 0)
before attempting to power-off modern SMP machines.
This fixes the hang-on-poweroff issue on my MythTV SMP box,
and also on Thomas Gleixner's new toybox.
Signed-off-by: Mark Lord <[EMAIL PROTECTED]>
Acked-by: Thomas Gleixner <[EMAIL PROTECTED]>
---
--- linux/kernel/sys.c.orig2007-09-13 09:49:11.0 -0400
+++ linux/kernel/sys.c2007-09-28 15:48:54.0 -0400
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
@@ -878,6 +879,7 @@
kernel_shutdown_prepare(SYSTEM_POWER_OFF);
if (pm_power_off_prepare)
pm_power_off_prepare();
+disable_nonboot_cpus();
sysdev_shutdown();
printk(KERN_EMERG "Power down.\n");
machine_power_off();
-static void
-acpi_power_off (void)
-{
- printk("%s called\n",__FUNCTION__);
- /* Some SMP machines only can poweroff in boot CPU */
- set_cpus_allowed(current, cpumask_of_cpu(0));
ACPI in kernel 2.6.12 did disable non-boot cpus too in powe_off.
Later only comment was left for some reason...
Regards,
Alex.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: regression in 2.6.23-rc8 - power off failed
Hi, This is known issue. Please try latest rc8-git2, it should contain the fix. Regards, Alex. Wolfgang Erig wrote: Hi, the latest kernel does not power off my system. 2.6.22 succeeded 2.6.23-rc8 failed $ git bisect bad Bisecting: 0 revisions left to test after this [f216cc3748a3a22c2b99390fddcdafa0583791a2] ACPI: suspend: consolidate handling of Sx states. Which more info is needed? Wolfgang - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: regression in 2.6.23-rc8 - power off failed
Hi, This is known issue. Please try latest rc8-git2, it should contain the fix. Regards, Alex. Wolfgang Erig wrote: Hi, the latest kernel does not power off my system. 2.6.22 succeeded 2.6.23-rc8 failed $ git bisect bad Bisecting: 0 revisions left to test after this [f216cc3748a3a22c2b99390fddcdafa0583791a2] ACPI: suspend: consolidate handling of Sx states. Which more info is needed? Wolfgang - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: regression in 2.6.23-rc8 - power off failed
Mark Lord wrote:
Wolfgang Erig wrote:
On Sat, Sep 29, 2007 at 01:30:33AM -0700, H. Peter Anvin wrote:
Wolfgang Erig wrote:
Both are bad.
Two different systems and two different bisections.
I sent the last step of each.
$ git bisect good Bisecting: 0 revisions left to test after this
[626073132b381684c4983e0d911e9aceb32e2cbc] Assembly header and
main routine for new x86 setup code
OK, so which one is the bad one?
This problem (no power off) persists after pull some minutes ago.
Sorry for the confusion.
I believe there must have been something wrong here (possibly
inconsistent experiments?) This checkin has *zero code changes* from
the previous one (and next one) -- the kernel should have been binarily
identical to the previous one. The code introduced in this checkin
doesn't even get compiled until two checkins later,
4fd06960f120e02e9abc802a09f9511c400042a5.
I have done two bisections simultanously and it was late at night.
I start again with a fresh tree and better controlled experiments.
If this is an SMP system, then you could just be getting random results,
depending upon which CPU is attempting the poweroff.
I have a newish patch in Andrew's tree now to fix SMP poweroff
(has been broken forever), reproduced here below in case you missed it.
* * *
We need to disable all CPUs other than the boot CPU (usually 0)
before attempting to power-off modern SMP machines.
This fixes the hang-on-poweroff issue on my MythTV SMP box,
and also on Thomas Gleixner's new toybox.
Signed-off-by: Mark Lord [EMAIL PROTECTED]
Acked-by: Thomas Gleixner [EMAIL PROTECTED]
---
--- linux/kernel/sys.c.orig2007-09-13 09:49:11.0 -0400
+++ linux/kernel/sys.c2007-09-28 15:48:54.0 -0400
@@ -32,6 +32,7 @@
#include linux/getcpu.h
#include linux/task_io_accounting_ops.h
#include linux/seccomp.h
+#include linux/cpu.h
#include linux/compat.h
#include linux/syscalls.h
@@ -878,6 +879,7 @@
kernel_shutdown_prepare(SYSTEM_POWER_OFF);
if (pm_power_off_prepare)
pm_power_off_prepare();
+disable_nonboot_cpus();
sysdev_shutdown();
printk(KERN_EMERG Power down.\n);
machine_power_off();
-static void
-acpi_power_off (void)
-{
- printk(%s called\n,__FUNCTION__);
- /* Some SMP machines only can poweroff in boot CPU */
- set_cpus_allowed(current, cpumask_of_cpu(0));
ACPI in kernel 2.6.12 did disable non-boot cpus too in powe_off.
Later only comment was left for some reason...
Regards,
Alex.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Torsten Kaiser wrote: > Back to debugging this: > http://marc.info/?l=linux-acpi=119052970904643=4 > fails to apply against 2.6.23-rc7-mm1, but moving that function by > hand was not to difficult. ;) > (With only the second patch I got a link error...) > > http://marc.info/?l=linux-acpi=119073173625910=4 > applied, and a test showed that my system now powers off again. Good, thanks, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Torsten Kaiser wrote: > On 9/25/07, Alexey Starikovskiy <[EMAIL PROTECTED]> wrote: >> Torsten Kaiser wrote: >>> No, I do not have CONFIG_ACPI_SLEEP set, >>> because I do not have CONFIG_PM_SLEEP set, >>> because I do not want SUSPEND and/or HIBERNATION. >> This is not the reason. SUSPEND is controlled with CONFIG_SUSPEND and >> HIBERNATION is controlled with CONFIG_HIBERNATION. >> But if you want S5 ACPI sleep state you might want to enable ACPI_SLEEP... > > What I meant with SUSPEND and/or HIBERNATION was CONFIG_SUSPEND / > CONFIG_HIBERNATION. > > And the relations where from Kconfig: > from drivers/acpi/Kconfig: > config ACPI_SLEEP > bool > depends on PM_SLEEP > default y > > -> no PM_SLEEP means no ACPI_SLEEP > > from kernel/power/Kconfig: > config PM_SLEEP > bool > depends on SUSPEND || HIBERNATION > default y > > -> No SUSPEND and/or HIBERNATION means no PM_SLEEP > > And if I select SUSPEND and/or HIBERNATION I will not only build this > feature into the kernel, but also HOTPLUG_CPU and I want to avoid > that. > > It's exactly as Linus said in his mail: Not everybody wants SUSPEND... > > I should have formulated that better in my mail, but that was what I > wanted to say. > > > Back to debugging this: > http://marc.info/?l=linux-acpi=119052970904643=4 > fails to apply against 2.6.23-rc7-mm1, but moving that function by > hand was not to difficult. ;) > (With only the second patch I got a link error...) > > http://marc.info/?l=linux-acpi=119073173625910=4 > applied, and a test showed that my system now powers off again. > > Torsten Understood, patches are available, please test. Regards, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Andrew, There are 2 patches, this is the second. Above, Rafael gave link to first. Here it is again: http://marc.info/?l=linux-kernel=119052978117735=4 Sorry for confusion, Alex. Andrew Morton wrote: > On Tue, 25 Sep 2007 18:45:15 +0400 Alexey Starikovskiy <[EMAIL PROTECTED]> > wrote: > >> [fix-ACPI_SLEEP_states.patch text/x-patch (2.0KB)] >> ACPI: suspend: fix ACPI_SLEEP states >> >> From: Alexey Starikovskiy <[EMAIL PROTECTED]> >> >> Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> >> --- >> >> drivers/acpi/sleep/Makefile |2 +- >> drivers/acpi/sleep/main.c |4 >> include/acpi/acpi_drivers.h |4 >> 3 files changed, 5 insertions(+), 5 deletions(-) > > I get a reject applying this to current mainline. Easy enough to fix it, > but I worry that the fix might be incorrect when applied to some tree other > than that which you were working on. > > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 16:19, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 15:15, Alexey Starikovskiy wrote: >>>> Rafael J. Wysocki wrote: >>>>> On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: >>>>>> Rafael J. Wysocki wrote: >>>>>>> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: >>>>>>>> Rafael J. Wysocki wrote: >>>>>>>>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: >>>>>>>>>> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: >>>>>>>>>>>>> No, I do not have CONFIG_ACPI_SLEEP set, >>>>>>>>>>>>> because I do not have CONFIG_PM_SLEEP set, >>>>>>>>>>>>> because I do not want SUSPEND and/or HIBERNATION. >>>>>>>>>>>> Same answer from my side: I do not have CONFIG_ACPI_SLEEP for the >>>>>>>>>>>> same >>>>>>>>>>>> reason (and this worked fine without them in rc7). I do not think >>>>>>>>>>>> these settings should have changed between rc7 and rc8. >>>>>>>>>> Well, we haven't changed much. >>>>>>>>>> >>>>>>>>>>> Also, another test I just did: on another computer, rc8 is fine >>>>>>>>>>> regarding ACPI power off, even if CONFIG_ACPI_SLEEP is not set. I >>>>>>>>>>> can >>>>>>>>>>> provide config if needed. >>>>>>>>>> On the box that fails to power off, can you please test -rc8 with >>>>>>>>>> these two >>>>>>>>>> commits reverted: >>>>>>>>>> >>>>>>>>>> commit 5a50fe709d527f31169263e36601dd83446d5744 >>>>>>>>>> ACPI: suspend: consolidate handling of Sx states addendum >>>>>>>>>> >>>>>>>>>> commit f216cc3748a3a22c2b99390fddcdafa0583791a2 >>>>>>>>>> ACPI: suspend: consolidate handling of Sx states. >>>>>>>>>> >>>>>>>>>> and see if it works? >>>>>>>>> If it does, please test the patch from this message >>>>>>>>> >>>>>>>>> http://marc.info/?l=linux-kernel=119052978117735=4 >>>>>>>>> >>>>>>>>> on top of vanilla 2.6.23-rc8. >>>>>>>> You will need one more patch on top of just mentioned one. >>>>>>> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? >>>>>>> >>>>>>> CONFIG_HIBERNATION needs acpi_target_sleep_state too. >>>>>> Agree, attaching updated patch. >>>>> Well, please use "ifdef CONFIG_PM_SLEEP" instead of >>>>> "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", >>>>> as you did with the second block. >>>> I was thinking about that, but it seem to be less clear... >>>> We need this variable only for suspend or hibernation, nothing else. >>>> with pm_sleep it is not visible at all. >>>> >>>> Thoughts? >>> Well, PM_SLEEP is defined as (SUSPEND || HIBERNATION), please have a look >>> at kernel/power/Kconfig, and it was introduced exactly for the conditions >>> like >>> this. >> I've seen this then I wrote the patch :) See my point, it is not clear, >> that PM_SLEEP is equivalent to SUSPEND || HIBERNATION, one needs to >> grep Kconfig files to find that -- it means that code becomes less readable, >> and I would like to avoid that. > > I see your point. Still, you are using PM_SLEEP in the same file, so someone > reading the code for the first time will have to find out what it is anyway. In the second place it depends on header file using PM_SLEEP, so it makes sense. > > OTOH, the only function of PM_SLEEP is to be a replacement for > (SUSPEND || HIBERNATION). It has no other meaning whatsoever. > > [Well, sorry, I couldn't invent a better name.] > >>> IOW, if we want something to be used for anything else than suspend or >>> hibernation, it shouldn't be defined under PM_SLEEP. >> Agree, but we should distinguish there it is better to use PM_SLEEP, &
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 15:15, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: >>>> Rafael J. Wysocki wrote: >>>>> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: >>>>>> Rafael J. Wysocki wrote: >>>>>>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: >>>>>>>> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: >>>>>>>>>>> No, I do not have CONFIG_ACPI_SLEEP set, >>>>>>>>>>> because I do not have CONFIG_PM_SLEEP set, >>>>>>>>>>> because I do not want SUSPEND and/or HIBERNATION. >>>>>>>>>> Same answer from my side: I do not have CONFIG_ACPI_SLEEP for the >>>>>>>>>> same >>>>>>>>>> reason (and this worked fine without them in rc7). I do not think >>>>>>>>>> these settings should have changed between rc7 and rc8. >>>>>>>> Well, we haven't changed much. >>>>>>>> >>>>>>>>> Also, another test I just did: on another computer, rc8 is fine >>>>>>>>> regarding ACPI power off, even if CONFIG_ACPI_SLEEP is not set. I can >>>>>>>>> provide config if needed. >>>>>>>> On the box that fails to power off, can you please test -rc8 with >>>>>>>> these two >>>>>>>> commits reverted: >>>>>>>> >>>>>>>> commit 5a50fe709d527f31169263e36601dd83446d5744 >>>>>>>> ACPI: suspend: consolidate handling of Sx states addendum >>>>>>>> >>>>>>>> commit f216cc3748a3a22c2b99390fddcdafa0583791a2 >>>>>>>> ACPI: suspend: consolidate handling of Sx states. >>>>>>>> >>>>>>>> and see if it works? >>>>>>> If it does, please test the patch from this message >>>>>>> >>>>>>> http://marc.info/?l=linux-kernel=119052978117735=4 >>>>>>> >>>>>>> on top of vanilla 2.6.23-rc8. >>>>>> You will need one more patch on top of just mentioned one. >>>>> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? >>>>> >>>>> CONFIG_HIBERNATION needs acpi_target_sleep_state too. >>>> Agree, attaching updated patch. >>> Well, please use "ifdef CONFIG_PM_SLEEP" instead of >>> "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", >>> as you did with the second block. >> I was thinking about that, but it seem to be less clear... >> We need this variable only for suspend or hibernation, nothing else. >> with pm_sleep it is not visible at all. >> >> Thoughts? > > Well, PM_SLEEP is defined as (SUSPEND || HIBERNATION), please have a look > at kernel/power/Kconfig, and it was introduced exactly for the conditions like > this. I've seen this then I wrote the patch :) See my point, it is not clear, that PM_SLEEP is equivalent to SUSPEND || HIBERNATION, one needs to grep Kconfig files to find that -- it means that code becomes less readable, and I would like to avoid that. > > IOW, if we want something to be used for anything else than suspend or > hibernation, it shouldn't be defined under PM_SLEEP. Agree, but we should distinguish there it is better to use PM_SLEEP, and there it is better to use (SUSPEND || HIBERNATION) just to be more expressive... Regards, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: >>>> Rafael J. Wysocki wrote: >>>>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: >>>>>> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: >>>>>>>>> No, I do not have CONFIG_ACPI_SLEEP set, >>>>>>>>> because I do not have CONFIG_PM_SLEEP set, >>>>>>>>> because I do not want SUSPEND and/or HIBERNATION. >>>>>>>> Same answer from my side: I do not have CONFIG_ACPI_SLEEP for the same >>>>>>>> reason (and this worked fine without them in rc7). I do not think >>>>>>>> these settings should have changed between rc7 and rc8. >>>>>> Well, we haven't changed much. >>>>>> >>>>>>> Also, another test I just did: on another computer, rc8 is fine >>>>>>> regarding ACPI power off, even if CONFIG_ACPI_SLEEP is not set. I can >>>>>>> provide config if needed. >>>>>> On the box that fails to power off, can you please test -rc8 with these >>>>>> two >>>>>> commits reverted: >>>>>> >>>>>> commit 5a50fe709d527f31169263e36601dd83446d5744 >>>>>> ACPI: suspend: consolidate handling of Sx states addendum >>>>>> >>>>>> commit f216cc3748a3a22c2b99390fddcdafa0583791a2 >>>>>> ACPI: suspend: consolidate handling of Sx states. >>>>>> >>>>>> and see if it works? >>>>> If it does, please test the patch from this message >>>>> >>>>> http://marc.info/?l=linux-kernel=119052978117735=4 >>>>> >>>>> on top of vanilla 2.6.23-rc8. >>>> You will need one more patch on top of just mentioned one. >>> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? >>> >>> CONFIG_HIBERNATION needs acpi_target_sleep_state too. >> Agree, attaching updated patch. > > Well, please use "ifdef CONFIG_PM_SLEEP" instead of > "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", > as you did with the second block. I was thinking about that, but it seem to be less clear... We need this variable only for suspend or hibernation, nothing else. with pm_sleep it is not visible at all. Thoughts? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote:
> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote:
>> Rafael J. Wysocki wrote:
>>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote:
>>>> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote:
>>>>>>> No, I do not have CONFIG_ACPI_SLEEP set,
>>>>>>> because I do not have CONFIG_PM_SLEEP set,
>>>>>>> because I do not want SUSPEND and/or HIBERNATION.
>>>>>> Same answer from my side: I do not have CONFIG_ACPI_SLEEP for the same
>>>>>> reason (and this worked fine without them in rc7). I do not think
>>>>>> these settings should have changed between rc7 and rc8.
>>>> Well, we haven't changed much.
>>>>
>>>>> Also, another test I just did: on another computer, rc8 is fine
>>>>> regarding ACPI power off, even if CONFIG_ACPI_SLEEP is not set. I can
>>>>> provide config if needed.
>>>> On the box that fails to power off, can you please test -rc8 with these two
>>>> commits reverted:
>>>>
>>>> commit 5a50fe709d527f31169263e36601dd83446d5744
>>>> ACPI: suspend: consolidate handling of Sx states addendum
>>>>
>>>> commit f216cc3748a3a22c2b99390fddcdafa0583791a2
>>>> ACPI: suspend: consolidate handling of Sx states.
>>>>
>>>> and see if it works?
>>> If it does, please test the patch from this message
>>>
>>> http://marc.info/?l=linux-kernel=119052978117735=4
>>>
>>> on top of vanilla 2.6.23-rc8.
>> You will need one more patch on top of just mentioned one.
>
> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND?
>
> CONFIG_HIBERNATION needs acpi_target_sleep_state too.
Agree, attaching updated patch.
Thanks,
Alex.
ACPI: suspend: fix ACPI_SLEEP states
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/sleep/Makefile |2 +-
drivers/acpi/sleep/main.c |5 +
include/acpi/acpi_drivers.h |4
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/sleep/Makefile b/drivers/acpi/sleep/Makefile
index ba9bd40..f1fb888 100644
--- a/drivers/acpi/sleep/Makefile
+++ b/drivers/acpi/sleep/Makefile
@@ -1,5 +1,5 @@
obj-y := wakeup.o
-obj-$(CONFIG_ACPI_SLEEP) += main.o
+obj-y += main.o
obj-$(CONFIG_ACPI_SLEEP) += proc.o
EXTRA_CFLAGS += $(ACPI_CFLAGS)
diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
index c79edcb..6ea0628 100644
--- a/drivers/acpi/sleep/main.c
+++ b/drivers/acpi/sleep/main.c
@@ -24,7 +24,9 @@
u8 sleep_states[ACPI_S_STATE_COUNT];
+#if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)
static u32 acpi_target_sleep_state = ACPI_STATE_S0;
+#endif
int acpi_sleep_prepare(u32 acpi_state)
{
@@ -48,6 +50,7 @@ int acpi_sleep_prepare(u32 acpi_state)
}
#ifdef CONFIG_SUSPEND
+
static struct pm_ops acpi_pm_ops;
extern void do_suspend_lowlevel(void);
@@ -299,6 +302,7 @@ int acpi_suspend(u32 acpi_state)
return -EINVAL;
}
+#ifdef CONFIG_PM_SLEEP
/**
* acpi_pm_device_sleep_state - return preferred power state of ACPI device
* in the system sleep state given by %acpi_target_sleep_state
@@ -373,6 +377,7 @@ int acpi_pm_device_sleep_state(struct device *dev, int wake, int *d_min_p)
*d_min_p = d_min;
return d_max;
}
+#endif
static void acpi_power_off_prepare(void)
{
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 202acb9..f85f77a 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -147,10 +147,6 @@ static inline void unregister_hotplug_dock_device(acpi_handle handle)
/*--
Suspend/Resume
-- */
-#ifdef CONFIG_ACPI_SLEEP
extern int acpi_sleep_init(void);
-#else
-static inline int acpi_sleep_init(void) { return 0; }
-#endif
#endif /*__ACPI_DRIVERS_H__*/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote:
> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote:
>> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote:
>>>>> No, I do not have CONFIG_ACPI_SLEEP set,
>>>>> because I do not have CONFIG_PM_SLEEP set,
>>>>> because I do not want SUSPEND and/or HIBERNATION.
>>>> Same answer from my side: I do not have CONFIG_ACPI_SLEEP for the same
>>>> reason (and this worked fine without them in rc7). I do not think
>>>> these settings should have changed between rc7 and rc8.
>> Well, we haven't changed much.
>>
>>> Also, another test I just did: on another computer, rc8 is fine
>>> regarding ACPI power off, even if CONFIG_ACPI_SLEEP is not set. I can
>>> provide config if needed.
>> On the box that fails to power off, can you please test -rc8 with these two
>> commits reverted:
>>
>> commit 5a50fe709d527f31169263e36601dd83446d5744
>> ACPI: suspend: consolidate handling of Sx states addendum
>>
>> commit f216cc3748a3a22c2b99390fddcdafa0583791a2
>> ACPI: suspend: consolidate handling of Sx states.
>>
>> and see if it works?
>
> If it does, please test the patch from this message
>
> http://marc.info/?l=linux-kernel=119052978117735=4
>
> on top of vanilla 2.6.23-rc8.
You will need one more patch on top of just mentioned one.
Regards,
Alex.
>
> Greetings,
> Rafael
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
ACPI: suspend: fix ACPI_SLEEP states
From: Alexey Starikovskiy <[EMAIL PROTECTED]>
Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]>
---
drivers/acpi/sleep/Makefile |2 +-
drivers/acpi/sleep/main.c |7 +--
include/acpi/acpi_drivers.h |4
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/sleep/Makefile b/drivers/acpi/sleep/Makefile
index ba9bd40..f1fb888 100644
--- a/drivers/acpi/sleep/Makefile
+++ b/drivers/acpi/sleep/Makefile
@@ -1,5 +1,5 @@
obj-y := wakeup.o
-obj-$(CONFIG_ACPI_SLEEP) += main.o
+obj-y += main.o
obj-$(CONFIG_ACPI_SLEEP) += proc.o
EXTRA_CFLAGS += $(ACPI_CFLAGS)
diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
index c79edcb..86415f1 100644
--- a/drivers/acpi/sleep/main.c
+++ b/drivers/acpi/sleep/main.c
@@ -24,8 +24,6 @@
u8 sleep_states[ACPI_S_STATE_COUNT];
-static u32 acpi_target_sleep_state = ACPI_STATE_S0;
-
int acpi_sleep_prepare(u32 acpi_state)
{
#ifdef CONFIG_ACPI_SLEEP
@@ -48,6 +46,9 @@ int acpi_sleep_prepare(u32 acpi_state)
}
#ifdef CONFIG_SUSPEND
+
+static u32 acpi_target_sleep_state = ACPI_STATE_S0;
+
static struct pm_ops acpi_pm_ops;
extern void do_suspend_lowlevel(void);
@@ -299,6 +300,7 @@ int acpi_suspend(u32 acpi_state)
return -EINVAL;
}
+#ifdef CONFIG_PM_SLEEP
/**
* acpi_pm_device_sleep_state - return preferred power state of ACPI device
* in the system sleep state given by %acpi_target_sleep_state
@@ -373,6 +375,7 @@ int acpi_pm_device_sleep_state(struct device *dev, int wake, int *d_min_p)
*d_min_p = d_min;
return d_max;
}
+#endif
static void acpi_power_off_prepare(void)
{
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 202acb9..f85f77a 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -147,10 +147,6 @@ static inline void unregister_hotplug_dock_device(acpi_handle handle)
/*--
Suspend/Resume
-- */
-#ifdef CONFIG_ACPI_SLEEP
extern int acpi_sleep_init(void);
-#else
-static inline int acpi_sleep_init(void) { return 0; }
-#endif
#endif /*__ACPI_DRIVERS_H__*/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Daniel, thanks for the patch, but patch for solving your issue is already done. And it is different from one we having here. If you feel "patchy" today you may try to remove ACPI_SLEEP from drivers/acpi/sleep/Makefile in the raw of main.o Regards, Alex. Daniel Ritz wrote: > does that one help? > > [ as attachment since i'm on webmail ] > > ACPI: acpi_sleep_prepare() should not depent on CONFIG_SUSPEND > Signed-off-by: Daniel Ritz <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Torsten Kaiser wrote: > On 9/25/07, Alexey Starikovskiy <[EMAIL PROTECTED]> wrote: >> Do you have CONFIG_ACPI_SLEEP enabled in your .config? > > I'm answering that too, because I suspect that my "2.6.23-rc7-mm1 does > not power off"-error might have the same cause. > > No, I do not have CONFIG_ACPI_SLEEP set, > because I do not have CONFIG_PM_SLEEP set, > because I do not want SUSPEND and/or HIBERNATION. This is not the reason. SUSPEND is controlled with CONFIG_SUSPEND and HIBERNATION is controlled with CONFIG_HIBERNATION. But if you want S5 ACPI sleep state you might want to enable ACPI_SLEEP... > > .config from 2.6.23-rc7-mm1 attached. > > Torsten > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
