Thanks for your comment.
2010/8/2 Sergei Shtylyov <sshtylyov@xxxxxxxxxx>:
> Hello.
>
> tom.leiming@xxxxxxxxx wrote:
>
>> From: Ming Lei <tom.leiming@xxxxxxxxx>
>
>> For shared fifo hw endpoint(with FIFO_TXRX style), only ep_in
>> field of musb_hw_ep is intialized in musb_g_init_endpoints, and
>> ep_out is not initialized, but musb_g_rx and rxstate may access
>> ep_out field of musb_hw_ep by the method below:
>
>> musb_ep = &musb->endpoints[epnum].ep_out
>
>> which can cause the kernel panic[1] below, this patch fixes the issue
>> by getting 'musb_ep' from '&musb->endpoints[epnum].ep_in' for shared fifo
>> endpoint.
>
> Perhaps the better solution is to initialize both 'ep_in' and 'ep_out'
> fields for the case of the shared FIFO, just like it's done in musb_host.c
> with 'in_qh' and 'out_qh' fields now -- this will result in a simpler
> code...
Simply adding
init_peripheral_ep(musb, &hw_ep->ep_out, epnum, 1);
into musb_g_init_endpoints for 'is_shared_fifo' case does not work.
>
> [...]
>
>> Signed-off-by: Ming Lei <tom.leiming@xxxxxxxxx>
>> Cc: stable <stable@xxxxxxxxxx>
>
>> diff --git a/drivers/usb/musb/musb_gadget.c
>> b/drivers/usb/musb/musb_gadget.c
>> index 6fca870..404b084 100644
>> --- a/drivers/usb/musb/musb_gadget.c
>> +++ b/drivers/usb/musb/musb_gadget.c
>> @@ -568,11 +568,20 @@ static void rxstate(struct musb *musb, struct
>> musb_request *req)
>> {
>> const u8 epnum = req->epnum;
>> struct usb_request *request = &req->request;
>> - struct musb_ep *musb_ep = &musb->endpoints[epnum].ep_out;
>> + struct musb_ep *musb_ep;
>> void __iomem *epio = musb->endpoints[epnum].regs;
>> unsigned fifo_count = 0;
>> - u16 len = musb_ep->packet_sz;
>> + u16 len;
>> u16 csr = musb_readw(epio, MUSB_RXCSR);
>> + struct musb_hw_ep *hw_ep;
>
> Please align '*hw_ep' with 'csr' and the rest of the variable names.
>
>> + hw_ep = &musb->endpoints[epnum];
>
> Could be initializer...
>
>> @@ -740,9 +749,16 @@ void musb_g_rx(struct musb *musb, u8 epnum)
>> u16 csr;
>> struct usb_request *request;
>> void __iomem *mbase = musb->mregs;
>> - struct musb_ep *musb_ep = &musb->endpoints[epnum].ep_out;
>> + struct musb_ep *musb_ep;
>> void __iomem *epio = musb->endpoints[epnum].regs;
>> struct dma_channel *dma;
>> + struct musb_hw_ep *hw_ep;
>> +
>> + hw_ep = &musb->endpoints[epnum];
>
> Same comments here...
The v1 should be posted after taking your comment.
--
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
