linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
@ 2015-09-14 15:12 Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 01/16] usb: gadget: amd5536udc: introduce free_dma_pools Sudip Mukherjee
                   ` (16 more replies)
  0 siblings, 17 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

This amd5536udc was a complete mess. The major problems that i could
find are:

1) if udc_pci_probe() fails in any stage then it just calls the
udc_pci_remove() to handle error. And udc_pci_remove() works with
struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
incase of error we are handling the error by dereferencing a NULL
pointer.

2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
will be set only if probe is success. So that means if probe fails then
probe will call udc_pci_remove() for error handling and udc_pci_remove()
will inturn halts the kernel by calling BUG().

And apart from these numerous memory leaks and not releasing of
resources. Here comes a rewrite of few of the functions in an
attempt to fix these.

regards
sudip

Sudip Mukherjee (16):
  usb: gadget: amd5536udc: introduce free_dma_pools
  usb: gadget: amd5536udc: rewrite init_dma_pools
  usb: gadget: amd5536udc: rewrite udc_pci_probe
  usb: gadget: amd5536udc: use WARN_ON
  usb: gadget: amd5536udc: use free_dma_pools
  usb: gadget: amd5536udc: remove unnecessary conditions
  usb: gadget: amd5536udc: unmap virt_addr
  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
  usb: gadget: amd5536udc: remove multiple blank lines
  usb: gadget: amd5536udc: match alignment

 drivers/usb/gadget/udc/amd5536udc.c | 797 ++++++++++++++++++------------------
 1 file changed, 390 insertions(+), 407 deletions(-)

-- 
1.9.1


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

* [PATCH 01/16] usb: gadget: amd5536udc: introduce free_dma_pools
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 02/16] usb: gadget: amd5536udc: rewrite init_dma_pools Sudip Mukherjee
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

dma pools are being created by init_dma_pools() but there was no
function for freeing them. Introduce the function now so that we can use
it later.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index fdacddb..384b824 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)
 {
-- 
1.9.1


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

* [PATCH 02/16] usb: gadget: amd5536udc: rewrite init_dma_pools
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 01/16] usb: gadget: amd5536udc: introduce free_dma_pools Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 03/16] usb: gadget: amd5536udc: rewrite udc_pci_probe Sudip Mukherjee
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

A rewrite of init_dma_pools() with proper error handling.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 384b824..8b700de 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3180,8 +3180,7 @@ static int init_dma_pools(struct udc *dev)
 		sizeof(struct udc_data_dma), 0, 0);
 	if (!dev->data_requests) {
 		DBG(dev, "can't get request data pool\n");
-		retval = -ENOMEM;
-		goto finished;
+		return -ENOMEM;
 	}
 
 	/* EP0 in dma regs = dev control regs */
@@ -3193,14 +3192,14 @@ static int init_dma_pools(struct udc *dev)
 	if (!dev->stp_requests) {
 		DBG(dev, "can't get stp request pool\n");
 		retval = -ENOMEM;
-		goto finished;
+		goto err_create_dma_pool;
 	}
 	/* setup */
 	td_stp = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
 				&dev->ep[UDC_EP0OUT_IX].td_stp_dma);
 	if (td_stp == NULL) {
 		retval = -ENOMEM;
-		goto finished;
+		goto err_alloc_dma;
 	}
 	dev->ep[UDC_EP0OUT_IX].td_stp = td_stp;
 
@@ -3209,12 +3208,20 @@ static int init_dma_pools(struct udc *dev)
 				&dev->ep[UDC_EP0OUT_IX].td_phys);
 	if (td_data == NULL) {
 		retval = -ENOMEM;
-		goto finished;
+		goto err_alloc_phys;
 	}
 	dev->ep[UDC_EP0OUT_IX].td = td_data;
 	return 0;
 
-finished:
+err_alloc_phys:
+	dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td_stp,
+		      dev->ep[UDC_EP0OUT_IX].td_stp_dma);
+err_alloc_dma:
+	dma_pool_destroy(dev->stp_requests);
+	dev->stp_requests = NULL;
+err_create_dma_pool:
+	dma_pool_destroy(dev->data_requests);
+	dev->data_requests = NULL;
 	return retval;
 }
 
-- 
1.9.1


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

* [PATCH 03/16] usb: gadget: amd5536udc: rewrite udc_pci_probe
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 01/16] usb: gadget: amd5536udc: introduce free_dma_pools Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 02/16] usb: gadget: amd5536udc: rewrite init_dma_pools Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 04/16] usb: gadget: amd5536udc: use WARN_ON Sudip Mukherjee
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

A rewrite of udc_pci_probe() with proper error handling. We use here
free_dma_pools() in error handling.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 8b700de..8646f6c 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3244,17 +3244,13 @@ static int udc_pci_probe(
 
 	/* init */
 	dev = kzalloc(sizeof(struct udc), GFP_KERNEL);
-	if (!dev) {
-		retval = -ENOMEM;
-		goto finished;
-	}
+	if (!dev)
+		return -ENOMEM;
 
 	/* pci setup */
 	if (pci_enable_device(pdev) < 0) {
-		kfree(dev);
-		dev = NULL;
 		retval = -ENODEV;
-		goto finished;
+		goto err_enabledev;
 	}
 	dev->active = 1;
 
@@ -3264,28 +3260,22 @@ static int udc_pci_probe(
 
 	if (!request_mem_region(resource, len, name)) {
 		dev_dbg(&pdev->dev, "pci device used already\n");
-		kfree(dev);
-		dev = NULL;
 		retval = -EBUSY;
-		goto finished;
+		goto err_requestmem;
 	}
 	dev->mem_region = 1;
 
 	dev->virt_addr = ioremap_nocache(resource, len);
 	if (dev->virt_addr == NULL) {
 		dev_dbg(&pdev->dev, "start address cannot be mapped\n");
-		kfree(dev);
-		dev = NULL;
 		retval = -EFAULT;
-		goto finished;
+		goto err_ioremap;
 	}
 
 	if (!pdev->irq) {
 		dev_err(&pdev->dev, "irq not set\n");
-		kfree(dev);
-		dev = NULL;
 		retval = -ENODEV;
-		goto finished;
+		goto err_irqreq;
 	}
 
 	spin_lock_init(&dev->lock);
@@ -3301,10 +3291,8 @@ static int udc_pci_probe(
 
 	if (request_irq(pdev->irq, udc_irq, IRQF_SHARED, name, dev) != 0) {
 		dev_dbg(&pdev->dev, "request_irq(%d) fail\n", pdev->irq);
-		kfree(dev);
-		dev = NULL;
 		retval = -EBUSY;
-		goto finished;
+		goto err_irqreq;
 	}
 	dev->irq_registered = 1;
 
@@ -3320,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;
@@ -3328,12 +3316,26 @@ static int udc_pci_probe(
 	dev->pdev = pdev;
 
 	/* general probing */
-	if (udc_probe(dev) == 0)
-		return 0;
+	if (udc_probe(dev) != 0) {
+		retval = -ENODEV;
+		goto err_probe;
+	}
+	return 0;
 
-finished:
-	if (dev)
-		udc_pci_remove(pdev);
+err_probe:
+	if (use_dma)
+		free_dma_pools(dev);
+err_dma:
+	free_irq(pdev->irq, dev);
+err_irqreq:
+	iounmap(dev->virt_addr);
+err_ioremap:
+	release_mem_region(resource, len);
+err_requestmem:
+	pci_disable_device(pdev);
+err_enabledev:
+	kfree(dev);
+	dev = NULL;
 	return retval;
 }
 
-- 
1.9.1


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

* [PATCH 04/16] usb: gadget: amd5536udc: use WARN_ON
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (2 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 03/16] usb: gadget: amd5536udc: rewrite udc_pci_probe Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 05/16] usb: gadget: amd5536udc: use free_dma_pools Sudip Mukherjee
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

Use WARN_ON() instead of halting the kernel with BUG_ON().

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 8646f6c..4edcfd4 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 != NULL))
+		return;
 
 	/* dma pool cleanup */
 	if (dev->data_requests)
-- 
1.9.1


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

* [PATCH 05/16] usb: gadget: amd5536udc: use free_dma_pools
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (3 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 04/16] usb: gadget: amd5536udc: use WARN_ON Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 06/16] usb: gadget: amd5536udc: remove unnecessary conditions Sudip Mukherjee
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

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 4edcfd4..3ae0bb8 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


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

* [PATCH 06/16] usb: gadget: amd5536udc: remove unnecessary conditions
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (4 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 05/16] usb: gadget: amd5536udc: use free_dma_pools Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 07/16] usb: gadget: amd5536udc: unmap virt_addr Sudip Mukherjee
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

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 | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 3ae0bb8..e2f6128 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->regs)
-		iounmap(dev->regs);
-	if (dev->mem_region)
-		release_mem_region(pci_resource_start(pdev, 0),
+	free_irq(pdev->irq, dev);
+	iounmap(dev->regs);
+	release_mem_region(pci_resource_start(pdev, 0),
 				pci_resource_len(pdev, 0));
-	if (dev->active)
-		pci_disable_device(pdev);
+	pci_disable_device(pdev);
 
 	udc_remove(dev);
 }
-- 
1.9.1


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

* [PATCH 07/16] usb: gadget: amd5536udc: unmap virt_addr
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (5 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 06/16] usb: gadget: amd5536udc: remove unnecessary conditions Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 08/16] usb: gadget: amd5536udc: remove forward declaration of udc_probe Sudip Mukherjee
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

We have actually ioremapped dev->virt_addr and dev->regs is
dev->virt_addr + UDC_DEVCFG_ADDR so while unmapping we should unmap
dev->virt_addr.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index e2f6128..b7753b2 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3136,7 +3136,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
 	/* reset controller */
 	writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), &dev->regs->cfg);
 	free_irq(pdev->irq, dev);
-	iounmap(dev->regs);
+	iounmap(dev->virt_addr);
 	release_mem_region(pci_resource_start(pdev, 0),
 				pci_resource_len(pdev, 0));
 	pci_disable_device(pdev);
-- 
1.9.1


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

* [PATCH 08/16] usb: gadget: amd5536udc: remove forward declaration of udc_probe
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (6 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 07/16] usb: gadget: amd5536udc: unmap virt_addr Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 09/16] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup Sudip Mukherjee
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

Rearrange the udc_probe function to remove the forward declarations.

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 b7753b2..01a6183 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,
@@ -3323,72 +3388,6 @@ err_enabledev:
 	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


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

* [PATCH 09/16] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (7 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 08/16] usb: gadget: amd5536udc: remove forward declaration of udc_probe Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 10/16] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain Sudip Mukherjee
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

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 | 40 ++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 01a6183..8b0ed74 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -1452,6 +1452,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)
 {
@@ -3388,26 +3408,6 @@ err_enabledev:
 	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


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

* [PATCH 10/16] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (8 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 09/16] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 11/16] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain Sudip Mukherjee
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

Rearrange udc_create_dma_chain to remove the forward declaration.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 8b0ed74..34edb9b 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -71,9 +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_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);
 
@@ -788,6 +785,125 @@ 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)
 {
@@ -974,125 +1090,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


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

* [PATCH 11/16] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (9 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 10/16] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:12 ` [PATCH 12/16] usb: gadget: amd5536udc: remove forward declaration of udc_pci_* Sudip Mukherjee
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

Rearrange udc_free_dma_chain to remove the forward declaration.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 34edb9b..778a424 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,32 @@ 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)
@@ -1028,32 +1053,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


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

* [PATCH 12/16] usb: gadget: amd5536udc: remove forward declaration of udc_pci_*
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (10 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 11/16] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain Sudip Mukherjee
@ 2015-09-14 15:12 ` Sudip Mukherjee
  2015-09-14 15:13 ` [PATCH 13/16] usb: gadget: amd5536udc: remove forward declaration of udc_basic_init Sudip Mukherjee
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:12 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

Remove the forward declarations of udc_pci_probe and udc_pci_remove.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 778a424..789441f 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -70,8 +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_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
-static void udc_pci_remove(struct pci_dev *pdev);
 
 /* description */
 static const char mod_desc[] = UDC_MOD_DESCRIPTION;
-- 
1.9.1


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

* [PATCH 13/16] usb: gadget: amd5536udc: remove forward declaration of udc_basic_init
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (11 preceding siblings ...)
  2015-09-14 15:12 ` [PATCH 12/16] usb: gadget: amd5536udc: remove forward declaration of udc_pci_* Sudip Mukherjee
@ 2015-09-14 15:13 ` Sudip Mukherjee
  2015-09-14 15:13 ` [PATCH 14/16] usb: gadget: amd5536udc: NULL comparison Sudip Mukherjee
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:13 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

Rearrange the udc_basic_init function to remove the forward declaration.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 789441f..f6d6097 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 void udc_basic_init(struct udc *dev);
 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);
@@ -1511,33 +1510,6 @@ static void make_ep_lists(struct udc *dev)
 	dev->ep[UDC_EPOUT_IX].fifo_depth = UDC_RXFIFO_SIZE;
 }
 
-/* init registers at driver load time */
-static int startup_registers(struct udc *dev)
-{
-	u32 tmp;
-
-	/* init controller by soft reset */
-	udc_soft_reset(dev);
-
-	/* mask not needed interrupts */
-	udc_mask_unused_interrupts(dev);
-
-	/* put into initial config */
-	udc_basic_init(dev);
-	/* link up all endpoints */
-	udc_setup_endpoints(dev);
-
-	/* program speed */
-	tmp = readl(&dev->regs->cfg);
-	if (use_fullspeed)
-		tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_FS, UDC_DEVCFG_SPD);
-	else
-		tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_HS, UDC_DEVCFG_SPD);
-	writel(tmp, &dev->regs->cfg);
-
-	return 0;
-}
-
 /* Inits UDC context */
 static void udc_basic_init(struct udc *dev)
 {
@@ -1576,6 +1548,33 @@ static void udc_basic_init(struct udc *dev)
 	dev->data_ep_queued = 0;
 }
 
+/* init registers at driver load time */
+static int startup_registers(struct udc *dev)
+{
+	u32 tmp;
+
+	/* init controller by soft reset */
+	udc_soft_reset(dev);
+
+	/* mask not needed interrupts */
+	udc_mask_unused_interrupts(dev);
+
+	/* put into initial config */
+	udc_basic_init(dev);
+	/* link up all endpoints */
+	udc_setup_endpoints(dev);
+
+	/* program speed */
+	tmp = readl(&dev->regs->cfg);
+	if (use_fullspeed)
+		tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_FS, UDC_DEVCFG_SPD);
+	else
+		tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_HS, UDC_DEVCFG_SPD);
+	writel(tmp, &dev->regs->cfg);
+
+	return 0;
+}
+
 /* Sets initial endpoint parameters */
 static void udc_setup_endpoints(struct udc *dev)
 {
-- 
1.9.1


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

* [PATCH 14/16] usb: gadget: amd5536udc: NULL comparison
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (12 preceding siblings ...)
  2015-09-14 15:13 ` [PATCH 13/16] usb: gadget: amd5536udc: remove forward declaration of udc_basic_init Sudip Mukherjee
@ 2015-09-14 15:13 ` Sudip Mukherjee
  2015-09-14 15:13 ` [PATCH 15/16] usb: gadget: amd5536udc: remove multiple blank lines Sudip Mukherjee
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:13 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

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 | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index f6d6097..3657a66 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -2189,7 +2189,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 */
@@ -3139,7 +3139,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
 
 	usb_del_gadget_udc(&udc->gadget);
 	/* gadget driver must not be registered */
