linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] usb: gadget: generic map/unmap routines
@ 2012-01-24 11:45 Felipe Balbi
  2012-01-24 11:45 ` [PATCH 1/9] usb: gadget: add generic map/unmap request utilities Felipe Balbi
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

Hi all,

here's a rebased version of generic map/unmap patchset.

Also available on generic-map-unmap branch of my tree, some
of the patches have been tested before and already have a
Tested-by tag.

Felipe Balbi (9):
  usb: gadget: add generic map/unmap request utilities
  usb: dwc3: gadget: use generic map/unmap routines
  usb: gadget: langwell: use generic map/unmap functions
  usb: renesas: gadget: use generic map/unmap routines
  usb: gadget: amd5536: use generic map/unmap routines
  usb: gadget: r8a66597: use generic map/unmap routines
  usb: gadget: net2272: use generic map/umap routines
  usb: gadget: net2280: use generic map/unmap routines
  usb: gadget: goku: use generic map/unmap routines

 drivers/usb/dwc3/core.h                |    2 -
 drivers/usb/dwc3/ep0.c                 |   16 +++++-
 drivers/usb/dwc3/gadget.c              |   93 ++++++--------------------------
 drivers/usb/dwc3/gadget.h              |    2 -
 drivers/usb/gadget/amd5536udc.c        |   33 ++---------
 drivers/usb/gadget/goku_udc.c          |   18 +++----
 drivers/usb/gadget/langwell_udc.c      |   45 +++-------------
 drivers/usb/gadget/net2272.c           |   18 +++----
 drivers/usb/gadget/net2280.c           |   19 +++----
 drivers/usb/gadget/r8a66597-udc.c      |   10 +---
 drivers/usb/gadget/udc-core.c          |   52 ++++++++++++++++++
 drivers/usb/renesas_usbhs/mod_gadget.c |   73 ++++++-------------------
 include/linux/usb/gadget.h             |   10 ++++
 13 files changed, 149 insertions(+), 242 deletions(-)

-- 
1.7.8.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/9] usb: gadget: add generic map/unmap request utilities
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-01-24 11:45 ` [PATCH 2/9] usb: dwc3: gadget: use generic map/unmap routines Felipe Balbi
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

such utilities are currently duplicated on all UDC
drivers basically with the same structure. Let's group
all implementations into one generic implementation
and get rid of that duplication.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/udc-core.c |   52 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/gadget.h    |   10 ++++++++
 2 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index 0b0d12c..56da49f 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -22,6 +22,7 @@
 #include <linux/device.h>
 #include <linux/list.h>
 #include <linux/err.h>
+#include <linux/dma-mapping.h>
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
@@ -49,6 +50,57 @@ static DEFINE_MUTEX(udc_lock);
 
 /* ------------------------------------------------------------------------- */
 
