On Mon, Dec 08, 2003 at 01:04:39PM -0500, Johannes Erdfelt wrote:
> 
> I'll echo what Alan said and say the real fix here is fixing the
> suspend/resume case.

I agree completely.  However, it would be nice to have a safety net
when things go wrong.  In particular, being able to remove the uhci-hcd
module when the interrupts stop working for whatever reason can't be
a bad thing.

> Regardless, this patch makes things worse. We need to wait for the next
> frame to know the hardware is done using the QHs and TDs before we can
> clean them up.
> 
> This patch changes this so we clean up those entries before the hardware
> is done, which is very bad.

Sorry about that.  Can you please comment on the following patch which
no longer frees the QHs and TDs outside of the interrupt routine.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Index: kernel-2.5/drivers/usb/host/uhci-hcd.c
===================================================================
RCS file: 
/home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/drivers/usb/host/uhci-hcd.c,v
retrieving revision 1.1.1.14
diff -u -r1.1.1.14 uhci-hcd.c
--- kernel-2.5/drivers/usb/host/uhci-hcd.c      17 Oct 2003 21:43:29 -0000      
1.1.1.14
+++ kernel-2.5/drivers/usb/host/uhci-hcd.c      8 Dec 2003 20:56:03 -0000
@@ -90,6 +90,7 @@
 static int uhci_get_current_frame_number(struct uhci_hcd *uhci);
 static int uhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb);
 static void uhci_unlink_generic(struct uhci_hcd *uhci, struct urb *urb);
+static void uhci_irq_tail(struct usb_hcd *hcd, struct pt_regs *regs);
 
 static void hc_state_transitions(struct uhci_hcd *uhci);
 
@@ -1667,9 +1668,9 @@
 
        spin_lock(&uhci->urb_remove_list_lock);
 
-       /* If we're the first, set the next interrupt bit */
+       /* If we're the first, schedule tasklet */
        if (list_empty(&uhci->urb_remove_list))
-               uhci_set_next_interrupt(uhci);
+               tasklet_schedule(&uhci->irq_tasklet);
        list_add(&urbp->urb_list, &uhci->urb_remove_list);
 
        spin_unlock(&uhci->urb_remove_list_lock);
@@ -1934,8 +1935,6 @@
 
        uhci_free_pending_tds(uhci);
 
-       uhci_remove_pending_qhs(uhci);
-
        uhci_clear_next_interrupt(uhci);
 
        /* Walk the list of pending URB's to see which ones completed */
@@ -1953,9 +1952,25 @@
        }
        spin_unlock(&uhci->urb_list_lock);
 
+       uhci_irq_tail(hcd, regs);
+}
+
+static void uhci_irq_tail(struct usb_hcd *hcd, struct pt_regs *regs)
+{
+       struct uhci_hcd *uhci = hcd_to_uhci(hcd);
+
+       uhci_remove_pending_qhs(uhci);
+
        uhci_finish_completion(hcd, regs);
 }
 
+static void uhci_irq_tasklet(unsigned long data)
+{
+       struct usb_hcd *hcd = (void *)data;
+
+       uhci_irq_tail(hcd, NULL);
+}
+
 static void reset_hc(struct uhci_hcd *uhci)
 {
        unsigned int io_addr = uhci->io_addr;
@@ -2455,7 +2470,8 @@
         */
        uhci_free_pending_qhs(uhci);
        uhci_free_pending_tds(uhci);
-       uhci_remove_pending_qhs(uhci);
+       tasklet_schedule(&uhci->irq_tasklet);
+       tasklet_kill(&uhci->irq_tasklet);
 
        reset_hc(uhci);
 
@@ -2505,6 +2521,8 @@
 
        memset(uhci, 0, sizeof(*uhci));
        uhci->hcd.product_desc = "UHCI Host Controller";
+       tasklet_init(&uhci->irq_tasklet, uhci_irq_tasklet,
+                    (unsigned long)&uhci->hcd);
        return &uhci->hcd;
 }
 
Index: kernel-2.5/drivers/usb/host/uhci-hcd.h
===================================================================
RCS file: 
/home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/drivers/usb/host/uhci-hcd.h,v
retrieving revision 1.1.1.4
retrieving revision 1.2
diff -u -r1.1.1.4 -r1.2
--- kernel-2.5/drivers/usb/host/uhci-hcd.h      27 Sep 2003 00:01:59 -0000      1.1.1.4
+++ kernel-2.5/drivers/usb/host/uhci-hcd.h      8 Dec 2003 08:57:52 -0000       1.2
@@ -3,6 +3,7 @@
 
 #include <linux/list.h>
 #include <linux/usb.h>
+#include <linux/interrupt.h>
 
 #define usb_packetid(pipe)     (usb_pipein(pipe) ? USB_PID_IN : USB_PID_OUT)
 #define PIPE_DEVEP_MASK                0x0007ff00
@@ -366,6 +367,8 @@
        int rh_numports;
 
        struct timer_list stall_timer;
+
+       struct tasklet_struct irq_tasklet;
 };
 
 struct urb_priv {

Reply via email to