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? > > 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. > > > 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". -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
