Greg:
This patch adds several relatively minor bug fixes and code
simplifications for the hub driver.
Perhaps most significantly, a test is added to usbfs to prevent
users from unbinding the hub driver from a hub with children.
That could cause some bad headaches, and it's best to avoid the
situation.
Don't set unused bits in the change_bits and event_bits fields
(this aids in debugging, nothing more).
Use the correct spinlock in the hub ioctl routine!
Add an argument to hub_port_disable, indicating whether the
device behind the disabled port should be marked NOTATTACHED.
Normally it should, but for retries during usb_reset_device
it should not.
Pass the child device as an argument to hub_port_suspend and
hub_port_resume rather than making them recalculate it.
Call usb_set_device_state in the suspend/resume code rather
than setting udev->state directly.
Set udev->power.power_state in the suspend/resume code so that
the correct values are visible in sysfs.
Recognize during hub_port_resume that the child device udev
might be NULL (this will happen when a new device is plugged
into a suspended port).
Don't allow hub_port_init to drop the usb_address0_sem lock
while a device on the bus is still using address 0!
Make khubd acquire a reference to the hub interface rather than
the hub device. This simplifies disconnect/unbind testing.
Don't print a debugging message about hub events while not
holding some lock to protect the hub.
Remember to drop the reference acquired above if we fail to
lock the hub.
Please apply.
Alan Stern
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
--- a/drivers/usb/core/devio.c Mon Nov 8 09:41:32 2004
+++ b/drivers/usb/core/devio.c Fri Nov 26 16:35:59 2004
@@ -1111,6 +1111,7 @@
int retval = 0;
struct usb_interface *intf = NULL;
struct usb_driver *driver = NULL;
+ int i;
/* get input parameters and alloc buffer */
if (copy_from_user(&ctrl, arg, sizeof (ctrl)))
@@ -1142,6 +1143,16 @@
/* disconnect kernel driver from interface */
case USBDEVFS_DISCONNECT:
+
+ /* don't allow the user to unbind the hub driver from
+ * a hub with children to manage */
+ for (i = 0; i < ps->dev->maxchild; ++i) {
+ if (ps->dev->children[i])
+ retval = -EBUSY;
+ }
+ if (retval)
+ break;
+
down_write(&usb_bus_type.subsys.rwsem);
if (intf->dev.driver) {
driver = to_usb_driver(intf->dev.driver);
--- a/drivers/usb/core/hub.c Wed Nov 24 16:26:24 2004
+++ b/drivers/usb/core/hub.c Sat Nov 27 17:35:07 2004
@@ -451,7 +451,7 @@
schedule_delayed_work(&hub->leds, LED_CYCLE_PERIOD);
/* scan all ports ASAP */
- hub->event_bits[0] = ~0;
+ hub->event_bits[0] = (1UL << (hub->descriptor->bNbrPorts + 1)) - 1;
kick_khubd(hub);
}
@@ -674,7 +674,7 @@
hub->indicator [0] = INDICATOR_CYCLE;
hub_power_on(hub);
- hub->change_bits[0] = ~0;
+ hub->change_bits[0] = (1UL << (hub->descriptor->bNbrPorts)) - 1;
hub_activate(hub);
return 0;
@@ -802,10 +802,9 @@
switch (code) {
case USBDEVFS_HUB_PORTINFO: {
struct usbdevfs_hub_portinfo *info = user_data;
- unsigned long flags;
int i;
- spin_lock_irqsave(&hub_event_lock, flags);
+ spin_lock_irq(&device_state_lock);
if (hdev->devnum <= 0)
info->nports = 0;
else {
@@ -818,7 +817,7 @@
hdev->children[i]->devnum;
}
}
- spin_unlock_irqrestore(&hub_event_lock, flags);
+ spin_unlock_irq(&device_state_lock);
return info->nports + 1;
}
@@ -1399,11 +1398,11 @@
return status;
}
-static int hub_port_disable(struct usb_device *hdev, int port)
+static int hub_port_disable(struct usb_device *hdev, int port, int set_state)
{
int ret;
- if (hdev->children[port]) {
+ if (hdev->children[port] && set_state) {
usb_set_device_state(hdev->children[port],
USB_STATE_NOTATTACHED);
}
@@ -1425,7 +1424,7 @@
struct usb_hub *hub;
dev_dbg(hubdev(hdev), "logical disconnect on port %d\n", port + 1);
- hub_port_disable(hdev, port);
+ hub_port_disable(hdev, port, 1);
/* FIXME let caller ask to power down the port:
* - some devices won't enumerate without a VBUS power cycle
@@ -1454,12 +1453,11 @@
* tree above them to deliver data, such as a keypress or packet. In
* some cases, this wakes the USB host.
*/
-static int hub_port_suspend(struct usb_device *hdev, int port)
+static int hub_port_suspend(struct usb_device *hdev, int port,
+ struct usb_device *udev)
{
- int status;
- struct usb_device *udev;
+ int status;
- udev = hdev->children[port];
// dev_dbg(hubdev(hdev), "suspend port %d\n", port + 1);
/* enable remote wakeup when appropriate; this lets the device
@@ -1499,7 +1497,7 @@
} else {
/* device has up to 10 msec to fully suspend */
dev_dbg(&udev->dev, "usb suspend\n");
- udev->state = USB_STATE_SUSPENDED;
+ usb_set_device_state(udev, USB_STATE_SUSPENDED);
msleep(10);
}
return status;
@@ -1606,8 +1604,10 @@
} else
status = -EOPNOTSUPP;
} else
- status = hub_port_suspend(udev->parent, port);
+ status = hub_port_suspend(udev->parent, port, udev);
+ if (status == 0)
+ udev->dev.power.power_state = state;
return status;
}
EXPORT_SYMBOL(__usb_suspend_device);
@@ -1661,9 +1661,10 @@
* first two on the host side; they'd be inside hub_port_init()
* during many timeouts, but khubd can't suspend until later.
*/
- udev->state = udev->actconfig
- ? USB_STATE_CONFIGURED
- : USB_STATE_ADDRESS;
+ usb_set_device_state(udev, udev->actconfig
+ ? USB_STATE_CONFIGURED
+ : USB_STATE_ADDRESS);
+ udev->dev.power.power_state = PM_SUSPEND_ON;
/* 10.5.4.5 says be sure devices in the tree are still there.
* For now let's assume the device didn't go crazy on resume,
@@ -1731,12 +1732,10 @@
}
static int
-hub_port_resume(struct usb_device *hdev, int port)
+hub_port_resume(struct usb_device *hdev, int port, struct usb_device *udev)
{
- int status;
- struct usb_device *udev;
+ int status;
- udev = hdev->children[port];
// dev_dbg(hubdev(hdev), "resume port %d\n", port + 1);
/* see 7.1.7.7; affects power usage, but not budgeting */
@@ -1750,7 +1749,8 @@
u16 portchange;
/* drive resume for at least 20 msec */
- dev_dbg(&udev->dev, "RESUME\n");
+ if (udev)
+ dev_dbg(&udev->dev, "RESUME\n");
msleep(25);
#define LIVE_FLAGS ( USB_PORT_STAT_POWER \
@@ -1774,7 +1774,8 @@
} else {
/* TRSMRCY = 10 msec */
msleep(10);
- status = finish_port_resume(udev);
+ if (udev)
+ status = finish_port_resume(udev);
}
}
if (status < 0)
@@ -1824,7 +1825,7 @@
}
} else if (udev->state == USB_STATE_SUSPENDED) {
// NOTE this fails if parent is also suspended...
- status = hub_port_resume(udev->parent, port);
+ status = hub_port_resume(udev->parent, port, udev);
} else {
status = 0;
}
@@ -1923,7 +1924,7 @@
continue;
down (&udev->serialize);
if (portstat & USB_PORT_STAT_SUSPEND)
- status = hub_port_resume(hdev, port);
+ status = hub_port_resume(hdev, port, udev);
else {
status = finish_port_resume(udev);
if (status < 0) {
@@ -2261,6 +2262,8 @@
retval = 0;
fail:
+ if (retval)
+ hub_port_disable(hdev, port, 0);
up(&usb_address0_sem);
return retval;
}
@@ -2393,11 +2396,15 @@
}
#ifdef CONFIG_USB_SUSPEND
- /* If something is connected, but the port is suspended, wake it up.. */
+ /* If something is connected, but the port is suspended, wake it up. */
if (portstatus & USB_PORT_STAT_SUSPEND) {
- status = hub_port_resume(hdev, port);
- if (status < 0)
- dev_dbg(hub_dev, "can't clear suspend on port %d;
%d\n", port+1, status);
+ status = hub_port_resume(hdev, port, NULL);
+ if (status < 0) {
+ dev_dbg(hub_dev,
+ "can't clear suspend on port %d; %d\n",
+ port+1, status);
+ goto done;
+ }
}
#endif
@@ -2443,7 +2450,7 @@
&devstat);
if (status < 0) {
dev_dbg(&udev->dev, "get status %d ?\n",
status);
- goto loop;
+ goto loop_disable;
}
cpu_to_le16s(&devstat);
if ((devstat & (1 << USB_DEVICE_SELF_POWERED)) == 0) {
@@ -2456,7 +2463,7 @@
schedule_work (&hub->leds);
}
status = -ENOTCONN; /* Don't retry */
- goto loop;
+ goto loop_disable;
}
}
@@ -2496,7 +2503,7 @@
up (&udev->serialize);
if (status)
- goto loop;
+ goto loop_disable;
status = hub_power_remaining(hub);
if (status)
@@ -2506,8 +2513,9 @@
return;
+loop_disable:
+ hub_port_disable(hdev, port, 1);
loop:
- hub_port_disable(hdev, port);
usb_disable_endpoint(udev, 0 + USB_DIR_IN);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT);
release_address(udev);
@@ -2517,13 +2525,14 @@
}
done:
- hub_port_disable(hdev, port);
+ hub_port_disable(hdev, port, 1);
}
static void hub_events(void)
{
struct list_head *tmp;
struct usb_device *hdev;
+ struct usb_interface *intf;
struct usb_hub *hub;
struct device *hub_dev;
u16 hubstatus;
@@ -2553,10 +2562,8 @@
hub = list_entry(tmp, struct usb_hub, event_list);
hdev = hub->hdev;
- hub_dev = &hub->intf->dev;
-
- usb_get_dev(hdev);
- spin_unlock_irq(&hub_event_lock);
+ intf = hub->intf;
+ hub_dev = &intf->dev;
dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
hdev->state, hub->descriptor
@@ -2566,14 +2573,16 @@
(u16) hub->change_bits[0],
(u16) hub->event_bits[0]);
+ usb_get_intf(intf);
+ spin_unlock_irq(&hub_event_lock);
+
/* Lock the device, then check to see if we were
* disconnected while waiting for the lock to succeed. */
- if (locktree(hdev) < 0)
- break;
- if (hdev->state != USB_STATE_CONFIGURED ||
- !hdev->actconfig ||
- hub != usb_get_intfdata(
- hdev->actconfig->interface[0]))
+ if (locktree(hdev) < 0) {
+ usb_put_intf(intf);
+ continue;
+ }
+ if (hub != usb_get_intfdata(intf) || hub->quiescing)
goto loop;
if (hub->error) {
@@ -2645,7 +2654,7 @@
connect_change = 1;
} else {
ret = -ENODEV;
- hub_port_disable(hdev, i);
+ hub_port_disable(hdev, i, 1);
}
dev_dbg (hub_dev,
"resume on port %d, status %d\n",
@@ -2694,7 +2703,7 @@
loop:
usb_unlock_device(hdev);
- usb_put_dev(hdev);
+ usb_put_intf(intf);
} /* end while (1) */
}
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
