[Qemu-devel] [PATCH] ohci: Clear the interrupt counter for erroneous transfers

[Qemu-devel] [PATCH] ohci: Clear the interrupt counter for erroneous transfers

[Sebastian Bauer]

This is mandated by the ohci specification. It tells at 6.4.4 on page 104
that for transfer descriptors that are retired with an error the done
queue interrupt counter is cleared as if the interrupt delay field of the
descriptions were zero.

Before this change, error conditions were handled similarly to the
successful condition which is especially troublesome for control transfers.
Some drivers (e.g., the AmigaOS-one) as well as the example code in the
spec, set the setup stage with an interrupt delay of seven (which means no
interrupt). This is fine under normal conditions, because one usually
doesn't want to be notified about the completion of this stage. However, if
an error occurs in this stage, these drivers will not get notified with the
current implementation. The fix addresses this by following the spec more
closely. Also, otherwise, the ability to set interrupt delay to seven would
be useless.

Note that Linux drivers that I looked at don't seem to be affected as they
set six as the interrupt delay presumably for the reason that they won't
get notified otherwise.

Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
---
 hw/usb/hcd-ohci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index d4c0293db5..98da5f0f04 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1156,6 +1156,9 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
                 OHCI_SET_BM(td.flags, TD_EC, 3);
                 break;
             }
+            /* An error occured so we have to clear the interrupt counter. See
+             * spec at 6.4.4 on page 104 */
+            ohci->done_count = 0;
         }
         ed->head |= OHCI_ED_H;
     }
-- 
2.18.0

Re: [Qemu-devel] [PATCH] ohci: Clear the interrupt counter for erroneous transfers

[Gerd Hoffmann]

On Sun, Jul 29, 2018 at 09:19:28PM +0200, Sebastian Bauer wrote:
> This is mandated by the ohci specification. It tells at 6.4.4 on page 104
> that for transfer descriptors that are retired with an error the done
> queue interrupt counter is cleared as if the interrupt delay field of the
> descriptions were zero.
> 
> Before this change, error conditions were handled similarly to the
> successful condition which is especially troublesome for control transfers.
> Some drivers (e.g., the AmigaOS-one) as well as the example code in the
> spec, set the setup stage with an interrupt delay of seven (which means no
> interrupt). This is fine under normal conditions, because one usually
> doesn't want to be notified about the completion of this stage. However, if
> an error occurs in this stage, these drivers will not get notified with the
> current implementation. The fix addresses this by following the spec more
> closely. Also, otherwise, the ability to set interrupt delay to seven would
> be useless.
> 
> Note that Linux drivers that I looked at don't seem to be affected as they
> set six as the interrupt delay presumably for the reason that they won't
> get notified otherwise.

Added to usb queue.

thanks,
  Gerd