[U-Boot] Question: issues for usb keyboard work with OHCI HCD

[Bo Shen <voice.shen@atmel.com>]

Hi All,

On 5/6/2013 11:01, Bo Shen wrote:
> Hi Marek,
>
> On 5/3/2013 21:40, Marek Vasut wrote:
>> Dear Bo Shen,
>>
>>> Hi All,
>>>     Now, I test usb host support with Atmel boards, for example,
>>> at91sam9x5ek board.
>>>
>>>     When test OHCI USB host with usb keyboard. I meet the following
>>> issue.
>>> --->8---
>>> U-Boot 2013.04-dirty (May 03 2013 - 11:00:34)
>>>
>>> CPU: AT91SAM9G35
>>> Crystal frequency:       12 MHz
>>> CPU clock        :      400 MHz
>>> Master clock     :  133.333 MHz
>>> DRAM:  128 MiB
>>> WARNING: Caches not enabled
>>> NAND:  256 MiB
>>> MMC:   mci: 0
>>> In:    serial
>>> Out:   serial
>>> Err:   serial
>>> Net:   macb0
>>> Hit any key to stop autoboot:  0
>>> U-Boot> usb start
>>> (Re)start USB...
>>> USB0:   scanning bus 0 for devices... 2 USB Device(s) found
>>>          scanning usb for storage devices... 0 Storage Device(s) found
>>> U-Boot> setenv stdin usbkbd
>>> U-Boot> ERROR: sohci_submit_job: ENOMEM
>>> ERROR: sohci_submit_job failed
>>> ... ...
>>> (repeat to print these two error line)
>>
>> So the USB subsystem is leaking memory? Or do you only have too small
>> MALLOC
>> area?
>
> I am not sure whether USB subsystem is leaking memory. I am digging it.
>
> This issue is not relative with MALLOC area.
> This issue come out when all ptd[i].usb_dev (the maximum value of i is
> 64) is not NULL. Each time to call td_alloc, it will check whether
> ptd[i].usb_dev is NULL (i from 0 to 63), if not find one of
> ptd[i].usb_dev is NULL, then report ENOMEM.

All clue for this issue?

>>> ---<8---
>>>
>>>     After dig the usb ohci-hcd.c driver, I found every time it call
>>> submit_int_msg -->
>>> submit_common_msg --> sohci_submit_job, it will allocate memory for td
>>> (just call td_alloc),
>>> however nowhere free the td. So, after allocate 64 times (#define NUM_TD
>>> 64),
>>> all the ptd[i].usb_dev is not NULL. So, it will report: ENOMEM.
>>>
>>>     I don't know why in sohci_return_job it call td_submit_job again?
>
> Any advice for this question?

Any clue for this question?

>>> --->8---
>>> static inline int sohci_return_job(struct ohci *hc, urb_priv_t *urb)
>>> {
>>>       struct ohci_regs *regs = hc->regs;
>>>
>>>       switch (usb_pipetype(urb->pipe)) {
>>>       case PIPE_INTERRUPT:
>>>           /* implicitly requeued */
>>>           if (urb->dev->irq_handle &&
>>>                   (urb->dev->irq_act_len = urb->actual_length)) {
>>>               ohci_writel(OHCI_INTR_WDH, &regs->intrenable);
>>>               ohci_readl(&regs->intrenable); /* PCI posting flush */
>>>               urb->dev->irq_handle(urb->dev);
>>>               ohci_writel(OHCI_INTR_WDH, &regs->intrdisable);
>>>               ohci_readl(&regs->intrdisable); /* PCI posting flush */
>>>           }
>>>           urb->actual_length = 0;
>>>           td_submit_job(
>>>                   urb->dev,
>>>                   urb->pipe,
>>>                   urb->transfer_buffer,
>>>                   urb->transfer_buffer_length,
>>>                   NULL,
>>>                   urb,
>>>                   urb->interval);
>>>           break
>>> ---<8---
>>>     Add where should we free the allocated td to fix the issue?
>
> If I comment the td_submit_job and add urb_free_priv(urb) here, then usb
> keyboard work properly. It looks like
>
> --->8---
> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index 9f47351..cc60bc5 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -516,6 +516,7 @@ static inline int sohci_return_job(struct ohci *hc,
> urb_priv_t *urb)
>                          ohci_readl(&regs->intrdisable); /* PCI posting
> flush */
>                  }
>                  urb->actual_length = 0;
> +#if 0
>                  td_submit_job(
>                                  urb->dev,
>                                  urb->pipe,
> @@ -524,6 +525,8 @@ static inline int sohci_return_job(struct ohci *hc,
> urb_priv_t *urb)
>                                  NULL,
>                                  urb,
>                                  urb->interval);
> +#endif
> +               urb_free_priv(urb);
> ---<8---
>

After digging more about the USB HCD driver, I don't find any better 
solution for this issue, except this one.

So, may I submit this as an RFC patch?

Best Regards,
Bo Shen