[SeaBIOS] usb ohci pipe free fix

Nils njacobs8 at adsltotaal.nl
Tue Feb 21 21:43:12 CET 2012


Op maandag 20-02-2012 om 13:17 uur [tijdzone -0500], schreef Kevin
O'Connor:
> Hi Nils,
> 
> I put together a patch to modify the way the OHCI controller does pipe
> free'ing.  This is a preliminary patch (similar batch free'ing should
> be done for the other controllers as well).  However, it might help
> with the USB issues you've been having.
> 
> -Kevin
> 
> 
> commit fa43976172295d3bad9209374bd68469c10ec918
> Author: Kevin O'Connor <kevin at koconnor.net>
> Date:   Mon Feb 20 12:54:49 2012 -0500
> 
>     Batch free USB pipes on OHCI controllers.
>     
>     Instead of unregistering each control "endpoint descriptor" after it
>     is used, keep them around for later users.  Free all unused
>     descriptors in one batch at the end of initialization.  This should
>     slightly optimize boot time, and it requires less overall interaction
>     with the controller.
>     
>     This also makes the descriptor free code more compliant with the spec.
>     The descriptor lists will only be modified after the list processing
>     has been disabled on the controller.
>     
>     Signed-off-by: Kevin O'Connor <kevin at koconnor.net>
> 
> diff --git a/src/usb-ohci.c b/src/usb-ohci.c
> index 9107db2..7a437ad 100644
> --- a/src/usb-ohci.c
> +++ b/src/usb-ohci.c
> @@ -19,6 +19,14 @@ struct usb_ohci_s {
>      struct ohci_regs *regs;
>  };
>  
> +struct ohci_pipe {
> +    struct ohci_ed ed;
> +    struct usb_pipe pipe;
> +    void *data;
> +    int count;
> +    struct ohci_td *tds;
> +};
> +
>  
>  /****************************************************************
>   * Root hub
> @@ -109,6 +117,56 @@ check_ohci_ports(struct usb_ohci_s *cntl)
>   * Setup
>   ****************************************************************/
>  
> +// Wait for next USB frame to start - for ensuring safe memory release.
> +static void
> +ohci_waittick(struct usb_ohci_s *cntl)
> +{
> +    barrier();
> +    struct ohci_hcca *hcca = (void*)cntl->regs->hcca;
> +    u32 startframe = hcca->frame_no;
> +    u64 end = calc_future_tsc(1000 * 5);
> +    for (;;) {
> +        if (hcca->frame_no != startframe)
> +            break;
> +        if (check_tsc(end)) {
> +            warn_timeout();
> +            return;
> +        }
> +        yield();
> +    }
> +}
> +
> +static void
> +ohci_free_pipes(struct usb_ohci_s *cntl)
> +{
> +    dprintf(7, "ohci_free_pipes %p\n", cntl);
> +
> +    u32 creg = readl(&cntl->regs->control);
> +    if (creg & (OHCI_CTRL_CLE|OHCI_CTRL_BLE)) {
> +        writel(&cntl->regs->control, creg & ~(OHCI_CTRL_CLE|OHCI_CTRL_BLE));
> +        ohci_waittick(cntl);
> +    }
> +
> +    u32 *pos = &cntl->regs->ed_controlhead;
> +    for (;;) {
> +        struct ohci_ed *next = (void*)*pos;
> +        if (!next)
> +            break;
> +        struct ohci_pipe *pipe = container_of(next, struct ohci_pipe, ed);
> +        if (pipe->pipe.cntl != &cntl->usb) {
> +            *pos = next->hwNextED;
> +            free(pipe);
> +        } else {
> +            pos = &next->hwNextED;
> +        }
> +    }
> +
> +    writel(&cntl->regs->ed_controlcurrent, 0);
> +    writel(&cntl->regs->ed_bulkcurrent, 0);
> +    writel(&cntl->regs->control, creg);
> +    cntl->usb.freelist = NULL;
> +}
> +
>  static int
>  start_ohci(struct usb_ohci_s *cntl, struct ohci_hcca *hcca)
>  {
> @@ -192,6 +250,7 @@ configure_ohci(void *data)
>  
>      int count = check_ohci_ports(cntl);
>      free_pipe(cntl->usb.defaultpipe);
> +    ohci_free_pipes(cntl);
>      if (! count)
>          goto err;
>      return;
> @@ -260,74 +319,13 @@ wait_ed(struct ohci_ed *ed)
>      }
>  }
>  
> -// Wait for next USB frame to start - for ensuring safe memory release.
> -static void
> -ohci_waittick(struct usb_ohci_s *cntl)
> -{
> -    barrier();
> -    struct ohci_hcca *hcca = (void*)cntl->regs->hcca;
> -    u32 startframe = hcca->frame_no;
> -    u64 end = calc_future_tsc(1000 * 5);
> -    for (;;) {
> -        if (hcca->frame_no != startframe)
> -            break;
> -        if (check_tsc(end)) {
> -            warn_timeout();
> -            return;
> -        }
> -        yield();
> -    }
> -}
> -
> -static void
> -signal_freelist(struct usb_ohci_s *cntl)
> -{
> -    u32 v = readl(&cntl->regs->control);
> -    if (v & OHCI_CTRL_CLE) {
> -        writel(&cntl->regs->control, v & ~(OHCI_CTRL_CLE|OHCI_CTRL_BLE));
> -        ohci_waittick(cntl);
> -        writel(&cntl->regs->ed_controlcurrent, 0);
> -        writel(&cntl->regs->ed_bulkcurrent, 0);
> -        writel(&cntl->regs->control, v);
> -    } else {
> -        ohci_waittick(cntl);
> -    }
> -}
> -
> -struct ohci_pipe {
> -    struct ohci_ed ed;
> -    struct usb_pipe pipe;
> -    void *data;
> -    int count;
> -    struct ohci_td *tds;
> -};
> -
>  void
> -ohci_free_pipe(struct usb_pipe *p)
> +ohci_free_pipe(struct usb_pipe *pipe)
>  {
> -    if (! CONFIG_USB_OHCI)
> -        return;
> -    dprintf(7, "ohci_free_pipe %p\n", p);
> -    struct ohci_pipe *pipe = container_of(p, struct ohci_pipe, pipe);
> -    struct usb_ohci_s *cntl = container_of(
> -        pipe->pipe.cntl, struct usb_ohci_s, usb);
> -
> -    u32 *pos = &cntl->regs->ed_controlhead;
> -    for (;;) {
> -        struct ohci_ed *next = (void*)*pos;
> -        if (!next) {
> -            // Not found?!  Exit without freeing.
> -            warn_internalerror();
> -            return;
> -        }
> -        if (next == &pipe->ed) {
> -            *pos = next->hwNextED;
> -            signal_freelist(cntl);
> -            free(pipe);
> -            return;
> -        }
> -        pos = &next->hwNextED;
> -    }
> +    // Add to controller's free list.
> +    struct usb_s *cntl = pipe->cntl;
> +    pipe->freenext = cntl->freelist;
> +    cntl->freelist = pipe;
>  }
>  
>  struct usb_pipe *
> @@ -339,7 +337,17 @@ ohci_alloc_control_pipe(struct usb_pipe *dummy)
>          dummy->cntl, struct usb_ohci_s, usb);
>      dprintf(7, "ohci_alloc_control_pipe %p\n", &cntl->usb);
>  
> -    // Allocate a queue head.
> +    if (cntl->usb.freelist) {
> +        // Use previously allocated queue head.
> +        struct ohci_pipe *pipe = container_of(cntl->usb.freelist
> +                                              , struct ohci_pipe, pipe);
> +        cntl->usb.freelist = pipe->pipe.freenext;
> +
> +        memcpy(&pipe->pipe, dummy, sizeof(pipe->pipe));
> +        return &pipe->pipe;
> +    }
> +
> +    // Allocate a new queue head.
>      struct ohci_pipe *pipe = malloc_tmphigh(sizeof(*pipe));
>      if (!pipe) {
>          warn_noalloc();
> diff --git a/src/usb.h b/src/usb.h
> index 8b2af40..cc32eb7 100644
> --- a/src/usb.h
> +++ b/src/usb.h
> @@ -6,7 +6,10 @@
>  
>  // Information on a USB end point.
>  struct usb_pipe {
> -    struct usb_s *cntl;
> +    union {
> +        struct usb_s *cntl;
> +        struct usb_pipe *freenext;
> +    };
>      u64 path;
>      u8 type;
>      u8 ep;
> @@ -20,6 +23,7 @@ struct usb_pipe {
>  // Common information for usb controllers.
>  struct usb_s {
>      struct usb_pipe *defaultpipe;
> +    struct usb_pipe *freelist;
>      struct mutex_s resetlock;
>      struct pci_device *pci;
>      int busid;
Thanks for the patch.
I tested this patch and the keyboard gets initialized now.
The keyboard doesn't seem to react on keystroke yet tough and i still
get a timeout error.(another bug?)

Thanks, Nils.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ohci.log
Type: text/x-log
Size: 11218 bytes
Desc: not available
URL: <http://www.seabios.org/pipermail/seabios/attachments/20120221/d31a6810/attachment-0001.log>


More information about the SeaBIOS mailing list