Commit 004c1968 authored by Alan Stern's avatar Alan Stern Committed by Greg Kroah-Hartman
Browse files

USB: EHCI: go back to using the system clock for QH unlinks 



This patch (as1477) fixes a problem affecting a few types of EHCI
controller.  Contrary to what one might expect, these controllers
automatically stop their internal frame counter when no ports are
enabled.  Since ehci-hcd currently relies on the frame counter for
determining when it should unlink QHs from the async schedule, those
controllers run into trouble: The frame counter stops and the QHs
never get unlinked.

Some systems have also experienced other problems traced back to
commit b9638011 (USB: ehci-hcd unlink
speedups), which made the original switch from using the system clock
to using the frame counter.  It never became clear what the reason was
for these problems, but evidently it is related to use of the frame
counter.

To fix all these problems, this patch more or less reverts that commit
and goes back to using the system clock.  But this can't be done
cleanly because other changes have since been made to the scan_async()
subroutine.  One of these changes involved the tricky logic that tries
to avoid rescanning QHs that have already been seen when the scanning
loop is restarted, which happens whenever an URB is given back.
Switching back to clock-based unlinks would make this logic even more
complicated.

Therefore the new code doesn't rescan the entire async list whenever a
giveback occurs.  Instead it rescans only the current QH and continues
on from there.  This requires the use of a separate pointer to keep
track of the next QH to scan, since the current QH may be unlinked
while the scanning is in progress.  That new pointer must be global,
so that it can be adjusted forward whenever the _next_ QH gets
unlinked.  (uhci-hcd uses this same trick.)

Simplification of the scanning loop removes a level of indentation,
which accounts for the size of the patch.  The amount of code changed
is relatively small, and it isn't exactly a reversion of the
b9638011 commit.

This fixes Bugzilla #32432.

Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
CC: <stable@kernel.org>
Tested-by: default avatarMatej Kenda <matejken@gmail.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent 0c42a4e8

drivers/usb/host/ehci-hcd.c

+3 −5
Original line number Diff line number Diff line
@@ -90,7 +90,8 @@ static const char hcd_name [] = "ehci_hcd"; 
#define EHCI_IAA_MSECS		10		/* arbitrary */

#define EHCI_IO_JIFFIES		(HZ/10)		/* io watchdog > irq_thresh */

#define EHCI_ASYNC_JIFFIES	(HZ/20)		/* async idle timeout */

#define EHCI_SHRINK_FRAMES	5		/* async qh unlink delay */

#define EHCI_SHRINK_JIFFIES	(DIV_ROUND_UP(HZ, 200) + 1)

						/* 200-ms async qh unlink delay */



/* Initial IRQ latency:  faster than hw default */

static int log2_irq_thresh = 0;		// 0 to 6
@@ -148,10 +149,7 @@ timer_action(struct ehci_hcd *ehci, enum ehci_timer_action action) 
			break;

		/* case TIMER_ASYNC_SHRINK: */

		default:

			/* add a jiffie since we synch against the

			 * 8 KHz uframe counter.

			 */

			t = DIV_ROUND_UP(EHCI_SHRINK_FRAMES * HZ, 1000) + 1;

			t = EHCI_SHRINK_JIFFIES;

			break;

		}

		mod_timer(&ehci->watchdog, t + jiffies);

drivers/usb/host/ehci-q.c

+40 −42
Original line number Diff line number Diff line
@@ -1231,6 +1231,8 @@ static void start_unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh) 


	prev->hw->hw_next = qh->hw->hw_next;

	prev->qh_next = qh->qh_next;

	if (ehci->qh_scan_next == qh)

		ehci->qh_scan_next = qh->qh_next.qh;

	wmb ();



	/* If the controller isn't running, we don't have to wait for it */
@@ -1256,34 +1258,34 @@ static void scan_async (struct ehci_hcd *ehci) 
	struct ehci_qh		*qh;

	enum ehci_timer_action	action = TIMER_IO_WATCHDOG;



	ehci->stamp = ehci_readl(ehci, &ehci->regs->frame_index);

	timer_action_done (ehci, TIMER_ASYNC_SHRINK);

rescan:

	stopped = !HC_IS_RUNNING(ehci_to_hcd(ehci)->state);

	qh = ehci->async->qh_next.qh;

	if (likely (qh != NULL)) {

		do {



	ehci->qh_scan_next = ehci->async->qh_next.qh;

	while (ehci->qh_scan_next) {

		qh = ehci->qh_scan_next;

		ehci->qh_scan_next = qh->qh_next.qh;

 rescan:

		/* clean any finished work for this qh */

			if (!list_empty(&qh->qtd_list) && (stopped ||

					qh->stamp != ehci->stamp)) {

		if (!list_empty(&qh->qtd_list)) {

			int temp;



				/* unlinks could happen here; completion

				 * reporting drops the lock.  rescan using

				 * the latest schedule, but don't rescan

				 * qhs we already finished (no looping)

				 * unless the controller is stopped.

			/*

			 * Unlinks could happen here; completion reporting

			 * drops the lock.  That's why ehci->qh_scan_next

			 * always holds the next qh to scan; if the next qh

			 * gets unlinked then ehci->qh_scan_next is adjusted

			 * in start_unlink_async().

			 */

			qh = qh_get(qh);

				qh->stamp = ehci->stamp;

			temp = qh_completions(ehci, qh);

			if (qh->needs_rescan)

				unlink_async(ehci, qh);

			qh->unlink_time = jiffies + EHCI_SHRINK_JIFFIES;

			qh_put(qh);

				if (temp != 0) {

			if (temp != 0)

				goto rescan;

		}

			}



		/* unlink idle entries, reducing DMA usage as well

		 * as HCD schedule-scanning costs.  delay for any qh
@@ -1294,15 +1296,11 @@ rescan: 
		if (list_empty(&qh->qtd_list)

				&& qh->qh_state == QH_STATE_LINKED) {

			if (!ehci->reclaim && (stopped ||

					((ehci->stamp - qh->stamp) & 0x1fff)

						>= EHCI_SHRINK_FRAMES * 8))

					time_after_eq(jiffies, qh->unlink_time)))

				start_unlink_async(ehci, qh);

			else

				action = TIMER_ASYNC_SHRINK;

		}



			qh = qh->qh_next.qh;

		} while (qh);

	}

	if (action == TIMER_ASYNC_SHRINK)

		timer_action (ehci, TIMER_ASYNC_SHRINK);

drivers/usb/host/ehci.h

+2 −1
Original line number Diff line number Diff line
@@ -75,6 +75,7 @@ struct ehci_hcd { /* one per controller */ 
	struct ehci_qh		*async;

	struct ehci_qh		*dummy;		/* For AMD quirk use */

	struct ehci_qh		*reclaim;

	struct ehci_qh		*qh_scan_next;

	unsigned		scanning : 1;



	/* periodic schedule support */
@@ -119,7 +120,6 @@ struct ehci_hcd { /* one per controller */ 
	struct timer_list	iaa_watchdog;

	struct timer_list	watchdog;

	unsigned long		actions;

	unsigned		stamp;

	unsigned		periodic_stamp;

	unsigned		random_frame;

	unsigned long		next_statechange;
@@ -345,6 +345,7 @@ struct ehci_qh { 
	struct ehci_qh		*reclaim;	/* next to reclaim */



	struct ehci_hcd		*ehci;

	unsigned long		unlink_time;



	/*

	 * Do NOT use atomic operations for QH refcounting. On some CPUs

CRA Git | Maintained and supported by SUSTech CRA and CCSE