[PATCH linux-acpi] Fix /proc/acpi/alarm set error

[PATCH linux-acpi] Fix /proc/acpi/alarm set error

[Yi Yang]

/proc/acpi/alarm can't be set correctly, here is a sample:

[root@localhost /]# echo "2006 09" > /proc/acpi/alarm
[root@localhost /]# cat /proc/acpi/alarm
2007-12-09 09:09:09
[root@localhost /]# echo "2006 04" > /proc/acpi/alarm
[root@localhost /]# cat /proc/acpi/alarm
2007-12-04 04:04:04
[root@localhost /]#

Obviously, it is wrong, it should consider it as an invalid input.

This patch'll fix this issue, after applying this patch, the result is:

[root@localhost /]# echo "2008 09" > /proc/acpi/alarm
-bash: echo: write error: Invalid argument
[root@localhost /]#

Signed-off by Yi Yang <yi.y.yang@intel.com>
 
diff --git a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c
index 1538355..fce78fb 100644
--- a/drivers/acpi/sleep/proc.c
+++ b/drivers/acpi/sleep/proc.c
@@ -178,6 +178,9 @@ static int get_date_field(char **p, u32 * value)
 	 * Try to find delimeter, only to insert null.  The end of the
 	 * string won't have one, but is still valid.
 	 */
+	if (*p == NULL)
+		return result;
+
 	next = strpbrk(*p, "- :");
 	if (next)
 		*next++ = '\0';
@@ -190,6 +193,8 @@ static int get_date_field(char **p, u32 * value)
 
 	if (next)
 		*p = next;
+	else
+		*p = NULL;
 
 	return result;
 }

[PATCH linux-acpi] Remove superfluous code and correct counting error in function acpi_system_write_alarm

[Yi Yang]

In function acpi_system_write_alarm in file drivers/acpi/sleep/proc.c,
big sec, min, hr, mo, day and yr are counted twice to get reasonable
values, that is very superfluous, we can do that only once.

In additon, /proc/acpi/alarm can set a related value which can be
specified as YYYY years MM months DD days HH hours MM minutes SS
senconds, it isn't a date, so you can specify as +0000-00-00 96:00:00
, that means 3 days later, current code can't handle such a case.

This patch removes unnecessary code and does with the aforementioned
situation.

Before applying this patch:

[root@localhost /]# cat /proc/acpi/alarm
2007-12-00 00:00:00
[root@localhost /]# echo "0000-00-00 96:180:180" > /proc/acpi/alarm
[root@localhost /]# cat /proc/acpi/alarm
0007-12-02 **:**:**
[root@localhost /]#

After applying this patch:

[root@localhost ~]# echo "2007-12-00 00:00:00" > /proc/acpi/alarm
[root@localhost ~]# cat /proc/acpi/alarm
2007-12-00 00:00:00
[root@localhost ~]# echo "0000-00-00 96:180:180" > /proc/acpi/alarm
[root@localhost ~]# cat /proc/acpi/alarm
0007-12-04 03:03:00
[root@localhost ~]#

Signed-off by Yi Yang <yi.y.yang@intel.com>

diff --git a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c
index fce78fb..f8df521 100644
--- a/drivers/acpi/sleep/proc.c
+++ b/drivers/acpi/sleep/proc.c
@@ -256,27 +256,6 @@ acpi_system_write_alarm(struct file *file,
 	if ((result = get_date_field(&p, &sec)))
 		goto end;
 
-	if (sec > 59) {
-		min += 1;
-		sec -= 60;
-	}
-	if (min > 59) {
-		hr += 1;
-		min -= 60;
-	}
-	if (hr > 23) {
-		day += 1;
-		hr -= 24;
-	}
-	if (day > 31) {
-		mo += 1;
-		day -= 31;
-	}
-	if (mo > 12) {
-		yr += 1;
-		mo -= 12;
-	}
-
 	spin_lock_irq(&rtc_lock);
 
 	rtc_control = CMOS_READ(RTC_CONTROL);
@@ -293,24 +272,24 @@ acpi_system_write_alarm(struct file *file,
 	spin_unlock_irq(&rtc_lock);
 
 	if (sec > 59) {
-		min++;
-		sec -= 60;
+		min += sec/60;
+		sec = sec%60;
 	}
 	if (min > 59) {
-		hr++;
-		min -= 60;
+		hr += min/60;
+		min = min%60;
 	}
 	if (hr > 23) {
-		day++;
-		hr -= 24;
+		day += hr/24;
+		hr = hr%24;
 	}
 	if (day > 31) {
-		mo++;
-		day -= 31;
+		mo += day/32;
+		day = day%32;
 	}
 	if (mo > 12) {
-		yr++;
-		mo -= 12;
+		yr += mo/13;
+		mo = mo%13;
 	}
 
 	spin_lock_irq(&rtc_lock);

[PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID

[Yi Yang]

Subject: ACPI: Correct wakeup set error and append a new column PCI ID
From: Yi Yang <yi.y.yang@intel.com>

The user can't get any information when echo an invalid value to
/proc/acpi/wakeup although it is failed, but the user can set
/proc/acpi/wakeup successfully if echo an value whose prefix is a
valid value for /proc/acpi/wakeup no matter what the suffix is.

/proc/acpi/wakeup is also case-sensitive, case-insensitive is better.

This patch will fix the aforementioned issues, it will return the error
information if input is an invalid value, it also make /proc/acpi/wakeup
case-insensitive, i.e. it consider 'SLPB' and 'sLpb'are the same.

In addtion, this patch appends a new column 'PCI ID' to /proc/acpi/wakeup
, the user can use it to get the corresponding device name very
conveniently because PCI ID is a unique identifier and platform-independent.

Before applying this patch, the output is:

[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node
C093      S5     disabled  pci:0000:00:1e.0
C0E8      S3     disabled  pci:0000:00:1d.0
C0EF      S3     disabled  pci:0000:00:1d.1
C0F0      S3     disabled  pci:0000:00:1d.2
C0F1      S3     disabled  pci:0000:00:1d.3
C0F2      S3     disabled  pci:0000:00:1d.7
C0F9      S0     disabled  pci:0000:00:1c.0
C21D      S0     disabled  pci:0000:08:00.0
C109      S5     disabled  pci:0000:00:1c.1
C228      S5     disabled  pci:0000:10:00.0
C10F      S5     disabled  pci:0000:00:1c.3
C229      S5     disabled
[root@localhost ~]# echo "xyzw" > /proc/acpi/wakeup
[root@localhost ~]# echo "C093xxxxxxxxx" > /proc/acpi/wakeup
[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node
C093      S5     enabled   pci:0000:00:1e.0
C0E8      S3     disabled  pci:0000:00:1d.0
C0EF      S3     disabled  pci:0000:00:1d.1
C0F0      S3     disabled  pci:0000:00:1d.2
C0F1      S3     disabled  pci:0000:00:1d.3
C0F2      S3     disabled  pci:0000:00:1d.7
C0F9      S0     disabled  pci:0000:00:1c.0
C21D      S0     disabled  pci:0000:08:00.0
C109      S5     disabled  pci:0000:00:1c.1
C228      S5     disabled  pci:0000:10:00.0
C10F      S5     disabled  pci:0000:00:1c.3
C229      S5     disabled
[root@localhost ~]# echo "C093xxxxxxxxx" > /proc/acpi/wakeup
[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node
C093      S5     disabled  pci:0000:00:1e.0
C0E8      S3     disabled  pci:0000:00:1d.0
C0EF      S3     disabled  pci:0000:00:1d.1
C0F0      S3     disabled  pci:0000:00:1d.2
C0F1      S3     disabled  pci:0000:00:1d.3
C0F2      S3     disabled  pci:0000:00:1d.7
C0F9      S0     disabled  pci:0000:00:1c.0
C21D      S0     disabled  pci:0000:08:00.0
C109      S5     disabled  pci:0000:00:1c.1
C228      S5     disabled  pci:0000:10:00.0
C10F      S5     disabled  pci:0000:00:1c.3
C229      S5     disabled
[root@localhost ~]# echo "c093" > /proc/acpi/wakeup
[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node
C093      S5     disabled  pci:0000:00:1e.0
C0E8      S3     disabled  pci:0000:00:1d.0
C0EF      S3     disabled  pci:0000:00:1d.1
C0F0      S3     disabled  pci:0000:00:1d.2
C0F1      S3     disabled  pci:0000:00:1d.3
C0F2      S3     disabled  pci:0000:00:1d.7
C0F9      S0     disabled  pci:0000:00:1c.0
C21D      S0     disabled  pci:0000:08:00.0
C109      S5     disabled  pci:0000:00:1c.1
C228      S5     disabled  pci:0000:10:00.0
C10F      S5     disabled  pci:0000:00:1c.3
C229      S5     disabled
[root@localhost ~]#

After applying this patch, the output is:

[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node           PCI ID
C093      S5     disabled  pci:0000:00:1e.0     0x2448
C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
C21D      S0     disabled  pci:0000:08:00.0     0x16fd
C109      S5     disabled  pci:0000:00:1c.1     0x27d2
C228      S5     disabled  pci:0000:10:00.0     0x4222
C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
C229      S5     disabled
[root@localhost ~]# echo "xyzw" > /proc/acpi/wakeup
-bash: echo: write error: Invalid argument
[root@localhost ~]# echo "C093xxxxxxxxx" > /proc/acpi/wakeup
-bash: echo: write error: Invalid argument
[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node           PCI ID
C093      S5     disabled  pci:0000:00:1e.0     0x2448
C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
C21D      S0     disabled  pci:0000:08:00.0     0x16fd
C109      S5     disabled  pci:0000:00:1c.1     0x27d2
C228      S5     disabled  pci:0000:10:00.0     0x4222
C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
C229      S5     disabled
[root@localhost ~]# echo "c093" > /proc/acpi/wakeup
[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node           PCI ID
C093      S5     enabled   pci:0000:00:1e.0     0x2448
C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
C21D      S0     disabled  pci:0000:08:00.0     0x16fd
C109      S5     disabled  pci:0000:00:1c.1     0x27d2
C228      S5     disabled  pci:0000:10:00.0     0x4222
C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
C229      S5     disabled
[root@localhost ~]# echo "C093" > /proc/acpi/wakeup
[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node           PCI ID
C093      S5     disabled  pci:0000:00:1e.0     0x2448
C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
C21D      S0     disabled  pci:0000:08:00.0     0x16fd
C109      S5     disabled  pci:0000:00:1c.1     0x27d2
C228      S5     disabled  pci:0000:10:00.0     0x4222
C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
C229      S5     disabled
[root@localhost ~]#

Signed-off-by: Yi Yang <yi.y.yang@intel.com>
---
 proc.c |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c
index f8df521..a19b6bd 100644
--- a/drivers/acpi/sleep/proc.c
+++ b/drivers/acpi/sleep/proc.c
@@ -3,6 +3,7 @@
 #include <linux/suspend.h>
 #include <linux/bcd.h>
 #include <asm/uaccess.h>
+#include <linux/pci.h>
 
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
@@ -343,7 +344,7 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
 {
 	struct list_head *node, *next;
 
-	seq_printf(seq, "Device\tS-state\t  Status   Sysfs node\n");
+	seq_printf(seq, "Device\tS-state\t  Status   Sysfs node\t\tPCI ID\n");
 
 	spin_lock(&acpi_device_lock);
 	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
@@ -362,9 +363,9 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
 			   dev->wakeup.flags.run_wake ? '*' : ' ',
 			   dev->wakeup.state.enabled ? "enabled" : "disabled");
 		if (ldev)
-			seq_printf(seq, "%s:%s",
+			seq_printf(seq, "%s:%-12s\t0x%04x",
 				   ldev->bus ? ldev->bus->name : "no-bus",
-				   ldev->bus_id);
+				   ldev->bus_id, to_pci_dev(ldev)->device);
 		seq_printf(seq, "\n");
 		put_device(ldev);
 
@@ -380,18 +381,18 @@ acpi_system_write_wakeup_device(struct file *file,
 				size_t count, loff_t * ppos)
 {
 	struct list_head *node, *next;
-	char strbuf[5];
-	char str[5] = "";
+	char str[6] = "";
 	int len = count;
 	struct acpi_device *found_dev = NULL;
 
-	if (len > 4)
-		len = 4;
+	if (len > 5)
+		return -EINVAL;
 
-	if (copy_from_user(strbuf, buffer, len))
+	if (copy_from_user(str, buffer, len))
 		return -EFAULT;
-	strbuf[len] = '\0';
-	sscanf(strbuf, "%s", str);
+	str[len] = '\0';
+	if (str[len-1] == '\n')
+		str[len-1] = '\0';
 
 	spin_lock(&acpi_device_lock);
 	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
@@ -400,7 +401,7 @@ acpi_system_write_wakeup_device(struct file *file,
 		if (!dev->wakeup.flags.valid)
 			continue;
 
-		if (!strncmp(dev->pnp.bus_id, str, 4)) {
+		if (!strnicmp(dev->pnp.bus_id, str, 4)) {
 			dev->wakeup.state.enabled =
 			    dev->wakeup.state.enabled ? 0 : 1;
 			found_dev = dev;
@@ -429,7 +430,10 @@ acpi_system_write_wakeup_device(struct file *file,
 		}
 	}
 	spin_unlock(&acpi_device_lock);
-	return count;
+	if (found_dev == NULL)
+		return -EINVAL;
+	else
+		return count;
 }
 
 static int

Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID

[Pavel Machek]

Hi!

> /proc/acpi/wakeup is also case-sensitive, case-insensitive is better.

Why?

> In addtion, this patch appends a new column 'PCI ID' to /proc/acpi/wakeup
> , the user can use it to get the corresponding device name very
> conveniently because PCI ID is a unique identifier and platform-independent.

Userland interface change...?

Maybe this file should be left for compatibility and we should present
something reasonable in /sys? Can't you already get PCI ID from sysfs
node?
								Pavel

> [root@localhost ~]# cat /proc/acpi/wakeup
> Device  S-state   Status   Sysfs node
> C093      S5     disabled  pci:0000:00:1e.0
> C0E8      S3     disabled  pci:0000:00:1d.0
> C0EF      S3     disabled  pci:0000:00:1d.1
> C0F0      S3     disabled  pci:0000:00:1d.2
> C0F1      S3     disabled  pci:0000:00:1d.3
> C0F2      S3     disabled  pci:0000:00:1d.7
> C0F9      S0     disabled  pci:0000:00:1c.0
> C21D      S0     disabled  pci:0000:08:00.0
> C109      S5     disabled  pci:0000:00:1c.1
> C228      S5     disabled  pci:0000:10:00.0
> C10F      S5     disabled  pci:0000:00:1c.3
> C229      S5     disabled
> [root@localhost ~]#
> 
> After applying this patch, the output is:
> 
> [root@localhost ~]# cat /proc/acpi/wakeup
> Device  S-state   Status   Sysfs node           PCI ID
> C093      S5     disabled  pci:0000:00:1e.0     0x2448
> C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
> C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
> C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
> C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
> C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
> C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
> C21D      S0     disabled  pci:0000:08:00.0     0x16fd
> C109      S5     disabled  pci:0000:00:1c.1     0x27d2
> C228      S5     disabled  pci:0000:10:00.0     0x4222
> C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
> C229      S5     disabled

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID

[Yi Yang]

On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote:
> Hi!
> 
> > /proc/acpi/wakeup is also case-sensitive, case-insensitive is better.
> 
> Why?
A user uses device bus id like 'C093' to enable or disable wakeup of the
device, for example

echo "C093" > /proc/acpi/wakeup

but i think "c093" should also be ok. i.e.

"echo 'c093' > /proc/acpi/wakeup" should have the same result as "echo
'C093' > /proc/acpi/wakeup".

That is to say, it should be case-insensitive.
> 
> > In addtion, this patch appends a new column 'PCI ID' to /proc/acpi/wakeup
> > , the user can use it to get the corresponding device name very
> > conveniently because PCI ID is a unique identifier and platform-independent.
> 
> Userland interface change...?
Not at all, i didn't find any userland application
assumes /proc/acpi/wakeup must be that kind of format.

In fact, /proc output is always changing. :-)
> 
> Maybe this file should be left for compatibility and we should present
> something reasonable in /sys? Can't you already get PCI ID from sysfs
> node?
PCI ID can be gotten from sysfs, but it is a unique identifier for a
device, a user can get device name from /usr/share/hwdata/pci.ids in any
dstribution by PCI ID, he/she is unnecessary to use bus number to get
device name, bus number is platform-specific, but PCI ID is
platform-independent.

> 								Pavel
> 
> > [root@localhost ~]# cat /proc/acpi/wakeup
> > Device  S-state   Status   Sysfs node
> > C093      S5     disabled  pci:0000:00:1e.0
> > C0E8      S3     disabled  pci:0000:00:1d.0
> > C0EF      S3     disabled  pci:0000:00:1d.1
> > C0F0      S3     disabled  pci:0000:00:1d.2
> > C0F1      S3     disabled  pci:0000:00:1d.3
> > C0F2      S3     disabled  pci:0000:00:1d.7
> > C0F9      S0     disabled  pci:0000:00:1c.0
> > C21D      S0     disabled  pci:0000:08:00.0
> > C109      S5     disabled  pci:0000:00:1c.1
> > C228      S5     disabled  pci:0000:10:00.0
> > C10F      S5     disabled  pci:0000:00:1c.3
> > C229      S5     disabled
> > [root@localhost ~]#
> > 
> > After applying this patch, the output is:
> > 
> > [root@localhost ~]# cat /proc/acpi/wakeup
> > Device  S-state   Status   Sysfs node           PCI ID
> > C093      S5     disabled  pci:0000:00:1e.0     0x2448
> > C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
> > C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
> > C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
> > C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
> > C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
> > C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
> > C21D      S0     disabled  pci:0000:08:00.0     0x16fd
> > C109      S5     disabled  pci:0000:00:1c.1     0x27d2
> > C228      S5     disabled  pci:0000:10:00.0     0x4222
> > C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
> > C229      S5     disabled
> 

Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID

[Pavel Machek]

On Wed 2008-01-02 10:03:59, Yi Yang wrote:
> On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > /proc/acpi/wakeup is also case-sensitive, case-insensitive is better.
> > 
> > Why?
> A user uses device bus id like 'C093' to enable or disable wakeup of the
> device, for example
> 
> echo "C093" > /proc/acpi/wakeup
> 
> but i think "c093" should also be ok. i.e.

Why do you think so? Unix is generally case-sensitive. I see ascii
text in .../wakeup. Maybe some bios vendor is crazy enough to have
wakeup devices called 'wake', 'Wake', 'wAke', 'waKe', 'wakE'?

> > Maybe this file should be left for compatibility and we should present
> > something reasonable in /sys? Can't you already get PCI ID from sysfs
> > node?
> PCI ID can be gotten from sysfs, but it is a unique identifier for a
> device, a user can get device name from /usr/share/hwdata/pci.ids in any
> dstribution by PCI ID, he/she is unnecessary to use bus number to get
> device name, bus number is platform-specific, but PCI ID is
> platform-independent.

If the same info can be gotten from 'sysfs node' field, new field
should not be added.


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID

[Yi Yang]

On Wed, 2008-01-02 at 17:09 +0100, Pavel Machek wrote:
> On Wed 2008-01-02 10:03:59, Yi Yang wrote:
> > On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > /proc/acpi/wakeup is also case-sensitive, case-insensitive is better.
> > > 
> > > Why?
> > A user uses device bus id like 'C093' to enable or disable wakeup of the
> > device, for example
> > 
> > echo "C093" > /proc/acpi/wakeup
> > 
> > but i think "c093" should also be ok. i.e.
> 
> Why do you think so? Unix is generally case-sensitive. I see ascii
> text in .../wakeup. Maybe some bios vendor is crazy enough to have
> wakeup devices called 'wake', 'Wake', 'wAke', 'waKe', 'wakE'?
This is just for users' convenience, i believe you must think 0xff and
0xFF are the same.

> 
> > > Maybe this file should be left for compatibility and we should present
> > > something reasonable in /sys? Can't you already get PCI ID from sysfs
> > > node?
> > PCI ID can be gotten from sysfs, but it is a unique identifier for a
> > device, a user can get device name from /usr/share/hwdata/pci.ids in any
> > dstribution by PCI ID, he/she is unnecessary to use bus number to get
> > device name, bus number is platform-specific, but PCI ID is
> > platform-independent.
> 
> If the same info can be gotten from 'sysfs node' field, new field
> should not be added.
Assume you are a user of /proc/acpi/wakeup, when you
cat /proc/acpi/wakeup, you only get PCI bus id, then you need use PCI
bus id to get the device info, that is platform-specific, if you want to
use this PCI bus id to get the device info from another machines, that
is absolutely impossible, but it is ok if it is PCI ID.

Moreover, you can very easily get the device info
from /usr/share/hwdata/pci.ids.

grep <PCI ID> /usr/share/hwdata/pci.ids

That is more convenient than PCI bus id.

If we can provide PCI ID in /proc/acpi/wakeup, why we let users get that
from /sys/devices/pci...?

> 
> 

Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID

[Yi Yang]

On Wed, 2008-01-02 at 17:09 +0100, Pavel Machek wrote:
> On Wed 2008-01-02 10:03:59, Yi Yang wrote:
> > On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > /proc/acpi/wakeup is also case-sensitive, case-insensitive is better.
> > > 
> > > Why?
> > A user uses device bus id like 'C093' to enable or disable wakeup of the
> > device, for example
> > 
> > echo "C093" > /proc/acpi/wakeup
> > 
> > but i think "c093" should also be ok. i.e.
> 
> Why do you think so? Unix is generally case-sensitive. I see ascii
> text in .../wakeup. Maybe some bios vendor is crazy enough to have
> wakeup devices called 'wake', 'Wake', 'wAke', 'waKe', 'wakE'?
Of course, when you cat/proc/acpi/wakeup, you get "wake", but when you
want to enable or disable wakeup of the device "wake", you can

echo "wAke" > /proc/acpi/wakeup

or

echo "wake" > /proc/acpi/wakeup

Don't you think it is reasonable? This is just for user's convenience.

> 
> > > Maybe this file should be left for compatibility and we should present
> > > something reasonable in /sys? Can't you already get PCI ID from sysfs
> > > node?
> > PCI ID can be gotten from sysfs, but it is a unique identifier for a
> > device, a user can get device name from /usr/share/hwdata/pci.ids in any
> > dstribution by PCI ID, he/she is unnecessary to use bus number to get
> > device name, bus number is platform-specific, but PCI ID is
> > platform-independent.
> 
> If the same info can be gotten from 'sysfs node' field, new field
> should not be added.
> 
> 

[PATCH linux-acpi] fix acpi fan state set error

[Yi Yang]

Subject: ACPI: fix acpi fan state set error
From: Yi Yang <yi.y.yang@intel.com>

Under /proc/acpi, there is a fan control interface, a user can
set 0 or 3 to /proc/acpi/fan/*/state, 0 denotes D0 state, 3
denotes D3 state, but in current implementation, a user can
set a fan to D1 state by any char excluding '1', '2' and '3'.

For example:

[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost acpi]# echo "" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  on
[root@localhost acpi]# echo "3" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost acpi]# echo "xxxxx" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  on

Obviously, such inputs as "" and "xxxxx" are invalid for fan state.

This patch fixes this issue, it strictly limits fan state only to
accept 0, 1, 2 and 3, any other inputs are invalid.

Before applying this patch, the test result is:

[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost acpi]# echo "" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  on
[root@localhost acpi]# echo "3" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost acpi]# echo "xxxxx" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  on
[root@localhost acpi]# echo "3" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost acpi]# echo "3x" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost acpi]# echo "-1x" > /proc/acpi/fan/C31B/state
[root@localhost acpi]# cat /proc/acpi/fan/C31B/state
status:                  on
[root@localhost acpi]#


After applying this patch, the test result is:

[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost ~]# echo "" > /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost ~]# echo "3" > /proc/acpi/fan/C31B/state
[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost ~]# echo "xxxxx" > /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost ~]# echo "-1x" > /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost ~]# echo "0" > //proc/acpi/fan/C31B/state
[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  on
[root@localhost ~]# echo "4" > //proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  on
[root@localhost ~]# echo "3" > //proc/acpi/fan/C31B/state
[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  off
[root@localhost ~]# echo "0" > //proc/acpi/fan/C31B/state
[root@localhost ~]# cat /proc/acpi/fan/C31B/state
status:                  on
[root@localhost ~]# echo "3x" > //proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[root@localhost ~]#


Signed-off-by: Yi Yang <yi.y.yang@intel.com>
---
 fan.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index a5a5532..53f7c72 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -98,7 +98,7 @@ acpi_fan_write_state(struct file *file, const char __user * buffer,
 	int result = 0;
 	struct seq_file *m = file->private_data;
 	struct acpi_device *device = m->private;
-	char state_string[12] = { '\0' };
+	char state_string[3] = { '\0' };
 
 	if (count > sizeof(state_string) - 1)
 		return -EINVAL;
@@ -107,6 +107,12 @@ acpi_fan_write_state(struct file *file, const char __user * buffer,
 		return -EFAULT;
 
 	state_string[count] = '\0';
+	if ((state_string[0] < '0') || (state_string[0] > '3'))
+		return -EINVAL;
+	if (state_string[1] == '\n')
+		state_string[1] = '\0';
+	if (state_string[1] != '\0')
+		return -EINVAL;
 
 	result = acpi_bus_set_power(device->handle,
 				    simple_strtoul(state_string, NULL, 0));

[PATCH] ACPI: fix processor throttling set error

[Yi Yang]

Subject: ACPI: fix processor throttling set error
From: Yi Yang <yi.y.yang@intel.com>

When echo some invalid values to /proc/acpi/processor/*/throttling,
there isn't any error info returned, on the contray, it sets
throttling value to some T* successfully, obviously, this is incorrect,
a correct way should be to let it fail and return error info.

This patch fixed the aforementioned issue, it also enables
/proc/acpi/processor/*/throttling to accept such values as 't0' and 'T0',
it also strictly limits /proc/acpi/processor/*/throttling only to accept
 "*", "t*" and "T*", "*" is the throttling state value the processor can
support, current, it is 0 - 7.

Before applying this patch, the test result is below:

[root@localhost acpi]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T1
state available: T0 to T7
states:
    T0:                  100%
   *T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost acpi]# echo "1xxxxxx" > /proc/acpi/processor/CPU0/throttling
[root@localhost acpi]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T1
state available: T0 to T7
states:
    T0:                  100%
   *T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost acpi]# echo "0" > /proc/acpi/processor/CPU0/throttling
[root@localhost acpi]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T0
state available: T0 to T7
states:
   *T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost acpi]# cd /
[root@localhost /]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T0
state available: T0 to T7
states:
   *T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost /]# echo "T0" > /proc/acpi/processor/CPU0/throttling
[root@localhost /]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T0
state available: T0 to T7
states:
   *T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost /]# echo "T7" > /proc/acpi/processor/CPU0/throttling
[root@localhost /]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T0
state available: T0 to T7
states:
   *T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost /]# echo "T100" > /proc/acpi/processor/CPU0/throttling
[root@localhost /]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T0
state available: T0 to T7
states:
   *T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost /]# echo "xxx" > /proc/acpi/processor/CPU0/throttling
[root@localhost /]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T0
state available: T0 to T7
states:
   *T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost /]# echo "2xxxx" > /proc/acpi/processor/CPU0/throttling
[root@localhost /]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T2
state available: T0 to T7
states:
    T0:                  100%
    T1:                  87%
   *T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost /]# echo "" > /proc/acpi/processor/CPU0/throttling
[root@localhost /]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T0
state available: T0 to T7
states:
   *T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost /]# echo "7777" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost /]# echo "7xxx" > /proc/acpi/processor/CPU0/throttling
[root@localhost /]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T7
state available: T0 to T7
states:
    T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
   *T7:                  12%
[root@localhost /]#


After applying this patch, the test result is below:

[root@localhost linux-2.6.24-rc6]# echo > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo "" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo "0" > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# echo "t0" > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# echo "T0" > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T0
state available: T0 to T7
states:
   *T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
    T7:                  12%
[root@localhost linux-2.6.24-rc6]# echo "T7" > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T7
state available: T0 to T7
states:
    T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
   *T7:                  12%
[root@localhost linux-2.6.24-rc6]# echo "T8" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# vi drivers/acpi/processor_throttling.c
[root@localhost linux-2.6.24-rc6]# echo "T8" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo "t7" > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# echo "t70" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo "70" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo "7000" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo "70" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo "xxx" > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo -n > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# echo -n "" > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# echo $?
0
[root@localhost linux-2.6.24-rc6]# echo -n "" > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T7
state available: T0 to T7
states:
    T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
   *T7:                  12%
[root@localhost linux-2.6.24-rc6]# echo -n "" > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# cat /proc/acpi/processor/CPU0/throttling
state count:             8
active state:            T7
state available: T0 to T7
states:
    T0:                  100%
    T1:                  87%
    T2:                  75%
    T3:                  62%
    T4:                  50%
    T5:                  37%
    T6:                  25%
   *T7:                  12%
[root@localhost linux-2.6.24-rc6]# echo t0 > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# echo T0 > /proc/acpi/processor/CPU0/throttling
[root@localhost linux-2.6.24-rc6]# echo Tt0 > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]# echo T > /proc/acpi/processor/CPU0/throttling
-bash: echo: write error: Invalid argument
[root@localhost linux-2.6.24-rc6]#

Signed-off-by: Yi Yang <yi.y.yang@intel.com>
---
 processor_throttling.c |   24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/processor_throttling.c b/drivers/acpi/processor_throttling.c
index 6742d7b..10035a6 100644
--- a/drivers/acpi/processor_throttling.c
+++ b/drivers/acpi/processor_throttling.c
@@ -898,7 +898,10 @@ static ssize_t acpi_processor_write_throttling(struct file *file,
 	int result = 0;
 	struct seq_file *m = file->private_data;
 	struct acpi_processor *pr = m->private;
-	char state_string[12] = { '\0' };
+	char state_string[5] = "";
+	char *charp = NULL;
+	size_t state_val = 0;
+	char tmpbuf[5] = "";
 
 	if (!pr || (count > sizeof(state_string) - 1))
 		return -EINVAL;
@@ -907,10 +910,23 @@ static ssize_t acpi_processor_write_throttling(struct file *file,
 		return -EFAULT;
 
 	state_string[count] = '\0';
+	if ((count > 0) && (state_string[count-1] == '\n'))
+		state_string[count-1] = '\0';
 
-	result = acpi_processor_set_throttling(pr,
-					       simple_strtoul(state_string,
-							      NULL, 0));
+	charp = state_string;
+	if ((state_string[0] == 't') || (state_string[0] == 'T'))
+		charp++;
+
+	state_val = simple_strtoul(charp, NULL, 0);
+	if (state_val >= pr->throttling.state_count)
+		return -EINVAL;
+
+	snprintf(tmpbuf, 5, "%d", state_val);
+
+	if (strcmp(tmpbuf, charp) != 0)
+		return -EINVAL;
+
+	result = acpi_processor_set_throttling(pr, state_val);
 	if (result)
 		return result;
 

[PATCH] ACPI: fix processor limit set error

[Yi Yang]

Subject: ACPI: fix processor limit set error
From: Yi Yang <yi.y.yang@intel.com>

when echo some invalid values to /proc/acpi/processor/CPU*/limit,
it doesn't return any error info, on the contrary, it successes
and sets some other values, for example:

[root@localhost /]# echo "0:0A" >/proc/acpi/processor/CPU0/limit
[root@localhost /]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T0
user limit:              P0:T0
thermal limit:           P0:T0
[root@localhost /]# echo "0:0F" >/proc/acpi/processor/CPU0/limit
[root@localhost /]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T0
user limit:              P0:T0
thermal limit:           P0:T0
[root@localhost /]# echo "0:0 0:1 0:2" >/proc/acpi/processor/CPU0/limit
[root@localhost /]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T0
user limit:              P0:T0
thermal limit:           P0:T0
[root@localhost /]#

A correct way is that it should fail and return error info.

This patch fixed this issue, it accepts not only such inputs as "*:*",
but also accepts such inputs as "p*:t*" or "P*:T*" or "p*:*" or "*:t*",
the former "*" in inputs means the allowed processor performance state,
currently it is only a stub and not set to the processor limit data
structure, the latter "*" means the allowed processor throttling state
which rages from 0 to 7 generally. This patch strictly limits inputs to
meet the above conditions, any input which can't meet the above conditions
is considered as invalid input.

Before applying this patch, the test result is below:

[root@localhost /]# echo "0:0A" >/proc/acpi/processor/CPU0/limit
[root@localhost /]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T0
user limit:              P0:T0
thermal limit:           P0:T0
[root@localhost /]# echo "0:0F" >/proc/acpi/processor/CPU0/limit
[root@localhost /]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T0
user limit:              P0:T0
thermal limit:           P0:T0
[root@localhost /]# echo "10:2F" >/proc/acpi/processor/CPU0/limit
[root@localhost /]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T2
user limit:              P0:T2
thermal limit:           P0:T0
[root@localhost /]# echo "0:0 0:1 0:2" >/proc/acpi/processor/CPU0/limit
[root@localhost /]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T0
user limit:              P0:T0
thermal limit:           P0:T0
[root@localhost /]#

After applying this patch, the test result is below:

[root@localhost ~]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T0
user limit:              P0:T0
thermal limit:           P0:T0
[root@localhost ~]# echo "0:0A" > /proc/acpi/processor/CPU0/limit
-bash: echo: write error: Invalid argument
[root@localhost ~]# echo "0:0F" > /proc/acpi/processor/CPU0/limit
-bash: echo: write error: Invalid argument
[root@localhost ~]# echo "P0:T1" > /proc/acpi/processor/CPU0/limit
[root@localhost ~]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T1
user limit:              P0:T1
thermal limit:           P0:T0
[root@localhost ~]# echo "p0:t1" > /proc/acpi/processor/CPU0/limit
[root@localhost ~]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T1
user limit:              P0:T1
thermal limit:           P0:T0
[root@localhost ~]# echo "p0:x1" > /proc/acpi/processor/CPU0/limit
-bash: echo: write error: Invalid argument
[root@localhost ~]# echo "q0:x1" > /proc/acpi/processor/CPU0/limit
-bash: echo: write error: Invalid argument
[root@localhost ~]# echo "p0:1" > /proc/acpi/processor/CPU0/limit
[root@localhost ~]# echo "q0:x1" > /proc/acpi/processor/CPU0/limit
-bash: echo: write error: Invalid argument
[root@localhost ~]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T1
user limit:              P0:T1
thermal limit:           P0:T0
[root@localhost ~]# echo "0:t1" > /proc/acpi/processor/CPU0/limit
[root@localhost ~]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T1
user limit:              P0:T1
thermal limit:           P0:T0
[root@localhost ~]# echo "0:t1F" > /proc/acpi/processor/CPU0/limit
-bash: echo: write error: Invalid argument
[root@localhost ~]# echo "" > /proc/acpi/processor/CPU0/limit
-bash: echo: write error: Invalid argument
[root@localhost ~]# echo "0:0 0:0 0:2 " > /proc/acpi/processor/CPU0/limit
-bash: echo: write error: Invalid argument
[root@localhost ~]# echo "P0:T1" > /proc/acpi/processor/CPU0/limit
[root@localhost ~]# cat /proc/acpi/processor/CPU0/limit
active limit:            P0:T1
user limit:              P0:T1
thermal limit:           P0:T0
[root@localhost ~]#


Signed-off-by: Yi Yang <yi.y.yang@intel.com>
---
 processor_thermal.c |   42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 06e6f3f..262d56e 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -349,9 +349,11 @@ static ssize_t acpi_processor_write_limit(struct file * file,
 	int result = 0;
 	struct seq_file *m = file->private_data;
 	struct acpi_processor *pr = m->private;
-	char limit_string[25] = { '\0' };
+	char limit_string[10] = "";
 	int px = 0;
 	int tx = 0;
+	char *tmpp = NULL;
+	char tmpstring[10] = "";
 
 
 	if (!pr || (count > sizeof(limit_string) - 1)) {
@@ -363,21 +365,39 @@ static ssize_t acpi_processor_write_limit(struct file * file,
 	}
 
 	limit_string[count] = '\0';
+	if ((count > 0) && (limit_string[count-1] == '\n'))
+		limit_string[count-1] = '\0';
 
-	if (sscanf(limit_string, "%d:%d", &px, &tx) != 2) {
-		printk(KERN_ERR PREFIX "Invalid data format\n");
+	tmpp = memchr(limit_string, ':', count);
+	if (tmpp == NULL)
+		return -EINVAL;
+
+	*tmpp = '\0';
+	tmpp = limit_string;
+	if ((limit_string[0] == 'p') || (limit_string[0] == 'P'))
+		tmpp++;
+	px = simple_strtoul(tmpp, NULL, 0);
+	snprintf(tmpstring, 10, "%d", px);
+	if (strcmp(tmpp, tmpstring) != 0)
+		return -EINVAL;
+
+	tmpp += strlen(tmpp) + 1;
+	if ((tmpp[0] == 't') || (tmpp[0] == 'T'))
+		tmpp++;
+	tx = simple_strtoul(tmpp, NULL, 0);
+	if (tx > (pr->throttling.state_count - 1))
+		return -EINVAL;
+	snprintf(tmpstring, 10, "%d", tx);
+	if (strcmp(tmpp, tmpstring) != 0)
 		return -EINVAL;
-	}
 
 	if (pr->flags.throttling) {
-		if ((tx < 0) || (tx > (pr->throttling.state_count - 1))) {
-			printk(KERN_ERR PREFIX "Invalid tx\n");
-			return -EINVAL;
-		}
 		pr->limit.user.tx = tx;
-	}
-
-	result = acpi_processor_apply_limit(pr);
+		result = acpi_processor_apply_limit(pr);
+		if (result != 0)
+			return result;
+	} else
+		return -ENODEV;
 
 	return count;
 }

[PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported

[Yi Yang]

Subject: ACPI: Create proc entry 'power' only C2 or C3 is supported
From: Yi Yang <yi.y.yang@intel.com>

ACPI processor idle driver makes sense only if the processor supports
C2 or C3. For legacy C0 and C1, just the original pm_idle is working
, statistics info about promotion, demotion, latency, usage and
duration are empty or 0, so these are misleading, users'll think their
CPUs support C states (C2 or C3), /proc/acpi/processor/CPU*/power
shouldn't exist for this case at all.

This patch fixes this issue, it makes ACPI processor idle driver to create
proc entry only if the processor supports C2 or C3.


Signed-off-by: Yi Yang <yi.y.yang@intel.com>
---
 processor_idle.c |   23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

--- a/drivers/acpi/processor_idle.c	2008-01-24 05:34:29.000000000 +0800
+++ b/drivers/acpi/processor_idle.c	2008-01-24 07:04:59.000000000 +0800
@@ -1738,17 +1738,17 @@ int __cpuinit acpi_processor_power_init(
 			pm_idle = acpi_processor_idle;
 		}
 #endif
-	}
 
-	/* 'power' [R] */
-	entry = create_proc_entry(ACPI_PROCESSOR_FILE_POWER,
-				  S_IRUGO, acpi_device_dir(device));
-	if (!entry)
-		return -EIO;
-	else {
-		entry->proc_fops = &acpi_processor_power_fops;
-		entry->data = acpi_driver_data(device);
-		entry->owner = THIS_MODULE;
+		/* 'power' [R] */
+		entry = create_proc_entry(ACPI_PROCESSOR_FILE_POWER,
+					  S_IRUGO, acpi_device_dir(device));
+		if (!entry)
+			return -EIO;
+		else {
+			entry->proc_fops = &acpi_processor_power_fops;
+			entry->data = acpi_driver_data(device);
+			entry->owner = THIS_MODULE;
+		}
 	}
 
 	return 0;
@@ -1763,7 +1763,8 @@ int acpi_processor_power_exit(struct acp
 #endif
 	pr->flags.power_setup_done = 0;
 
-	if (acpi_device_dir(device))
+	if (acpi_device_dir(device) &&
+		((pr->flags.power) && (!boot_option_idle_override)))
 		remove_proc_entry(ACPI_PROCESSOR_FILE_POWER,
 				  acpi_device_dir(device));
 

Re: [PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported

[Mark Lord]

> Subject: ACPI: Create proc entry 'power' only C2 or C3 is supported
> From: Yi Yang <yi.y.yang@intel.com>
> 
> ACPI processor idle driver makes sense only if the processor supports
> C2 or C3. For legacy C0 and C1, just the original pm_idle is working
> , statistics info about promotion, demotion, latency, usage and
> duration are empty or 0, so these are misleading, users'll think their
> CPUs support C states (C2 or C3), /proc/acpi/processor/CPU*/power
> shouldn't exist for this case at all.
..

On the other hand, this change might send many of us scrambling
to try and figure out which kernel CONFIG option is responsible
for the expected /proc/acpi/processor/CPU*/power entries not showing up.
Thus just adding to the confusion by as much as it saves.

What do others think?

[PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Yi Yang]

Subject: ACPI: convert procfs to sysfs for /proc/acpi/wakeup
From: Yi Yang <yi.y.yang@intel.com>

/proc/acpi/wakeup is deprecated but it has to exist because
we haven't a sysfs interface to replace it yet, this patch
converts /proc/acpi/wakeup to sysfs interface, under every
acpi device sysfs node, a user can see a directory "wakeup"
if the acpi device can support wakeup, there are six files
under this directory:

acpi_bus_id  bus_id  pci_id  run_wakeup  sleep_state  status

All the files are read-only exclude "status" which is used
to enable or disable wakeup of the acpi device.

"acpi_bus_id" is acpi bus ID of the acpi device.

"bus_id" is pci bus id of the device associated to the acpi
device, it will be empty if there isn't any device associated
to it.

"pci_id" is PCI ID of the pci device associated to the acpi
device, it will be empty if there isn't any device associated
to it.

"run_wake" is a flag indicating if a wakeup process is being
handled.

"sleep_state" is sleep state of the acpi device such as "S0".

"status" is wakeup status of the acpi device, it is enabled
or disabled, a user can change it be echoing "0", "1",
"disabled" or "enabled" to /sys/devices/.../wakeup/status. 

Here is the test result:

[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node           PCI ID
C093      S5     disabled  pci:0000:00:1e.0     0x2448
C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
C21D      S0     disabled  pci:0000:08:00.0     0x16fd
C109      S5     disabled  pci:0000:00:1c.1     0x27d2
C228      S5     disabled  pci:0000:10:00.0     0x4222
C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
C229      S5     disabled
[root@localhost ~]# find /sys -name "*" | grep sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:05/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:0d/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:11/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:15/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:1d/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/device:2c/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/device:2e/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/wakeup/sleep_state
/sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state
[root@localhost ~]# ls /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup
acpi_bus_id  bus_id  pci_id  run_wakeup  sleep_state  status
[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id
cat: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id: No such file or directory
[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/acpi_bus_id
C229
[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state
S5
[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
disabled
[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/bus_id

[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/pci_id

[root@localhost ~]# echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
enabled
[root@localhost ~]# cat /proc/acpi/wakeup
Device  S-state   Status   Sysfs node           PCI ID
C093      S5     disabled  pci:0000:00:1e.0     0x2448
C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
C0F9      S0     enabled   pci:0000:00:1c.0     0x27d0
C21D      S0     enabled   pci:0000:08:00.0     0x16fd
C109      S5     enabled   pci:0000:00:1c.1     0x27d2
C228      S5     enabled   pci:0000:10:00.0     0x4222
C10F      S5     enabled   pci:0000:00:1c.3     0x27d6
C229      S5     enabled
[root@localhost ~]# vi /var/log/dmesg
[root@localhost ~]# dmesg | grep "same GPE"
ACPI: 'C0F9' and 'C229' have the same GPE, can't disable/enable one seperately
ACPI: 'C21D' and 'C229' have the same GPE, can't disable/enable one seperately
ACPI: 'C109' and 'C229' have the same GPE, can't disable/enable one seperately
ACPI: 'C228' and 'C229' have the same GPE, can't disable/enable one seperately
ACPI: 'C10F' and 'C229' have the same GPE, can't disable/enable one seperately
[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/*/wakeup/status
disabled
disabled
disabled
disabled
disabled
disabled
enabled
enabled
enabled
[root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/*/*/wakeup/status
enabled
enabled
enabled
[root@localhost ~]#


Signed-off-by: Yi Yang <yi.y.yang@intel.com>
---
 scan.c |  184 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 180 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5b4d462..c3786a0 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -6,8 +6,10 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/acpi.h>
+#include <linux/pci.h>
 
 #include <acpi/acpi_drivers.h>
+#include <acpi/acpi_bus.h>
 #include <acpi/acinterp.h>	/* for acpi_ex_eisa_id_to_string() */
 
 #define _COMPONENT		ACPI_BUS_COMPONENT
@@ -188,8 +190,171 @@ acpi_device_path_show(struct device *dev, struct device_attribute *attr, char *b
   end:
 	return result;
 }
+
 static DEVICE_ATTR(path, 0444, acpi_device_path_show, NULL);
 
+static char enabled[] = "enabled";
+static char disabled[] = "disabled";
+static char unsupported[] = "unsupported";
+
+static ssize_t
+acpi_device_status_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+	int result = 0;
+	char *tmpp = NULL;
+
+	if (!acpi_dev->wakeup.flags.valid)
+		tmpp = unsupported;
+	else
+		tmpp = acpi_dev->wakeup.state.enabled ? enabled : disabled;
+	result = sprintf(buf, "%s\n", tmpp);
+	return result;
+}
+
+static ssize_t
+acpi_device_status_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct list_head *node, *next;
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+	size_t len = count;
+	char *tmpp = NULL;
+
+	tmpp = memchr(buf, '\n', count);
+	if (tmpp != NULL)
+		len = tmpp - buf;
+
+	spin_lock(&acpi_device_lock);
+	if ((len == 1) && ((buf[0] == '0') || (buf[0] == '1')))
+		acpi_dev->wakeup.state.enabled = buf[0] - '0';
+	else if ((len == strlen(enabled))
+		 && (strnicmp(buf, enabled, strlen(enabled)) == 0))
+		acpi_dev->wakeup.state.enabled = 1;
+	else if ((len == strlen(disabled))
+		 && (strnicmp(buf, disabled, strlen(disabled)) == 0))
+		acpi_dev->wakeup.state.enabled = 0;
+	else {
+		spin_unlock(&acpi_device_lock);
+		return -EINVAL;
+	}
+
+	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
+		struct acpi_device *dev = container_of(node,
+						       struct
+						       acpi_device,
+						       wakeup_list);
+
+		if ((dev != acpi_dev)
+		    && (dev->wakeup.gpe_number == acpi_dev->wakeup.gpe_number)
+		    && (dev->wakeup.gpe_device ==
+				acpi_dev->wakeup.gpe_device)) {
+			printk(KERN_WARNING
+			       "ACPI: '%s' and '%s' have the same GPE, "
+			       "can't disable/enable one seperately\n",
+			       dev->pnp.bus_id, acpi_dev->pnp.bus_id);
+			dev->wakeup.state.enabled =
+					acpi_dev->wakeup.state.enabled;
+		}
+	}
+	spin_unlock(&acpi_device_lock);
+
+	return count;
+}
+
+static DEVICE_ATTR(status, 0644, acpi_device_status_show,
+			acpi_device_status_store);
+
+static ssize_t
+acpi_device_acpi_bus_id_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+
+	return sprintf(buf, "%s\n", acpi_dev->pnp.bus_id);
+}
+
+static DEVICE_ATTR(acpi_bus_id, 0444, acpi_device_acpi_bus_id_show, NULL);
+
+
+static ssize_t
+acpi_device_sleep_state_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+
+	return sprintf(buf, "S%d\n", (u32) acpi_dev->wakeup.sleep_state);
+}
+
+static DEVICE_ATTR(sleep_state, 0444, acpi_device_sleep_state_show, NULL);
+
+static ssize_t
+acpi_device_run_wakeup_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+
+	return sprintf(buf, "%d\n", acpi_dev->wakeup.flags.run_wake ? 1 : 0);
+}
+
+static DEVICE_ATTR(run_wakeup, 0444, acpi_device_run_wakeup_show, NULL);
+
+static ssize_t
+acpi_device_bus_id_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+	struct device *ldev = NULL;
+	int result = 0;
+
+	ldev = acpi_get_physical_device(acpi_dev->handle);
+	if (ldev)
+		result = sprintf(buf, "%s:%s\n",
+				 ldev->bus ? ldev->bus->name : "no-bus",
+				 ldev->bus_id);
+	else
+		result = sprintf(buf, "\n");
+
+	return result;
+}
+
+static DEVICE_ATTR(bus_id, 0444, acpi_device_bus_id_show, NULL);
+
+static ssize_t
+acpi_device_pci_id_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+	struct device *ldev = NULL;
+	int result = 0;
+
+	ldev = acpi_get_physical_device(acpi_dev->handle);
+	if (ldev)
+		result = sprintf(buf, "0x%04x\n", to_pci_dev(ldev)->device);
+	else
+		result = sprintf(buf, "\n");
+
+	return result;
+}
+
+static DEVICE_ATTR(pci_id, 0444, acpi_device_pci_id_show, NULL);
+
+static struct attribute *acpi_wakeup_attrs[] = {
+	&dev_attr_acpi_bus_id.attr,
+	&dev_attr_sleep_state.attr,
+	&dev_attr_status.attr,
+	&dev_attr_run_wakeup.attr,
+	&dev_attr_bus_id.attr,
+	&dev_attr_pci_id.attr,
+	NULL,
+};
+
+static struct attribute_group acpi_wakeup_attr_group = {
+	.name   = "wakeup",
+	.attrs  = acpi_wakeup_attrs,
+};
+
 static int acpi_device_setup_files(struct acpi_device *dev)
 {
 	acpi_status status;
@@ -201,19 +366,19 @@ static int acpi_device_setup_files(struct acpi_device *dev)
 	 */
 	if(dev->handle) {
 		result = device_create_file(&dev->dev, &dev_attr_path);
-		if(result)
+		if (result)
 			goto end;
 	}
 
 	if(dev->flags.hardware_id) {
 		result = device_create_file(&dev->dev, &dev_attr_hid);
-		if(result)
+		if (result)
 			goto end;
 	}
 
 	if (dev->flags.hardware_id || dev->flags.compatible_ids){
 		result = device_create_file(&dev->dev, &dev_attr_modalias);
-		if(result)
+		if (result)
 			goto end;
 	}
 
@@ -222,8 +387,16 @@ static int acpi_device_setup_files(struct acpi_device *dev)
          * hot-removal function from userland.
          */
 	status = acpi_get_handle(dev->handle, "_EJ0", &temp);
-	if (ACPI_SUCCESS(status))
+	if (ACPI_SUCCESS(status)) {
 		result = device_create_file(&dev->dev, &dev_attr_eject);
+		if (result)
+			goto end;
+	}
+
+	if (dev->wakeup.flags.valid)
+		result = sysfs_create_group(&dev->dev.kobj,
+				&acpi_wakeup_attr_group);
+
   end:
 	return result;
 }
@@ -248,6 +421,9 @@ static void acpi_device_remove_files(struct acpi_device *dev)
 		device_remove_file(&dev->dev, &dev_attr_hid);
 	if(dev->handle)
 		device_remove_file(&dev->dev, &dev_attr_path);
+
+	if (dev->wakeup.flags.valid)
+		sysfs_remove_group(&dev->dev.kobj, &acpi_wakeup_attr_group);
 }
 /* --------------------------------------------------------------------------
 			ACPI Bus operations

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Maxim Levitsky]

On Thursday, 10 January 2008 00:21:46 Yi Yang wrote:
> Subject: ACPI: convert procfs to sysfs for /proc/acpi/wakeup
> From: Yi Yang <yi.y.yang@intel.com>
> 
> /proc/acpi/wakeup is deprecated but it has to exist because
> we haven't a sysfs interface to replace it yet, this patch
> converts /proc/acpi/wakeup to sysfs interface, under every
> acpi device sysfs node, a user can see a directory "wakeup"
> if the acpi device can support wakeup, there are six files
> under this directory:
> 
> acpi_bus_id  bus_id  pci_id  run_wakeup  sleep_state  status
> 
> All the files are read-only exclude "status" which is used
> to enable or disable wakeup of the acpi device.
> 
> "acpi_bus_id" is acpi bus ID of the acpi device.
> 
> "bus_id" is pci bus id of the device associated to the acpi
> device, it will be empty if there isn't any device associated
> to it.
> 
> "pci_id" is PCI ID of the pci device associated to the acpi
> device, it will be empty if there isn't any device associated
> to it.
> 
> "run_wake" is a flag indicating if a wakeup process is being
> handled.
> 
> "sleep_state" is sleep state of the acpi device such as "S0".
> 
> "status" is wakeup status of the acpi device, it is enabled
> or disabled, a user can change it be echoing "0", "1",
> "disabled" or "enabled" to /sys/devices/.../wakeup/status. 
> 
> Here is the test result:
> 
> [root@localhost ~]# cat /proc/acpi/wakeup
> Device  S-state   Status   Sysfs node           PCI ID
> C093      S5     disabled  pci:0000:00:1e.0     0x2448
> C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
> C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
> C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
> C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
> C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
> C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
> C21D      S0     disabled  pci:0000:08:00.0     0x16fd
> C109      S5     disabled  pci:0000:00:1c.1     0x27d2
> C228      S5     disabled  pci:0000:10:00.0     0x4222
> C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
> C229      S5     disabled
> [root@localhost ~]# find /sys -name "*" | grep sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:05/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:0d/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:11/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:15/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:1d/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/device:2c/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/device:2e/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/wakeup/sleep_state
> /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state
> [root@localhost ~]# ls /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup
> acpi_bus_id  bus_id  pci_id  run_wakeup  sleep_state  status
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id
> cat: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id: No such file or directory
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/acpi_bus_id
> C229
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state
> S5
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> disabled
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/bus_id
> 
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/pci_id
> 
> [root@localhost ~]# echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> enabled
> [root@localhost ~]# cat /proc/acpi/wakeup
> Device  S-state   Status   Sysfs node           PCI ID
> C093      S5     disabled  pci:0000:00:1e.0     0x2448
> C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
> C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
> C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
> C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
> C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
> C0F9      S0     enabled   pci:0000:00:1c.0     0x27d0
> C21D      S0     enabled   pci:0000:08:00.0     0x16fd
> C109      S5     enabled   pci:0000:00:1c.1     0x27d2
> C228      S5     enabled   pci:0000:10:00.0     0x4222
> C10F      S5     enabled   pci:0000:00:1c.3     0x27d6
> C229      S5     enabled
> [root@localhost ~]# vi /var/log/dmesg
> [root@localhost ~]# dmesg | grep "same GPE"
> ACPI: 'C0F9' and 'C229' have the same GPE, can't disable/enable one seperately
> ACPI: 'C21D' and 'C229' have the same GPE, can't disable/enable one seperately
> ACPI: 'C109' and 'C229' have the same GPE, can't disable/enable one seperately
> ACPI: 'C228' and 'C229' have the same GPE, can't disable/enable one seperately
> ACPI: 'C10F' and 'C229' have the same GPE, can't disable/enable one seperately
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/*/wakeup/status
> disabled
> disabled
> disabled
> disabled
> disabled
> disabled
> enabled
> enabled
> enabled
> [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/*/*/wakeup/status
> enabled
> enabled
> enabled
> [root@localhost ~]#


I think that it would be much much better to place wake-up attributes under
corresponding PCI and PNP devices.

Probably it is even better to link this code to PCI code, so PCI drivers will be aware of ACPI.
For example it will fix the 'EHCI instantly wakes up the system from S4' on my system, since here USB doesn't wake
up anything from S4, and ACPI tables correctly show that.

If ehci driver was aware of that it could disable #PME on entrance to S4.
And we even can reuse its 'wakeup' attribute, thus enabling wakeup automatically.

Going ever further, I think that it will be great to get rid of ACPI device tree, since
most acpi devices are ether PCI of PNP ones.

Or, even better have a small ACPI tree, that will contain 'true' ACPI devices, like cpus
thermal sensors, buttons, etc. 

Best regards,
	Maxim Levitsky

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Yi Yang]

On Thu, 2008-01-10 at 09:43 +0200, Maxim Levitsky wrote:
> On Thursday, 10 January 2008 00:21:46 Yi Yang wrote:
> > Subject: ACPI: convert procfs to sysfs for /proc/acpi/wakeup
> > From: Yi Yang <yi.y.yang@intel.com>
> > 
> > /proc/acpi/wakeup is deprecated but it has to exist because
> > we haven't a sysfs interface to replace it yet, this patch
> > converts /proc/acpi/wakeup to sysfs interface, under every
> > acpi device sysfs node, a user can see a directory "wakeup"
> > if the acpi device can support wakeup, there are six files
> > under this directory:
> > 
> > acpi_bus_id  bus_id  pci_id  run_wakeup  sleep_state  status
> > 
> > All the files are read-only exclude "status" which is used
> > to enable or disable wakeup of the acpi device.
> > 
> > "acpi_bus_id" is acpi bus ID of the acpi device.
> > 
> > "bus_id" is pci bus id of the device associated to the acpi
> > device, it will be empty if there isn't any device associated
> > to it.
> > 
> > "pci_id" is PCI ID of the pci device associated to the acpi
> > device, it will be empty if there isn't any device associated
> > to it.
> > 
> > "run_wake" is a flag indicating if a wakeup process is being
> > handled.
> > 
> > "sleep_state" is sleep state of the acpi device such as "S0".
> > 
> > "status" is wakeup status of the acpi device, it is enabled
> > or disabled, a user can change it be echoing "0", "1",
> > "disabled" or "enabled" to /sys/devices/.../wakeup/status. 
> > 
> > Here is the test result:
> > 
> > [root@localhost ~]# cat /proc/acpi/wakeup
> > Device  S-state   Status   Sysfs node           PCI ID
> > C093      S5     disabled  pci:0000:00:1e.0     0x2448
> > C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
> > C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
> > C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
> > C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
> > C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
> > C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
> > C21D      S0     disabled  pci:0000:08:00.0     0x16fd
> > C109      S5     disabled  pci:0000:00:1c.1     0x27d2
> > C228      S5     disabled  pci:0000:10:00.0     0x4222
> > C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
> > C229      S5     disabled
> > [root@localhost ~]# find /sys -name "*" | grep sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:05/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:0d/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:11/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:15/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:1d/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/device:2c/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/device:2e/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state
> > [root@localhost ~]# ls /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup
> > acpi_bus_id  bus_id  pci_id  run_wakeup  sleep_state  status
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id
> > cat: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id: No such file or directory
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/acpi_bus_id
> > C229
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state
> > S5
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> > disabled
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/bus_id
> > 
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/pci_id
> > 
> > [root@localhost ~]# echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> > enabled
> > [root@localhost ~]# cat /proc/acpi/wakeup
> > Device  S-state   Status   Sysfs node           PCI ID
> > C093      S5     disabled  pci:0000:00:1e.0     0x2448
> > C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
> > C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
> > C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
> > C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
> > C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
> > C0F9      S0     enabled   pci:0000:00:1c.0     0x27d0
> > C21D      S0     enabled   pci:0000:08:00.0     0x16fd
> > C109      S5     enabled   pci:0000:00:1c.1     0x27d2
> > C228      S5     enabled   pci:0000:10:00.0     0x4222
> > C10F      S5     enabled   pci:0000:00:1c.3     0x27d6
> > C229      S5     enabled
> > [root@localhost ~]# vi /var/log/dmesg
> > [root@localhost ~]# dmesg | grep "same GPE"
> > ACPI: 'C0F9' and 'C229' have the same GPE, can't disable/enable one seperately
> > ACPI: 'C21D' and 'C229' have the same GPE, can't disable/enable one seperately
> > ACPI: 'C109' and 'C229' have the same GPE, can't disable/enable one seperately
> > ACPI: 'C228' and 'C229' have the same GPE, can't disable/enable one seperately
> > ACPI: 'C10F' and 'C229' have the same GPE, can't disable/enable one seperately
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/*/wakeup/status
> > disabled
> > disabled
> > disabled
> > disabled
> > disabled
> > disabled
> > enabled
> > enabled
> > enabled
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/*/*/wakeup/status
> > enabled
> > enabled
> > enabled
> > [root@localhost ~]#
> 
> 
> I think that it would be much much better to place wake-up attributes under
> corresponding PCI and PNP devices.
Currently, all devices have had an wakeup attribute, it
is /sys/.../power/wakeup, for example:

/sys/devices/pci0000\:00/0000\:00\:00.0/power/wakeup
/sys/class/tty/console/power/wakeup

But it isn't the same as acpi device's, you can get all acpi devices
with wakeup features from /proc/acpi/wakeup, you can also get all the
"power/wakeup" from /sys, they aren't 1:1.

[yangyi@yangyi-dev ~]$ cat /proc/acpi/wakeup 
Device  S-state   Status   Sysfs node           PCI ID
SLPB      S4    *enabled   
P32       S4     disabled  pci:0000:00:1e.0     0x244e
UAR1      S4     disabled  pnp:00:09            0x0000
ILAN      S4     disabled  pci:0000:00:19.0     0x104b
PEGP      S4     disabled  pci:0000:00:01.0     0x29a1
PEX0      S4     disabled  pci:0000:00:1c.0     0x283f
PEX1      S4     disabled  pci:0000:00:1c.1     0x2841
PEX2      S4     disabled  pci:0000:00:1c.2     0x2843
PEX3      S4     disabled  pci:0000:00:1c.3     0x2845
PEX4      S4     disabled  pci:0000:00:1c.4     0x2847
PEX5      S4     disabled  
UHC1      S3     disabled  pci:0000:00:1d.0     0x2830
UHC2      S3     disabled  pci:0000:00:1d.1     0x2831
UHC3      S3     disabled  pci:0000:00:1d.2     0x2832
UHC4      S3     disabled  
EHCI      S3     disabled  pci:0000:00:1d.7     0x2836
EHC2      S3     disabled  pci:0000:00:1a.7     0x283a
UH42      S3     disabled  pci:0000:00:1a.0     0x2834
UHC5      S3     disabled  pci:0000:00:1a.1     0x2835
AZAL      S3     disabled  pci:0000:00:1b.0     0x284b
[yangyi@yangyi-dev ~]$ /home/yangyi/wakeup.sh 
/sys/devices/pci0000:00/0000:00:1a.0/usb1/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1a.1/usb2/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1a.7/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1a.7/usb6/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.0/usb3/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.0/usb3/3-2/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.1/usb4/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.2/usb5/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.7/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.7/usb7/power/wakeup
enabled
/sys/class/tty/ttyS0/power/wakeup
disabled
/sys/class/tty/ttyS1/power/wakeup
disabled
/sys/class/tty/ttyS2/power/wakeup
disabled
/sys/class/tty/ttyS3/power/wakeup
disabled
[yangyi@yangyi-dev ~]$

>From source code, it seems they are different things.

> 
> Probably it is even better to link this code to PCI code, so PCI drivers will be aware of ACPI.
> For example it will fix the 'EHCI instantly wakes up the system from S4' on my system, since here USB doesn't wake
> up anything from S4, and ACPI tables correctly show that.
> 
> If ehci driver was aware of that it could disable #PME on entrance to S4.
> And we even can reuse its 'wakeup' attribute, thus enabling wakeup automatically.
> 
> Going ever further, I think that it will be great to get rid of ACPI device tree, since
> most acpi devices are ether PCI of PNP ones.
> 
> Or, even better have a small ACPI tree, that will contain 'true' ACPI devices, like cpus
> thermal sensors, buttons, etc. 
Maybe this is a good idea, but i don't know the relationships between
acpi devices, devices, pci devices and pnp devices. If we can merge all
these things together, that will be a great job.
> 
> Best regards,
> 	Maxim Levitsky

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Matthew Garrett]

On Thu, Jan 10, 2008 at 07:59:36AM +0800, Yi Yang wrote:

> Maybe this is a good idea, but i don't know the relationships between
> acpi devices, devices, pci devices and pnp devices. If we can merge all
> these things together, that will be a great job.

Let's not merge this yet, then, otherwise we'll be forced to carry 
around a sysfs API that's of no real use.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Pavel Machek]

> [yangyi@yangyi-dev ~]$ cat /proc/acpi/wakeup 
> Device  S-state   Status   Sysfs node           PCI ID
> SLPB      S4    *enabled   
> P32       S4     disabled  pci:0000:00:1e.0     0x244e
> UAR1      S4     disabled  pnp:00:09            0x0000

This should tell you how bad is placing PCI ID into generic file.

NAK.
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Zhang Rui]

On Thu, 2008-01-10 at 09:43 +0200, Maxim Levitsky wrote:
> On Thursday, 10 January 2008 00:21:46 Yi Yang wrote:
> > Subject: ACPI: convert procfs to sysfs for /proc/acpi/wakeup
> > From: Yi Yang <yi.y.yang@intel.com>
> > 
> > /proc/acpi/wakeup is deprecated but it has to exist because
> > we haven't a sysfs interface to replace it yet, this patch
> > converts /proc/acpi/wakeup to sysfs interface, under every
> > acpi device sysfs node, a user can see a directory "wakeup"
> > if the acpi device can support wakeup, there are six files
> > under this directory:
> > 
> > acpi_bus_id  bus_id  pci_id  run_wakeup  sleep_state  status
> > 
> > All the files are read-only exclude "status" which is used
> > to enable or disable wakeup of the acpi device.
> > 
> > "acpi_bus_id" is acpi bus ID of the acpi device.
> > 
> > "bus_id" is pci bus id of the device associated to the acpi
> > device, it will be empty if there isn't any device associated
> > to it.
> > 
> > "pci_id" is PCI ID of the pci device associated to the acpi
> > device, it will be empty if there isn't any device associated
> > to it.
> > 
> > "run_wake" is a flag indicating if a wakeup process is being
> > handled.
> > 
> > "sleep_state" is sleep state of the acpi device such as "S0".
> > 
> > "status" is wakeup status of the acpi device, it is enabled
> > or disabled, a user can change it be echoing "0", "1",
> > "disabled" or "enabled" to /sys/devices/.../wakeup/status. 
> > 
> > Here is the test result:
> > 
> > [root@localhost ~]# cat /proc/acpi/wakeup
> > Device  S-state   Status   Sysfs node           PCI ID
> > C093      S5     disabled  pci:0000:00:1e.0     0x2448
> > C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
> > C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
> > C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
> > C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
> > C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
> > C0F9      S0     disabled  pci:0000:00:1c.0     0x27d0
> > C21D      S0     disabled  pci:0000:08:00.0     0x16fd
> > C109      S5     disabled  pci:0000:00:1c.1     0x27d2
> > C228      S5     disabled  pci:0000:10:00.0     0x4222
> > C10F      S5     disabled  pci:0000:00:1c.3     0x27d6
> > C229      S5     disabled
> > [root@localhost ~]# find /sys -name "*" | grep sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:05/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:0d/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:11/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:15/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:1d/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/device:2c/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/device:2e/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/wakeup/sleep_state
> > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state
> > [root@localhost ~]# ls /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup
> > acpi_bus_id  bus_id  pci_id  run_wakeup  sleep_state  status
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id
> > cat: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id: No such file or directory
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/acpi_bus_id
> > C229
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state
> > S5
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> > disabled
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/bus_id
> > 
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/pci_id
> > 
> > [root@localhost ~]# echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status
> > enabled
> > [root@localhost ~]# cat /proc/acpi/wakeup
> > Device  S-state   Status   Sysfs node           PCI ID
> > C093      S5     disabled  pci:0000:00:1e.0     0x2448
> > C0E8      S3     disabled  pci:0000:00:1d.0     0x27c8
> > C0EF      S3     disabled  pci:0000:00:1d.1     0x27c9
> > C0F0      S3     disabled  pci:0000:00:1d.2     0x27ca
> > C0F1      S3     disabled  pci:0000:00:1d.3     0x27cb
> > C0F2      S3     disabled  pci:0000:00:1d.7     0x27cc
> > C0F9      S0     enabled   pci:0000:00:1c.0     0x27d0
> > C21D      S0     enabled   pci:0000:08:00.0     0x16fd
> > C109      S5     enabled   pci:0000:00:1c.1     0x27d2
> > C228      S5     enabled   pci:0000:10:00.0     0x4222
> > C10F      S5     enabled   pci:0000:00:1c.3     0x27d6
> > C229      S5     enabled
> > [root@localhost ~]# vi /var/log/dmesg
> > [root@localhost ~]# dmesg | grep "same GPE"
> > ACPI: 'C0F9' and 'C229' have the same GPE, can't disable/enable one seperately
> > ACPI: 'C21D' and 'C229' have the same GPE, can't disable/enable one seperately
> > ACPI: 'C109' and 'C229' have the same GPE, can't disable/enable one seperately
> > ACPI: 'C228' and 'C229' have the same GPE, can't disable/enable one seperately
> > ACPI: 'C10F' and 'C229' have the same GPE, can't disable/enable one seperately
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/*/wakeup/status
> > disabled
> > disabled
> > disabled
> > disabled
> > disabled
> > disabled
> > enabled
> > enabled
> > enabled
> > [root@localhost ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/*/*/wakeup/status
> > enabled
> > enabled
> > enabled
> > [root@localhost ~]#
> 
> 
> I think that it would be much much better to place wake-up attributes under
> corresponding PCI and PNP devices.
> Probably it is even better to link this code to PCI code, so PCI drivers will be aware of ACPI.
I like this idea, maxim. :)
And that's what we actually did about half a year ago.

Yi,
Please refer to http://bugzilla.kernel.org/show_bug.cgi?id=6892
and David's patch set here:
http://marc.info/?l=linux-mm-commits&m=117701595209299&w=2
http://marc.info/?l=linux-mm-commits&m=117701866524935&w=2
You can have a look at this thread as well:
http://marc.info/?l=linux-acpi&m=119982937409968&w=2

thanks,
Rui
> For example it will fix the 'EHCI instantly wakes up the system from S4' on my system, since here USB doesn't wake
> up anything from S4, and ACPI tables correctly show that.
> 
> If ehci driver was aware of that it could disable #PME on entrance to S4.
> And we even can reuse its 'wakeup' attribute, thus enabling wakeup automatically.
> 
> Going ever further, I think that it will be great to get rid of ACPI device tree, since
> most acpi devices are ether PCI of PNP ones.
> 
> Or, even better have a small ACPI tree, that will contain 'true' ACPI devices, like cpus
> thermal sensors, buttons, etc. 
> 

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Yi Yang]

> > I think that it would be much much better to place wake-up attributes under
> > corresponding PCI and PNP devices.
> > Probably it is even better to link this code to PCI code, so PCI drivers will be aware of ACPI.
> I like this idea, maxim. :)
> And that's what we actually did about half a year ago.
> 
> Yi,
> Please refer to http://bugzilla.kernel.org/show_bug.cgi?id=6892
> and David's patch set here:
> http://marc.info/?l=linux-mm-commits&m=117701595209299&w=2
> http://marc.info/?l=linux-mm-commits&m=117701866524935&w=2
> You can have a look at this thread as well:
> http://marc.info/?l=linux-acpi&m=119982937409968&w=2
> 
I checked those patches you mentioned, they did bind two wakeup flag to
some extent, but they can't be exchanged to use and they are just partly
in current linus tree, two flags bind isn't in linus tree.

