Re: [patch 2.6.21-rc5-git] make /proc/acpi/wakeup more useful

David Brownell
2007-04-06T15:43:34Z

On Friday 06 April 2007 2:36 am, Zhang Rui wrote:
> On Thu, 2007-04-05 at 03:58 -0700, David Brownell wrote:
> >
> > For wakeup devices, the main issue I've seen is with button devices.
> > In my limited set of test sytems, everything else is either PCI, PNP,
> > or a bug (listing a non-existent device).
>
> There may be other wakeup-enabled devices like legacy serial,
> PS2 devices described in ACPI name space.

Do you mean they wouldn't be listed in /proc/acpi/wakeup though?

Or just that they wouldn't be listed as PNP devices (like the button
devices)? That seems buglike. Not that BIOS bugs are so rare; and
I sure wouldn't know which types are common...

Because that example file certainly included UART and PS2 devices,
which were listed both as PNP devices and as ACPI devices.

> > If this patch starts to get deployed, I expect other people will find
> > a few other curiousities ... and likely some things to be fixed.
>
> > The /sys/devices/acpi_system:00/ tree is kind of new. I suspect one
> > way it could be more informative is to set up cross-links in sysfs
> > between the ACPI devices and the "real" device nodes ... e.g. on the
> > system I'm using right now .../device:00/PNP0A03:00/device:15/PNP0B00:00
> > could have a link pointing to /sys/devices/pnp0/00:06 ... and that PNP
> > node in turn could have an "acpi" link pointing back to the ACPI thing.
> >
> > Such cross-links would let people see those relationships, and observe
> > which links are missing or otherwise strange. Fixing the bugs would
> > seem unlikely until those things become visible.
>
> Sounds nice.
> The patch below should make sense.

Yeah, that's the idea. Let's see if it's any good. :)

I'd just call it "acpi_node" not "ACPI_node" (NO POINT IN SHOUTING),
and might not even use the "_node" suffix.

I cc'd Greg, who's our resident (or is that itinerant?) sysfs guru,
in case he has comments on this issue.

This applied over the "ACPI driver model flags and platform_enable_wake()"
patch with a bit of fuzz, on RC6.

- Dave

> Add cross-links between ACPI device and "real" devices in sysfs.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> drivers/acpi/glue.c | 14 ++++++++++++++
> 1 files changed, 14 insertions(+)
>
> Index: linux-2.6.21-rc5/drivers/acpi/glue.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/drivers/acpi/glue.c 2007-04-06 16:06:04.000000000 +0800
> +++ linux-2.6.21-rc5/drivers/acpi/glue.c 2007-04-06 17:32:40.000000000 +0800
> @@ -147,6 +147,7 @@ EXPORT_SYMBOL(acpi_get_physical_device);
> static int acpi_bind_one(struct device *dev, acpi_handle handle)
> {
> acpi_status status;
> + struct acpi_device *acpi_dev;
>
> if (dev->archdata.acpi_handle) {
> printk(KERN_WARNING PREFIX
> @@ -161,16 +162,29 @@ static int acpi_bind_one(struct device *
> }
> dev->archdata.acpi_handle = handle;
>
> + if (!acpi_bus_get_device(handle, &acpi_dev)) {
> + sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj, "ACPI_node");
> + sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj, "physical_node");
> + }
> +
> return 0;
> }
>
> static int acpi_unbind_one(struct device *dev)
> {
> + struct acpi_device *acpi_dev;
> +
> if (!dev->archdata.acpi_handle)
> return 0;
> if (dev == acpi_get_physical_device(dev->archdata.acpi_handle)) {
> /* acpi_get_physical_device increase refcnt by one */
> put_device(dev);
> +
> + if (!acpi_bus_get_device(dev->archdata.acpi_handle, &acpi_dev)) {
> + sysfs_remove_link(&dev->kobj, "ACPI_node");
> + sysfs_remove_link(&acpi_dev->dev.kobj, "physical_node");
> + }
> +
> acpi_detach_data(dev->archdata.acpi_handle,
> acpi_glue_data_handler);
> dev->archdata.acpi_handle = NULL;
>