[PATCH V2] ACPI: Allow acpi binding with usb3.0 hub

[PATCH V2] ACPI: Allow acpi binding with usb3.0 hub

[Lan Tianyu]

usb port's position and connectability can't be identified on some boards
via usb hub's register. ACPI _UPC and _PLD can help to resolve such issue
and so it is necessary to bind usb with acpi. This patch is to allow acpi
binding with usb3.0 hub.

Current acpi only can bind one struct device to one acpi device node.
This can not work with usb3.0 hub. Because the usb3.0 hub has two logical
devices. Each works for usb2.0 and usb3.0 devices. In the liux usb subsystem,
those two logical hubs are treated as two seperate devices that have two struct
devices. But in the ACPI DSDT, these two logical hubs share one acpi device
node. So there is a requirement to bind multi struct devices to one acpi
node. This patch is to resolve such problem.

Following is the acpi device nodes' description under xhci hcd.
Device (XHC)
            Device (RHUB)
                Device (HSP1)
                Device (HSP2)
                Device (HSP3)
                Device (HSP4)
                Device (SSP1)
                Device (SSP2)
                Device (SSP3)
                Device (SSP4)

Topology in the linux
	device XHC
	   usb2.0 logical hub    usb3.0 logical hub
		HSP1			SSP1
		HSP2			SSP2
		HSP3			SSP3
		HSP4			SSP4

This patch also modifies the output of /proc/acpi/wakeup. One acpi node
can be associated with multi devices. Just like following.

XHC		S4	*enabled	pci:0000:00:14.0
RHUB	S0	disabled	usb:usb1
			disabled	usb:usb2

Signed-off-by: Lan Tianyu <tianyu.lan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
v2: Add lock to protect physical_node related data in bind() and unbind().
Add an example here of the syntax change to the contents of /proc/acpi/wakeup. Replace
link file name physical_node0 with physical_node when physical_node_id is 0.
---
 drivers/acpi/glue.c        |  135 ++++++++++++++++++++++++++++----------------
 drivers/acpi/proc.c        |   57 ++++++++++++-------
 drivers/acpi/scan.c        |    2 +
 drivers/pnp/pnpacpi/core.c |    7 +--
 include/acpi/acpi_bus.h    |   15 ++++-
 5 files changed, 139 insertions(+), 77 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 243ee85..d1a2d74 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -25,6 +25,8 @@
 static LIST_HEAD(bus_type_list);
 static DECLARE_RWSEM(bus_type_sem);
 