According to my test on the latest linus tree, wakeup flag of
acpi_device hasn't any association with device's, i don't know if they
are the same thing. if we enbale or disable it manually, what will
happen? From source code, it is just a flag, it doesn't trigger any
event or hardware operation.

> thanks,
> Rui
> > For example it will fix the 'EHCI instantly wakes up the system from S4' on my system, since here USB doesn't wake
> > up anything from S4, and ACPI tables correctly show that.
> > 
> > If ehci driver was aware of that it could disable #PME on entrance to S4.
> > And we even can reuse its 'wakeup' attribute, thus enabling wakeup automatically.
> > 
> > Going ever further, I think that it will be great to get rid of ACPI device tree, since
> > most acpi devices are ether PCI of PNP ones.
> > 
> > Or, even better have a small ACPI tree, that will contain 'true' ACPI devices, like cpus
> > thermal sensors, buttons, etc. 
> > 
> 
> 

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Thomas Renninger]

On Fri, 2008-01-11 at 07:55 +0800, Yi Yang wrote:
> > > I think that it would be much much better to place wake-up attributes under
> > > corresponding PCI and PNP devices.
> > > Probably it is even better to link this code to PCI code, so PCI drivers will be aware of ACPI.
> > I like this idea, maxim. :)
> > And that's what we actually did about half a year ago.
> > 
> > Yi,
> > Please refer to http://bugzilla.kernel.org/show_bug.cgi?id=6892
> > and David's patch set here:
> > http://marc.info/?l=linux-mm-commits&m=117701595209299&w=2
> > http://marc.info/?l=linux-mm-commits&m=117701866524935&w=2
> > You can have a look at this thread as well:
> > http://marc.info/?l=linux-acpi&m=119982937409968&w=2
> > 
> I checked those patches you mentioned, they did bind two wakeup flag to
> some extent, but they can't be exchanged to use and they are just partly
> in current linus tree, two flags bind isn't in linus tree.
> 
> According to my test on the latest linus tree, wakeup flag of
> acpi_device hasn't any association with device's, i don't know if they
> are the same thing. if we enbale or disable it manually, what will
> happen? From source code, it is just a flag, it doesn't trigger any
> event or hardware operation.
> 
> > thanks,
> > Rui
> > > For example it will fix the 'EHCI instantly wakes up the system from S4' on my system, since here USB doesn't wake
> > > up anything from S4, and ACPI tables correctly show that.
> > > 
> > > If ehci driver was aware of that it could disable #PME on entrance to S4.
> > > And we even can reuse its 'wakeup' attribute, thus enabling wakeup automatically.
> > > 
> > > Going ever further, I think that it will be great to get rid of ACPI device tree, since
> > > most acpi devices are ether PCI of PNP ones.
> > > 
> > > Or, even better have a small ACPI tree, that will contain 'true' ACPI devices, like cpus
> > > thermal sensors, buttons, etc. 

