Hi, On Fri, Mar 15, 2013 at 04:05:52PM +0800, Peter Chen wrote: > > > > > > > > @@ -160,6 +163,24 @@ usb_phy_shutdown(struct usb_phy *x) > > > > > > > > x->shutdown(x); > > > > > > > > } > > > > > > > > > > > > > > > > +static inline int > > > > > > > > +usb_phy_vbus_on(struct usb_phy *x) > > > > > > > > +{ > > > > > > > > + if (!x->set_vbus) > > > > > > > > + return 0; > > > > > > > > + > > > > > > > > + return x->set_vbus(x, true); > > > > > > > > +} > > > > > > > > + > > > > > > > > +static inline int > > > > > > > > +usb_phy_vbus_off(struct usb_phy *x) > > > > > > > > +{ > > > > > > > > + if (!x->set_vbus) > > > > > > > > + return 0; > > > > > > > > + > > > > > > > > + return x->set_vbus(x, false); > > > > > > > > +} > > > > > > > > + > > > > > > > > > > > > > > What's plan for otg's set_vbus? I suggest using phy's to instead of > > > > > > > otg's. > > > > > > > > > > > > > > > > When I think it more, does it work like below: > > > > > > > > > > - If PHY supplies vbus_setting function (like ulpi.c), the callback > > > > > is assigned at PHY driver. > > > > > - If board supplies vbus on/off function (through gpio), the callback > > > > > is assigned at controller driver. > > > > > Here vbus setting is USB staff, no limited to PHY. > > > > > > > > I wonder if the whole vbus toggling logic shouldn't, eventually, become > > > > a regulator. That would clean up a bunch of these details. I mean, we > > > > can still provide usb_phy_vbus_on()/off() but that would just call into > > > > the proper regulator. > > > > > > > > Also, I think we should have a single location where VBUS is controlled, > > > > since the VBUS line is directly routed to a pin in the PHY, the PHY > > > > > > Not for all, for chipidea phy, it has two regulators (2.5v and 3.3v) > > > from outside as phy's power. > > > > > > Even vbus pin is input for PHY, how can PHY turns on or off it without > > > external control like GPIO? > > > > that external control is a Fixed Regulator which your PHY driver > > requests and manages. Then you implement ->set_vbus() with: > > > > my_fancy_set_vbus() > > { > > if (on) > > ret = regulator_enable(reg); > > else > > ret = regulator_disable(reg); > > > > return ret; > > } > > Yes, that is the solution. > My most concern is we design the API, how make the user > understand relationship between vbus and PHY, why .set_vbus > is put at PHY driver? if you look at your PCB, VBUS, D+ and D- are directly routed to the PHY, not the USB controller itself. If you read ULPI and UTMI specs, you'll see that there are methods for the PHY to tell the USB controller about VBUS levels, so the PHY manages VBUS. -- balbi
Attachment:
signature.asc
Description: Digital signature