On Fri, Mar 15, 2013 at 09:37:36PM +0530, kishon wrote:
> On Friday 15 March 2013 08:09 PM, Felipe Balbi wrote:
> >From: Kishon Vijay Abraham I <kishon@xxxxxx>
> >
> >musb does not use DMA buffer for ep0 but it uses the same giveback
> >function *musb_g_giveback* for all endpoints (*musb_g_ep0_giveback* calls
> >*musb_g_giveback*). So for ep0 case request.dma will be '0'
> >and will result in kernel OOPS if tried to *unmap_dma_buffer* for requests in
> >ep0. Fixed it by doing *unmap_dma_buffer* only for valid DMA addr and
> >checking that musb_ep->dma is valid when unmapping.
> >
> >Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
> >Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> >---
> >
> >Hi Kishon, this is how your patch looks like now,
> >
> >do you think it's alright ? Since map_dma_buffer() checks
> >that musb_ep->dma is valid, I think we should do the same
> >thing when unmapping, what do you think ?
> >
> > drivers/usb/musb/musb_gadget.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> >index be18537..d9712bf 100644
> >--- a/drivers/usb/musb/musb_gadget.c
> >+++ b/drivers/usb/musb/musb_gadget.c
> >@@ -141,7 +141,7 @@ static inline void map_dma_buffer(struct musb_request *request,
> > static inline void unmap_dma_buffer(struct musb_request *request,
> > struct musb *musb)
> > {
> >- if (!is_buffer_mapped(request))
> >+ if (!is_buffer_mapped(request) || !musb_ep->dma)
> > return;
> >
> > if (request->request.dma == DMA_ADDR_INVALID) {
> >@@ -195,7 +195,10 @@ __acquires(ep->musb->lock)
> >
> > ep->busy = 1;
> > spin_unlock(&musb->lock);
> >- unmap_dma_buffer(req, musb);
> >+
> >+ if (!dma_mapping_error(request->dma))
>
> this should have been *dma_mapping_error(&musb->g.dev, request->dma)*
indeed :-)
> ;-) But this doesn't work quite right. The dma_mapping_error
> considers only *DMA_ERROR_CODE* as error. Maybe we should have
> something like this
>
> *if (!dma_mapping_error(&musb->g.dev, request->dma) && request->dma)*
won't 'is_buffer_mapped()' take care of the second check ?
--
balbi
Attachment:
signature.asc
Description: Digital signature