Any news on this?
I ran into a problem with the current implementation:

If one GPE is tight to several devices you get a message:
echo XYZ >/tmp/acpi/wakeup
ACPI: 'XXX' and 'XYZ' have the same GPE, can't disable/enable one
seperately
ACPI: 'YYY' and 'XYZ' have the same GPE, can't disable/enable one
seperately
and none of the devices are activated to be able to wake the machine up.
Which I expect is wrong, all should be enabled/disabled then IMO, but
it's probably not worth much fixing in /proc/acpi/...

The correct interface to use seem to be:
drivers/base/power/sysfs.c
But this is rather broken?
Here an output of /proc/acpi/wakeup and /sys/...:
for x in `find /sys/ |grep wakeup`;do if [ $(cat $x) ];then echo $x; cat $x;fi;done
/sys/devices/pnp0/00:04/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.7/usb4/4-5/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.7/usb4/4-1/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.7/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.1/usb2/power/wakeup
enabled
/sys/devices/pci0000:00/0000:00:1d.0/usb1/power/wakeup
enabled
trenn@stravinsky:/extern/trenn/packages/home:trenn> cat /proc/acpi/wakeup 
Device  S-state   Status   Sysfs node
PCI0      S5     disabled  no-bus:pci0000:00

I still think (from comments in drivers/base/power/sysfs.c, not sure
whether it really is that appropriate) it is wakeup sysfs file that
should be used for this.
I wonder why each device has a wakeup file, it should be enough to
create them dynamically if wakeup enable/disable is supported for a
specific device?
Also a second file is missing from which state (S3,S4,S5) the device can
wake the machine up.

