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

From: Gustavo A. R. Silva
Date: Wed Feb 08 2017 - 05:59:56 EST

Hello Felipe,

Quoting Felipe Balbi <balbi@xxxxxxxxxx>:


"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,
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

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:

ios = 1;
mult = 0;

Thank you
Gustavo A. R. Silva