Hi, >-----Original Message----- >From: Sergei Shtylyov [mailto:sshtylyov@xxxxxxxxxx] >Sent: Friday, May 14, 2010 5:04 PM >To: Kalliguddi, Hema >Cc: linux-usb@xxxxxxxxxxxxxxx >Subject: Re: [PATCH V2] usb: musb: Unmapping the dma buffer >when switching to PIO mode > >Hello. > >HEMA HK wrote: > >> From: Hema HK <hemahk@xxxxxx> > >> Buffer is mapped to dma when dma channel is allocated. buffer needs >> to be unmapped when fallback to PIO mode if dma channel_program >> fails. >> >> Signed-off-by: Hema HK <hemahk@xxxxxx> >> --- >> >> Addressed review comments. >> >> Index: linux-2.6/drivers/usb/musb/musb_gadget.c >> =================================================================== >> --- linux-2.6.orig/drivers/usb/musb/musb_gadget.c >> +++ linux-2.6/drivers/usb/musb/musb_gadget.c >> @@ -92,6 +92,51 @@ >> >> /* >--------------------------------------------------------------- >-------- */ >> >> +/* Mapping unmapping the buffer to and from dma */ >> + >> +static inline void map_dma_buffer(struct musb_request *request, >> + struct, musb *musb, bool map) >> +{ >> + if (map) { >> + if (request->request.dma == DMA_ADDR_INVALID) { >> + request->request.dma = dma_map_single( >> + musb->controller, >> + request->request.buf, >> + request->request.length, >> + request->tx >> + ? DMA_TO_DEVICE >> + : DMA_FROM_DEVICE); >> + request->mapped = 1; >> + } else { >> + dma_sync_single_for_device(musb->controller, >> + request->request.dma, >> + request->request.length, >> + request->tx >> + ? DMA_TO_DEVICE >> + : DMA_FROM_DEVICE); >> + request->mapped = 0; >> + } >> + } else if (request->mapped) { >> + dma_unmap_single(musb->controller, >> + request->request.dma, >> + request->request.length, >> + request->tx >> + ? DMA_TO_DEVICE >> + : DMA_FROM_DEVICE); >> + request->request.dma = DMA_ADDR_INVALID; >> + request->mapped = 0; >> + } else { >> + dma_sync_single_for_cpu(musb->controller, >> + request->request.dma, >> + request->request.length, >> + request->tx >> + ? DMA_TO_DEVICE >> + : DMA_FROM_DEVICE); >> + >> + } >> +} > > I don't think it's really a good idea to have single >function called >map_dma_buffer() that both maps and unmaps a buffer. Why not have two >separate functions? > Since it is very simple function which calls 2 functions and initializes one variable, I defined Single function. I am OK to have 2 deperate inline functions also. >> @@ -711,6 +748,12 @@ static void rxstate(struct musb *musb, s >> return; >> } >> #endif >> + /* unmap the dma buffer back to cpu if dma channel >> + * programming fails. This buffer is mapped if >the channel >> + * allocation is successful >> + */ >> + if (is_dma_capable()) > > So, which indentation level is correct, that one of the >comment, aor >that one of the following code? > It should be of the code below. I will correct this. ~Hema >> + map_dma_buffer(req, musb, false); >> >> musb_read_fifo(musb_ep->hw_ep, >fifo_count, (u8 *) >> (request->buf + >request->actual)); > >WBR, Sergei > >-- 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
