| Message ID | 1346779499-6085-1-git-send-email-enrico.scholz@sigma-chemnitz.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
> @@ -195,14 +195,13 @@ static void done(struct fsl_ep *ep, struct fsl_req > *req, int status) > dma_pool_free(udc->td_pool, curr_td, curr_td->td_dma); > } > > - if (req->mapped) { > + if (req->req.dma != DMA_ADDR_INVALID) { > dma_unmap_single(ep->udc->gadget.dev.parent, > req->req.dma, req->req.length, > ep_is_in(ep) > ? DMA_TO_DEVICE > : DMA_FROM_DEVICE); > req->req.dma = DMA_ADDR_INVALID; > - req->mapped = 0; > } else > dma_sync_single_for_cpu(ep->udc->gadget.dev.parent, If the class driver has already mapped this address, the req->req.dma is not DMA_ADDR_INVALID either, in this case, the dma_sync_single_for_cpu is enough. Peter
Hello. On 04-09-2012 21:24, Enrico Scholz wrote: > The 'mapped' flag in 'struct fsl_req' flag is redundant with checking > for 'req.dma != DMA_ADDR_INVALID' and it was also set to a wrong value > (see 2nd hunk of patch). > Replacing it in the way described above saves 60 bytes: > function old new delta > fsl_udc_irq 2952 2940 -12 > ep0_prime_status 380 368 -12 > done 448 432 -16 > fsl_ep_queue 668 648 -20 > and has same (or less) runtime costs like evaluating 'req->mapped'. > Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> > --- > drivers/usb/gadget/fsl_udc_core.c | 10 ++-------- > drivers/usb/gadget/fsl_usb2_udc.h | 1 - > 2 files changed, 2 insertions(+), 9 deletions(-) > diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c > index 55c4a61..1282a11 100644 > --- a/drivers/usb/gadget/fsl_udc_core.c > +++ b/drivers/usb/gadget/fsl_udc_core.c > @@ -195,14 +195,13 @@ static void done(struct fsl_ep *ep, struct fsl_req *req, int status) > dma_pool_free(udc->td_pool, curr_td, curr_td->td_dma); > } > > - if (req->mapped) { > + if (req->req.dma != DMA_ADDR_INVALID) { > dma_unmap_single(ep->udc->gadget.dev.parent, > req->req.dma, req->req.length, > ep_is_in(ep) > ? DMA_TO_DEVICE > : DMA_FROM_DEVICE); > req->req.dma = DMA_ADDR_INVALID; > - req->mapped = 0; > } else > dma_sync_single_for_cpu(ep->udc->gadget.dev.parent, > req->req.dma, req->req.length, > @@ -915,15 +914,12 @@ fsl_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags) > req->req.length, ep_is_in(ep) > ? DMA_TO_DEVICE > : DMA_FROM_DEVICE); > - req->mapped = 1; > - } else { > + } else You should keep the { here, according to the Documentation/CodingStyle. If one branch of the *if* statement uses {}, so should the other. WBR, Sergei
Hi, On Tue, Sep 04, 2012 at 07:24:59PM +0200, Enrico Scholz wrote: > The 'mapped' flag in 'struct fsl_req' flag is redundant with checking > for 'req.dma != DMA_ADDR_INVALID' and it was also set to a wrong value you should not be using DMA_ADDR_INVALID anymore. Use the generic map/unmap routines from udc-core.c > (see 2nd hunk of patch). > > Replacing it in the way described above saves 60 bytes: > > function old new delta > fsl_udc_irq 2952 2940 -12 > ep0_prime_status 380 368 -12 > done 448 432 -16 > fsl_ep_queue 668 648 -20 > > and has same (or less) runtime costs like evaluating 'req->mapped'. > > Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> > --- > drivers/usb/gadget/fsl_udc_core.c | 10 ++-------- > drivers/usb/gadget/fsl_usb2_udc.h | 1 - > 2 files changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c > index 55c4a61..1282a11 100644 > --- a/drivers/usb/gadget/fsl_udc_core.c > +++ b/drivers/usb/gadget/fsl_udc_core.c > @@ -195,14 +195,13 @@ static void done(struct fsl_ep *ep, struct fsl_req *req, int status) > dma_pool_free(udc->td_pool, curr_td, curr_td->td_dma); > } > > - if (req->mapped) { > + if (req->req.dma != DMA_ADDR_INVALID) { > dma_unmap_single(ep->udc->gadget.dev.parent, > req->req.dma, req->req.length, > ep_is_in(ep) > ? DMA_TO_DEVICE > : DMA_FROM_DEVICE); > req->req.dma = DMA_ADDR_INVALID; > - req->mapped = 0; > } else > dma_sync_single_for_cpu(ep->udc->gadget.dev.parent, > req->req.dma, req->req.length, > @@ -915,15 +914,12 @@ fsl_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags) > req->req.length, ep_is_in(ep) > ? DMA_TO_DEVICE > : DMA_FROM_DEVICE); > - req->mapped = 1; > - } else { > + } else > dma_sync_single_for_device(ep->udc->gadget.dev.parent, > req->req.dma, req->req.length, > ep_is_in(ep) > ? DMA_TO_DEVICE > : DMA_FROM_DEVICE); > - req->mapped = 0; > - } > > req->req.status = -EINPROGRESS; > req->req.actual = 0; > @@ -1306,7 +1302,6 @@ static int ep0_prime_status(struct fsl_udc *udc, int direction) > req->req.dma = dma_map_single(ep->udc->gadget.dev.parent, > req->req.buf, req->req.length, > ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); > - req->mapped = 1; > > if (fsl_req_to_dtd(req, GFP_ATOMIC) == 0) > fsl_queue_td(ep, req); > @@ -1389,7 +1384,6 @@ static void ch9getstatus(struct fsl_udc *udc, u8 request_type, u16 value, > req->req.dma = dma_map_single(ep->udc->gadget.dev.parent, > req->req.buf, req->req.length, > ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); > - req->mapped = 1; > > /* prime the data phase */ > if ((fsl_req_to_dtd(req, GFP_ATOMIC) == 0)) > diff --git a/drivers/usb/gadget/fsl_usb2_udc.h b/drivers/usb/gadget/fsl_usb2_udc.h > index fbd77ba..9aab166 100644 > --- a/drivers/usb/gadget/fsl_usb2_udc.h > +++ b/drivers/usb/gadget/fsl_usb2_udc.h > @@ -436,7 +436,6 @@ struct fsl_req { > /* ep_queue() func will add > a request->queue into a udc_ep->queue 'd tail */ > struct fsl_ep *ep; > - unsigned mapped:1; > > struct ep_td_struct *head, *tail; /* For dTD List > cpu endian Virtual addr */ > -- > 1.7.11.4 >
On Thu, Sep 06, 2012 at 04:32:28PM +0200, Enrico Scholz wrote: > Chen Peter-B29397 <B29397@freescale.com> writes: > > > If the class driver has already mapped this address, the req->req.dma is not > > DMA_ADDR_INVALID either, in this case, the dma_sync_single_for_cpu is > > enough. > > ok; forget the patch then. fair enough, when can I get generic map/unmap conversion patch ?
Chen Peter-B29397 <B29397@freescale.com> writes: > If the class driver has already mapped this address, the req->req.dma is not > DMA_ADDR_INVALID either, in this case, the dma_sync_single_for_cpu is > enough. ok; forget the patch then. Thanks Enrico
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c index 55c4a61..1282a11 100644 --- a/drivers/usb/gadget/fsl_udc_core.c +++ b/drivers/usb/gadget/fsl_udc_core.c @@ -195,14 +195,13 @@ static void done(struct fsl_ep *ep, struct fsl_req *req, int status) dma_pool_free(udc->td_pool, curr_td, curr_td->td_dma); } - if (req->mapped) { + if (req->req.dma != DMA_ADDR_INVALID) { dma_unmap_single(ep->udc->gadget.dev.parent, req->req.dma, req->req.length, ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); req->req.dma = DMA_ADDR_INVALID; - req->mapped = 0; } else dma_sync_single_for_cpu(ep->udc->gadget.dev.parent, req->req.dma, req->req.length, @@ -915,15 +914,12 @@ fsl_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags) req->req.length, ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); - req->mapped = 1; - } else { + } else dma_sync_single_for_device(ep->udc->gadget.dev.parent, req->req.dma, req->req.length, ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); - req->mapped = 0; - } req->req.status = -EINPROGRESS; req->req.actual = 0; @@ -1306,7 +1302,6 @@ static int ep0_prime_status(struct fsl_udc *udc, int direction) req->req.dma = dma_map_single(ep->udc->gadget.dev.parent, req->req.buf, req->req.length, ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); - req->mapped = 1; if (fsl_req_to_dtd(req, GFP_ATOMIC) == 0) fsl_queue_td(ep, req); @@ -1389,7 +1384,6 @@ static void ch9getstatus(struct fsl_udc *udc, u8 request_type, u16 value, req->req.dma = dma_map_single(ep->udc->gadget.dev.parent, req->req.buf, req->req.length, ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); - req->mapped = 1; /* prime the data phase */ if ((fsl_req_to_dtd(req, GFP_ATOMIC) == 0)) diff --git a/drivers/usb/gadget/fsl_usb2_udc.h b/drivers/usb/gadget/fsl_usb2_udc.h index fbd77ba..9aab166 100644 --- a/drivers/usb/gadget/fsl_usb2_udc.h +++ b/drivers/usb/gadget/fsl_usb2_udc.h @@ -436,7 +436,6 @@ struct fsl_req { /* ep_queue() func will add a request->queue into a udc_ep->queue 'd tail */ struct fsl_ep *ep; - unsigned mapped:1; struct ep_td_struct *head, *tail; /* For dTD List cpu endian Virtual addr */
The 'mapped' flag in 'struct fsl_req' flag is redundant with checking for 'req.dma != DMA_ADDR_INVALID' and it was also set to a wrong value (see 2nd hunk of patch). Replacing it in the way described above saves 60 bytes: function old new delta fsl_udc_irq 2952 2940 -12 ep0_prime_status 380 368 -12 done 448 432 -16 fsl_ep_queue 668 648 -20 and has same (or less) runtime costs like evaluating 'req->mapped'. Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> --- drivers/usb/gadget/fsl_udc_core.c | 10 ++-------- drivers/usb/gadget/fsl_usb2_udc.h | 1 - 2 files changed, 2 insertions(+), 9 deletions(-)