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
| ||
|
Message-ID: <20151001005156.GA19319@codeaurora.org> Date: Wed, 30 Sep 2015 17:51:56 -0700 From: Stephen Boyd <sboyd@...eaurora.org> To: Heiko Stübner <heiko@...ech.de> Cc: Xing Zheng <zhengxing@...k-chips.com>, linux-rockchip@...ts.infradead.org, Michael Turquette <mturquette@...libre.com>, linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 4/9] clk: rockchip: add new clock type and controller for rk3036 On 10/01, Heiko Stübner wrote: > Hi Stephen, > > Am Dienstag, 22. September 2015, 16:19:00 schrieb Stephen Boyd: > > On 09/23, Heiko Stübner wrote: > > > Am Dienstag, 22. September 2015, 15:41:25 schrieb Stephen Boyd: > > > > On 09/17, Xing Zheng wrote: > > > > > +{ > > > > > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > > > > > + const struct rockchip_pll_rate_table *rate; > > > > > + unsigned int fbdiv, postdiv1, refdiv, postdiv2, dsmpd, frac; > > > > > + unsigned long drate; > > > > > + u32 pllcon; > > > > > + > > > > > + if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) > > > > > + return; > > > > > > > > I don't understand what this one does though. This check isn't in > > > > the set rate ops. > > > > > > And it shouldn't be :-) > > > > > > The issue this whole thing is trying to solve is aligning the pll settings > > > which what we have in the rate table, not what the bootloader set. > > > > > > For example the bootloader could set up a pll at 594MHz with one set of > > > parameters and after some time - when you don't want to exchange > > > bootloaders on shipping devices anymore - it comes to light that a > > > different set of parameters for the same frequency produces for example a > > > more stable hdmi signal [I think that was the main reason for the initial > > > change]. > > > > > > So we're not changing the frequency x -> y, which could be easily done > > > [and is done already] via assigned-rates, but instead > > > > > > x {params a,b,c} -> x {params d,e,f} > > > > > > so the rate itself stays the same, only the frequency generation is > > > adapted. > > Ok. It would be nice if this sort of information was made into a > > comment and put in the code. Or at least the commit text for the > > change. > > > > And is there any reason that we need to get the parent clock and > > parent rate to align the PLL settings? > > It would be nice if we > > avoided using clk_* APIs in here, by extracting the pll set rate > > code into another function that we can call from init to make the > > values the same without all the fallback to old rates, etc. > > I guess you want Xing Zheng to change his pll code somewhat like the > following, right? While starting off as proof-of-concept, that change > below actually does work quite nicely on rk3288 boards. > > ---------------- 8< -------------------- > From: Heiko Stuebner <heiko@...ech.de> > Subject: [PATCH] clk: rockchip: don't use clk_ APIs in the pll init-callback > > Separate the update of pll registers from the actual set_rate function > so that the init callback does not need to access clk-API functions. > > As we now have separated the getting and setting of the pll parameters > we can also directly use these new functions in other places too. > > Signed-off-by: Heiko Stuebner <heiko@...ech.de> Yep, it looks much better this way. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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/
Powered by blists - more mailing lists