Hi, On Tue, Nov 08, 2011 at 03:55:15PM +0200, Heikki Krogerus wrote: > One more change. Sorry about this. > > Changes since v6: > - Rename struct usb_otg member "xceiv" to "phy" > > > Heikki Krogerus (19): > usb: otg: Rename otg_transceiver to usb_phy > usb: otg: Rename usb_otg and usb_xceiv to usb_phy > usb: otg: Separate otg members from usb_phy > usb: otg: ab8500: Start using struct usb_otg > usb: otg: fsl: Start using struct usb_otg > usb: otg: gpio_vbus: Start using struct usb_otg > usb: otg: isp1301_omap: Start using struct usb_otg > usb: otg: msm: Start using struct usb_otg > usb: otg: langwell: Start using struct usb_otg > usb: otg: nop: Start using struct usb_otg > usb: otg: twl4030: Start using struct usb_otg > usb: otg: twl6030: Start using struct usb_otg > usb: otg: ulpi: Start using struct usb_otg > arm: imx: Start using struct usb_otg > usb: musb: Start using struct usb_otg > power_supply: Convert all users to new usb_phy > usb: Convert all users to new usb_phy > usb: otg: Remove OTG specific members from usb_phy > usb: otg: Convert all users to pass struct usb_otg for OTG functions All in all, except for a few comments I had, the series is ok. But I truly don't see the big benefit we have out of this. You're just renaming a bunch of structures, enumerations and functions without a real reason behind it. What we need in the long run, like I had told you back in Nokia, is to be able to compile several transceiver drivers into the same zImage or as dynamically loadable modules and still have it working somehow. Which means that we need a way for a Link (dwc3, musb, omap_udc, renesas_udc, etc) to request a transceiver, or that transceiver has to be assigned to a link as a resource. Also keep in mind that on USB3 device controllers, we will have at least two PHYs: one which will be handling USB2 signalling and another which will be handling USB3 signalling. I say "at least" because OMAP boards generally have split PHY functionality between internal (into the SoC) entities and an external PMIC. If you take OMAP4, for example, we have an internal IP handling the data parts and communicating VBUS/ID levels to MUSB while the actual VBUS/ID comparators are inside the PMIC. With this series of yours I don't see any way we could easily model this split PHY functionality. Some OEMs might also decide to ditch completely the internal and PMIC PHY functionality in exchange for their own solution of an all-in-one PHY (e.g. N900). So all of that is still a pain in the ass to get sorted out and your series isn't helping at all towards that goal so I can't see the real benefit in accepting these patches other than having better matching structure names. You didn't even cleanup the <linux/usb/otg.h> header. There's a bunch of stuff in there which isn't OTG-related at all and you seem to not have cared too much about it. At the end of the day we need to solve the problems we have and make the Embedded part of the USB suport on linux (mainly the gadget framework and OTG support) as easy as possible to generate a product out of it. If you remember the amount of patches we had to get N900 sorted out, you know what I'm talking about. To prevent our users from ever rebasing their own hacks on their internal trees, we need to provide solutions which are flexible enough to have boards with USB2-only, USB3, Host-only, device-only, OTG, with and without USB Battery Charging. As of now we can't. I hope you have some other patches/ideas behind this series, otherwise there's not enough reason to merge it. I suggest you start laying down your ideas and, where possible, code so we can come up with a solution to help everybody. -- balbi
Attachment:
signature.asc
Description: Digital signature