| Message ID | 20140902173149.GM16872@saruman.home |
|---|---|
| State | New |
| Headers | show |
On Tue, 2 Sep 2014, Felipe Balbi wrote: > > > 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: There _are_ only four gadget drivers: dbgp, gadgetfs, configfs, and composite. (Unless some are hiding outside of drivers/usb/gadget -- I didn't bother to check the entire kernel tree.) > 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. So make reset mandatory from the start. > > 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 ? 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(). I had forgotten that little detail. :-( Well, strictly speaking, the disconnect handler doesn't have to call usb_gadget_disconnect if the UDC driver takes care of it. And currently all of the UDC drivers do. Still, in the end it would be cleaner to allow disconnect handlers to call usb_gadget_disconnect. And that means every UDC driver has to support ->reset callbacks. I don't know how practical that is -- in some cases there may not be anybody capable of making the necessary changes. > > 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. That's not the only additional thing. The gadget driver also should remember that the host isn't connected, so that it won't call usb_gadget_connect when all the userspace components become ready. You know, more and more it seems like this ought to be handled by the UDC core. There are two conditions for turning on the pullup: Vbus must be active, and the gadget's functions must all be activated. The UDC driver knows about the first and the gadget driver knows about the second -- so the UDC core is where those two pieces of knowledge should be brought together. That might be more of a change than you want to make, however... 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
On Tue, Sep 02, 2014 at 02:11:52PM -0400, Alan Stern wrote: > On Tue, 2 Sep 2014, Felipe Balbi wrote: > > > > > 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: > > There _are_ only four gadget drivers: dbgp, gadgetfs, configfs, and > composite. (Unless some are hiding outside of drivers/usb/gadget -- I heh, good point :-) > didn't bother to check the entire kernel tree.) > > > 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. > > So make reset mandatory from the start. Sure thing, that can be done :-) > > > 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 ? > > 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(). > > I had forgotten that little detail. :-( > > Well, strictly speaking, the disconnect handler doesn't have to call > usb_gadget_disconnect if the UDC driver takes care of it. And > currently all of the UDC drivers do. > > Still, in the end it would be cleaner to allow disconnect handlers to > call usb_gadget_disconnect. And that means every UDC driver has to > support ->reset callbacks. I don't know how practical that is -- in > some cases there may not be anybody capable of making the necessary > changes. > > > > 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. > > That's not the only additional thing. The gadget driver also should > remember that the host isn't connected, so that it won't call > usb_gadget_connect when all the userspace components become ready. > > You know, more and more it seems like this ought to be handled by the > UDC core. There are two conditions for turning on the pullup: Vbus > must be active, and the gadget's functions must all be activated. The > UDC driver knows about the first and the gadget driver knows about the > second -- so the UDC core is where those two pieces of knowledge should > be brought together. I'll agree with this, 100%. > That might be more of a change than you want to make, however... I wouldn't say that. As long as regressions can be avoided, I have no issues modifying the framework and every single driver :-) I guess we won't be able to do everything in one go, however, so we might need to split this accross two merge windows to give people time to test all the changes.
On Tue, 2 Sep 2014, Felipe Balbi wrote: > > You know, more and more it seems like this ought to be handled by the > > UDC core. There are two conditions for turning on the pullup: Vbus > > must be active, and the gadget's functions must all be activated. The > > UDC driver knows about the first and the gadget driver knows about the > > second -- so the UDC core is where those two pieces of knowledge should > > be brought together. > > I'll agree with this, 100%. > > > That might be more of a change than you want to make, however... > > I wouldn't say that. As long as regressions can be avoided, I have no > issues modifying the framework and every single driver :-) I guess we > won't be able to do everything in one go, however, so we might need to > split this accross two merge windows to give people time to test all the > changes. Do we need a ->connect callback in the gadget drivers? If the UDC core does all the work of tracking the connection status, maybe we don't. For now, I'll assume we don't need it. Okay. Let's start by adding the reset field to struct usb_gadget_driver. The initial implementation in the four gadget drivers can be very simple: It calls the disconnect handler. (At some later time we can add a ->reset callback to struct usb_composite_driver; then the reset handler in composite.c can invoke that callback for all function drivers that define it.) Next, add routines to udc-core to handle Vbus changes, function activation changes, and resets. The Vbus routine will invoke the gadget driver's ->disconnect callback when Vbus goes off and then call usb_gadget_disconnect. The activation routine will call usb_gadget_disconnect if any functions are deactivated, and usb_gadget_connect if all the functions are activated (basically, do whatever composite.c would do). And the reset routine will invoke the gadget driver's ->reset callback. I suppose we'll also have to add fields to struct usb_gadget, to store the current Vbus and activation statuses. Then modify all the UDC drivers; make them invoke the new udc-core routines whenever there's a change in Vbus status or a reset, instead of invoking the gadget driver's callbacks directly. At the same time we can remove the code that turns off the pullup when Vbus goes down, because now the udc-core routine will take care of it. They don't all have to be updated at once. Unchanged UDC drivers will continue to work exactly as before, because they will bypass the new code in udc-core. How does that sound? 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
On Tue, Sep 02, 2014 at 03:25:05PM -0400, Alan Stern wrote: > On Tue, 2 Sep 2014, Felipe Balbi wrote: > > > > You know, more and more it seems like this ought to be handled by the > > > UDC core. There are two conditions for turning on the pullup: Vbus > > > must be active, and the gadget's functions must all be activated. The > > > UDC driver knows about the first and the gadget driver knows about the > > > second -- so the UDC core is where those two pieces of knowledge should > > > be brought together. > > > > I'll agree with this, 100%. > > > > > That might be more of a change than you want to make, however... > > > > I wouldn't say that. As long as regressions can be avoided, I have no > > issues modifying the framework and every single driver :-) I guess we > > won't be able to do everything in one go, however, so we might need to > > split this accross two merge windows to give people time to test all the > > changes. > > Do we need a ->connect callback in the gadget drivers? If the UDC core > does all the work of tracking the connection status, maybe we don't. > For now, I'll assume we don't need it. > > Okay. Let's start by adding the reset field to struct > usb_gadget_driver. The initial implementation in the four gadget > drivers can be very simple: It calls the disconnect handler. (At some > later time we can add a ->reset callback to struct > usb_composite_driver; then the reset handler in composite.c can invoke > that callback for all function drivers that define it.) > > Next, add routines to udc-core to handle Vbus changes, function > activation changes, and resets. The Vbus routine will invoke the > gadget driver's ->disconnect callback when Vbus goes off and then call > usb_gadget_disconnect. The activation routine will call > usb_gadget_disconnect if any functions are deactivated, and > usb_gadget_connect if all the functions are activated (basically, do > whatever composite.c would do). And the reset routine will invoke the > gadget driver's ->reset callback. I suppose we'll also have to add > fields to struct usb_gadget, to store the current Vbus and activation > statuses. If the usb_gadget_disconnect will be not called at gadget driver's ->disconnect, it has no much meaning we still have gadget driver's ->reset, since the content of its ->disconnect and ->reset are the same if we don't consider to add function's reset handler. > > Then modify all the UDC drivers; make them invoke the new udc-core > routines whenever there's a change in Vbus status or a reset, instead > of invoking the gadget driver's callbacks directly. At the same time > we can remove the code that turns off the pullup when Vbus > goes down, because now the udc-core routine will take care of it. Why we need to record the reset at udc-core? > > They don't all have to be updated at once. Unchanged UDC drivers will > continue to work exactly as before, because they will bypass the new > code in udc-core. > > How does that sound? > > Alan Stern >
On Wed, 3 Sep 2014, Peter Chen wrote: > > Okay. Let's start by adding the reset field to struct > > usb_gadget_driver. The initial implementation in the four gadget > > drivers can be very simple: It calls the disconnect handler. (At some > > later time we can add a ->reset callback to struct > > usb_composite_driver; then the reset handler in composite.c can invoke > > that callback for all function drivers that define it.) > > > > Next, add routines to udc-core to handle Vbus changes, function > > activation changes, and resets. The Vbus routine will invoke the > > gadget driver's ->disconnect callback when Vbus goes off and then call > > usb_gadget_disconnect. The activation routine will call > > usb_gadget_disconnect if any functions are deactivated, and > > usb_gadget_connect if all the functions are activated (basically, do > > whatever composite.c would do). And the reset routine will invoke the > > gadget driver's ->reset callback. I suppose we'll also have to add > > fields to struct usb_gadget, to store the current Vbus and activation > > statuses. > > If the usb_gadget_disconnect will be not called at gadget driver's > ->disconnect, it has no much meaning we still have gadget driver's > ->reset, since the content of its ->disconnect and ->reset are the > same if we don't consider to add function's reset handler. No, they aren't always the same. For example, g_mass_storage ought to flush its dirty buffers when a disconnect occurs, but it doesn't have to flush them when a reset occurs. > > Then modify all the UDC drivers; make them invoke the new udc-core > > routines whenever there's a change in Vbus status or a reset, instead > > of invoking the gadget driver's callbacks directly. At the same time > > we can remove the code that turns off the pullup when Vbus > > goes down, because now the udc-core routine will take care of it. > > Why we need to record the reset at udc-core? We don't really need to. But it seems like a good idea to have events that affect the entire device (including connect, disconnect, reset, and probably also bus suspend and bus resume) all pass through udc-core. 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
On Wed, Sep 03, 2014 at 01:53:23PM -0400, Alan Stern wrote: > On Wed, 3 Sep 2014, Peter Chen wrote: > > > > Okay. Let's start by adding the reset field to struct > > > usb_gadget_driver. The initial implementation in the four gadget > > > drivers can be very simple: It calls the disconnect handler. (At some > > > later time we can add a ->reset callback to struct > > > usb_composite_driver; then the reset handler in composite.c can invoke > > > that callback for all function drivers that define it.) > > > > > > Next, add routines to udc-core to handle Vbus changes, function > > > activation changes, and resets. The Vbus routine will invoke the > > > gadget driver's ->disconnect callback when Vbus goes off and then call > > > usb_gadget_disconnect. The activation routine will call > > > usb_gadget_disconnect if any functions are deactivated, and > > > usb_gadget_connect if all the functions are activated (basically, do > > > whatever composite.c would do). And the reset routine will invoke the > > > gadget driver's ->reset callback. I suppose we'll also have to add > > > fields to struct usb_gadget, to store the current Vbus and activation > > > statuses. > > > > If the usb_gadget_disconnect will be not called at gadget driver's > > ->disconnect, it has no much meaning we still have gadget driver's > > ->reset, since the content of its ->disconnect and ->reset are the > > same if we don't consider to add function's reset handler. > > No, they aren't always the same. For example, g_mass_storage ought to > flush its dirty buffers when a disconnect occurs, but it doesn't have > to flush them when a reset occurs. > > > > Then modify all the UDC drivers; make them invoke the new udc-core > > > routines whenever there's a change in Vbus status or a reset, instead > > > of invoking the gadget driver's callbacks directly. At the same time > > > we can remove the code that turns off the pullup when Vbus > > > goes down, because now the udc-core routine will take care of it. > > > > Why we need to record the reset at udc-core? > > We don't really need to. But it seems like a good idea to have events > that affect the entire device (including connect, disconnect, reset, > and probably also bus suspend and bus resume) all pass through > udc-core. > Hi Felipe & Alan, how about using below steps for this reset/vbus/pullup changes? It mainly uses Alan's suggestion. Step 1: The initial implementation in the four gadget drivers can be very simple: It calls the disconnect handler. the ->reset is mandatory for all gadget drivers (currently, only four, they are composite, configfs, gadgetfs , dbgp. Step 2: Add routines to udc-core to handle Vbus changes, function activation changes, and resets. It will include three sub-steps, from easy to hard: reset handler, vbus handler, and activation handler. Step 3: Modify all UDCs to call udc-core's reset and vbus handler, delete usb_gadget_connect/usb_gadget_disconnect operation at all UDC drivers, and delete invoking usb_gadget_connect unconditional at udc_bind_to_driver Step 4: Modify the composite.c to disable pullup for adding every function, and enable pullup when necessary.
On Thu, 4 Sep 2014, Peter Chen wrote: > Hi Felipe & Alan, how about using below steps for this reset/vbus/pullup > changes? It mainly uses Alan's suggestion. > > Step 1: > The initial implementation in the four gadget > drivers can be very simple: It calls the disconnect handler. > the ->reset is mandatory for all gadget drivers (currently, > only four, they are composite, configfs, gadgetfs , dbgp. > Step 2: > Add routines to udc-core to handle Vbus changes, function > activation changes, and resets. It will include three sub-steps, > from easy to hard: reset handler, vbus handler, and activation > handler. Perhaps the activation handler can be delayed until step 4. It won't get used until composite.c is modified to call it. > Step 3: > Modify all UDCs to call udc-core's reset and vbus handler, delete > usb_gadget_connect/usb_gadget_disconnect operation at all UDC drivers, > and delete invoking usb_gadget_connect unconditional at udc_bind_to_driver > Step 4: > Modify the composite.c to disable pullup for adding every function, and > enable pullup when necessary. Actually, composite.c should be modified to call the activation handler in udc-core, and then _that_ routine would adjust the pullup as necessary. This plan is okay with me. 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
On Thu, Sep 04, 2014 at 11:04:38AM -0400, Alan Stern wrote: > On Thu, 4 Sep 2014, Peter Chen wrote: > > > Hi Felipe & Alan, how about using below steps for this reset/vbus/pullup > > changes? It mainly uses Alan's suggestion. > > > > Step 1: > > The initial implementation in the four gadget > > drivers can be very simple: It calls the disconnect handler. > > the ->reset is mandatory for all gadget drivers (currently, > > only four, they are composite, configfs, gadgetfs , dbgp. > > Step 2: > > Add routines to udc-core to handle Vbus changes, function > > activation changes, and resets. It will include three sub-steps, > > from easy to hard: reset handler, vbus handler, and activation > > handler. > > Perhaps the activation handler can be delayed until step 4. It won't > get used until composite.c is modified to call it. > > > Step 3: > > Modify all UDCs to call udc-core's reset and vbus handler, delete > > usb_gadget_connect/usb_gadget_disconnect operation at all UDC drivers, > > and delete invoking usb_gadget_connect unconditional at udc_bind_to_driver > > Step 4: > > Modify the composite.c to disable pullup for adding every function, and > > enable pullup when necessary. > > Actually, composite.c should be modified to call the activation handler > in udc-core, and then _that_ routine would adjust the pullup as > necessary. > > This plan is okay with me. > OK, let's put the function activation change to the last. If the Felipe has no other comments, I will send the patch for step one next Monday.
On Fri, Sep 05, 2014 at 08:44:08AM +0800, Peter Chen wrote: > On Thu, Sep 04, 2014 at 11:04:38AM -0400, Alan Stern wrote: > > On Thu, 4 Sep 2014, Peter Chen wrote: > > > > > Hi Felipe & Alan, how about using below steps for this reset/vbus/pullup > > > changes? It mainly uses Alan's suggestion. > > > > > > Step 1: > > > The initial implementation in the four gadget > > > drivers can be very simple: It calls the disconnect handler. > > > the ->reset is mandatory for all gadget drivers (currently, > > > only four, they are composite, configfs, gadgetfs , dbgp. > > > Step 2: > > > Add routines to udc-core to handle Vbus changes, function > > > activation changes, and resets. It will include three sub-steps, > > > from easy to hard: reset handler, vbus handler, and activation > > > handler. > > > > Perhaps the activation handler can be delayed until step 4. It won't > > get used until composite.c is modified to call it. > > > > > Step 3: > > > Modify all UDCs to call udc-core's reset and vbus handler, delete > > > usb_gadget_connect/usb_gadget_disconnect operation at all UDC drivers, > > > and delete invoking usb_gadget_connect unconditional at udc_bind_to_driver > > > Step 4: > > > Modify the composite.c to disable pullup for adding every function, and > > > enable pullup when necessary. > > > > Actually, composite.c should be modified to call the activation handler > > in udc-core, and then _that_ routine would adjust the pullup as > > necessary. > > > > This plan is okay with me. > > > > OK, let's put the function activation change to the last. If the Felipe has > no other comments, I will send the patch for step one next Monday. Please do, the thread already has too much information and we better put all that in code. Keep in mind that me and Alan have resurected an old patchset adding ->reset() callback to the gadget framework. If you want to take a look it's at [1] https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/log/?h=gadget-add-reset-method
> On Fri, Sep 05, 2014 at 08:44:08AM +0800, Peter Chen wrote: > > On Thu, Sep 04, 2014 at 11:04:38AM -0400, Alan Stern wrote: > > > On Thu, 4 Sep 2014, Peter Chen wrote: > > > > > > > Hi Felipe & Alan, how about using below steps for this > > > > reset/vbus/pullup changes? It mainly uses Alan's suggestion. > > > > > > > > Step 1: > > > > The initial implementation in the four gadget > > > > drivers can be very simple: It calls the disconnect handler. > > > > the ->reset is mandatory for all gadget drivers (currently, > > > > only four, they are composite, configfs, gadgetfs , dbgp. > > > > Step 2: > > > > Add routines to udc-core to handle Vbus changes, function > > > > activation changes, and resets. It will include three sub-steps, > > > > from easy to hard: reset handler, vbus handler, and activation > > > > handler. > > > > > > Perhaps the activation handler can be delayed until step 4. It > > > won't get used until composite.c is modified to call it. > > > > > > > Step 3: > > > > Modify all UDCs to call udc-core's reset and vbus handler, delete > > > > usb_gadget_connect/usb_gadget_disconnect operation at all UDC > drivers, > > > > and delete invoking usb_gadget_connect unconditional at > > > > udc_bind_to_driver Step 4: > > > > Modify the composite.c to disable pullup for adding every function, and > > > > enable pullup when necessary. > > > > > > Actually, composite.c should be modified to call the activation > > > handler in udc-core, and then _that_ routine would adjust the pullup > > > as necessary. > > > > > > This plan is okay with me. > > > > > > > OK, let's put the function activation change to the last. If the > > Felipe has no other comments, I will send the patch for step one next > Monday. > > Please do, the thread already has too much information and we better put all > that in code. Keep in mind that me and Alan have resurected an old patchset > adding ->reset() callback to the gadget framework. If you want to take a look > it's at [1] > > https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/log/?h=gadget-add- > reset-method > Thanks, Felipe. It still takes gadget driver's ->reset as optional, but we want to take it as mandatory per our discussion. Peter -- 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
On Sat, Sep 06, 2014 at 12:09:22AM +0000, Peter Chen wrote: > > > On Fri, Sep 05, 2014 at 08:44:08AM +0800, Peter Chen wrote: > > > On Thu, Sep 04, 2014 at 11:04:38AM -0400, Alan Stern wrote: > > > > On Thu, 4 Sep 2014, Peter Chen wrote: > > > > > > > > > Hi Felipe & Alan, how about using below steps for this > > > > > reset/vbus/pullup changes? It mainly uses Alan's suggestion. > > > > > > > > > > Step 1: > > > > > The initial implementation in the four gadget > > > > > drivers can be very simple: It calls the disconnect handler. > > > > > the ->reset is mandatory for all gadget drivers (currently, > > > > > only four, they are composite, configfs, gadgetfs , dbgp. > > > > > Step 2: > > > > > Add routines to udc-core to handle Vbus changes, function > > > > > activation changes, and resets. It will include three sub-steps, > > > > > from easy to hard: reset handler, vbus handler, and activation > > > > > handler. > > > > > > > > Perhaps the activation handler can be delayed until step 4. It > > > > won't get used until composite.c is modified to call it. > > > > > > > > > Step 3: > > > > > Modify all UDCs to call udc-core's reset and vbus handler, delete > > > > > usb_gadget_connect/usb_gadget_disconnect operation at all UDC > > drivers, > > > > > and delete invoking usb_gadget_connect unconditional at > > > > > udc_bind_to_driver Step 4: > > > > > Modify the composite.c to disable pullup for adding every function, and > > > > > enable pullup when necessary. > > > > > > > > Actually, composite.c should be modified to call the activation > > > > handler in udc-core, and then _that_ routine would adjust the pullup > > > > as necessary. > > > > > > > > This plan is okay with me. > > > > > > > > > > OK, let's put the function activation change to the last. If the > > > Felipe has no other comments, I will send the patch for step one next > > Monday. > > > > Please do, the thread already has too much information and we better put all > > that in code. Keep in mind that me and Alan have resurected an old patchset > > adding ->reset() callback to the gadget framework. If you want to take a look > > it's at [1] > > > > https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/log/?h=gadget-add- > > reset-method > > > > Thanks, Felipe. > > It still takes gadget driver's ->reset as optional, but we want to > take it as mandatory per our discussion. right, I was going to change that but since you're already working on it I figured you might prefer doing it all yourself :-) cheers
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"