[PATCH 0/6] usb: gadget: omap_udc: cleanup patches

[PATCH 0/6] usb: gadget: omap_udc: cleanup patches

[Felipe Balbi]

Here are a few cleanups for the omap_udc.c driver.

Compile tested only with omap1_defconfig + enabling omap_udc.

There's still a bunch that needs to change on that driver,
but this is a start, at least.

Once -rc1 is tagged, I'll queue these patches for v3.6 merge
window together with Tony's removal of OMAP2 support from
omap_udc [1]

[1] http://marc.info/?l=linux-usb&m=133762704531095&w=2

Felipe Balbi (6):
  usb: gadget: omap_udc: make checkpatch.pl happy
  usb: gadget: omap_udc: remove useless print
  usb: gadget: omap_udc: let it work as a module
  usb: gadget: omap_udc: remove possiblity of NULL pointer de-reference
  usb: gadget: omap_udc: kfree(NULL) is safe
  usb: gadget: omap_udc: use generic map/unmap routines

 drivers/usb/gadget/omap_udc.c |  290 ++++++++++++++++++++---------------------
 1 file changed, 142 insertions(+), 148 deletions(-)

-- 
1.7.10.2

[PATCH 1/6] usb: gadget: omap_udc: make checkpatch.pl happy

[Felipe Balbi]

This patch is just a cleanup patch to make
checkpatch.pl a little happier with the
omap_udc.c driver.

No functional changes.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/omap_udc.c |  201 +++++++++++++++++++++++++----------------
 1 file changed, 121 insertions(+), 80 deletions(-)

diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
index 24d81dc..aa2d174 100644
--- a/drivers/usb/gadget/omap_udc.c
+++ b/drivers/usb/gadget/omap_udc.c
@@ -36,9 +36,9 @@
 #include <linux/dma-mapping.h>
 #include <linux/clk.h>
 #include <linux/prefetch.h>
+#include <linux/io.h>
 
 #include <asm/byteorder.h>
-#include <asm/io.h>
 #include <asm/irq.h>
 #include <asm/unaligned.h>
 #include <asm/mach-types.h>
@@ -84,14 +84,14 @@
 #ifdef	USE_ISO
 static unsigned fifo_mode = 3;
 #else
-static unsigned fifo_mode = 0;
+static unsigned fifo_mode;
 #endif
 
 /* "modprobe omap_udc fifo_mode=42", or else as a kernel
  * boot parameter "omap_udc:fifo_mode=42"
  */
-module_param (fifo_mode, uint, 0);
-MODULE_PARM_DESC (fifo_mode, "endpoint configuration");
+module_param(fifo_mode, uint, 0);
+MODULE_PARM_DESC(fifo_mode, "endpoint configuration");
 
 #ifdef	USE_DMA
 static bool use_dma = 1;
@@ -99,8 +99,8 @@ static bool use_dma = 1;
 /* "modprobe omap_udc use_dma=y", or else as a kernel
  * boot parameter "omap_udc:use_dma=y"
  */
-module_param (use_dma, bool, 0);
-MODULE_PARM_DESC (use_dma, "enable/disable DMA");
+module_param(use_dma, bool, 0);
+MODULE_PARM_DESC(use_dma, "enable/disable DMA");
 #else	/* !USE_DMA */
 
 /* save a bit of code */
@@ -108,8 +108,8 @@ MODULE_PARM_DESC (use_dma, "enable/disable DMA");
 #endif	/* !USE_DMA */
 
 
-static const char driver_name [] = "omap_udc";
-static const char driver_desc [] = DRIVER_DESC;
+static const char driver_name[] = "omap_udc";
+static const char driver_desc[] = DRIVER_DESC;
 
 /*-------------------------------------------------------------------------*/
 
@@ -247,7 +247,7 @@ static int omap_ep_disable(struct usb_ep *_ep)
 
 	spin_lock_irqsave(&ep->udc->lock, flags);
 	ep->ep.desc = NULL;
-	nuke (ep, -ESHUTDOWN);
+	nuke(ep, -ESHUTDOWN);
 	ep->ep.maxpacket = ep->maxpacket;
 	ep->has_dma = 0;
 	omap_writew(UDC_SET_HALT, UDC_CTRL);
@@ -270,7 +270,7 @@ omap_alloc_request(struct usb_ep *ep, gfp_t gfp_flags)
 	req = kzalloc(sizeof(*req), gfp_flags);
 	if (req) {
 		req->req.dma = DMA_ADDR_INVALID;
-		INIT_LIST_HEAD (&req->queue);
+		INIT_LIST_HEAD(&req->queue);
 	}
 	return &req->req;
 }
@@ -281,7 +281,7 @@ omap_free_request(struct usb_ep *ep, struct usb_request *_req)
 	struct omap_req	*req = container_of(_req, struct omap_req, req);
 
 	if (_req)
-		kfree (req);
+		kfree(req);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -361,10 +361,10 @@ write_packet(u8 *buf, struct omap_req *req, unsigned max)
 	return len;
 }
 
