On Thu, Feb 26, 2015 at 04:48:30PM +0800, zhangfei wrote: > Hi, Roger > > On 02/24/2015 06:13 PM, Roger Quadros wrote: > >>>On Sat, Feb 21, 2015 at 11:03:05PM +0800, zhangfei wrote: > >>>>>>>>+static void hi6220_start_peripheral(struct hi6220_priv *priv, bool on) > >>>>>>>>+{ > >>>>>>>>+ struct usb_otg *otg = priv->phy.otg; > >>>>>>>>+ > >>>>>>>>+ if (!otg->gadget) > >>>>>>>>+ return; > >>>>>>>>+ > >>>>>>>>+ if (on) > >>>>>>>>+ usb_gadget_connect(otg->gadget); > >>>>>>>>+ else > >>>>>>>>+ usb_gadget_disconnect(otg->gadget); > >>>>>>> > >>>>>>>why is the PHY fiddling with pullups ? > >>>>>> > >>>>>>We use this to enable/disable otg gadget mode. > >>>>> > >>>>>I got that, but the pullups don't belong to the PHY, they belong to the > >>>>>gadget. > >>>>> > >>>>>>The gpio_id & gpio_vbus are used to distinguish otg gadget mode or > >>>>>>host mode. > >>>>>>When micro usb or otg device attached to otg, gpio_vbus falling down. > >>>>>>And gpio_id = 1 is micro usb, gpio_id = 0 is otg device. > >>>>> > >>>>>all of that I understood clearly :-) > >>>>> > >>>>>>So when micro usb attached, we enable gadget mode; while micro usb > >>>>>>detached, we disable gadget mode, and dwc2 will automatically set to > >>>>>>host mode. > >>>>> > >>>>>that's all fine, I'm concerned about letting the PHY fiddle with > >>>>>something it doesn't own. If I am to change pullups rules in udc-core, > >>>>>this is likely to break down miserably and I don't want to have to go > >>>>>through that. > >>>> > >>>>Thanks for the clarifying. > >>> > >>>no problem. > >>> > >>>>How about using usb_gadget_vbus_connect/disconnect, which are used in many > >>>>files under drivers/usb/phy. > >>>>There is no vbus_session in dwc2/gadget.c, I thought it would be same as > >>>>pullup. > >>>> > >>>>However, usb_gadget_vbus_connect still need para gadget, where should we put > >>>>this file, drivers/usb/phy or drivers/phy > >>> > >>>drivers/phy, if the framework misses anything you need, it's a great > >>>opportunity to give back to the community by extending the framework. > >> > >>Sorry, I am a little confused. > >>I need some concrete suggestion for the next step of this patch, which is required for the community board, hikey board. > >> > >>Do you mean in the future we need use hsotg->phy instead of hsotg->uphy. > >> struct phy *phy; > >> struct usb_phy *uphy; > >>usb_phy has many members that struct phy does not have, including otg. > >>struct usb_otg *otg; > >>Is that mean we need port such member from usb_phy to phy. > > > >In my opinion otg structure should belong to the USB core part that takes care > >of the OTG/DRD state machine. We still don't have a clear solution here and > >I'm currently investigating this. > >My current work is to get Dual role functionality working with DWC3 controller and TI > >platforms. > > > >Currently phy drivers take care of OTG operation themselves but there is an opportunity > >to share code and centralize USB role switching. > >The USB core should be the owner of the Host controller, Gadget controller and the OTG phy > >and should take care of the that. > > Good idea. > If you have any patch, I will be very happy to verify. > > How about adding these things in drivers/phy/phy-core.c, it is also > sharable, though not in usb core. > > Just tried adding one member struct usb_otg otg to struct phy, since > not find any good member can hold usb_otg. > In drivers/phy/phy-core.c, adding extcon_register_interest, > phy_vbus_notifier, phy_set_peripheral, it works for me, dwc2 on > hikey board. Just thinking if we can follow struct usb_hcd and struct ehci_hcd design way, the generic phy just like hcd, and the usb phy like ehci hcd which is a private data for hcd. zhangfei, maybe you can have a try. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html