[patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[David Brownell]

Add cross-links between ACPI device and "real" devices in sysfs,
exposing otherwise-hidden interrelationships between the various
device nodes for ACPI stuff.  As a representative example, one
hardware device is exposed as two logical devices (PNP and ACPI):

  .../pnp0/00:06/
  .../LNXSYSTM:00/device:00/PNP0A03:00/device:15/PNP0B00:00/

The PNP device gets a "firmware_node" link pointing to the ACPI device,
and is what a Linux device driver binds to.  The ACPI device has instead
a "physical_node" link pointing back to the PNP device.  Other firmware
frameworks, like OpenFirmware, could do the same thing to couple their
firmware tables to the rest of the system.

(Based on a patch from Zhang Rui.  This version is modified to not
depend on the patch makig ACPI initialize driver model wakeup flags.)

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Cc: Zhang Rui <rui.zhang@intel.com> 
---
Another extract from the "teach ACPI how to use driver model wakeup
flags" patch series.  The linkage helps sort out confusion about which
devices are wakeup-capable, among other things, and can help when
interpreting /proc/acpi/wakeup contents.

 drivers/acpi/glue.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

--- g26.orig/drivers/acpi/glue.c	2008-02-22 20:39:19.000000000 -0800
+++ g26/drivers/acpi/glue.c	2008-02-22 20:46:40.000000000 -0800
@@ -142,6 +142,7 @@ EXPORT_SYMBOL(acpi_get_physical_device);
 
 static int acpi_bind_one(struct device *dev, acpi_handle handle)
 {
+	struct acpi_device *acpi_dev;
 	acpi_status status;
 
 	if (dev->archdata.acpi_handle) {
@@ -157,6 +158,16 @@ static int acpi_bind_one(struct device *
 	}
 	dev->archdata.acpi_handle = handle;
 
+	status = acpi_bus_get_device(handle, &acpi_dev);
+	if (!ACPI_FAILURE(status)) {
+		int ret;
+
+		ret = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
+				"firmware_node");
+		ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
+				"physical_node");
+	}
+
 	return 0;
 }
 
