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

David Brownell wrote:
> I think I see what's going on.  Since requests passed into
> usb_ep_queue() must have a completion handler, I thought your
> original code -- testing for a null handler -- was wrong.
>
> But I didn't notice that your internal GET_STATUS request
> didn't have a completion handler.  Good thing I didn't add
> a test for non-null completion in  m66592_queue()!
>
> A better fix would be to supply a NOP completion handler
> for that internal request.  That way there would be less risk
> of code working on m66592-udc which would not work with other
> controllers.

Thank you very much for your comment.

I thought that I should have written down such a comment
when I submitted it first.

> When looking at this, I observed a glitch in the locking
> I added ... I'm assuming you still didn't run with locking
> checks enabled, or you would have seen that issue.
>
> 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...

When I tested "usbtest", there was the problem that
NULL pointer access in get_status().
I made the following patch.

Thanks,
Y.Shimoda



====== CUT HERE
Fix the problem that accesses NULL pointer in get_status().

Signed-off-by: Yoshihiro Shimoda <[email protected]>
---
 drivers/usb/gadget/m66592-udc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-2.6.org/drivers/usb/gadget/m66592-udc.c	2007-07-14 19:58:31.000000000 +0900
+++ linux-2.6/drivers/usb/gadget/m66592-udc.c	2007-07-14 20:01:03.000000000 +0900
@@ -946,9 +946,9 @@ __acquires(m66592->lock)
 	*m66592->ep0_buf = status;
 	m66592->ep0_req->buf = m66592->ep0_buf;
 	m66592->ep0_req->length = 2;
-	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);
 }

 static void clear_feature(struct m66592 *m66592, struct usb_ctrlrequest *ctrl)


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