-// FIXME change r/w fifo calling convention
+/* FIXME change r/w fifo calling convention */
 
 
-// return:  0 = still running, 1 = completed, negative = errno
+/* return:  0 = still running, 1 = completed, negative = errno */
 static int write_fifo(struct omap_ep *ep, struct omap_req *req)
 {
 	u8		*buf;
@@ -426,7 +426,7 @@ read_packet(u8 *buf, struct omap_req *req, unsigned avail)
 	return len;
 }
 
-// return:  0 = still running, 1 = queue empty, negative = errno
+/* return:  0 = still running, 1 = queue empty, negative = errno */
 static int read_fifo(struct omap_ep *ep, struct omap_req *req)
 {
 	u8		*buf;
@@ -662,7 +662,7 @@ static void dma_irq(struct omap_udc *udc, u16 irq_src)
 		}
 		omap_writew(UDC_TXN_DONE, UDC_IRQ_SRC);
 
-		if (!list_empty (&ep->queue)) {
+		if (!list_empty(&ep->queue)) {
 			req = container_of(ep->queue.next,
 					struct omap_req, queue);
 			next_in_dma(ep, req);
@@ -681,7 +681,7 @@ static void dma_irq(struct omap_udc *udc, u16 irq_src)
 		}
 		omap_writew(UDC_RXN_EOT, UDC_IRQ_SRC);
 
-		if (!list_empty (&ep->queue)) {
+		if (!list_empty(&ep->queue)) {
 			req = container_of(ep->queue.next,
 					struct omap_req, queue);
 			next_out_dma(ep, req);
@@ -954,7 +954,7 @@ omap_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
 		int	is_in;
 
 		if (ep->bEndpointAddress == 0) {
-			if (!udc->ep0_pending || !list_empty (&ep->queue)) {
+			if (!udc->ep0_pending || !list_empty(&ep->queue)) {
 				spin_unlock_irqrestore(&udc->lock, flags);
 				return -EL2HLT;
 			}
@@ -981,7 +981,8 @@ omap_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
 				 * always an IN ... even for IN transfers,
 				 * a weird case which seem to stall OMAP.
 				 */
-				omap_writew(UDC_EP_SEL | UDC_EP_DIR, UDC_EP_NUM);
+				omap_writew(UDC_EP_SEL | UDC_EP_DIR,
+						UDC_EP_NUM);
 				omap_writew(UDC_CLR_EP, UDC_CTRL);
 				omap_writew(UDC_SET_FIFO_EN, UDC_CTRL);
 				omap_writew(UDC_EP_DIR, UDC_EP_NUM);
@@ -993,7 +994,8 @@ omap_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
 
 			/* non-empty DATA stage */
 			} else if (is_in) {
-				omap_writew(UDC_EP_SEL | UDC_EP_DIR, UDC_EP_NUM);
+				omap_writew(UDC_EP_SEL | UDC_EP_DIR,
+						UDC_EP_NUM);
 			} else {
 				if (udc->ep0_setup)
 					goto irq_wait;
@@ -1041,7 +1043,7 @@ static int omap_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 	spin_lock_irqsave(&ep->udc->lock, flags);
 
 	/* make sure it's actually queued on this endpoint */
-	list_for_each_entry (req, &ep->queue, queue) {
+	list_for_each_entry(req, &ep->queue, queue) {
 		if (&req->req == _req)
 			break;
 	}
@@ -1148,8 +1150,8 @@ static struct usb_ep_ops omap_ep_ops = {
 	.dequeue	= omap_ep_dequeue,
 
 	.set_halt	= omap_ep_set_halt,
-	// fifo_status ... report bytes in fifo
-	// fifo_flush ... flush fifo
+	/* fifo_status ... report bytes in fifo */
+	/* fifo_flush ... flush fifo */
 };
 
 /*-------------------------------------------------------------------------*/
@@ -1379,7 +1381,7 @@ static void udc_quiesce(struct omap_udc *udc)
 
 	udc->gadget.speed = USB_SPEED_UNKNOWN;
 	nuke(&udc->ep[0], -ESHUTDOWN);
-	list_for_each_entry (ep, &udc->gadget.ep_list, ep.ep_list)
+	list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list)
 		nuke(ep, -ESHUTDOWN);
 }
 
@@ -1495,7 +1497,8 @@ static void ep0_irq(struct omap_udc *udc, u16 irq_src)
 				/* read next OUT packet of request, maybe
 				 * reactiviting the fifo; stall on errors.
 				 */
-				if (!req || (stat = read_fifo(ep0, req)) < 0) {
+				stat = read_fifo(ep0, req);
+				if (!req || stat < 0) {
 					omap_writew(UDC_STALL_CMD, UDC_SYSCON2);
 					udc->ep0_pending = 0;
 					stat = 0;
@@ -1628,7 +1631,7 @@ static void ep0_irq(struct omap_udc *udc, u16 irq_src)
 				/* this has rude side-effects (aborts) and
 				 * can't really work if DMA-IN is active
 				 */
-				DBG("%s host set_halt, NYET \n", ep->name);
+				DBG("%s host set_halt, NYET\n", ep->name);
 				goto do_stall;
 			}
 			use_ep(ep, 0);
@@ -1719,7 +1722,7 @@ delegate:
 			 */
 			udc->ep0_setup = 1;
 			spin_unlock(&udc->lock);
-			status = udc->driver->setup (&udc->gadget, &u.r);
+			status = udc->driver->setup(&udc->gadget, &u.r);
 			spin_lock(&udc->lock);
 			udc->ep0_setup = 0;
 		}
@@ -1764,7 +1767,7 @@ static void devstate_irq(struct omap_udc *udc, u16 irq_src)
 				VDBG("connect\n");
 				if (!udc->transceiver)
 					pullup_enable(udc);
-				// if (driver->connect) call it
+				/* if (driver->connect) call it */
 			} else if (udc->gadget.speed != USB_SPEED_UNKNOWN) {
 				udc->gadget.speed = USB_SPEED_UNKNOWN;
 				if (!udc->transceiver)
@@ -1796,7 +1799,7 @@ static void devstate_irq(struct omap_udc *udc, u16 irq_src)
 	}
 	if (change & UDC_SUS) {
 		if (udc->gadget.speed != USB_SPEED_UNKNOWN) {
-			// FIXME tell isp1301 to suspend/resume (?)
+			/* FIXME tell isp1301 to suspend/resume (?) */
 			if (devstat & UDC_SUS) {
 				VDBG("suspend\n");
 				update_otg(udc);
@@ -1999,7 +2002,7 @@ static irqreturn_t omap_udc_iso_irq(int irq, void *_dev)
 	spin_lock_irqsave(&udc->lock, flags);
 
 	/* handle all non-DMA ISO transfers */
-	list_for_each_entry (ep, &udc->iso, iso) {
+	list_for_each_entry(ep, &udc->iso, iso) {
 		u16		stat;
 		struct omap_req	*req;
 
@@ -2058,11 +2061,11 @@ static irqreturn_t omap_udc_iso_irq(int irq, void *_dev)
 
 static inline int machine_without_vbus_sense(void)
 {
-	return (machine_is_omap_innovator()
+	return machine_is_omap_innovator()
 		|| machine_is_omap_osk()
 		|| machine_is_sx1()
-		|| cpu_is_omap7xx() /* No known omap7xx boards with vbus sense */
-		);
+		/* No known omap7xx boards with vbus sense */
+		|| cpu_is_omap7xx();
 }
 
 static int omap_udc_start(struct usb_gadget_driver *driver,
@@ -2076,7 +2079,7 @@ static int omap_udc_start(struct usb_gadget_driver *driver,
 	if (!udc)
 		return -ENODEV;
 	if (!driver
-			// FIXME if otg, check:  driver->is_otg
+			/* FIXME if otg, check:  driver->is_otg */
 			|| driver->max_speed < USB_SPEED_FULL
 			|| !bind || !driver->setup)
 		return -EINVAL;
@@ -2088,7 +2091,7 @@ static int omap_udc_start(struct usb_gadget_driver *driver,
 	}
 
 	/* reset state */
-	list_for_each_entry (ep, &udc->gadget.ep_list, ep.ep_list) {
+	list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list) {
 		ep->irqs = 0;
 		if (ep->bmAttributes == USB_ENDPOINT_XFER_ISOC)
 			continue;
@@ -2126,7 +2129,7 @@ static int omap_udc_start(struct usb_gadget_driver *driver,
 		if (status < 0) {
 			ERR("can't bind to transceiver\n");
 			if (driver->unbind) {
-				driver->unbind (&udc->gadget);
+				driver->unbind(&udc->gadget);
 				udc->gadget.dev.driver = NULL;
 				udc->driver = NULL;
 			}
@@ -2134,9 +2137,9 @@ static int omap_udc_start(struct usb_gadget_driver *driver,
 		}
 	} else {
 		if (can_pullup(udc))
-			pullup_enable (udc);
+			pullup_enable(udc);
 		else
-			pullup_disable (udc);
+			pullup_disable(udc);
 	}
 
 	/* boards that don't have VBUS sensing can't autogate 48MHz;
@@ -2195,7 +2198,7 @@ static int omap_udc_stop(struct usb_gadget_driver *driver)
 static const char proc_filename[] = "driver/udc";
 
 #define FOURBITS "%s%s%s%s"
-#define EIGHTBITS FOURBITS FOURBITS
+#define EIGHTBITS "%s%s%s%s%s%s%s%s"
 
 static void proc_ep_show(struct seq_file *s, struct omap_ep *ep)
 {
@@ -2217,12 +2220,21 @@ static void proc_ep_show(struct seq_file *s, struct omap_ep *ep)
 		"\n%s %s%s%sirqs %ld stat %04x " EIGHTBITS FOURBITS "%s\n",
 		ep->name, buf,
 		ep->double_buf ? "dbuf " : "",
-		({char *s; switch(ep->ackwait){
-		case 0: s = ""; break;
-		case 1: s = "(ackw) "; break;
-		case 2: s = "(ackw2) "; break;
-		default: s = "(?) "; break;
-		} s;}),
+		({ char *s;
+		switch (ep->ackwait) {
+		case 0:
+			s = "";
+			break;
+		case 1:
+			s = "(ackw) ";
+			break;
+		case 2:
+			s = "(ackw2) ";
+			break;
+		default:
+			s = "(?) ";
+			break;
+		} s; }),
 		ep->irqs, stat_flg,
 		(stat_flg & UDC_NO_RXPACKET) ? "no_rxpacket " : "",
 		(stat_flg & UDC_MISS_IN) ? "miss_in " : "",
@@ -2238,10 +2250,10 @@ static void proc_ep_show(struct seq_file *s, struct omap_ep *ep)
 		(stat_flg & UDC_NON_ISO_FIFO_EMPTY) ? "fifo_empty " : "",
 		(stat_flg & UDC_NON_ISO_FIFO_FULL) ? "fifo_full " : "");
 
-	if (list_empty (&ep->queue))
+	if (list_empty(&ep->queue))
 		seq_printf(s, "\t(queue empty)\n");
 	else
-		list_for_each_entry (req, &ep->queue, queue) {
+		list_for_each_entry(req, &ep->queue, queue) {
 			unsigned	length = req->req.actual;
 
 			if (use_dma && buf[0]) {
@@ -2259,11 +2271,16 @@ static void proc_ep_show(struct seq_file *s, struct omap_ep *ep)
 static char *trx_mode(unsigned m, int enabled)
 {
 	switch (m) {
-	case 0:		return enabled ? "*6wire" : "unused";
-	case 1:		return "4wire";
-	case 2:		return "3wire";
-	case 3:		return "6wire";
-	default:	return "unknown";
+	case 0:
+		return enabled ? "*6wire" : "unused";
+	case 1:
+		return "4wire";
+	case 2:
+		return "3wire";
+	case 3:
+		return "6wire";
+	default:
+		return "unknown";
 	}
 }
 
@@ -2295,7 +2312,7 @@ static int proc_otg_show(struct seq_file *s)
 			" b_ase_brst=%d hmc=%d\n", tmp,
 		(tmp & OTG_EN) ? " otg_en" : "",
 		(tmp & USBX_SYNCHRO) ? " synchro" : "",
-		// much more SRP stuff
+		/* much more SRP stuff */
 		(tmp & SRP_DATA) ? " srp_data" : "",
 		(tmp & SRP_VBUS) ? " srp_vbus" : "",
 		(tmp & OTG_PADEN) ? " otg_paden" : "",
@@ -2383,7 +2400,7 @@ static int proc_udc_show(struct seq_file *s, void *_)
 		(tmp & UDC_SELF_PWR) ? " self_pwr" : "",
 		(tmp & UDC_SOFF_DIS) ? " soff_dis" : "",
 		(tmp & UDC_PULLUP_EN) ? " PULLUP" : "");
-	// syscon2 is write-only
+	/* syscon2 is write-only */
 
 	/* UDC controller registers */
 	if (!(tmp & UDC_PULLUP_EN)) {
@@ -2467,7 +2484,7 @@ static int proc_udc_show(struct seq_file *s, void *_)
 	if (tmp & UDC_ATT) {
 		proc_ep_show(s, &udc->ep[0]);
 		if (tmp & UDC_ADD) {
-			list_for_each_entry (ep, &udc->gadget.ep_list,
+			list_for_each_entry(ep, &udc->gadget.ep_list,
 					ep.ep_list) {
 				if (ep->ep.desc)
 					proc_ep_show(s, ep);
@@ -2536,14 +2553,29 @@ omap_ep_setup(char *name, u8 addr, u8 type,
 	/* chip setup ... bit values are same for IN, OUT */
 	if (type == USB_ENDPOINT_XFER_ISOC) {
 		switch (maxp) {
-		case 8:		epn_rxtx = 0 << 12; break;
-		case 16:	epn_rxtx = 1 << 12; break;
-		case 32:	epn_rxtx = 2 << 12; break;
-		case 64:	epn_rxtx = 3 << 12; break;
-		case 128:	epn_rxtx = 4 << 12; break;
-		case 256:	epn_rxtx = 5 << 12; break;
-		case 512:	epn_rxtx = 6 << 12; break;
-		default:	BUG();
+		case 8:
+			epn_rxtx = 0 << 12;
+			break;
+		case 16:
+			epn_rxtx = 1 << 12;
+			break;
+		case 32:
+			epn_rxtx = 2 << 12;
+			break;
+		case 64:
+			epn_rxtx = 3 << 12;
+			break;
+		case 128:
+			epn_rxtx = 4 << 12;
+			break;
+		case 256:
+			epn_rxtx = 5 << 12;
+			break;
+		case 512:
+			epn_rxtx = 6 << 12;
+			break;
+		default:
+			BUG();
 		}
 		epn_rxtx |= UDC_EPN_RX_ISO;
 		dbuf = 1;
@@ -2556,11 +2588,20 @@ omap_ep_setup(char *name, u8 addr, u8 type,
 			dbuf = 0;
 
 		switch (maxp) {
-		case 8:		epn_rxtx = 0 << 12; break;
-		case 16:	epn_rxtx = 1 << 12; break;
-		case 32:	epn_rxtx = 2 << 12; break;
-		case 64:	epn_rxtx = 3 << 12; break;
-		default:	BUG();
+		case 8:
+			epn_rxtx = 0 << 12;
+			break;
+		case 16:
+			epn_rxtx = 1 << 12;
+			break;
+		case 32:
+			epn_rxtx = 2 << 12;
+			break;
+		case 64:
+			epn_rxtx = 3 << 12;
+			break;
+		default:
+			BUG();
 		}
 		if (dbuf && addr)
 			epn_rxtx |= UDC_EPN_RX_DB;
@@ -2600,7 +2641,7 @@ omap_ep_setup(char *name, u8 addr, u8 type,
 	ep->ep.name = ep->name;
 	ep->ep.ops = &omap_ep_ops;
 	ep->ep.maxpacket = ep->maxpacket = maxp;
-	list_add_tail (&ep->ep.ep_list, &udc->gadget.ep_list);
+	list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
 
 	return buf;
 }
@@ -2608,7 +2649,7 @@ omap_ep_setup(char *name, u8 addr, u8 type,
 static void omap_udc_release(struct device *dev)
 {
 	complete(udc->done);
-	kfree (udc);
+	kfree(udc);
 	udc = NULL;
 }
 
@@ -2626,13 +2667,13 @@ omap_udc_setup(struct platform_device *odev, struct usb_phy *xceiv)
 	omap_writew(0, UDC_TXDMA_CFG);
 
 	/* UDC_PULLUP_EN gates the chip clock */
-	// OTG_SYSCON_1 |= DEV_IDLE_EN;
+	/* OTG_SYSCON_1 |= DEV_IDLE_EN; */
 
 	udc = kzalloc(sizeof(*udc), GFP_KERNEL);
 	if (!udc)
 		return -ENOMEM;
 
-	spin_lock_init (&udc->lock);
+	spin_lock_init(&udc->lock);
 
 	udc->gadget.ops = &omap_gadget_ops;
 	udc->gadget.ep0 = &udc->ep[0].ep;
@@ -2662,13 +2703,13 @@ omap_udc_setup(struct platform_device *odev, struct usb_phy *xceiv)
 		omap_writew(0, UDC_EP_TX(tmp));
 	}
 
-#define OMAP_BULK_EP(name,addr) \
+#define OMAP_BULK_EP(name, addr) \
 	buf = omap_ep_setup(name "-bulk", addr, \
 			USB_ENDPOINT_XFER_BULK, buf, 64, 1);
-#define OMAP_INT_EP(name,addr, maxp) \
+#define OMAP_INT_EP(name, addr, maxp) \
 	buf = omap_ep_setup(name "-int", addr, \
 			USB_ENDPOINT_XFER_INT, buf, maxp, 0);
-#define OMAP_ISO_EP(name,addr, maxp) \
+#define OMAP_ISO_EP(name, addr, maxp) \
 	buf = omap_ep_setup(name "-iso", addr, \
 			USB_ENDPOINT_XFER_ISOC, buf, maxp, 1);
 
@@ -2869,11 +2910,11 @@ bad_on_1710:
 
 	/* a "gadget" abstracts/virtualizes the controller */
 	status = omap_udc_setup(pdev, xceiv);
-	if (status) {
+	if (status)
 		goto cleanup0;
-	}
+
 	xceiv = NULL;
-	// "udc" is now valid
+	/* "udc" is now valid */
 	pullup_disable(udc);
 #if	defined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE)
 	udc->gadget.is_otg = (config->otg != 0);
@@ -2887,7 +2928,7 @@ bad_on_1710:
 
 	/* USB general purpose IRQ:  ep0, state changes, dma, etc */
 	status = request_irq(pdev->resource[1].start, omap_udc_irq,
-			IRQF_SAMPLE_RANDOM, driver_name, udc);
+			0, driver_name, udc);
 	if (status != 0) {
 		ERR("can't get irq %d, err %d\n",
 			(int) pdev->resource[1].start, status);
@@ -2896,7 +2937,7 @@ bad_on_1710:
 
 	/* USB "non-iso" IRQ (PIO for all but ep0) */
 	status = request_irq(pdev->resource[2].start, omap_udc_pio_irq,
-			IRQF_SAMPLE_RANDOM, "omap_udc pio", udc);
+			0, "omap_udc pio", udc);
 	if (status != 0) {
 		ERR("can't get irq %d, err %d\n",
 			(int) pdev->resource[2].start, status);
@@ -2939,7 +2980,7 @@ cleanup2:
 	free_irq(pdev->resource[1].start, udc);
 
 cleanup1:
-	kfree (udc);
+	kfree(udc);
 	udc = NULL;
 
 cleanup0:
-- 
1.7.10.2

[PATCH 2/6] usb: gadget: omap_udc: remove useless print

[Felipe Balbi]

that print isn't needed at all. Remove it
and move the use_dma reinitialization to
probe() function.

Note that ideally we would drop all
cpu_is_* and machine_is_* checks from
this driver instead. Later patches will
come to get rid of those.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/omap_udc.c |   13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
index aa2d174..5e7b891 100644
--- a/drivers/usb/gadget/omap_udc.c
+++ b/drivers/usb/gadget/omap_udc.c
@@ -2800,6 +2800,9 @@ static int __init omap_udc_probe(struct platform_device *pdev)
 	struct clk		*dc_clk = NULL;
 	struct clk		*hhc_clk = NULL;
 
+	if (cpu_is_omap7xx())
+		use_dma = 0;
+
 	/* NOTE:  "knows" the order of the resources! */
 	if (!request_mem_region(pdev->resource[0].start,
 			pdev->resource[0].end - pdev->resource[0].start + 1,
@@ -3096,16 +3099,6 @@ static struct platform_driver udc_driver = {
 
 static int __init udc_init(void)
 {
-	/* Disable DMA for omap7xx -- it doesn't work right. */
-	if (cpu_is_omap7xx())
-		use_dma = 0;
-
-	INFO("%s, version: " DRIVER_VERSION
-#ifdef	USE_ISO
-		" (iso)"
-#endif
-		"%s\n", driver_desc,
-		use_dma ?  " (dma)" : "");
 	return platform_driver_probe(&udc_driver, omap_udc_probe);
 }
 module_init(udc_init);
-- 
1.7.10.2

Re: [PATCH 2/6] usb: gadget: omap_udc: remove useless print

[Tony Lindgren]

* Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> [120529 05:22]:
> that print isn't needed at all. Remove it
> and move the use_dma reinitialization to
> probe() function.
> 
> Note that ideally we would drop all
> cpu_is_* and machine_is_* checks from
> this driver instead. Later patches will
> come to get rid of those.
> 
> Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/usb/gadget/omap_udc.c |   13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
> index aa2d174..5e7b891 100644
> --- a/drivers/usb/gadget/omap_udc.c
> +++ b/drivers/usb/gadget/omap_udc.c
> @@ -2800,6 +2800,9 @@ static int __init omap_udc_probe(struct platform_device *pdev)
>  	struct clk		*dc_clk = NULL;
>  	struct clk		*hhc_clk = NULL;
>  
> +	if (cpu_is_omap7xx())
> +		use_dma = 0;
> +
>  	/* NOTE:  "knows" the order of the resources! */
>  	if (!request_mem_region(pdev->resource[0].start,
>  			pdev->resource[0].end - pdev->resource[0].start + 1,

I think this would break omap_udc for 7xx as it then tries to use
DMA as USED_MA is set by default?

Setting use_dma = 0 twice is unncessary though :)

Regards,

Tony


> @@ -3096,16 +3099,6 @@ static struct platform_driver udc_driver = {
>  
>  static int __init udc_init(void)
>  {
> -	/* Disable DMA for omap7xx -- it doesn't work right. */
> -	if (cpu_is_omap7xx())
> -		use_dma = 0;
> -
> -	INFO("%s, version: " DRIVER_VERSION
> -#ifdef	USE_ISO
> -		" (iso)"
> -#endif
> -		"%s\n", driver_desc,
> -		use_dma ?  " (dma)" : "");
>  	return platform_driver_probe(&udc_driver, omap_udc_probe);
>  }
>  module_init(udc_init);
> -- 
> 1.7.10.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] usb: gadget: omap_udc: remove useless print

[Felipe Balbi]

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

Hi,

On Fri, Jun 01, 2012 at 04:01:24AM -0700, Tony Lindgren wrote:
> * Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> [120529 05:22]:
> > that print isn't needed at all. Remove it
> > and move the use_dma reinitialization to
> > probe() function.
> > 
> > Note that ideally we would drop all
> > cpu_is_* and machine_is_* checks from
> > this driver instead. Later patches will
> > come to get rid of those.
> > 
> > Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> > ---
> >  drivers/usb/gadget/omap_udc.c |   13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
> > index aa2d174..5e7b891 100644
> > --- a/drivers/usb/gadget/omap_udc.c
> > +++ b/drivers/usb/gadget/omap_udc.c
> > @@ -2800,6 +2800,9 @@ static int __init omap_udc_probe(struct platform_device *pdev)
> >  	struct clk		*dc_clk = NULL;
> >  	struct clk		*hhc_clk = NULL;
> >  
> > +	if (cpu_is_omap7xx())
> > +		use_dma = 0;
> > +
> >  	/* NOTE:  "knows" the order of the resources! */
> >  	if (!request_mem_region(pdev->resource[0].start,
> >  			pdev->resource[0].end - pdev->resource[0].start + 1,
> 
> I think this would break omap_udc for 7xx as it then tries to use
> DMA as USED_MA is set by default?

I'm clearing it in the start of probe function. There's not functional
changes at all. use_dma is ON by default, then our probe gets called and
first thing we do is clear use_dma to zero when we're running on
omap7xx. I don't see the issue, maybe I'm missing something.

-- 
balbi

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

Re: [PATCH 2/6] usb: gadget: omap_udc: remove useless print

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [120601 04:13]:
> Hi,
> 
> On Fri, Jun 01, 2012 at 04:01:24AM -0700, Tony Lindgren wrote:
> > * Felipe Balbi <balbi@ti.com> [120529 05:22]:
> > > that print isn't needed at all. Remove it
> > > and move the use_dma reinitialization to
> > > probe() function.
> > > 
> > > Note that ideally we would drop all
> > > cpu_is_* and machine_is_* checks from
> > > this driver instead. Later patches will
> > > come to get rid of those.
> > > 
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > ---
> > >  drivers/usb/gadget/omap_udc.c |   13 +++----------
> > >  1 file changed, 3 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
> > > index aa2d174..5e7b891 100644
> > > --- a/drivers/usb/gadget/omap_udc.c
> > > +++ b/drivers/usb/gadget/omap_udc.c
> > > @@ -2800,6 +2800,9 @@ static int __init omap_udc_probe(struct platform_device *pdev)
> > >  	struct clk		*dc_clk = NULL;
> > >  	struct clk		*hhc_clk = NULL;
> > >  
> > > +	if (cpu_is_omap7xx())
> > > +		use_dma = 0;
> > > +
> > >  	/* NOTE:  "knows" the order of the resources! */
> > >  	if (!request_mem_region(pdev->resource[0].start,
> > >  			pdev->resource[0].end - pdev->resource[0].start + 1,
> > 
> > I think this would break omap_udc for 7xx as it then tries to use
> > DMA as USED_MA is set by default?
> 
> I'm clearing it in the start of probe function. There's not functional
> changes at all. use_dma is ON by default, then our probe gets called and
> first thing we do is clear use_dma to zero when we're running on
> omap7xx. I don't see the issue, maybe I'm missing something.

OK thanks for clarifying, sounds like it should work then.

Tony

[PATCH 3/6] usb: gadget: omap_udc: let it work as a module

[Felipe Balbi]

this also helps removing a few lines of boilerplate
code.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/gadget/omap_udc.c |   23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
index 5e7b891..814aafb 100644
--- a/drivers/usb/gadget/omap_udc.c
+++ b/drivers/usb/gadget/omap_udc.c
@@ -2535,7 +2535,7 @@ static inline void remove_proc_file(void) {}
  * UDC_SYSCON_1.CFG_LOCK is set can now work.  We won't use that
  * capability yet though.
  */
-static unsigned __init
+static unsigned __devinit
 omap_ep_setup(char *name, u8 addr, u8 type,
 		unsigned buf, unsigned maxp, int dbuf)
 {
@@ -2653,7 +2653,7 @@ static void omap_udc_release(struct device *dev)
 	udc = NULL;
 }
 
-static int __init
+static int __devinit
 omap_udc_setup(struct platform_device *odev, struct usb_phy *xceiv)
 {
 	unsigned	tmp, buf;
@@ -2790,7 +2790,7 @@ omap_udc_setup(struct platform_device *odev, struct usb_phy *xceiv)
 	return 0;
 }
 
-static int __init omap_udc_probe(struct platform_device *pdev)
+static int __devinit omap_udc_probe(struct platform_device *pdev)
 {
 	int			status = -ENODEV;
 	int			hmc;
@@ -3003,7 +3003,7 @@ cleanup0:
 	return status;
 }
 
-static int __exit omap_udc_remove(struct platform_device *pdev)
+static int __devexit omap_udc_remove(struct platform_device *pdev)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
 
@@ -3088,7 +3088,8 @@ static int omap_udc_resume(struct platform_device *dev)
 /*-------------------------------------------------------------------------*/
 
 static struct platform_driver udc_driver = {
-	.remove		= __exit_p(omap_udc_remove),
+	.probe		= omap_udc_probe,
+	.remove		= __devexit_p(omap_udc_remove),
 	.suspend	= omap_udc_suspend,
 	.resume		= omap_udc_resume,
 	.driver		= {
@@ -3097,17 +3098,7 @@ static struct platform_driver udc_driver = {
 	},
 };
 
-static int __init udc_init(void)
-{
-	return platform_driver_probe(&udc_driver, omap_udc_probe);
-}
-module_init(udc_init);

[PATCH 4/6] usb: gadget: omap_udc: remove possiblity of NULL pointer de-reference

[Felipe Balbi]

when allocating a request, it's better programming
practice to make sure we return NULL if allocation
failed.

This will ensure that, if struct usb_request isn't
the first member on our structure, we don't cheat
the gadget driver into thinking allocating worked
because pointer isn't 0.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/gadget/omap_udc.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
index 814aafb..f13bcdc 100644
--- a/drivers/usb/gadget/omap_udc.c
+++ b/drivers/usb/gadget/omap_udc.c
@@ -268,10 +268,12 @@ omap_alloc_request(struct usb_ep *ep, gfp_t gfp_flags)
 	struct omap_req	*req;
 
 	req = kzalloc(sizeof(*req), gfp_flags);
-	if (req) {
-		req->req.dma = DMA_ADDR_INVALID;
-		INIT_LIST_HEAD(&req->queue);
-	}
+	if (!req)
+		return NULL;
+
+	req->req.dma = DMA_ADDR_INVALID;
+	INIT_LIST_HEAD(&req->queue);
+
 	return &req->req;
 }
 
-- 
1.7.10.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] usb: gadget: omap_udc: kfree(NULL) is safe

[Felipe Balbi]

we don't need to check for _req because
kfree(NULL) is safe. Also, if someone
actually passes a NULL pointer to be freed
by usb_ep_free_request(), he deserves any
issue he faces.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/gadget/omap_udc.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
index f13bcdc..50e8490 100644
--- a/drivers/usb/gadget/omap_udc.c
+++ b/drivers/usb/gadget/omap_udc.c
@@ -282,8 +282,7 @@ omap_free_request(struct usb_ep *ep, struct usb_request *_req)
 {
 	struct omap_req	*req = container_of(_req, struct omap_req, req);
 
-	if (_req)
-		kfree(req);
+	kfree(req);
 }
 
 /*-------------------------------------------------------------------------*/
-- 
1.7.10.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] usb: gadget: omap_udc: use generic map/unmap routines

[Felipe Balbi]

This patch makes use of the generic map/unmap
routines on the omap_udc driver. Removes
some useless and duplicated code.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/omap_udc.c |   46 +++++++----------------------------------
 1 file changed, 7 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
index 50e8490..b6cc8b1 100644
--- a/drivers/usb/gadget/omap_udc.c
+++ b/drivers/usb/gadget/omap_udc.c
@@ -59,8 +59,6 @@
 #define	DRIVER_DESC	"OMAP UDC driver"
 #define	DRIVER_VERSION	"4 October 2004"
 
-#define	DMA_ADDR_INVALID	(~(dma_addr_t)0)
-
 /*
  * The OMAP UDC needs _very_ early endpoint setup:  before enabling the
  * D+ pullup to allow enumeration.  That's too early for the gadget
@@ -271,7 +269,6 @@ omap_alloc_request(struct usb_ep *ep, gfp_t gfp_flags)
 	if (!req)
 		return NULL;
 
-	req->req.dma = DMA_ADDR_INVALID;
 	INIT_LIST_HEAD(&req->queue);
 
 	return &req->req;
@@ -290,6 +287,7 @@ omap_free_request(struct usb_ep *ep, struct usb_request *_req)
 static void
 done(struct omap_ep *ep, struct omap_req *req, int status)
 {
+	struct omap_udc		*udc = ep->udc;
 	unsigned		stopped = ep->stopped;
 
 	list_del_init(&req->queue);
@@ -299,22 +297,9 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
 	else
 		status = req->req.status;
 
-	if (use_dma && ep->has_dma) {
-		if (req->mapped) {
-			dma_unmap_single(ep->udc->gadget.dev.parent,
-				req->req.dma, req->req.length,
-				(ep->bEndpointAddress & USB_DIR_IN)
-					? DMA_TO_DEVICE
-					: DMA_FROM_DEVICE);
-			req->req.dma = DMA_ADDR_INVALID;
-			req->mapped = 0;
-		} else
-			dma_sync_single_for_cpu(ep->udc->gadget.dev.parent,
-				req->req.dma, req->req.length,
-				(ep->bEndpointAddress & USB_DIR_IN)
-					? DMA_TO_DEVICE
-					: DMA_FROM_DEVICE);
-	}
+	if (use_dma && ep->has_dma)
+		usb_gadget_unmap_request(&udc->gadget, &req->req,
+				(ep->bEndpointAddress & USB_DIR_IN));
 
 #ifndef	USB_TRACE
 	if (status && status != -ESHUTDOWN)
@@ -915,26 +900,9 @@ omap_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
 	if (!udc->driver || udc->gadget.speed == USB_SPEED_UNKNOWN)
 		return -ESHUTDOWN;
 
-	if (use_dma && ep->has_dma) {
-		if (req->req.dma == DMA_ADDR_INVALID) {
-			req->req.dma = dma_map_single(
-				ep->udc->gadget.dev.parent,
-				req->req.buf,
-				req->req.length,
-				(ep->bEndpointAddress & USB_DIR_IN)
-					? DMA_TO_DEVICE
-					: DMA_FROM_DEVICE);
-			req->mapped = 1;
-		} else {
-			dma_sync_single_for_device(
-				ep->udc->gadget.dev.parent,
-				req->req.dma, req->req.length,
-				(ep->bEndpointAddress & USB_DIR_IN)
-					? DMA_TO_DEVICE
-					: DMA_FROM_DEVICE);
-			req->mapped = 0;
-		}
-	}
+	if (use_dma && ep->has_dma)
+		usb_gadget_map_request(&udc->gadget, &req->req,
+				(ep->bEndpointAddress & USB_DIR_IN));
 
 	VDBG("%s queue req %p, len %d buf %p\n",
 		ep->ep.name, _req, _req->length, _req->buf);
-- 
1.7.10.2