linux-kernel - Re: [PATCH v2 4/8] phy: rockchip-usb: expose the phy-internal PLLs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 9 Nov 2015 12:59:58 -0800
From:	Doug Anderson <dianders@...omium.org>
To:	Heiko Stuebner <heiko@...ech.de>
Cc:	Kishon Vijay Abraham I <kishon@...com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	Romain Perier <romain.perier@...il.com>,
	Arnd Bergmann <arnd@...db.de>, Lin Huang <hl@...k-chips.com>
Subject: Re: [PATCH v2 4/8] phy: rockchip-usb: expose the phy-internal PLLs

Heiko,

On Sun, Nov 8, 2015 at 8:04 AM, Heiko Stuebner <heiko@...ech.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/