-	if (WARN_ON(dev->driver != NULL))
+	if (WARN_ON(dev->driver))
 		return;
 
 	/* dma pool cleanup */
@@ -3193,7 +3193,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;
 	}
@@ -3202,7 +3202,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;
 	}
@@ -3328,7 +3328,7 @@ static int udc_pci_probe(
 	dev->mem_region = 1;
 
 	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


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

* [PATCH 15/16] usb: gadget: amd5536udc: remove multiple blank lines
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (13 preceding siblings ...)
  2015-09-14 15:13 ` [PATCH 14/16] usb: gadget: amd5536udc: NULL comparison Sudip Mukherjee
@ 2015-09-14 15:13 ` Sudip Mukherjee
  2015-09-14 15:13 ` [PATCH 16/16] usb: gadget: amd5536udc: match alignment Sudip Mukherjee
  2015-09-18 18:39 ` [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Felipe Balbi
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:13 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

checkpatch complains about multiple blank lines.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 3657a66..98b841d 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -62,7 +62,6 @@
 /* udc specific */
 #include "amd5536udc.h"
 
-
 static void udc_tasklet_disconnect(unsigned long);
 static void empty_req_queue(struct udc_ep *);
 static void udc_setup_endpoints(struct udc *dev);
@@ -127,7 +126,6 @@ static DECLARE_COMPLETION(on_pollstall_exit);
 static DECLARE_TASKLET(disconnect_tasklet, udc_tasklet_disconnect,
 		(unsigned long) &udc);
 
-
 /* endpoint names used for print */
 static const char ep0_string[] = "ep0in";
 static const struct {
@@ -364,7 +362,6 @@ static void UDC_QUEUE_CNAK(struct udc_ep *ep, unsigned num)
 		cnak_pending = cnak_pending & (~(1 << (num)));
 }
 
-
 /* Enables endpoint, is called by gadget driver */
 static int
 udc_ep_enable(struct usb_ep *usbep, const struct usb_endpoint_descriptor *desc)
@@ -866,7 +863,6 @@ static int udc_create_dma_chain(
 			td->status = 0;
 		}
 
-
 		if (td)
 			td->bufptr = req->req.dma + i; /* assign buffer */
 		else
@@ -1000,7 +996,6 @@ static int prep_dma(struct udc_ep *ep, struct udc_request *req, gfp_t gfp)
 				UDC_DMA_STP_STS_BS_HOST_READY,
 				UDC_DMA_STP_STS_BS);
 
-
 			/* clear NAK by writing CNAK */
 			if (ep->naking) {
 				tmp = readl(&ep->regs->ctl);
@@ -1728,7 +1723,6 @@ static void udc_tasklet_disconnect(unsigned long par)
 	ep_init(dev->regs,
 			&dev->ep[UDC_EP0IN_IX]);
 
-
 	if (!soft_reset_occured) {
 		/* init controller by soft reset */
 		udc_soft_reset(dev);
@@ -2120,7 +2114,6 @@ static void udc_ep0_set_rde(struct udc *dev)
 	}
 }
 
-
 /* Interrupt handler for data OUT traffic */
 static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 {
@@ -2634,7 +2627,6 @@ __acquires(dev->lock)
 		} else
 			dev->waiting_zlp_ack_ep0in = 1;
 
-
 		/* clear NAK by writing CNAK in EP0_OUT */
 		if (!set) {
 			tmp = readl(&dev->ep[UDC_EP0OUT_IX].regs->ctl);
@@ -2809,7 +2801,6 @@ static irqreturn_t udc_control_in_isr(struct udc *dev)
 	return ret_val;
 }
 
-
 /* Interrupt handler for global device events */
 static irqreturn_t udc_dev_isr(struct udc *dev, u32 dev_irq)
 __releases(dev->lock)
@@ -2846,7 +2837,6 @@ __acquires(dev->lock)
 				/* ep ix in UDC CSR register space */
 				udc_csr_epix = ep->num;
 
-
 			/* OUT ep */
 			} else {
 				/* ep ix in UDC CSR register space */
@@ -2899,7 +2889,6 @@ __acquires(dev->lock)
 				/* ep ix in UDC CSR register space */
 				udc_csr_epix = ep->num;
 
-
 			/* OUT ep */
 			} else {
 				/* ep ix in UDC CSR register space */
@@ -3080,7 +3069,6 @@ static irqreturn_t udc_irq(int irq, void *pdev)
 
 	}
 
-
 	/* check for dev irq */
 	reg = readl(&dev->regs->irqsts);
 	if (reg) {
@@ -3089,7 +3077,6 @@ static irqreturn_t udc_irq(int irq, void *pdev)
 		ret_val |= udc_dev_isr(dev, reg);
 	}
 
-
 	spin_unlock(&dev->lock);
 	return ret_val;
 }
-- 
1.9.1


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

