Hi,
On Tue, Sep 02, 2014 at 01:15:18PM -0400, Alan Stern wrote:
> On Tue, 2 Sep 2014, Felipe Balbi wrote:
>
> > > That needn't be a problem. If Peter updates the four gadget drivers,
> > > adding reset callbacks, then we can remove the parts of our patches
> > > that invoke the disconnect callback if there is no reset callback. In
> > > other words, we can make reset callbacks mandatory for gadget drivers.
> >
> > alright, but we need to do this in steps to avoid regressions or a
> > non-bisectable tree. So maybe we add ->reset as an optional method,
> > implement support for it to all UDC drivers, patch all gadget drivers to
> > implement reset, make reset mandatory. Does that work ?
>
> It would be okay, but doing things in a different order would be a
> little better: Add the reset callback to the usb_gadget_driver
> structure, implement it in all four gadget drivers, and then implement
> it in the UDC drivers. That way we don't have to make a second round
it can't be only in these four gadget drivers. It needs to be
implemented in all of them even if:
diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 986fc51..2210289 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -409,6 +409,7 @@ static __refdata struct usb_gadget_driver dbgp_driver = {
.unbind = dbgp_unbind,
.setup = dbgp_setup,
.disconnect = dbgp_disconnect,
+ .reset = dbgp_disconnect,
.driver = {
.owner = THIS_MODULE,
.name = "dbgp"
otherwise ->reset() will be optional and that means UDC will have to:
if (gadget_driver->reset)
gadget_driver->reset(g);
else if (gadget_driver->disconnect && g.speed != UNKNOWN)
gadget_driver->disconnect(g);
and then we end up with a possibility of disconnecting from the host
when we don't want to. Bottom line, we _must_ tell the gadget driver
about a reset IRQ, so it can reset its internal state.
> of modifications to the UDC drivers, removing the fallback calls to
> ->disconnect for when ->reset is missing.
->reset() can't be missing if it were to be mandatory, right ?
> The kerneldoc could state that some UDC drivers may call ->disconnect
> when a reset occurs, instead of calling ->reset. Then we won't have to
> fix up all the UDC drivers at once.
we won't be able to do that because the discussion on this thread is
that ->disconnect() should call usb_gadget_disconnect().
> If you want, you could add a remark to the kerneldoc saying that a
> disconnect handler generally should do everything that a reset handler
> does plus perhaps a few additional things.
yeah, that additional thing is usb_gadget_disconnect() and we don't want
to call that from a simple USB reset.
--
balbi
Attachment:
signature.asc
Description: Digital signature
