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

[David Brownell <david-b@pacbell.net>]

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;
>