| Message ID | Pine.LNX.4.44L0.1808071517570.1385-100000@iolanthe.rowland.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Should usb_gadget_disconnect() call driver's ->disconnect? | expand |
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?
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
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?
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
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 :)
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)