On Thu, 22 Jan 2004, Axel Waggershauser wrote:
> I included an additional "inw(io_addr + USBSTS)" call right after the
> "outw(status, io_addr + USBSTS);" line in uhci_irq to check weather the
> read/write-clear mechanism is working at all. I made the observation
> that it seems to work for some bits (at least for 0x1, 0x2 and 0x4 -
> meaning the first three least significant bits) but not for the 0x20 bit
> (the state = 32). I observed the transitions 1->0, 2->0, 3->0, 36->32
> and 32->32.
I made the same test on my computer and got the same result. Apparently
this is one respect in which the standard differs from the reality.
> > 2. When you unplug your device, the bulk-in transaction that is
> > currently running should abort and complete with an error code -84
> > (-EILSEQ). You said before that sometimes that happened. Well, it should
> > always happen. When your computer sends an IN packet the device is
> > supposed to respond with either NAK, STALL, DATA0, or DATA1 within a
> > certain timeout. Since your device is unplugged it can't respond, so the
> > computer tries again, up to 3 times. After 3 timeouts the controller is
> > supposed to interrupt and signal a timeout error. That didn't happen in
> > your log.
>
> These news made me investigate some not yet mentioned aspect of the
> "unplugging" process of my device. Due to the devices need for more
> power than 500mA we had to provide external power which we just supplied
> by cutting the host power supply and provide it external. The
> "unplugging" I used to talk about is done by just switching the power
> off. This is violating the spec, since the plugs are designed to make it
> the other way round: first disconnect the data wires, than the power.
> However tests with the correct unplugging method (pull the cable)
> revealed that the problem is basically the same, just the statistics
> "inverted" - meaning instead of say 1 of 10 tries of correct -84 results
> and 9 unlink_urb failures, I get 9 correct runs and 1 failure.
That's progress. I think that 9 vs. 1 may be connected with the relative
timing of the retry mechanism and the hub status polling. However, it
would be nice if the driver worked correctly even when you perform an
illegal disconnect.
> > 3. When the controller is suspended or halted, it is supposed to
> > generate an interrupt with status = 32, to let the driver know that the
> > suspend has occurred. Your controller isn't doing that.
>
> I tried another peace of hardware, a laptop with an Intel UHCI
> controller. This machine is running with a vanilla 2.6.0 kernel. I just
> added one printk to the uhci_irq() function. With respect to the
> suspension behavior and the read/write-clear mechanism, it behaves
> exactly like the other devices (see above). I made the "mouse
> plug/unplug test". I did not test if the controller behaves differently
> with my device than the others.
>
> This lets me suspect that your information about the specification in
> this regard is either wrong or a lot of hardware (at least 3 different
> types) just violate the USB spec. One last possibility would probably be
> that our device somehow trashed all that hardware, which is unlikely,
> because we use it under Windows for about 5 years now.
Again, I got the same result you did. This is another respect in which
the standard differs from the reality. Probably (if anyone cares) this
will be fixed by changing the specification!
> > There's one more thing you could try testing: Set the debug level to 3 and
> > read (and post!) the /proc/driver/uhci... file for your controller after
> > unplugging but before the controller is suspended. You can set the debug
> > level from 1 (the default) to 3 by editing the line that says
>
> Attached is the result.
It shows about what I expected.
On the whole, it look like you're running up across one of the entries on
my list of UHCI driver problems, although in a very severe form. The
basic problem is that the driver doesn't handle unlinking correctly when
the controller is suspended or reset. The unlink procedure is
interrupt-driven, but there aren't any interrupts when the controller
isn't running. And for you it's even worse, since there isn't an
interrupt to mark when the controller is suspended.
Below is a quick patch that may help solve this problem. It's not a
proper fix -- I wasn't too careful about some things, it's rather
inelegant, and it totally ignores proper locking. Actually, the driver
needs so many other changes that it's not worth spending much time on
this one issue until some of the others have been addressed.
Anyway, try the patch and tell how it works out.
Alan Stern
===== uhci-hcd.h 1.15 vs edited =====
--- 1.15/drivers/usb/host/uhci-hcd.h Mon Sep 8 12:21:51 2003
+++ edited/drivers/usb/host/uhci-hcd.h Thu Jan 22 15:37:04 2004
@@ -342,6 +342,7 @@
enum uhci_state state; /* FIXME: needs a spinlock */
unsigned long state_end; /* Time of next transition */
int resume_detect; /* Need a Global Resume */
+ int need_fake_irq; /* Need a fake interrupt */
/* Main list of URB's currently controlled by this HC */
spinlock_t urb_list_lock;
--- 1.15/drivers/usb/host/uhci-hcd.c Thu Jan 22 14:55:20 2004
+++ edited/drivers/usb/host/uhci-hcd.c Thu Jan 22 16:11:01 2004
@@ -457,9 +457,12 @@
/* Check to see if the remove list is empty. Set the IOC bit */
/* to force an interrupt so we can remove the QH */
- if (list_empty(&uhci->qh_remove_list))
- uhci_set_next_interrupt(uhci);
-
+ if (list_empty(&uhci->qh_remove_list)) {
+ if (uhci->need_fake_irq == 0)
+ uhci_set_next_interrupt(uhci);
+ else
+ uhci->need_fake_irq = 1;
+ }
list_add(&qh->remove_list, &uhci->qh_remove_list);
spin_unlock_irqrestore(&uhci->qh_remove_list_lock, flags);
@@ -730,8 +733,12 @@
/* Check to see if the remove list is empty. Set the IOC bit */
/* to force an interrupt so we can remove the TD's*/
- if (list_empty(&uhci->td_remove_list))
- uhci_set_next_interrupt(uhci);
+ if (list_empty(&uhci->td_remove_list)) {
+ if (uhci->need_fake_irq == 0)
+ uhci_set_next_interrupt(uhci);
+ else
+ uhci->need_fake_irq = 1;
+ }
head = &urbp->td_list;
tmp = head->next;
@@ -1677,8 +1684,12 @@
spin_lock(&uhci->urb_remove_list_lock);
/* If we're the first, set the next interrupt bit */
- if (list_empty(&uhci->urb_remove_list))
- uhci_set_next_interrupt(uhci);
+ if (list_empty(&uhci->urb_remove_list)) {
+ if (uhci->need_fake_irq == 0)
+ uhci_set_next_interrupt(uhci);
+ else
+ uhci->need_fake_irq = 1;
+ }
list_add(&urbp->urb_list, &uhci->urb_remove_list);
spin_unlock(&uhci->urb_remove_list_lock);
@@ -1909,19 +1920,48 @@
spin_unlock_irqrestore(&uhci->urb_remove_list_lock, flags);
}
+static void uhci_interrupt_processing(struct usb_hcd *hcd, struct pt_regs *regs)
+{
+ struct uhci_hcd *uhci = hcd_to_uhci(hcd);
+ struct list_head *tmp, *head;
+
+ uhci_free_pending_qhs(uhci);
+ uhci_free_pending_tds(uhci);
+ uhci_remove_pending_qhs(uhci);
+
+ /* Walk the list of pending URB's to see which ones completed */
+ spin_lock(&uhci->urb_list_lock);
+ head = &uhci->urb_list;
+ tmp = head->next;
+ while (tmp != head) {
+ struct urb_priv *urbp = list_entry(tmp, struct urb_priv, urb_list);
+ struct urb *urb = urbp->urb;
+
+ tmp = tmp->next;
+
+ /* Checks the status and does all of the magic necessary */
+ uhci_transfer_result(uhci, urb);
+ }
+ spin_unlock(&uhci->urb_list_lock);
+
+ uhci_finish_completion(hcd, regs);
+}
+
static void uhci_irq(struct usb_hcd *hcd, struct pt_regs *regs)
{
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
unsigned int io_addr = uhci->io_addr;
unsigned short status;
- struct list_head *tmp, *head;
/*
* Read the interrupt status, and write it back to clear the
- * interrupt cause
+ * interrupt cause.
+ *
+ * Note: Intel controllers, and maybe others as well, don't
+ * clear the USBSTS_HCH (halted) bit when it is written back.
*/
status = inw(io_addr + USBSTS);
- if (!status) /* shared interrupt, not mine */
+ if (!(status & ~USBSTS_HCH)) /* shared interrupt, not mine */
return;
outw(status, io_addr + USBSTS); /* Clear it */
@@ -1939,30 +1979,8 @@
if (status & USBSTS_RD)
uhci->resume_detect = 1;
- uhci_free_pending_qhs(uhci);
-
- 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 */
- spin_lock(&uhci->urb_list_lock);
- head = &uhci->urb_list;
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *urbp = list_entry(tmp, struct urb_priv, urb_list);
- struct urb *urb = urbp->urb;
-
- tmp = tmp->next;
-
- /* Checks the status and does all of the magic necessary */
- uhci_transfer_result(uhci, urb);
- }
- spin_unlock(&uhci->urb_list_lock);
-
- uhci_finish_completion(hcd, regs);
+ uhci_interrupt_processing(hcd, regs);
}
static void reset_hc(struct uhci_hcd *uhci)
@@ -1980,6 +1998,7 @@
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout((HZ*10+999) / 1000);
uhci->resume_detect = 0;
+ uhci->need_fake_irq = 0;
}
static void suspend_hc(struct uhci_hcd *uhci)
@@ -1989,6 +2008,7 @@
dbg("%x: suspend_hc", io_addr);
uhci->state = UHCI_SUSPENDED;
uhci->resume_detect = 0;
+ uhci->need_fake_irq = 1;
outw(USBCMD_EGSM, io_addr + USBCMD);
}
@@ -2007,6 +2027,7 @@
break;
case UHCI_RESUMING_1: /* End global resume */
+ uhci->need_fake_irq = 0;
uhci->state = UHCI_RESUMING_2;
outw(0, io_addr + USBCMD);
/* Falls through */
@@ -2095,6 +2116,17 @@
/* wakeup if requested by a device */
if (uhci->resume_detect)
wakeup_hc(uhci);
+
+ /* fake an interrupt request after a halt */
+ else if (uhci->need_fake_irq > 0) {
+ unsigned short status;
+
+ status = inw(uhci->io_addr + USBSTS);
+ if (status & USBSTS_HCH) {
+ uhci->need_fake_irq = -1;
+ uhci_interrupt_processing(&uhci->hcd, NULL);
+ }
+ }
break;
case UHCI_RESUMING_1:
@@ -2127,6 +2159,7 @@
break;
}
}
+ uhci->need_fake_irq = 0;
/* Turn on all interrupts */
outw(USBINTR_TIMEOUT | USBINTR_RESUME | USBINTR_IOC | USBINTR_SP,
@@ -2465,14 +2498,9 @@
* At this point, we're guaranteed that no new connects can be made
* to this bus since there are no more parents
*/
- uhci_free_pending_qhs(uhci);
- uhci_free_pending_tds(uhci);
- uhci_remove_pending_qhs(uhci);
-
reset_hc(uhci);
-
- uhci_free_pending_qhs(uhci);
- uhci_free_pending_tds(uhci);
+ uhci_interrupt_processing(&uhci->hcd, NULL);
+ uhci_interrupt_processing(&uhci->hcd, NULL);
release_uhci(uhci);
}
@@ -2487,6 +2515,7 @@
suspend_hc(uhci);
else
reset_hc(uhci);
+ uhci->need_fake_irq = 1;
return 0;
}
-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
