On Fri, Nov 8, 2013 at 2:42 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> On Fri, Nov 08, 2013 at 02:20:24PM -0600, Bin Liu wrote:
>
> no commit log == no commit ;-)
I am not sure what else needs to say other than the subject. But I
will try to make some...
>
>> Signed-off-by: Bin Liu <b-liu@xxxxxx>
>> ---
>> drivers/usb/musb/musb_gadget_ep0.c | 23 ++++++++++++++++++++++-
>> 1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c
>> index 3e9ec7c..f31b9b1 100644
>> --- a/drivers/usb/musb/musb_gadget_ep0.c
>> +++ b/drivers/usb/musb/musb_gadget_ep0.c
>> @@ -664,6 +664,24 @@ __acquires(musb->lock)
>> return retval;
>> }
>>
>> +/* dump the EP0 fifo for debug */
>> +static void musb_g_ep0_dump_fifo(struct musb *musb, int len)
>> +{
>
> I would wrap this on #ifdef DEBUG
>
>> + u8 *t;
>
> u8 buf[20];
>
>> +
>> + if (len <= 0)
>> + return;
>> +
>> + t = kzalloc(len, GFP_KERNEL);
>
> this is called from IRQ handler, right ? It can't be GFP_KERNEL. Also,
> you might as well just have a 20 byte buffer in the stack itself. And
> only read up to 20 bytes, if it's more than 20 bytes we really don't
> need everything to know that the setup packet is definitely broken.
>
>> + if (!t)
>> + return;
>> +
>> + musb->ops->read_fifo(&musb->endpoints[0], len, t);
>
> ... read_fifo(&musb->endpointsp[0], min(len, 20), buf);
>
>> + print_hex_dump(KERN_ERR, "packet: ", DUMP_PREFIX_NONE,
>
> please make this KERN_DEBUG instead.
Agreed on all your comments. Will fix it in next version.
Thanks,
-Bin.
>
> --
> balbi
--
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
