On 07/08/2013 12:43 PM, George Cherian wrote: > This patch series adds phy support for AM335X platform. > This patch series is based on Generic PHY framework [1]. > > > This series has > - adds dual musb instances support for am335x platform (just for testing) > - adds phy-amxxxx-usb driver used in AMxxxx platforms > - adds dt bindings for the phys > - removes usb-phy and replaced with generic phy apis in glue layer No, I don't like this all. You did the one thing I tried to avoid while posting my quick-and-dirty phy driver recently: You duplicated a lot of code which can be served by the nop driver and added only power on/power off callbacks. In numbers: > 7 files changed, 316 insertions(+), 70 deletions(-) vs 2 files changed, 117 insertions, 12 deletions I assumed you had also the OTG callbacks (set host/device mode) and wake up but I don't see it there. Adding a power regulator would do the same job, wouldn't it? If the phy driver remains just doing power on/off I suggest simply adding a power regulator. If it will do more I would move the am35xx specific bits into a separate file and glue it to the nop driver. What else? The abstraction in device tree is wrong. It remains wrong if add stuff on top to it. We need two nodes each one with a glue layer and a musb child node. The instances crap in kernel has to vanish. Also that means your phy nodes are wrong. This is not musb with two ports but two musb instances each with one port. Sebastian -- 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