On Fri, Aug 10, 2012 at 08:12:07AM +0000, Chen Peter-B29397 wrote: > > > > So, the others are usb_state not usb_otg_state. Then, when we > > generalize > > > otg driver, what kinds of **states** do you plan to use to stand for > > otg > > > state machine? Besides, if we name something different with spec, does > > it > > > will confuse the users? > > > > We could have just a single enum usb_state defined something like: > > > > enum usb_otg_state { > > USB_STATE_UNDEFINED = 0, > > > > /* single-role peripheral, and dual-role default-b */ > > USB_STATE_B_IDLE, > > USB_STATE_B_SRP_INIT, > > USB_STATE_B_PERIPHERAL, > > > > /* single-role host, and dual-role default-a */ > > USB_STATE_A_IDLE, > > USB_STATE_A_WAIT_VRISE, > > USB_STATE_A_WAIT_BCON, > > USB_STATE_A_HOST, > > USB_STATE_A_SUSPEND, > > USB_STATE_A_WAIT_VFALL, > > USB_STATE_A_VBUS_ERR, > > > > #ifdef CONFIG_USB_OTG > > /* extra dual-role default-b states */ > > USB_STATE_B_WAIT_ACON, > > USB_STATE_B_HOST, > > USB_STATE_A_PERIPHERAL, > > #endif /* CONFIG_USB_OTG > > }; > > > > or something similar. Then we shouldn't allow anyone to access > > phy->state directly, so we would need: > > > > static inline void usb_phy_set_state(struct usb_phy *phy, > > enum usb_state state) > > { > > phy->state = state; > > } > > > > static inline enum usb_state usb_phy_get_state(struct usb_phy *phy) > > { > > return phy->state; > > } > > > Then, at later general OTG driver, we need to know struct usb_phy *phy, > but how can we get it at OTG driver? you could make otg point to its phy. That's simple to handle at any time. > I am a little confused by using phy->state to stand for usb_state that I think > there is no relationship between usb_state with USB PHY. well, there's no relationship between usb_state and OTG. The state isn't OTG-specific, it's USB specific. This is a difficult detail to find the proper owner, but I don't think we should tie the state to OTG, because systems without OTG wouldn't be able to track their states too. -- balbi
Attachment:
signature.asc
Description: Digital signature