If there can be multiple devices for one GPE, this information (the
power directory of multiple devices) could be linked together in sysfs?
E.g.
/sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
is a link to:
/sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
If both are using one wake-up GPE.

Also if the ACPI device caught through acpi_get_physical device is a PCI
bridge, it should get evaluated what is behind the bridge and this
device (e.g. a network card) should get the wakeup stuff set up, not the
bridge?

Does someone still look at this?
If not, shall I or is it on some queue?
Should this be discussed a bit more detailed first?

   Thomas

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Yi Yang]

> Any news on this?
> I ran into a problem with the current implementation:
> 
> If one GPE is tight to several devices you get a message:
> echo XYZ >/tmp/acpi/wakeup
> ACPI: 'XXX' and 'XYZ' have the same GPE, can't disable/enable one
> seperately
> ACPI: 'YYY' and 'XYZ' have the same GPE, can't disable/enable one
> seperately
> and none of the devices are activated to be able to wake the machine up.
> Which I expect is wrong, all should be enabled/disabled then IMO, but
> it's probably not worth much fixing in /proc/acpi/...
"Can't disable/enable one seperately" is just a warning, all the devices
with the same GPE can be disabled/enabled once.

> 
> The correct interface to use seem to be:
> drivers/base/power/sysfs.c
wakeup flag in this driver is a generic software flag in "struct
device", but the wakeup flag you see in /proc/acpi/wakeup is a hardware
wakeup flag in ACPI device, all the wakeup events triggered by hardware
devices are handled by ACPI driver.