* [PATCH 16/16] usb: gadget: amd5536udc: match alignment
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (14 preceding siblings ...)
  2015-09-14 15:13 ` [PATCH 15/16] usb: gadget: amd5536udc: remove multiple blank lines Sudip Mukherjee
@ 2015-09-14 15:13 ` Sudip Mukherjee
  2015-09-18 18:39 ` [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Felipe Balbi
  16 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-14 15:13 UTC (permalink / raw)
  To: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman
  Cc: linux-kernel, linux-geode, linux-usb, Sudip Mukherjee

checkpatch complains that the alignment should match with open
parenthesis.

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

diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 98b841d..9f73de8 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -226,7 +226,7 @@ module_param(use_dma_ppb, bool, S_IRUGO);
 MODULE_PARM_DESC(use_dma_ppb, "true for DMA in packet per buffer mode");
 module_param(use_dma_ppb_du, bool, S_IRUGO);
 MODULE_PARM_DESC(use_dma_ppb_du,
-	"true for DMA in packet per buffer mode with descriptor update");
+		 "true for DMA in packet per buffer mode with descriptor update");
 module_param(use_fullspeed, bool, S_IRUGO);
 MODULE_PARM_DESC(use_fullspeed, "true for fullspeed only");
 
@@ -436,7 +436,7 @@ udc_ep_enable(struct usb_ep *usbep, const struct usb_endpoint_descriptor *desc)
 		/* set max packet size UDC CSR	*/
 		tmp = readl(&dev->csr->ne[ep->num - UDC_CSR_EP_OUT_IX_OFS]);
 		tmp = AMD_ADDBITS(tmp, maxpacket,
-					UDC_CSR_NE_MAX_PKT);
+				  UDC_CSR_NE_MAX_PKT);
 		writel(tmp, &dev->csr->ne[ep->num - UDC_CSR_EP_OUT_IX_OFS]);
 
 		if (use_dma && !ep->in) {
@@ -581,7 +581,7 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 	if (ep->dma) {
 		/* ep0 in requests are allocated from data pool here */
 		dma_desc = pci_pool_alloc(ep->dev->data_requests, gfp,
-						&req->td_phys);
+					  &req->td_phys);
 		if (!dma_desc) {
 			kfree(req);
 			return NULL;
@@ -622,7 +622,7 @@ static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
 	for (i = 1; i < req->chain_len; i++) {
 
 		pci_pool_free(dev->data_requests, td,
-				(dma_addr_t) td_last->next);
+			      (dma_addr_t) td_last->next);
 		td_last = td;
 		td = phys_to_virt(td_last->next);
 	}
@@ -652,7 +652,7 @@ udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq)
 			udc_free_dma_chain(ep->dev, req);
 
 		pci_pool_free(ep->dev->data_requests, req->td_data,
-							req->td_phys);
+			      req->td_phys);
 	}
 	kfree(req);
 }
@@ -672,7 +672,7 @@ static void udc_init_bna_dummy(struct udc_request *req)
 					UDC_DMA_STP_STS_BS);
 #ifdef UDC_VERBOSE
 		pr_debug("bna desc = %p, sts = %08x\n",
-			req->td_data, req->td_data->status);
+			 req->td_data, req->td_data->status);
 #endif
 	}
 }
@@ -723,7 +723,7 @@ udc_txfifo_write(struct udc_ep *ep, struct usb_request *req)
 	/* remaining bytes must be written by byte access */
 	for (j = 0; j < bytes % UDC_DWORD_BYTES; j++) {
 		writeb((u8)(*(buf + i) >> (j << UDC_BITS_PER_BYTE_SHIFT)),
-							ep->txfifo);
+		       ep->txfifo);
 	}
 
 	/* dummy write confirm */
@@ -784,7 +784,7 @@ udc_rxfifo_read(struct udc_ep *ep, struct udc_request *req)
 	if (bytes > buf_space) {
 		if ((buf_space % ep->ep.maxpacket) != 0) {
 			DBG(ep->dev,
-				"%s: rx %d bytes, rx-buf space = %d bytesn\n",
+			    "%s: rx %d bytes, rx-buf space = %d bytesn\n",
 				ep->ep.name, bytes, buf_space);
 			req->req.status = -EOVERFLOW;
 		}
@@ -821,7 +821,7 @@ static int udc_create_dma_chain(
 	unsigned len;
 
 	VDBG(ep->dev, "udc_create_dma_chain: bytes=%ld buf_len=%ld\n",
-			bytes, buf_len);
+	     bytes, buf_len);
 	dma_addr = DMA_DONT_USE;
 
 	/* unset L bit in first desc for OUT */
@@ -848,7 +848,7 @@ static int udc_create_dma_chain(
 		if (create_new_chain) {
 
 			td = pci_pool_alloc(ep->dev->data_requests,
-					gfp_flags, &dma_addr);
+					    gfp_flags, &dma_addr);
 			if (!td)
 				return -ENOMEM;
 
@@ -889,7 +889,7 @@ static int udc_create_dma_chain(
 				/* first desc */
 				req->td_data->status =
 					AMD_ADDBITS(req->td_data->status,
-							ep->ep.maxpacket,
+						    ep->ep.maxpacket,
 							UDC_DMA_IN_STS_TXBYTES);
 				/* second desc */
 				td->status = AMD_ADDBITS(td->status,
@@ -930,7 +930,7 @@ static int prep_dma(struct udc_ep *ep, struct udc_request *req, gfp_t gfp)
 
 	VDBG(ep->dev, "prep_dma\n");
 	VDBG(ep->dev, "prep_dma ep%d req->td_data=%p\n",
-			ep->num, req->td_data);
+	     ep->num, req->td_data);
 
 	/* set buffer pointer */
 	req->td_data->bufptr = req->req.dma;
@@ -952,7 +952,7 @@ static int prep_dma(struct udc_ep *ep, struct udc_request *req, gfp_t gfp)
 				/* write tx bytes */
 				req->td_data->status =
 					AMD_ADDBITS(req->td_data->status,
-						ep->ep.maxpacket,
+						    ep->ep.maxpacket,
 						UDC_DMA_IN_STS_TXBYTES);
 
 			}
@@ -975,25 +975,25 @@ static int prep_dma(struct udc_ep *ep, struct udc_request *req, gfp_t gfp)
 			/* write tx bytes */
 			req->td_data->status =
 				AMD_ADDBITS(req->td_data->status,
-						req->req.length,
+					    req->req.length,
 						UDC_DMA_IN_STS_TXBYTES);
 			/* reset frame num */
 			req->td_data->status =
 				AMD_ADDBITS(req->td_data->status,
-						0,
+					    0,
 						UDC_DMA_IN_STS_FRAMENUM);
 		}
 		/* set HOST BUSY */
 		req->td_data->status =
 			AMD_ADDBITS(req->td_data->status,
-				UDC_DMA_STP_STS_BS_HOST_BUSY,
+				    UDC_DMA_STP_STS_BS_HOST_BUSY,
 				UDC_DMA_STP_STS_BS);
 	} else {
 		VDBG(ep->dev, "OUT set host ready\n");
 		/* set HOST READY */
 		req->td_data->status =
 			AMD_ADDBITS(req->td_data->status,
-				UDC_DMA_STP_STS_BS_HOST_READY,
+				    UDC_DMA_STP_STS_BS_HOST_READY,
 				UDC_DMA_STP_STS_BS);
 
 			/* clear NAK by writing CNAK */
@@ -1037,7 +1037,7 @@ __acquires(ep->dev->lock)
 	list_del_init(&req->queue);
 
 	VDBG(ep->dev, "req %p => complete %d bytes at %s with sts %d\n",
-		&req->req, req->req.length, ep->ep.name, sts);
+	     &req->req, req->req.length, ep->ep.name, sts);
 
 	spin_unlock(&dev->lock);
 	usb_gadget_giveback_request(&ep->ep, &req->req);
@@ -1136,7 +1136,7 @@ udc_queue(struct usb_ep *usbep, struct usb_request *usbreq, gfp_t gfp)
 	}
 
 	VDBG(dev, "%s queue req %p, len %d req->td_data=%p buf %p\n",
-			usbep->name, usbreq, usbreq->length,
+	     usbep->name, usbreq, usbreq->length,
 			req->td_data, usbreq->buf);
 
 	spin_lock_irqsave(&dev->lock, iflags);
@@ -1169,7 +1169,7 @@ udc_queue(struct usb_ep *usbep, struct usb_request *usbreq, gfp_t gfp)
 				writel(tmp, &dev->ep[UDC_EP0IN_IX].regs->ctl);
 				dev->ep[UDC_EP0IN_IX].naking = 0;
 				UDC_QUEUE_CNAK(&dev->ep[UDC_EP0IN_IX],
-							UDC_EP0IN_IX);
+					       UDC_EP0IN_IX);
 				dev->waiting_zlp_ack_ep0in = 0;
 			}
 			goto finished;
@@ -1183,7 +1183,7 @@ udc_queue(struct usb_ep *usbep, struct usb_request *usbreq, gfp_t gfp)
 				/* set HOST READY */
 				req->td_data->status =
 					AMD_ADDBITS(req->td_data->status,
-						UDC_DMA_IN_STS_BS_HOST_READY,
+						    UDC_DMA_IN_STS_BS_HOST_READY,
 						UDC_DMA_IN_STS_BS);
 			}
 
@@ -1207,7 +1207,7 @@ udc_queue(struct usb_ep *usbep, struct usb_request *usbreq, gfp_t gfp)
 				if (ep->bna_occurred) {
 					VDBG(dev, "copy to BNA dummy desc.\n");
 					memcpy(ep->bna_dummy_req->td_data,
-						req->td_data,
+					       req->td_data,
 						sizeof(struct udc_data_dma));
 				}
 			}
@@ -1294,7 +1294,7 @@ static void empty_req_queue(struct udc_ep *ep)
 	ep->halted = 1;
 	while (!list_empty(&ep->queue)) {
 		req = list_entry(ep->queue.next,
-			struct udc_request,
+				 struct udc_request,
 			queue);
 		complete_req(ep, req, -ESHUTDOWN);
 	}
@@ -1329,19 +1329,19 @@ static int udc_dequeue(struct usb_ep *usbep, struct usb_request *usbreq)
 				/* stop potential receive DMA */
 				tmp = readl(&udc->regs->ctl);
 				writel(tmp & AMD_UNMASK_BIT(UDC_DEVCTL_RDE),
-							&udc->regs->ctl);
+				       &udc->regs->ctl);
 				/*
 				 * Cancel transfer later in ISR
 				 * if descriptor was touched.
 				 */
 				dma_sts = AMD_GETBITS(req->td_data->status,
-							UDC_DMA_OUT_STS_BS);
+						      UDC_DMA_OUT_STS_BS);
 				if (dma_sts != UDC_DMA_OUT_STS_BS_HOST_READY)
 					ep->cancel_transfer = 1;
 				else {
 					udc_init_bna_dummy(ep->req);
 					writel(ep->bna_dummy_req->td_phys,
-						&ep->regs->desptr);
+					       &ep->regs->desptr);
 				}
 				writel(tmp, &udc->regs->ctl);
 			}
@@ -1474,7 +1474,7 @@ static int udc_wakeup(struct usb_gadget *gadget)
 }
 
 static int amd5536_udc_start(struct usb_gadget *g,
-		struct usb_gadget_driver *driver);
+			     struct usb_gadget_driver *driver);
 static int amd5536_udc_stop(struct usb_gadget *g);
 
 static const struct usb_gadget_ops udc_ops = {
@@ -1490,11 +1490,11 @@ static void make_ep_lists(struct udc *dev)
 	/* make gadget ep lists */
 	INIT_LIST_HEAD(&dev->gadget.ep_list);
 	list_add_tail(&dev->ep[UDC_EPIN_STATUS_IX].ep.ep_list,
-						&dev->gadget.ep_list);
+		      &dev->gadget.ep_list);
 	list_add_tail(&dev->ep[UDC_EPIN_IX].ep.ep_list,
-						&dev->gadget.ep_list);
+		      &dev->gadget.ep_list);
 	list_add_tail(&dev->ep[UDC_EPOUT_IX].ep.ep_list,
-						&dev->gadget.ep_list);
+		      &dev->gadget.ep_list);
 
 	/* fifo config */
 	dev->ep[UDC_EPIN_STATUS_IX].fifo_depth = UDC_EPIN_SMALLINT_BUFF_SIZE;
@@ -1721,7 +1721,7 @@ static void udc_tasklet_disconnect(unsigned long par)
 
 	/* disable ep0 */
 	ep_init(dev->regs,
-			&dev->ep[UDC_EP0IN_IX]);
+		&dev->ep[UDC_EP0IN_IX]);
 
 	if (!soft_reset_occured) {
 		/* init controller by soft reset */
@@ -1897,40 +1897,40 @@ static void activate_control_endpoints(struct udc *dev)
 	tmp = readl(&dev->ep[UDC_EP0IN_IX].regs->bufin_framenum);
 	if (dev->gadget.speed == USB_SPEED_FULL)
 		tmp = AMD_ADDBITS(tmp, UDC_FS_EPIN0_BUFF_SIZE,
-					UDC_EPIN_BUFF_SIZE);
+				  UDC_EPIN_BUFF_SIZE);
 	else if (dev->gadget.speed == USB_SPEED_HIGH)
 		tmp = AMD_ADDBITS(tmp, UDC_EPIN0_BUFF_SIZE,
-					UDC_EPIN_BUFF_SIZE);
+				  UDC_EPIN_BUFF_SIZE);
 	writel(tmp, &dev->ep[UDC_EP0IN_IX].regs->bufin_framenum);
 
 	/* set max packet size of EP0_IN */
 	tmp = readl(&dev->ep[UDC_EP0IN_IX].regs->bufout_maxpkt);
 	if (dev->gadget.speed == USB_SPEED_FULL)
 		tmp = AMD_ADDBITS(tmp, UDC_FS_EP0IN_MAX_PKT_SIZE,
-					UDC_EP_MAX_PKT_SIZE);
+				  UDC_EP_MAX_PKT_SIZE);
 	else if (dev->gadget.speed == USB_SPEED_HIGH)
 		tmp = AMD_ADDBITS(tmp, UDC_EP0IN_MAX_PKT_SIZE,
-				UDC_EP_MAX_PKT_SIZE);
+				  UDC_EP_MAX_PKT_SIZE);
 	writel(tmp, &dev->ep[UDC_EP0IN_IX].regs->bufout_maxpkt);
 
 	/* set max packet size of EP0_OUT */
 	tmp = readl(&dev->ep[UDC_EP0OUT_IX].regs->bufout_maxpkt);
 	if (dev->gadget.speed == USB_SPEED_FULL)
 		tmp = AMD_ADDBITS(tmp, UDC_FS_EP0OUT_MAX_PKT_SIZE,
-					UDC_EP_MAX_PKT_SIZE);
+				  UDC_EP_MAX_PKT_SIZE);
 	else if (dev->gadget.speed == USB_SPEED_HIGH)
 		tmp = AMD_ADDBITS(tmp, UDC_EP0OUT_MAX_PKT_SIZE,
-					UDC_EP_MAX_PKT_SIZE);
+				  UDC_EP_MAX_PKT_SIZE);
 	writel(tmp, &dev->ep[UDC_EP0OUT_IX].regs->bufout_maxpkt);
 
 	/* set max packet size of EP0 in UDC CSR */
 	tmp = readl(&dev->csr->ne[0]);
 	if (dev->gadget.speed == USB_SPEED_FULL)
 		tmp = AMD_ADDBITS(tmp, UDC_FS_EP0OUT_MAX_PKT_SIZE,
-					UDC_CSR_NE_MAX_PKT);
+				  UDC_CSR_NE_MAX_PKT);
 	else if (dev->gadget.speed == USB_SPEED_HIGH)
 		tmp = AMD_ADDBITS(tmp, UDC_EP0OUT_MAX_PKT_SIZE,
-					UDC_CSR_NE_MAX_PKT);
+				  UDC_CSR_NE_MAX_PKT);
 	writel(tmp, &dev->csr->ne[0]);
 
 	if (use_dma) {
@@ -1938,9 +1938,9 @@ static void activate_control_endpoints(struct udc *dev)
 			AMD_BIT(UDC_DMA_OUT_STS_L);
 		/* write dma desc address */
 		writel(dev->ep[UDC_EP0OUT_IX].td_stp_dma,
-			&dev->ep[UDC_EP0OUT_IX].regs->subptr);
+		       &dev->ep[UDC_EP0OUT_IX].regs->subptr);
 		writel(dev->ep[UDC_EP0OUT_IX].td_phys,
-			&dev->ep[UDC_EP0OUT_IX].regs->desptr);
+		       &dev->ep[UDC_EP0OUT_IX].regs->desptr);
 		/* stop RDE timer */
 		if (timer_pending(&udc_timer)) {
 			set_rde = 0;
@@ -1990,7 +1990,7 @@ static int setup_ep0(struct udc *dev)
 
 /* Called by gadget driver to register itself */
 static int amd5536_udc_start(struct usb_gadget *g,
-		struct usb_gadget_driver *driver)
+			     struct usb_gadget_driver *driver)
 {
 	struct udc *dev = to_amd5536_udc(g);
 	u32 tmp;
@@ -2084,7 +2084,7 @@ static void udc_process_cnak_queue(struct udc *dev)
 		writel(reg, &dev->ep[UDC_EP0OUT_IX].regs->ctl);
 		dev->ep[UDC_EP0OUT_IX].naking = 0;
 		UDC_QUEUE_CNAK(&dev->ep[UDC_EP0OUT_IX],
-				dev->ep[UDC_EP0OUT_IX].num);
+			       dev->ep[UDC_EP0OUT_IX].num);
 	}
 }
 
@@ -2133,7 +2133,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 		/* BNA event ? */
 		if (tmp & AMD_BIT(UDC_EPSTS_BNA)) {
 			DBG(dev, "BNA ep%dout occurred - DESPTR = %x\n",
-					ep->num, readl(&ep->regs->desptr));
+			    ep->num, readl(&ep->regs->desptr));
 			/* clear BNA */
 			writel(tmp | AMD_BIT(UDC_EPSTS_BNA), &ep->regs->sts);
 			if (!ep->cancel_transfer)
@@ -2158,7 +2158,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 
 		/* next request */
 		req = list_entry(ep->queue.next,
-			struct udc_request, queue);
+				 struct udc_request, queue);
 	} else {
 		req = NULL;
 		udc_rxfifo_pending = 1;
@@ -2176,7 +2176,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 			/* next request */
 			if (!list_empty(&ep->queue) && !ep->halted) {
 				req = list_entry(ep->queue.next,
-					struct udc_request, queue);
+						 struct udc_request, queue);
 			} else
 				req = NULL;
 		}
@@ -2188,7 +2188,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 		/* check for DMA done */
 		if (!use_dma_ppb) {
 			dma_done = AMD_GETBITS(req->td_data->status,
-						UDC_DMA_OUT_STS_BS);
+					       UDC_DMA_OUT_STS_BS);
 		/* packet per buffer mode - rx bytes */
 		} else {
 			/*
@@ -2198,7 +2198,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 			if (ep->bna_occurred) {
 				VDBG(dev, "Recover desc. from BNA dummy\n");
 				memcpy(req->td_data, ep->bna_dummy_req->td_data,
-						sizeof(struct udc_data_dma));
+				       sizeof(struct udc_data_dma));
 				ep->bna_occurred = 0;
 				udc_init_bna_dummy(ep->req);
 			}
@@ -2210,7 +2210,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 			if (!use_dma_ppb) {
 				/* received number bytes */
 				count = AMD_GETBITS(req->td_data->status,
-						UDC_DMA_OUT_STS_RXBYTES);
+						    UDC_DMA_OUT_STS_RXBYTES);
 				VDBG(dev, "rx bytes=%u\n", count);
 			/* packet per buffer mode - rx bytes */
 			} else {
@@ -2223,7 +2223,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 				} else {
 					/* last desc. counts bytes */
 					count = AMD_GETBITS(td->status,
-						UDC_DMA_OUT_STS_RXBYTES);
+							    UDC_DMA_OUT_STS_RXBYTES);
 					if (!count && req->req.length
 						== UDC_DMA_MAXPACKET) {
 						/*
@@ -2240,7 +2240,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 			if (count > tmp) {
 				if ((tmp % ep->ep.maxpacket) != 0) {
 					DBG(dev, "%s: rx %db, space=%db\n",
-						ep->ep.name, count, tmp);
+					    ep->ep.name, count, tmp);
 					req->req.status = -EOVERFLOW;
 				}
 				count = tmp;
@@ -2253,7 +2253,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 			/* next request */
 			if (!list_empty(&ep->queue) && !ep->halted) {
 				req = list_entry(ep->queue.next,
-					struct udc_request,
+						 struct udc_request,
 					queue);
 				/*
 				 * DMA may be already started by udc_queue()
@@ -2267,7 +2267,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 						goto finished;
 					/* write desc pointer */
 					writel(req->td_phys,
-						&ep->regs->desptr);
+					       &ep->regs->desptr);
 					req->dma_going = 1;
 					/* enable DMA */
 					udc_set_rde(dev);
@@ -2280,7 +2280,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 				if (ep->bna_dummy_req) {
 					/* write desc pointer */
 					writel(ep->bna_dummy_req->td_phys,
-						&ep->regs->desptr);
+					       &ep->regs->desptr);
 					ep->bna_occurred = 0;
 				}
 
@@ -2375,7 +2375,7 @@ static irqreturn_t udc_data_in_isr(struct udc *dev, int ep_ix)
 		ret_val = IRQ_HANDLED;
 		if (!ep->cancel_transfer && !list_empty(&ep->queue)) {
 			req = list_entry(ep->queue.next,
-					struct udc_request, queue);
+					 struct udc_request, queue);
 			/*
 			 * length bytes transferred
 			 * check dma done of last desc. in PPBDU mode
@@ -2385,7 +2385,7 @@ static irqreturn_t udc_data_in_isr(struct udc *dev, int ep_ix)
 				if (td) {
 					dma_done =
 						AMD_GETBITS(td->status,
-						UDC_DMA_IN_STS_BS);
+							    UDC_DMA_IN_STS_BS);
 					/* don't care DMA done */
 					req->req.actual = req->req.length;
 				}
@@ -2420,7 +2420,7 @@ static irqreturn_t udc_data_in_isr(struct udc *dev, int ep_ix)
 		if (!list_empty(&ep->queue)) {
 			/* next request */
 			req = list_entry(ep->queue.next,
-					struct udc_request, queue);
+					 struct udc_request, queue);
 			/* FIFO mode */
 			if (!use_dma) {
 				/* write fifo */
@@ -2437,7 +2437,7 @@ static irqreturn_t udc_data_in_isr(struct udc *dev, int ep_ix)
 			/* DMA */
 			} else if (req && !req->dma_going) {
 				VDBG(dev, "IN DMA : req=%p req->td_data=%p\n",
-					req, req->td_data);
+				     req, req->td_data);
 				if (req->td_data) {
 
 					req->dma_going = 1;
@@ -2476,7 +2476,7 @@ static irqreturn_t udc_data_in_isr(struct udc *dev, int ep_ix)
 				&dev->regs->ep_irqmsk);
 			tmp |= AMD_BIT(ep->num);
 			writel(tmp,
-				&dev->regs->ep_irqmsk);
+			       &dev->regs->ep_irqmsk);
 		}
 	}
 	/* clear status bits */
@@ -2510,7 +2510,7 @@ __acquires(dev->lock)
 	if (tmp & AMD_BIT(UDC_EPSTS_BNA)) {
 		VDBG(dev, "ep0: BNA set\n");
 		writel(AMD_BIT(UDC_EPSTS_BNA),
-			&dev->ep[UDC_EP0OUT_IX].regs->sts);
+		       &dev->ep[UDC_EP0OUT_IX].regs->sts);
 		ep->bna_occurred = 1;
 		ret_val = IRQ_HANDLED;
 		goto finished;
@@ -2537,7 +2537,7 @@ __acquires(dev->lock)
 
 			/* clear OUT bits in ep status */
 			writel(UDC_EPSTS_OUT_CLEAR,
-				&dev->ep[UDC_EP0OUT_IX].regs->sts);
+			       &dev->ep[UDC_EP0OUT_IX].regs->sts);
 
 			setup_data.data[0] =
 				dev->ep[UDC_EP0OUT_IX].td_stp->data12;
@@ -2566,7 +2566,7 @@ __acquires(dev->lock)
 			if (ep->bna_dummy_req) {
 				/* write desc pointer */
 				writel(ep->bna_dummy_req->td_phys,
-					&dev->ep[UDC_EP0OUT_IX].regs->desptr);
+				       &dev->ep[UDC_EP0OUT_IX].regs->desptr);
 				ep->bna_occurred = 0;
 			}
 
@@ -2639,7 +2639,7 @@ __acquires(dev->lock)
 		if (!use_dma) {
 			/* clear OUT bits in ep status */
 			writel(UDC_EPSTS_OUT_CLEAR,
-				&dev->ep[UDC_EP0OUT_IX].regs->sts);
+			       &dev->ep[UDC_EP0OUT_IX].regs->sts);
 		}
 
 	/* data packet 0 bytes */
@@ -2668,7 +2668,7 @@ __acquires(dev->lock)
 				ret_val |= udc_data_out_isr(dev, UDC_EP0OUT_IX);
 				/* re-program desc. pointer for possible ZLPs */
 				writel(dev->ep[UDC_EP0OUT_IX].td_phys,
-					&dev->ep[UDC_EP0OUT_IX].regs->desptr);
+				       &dev->ep[UDC_EP0OUT_IX].regs->desptr);
 				/* enable RDE */
 				udc_ep0_set_rde(dev);
 			}
@@ -2724,7 +2724,7 @@ static irqreturn_t udc_control_in_isr(struct udc *dev)
 
 		/* clear TDC bit */
 		writel(AMD_BIT(UDC_EPSTS_TDC),
-				&dev->ep[UDC_EP0IN_IX].regs->sts);
+		       &dev->ep[UDC_EP0IN_IX].regs->sts);
 
 	/* status reg has IN bit set ? */
 	} else if (tmp & AMD_BIT(UDC_EPSTS_IN)) {
@@ -2733,7 +2733,7 @@ static irqreturn_t udc_control_in_isr(struct udc *dev)
 		if (ep->dma) {
 			/* clear IN bit */
 			writel(AMD_BIT(UDC_EPSTS_IN),
-				&dev->ep[UDC_EP0IN_IX].regs->sts);
+			       &dev->ep[UDC_EP0IN_IX].regs->sts);
 		}
 		if (dev->stall_ep0in) {
 			DBG(dev, "stall ep0in\n");
@@ -2745,7 +2745,7 @@ static irqreturn_t udc_control_in_isr(struct udc *dev)
 			if (!list_empty(&ep->queue)) {
 				/* next request */
 				req = list_entry(ep->queue.next,
-						struct udc_request, queue);
+						 struct udc_request, queue);
 
 				if (ep->dma) {
 					/* write desc pointer */
@@ -2762,7 +2762,7 @@ static irqreturn_t udc_control_in_isr(struct udc *dev)
 					readl(&dev->ep[UDC_EP0IN_IX].regs->ctl);
 					tmp |= AMD_BIT(UDC_EPCTL_P);
 					writel(tmp,
-					&dev->ep[UDC_EP0IN_IX].regs->ctl);
+					       &dev->ep[UDC_EP0IN_IX].regs->ctl);
 
 					/* all bytes will be transferred */
 					req->req.actual = req->req.length;
@@ -2794,7 +2794,7 @@ static irqreturn_t udc_control_in_isr(struct udc *dev)
 		if (!ep->dma) {
 			/* clear IN bit */
 			writel(AMD_BIT(UDC_EPSTS_IN),
-				&dev->ep[UDC_EP0IN_IX].regs->sts);
+			       &dev->ep[UDC_EP0IN_IX].regs->sts);
 		}
 	}
 
@@ -2846,7 +2846,7 @@ __acquires(dev->lock)
 			tmp = readl(&dev->csr->ne[udc_csr_epix]);
 			/* ep cfg */
 			tmp = AMD_ADDBITS(tmp, ep->dev->cur_config,
-						UDC_CSR_NE_CFG);
+					  UDC_CSR_NE_CFG);
 			/* write reg */
 			writel(tmp, &dev->csr->ne[udc_csr_epix]);
 
@@ -2879,7 +2879,7 @@ __acquires(dev->lock)
 		setup_data.request.wIndex = cpu_to_le16(dev->cur_intf);
 
 		DBG(dev, "SET_INTERFACE interrupt: alt=%d intf=%d\n",
-				dev->cur_alt, dev->cur_intf);
+		    dev->cur_alt, dev->cur_intf);
 
 		/* programm the NE registers */
 		for (i = 0; i < UDC_EP_NUM; i++) {
@@ -2900,11 +2900,11 @@ __acquires(dev->lock)
 			tmp = readl(&dev->csr->ne[udc_csr_epix]);
 			/* ep interface */
 			tmp = AMD_ADDBITS(tmp, ep->dev->cur_intf,
-						UDC_CSR_NE_INTF);
+					  UDC_CSR_NE_INTF);
 			/* tmp = AMD_ADDBITS(tmp, 2, UDC_CSR_NE_INTF); */
 			/* ep alt */
 			tmp = AMD_ADDBITS(tmp, ep->dev->cur_alt,
-						UDC_CSR_NE_ALT);
+					  UDC_CSR_NE_ALT);
 			/* write reg */
 			writel(tmp, &dev->csr->ne[udc_csr_epix]);
 
@@ -3137,7 +3137,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
 	free_irq(pdev->irq, dev);
 	iounmap(dev->virt_addr);
 	release_mem_region(pci_resource_start(pdev, 0),
-				pci_resource_len(pdev, 0));
+			   pci_resource_len(pdev, 0));
 	pci_disable_device(pdev);
 
 	udc_remove(dev);
@@ -3188,7 +3188,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);
+				 &dev->ep[UDC_EP0OUT_IX].td_phys);
 	if (!td_data) {
 		retval = -ENOMEM;
 		goto err_alloc_phys;
@@ -3233,7 +3233,7 @@ static int udc_probe(struct udc *dev)
 
 	snprintf(tmp, sizeof tmp, "%d", dev->irq);
 	dev_info(&dev->pdev->dev,
-		"irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
+		 "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);
@@ -3243,11 +3243,11 @@ static int udc_probe(struct udc *dev)
 		goto finished;
 	}
 	dev_info(&dev->pdev->dev,
-		"driver version: %s(for Geode5536 B1)\n", tmp);
+		 "driver version: %s(for Geode5536 B1)\n", tmp);
 	udc = dev;
 
 	retval = usb_add_gadget_udc_release(&udc->pdev->dev, &dev->gadget,
-			gadget_release);
+					    gadget_release);
 	if (retval)
 		goto finished;
 
-- 
1.9.1


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

* Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
  2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
                   ` (15 preceding siblings ...)
  2015-09-14 15:13 ` [PATCH 16/16] usb: gadget: amd5536udc: match alignment Sudip Mukherjee
@ 2015-09-18 18:39 ` Felipe Balbi
  2015-09-19  3:54   ` Sudip Mukherjee
  16 siblings, 1 reply; 24+ messages in thread
From: Felipe Balbi @ 2015-09-18 18:39 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Thomas Dahlmann, Felipe Balbi, Greg Kroah-Hartman, linux-kernel,
	linux-geode, linux-usb

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

On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> This amd5536udc was a complete mess. The major problems that i could
> find are:
> 
> 1) if udc_pci_probe() fails in any stage then it just calls the
> udc_pci_remove() to handle error. And udc_pci_remove() works with
> struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
> pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
> incase of error we are handling the error by dereferencing a NULL
> pointer.
> 
> 2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
> will be set only if probe is success. So that means if probe fails then
> probe will call udc_pci_remove() for error handling and udc_pci_remove()
> will inturn halts the kernel by calling BUG().
> 
> And apart from these numerous memory leaks and not releasing of
> resources. Here comes a rewrite of few of the functions in an
> attempt to fix these.

run checkpatch.pl and try again

-- 
balbi

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

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

* Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
  2015-09-18 18:39 ` [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Felipe Balbi
@ 2015-09-19  3:54   ` Sudip Mukherjee
  2015-09-20  8:12     ` Sudip Mukherjee
  2015-09-20 16:16     ` Felipe Balbi
  0 siblings, 2 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-19  3:54 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Thomas Dahlmann, Greg Kroah-Hartman, linux-kernel, linux-geode,
	linux-usb

On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > This amd5536udc was a complete mess. The major problems that i could
> > find are:
> > 
> > 1) if udc_pci_probe() fails in any stage then it just calls the
> > udc_pci_remove() to handle error. And udc_pci_remove() works with
> > struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
> > pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
> > incase of error we are handling the error by dereferencing a NULL
> > pointer.
> > 
> > 2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
> > will be set only if probe is success. So that means if probe fails then
> > probe will call udc_pci_remove() for error handling and udc_pci_remove()
> > will inturn halts the kernel by calling BUG().
> > 
> > And apart from these numerous memory leaks and not releasing of
> > resources. Here comes a rewrite of few of the functions in an
> > attempt to fix these.
> 
> run checkpatch.pl and try again
I know checkpatch gives warning on some of my patches but as the warning
was not related to the part I have modified so I have not done any thing
with them as they will become unrelated changes than what is mentioned
in the commit log.
Anyways, I will fix up all the warnings and send v2. But do you want me
to also fix the checkpatch warnings in those patch where functions are
rearranged? Because in those patches functions were just moved and there
was no change in the body of the function.

regards
sudip

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

* Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
  2015-09-19  3:54   ` Sudip Mukherjee
@ 2015-09-20  8:12     ` Sudip Mukherjee
  2015-09-20 16:17       ` Felipe Balbi
  2015-09-20 16:16     ` Felipe Balbi
  1 sibling, 1 reply; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-20  8:12 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Thomas Dahlmann, Greg Kroah-Hartman, linux-kernel, linux-geode,
	linux-usb

On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > This amd5536udc was a complete mess. The major problems that i could
> > > find are:
> > > 
<snip>
> > 
> > run checkpatch.pl and try again
<snip>
> Anyways, I will fix up all the warnings and send v2.

I guess v2 is not required any more. The main thing that this series was
trying to do has already been done by:
6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")

regards
sudip

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

* Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
  2015-09-19  3:54   ` Sudip Mukherjee
  2015-09-20  8:12     ` Sudip Mukherjee
@ 2015-09-20 16:16     ` Felipe Balbi
  1 sibling, 0 replies; 24+ messages in thread
From: Felipe Balbi @ 2015-09-20 16:16 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Felipe Balbi, Thomas Dahlmann, Greg Kroah-Hartman, linux-kernel,
	linux-geode, linux-usb

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

Hi,

On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > This amd5536udc was a complete mess. The major problems that i could
> > > find are:
> > > 
> > > 1) if udc_pci_probe() fails in any stage then it just calls the
> > > udc_pci_remove() to handle error. And udc_pci_remove() works with
> > > struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
> > > pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
> > > incase of error we are handling the error by dereferencing a NULL
> > > pointer.
> > > 
> > > 2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
> > > will be set only if probe is success. So that means if probe fails then
> > > probe will call udc_pci_remove() for error handling and udc_pci_remove()
> > > will inturn halts the kernel by calling BUG().
> > > 
> > > And apart from these numerous memory leaks and not releasing of
> > > resources. Here comes a rewrite of few of the functions in an
> > > attempt to fix these.
> > 
> > run checkpatch.pl and try again
> I know checkpatch gives warning on some of my patches but as the warning
> was not related to the part I have modified so I have not done any thing
> with them as they will become unrelated changes than what is mentioned
> in the commit log.
> Anyways, I will fix up all the warnings and send v2. But do you want me
> to also fix the checkpatch warnings in those patch where functions are
> rearranged? Because in those patches functions were just moved and there
> was no change in the body of the function.

sure, just add a note "while at that, also fix checkpatch warnings"

-- 
balbi

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

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

* Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
  2015-09-20  8:12     ` Sudip Mukherjee
@ 2015-09-20 16:17       ` Felipe Balbi
  2015-09-21 12:48         ` Sudip Mukherjee
  0 siblings, 1 reply; 24+ messages in thread
From: Felipe Balbi @ 2015-09-20 16:17 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Felipe Balbi, Thomas Dahlmann, Greg Kroah-Hartman, linux-kernel,
	linux-geode, linux-usb

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

On Sun, Sep 20, 2015 at 01:42:42PM +0530, Sudip Mukherjee wrote:
> On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> > On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > > This amd5536udc was a complete mess. The major problems that i could
> > > > find are:
> > > > 
> <snip>
> > > 
> > > run checkpatch.pl and try again
> <snip>
> > Anyways, I will fix up all the warnings and send v2.
> 
> I guess v2 is not required any more. The main thing that this series was
> trying to do has already been done by:
> 6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")

all right, see if there's anything missing, please.

-- 
balbi

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

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

* Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
  2015-09-20 16:17       ` Felipe Balbi
@ 2015-09-21 12:48         ` Sudip Mukherjee
  2015-09-21 14:42           ` Felipe Balbi
  0 siblings, 1 reply; 24+ messages in thread