+#define PHYSICAL_NODE_STRING "physical_node"
+
 int register_acpi_bus_type(struct acpi_bus_type *type)
 {
 	if (acpi_disabled)
@@ -124,84 +126,119 @@ acpi_handle acpi_get_child(acpi_handle parent, u64 address)
 
 EXPORT_SYMBOL(acpi_get_child);
 
-/* Link ACPI devices with physical devices */
-static void acpi_glue_data_handler(acpi_handle handle,
-				   void *context)
-{
-	/* we provide an empty handler */
-}
-
-/* Note: a success call will increase reference count by one */
-struct device *acpi_get_physical_device(acpi_handle handle)
-{
-	acpi_status status;
-	struct device *dev;
-
-	status = acpi_get_data(handle, acpi_glue_data_handler, (void **)&dev);
-	if (ACPI_SUCCESS(status))
-		return get_device(dev);
-	return NULL;
-}
-
-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;
+	struct acpi_device_physical_node *physical_node;
+	char physical_node_name[sizeof(PHYSICAL_NODE_STRING) + 2];
+	int retval = -EINVAL;
 
 	if (dev->archdata.acpi_handle) {
 		dev_warn(dev, "Drivers changed 'acpi_handle'\n");
 		return -EINVAL;
 	}
+
 	get_device(dev);
-	status = acpi_attach_data(handle, acpi_glue_data_handler, dev);
-	if (ACPI_FAILURE(status)) {
-		put_device(dev);
-		return -EINVAL;
+	status = acpi_bus_get_device(handle, &acpi_dev);
+	if (ACPI_FAILURE(status))
+		goto err;
+
+	physical_node = kzalloc(sizeof(struct acpi_device_physical_node),
+		GFP_KERNEL);
+	if (!physical_node) {
+		retval = -ENOMEM;
+		goto err;
 	}
-	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");
-		if (acpi_dev->wakeup.flags.valid)
-			device_set_wakeup_capable(dev, true);
+	mutex_lock(&acpi_dev->physical_node_lock);
+	/* allocate physical node id according to physical_node_id_bitmap */
+	physical_node->node_id =
+		find_first_zero_bit(acpi_dev->physical_node_id_bitmap,
+		ACPI_MAX_PHYSICAL_NODE);
+	if (physical_node->node_id >= ACPI_MAX_PHYSICAL_NODE) {
+		retval = -ENOSPC;
+		mutex_unlock(&acpi_dev->physical_node_lock);
+		goto err;
 	}
 
+	set_bit(physical_node->node_id, acpi_dev->physical_node_id_bitmap);
+	physical_node->dev = dev;
+	list_add_tail(&physical_node->node, &acpi_dev->physical_node_list);
+	acpi_dev->physical_node_count++;
+	mutex_unlock(&acpi_dev->physical_node_lock);
+
+	dev->archdata.acpi_handle = handle;
+
+	if (!physical_node->node_id)
+		strcpy(physical_node_name, PHYSICAL_NODE_STRING);
+	else
+		sprintf(physical_node_name,
+			"physical_node%d", physical_node->node_id);
+	retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
+			physical_node_name);
+	retval = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
+		"firmware_node");
+
+	if (acpi_dev->wakeup.flags.valid)
+		device_set_wakeup_capable(dev, true);
+
 	return 0;
+
+ err:
+	put_device(dev);
+	return retval;
 }
 
 static int acpi_unbind_one(struct device *dev)
 {
+	struct acpi_device_physical_node *entry;
+	struct acpi_device *acpi_dev;
+	acpi_status status;
+	struct list_head *node, *next;
+
 	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);
+	status = acpi_bus_get_device(dev->archdata.acpi_handle,
+		&acpi_dev);
+	if (ACPI_FAILURE(status))
+		goto err;
 
-		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");
-		}
+	mutex_lock(&acpi_dev->physical_node_lock);
+	list_for_each_safe(node, next, &acpi_dev->physical_node_list) {
+		char physical_node_name[sizeof(PHYSICAL_NODE_STRING) + 2];
+
+		entry = list_entry(node, struct acpi_device_physical_node,
+			node);
+		if (entry->dev != dev)
+			continue;
+
+		list_del(node);
+		clear_bit(entry->node_id, acpi_dev->physical_node_id_bitmap);
 
-		acpi_detach_data(dev->archdata.acpi_handle,
-				 acpi_glue_data_handler);
+		acpi_dev->physical_node_count--;
+
+		if (!entry->node_id)
+			strcpy(physical_node_name, PHYSICAL_NODE_STRING);
+		else
+			sprintf(physical_node_name,
+				"physical_node%d", entry->node_id);
+
+		sysfs_remove_link(&acpi_dev->dev.kobj, physical_node_name);
+		sysfs_remove_link(&dev->kobj, "firmware_node");
 		dev->archdata.acpi_handle = NULL;
 		/* acpi_bind_one increase refcnt by one */
 		put_device(dev);
-	} else {
-		dev_err(dev, "Oops, 'acpi_handle' corrupt\n");
+		kfree(entry);
 	}
+	mutex_unlock(&acpi_dev->physical_node_lock);
+
 	return 0;
+
+err:
+	dev_err(dev, "Oops, 'acpi_handle' corrupt\n");
+	return -EINVAL;
 }
 
 static int acpi_platform_notify(struct device *dev)
diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
index 251c7b62..e4c242f 100644
--- a/drivers/acpi/proc.c
+++ b/drivers/acpi/proc.c
@@ -302,26 +302,41 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
 	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
 		struct acpi_device *dev =
 		    container_of(node, struct acpi_device, wakeup_list);
-		struct device *ldev;
+		struct acpi_device_physical_node *entry;
 
 		if (!dev->wakeup.flags.valid)
 			continue;
 
-		ldev = acpi_get_physical_device(dev->handle);
-		seq_printf(seq, "%s\t  S%d\t%c%-8s  ",
+		seq_printf(seq, "%s\t  S%d\t",
 			   dev->pnp.bus_id,
-			   (u32) dev->wakeup.sleep_state,
-			   dev->wakeup.flags.run_wake ? '*' : ' ',
-			   (device_may_wakeup(&dev->dev)
-			     || (ldev && device_may_wakeup(ldev))) ?
-			       "enabled" : "disabled");
-		if (ldev)
-			seq_printf(seq, "%s:%s",
-				   ldev->bus ? ldev->bus->name : "no-bus",
-				   dev_name(ldev));
-		seq_printf(seq, "\n");
-		put_device(ldev);
-
+			   (u32) dev->wakeup.sleep_state);
+
+		if (!dev->physical_node_count)
+			seq_printf(seq, "%c%-8s\n",
+				dev->wakeup.flags.run_wake ?
+				'*' : ' ', "disabled");
+		else {
+			struct device *ldev;
+			list_for_each_entry(entry, &dev->physical_node_list,
+					node) {
+				ldev = get_device(entry->dev);
+				if (!ldev)
+					continue;
+
+				if (&entry->node !=
+						dev->physical_node_list.next)
+					seq_printf(seq, "\t\t");
+
+				seq_printf(seq, "%c%-8s  %s:%s\n",
+					dev->wakeup.flags.run_wake ? '*' : ' ',
+					(device_may_wakeup(&dev->dev) ||
+					(ldev && device_may_wakeup(ldev))) ?
+					"enabled" : "disabled",
+					ldev->bus ? ldev->bus->name :
+					"no-bus", dev_name(ldev));
+				put_device(ldev);
+			}
+		}
 	}
 	mutex_unlock(&acpi_device_lock);
 	return 0;
@@ -329,12 +344,14 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
 
 static void physical_device_enable_wakeup(struct acpi_device *adev)
 {
-	struct device *dev = acpi_get_physical_device(adev->handle);
+	struct acpi_device_physical_node *entry;
 
-	if (dev && device_can_wakeup(dev)) {
-		bool enable = !device_may_wakeup(dev);
-		device_set_wakeup_enable(dev, enable);
-	}
+	list_for_each_entry(entry,
+		&adev->physical_node_list, node)
+		if (entry->dev && device_can_wakeup(entry->dev)) {
+			bool enable = !device_may_wakeup(entry->dev);
+			device_set_wakeup_enable(entry->dev, enable);
+		}
 }
 
 static ssize_t
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d1ecca2..d730a93 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -481,6 +481,8 @@ static int acpi_device_register(struct acpi_device *device)
 	INIT_LIST_HEAD(&device->children);
 	INIT_LIST_HEAD(&device->node);
 	INIT_LIST_HEAD(&device->wakeup_list);
