On Mon, Mar 18, 2013 at 02:17:52PM +0530, kishon wrote:
> On Monday 18 March 2013 02:12 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Mon, Mar 18, 2013 at 01:50:55PM +0530, kishon wrote:
> >>>>>@@ -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 ?
> >>
> >>No, it doesn't. Btw you've removed is_buffer_mapped() macro in *usb:
> >>musb: gadget: switch over to usb_gadget_map/unmap_request()* patch..
> >
> >I dropped that patch since MUSB needs a bit more care when switching to
> >generic map/unmap routines. We can do that for v3.11 instead.
>
> Oh ok. Then we need to take care when we have that patch.
right, so what about this?
commit 06d9db7273c7bd5b07624b313faeea57a4b31056
Author: Kishon Vijay Abraham I <kishon@xxxxxx>
Date: Fri Mar 15 18:58:50 2013 +0530
usb: musb: gadget: do *unmap_dma_buffer* only for valid DMA addr
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>
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index be18537..83edded 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -141,7 +141,9 @@ 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))
+ struct musb_ep *musb_ep = request->ep;
+
+ if (!is_buffer_mapped(request) || !musb_ep->dma)
return;
if (request->request.dma == DMA_ADDR_INVALID) {
@@ -195,7 +197,10 @@ __acquires(ep->musb->lock)
ep->busy = 1;
spin_unlock(&musb->lock);
- unmap_dma_buffer(req, musb);
+
+ if (!dma_mapping_error(&musb->g.dev, request->dma))
+ unmap_dma_buffer(req, musb);
+
if (request->status == 0)
dev_dbg(musb->controller, "%s done request %p, %d/%d\n",
ep->end_point.name, request,
note that is_buffer_mapped() knows that request->dma is valid or not
since request->mapped is set only if dma_map has succeeded.
--
balbi
Attachment:
signature.asc
Description: Digital signature
