Re: [Qemu-devel] [PATCH] usb-ehci: frindex always is a 14 bits counter

[Gerd Hoffmann]

On 03/28/12 20:47, Hans de Goede wrote:
> frindex always is a 14 bits counter, and not a 13 bits one as we were
> emulating. There are some subtle hints to this in the spec, first of all
> "Table 2-12. FRINDEX - Frame Index Register" says:
> "Bit 13:0 Frame Index. The value in this register increments at the end of
> each time frame (e.g. micro-frame). Bits [N:3] are used for the Frame List
> current index. This means that each location of the frame list is accessed
> 8 times (frames or micro-frames) before moving to the next index. The
> following illustrates values of N based on the value of the Frame List
> Size field in the USBCMD register.
> 
> USBCMD[Frame List Size]       Number Elements          N
> 00b                           1024            12
> 01b                            512            11
> 10b                            256            10
> 11b                       Reserved"
> 
> Notice how the text talks about "Bits [N:3]" are used ..., it does
> NOT say that when N == 12 (our case) the counter will wrap from 8191 to 0,
> or in otherwords that it is a 13 bits counter (bits 0 - 12).
> 
> The other hint is in "Table 2-10. USBSTS USB Status Register Bit Definitions":
> 
> "Bit 3 Frame List Rollover - R/WC. The Host Controller sets this bit to a one
> when the Frame List Index (see Section 2.3.4) rolls over from its maximum 
> value
> to zero. The exact value at which the rollover occurs depends on the frame
> list size. For example, if the frame list size (as programmed in the Frame
> List Size field of the USBCMD register) is 1024, the Frame Index Register
> rolls over every time FRINDEX[13] toggles. Similarly, if the size is 512,
> the Host Controller sets this bit to a one every time FRINDEX[12] toggles."
> 
> Notice how this text talks about setting bit 3 when bit 13 of frindex toggles
> (when there are 1024 entries, so our case), so this indicates that frindex
> has a bit 13 making it a 14 bit counter.
> 
> Besides these clear hints the real proof is in the pudding. Before this
> patch I could not stream data from a USB2 webcam under Windows XP, after
> this cam using a USB2 webcam under Windows XP works fine, and no regressions
> with other operating systems were seen.
> 
> Signed-off-by: Hans de Goede <address@hidden>
> ---
>  hw/usb-ehci.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
> index b5d7037..3934bf0 100644
> --- a/hw/usb-ehci.c
> +++ b/hw/usb-ehci.c
> @@ -2157,11 +2157,15 @@ static void ehci_frame_timer(void *opaque)
>          if ( !(ehci->usbsts & USBSTS_HALT)) {
>              ehci->frindex += 8;
>  
> -            if (ehci->frindex > 0x00001fff) {
> -                ehci->frindex = 0;
> +            if (ehci->frindex == 0x00002000) {
>                  ehci_set_interrupt(ehci, USBSTS_FLR);
>              }
>  
> +            if (ehci->frindex == 0x00004000) {
> +                ehci_set_interrupt(ehci, USBSTS_FLR);
> +                ehci->frindex = 0;
> +            }
> +
>              ehci->sofv = (ehci->frindex - 1) >> 3;
>              ehci->sofv &= 0x000003ff;
>          }

Patch added to usb patch queue.

thanks,
  Gerd