Re: [PATCH v2 0/5] usb: gadget: Fix gadget deactivaton feature

Robert Baldyga
2015-05-27T06:59:59Z

Hi Felipe,

On 05/26/2015 07:08 PM, Felipe Balbi wrote:
> On Mon, May 04, 2015 at 02:55:10PM +0200, Robert Baldyga wrote:
>> Hi,
>>
>> This patch set introduces two functions usb_gadget_deactivate() and
>> usb_gadget_activate(), designed to prevent udc-core from showing binded
>> gadget to host until it will be ready to work. It also makes
>> usb_function_deactivate()/activate() using these functions.
>>
>> So far gadget deactivation was made by calling usb_gadget_disconnect(),
>> but since we have usb_gadget_connect() called after gadget->bind()
>> (in udc_bind_to_driver()) this method doesn't provide expected result.
>> Calling function usb_gadget_disconnect() before gadget connection doesn't
>> prevent udc-core from connecting gadget to driver - usb_gadget_disconnect()
>> call is ignored and gadget is connected regardless to it. This usually
>> results with errors, for example because we binded gadget with 0
>> configurations.
>>
>> This patch set fixes this problem adding functions allowing to perform
>> effective gadget deactivation from gadget->bind().
>>
>> According to Felipe's suggestion, in v2 there is one new patch adding
>> 'bind_deactivated' flag, which should be used by functions which want
>> to be binded as deactivated (for example because they need to wait for
>> userspace daemon). Functions using this flag have to call
>> usb_function_activate() to tell composite core they are ready to work.
>> I have also converted functions which are using deactivation feature
>> to make them using 'bind_deactivated' flag. Patches are also attached
>> to this patch set.
>>
>> I was considering removing usb_function_deactivate() function since we
>> have this flag, but I see that some of USB functions use it not only
>> in bind() but also e.g. when userspace daemon disconnects. This is also
>> the reason why I haven't named this flag 'controls_pullup', because this
>> name doesn't describe precisely what does it mean.
>>
>> We also need to consider what to do when deactivation fails (which can
>> happen if our UDC driver doesn't support the pullup callback). So far,
>> when usb_function_deactivate() was called in bind(), we have had ability
>> to decide what to do, when it fails. Now we preform function deactivation
>> before its bind(), and when deactivation fails we fail to add the function
>> to gadget. Maybe we should to allow it to continue despite deactivation
>> failure and inform function (somehow) about this situation. If only it
>> makes any sense, because in this form it looks more complicated that
>> it was, and moreover it actually doesn't add any new features.
>
> I'll defer this series for v4.3 merge window because I'm still not
> entirely convinced we need this new reference counter. Sorry about that.
>

What reference counter did you mean?

Thanks,
Robert