On Tue, Nov 8, 2011 at 5:36 PM, Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> wrote: > Hi, > > On Tue, Nov 08, 2011 at 11:27:12AM +0530, ABRAHAM, KISHON VIJAY wrote: >> Hi Heikki, >> >> I tend to defer with your opinion of renaming otg_transceiver to >> usb_phy. According to me otg_transceiver should program hardware >> mechanisms associated to VBUS, ID lines, etc.. and phy is responsible >> for transmitting data over differential data lines (with its own >> programming of phy_init, phy_shutdown, setting phy clocks etc..). So >> in my opinion otg_transceiver and usb_phy should be two different and >> separate entities. > > Maybe I'm misunderstanding you, but sounds like you are still marrying > OTG with transceivers. I'm against the idea of usb_phy married with the transceiver (i.e transceiver = twl6030 for instance) > > Things like the otg_transceiver are dropped. The idea is to have the > OTG utility independent of any hardware. OTG does not need to be tied > to a transceiver or to a controller. But currently most of OTG function pointers operates on the transceiver. Pls see my comment for [PATCHv6 12/19] usb: otg: twl6030: Start using struct usb_otg for a little more explanation. > > On most platforms the controller is perfectly capable of handling any > OTG related function without any need for the software to program the > PHY, so functions like "drive VBUS" will in practice be implemented by > the controller driver. There may not be PHY driver at all but OTG is > still supported. > > If on your platform the PHY needs to be programmed in order to drive > VBUS, or handle some other OTG function, then your PHY driver will > fill the hooks in your struct usb_otg. The PHY driver shouldn't do any OTG function. According to me, the PHY driver should take care of only phy_int, phy_shutdown, phy_suspend. The utility does not need to > care which driver implements these hooks. > >> I would have preferred to rename otg_transceiver as usb_otg as the >> first step. (this differs from your implementation where you rename >> otg_transceiver to usb_phy and create a new structure usb_otg to >> separate otg members from usb_phy). >> >> But it should have been first rename otg_transceiver as usb_otg. Then >> create a new structure usb_phy to move all the phy specific >> implementation there. This kind of implementation will also help when >> we want to have independent phy drivers. > > This would not have changed the outcome. It would have :-) I could have directly taken your patch and implemented independent PHY drivers (thats not tied to any transceivers). Thanks Kishon > > Oh, and keep in mind that this really is only the first step in this > rework. After this the idea is to provide support for multiple > transceivers and move USB PHY structures and functions into separate > files from OTG. The final step, and my ultimate goal, is implementing > a generic OTG state machine for the OTG utility. > > Br, > > -- > heikki > -- 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