Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
On Wednesday 31 October 2007, Alexey Starikovskiy wrote: > 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 :) > For HAL regression: Tested-by: Andrey Borzenkov <[EMAIL PROTECTED]> But the patch is incomplete. This synthetic event should be the same as if OS were running; i.e. you should send both other events as well. As is you just create latent bug to be hit by some netlink consumer sooner or later. signature.asc Description: This is a digitally signed message part.
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
On Wednesday 31 October 2007, Alexey Starikovskiy wrote: 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 :) For HAL regression: Tested-by: Andrey Borzenkov [EMAIL PROTECTED] But the patch is incomplete. This synthetic event should be the same as if OS were running; i.e. you should send both other events as well. As is you just create latent bug to be hit by some netlink consumer sooner or later. signature.asc Description: This is a digitally signed message part.
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
On Wednesday 31 October 2007, Rafael J. Wysocki wrote: > On Tuesday, 30 October 2007 21:24, 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. > > Is this a resume from RAM or from disk? > RAM > Does 2.6.23 work correctly? > yes signature.asc Description: This is a digitally signed message part.
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
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. signature.asc Description: This is a digitally signed message part.
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
On Tuesday, 30 October 2007 21:24, 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. Is this a resume from RAM or from disk? Does 2.6.23 work correctly? Rafael - 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/
[2.6.24-rc1 regression] AC adapter state does not change after resume
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 signature.asc Description: This is a digitally signed message part.
[2.6.24-rc1 regression] AC adapter state does not change after resume
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 signature.asc Description: This is a digitally signed message part.
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
On Tuesday, 30 October 2007 21:24, 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. Is this a resume from RAM or from disk? Does 2.6.23 work correctly? Rafael - 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
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. signature.asc Description: This is a digitally signed message part.
Re: [2.6.24-rc1 regression] AC adapter state does not change after resume
On Wednesday 31 October 2007, Rafael J. Wysocki wrote: On Tuesday, 30 October 2007 21:24, 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. Is this a resume from RAM or from disk? RAM Does 2.6.23 work correctly? yes signature.asc Description: This is a digitally signed message part.
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;
