Hi, On Tue, Aug 26, 2014 at 03:30:32PM +0800, Peter Chen wrote: > On Mon, Aug 25, 2014 at 11:27:47AM -0400, Alan Stern wrote: > > On Mon, 25 Aug 2014, Peter Chen wrote: > > > > > Hi Felipe & Alan, > > > > > > It is the follow-up for: > > > http://www.spinics.net/lists/linux-usb/msg112193.html > > > > > > This patchset adds reset API at usb_gadget_driver, the UDC driver > > > can call it at bus_reset handler instead of calling disconnect. > > > The benefits of this patchset are: > > > - We can let the gadget driver do different things for bus reset. > > > and host is disconnected, eg, whether pull down dp or not. > > > - Match kernel doc for disconnect API, it says it is invoked > > > "when the host is disconnected". > > > - Will be more match for the real things the gadget driver > > > does for connect and disconnect, when we introduce connect API later. > > > > > > The reason for I mark RFC is I don't add the modification for mass > > > UDC driver changes, if you consider this patchset is ok, I will > > > add them without RFC later. > > > > This looks good. > > > > In patches 2, 4, and 5, shouldn't you call usb_gadget_disconnect() > > _before_ the gadget driver's original disconnect handler, instead of > > _after_? That's how we do it now. > > > > Hi Alan & Felipe, > > During the changing UDC drivers process, I find we can't simply move > usb_gadget_disconnect to usb_gadget_driver.disconnect, some UDC > drivers (like dwc3) will be no chance to set software pullup bit > (dp control always means software dp pullup bit if no specific at below) > again between disconnection and re-connection. sorry, can you rephrase this a bit, I really can't get what you mean. DWC3 doesn't really have a pullup bit. It's got a RUN/STOP bit. Sure, it controls the pullups but it also kills the internal DMA and quite a bit of other internal blocks. The way the code is today, we will have a pullup connected when a gadget driver is loaded and not before that. > There are two kinds of UDCs, dp is always pullup(UDC-A), pullup dp relies > on vbus (UDC-B), so we may need to introduce a flag like pullup_on_vbus > at struct usb_gadget, UDC-B needs to set this flag at .udc_start. > > For the whole gadget life cycle, the dp change for the two kinds of > UDC like below: > Process dp at UDC-A dp at UDC-B > insmod 1 0 > connect 1 1 > bus_reset 1 1 > disconnect 1 0 > rmmod 0 0 > > For insmod/rmmod gadget driver, we can control dp at udc-core relies > on flag pullup_on_vbus. > > For other three stages (no need to control at bus_reset), we can control dp > at two gadget driver's API relies on flag pullup_on_vbus. I also can't get what you mean here. > Do you agree above? > > If you agree, the first patchset for adding reset API at usb_gadget_driver may > do less thing, and the reset API implementation at each gadget driver is the > same with disconnect. that's why we never had a ->reset() callback so far. From a gadget driver perspective, it's the same as disconnect followed by a connect. > At the second patchset, we introduce the flag pullup_on_vbus, connect API > at usb_gadget_driver, change the disconnect implementation at > each gadget driver, and add control dp through function driver. IIRC, only mass storage gadget was showing a need for a ->reset() callback, why would you need to modify every gadget driver ? -- balbi
Attachment:
signature.asc
Description: Digital signature
