Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754407AbbGXIgD (ORCPT ); Fri, 24 Jul 2015 04:36:03 -0400 Received: from mga01.intel.com ([192.55.52.88]:35539 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754340AbbGXIfl convert rfc822-to-8bit (ORCPT ); Fri, 24 Jul 2015 04:35:41 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,537,1432623600"; d="scan'208";a="612029049" From: "Du, Changbin" To: Andrzej Pietrasiewicz , Felipe Balbi CC: Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw Thread-Topic: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw Thread-Index: AdDFQ/v32ELIUcFWTwaMp7sIW5cuU///mb2A//6andCAAoUkAP//b5qA Date: Fri, 24 Jul 2015 08:33:51 +0000 Message-ID: <0C18FE92A7765D4EB9EE5D38D86A563A01D4BDC4@SHSMSX103.ccr.corp.intel.com> References: <0C18FE92A7765D4EB9EE5D38D86A563A01D14FD5@SHSMSX103.ccr.corp.intel.com> <55B0FA29.5080402@samsung.com> <0C18FE92A7765D4EB9EE5D38D86A563A01D49C90@SHSMSX103.ccr.corp.intel.com> <55B1EB8C.2080303@samsung.com> In-Reply-To: <55B1EB8C.2080303@samsung.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3399 Lines: 100 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@samsung.com] > Hi, > > W dniu 24.07.2015 o 06:11, Du, Changbin pisze: > > Thanks, Pietrasiewicz. > >> From: Andrzej Pietrasiewicz [mailto:andrzej.p@samsung.com] > >> 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/