Hi, On Wednesday 12 of June 2013 13:33:50 Doug Anderson wrote: > Yadwinder, > > On Mon, Jun 3, 2013 at 8:09 AM, Yadwinder Singh Brar > > <yadi.brar@xxxxxxxxxxx> wrote: > > This patch unifies clk strutures used for PLL35xx & PLL36xx and uses > > clk->base instead of directly using clk->con0, so that possible > > common code can be factored out. > > It also introdues common pll_[readl/writel] macros for the users of > > common samsung_clk_pll struct. > > > > Reviewed-by: Tomasz Figa <t.figa@xxxxxxxxxxx> > > Reviewed-by: Doug Anderson <dianders@xxxxxxxxxxxx> > > Tested-by: Doug Anderson <dianders@xxxxxxxxxxxx> > > Signed-off-by: Yadwinder Singh Brar <yadi.brar@xxxxxxxxxxx> > > --- > > > > drivers/clk/samsung/clk-exynos4.c | 10 ++++-- > > drivers/clk/samsung/clk-exynos5250.c | 14 ++++---- > > drivers/clk/samsung/clk-pll.c | 54 > > ++++++++++++++++++--------------- drivers/clk/samsung/clk-pll.h > > | 4 +- > > 4 files changed, 44 insertions(+), 38 deletions(-) > > So. We just found that this type of solution doesn't work on > exynos5420, since the LOCK and CON registers aren't always 0x100 away > from each other. Oops, this is what I've been afraid of, ever since we assumed this first time in our internal patches. > Perhaps you can adjust to use a solution like Andrew > proposed in <https://gerrit.chromium.org/gerrit/#/c/58411/>? That way > we can avoid some churn of changing this code twice. > > The number of parameters to the register PLL function is starting to > get unwieldy. At some point we'll probably want to pass in a > structure. I wonder if now would be the time? Certainly it would be > easier to handle changes to the code without touching all of the > exynos variants... Hmm, if done properly, it could simplify PLL registration in SoC clock initialization code a lot. I'm not sure if this is really the best solution (feel free to suggest anything better), but we could put PLLs in an array, like other clocks, e.g. ... exynos4210_pll_clks[] = { CLK_PLL45XX(...), CLK_PLL45XX(...), CLK_PLL46XX(...), CLK_PLL46XX(...), }; and then just call a helper like samsung_clk_register_pll(exynos4210_pll_clks, ARRAY_SIZE(exynos4210_pll_clks)); Best regards, Tomasz > Thanks! > > -Doug > -- > To unsubscribe from this list: send the line "unsubscribe > linux-samsung-soc" in the body of a message to > majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html