From: Sudip Mukherjee @ 2015-09-21 12:48 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Thomas Dahlmann, Greg Kroah-Hartman, linux-kernel, linux-geode,
	linux-usb

On Sun, Sep 20, 2015 at 11:17:36AM -0500, Felipe Balbi wrote:
> On Sun, Sep 20, 2015 at 01:42:42PM +0530, Sudip Mukherjee wrote:
> > On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> > > On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > > > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > > > This amd5536udc was a complete mess. The major problems that i could
> > > > > find are:
> > > > > 
> > <snip>
> > > > 
> > > > run checkpatch.pl and try again
> > <snip>
> > > Anyways, I will fix up all the warnings and send v2.
> > 
> > I guess v2 is not required any more. The main thing that this series was
> > trying to do has already been done by:
> > 6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")
> 
> all right, see if there's anything missing, please.
I think something still needs to be done there. I will send a v2 for
your review.

regards
sudip

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

* Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks
  2015-09-21 12:48         ` Sudip Mukherjee
@ 2015-09-21 14:42           ` Felipe Balbi
  0 siblings, 0 replies; 24+ messages in thread
From: Felipe Balbi @ 2015-09-21 14:42 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Felipe Balbi, Thomas Dahlmann, Greg Kroah-Hartman, linux-kernel,
	linux-geode, linux-usb

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

