lkml.org 
[lkml]   [2017]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] drivers: usb: gadget: udc: add missing break in switch
Hello Felipe,

Quoting Felipe Balbi <balbi@kernel.org>:

> Hi,
>
> "Gustavo A. R. Silva" <garsilva@embeddedor.com> writes:
>> Add missing break in switch.
>>
>> Addresses-Coverity-ID: 201385
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>> drivers/usb/gadget/udc/mv_udc_core.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/gadget/udc/mv_udc_core.c
>> b/drivers/usb/gadget/udc/mv_udc_core.c
>> index 27ebb0d..56b3574 100644
>> --- a/drivers/usb/gadget/udc/mv_udc_core.c
>> +++ b/drivers/usb/gadget/udc/mv_udc_core.c
>> @@ -489,6 +489,7 @@ static int mv_ep_enable(struct usb_ep *_ep,
>> break;
>> case USB_ENDPOINT_XFER_CONTROL:
>> ios = 1;
>> + break;
>
> are you SURE this is supposed to have this break statement? What if we
> want to initialize mult to 0 *also* for control endpoints? How did you
> test this? Do you have access to Marvel's documentation for this
> controller?
>

Certainly I wasn't sure, but I also think this is kind of obscure
code. If that is the case that we also want to initialize mult to 0,
wouldn't it be clearer (for maintenance purposes) to add mult = 0 and
the break statement after ios = 1?

What do you think if I modify that piece of code as follows:

case USB_ENDPOINT_XFER_CONTROL:
ios = 1;
mult = 0;
break;


Thank you
--
Gustavo A. R. Silva






\
 
 \ /
  Last update: 2017-02-08 12:00    [W:0.177 / U:0.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site