Re: [PATCH] usb: gadget: add claimed field in struct usb_ep

[Robert Baldyga <r.baldyga@samsung.com>]

On 06/30/2014 08:33 PM, Felipe Balbi wrote:
> On Tue, Jun 24, 2014 at 02:16:35PM +0200, Robert Baldyga wrote:
>> On 06/23/2014 08:27 PM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Mon, Jun 23, 2014 at 08:07:43AM +0200, Robert Baldyga wrote:
>>>> On 06/19/2014 05:08 PM, Felipe Balbi wrote:
>>>>> On Mon, Jun 16, 2014 at 10:20:36AM +0200, Robert Baldyga wrote:
>>>>>> This field allows to mark ep as claimed in more clear way. Claiming
>>>>>> endpoint by setting driver_data to non-null value is leaky solution
>>>>>> and makes code unreadable.
>>>>>
>>>>> how come ? How can it be unreadable ? how can it be leaky ?
>>>>>
>>>>
>>>> What if gadget will not assign any value to driver_data (just like
>>>> Gadget Zero do)? Endpoint will be seen as not used, and autoconfig will
>>>
>>> huh ??? The gadget isn't the endpoint user, the function is. Look at
>>> f_sourcesink.c and f_loopback.c. If the function doesn't set anything to
>>> driver_data, then that's a bug on the function which needs fixing.
>>>
>>> Moreover, if there's a function which doesn't set driver_data, we could
>>> just as well have a function which doesn't set "claimed", so the problem
>>> is the same.
>>>
>>
>> I mean the function, not the gadget. Sorry for confusion.
>> Mechanism I developed marks endpoint as claimed *inside autoconfig
>> function*. It's significant difference, because there's not possible to
>> forget to mark that endpoint is claimed. When endpoint is returned from
>> autoconfig function, it belongs to function, and the function doesn't
>> need to do anything to claim obtained endpoint - it's already done.
> 
> we still might need to keep driver_data though. Some functions might
> need it. But now that you explained your goal, I can see how that might
> help. Please send a complete patchset also with the implementation for
> ep_autoconfig so we can all review your idea.
> 

The patch that I send contains entire implementation. It's quite simple
idea:
- in ep_matches() we check if ep->claimed is true instead of checking if
ep->driver_data is set
- in usb_ep_autoconfig_ss() we set ep->claimed to true if we have found
suitable endpoint
- usb_ep_autoconfig_reset() we set ep->claimed to false

The struct usb_ep still have field driver_data, which can be used by
function driver. The only difference is that this field is not used to
indicate if endpoint is claimed or not, it just contains assigned pointer.

Best regards
Robert Baldyga