Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1230184 > unrolled thread

[PATCH v2 00/12] usb: gadget: amd5536udc: fix memory leaks

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/12] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:30 +0200
    [PATCH v2 08/12] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:30 +0200
    [PATCH v2 03/12] usb: gadget: amd5536udc: use WARN_ON Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:30 +0200
    [PATCH v2 05/12] usb: gadget: amd5536udc: remove unnecessary conditions Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:30 +0200
    [PATCH v2 12/12] usb: gadget: amd5536udc: NULL comparison Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:30 +0200
    [PATCH v2 06/12] usb: gadget: amd5536udc: remove forward declaration of udc_probe Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:30 +0200
    [PATCH v2 09/12] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:30 +0200
    [PATCH v2 04/12] usb: gadget: amd5536udc: use free_dma_pools Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:40 +0200
    [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:40 +0200
      Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path Felipe Balbi <balbi@ti.com> - 2015-09-22 16:40 +0200
        Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 16:50 +0200
          Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 17:10 +0200
            Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path Felipe Balbi <balbi@ti.com> - 2015-09-22 17:30 +0200
              Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 18:40 +0200
              Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-30 16:00 +0200
    [PATCH v2 07/12] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-22 15:40 +0200

#1230184 — [PATCH v2 00/12] usb: gadget: amd5536udc: fix memory leaks

Proper error handling was done in the probe and some unwanted comparison
was removed. Also some forward declaration was removed.

regards
sudip

Sudip Mukherjee (12):
  usb: gadget: amd5536udc: rewrite init_dma_pools
  usb: gadget: amd5536udc: fix error path
  usb: gadget: amd5536udc: use WARN_ON
  usb: gadget: amd5536udc: use free_dma_pools
  usb: gadget: amd5536udc: remove unnecessary conditions
  usb: gadget: amd5536udc: remove forward declaration of udc_probe
  usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup
  usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain
  usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain
  usb: gadget: amd5536udc: remove forward declaration of udc_pci_*
  usb: gadget: amd5536udc: remove forward declaration of udc_basic_init
  usb: gadget: amd5536udc: NULL comparison

 drivers/usb/gadget/udc/amd5536udc.c | 609 ++++++++++++++++++------------------
 drivers/usb/gadget/udc/amd5536udc.h |   5 +-
 2 files changed, 301 insertions(+), 313 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1230186 — [PATCH v2 08/12] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain

Rearrange udc_create_dma_chain to remove the forward declaration. While
rearranging fixed the relevant checkpatch warnings.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 238 ++++++++++++++++++------------------
 1 file changed, 117 insertions(+), 121 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index f218520..2602173 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -71,8 +71,6 @@ static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
 static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
-static int udc_create_dma_chain(struct udc_ep *ep, struct udc_request *req,
-				unsigned long buf_len, gfp_t gfp_flags);
 static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void udc_pci_remove(struct pci_dev *pdev);
 
@@ -787,6 +785,123 @@ udc_rxfifo_read(struct udc_ep *ep, struct udc_request *req)
 	return finished;
 }
 
+/* Creates or re-inits a DMA chain */
+static int udc_create_dma_chain(
+	struct udc_ep *ep,
+	struct udc_request *req,
+	unsigned long buf_len, gfp_t gfp_flags
+)
+{
+	unsigned long bytes = req->req.length;
+	unsigned int i;
+	dma_addr_t dma_addr;
+	struct udc_data_dma	*td = NULL;
+	struct udc_data_dma	*last = NULL;
+	unsigned long txbytes;
+	unsigned create_new_chain = 0;
+	unsigned len;
+
+	VDBG(ep->dev, "udc_create_dma_chain: bytes=%ld buf_len=%ld\n",
+	     bytes, buf_len);
+	dma_addr = DMA_DONT_USE;
+
+	/* unset L bit in first desc for OUT */
+	if (!ep->in)
+		req->td_data->status &= AMD_CLEAR_BIT(UDC_DMA_IN_STS_L);
+
+	/* alloc only new desc's if not already available */
+	len = req->req.length / ep->ep.maxpacket;
+	if (req->req.length % ep->ep.maxpacket)
+		len++;
+
+	if (len > req->chain_len) {
+		/* shorter chain already allocated before */
+		if (req->chain_len > 1)
+			udc_free_dma_chain(ep->dev, req);
+		req->chain_len = len;
+		create_new_chain = 1;
+	}
+
+	td = req->td_data;
+	/* gen. required number of descriptors and buffers */
+	for (i = buf_len; i < bytes; i += buf_len) {
+		/* create or determine next desc. */
+		if (create_new_chain) {
+			td = pci_pool_alloc(ep->dev->data_requests,
+					    gfp_flags, &dma_addr);
+			if (!td)
+				return -ENOMEM;
+
+			td->status = 0;
+		} else if (i == buf_len) {
+			/* first td */
+			td = (struct udc_data_dma *)phys_to_virt(
+						req->td_data->next);
+			td->status = 0;
+		} else {
+			td = (struct udc_data_dma *)phys_to_virt(last->next);
+			td->status = 0;
+		}
+
+		if (td)
+			td->bufptr = req->req.dma + i; /* assign buffer */
+		else
+			break;
+
+		/* short packet ? */
+		if ((bytes - i) >= buf_len) {
+			txbytes = buf_len;
+		} else {
+			/* short packet */
+			txbytes = bytes - i;
+		}
+
+		/* link td and assign tx bytes */
+		if (i == buf_len) {
+			if (create_new_chain)
+				req->td_data->next = dma_addr;
+			/*
+			 * else
+			 *	req->td_data->next = virt_to_phys(td);
+			 */
+			/* write tx bytes */
+			if (ep->in) {
+				/* first desc */
+				req->td_data->status =
+					AMD_ADDBITS(req->td_data->status,
+						    ep->ep.maxpacket,
+						    UDC_DMA_IN_STS_TXBYTES);
+				/* second desc */
+				td->status = AMD_ADDBITS(td->status,
+							txbytes,
+							UDC_DMA_IN_STS_TXBYTES);
+			}
+		} else {
+			if (create_new_chain)
+				last->next = dma_addr;
+			/*
+			 * else
+			 *	last->next = virt_to_phys(td);
+			 */
+			if (ep->in) {
+				/* write tx bytes */
+				td->status = AMD_ADDBITS(td->status,
+							txbytes,
+							UDC_DMA_IN_STS_TXBYTES);
+			}
+		}
+		last = td;
+	}
+	/* set last bit */
+	if (td) {
+		td->status |= AMD_BIT(UDC_DMA_IN_STS_L);
+		/* last desc. points to itself */
+		req->td_data_last = td;
+	}
+
+	return 0;
+}
+
 /* create/re-init a DMA descriptor or a DMA descriptor chain */
 static int prep_dma(struct udc_ep *ep, struct udc_request *req, gfp_t gfp)
 {
@@ -973,125 +1088,6 @@ static u32 udc_get_ppbdu_rxbytes(struct udc_request *req)
 
 }
 