On Mon, Sep 21, 2015 at 06:18:04PM +0530, Sudip Mukherjee wrote:
> On Sun, Sep 20, 2015 at 11:17:36AM -0500, Felipe Balbi wrote:
> > On Sun, Sep 20, 2015 at 01:42:42PM +0530, Sudip Mukherjee wrote:
> > > On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> > > > On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > > > > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > > > > This amd5536udc was a complete mess. The major problems that i could
> > > > > > find are:
> > > > > > 
> > > <snip>
> > > > > 
> > > > > run checkpatch.pl and try again
> > > <snip>
> > > > Anyways, I will fix up all the warnings and send v2.
> > > 
> > > I guess v2 is not required any more. The main thing that this series was
> > > trying to do has already been done by:
> > > 6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")
> > 
> > all right, see if there's anything missing, please.
> I think something still needs to be done there. I will send a v2 for
> your review.

cool, thanks

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

end of thread, other threads:[~2015-09-21 14:42 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 15:12 [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 01/16] usb: gadget: amd5536udc: introduce free_dma_pools Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 02/16] usb: gadget: amd5536udc: rewrite init_dma_pools Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 03/16] usb: gadget: amd5536udc: rewrite udc_pci_probe Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 04/16] usb: gadget: amd5536udc: use WARN_ON Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 05/16] usb: gadget: amd5536udc: use free_dma_pools Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 06/16] usb: gadget: amd5536udc: remove unnecessary conditions Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 07/16] usb: gadget: amd5536udc: unmap virt_addr Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 08/16] usb: gadget: amd5536udc: remove forward declaration of udc_probe Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 09/16] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 10/16] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 11/16] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain Sudip Mukherjee
2015-09-14 15:12 ` [PATCH 12/16] usb: gadget: amd5536udc: remove forward declaration of udc_pci_* Sudip Mukherjee
2015-09-14 15:13 ` [PATCH 13/16] usb: gadget: amd5536udc: remove forward declaration of udc_basic_init Sudip Mukherjee
2015-09-14 15:13 ` [PATCH 14/16] usb: gadget: amd5536udc: NULL comparison Sudip Mukherjee
2015-09-14 15:13 ` [PATCH 15/16] usb: gadget: amd5536udc: remove multiple blank lines Sudip Mukherjee
2015-09-14 15:13 ` [PATCH 16/16] usb: gadget: amd5536udc: match alignment Sudip Mukherjee
2015-09-18 18:39 ` [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Felipe Balbi
2015-09-19  3:54   ` Sudip Mukherjee
2015-09-20  8:12     ` Sudip Mukherjee
2015-09-20 16:17       ` Felipe Balbi
2015-09-21 12:48         ` Sudip Mukherjee
2015-09-21 14:42           ` Felipe Balbi
2015-09-20 16:16     ` Felipe Balbi

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).