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
> 
> <[email protected]> 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 <[email protected]>
> > Reviewed-by: Doug Anderson <[email protected]>
> > Tested-by: Doug Anderson <[email protected]>
> > Signed-off-by: Yadwinder Singh Brar <[email protected]>
> > ---
> > 
> >  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
> [email protected]
> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to