[PATCH] usb: gadget: fsl_udc_core: remove mapped flag

STALE5083d

6 messages, 4 authors, 2012-09-06 · open the first message on its own page

[PATCH] usb: gadget: fsl_udc_core: remove mapped flag

From: Enrico Scholz <hidden>
Date: 2012-09-04 17:25:09

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 <redacted>
---
 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

RE: [PATCH] usb: gadget: fsl_udc_core: remove mapped flag

From: Chen Peter-B29397 <hidden>
Date: 2012-09-05 02:17:16

=20

quoted hunk

@@ -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);
 	}
=20
-	if (req->mapped) {
+	if (req->req.dma !=3D 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 =3D DMA_ADDR_INVALID;
-		req->mapped =3D 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 no=
t
DMA_ADDR_INVALID either, in this case, the dma_sync_single_for_cpu is enoug=
h.


Peter

Re: [PATCH] usb: gadget: fsl_udc_core: remove mapped flag

From: Sergei Shtylyov <hidden>
Date: 2012-09-05 12:33:04

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 <redacted>
---
  drivers/usb/gadget/fsl_udc_core.c | 10 ++--------
  drivers/usb/gadget/fsl_usb2_udc.h |  1 -
  2 files changed, 2 insertions(+), 9 deletions(-)

quoted hunk

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

Re: [PATCH] usb: gadget: fsl_udc_core: remove mapped flag

From: Felipe Balbi <hidden>
Date: 2012-09-05 12:47:51

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

quoted hunk

(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 <redacted>
---
 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

-- 
balbi

Re: [PATCH] usb: gadget: fsl_udc_core: remove mapped flag

From: Enrico Scholz <hidden>
Date: 2012-09-06 14:32:28

Chen Peter-B29397 [off-list ref] 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

Re: [PATCH] usb: gadget: fsl_udc_core: remove mapped flag

From: Felipe Balbi <hidden>
Date: 2012-09-06 14:36:38

On Thu, Sep 06, 2012 at 04:32:28PM +0200, Enrico Scholz wrote:

Chen Peter-B29397 [off-list ref] writes:

quoted

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 ?

-- 
balbi

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help