-/* Creates or re-inits a DMA chain */
-static int udc_create_dma_chain(
-	struct udc_ep *ep,
-	struct udc_request *req,
-	unsigned long buf_len, gfp_t gfp_flags
-)
-{
-	unsigned long bytes = req->req.length;
-	unsigned int i;
-	dma_addr_t dma_addr;
-	struct udc_data_dma	*td = NULL;
-	struct udc_data_dma	*last = NULL;
-	unsigned long txbytes;
-	unsigned create_new_chain = 0;
-	unsigned len;
-
-	VDBG(ep->dev, "udc_create_dma_chain: bytes=%ld buf_len=%ld\n",
-			bytes, buf_len);
-	dma_addr = DMA_DONT_USE;
-
-	/* unset L bit in first desc for OUT */
-	if (!ep->in)
-		req->td_data->status &= AMD_CLEAR_BIT(UDC_DMA_IN_STS_L);
-
-	/* alloc only new desc's if not already available */
-	len = req->req.length / ep->ep.maxpacket;
-	if (req->req.length % ep->ep.maxpacket)
-		len++;
-
-	if (len > req->chain_len) {
-		/* shorter chain already allocated before */
-		if (req->chain_len > 1)
-			udc_free_dma_chain(ep->dev, req);
-		req->chain_len = len;
-		create_new_chain = 1;
-	}
-
-	td = req->td_data;
-	/* gen. required number of descriptors and buffers */
-	for (i = buf_len; i < bytes; i += buf_len) {
-		/* create or determine next desc. */
-		if (create_new_chain) {
-
-			td = pci_pool_alloc(ep->dev->data_requests,
-					gfp_flags, &dma_addr);
-			if (!td)
-				return -ENOMEM;
-
-			td->status = 0;
-		} else if (i == buf_len) {
-			/* first td */
-			td = (struct udc_data_dma *) phys_to_virt(
-						req->td_data->next);
-			td->status = 0;
-		} else {
-			td = (struct udc_data_dma *) phys_to_virt(last->next);
-			td->status = 0;
-		}
-
-
-		if (td)
-			td->bufptr = req->req.dma + i; /* assign buffer */
-		else
-			break;
-
-		/* short packet ? */
-		if ((bytes - i) >= buf_len) {
-			txbytes = buf_len;
-		} else {
-			/* short packet */
-			txbytes = bytes - i;
-		}
-
-		/* link td and assign tx bytes */
-		if (i == buf_len) {
-			if (create_new_chain)
-				req->td_data->next = dma_addr;
-			/*
-			else
-				req->td_data->next = virt_to_phys(td);
-			*/
-			/* write tx bytes */
-			if (ep->in) {
-				/* first desc */
-				req->td_data->status =
-					AMD_ADDBITS(req->td_data->status,
-							ep->ep.maxpacket,
-							UDC_DMA_IN_STS_TXBYTES);
-				/* second desc */
-				td->status = AMD_ADDBITS(td->status,
-							txbytes,
-							UDC_DMA_IN_STS_TXBYTES);
-			}
-		} else {
-			if (create_new_chain)
-				last->next = dma_addr;
-			/*
-			else
-				last->next = virt_to_phys(td);
-			*/
-			if (ep->in) {
-				/* write tx bytes */
-				td->status = AMD_ADDBITS(td->status,
-							txbytes,
-							UDC_DMA_IN_STS_TXBYTES);
-			}
-		}
-		last = td;
-	}
-	/* set last bit */
-	if (td) {
-		td->status |= AMD_BIT(UDC_DMA_IN_STS_L);
-		/* last desc. points to itself */
-		req->td_data_last = td;
-	}
-
-	return 0;
-}
-
 /* Enabling RX DMA */
 static void udc_set_rde(struct udc *dev)
 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230191 — [PATCH v2 03/12] usb: gadget: amd5536udc: use WARN_ON

Use WARN_ON() instead of halting the kernel with BUG_ON() and also fix
the checkpatch warning.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 3f85044..6223b1b 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3127,7 +3127,8 @@ static void udc_pci_remove(struct pci_dev *pdev)
 
 	usb_del_gadget_udc(&udc->gadget);
 	/* gadget driver must not be registered */
-	BUG_ON(dev->driver != NULL);
+	if (WARN_ON(dev->driver))
+		return;
 
 	/* dma pool cleanup */
 	if (dev->data_requests)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230193 — [PATCH v2 05/12] usb: gadget: amd5536udc: remove unnecessary conditions

The condition checking for irq_registered, regs, mem_region and active
are not required as this is the remove function. And we are in the
remove means that probe was successful and they can never be NULL at
this point of code.
It was required in the original code as the remove function was part of
the error handler of probe function.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 17 +++++------------
 drivers/usb/gadget/udc/amd5536udc.h |  5 +----
 2 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 7805b29..89e83e4 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3135,15 +3135,11 @@ static void udc_pci_remove(struct pci_dev *pdev)
 
 	/* reset controller */
 	writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), &dev->regs->cfg);
-	if (dev->irq_registered)
-		free_irq(pdev->irq, dev);
-	if (dev->virt_addr)
-		iounmap(dev->virt_addr);
-	if (dev->mem_region)
-		release_mem_region(pci_resource_start(pdev, 0),
-				pci_resource_len(pdev, 0));
-	if (dev->active)
-		pci_disable_device(pdev);
+	free_irq(pdev->irq, dev);
+	iounmap(dev->virt_addr);
+	release_mem_region(pci_resource_start(pdev, 0),
+			   pci_resource_len(pdev, 0));
+	pci_disable_device(pdev);
 
 	udc_remove(dev);
 }
