[PATCH v3] ehci: Respect IST when scheduling new iTDs.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The EHCI specification says that an EHCI host controller may cache part of
the isochronous schedule.  The EHCI controller must advertise how much it
caches in the schedule through the HCCPARAMS register isochronous
scheduling threshold (IST) bits.

In theory, adding new iTDs within the IST should be harmless.  The HW will
follow the old cached linked list and miss the new iTD.  SW will notice HW
missed the iTD and return 0 for the transfer length.

However, Intel ICH9 chipsets (and some later chipsets) have issues when SW
attempts to schedule a split transaction within the IST.  All transfers
will cease being sent out that port, and the drivers will see isochronous
packets complete with a length of zero.  Start of frames may or may not
also disappear, causing the device to go into auto-suspend.  This "bus
stall" will continue until a control or bulk transfer is queued to a
device under that roothub.  High speed scheduling within the IST does not
cause this problem.

Most drivers will never cause this behavior, because they use multiple
URBs with multiple packets to keep the bus busy.  However, Sarah was able
to trigger this bug with a Logitech Clicksmart webcam by modifying
drivers/media/video/gspca/gspca.c by setting DEF_NURBS to 1 and npkt to 1
(in create_urbs(), line 518).  Sarah would see the bus stall happen on the
second or third isoc packet (whenever the IST was violated) and the USB bus
analyzer would show no isoc packets being transferred.

To fix this bug, we need to make sure the EHCI driver does not schedule
packets within the isochronous scheduling threshold (IST).  Make sure
that when we fall behind the current microframe plus IST, we schedule
the new transfer at the next periodic interval after the IST.

Do not change the scheduling for new transfers, since the schedule slop will
always be greater than the IST.

Make sure that if the host caches the full frame, the EHCI driver's
internal isochronous threshold (ehci->i_thresh) is set to
8 microframes + 2 microframes wiggle room.  This is similar to what is done in
the case where the host caches less than the full frame.

Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---
 drivers/usb/host/ehci-hcd.c   |    2 +-
 drivers/usb/host/ehci-sched.c |   13 +++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 11c627c..4c285dd 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -528,7 +528,7 @@ static int ehci_init(struct usb_hcd *hcd)
 	/* controllers may cache some of the periodic schedule ... */
 	hcc_params = ehci_readl(ehci, &ehci->caps->hcc_params);
 	if (HCC_ISOC_CACHE(hcc_params))		// full frame cache
-		ehci->i_thresh = 8;
+		ehci->i_thresh = 2 + 8;
 	else					// N microframes cached
 		ehci->i_thresh = 2 + HCC_ISOC_THRES(hcc_params);
 
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index d163371..fe541c9 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1331,7 +1331,7 @@ iso_stream_schedule (
 	struct ehci_iso_stream	*stream
 )
 {
-	u32			now, start, max, period;
+	u32			now, next, start, max, period;
 	int			status;
 	unsigned		mod = ehci->periodic_size << 3;
 	struct ehci_iso_sched	*sched = urb->hcpriv;
@@ -1354,6 +1354,7 @@ iso_stream_schedule (
 		period <<= 3;
 
 	now = ehci_readl(ehci, &ehci->regs->frame_index) % mod;
+	next = now + ehci->i_thresh;
 
 	/* when's the last uframe this urb could start? */
 	max = now + mod;
@@ -1365,16 +1366,16 @@ iso_stream_schedule (
 	 */
 	if (likely (!list_empty (&stream->td_list))) {
 		start = stream->next_uframe;
-		if (start < now)
-			start += mod;
 
 		/* Fell behind (by up to twice the slop amount)? */
-		if (start >= max - 2 * 8 * SCHEDULE_SLOP)
+		if (((start - next) & (mod - 1)) >= mod - 2 * 8 * SCHEDULE_SLOP)
 			start += period * DIV_ROUND_UP(
-					max - start, period) - mod;
+					(next - start) & (mod - 1),
+					period);
 
 		/* Tried to schedule too far into the future? */
-		if (unlikely((start + sched->span) >= max)) {
+		if (unlikely(((start - now) & (mod - 1)) + sched->span
+					>= max - 2 * 8 * SCHEDULE_SLOP)) {
 			status = -EFBIG;
 			goto fail;
 		}
-- 
1.6.0.4

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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux