Hi, On Thu, Aug 28, 2014 at 05:08:08PM +0800, Peter Chen wrote: > On Wed, Aug 27, 2014 at 02:37:35PM -0500, Felipe Balbi wrote: > > 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. > > It is the same with chipidea, it has RUN/STOP bit at usbcmd, and its > function is most like dwc3's. > > If the RUN/STOP bit is cleared at usb_gadget_driver.disconnect when > the cable is disconnected, when we set it again after cable is connected? > The UDC drivers who has vbus handler can set run/stop bit at .vbus_session, > but if the UDC drivers do not have vbus handler, where we can set it? some UDC drivers don't have a VBUS handler because they don't have internal VBUS comparators and there's no way to implement that for that UDC. > > 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. > > If the UDC driver doesn't want to pull up dp at insmod gadget driver, it > can pull up dp when the cable is connected. how will UDC know that cable was connected if its pullups aren't connected ? > > > 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. > > Yes, it is the current design, but if we want to call usb_gadget_disconnect > at ->disconnect callback, things will be different, besides, the kernel > doc says it is invoked when "when the host is disconnected". you shouldn't call usb_gadget_disconnect() from ->disconnect(). It's the other way around. You get a disconnect IRQ, then you call gadget driver's ->disconnect(), but gadget driver doesn't need to ask UDC to remove pullup at that time. This has been getting more and more confusing of a problem. I'll go curl down in a corner and think about it :-) -- balbi
Attachment:
signature.asc
Description: Digital signature
