[PATCH 2/2] 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, there have been reports of issues with a new Intel EHCI host
controller and full speed isoc endpoints.  If the EHCI driver 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.

Most drivers will never cause this behavior, because they use multiple
URBs with multiple packets to keep the bus busy.  However, I 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).  I would see the bus stall happen on the
second or third isoc packet (whenever the IST was violated) and my 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 |    9 ++++++---
 2 files changed, 7 insertions(+), 4 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..b84ffe7 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,13 +1366,15 @@ iso_stream_schedule (
 	 */
 	if (likely (!list_empty (&stream->td_list))) {
 		start = stream->next_uframe;
-		if (start < now)
+		if (start < next)
 			start += mod;
 
 		/* Fell behind (by up to twice the slop amount)? */
 		if (start >= max - 2 * 8 * SCHEDULE_SLOP)
 			start += period * DIV_ROUND_UP(
-					max - start, period) - mod;
+					max - start + ehci->i_thresh,
+					period)
+				- mod;
 
 		/* Tried to schedule too far into the future? */
 		if (unlikely((start + sched->span) >= max)) {
-- 
1.5.6.5

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