On Thu, Nov 05, 2015 at 08:36:41AM -0600, Felipe Balbi wrote: > > Hi, > > Peter Chen <peter.chen@xxxxxxxxxxxxx> writes: > > On Tue, Nov 03, 2015 at 09:51:11PM -0600, Felipe Balbi wrote: > >> > >> Hi, > >> > >> Peter Chen <peter.chen@xxxxxxxxxxxxx> writes: > >> > On Tue, Nov 03, 2015 at 07:56:55AM -0600, Felipe Balbi wrote: > >> >> > >> >> Hi, > >> >> > >> >> Nathan Sullivan <nathan.sullivan@xxxxxx> writes: > >> >> > The USB OTG support currently depends on power management > >> >> > (CONFIG_PM) being enabled, but does not actually need it enabled. > >> >> > Remove this dependency. > >> >> > > >> >> > Tested on Bay Trail hardware with dwc3 USB. > >> >> > > >> >> > Signed-off-by: Nathan Sullivan <nathan.sullivan@xxxxxx> > >> >> > --- > >> >> > drivers/usb/core/Kconfig | 1 - > >> >> > 1 file changed, 1 deletion(-) > >> >> > > >> >> > diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig > >> >> > index a99c89e..9c5cdf3 100644 > >> >> > --- a/drivers/usb/core/Kconfig > >> >> > +++ b/drivers/usb/core/Kconfig > >> >> > @@ -43,7 +43,6 @@ config USB_DYNAMIC_MINORS > >> >> > > >> >> > config USB_OTG > >> >> > bool "OTG support" > >> >> > - depends on PM > >> >> > >> >> I don't think this is correct. OTG depends on USB bus suspend, which is > >> >> only available on PM builds. Care to further detail why you think PM is > >> >> not needed on OTG ? > >> >> > >> > > >> > OTG depends on USB bus suspend is not a must, the hardware controlled OTG > >> > design do HNP when the bus goes to suspend; but if the software > >> > implements OTG FSM, it is the user option whether do HNP, and bus > >> > suspend is controlled by OTG FSM software (stop SOF), but not by host > >> > stack (eg, ehci). > >> > > >> > I am sorry I did not consider the legacy OTG design, this patch should > >> > be dropped. > >> > >> there is no "legacy" OTG design. OTG requires a bus suspend to enter > >> HNP, and that's achieved by stopping all transfers and avoid new URB > >> submission so usbcore can put the bus in suspend (by means of USB > >> autosuspend). If you're bypassing that in the OTG FSM thing, that needs > >> to be fixed ASAP as that makes it a lot harder for any generic changes > >> in usbcore to be validated. Specially when you consider not many will > >> have whatever special HW which, likely, doesn't even work with mainline > >> to validate a change. > >> > > > > Felipe, thanks for your comments. > > > > But you may need to consider the user option ~a_bus_req (for A) and > > b_bus_req (for B) when do HNP, we can't do HNP without user option. > > > > - Eg, if the bus enters suspend, but the A does not want role switch, > > we can't try to do HNP, the same for B device. > > - The A device may want to support auto-suspend but without role switch. > > > > You are absolutely right, the SAFE HNP needs to do auto-suspend first, we > > need to add this in documentation. But it does not mean OTG FSM design is > > incorrect, there are two things, eg, if we want to do HNP safely, we > > need > > sure it does. If you're bypassing anything in usbcore, it's wrong :-) > For that [ab]_bus_req polling, as soon as you notice that flag, you > should just giveback all URBs to the link partner so the link can > autosuspend and HNP will happen exactly the same. > > > to do below things: > > /* B requests HNP */ > > echo 1 > /sys/bus/../otg_fsm//inputs/b_bus_req > > > > /* Put the A to suspend */ > > echo auto > /sys/bus/usb/devices/1-1/power/control > > /* A goes to try HNP */ > > echo 0 > /sys/bus/../otg_fsm/inputs/a_bus_req > > When you echo 0 to a_bus_req, A should stop all in-flight transfers to > the link partner and wait for the bus to autosuspend. There should be no > differences other than the need to set these flags. > > This even gives us opportunity to tune "how fast" we wand HNP to happen > by fiddling with autosuspend delay ;-) > That's reasonable, we will add this in OTG FSM, thanks. -- 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
