Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]

Hi,

On Tue, Sep 15, 2015 at 06:15:25PM +0200, Krzysztof Opasiak wrote:
> >>>+	}
> >>>+
> >>>+	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.
> >
> >usb_ep_is_enabled() should be a good addition but I don't see an issue
> >ignoring usb_ep_enabled() for something that's already enabled.
> >
> >Imagine if you got an error when you tried to push the light switch to
> >the 'on' position while the light was already on :-p
> >
> 
> Hmmm not sure right now, didn't test this recently :D as usually I check if
> light isn't already "on" before I touch the switch to turn it on:P

not my best analogy :)

> Just joking. Personally I just prefer to don't touch things which are
> already in desired condition. Let's take close() as example which
> could be a little bit equivalent of our usb_ep_disable(). It is not
> legal to call it twice on some fd and second call ends up with error.

I understand that, it's just the difference between adding
usb_ep_is_enabled() to every single gadget/function driver or adding it
to usb_ep_enable() itself to keep gadget/function drivers cleaner.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]