lkml.org 
[lkml]   [2008]   [Mar]   [4]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    FromDavid Brownell <>
    SubjectRe: WARNING: at drivers/usb/host/ehci-hcd.c:287
    DateTue, 4 Mar 2008 20:25:30 -0800
    On Tuesday 04 March 2008, Christian Kujau wrote:
    > On Tue, 4 Mar 2008, David Brownell wrote:
    
    > > There are other lurking issues though.
    > 
    > uuh...let's hope they don't wake up?
    
    Nah.  See the patch I just attached to
    
    	http://bugzilla.kernel.org/show_bug.cgi?id=10078
    
    ... appended here for reference.
    
    - Dave
    
    
    ======== CUT HERE
    The recent EHCI driver update to split the IAA watchdog timer out from
    the other timers made several things work better, but not everything;
    and it created a couple new issues in bugzilla.  Ergo this patch:
      - Handle a should-be-rare SMP race between the watchdog firing
        and (very late) IAA interrupts;
    
      - Remove a shouldn't-have-been-added WARN_ON() test;
    
      - Guard against one observed OOPS;
    
      - If this watchdog fires during clean HC shutdown, it should act
        as a NOP instead of interfering with the shutdown sequence;
    
      - Guard against silicon errata hypothesized by some vendors:
          * IAA status latch broken, but IAAD cleared OK;
          * IAAD wasn't cleared when IAA status got reported;
    
    The WARN_ON is in bugzilla as 10168; the OOPS as 10078.
    
    Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
    ---
     drivers/usb/host/ehci-hcd.c |   60 +++++++++++++++++++++++++++++++++-----------
     1 file changed, 45 insertions(+), 15 deletions(-)
    --- g26.orig/drivers/usb/host/ehci-hcd.c	2008-03-04 17:24:22.000000000 -0800
    +++ g26/drivers/usb/host/ehci-hcd.c	2008-03-04 20:08:28.000000000 -0800
    @@ -257,23 +257,44 @@ static void ehci_iaa_watchdog(unsigned l
     {
     	struct ehci_hcd		*ehci = (struct ehci_hcd *) param;
     	unsigned long		flags;
    -	u32			status, cmd;
     
     	spin_lock_irqsave (&ehci->lock, flags);
    -	WARN_ON(!ehci->reclaim);
     
    -	status = ehci_readl(ehci, &ehci->regs->status);
    -	cmd = ehci_readl(ehci, &ehci->regs->command);
    -	ehci_dbg(ehci, "IAA watchdog: status %x cmd %x\n", status, cmd);
    -
    -	/* lost IAA irqs wedge things badly; seen first with a vt8235 */
    -	if (ehci->reclaim) {
    -		if (status & STS_IAA) {
    -			ehci_vdbg (ehci, "lost IAA\n");
    +	/* Lost IAA irqs wedge things badly; seen first with a vt8235.
    +	 * So we need this watchdog, but must protect it against both
    +	 * (a) SMP races against real IAA firing and retriggering, and
    +	 * (b) clean HC shutdown, when IAA watchdog was pending.
    +	 */
    +	if (ehci->reclaim
    +			&& !timer_pending(&ehci->iaa_watchdog)
    +			&& HC_IS_RUNNING(ehci_to_hcd(ehci)->state)) {
    +		u32 cmd, status;
    +
    +		/* If we get here, IAA is *REALLY* late.  It's barely
    +		 * conceivable that the system is so busy that CMD_IAAD
    +		 * is still legitimately set, so let's be sure it's
    +		 * clear before we read STS_IAA.  (The HC should clear
    +		 * CMD_IAAD when it sets STS_IAA.)
    +		 */
    +		cmd = ehci_readl(ehci, &ehci->regs->command);
    +		if (cmd & CMD_IAAD)
    +			ehci_writel(ehci, cmd & ~CMD_IAAD,
    +					&ehci->regs->command);
    +
    +		/* If IAA is set here it either legitimately triggered
    +		 * before we cleared IAAD above (but _way_ late, so we'll
    +		 * still count it as lost) ... or a silicon erratum:
    +		 * - VIA seems to set IAA without triggering the IRQ;
    +		 * - IAAD potentially cleared without setting IAA.
    +		 */
    +		status = ehci_readl(ehci, &ehci->regs->status);
    +		if ((status & STS_IAA) || !(cmd & CMD_IAAD)) {
     			COUNT (ehci->stats.lost_iaa);
     			ehci_writel(ehci, STS_IAA, &ehci->regs->status);
     		}
    -		ehci_writel(ehci, cmd & ~CMD_IAAD, &ehci->regs->command);
    +
    +		ehci_vdbg(ehci, "IAA watchdog: status %x cmd %x\n",
    +				status, cmd);
     		end_unlink_async(ehci);
     	}
     
    @@ -607,7 +628,7 @@ static int ehci_run (struct usb_hcd *hcd
     static irqreturn_t ehci_irq (struct usb_hcd *hcd)
     {
     	struct ehci_hcd		*ehci = hcd_to_ehci (hcd);
    -	u32			status, pcd_status = 0;
    +	u32			status, pcd_status = 0, cmd;
     	int			bh;
     
     	spin_lock (&ehci->lock);
    @@ -628,7 +649,7 @@ static irqreturn_t ehci_irq (struct usb_
     
     	/* clear (just) interrupts */
     	ehci_writel(ehci, status, &ehci->regs->status);
    -	ehci_readl(ehci, &ehci->regs->command);	/* unblock posted write */
    +	cmd = ehci_readl(ehci, &ehci->regs->command);
     	bh = 0;
     
     #ifdef	VERBOSE_DEBUG
    @@ -649,8 +670,17 @@ static irqreturn_t ehci_irq (struct usb_
     
     	/* complete the unlinking of some qh [4.15.2.3] */
     	if (status & STS_IAA) {
    -		COUNT (ehci->stats.reclaim);
    -		end_unlink_async(ehci);
    +		/* guard against (alleged) silicon errata */
    +		if (cmd & CMD_IAAD) {
    +			ehci_writel(ehci, cmd & ~CMD_IAAD,
    +					&ehci->regs->command);
    +			ehci_dbg(ehci, "IAA with IAAD still set?\n");
    +		}
    +		if (ehci->reclaim) {
    +			COUNT(ehci->stats.reclaim);
    +			end_unlink_async(ehci);
    +		} else
    +			ehci_dbg(ehci, "IAA with nothing to reclaim?\n");
     	}
     
     	/* remote wakeup [4.3.1] */
    
    
    \
     
     \ /
      Last update: 2008-03-05 05:29    [from the cache]
    ©2003-2008