But i indeed regret wakeup flags in /sys/... and /proc/acpi/wakeup
haven't any association. They should be consolidated in my opinion.

Zhang Rui said they are doing it, but i didn't get any information about
progress, i completely agree it should be done ASAP.

If you need to enbale wakeup, you.d better enable wakeup flag in
both /proc/acpi/wakeup and /sys/..., i can use USB mouse to wake up my
machine from S3.

> But this is rather broken?
> Here an output of /proc/acpi/wakeup and /sys/...:
> for x in `find /sys/ |grep wakeup`;do if [ $(cat $x) ];then echo $x; cat $x;fi;done
> /sys/devices/pnp0/00:04/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.7/usb4/4-5/power/wakeu
> enabled
> /sys/devices/pci0000:00/0000:00:1d.7/usb4/4-1/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.7/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.1/usb2/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.0/usb1/power/wakeup
> enabled
> trenn@stravinsky:/extern/trenn/packages/home:trenn> cat /proc/acpi/wakeup 
> Device  S-state   Status   Sysfs node
> PCI0      S5     disabled  no-bus:pci0000:00
> 
> I still think (from comments in drivers/base/power/sysfs.c, not sure
> whether it really is that appropriate) it is wakeup sysfs file that
> should be used for this.
> I wonder why each device has a wakeup file, it should be enough to
> create them dynamically if wakeup enable/disable is supported for a
> specific device?
> Also a second file is missing from which state (S3,S4,S5) the device can
> wake the machine up.
> 
> If there can be multiple devices for one GPE, this information (the
> power directory of multiple devices) could be linked together in sysfs?
> E.g.
> /sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
> is a link to:
> /sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
> If both are using one wake-up GPE.
> 
> Also if the ACPI device caught through acpi_get_physical device is a PCI
> bridge, it should get evaluated what is behind the bridge and this
> device (e.g. a network card) should get the wakeup stuff set up, not the
> bridge?
> 
> Does someone still look at this?
> If not, shall I or is it on some queue?
> Should this be discussed a bit more detailed first?
> 
>    Thomas
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Zhao Yakui]

