[PATCH 0/5] usb/acpi: To bind usb hub ports with acpi when not attached usb devices.

[PATCH 0/5] usb/acpi: To bind usb hub ports with acpi when not attached usb devices.

[Lan Tianyu]

Based on the Matthew Garrett's patch set.
[PATCH V2 1/4] ACPI: Fix up _PLD methods
[PATCH V2 2/4] ACPI: Add _PLD support
[PATCH V2 3/4] usb: Bind devices to ACPI devices when possible
[PATCH V2 4/4] usb: Set device removable state based on ACPI USB data

Add usb hub ports' platform_data to store acpi_handle. When a usb
hub is found, getting all ports' acpi handle and check port's connect
type through acpi method "_UPC" and "_PLD". When a usb device is found,
set it's removalbe depending on the port's connect type.


git diff --stat
 drivers/usb/core/devices.c      |    4 +-
 drivers/usb/core/hub.c          |  141 ++++++++++++++++++++++++++++-----------
 drivers/usb/core/usb-acpi.c     |  102 +++++++++++++++++++---------
 drivers/usb/core/usb.h          |   10 +++
 drivers/usb/host/r8a66597-hcd.c |    4 +-
 include/linux/usb.h             |   10 +++-
 6 files changed, 196 insertions(+), 75 deletions(-)

[PATCH 1/5] usb: add struct usb_hub_port to store port related members.

[Lan Tianyu]

Add struct usb_hub_port pointer port_data in the struct usb_hub and allocate
struct usb_hub_port perspectively for each ports to store private data.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/usb/core/hub.c |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index aeefbab..1ee130d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -37,6 +37,10 @@
 #endif
 #endif
 