@@ -3240,7 +3236,6 @@ static int udc_pci_probe(
 		retval = -ENODEV;
 		goto err_pcidev;
 	}
-	dev->active = 1;
 
 	/* PCI resource allocation */
 	resource = pci_resource_start(pdev, 0);
@@ -3251,7 +3246,6 @@ static int udc_pci_probe(
 		retval = -EBUSY;
 		goto err_memreg;
 	}
-	dev->mem_region = 1;
 
 	dev->virt_addr = ioremap_nocache(resource, len);
 	if (dev->virt_addr == NULL) {
@@ -3282,7 +3276,6 @@ static int udc_pci_probe(
 		retval = -EBUSY;
 		goto err_irq;
 	}
-	dev->irq_registered = 1;
 
 	pci_set_drvdata(pdev, dev);
 
diff --git a/drivers/usb/gadget/udc/amd5536udc.h b/drivers/usb/gadget/udc/amd5536udc.h
index 6744d3b..4638d70 100644
--- a/drivers/usb/gadget/udc/amd5536udc.h
+++ b/drivers/usb/gadget/udc/amd5536udc.h
@@ -526,14 +526,11 @@ struct udc {
 	struct udc_ep			ep[UDC_EP_NUM];
 	struct usb_gadget_driver	*driver;
 	/* operational flags */
-	unsigned			active : 1,
-					stall_ep0in : 1,
+	unsigned			stall_ep0in : 1,
 					waiting_zlp_ack_ep0in : 1,
 					set_cfg_not_acked : 1,
-					irq_registered : 1,
 					data_ep_enabled : 1,
 					data_ep_queued : 1,
-					mem_region : 1,
 					sys_suspended : 1,
 					connected;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230195 — [PATCH v2 12/12] usb: gadget: amd5536udc: NULL comparison

A NULL comparison can be written as if (var) or if (!var).

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 157bff1..cd87641 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -2185,7 +2185,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 		}
 
 	/* DMA */
-	} else if (!ep->cancel_transfer && req != NULL) {
+	} else if (!ep->cancel_transfer && req) {
 		ret_val = IRQ_HANDLED;
 
 		/* check for DMA done */
@@ -3189,7 +3189,7 @@ static int init_dma_pools(struct udc *dev)
 	/* setup */
 	td_stp = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
 				&dev->ep[UDC_EP0OUT_IX].td_stp_dma);
-	if (td_stp == NULL) {
+	if (!td_stp) {
 		retval = -ENOMEM;
 		goto err_alloc_dma;
 	}
@@ -3198,7 +3198,7 @@ static int init_dma_pools(struct udc *dev)
 	/* data: 0 packets !? */
 	td_data = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
 				&dev->ep[UDC_EP0OUT_IX].td_phys);
