Should usb_gadget_disconnect() call driver's ->disconnect?

Should usb_gadget_disconnect() call driver's ->disconnect?

[Felipe Balbi]

Hi,

Alan Stern <stern@rowland.harvard.edu> writes:
>> Alan Stern <stern@rowland.harvard.edu> writes:
>> > Felipe:
>> >
>> > The documentation doesn't state whether a gadget driver's 
>> > ->disconnect() callback will be invoked when usb_gadget_disconnect() 
>> > runs.  Probably the UDC drivers' behavior has changed over the years.
>> >
>> > In any case, it's likely that various UDC drivers do behave
>> > differently.  My current feeling is that ->disconnect() should be
>> > invoked only when Vbus turns off, but I can't guarantee that all UDCs 
>> > will do this.
>> 
>> my feeling is that ->disconnect() should be called everytime we cause
>> the session to be killed. VBUS Turning off is a guaratee that session is
>> gone, but so is disconnecting data pullup, which is what
>> usb_gadget_disconnect() does.
>
> Okay.  I don't mind either way, so long as it is settled.
>
>> > How do you feel about documenting this ambiguity as in the patch below?
>> > If you think this is okay, I'll submit it formally.
>> 
>> Perhaps a better approach would be to start auditing all UDCs to make
>> sure that ->disconnect() is called when pullup is disconnected. Then we
>> can move ->disconnect() to usb_gadget_disconnect() itself. No?
>
> Rather the other way around: Call ->disconnect() from
> usb_gadget_disconnect() itself, then audit all the UDCs to make sure
> they don't invoke the callbackup.  That's better than adding a callback
> to the UDCs that are missing it, only then to remove it from all of
> them later on.

Fair enough. Do you want to handle this or would you prefer that I add
to my list?

Should usb_gadget_disconnect() call driver's ->disconnect?

[Felipe Balbi]

Hi,

Alan Stern <stern@rowland.harvard.edu> writes:
>> >> > The documentation doesn't state whether a gadget driver's 
>> >> > ->disconnect() callback will be invoked when usb_gadget_disconnect() 
>> >> > runs.  Probably the UDC drivers' behavior has changed over the years.
>> >> >
>> >> > In any case, it's likely that various UDC drivers do behave
>> >> > differently.  My current feeling is that ->disconnect() should be
>> >> > invoked only when Vbus turns off, but I can't guarantee that all UDCs 
>> >> > will do this.
>> >> 
>> >> my feeling is that ->disconnect() should be called everytime we cause
>> >> the session to be killed. VBUS Turning off is a guaratee that session is
>> >> gone, but so is disconnecting data pullup, which is what
>> >> usb_gadget_disconnect() does.
>> >
>> > Okay.  I don't mind either way, so long as it is settled.
>> >
>> >> > How do you feel about documenting this ambiguity as in the patch below?
>> >> > If you think this is okay, I'll submit it formally.
>> >> 
>> >> Perhaps a better approach would be to start auditing all UDCs to make
>> >> sure that ->disconnect() is called when pullup is disconnected. Then we
>> >> can move ->disconnect() to usb_gadget_disconnect() itself. No?
>> >
>> > Rather the other way around: Call ->disconnect() from
>> > usb_gadget_disconnect() itself, then audit all the UDCs to make sure
>> > they don't invoke the callbackup.  That's better than adding a callback
>> > to the UDCs that are missing it, only then to remove it from all of
>> > them later on.
>> 
>> Fair enough. Do you want to handle this or would you prefer that I add
>> to my list?
>
> I'll send in a patch modifying usb_gadget_disconnect(), and I'll check 
> up on dummy-hcd and net2280.  The rest I prefer to leave for you.  
> Okay?

Sure, no issues :)

Should usb_gadget_disconnect() call driver's ->disconnect?

[Alan Stern]

On Thu, 9 Aug 2018, Felipe Balbi wrote:

> 
> Hi,
> 
> Alan Stern <stern@rowland.harvard.edu> writes:
> >> Alan Stern <stern@rowland.harvard.edu> writes:
> >> > Felipe:
> >> >
> >> > The documentation doesn't state whether a gadget driver's 
> >> > ->disconnect() callback will be invoked when usb_gadget_disconnect() 
> >> > runs.  Probably the UDC drivers' behavior has changed over the years.
> >> >
> >> > In any case, it's likely that various UDC drivers do behave
> >> > differently.  My current feeling is that ->disconnect() should be
> >> > invoked only when Vbus turns off, but I can't guarantee that all UDCs 
> >> > will do this.
> >> 
> >> my feeling is that ->disconnect() should be called everytime we cause
> >> the session to be killed. VBUS Turning off is a guaratee that session is
> >> gone, but so is disconnecting data pullup, which is what
> >> usb_gadget_disconnect() does.
> >
> > Okay.  I don't mind either way, so long as it is settled.
> >
> >> > How do you feel about documenting this ambiguity as in the patch below?
> >> > If you think this is okay, I'll submit it formally.
> >> 
> >> Perhaps a better approach would be to start auditing all UDCs to make
> >> sure that ->disconnect() is called when pullup is disconnected. Then we
> >> can move ->disconnect() to usb_gadget_disconnect() itself. No?
> >
> > Rather the other way around: Call ->disconnect() from
> > usb_gadget_disconnect() itself, then audit all the UDCs to make sure
> > they don't invoke the callbackup.  That's better than adding a callback
> > to the UDCs that are missing it, only then to remove it from all of
> > them later on.
> 
> Fair enough. Do you want to handle this or would you prefer that I add
> to my list?

