On Sunday 15 July 2007, Yoshihiro Shimoda wrote:
> 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.
It's rather far afield from USB!
>
> > 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
It doesn't fix the probe() path which returns after error
without unregistering the IRQ handler, though ... a leak,
but not a memory leak.
> Should I submit an email to LKML and linux-usb-devel mailing list
> after having made a thing including all-rolled-up patch?
Unfortunately Linus merged a partial fix yesterday ... rather
than the more complete update which predated it. (Sigh).
Greg, do you have a suggestion about how to resolve this mess?
Unfortunately it seems Linus isn't taking patches direct from
me lately...
- Dave
>
> 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/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
