On Sat, May 19, 2012 at 04:59:58PM +0800, Yu Xu wrote:
> Hi Balbi
>
> >> +
> >> +#define EP_CONTEXT_ALIGNMENT 32
> >> +#define MV_U3D_TRB_ALIGNMENT 16
> >> +#define MV_U3D_DMA_BOUNDARY 4096
> >> +
> >> +#define EP_DIR_IN 1
> >> +#define EP_DIR_OUT 0
> >> +
> >> +#define DMA_ADDR_INVALID (~(dma_addr_t)0)
> >
> > NAK, use generic map/unmap routines
> >
> Sorry, I cannot catch it. Could you explain more?
we have generic usb_gadget_map_request() and usb_gadget_unmap_request(),
use those and drop this DMA_ADDR_INVALID nonsense that has sprinkled on
all drivers.
> >> + u32 itpinfo1;
> >> + u32 rsvd8[61];
> >> + struct epxcr epcr[16];
> >> + u32 rsvd9[64];
> >> + u32 phyaddr;
> >> + u32 phydata;
> >> +};
> >
> > you're assuming endianess here. Is it safe for Marvell ?
> >
> Thanks for the reminder. Actually, we support little endian only.
Will this be true for the next 20 years ?
> >> +/* request data structure */
> >> +struct mv_req {
> >> + struct usb_request req;
> >> + struct trb *trb, *head, *tail;
> >
> > why do you need head and tail pointers ?
> >
> >> + struct mv_ep *ep;
> >> + struct list_head queue;
> >
> > what's this queue ?
> >
> The queue is to manage all of the usb request that is waiting for
> transfer, since the request cannot be queue to the controller until
> the ep is free.
>
> >> + struct list_head list; /* add to list of ep's request */
> >> + unsigned trb_count;
> >> + unsigned chain;
> >> + unsigned mapped:1;
> >> +};
> >
> > Overall comment: Add kernel doc to all structures on this comment.
> >
> What's the meaning of kernel doc? And where should I put?
read Documentation/kernel-doc-nano-HOWTO.txt
--
balbi
Attachment:
signature.asc
Description: Digital signature