+int usb_gadget_map_request(struct usb_gadget *gadget,
+		struct usb_request *req, int is_in)
+{
+	if (req->length == 0)
+		return 0;
+
+	if (req->num_sgs) {
+		int     mapped;
+
+		mapped = dma_map_sg(&gadget->dev, req->sg, req->num_sgs,
+				is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+		if (mapped == 0) {
+			dev_err(&gadget->dev, "failed to map SGs\n");
+			return -EFAULT;
+		}
+
+		req->num_mapped_sgs = mapped;
+	} else {
+		req->dma = dma_map_single(&gadget->dev, req->buf, req->length,
+				is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+
+		if (dma_mapping_error(&gadget->dev, req->dma)) {
+			dev_err(&gadget->dev, "failed to map buffer\n");
+			return -EFAULT;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_map_request);
+
+void usb_gadget_unmap_request(struct usb_gadget *gadget,
+		struct usb_request *req, int is_in)
+{
+	if (req->length == 0)
+		return;
+
+	if (req->num_mapped_sgs) {
+		dma_unmap_sg(&gadget->dev, req->sg, req->num_mapped_sgs,
+				is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+
+		req->num_mapped_sgs = 0;
+	} else {
+		dma_unmap_single(&gadget->dev, req->dma, req->length,
+				is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	}
+}
+EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
+
+/* ------------------------------------------------------------------------- */
+
 /**
  * usb_gadget_start - tells usb device controller to start up
  * @gadget: The gadget we want to get started
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index da653b5..9517466 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -950,6 +950,16 @@ static inline void usb_free_descriptors(struct usb_descriptor_header **v)
 
 /*-------------------------------------------------------------------------*/
 
+/* utility to simplify map/unmap of usb_requests to/from DMA */
+
+extern int usb_gadget_map_request(struct usb_gadget *gadget,
+		struct usb_request *req, int is_in);
+
+extern void usb_gadget_unmap_request(struct usb_gadget *gadget,
+		struct usb_request *req, int is_in);
+
+/*-------------------------------------------------------------------------*/
+
 /* utility wrapping a simple endpoint selection policy */
 
 extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/9] usb: dwc3: gadget: use generic map/unmap routines
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
  2012-01-24 11:45 ` [PATCH 1/9] usb: gadget: add generic map/unmap request utilities Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-01-24 11:45 ` [PATCH 3/9] usb: gadget: langwell: use generic map/unmap functions Felipe Balbi
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

those routines have everything we need to map/unmap
USB requests and it's better to use them.

In order to achieve that, we had to add a simple
change on how we allocate and use our setup buffer;
we cannot allocate it from coherent anymore otherwise
the generic map/unmap routines won't be able to easily
know that the GetStatus request already has a DMA
address.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/dwc3/core.h   |    2 -
 drivers/usb/dwc3/ep0.c    |   16 ++++++-
 drivers/usb/dwc3/gadget.c |   93 ++++++++------------------------------------
 drivers/usb/dwc3/gadget.h |    2 -
 4 files changed, 30 insertions(+), 83 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 9e57f8e..a72f42f 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -572,7 +572,6 @@ struct dwc3_request {
  * @ctrl_req_addr: dma address of ctrl_req
  * @ep0_trb: dma address of ep0_trb
  * @ep0_usb_req: dummy req used while handling STD USB requests
- * @setup_buf_addr: dma address of setup_buf
  * @ep0_bounce_addr: dma address of ep0_bounce
  * @lock: for synchronizing
  * @dev: pointer to our struct device
@@ -609,7 +608,6 @@ struct dwc3 {
 	u8			*setup_buf;
 	dma_addr_t		ctrl_req_addr;
 	dma_addr_t		ep0_trb_addr;
-	dma_addr_t		setup_buf_addr;
 	dma_addr_t		ep0_bounce_addr;
 	struct dwc3_request	ep0_usb_req;
 	/* device lock */
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 2f51de5..24137d8 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -309,7 +309,7 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc,
 	dep = dwc->eps[0];
 	dwc->ep0_usb_req.dep = dep;
 	dwc->ep0_usb_req.request.length = sizeof(*response_pkt);
-	dwc->ep0_usb_req.request.dma = dwc->setup_buf_addr;
+	dwc->ep0_usb_req.request.buf = dwc->setup_buf;
 	dwc->ep0_usb_req.request.complete = dwc3_ep0_status_cmpl;
 
 	return __dwc3_gadget_ep0_queue(dep, &dwc->ep0_usb_req);
@@ -686,7 +686,12 @@ static void dwc3_ep0_do_control_data(struct dwc3 *dwc,
 				DWC3_TRBCTL_CONTROL_DATA);
 	} else if ((req->request.length % dep->endpoint.maxpacket)
 			&& (event->endpoint_number == 0)) {
-		dwc3_map_buffer_to_dma(req);
+		ret = usb_gadget_map_request(&dwc->gadget, &req->request,
+				event->endpoint_number);
+		if (ret) {
+			dev_dbg(dwc->dev, "failed to map request\n");
+			return;
+		}
 
 		WARN_ON(req->request.length > dep->endpoint.maxpacket);
 
@@ -701,7 +706,12 @@ static void dwc3_ep0_do_control_data(struct dwc3 *dwc,
 				dwc->ep0_bounce_addr, dep->endpoint.maxpacket,
 				DWC3_TRBCTL_CONTROL_DATA);
 	} else {
-		dwc3_map_buffer_to_dma(req);
+		ret = usb_gadget_map_request(&dwc->gadget, &req->request,
+				event->endpoint_number);
+		if (ret) {
+			dev_dbg(dwc->dev, "failed to map request\n");
+			return;
+		}
 
 		ret = dwc3_ep0_start_trans(dwc, event->endpoint_number,
 				req->request.dma, req->request.length,
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a696bde..76036d0 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -54,70 +54,6 @@
 #include "gadget.h"
 #include "io.h"
 
-#define	DMA_ADDR_INVALID	(~(dma_addr_t)0)
-
-void dwc3_map_buffer_to_dma(struct dwc3_request *req)
-{
-	struct dwc3			*dwc = req->dep->dwc;
-
-	if (req->request.length == 0) {
-		/* req->request.dma = dwc->setup_buf_addr; */
-		return;
-	}
-
-	if (req->request.num_sgs) {
-		int	mapped;
-
-		mapped = dma_map_sg(dwc->dev, req->request.sg,
-				req->request.num_sgs,
-				req->direction ? DMA_TO_DEVICE
-				: DMA_FROM_DEVICE);
-		if (mapped < 0) {
-			dev_err(dwc->dev, "failed to map SGs\n");
-			return;
-		}
-
-		req->request.num_mapped_sgs = mapped;
-		return;
-	}
-
-	if (req->request.dma == DMA_ADDR_INVALID) {
-		req->request.dma = dma_map_single(dwc->dev, req->request.buf,
-				req->request.length, req->direction
-				? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-		req->mapped = true;
-	}
-}
-
-void dwc3_unmap_buffer_from_dma(struct dwc3_request *req)
-{
-	struct dwc3			*dwc = req->dep->dwc;
-
-	if (req->request.length == 0) {
-		req->request.dma = DMA_ADDR_INVALID;
-		return;
-	}
-
-	if (req->request.num_mapped_sgs) {
-		req->request.dma = DMA_ADDR_INVALID;
-		dma_unmap_sg(dwc->dev, req->request.sg,
-				req->request.num_sgs,
-				req->direction ? DMA_TO_DEVICE
-				: DMA_FROM_DEVICE);
-
-		req->request.num_mapped_sgs = 0;
-		return;
-	}
-
-	if (req->mapped) {
-		dma_unmap_single(dwc->dev, req->request.dma,
-				req->request.length, req->direction
-				? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-		req->mapped = 0;
-		req->request.dma = DMA_ADDR_INVALID;
-	}
-}
-
 void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
 		int status)
 {
@@ -144,14 +80,15 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
 	if (req->request.status == -EINPROGRESS)
 		req->request.status = status;
 
-	dwc3_unmap_buffer_from_dma(req);
+	usb_gadget_unmap_request(&dwc->gadget, &req->request,
+			req->direction);
 
 	dev_dbg(dwc->dev, "request %p from %s completed %d/%d ===> %d\n",
 			req, dep->name, req->request.actual,
 			req->request.length, status);
 
 	spin_unlock(&dwc->lock);
-	req->request.complete(&req->dep->endpoint, &req->request);
+	req->request.complete(&dep->endpoint, &req->request);
 	spin_lock(&dwc->lock);
 }
 
@@ -562,7 +499,6 @@ static struct usb_request *dwc3_gadget_ep_alloc_request(struct usb_ep *ep,
 
 	req->epnum	= dep->number;
 	req->dep	= dep;
-	req->request.dma = DMA_ADDR_INVALID;
 
 	return &req->request;
 }
@@ -821,7 +757,8 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep, u16 cmd_param,
 		 * here and stop, unmap, free and del each of the linked
 		 * requests instead of we do now.
 		 */
-		dwc3_unmap_buffer_from_dma(req);
+		usb_gadget_unmap_request(&dwc->gadget, &req->request,
+				req->direction);
 		list_del(&req->list);
 		return ret;
 	}
@@ -837,6 +774,9 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep, u16 cmd_param,
 
 static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)
 {
+	struct dwc3		*dwc = dep->dwc;
+	int			ret;
+
 	req->request.actual	= 0;
 	req->request.status	= -EINPROGRESS;
 	req->direction		= dep->direction;
@@ -854,7 +794,11 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)
 	 * This will also avoid Host cancelling URBs due to too
 	 * many NACKs.
 	 */
-	dwc3_map_buffer_to_dma(req);
+	ret = usb_gadget_map_request(&dwc->gadget, &req->request,
+			dep->direction);
+	if (ret)
+		return ret;
+
 	list_add_tail(&req->list, &dep->request_list);
 
 	/*
@@ -2149,9 +2093,8 @@ int __devinit dwc3_gadget_init(struct dwc3 *dwc)
 		goto err1;
 	}
 
-	dwc->setup_buf = dma_alloc_coherent(dwc->dev,
-			sizeof(*dwc->setup_buf) * 2,
-			&dwc->setup_buf_addr, GFP_KERNEL);
+	dwc->setup_buf = kzalloc(sizeof(*dwc->setup_buf) * 2,
+			GFP_KERNEL);
 	if (!dwc->setup_buf) {
 		dev_err(dwc->dev, "failed to allocate setup buffer\n");
 		ret = -ENOMEM;
@@ -2242,8 +2185,7 @@ err4:
 			dwc->ep0_bounce_addr);
 
 err3:
-	dma_free_coherent(dwc->dev, sizeof(*dwc->setup_buf) * 2,
-			dwc->setup_buf, dwc->setup_buf_addr);
+	kfree(dwc->setup_buf);
 
 err2:
 	dma_free_coherent(dwc->dev, sizeof(*dwc->ep0_trb),
@@ -2272,8 +2214,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
 	dma_free_coherent(dwc->dev, 512, dwc->ep0_bounce,
 			dwc->ep0_bounce_addr);
 
-	dma_free_coherent(dwc->dev, sizeof(*dwc->setup_buf) * 2,
-			dwc->setup_buf, dwc->setup_buf_addr);
+	kfree(dwc->setup_buf);
 
 	dma_free_coherent(dwc->dev, sizeof(*dwc->ep0_trb),
 			dwc->ep0_trb, dwc->ep0_trb_addr);
diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index d97f467..12f1e10 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -108,8 +108,6 @@ int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct usb_request *request,
 int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value);
 int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
 		unsigned cmd, struct dwc3_gadget_ep_cmd_params *params);
-void dwc3_map_buffer_to_dma(struct dwc3_request *req);
-void dwc3_unmap_buffer_from_dma(struct dwc3_request *req);
 
 /**
  * dwc3_gadget_ep_get_transfer_index - Gets transfer index from HW
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/9] usb: gadget: langwell: use generic map/unmap functions
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
  2012-01-24 11:45 ` [PATCH 1/9] usb: gadget: add generic map/unmap request utilities Felipe Balbi
  2012-01-24 11:45 ` [PATCH 2/9] usb: dwc3: gadget: use generic map/unmap routines Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-01-24 11:45 ` [PATCH 4/9] usb: renesas: gadget: use generic map/unmap routines Felipe Balbi
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

those routines have everything we need to map/unmap
USB requests and it's better to use them.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/langwell_udc.c |   45 +++++-------------------------------
 1 files changed, 7 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/gadget/langwell_udc.c b/drivers/usb/gadget/langwell_udc.c
index fa0fcc1..fe7fbe3 100644
--- a/drivers/usb/gadget/langwell_udc.c
+++ b/drivers/usb/gadget/langwell_udc.c
@@ -406,16 +406,7 @@ static void done(struct langwell_ep *ep, struct langwell_request *req,
 		dma_pool_free(dev->dtd_pool, curr_dtd, curr_dtd->dtd_dma);
 	}
 
-	if (req->mapped) {
-		dma_unmap_single(&dev->pdev->dev,
-			req->req.dma, req->req.length,
-			is_in(ep) ? PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
-		req->req.dma = DMA_ADDR_INVALID;
-		req->mapped = 0;
-	} else
-		dma_sync_single_for_cpu(&dev->pdev->dev, req->req.dma,
-				req->req.length,
-				is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	usb_gadget_unmap_request(&dev->gadget, &req->req, is_in(ep));
 
 	if (status != -ESHUTDOWN)
 		dev_dbg(&dev->pdev->dev,
@@ -754,7 +745,8 @@ static int langwell_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
 	struct langwell_ep	*ep;
 	struct langwell_udc	*dev;
 	unsigned long		flags;
-	int			is_iso = 0, zlflag = 0;
+	int			is_iso = 0;
+	int			ret;
 
 	/* always require a cpu-view buffer */
 	req = container_of(_req, struct langwell_request, req);
@@ -781,33 +773,10 @@ static int langwell_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
 	if (unlikely(!dev->driver || dev->gadget.speed == USB_SPEED_UNKNOWN))
 		return -ESHUTDOWN;
 
-	/* set up dma mapping in case the caller didn't */
-	if (_req->dma == DMA_ADDR_INVALID) {
-		/* WORKAROUND: WARN_ON(size == 0) */
-		if (_req->length == 0) {
-			dev_vdbg(&dev->pdev->dev, "req->length: 0->1\n");
-			zlflag = 1;
-			_req->length++;
-		}
-
-		_req->dma = dma_map_single(&dev->pdev->dev,
-				_req->buf, _req->length,
-				is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-		if (zlflag && (_req->length == 1)) {
-			dev_vdbg(&dev->pdev->dev, "req->length: 1->0\n");
-			zlflag = 0;
-			_req->length = 0;
-		}
-
-		req->mapped = 1;
-		dev_vdbg(&dev->pdev->dev, "req->mapped = 1\n");
-	} else {
-		dma_sync_single_for_device(&dev->pdev->dev,
-				_req->dma, _req->length,
-				is_in(ep) ?  DMA_TO_DEVICE : DMA_FROM_DEVICE);
-		req->mapped = 0;
-		dev_vdbg(&dev->pdev->dev, "req->mapped = 0\n");
-	}
+	/* set up dma mapping */
+	ret = usb_gadget_map_request(&dev->gadget, &req->req, is_in(ep));
+	if (ret)
+		return ret;
 
 	dev_dbg(&dev->pdev->dev,
 			"%s queue req %p, len %u, buf %p, dma 0x%08x\n",
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/9] usb: renesas: gadget: use generic map/unmap routines
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
                   ` (2 preceding siblings ...)
  2012-01-24 11:45 ` [PATCH 3/9] usb: gadget: langwell: use generic map/unmap functions Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-01-24 11:45 ` [PATCH 5/9] usb: gadget: amd5536: " Felipe Balbi
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

those routines have everything we need to map/unmap
USB requests and it's better to use them.

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/renesas_usbhs/mod_gadget.c |   73 +++++++------------------------
 1 files changed, 17 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index 528691d5f..5339741 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -165,69 +165,32 @@ static void usbhsg_queue_push(struct usbhsg_uep *uep,
 /*
  *		dma map/unmap
  */
-static int usbhsg_dma_map(struct device *dev,
-			  struct usbhs_pkt *pkt,
-			  enum dma_data_direction dir)
-{
-	struct usbhsg_request *ureq = usbhsg_pkt_to_ureq(pkt);
-	struct usb_request *req = &ureq->req;
-
-	if (pkt->dma != DMA_ADDR_INVALID) {
-		dev_err(dev, "dma is already mapped\n");
-		return -EIO;
-	}
-
-	if (req->dma == DMA_ADDR_INVALID) {
-		pkt->dma = dma_map_single(dev, pkt->buf, pkt->length, dir);
-	} else {
-		dma_sync_single_for_device(dev, req->dma, req->length, dir);
-		pkt->dma = req->dma;
-	}
-
-	if (dma_mapping_error(dev, pkt->dma)) {
-		dev_err(dev, "dma mapping error %llx\n", (u64)pkt->dma);
-		return -EIO;
-	}
-
-	return 0;
-}
-
-static int usbhsg_dma_unmap(struct device *dev,
-			    struct usbhs_pkt *pkt,
-			    enum dma_data_direction dir)
+static int usbhsg_dma_map_ctrl(struct usbhs_pkt *pkt, int map)
 {
 	struct usbhsg_request *ureq = usbhsg_pkt_to_ureq(pkt);
 	struct usb_request *req = &ureq->req;
-
-	if (pkt->dma == DMA_ADDR_INVALID) {
-		dev_err(dev, "dma is not mapped\n");
-		return -EIO;
-	}
-
-	if (req->dma == DMA_ADDR_INVALID)
-		dma_unmap_single(dev, pkt->dma, pkt->length, dir);
-	else
-		dma_sync_single_for_cpu(dev, req->dma, req->length, dir);
-
-	pkt->dma = DMA_ADDR_INVALID;
-
-	return 0;
-}
-
-static int usbhsg_dma_map_ctrl(struct usbhs_pkt *pkt, int map)
-{
 	struct usbhs_pipe *pipe = pkt->pipe;
 	struct usbhsg_uep *uep = usbhsg_pipe_to_uep(pipe);
 	struct usbhsg_gpriv *gpriv = usbhsg_uep_to_gpriv(uep);
-	struct device *dev = usbhsg_gpriv_to_dev(gpriv);
 	enum dma_data_direction dir;
+	int ret = 0;
 
-	dir = usbhs_pipe_is_dir_in(pipe) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	dir = usbhs_pipe_is_dir_host(pipe);
 
-	if (map)
-		return usbhsg_dma_map(dev, pkt, dir);
-	else
-		return usbhsg_dma_unmap(dev, pkt, dir);
+	if (map) {
+		/* it can not use scatter/gather */
+		WARN_ON(req->num_sgs);
+
+		ret = usb_gadget_map_request(&gpriv->gadget, req, dir);
+		if (ret < 0)
+			return ret;
+
+		pkt->dma = req->dma;
+	} else {
+		usb_gadget_unmap_request(&gpriv->gadget, req, dir);
+	}
+
+	return ret;
 }
 
 /*
@@ -657,8 +620,6 @@ static struct usb_request *usbhsg_ep_alloc_request(struct usb_ep *ep,
 
 	usbhs_pkt_init(usbhsg_ureq_to_pkt(ureq));
 
-	ureq->req.dma = DMA_ADDR_INVALID;
-
 	return &ureq->req;
 }
 
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/9] usb: gadget: amd5536: use generic map/unmap routines
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
                   ` (3 preceding siblings ...)
  2012-01-24 11:45 ` [PATCH 4/9] usb: renesas: gadget: use generic map/unmap routines Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-01-24 11:45 ` [PATCH 6/9] usb: gadget: r8a66597: " Felipe Balbi
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

those routines have everything we need to map/unmap
USB requests and it's better to use them.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/amd5536udc.c |   33 ++++++---------------------------
 1 files changed, 6 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index c16ff55..e0ad9ee 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -831,20 +831,8 @@ __acquires(ep->dev->lock)
 
 	dev = ep->dev;
 	/* unmap DMA */
-	if (req->dma_mapping) {
-		if (ep->in)
-			pci_unmap_single(dev->pdev,
-					req->req.dma,
-					req->req.length,
-					PCI_DMA_TODEVICE);
-		else
-			pci_unmap_single(dev->pdev,
-					req->req.dma,
-					req->req.length,
-					PCI_DMA_FROMDEVICE);
-		req->dma_mapping = 0;
-		req->req.dma = DMA_DONT_USE;
-	}
+	if (ep->dma)
+		usb_gadget_unmap_request(&dev->gadget, &req->req, ep->in);
 
 	halted = ep->halted;
 	ep->halted = 1;
@@ -1095,20 +1083,11 @@ udc_queue(struct usb_ep *usbep, struct usb_request *usbreq, gfp_t gfp)
 		return -ESHUTDOWN;
 
 	/* map dma (usually done before) */
-	if (ep->dma && usbreq->length != 0
-			&& (usbreq->dma == DMA_DONT_USE || usbreq->dma == 0)) {
+	if (ep->dma) {
 		VDBG(dev, "DMA map req %p\n", req);
-		if (ep->in)
-			usbreq->dma = pci_map_single(dev->pdev,
-						usbreq->buf,
-						usbreq->length,
-						PCI_DMA_TODEVICE);
-		else
-			usbreq->dma = pci_map_single(dev->pdev,
-						usbreq->buf,
-						usbreq->length,
-						PCI_DMA_FROMDEVICE);
-		req->dma_mapping = 1;
+		retval = usb_gadget_map_request(&udc->gadget, usbreq, ep->in);
+		if (retval)
+			return retval;
 	}
 
 	VDBG(dev, "%s queue req %p, len %d req->td_data=%p buf %p\n",
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/9] usb: gadget: r8a66597: use generic map/unmap routines
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
                   ` (4 preceding siblings ...)
  2012-01-24 11:45 ` [PATCH 5/9] usb: gadget: amd5536: " Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-01-24 11:45 ` [PATCH 7/9] usb: gadget: net2272: use generic map/umap routines Felipe Balbi
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

those routines have everything we need to map/unmap
USB requests and it's better to use them.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/r8a66597-udc.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c b/drivers/usb/gadget/r8a66597-udc.c
index f5b8d21..c4401e7 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -663,11 +663,7 @@ static int sudmac_alloc_channel(struct r8a66597 *r8a66597,
 	ep->fifoctr = D0FIFOCTR;
 
 	/* dma mapping */
-	req->req.dma = dma_map_single(r8a66597_to_dev(ep->r8a66597),
-				req->req.buf, req->req.length,
-				dma->dir ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-
-	return 0;
+	return usb_gadget_map_request(&r8a66597->gadget, &req->req, dma->dir);
 }
 
 static void sudmac_free_channel(struct r8a66597 *r8a66597,
@@ -677,9 +673,7 @@ static void sudmac_free_channel(struct r8a66597 *r8a66597,
 	if (!r8a66597_is_sudmac(r8a66597))
 		return;
 
-	dma_unmap_single(r8a66597_to_dev(ep->r8a66597),
-			 req->req.dma, req->req.length,
-			 ep->dma->dir ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	usb_gadget_unmap_request(&r8a66597->gadget, &req->req, ep->dma->dir);
 
 	r8a66597_bclr(r8a66597, DREQE, ep->fifosel);
 	r8a66597_change_curpipe(r8a66597, 0, 0, ep->fifosel);
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/9] usb: gadget: net2272: use generic map/umap routines
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
                   ` (5 preceding siblings ...)
  2012-01-24 11:45 ` [PATCH 6/9] usb: gadget: r8a66597: " Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-01-24 11:45 ` [PATCH 8/9] usb: gadget: net2280: use generic map/unmap routines Felipe Balbi
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

those routines have everything we need to map/unmap
USB requests and it's better to use them.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/net2272.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/net2272.c b/drivers/usb/gadget/net2272.c
index 7322d29..01ae56f 100644
--- a/drivers/usb/gadget/net2272.c
+++ b/drivers/usb/gadget/net2272.c
@@ -385,12 +385,9 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
 		status = req->req.status;
 
 	dev = ep->dev;
-	if (use_dma && req->mapped) {
-		dma_unmap_single(dev->dev, req->req.dma, req->req.length,
-			ep->is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-		req->req.dma = DMA_ADDR_INVALID;
-		req->mapped = 0;
-	}
+	if (use_dma && ep->dma)
+		usb_gadget_unmap_request(&dev->gadget, &req->req,
+				ep->is_in);
 
 	if (status && status != -ESHUTDOWN)
 		dev_vdbg(dev->dev, "complete %s req %p stat %d len %u/%u buf %p\n",
@@ -850,10 +847,11 @@ net2272_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
 		return -ESHUTDOWN;
 
 	/* set up dma mapping in case the caller didn't */
-	if (use_dma && ep->dma && _req->dma == DMA_ADDR_INVALID) {
-		_req->dma = dma_map_single(dev->dev, _req->buf, _req->length,
-			ep->is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-		req->mapped = 1;
+	if (use_dma && ep->dma) {
+		status = usb_gadget_map_request(&dev->gadget, _req,
+				ep->is_in);
+		if (status)
+			return status;
 	}
 
 	dev_vdbg(dev->dev, "%s queue req %p, len %d buf %p dma %08llx %s\n",
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/9] usb: gadget: net2280: use generic map/unmap routines
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
                   ` (6 preceding siblings ...)
  2012-01-24 11:45 ` [PATCH 7/9] usb: gadget: net2272: use generic map/umap routines Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-01-24 11:45 ` [PATCH 9/9] usb: gadget: goku: " Felipe Balbi
  2012-02-13 13:08 ` [PATCH 0/9] usb: gadget: " Felipe Balbi
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

those routines have everything we need to map/unmap
USB requests and it's better to use them.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/net2280.c |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index cdedd13..a5ccabc 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -806,12 +806,8 @@ done (struct net2280_ep *ep, struct net2280_request *req, int status)
 		status = req->req.status;
 
 	dev = ep->dev;
-	if (req->mapped) {
-		pci_unmap_single (dev->pdev, req->req.dma, req->req.length,
-			ep->is_in ? PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
-		req->req.dma = DMA_ADDR_INVALID;
-		req->mapped = 0;
-	}
+	if (ep->dma)
+		usb_gadget_unmap_request(&dev->gadget, &req->req, ep->is_in);
 
 	if (status && status != -ESHUTDOWN)
 		VDEBUG (dev, "complete %s req %p stat %d len %u/%u\n",
@@ -857,10 +853,13 @@ net2280_queue (struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
 		return -EOPNOTSUPP;
 
 	/* set up dma mapping in case the caller didn't */
-	if (ep->dma && _req->dma == DMA_ADDR_INVALID) {
-		_req->dma = pci_map_single (dev->pdev, _req->buf, _req->length,
-			ep->is_in ? PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
-		req->mapped = 1;
+	if (ep->dma) {
+		int ret;
+
+		ret = usb_gadget_map_request(&dev->gadget, _req,
+				ep->is_in);
+		if (ret)
+			return ret;
 	}
 
 #if 0
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 9/9] usb: gadget: goku: use generic map/unmap routines
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
                   ` (7 preceding siblings ...)
  2012-01-24 11:45 ` [PATCH 8/9] usb: gadget: net2280: use generic map/unmap routines Felipe Balbi
@ 2012-01-24 11:45 ` Felipe Balbi
  2012-02-13 13:08 ` [PATCH 0/9] usb: gadget: " Felipe Balbi
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-01-24 11:45 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

those routines have everything we need to map/unmap
USB requests and it's better to use them.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/goku_udc.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
index 5af70fcc..5e1b766 100644
--- a/drivers/usb/gadget/goku_udc.c
+++ b/drivers/usb/gadget/goku_udc.c
@@ -310,12 +310,9 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
 		status = req->req.status;
 
 	dev = ep->dev;
-	if (req->mapped) {
-		pci_unmap_single(dev->pdev, req->req.dma, req->req.length,
-			ep->is_in ? PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
-		req->req.dma = DMA_ADDR_INVALID;
-		req->mapped = 0;
-	}
+
+	if (ep->dma)
+		usb_gadget_unmap_request(&dev->gadget, &req->req, ep->is_in);
 
 #ifndef USB_TRACE
 	if (status && status != -ESHUTDOWN)
@@ -736,10 +733,11 @@ goku_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
 		return -EBUSY;
 
 	/* set up dma mapping in case the caller didn't */
-	if (ep->dma && _req->dma == DMA_ADDR_INVALID) {
-		_req->dma = pci_map_single(dev->pdev, _req->buf, _req->length,
-			ep->is_in ? PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
-		req->mapped = 1;
+	if (ep->dma) {
+		status = usb_gadget_map_request(&dev->gadget, &req->req,
+				ep->is_in);
+		if (status)
+			return status;
 	}
 
 #ifdef USB_TRACE
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] usb: gadget: generic map/unmap routines
  2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
                   ` (8 preceding siblings ...)
  2012-01-24 11:45 ` [PATCH 9/9] usb: gadget: goku: " Felipe Balbi
@ 2012-02-13 13:08 ` Felipe Balbi
  9 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2012-02-13 13:08 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Linux USB Mailing List, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

[-- Attachment #1: Type: text/plain, Size: 1038 bytes --]

Hi,

On Tue, Jan 24, 2012 at 01:45:21PM +0200, Felipe Balbi wrote:
> Hi all,
> 
> here's a rebased version of generic map/unmap patchset.
> 
> Also available on generic-map-unmap branch of my tree, some
> of the patches have been tested before and already have a
> Tested-by tag.
> 
> Felipe Balbi (9):
>   usb: gadget: add generic map/unmap request utilities
>   usb: dwc3: gadget: use generic map/unmap routines
>   usb: gadget: langwell: use generic map/unmap functions
>   usb: renesas: gadget: use generic map/unmap routines
>   usb: gadget: amd5536: use generic map/unmap routines
>   usb: gadget: r8a66597: use generic map/unmap routines
>   usb: gadget: net2272: use generic map/umap routines
>   usb: gadget: net2280: use generic map/unmap routines
>   usb: gadget: goku: use generic map/unmap routines

Tomorrow, will complete 3 weeks since I re-sent these patches. Since
nobody has had any issues with them, I'm considering people are happy,
so I'll queued them up for v3.4 merge window.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/9] usb: gadget: add generic map/unmap request utilities
  2011-12-19 15:49     ` Felipe Balbi
@ 2011-12-19 16:27       ` Alan Stern
  0 siblings, 0 replies; 15+ messages in thread
From: Alan Stern @ 2011-12-19 16:27 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Linux USB Mailing List, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

On Mon, 19 Dec 2011, Felipe Balbi wrote:

> > > +		if (dma_mapping_error(&gadget->dev, req->dma)) {
> > > +			dev_err(&gadget->dev, "failed to map buffer\n");
> > > +			return -EFAULT;
> > > +		}
> > > +	}
> > > +
> > > +	return 0;
> > 
> > You forgot to set req->mapped.
> 
> actually there's no 'mapped' field on struct usb_request. That's a

Whoops, you're right.  It's part of the UDC-private structure.

> nonsense added to all struct my_controller_request just because of that
> DMA_ADDR_INVALID hackery. I'm dropping that completely. There were no
> gadget drivers allocating memory from coherent or mapping requests
> themselves, so req->mapped becomes useless.

There's more to it than that.  A controller might support DMA on some
endpoints but not others, or for certain request lengths but not
others.  Hence when a request finishes, the driver needs to know
whether the request was mapped for DMA.

For example, your net2280 patch needs to be fixed.  Here's how it looks
now:

> --- a/drivers/usb/gadget/net2280.c
> +++ b/drivers/usb/gadget/net2280.c
> @@ -806,12 +806,8 @@ done (struct net2280_ep *ep, struct net2280_request *req, int status)
>  		status = req->req.status;
>  
>  	dev = ep->dev;
> -	if (req->mapped) {
> -		pci_unmap_single (dev->pdev, req->req.dma, req->req.length,
> -			ep->is_in ? PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
> -		req->req.dma = DMA_ADDR_INVALID;
> -		req->mapped = 0;
> -	}
> +	if (req->mapped)
> +		usb_gadget_unmap_request(&dev->gadget, &req->req, ep->is_in);

This code will never be executed because you never set req->mapped.  
And after that's fixed, when the unmapping occurs, req->mapped has to
be set back to 0.

> @@ -857,10 +853,13 @@ net2280_queue (struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
>  		return -EOPNOTSUPP;
>  
>  	/* set up dma mapping in case the caller didn't */
> -	if (ep->dma && _req->dma == DMA_ADDR_INVALID) {
> -		_req->dma = pci_map_single (dev->pdev, _req->buf, _req->length,
> -			ep->is_in ? PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
> -		req->mapped = 1;
> +	if (ep->dma) {
> +		int ret;
> +
> +		ret = usb_gadget_map_request(&dev->gadget, &req->req,
> +				ep->is_in);
> +		if (ret)
> +			return ret;

You probably need to set req->mapped right here.  Or rather, avoid 
removing the line that sets it.

>  	}

Alan Stern


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/9] usb: gadget: add generic map/unmap request utilities
  2011-12-19 15:19   ` Alan Stern
@ 2011-12-19 15:49     ` Felipe Balbi
  2011-12-19 16:27       ` Alan Stern
  0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2011-12-19 15:49 UTC (permalink / raw)
  To: Alan Stern
  Cc: Felipe Balbi, Linux USB Mailing List, Greg Kroah-Hartman,
	Thomas Dahlmann, Kuninori Morimoto,
	open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

[-- Attachment #1: Type: text/plain, Size: 2400 bytes --]

Hi,

On Mon, Dec 19, 2011 at 10:19:46AM -0500, Alan Stern wrote:
> On Mon, 19 Dec 2011, Felipe Balbi wrote:
> 
> > such utilities are currently duplicated on all UDC
> > drivers basically with the same structure. Let's group
> > all implementations into one generic implementation
> > and get rid of that duplication.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  drivers/usb/gadget/udc-core.c |   53 +++++++++++++++++++++++++++++++++++++++++
> >  include/linux/usb/gadget.h    |   10 +++++++
> >  2 files changed, 63 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
> > index 0b0d12c..2031c11 100644
> > --- a/drivers/usb/gadget/udc-core.c
> > +++ b/drivers/usb/gadget/udc-core.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/device.h>
> >  #include <linux/list.h>
> >  #include <linux/err.h>
> > +#include <linux/dma-mapping.h>
> >  
> >  #include <linux/usb/ch9.h>
> >  #include <linux/usb/gadget.h>
> > @@ -49,6 +50,58 @@ static DEFINE_MUTEX(udc_lock);
> >  
> >  /* ------------------------------------------------------------------------- */
> >  
> > +int usb_gadget_map_request(struct usb_gadget *gadget,
> > +		struct usb_request *req, int direction)
> > +{
> > +	if (req->length == 0)
> > +		return 0;
> > +
> > +	if (req->num_sgs) {
> > +		int     mapped;
> > +
> > +		mapped = dma_map_sg(&gadget->dev, req->sg, req->num_sgs,
> > +				direction ? DMA_TO_DEVICE
> > +				: DMA_FROM_DEVICE);
> > +		if (mapped == 0) {
> > +			dev_err(&gadget->dev, "failed to map SGs\n");
> > +			return -EFAULT;
> > +		}
> > +
> > +		req->num_mapped_sgs = mapped;
> > +	} else {
> > +		req->dma = dma_map_single(&gadget->dev, req->buf, req->length,
> > +				direction ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
> > +
> > +		if (dma_mapping_error(&gadget->dev, req->dma)) {
> > +			dev_err(&gadget->dev, "failed to map buffer\n");
> > +			return -EFAULT;
> > +		}
> > +	}
> > +
> > +	return 0;
> 
> You forgot to set req->mapped.

actually there's no 'mapped' field on struct usb_request. That's a
nonsense added to all struct my_controller_request just because of that
DMA_ADDR_INVALID hackery. I'm dropping that completely. There were no
gadget drivers allocating memory from coherent or mapping requests
themselves, so req->mapped becomes useless.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/9] usb: gadget: add generic map/unmap request utilities
  2011-12-19 10:30 ` [PATCH 1/9] usb: gadget: add generic map/unmap request utilities Felipe Balbi
@ 2011-12-19 15:19   ` Alan Stern
  2011-12-19 15:49     ` Felipe Balbi
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Stern @ 2011-12-19 15:19 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Linux USB Mailing List, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

On Mon, 19 Dec 2011, Felipe Balbi wrote:

> such utilities are currently duplicated on all UDC
> drivers basically with the same structure. Let's group
> all implementations into one generic implementation
> and get rid of that duplication.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/usb/gadget/udc-core.c |   53 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/gadget.h    |   10 +++++++
>  2 files changed, 63 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
> index 0b0d12c..2031c11 100644
> --- a/drivers/usb/gadget/udc-core.c
> +++ b/drivers/usb/gadget/udc-core.c
> @@ -22,6 +22,7 @@
>  #include <linux/device.h>
>  #include <linux/list.h>
>  #include <linux/err.h>
> +#include <linux/dma-mapping.h>
>  
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> @@ -49,6 +50,58 @@ static DEFINE_MUTEX(udc_lock);
>  
>  /* ------------------------------------------------------------------------- */
>  
> +int usb_gadget_map_request(struct usb_gadget *gadget,
> +		struct usb_request *req, int direction)
> +{
> +	if (req->length == 0)
> +		return 0;
> +
> +	if (req->num_sgs) {
> +		int     mapped;
> +
> +		mapped = dma_map_sg(&gadget->dev, req->sg, req->num_sgs,
> +				direction ? DMA_TO_DEVICE
> +				: DMA_FROM_DEVICE);
> +		if (mapped == 0) {
> +			dev_err(&gadget->dev, "failed to map SGs\n");
> +			return -EFAULT;
> +		}
> +
> +		req->num_mapped_sgs = mapped;
> +	} else {
> +		req->dma = dma_map_single(&gadget->dev, req->buf, req->length,
> +				direction ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
> +
> +		if (dma_mapping_error(&gadget->dev, req->dma)) {
> +			dev_err(&gadget->dev, "failed to map buffer\n");
> +			return -EFAULT;
> +		}
> +	}
> +
> +	return 0;

You forgot to set req->mapped.

> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_map_request);
> +
> +void usb_gadget_unmap_request(struct usb_gadget *gadget,
> +		struct usb_request *req, int direction)
> +{
> +	if (req->length == 0)
> +		return;
> +
> +	if (req->num_mapped_sgs) {
> +		dma_unmap_sg(&gadget->dev, req->sg, req->num_sgs,
> +				direction ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
> +
> +		req->num_mapped_sgs = 0;
> +	} else {
> +		dma_unmap_single(&gadget->dev, req->dma, req->length,
> +				direction ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
> +	}

You forgot to clear req->mapped.

Alan Stern


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/9] usb: gadget: add generic map/unmap request utilities
  2011-12-19 10:30 [PATCH 0/9] Add a generic request map/unmap routine Felipe Balbi
@ 2011-12-19 10:30 ` Felipe Balbi
  2011-12-19 15:19   ` Alan Stern
  0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2011-12-19 10:30 UTC (permalink / raw)
  To: Linux USB Mailing List
  Cc: Felipe Balbi, Greg Kroah-Hartman, Thomas Dahlmann,
	Kuninori Morimoto, open list:DESIGNWARE USB3 D...,
	open list, open list:AMD GEODE CS5536...

such utilities are currently duplicated on all UDC
drivers basically with the same structure. Let's group
all implementations into one generic implementation
and get rid of that duplication.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/udc-core.c |   53 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/gadget.h    |   10 +++++++
 2 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index 0b0d12c..2031c11 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -22,6 +22,7 @@
 #include <linux/device.h>
 #include <linux/list.h>
 #include <linux/err.h>
+#include <linux/dma-mapping.h>
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
@@ -49,6 +50,58 @@ static DEFINE_MUTEX(udc_lock);
 
 /* ------------------------------------------------------------------------- */
 
+int usb_gadget_map_request(struct usb_gadget *gadget,
+		struct usb_request *req, int direction)
+{
+	if (req->length == 0)
+		return 0;
+
+	if (req->num_sgs) {
+		int     mapped;
+
+		mapped = dma_map_sg(&gadget->dev, req->sg, req->num_sgs,
+				direction ? DMA_TO_DEVICE
+				: DMA_FROM_DEVICE);
+		if (mapped == 0) {
+			dev_err(&gadget->dev, "failed to map SGs\n");
+			return -EFAULT;
+		}
+
+		req->num_mapped_sgs = mapped;
+	} else {
+		req->dma = dma_map_single(&gadget->dev, req->buf, req->length,
+				direction ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+
+		if (dma_mapping_error(&gadget->dev, req->dma)) {
+			dev_err(&gadget->dev, "failed to map buffer\n");
+			return -EFAULT;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_map_request);
+
+void usb_gadget_unmap_request(struct usb_gadget *gadget,
+		struct usb_request *req, int direction)
+{
+	if (req->length == 0)
+		return;
+
+	if (req->num_mapped_sgs) {
+		dma_unmap_sg(&gadget->dev, req->sg, req->num_sgs,
+				direction ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+
+		req->num_mapped_sgs = 0;
+	} else {
+		dma_unmap_single(&gadget->dev, req->dma, req->length,
+				direction ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	}
+}
+EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
+
+/* ------------------------------------------------------------------------- */
+
 /**
  * usb_gadget_start - tells usb device controller to start up
  * @gadget: The gadget we want to get started
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 911d4c7..9f84fdf 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -950,6 +950,16 @@ static inline void usb_free_descriptors(struct usb_descriptor_header **v)
 
 /*-------------------------------------------------------------------------*/
 
+/* utility to simplify map/unmap of usb_requests to/from DMA */
+
+extern int usb_gadget_map_request(struct usb_gadget *gadget,
+		struct usb_request *req, int direction);
+
+extern void usb_gadget_unmap_request(struct usb_gadget *gadget,
+		struct usb_request *req, int direction);
+
+/*-------------------------------------------------------------------------*/
+
 /* utility wrapping a simple endpoint selection policy */
 
 extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
-- 
1.7.8.rc3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-02-13 13:08 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-24 11:45 [PATCH 0/9] usb: gadget: generic map/unmap routines Felipe Balbi
2012-01-24 11:45 ` [PATCH 1/9] usb: gadget: add generic map/unmap request utilities Felipe Balbi
2012-01-24 11:45 ` [PATCH 2/9] usb: dwc3: gadget: use generic map/unmap routines Felipe Balbi
2012-01-24 11:45 ` [PATCH 3/9] usb: gadget: langwell: use generic map/unmap functions Felipe Balbi
2012-01-24 11:45 ` [PATCH 4/9] usb: renesas: gadget: use generic map/unmap routines Felipe Balbi
2012-01-24 11:45 ` [PATCH 5/9] usb: gadget: amd5536: " Felipe Balbi
2012-01-24 11:45 ` [PATCH 6/9] usb: gadget: r8a66597: " Felipe Balbi
2012-01-24 11:45 ` [PATCH 7/9] usb: gadget: net2272: use generic map/umap routines Felipe Balbi
2012-01-24 11:45 ` [PATCH 8/9] usb: gadget: net2280: use generic map/unmap routines Felipe Balbi
2012-01-24 11:45 ` [PATCH 9/9] usb: gadget: goku: " Felipe Balbi
2012-02-13 13:08 ` [PATCH 0/9] usb: gadget: " Felipe Balbi
  -- strict thread matches above, loose matches on Subject: below --
2011-12-19 10:30 [PATCH 0/9] Add a generic request map/unmap routine Felipe Balbi
2011-12-19 10:30 ` [PATCH 1/9] usb: gadget: add generic map/unmap request utilities Felipe Balbi
2011-12-19 15:19   ` Alan Stern
2011-12-19 15:49     ` Felipe Balbi
2011-12-19 16:27       ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).