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

Felipe Balbi
2015-05-26T17:10:21Z

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

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.

--
balbi

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