-	if (td_data == NULL) {
+	if (!td_data) {
 		retval = -ENOMEM;
 		goto err_alloc_phys;
 	}
@@ -3322,7 +3322,7 @@ static int udc_pci_probe(
 	}
 
 	dev->virt_addr = ioremap_nocache(resource, len);
-	if (dev->virt_addr == NULL) {
+	if (!dev->virt_addr) {
 		dev_dbg(&pdev->dev, "start address cannot be mapped\n");
 		retval = -EFAULT;
 		goto err_ioremap;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230197 — [PATCH v2 06/12] usb: gadget: amd5536udc: remove forward declaration of udc_probe

Rearrange the udc_probe function to remove the forward declarations.
While rearranging also fixed the relevant checkpatch warnings.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 133 ++++++++++++++++++------------------
 1 file changed, 66 insertions(+), 67 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 89e83e4..6c16737 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -65,7 +65,6 @@
 
 static void udc_tasklet_disconnect(unsigned long);
 static void empty_req_queue(struct udc_ep *);
-static int udc_probe(struct udc *dev);
 static void udc_basic_init(struct udc *dev);
 static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
@@ -3209,6 +3208,72 @@ err_create_dma_pool:
 	return retval;
 }
 
+/* general probe */
+static int udc_probe(struct udc *dev)
+{
+	char		tmp[128];
+	u32		reg;
+	int		retval;
+
+	/* mark timer as not initialized */
+	udc_timer.data = 0;
+	udc_pollstall_timer.data = 0;
+
+	/* device struct setup */
+	dev->gadget.ops = &udc_ops;
+
+	dev_set_name(&dev->gadget.dev, "gadget");
+	dev->gadget.name = name;
+	dev->gadget.max_speed = USB_SPEED_HIGH;
+
+	/* init registers, interrupts, ... */
+	startup_registers(dev);
+
+	dev_info(&dev->pdev->dev, "%s\n", mod_desc);
+
+	snprintf(tmp, sizeof(tmp), "%d", dev->irq);
+	dev_info(&dev->pdev->dev,
+		 "irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
+		 tmp, dev->phys_addr, dev->chiprev,
+		 (dev->chiprev == UDC_HSA0_REV) ? "A0" : "B1");
+	strcpy(tmp, UDC_DRIVER_VERSION_STRING);
+	if (dev->chiprev == UDC_HSA0_REV) {
+		dev_err(&dev->pdev->dev, "chip revision is A0; too old\n");
+		retval = -ENODEV;
+		goto finished;
+	}
+	dev_info(&dev->pdev->dev,
+		 "driver version: %s(for Geode5536 B1)\n", tmp);
+	udc = dev;
+
+	retval = usb_add_gadget_udc_release(&udc->pdev->dev, &dev->gadget,
+					    gadget_release);
+	if (retval)
+		goto finished;
+
+	/* timer init */
+	init_timer(&udc_timer);
+	udc_timer.function = udc_timer_function;
+	udc_timer.data = 1;
+	/* timer pollstall init */
+	init_timer(&udc_pollstall_timer);
+	udc_pollstall_timer.function = udc_pollstall_timer_function;
+	udc_pollstall_timer.data = 1;
+
+	/* set SD */
+	reg = readl(&dev->regs->ctl);
+	reg |= AMD_BIT(UDC_DEVCTL_SD);
+	writel(reg, &dev->regs->ctl);
+
+	/* print dev register info */
+	print_regs(dev);
+
+	return 0;
+
+finished:
+	return retval;
+}
+
 /* Called by pci bus driver to init pci context */
 static int udc_pci_probe(
 	struct pci_dev *pdev,
@@ -3319,72 +3384,6 @@ err_pcidev:
 	return retval;
 }
 
-/* general probe */
-static int udc_probe(struct udc *dev)
-{
-	char		tmp[128];
-	u32		reg;
-	int		retval;
-
-	/* mark timer as not initialized */
-	udc_timer.data = 0;
-	udc_pollstall_timer.data = 0;
-
-	/* device struct setup */
-	dev->gadget.ops = &udc_ops;
-
-	dev_set_name(&dev->gadget.dev, "gadget");
-	dev->gadget.name = name;
-	dev->gadget.max_speed = USB_SPEED_HIGH;
-
-	/* init registers, interrupts, ... */
-	startup_registers(dev);
-
-	dev_info(&dev->pdev->dev, "%s\n", mod_desc);
-
-	snprintf(tmp, sizeof tmp, "%d", dev->irq);
-	dev_info(&dev->pdev->dev,
-		"irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
-		tmp, dev->phys_addr, dev->chiprev,
-		(dev->chiprev == UDC_HSA0_REV) ? "A0" : "B1");
-	strcpy(tmp, UDC_DRIVER_VERSION_STRING);
-	if (dev->chiprev == UDC_HSA0_REV) {
-		dev_err(&dev->pdev->dev, "chip revision is A0; too old\n");
-		retval = -ENODEV;
-		goto finished;
-	}
-	dev_info(&dev->pdev->dev,
-		"driver version: %s(for Geode5536 B1)\n", tmp);
-	udc = dev;
-
-	retval = usb_add_gadget_udc_release(&udc->pdev->dev, &dev->gadget,
-			gadget_release);
-	if (retval)
-		goto finished;
-
-	/* timer init */
-	init_timer(&udc_timer);
-	udc_timer.function = udc_timer_function;
-	udc_timer.data = 1;
-	/* timer pollstall init */
-	init_timer(&udc_pollstall_timer);
-	udc_pollstall_timer.function = udc_pollstall_timer_function;
-	udc_pollstall_timer.data = 1;
-
-	/* set SD */
-	reg = readl(&dev->regs->ctl);
-	reg |= AMD_BIT(UDC_DEVCTL_SD);
-	writel(reg, &dev->regs->ctl);
-
-	/* print dev register info */
-	print_regs(dev);
-
-	return 0;
-
-finished:
-	return retval;
-}
-
 /* Initiates a remote wakeup */
 static int udc_remote_wakeup(struct udc *dev)
 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230199 — [PATCH v2 09/12] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain

Rearrange udc_free_dma_chain to remove the forward declaration. While at
it fixed all the relevant checkpatch warnings.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 51 +++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 2602173..6d64129 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -70,7 +70,6 @@ static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
 static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
 static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void udc_pci_remove(struct pci_dev *pdev);
 
@@ -611,6 +610,30 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 	return &req->req;
 }
 
+/* frees pci pool descriptors of a DMA chain */
+static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+{
+	int ret_val = 0;
+	struct udc_data_dma	*td;
+	struct udc_data_dma	*td_last = NULL;
+	unsigned int i;
+
+	DBG(dev, "free chain req = %p\n", req);
+
+	/* do not free first desc., will be done by free for request */
+	td_last = req->td_data;
+	td = phys_to_virt(td_last->next);
+
+	for (i = 1; i < req->chain_len; i++) {
+		pci_pool_free(dev->data_requests, td,
+			      (dma_addr_t)td_last->next);
+		td_last = td;
+		td = phys_to_virt(td_last->next);
+	}
+
+	return ret_val;
+}
+
 /* Frees request packet, called by gadget driver */
 static void
 udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq)
