Thanks for explanation. I am agree with you that separate changes into two patches. Will send out new patch soon. Regards Du, Changbin > From: Andrzej Pietrasiewicz [mailto:andrzej.p@xxxxxxxxxxx] > Hi, > > W dniu 24.07.2015 o 06:11, Du, Changbin pisze: > > Thanks, Pietrasiewicz. > >> From: Andrzej Pietrasiewicz [mailto:andrzej.p@xxxxxxxxxxx] > >> W dniu 23.07.2015 o 14:34, Du, Changbin pisze: > >>> >From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 > 00:00:00 > >>> void composite_disconnect(struct usb_gadget *gadget) > >>> { > >>> struct usb_composite_dev *cdev = get_gadget_data(gadget); > >>> @@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget > >> *gadget) > >>> > >>> cdev->suspended = 1; > >>> > >>> - usb_gadget_vbus_draw(gadget, 2); > >>> + usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND); > >>> } > >> > >> This looks like an unrelated change. I think it should go first > >> in a separate patch which eliminates usage of "magic" numbers. > >> > > This change does make sense. As you know, when device is reset, it is in a > 'unconfigured' > > state. Compliance Test equipment may also measure vbus current at this > moment. > > I am not questioning the change itself. > > What I mean is that in my opinion it should be done in a separate patch, > because the newly introduced USB_VBUS_DRAW_SUSPEND is not used > anywhere else in your patch. The meaning of this change is "use a symbolic > name rather than an explicit number" and it is unrelated to > making composite gadget meet usb compliance for vbus draw. In other > words, > if you don't do this change at all the compliance is still maintained, > because the value of USB_VBUS_DRAW_SUSPEND is 2 anyway, so what the > compiler eventually sees is the same whether the change is made or not. > > My idea: > > [PATCHv2 0/2] usb gadget vbus draw compilance > [PATCHv2 1/2] usb: gadget: composite: avoid using a magic number > >> substituting an explicit "2" with USB_VBUS_DRAW_SUSPEND goes > here << > [PATCHv2 2/2] usb: gadget: composite: meet usb compliance for vbus draw > >> the rest of your changes go here << > > > > >>> } > >>> @@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver > >> composite_driver_template = { > >>> .unbind = composite_unbind, > >>> > >>> .setup = composite_setup, > >>> - .reset = composite_disconnect, > >>> + .reset = composite_reset, > >>> .disconnect = composite_disconnect, > >>> > >> > >> A similar "template" is in drivers/usb/gadget/configfs.c. Shouldn't the > "reset" > >> method be changed there as well? > >> > > Yes, it also need to change. I will change it as well. > > > >> > >>> > >>> +/* USB2 compliance requires that un-configured current draw <= > 100mA, > >>> + * USB3 requires it <= 150mA, OTG requires it <= 2.5mA. > >>> + */ > >>> +#define USB2_VBUS_DRAW_UNCONF 100 > >>> +#define USB3_VBUS_DRAW_UNCONF 150 > >>> +#define USB_OTG_VBUS_DRAW_UNCONF 2 > >> > >> > >>> +#define USB_VBUS_DRAW_SUSPEND 2 > >> > >> separate patch > >> > > Sorry, I didn't get your idea. Why separate these macros definition? > > Please see above. > > Andrzej -- 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