@@ -165,8 +176,17 @@ static int acpi_unbind_one(struct device
 	if (!dev->archdata.acpi_handle)
 		return 0;
 	if (dev == acpi_get_physical_device(dev->archdata.acpi_handle)) {
+		struct acpi_device *acpi_dev;
+
 		/* 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, "firmware_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;

Re: [patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[Zhang, Rui]

On Sat, 2008-02-23 at 13:54 +0800, David Brownell wrote:
> Add cross-links between ACPI device and "real" devices in sysfs,
> exposing otherwise-hidden interrelationships between the various
> device nodes for ACPI stuff.  As a representative example, one
> hardware device is exposed as two logical devices (PNP and ACPI):
> 
>   .../pnp0/00:06/
>   .../LNXSYSTM:00/device:00/PNP0A03:00/device:15/PNP0B00:00/
> 
> The PNP device gets a "firmware_node" link pointing to the ACPI
> device,
> and is what a Linux device driver binds to.  The ACPI device has
> instead
> a "physical_node" link pointing back to the PNP device.  Other
> firmware
> frameworks, like OpenFirmware, could do the same thing to couple their
> firmware tables to the rest of the system.
> 
> (Based on a patch from Zhang Rui.  This version is modified to not
> depend on the patch makig ACPI initialize driver model wakeup flags.)
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> Cc: Zhang Rui <rui.zhang@intel.com>
> ---
> Another extract from the "teach ACPI how to use driver model wakeup
> flags" patch series.  The linkage helps sort out confusion about which
> devices are wakeup-capable, among other things, and can help when
> interpreting /proc/acpi/wakeup contents.
> 
>  drivers/acpi/glue.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> --- g26.orig/drivers/acpi/glue.c        2008-02-22 20:39:19.000000000
> -0800
> +++ g26/drivers/acpi/glue.c     2008-02-22 20:46:40.000000000 -0800
> @@ -142,6 +142,7 @@ EXPORT_SYMBOL(acpi_get_physical_device);
> 
>  static int acpi_bind_one(struct device *dev, acpi_handle handle)
>  {
> +       struct acpi_device *acpi_dev;
>         acpi_status status;
> 
>         if (dev->archdata.acpi_handle) {
> @@ -157,6 +158,16 @@ static int acpi_bind_one(struct device *
>         }
>         dev->archdata.acpi_handle = handle;
> 
> +       status = acpi_bus_get_device(handle, &acpi_dev);
> +       if (!ACPI_FAILURE(status)) {
> +               int ret;
> +
> +               ret = sysfs_create_link(&dev->kobj,
> &acpi_dev->dev.kobj,
> +                               "firmware_node");
> +               ret = sysfs_create_link(&acpi_dev->dev.kobj,
> &dev->kobj,
> +                               "physical_node");
> +       }
ret is not needed here...

>         return 0;
>  }
> 
> @@ -165,8 +176,17 @@ static int acpi_unbind_one(struct device
>         if (!dev->archdata.acpi_handle)
>                 return 0;
>         if (dev ==
> acpi_get_physical_device(dev->archdata.acpi_handle)) {
> +               struct acpi_device *acpi_dev;
> +
>                 /* 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,
> "firmware_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;
> 
> 

Re: [patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[Zhang, Rui]

On Tue, 2008-02-26 at 14:45 +0800, David Brownell wrote:
> On Monday 25 February 2008, Zhang, Rui wrote:
> > > +       if (!ACPI_FAILURE(status)) {
> > > +               int ret;
> > > +
> > > +               ret = sysfs_create_link(&dev->kobj,
> &acpi_dev->dev.kobj,
> > > +                               "firmware_node");
> > > +               ret = sysfs_create_link(&acpi_dev->dev.kobj,
> &dev->kobj,
> > > +                               "physical_node");
> > > +       }
> >
> > ret is not needed here...
> 
> But what compiler warnings do you see when you omit it?  :)
I tried but NO warnings at all

thanks,
rui

Re: [patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[David Brownell]

On Monday 25 February 2008, Zhang, Rui wrote:
> > +       if (!ACPI_FAILURE(status)) {
> > +               int ret;
> > +
> > +               ret = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
> > +                               "firmware_node");
> > +               ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
> > +                               "physical_node");
> > +       }
>
> ret is not needed here...

But what compiler warnings do you see when you omit it?  :)

-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[David Brownell]

On Monday 25 February 2008, Zhang, Rui wrote:
> 
> On Tue, 2008-02-26 at 14:45 +0800, David Brownell wrote:
> > On Monday 25 February 2008, Zhang, Rui wrote:
> > > > +       if (!ACPI_FAILURE(status)) {
> > > > +               int ret;
> > > > +
> > > > +               ret = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
> > > > +                               "firmware_node");
> > > > +               ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
> > > > +                               "physical_node");
> > > > +       }
> > >
> > > ret is not needed here...
> > 
> > But what compiler warnings do you see when you omit it?  :)
>
> I tried but NO warnings at all

Really???  In <linux/sysfs.h> I found:

int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
                                   const char *name);

Perhaps you disabled those warnings.  (And FWIW, I don't see any
reasonable way to handle such faults, so ignoring them is thus
the right "solution".)

Re: [patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[Henrique de Moraes Holschuh]

On Mon, 25 Feb 2008, David Brownell wrote:
> int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
>                                    const char *name);
> 
> Perhaps you disabled those warnings.  (And FWIW, I don't see any
> reasonable way to handle such faults, so ignoring them is thus
> the right "solution".)

You issue a printk warning the user, and try to continue.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[David Brownell]

On Tuesday 26 February 2008, Henrique de Moraes Holschuh wrote:
> On Mon, 25 Feb 2008, David Brownell wrote:
> > int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
> >                                    const char *name);
> > 
> > Perhaps you disabled those warnings.  (And FWIW, I don't see any
> > reasonable way to handle such faults, so ignoring them is thus
> > the right "solution".)
> 
> You issue a printk warning the user, and try to continue.

That's not "handling" in any useful sense though.  There's no
question about "try" to continue, by the way ... from inside
the kernel this information is completely non-essential.

- Dave

Re: [patch 2.6.25-rc2-git] crosslink ACPI and "real" device nodes

[David Brownell]

On Tuesday 26 February 2008, David Brownell wrote:
> On Tuesday 26 February 2008, Henrique de Moraes Holschuh wrote:
> > On Mon, 25 Feb 2008, David Brownell wrote:
> > > int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
> > >                                    const char *name);
> > > 
> > > Perhaps you disabled those warnings.  (And FWIW, I don't see any
> > > reasonable way to handle such faults, so ignoring them is thus
> > > the right "solution".)
> > 
> > You issue a printk warning the user, and try to continue.
> 
> That's not "handling" in any useful sense though.  There's no
> question about "try" to continue, by the way ... from inside
> the kernel this information is completely non-essential.

Here's a version with pr_debug() to report those case.
Against git-current.

======== CUT HERE
From: David Brownell <dbrownell@users.sourceforge.net>

Add cross-links between ACPI device and "real" devices in sysfs,
exposing otherwise-hidden interrelationships between the various
device nodes which ACPI creates.  As a representative example one
hardware device is exposed as two logical devices (PNP and ACPI):

  .../pnp0/00:06/
  .../LNXSYSTM:00/device:00/PNP0A03:00/device:15/PNP0B00:00/

The PNP device gets a "firmware_node" link pointing to the ACPI device.
The ACPI device has a "physical_node" link pointing to the PNP device.

Linux drivers currently bind only to the "physical" device nodes.

Other device trees provided by firmware tables could be exported
using the same scheme; these names aren't ACPI-specific.

(Based on a patch from Zhang Rui.  This version is modified to not
depend on the patch teaching ACPI about driver model wakeup flags.)

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Cc: Zhang Rui <rui.zhang@intel.com> 
---
Change from previous version:  if creating the link fails, a
pr_debug level message is emitted.

 drivers/acpi/glue.c |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

--- g26.orig/drivers/acpi/glue.c	2008-03-08 11:11:18.000000000 -0800
+++ g26/drivers/acpi/glue.c	2008-03-08 11:29:07.000000000 -0800
@@ -142,6 +142,7 @@ EXPORT_SYMBOL(acpi_get_physical_device);
 
 static int acpi_bind_one(struct device *dev, acpi_handle handle)
 {
+	struct acpi_device *acpi_dev;
 	acpi_status status;
 
 	if (dev->archdata.acpi_handle) {
@@ -157,6 +158,23 @@ static int acpi_bind_one(struct device *
 	}
 	dev->archdata.acpi_handle = handle;
 
+	status = acpi_bus_get_device(handle, &acpi_dev);
+	if (!ACPI_FAILURE(status)) {
+		int ret;
+
+		ret = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
+				"firmware_node");
+		if (ret != 0)
+			pr_debug(PREFIX "Can't create %s link for %s\n",
+				"firmware_node", dev->bus_id);
+
+		ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
+				"physical_node");
+		if (ret != 0)
+			pr_debug(PREFIX "Can't create %s link for %s\n",
+				"physical_node", acpi_dev->dev.bus_id);
+	}
+
 	return 0;
 }
 
@@ -165,8 +183,17 @@ static int acpi_unbind_one(struct device
 	if (!dev->archdata.acpi_handle)
 		return 0;
 	if (dev == acpi_get_physical_device(dev->archdata.acpi_handle)) {
+		struct acpi_device *acpi_dev;
+
 		/* 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, "firmware_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;