It'd also be good to let USB device drivers know the biggest iso period that can be scheduled. It's not necessarily the same as the biggest interrupt period, and on EHCI it's also configurable. Otherwise drivers will not be able to tell in advance what requests are legal.
Is it always as large or larger than 1024 frames?
UHCI has a fixed schedule of 1024 frames....
We shouldn't rely on a fixed bound like 1024 because in the future someone might come along with a HCD that only uses 512.
Including EHCI, which we currently configure to use only 256 by default
I'd go for something like this patch. It resolves a FIXME and makes a small behavior change: if the period is too big, it no longer automagically limits it except for the case of full speed interrupt transfers. (Which continue with the current behavior, making a lot of drivers happier on OHCI.)
Comments? Perhaps the bounds tests should use "limit - 1" to behave better, but they've behaved so far (most periods are short enough not to have issues).
- Dave
--- 1.18/drivers/usb/core/urb.c Thu Jun 12 07:28:01 2003 +++ edited/drivers/usb/core/urb.c Thu Jul 24 12:49:14 2003 @@ -327,38 +327,39 @@ #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). + * a power of two (so HCDs don't need to), and within the range + * supported with this HCD. */ switch (temp) { + struct usb_bus *bus; + case PIPE_ISOCHRONOUS: case PIPE_INTERRUPT: /* too small? */ if (urb->interval <= 0) return -EINVAL; /* too big? */ + bus = urb->dev->bus; 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; + if (urb->interval > (bus->periodic_limit * 8)) + return -EINVAL; + temp = bus->periodic_limit * 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; + // ohci only handles up to 32, + // so be forgiving here + temp = bus->periodic_limit; } else { - if (urb->interval > 1024) - urb->interval = 1024; + if (urb->interval > bus->periodic_iso_limit) + return -EINVAL; // NOTE usb and ohci handle up to 2^15 - temp = 1024; + temp = bus->periodic_iso_limit; } break; default: --- 1.53/drivers/usb/host/ehci-hcd.c Wed Jun 18 21:50:06 2003 +++ edited/drivers/usb/host/ehci-hcd.c Thu Jul 24 12:07:17 2003 @@ -447,6 +451,8 @@ default: BUG (); } } + hcd->self.periodic_limit = ehci->periodic_size; + hcd->self.periodic_iso_limit = ehci->periodic_size; temp &= ~(CMD_IAAD | CMD_ASE | CMD_PSE), // Philips, Intel, and maybe others need CMD_RUN before the // root hub will detect new devices (why?); NEC doesn't --- 1.44/drivers/usb/host/ohci-hcd.c Mon Jul 14 12:18:51 2003 +++ edited/drivers/usb/host/ohci-hcd.c Thu Jul 24 12:02:59 2003 @@ -469,6 +469,8 @@ spin_lock_init (&ohci->lock); ohci->disabled = 1; ohci->sleeping = 0; + ohci->hcd.self.periodic_limit = NUM_INTS; + ohci->hcd.self.periodic_iso_limit = (1 << 15); /* Tell the controller where the control and bulk lists are * The lists are empty now. */ --- 1.37/drivers/usb/host/uhci-hcd.c Thu Jul 17 09:44:36 2003 +++ edited/drivers/usb/host/uhci-hcd.c Thu Jul 24 12:15:31 2003 @@ -2270,6 +2270,9 @@ goto err_alloc_term_td; } + hcd->self.periodic_limit = UHCI_NUMFRAMES; + hcd->self.periodic_iso_limit = UHCI_NUMFRAMES; + for (i = 0; i < UHCI_NUM_SKELQH; i++) { uhci->skelqh[i] = uhci_alloc_qh(uhci, udev); if (!uhci->skelqh[i]) { --- 1.82/include/linux/usb.h Tue Jul 15 07:08:20 2003 +++ edited/include/linux/usb.h Thu Jul 24 11:47:58 2003 @@ -205,6 +205,9 @@ int bandwidth_int_reqs; /* number of Interrupt requests */ int bandwidth_isoc_reqs; /* number of Isoc. requests */ + unsigned periodic_limit; /* maximum urb->interval */ + unsigned periodic_iso_limit; /* schedule this many frames in advance */ + struct dentry *usbfs_dentry; /* usbfs dentry entry for the bus */ struct dentry *usbdevfs_dentry; /* usbdevfs dentry entry for the bus */