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 {