From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753353AbbIOPhh (ORCPT ); Tue, 15 Sep 2015 11:37:37 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:55614 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752868AbbIOPhf (ORCPT ); Tue, 15 Sep 2015 11:37:35 -0400 X-AuditID: cbfec7f4-f79c56d0000012ee-90-55f83b3c2e4c Subject: Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep To: Robert Baldyga , balbi@ti.com References: <1442327229-28320-1-git-send-email-r.baldyga@samsung.com> <1442327229-28320-5-git-send-email-r.baldyga@samsung.com> Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, b.zolnierkie@samsung.com, m.szyprowski@samsung.com, andrzej.p@samsung.com From: Krzysztof Opasiak Message-id: <55F83B37.20408@samsung.com> Date: Tue, 15 Sep 2015 17:37:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-version: 1.0 In-reply-to: <1442327229-28320-5-git-send-email-r.baldyga@samsung.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrBLMWRmVeSWpSXmKPExsVy+t/xK7o21j9CDVYstLCY9bKdxWLjjPWs Fgfv11s0L17PZnF51xw2i0XLWpkt1h65y27x4PBOdgcOj/1z17B79G1Zxehx/MZ2Jo/Pm+QC WKK4bFJSczLLUov07RK4MibM2sJWcEmw4tWh+UwNjD94uxg5OCQETCTmXgnsYuQEMsUkLtxb zwZiCwksZZSY8jS6i5ELyH7OKLF52SlmkISwQLDEodmrWEBsEQELift7j7JBFDUyShy8sIkJ xGEWWMYo8WTCYnaQDWwC+hLzdomCNPAKaEg0rr7JCBJmEVCVWNcfAhIWFYiQOHX2LRtEiaDE j8n3WEBKOAXcJLr2poGEmQVsJRa8X8cCYctLbF7zlnkCo8AsJB2zkJTNQlK2gJF5FaNoamly QXFSeq6hXnFibnFpXrpecn7uJkZIeH/Zwbj4mNUhRgEORiUe3oj276FCrIllxZW5hxglOJiV RHgbdX6ECvGmJFZWpRblxxeV5qQWH2KU5mBREuedu+t9iJBAemJJanZqakFqEUyWiYNTqoGR xflMgfeHFEEu14U3Zh7tXePscWebwFvdq7W90ybk+3d847oleC0hMcWPy0jPzUyVQ9T/k6ND ffS+j1zBrV+DNFjLg/XsvuZfPpif8eZF8gEN5hOnJk5gqnoslHhabXrFsskfuH0TwgOD7I08 zFKOzdnNlL2hedu008eeJexu/m748mJwcY4SS3FGoqEWc1FxIgDna6VlawIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On 09/15/2015 04:26 PM, Robert Baldyga wrote: > This patch introduces 'enabled' flag in struct usb_ep, and modifies > usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint > enabled/disabled state. It helps to avoid enabling endpoints which are > already enabled, and disabling endpoints which are already disables. > >>>From now USB functions don't have to remember current endpoint > enable/disable state, as this state is now handled automatically which > makes this API less bug-prone. > > Signed-off-by: Robert Baldyga > --- > include/linux/usb/gadget.h | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index 3f299e2..63375cd 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -215,6 +215,7 @@ struct usb_ep { > struct list_head ep_list; > struct usb_ep_caps caps; > bool claimed; > + bool enabled; > unsigned maxpacket:16; > unsigned maxpacket_limit:16; > unsigned max_streams:16; > @@ -264,7 +265,15 @@ static inline void usb_ep_set_maxpacket_limit(struct usb_ep *ep, > */ > static inline int usb_ep_enable(struct usb_ep *ep) > { > - return ep->ops->enable(ep, ep->desc); > + int ret = 0; > + > + if (!ep->enabled) { > + ret = ep->ops->enable(ep, ep->desc); > + if (!ret) > + ep->enabled = true; > + } > + > + return ret; > } > > /** > @@ -281,7 +290,15 @@ static inline int usb_ep_enable(struct usb_ep *ep) > */ > static inline int usb_ep_disable(struct usb_ep *ep) > { > - return ep->ops->disable(ep); > + int ret = 0; > + > + if (ep->enabled) { > + ret = ep->ops->disable(ep); > + if (!ret) > + ep->enabled = false; > + } > + > + return ret; > } > Personally I don't like this convention. In my opinion usb_ep_disable() & usb_ep_enable() should fail if ep is already disabled/enabled. Then in function code we should check if endpoint is enabled (maybe even we should have usb_ep_is_enabled()) and call disable only when it is really enabled. Best Regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics