Hi,

On Thu, Aug 20, 2015 at 07:16:48PM +0200, Robert Baldyga wrote:

On 08/20/2015 06:48 PM, Felipe Balbi wrote:

quoted

On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote:

quoted

Hi Felipe,

On 08/20/2015 05:35 PM, Felipe Balbi wrote:
[...]

quoted

just letting you know that this regresses all gadget drivers making them
try to disable previously disabled endpoints and enable previously
enabled endpoints.

I have a possible fix (see below) but then it shows a problem on the
host side when using with g_zero (see further below):

commit 3b8932100aacb6cfbffe288ca93025d8b8430c00
Author: Felipe Balbi [off-list ref]
Date:   Wed Aug 19 18:05:27 2015 -0500

    usb: gadget: fix ep->claimed lifetime

    In order to fix a regression introduced by commit
    cc476b42a39d ("usb: gadget: encapsulate endpoint
    claiming mechanism") we have to introduce a simple
    helper to check if a particular is enabled or not.

    After that, we need to move ep->claimed lifetime to
    usb_ep_enable() and usb_ep_disable() since those
    are the only functions which actually enable and
    disable endpoints.

    A follow-up patch will come to drop all driver_data
    checks from function drivers, since those are, now,
    pointless.

    Fixes: cc476b42a39d ("usb: gadget: encapsulate endpoint
    	claiming mechanism")
    Cc: Robert Baldyga [off-list ref]
    Signed-off-by: Felipe Balbi [off-list ref]

diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 978435a51038..ad45070cd76f 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c

@@ -126,7 +126,6 @@ found_ep:
 	ep->address = desc->bEndpointAddress;
 	ep->desc = NULL;
 	ep->comp_desc = NULL;
-	ep->claimed = true;

Removing this line causes autoconfig can return the same endpoint many
times. This probably causes problems with g_zero.

I will try to fix it ASAP.

I was considering the same thing, but the lifetime of ->claimed doesn't
look correct to me either way. Note that once the flag is enabled, it
won't get disabled by most gadget drivers.

And it should not be. This flag is indicator, that endpoint is used by some
function. It should be set once by usb_ep_autoconfig() and cleared by
usb_ep_autoconfig_reset().

have you considered switching interfaces and/or alternate settings ?

I wonder what is reason of this enable/disable regression. Maybe the problem
is that we don't set ep->driver_data to NULL in usb_ep_autoconfig_reset()
(so far it was done). Does this problem occur while gadget is binded to UDC
for the first time, or at any next time? Unfortunately at this moment I
don't have access to my hardware, so it will take a moment before I will
setup some testing environment.

yeah, that's okay. We've got time.

-- 
balbi