Re: [PATCH 1/2] drivers: usb: gadget: udc: add missing break in switch

[Gustavo A. R. Silva]

Hello Felipe,

Quoting Felipe Balbi <balbi@xxxxxxxxxx>:

> Hi,
>
> "Gustavo A. R. Silva" <garsilva@xxxxxxxxxxxxxx> writes:
> > Add missing break in switch.
> >
> > Addresses-Coverity-ID: 201385
> > Signed-off-by: Gustavo A. R. Silva <garsilva@xxxxxxxxxxxxxx>
> > ---
> >  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