On Wed, 2008-03-19 at 22:37 +0800, Yi Yang wrote:
> > Any news on this?
> > I ran into a problem with the current implementation:
> > 
> > If one GPE is tight to several devices you get a message:
> > echo XYZ >/tmp/acpi/wakeup
> > ACPI: 'XXX' and 'XYZ' have the same GPE, can't disable/enable one
> > seperately
> > ACPI: 'YYY' and 'XYZ' have the same GPE, can't disable/enable one
> > seperately
> > and none of the devices are activated to be able to wake the machine up.
> > Which I expect is wrong, all should be enabled/disabled then IMO, but
> > it's probably not worth much fixing in /proc/acpi/...
> "Can't disable/enable one seperately" is just a warning, all the devices
> with the same GPE can be disabled/enabled once.
> 
> > 
> > The correct interface to use seem to be:
> > drivers/base/power/sysfs.c
> wakeup flag in this driver is a generic software flag in "struct
> device", but the wakeup flag you see in /proc/acpi/wakeup is a hardware
> wakeup flag in ACPI device, all the wakeup events triggered by hardware
> devices are handled by ACPI driver.
> 
In current kernel the wakeup flag in /proc/acpi/wakeup means whether the
device can wakeup the sleeping system. It is only used to wake the
system from S1/S3/S4. 
Now the power/wakeup sys I/F is created for every device. Maybe it is
better that  the power/wakeup sys I/F is only created for the device
that can wake up the sleeping system. In order to determine whether the
device can wake up the sleeping system, it is necessary to check whether
the associated ACPI device can support wakeup  and whether the native
device can support wakeup (For example: PCI device should support PME
function.)
> But i indeed regret wakeup flags in /sys/... and /proc/acpi/wakeup
> haven't any association. They should be consolidated in my opinion.

> Zhang Rui said they are doing it, but i didn't get any information about
> progress, i completely agree it should be done ASAP.

Now I am trying to do them. But the difficulty is that we should support
the runtime wakeup. 
> If you need to enbale wakeup, you.d better enable wakeup flag in
> both /proc/acpi/wakeup and /sys/..., i can use USB mouse to wake up my
> machine from S3.
> 
> > But this is rather broken?
> > Here an output of /proc/acpi/wakeup and /sys/...:
> > for x in `find /sys/ |grep wakeup`;do if [ $(cat $x) ];then echo $x; cat $x;fi;done
> > /sys/devices/pnp0/00:04/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.7/usb4/4-5/power/wakeu
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.7/usb4/4-1/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.7/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.1/usb2/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.0/usb1/power/wakeup
> > enabled
> > trenn@stravinsky:/extern/trenn/packages/home:trenn> cat /proc/acpi/wakeup 
> > Device  S-state   Status   Sysfs node
> > PCI0      S5     disabled  no-bus:pci0000:00
> > 
> > I still think (from comments in drivers/base/power/sysfs.c, not sure
> > whether it really is that appropriate) it is wakeup sysfs file that
> > should be used for this.
> > I wonder why each device has a wakeup file, it should be enough to
> > create them dynamically if wakeup enable/disable is supported for a
> > specific device?
> > Also a second file is missing from which state (S3,S4,S5) the device can
> > wake the machine up.
> > 
> > If there can be multiple devices for one GPE, this information (the
> > power directory of multiple devices) could be linked together in sysfs?
> > E.g.
> > /sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
> > is a link to:
> > /sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
> > If both are using one wake-up GPE.
> > 
> > Also if the ACPI device caught through acpi_get_physical device is a PCI
> > bridge, it should get evaluated what is behind the bridge and this
> > device (e.g. a network card) should get the wakeup stuff set up, not the
> > bridge?
> > 
> > Does someone still look at this?
> > If not, shall I or is it on some queue?
> > Should this be discussed a bit more detailed first?
> > 
> >    Thomas
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[David Brownell]

On Wednesday 19 March 2008, Thomas Renninger wrote:
> On Fri, 2008-01-11 at 07:55 +0800, Yi Yang wrote:
> > > > I think that it would be much much better to place wake-up attributes under
> > > > corresponding PCI and PNP devices.
> > > > Probably it is even better to link this code to PCI code, so PCI
> > > > drivers will be aware of ACPI. 
> > >
> > > I like this idea, maxim. :)
> > > And that's what we actually did about half a year ago.
> > > 
> > > Yi,
> > > Please refer to http://bugzilla.kernel.org/show_bug.cgi?id=6892
> > > and David's patch set here:
> > > http://marc.info/?l=linux-mm-commits&m=117701595209299&w=2
> > > http://marc.info/?l=linux-mm-commits&m=117701866524935&w=2
> > > You can have a look at this thread as well:
> > > http://marc.info/?l=linux-acpi&m=119982937409968&w=2

And I have some more split-out versions of those patches
now, too.  Cleanups and simplifications should be more
directly mergeable, and the tricky bits affecting GPE use
can be addressed by folk who are actively involved in the
nitty-gritty of making the power management parts of ACPI
work right for Linux.

I think I'll repost them soonish, to linux-pm and, as relevant,
to linux-acpi.  LKML for stuff that's IMO mergeable as-is.

 
> > I checked those patches you mentioned, they did bind two wakeup flag to
> > some extent, but they can't be exchanged to use and they are just partly
> > in current linus tree, two flags bind isn't in linus tree.

Right.


> > According to my test on the latest linus tree, wakeup flag of
> > acpi_device hasn't any association with device's, i don't know if they
> > are the same thing. if we enbale or disable it manually, what will
> > happen? From source code, it is just a flag, it doesn't trigger any
> > event or hardware operation.

Currently ACPI wakeup mechanisms are not at all integrated with
driver model mechanisms, or with non-ACPI bits of the system.

The "why" of that is that those patches still haven't been merged;
and the "why" of *that* is, AFAICT, that ACPI sleep/wake/resume
support is still in serious flux.

The current model is, yes, those are just flags ... and they only
kick in during driver state transitions.  Someday we can hope
they will support runtime power management (e.g. putting PCI
devices in PCI_D3hot to save power, then letting them trigger
runtime wake events when external hardware asks for that) ...
but for now, those transitions only kick in when the system as
a whole enters a sleep state, via /sys/power/state writes.


> > > thanks,
> > > Rui
> > > > For example it will fix the 'EHCI instantly wakes up the system
> > > > from S4' on my system, since here USB doesn't wake 
> > > > up anything from S4, and ACPI tables correctly show that.
> > > > 
> > > > If ehci driver was aware of that it could disable #PME on entrance to S4.
> > > > And we even can reuse its 'wakeup' attribute, thus enabling wakeup
> > > > automatically. 
> > > > 
> > > > Going ever further, I think that it will be great to get rid of ACPI
> > > > device tree, since 
> > > > most acpi devices are ether PCI of PNP ones.
> > > > 
> > > > Or, even better have a small ACPI tree, that will contain 'true'
> > > > ACPI devices, like cpus thermal sensors, buttons, etc. 

There's a bit of state in the ACPI device nodes that's not
currently visible through PCI or PNP.  The patch I posted
a while back to cross-link ACPI nodes to the "real" nodes
should help sort out some of that.  (Basically, it's just
an updated version of a patch from Zhang Rui.)


> Any news on this?

Not from me, but that sounds like a useful direction.  In
the same vein, it'd make sense to properly root PCI from the
PNP record for the PCI root.


> I ran into a problem with the current implementation:
> 
> If one GPE is tight to several devices you get a message:
> echo XYZ >/tmp/acpi/wakeup
> ACPI: 'XXX' and 'XYZ' have the same GPE, can't disable/enable one
> seperately
> ACPI: 'YYY' and 'XYZ' have the same GPE, can't disable/enable one
> seperately
> and none of the devices are activated to be able to wake the machine up.

I've not happened across that myself.


> Which I expect is wrong, all should be enabled/disabled then IMO, but
> it's probably not worth much fixing in /proc/acpi/...

Agreed.

 
> The correct interface to use seem to be:
> drivers/base/power/sysfs.c
> But this is rather broken?
> Here an output of /proc/acpi/wakeup and /sys/...:
> for x in `find /sys/ |grep wakeup`;do if [ $(cat $x) ];then echo $x; cat $x;fi;done
> /sys/devices/pnp0/00:04/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.7/usb4/4-5/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.7/usb4/4-1/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.7/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.1/usb2/power/wakeup
> enabled
> /sys/devices/pci0000:00/0000:00:1d.0/usb1/power/wakeup
> enabled

In short:  only USB portions of the tree have those flags set,
since USB (a) has some workarounds for the lack of ACPI support
on OHCI and EHCI controllers, like 00:1d.7, and (b) supports
those flags for devices that ACPI doesn't know about, such as
most USB keyboards, hubs, mice, and so forth.  Plus, (c) you
aren't using the rtc-cmos driver, which works better with the
rest of Linux than the legacy drivers/char/rtc driver.

If you had applied patches like my "teach ACPI how to use the
wakeup flags", you should see more devices with such flags.


> trenn@stravinsky:/extern/trenn/packages/home:trenn> cat /proc/acpi/wakeup 
> Device  S-state   Status   Sysfs node
> PCI0      S5     disabled  no-bus:pci0000:00

Hmm, and (d) you've got a system that doesn't do much in terms
of ACPI wakeup:  a PCI root bridge, and maybe some buttons.  Why
that root bridge shows up as a "no-bus" node I don't know... 
there's usually a PNP node for it, and otherwise it'd seem like
it should be a PCI device.  (The buttons seem to never show up.)

It's not uncommon for the ACPI device tables to list devices
that don't actually exist in /proc/acpi/wakeup.  Many of the
devices with no sysfs node seem to be of that type.  To me,
this just highlights the current weak handling of wakeups in
the ACPI stack.


> I still think (from comments in drivers/base/power/sysfs.c, not sure
> whether it really is that appropriate) it is wakeup sysfs file that
> should be used for this.

Yes.


> I wonder why each device has a wakeup file, it should be enough to
> create them dynamically if wakeup enable/disable is supported for a
> specific device?

Because that can change dynamically.  Classic example:  a USB
device with two active configurations, plus "configuration zero".
Whether it's wakeup-capable depends on a bit in the descriptor
for the active configuration ... so config zero may never waake
the system, while either (or both!) of the active configs might.


> Also a second file is missing from which state (S3,S4,S5) the device can
> wake the machine up.

Those labels are ACPI-specific, and anything at the core of
Linux (like the driver model and its wakeup flags) should
never be ACPI-specific!

Plus, it's not clear how much that matters.  It's not as if
drivers should prevent entering sleep states if they can't
act as wake event sources in that level (e.g. S3 == "mem").
That information can stay in /proc/acpi/wakeup until that's
finally removed; if no userspace tools need that info, I see
no good reason for exporting it.


> If there can be multiple devices for one GPE, this information (the
> power directory of multiple devices) could be linked together in sysfs?
> E.g.
> /sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
> is a link to:
> /sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
> If both are using one wake-up GPE.

That doesn't seem helpful ... it'd mean that you couldn't
manage wakeup policy for individual devices. 

This issue is an ACPI-ism, having to do with how wakeup
is implemented on one platform:  "GPE" signals, which are
not exposed in any useful way, rather than IRQs (fully
exposed and shareable, see irq_chip.set_wake) or some other
mechanism.  So it doesn't belong in the driver model core.


> Also if the ACPI device caught through acpi_get_physical device is a PCI
> bridge, it should get evaluated what is behind the bridge and this
> device (e.g. a network card) should get the wakeup stuff set up, not the
> bridge?

One part of the bug 6892 discussion is specifically about
how PCI bridges don't support wakeup events ... PME# signals,
or their PCIE equivalent, as issued by add-in PCI cards.

I think that'll work better if ACPI gets sane support for
wakeup from the built-in devices first.  And oddly enough,
basic patches supporting that have been around for well over
a year now ...


> Does someone still look at this?
> If not, shall I or is it on some queue?
> Should this be discussed a bit more detailed first?

I'll do my bit by (re)reposting the patches I have.

- Dave

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[Zhao Yakui]

