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 {