+struct usb_hub_port {
+	void			*port_owner;
+};
+
 struct usb_hub {
 	struct device		*intfdev;	/* the "interface" device */
 	struct usb_device	*hdev;
@@ -79,7 +83,7 @@ struct usb_hub {
 	u8			indicator[USB_MAXCHILDREN];
 	struct delayed_work	leds;
 	struct delayed_work	init_work;
-	void			**port_owners;
+	struct usb_hub_port	*port_data;
 };
 
 static inline int hub_is_superspeed(struct usb_device *hdev)
@@ -1021,8 +1025,9 @@ static int hub_configure(struct usb_hub *hub,
 	dev_info (hub_dev, "%d port%s detected\n", hdev->maxchild,
 		(hdev->maxchild == 1) ? "" : "s");
 
-	hub->port_owners = kzalloc(hdev->maxchild * sizeof(void *), GFP_KERNEL);
-	if (!hub->port_owners) {
+	hub->port_data = kzalloc(hdev->maxchild * sizeof(struct usb_hub_port),
+			GFP_KERNEL);
+	if (!hub->port_data) {
 		ret = -ENOMEM;
 		goto fail;
 	}
@@ -1275,7 +1280,7 @@ static void hub_disconnect(struct usb_interface *intf)
 		highspeed_hubs--;
 
 	usb_free_urb(hub->urb);
-	kfree(hub->port_owners);
+	kfree(hub->port_data);
 	kfree(hub->descriptor);
 	kfree(hub->status);
 	kfree(hub->buffer);
@@ -1413,7 +1418,7 @@ static int find_port_owner(struct usb_device *hdev, unsigned port1,
 	/* This assumes that devices not managed by the hub driver
 	 * will always have maxchild equal to 0.
 	 */
-	*ppowner = &(hdev_to_hub(hdev)->port_owners[port1 - 1]);
+	*ppowner = &(hdev_to_hub(hdev)->port_data[port1 - 1].port_owner);
 	return 0;
 }
 
@@ -1448,16 +1453,12 @@ int usb_hub_release_port(struct usb_device *hdev, unsigned port1, void *owner)
 
 void usb_hub_release_all_ports(struct usb_device *hdev, void *owner)
 {
+	struct usb_hub *hub = hdev_to_hub(hdev);
 	int n;
-	void **powner;
 
-	n = find_port_owner(hdev, 1, &powner);
-	if (n == 0) {
-		for (; n < hdev->maxchild; (++n, ++powner)) {
-			if (*powner == owner)
-				*powner = NULL;
-		}
-	}
+	for (n = 0; n < hdev->maxchild; n++)
+		hub->port_data[n].port_owner = NULL;
+
 }
 
 /* The caller must hold udev's lock */
@@ -1468,7 +1469,7 @@ bool usb_device_is_owned(struct usb_device *udev)
 	if (udev->state == USB_STATE_NOTATTACHED || !udev->parent)
 		return false;
 	hub = hdev_to_hub(udev->parent);
-	return !!hub->port_owners[udev->portnum - 1];
+	return !!hub->port_data[udev->portnum - 1].port_owner;
 }
 
 
-- 
1.7.6.rc2.8.g28eb

[PATCH 2/5] usb: move struct usb_device->children to struct usb_hub_port->child

[Lan Tianyu]

Move child's pointer to the struct usb_hub_port since the child device
is directly associated with the port. Provide usb_get_hub_child_device()
to get child's pointer.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/usb/core/devices.c      |    4 ++-
 drivers/usb/core/hub.c          |   64 ++++++++++++++++++++++++--------------
 drivers/usb/host/r8a66597-hcd.c |    4 ++-
 include/linux/usb.h             |    3 +-
 4 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index d956965..b372531 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -590,7 +590,9 @@ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes,
 
 	/* Now look at all of this device's children. */
 	for (chix = 0; chix < usbdev->maxchild; chix++) {
-		struct usb_device *childdev = usbdev->children[chix];
+		struct usb_device *childdev;
+		if (usb_get_hub_child_device(usbdev, chix + 1, &childdev) < 0)
+			continue;
 
 		if (childdev) {
 			usb_lock_device(childdev);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 1ee130d..51bf5d4 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -39,6 +39,7 @@
 
 struct usb_hub_port {
 	void			*port_owner;
+	struct usb_device	*child;
 };
 
 struct usb_hub {
@@ -91,7 +92,7 @@ static inline int hub_is_superspeed(struct usb_device *hdev)
 	return (hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS);
 }
 
-/* Protect struct usb_device->state and ->children members
+/* Protect struct usb_device->state and struct usb_hub_port->child members
  * Note: Both are also protected by ->dev.sem, except that ->state can
  * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
 static DEFINE_SPINLOCK(device_state_lock);
@@ -624,8 +625,8 @@ static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
 	struct usb_device *hdev = hub->hdev;
 	int ret = 0;
 
-	if (hdev->children[port1-1] && set_state)
-		usb_set_device_state(hdev->children[port1-1],
+	if (hub->port_data[port1-1].child && set_state)
+		usb_set_device_state(hub->port_data[port1-1].child,
 				USB_STATE_NOTATTACHED);
 	if (!hub->error && !hub_is_superspeed(hub->hdev))
 		ret = clear_port_feature(hdev, port1, USB_PORT_FEAT_ENABLE);
@@ -781,7 +782,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 	 * which ports need attention.
 	 */
 	for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
-		struct usb_device *udev = hdev->children[port1-1];
+		struct usb_device *udev = hub->port_data[port1-1].child;
 		u16 portstatus, portchange;
 
 		portstatus = portchange = 0;
@@ -945,8 +946,8 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type)
 	if (type != HUB_SUSPEND) {
 		/* Disconnect all the children */
 		for (i = 0; i < hdev->maxchild; ++i) {
-			if (hdev->children[i])
-				usb_disconnect(&hdev->children[i]);
+			if (hub->port_data[i].child)
+				usb_disconnect(&hub->port_data[i].child);
 		}
 	}
 
@@ -1373,6 +1374,7 @@ static int
 hub_ioctl(struct usb_interface *intf, unsigned int code, void *user_data)
 {
 	struct usb_device *hdev = interface_to_usbdev (intf);
+	struct usb_hub *hub = usb_get_intfdata(intf);
 
 	/* assert ifno == 0 (part of hub spec) */
 	switch (code) {
@@ -1386,11 +1388,11 @@ hub_ioctl(struct usb_interface *intf, unsigned int code, void *user_data)
 		else {
 			info->nports = hdev->maxchild;
 			for (i = 0; i < info->nports; i++) {
-				if (hdev->children[i] == NULL)
+				if (hub->port_data[i].child == NULL)
 					info->port[i] = 0;
 				else
 					info->port[i] =
-						hdev->children[i]->devnum;
+						hub->port_data[i].child->devnum;
 			}
 		}
 		spin_unlock_irq(&device_state_lock);
@@ -1475,11 +1477,12 @@ bool usb_device_is_owned(struct usb_device *udev)
 
 static void recursively_mark_NOTATTACHED(struct usb_device *udev)
 {
+	struct usb_hub	*hub = hdev_to_hub(udev);
 	int i;
 
 	for (i = 0; i < udev->maxchild; ++i) {
-		if (udev->children[i])
-			recursively_mark_NOTATTACHED(udev->children[i]);
+		if (hub->port_data[i].child)
+			recursively_mark_NOTATTACHED(hub->port_data[i].child);
 	}
 	if (udev->state == USB_STATE_SUSPENDED)
 		udev->active_duration -= jiffies;
@@ -1645,6 +1648,7 @@ void usb_disconnect(struct usb_device **pdev)
 	struct usb_device	*udev = *pdev;
 	int			i;
 	struct usb_hcd		*hcd = bus_to_hcd(udev->bus);
+	struct usb_hub		*hub = hdev_to_hub(udev);
 
 	/* mark the device as inactive, so any further urb submissions for
 	 * this device (and any of its children) will fail immediately.
@@ -1657,9 +1661,9 @@ void usb_disconnect(struct usb_device **pdev)
 	usb_lock_device(udev);
 
 	/* Free up all the children before we remove this device */
-	for (i = 0; i < USB_MAXCHILDREN; i++) {
-		if (udev->children[i])
-			usb_disconnect(&udev->children[i]);
+	for (i = 0; i < udev->maxchild; i++) {
+		if (hub->port_data[i].child)
+			usb_disconnect(&hub->port_data[i].child);
 	}
 
 	/* deallocate hcd/hardware state ... nuking all pending urbs and
@@ -2725,7 +2729,7 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
 	for (port1 = 1; port1 <= hdev->maxchild; port1++) {
 		struct usb_device	*udev;
 
-		udev = hdev->children [port1-1];
+		udev = hub->port_data[port1-1].child;
 		if (udev && udev->can_submit) {
 			dev_warn(&intf->dev, "port %d nyet suspended\n", port1);
 			if (PMSG_IS_AUTO(msg))
@@ -3199,7 +3203,7 @@ hub_power_remaining (struct usb_hub *hub)
 
 	remaining = hdev->bus_mA - hub->descriptor->bHubContrCurrent;
 	for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
-		struct usb_device	*udev = hdev->children[port1 - 1];
+		struct usb_device	*udev = hub->port_data[port1 - 1].child;
 		int			delta;
 
 		if (!udev)
@@ -3263,7 +3267,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
 #endif
 
 	/* Try to resuscitate an existing device */
-	udev = hdev->children[port1-1];
+	udev = hub->port_data[port1-1].child;
 	if ((portstatus & USB_PORT_STAT_CONNECTION) && udev &&
 			udev->state != USB_STATE_NOTATTACHED) {
 		usb_lock_device(udev);
@@ -3292,7 +3296,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
 
 	/* Disconnect any existing devices under this port */
 	if (udev)
-		usb_disconnect(&hdev->children[port1-1]);
+		usb_disconnect(&hub->port_data[port1-1].child);
 	clear_bit(port1, hub->change_bits);
 
 	/* We can forget about a "removed" device when there's a physical
@@ -3407,7 +3411,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
 				&& highspeed_hubs != 0)
 			check_highspeed (hub, udev, port1);
 
-		/* Store the parent's children[] pointer.  At this point
+		/* Store the hub port's child pointer.  At this point
 		 * udev becomes globally accessible, although presumably
 		 * no one will look at it until hdev is unlocked.
 		 */
@@ -3421,7 +3425,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
 		if (hdev->state == USB_STATE_NOTATTACHED)
 			status = -ENOTCONN;
 		else
-			hdev->children[port1-1] = udev;
+			hub->port_data[port1-1].child = udev;
 		spin_unlock_irq(&device_state_lock);
 
 		/* Run it through the hoops (find a driver, etc) */
@@ -3429,7 +3433,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
 			status = usb_new_device(udev);
 			if (status) {
 				spin_lock_irq(&device_state_lock);
-				hdev->children[port1-1] = NULL;
+				hub->port_data[port1-1].child = NULL;
 				spin_unlock_irq(&device_state_lock);
 			}
 		}
@@ -3588,7 +3592,7 @@ static void hub_events(void)
 				 */
 				if (!(portstatus & USB_PORT_STAT_ENABLE)
 				    && !connect_change
-				    && hdev->children[i-1]) {
+				    && hub->port_data[i-1].child) {
 					dev_err (hub_dev,
 					    "port %i "
 					    "disabled by hub (EMI?), "
@@ -3603,14 +3607,14 @@ static void hub_events(void)
 
 				clear_port_feature(hdev, i,
 					USB_PORT_FEAT_C_SUSPEND);
-				udev = hdev->children[i-1];
+				udev = hub->port_data[i-1].child;
 				if (udev) {
 					/* TRSMRCY = 10 msec */
 					msleep(10);
 
 					usb_lock_device(udev);
-					ret = usb_remote_wakeup(hdev->
-							children[i-1]);
+					ret = usb_remote_wakeup(hub->
+							port_data[i-1].child);
 					usb_unlock_device(udev);
 					if (ret < 0)
 						connect_change = 1;
@@ -4147,3 +4151,15 @@ void usb_queue_reset_device(struct usb_interface *iface)
 	schedule_work(&iface->reset_ws);
 }
 EXPORT_SYMBOL_GPL(usb_queue_reset_device);
+
+int usb_get_hub_child_device(struct usb_device *hdev, int port1,
+	struct usb_device **child)
+{
+	struct usb_hub *hub = hdev_to_hub(hdev);
+
+	if (port1 > hdev->maxchild || port1 < 1)
+		return -EINVAL;
+	*child = hub->port_data[port1 - 1].child;
+	return 0;
+}
+
diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index e84ca19..7295a1b 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -2040,7 +2040,9 @@ static void collect_usb_address_map(struct usb_device *udev, unsigned long *map)
 		map[udev->devnum/32] |= (1 << (udev->devnum % 32));
 
 	for (chix = 0; chix < udev->maxchild; chix++) {
-		struct usb_device *childdev = udev->children[chix];
+		struct usb_device *childdev;
+		if (usb_get_hub_child_device(usbdev, chix + 1, &childdev) < 0)
+			continue;
 
 		if (childdev)
 			collect_usb_address_map(childdev, map);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 0c51663..6d41c8d 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -501,7 +501,6 @@ struct usb_device {
 #endif
 
 	int maxchild;
-	struct usb_device *children[USB_MAXCHILDREN];
 
 	u32 quirks;
 	atomic_t urbnum;
@@ -527,6 +526,8 @@ static inline struct usb_device *interface_to_usbdev(struct usb_interface *intf)
 
 extern struct usb_device *usb_get_dev(struct usb_device *dev);
 extern void usb_put_dev(struct usb_device *dev);
+extern int usb_get_hub_child_device(struct usb_device *hdev, int port1,
+	struct usb_device **child);
 
 /* USB device locking */
 #define usb_lock_device(udev)		device_lock(&(udev)->dev)
-- 
1.7.6.rc2.8.g28eb

[PATCH 3/5] usb: Add platform_data in the struct usb_hub_port

[Lan Tianyu]

Add the member platform_data in the usb_hub_port to store port's
platform related data. Provide usb_set_hub_port_platform_data() and
usb_get_hub_port_platform_data() to access the member.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/usb/core/hub.c |   22 ++++++++++++++++++++++
 drivers/usb/core/usb.h |    4 ++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 51bf5d4..ac7bd89 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -40,6 +40,7 @@
 struct usb_hub_port {
 	void			*port_owner;
 	struct usb_device	*child;
+	unsigned long		platform_data;
 };
 
 struct usb_hub {
@@ -4163,3 +4164,24 @@ int usb_get_hub_child_device(struct usb_device *hdev, int port1,
 	return 0;
 }
 
+int usb_set_hub_port_platform_data(struct usb_device *hdev, int port1,
+	unsigned long data)
+{
+	struct usb_hub *hub = hdev_to_hub(hdev);
+
+	if (port1 > hdev->maxchild || port1 < 1)
+		return -EINVAL;
+	hub->port_data[port1 - 1].platform_data = data;
+	return 0;
+}
+
+int usb_get_hub_port_platform_data(struct usb_device *hdev, int port1,
+	unsigned long *data)
+{
+	struct usb_hub *hub = hdev_to_hub(hdev);
+
+	if (port1 > hdev->maxchild || port1 < 1)
+		return -EINVAL;
+	*data = hub->port_data[port1 - 1].platform_data;
+	return 0;
+}
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 636d98e..694c78d 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -154,6 +154,10 @@ extern void usb_notify_add_device(struct usb_device *udev);
 extern void usb_notify_remove_device(struct usb_device *udev);
 extern void usb_notify_add_bus(struct usb_bus *ubus);
 extern void usb_notify_remove_bus(struct usb_bus *ubus);
+extern int usb_get_hub_port_platform_data(struct usb_device *hdev,
+	int port1, unsigned long *data);
+extern int usb_set_hub_port_platform_data(struct usb_device *hdev,
+	int port1, unsigned long data);
 
 #ifdef CONFIG_ACPI
 extern int usb_acpi_register(void);
-- 
1.7.6.rc2.8.g28eb

[PATCH 4/5] usb/acpi: add the support of usb hub ports' acpi binding without attached devices.

[Lan Tianyu]

The usb port is a device in the acpi table but it's not in the linux
usb subsystem. USB hub port doesn't have struct device. So the acpi
glue framework only can cover the usb port connected with usb device
and store the acpi handle to struct device.archdata.acpi_handle. This
patch gets the hub port's acpi_handle and store it in the port's platform_data
to resolve no attached device no binding problem. The acpi method "_UPC"
and "_PLD" can be accessed without attached device.

Signed-off-by: Lan Tianyu <tianyu.lan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/usb/core/hub.c      |    1 +
 drivers/usb/core/usb-acpi.c |   38 +++++++++++++++++++++++++++++++++++++-
 drivers/usb/core/usb.h      |    2 ++
 3 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index ac7bd89..97b8993 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1239,6 +1239,7 @@ static int hub_configure(struct usb_hub *hub,
 		hub->indicator [0] = INDICATOR_CYCLE;
 
 	hub_activate(hub, HUB_INIT);
+	usb_acpi_bind_hub_ports(hdev);
 	return 0;
 
 fail:
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index e49373a..d287e1e 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -82,7 +82,16 @@ static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
 	if (!parent_handle)
 		return -ENODEV;
 
-	*handle = acpi_get_child(parent_handle, udev->portnum);
+	/**
+	 * The root hub's acpi handle is got from acpi method.
+	 * Other device's acpi handle can be got from the usb hub
+	 * port's platform_data.
+	 */
+	if (!udev->parent)
+		*handle = acpi_get_child(parent_handle, udev->portnum);
+	else
+		usb_get_hub_port_platform_data(udev->parent, udev->portnum,
+			(unsigned long *)handle);
 
 	if (!*handle)
 		return -ENODEV;
@@ -105,6 +114,33 @@ static struct acpi_bus_type usb_acpi_bus = {
 	.find_device = usb_acpi_find_device,
 };
 
+int usb_acpi_bind_hub_ports(struct usb_device *hdev)
+{
+	acpi_handle hub_handle = NULL;
+	acpi_handle port_handle = NULL;
+	struct device *dev = &hdev->dev;
+	int i;
+
+	hub_handle = DEVICE_ACPI_HANDLE(dev);
+	if (!hub_handle)
+		return -ENODEV;
+
+	/**
+	 * The usb hub port is not a device in the usb subsystem but it is a device
+	 * in the acpi table. Store its acpi handle in the platform data of usb
+	 * hub port.
+	 */
+	for (i = 1; i <= hdev->maxchild; i++) {
+		port_handle = acpi_get_child(hub_handle, i);
+		if (!port_handle)
+			continue;
+		if (usb_set_hub_port_platform_data(hdev, i,
+				(unsigned long)port_handle) < 0)
+			return -ENODEV;
+	}
+	return 0;
+}
+
 int usb_acpi_register(void)
 {
 	return register_acpi_bus_type(&usb_acpi_bus);
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 694c78d..3865da1 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -162,7 +162,9 @@ extern int usb_set_hub_port_platform_data(struct usb_device *hdev,
 #ifdef CONFIG_ACPI
 extern int usb_acpi_register(void);
 extern void usb_acpi_unregister(void);
+extern int usb_acpi_bind_hub_ports(struct usb_device *hdev);
 #else
 static inline int usb_acpi_register(void) { return 0; };
 static inline void usb_acpi_unregister(void) { };
+static inline int usb_acpi_bind_hub_ports(struct usb_device *dev) { return 0; };
 #endif
-- 
1.7.6.rc2.8.g28eb

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

[PATCH 5/5] usb/acpi: add usb check for the connect type of usb port

[Lan Tianyu]

Check the connect type of usb port when getting the usb port's
acpi_handle and store result into connect_type in the struct
usb_hub_port.

Accoding to ACPI Spec 9.13. PLD indicates whether usb port is
user visible and _UPC indicates whether it is connectable. If
the port was visible and connectable, it could be freely connected
and disconnected with USB devices. If no visible and connectable,
a usb device is directly hard-wired to the port. If no visible and
no connectable, the port would be not used.

When a device was found on the port, if the connect_type was hot-plug,
then the device would be removable. If the connect_type was hard-wired,
the device would be non-removable.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/usb/core/hub.c      |   25 +++++++++++++++++
 drivers/usb/core/usb-acpi.c |   64 +++++++++++++++++++++----------------------
 drivers/usb/core/usb.h      |    4 +++
 include/linux/usb.h         |    7 +++++
 4 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 97b8993..c70c170 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -41,6 +41,7 @@ struct usb_hub_port {
 	void			*port_owner;
 	struct usb_device	*child;
 	unsigned long		platform_data;
+	enum usb_port_connect_type connect_type;
 };
 
 struct usb_hub {
@@ -4186,3 +4187,27 @@ int usb_get_hub_port_platform_data(struct usb_device *hdev, int port1,
 	*data = hub->port_data[port1 - 1].platform_data;
 	return 0;
 }
+
+int usb_set_hub_port_connect_type(struct usb_device *hdev, int port1,
+	enum usb_port_connect_type type)
+{
+	struct usb_hub *hub = hdev_to_hub(hdev);
+
+	if (port1 > hdev->maxchild || port1 < 1)
+		return -EINVAL;
+
+	hub->port_data[port1 - 1].connect_type = type;
+	return 0;
+}
+
+int usb_get_hub_port_connect_type(struct usb_device *hdev, int port1,
+	enum usb_port_connect_type *type)
+{
+	struct usb_hub *hub = hdev_to_hub(hdev);
+
+	if (port1 > hdev->maxchild || port1 < 1)
+		return -EINVAL;
+
+	*type = hub->port_data[port1 - 1].connect_type;
+	return 0;
+}
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index d287e1e..5c26bd4 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -19,58 +19,57 @@
 
 #include "usb.h"
 
-static int usb_acpi_check_upc(struct usb_device *udev, acpi_handle handle)
+static int usb_acpi_check_port_connect_type(struct usb_device *hdev,
+	acpi_handle handle, int port1)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *upc;
+	struct acpi_pld pld;
 	int ret = 0;
 
-	status = acpi_evaluate_object(handle, "_UPC", NULL, &buffer);
+	/**
+	 * Accoding to ACPI Spec 9.13. PLD indicates whether usb port is
+	 * user visible and _UPC indicates whether it is connectable. If
+	 * the port was visible and connectable, it could be freely connected
+	 * and disconnected with USB devices. If no visible and connectable,
+	 * a usb device is directly hard-wired to the port. If no visible and
+	 * no connectable, the port would be not used.
+	 */
+	status = acpi_get_physical_device_location(handle, &pld);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
 
+	status = acpi_evaluate_object(handle, "_UPC", NULL, &buffer);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
 	upc = buffer.pointer;
-
 	if (!upc || (upc->type != ACPI_TYPE_PACKAGE) || upc->package.count != 4) {
 		ret = -EINVAL;
 		goto out;
 	}
 
-	if (upc->package.elements[0].integer.value)
-		udev->removable = USB_DEVICE_REMOVABLE;
-	else
-		udev->removable = USB_DEVICE_FIXED;
+	if (upc->package.elements[0].integer.value && pld.user_visible)
+		usb_set_hub_port_connect_type(hdev, port1,
+			USB_PORT_CONNECT_TYPE_HOT_PLUG);
+	else if (upc->package.elements[0].integer.value && !pld.user_visible)
+		usb_set_hub_port_connect_type(hdev, port1,
+			USB_PORT_CONNECT_TYPE_HARD_WIRED);
+	else if (!upc->package.elements[0].integer.value && !pld.user_visible)
+		usb_set_hub_port_connect_type(hdev, port1, USB_PORT_NOT_USED);
 
 out:
 	kfree(upc);
 	return ret;
 }
 
-static int usb_acpi_check_pld(struct usb_device *udev, acpi_handle handle)
-{
-	acpi_status status;
-	struct acpi_pld pld;
-
-	status = acpi_get_physical_device_location(handle, &pld);
-
-	if (ACPI_FAILURE(status))
-		return -ENODEV;
-
-	if (pld.user_visible)
-		udev->removable = USB_DEVICE_REMOVABLE;
-	else
-		udev->removable = USB_DEVICE_FIXED;
-
-	return 0;
-}
-
 static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
 {
 	struct usb_device *udev;
 	struct device *parent;
 	acpi_handle *parent_handle;
+	enum usb_port_connect_type type;
 
 	if (!is_usb_device(dev))
 		return -ENODEV;
@@ -96,14 +95,12 @@ static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
 	if (!*handle)
 		return -ENODEV;
 
-	/*
-	 * PLD will tell us whether a port is removable to the user or
-	 * not. If we don't get an answer from PLD (it's not present
-	 * or it's malformed) then try to infer it from UPC. If a
-	 * device isn't connectable then it's probably not removable.
-	 */
-	if (usb_acpi_check_pld(udev, *handle) != 0)
-		usb_acpi_check_upc(udev, *handle);
+	usb_get_hub_port_connect_type(udev->parent, udev->portnum, &type);
+
+	if (type == USB_PORT_CONNECT_TYPE_HOT_PLUG)
+		udev->removable = USB_DEVICE_REMOVABLE;
+	else if (type == USB_PORT_CONNECT_TYPE_HARD_WIRED)
+		udev->removable = USB_DEVICE_FIXED;
 
 	return 0;
 }
@@ -137,6 +134,7 @@ int usb_acpi_bind_hub_ports(struct usb_device *hdev)
 		if (usb_set_hub_port_platform_data(hdev, i,
 				(unsigned long)port_handle) < 0)
 			return -ENODEV;
+		usb_acpi_check_port_connect_type(hdev, port_handle, i);
 	}
 	return 0;
 }
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 3865da1..3da0819 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -158,6 +158,10 @@ extern int usb_get_hub_port_platform_data(struct usb_device *hdev,
 	int port1, unsigned long *data);
 extern int usb_set_hub_port_platform_data(struct usb_device *hdev,
 	int port1, unsigned long data);
+extern int usb_get_hub_port_connect_type(struct usb_device *hdev, int port1,
+	enum usb_port_connect_type *type);
+extern int usb_set_hub_port_connect_type(struct usb_device *hdev, int port1,
+	enum usb_port_connect_type type);
 
 #ifdef CONFIG_ACPI
 extern int usb_acpi_register(void);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 6d41c8d..8b3f356 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -382,6 +382,13 @@ enum usb_device_removable {
 	USB_DEVICE_FIXED,
 };
 
+enum usb_port_connect_type {
+	USB_PORT_CONNECT_TYPE_UNKNOWN = 0,
+	USB_PORT_CONNECT_TYPE_HOT_PLUG,
+	USB_PORT_CONNECT_TYPE_HARD_WIRED,
+	USB_PORT_NOT_USED,
+};
+
 /**
  * struct usb_device - kernel's representation of a USB device
  * @devnum: device number; address on a USB bus
-- 
1.7.6.rc2.8.g28eb

Re: [PATCH 1/5] usb: add struct usb_hub_port to store port related members.

[Alan Stern]

On Wed, 28 Mar 2012, Lan Tianyu wrote:

> Add struct usb_hub_port pointer port_data in the struct usb_hub and allocate
> struct usb_hub_port perspectively for each ports to store private data.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  drivers/usb/core/hub.c |   29 +++++++++++++++--------------
>  1 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index aeefbab..1ee130d 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c

>  void usb_hub_release_all_ports(struct usb_device *hdev, void *owner)
>  {
> +	struct usb_hub *hub = hdev_to_hub(hdev);
>  	int n;
> -	void **powner;
>  
> -	n = find_port_owner(hdev, 1, &powner);
> -	if (n == 0) {
> -		for (; n < hdev->maxchild; (++n, ++powner)) {
> -			if (*powner == owner)
> -				*powner = NULL;
> -		}
> -	}
> +	for (n = 0; n < hdev->maxchild; n++)
> +		hub->port_data[n].port_owner = NULL;

This is not quite right.  It should be:

	for (n = 0; n < hdev->maxchild; n++) {
		if (hub->port_data[n].port_owner == owner)
			hub->port_data[n].port_owner = NULL;
	}

Otherwise the patch is okay.

Alan Stern

Re: [PATCH 2/5] usb: move struct usb_device->children to struct usb_hub_port->child

[Alan Stern]

On Wed, 28 Mar 2012, Lan Tianyu wrote:

> Move child's pointer to the struct usb_hub_port since the child device
> is directly associated with the port. Provide usb_get_hub_child_device()
> to get child's pointer.

> --- a/drivers/usb/core/devices.c
> +++ b/drivers/usb/core/devices.c
> @@ -590,7 +590,9 @@ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes,
>  
>  	/* Now look at all of this device's children. */
>  	for (chix = 0; chix < usbdev->maxchild; chix++) {
> -		struct usb_device *childdev = usbdev->children[chix];
> +		struct usb_device *childdev;
> +		if (usb_get_hub_child_device(usbdev, chix + 1, &childdev) < 0)
> +			continue;
>  

Whitespace issue: The blank line should come before the new executable 
statement, not after it.

> @@ -1475,11 +1477,12 @@ bool usb_device_is_owned(struct usb_device *udev)
>  
>  static void recursively_mark_NOTATTACHED(struct usb_device *udev)
>  {
> +	struct usb_hub	*hub = hdev_to_hub(udev);

I'm not sure this is good.  You are calling hdev_to_hub without knowing
beforehand that udev really is a hub -- in fact, most of the time udev
won't be a hub.  The routine wasn't meant to be used that way.  Have
you checked that hdev_to_hub won't crash when the argument isn't a hub?  
What happens if the active configuration has no interfaces?

> @@ -4147,3 +4151,15 @@ void usb_queue_reset_device(struct usb_interface *iface)
>  	schedule_work(&iface->reset_ws);
>  }
>  EXPORT_SYMBOL_GPL(usb_queue_reset_device);
> +
> +int usb_get_hub_child_device(struct usb_device *hdev, int port1,
> +	struct usb_device **child)
> +{
> +	struct usb_hub *hub = hdev_to_hub(hdev);
> +
> +	if (port1 > hdev->maxchild || port1 < 1)
> +		return -EINVAL;
> +	*child = hub->port_data[port1 - 1].child;
> +	return 0;
> +}

This new routine is more complicated that it needs to be.  You don't
have to return an error code; none of the callers make use of it.
Just return the child pointer, and return NULL if there's an error.

Alan Stern

Re: [PATCH 4/5] usb/acpi: add the support of usb hub ports' acpi binding without attached devices.

[Alan Stern]

On Wed, 28 Mar 2012, Lan Tianyu wrote:

> The usb port is a device in the acpi table but it's not in the linux
> usb subsystem. USB hub port doesn't have struct device. So the acpi
> glue framework only can cover the usb port connected with usb device
> and store the acpi handle to struct device.archdata.acpi_handle. This
> patch gets the hub port's acpi_handle and store it in the port's platform_data
> to resolve no attached device no binding problem. The acpi method "_UPC"
> and "_PLD" can be accessed without attached device.

You can merge this patch with the previous one.  As it stands now, the 
previous patch adds new routines but no callers.

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index ac7bd89..97b8993 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1239,6 +1239,7 @@ static int hub_configure(struct usb_hub *hub,
>  		hub->indicator [0] = INDICATOR_CYCLE;
>  
>  	hub_activate(hub, HUB_INIT);
> +	usb_acpi_bind_hub_ports(hdev);
>  	return 0;

You probably should put usb_acpi_bind_hub_ports before hub_activate 
instead of after.  The hub can start to operate as soon as hub_activate 
runs.

> --- a/drivers/usb/core/usb-acpi.c
> +++ b/drivers/usb/core/usb-acpi.c
> @@ -82,7 +82,16 @@ static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
>  	if (!parent_handle)
>  		return -ENODEV;
>  
> -	*handle = acpi_get_child(parent_handle, udev->portnum);
> +	/**
> +	 * The root hub's acpi handle is got from acpi method.
> +	 * Other device's acpi handle can be got from the usb hub
> +	 * port's platform_data.
> +	 */
> +	if (!udev->parent)
> +		*handle = acpi_get_child(parent_handle, udev->portnum);
> +	else
> +		usb_get_hub_port_platform_data(udev->parent, udev->portnum,
> +			(unsigned long *)handle);
>  
>  	if (!*handle)

What happens if usb_get_hub_port_platform_data returns an error?  You 
don't check the error code, and then handle never gets assigned any 
value.  So the test "if (!*handle)" won't work right.

Instead of returning an error code, make usb_get_hub_port_platform_data 
return the data directly.

>  		return -ENODEV;
> @@ -105,6 +114,33 @@ static struct acpi_bus_type usb_acpi_bus = {
>  	.find_device = usb_acpi_find_device,
>  };
>  
> +int usb_acpi_bind_hub_ports(struct usb_device *hdev)
> +{
> +	acpi_handle hub_handle = NULL;
> +	acpi_handle port_handle = NULL;
> +	struct device *dev = &hdev->dev;
> +	int i;
> +
> +	hub_handle = DEVICE_ACPI_HANDLE(dev);
> +	if (!hub_handle)
> +		return -ENODEV;
> +
> +	/**
> +	 * The usb hub port is not a device in the usb subsystem but it is a device
> +	 * in the acpi table. Store its acpi handle in the platform data of usb
> +	 * hub port.
> +	 */
> +	for (i = 1; i <= hdev->maxchild; i++) {
> +		port_handle = acpi_get_child(hub_handle, i);
> +		if (!port_handle)
> +			continue;
> +		if (usb_set_hub_port_platform_data(hdev, i,
> +				(unsigned long)port_handle) < 0)
> +			return -ENODEV;

Similarly, make usb_set_hub_port_platform_data return void.  There's no 
reason to check for errors here; they can never happen.

Alan Stern

Re: [PATCH 5/5] usb/acpi: add usb check for the connect type of usb port

[Alan Stern]

On Wed, 28 Mar 2012, Lan Tianyu wrote:

> Check the connect type of usb port when getting the usb port's
> acpi_handle and store result into connect_type in the struct
> usb_hub_port.
> 
> Accoding to ACPI Spec 9.13. PLD indicates whether usb port is
> user visible and _UPC indicates whether it is connectable. If
> the port was visible and connectable, it could be freely connected
> and disconnected with USB devices. If no visible and connectable,
> a usb device is directly hard-wired to the port. If no visible and
> no connectable, the port would be not used.
> 
> When a device was found on the port, if the connect_type was hot-plug,
> then the device would be removable. If the connect_type was hard-wired,
> the device would be non-removable.

> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c

> @@ -4186,3 +4187,27 @@ int usb_get_hub_port_platform_data(struct usb_device *hdev, int port1,
>  	*data = hub->port_data[port1 - 1].platform_data;
>  	return 0;
>  }
> +
> +int usb_set_hub_port_connect_type(struct usb_device *hdev, int port1,
> +	enum usb_port_connect_type type)
> +{
> +	struct usb_hub *hub = hdev_to_hub(hdev);
> +
> +	if (port1 > hdev->maxchild || port1 < 1)
> +		return -EINVAL;
> +
> +	hub->port_data[port1 - 1].connect_type = type;
> +	return 0;
> +}
> +
> +int usb_get_hub_port_connect_type(struct usb_device *hdev, int port1,
> +	enum usb_port_connect_type *type)
> +{
> +	struct usb_hub *hub = hdev_to_hub(hdev);
> +
> +	if (port1 > hdev->maxchild || port1 < 1)
> +		return -EINVAL;
> +
> +	*type = hub->port_data[port1 - 1].connect_type;
> +	return 0;
> +}

Once again, you should return void or the data value, not an error
code.

> --- a/drivers/usb/core/usb-acpi.c
> +++ b/drivers/usb/core/usb-acpi.c

> +	if (upc->package.elements[0].integer.value && pld.user_visible)
> +		usb_set_hub_port_connect_type(hdev, port1,
> +			USB_PORT_CONNECT_TYPE_HOT_PLUG);
> +	else if (upc->package.elements[0].integer.value && !pld.user_visible)
> +		usb_set_hub_port_connect_type(hdev, port1,
> +			USB_PORT_CONNECT_TYPE_HARD_WIRED);
> +	else if (!upc->package.elements[0].integer.value && !pld.user_visible)
> +		usb_set_hub_port_connect_type(hdev, port1, USB_PORT_NOT_USED);

The logic here could be improved:

	if (upc->package.elements[0].integer.value)
		usb_set_hub_port_connect_type(hdev, port1,
				pld.user_visible ?
					USB_PORT_CONNECT_TYPE_HOT_PLUG :
					USB_PORT_CONNECT_TYPE_HARD_WIRED);
	else if (!pld.user_visible)
		usb_set_hub_port_connect_type(hdev, port1, USB_PORT_NOT_USED);


> +	usb_get_hub_port_connect_type(udev->parent, udev->portnum, &type);
> +
> +	if (type == USB_PORT_CONNECT_TYPE_HOT_PLUG)
> +		udev->removable = USB_DEVICE_REMOVABLE;
> +	else if (type == USB_PORT_CONNECT_TYPE_HARD_WIRED)
> +		udev->removable = USB_DEVICE_FIXED;

This should be a switch statement, and it should handle the other
possible values of type.

Alan Stern

--
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 2/5] usb: move struct usb_device->children to struct usb_hub_port->child

[Lan Tianyu]

hi alan:
	Great thanks for your such careful review.
On 2012年03月29日 03:13, Alan Stern wrote:
> On Wed, 28 Mar 2012, Lan Tianyu wrote:
>
>> Move child's pointer to the struct usb_hub_port since the child device
>> is directly associated with the port. Provide usb_get_hub_child_device()
>> to get child's pointer.
>
>> --- a/drivers/usb/core/devices.c
>> +++ b/drivers/usb/core/devices.c
>> @@ -590,7 +590,9 @@ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes,
>>
>>   	/* Now look at all of this device's children. */
>>   	for (chix = 0; chix<  usbdev->maxchild; chix++) {
>> -		struct usb_device *childdev = usbdev->children[chix];
>> +		struct usb_device *childdev;
>> +		if (usb_get_hub_child_device(usbdev, chix + 1,&childdev)<  0)
>> +			continue;
>>
>
> Whitespace issue: The blank line should come before the new executable
> statement, not after it.
>
>> @@ -1475,11 +1477,12 @@ bool usb_device_is_owned(struct usb_device *udev)
>>
>>   static void recursively_mark_NOTATTACHED(struct usb_device *udev)
>>   {
>> +	struct usb_hub	*hub = hdev_to_hub(udev);
>
> I'm not sure this is good.  You are calling hdev_to_hub without knowing
> beforehand that udev really is a hub -- in fact, most of the time udev
> won't be a hub.  The routine wasn't meant to be used that way.  Have
> you checked that hdev_to_hub won't crash when the argument isn't a hub?
> What happens if the active configuration has no interfaces?
About this, I have an idea to add a check of hdev->maxchild. I have
verified that hdev->maxchild only is set in the hub_configure() and
hub_disconnect(). So we can identify whether the udev is a usb hub or
not through hdev->maxchild. Does this make sense?

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index c70c170..d29a5dc 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -177,7 +177,7 @@ static inline char *portspeed(struct usb_hub *hub, int 
portstatus)
   /* Note that hdev or one of its children must be locked! */
   static struct usb_hub *hdev_to_hub(struct usb_device *hdev)
   {
-       if (!hdev || !hdev->actconfig)
+       if (!hdev || !hdev->actconfig || hdev->maxchild == 0)
                  return NULL;
          return usb_get_intfdata(hdev->actconfig->interface[0]);
   }


>> @@ -4147,3 +4151,15 @@ void usb_queue_reset_device(struct usb_interface *iface)
>>   	schedule_work(&iface->reset_ws);
>>   }
>>   EXPORT_SYMBOL_GPL(usb_queue_reset_device);
>> +
>> +int usb_get_hub_child_device(struct usb_device *hdev, int port1,
>> +	struct usb_device **child)
>> +{
>> +	struct usb_hub *hub = hdev_to_hub(hdev);
>> +
>> +	if (port1>  hdev->maxchild || port1<  1)
>> +		return -EINVAL;
>> +	*child = hub->port_data[port1 - 1].child;
>> +	return 0;
>> +}
>
> This new routine is more complicated that it needs to be.  You don't
> have to return an error code; none of the callers make use of it.
> Just return the child pointer, and return NULL if there's an error.
>
> Alan Stern
>
>

-- 
Best Regards
Tianyu Lan
linux kernel enabling team
--
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 2/5] usb: move struct usb_device->children to struct usb_hub_port->child

[Alan Stern]

On Thu, 29 Mar 2012, Lan Tianyu wrote:

> >> @@ -1475,11 +1477,12 @@ bool usb_device_is_owned(struct usb_device *udev)
> >>
> >>   static void recursively_mark_NOTATTACHED(struct usb_device *udev)
> >>   {
> >> +	struct usb_hub	*hub = hdev_to_hub(udev);
> >
> > I'm not sure this is good.  You are calling hdev_to_hub without knowing
> > beforehand that udev really is a hub -- in fact, most of the time udev
> > won't be a hub.  The routine wasn't meant to be used that way.  Have
> > you checked that hdev_to_hub won't crash when the argument isn't a hub?
> > What happens if the active configuration has no interfaces?
> About this, I have an idea to add a check of hdev->maxchild. I have
> verified that hdev->maxchild only is set in the hub_configure() and
> hub_disconnect(). So we can identify whether the udev is a usb hub or
> not through hdev->maxchild. Does this make sense?

Yes, okay.  If you add such a check to hdev_to_hub() then it will work 
properly.  Unless somebody markets a hub with no ports -- but we don't 
need to worry about that!  :-)

Alan Stern