usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup

From: "Du, Changbin" <[email protected]>

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.

Signed-off-by Du, Changbin <[email protected]>

---
 drivers/usb/dwc3/gadget.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

drivers/usb/dwc3/gadget.c +14 -0

index 8e4a1b1..cd73187 100644
@@ -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(&dwc->lock);
+		dwc->gadget_driver->disconnect(&dwc->gadget);
+		spin_lock(&dwc->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(&dwc->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.

Hi,

Quoted 4 lines · [email protected] writes:

From: "Du, Changbin" <[email protected]>

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.

Quoted 1 line

soft dissconnect. Call dissconnect here to workaround this issue.

I'm rather sure this is a bug elsewhere. How do you trigger this ?

Quoted 4 lines

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.

drivers/usb/dwc3/gadget.c static int dwc3_gadget_run_stop(struct dwc3

Quoted 10 lines

Signed-off-by Du, Changbin <[email protected]>

---
 drivers/usb/dwc3/gadget.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

drivers/usb/dwc3/gadget.c +0 -0

index 8e4a1b1..cd73187 100644
@@ -1566,6 +1566,15 @@ static int dwc3_gadget_run_stop(struct dwc3

*dwc, int is_on, int suspend)

drivers/usb/dwc3/gadget.c static int dwc3_gadget_pullup(struct usb_gadget

Quoted 16 lines

drivers/usb/dwc3/gadget.c +9 -0

 	return 0;
 }
 
+static void dwc3_disconnect_gadget(struct dwc3 *dwc)
+{
+	if (dwc->gadget_driver && dwc->gadget_driver->disconnect) {
+		spin_unlock(&dwc->lock);
+		dwc->gadget_driver->disconnect(&dwc->gadget);
+		spin_lock(&dwc->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)

Quoted 5 lines

drivers/usb/dwc3/gadget.c +2 -0

 	is_on = !!is_on;
 
 	spin_lock_irqsave(&dwc->lock, flags);
+	/**
+	 * WORKAROUND: The dwc3 controller can't generate a disconnect

drivers/usb/dwc3/gadget.c static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)

Quoted 10 lines

drivers/usb/dwc3/gadget.c +10 -0

+	 * 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");

drivers/usb/dwc3/gadget.c static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) · line 1601

Quoted 3 lines

drivers/usb/dwc3/gadget.c +3 -0

+		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

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,

(we don't top-post on this forum ;-)

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 25 lines

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 7 lines

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 :)

Hi,

Quoted 33 lines · John Youn

Quoted 25 lines

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 2 lines

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.

Quoted 1 line

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 :-)

Quoted 5 lines

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 ? :-)

Quoted 10 lines

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.

Quoted 43 lines

Quoted 33 lines · John Youn

Quoted 25 lines

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 6 lines

Quoted 2 lines

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?

Quoted 8 lines

Quoted 1 line

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 :-)

Quoted 3 lines

both functions are "soft" disconnects and both are called "disconnect".

Quoted 4 lines

Quoted 2 lines

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.

Hi,

Quoted 46 lines · John Youn

Quoted 46 lines

Quoted 33 lines · John Youn

Quoted 25 lines

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 2 lines

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

Quoted 5 lines

Quoted 3 lines

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 ? :-)

Quoted 13 lines

Quoted 12 lines

Quoted 1 line

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 :-)

Quoted 5 lines

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 ? :-)

Quoted 2 lines

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 :-)

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Quoted 2 lines

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.

Quoted 3 lines

Quoted 1 line

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 :)

Quoted 2 lines

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.

Quoted 13 lines

Quoted 13 lines

Quoted 12 lines

Quoted 1 line

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 :-)

Quoted 5 lines

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 ? :-)

Quoted 8 lines

Quoted 2 lines

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.

Hi,

(we don't top-post on this forum ;-)

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 25 lines

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 7 lines

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?

Hi,

Quoted 37 lines · Peter Chen

Quoted 25 lines

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 3 lines

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 ?

Quoted 38 lines

Quoted 37 lines · Peter Chen

Quoted 25 lines

Quoted 14 lines · Changbin

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".

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 7 lines

Quoted 3 lines

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:)

From: "Du, Changbin" <[email protected]>

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.

Signed-off-by Du, Changbin <[email protected]>

---
 drivers/usb/dwc3/gadget.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

drivers/usb/dwc3/gadget.c +0 -0

index 8e4a1b1..cd73187 100644