> From: Paul Zimmerman > Sent: Monday, August 18, 2014 11:03 AM > > > From: Jingoo Han [mailto:jg1.han@xxxxxxxxxxx] > > Sent: Monday, August 18, 2014 2:17 AM > > > > On Monday, August 18, 2014 5:32 PM, Peter Chen wrote: > > > > > > linux-2.6/drivers/usb/dwc2/gadget.c: In function 's3c_hsotg_irq_enumdone': > > > linux-2.6/drivers/usb/dwc2/gadget.c:1904: warning: 'ep_mps' may be used uninitialized in this function > > > > (+cc Dinh Nguyen) > > > > I think that this warning is false. > > Because, 'ep_mps' cannot be used uninitialized in s3c_hsotg_irq_enumdone(). > > > > static void s3c_hsotg_irq_enumdone(struct s3c_hsotg *hsotg) > > { > > int ep0_mps = 0, ep_mps; > > > > case DSTS_ENUMSPD_FS: > > case DSTS_ENUMSPD_FS48: > > ep0_mps = EP0_MPS_LIMIT; > > ep_mps = 1023; > > break; > > > > case DSTS_ENUMSPD_HS: > > ep0_mps = EP0_MPS_LIMIT; > > ep_mps = 1024; > > break; > > > > In the case of DSTS_ENUMSPD_FS, DSTS_ENUMSPD_FS48, and DSTS_ENUMSPD_HS, > > both 'ep0_mps' and 'ep_mps' are initialized. > > > > case DSTS_ENUMSPD_LS: > > hsotg->gadget.speed = USB_SPEED_LOW; > > break; > > > > In the case of DSTS_ENUMSPD_LS, 'ep_mps' is uninitialized. > > But, 'ep0_mps' is also '0'. > > > > ..... > > > > if (ep0_mps) { > > int i; > > s3c_hsotg_set_ep_maxpacket(hsotg, 0, ep0_mps); > > for (i = 1; i < hsotg->num_of_eps; i++) > > s3c_hsotg_set_ep_maxpacket(hsotg, i, ep_mps); > > } > > > > If 'ep0_mps' is '0', 'ep_mps' will not be used. > > So, uninitialized 'ep_mps' cannot be used. > > > > However, in the future, it is not sure that the current scheme > > will not be modified. Thus, in order to make sure the code safety, > > this patch may be necessary. > > But 0 is not a valid default value for an endpoint mps. So I think the > patch serves no purpose other than to shut up the compiler. That's OK > with me, but I think the usual kernel practice is not to add unnecessary > initializations just to silence a compiler warning. Sorry, I lost the context of the original patch because Jingoo didn't quote it. I see the patch sets the default mps to 1024, not 0. However, 1024 is not a valid mps in the case of full-speed. If you change the value to 1023, you can add my acked-by. -- Paul -- 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
