> Here's a patch against 2.5.20 that splits out the pci specific parts of
> hcd.c into hcd-pci.c.
>
> ...
>
> Christopher, does this patch make your patch easier?
>
> David, any complaints about this patch?
Only the interference with this patch. It's certainly
along the right lines, and I'll look at it at bit more
later this AM.
It does a few things, could be partially split up:
- The symbol exports from Christopher's patch
... with a couple renames, and most importantly
with kerneldoc for the new symbols
- Moves some urb submit logic into the generic urb
submit code (slightly affected by the above)
- Minor/separable cleanup to "uhci-hcd"
Let me suggest that if the sorted-out version of Greg's "hcd-pci.c"
patch plus this one both behave as expected (in "2.5.21"?) that
Christopher do his patch against the 2.5.latest tree with those
patches rather than against 2.5.18rmk or 2.5.20 ...
- Dave
p.s. Also I notice that in this case the copyright notices
were copied -- "hcd-pci.c" is basically all my code
from what I saw, nothing from those other folk (I don't
think I even did any cut/paste from usb-ohci.c) -- and
in the other newish core files none of them were.
Worth fixing such stuff.
--- ./drivers/usb-dist/core/hcd.h Mon Jun 3 11:11:11 2002
+++ ./drivers/usb/core/hcd.h Wed Jun 5 19:46:00 2002
@@ -50,9 +50,9 @@
int irq; /* irq allocated */
void *regs; /* device memory/io */
-#ifdef CONFIG_PCI
/* a few non-PCI controllers exist, mostly for OHCI */
- struct pci_dev *pdev; /* pci is typical */
+ struct pci_dev *pdev; /* may be emulated */
+#ifdef CONFIG_PCI
int region; /* pci region for regs */
u32 pci_state [16]; /* for PM state save */
atomic_t resume_count; /* multiple resumes issue */
@@ -162,6 +162,7 @@
extern void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb);
+/* PCI-specific bus glue, used for most USB host controller drivers */
#ifdef CONFIG_PCI
struct pci_dev;
struct pci_device_id;
@@ -179,6 +180,13 @@
#endif /* CONFIG_PCI */
+/* generic bus glue, needed for host controllers that don't use PCI */
+extern struct usb_operations usb_hcd_operations;
+extern void usb_hc_died (struct usb_hcd *hcd);
+struct pt_regs;
+extern void usb_hcd_irq (int irq, void *__hcd, struct pt_regs * r);
+
+
/* -------------------------------------------------------------------------- */
/* Enumeration is only for the hub driver, or HCD virtual root hubs */
@@ -297,6 +305,7 @@
extern struct list_head usb_bus_list;
extern struct semaphore usb_bus_list_lock;
+extern void usb_bus_init (struct usb_bus *bus);
extern void usb_bus_get (struct usb_bus *bus);
extern void usb_bus_put (struct usb_bus *bus);
--- ./drivers/usb-dist/core/hcd.c Mon Jun 3 11:11:11 2002
+++ ./drivers/usb/core/hcd.c Wed Jun 5 19:45:01 2002
@@ -112,8 +112,6 @@
/* used when updating hcd data */
static spinlock_t hcd_data_lock = SPIN_LOCK_UNLOCKED;
-static struct usb_operations hcd_operations;
-
/*-------------------------------------------------------------------------*/
/*
@@ -589,8 +587,14 @@
/*-------------------------------------------------------------------------*/
-/* shared initialization code */
-static void usb_init_bus (struct usb_bus *bus)
+/**
+ * usb_bus_init - shared initialization code
+ * @bus: the bus structure being initialized
+ *
+ * This code is used to initialize a usb_bus structure, memory for which is
+ * separately managed.
+ */
+void usb_bus_init (struct usb_bus *bus)
{
memset (&bus->devmap, 0, sizeof(struct usb_devmap));
@@ -609,6 +613,7 @@
atomic_set (&bus->refcnt, 1);
}
+EXPORT_SYMBOL (usb_bus_init);
/**
* usb_alloc_bus - creates a new USB host controller structure
@@ -629,7 +634,7 @@
bus = kmalloc (sizeof *bus, GFP_KERNEL);
if (!bus)
return NULL;
- usb_init_bus (bus);
+ usb_bus_init (bus);
bus->op = op;
return bus;
}
@@ -928,9 +933,6 @@
/* PCI-based HCs are normal, but custom bus glue should be ok */
-static void hcd_irq (int irq, void *__hcd, struct pt_regs *r);
-static void hc_died (struct usb_hcd *hcd);
-
/*-------------------------------------------------------------------------*/
/* configure so an HC device and id are always provided */
@@ -1045,7 +1047,7 @@
#else
bufp = __irq_itoa(dev->irq);
#endif
- if (request_irq (dev->irq, hcd_irq, SA_SHIRQ, hcd->description, hcd)
+ if (request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ, hcd->description, hcd)
!= 0) {
err ("request interrupt %s failed", bufp);
retval = -EBUSY;
@@ -1060,8 +1062,8 @@
(driver->flags & HCD_MEMORY) ? "pci mem" : "io base",
base);
- usb_init_bus (&hcd->self);
- hcd->self.op = &hcd_operations;
+ usb_bus_init (&hcd->self);
+ hcd->self.op = &usb_hcd_operations;
hcd->self.hcpriv = (void *) hcd;
hcd->self.bus_name = dev->slot_name;
hcd->product_desc = dev->name;
@@ -1225,7 +1227,7 @@
retval = hcd->driver->resume (hcd);
if (!HCD_IS_RUNNING (hcd->state)) {
dbg ("resume %s failure, retval %d", hcd->self.bus_name, retval);
- hc_died (hcd);
+ usb_hc_died (hcd);
// FIXME: recover, reset etc.
} else {
// FIXME for all connected devices, root-to-leaf:
@@ -1285,7 +1287,15 @@
/*-------------------------------------------------------------------------*/
-static void hc_died (struct usb_hcd *hcd)
+/**
+ * usb_hc_died - report abnormal shutdown of a host controller (bus glue)
+ * @hcd: pointer to the HCD representing the controller
+ *
+ * This is called by bus glue to report a USB host controller that died
+ * while operations may still have been pending. It's called automatically
+ * by the PCI glue, so only glue for non-PCI busses should need to call it.
+ */
+void usb_hc_died (struct usb_hcd *hcd)
{
struct list_head *devlist, *urblist;
struct hcd_dev *dev;
@@ -1313,6 +1323,7 @@
rh_status_dequeue (hcd, urb);
hcd->driver->stop (hcd);
}
+EXPORT_SYMBOL (usb_hc_died);
/*-------------------------------------------------------------------------*/
@@ -1337,170 +1348,21 @@
}
-/* may be called in any context with a valid urb->dev usecount */
-/* caller surrenders "ownership" of urb */
-
+/* may be called in any context with a valid urb->dev usecount
+ * caller surrenders "ownership" of urb
+ * expects usb_submit_urb() to have sanity checked and conditioned all
+ * inputs in the urb
+ */
static int hcd_submit_urb (struct urb *urb, int mem_flags)
{
int status;
- struct usb_hcd *hcd;
- struct hcd_dev *dev;
+ struct usb_hcd *hcd = urb->dev->bus->hcpriv;
+ struct hcd_dev *dev = urb->dev->hcpriv;
unsigned long flags;
- int pipe, temp, max;
-
- if (!urb || urb->hcpriv || !urb->complete)
- return -EINVAL;
- urb->status = -EINPROGRESS;
- urb->actual_length = 0;
- urb->bandwidth = 0;
- INIT_LIST_HEAD (&urb->urb_list);
-
- if (!urb->dev || !urb->dev->bus || urb->dev->devnum <= 0)
- return -ENODEV;
- hcd = urb->dev->bus->hcpriv;
- dev = urb->dev->hcpriv;
if (!hcd || !dev)
return -ENODEV;
- /* can't submit new urbs when quiescing, halted, ... */
- if (hcd->state == USB_STATE_QUIESCING || !HCD_IS_RUNNING (hcd->state))
- return -ESHUTDOWN;
- pipe = urb->pipe;
- temp = usb_pipetype (urb->pipe);
- if (usb_endpoint_halted (urb->dev, usb_pipeendpoint (pipe),
- usb_pipeout (pipe)))
- return -EPIPE;
-
- /* FIXME there should be a sharable lock protecting us against
- * config/altsetting changes and disconnects, kicking in here.
- */
-
- /* Sanity check, so HCDs can rely on clean data */
- max = usb_maxpacket (urb->dev, pipe, usb_pipeout (pipe));
- if (max <= 0) {
- err ("bogus endpoint (bad maxpacket)");
- return -EINVAL;
- }
-
- /* "high bandwidth" mode, 1-3 packets/uframe? */
- if (urb->dev->speed == USB_SPEED_HIGH) {
- int mult;
- switch (temp) {
- case PIPE_ISOCHRONOUS:
- case PIPE_INTERRUPT:
- mult = 1 + ((max >> 11) & 0x03);
- max &= 0x03ff;
- max *= mult;
- }
- }
-
- /* periodic transfers limit size per frame/uframe */
- switch (temp) {
- case PIPE_ISOCHRONOUS: {
- int n, len;
-
- if (urb->number_of_packets <= 0)
- return -EINVAL;
- for (n = 0; n < urb->number_of_packets; n++) {
- len = urb->iso_frame_desc [n].length;
- if (len < 0 || len > max)
- return -EINVAL;
- }
-
- }
- break;
- case PIPE_INTERRUPT:
- if (urb->transfer_buffer_length > max)
- return -EINVAL;
- }
-
- /* the I/O buffer must usually be mapped/unmapped */
- if (urb->transfer_buffer_length < 0)
- return -EINVAL;
-
-#ifdef DEBUG
- /* stuff that drivers shouldn't do, but which shouldn't
- * cause problems in HCDs if they get it wrong.
- */
- {
- unsigned int orig_flags = urb->transfer_flags;
- unsigned int allowed;
-
- /* enforce simple/standard policy */
- allowed = USB_ASYNC_UNLINK; // affects later unlinks
- allowed |= USB_NO_FSBR; // only affects UHCI
- switch (temp) {
- case PIPE_CONTROL:
- allowed |= USB_DISABLE_SPD;
- break;
- case PIPE_BULK:
- allowed |= USB_DISABLE_SPD | USB_QUEUE_BULK
- | USB_ZERO_PACKET | URB_NO_INTERRUPT;
- break;
- case PIPE_INTERRUPT:
- allowed |= USB_DISABLE_SPD;
- break;
- case PIPE_ISOCHRONOUS:
- allowed |= USB_ISO_ASAP;
- break;
- }
- urb->transfer_flags &= allowed;
-
- /* fail if submitter gave bogus flags */
- if (urb->transfer_flags != orig_flags) {
- err ("BOGUS urb flags, %x --> %x",
- orig_flags, urb->transfer_flags);
- return -EINVAL;
- }
- }
-#endif
- /*
- * Force periodic transfer intervals to be legal values that are
- * a power of two (so HCDs don't need to).
- *
- * FIXME want bus->{intr,iso}_sched_horizon values here. Each HC
- * supports different values... this uses EHCI/UHCI defaults (and
- * EHCI can use smaller non-default values).
- */
- switch (temp) {
- case PIPE_ISOCHRONOUS:
- case PIPE_INTERRUPT:
- /* too small? */
- if (urb->interval <= 0)
- return -EINVAL;
- /* too big? */
- switch (urb->dev->speed) {
- case USB_SPEED_HIGH: /* units are microframes */
- // NOTE usb handles 2^15
- if (urb->interval > (1024 * 8))
- urb->interval = 1024 * 8;
- temp = 1024 * 8;
- break;
- case USB_SPEED_FULL: /* units are frames/msec */
- case USB_SPEED_LOW:
- if (temp == PIPE_INTERRUPT) {
- if (urb->interval > 255)
- return -EINVAL;
- // NOTE ohci only handles up to 32
- temp = 128;
- } else {
- if (urb->interval > 1024)
- urb->interval = 1024;
- // NOTE usb and ohci handle up to 2^15
- temp = 1024;
- }
- break;
- default:
- return -EINVAL;
- }
- /* power of two? */
- while (temp > urb->interval)
- temp >>= 1;
- urb->interval = temp;
- }
-
-
/*
* FIXME: make urb timeouts be generic, keeping the HCD cores
* as simple as possible.
@@ -1510,9 +1372,6 @@
// hcd_monitor_hook(MONITOR_URB_SUBMIT, urb)
// It would catch submission paths for all urbs.
- /* increment urb's reference count, we now control it. */
- urb = usb_get_urb(urb);
-
/*
* Atomically queue the urb, first to our records, then to the HCD.
* Access to urb->status is controlled by urb->lock ... changes on
@@ -1534,18 +1393,15 @@
if (status)
return status;
- /* temporarily up refcount while queueing it in the HCD,
- * since we report some queuing/setup errors ourselves
+ /* increment urb's reference count as part of giving it to the HCD
+ * (which now controls it). HCD guarantees that it either returns
+ * an error or calls giveback(), but not both.
*/
- urb = usb_get_urb (urb);
+ urb = usb_get_urb(urb);
if (urb->dev == hcd->self.root_hub)
status = rh_urb_enqueue (hcd, urb);
else
status = hcd->driver->urb_enqueue (hcd, urb, mem_flags);
- /* urb->dev got nulled if hcd called giveback for us */
- if (status && urb->dev)
- urb_unlink (urb);
- usb_put_urb (urb);
return status;
}
@@ -1751,17 +1607,35 @@
return 0;
}
-static struct usb_operations hcd_operations = {
+/**
+ * usb_hcd_operations - adapts usb_bus framework to HCD framework (bus glue)
+ *
+ * When registering a USB bus through the HCD framework code, use this
+ * usb_operations vector. The PCI glue layer does so automatically; only
+ * bus glue for non-PCI system busses will need to use this.
+ */
+struct usb_operations usb_hcd_operations = {
allocate: hcd_alloc_dev,
get_frame_number: hcd_get_frame_number,
submit_urb: hcd_submit_urb,
unlink_urb: hcd_unlink_urb,
deallocate: hcd_free_dev,
};
+EXPORT_SYMBOL (usb_hcd_operations);
/*-------------------------------------------------------------------------*/
-static void hcd_irq (int irq, void *__hcd, struct pt_regs * r)
+/**
+ * usb_hcd_irq - hook IRQs to HCD framework (bus glue)
+ * @irq: the IRQ being raised
+ * @__hcd: pointer to the HCD whose IRQ is beinng signaled
+ * @r: saved hardware registers (not passed to HCD)
+ *
+ * When registering a USB bus through the HCD framework code, use this
+ * to handle interrupts. The PCI glue layer does so automatically; only
+ * bus glue for non-PCI system busses will need to use this.
+ */
+void usb_hcd_irq (int irq, void *__hcd, struct pt_regs * r)
{
struct usb_hcd *hcd = __hcd;
int start = hcd->state;
@@ -1771,8 +1645,9 @@
hcd->driver->irq (hcd);
if (hcd->state != start && hcd->state == USB_STATE_HALT)
- hc_died (hcd);
+ usb_hc_died (hcd);
}
+EXPORT_SYMBOL (usb_hcd_irq);
/*-------------------------------------------------------------------------*/
--- ./drivers/usb-dist/core/urb.c Mon Jun 3 11:11:11 2002
+++ ./drivers/usb/core/urb.c Wed Jun 5 09:37:53 2002
@@ -182,15 +182,170 @@
*/
int usb_submit_urb(struct urb *urb, int mem_flags)
{
+ int pipe, temp, max;
+ struct usb_device *dev;
+ struct usb_operations *op;
+ int is_out;
- if (urb && urb->dev && urb->dev->bus && urb->dev->bus->op) {
- if (usb_maxpacket(urb->dev, urb->pipe, usb_pipeout(urb->pipe)) <= 0) {
- err("%s: pipe %x has invalid size (<= 0)", __FUNCTION__,
urb->pipe);
+ if (!urb || urb->hcpriv || !urb->complete)
+ return -EINVAL;
+ if (!(dev = urb->dev) || !dev->bus || dev->devnum <= 0)
+ return -ENODEV;
+ if (!(op = dev->bus->op) || !op->submit_urb)
+ return -ENODEV;
+
+ urb->status = -EINPROGRESS;
+ urb->actual_length = 0;
+ urb->bandwidth = 0;
+
+ /* Lots of sanity checks, so HCDs can rely on clean data
+ * and don't need to duplicate tests
+ */
+ pipe = urb->pipe;
+ temp = usb_pipetype (pipe);
+ is_out = usb_pipeout (pipe);
+
+ /* (actually HCDs may need to duplicate this, endpoint might yet
+ * stall due to queued bulk/intr transactions that complete after
+ * we check)
+ */
+ if (usb_endpoint_halted (dev, usb_pipeendpoint (pipe), is_out))
+ return -EPIPE;
+
+ /* FIXME there should be a sharable lock protecting us against
+ * config/altsetting changes and disconnects, kicking in here.
+ * (here == before maxpacket, and eventually endpoint type,
+ * checks get made.)
+ */
+
+ max = usb_maxpacket (dev, pipe, is_out);
+ if (max <= 0) {
+ dbg ("%s: bogus endpoint %d-%s on usb-%s-%s (bad maxpacket %d)",
+ __FUNCTION__,
+ usb_pipeendpoint (pipe), is_out ? "OUT" : "IN",
+ dev->bus->bus_name, dev->devpath,
+ max);
+ return -EMSGSIZE;
+ }
+
+ /* "high bandwidth" mode, 1-3 packets/uframe? */
+ if (dev->speed == USB_SPEED_HIGH) {
+ int mult;
+ switch (temp) {
+ case PIPE_ISOCHRONOUS:
+ case PIPE_INTERRUPT:
+ mult = 1 + ((max >> 11) & 0x03);
+ max &= 0x03ff;
+ max *= mult;
+ }
+ }
+
+ /* periodic transfers limit size per frame/uframe */
+ switch (temp) {
+ case PIPE_ISOCHRONOUS: {
+ int n, len;
+
+ if (urb->number_of_packets <= 0)
+ return -EINVAL;
+ for (n = 0; n < urb->number_of_packets; n++) {
+ len = urb->iso_frame_desc [n].length;
+ if (len < 0 || len > max)
+ return -EMSGSIZE;
+ }
+
+ }
+ break;
+ case PIPE_INTERRUPT:
+ if (urb->transfer_buffer_length > max)
return -EMSGSIZE;
+ }
+
+ /* the I/O buffer must be mapped/unmapped, except when length=0 */
+ if (urb->transfer_buffer_length < 0)
+ return -EMSGSIZE;
+
+#ifdef DEBUG
+ /* stuff that drivers shouldn't do, but which shouldn't
+ * cause problems in HCDs if they get it wrong.
+ */
+ {
+ unsigned int orig_flags = urb->transfer_flags;
+ unsigned int allowed;
+
+ /* enforce simple/standard policy */
+ allowed = USB_ASYNC_UNLINK; // affects later unlinks
+ allowed |= USB_NO_FSBR; // only affects UHCI
+ switch (temp) {
+ case PIPE_CONTROL:
+ allowed |= USB_DISABLE_SPD;
+ break;
+ case PIPE_BULK:
+ allowed |= USB_DISABLE_SPD | USB_QUEUE_BULK
+ | USB_ZERO_PACKET | URB_NO_INTERRUPT;
+ break;
+ case PIPE_INTERRUPT:
+ allowed |= USB_DISABLE_SPD;
+ break;
+ case PIPE_ISOCHRONOUS:
+ allowed |= USB_ISO_ASAP;
+ break;
+ }
+ urb->transfer_flags &= allowed;
+
+ /* fail if submitter gave bogus flags */
+ if (urb->transfer_flags != orig_flags) {
+ err ("BOGUS urb flags, %x --> %x",
+ orig_flags, urb->transfer_flags);
+ return -EINVAL;
+ }
+ }
+#endif
+ /*
+ * Force periodic transfer intervals to be legal values that are
+ * a power of two (so HCDs don't need to).
+ *
+ * FIXME want bus->{intr,iso}_sched_horizon values here. Each HC
+ * supports different values... this uses EHCI/UHCI defaults (and
+ * EHCI can use smaller non-default values).
+ */
+ switch (temp) {
+ case PIPE_ISOCHRONOUS:
+ case PIPE_INTERRUPT:
+ /* too small? */
+ if (urb->interval <= 0)
+ return -EINVAL;
+ /* too big? */
+ switch (dev->speed) {
+ case USB_SPEED_HIGH: /* units are microframes */
+ // NOTE usb handles 2^15
+ if (urb->interval > (1024 * 8))
+ urb->interval = 1024 * 8;
+ temp = 1024 * 8;
+ break;
+ case USB_SPEED_FULL: /* units are frames/msec */
+ case USB_SPEED_LOW:
+ if (temp == PIPE_INTERRUPT) {
+ if (urb->interval > 255)
+ return -EINVAL;
+ // NOTE ohci only handles up to 32
+ temp = 128;
+ } else {
+ if (urb->interval > 1024)
+ urb->interval = 1024;
+ // NOTE usb and ohci handle up to 2^15
+ temp = 1024;
+ }
+ break;
+ default:
+ return -EINVAL;
}
- return urb->dev->bus->op->submit_urb(urb, mem_flags);
+ /* power of two? */
+ while (temp > urb->interval)
+ temp >>= 1;
+ urb->interval = temp;
}
- return -ENODEV;
+
+ return op->submit_urb (urb, mem_flags);
}
/*-------------------------------------------------------------------*/
--- ./drivers/usb-dist/host/uhci-hcd.c Mon Jun 3 11:11:11 2002
+++ ./drivers/usb/host/uhci-hcd.c Wed Jun 5 09:51:56 2002
@@ -1502,10 +1502,6 @@
break;
case PIPE_ISOCHRONOUS:
if (urb->bandwidth == 0) { /* not yet checked/allocated */
- if (urb->number_of_packets <= 0) {
- ret = -EINVAL;
- break;
- }
bustime = usb_check_bandwidth(urb->dev, urb);
if (bustime < 0) {
ret = bustime;
--- ./drivers/usb-dist/host/uhci-hub.c Mon Jun 3 11:11:11 2002
+++ ./drivers/usb/host/uhci-hub.c Wed Jun 5 09:57:47 2002
@@ -52,7 +52,7 @@
outw(status, io_addr + USBPORTSC1 + 2 * (wIndex-1))
-// FIXME: Shouldn't this return the length of the data too?
+/* size of returned buffer is part of USB spec */
static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u16 wIndex, char *buf, u16 wLength)
{
