LKML Archive on lore.kernel.org help / color / mirror / Atom feed
* [PATCH linux-acpi] Fix /proc/acpi/alarm set error @ 2007-12-27 6:47 Yi Yang 2007-12-27 8:41 ` [PATCH linux-acpi] Remove superfluous code and correct counting error in function acpi_system_write_alarm Yi Yang 0 siblings, 1 reply; 26+ messages in thread From: Yi Yang @ 2007-12-27 6:47 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb, acpi-bugzilla /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; } ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH linux-acpi] Remove superfluous code and correct counting error in function acpi_system_write_alarm 2007-12-27 6:47 [PATCH linux-acpi] Fix /proc/acpi/alarm set error Yi Yang @ 2007-12-27 8:41 ` Yi Yang 2007-12-29 8:22 ` [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID Yi Yang 0 siblings, 1 reply; 26+ messages in thread From: Yi Yang @ 2007-12-27 8:41 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb, acpi-bugzilla 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); ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID 2007-12-27 8:41 ` [PATCH linux-acpi] Remove superfluous code and correct counting error in function acpi_system_write_alarm Yi Yang @ 2007-12-29 8:22 ` Yi Yang 2008-01-01 23:20 ` Pavel Machek 2008-01-04 8:16 ` [PATCH linux-acpi] fix acpi fan state set error Yi Yang 0 siblings, 2 replies; 26+ messages in thread From: Yi Yang @ 2007-12-29 8:22 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb, acpi-bugzilla 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID 2007-12-29 8:22 ` [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID Yi Yang @ 2008-01-01 23:20 ` Pavel Machek 2008-01-02 2:03 ` Yi Yang 2008-01-04 8:16 ` [PATCH linux-acpi] fix acpi fan state set error Yi Yang 1 sibling, 1 reply; 26+ messages in thread From: Pavel Machek @ 2008-01-01 23:20 UTC (permalink / raw) To: Yi Yang; +Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID 2008-01-01 23:20 ` Pavel Machek @ 2008-01-02 2:03 ` Yi Yang 2008-01-02 16:09 ` Pavel Machek 0 siblings, 1 reply; 26+ messages in thread From: Yi Yang @ 2008-01-02 2:03 UTC (permalink / raw) To: Pavel Machek; +Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID 2008-01-02 2:03 ` Yi Yang @ 2008-01-02 16:09 ` Pavel Machek 2008-01-03 2:02 ` Yi Yang 2008-01-03 2:11 ` Yi Yang 0 siblings, 2 replies; 26+ messages in thread From: Pavel Machek @ 2008-01-02 16:09 UTC (permalink / raw) To: Yi Yang; +Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID 2008-01-02 16:09 ` Pavel Machek @ 2008-01-03 2:02 ` Yi Yang 2008-01-03 2:11 ` Yi Yang 1 sibling, 0 replies; 26+ messages in thread From: Yi Yang @ 2008-01-03 2:02 UTC (permalink / raw) To: Pavel Machek; +Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla 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...? > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID 2008-01-02 16:09 ` Pavel Machek 2008-01-03 2:02 ` Yi Yang @ 2008-01-03 2:11 ` Yi Yang 1 sibling, 0 replies; 26+ messages in thread From: Yi Yang @ 2008-01-03 2:11 UTC (permalink / raw) To: Pavel Machek; +Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla 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. > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH linux-acpi] fix acpi fan state set error 2007-12-29 8:22 ` [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID Yi Yang 2008-01-01 23:20 ` Pavel Machek @ 2008-01-04 8:16 ` Yi Yang 2008-01-07 6:56 ` [PATCH] ACPI: fix processor throttling " Yi Yang 1 sibling, 1 reply; 26+ messages in thread From: Yi Yang @ 2008-01-04 8:16 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb, acpi-bugzilla 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)); ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] ACPI: fix processor throttling set error 2008-01-04 8:16 ` [PATCH linux-acpi] fix acpi fan state set error Yi Yang @ 2008-01-07 6:56 ` Yi Yang 2008-01-08 3:21 ` [PATCH] ACPI: fix processor limit " Yi Yang 2008-01-09 22:21 ` [PATCH] ACPI: Add sysfs interface for acpi device wakeup Yi Yang 0 siblings, 2 replies; 26+ messages in thread From: Yi Yang @ 2008-01-07 6:56 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb, acpi-bugzilla 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; ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] ACPI: fix processor limit set error 2008-01-07 6:56 ` [PATCH] ACPI: fix processor throttling " Yi Yang @ 2008-01-08 3:21 ` Yi Yang 2008-01-24 0:45 ` [PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported Yi Yang 2008-01-09 22:21 ` [PATCH] ACPI: Add sysfs interface for acpi device wakeup Yi Yang 1 sibling, 1 reply; 26+ messages in thread From: Yi Yang @ 2008-01-08 3:21 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb, acpi-bugzilla 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; } ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported 2008-01-08 3:21 ` [PATCH] ACPI: fix processor limit " Yi Yang @ 2008-01-24 0:45 ` Yi Yang 2008-01-24 14:43 ` Mark Lord 0 siblings, 1 reply; 26+ messages in thread From: Yi Yang @ 2008-01-24 0:45 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb, acpi-bugzilla 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)); ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported 2008-01-24 0:45 ` [PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported Yi Yang @ 2008-01-24 14:43 ` Mark Lord 0 siblings, 0 replies; 26+ messages in thread From: Mark Lord @ 2008-01-24 14:43 UTC (permalink / raw) To: yi.y.yang; +Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla > 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? ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-01-07 6:56 ` [PATCH] ACPI: fix processor throttling " Yi Yang 2008-01-08 3:21 ` [PATCH] ACPI: fix processor limit " Yi Yang @ 2008-01-09 22:21 ` Yi Yang 2008-01-10 7:43 ` Maxim Levitsky 1 sibling, 1 reply; 26+ messages in thread From: Yi Yang @ 2008-01-09 22:21 UTC (permalink / raw) To: linux-acpi; +Cc: linux-kernel, lenb, acpi-bugzilla 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-01-09 22:21 ` [PATCH] ACPI: Add sysfs interface for acpi device wakeup Yi Yang @ 2008-01-10 7:43 ` Maxim Levitsky 2008-01-09 23:59 ` Yi Yang 2008-01-11 8:16 ` Zhang Rui 0 siblings, 2 replies; 26+ messages in thread From: Maxim Levitsky @ 2008-01-10 7:43 UTC (permalink / raw) To: yi.y.yang; +Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-01-10 7:43 ` Maxim Levitsky @ 2008-01-09 23:59 ` Yi Yang 2008-01-10 10:30 ` Matthew Garrett 2008-01-13 18:16 ` Pavel Machek 2008-01-11 8:16 ` Zhang Rui 1 sibling, 2 replies; 26+ messages in thread From: Yi Yang @ 2008-01-09 23:59 UTC (permalink / raw) To: Maxim Levitsky; +Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-01-09 23:59 ` Yi Yang @ 2008-01-10 10:30 ` Matthew Garrett 2008-01-13 18:16 ` Pavel Machek 1 sibling, 0 replies; 26+ messages in thread From: Matthew Garrett @ 2008-01-10 10:30 UTC (permalink / raw) To: Yi Yang; +Cc: Maxim Levitsky, linux-acpi, linux-kernel, lenb, acpi-bugzilla 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-01-09 23:59 ` Yi Yang 2008-01-10 10:30 ` Matthew Garrett @ 2008-01-13 18:16 ` Pavel Machek 1 sibling, 0 replies; 26+ messages in thread From: Pavel Machek @ 2008-01-13 18:16 UTC (permalink / raw) To: Yi Yang; +Cc: Maxim Levitsky, linux-acpi, linux-kernel, lenb, acpi-bugzilla > [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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-01-10 7:43 ` Maxim Levitsky 2008-01-09 23:59 ` Yi Yang @ 2008-01-11 8:16 ` Zhang Rui 2008-01-10 23:55 ` Yi Yang 1 sibling, 1 reply; 26+ messages in thread From: Zhang Rui @ 2008-01-11 8:16 UTC (permalink / raw) To: Maxim Levitsky, yi.y.yang, david-b Cc: linux-acpi, linux-kernel, lenb, acpi-bugzilla 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. > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-01-11 8:16 ` Zhang Rui @ 2008-01-10 23:55 ` Yi Yang 2008-03-19 13:06 ` Thomas Renninger 0 siblings, 1 reply; 26+ messages in thread From: Yi Yang @ 2008-01-10 23:55 UTC (permalink / raw) To: Zhang Rui Cc: Maxim Levitsky, david-b, linux-acpi, linux-kernel, lenb, acpi-bugzilla > > 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. > > > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-01-10 23:55 ` Yi Yang @ 2008-03-19 13:06 ` Thomas Renninger 2008-03-19 14:37 ` Yi Yang 2008-03-19 18:52 ` David Brownell 0 siblings, 2 replies; 26+ messages in thread From: Thomas Renninger @ 2008-03-19 13:06 UTC (permalink / raw) To: yi.y.yang Cc: Zhang Rui, Maxim Levitsky, david-b, linux-acpi, linux-kernel, lenb, acpi-bugzilla, Holger Macht 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-03-19 13:06 ` Thomas Renninger @ 2008-03-19 14:37 ` Yi Yang 2008-03-20 4:32 ` Zhao Yakui 2008-03-19 18:52 ` David Brownell 1 sibling, 1 reply; 26+ messages in thread From: Yi Yang @ 2008-03-19 14:37 UTC (permalink / raw) To: trenn Cc: Zhang Rui, Maxim Levitsky, david-b, linux-acpi, linux-kernel, lenb, acpi-bugzilla, Holger Macht > 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-03-19 14:37 ` Yi Yang @ 2008-03-20 4:32 ` Zhao Yakui 0 siblings, 0 replies; 26+ messages in thread From: Zhao Yakui @ 2008-03-20 4:32 UTC (permalink / raw) To: yi.y.yang Cc: trenn, Zhang Rui, Maxim Levitsky, david-b, linux-acpi, linux-kernel, lenb, acpi-bugzilla, Holger Macht 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-03-19 13:06 ` Thomas Renninger 2008-03-19 14:37 ` Yi Yang @ 2008-03-19 18:52 ` David Brownell 2008-03-20 5:12 ` Zhao Yakui 1 sibling, 1 reply; 26+ messages in thread From: David Brownell @ 2008-03-19 18:52 UTC (permalink / raw) To: trenn, yi.y.yang Cc: Zhang Rui, Maxim Levitsky, linux-acpi, linux-kernel, lenb, acpi-bugzilla, Holger Macht 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-03-19 18:52 ` David Brownell @ 2008-03-20 5:12 ` Zhao Yakui 2008-03-20 6:12 ` David Brownell 0 siblings, 1 reply; 26+ messages in thread From: Zhao Yakui @ 2008-03-20 5:12 UTC (permalink / raw) To: David Brownell Cc: trenn, yi.y.yang, Zhang Rui, Maxim Levitsky, linux-acpi, linux-kernel, lenb, acpi-bugzilla, Holger Macht 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup 2008-03-20 5:12 ` Zhao Yakui @ 2008-03-20 6:12 ` David Brownell 0 siblings, 0 replies; 26+ messages in thread From: David Brownell @ 2008-03-20 6:12 UTC (permalink / raw) To: Zhao Yakui Cc: trenn, yi.y.yang, Zhang Rui, Maxim Levitsky, linux-acpi, linux-kernel, lenb, acpi-bugzilla, Holger Macht 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. > ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2008-03-20 6:12 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-12-27 6:47 [PATCH linux-acpi] Fix /proc/acpi/alarm set error Yi Yang 2007-12-27 8:41 ` [PATCH linux-acpi] Remove superfluous code and correct counting error in function acpi_system_write_alarm Yi Yang 2007-12-29 8:22 ` [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID Yi Yang 2008-01-01 23:20 ` Pavel Machek 2008-01-02 2:03 ` Yi Yang 2008-01-02 16:09 ` Pavel Machek 2008-01-03 2:02 ` Yi Yang 2008-01-03 2:11 ` Yi Yang 2008-01-04 8:16 ` [PATCH linux-acpi] fix acpi fan state set error Yi Yang 2008-01-07 6:56 ` [PATCH] ACPI: fix processor throttling " Yi Yang 2008-01-08 3:21 ` [PATCH] ACPI: fix processor limit " Yi Yang 2008-01-24 0:45 ` [PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported Yi Yang 2008-01-24 14:43 ` Mark Lord 2008-01-09 22:21 ` [PATCH] ACPI: Add sysfs interface for acpi device wakeup Yi Yang 2008-01-10 7:43 ` Maxim Levitsky 2008-01-09 23:59 ` Yi Yang 2008-01-10 10:30 ` Matthew Garrett 2008-01-13 18:16 ` Pavel Machek 2008-01-11 8:16 ` Zhang Rui 2008-01-10 23:55 ` Yi Yang 2008-03-19 13:06 ` Thomas Renninger 2008-03-19 14:37 ` Yi Yang 2008-03-20 4:32 ` Zhao Yakui 2008-03-19 18:52 ` David Brownell 2008-03-20 5:12 ` Zhao Yakui 2008-03-20 6:12 ` David Brownell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).