I'll send in a patch modifying usb_gadget_disconnect(), and I'll check 
up on dummy-hcd and net2280.  The rest I prefer to leave for you.  
Okay?

Alan Stern
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Should usb_gadget_disconnect() call driver's ->disconnect?

[Alan Stern]

On Wed, 8 Aug 2018, Felipe Balbi wrote:

> 
> Hi,
> 
> Alan Stern <stern@rowland.harvard.edu> writes:
> > Felipe:
> >
> > The documentation doesn't state whether a gadget driver's 
> > ->disconnect() callback will be invoked when usb_gadget_disconnect() 
> > runs.  Probably the UDC drivers' behavior has changed over the years.
> >
> > In any case, it's likely that various UDC drivers do behave
> > differently.  My current feeling is that ->disconnect() should be
> > invoked only when Vbus turns off, but I can't guarantee that all UDCs 
> > will do this.
> 
> my feeling is that ->disconnect() should be called everytime we cause
> the session to be killed. VBUS Turning off is a guaratee that session is
> gone, but so is disconnecting data pullup, which is what
> usb_gadget_disconnect() does.

Okay.  I don't mind either way, so long as it is settled.

> > How do you feel about documenting this ambiguity as in the patch below?
> > If you think this is okay, I'll submit it formally.
> 
> Perhaps a better approach would be to start auditing all UDCs to make
> sure that ->disconnect() is called when pullup is disconnected. Then we
> can move ->disconnect() to usb_gadget_disconnect() itself. No?

Rather the other way around: Call ->disconnect() from
usb_gadget_disconnect() itself, then audit all the UDCs to make sure
they don't invoke the callbackup.  That's better than adding a callback
to the UDCs that are missing it, only then to remove it from all of
them later on.

Alan Stern
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Should usb_gadget_disconnect() call driver's ->disconnect?

[Felipe Balbi]

Hi,

Alan Stern <stern@rowland.harvard.edu> writes:
> Felipe:
>
> The documentation doesn't state whether a gadget driver's 
> ->disconnect() callback will be invoked when usb_gadget_disconnect() 
> runs.  Probably the UDC drivers' behavior has changed over the years.
>
> In any case, it's likely that various UDC drivers do behave
> differently.  My current feeling is that ->disconnect() should be
> invoked only when Vbus turns off, but I can't guarantee that all UDCs 
> will do this.

my feeling is that ->disconnect() should be called everytime we cause
the session to be killed. VBUS Turning off is a guaratee that session is
gone, but so is disconnecting data pullup, which is what
usb_gadget_disconnect() does.

> How do you feel about documenting this ambiguity as in the patch below?
> If you think this is okay, I'll submit it formally.

Perhaps a better approach would be to start auditing all UDCs to make
sure that ->disconnect() is called when pullup is disconnected. Then we
can move ->disconnect() to usb_gadget_disconnect() itself. No?

Should usb_gadget_disconnect() call driver's ->disconnect?

[Alan Stern]

Felipe:

The documentation doesn't state whether a gadget driver's 
->disconnect() callback will be invoked when usb_gadget_disconnect() 
runs.  Probably the UDC drivers' behavior has changed over the years.

In any case, it's likely that various UDC drivers do behave
differently.  My current feeling is that ->disconnect() should be
invoked only when Vbus turns off, but I can't guarantee that all UDCs 
will do this.

How do you feel about documenting this ambiguity as in the patch below?
If you think this is okay, I'll submit it formally.

Alan Stern
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Index: usb-4.x/drivers/usb/gadget/udc/core.c
===================================================================
--- usb-4.x.orig/drivers/usb/gadget/udc/core.c
+++ usb-4.x/drivers/usb/gadget/udc/core.c
@@ -672,6 +672,10 @@ EXPORT_SYMBOL_GPL(usb_gadget_connect);
  * as a disconnect (when a VBUS session is active).  Not all systems
  * support software pullup controls.
  *
+ * Whether or not the gadget driver's ->disconnect() callback will be
+ * invoked is undefined.  Gadget drivers should be prepared for either
+ * possibility.
+ *
  * Returns zero on success, else negative errno.
  */
 int usb_gadget_disconnect(struct usb_gadget *gadget)