@@ -1026,32 +1049,6 @@ __acquires(ep->dev->lock)
 	ep->halted = halted;
 }
 
-/* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
-{
-
-	int ret_val = 0;
-	struct udc_data_dma	*td;
-	struct udc_data_dma	*td_last = NULL;
-	unsigned int i;
-
-	DBG(dev, "free chain req = %p\n", req);
-
-	/* do not free first desc., will be done by free for request */
-	td_last = req->td_data;
-	td = phys_to_virt(td_last->next);
-
-	for (i = 1; i < req->chain_len; i++) {
-
-		pci_pool_free(dev->data_requests, td,
-				(dma_addr_t) td_last->next);
-		td_last = td;
-		td = phys_to_virt(td_last->next);
-	}
-
-	return ret_val;
-}
-
 /* Iterates to the end of a DMA chain and returns last descriptor */
 static struct udc_data_dma *udc_get_last_dma_desc(struct udc_request *req)
 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230203 — [PATCH v2 04/12] usb: gadget: amd5536udc: use free_dma_pools

We have the function free_dma_pools() which frees all the dma pools. Use
it instead of calling all the functions separately. The if conditions
for data_requests and stp_requests are also not required here as this is
the remove function and we are here means probe has succeeded and dma
has been successfully allocated, so they cannot be NULL here.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 6223b1b..7805b29 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3131,20 +3131,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
 		return;
 
 	/* dma pool cleanup */
-	if (dev->data_requests)
-		pci_pool_destroy(dev->data_requests);
-
-	if (dev->stp_requests) {
-		/* cleanup DMA desc's for ep0in */
-		pci_pool_free(dev->stp_requests,
-			dev->ep[UDC_EP0OUT_IX].td_stp,
-			dev->ep[UDC_EP0OUT_IX].td_stp_dma);
-		pci_pool_free(dev->stp_requests,
-			dev->ep[UDC_EP0OUT_IX].td,
-			dev->ep[UDC_EP0OUT_IX].td_phys);
-
-		pci_pool_destroy(dev->stp_requests);
-	}
+	free_dma_pools(dev);
 
 	/* reset controller */
 	writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), &dev->regs->cfg);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230210 — [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path

Handle the error properly instead of calling the pci remove function.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 38a6858..3f85044 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3107,6 +3107,17 @@ static void udc_remove(struct udc *dev)
 	udc = NULL;
 }
 
