Hi Daniel,

On Tue, Aug 12, 2014 at 11:58 AM, Daniel Mack <zon...@gmail.com> wrote:
> Hi Bin,
>
> On 08/12/2014 06:46 PM, Bin Liu wrote:
>>  drivers/usb/musb/musb_cppi41.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
>> index 5989def..e9a0e54 100644
>> --- a/drivers/usb/musb/musb_cppi41.c
>> +++ b/drivers/usb/musb/musb_cppi41.c
>> @@ -39,6 +39,7 @@ struct cppi41_dma_channel {
>>       u32 transferred;
>>       u32 packet_sz;
>>       struct list_head tx_check;
>> +     u32 tx_zlp;
>
> 'bool' might be easier to read later.

Sure.

>
>>  };
>>
>>  #define MUSB_DMA_NUM_CHANNELS 15
>> @@ -122,6 +123,8 @@ static void cppi41_trans_done(struct cppi41_dma_channel 
>> *cppi41_channel)
>>  {
>>       struct musb_hw_ep *hw_ep = cppi41_channel->hw_ep;
>>       struct musb *musb = hw_ep->musb;
>> +     void __iomem *epio = hw_ep->regs;
>> +     u16 csr;
>>
>>       if (!cppi41_channel->prog_len) {
>>
>> @@ -130,15 +133,22 @@ static void cppi41_trans_done(struct 
>> cppi41_dma_channel *cppi41_channel)
>>                       cppi41_channel->transferred;
>>               cppi41_channel->channel.status = MUSB_DMA_STATUS_FREE;
>>               cppi41_channel->channel.rx_packet_done = true;
>> +
>> +             /* transmit ZLP using PIO mode for transfers which size is
>> +              * multiple of EP packet size. */
>
> I'd prefer this block to follow the kernel-style of comments, even
> though this driver is full of other flavors.

I will read the kernel style doc and fix this.

>
>> +             if (cppi41_channel->tx_zlp && (cppi41_channel->transferred %
>> +                                     cppi41_channel->packet_sz) == 0) {
>> +                     musb_ep_select(musb->mregs, hw_ep->epnum);
>> +                     csr = MUSB_TXCSR_MODE | MUSB_TXCSR_TXPKTRDY;
>> +                     musb_writew(epio, MUSB_TXCSR, csr);
>> +             }
>>               musb_dma_completion(musb, hw_ep->epnum, cppi41_channel->is_tx);
>>       } else {
>>               /* next iteration, reload */
>>               struct dma_chan *dc = cppi41_channel->dc;
>>               struct dma_async_tx_descriptor *dma_desc;
>>               enum dma_transfer_direction direction;
>> -             u16 csr;
>>               u32 remain_bytes;
>> -             void __iomem *epio = cppi41_channel->hw_ep->regs;
>>
>>               cppi41_channel->buf_addr += cppi41_channel->packet_sz;
>>
>> @@ -362,6 +372,7 @@ static bool cppi41_configure_channel(struct dma_channel 
>> *channel,
>>       cppi41_channel->total_len = len;
>>       cppi41_channel->transferred = 0;
>>       cppi41_channel->packet_sz = packet_sz;
>> +     cppi41_channel->tx_zlp = (cppi41_channel->is_tx && mode) ? 1 : 0;
>
> Don't you need to check for (urb->transfer_flags & URB_ZERO_PACKET)
> somewhere here? I'd not expect an extra 0-byte packet at the end of a
> transfer unless this flag is set.

mode is set to 1 when (urb->transfer_flags & URB_ZERO_PACKET), in
musb_tx_dma_program in musb_host.c.

Regards,
-Bin.

>
>
> Thanks,
> Daniel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to