| Message ID | BANLkTik3hmvH6_tUuEZjR8j0L04PdCk7mw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, 16 Apr 2011 06:57:00 am Jan Vesely wrote: > + s->status |= UHCI_STS_USBERR; This is per UHCI 1.1D Section 4.1.5. Looks good. > + *int_mask |= 0x02; > + if (td->ctrl & TD_CTRL_IOC) > + *int_mask |= 0x01; > + uhci_update_irq(s); I see "A hardware interrupt is signalled to the system", but can you provide a little explanation of why this particular interrupt mask? > + s->status |= UHCI_STS_USBERR; This is per UHCI 1.1d Section 4.1.4. Looks good. > + *int_mask |= 0x02; > + if (td->ctrl & TD_CTRL_IOC) > + *int_mask |= 0x01; > + uhci_update_irq(s); I see "A hardware interrupt is signalled to the system", but can you provide a little explanation of why this particular interrupt mask?
On Sat, Apr 16, 2011 at 8:33 AM, Brad Hards <bradh@frogmouth.net> wrote: > On Sat, 16 Apr 2011 06:57:00 am Jan Vesely wrote: >> + s->status |= UHCI_STS_USBERR; > This is per UHCI 1.1D Section 4.1.5. Looks good. > >> + *int_mask |= 0x02; >> + if (td->ctrl & TD_CTRL_IOC) >> + *int_mask |= 0x01; >> + uhci_update_irq(s); > I see "A hardware interrupt is signalled to the system", but can you provide a > little explanation of why this particular interrupt mask? I used th code I found around in that same file (hw/usb-uhci.c), lines 705-724 contain both masks. "if (td->ctrl & TD_CTRL_IOC) *int_mask |= 0x01;", is in more places so I just copied that lines. *int_mask |= 0x2, is used when SPD condition is detected. that is strange, SPD should use the same interrupt as IOC, but return value indicates that it is treated as error condition (unsuccessful td) so I figured *int_mask |= 0x2 signals error interrupt (it does not match bits in interrupt enable register- that was my first guess) uhci_update_irq(s); to me it looks like a duplicate functionality to int_mask parameter, I did not investigate further and included it just to be sure (it's used on line 775, when error countdown reaches zero). > >> + s->status |= UHCI_STS_USBERR; > This is per UHCI 1.1d Section 4.1.4. Looks good. > >> + *int_mask |= 0x02; >> + if (td->ctrl & TD_CTRL_IOC) >> + *int_mask |= 0x01; >> + uhci_update_irq(s); > I see "A hardware interrupt is signalled to the system", but can you provide a > little explanation of why this particular interrupt mask? > >
On Fri, Apr 15, 2011 at 9:57 PM, Jan Vesely <jano.vesely@gmail.com> wrote: > I'm sending a patch for bug #757654. The bug does not really break > anything it just makes USB error detection harder. > It's a quick fix and might need some polishing but it works (I am > currently using it). > > thx, > jan > > PS: I guess you need this line: > Signed-off-by: Jan Vesely <jano.vesely@gmail.com> Not trivial. CCing Gerd for USB review instead. Stefan
> diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c > index 346db3e..a51d89b 100644 > --- a/hw/usb-uhci.c > +++ b/hw/usb-uhci.c > @@ -732,11 +732,21 @@ out: > case USB_RET_STALL: > td->ctrl |= TD_CTRL_STALL; > td->ctrl&= ~TD_CTRL_ACTIVE; > + s->status |= UHCI_STS_USBERR; Just this line should be enougth. > case USB_RET_BABBLE: > td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL; > td->ctrl&= ~TD_CTRL_ACTIVE; > + s->status |= UHCI_STS_USBERR; Likewise. Tried that? cheers, Gerd
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c index 346db3e..a51d89b 100644 --- a/hw/usb-uhci.c +++ b/hw/usb-uhci.c @@ -732,11 +732,21 @@ out: case USB_RET_STALL: td->ctrl |= TD_CTRL_STALL; td->ctrl &= ~TD_CTRL_ACTIVE; + s->status |= UHCI_STS_USBERR; + *int_mask |= 0x02; + if (td->ctrl & TD_CTRL_IOC) + *int_mask |= 0x01; + uhci_update_irq(s); return 1; case USB_RET_BABBLE: td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL; td->ctrl &= ~TD_CTRL_ACTIVE; + s->status |= UHCI_STS_USBERR; + *int_mask |= 0x02; + if (td->ctrl & TD_CTRL_IOC) + *int_mask |= 0x01; + uhci_update_irq(s); /* frame interrupted */ return -1;
Hi, I'm sending a patch for bug #757654. The bug does not really break anything it just makes USB error detection harder. It's a quick fix and might need some polishing but it works (I am currently using it). thx, jan PS: I guess you need this line: Signed-off-by: Jan Vesely <jano.vesely@gmail.com>