Thanks for your comments.
I'll resubmit the v1 of the patch after taking your suggestions.
2010/8/2 Sergei Shtylyov <sshtylyov@xxxxxxxxxx>:
> Hello.
>
> tom.leiming@xxxxxxxxx wrote:
>
>> From: Ming Lei <tom.leiming@xxxxxxxxx>
>
>> This has been tested OK on beagle B5 board.
>
> Unfortunately, it's full of errors anyway...
>
>> Signed-off-by: Ming Lei <tom.leiming@xxxxxxxxx>
>> ---
>> drivers/usb/musb/musb_gadget.c | 51
>> ++++++++++++++++++++++++++++++++++-----
>> drivers/usb/musb/musb_gadget.h | 2 +
>> 2 files changed, 46 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_gadget.c
>> b/drivers/usb/musb/musb_gadget.c
>> index 404b084..8f70746 100644
>> --- a/drivers/usb/musb/musb_gadget.c
>> +++ b/drivers/usb/musb/musb_gadget.c
>> @@ -344,6 +344,9 @@ static void txstate(struct musb *musb, struct
>> musb_request *req)
>> | MUSB_TXCSR_MODE);
>> csr &= ~MUSB_TXCSR_P_UNDERRUN;
>> + if (musb_ep->hb_mult)
>> + csr &= ~MUSB_TXCSR_AUTOSET;
>> +
>
> Perhaps should not set the bit in this case, i.e. you should modify the
> preceding statement...
>
>> @@ -353,6 +356,9 @@ static void txstate(struct musb *musb, struct
>> musb_request *req)
>> csr &= ~(MUSB_TXCSR_P_UNDERRUN | MUSB_TXCSR_TXPKTRDY);
>> csr |= MUSB_TXCSR_DMAENAB | MUSB_TXCSR_DMAMODE |
>> MUSB_TXCSR_MODE;
>> + if (musb_ep->hb_mult)
>> + csr &= ~MUSB_TXCSR_AUTOSET;
>
> This is pointless -- DaVincis don't have this bit at all, it's reserved.
>
>> @@ -446,6 +452,9 @@ void musb_g_tx(struct musb *musb, u8 epnum)
>> /* We NAKed, no big deal... little reason to care. */
>> csr |= MUSB_TXCSR_P_WZC_BITS;
>> csr &= ~(MUSB_TXCSR_P_UNDERRUN | MUSB_TXCSR_TXPKTRDY);
>> + if (musb_ep->hb_mult)
>> + csr &= ~MUSB_TXCSR_AUTOSET;
>
> Why reset the bit specifically on the underrun interrupt?
>
>> @@ -617,6 +626,8 @@ static void rxstate(struct musb *musb, struct
>> musb_request *req)
>> csr &= ~(MUSB_RXCSR_AUTOCLEAR
>> | MUSB_RXCSR_DMAMODE);
>> csr |= MUSB_RXCSR_DMAENAB | MUSB_RXCSR_P_WZC_BITS;
>> + if (musb_ep->hb_mult)
>> + csr &= ~MUSB_TXCSR_AUTOSET;
>
> Wait! We're modifying RXCSR here, not TXCSR...
>
>> musb_writew(epio, MUSB_RXCSR, csr);
>> return;
>> }
>> @@ -664,6 +675,8 @@ static void rxstate(struct musb *musb, struct
>> musb_request *req)
>> * disabling MUSB_RXCSR_DMAMODE) is
>> required
>> * to get DMAReq to activate
>> */
>> + if (musb_ep->hb_mult)
>> + csr &= ~MUSB_TXCSR_AUTOSET;
>
> Again, we're modifying RXCSR here, not TXCSR...
>
>> musb_writew(epio, MUSB_RXCSR,
>> csr | MUSB_RXCSR_DMAMODE);
>> #endif
>> @@ -732,6 +745,8 @@ static void rxstate(struct musb *musb, struct
>> musb_request *req)
>> /* ack the read! */
>> csr |= MUSB_RXCSR_P_WZC_BITS;
>> csr &= ~MUSB_RXCSR_RXPKTRDY;
>> + if (musb_ep->hb_mult)
>> + csr &= ~MUSB_TXCSR_AUTOSET;
>
> And again here...
>
>> musb_writew(epio, MUSB_RXCSR, csr);
>> }
>> }
>
> [...]
>>
>> diff --git a/drivers/usb/musb/musb_gadget.h
>> b/drivers/usb/musb/musb_gadget.h
>> index c8b1403..353c619 100644
>> --- a/drivers/usb/musb/musb_gadget.h
>> +++ b/drivers/usb/musb/musb_gadget.h
>> @@ -79,6 +79,8 @@ struct musb_ep {
>> /* true if lock must be dropped but req_list may not be advanced */
>> u8 busy;
>> +
>> + u8 hb_mult;
>
> Please, align 'hb_mult' wit 'busy', and use tabs, not spaces for that.
>
> WBR, Sergei
>
>
--
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