+/* free all the dma pools */
+static void free_dma_pools(struct udc *dev)
+{
+	dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td,
+		      dev->ep[UDC_EP0OUT_IX].td_phys);
+	dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td_stp,
+		      dev->ep[UDC_EP0OUT_IX].td_stp_dma);
+	dma_pool_destroy(dev->stp_requests);
+	dma_pool_destroy(dev->data_requests);
+}
+
 /* Reset all pci context */
 static void udc_pci_remove(struct pci_dev *pdev)
 {
@@ -3297,7 +3308,7 @@ static int udc_pci_probe(
 	if (use_dma) {
 		retval = init_dma_pools(dev);
 		if (retval != 0)
-			goto finished;
+			goto err_dma;
 	}
 
 	dev->phys_addr = resource;
@@ -3305,13 +3316,17 @@ static int udc_pci_probe(
 	dev->pdev = pdev;
 
 	/* general probing */
-	if (udc_probe(dev) == 0)
-		return 0;
-
-finished:
-	udc_pci_remove(pdev);
-	return retval;
+	if (udc_probe(dev)) {
+		retval = -ENODEV;
+		goto err_probe;
+	}
+	return 0;
 
+err_probe:
+	if (use_dma)
+		free_dma_pools(dev);
+err_dma:
+	free_irq(pdev->irq, dev);
 err_irq:
 	iounmap(dev->virt_addr);
 err_ioremap:
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230269 — Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path

[Multipart message — attachments visible in raw view] — view raw

On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> Handle the error properly instead of calling the pci remove function.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

this doesn't apply. Where did you rebase this series ? Please rebase on my
testing/next

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1230290 — Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path

On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > Handle the error properly instead of calling the pci remove function.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> 
> this doesn't apply. Where did you rebase this series ? Please rebase on my
> testing/next
This was done on next-20150922. I will rebase on your tree.

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230300 — Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path

On Tue, Sep 22, 2015 at 08:12:29PM +0530, Sudip Mukherjee wrote:
> On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> > On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > > Handle the error properly instead of calling the pci remove function.
> > > 
> > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > 
> > this doesn't apply. Where did you rebase this series ? Please rebase on my
> > testing/next
> This was done on next-20150922. I will rebase on your tree.
Looks like today is a day of confusion for me.
I asked you to discard my v1 as I saw some part of the change was done
by:  6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")

But now I dont see that patch in your testing/next , but I can see that
patch in next-20150922. Confused. :(
Now?		
 
regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230320 — Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path

[Multipart message — attachments visible in raw view] — view raw

On Tue, Sep 22, 2015 at 08:29:52PM +0530, Sudip Mukherjee wrote:
> On Tue, Sep 22, 2015 at 08:12:29PM +0530, Sudip Mukherjee wrote:
> > On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> > > On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > > > Handle the error properly instead of calling the pci remove function.
> > > > 
> > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > 
> > > this doesn't apply. Where did you rebase this series ? Please rebase on my
> > > testing/next
> > This was done on next-20150922. I will rebase on your tree.
> Looks like today is a day of confusion for me.
> I asked you to discard my v1 as I saw some part of the change was done
> by:  6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")
> 
> But now I dont see that patch in your testing/next , but I can see that
> patch in next-20150922. Confused. :(
> Now?

heh, let's wait a bit then, seems like I need to wait for -rc3 and merge that
in my testing/next.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1230455 — Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path

On Tue, Sep 22, 2015 at 10:19:46AM -0500, Felipe Balbi wrote:
> On Tue, Sep 22, 2015 at 08:29:52PM +0530, Sudip Mukherjee wrote:
> > On Tue, Sep 22, 2015 at 08:12:29PM +0530, Sudip Mukherjee wrote:
> > > On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> > > > On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > > > > Handle the error properly instead of calling the pci remove function.
> > > > > 
> > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > > 
> > > > this doesn't apply. Where did you rebase this series ? Please rebase on my
> > > > testing/next
> > > This was done on next-20150922. I will rebase on your tree.
> > Looks like today is a day of confusion for me.
> > I asked you to discard my v1 as I saw some part of the change was done
> > by:  6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")
> > 
> > But now I dont see that patch in your testing/next , but I can see that
> > patch in next-20150922. Confused. :(
> > Now?
> 
> heh, let's wait a bit then, seems like I need to wait for -rc3 and merge that
> in my testing/next.
It is in your usb/fixes tree. I am not rebasing then.

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1236309 — Re: [PATCH v2 02/12] usb: gadget: amd5536udc: fix error path

On Tue, Sep 22, 2015 at 10:19:46AM -0500, Felipe Balbi wrote:
> On Tue, Sep 22, 2015 at 08:29:52PM +0530, Sudip Mukherjee wrote:
> > On Tue, Sep 22, 2015 at 08:12:29PM +0530, Sudip Mukherjee wrote:
> > > On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> > > > On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > > > > Handle the error properly instead of calling the pci remove function.
> > > > > 
> > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > > 
> > > > this doesn't apply. Where did you rebase this series ? Please rebase on my
> > > > testing/next
> > > This was done on next-20150922. I will rebase on your tree.
> > Looks like today is a day of confusion for me.
> > I asked you to discard my v1 as I saw some part of the change was done
> > by:  6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")
> > 
> > But now I dont see that patch in your testing/next , but I can see that
> > patch in next-20150922. Confused. :(
> > Now?
> 
> heh, let's wait a bit then, seems like I need to wait for -rc3 and merge that
> in my testing/next.
Hi Felipe,
A gentle reminder. This series was waiting for -rc3 to be merged.

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230214 — [PATCH v2 07/12] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup

Rearrange the udc_remote_wakeup function to remove the forward
declaration.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 41 ++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 6c16737..f218520 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -73,7 +73,6 @@ static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
 static int udc_create_dma_chain(struct udc_ep *ep, struct udc_request *req,
 				unsigned long buf_len, gfp_t gfp_flags);
-static int udc_remote_wakeup(struct udc *dev);
 static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void udc_pci_remove(struct pci_dev *pdev);
 
@@ -1452,6 +1451,26 @@ static int udc_get_frame(struct usb_gadget *gadget)
 	return -EOPNOTSUPP;
 }
 
+/* Initiates a remote wakeup */
+static int udc_remote_wakeup(struct udc *dev)
+{
+	unsigned long flags;
+	u32 tmp;
+
+	DBG(dev, "UDC initiates remote wakeup\n");
+
+	spin_lock_irqsave(&dev->lock, flags);
+
+	tmp = readl(&dev->regs->ctl);
+	tmp |= AMD_BIT(UDC_DEVCTL_RES);
+	writel(tmp, &dev->regs->ctl);
+	tmp &= AMD_CLEAR_BIT(UDC_DEVCTL_RES);
+	writel(tmp, &dev->regs->ctl);
+
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return 0;
+}
+
 /* Remote wakeup gadget interface */
 static int udc_wakeup(struct usb_gadget *gadget)
 {
@@ -3384,26 +3403,6 @@ err_pcidev:
 	return retval;
 }
 
-/* Initiates a remote wakeup */
-static int udc_remote_wakeup(struct udc *dev)
-{
-	unsigned long flags;
-	u32 tmp;
-
-	DBG(dev, "UDC initiates remote wakeup\n");
-
-	spin_lock_irqsave(&dev->lock, flags);
-
-	tmp = readl(&dev->regs->ctl);
-	tmp |= AMD_BIT(UDC_DEVCTL_RES);
-	writel(tmp, &dev->regs->ctl);
-	tmp &= AMD_CLEAR_BIT(UDC_DEVCTL_RES);
-	writel(tmp, &dev->regs->ctl);
-
-	spin_unlock_irqrestore(&dev->lock, flags);
-	return 0;
-}
-
 /* PCI device parameters */
 static const struct pci_device_id pci_id[] = {
 	{
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web