+	INIT_LIST_HEAD(&device->physical_node_list);
+	mutex_init(&device->physical_node_lock);
 
 	new_bus_id = kzalloc(sizeof(struct acpi_device_bus_id), GFP_KERNEL);
 	if (!new_bus_id) {
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index 507a8e2..26b5d4b 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -321,14 +321,9 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
 {
 	struct acpi_device *acpi = to_acpi_device(dev);
 	struct pnp_dev *pnp = _pnp;
-	struct device *physical_device;
-
-	physical_device = acpi_get_physical_device(acpi->handle);
-	if (physical_device)
-		put_device(physical_device);
 
 	/* true means it matched */
-	return !physical_device
+	return !acpi->physical_node_count
 	    && compare_pnp_id(pnp->id, acpi_device_hid(acpi));
 }
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index bde976e..453596d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -282,8 +282,16 @@ struct acpi_device_wakeup {
 	int prepare_count;
 };
 
-/* Device */
+struct acpi_device_physical_node {
+	u8 node_id;
+	struct list_head node;
+	struct device *dev;
+};
 
+/* set maximum of physical nodes to 32 for expansibility */
+#define ACPI_MAX_PHYSICAL_NODE	32
+
+/* Device */
 struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
@@ -304,6 +312,10 @@ struct acpi_device {
 	struct device dev;
 	struct acpi_bus_ops bus_ops;	/* workaround for different code path for hotplug */
 	enum acpi_bus_removal_type removal_type;	/* indicate for different removal type */
+	u8 physical_node_count;
+	struct list_head physical_node_list;
+	struct mutex physical_node_lock;
+	DECLARE_BITMAP(physical_node_id_bitmap, ACPI_MAX_PHYSICAL_NODE);
 };
 
 static inline void *acpi_driver_data(struct acpi_device *d)
@@ -394,7 +406,6 @@ struct acpi_bus_type {
 };
 int register_acpi_bus_type(struct acpi_bus_type *);
 int unregister_acpi_bus_type(struct acpi_bus_type *);
-struct device *acpi_get_physical_device(acpi_handle);
 
 struct acpi_pci_root {
 	struct list_head node;
-- 
1.7.9.5

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

Re: [PATCH V2] ACPI: Allow acpi binding with usb3.0 hub

[Sarah Sharp]

Len, do you have any comments on the ACPI code itself?  The description
about why the code is needed for the USB core seems sane, but the actual
patch code is all ACPI core stuff, so you need to review that.

On Fri, Aug 17, 2012 at 02:44:09PM +0800, Lan Tianyu wrote:
> usb port's position and connectability can't be identified on some boards
> via usb hub's register. ACPI _UPC and _PLD can help to resolve such issue
> and so it is necessary to bind usb with acpi. This patch is to allow acpi
> binding with usb3.0 hub.
> 
> Current acpi only can bind one struct device to one acpi device node.
> This can not work with usb3.0 hub. Because the usb3.0 hub has two logical
> devices. Each works for usb2.0 and usb3.0 devices. In the liux usb subsystem,
> those two logical hubs are treated as two seperate devices that have two struct
> devices. But in the ACPI DSDT, these two logical hubs share one acpi device
> node. So there is a requirement to bind multi struct devices to one acpi
> node. This patch is to resolve such problem.
> 
> Following is the acpi device nodes' description under xhci hcd.
> Device (XHC)
>             Device (RHUB)
>                 Device (HSP1)
>                 Device (HSP2)
>                 Device (HSP3)
>                 Device (HSP4)
>                 Device (SSP1)
>                 Device (SSP2)
>                 Device (SSP3)
>                 Device (SSP4)
> 
> Topology in the linux
> 	device XHC
> 	   usb2.0 logical hub    usb3.0 logical hub
> 		HSP1			SSP1
> 		HSP2			SSP2
> 		HSP3			SSP3
> 		HSP4			SSP4

Hi Tianyu,

You're correct that the USB core and the xHCI driver has a slightly
different architecture than the ACPI representation.  We have one PCI
device that has two USB buses/roothubs underneath it.  That's because
external USB 3.0 hubs show up as two USB devices: a USB 3.0 only
portion, and a USB 2.0 portion.  We wanted roothubs to act the same way.

The architecture looks something like this:

      struct pci_dev
      |
struct usb_hcd *primary_hcd -- struct usb_hcd *secondary_hcd
      |                               |
struct usb_bus                  struct usb_bus
      |                               |
 struct usb_device *root_hub     struct usb_device *root_hub
      |                               |
   struct device                   struct device

Where the primary_hcd is for the USB 2.0 roothub, and the secondary_hcd
is for the USB 3.0 roothub.

So you're trying to bind one ACPI node (RHUB) to the struct device
contained in those two root_hubs, correct?  That goal seems sane to me.

Sarah Sharp

> 
> This patch also modifies the output of /proc/acpi/wakeup. One acpi node
> can be associated with multi devices. Just like following.
> 
> XHC		S4	*enabled	pci:0000:00:14.0
> RHUB	S0	disabled	usb:usb1
> 			disabled	usb:usb2
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> v2: Add lock to protect physical_node related data in bind() and unbind().
> Add an example here of the syntax change to the contents of /proc/acpi/wakeup. Replace
> link file name physical_node0 with physical_node when physical_node_id is 0.
> ---
>  drivers/acpi/glue.c        |  135 ++++++++++++++++++++++++++++----------------
>  drivers/acpi/proc.c        |   57 ++++++++++++-------
>  drivers/acpi/scan.c        |    2 +
>  drivers/pnp/pnpacpi/core.c |    7 +--
>  include/acpi/acpi_bus.h    |   15 ++++-
>  5 files changed, 139 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
> index 243ee85..d1a2d74 100644
> --- a/drivers/acpi/glue.c
> +++ b/drivers/acpi/glue.c
> @@ -25,6 +25,8 @@
>  static LIST_HEAD(bus_type_list);
>  static DECLARE_RWSEM(bus_type_sem);
>  
> +#define PHYSICAL_NODE_STRING "physical_node"
> +
>  int register_acpi_bus_type(struct acpi_bus_type *type)
>  {
>  	if (acpi_disabled)
> @@ -124,84 +126,119 @@ acpi_handle acpi_get_child(acpi_handle parent, u64 address)
>  
>  EXPORT_SYMBOL(acpi_get_child);
>  
> -/* Link ACPI devices with physical devices */
> -static void acpi_glue_data_handler(acpi_handle handle,
> -				   void *context)
> -{
> -	/* we provide an empty handler */
> -}
> -
> -/* Note: a success call will increase reference count by one */
> -struct device *acpi_get_physical_device(acpi_handle handle)
> -{
> -	acpi_status status;
> -	struct device *dev;
> -
> -	status = acpi_get_data(handle, acpi_glue_data_handler, (void **)&dev);
> -	if (ACPI_SUCCESS(status))
> -		return get_device(dev);
> -	return NULL;
> -}
> -
> -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;
> +	struct acpi_device_physical_node *physical_node;
> +	char physical_node_name[sizeof(PHYSICAL_NODE_STRING) + 2];
> +	int retval = -EINVAL;
>  
>  	if (dev->archdata.acpi_handle) {
>  		dev_warn(dev, "Drivers changed 'acpi_handle'\n");
>  		return -EINVAL;
>  	}
> +
>  	get_device(dev);
> -	status = acpi_attach_data(handle, acpi_glue_data_handler, dev);
> -	if (ACPI_FAILURE(status)) {
> -		put_device(dev);
> -		return -EINVAL;
> +	status = acpi_bus_get_device(handle, &acpi_dev);
> +	if (ACPI_FAILURE(status))
> +		goto err;
> +
> +	physical_node = kzalloc(sizeof(struct acpi_device_physical_node),
> +		GFP_KERNEL);
> +	if (!physical_node) {
> +		retval = -ENOMEM;
> +		goto err;
>  	}
> -	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");
> -		if (acpi_dev->wakeup.flags.valid)
> -			device_set_wakeup_capable(dev, true);
> +	mutex_lock(&acpi_dev->physical_node_lock);
> +	/* allocate physical node id according to physical_node_id_bitmap */
> +	physical_node->node_id =
> +		find_first_zero_bit(acpi_dev->physical_node_id_bitmap,
> +		ACPI_MAX_PHYSICAL_NODE);
> +	if (physical_node->node_id >= ACPI_MAX_PHYSICAL_NODE) {
> +		retval = -ENOSPC;
> +		mutex_unlock(&acpi_dev->physical_node_lock);
> +		goto err;
>  	}
>  
> +	set_bit(physical_node->node_id, acpi_dev->physical_node_id_bitmap);
> +	physical_node->dev = dev;
> +	list_add_tail(&physical_node->node, &acpi_dev->physical_node_list);
> +	acpi_dev->physical_node_count++;
> +	mutex_unlock(&acpi_dev->physical_node_lock);
> +
> +	dev->archdata.acpi_handle = handle;
> +
> +	if (!physical_node->node_id)
> +		strcpy(physical_node_name, PHYSICAL_NODE_STRING);
> +	else
> +		sprintf(physical_node_name,
> +			"physical_node%d", physical_node->node_id);
> +	retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
> +			physical_node_name);
> +	retval = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
> +		"firmware_node");
> +
> +	if (acpi_dev->wakeup.flags.valid)
> +		device_set_wakeup_capable(dev, true);
> +
>  	return 0;
> +
> + err:
> +	put_device(dev);
> +	return retval;
>  }
>  
>  static int acpi_unbind_one(struct device *dev)
>  {
> +	struct acpi_device_physical_node *entry;
> +	struct acpi_device *acpi_dev;
> +	acpi_status status;
> +	struct list_head *node, *next;
> +
>  	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);
> +	status = acpi_bus_get_device(dev->archdata.acpi_handle,
> +		&acpi_dev);
> +	if (ACPI_FAILURE(status))
> +		goto err;
>  
> -		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");
> -		}
> +	mutex_lock(&acpi_dev->physical_node_lock);
> +	list_for_each_safe(node, next, &acpi_dev->physical_node_list) {
> +		char physical_node_name[sizeof(PHYSICAL_NODE_STRING) + 2];
> +
> +		entry = list_entry(node, struct acpi_device_physical_node,
> +			node);
> +		if (entry->dev != dev)
> +			continue;
> +
> +		list_del(node);
> +		clear_bit(entry->node_id, acpi_dev->physical_node_id_bitmap);
>  
> -		acpi_detach_data(dev->archdata.acpi_handle,
> -				 acpi_glue_data_handler);
> +		acpi_dev->physical_node_count--;
> +
> +		if (!entry->node_id)
> +			strcpy(physical_node_name, PHYSICAL_NODE_STRING);
> +		else
> +			sprintf(physical_node_name,
> +				"physical_node%d", entry->node_id);
> +
> +		sysfs_remove_link(&acpi_dev->dev.kobj, physical_node_name);
> +		sysfs_remove_link(&dev->kobj, "firmware_node");
>  		dev->archdata.acpi_handle = NULL;
>  		/* acpi_bind_one increase refcnt by one */
>  		put_device(dev);
> -	} else {
> -		dev_err(dev, "Oops, 'acpi_handle' corrupt\n");
> +		kfree(entry);
>  	}
> +	mutex_unlock(&acpi_dev->physical_node_lock);
> +
>  	return 0;
> +
> +err:
> +	dev_err(dev, "Oops, 'acpi_handle' corrupt\n");
> +	return -EINVAL;
>  }
>  
>  static int acpi_platform_notify(struct device *dev)
> diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
> index 251c7b62..e4c242f 100644
> --- a/drivers/acpi/proc.c
> +++ b/drivers/acpi/proc.c
> @@ -302,26 +302,41 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
>  	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
>  		struct acpi_device *dev =
>  		    container_of(node, struct acpi_device, wakeup_list);
> -		struct device *ldev;
> +		struct acpi_device_physical_node *entry;
>  
>  		if (!dev->wakeup.flags.valid)
>  			continue;
>  
> -		ldev = acpi_get_physical_device(dev->handle);
> -		seq_printf(seq, "%s\t  S%d\t%c%-8s  ",
> +		seq_printf(seq, "%s\t  S%d\t",
>  			   dev->pnp.bus_id,
> -			   (u32) dev->wakeup.sleep_state,
> -			   dev->wakeup.flags.run_wake ? '*' : ' ',
> -			   (device_may_wakeup(&dev->dev)
> -			     || (ldev && device_may_wakeup(ldev))) ?
> -			       "enabled" : "disabled");
> -		if (ldev)
> -			seq_printf(seq, "%s:%s",
> -				   ldev->bus ? ldev->bus->name : "no-bus",
> -				   dev_name(ldev));
> -		seq_printf(seq, "\n");
> -		put_device(ldev);
> -
> +			   (u32) dev->wakeup.sleep_state);
> +
> +		if (!dev->physical_node_count)
> +			seq_printf(seq, "%c%-8s\n",
> +				dev->wakeup.flags.run_wake ?
> +				'*' : ' ', "disabled");
> +		else {
> +			struct device *ldev;
> +			list_for_each_entry(entry, &dev->physical_node_list,
> +					node) {
> +				ldev = get_device(entry->dev);
> +				if (!ldev)
> +					continue;
> +
> +				if (&entry->node !=
> +						dev->physical_node_list.next)
> +					seq_printf(seq, "\t\t");
> +
> +				seq_printf(seq, "%c%-8s  %s:%s\n",
> +					dev->wakeup.flags.run_wake ? '*' : ' ',
> +					(device_may_wakeup(&dev->dev) ||
> +					(ldev && device_may_wakeup(ldev))) ?
> +					"enabled" : "disabled",
> +					ldev->bus ? ldev->bus->name :
> +					"no-bus", dev_name(ldev));
> +				put_device(ldev);
> +			}
> +		}
>  	}
>  	mutex_unlock(&acpi_device_lock);
>  	return 0;
> @@ -329,12 +344,14 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
>  
>  static void physical_device_enable_wakeup(struct acpi_device *adev)
>  {
> -	struct device *dev = acpi_get_physical_device(adev->handle);
> +	struct acpi_device_physical_node *entry;
>  
> -	if (dev && device_can_wakeup(dev)) {
> -		bool enable = !device_may_wakeup(dev);
> -		device_set_wakeup_enable(dev, enable);
> -	}
> +	list_for_each_entry(entry,
> +		&adev->physical_node_list, node)
> +		if (entry->dev && device_can_wakeup(entry->dev)) {
> +			bool enable = !device_may_wakeup(entry->dev);
> +			device_set_wakeup_enable(entry->dev, enable);
> +		}
>  }
>  
>  static ssize_t
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..d730a93 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -481,6 +481,8 @@ static int acpi_device_register(struct acpi_device *device)
>  	INIT_LIST_HEAD(&device->children);
>  	INIT_LIST_HEAD(&device->node);
>  	INIT_LIST_HEAD(&device->wakeup_list);
> +	INIT_LIST_HEAD(&device->physical_node_list);
> +	mutex_init(&device->physical_node_lock);
>  
>  	new_bus_id = kzalloc(sizeof(struct acpi_device_bus_id), GFP_KERNEL);
>  	if (!new_bus_id) {
> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> index 507a8e2..26b5d4b 100644
> --- a/drivers/pnp/pnpacpi/core.c
> +++ b/drivers/pnp/pnpacpi/core.c
> @@ -321,14 +321,9 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
>  {
>  	struct acpi_device *acpi = to_acpi_device(dev);
>  	struct pnp_dev *pnp = _pnp;
> -	struct device *physical_device;
> -
> -	physical_device = acpi_get_physical_device(acpi->handle);
> -	if (physical_device)
> -		put_device(physical_device);
>  
>  	/* true means it matched */
> -	return !physical_device
> +	return !acpi->physical_node_count
>  	    && compare_pnp_id(pnp->id, acpi_device_hid(acpi));
>  }
>  
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index bde976e..453596d 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -282,8 +282,16 @@ struct acpi_device_wakeup {
>  	int prepare_count;
>  };
>  
> -/* Device */
> +struct acpi_device_physical_node {
> +	u8 node_id;
> +	struct list_head node;
> +	struct device *dev;
> +};
>  
> +/* set maximum of physical nodes to 32 for expansibility */
> +#define ACPI_MAX_PHYSICAL_NODE	32
> +
> +/* Device */
>  struct acpi_device {
>  	int device_type;
>  	acpi_handle handle;		/* no handle for fixed hardware */
> @@ -304,6 +312,10 @@ struct acpi_device {
>  	struct device dev;
>  	struct acpi_bus_ops bus_ops;	/* workaround for different code path for hotplug */
>  	enum acpi_bus_removal_type removal_type;	/* indicate for different removal type */
> +	u8 physical_node_count;
> +	struct list_head physical_node_list;
> +	struct mutex physical_node_lock;
> +	DECLARE_BITMAP(physical_node_id_bitmap, ACPI_MAX_PHYSICAL_NODE);
>  };
>  
>  static inline void *acpi_driver_data(struct acpi_device *d)
> @@ -394,7 +406,6 @@ struct acpi_bus_type {
>  };
>  int register_acpi_bus_type(struct acpi_bus_type *);
>  int unregister_acpi_bus_type(struct acpi_bus_type *);
> -struct device *acpi_get_physical_device(acpi_handle);
>  
>  struct acpi_pci_root {
>  	struct list_head node;
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] ACPI: Allow acpi binding with usb3.0 hub

[Lan Tianyu]

On 2012年09月21日 08:25, Sarah Sharp wrote:
> Hi Tianyu,
> 
> You're correct that the USB core and the xHCI driver has a slightly
> different architecture than the ACPI representation.  We have one PCI
> device that has two USB buses/roothubs underneath it.  That's because
> external USB 3.0 hubs show up as two USB devices: a USB 3.0 only
> portion, and a USB 2.0 portion.  We wanted roothubs to act the same way.
> 
> The architecture looks something like this:
> 
>       struct pci_dev
>       |
> struct usb_hcd *primary_hcd -- struct usb_hcd *secondary_hcd
>       |                               |
> struct usb_bus                  struct usb_bus
>       |                               |
>  struct usb_device *root_hub     struct usb_device *root_hub
>       |                               |
>    struct device                   struct device
> 
> Where the primary_hcd is for the USB 2.0 roothub, and the secondary_hcd
> is for the USB 3.0 roothub.
> 
> So you're trying to bind one ACPI node (RHUB) to the struct device
> contained in those two root_hubs, correct?  That goal seems sane to me.
> 
Yes, that is my patch's purpose.

> Sarah Sharp


-- 
Best regards
Tianyu Lan
--
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 V2] ACPI: Allow acpi binding with usb3.0 hub

[Sarah Sharp]

On Fri, Sep 21, 2012 at 11:16:01AM +0800, Lan Tianyu wrote:
> On 2012年09月21日 08:25, Sarah Sharp wrote:
> > Hi Tianyu,
> > 
> > You're correct that the USB core and the xHCI driver has a slightly
> > different architecture than the ACPI representation.  We have one PCI
> > device that has two USB buses/roothubs underneath it.  That's because
> > external USB 3.0 hubs show up as two USB devices: a USB 3.0 only
> > portion, and a USB 2.0 portion.  We wanted roothubs to act the same way.
> > 
> > The architecture looks something like this:
> > 
> >       struct pci_dev
> >       |
> > struct usb_hcd *primary_hcd -- struct usb_hcd *secondary_hcd
> >       |                               |
> > struct usb_bus                  struct usb_bus
> >       |                               |
> >  struct usb_device *root_hub     struct usb_device *root_hub
> >       |                               |
> >    struct device                   struct device
> > 
> > Where the primary_hcd is for the USB 2.0 roothub, and the secondary_hcd
> > is for the USB 3.0 roothub.
> > 
> > So you're trying to bind one ACPI node (RHUB) to the struct device
> > contained in those two root_hubs, correct?  That goal seems sane to me.
> > 
> Yes, that is my patch's purpose.

Great!  Len should queue it up then.

Acked-by: Sarah Sharp <sarah.a.sharp-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] ACPI: Allow acpi binding with usb3.0 hub

[Len Brown]

Applied.

thanks,
Len Brown, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html