Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, John Younwrites: > As you mentioned this is handled in the soft_disconnect sysfs. Why > shouldn't usb_gadget_disconnect() do the same thing, if not the gadget because there might be cases where we don't need/want the gadget to know about this disconnect. >>> >>> But what if we do? >> >> well, if the gadget is the one faking a disconnect, then it ought to > > It's not a really "faking" since the dwc3 controller supports soft > disconnect :) It's "fake" in the sense that pins are still mated in the connector, that's what I mean by fake ;) >> cancel requests and do all the other necessary steps, right ? :-) >> > > It does take those steps whenever it's notified of disconnect via its > disconnect callback. But since it doesn't get the callback when the > disconnect happens, it has to call it explicitly. What I'm saying is right, and that's the requirement. You're implementing it correctly. > that the API or UDC is the one that should call it since it knows that > the usb_gadget_disconnect() API was called and/or the UDC > disconnected. but udc-core doesn't know why it was called :) see usb_function_activate()/deactivate(), we don't want to cancel requests in that case, just delay the moment when host notices a port change. > itself? Exposing the sysfs as an API function would work too. Though it already _is_ exported. I just don't know why people are re-inventing the same solution :-) > both functions are "soft" disconnects and both are called > "disconnect". > > In our gadget_driver we do the workaround where we notify ourself that > we called the usb_gadget_disconnect() and that we should now be you could just rely on the sysfs interface, right ? :-) >>> >>> Not from the gadget driver, at least I don't think so. The gadget >>> driver itself is the one that wants to initiate the soft disconnect. >> >> I need to understand this requirement of yours a little better. Can you >> describe exactly what your gadget is doing ? Also, any chance of showing >> the code for that gadget ? I don't mind carrying an extra function >> driver if it helps you validate your IP :-) >> > > This gadget driver does a programmable disconnect during testing. I > don't think it will be released anytime soon. Which is why I never > bothered to submit a fix. Also note that this isn't a function but a > gadget driver (same place in the stack as libcomposite framework). I'm > not sure if we have those anymore in the kernel. We still have those, yes. But it seems like your stuff should be integrated into composite.c itself under a #ifdef USB_GADGET_TESTING or something like that. If it helps IP providers using a real OS (linux, of course heh) for silicon validation, I'd be very happy to integrate these changes. There's a lot which we can still do to make Linux more interesting for IP providers and SoC integrators (from validation point of view), if this minimal change helps, we should do it :) In fact, if you want to list out some extra requirements for dwc3's trace functionality, I'd be happy to implement those. Are you missing any visibility into some internal controller state ? Any sort of tooling which you'd like to have ? Let me know ;) -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, John Youn writes: > As you mentioned this is handled in the soft_disconnect sysfs. Why > shouldn't usb_gadget_disconnect() do the same thing, if not the gadget because there might be cases where we don't need/want the gadget to know about this disconnect. >>> >>> But what if we do? >> >> well, if the gadget is the one faking a disconnect, then it ought to > > It's not a really "faking" since the dwc3 controller supports soft > disconnect :) It's "fake" in the sense that pins are still mated in the connector, that's what I mean by fake ;) >> cancel requests and do all the other necessary steps, right ? :-) >> > > It does take those steps whenever it's notified of disconnect via its > disconnect callback. But since it doesn't get the callback when the > disconnect happens, it has to call it explicitly. What I'm saying is right, and that's the requirement. You're implementing it correctly. > that the API or UDC is the one that should call it since it knows that > the usb_gadget_disconnect() API was called and/or the UDC > disconnected. but udc-core doesn't know why it was called :) see usb_function_activate()/deactivate(), we don't want to cancel requests in that case, just delay the moment when host notices a port change. > itself? Exposing the sysfs as an API function would work too. Though it already _is_ exported. I just don't know why people are re-inventing the same solution :-) > both functions are "soft" disconnects and both are called > "disconnect". > > In our gadget_driver we do the workaround where we notify ourself that > we called the usb_gadget_disconnect() and that we should now be you could just rely on the sysfs interface, right ? :-) >>> >>> Not from the gadget driver, at least I don't think so. The gadget >>> driver itself is the one that wants to initiate the soft disconnect. >> >> I need to understand this requirement of yours a little better. Can you >> describe exactly what your gadget is doing ? Also, any chance of showing >> the code for that gadget ? I don't mind carrying an extra function >> driver if it helps you validate your IP :-) >> > > This gadget driver does a programmable disconnect during testing. I > don't think it will be released anytime soon. Which is why I never > bothered to submit a fix. Also note that this isn't a function but a > gadget driver (same place in the stack as libcomposite framework). I'm > not sure if we have those anymore in the kernel. We still have those, yes. But it seems like your stuff should be integrated into composite.c itself under a #ifdef USB_GADGET_TESTING or something like that. If it helps IP providers using a real OS (linux, of course heh) for silicon validation, I'd be very happy to integrate these changes. There's a lot which we can still do to make Linux more interesting for IP providers and SoC integrators (from validation point of view), if this minimal change helps, we should do it :) In fact, if you want to list out some extra requirements for dwc3's trace functionality, I'd be happy to implement those. Are you missing any visibility into some internal controller state ? Any sort of tooling which you'd like to have ? Let me know ;) -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, Peter Chenwrites: >> Peter Chen writes: >> >> "Du, Changbin" writes: >> >> > Hi, Balbi, >> >> > >> >> > The step to reproduce this issue is: >> >> > 1) connect device to a host and wait its enumeration. >> >> > 2) trigger software disconnect by calling function >> >> > usb_gadget_disconnect(), which finally call >> >> >dwc3_gadget_pullup(false). Do not reconnect device >> >> > (I mean no enumeration go on, keep bit Run/Stop 0.). >> >> > >> >> > At here, gadget driver's disconnect callback should be >> >> > Called, right? We has been disconnected. But no, as >> >> > You said " not generating disconnect IRQ after you >> >> > drop Run/Stop is expected". >> >> > >> >> > And I am testing on an Android device, Android only >> >> > use dwc3_gadget_pullup(false) to issue a soft disconnection. >> >> > This confused user that the UI still show usb as connected >> >> > State, caused by missing a disconnect event. >> >> >> >> okay, so I know what this is. This is caused by Android gadget itself >> >> not notifying the gadget that a disconnect has happened. Just look at >> >> udc-core's soft_connect implementation for the sysfs interface, and >> >> you'll see what I mean. >> >> >> >> This should be fixed at Android gadget itself. The only thing we could >> >> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the >> >> logic so it's easier for Android gadget to use; but even that I'm a >> >> little bit reluctant to do because Android should be using our >> >> soft_connect interface instead of reimplementing it (wrongly) by its >> >> own. >> >> >> > >> > If it is a gadget driver, it can call its disconnect explicitly. >> > Another thing is the gadget driver should not call usb_gadget_disconnect >> > directly, it should call usb_gadget_deactivate or usb_function_deactivate. >> > >> > Since currently, calling usb_gadget_disconnect may not do real pull down >> > dp, Felipe, will you consider adding gadget_driver->disconnect into >> > usb_gadget_disconnect after pull down dp? >> >> this is the detail that I'm not yet entirely sure is always valid. Would >> there ever be a situation where we want to drop pull-ups but not tell >> the gadget about it ? > > Yes, we have. > > - We have enabled pullup dp default for most of platforms, and for > gadget obex and uvc will pull down it since it wants app to > control it > > - Some USB charger detection design may need it for the secondary > detection. > > After checking again for usb_gadget_disconnect and > usb_gadget_disconnect, I think there is no possible that the dp is > still pulled after we call usb_gadget_disconnect, so we don't need > to change anything:) alright, cool. So we'll keep the requirement to call gadget_driver->disconnect() for anybody who calls usb_gadget_disconnect(). Caller should know if we need to notify gadget_driver or not about that disconnect. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, Peter Chen writes: >> Peter Chen writes: >> >> "Du, Changbin" writes: >> >> > Hi, Balbi, >> >> > >> >> > The step to reproduce this issue is: >> >> > 1) connect device to a host and wait its enumeration. >> >> > 2) trigger software disconnect by calling function >> >> > usb_gadget_disconnect(), which finally call >> >> >dwc3_gadget_pullup(false). Do not reconnect device >> >> > (I mean no enumeration go on, keep bit Run/Stop 0.). >> >> > >> >> > At here, gadget driver's disconnect callback should be >> >> > Called, right? We has been disconnected. But no, as >> >> > You said " not generating disconnect IRQ after you >> >> > drop Run/Stop is expected". >> >> > >> >> > And I am testing on an Android device, Android only >> >> > use dwc3_gadget_pullup(false) to issue a soft disconnection. >> >> > This confused user that the UI still show usb as connected >> >> > State, caused by missing a disconnect event. >> >> >> >> okay, so I know what this is. This is caused by Android gadget itself >> >> not notifying the gadget that a disconnect has happened. Just look at >> >> udc-core's soft_connect implementation for the sysfs interface, and >> >> you'll see what I mean. >> >> >> >> This should be fixed at Android gadget itself. The only thing we could >> >> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the >> >> logic so it's easier for Android gadget to use; but even that I'm a >> >> little bit reluctant to do because Android should be using our >> >> soft_connect interface instead of reimplementing it (wrongly) by its >> >> own. >> >> >> > >> > If it is a gadget driver, it can call its disconnect explicitly. >> > Another thing is the gadget driver should not call usb_gadget_disconnect >> > directly, it should call usb_gadget_deactivate or usb_function_deactivate. >> > >> > Since currently, calling usb_gadget_disconnect may not do real pull down >> > dp, Felipe, will you consider adding gadget_driver->disconnect into >> > usb_gadget_disconnect after pull down dp? >> >> this is the detail that I'm not yet entirely sure is always valid. Would >> there ever be a situation where we want to drop pull-ups but not tell >> the gadget about it ? > > Yes, we have. > > - We have enabled pullup dp default for most of platforms, and for > gadget obex and uvc will pull down it since it wants app to > control it > > - Some USB charger detection design may need it for the secondary > detection. > > After checking again for usb_gadget_disconnect and > usb_gadget_disconnect, I think there is no possible that the dp is > still pulled after we call usb_gadget_disconnect, so we don't need > to change anything:) alright, cool. So we'll keep the requirement to call gadget_driver->disconnect() for anybody who calls usb_gadget_disconnect(). Caller should know if we need to notify gadget_driver or not about that disconnect. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On Fri, May 06, 2016 at 10:01:20AM +0300, Felipe Balbi wrote: > > Hi, > > Peter Chenwrites: > >> "Du, Changbin" writes: > >> > Hi, Balbi, > >> > > >> > The step to reproduce this issue is: > >> > 1) connect device to a host and wait its enumeration. > >> > 2) trigger software disconnect by calling function > >> > usb_gadget_disconnect(), which finally call > >> >dwc3_gadget_pullup(false). Do not reconnect device > >> > (I mean no enumeration go on, keep bit Run/Stop 0.). > >> > > >> > At here, gadget driver's disconnect callback should be > >> > Called, right? We has been disconnected. But no, as > >> > You said " not generating disconnect IRQ after you > >> > drop Run/Stop is expected". > >> > > >> > And I am testing on an Android device, Android only > >> > use dwc3_gadget_pullup(false) to issue a soft disconnection. > >> > This confused user that the UI still show usb as connected > >> > State, caused by missing a disconnect event. > >> > >> okay, so I know what this is. This is caused by Android gadget itself > >> not notifying the gadget that a disconnect has happened. Just look at > >> udc-core's soft_connect implementation for the sysfs interface, and > >> you'll see what I mean. > >> > >> This should be fixed at Android gadget itself. The only thing we could > >> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the > >> logic so it's easier for Android gadget to use; but even that I'm a > >> little bit reluctant to do because Android should be using our > >> soft_connect interface instead of reimplementing it (wrongly) by its > >> own. > >> > > > > If it is a gadget driver, it can call its disconnect explicitly. > > Another thing is the gadget driver should not call usb_gadget_disconnect > > directly, it should call usb_gadget_deactivate or usb_function_deactivate. > > > > Since currently, calling usb_gadget_disconnect may not do real pull down > > dp, Felipe, will you consider adding gadget_driver->disconnect into > > usb_gadget_disconnect after pull down dp? > > this is the detail that I'm not yet entirely sure is always valid. Would > there ever be a situation where we want to drop pull-ups but not tell > the gadget about it ? Yes, we have. - We have enabled pullup dp default for most of platforms, and for gadget obex and uvc will pull down it since it wants app to control it - Some USB charger detection design may need it for the secondary detection. After checking again for usb_gadget_disconnect and usb_gadget_disconnect, I think there is no possible that the dp is still pulled after we call usb_gadget_disconnect, so we don't need to change anything:) -- Best Regards, Peter Chen
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On Fri, May 06, 2016 at 10:01:20AM +0300, Felipe Balbi wrote: > > Hi, > > Peter Chen writes: > >> "Du, Changbin" writes: > >> > Hi, Balbi, > >> > > >> > The step to reproduce this issue is: > >> > 1) connect device to a host and wait its enumeration. > >> > 2) trigger software disconnect by calling function > >> > usb_gadget_disconnect(), which finally call > >> >dwc3_gadget_pullup(false). Do not reconnect device > >> > (I mean no enumeration go on, keep bit Run/Stop 0.). > >> > > >> > At here, gadget driver's disconnect callback should be > >> > Called, right? We has been disconnected. But no, as > >> > You said " not generating disconnect IRQ after you > >> > drop Run/Stop is expected". > >> > > >> > And I am testing on an Android device, Android only > >> > use dwc3_gadget_pullup(false) to issue a soft disconnection. > >> > This confused user that the UI still show usb as connected > >> > State, caused by missing a disconnect event. > >> > >> okay, so I know what this is. This is caused by Android gadget itself > >> not notifying the gadget that a disconnect has happened. Just look at > >> udc-core's soft_connect implementation for the sysfs interface, and > >> you'll see what I mean. > >> > >> This should be fixed at Android gadget itself. The only thing we could > >> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the > >> logic so it's easier for Android gadget to use; but even that I'm a > >> little bit reluctant to do because Android should be using our > >> soft_connect interface instead of reimplementing it (wrongly) by its > >> own. > >> > > > > If it is a gadget driver, it can call its disconnect explicitly. > > Another thing is the gadget driver should not call usb_gadget_disconnect > > directly, it should call usb_gadget_deactivate or usb_function_deactivate. > > > > Since currently, calling usb_gadget_disconnect may not do real pull down > > dp, Felipe, will you consider adding gadget_driver->disconnect into > > usb_gadget_disconnect after pull down dp? > > this is the detail that I'm not yet entirely sure is always valid. Would > there ever be a situation where we want to drop pull-ups but not tell > the gadget about it ? Yes, we have. - We have enabled pullup dp default for most of platforms, and for gadget obex and uvc will pull down it since it wants app to control it - Some USB charger detection design may need it for the secondary detection. After checking again for usb_gadget_disconnect and usb_gadget_disconnect, I think there is no possible that the dp is still pulled after we call usb_gadget_disconnect, so we don't need to change anything:) -- Best Regards, Peter Chen
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, Peter Chenwrites: >> "Du, Changbin" writes: >> > Hi, Balbi, >> > >> > The step to reproduce this issue is: >> > 1) connect device to a host and wait its enumeration. >> > 2) trigger software disconnect by calling function >> > usb_gadget_disconnect(), which finally call >> >dwc3_gadget_pullup(false). Do not reconnect device >> > (I mean no enumeration go on, keep bit Run/Stop 0.). >> > >> > At here, gadget driver's disconnect callback should be >> > Called, right? We has been disconnected. But no, as >> > You said " not generating disconnect IRQ after you >> > drop Run/Stop is expected". >> > >> > And I am testing on an Android device, Android only >> > use dwc3_gadget_pullup(false) to issue a soft disconnection. >> > This confused user that the UI still show usb as connected >> > State, caused by missing a disconnect event. >> >> okay, so I know what this is. This is caused by Android gadget itself >> not notifying the gadget that a disconnect has happened. Just look at >> udc-core's soft_connect implementation for the sysfs interface, and >> you'll see what I mean. >> >> This should be fixed at Android gadget itself. The only thing we could >> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the >> logic so it's easier for Android gadget to use; but even that I'm a >> little bit reluctant to do because Android should be using our >> soft_connect interface instead of reimplementing it (wrongly) by its >> own. >> > > If it is a gadget driver, it can call its disconnect explicitly. > Another thing is the gadget driver should not call usb_gadget_disconnect > directly, it should call usb_gadget_deactivate or usb_function_deactivate. > > Since currently, calling usb_gadget_disconnect may not do real pull down > dp, Felipe, will you consider adding gadget_driver->disconnect into > usb_gadget_disconnect after pull down dp? this is the detail that I'm not yet entirely sure is always valid. Would there ever be a situation where we want to drop pull-ups but not tell the gadget about it ? I'm not sure. Anybody wants to investigate ? Otherwise I'll add it to my TODO list. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, Peter Chen writes: >> "Du, Changbin" writes: >> > Hi, Balbi, >> > >> > The step to reproduce this issue is: >> > 1) connect device to a host and wait its enumeration. >> > 2) trigger software disconnect by calling function >> > usb_gadget_disconnect(), which finally call >> >dwc3_gadget_pullup(false). Do not reconnect device >> > (I mean no enumeration go on, keep bit Run/Stop 0.). >> > >> > At here, gadget driver's disconnect callback should be >> > Called, right? We has been disconnected. But no, as >> > You said " not generating disconnect IRQ after you >> > drop Run/Stop is expected". >> > >> > And I am testing on an Android device, Android only >> > use dwc3_gadget_pullup(false) to issue a soft disconnection. >> > This confused user that the UI still show usb as connected >> > State, caused by missing a disconnect event. >> >> okay, so I know what this is. This is caused by Android gadget itself >> not notifying the gadget that a disconnect has happened. Just look at >> udc-core's soft_connect implementation for the sysfs interface, and >> you'll see what I mean. >> >> This should be fixed at Android gadget itself. The only thing we could >> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the >> logic so it's easier for Android gadget to use; but even that I'm a >> little bit reluctant to do because Android should be using our >> soft_connect interface instead of reimplementing it (wrongly) by its >> own. >> > > If it is a gadget driver, it can call its disconnect explicitly. > Another thing is the gadget driver should not call usb_gadget_disconnect > directly, it should call usb_gadget_deactivate or usb_function_deactivate. > > Since currently, calling usb_gadget_disconnect may not do real pull down > dp, Felipe, will you consider adding gadget_driver->disconnect into > usb_gadget_disconnect after pull down dp? this is the detail that I'm not yet entirely sure is always valid. Would there ever be a situation where we want to drop pull-ups but not tell the gadget about it ? I'm not sure. Anybody wants to investigate ? Otherwise I'll add it to my TODO list. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On Thu, Apr 28, 2016 at 09:46:15AM +0300, Felipe Balbi wrote: > > Hi, > > (we don't top-post on this forum ;-) > > "Du, Changbin"writes: > > Hi, Balbi, > > > > The step to reproduce this issue is: > > 1) connect device to a host and wait its enumeration. > > 2) trigger software disconnect by calling function > > usb_gadget_disconnect(), which finally call > >dwc3_gadget_pullup(false). Do not reconnect device > > (I mean no enumeration go on, keep bit Run/Stop 0.). > > > > At here, gadget driver's disconnect callback should be > > Called, right? We has been disconnected. But no, as > > You said " not generating disconnect IRQ after you > > drop Run/Stop is expected". > > > > And I am testing on an Android device, Android only > > use dwc3_gadget_pullup(false) to issue a soft disconnection. > > This confused user that the UI still show usb as connected > > State, caused by missing a disconnect event. > > okay, so I know what this is. This is caused by Android gadget itself > not notifying the gadget that a disconnect has happened. Just look at > udc-core's soft_connect implementation for the sysfs interface, and > you'll see what I mean. > > This should be fixed at Android gadget itself. The only thing we could > do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the > logic so it's easier for Android gadget to use; but even that I'm a > little bit reluctant to do because Android should be using our > soft_connect interface instead of reimplementing it (wrongly) by its > own. > If it is a gadget driver, it can call its disconnect explicitly. Another thing is the gadget driver should not call usb_gadget_disconnect directly, it should call usb_gadget_deactivate or usb_function_deactivate. Since currently, calling usb_gadget_disconnect may not do real pull down dp, Felipe, will you consider adding gadget_driver->disconnect into usb_gadget_disconnect after pull down dp? -- Best Regards, Peter Chen
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On Thu, Apr 28, 2016 at 09:46:15AM +0300, Felipe Balbi wrote: > > Hi, > > (we don't top-post on this forum ;-) > > "Du, Changbin" writes: > > Hi, Balbi, > > > > The step to reproduce this issue is: > > 1) connect device to a host and wait its enumeration. > > 2) trigger software disconnect by calling function > > usb_gadget_disconnect(), which finally call > >dwc3_gadget_pullup(false). Do not reconnect device > > (I mean no enumeration go on, keep bit Run/Stop 0.). > > > > At here, gadget driver's disconnect callback should be > > Called, right? We has been disconnected. But no, as > > You said " not generating disconnect IRQ after you > > drop Run/Stop is expected". > > > > And I am testing on an Android device, Android only > > use dwc3_gadget_pullup(false) to issue a soft disconnection. > > This confused user that the UI still show usb as connected > > State, caused by missing a disconnect event. > > okay, so I know what this is. This is caused by Android gadget itself > not notifying the gadget that a disconnect has happened. Just look at > udc-core's soft_connect implementation for the sysfs interface, and > you'll see what I mean. > > This should be fixed at Android gadget itself. The only thing we could > do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the > logic so it's easier for Android gadget to use; but even that I'm a > little bit reluctant to do because Android should be using our > soft_connect interface instead of reimplementing it (wrongly) by its > own. > If it is a gadget driver, it can call its disconnect explicitly. Another thing is the gadget driver should not call usb_gadget_disconnect directly, it should call usb_gadget_deactivate or usb_function_deactivate. Since currently, calling usb_gadget_disconnect may not do real pull down dp, Felipe, will you consider adding gadget_driver->disconnect into usb_gadget_disconnect after pull down dp? -- Best Regards, Peter Chen
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On 5/4/2016 3:42 AM, Felipe Balbi wrote: > > Hi, > > John Younwrites: >>> John Youn writes: > "Du, Changbin" writes: >> Hi, Balbi, >> >> The step to reproduce this issue is: >> 1) connect device to a host and wait its enumeration. >> 2) trigger software disconnect by calling function >> usb_gadget_disconnect(), which finally call >>dwc3_gadget_pullup(false). Do not reconnect device >> (I mean no enumeration go on, keep bit Run/Stop 0.). >> >> At here, gadget driver's disconnect callback should be >> Called, right? We has been disconnected. But no, as >> You said " not generating disconnect IRQ after you >> drop Run/Stop is expected". >> >> And I am testing on an Android device, Android only >> use dwc3_gadget_pullup(false) to issue a soft disconnection. >> This confused user that the UI still show usb as connected >> State, caused by missing a disconnect event. > > okay, so I know what this is. This is caused by Android gadget itself > not notifying the gadget that a disconnect has happened. Just look at > udc-core's soft_connect implementation for the sysfs interface, and > you'll see what I mean. > > This should be fixed at Android gadget itself. The only thing we could > do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the > logic so it's easier for Android gadget to use; but even that I'm a > little bit reluctant to do because Android should be using our > soft_connect interface instead of reimplementing it (wrongly) by its > own. > We've run in to the same issue with our usb_gadget_driver. If the usb_gadget_disconnect() API function, which seems like it is intended to be called by the gadget_driver, does cause the gadget to disconnect, it seems reasonable to expect the gadget or the UDC core to notify the gadget_driver via the callback. >>> >>> Well, the API is supposed to disconnect D+ pullup and that's about it. >>> As you mentioned this is handled in the soft_disconnect sysfs. Why shouldn't usb_gadget_disconnect() do the same thing, if not the gadget >>> >>> because there might be cases where we don't need/want the gadget to know >>> about this disconnect. >>> >> >> But what if we do? > > well, if the gadget is the one faking a disconnect, then it ought to It's not a really "faking" since the dwc3 controller supports soft disconnect :) > cancel requests and do all the other necessary steps, right ? :-) > It does take those steps whenever it's notified of disconnect via its disconnect callback. But since it doesn't get the callback when the disconnect happens, it has to call it explicitly. What I'm saying is that the API or UDC is the one that should call it since it knows that the usb_gadget_disconnect() API was called and/or the UDC disconnected. itself? Exposing the sysfs as an API function would work too. Though >>> >>> it already _is_ exported. I just don't know why people are re-inventing >>> the same solution :-) >>> both functions are "soft" disconnects and both are called "disconnect". In our gadget_driver we do the workaround where we notify ourself that we called the usb_gadget_disconnect() and that we should now be >>> >>> you could just rely on the sysfs interface, right ? :-) >> >> Not from the gadget driver, at least I don't think so. The gadget >> driver itself is the one that wants to initiate the soft disconnect. > > I need to understand this requirement of yours a little better. Can you > describe exactly what your gadget is doing ? Also, any chance of showing > the code for that gadget ? I don't mind carrying an extra function > driver if it helps you validate your IP :-) > This gadget driver does a programmable disconnect during testing. I don't think it will be released anytime soon. Which is why I never bothered to submit a fix. Also note that this isn't a function but a gadget driver (same place in the stack as libcomposite framework). I'm not sure if we have those anymore in the kernel. John
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On 5/4/2016 3:42 AM, Felipe Balbi wrote: > > Hi, > > John Youn writes: >>> John Youn writes: > "Du, Changbin" writes: >> Hi, Balbi, >> >> The step to reproduce this issue is: >> 1) connect device to a host and wait its enumeration. >> 2) trigger software disconnect by calling function >> usb_gadget_disconnect(), which finally call >>dwc3_gadget_pullup(false). Do not reconnect device >> (I mean no enumeration go on, keep bit Run/Stop 0.). >> >> At here, gadget driver's disconnect callback should be >> Called, right? We has been disconnected. But no, as >> You said " not generating disconnect IRQ after you >> drop Run/Stop is expected". >> >> And I am testing on an Android device, Android only >> use dwc3_gadget_pullup(false) to issue a soft disconnection. >> This confused user that the UI still show usb as connected >> State, caused by missing a disconnect event. > > okay, so I know what this is. This is caused by Android gadget itself > not notifying the gadget that a disconnect has happened. Just look at > udc-core's soft_connect implementation for the sysfs interface, and > you'll see what I mean. > > This should be fixed at Android gadget itself. The only thing we could > do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the > logic so it's easier for Android gadget to use; but even that I'm a > little bit reluctant to do because Android should be using our > soft_connect interface instead of reimplementing it (wrongly) by its > own. > We've run in to the same issue with our usb_gadget_driver. If the usb_gadget_disconnect() API function, which seems like it is intended to be called by the gadget_driver, does cause the gadget to disconnect, it seems reasonable to expect the gadget or the UDC core to notify the gadget_driver via the callback. >>> >>> Well, the API is supposed to disconnect D+ pullup and that's about it. >>> As you mentioned this is handled in the soft_disconnect sysfs. Why shouldn't usb_gadget_disconnect() do the same thing, if not the gadget >>> >>> because there might be cases where we don't need/want the gadget to know >>> about this disconnect. >>> >> >> But what if we do? > > well, if the gadget is the one faking a disconnect, then it ought to It's not a really "faking" since the dwc3 controller supports soft disconnect :) > cancel requests and do all the other necessary steps, right ? :-) > It does take those steps whenever it's notified of disconnect via its disconnect callback. But since it doesn't get the callback when the disconnect happens, it has to call it explicitly. What I'm saying is that the API or UDC is the one that should call it since it knows that the usb_gadget_disconnect() API was called and/or the UDC disconnected. itself? Exposing the sysfs as an API function would work too. Though >>> >>> it already _is_ exported. I just don't know why people are re-inventing >>> the same solution :-) >>> both functions are "soft" disconnects and both are called "disconnect". In our gadget_driver we do the workaround where we notify ourself that we called the usb_gadget_disconnect() and that we should now be >>> >>> you could just rely on the sysfs interface, right ? :-) >> >> Not from the gadget driver, at least I don't think so. The gadget >> driver itself is the one that wants to initiate the soft disconnect. > > I need to understand this requirement of yours a little better. Can you > describe exactly what your gadget is doing ? Also, any chance of showing > the code for that gadget ? I don't mind carrying an extra function > driver if it helps you validate your IP :-) > This gadget driver does a programmable disconnect during testing. I don't think it will be released anytime soon. Which is why I never bothered to submit a fix. Also note that this isn't a function but a gadget driver (same place in the stack as libcomposite framework). I'm not sure if we have those anymore in the kernel. John
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, John Younwrites: >> John Youn writes: "Du, Changbin" writes: > Hi, Balbi, > > The step to reproduce this issue is: > 1) connect device to a host and wait its enumeration. > 2) trigger software disconnect by calling function > usb_gadget_disconnect(), which finally call >dwc3_gadget_pullup(false). Do not reconnect device > (I mean no enumeration go on, keep bit Run/Stop 0.). > > At here, gadget driver's disconnect callback should be > Called, right? We has been disconnected. But no, as > You said " not generating disconnect IRQ after you > drop Run/Stop is expected". > > And I am testing on an Android device, Android only > use dwc3_gadget_pullup(false) to issue a soft disconnection. > This confused user that the UI still show usb as connected > State, caused by missing a disconnect event. okay, so I know what this is. This is caused by Android gadget itself not notifying the gadget that a disconnect has happened. Just look at udc-core's soft_connect implementation for the sysfs interface, and you'll see what I mean. This should be fixed at Android gadget itself. The only thing we could do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the logic so it's easier for Android gadget to use; but even that I'm a little bit reluctant to do because Android should be using our soft_connect interface instead of reimplementing it (wrongly) by its own. >>> >>> We've run in to the same issue with our usb_gadget_driver. >>> >>> If the usb_gadget_disconnect() API function, which seems like it is >>> intended to be called by the gadget_driver, does cause the gadget to >>> disconnect, it seems reasonable to expect the gadget or the UDC core >>> to notify the gadget_driver via the callback. >> >> Well, the API is supposed to disconnect D+ pullup and that's about it. >> >>> As you mentioned this is handled in the soft_disconnect sysfs. Why >>> shouldn't usb_gadget_disconnect() do the same thing, if not the gadget >> >> because there might be cases where we don't need/want the gadget to know >> about this disconnect. >> > > But what if we do? well, if the gadget is the one faking a disconnect, then it ought to cancel requests and do all the other necessary steps, right ? :-) >>> itself? Exposing the sysfs as an API function would work too. Though >> >> it already _is_ exported. I just don't know why people are re-inventing >> the same solution :-) >> >>> both functions are "soft" disconnects and both are called >>> "disconnect". >>> >>> In our gadget_driver we do the workaround where we notify ourself that >>> we called the usb_gadget_disconnect() and that we should now be >> >> you could just rely on the sysfs interface, right ? :-) > > Not from the gadget driver, at least I don't think so. The gadget > driver itself is the one that wants to initiate the soft disconnect. I need to understand this requirement of yours a little better. Can you describe exactly what your gadget is doing ? Also, any chance of showing the code for that gadget ? I don't mind carrying an extra function driver if it helps you validate your IP :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, John Youn writes: >> John Youn writes: "Du, Changbin" writes: > Hi, Balbi, > > The step to reproduce this issue is: > 1) connect device to a host and wait its enumeration. > 2) trigger software disconnect by calling function > usb_gadget_disconnect(), which finally call >dwc3_gadget_pullup(false). Do not reconnect device > (I mean no enumeration go on, keep bit Run/Stop 0.). > > At here, gadget driver's disconnect callback should be > Called, right? We has been disconnected. But no, as > You said " not generating disconnect IRQ after you > drop Run/Stop is expected". > > And I am testing on an Android device, Android only > use dwc3_gadget_pullup(false) to issue a soft disconnection. > This confused user that the UI still show usb as connected > State, caused by missing a disconnect event. okay, so I know what this is. This is caused by Android gadget itself not notifying the gadget that a disconnect has happened. Just look at udc-core's soft_connect implementation for the sysfs interface, and you'll see what I mean. This should be fixed at Android gadget itself. The only thing we could do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the logic so it's easier for Android gadget to use; but even that I'm a little bit reluctant to do because Android should be using our soft_connect interface instead of reimplementing it (wrongly) by its own. >>> >>> We've run in to the same issue with our usb_gadget_driver. >>> >>> If the usb_gadget_disconnect() API function, which seems like it is >>> intended to be called by the gadget_driver, does cause the gadget to >>> disconnect, it seems reasonable to expect the gadget or the UDC core >>> to notify the gadget_driver via the callback. >> >> Well, the API is supposed to disconnect D+ pullup and that's about it. >> >>> As you mentioned this is handled in the soft_disconnect sysfs. Why >>> shouldn't usb_gadget_disconnect() do the same thing, if not the gadget >> >> because there might be cases where we don't need/want the gadget to know >> about this disconnect. >> > > But what if we do? well, if the gadget is the one faking a disconnect, then it ought to cancel requests and do all the other necessary steps, right ? :-) >>> itself? Exposing the sysfs as an API function would work too. Though >> >> it already _is_ exported. I just don't know why people are re-inventing >> the same solution :-) >> >>> both functions are "soft" disconnects and both are called >>> "disconnect". >>> >>> In our gadget_driver we do the workaround where we notify ourself that >>> we called the usb_gadget_disconnect() and that we should now be >> >> you could just rely on the sysfs interface, right ? :-) > > Not from the gadget driver, at least I don't think so. The gadget > driver itself is the one that wants to initiate the soft disconnect. I need to understand this requirement of yours a little better. Can you describe exactly what your gadget is doing ? Also, any chance of showing the code for that gadget ? I don't mind carrying an extra function driver if it helps you validate your IP :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On 4/28/2016 11:12 PM, Felipe Balbi wrote: > > Hi, > > John Younwrites: >>> "Du, Changbin" writes: Hi, Balbi, The step to reproduce this issue is: 1) connect device to a host and wait its enumeration. 2) trigger software disconnect by calling function usb_gadget_disconnect(), which finally call dwc3_gadget_pullup(false). Do not reconnect device (I mean no enumeration go on, keep bit Run/Stop 0.). At here, gadget driver's disconnect callback should be Called, right? We has been disconnected. But no, as You said " not generating disconnect IRQ after you drop Run/Stop is expected". And I am testing on an Android device, Android only use dwc3_gadget_pullup(false) to issue a soft disconnection. This confused user that the UI still show usb as connected State, caused by missing a disconnect event. >>> >>> okay, so I know what this is. This is caused by Android gadget itself >>> not notifying the gadget that a disconnect has happened. Just look at >>> udc-core's soft_connect implementation for the sysfs interface, and >>> you'll see what I mean. >>> >>> This should be fixed at Android gadget itself. The only thing we could >>> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the >>> logic so it's easier for Android gadget to use; but even that I'm a >>> little bit reluctant to do because Android should be using our >>> soft_connect interface instead of reimplementing it (wrongly) by its >>> own. >>> >> >> We've run in to the same issue with our usb_gadget_driver. >> >> If the usb_gadget_disconnect() API function, which seems like it is >> intended to be called by the gadget_driver, does cause the gadget to >> disconnect, it seems reasonable to expect the gadget or the UDC core >> to notify the gadget_driver via the callback. > > Well, the API is supposed to disconnect D+ pullup and that's about it. > >> As you mentioned this is handled in the soft_disconnect sysfs. Why >> shouldn't usb_gadget_disconnect() do the same thing, if not the gadget > > because there might be cases where we don't need/want the gadget to know > about this disconnect. > But what if we do? >> itself? Exposing the sysfs as an API function would work too. Though > > it already _is_ exported. I just don't know why people are re-inventing > the same solution :-) > >> both functions are "soft" disconnects and both are called >> "disconnect". >> >> In our gadget_driver we do the workaround where we notify ourself that >> we called the usb_gadget_disconnect() and that we should now be > > you could just rely on the sysfs interface, right ? :-) Not from the gadget driver, at least I don't think so. The gadget driver itself is the one that wants to initiate the soft disconnect. > >> disconnected. It just seems more correct that we shouldn't have to >> handle that. >> >> By the way, I'm not completely sure of the correct terminology, but >> I'm referring to the struct usb_gadget (dwc3, dwc2, etc) as the >> "gadget" and the struct usb_gadget_driver as the "gadget_driver" >> (normally this would be the composite gadget framework, but we are >> using our own driver in this case). Is there a less confusing way to >> refer to these :) > > what I've been doing is that I refer to dwc3, dwc3, etc as UDC (as in > USB Device Controller) and g_mass_storage, g_ether, g_zero, etc as > gadget driver. > Ok thanks, that's less confusing than calling them gadgets :) John
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On 4/28/2016 11:12 PM, Felipe Balbi wrote: > > Hi, > > John Youn writes: >>> "Du, Changbin" writes: Hi, Balbi, The step to reproduce this issue is: 1) connect device to a host and wait its enumeration. 2) trigger software disconnect by calling function usb_gadget_disconnect(), which finally call dwc3_gadget_pullup(false). Do not reconnect device (I mean no enumeration go on, keep bit Run/Stop 0.). At here, gadget driver's disconnect callback should be Called, right? We has been disconnected. But no, as You said " not generating disconnect IRQ after you drop Run/Stop is expected". And I am testing on an Android device, Android only use dwc3_gadget_pullup(false) to issue a soft disconnection. This confused user that the UI still show usb as connected State, caused by missing a disconnect event. >>> >>> okay, so I know what this is. This is caused by Android gadget itself >>> not notifying the gadget that a disconnect has happened. Just look at >>> udc-core's soft_connect implementation for the sysfs interface, and >>> you'll see what I mean. >>> >>> This should be fixed at Android gadget itself. The only thing we could >>> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the >>> logic so it's easier for Android gadget to use; but even that I'm a >>> little bit reluctant to do because Android should be using our >>> soft_connect interface instead of reimplementing it (wrongly) by its >>> own. >>> >> >> We've run in to the same issue with our usb_gadget_driver. >> >> If the usb_gadget_disconnect() API function, which seems like it is >> intended to be called by the gadget_driver, does cause the gadget to >> disconnect, it seems reasonable to expect the gadget or the UDC core >> to notify the gadget_driver via the callback. > > Well, the API is supposed to disconnect D+ pullup and that's about it. > >> As you mentioned this is handled in the soft_disconnect sysfs. Why >> shouldn't usb_gadget_disconnect() do the same thing, if not the gadget > > because there might be cases where we don't need/want the gadget to know > about this disconnect. > But what if we do? >> itself? Exposing the sysfs as an API function would work too. Though > > it already _is_ exported. I just don't know why people are re-inventing > the same solution :-) > >> both functions are "soft" disconnects and both are called >> "disconnect". >> >> In our gadget_driver we do the workaround where we notify ourself that >> we called the usb_gadget_disconnect() and that we should now be > > you could just rely on the sysfs interface, right ? :-) Not from the gadget driver, at least I don't think so. The gadget driver itself is the one that wants to initiate the soft disconnect. > >> disconnected. It just seems more correct that we shouldn't have to >> handle that. >> >> By the way, I'm not completely sure of the correct terminology, but >> I'm referring to the struct usb_gadget (dwc3, dwc2, etc) as the >> "gadget" and the struct usb_gadget_driver as the "gadget_driver" >> (normally this would be the composite gadget framework, but we are >> using our own driver in this case). Is there a less confusing way to >> refer to these :) > > what I've been doing is that I refer to dwc3, dwc3, etc as UDC (as in > USB Device Controller) and g_mass_storage, g_ether, g_zero, etc as > gadget driver. > Ok thanks, that's less confusing than calling them gadgets :) John
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, John Younwrites: >> "Du, Changbin" writes: >>> Hi, Balbi, >>> >>> The step to reproduce this issue is: >>> 1) connect device to a host and wait its enumeration. >>> 2) trigger software disconnect by calling function >>> usb_gadget_disconnect(), which finally call >>>dwc3_gadget_pullup(false). Do not reconnect device >>> (I mean no enumeration go on, keep bit Run/Stop 0.). >>> >>> At here, gadget driver's disconnect callback should be >>> Called, right? We has been disconnected. But no, as >>> You said " not generating disconnect IRQ after you >>> drop Run/Stop is expected". >>> >>> And I am testing on an Android device, Android only >>> use dwc3_gadget_pullup(false) to issue a soft disconnection. >>> This confused user that the UI still show usb as connected >>> State, caused by missing a disconnect event. >> >> okay, so I know what this is. This is caused by Android gadget itself >> not notifying the gadget that a disconnect has happened. Just look at >> udc-core's soft_connect implementation for the sysfs interface, and >> you'll see what I mean. >> >> This should be fixed at Android gadget itself. The only thing we could >> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the >> logic so it's easier for Android gadget to use; but even that I'm a >> little bit reluctant to do because Android should be using our >> soft_connect interface instead of reimplementing it (wrongly) by its >> own. >> > > We've run in to the same issue with our usb_gadget_driver. > > If the usb_gadget_disconnect() API function, which seems like it is > intended to be called by the gadget_driver, does cause the gadget to > disconnect, it seems reasonable to expect the gadget or the UDC core > to notify the gadget_driver via the callback. Well, the API is supposed to disconnect D+ pullup and that's about it. > As you mentioned this is handled in the soft_disconnect sysfs. Why > shouldn't usb_gadget_disconnect() do the same thing, if not the gadget because there might be cases where we don't need/want the gadget to know about this disconnect. > itself? Exposing the sysfs as an API function would work too. Though it already _is_ exported. I just don't know why people are re-inventing the same solution :-) > both functions are "soft" disconnects and both are called > "disconnect". > > In our gadget_driver we do the workaround where we notify ourself that > we called the usb_gadget_disconnect() and that we should now be you could just rely on the sysfs interface, right ? :-) > disconnected. It just seems more correct that we shouldn't have to > handle that. > > By the way, I'm not completely sure of the correct terminology, but > I'm referring to the struct usb_gadget (dwc3, dwc2, etc) as the > "gadget" and the struct usb_gadget_driver as the "gadget_driver" > (normally this would be the composite gadget framework, but we are > using our own driver in this case). Is there a less confusing way to > refer to these :) what I've been doing is that I refer to dwc3, dwc3, etc as UDC (as in USB Device Controller) and g_mass_storage, g_ether, g_zero, etc as gadget driver. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, John Youn writes: >> "Du, Changbin" writes: >>> Hi, Balbi, >>> >>> The step to reproduce this issue is: >>> 1) connect device to a host and wait its enumeration. >>> 2) trigger software disconnect by calling function >>> usb_gadget_disconnect(), which finally call >>>dwc3_gadget_pullup(false). Do not reconnect device >>> (I mean no enumeration go on, keep bit Run/Stop 0.). >>> >>> At here, gadget driver's disconnect callback should be >>> Called, right? We has been disconnected. But no, as >>> You said " not generating disconnect IRQ after you >>> drop Run/Stop is expected". >>> >>> And I am testing on an Android device, Android only >>> use dwc3_gadget_pullup(false) to issue a soft disconnection. >>> This confused user that the UI still show usb as connected >>> State, caused by missing a disconnect event. >> >> okay, so I know what this is. This is caused by Android gadget itself >> not notifying the gadget that a disconnect has happened. Just look at >> udc-core's soft_connect implementation for the sysfs interface, and >> you'll see what I mean. >> >> This should be fixed at Android gadget itself. The only thing we could >> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the >> logic so it's easier for Android gadget to use; but even that I'm a >> little bit reluctant to do because Android should be using our >> soft_connect interface instead of reimplementing it (wrongly) by its >> own. >> > > We've run in to the same issue with our usb_gadget_driver. > > If the usb_gadget_disconnect() API function, which seems like it is > intended to be called by the gadget_driver, does cause the gadget to > disconnect, it seems reasonable to expect the gadget or the UDC core > to notify the gadget_driver via the callback. Well, the API is supposed to disconnect D+ pullup and that's about it. > As you mentioned this is handled in the soft_disconnect sysfs. Why > shouldn't usb_gadget_disconnect() do the same thing, if not the gadget because there might be cases where we don't need/want the gadget to know about this disconnect. > itself? Exposing the sysfs as an API function would work too. Though it already _is_ exported. I just don't know why people are re-inventing the same solution :-) > both functions are "soft" disconnects and both are called > "disconnect". > > In our gadget_driver we do the workaround where we notify ourself that > we called the usb_gadget_disconnect() and that we should now be you could just rely on the sysfs interface, right ? :-) > disconnected. It just seems more correct that we shouldn't have to > handle that. > > By the way, I'm not completely sure of the correct terminology, but > I'm referring to the struct usb_gadget (dwc3, dwc2, etc) as the > "gadget" and the struct usb_gadget_driver as the "gadget_driver" > (normally this would be the composite gadget framework, but we are > using our own driver in this case). Is there a less confusing way to > refer to these :) what I've been doing is that I refer to dwc3, dwc3, etc as UDC (as in USB Device Controller) and g_mass_storage, g_ether, g_zero, etc as gadget driver. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On 4/27/2016 11:48 PM, Felipe Balbi wrote: > > Hi, > > (we don't top-post on this forum ;-) > > "Du, Changbin"writes: >> Hi, Balbi, >> >> The step to reproduce this issue is: >> 1) connect device to a host and wait its enumeration. >> 2) trigger software disconnect by calling function >> usb_gadget_disconnect(), which finally call >>dwc3_gadget_pullup(false). Do not reconnect device >> (I mean no enumeration go on, keep bit Run/Stop 0.). >> >> At here, gadget driver's disconnect callback should be >> Called, right? We has been disconnected. But no, as >> You said " not generating disconnect IRQ after you >> drop Run/Stop is expected". >> >> And I am testing on an Android device, Android only >> use dwc3_gadget_pullup(false) to issue a soft disconnection. >> This confused user that the UI still show usb as connected >> State, caused by missing a disconnect event. > > okay, so I know what this is. This is caused by Android gadget itself > not notifying the gadget that a disconnect has happened. Just look at > udc-core's soft_connect implementation for the sysfs interface, and > you'll see what I mean. > > This should be fixed at Android gadget itself. The only thing we could > do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the > logic so it's easier for Android gadget to use; but even that I'm a > little bit reluctant to do because Android should be using our > soft_connect interface instead of reimplementing it (wrongly) by its > own. > We've run in to the same issue with our usb_gadget_driver. If the usb_gadget_disconnect() API function, which seems like it is intended to be called by the gadget_driver, does cause the gadget to disconnect, it seems reasonable to expect the gadget or the UDC core to notify the gadget_driver via the callback. As you mentioned this is handled in the soft_disconnect sysfs. Why shouldn't usb_gadget_disconnect() do the same thing, if not the gadget itself? Exposing the sysfs as an API function would work too. Though both functions are "soft" disconnects and both are called "disconnect". In our gadget_driver we do the workaround where we notify ourself that we called the usb_gadget_disconnect() and that we should now be disconnected. It just seems more correct that we shouldn't have to handle that. By the way, I'm not completely sure of the correct terminology, but I'm referring to the struct usb_gadget (dwc3, dwc2, etc) as the "gadget" and the struct usb_gadget_driver as the "gadget_driver" (normally this would be the composite gadget framework, but we are using our own driver in this case). Is there a less confusing way to refer to these :) John
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
On 4/27/2016 11:48 PM, Felipe Balbi wrote: > > Hi, > > (we don't top-post on this forum ;-) > > "Du, Changbin" writes: >> Hi, Balbi, >> >> The step to reproduce this issue is: >> 1) connect device to a host and wait its enumeration. >> 2) trigger software disconnect by calling function >> usb_gadget_disconnect(), which finally call >>dwc3_gadget_pullup(false). Do not reconnect device >> (I mean no enumeration go on, keep bit Run/Stop 0.). >> >> At here, gadget driver's disconnect callback should be >> Called, right? We has been disconnected. But no, as >> You said " not generating disconnect IRQ after you >> drop Run/Stop is expected". >> >> And I am testing on an Android device, Android only >> use dwc3_gadget_pullup(false) to issue a soft disconnection. >> This confused user that the UI still show usb as connected >> State, caused by missing a disconnect event. > > okay, so I know what this is. This is caused by Android gadget itself > not notifying the gadget that a disconnect has happened. Just look at > udc-core's soft_connect implementation for the sysfs interface, and > you'll see what I mean. > > This should be fixed at Android gadget itself. The only thing we could > do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the > logic so it's easier for Android gadget to use; but even that I'm a > little bit reluctant to do because Android should be using our > soft_connect interface instead of reimplementing it (wrongly) by its > own. > We've run in to the same issue with our usb_gadget_driver. If the usb_gadget_disconnect() API function, which seems like it is intended to be called by the gadget_driver, does cause the gadget to disconnect, it seems reasonable to expect the gadget or the UDC core to notify the gadget_driver via the callback. As you mentioned this is handled in the soft_disconnect sysfs. Why shouldn't usb_gadget_disconnect() do the same thing, if not the gadget itself? Exposing the sysfs as an API function would work too. Though both functions are "soft" disconnects and both are called "disconnect". In our gadget_driver we do the workaround where we notify ourself that we called the usb_gadget_disconnect() and that we should now be disconnected. It just seems more correct that we shouldn't have to handle that. By the way, I'm not completely sure of the correct terminology, but I'm referring to the struct usb_gadget (dwc3, dwc2, etc) as the "gadget" and the struct usb_gadget_driver as the "gadget_driver" (normally this would be the composite gadget framework, but we are using our own driver in this case). Is there a less confusing way to refer to these :) John
RE: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, (we don't top-post on this forum ;-) "Du, Changbin"writes: > Hi, Balbi, > > The step to reproduce this issue is: > 1) connect device to a host and wait its enumeration. > 2) trigger software disconnect by calling function > usb_gadget_disconnect(), which finally call >dwc3_gadget_pullup(false). Do not reconnect device > (I mean no enumeration go on, keep bit Run/Stop 0.). > > At here, gadget driver's disconnect callback should be > Called, right? We has been disconnected. But no, as > You said " not generating disconnect IRQ after you > drop Run/Stop is expected". > > And I am testing on an Android device, Android only > use dwc3_gadget_pullup(false) to issue a soft disconnection. > This confused user that the UI still show usb as connected > State, caused by missing a disconnect event. okay, so I know what this is. This is caused by Android gadget itself not notifying the gadget that a disconnect has happened. Just look at udc-core's soft_connect implementation for the sysfs interface, and you'll see what I mean. This should be fixed at Android gadget itself. The only thing we could do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the logic so it's easier for Android gadget to use; but even that I'm a little bit reluctant to do because Android should be using our soft_connect interface instead of reimplementing it (wrongly) by its own. -- balbi signature.asc Description: PGP signature
RE: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, (we don't top-post on this forum ;-) "Du, Changbin" writes: > Hi, Balbi, > > The step to reproduce this issue is: > 1) connect device to a host and wait its enumeration. > 2) trigger software disconnect by calling function > usb_gadget_disconnect(), which finally call >dwc3_gadget_pullup(false). Do not reconnect device > (I mean no enumeration go on, keep bit Run/Stop 0.). > > At here, gadget driver's disconnect callback should be > Called, right? We has been disconnected. But no, as > You said " not generating disconnect IRQ after you > drop Run/Stop is expected". > > And I am testing on an Android device, Android only > use dwc3_gadget_pullup(false) to issue a soft disconnection. > This confused user that the UI still show usb as connected > State, caused by missing a disconnect event. okay, so I know what this is. This is caused by Android gadget itself not notifying the gadget that a disconnect has happened. Just look at udc-core's soft_connect implementation for the sysfs interface, and you'll see what I mean. This should be fixed at Android gadget itself. The only thing we could do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the logic so it's easier for Android gadget to use; but even that I'm a little bit reluctant to do because Android should be using our soft_connect interface instead of reimplementing it (wrongly) by its own. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hello. On 4/27/2016 11:29 AM, changbin...@intel.com wrote: From: "Du, Changbin"The dwc3 controller can't generate a disconnect event after it is stopped. Thus gadget dissconnect callback is not invoked when do soft dissconnect. Call dissconnect here to workaround this issue. "Disconnect" everywhere. Note, most time we still see disconnect be called that because it is invoked by dwc3_gadget_reset_interrupt(). But if we disconnect cable once pullup disabled quickly, issue can be observed. Signed-off-by: Du, Changbin --- drivers/usb/dwc3/gadget.c | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 8e4a1b1..cd73187 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c [...] @@ -1575,6 +1584,21 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) is_on = !!is_on; spin_lock_irqsave(>lock, flags); + /** +* WORKAROUND: The dwc3 controller can't generate a disconnect +* event after it is stopped. Thus gadget dissconnect callback +* is not invoked when do soft dissconnect. Call dissconnect +* here to workaround this issue. "Disconnect" everywhere. +* Note, most time we still see disconnect be called that because I couldn't parse that. +* it is invoked by dwc3_gadget_reset_interrupt(). But if we +* disconnect cable once pullup disabled quickly, issue can be +* observed. +*/ + if (!is_on && (dwc->gadget.speed != USB_SPEED_UNKNOWN)) { + dev_dbg(dwc->dev, "fake dissconnect event on pullup off\n"); Disconnect. + dwc3_disconnect_gadget(dwc); + dwc->gadget.speed = USB_SPEED_UNKNOWN; + } ret = dwc3_gadget_run_stop(dwc, is_on, false); spin_unlock_irqrestore(>lock, flags); [...] MBR, Sergei
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hello. On 4/27/2016 11:29 AM, changbin...@intel.com wrote: From: "Du, Changbin" The dwc3 controller can't generate a disconnect event after it is stopped. Thus gadget dissconnect callback is not invoked when do soft dissconnect. Call dissconnect here to workaround this issue. "Disconnect" everywhere. Note, most time we still see disconnect be called that because it is invoked by dwc3_gadget_reset_interrupt(). But if we disconnect cable once pullup disabled quickly, issue can be observed. Signed-off-by: Du, Changbin --- drivers/usb/dwc3/gadget.c | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 8e4a1b1..cd73187 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c [...] @@ -1575,6 +1584,21 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) is_on = !!is_on; spin_lock_irqsave(>lock, flags); + /** +* WORKAROUND: The dwc3 controller can't generate a disconnect +* event after it is stopped. Thus gadget dissconnect callback +* is not invoked when do soft dissconnect. Call dissconnect +* here to workaround this issue. "Disconnect" everywhere. +* Note, most time we still see disconnect be called that because I couldn't parse that. +* it is invoked by dwc3_gadget_reset_interrupt(). But if we +* disconnect cable once pullup disabled quickly, issue can be +* observed. +*/ + if (!is_on && (dwc->gadget.speed != USB_SPEED_UNKNOWN)) { + dev_dbg(dwc->dev, "fake dissconnect event on pullup off\n"); Disconnect. + dwc3_disconnect_gadget(dwc); + dwc->gadget.speed = USB_SPEED_UNKNOWN; + } ret = dwc3_gadget_run_stop(dwc, is_on, false); spin_unlock_irqrestore(>lock, flags); [...] MBR, Sergei
RE: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, Balbi, The step to reproduce this issue is: 1) connect device to a host and wait its enumeration. 2) trigger software disconnect by calling function usb_gadget_disconnect(), which finally call dwc3_gadget_pullup(false). Do not reconnect device (I mean no enumeration go on, keep bit Run/Stop 0.). At here, gadget driver's disconnect callback should be Called, right? We has been disconnected. But no, as You said " not generating disconnect IRQ after you drop Run/Stop is expected". And I am testing on an Android device, Android only use dwc3_gadget_pullup(false) to issue a soft disconnection. This confused user that the UI still show usb as connected State, caused by missing a disconnect event. > Hi, > > changbin...@intel.com writes: > > From: "Du, Changbin"> > > > The dwc3 controller can't generate a disconnect event after it is > > stopped. Thus gadget dissconnect callback is not invoked when do > > not generating disconnect IRQ after you drop Run/Stop is expected. > > > soft dissconnect. Call dissconnect here to workaround this issue. > > I'm rather sure this is a bug elsewhere. How do you trigger this ? > > > Note, most time we still see disconnect be called that because > > it is invoked by dwc3_gadget_reset_interrupt(). But if we > > disconnect cable once pullup disabled quickly, issue can be > > observed. > > I can't understand what you're trying to say with this. > > > Signed-off-by: Du, Changbin > > --- > > drivers/usb/dwc3/gadget.c | 33 - > > 1 file changed, 24 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index 8e4a1b1..cd73187 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -1566,6 +1566,15 @@ static int dwc3_gadget_run_stop(struct dwc3 > *dwc, int is_on, int suspend) > > return 0; > > } > > > > +static void dwc3_disconnect_gadget(struct dwc3 *dwc) > > +{ > > + if (dwc->gadget_driver && dwc->gadget_driver->disconnect) { > > + spin_unlock(>lock); > > + dwc->gadget_driver->disconnect(>gadget); > > + spin_lock(>lock); > > + } > > +} > > + > > static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > > { > > struct dwc3 *dwc = gadget_to_dwc(g); > > @@ -1575,6 +1584,21 @@ static int dwc3_gadget_pullup(struct usb_gadget > *g, int is_on) > > is_on = !!is_on; > > > > spin_lock_irqsave(>lock, flags); > > + /** > > +* WORKAROUND: The dwc3 controller can't generate a disconnect > > +* event after it is stopped. Thus gadget dissconnect callback > > +* is not invoked when do soft dissconnect. Call dissconnect > > +* here to workaround this issue. > > +* Note, most time we still see disconnect be called that because > > +* it is invoked by dwc3_gadget_reset_interrupt(). But if we > > +* disconnect cable once pullup disabled quickly, issue can be > > +* observed. > > +*/ > > + if (!is_on && (dwc->gadget.speed != USB_SPEED_UNKNOWN)) { > > + dev_dbg(dwc->dev, "fake dissconnect event on pullup > off\n"); > > + dwc3_disconnect_gadget(dwc); > > + dwc->gadget.speed = USB_SPEED_UNKNOWN; > > + } > > this is *really* wrong. You shouldn't be calling pullup directly. This > patch looks wrong and you didn't even explain how to trigger this > problem; when does the problem happen ? > > Gadget load/unload does the right thing here, so that can't be the > case. We also do the right thing on soft_connect sysfs file: > > static ssize_t usb_udc_softconn_store(struct device *dev, > struct device_attribute *attr, const char *buf, size_t n) > { > struct usb_udc *udc = container_of(dev, struct usb_udc, > dev); > > if (!udc->driver) { > dev_err(dev, "soft-connect without a gadget driver\n"); > return -EOPNOTSUPP; > } > > if (sysfs_streq(buf, "connect")) { > usb_gadget_udc_start(udc); > usb_gadget_connect(udc->gadget); > } else if (sysfs_streq(buf, "disconnect")) { > usb_gadget_disconnect(udc->gadget); > udc->driver->disconnect(udc->gadget); > usb_gadget_udc_stop(udc); > } else { > dev_err(dev, "unsupported command '%s'\n", buf); > return -EINVAL; > } > > return n; > } > static DEVICE_ATTR(soft_connect, S_IWUSR, NULL, > usb_udc_softconn_store); > > So really, what _is_ the problem ? > > -- > balbi
RE: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, Balbi, The step to reproduce this issue is: 1) connect device to a host and wait its enumeration. 2) trigger software disconnect by calling function usb_gadget_disconnect(), which finally call dwc3_gadget_pullup(false). Do not reconnect device (I mean no enumeration go on, keep bit Run/Stop 0.). At here, gadget driver's disconnect callback should be Called, right? We has been disconnected. But no, as You said " not generating disconnect IRQ after you drop Run/Stop is expected". And I am testing on an Android device, Android only use dwc3_gadget_pullup(false) to issue a soft disconnection. This confused user that the UI still show usb as connected State, caused by missing a disconnect event. > Hi, > > changbin...@intel.com writes: > > From: "Du, Changbin" > > > > The dwc3 controller can't generate a disconnect event after it is > > stopped. Thus gadget dissconnect callback is not invoked when do > > not generating disconnect IRQ after you drop Run/Stop is expected. > > > soft dissconnect. Call dissconnect here to workaround this issue. > > I'm rather sure this is a bug elsewhere. How do you trigger this ? > > > Note, most time we still see disconnect be called that because > > it is invoked by dwc3_gadget_reset_interrupt(). But if we > > disconnect cable once pullup disabled quickly, issue can be > > observed. > > I can't understand what you're trying to say with this. > > > Signed-off-by: Du, Changbin > > --- > > drivers/usb/dwc3/gadget.c | 33 - > > 1 file changed, 24 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index 8e4a1b1..cd73187 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -1566,6 +1566,15 @@ static int dwc3_gadget_run_stop(struct dwc3 > *dwc, int is_on, int suspend) > > return 0; > > } > > > > +static void dwc3_disconnect_gadget(struct dwc3 *dwc) > > +{ > > + if (dwc->gadget_driver && dwc->gadget_driver->disconnect) { > > + spin_unlock(>lock); > > + dwc->gadget_driver->disconnect(>gadget); > > + spin_lock(>lock); > > + } > > +} > > + > > static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > > { > > struct dwc3 *dwc = gadget_to_dwc(g); > > @@ -1575,6 +1584,21 @@ static int dwc3_gadget_pullup(struct usb_gadget > *g, int is_on) > > is_on = !!is_on; > > > > spin_lock_irqsave(>lock, flags); > > + /** > > +* WORKAROUND: The dwc3 controller can't generate a disconnect > > +* event after it is stopped. Thus gadget dissconnect callback > > +* is not invoked when do soft dissconnect. Call dissconnect > > +* here to workaround this issue. > > +* Note, most time we still see disconnect be called that because > > +* it is invoked by dwc3_gadget_reset_interrupt(). But if we > > +* disconnect cable once pullup disabled quickly, issue can be > > +* observed. > > +*/ > > + if (!is_on && (dwc->gadget.speed != USB_SPEED_UNKNOWN)) { > > + dev_dbg(dwc->dev, "fake dissconnect event on pullup > off\n"); > > + dwc3_disconnect_gadget(dwc); > > + dwc->gadget.speed = USB_SPEED_UNKNOWN; > > + } > > this is *really* wrong. You shouldn't be calling pullup directly. This > patch looks wrong and you didn't even explain how to trigger this > problem; when does the problem happen ? > > Gadget load/unload does the right thing here, so that can't be the > case. We also do the right thing on soft_connect sysfs file: > > static ssize_t usb_udc_softconn_store(struct device *dev, > struct device_attribute *attr, const char *buf, size_t n) > { > struct usb_udc *udc = container_of(dev, struct usb_udc, > dev); > > if (!udc->driver) { > dev_err(dev, "soft-connect without a gadget driver\n"); > return -EOPNOTSUPP; > } > > if (sysfs_streq(buf, "connect")) { > usb_gadget_udc_start(udc); > usb_gadget_connect(udc->gadget); > } else if (sysfs_streq(buf, "disconnect")) { > usb_gadget_disconnect(udc->gadget); > udc->driver->disconnect(udc->gadget); > usb_gadget_udc_stop(udc); > } else { > dev_err(dev, "unsupported command '%s'\n", buf); > return -EINVAL; > } > > return n; > } > static DEVICE_ATTR(soft_connect, S_IWUSR, NULL, > usb_udc_softconn_store); > > So really, what _is_ the problem ? > > -- > balbi
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, changbin...@intel.com writes: > From: "Du, Changbin"> > The dwc3 controller can't generate a disconnect event after it is > stopped. Thus gadget dissconnect callback is not invoked when do not generating disconnect IRQ after you drop Run/Stop is expected. > soft dissconnect. Call dissconnect here to workaround this issue. I'm rather sure this is a bug elsewhere. How do you trigger this ? > Note, most time we still see disconnect be called that because > it is invoked by dwc3_gadget_reset_interrupt(). But if we > disconnect cable once pullup disabled quickly, issue can be > observed. I can't understand what you're trying to say with this. > Signed-off-by: Du, Changbin > --- > drivers/usb/dwc3/gadget.c | 33 - > 1 file changed, 24 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 8e4a1b1..cd73187 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1566,6 +1566,15 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int > is_on, int suspend) > return 0; > } > > +static void dwc3_disconnect_gadget(struct dwc3 *dwc) > +{ > + if (dwc->gadget_driver && dwc->gadget_driver->disconnect) { > + spin_unlock(>lock); > + dwc->gadget_driver->disconnect(>gadget); > + spin_lock(>lock); > + } > +} > + > static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > { > struct dwc3 *dwc = gadget_to_dwc(g); > @@ -1575,6 +1584,21 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, > int is_on) > is_on = !!is_on; > > spin_lock_irqsave(>lock, flags); > + /** > + * WORKAROUND: The dwc3 controller can't generate a disconnect > + * event after it is stopped. Thus gadget dissconnect callback > + * is not invoked when do soft dissconnect. Call dissconnect > + * here to workaround this issue. > + * Note, most time we still see disconnect be called that because > + * it is invoked by dwc3_gadget_reset_interrupt(). But if we > + * disconnect cable once pullup disabled quickly, issue can be > + * observed. > + */ > + if (!is_on && (dwc->gadget.speed != USB_SPEED_UNKNOWN)) { > + dev_dbg(dwc->dev, "fake dissconnect event on pullup off\n"); > + dwc3_disconnect_gadget(dwc); > + dwc->gadget.speed = USB_SPEED_UNKNOWN; > + } this is *really* wrong. You shouldn't be calling pullup directly. This patch looks wrong and you didn't even explain how to trigger this problem; when does the problem happen ? Gadget load/unload does the right thing here, so that can't be the case. We also do the right thing on soft_connect sysfs file: static ssize_t usb_udc_softconn_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t n) { struct usb_udc *udc = container_of(dev, struct usb_udc, dev); if (!udc->driver) { dev_err(dev, "soft-connect without a gadget driver\n"); return -EOPNOTSUPP; } if (sysfs_streq(buf, "connect")) { usb_gadget_udc_start(udc); usb_gadget_connect(udc->gadget); } else if (sysfs_streq(buf, "disconnect")) { usb_gadget_disconnect(udc->gadget); udc->driver->disconnect(udc->gadget); usb_gadget_udc_stop(udc); } else { dev_err(dev, "unsupported command '%s'\n", buf); return -EINVAL; } return n; } static DEVICE_ATTR(soft_connect, S_IWUSR, NULL, usb_udc_softconn_store); So really, what _is_ the problem ? -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup
Hi, changbin...@intel.com writes: > From: "Du, Changbin" > > The dwc3 controller can't generate a disconnect event after it is > stopped. Thus gadget dissconnect callback is not invoked when do not generating disconnect IRQ after you drop Run/Stop is expected. > soft dissconnect. Call dissconnect here to workaround this issue. I'm rather sure this is a bug elsewhere. How do you trigger this ? > Note, most time we still see disconnect be called that because > it is invoked by dwc3_gadget_reset_interrupt(). But if we > disconnect cable once pullup disabled quickly, issue can be > observed. I can't understand what you're trying to say with this. > Signed-off-by: Du, Changbin > --- > drivers/usb/dwc3/gadget.c | 33 - > 1 file changed, 24 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 8e4a1b1..cd73187 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1566,6 +1566,15 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int > is_on, int suspend) > return 0; > } > > +static void dwc3_disconnect_gadget(struct dwc3 *dwc) > +{ > + if (dwc->gadget_driver && dwc->gadget_driver->disconnect) { > + spin_unlock(>lock); > + dwc->gadget_driver->disconnect(>gadget); > + spin_lock(>lock); > + } > +} > + > static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > { > struct dwc3 *dwc = gadget_to_dwc(g); > @@ -1575,6 +1584,21 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, > int is_on) > is_on = !!is_on; > > spin_lock_irqsave(>lock, flags); > + /** > + * WORKAROUND: The dwc3 controller can't generate a disconnect > + * event after it is stopped. Thus gadget dissconnect callback > + * is not invoked when do soft dissconnect. Call dissconnect > + * here to workaround this issue. > + * Note, most time we still see disconnect be called that because > + * it is invoked by dwc3_gadget_reset_interrupt(). But if we > + * disconnect cable once pullup disabled quickly, issue can be > + * observed. > + */ > + if (!is_on && (dwc->gadget.speed != USB_SPEED_UNKNOWN)) { > + dev_dbg(dwc->dev, "fake dissconnect event on pullup off\n"); > + dwc3_disconnect_gadget(dwc); > + dwc->gadget.speed = USB_SPEED_UNKNOWN; > + } this is *really* wrong. You shouldn't be calling pullup directly. This patch looks wrong and you didn't even explain how to trigger this problem; when does the problem happen ? Gadget load/unload does the right thing here, so that can't be the case. We also do the right thing on soft_connect sysfs file: static ssize_t usb_udc_softconn_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t n) { struct usb_udc *udc = container_of(dev, struct usb_udc, dev); if (!udc->driver) { dev_err(dev, "soft-connect without a gadget driver\n"); return -EOPNOTSUPP; } if (sysfs_streq(buf, "connect")) { usb_gadget_udc_start(udc); usb_gadget_connect(udc->gadget); } else if (sysfs_streq(buf, "disconnect")) { usb_gadget_disconnect(udc->gadget); udc->driver->disconnect(udc->gadget); usb_gadget_udc_stop(udc); } else { dev_err(dev, "unsupported command '%s'\n", buf); return -EINVAL; } return n; } static DEVICE_ATTR(soft_connect, S_IWUSR, NULL, usb_udc_softconn_store); So really, what _is_ the problem ? -- balbi signature.asc Description: PGP signature