On Wed, 2008-03-19 at 11:52 -0700, David Brownell wrote:
> On Wednesday 19 March 2008, Thomas Renninger wrote:
> > On Fri, 2008-01-11 at 07:55 +0800, Yi Yang wrote:
> > > > > I think that it would be much much better to place wake-up attributes under
> > > > > corresponding PCI and PNP devices.
> > > > > Probably it is even better to link this code to PCI code, so PCI
> > > > > drivers will be aware of ACPI. 
> > > >
> > > > I like this idea, maxim. :)
> > > > And that's what we actually did about half a year ago.
> > > > 
> > > > Yi,
> > > > Please refer to http://bugzilla.kernel.org/show_bug.cgi?id=6892
> > > > and David's patch set here:
> > > > http://marc.info/?l=linux-mm-commits&m=117701595209299&w=2
> > > > http://marc.info/?l=linux-mm-commits&m=117701866524935&w=2
> > > > You can have a look at this thread as well:
> > > > http://marc.info/?l=linux-acpi&m=119982937409968&w=2
> 
> And I have some more split-out versions of those patches
> now, too.  Cleanups and simplifications should be more
> directly mergeable, and the tricky bits affecting GPE use
> can be addressed by folk who are actively involved in the
> nitty-gritty of making the power management parts of ACPI
> work right for Linux.
> 
> I think I'll repost them soonish, to linux-pm and, as relevant,
> to linux-acpi.  LKML for stuff that's IMO mergeable as-is.
> 
>  
> > > I checked those patches you mentioned, they did bind two wakeup flag to
> > > some extent, but they can't be exchanged to use and they are just partly
> > > in current linus tree, two flags bind isn't in linus tree.
> 
> Right.
> 
> 
> > > According to my test on the latest linus tree, wakeup flag of
> > > acpi_device hasn't any association with device's, i don't know if they
> > > are the same thing. if we enbale or disable it manually, what will
> > > happen? From source code, it is just a flag, it doesn't trigger any
> > > event or hardware operation.
> 
> Currently ACPI wakeup mechanisms are not at all integrated with
> driver model mechanisms, or with non-ACPI bits of the system.
> 
> The "why" of that is that those patches still haven't been merged;
> and the "why" of *that* is, AFAICT, that ACPI sleep/wake/resume
> support is still in serious flux.
> 
> The current model is, yes, those are just flags ... and they only
> kick in during driver state transitions.  Someday we can hope
> they will support runtime power management (e.g. putting PCI
> devices in PCI_D3hot to save power, then letting them trigger
> runtime wake events when external hardware asks for that) ...
> but for now, those transitions only kick in when the system as
> a whole enters a sleep state, via /sys/power/state writes.
> 
Right. Now the system only supports that the device wakes the whole
sleeping system. Maybe it is necessary to add the runtime wakeup
support.(For example: the PCI device that supports PME)
> 
> > > > thanks,
> > > > Rui
> > > > > For example it will fix the 'EHCI instantly wakes up the system
> > > > > from S4' on my system, since here USB doesn't wake 
> > > > > up anything from S4, and ACPI tables correctly show that.
> > > > > 
> > > > > If ehci driver was aware of that it could disable #PME on entrance to S4.
> > > > > And we even can reuse its 'wakeup' attribute, thus enabling wakeup
> > > > > automatically. 
> > > > > 
> > > > > Going ever further, I think that it will be great to get rid of ACPI
> > > > > device tree, since 
> > > > > most acpi devices are ether PCI of PNP ones.
> > > > > 
> > > > > Or, even better have a small ACPI tree, that will contain 'true'
> > > > > ACPI devices, like cpus thermal sensors, buttons, etc. 
> 
> There's a bit of state in the ACPI device nodes that's not
> currently visible through PCI or PNP.  The patch I posted
> a while back to cross-link ACPI nodes to the "real" nodes
> should help sort out some of that.  (Basically, it's just
> an updated version of a patch from Zhang Rui.)
> 
> 
> > Any news on this?
> 
> Not from me, but that sounds like a useful direction.  In
> the same vein, it'd make sense to properly root PCI from the
> PNP record for the PCI root.
> 
> 
> > I ran into a problem with the current implementation:
> > 
> > If one GPE is tight to several devices you get a message:
> > echo XYZ >/tmp/acpi/wakeup
> > ACPI: 'XXX' and 'XYZ' have the same GPE, can't disable/enable one
> > seperately
> > ACPI: 'YYY' and 'XYZ' have the same GPE, can't disable/enable one
> > seperately
> > and none of the devices are activated to be able to wake the machine up.
> 
> I've not happened across that myself.
> 
> 
> > Which I expect is wrong, all should be enabled/disabled then IMO, but
> > it's probably not worth much fixing in /proc/acpi/...
> 
> Agreed.
> 
>  
> > The correct interface to use seem to be:
> > drivers/base/power/sysfs.c
> > But this is rather broken?
> > Here an output of /proc/acpi/wakeup and /sys/...:
> > for x in `find /sys/ |grep wakeup`;do if [ $(cat $x) ];then echo $x; cat $x;fi;done
> > /sys/devices/pnp0/00:04/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.7/usb4/4-5/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.7/usb4/4-1/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.7/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.1/usb2/power/wakeup
> > enabled
> > /sys/devices/pci0000:00/0000:00:1d.0/usb1/power/wakeup
> > enabled
> 
> In short:  only USB portions of the tree have those flags set,
> since USB (a) has some workarounds for the lack of ACPI support
> on OHCI and EHCI controllers, like 00:1d.7, and (b) supports
> those flags for devices that ACPI doesn't know about, such as
> most USB keyboards, hubs, mice, and so forth.  Plus, (c) you
> aren't using the rtc-cmos driver, which works better with the
> rest of Linux than the legacy drivers/char/rtc driver.
It seems that the following only means that the PME is supported by the
USB PCI device. 
 > /sys/devices/pci0000:00/0000:00:1d.7/power/wakeup 
When the system enters the sleeping state, whether the 1d.7 PCI device
can wake the system is related to the following two factors:
    a. /proc/acpi/wakeup flag for 1d.7 PCI device is enabled.
    b. the Power/wakeup flag in Sys I/F is enabled. ( It means that PME
is supported and configured)
  
> > enabled
> 
> If you had applied patches like my "teach ACPI how to use the
> wakeup flags", you should see more devices with such flags.
> 
> 
> > trenn@stravinsky:/extern/trenn/packages/home:trenn> cat /proc/acpi/wakeup 
> > Device  S-state   Status   Sysfs node
> > PCI0      S5     disabled  no-bus:pci0000:00
> 
> Hmm, and (d) you've got a system that doesn't do much in terms
> of ACPI wakeup:  a PCI root bridge, and maybe some buttons.  Why
> that root bridge shows up as a "no-bus" node I don't know... 
> there's usually a PNP node for it, and otherwise it'd seem like
> it should be a PCI device.  (The buttons seem to never show up.)
> 
> It's not uncommon for the ACPI device tables to list devices
> that don't actually exist in /proc/acpi/wakeup.  Many of the
> devices with no sysfs node seem to be of that type.  To me,
> this just highlights the current weak handling of wakeups in
> the ACPI stack.
> 
> 
> > I still think (from comments in drivers/base/power/sysfs.c, not sure
> > whether it really is that appropriate) it is wakeup sysfs file that
> > should be used for this.
> 
> Yes.
> 
> 
> > I wonder why each device has a wakeup file, it should be enough to
> > create them dynamically if wakeup enable/disable is supported for a
> > specific device?
> 
> Because that can change dynamically.  Classic example:  a USB
> device with two active configurations, plus "configuration zero".
> Whether it's wakeup-capable depends on a bit in the descriptor
> for the active configuration ... so config zero may never waake
> the system, while either (or both!) of the active configs might.
> 
> 
> > Also a second file is missing from which state (S3,S4,S5) the device can
> > wake the machine up.
> 
> Those labels are ACPI-specific, and anything at the core of
> Linux (like the driver model and its wakeup flags) should
> never be ACPI-specific!
> 
Yes. the second file is ACPI-specific. We should add this file. And the
info should be obtained by the associated ACPI device. Maybe it is
better that it is create in the subdirectory of ACPI device and the link
is created.
> Plus, it's not clear how much that matters.  It's not as if
> drivers should prevent entering sleep states if they can't
> act as wake event sources in that level (e.g. S3 == "mem").
> That information can stay in /proc/acpi/wakeup until that's
> finally removed; if no userspace tools need that info, I see
> no good reason for exporting it.
> 
> 
> > If there can be multiple devices for one GPE, this information (the
> > power directory of multiple devices) could be linked together in sysfs?
> > E.g.
> > /sys/devices/pci0000:00/0000:00:1d.7/usb4/power/wakeup
> > is a link to:
> > /sys/devices/pci0000:00/0000:00:1d.2/usb3/power/wakeup
> > If both are using one wake-up GPE.
> 
> That doesn't seem helpful ... it'd mean that you couldn't
> manage wakeup policy for individual devices. 

> This issue is an ACPI-ism, having to do with how wakeup
> is implemented on one platform:  "GPE" signals, which are
> not exposed in any useful way, rather than IRQs (fully
> exposed and shareable, see irq_chip.set_wake) or some other
> mechanism.  So it doesn't belong in the driver model core.
> 
> 
> > Also if the ACPI device caught through acpi_get_physical device is a PCI
> > bridge, it should get evaluated what is behind the bridge and this
> > device (e.g. a network card) should get the wakeup stuff set up, not the
> > bridge?
> 
> One part of the bug 6892 discussion is specifically about
> how PCI bridges don't support wakeup events ... PME# signals,
> or their PCIE equivalent, as issued by add-in PCI cards.
> I think that'll work better if ACPI gets sane support for
> wakeup from the built-in devices first.  And oddly enough,
> basic patches supporting that have been around for well over
> a year now ...
> 
> 
> > Does someone still look at this?
> > If not, shall I or is it on some queue?
> > Should this be discussed a bit more detailed first?
> 
> I'll do my bit by (re)reposting the patches I have.
> 
> - Dave
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup

[David Brownell]

On Wednesday 19 March 2008, Zhao Yakui wrote:
> On Wed, 2008-03-19 at 11:52 -0700, David Brownell wrote:

> > Currently ACPI wakeup mechanisms are not at all integrated with
> > driver model mechanisms, or with non-ACPI bits of the system.
> > 
> > The "why" of that is that those patches still haven't been merged;
> > and the "why" of *that* is, AFAICT, that ACPI sleep/wake/resume
> > support is still in serious flux.
> > 
> > The current model is, yes, those are just flags ... and they only
> > kick in during driver state transitions.  Someday we can hope
> > they will support runtime power management (e.g. putting PCI
> > devices in PCI_D3hot to save power, then letting them trigger
> > runtime wake events when external hardware asks for that) ...
> > but for now, those transitions only kick in when the system as
> > a whole enters a sleep state, via /sys/power/state writes.
> 
> Right. Now the system only supports that the device wakes the whole
> sleeping system.

That's "barely" supported ... disabled by default, hard to
turn on, rarely (if ever) used, and so on.


> Maybe it is necessary to add the runtime wakeup 
> support. (For example: the PCI device that supports PME)

That'd go more smoothly if we first made the "easier" wake
event support work properly.  After all, that's basically
just making code that's been there for years always kick
in during system sleep transitions, and helping to make sure
the relevant drivers know how to use it ...


> > In short:  only USB portions of the tree have those flags set,
> > since USB (a) has some workarounds for the lack of ACPI support
> > on OHCI and EHCI controllers, like 00:1d.7, and (b) supports
> > those flags for devices that ACPI doesn't know about, such as
> > most USB keyboards, hubs, mice, and so forth.  Plus, (c) you
> > aren't using the rtc-cmos driver, which works better with the
> > rest of Linux than the legacy drivers/char/rtc driver.
> 
> It seems that the following only means that the PME is supported by the
> USB PCI device.
> 
> > /sys/devices/pci0000:00/0000:00:1d.7/power/wakeup

It's set by that HCD as it initializes, because ACPI still
doesn't do so.  There are hardware flags the BIOS sets and
the HCD sees, which in this case partially make up for the
weak support from ACPI.

And it's not specific to PME#, except with EHCI.  With
OHCI for example those flags get set with "legacy" PCI
power management too.

 
> When the system enters the sleeping state, whether the 1d.7 PCI device
> can wake the system is related to the following two factors:
>     a. /proc/acpi/wakeup flag for 1d.7 PCI device is enabled.
>     b. the Power/wakeup flag in Sys I/F is enabled. ( It means that PME
> is supported and configured)

And the /proc/acpi/wakeup stuff needs to go away, in favor
of standard driver model mechanisms that (a) aren't specific
to ACPI, and (b) don't default to "off, and hard to turn on".

Note that on at least some systems it seems that the ACPI bits
aren't entirely necessary.  When the driver enables PCI wakeup
mechanisms, the hardware reacts well enough to wake the system
even if ACPI has not *also* told it do do so.  (Of course it'd
be better if there were no issues about whether ACPI has been
appropriately stroked.)
   

> > > Also a second file is missing from which state (S3,S4,S5) the device can
> > > wake the machine up.
> > 
> > Those labels are ACPI-specific, and anything at the core of
> > Linux (like the driver model and its wakeup flags) should
> > never be ACPI-specific!
> 
> Yes. the second file is ACPI-specific. We should add this file.

Why?  "Just because" or is there a real need it would address?


> And the 
> info should be obtained by the associated ACPI device. Maybe it is
> better that it is create in the subdirectory of ACPI device and the link
> is created.

If ACPI-specific state like that "should" be exported, it should
be in an ACPI-specific portion of the tree.  And as for that link,
I'm still not clear on why the patch in

   http://marc.info/?l=linux-acpi&m=120500563430488&w=2

still hasn't merged ... that provides the relevant linkage in
as neutral a way as possible.


> > Plus, it's not clear how much that matters.  It's not as if
> > drivers should prevent entering sleep states if they can't
> > act as wake event sources in that level (e.g. S3 == "mem").
> > That information can stay in /proc/acpi/wakeup until that's
> > finally removed; if no userspace tools need that info, I see
> > no good reason for exporting it.
>