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

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

[Sudip Mukherjee]

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

[PATCH v2 01/12] usb: gadget: amd5536udc: rewrite init_dma_pools

[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 175ca93..38a6858 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3169,8 +3169,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 */
@@ -3182,14 +3181,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;
 
@@ -3198,12 +3197,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

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

[Sudip Mukherjee]

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

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

[Sudip Mukherjee]

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

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

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

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

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

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

[Sudip Mukherjee]

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

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

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

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

[Sudip Mukherjee]

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

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

[Sudip Mukherjee]

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

[PATCH v2 10/12] usb: gadget: amd5536udc: remove forward declaration of udc_pci_*

[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 6d64129..00ae069 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

[PATCH v2 11/12] usb: gadget: amd5536udc: remove forward declaration of udc_basic_init

[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 00ae069..157bff1 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);
@@ -1507,33 +1506,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)
 {
@@ -1572,6 +1544,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

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

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

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

[Felipe Balbi]

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

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

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

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

[Sudip Mukherjee]

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

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

[Sudip Mukherjee]

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

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

[Felipe Balbi]

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

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

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

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

[Sudip Mukherjee]

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

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

[Sudip Mukherjee]

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