[U-Boot] [PATCH V2] USB:gadget:designware USB OTG implementation

[Marek Vasut <marex@denx.de>]

Dear Amit Virdi,

> Dear Marek,
> 
> [...]
> 
> >> +static void dwc_otg_read_packet(struct dwc_ep *ep, u16 bytes)
> >> +{
> >> +     u32 i;
> >> +     int word_count = (bytes + 3) / 4;
> >> +     u32 *fifo = dev_if->data_fifo[0];
> >> +     u32 *data_buff = (u32 *) ep->xfer_buff;
> >> +     u32 unaligned;
> >> +     /*
> >> +      * This requires reading data from the FIFO into a u32 temp
> >> buffer, +      * then moving it into the data buffer.
> >> +      */
> >> +     if ((bytes<  4)&&  (bytes>  0)) {
> >> +             unaligned = readl(fifo);
> >> +             memcpy(data_buff,&unaligned, bytes);
> >> +     } else {
> >> +             for (i = 0; i<  word_count; i++, data_buff++)
> >> +                     *data_buff = readl(fifo);
> > 
> > Thinking of this, will this really handle unaligned access of length for
> > example 5 ?
> 
> If data_buff is unaligned, there will be a problem!
> 
> >> +     }
> >> +}
> >> +
> >> +/* Handle RX transaction on non-ISO endpoint. */
> >> +static void dw_udc_epn_rx(struct dwc_ep *ep, int bcnt)
> >> +{
> >> +     struct urb *urb;
> >> +     struct usb_endpoint_instance *endpoint = dw_find_ep(ep->num);
> >> +
> >> +     if (endpoint) {
> > 
> > if (!endpoint)
> > 
> >   return;
> > 
> > ... code ...
> 
> Ok.
> 
> >> +             urb = endpoint->rcv_urb;
> >> +
> >> +             if (urb) {
> >> +                     ep->xfer_buff = urb->buffer + urb->actual_length;
> >> +                     dwc_otg_read_packet(ep, bcnt);
> >> +                     usbd_rcv_complete(endpoint, bcnt, 0);
> >> +             }
> >> +     }
> >> +}
> 
> [...]
> 
> >> +     /* program pkt count */
> >> +     temp = ep->xfer_len;
> >> +     temp |= (1<<  PKTCNT_SHIFT);
> >> +     writel(temp,&in_ep_regs->dieptsiz);
> >> +
> >> +     /* enable EP*/
> > 
> > missing space before ending comment
> 
> Ok, 'll correct.
> 
> >> +     setbits_le32(&in_ep_regs->diepctl, EPENA | CNAK);
> >> +
> >> +     /* clear TX Fifo Empty intr*/
> >> +     writel(NPTXFEMPTY,&core_global_regs->gintsts);
> >> +
> >> +     setbits_le32(&core_global_regs->gintmsk, NPTXFEMPTY);
> >> +
> >> +     start = get_timer(0);
> >> +     while (!(readl(&core_global_regs->gintsts)&  NPTXFEMPTY)) {
> >> +             if (get_timer(start)>  timeout) {
> >> +                     printf("%s: NPTXFEMPTY: TimeOUT\n", __func__);
> >> +                     WATCHDOG_RESET();
> >> +             }
> >> +     }
> >> +
> >> +     /* write to fifo */
> >> +     if ((ep->xfer_len<  4)&&  (ep->xfer_len>  0)) {
> >> +             memcpy(&unaligned, data_buff, ep->xfer_len);
> >> +             *fifo = unaligned;
> >> +     } else {
> >> +             for (i = 0; i<  dword_count; i++, data_buff++)
> >> +                     *fifo = *data_buff;
> > 
> > DTTO, will this handle unaligned xfer of size>  4 properly ?
> 
> Again, this part needs to be made more generic. I'll amend it in V3.
> 
> >> +     }
> >> +
> >> +     writel(NPTXFEMPTY,&core_global_regs->gintsts);
> >> +
> 
> [...]
> 
> >> +/* This function returns pointer to out ep struct with number num */
> >> +static struct dwc_ep *get_out_ep(u32 num)
> >> +{
> >> +     u32 i;
> >> +     int num_out_eps = MAX_EPS_CHANNELS;
> >> +     struct dwc_pcd *pcd =&dev_if->pcd;
> >> +
> >> +     if (num == 0)
> >> +             return&pcd->ep0;
> >> +
> >> +     for (i = 0; i<  num_out_eps; ++i) {
> > 
> > i++ ... ++i has no meaning here (not even a compiler hint).
> 
> Ok, 'll amend
> 
> >> +             if (pcd->out_ep[i].num == num)
> >> +                     return&pcd->out_ep[i];
> >> +     }
> >> +
> >> +     return 0;
> >> +}
> >> +
> 
> [...]
> 
> >> + *           to the destination buffer
> >> + */
> > 
> > Otherwise, I think you did a pretty decent job, one more round and I'm
> > queueing this ;-)
> > 
> > .
> :
> :)
> 
> Sorry for my delayed response.

Don't worry about it :)

> 
> Best Regards
> Amit Virdi