Re: [PATCH 006/149] USB: m66592-udc: peripheral controller driver for M66592

David Brownell wrote:
> On Saturday 14 July 2007, Yoshihiro Shimoda wrote:
> > > Does the following patch behave, with all the locking test
> > > options in the kernel debug menu enabled?
> >
> > I applied this patch and I tested on following kernel debug
> > menu enabled.
> >  - CONFIG_DEBUG_MUTEXES
> >  - CONFIG_DEBUG_SPINLOCK
> >  - CONFIG_DEBUG_LOCK_ALLOC
> >  - CONFIG_DEBUG_SPINLOCK_SLEEP
> >  - CONFIG_PROVE_LOCKING
> > When I tested CONFIG_DEBUG_LOCKDEP enabled, A kernel did not
> > boot at all in my development environment...
>
> Whoops!  That needs fixing.  :(

I try fixing this problem.

> > -	spin_unlock(&ep->m66592->lock);
> > +	spin_unlock(&m66592->lock);
> >  	m66592_queue(m66592->gadget.ep0, m66592->ep0_req, GFP_KERNEL);
> > -	spin_lock(&ep->m66592->lock);
> > +	spin_lock(&m66592->lock);
>
> Right; simpler *and* more correct.
>
> Here's the all-rolled-up patch ... if you sign off on the whole
> thing, then I expect it should go upstream ASAP.

I made the patch which fixed a problem pointed out in LKML.
 - fixed memory leak
 - change ep0_buf
 - fixed byteswapping

Should I submit an email to LKML and linux-usb-devel mailing list
after having made a thing including all-rolled-up patch?

Thanks,
Y.Shimoda

---
 drivers/usb/gadget/m66592-udc.c |   21 +++++++++++----------
 drivers/usb/gadget/m66592-udc.h |    2 +-
 2 files changed, 12 insertions(+), 11 deletions(-)

--- linux-2.6.org/drivers/usb/gadget/m66592-udc.c	2007-07-16 11:44:00.000000000 +0900
+++ linux-2.6/drivers/usb/gadget/m66592-udc.c	2007-07-16 12:36:41.000000000 +0900
@@ -384,7 +384,7 @@ static void m66592_ep_setting(struct m66

 	ep->pipectr = get_pipectr_addr(pipenum);
 	ep->pipenum = pipenum;
-	ep->ep.maxpacket = desc->wMaxPacketSize;
+	ep->ep.maxpacket = le16_to_cpu(desc->wMaxPacketSize);
 	m66592->pipenum2ep[pipenum] = ep;
 	m66592->epaddr2ep[desc->bEndpointAddress&USB_ENDPOINT_NUMBER_MASK] = ep;
 	INIT_LIST_HEAD(&ep->queue);
@@ -461,7 +461,7 @@ static int alloc_pipe_config(struct m665
 	ep->type = info.type;

 	info.epnum = desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
-	info.maxpacket = desc->wMaxPacketSize;
+	info.maxpacket = le16_to_cpu(desc->wMaxPacketSize);
 	info.interval = desc->bInterval;
 	if (desc->bEndpointAddress & USB_ENDPOINT_DIR_MASK)
 		info.dir_in = 1;
@@ -922,6 +922,7 @@ __acquires(m66592->lock)
 	struct m66592_ep *ep;
 	u16 pid;
 	u16 status = 0;
+	u16 w_index = le16_to_cpu(ctrl->wIndex);

 	switch (ctrl->bRequestType & USB_RECIP_MASK) {
 	case USB_RECIP_DEVICE:
@@ -931,7 +932,7 @@ __acquires(m66592->lock)
 		status = 0;
 		break;
 	case USB_RECIP_ENDPOINT:
-		ep = m66592->epaddr2ep[ctrl->wIndex&USB_ENDPOINT_NUMBER_MASK];
+		ep = m66592->epaddr2ep[w_index & USB_ENDPOINT_NUMBER_MASK];
 		pid = control_reg_get_pid(m66592, ep->pipenum);
 		if (pid == M66592_PID_STALL)
 			status = 1 << USB_ENDPOINT_HALT;
@@ -943,8 +944,8 @@ __acquires(m66592->lock)
 		return;		/* exit */
 	}

-	*m66592->ep0_buf = status;
-	m66592->ep0_req->buf = m66592->ep0_buf;
+	m66592->ep0_data = cpu_to_le16(status);
+	m66592->ep0_req->buf = &m66592->ep0_data;
 	m66592->ep0_req->length = 2;
 	spin_unlock(&m66592->lock);
 	m66592_queue(m66592->gadget.ep0, m66592->ep0_req, GFP_KERNEL);
@@ -963,8 +964,9 @@ static void clear_feature(struct m66592
 	case USB_RECIP_ENDPOINT: {
 		struct m66592_ep *ep;
 		struct m66592_request *req;
+		u16 w_index = le16_to_cpu(ctrl->wIndex);

-		ep = m66592->epaddr2ep[ctrl->wIndex&USB_ENDPOINT_NUMBER_MASK];
+		ep = m66592->epaddr2ep[w_index & USB_ENDPOINT_NUMBER_MASK];
 		pipe_stop(m66592, ep->pipenum);
 		control_reg_sqclr(m66592, ep->pipenum);

@@ -999,8 +1001,9 @@ static void set_feature(struct m66592 *m
 		break;
 	case USB_RECIP_ENDPOINT: {
 		struct m66592_ep *ep;
+		u16 w_index = le16_to_cpu(ctrl->wIndex);

-		ep = m66592->epaddr2ep[ctrl->wIndex&USB_ENDPOINT_NUMBER_MASK];
+		ep = m66592->epaddr2ep[w_index & USB_ENDPOINT_NUMBER_MASK];
 		pipe_stall(m66592, ep->pipenum);

 		control_end(m66592, 1);
@@ -1480,6 +1483,7 @@ static int __exit m66592_remove(struct p
 	del_timer_sync(&m66592->timer);
 	iounmap(m66592->reg);
 	free_irq(platform_get_irq(pdev, 0), m66592);
+	m66592_free_request(&m66592->ep[0].ep, m66592->ep0_req);
 	kfree(m66592);
 	return 0;
 }
@@ -1587,9 +1591,6 @@ static int __init m66592_probe(struct pl
 	if (m66592->ep0_req == NULL)
 		goto clean_up;
 	m66592->ep0_req->complete = nop_completion;
-	m66592->ep0_buf = kmalloc(2, GFP_KERNEL);
-	if (m66592->ep0_buf == NULL)
-		goto clean_up;

 	init_controller(m66592);

--- linux-2.6.org/drivers/usb/gadget/m66592-udc.h	2007-07-16 11:44:01.000000000 +0900
+++ linux-2.6/drivers/usb/gadget/m66592-udc.h	2007-07-16 11:46:33.000000000 +0900
@@ -475,7 +475,7 @@ struct m66592 {
 	struct m66592_ep	*epaddr2ep[16];

 	struct usb_request	*ep0_req;	/* for internal request */
-	u16			*ep0_buf;	/* for internal request */
+	u16			ep0_data;	/* for internal request */

 	struct timer_list	timer;


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel