Heiko, On Sun, Nov 8, 2015 at 8:04 AM, Heiko Stuebner <heiko at sntech.de> wrote: > +static const struct rockchip_usb_phy_pdata rk3066a_pdata = { > + .phys = (struct rockchip_usb_phys[]){ > + { .reg = 0x17c, .pll_name = "sclk_otgphy0_480m" }, > + { .reg = 0x188, .pll_name = "sclk_otgphy1_480m" }, > + { /* sentinel */ } > + }, > +}; > + > +static const struct rockchip_usb_phy_pdata rk3188_pdata = { > + .phys = (struct rockchip_usb_phys[]){ > + { .reg = 0x10c, .pll_name = "sclk_otgphy0_480m" }, > + { .reg = 0x11c, .pll_name = "sclk_otgphy1_480m" }, > + { /* sentinel */ } > + }, > +}; > + > +static const struct rockchip_usb_phy_pdata rk3288_pdata = { > + .phys = (struct rockchip_usb_phys[]){ > + { .reg = 0x320, .pll_name = "sclk_otgphy0_480m" }, > + { .reg = 0x334, .pll_name = "sclk_otgphy1_480m" }, > + { .reg = 0x348, .pll_name = "sclk_otgphy2_480m" }, > + { /* sentinel */ } > + }, Slighly annoying to have to add a table for each SoC. I'd imagine this growing quite large. Would it be possible to query our parent clock name and then append a "_480m" suffix? Then you can magically get the name you want, right? That doesn't preclude you from later overriding this or adding a "dts" property for it, but it means no tables for now and (hopefully) for the forseeable future. ...and you get to kill a bunch of code in this patch... You could fall back to some generic name based on the PHY device tree node name if you wanted, or use the ID allocator... Otherwise, this all looks good to me. If you have a good reason why using the parent name isn't a good idea, let me know and I can add my Reviewed-by on this patch. -Doug