Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs

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. 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...

Yadwinder,

On Mon, Jun 3, 2013 at 8:09 AM, Yadwinder Singh Brar

Quoted 10 lines · <[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]>

Quoted 8 lines

---

 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.

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...

Thanks!

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

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));

Quoted 14 lines

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(...),
        };

Quoted 4 lines

and then just call a helper like

        samsung_clk_register_pll(exynos4210_pll_clks,
                        ARRAY_SIZE(exynos4210_pll_clks));

Something like that looks like what I was thinking. I'd have to see it actually coded up to see if there's something I'm missing that would prevent us from doing that, but I don't see anything.

Doug,

Quoted 14 lines

Quoted 14 lines

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(...),
        };

Quoted 8 lines

Quoted 4 lines

and then just call a helper like

        samsung_clk_register_pll(exynos4210_pll_clks,
                        ARRAY_SIZE(exynos4210_pll_clks));

Something like that looks like what I was thinking. I'd have to see it actually coded up to see if there's something I'm missing that would prevent us from doing that, but I don't see anything.

The only issue I see with this is that we may only want to register a rate table with a PLL only if fin_pll is running at a certain rate. On 5250 and 5420, for example, we have EPLL and VPLL rate tables that should only be registered if fin_pll is 24Mhz. We may have to register those separately, but this approach seems fine otherwise.

On Thu, Jun 13, 2013 at 3:32 AM, Andrew Bresticker

Quoted 31 lines · <[email protected]> wrote:

Doug,

Quoted 21 lines

Quoted 21 lines

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

Quoted 7 lines

Quoted 3 lines

        samsung_clk_register_pll(exynos4210_pll_clks,
        
                        ARRAY_SIZE(exynos4210_pll_clks));

Something like that looks like what I was thinking. I'd have to see it actually coded up to see if there's something I'm missing that would prevent us from doing that, but I don't see anything.

Quoted 5 lines

The only issue I see with this is that we may only want to register a rate table with a PLL only if fin_pll is running at a certain rate. On 5250 and 5420, for example, we have EPLL and VPLL rate tables that should only be registered if fin_pll is 24Mhz. We may have to register those separately, but this approach seems fine otherwise.

As Andrew Bresticker said, we will face problem with different table, and it will give some pain while handling such cases but I think overall code may look better.

Similar thoughts were their in my mind also, but i didn't want to disturb this series :).

Anyways, I think we can do it now only rather going for incremental patches after this series.
I was thinking to make samsung_clk_register_pllxxxx itself little generic instead
of using helper, as we are almost duplicating code for most PLLs.

A rough picture in my mind was,
After implementing generic samung_clk_register_pll(), code may look like below. Its just an idea, please feel free to correct it. Later we can factor out other common clk.ops for PLLs also.

this diff is over this series.
Assuming a generic samung_clk_register_pll() is their(which i think is not impossible) 8<----------------------------------------------------------------------
----

drivers/clk/samsung/clk-exynos5250.c +2 -0

@@ -493,6 +493,20 @@ static __initdata struct samsung_pll_rate_table
 epll_24mhz_tbl[] = {
        PLL_36XX_RATE(32768000, 131, 3, 5, 4719),
 };
 
+struct __initdata samsung_pll_init_data samsung_plls[] = {
+       PLL(pll_3500, "fout_apll", "fin_pll", APLL_LOCK, APLL_CON0,
 NULL), +       PLL(pll_3500, "fout_mpll", "fin_pll", MPLL_LOCK,
 MPLL_CON0, NULL), +       PLL(pll_3500, "fout_bpll",

"fin_pll",BPLL_LOCK, BPLL_CON0, NULL), + PLL(pll_3500,
"fout_gpll", "fin_pll",GPLL_LOCK, GPLL_CON0, NULL), + PLL(pll_3500, "fout_cpll", "fin_pll",BPLL_LOCK, CPLL_CON0, NULL), +};

drivers/clk/samsung/clk-exynos5250.c +3 -0

+
+struct __initdata samsung_pll_init_data epll_init_data =
+       PLL(pll_3600, "fout_epll", "fin_pll", EPLL_LOCK, EPLL_CON0,

NULL); +

drivers/clk/samsung/clk-exynos5250.c +2 -0

+struct __initdata samsung_pll_init_data vpll_init_data =
+       PLL(pll_3600, "fout_epll", "fin_pll", VPLL_LOCK, VPLL_CON0,

NULL); +

/* register exynox5250 clocks */
void __init exynos5250_clk_init(struct device_node *np) {

drivers/clk/samsung/clk-exynos5250.c +1 -1

@@ -519,44 +533,22 @@ void __init exynos5250_clk_init(struct device_node
 *np) samsung_clk_register_mux(exynos5250_pll_pmux_clks,
                        ARRAY_SIZE(exynos5250_pll_pmux_clks));
 
-       fin_pll_rate = _get_rate("fin_pll");
+       samsung_clk_register_pll(samsung_plls,

ARRAY_SIZE(samsung_plls)); +

        vpllsrc = __clk_lookup("mout_vpllsrc");
        if (vpllsrc)
                mout_vpllsrc_rate = clk_get_rate(vpllsrc);

drivers/clk/samsung/clk-exynos5250.c +1 -12

-       apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-                       reg_base, NULL, 0);
-       mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-                       reg_base + 0x4000, NULL, 0);
-       bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
-                       reg_base + 0x20010, NULL, 0);
-       gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
-                       reg_base + 0x10050, NULL, 0);
-       cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
-                       reg_base + 0x10020, NULL, 0);
-
+       fin_pll_rate = _get_rate("fin_pll");
        if (fin_pll_rate == (24 * MHZ)) {
-               epll = samsung_clk_register_pll36xx("fout_epll",

"fin_pll", - reg_base + 0x10030,
epll_24mhz_tbl, -
ARRAY_SIZE(epll_24mhz_tbl));

drivers/clk/samsung/clk-exynos5250.c +0 -3

-       } else {
-               pr_warn("%s: valid epll rate_table missing for\n"
-                       "parent fin_pll:%lu hz\n", __func__,

fin_pll_rate); - epll =
samsung_clk_register_pll36xx("fout_epll", "fin_pll", -

             reg_base + 0x10030, NULL, 0);

drivers/clk/samsung/clk-exynos5250.c +2 -1

+               epll_init_data.rate_table =  epll_24mhz_tb;
        }
+       samsung_clk_register_pll(&fout_epll_data, 1);
 
        if (mout_vpllsrc_rate == (24 * MHZ)) {
-               vpll = samsung_clk_register_pll36xx("fout_vpll",

"mout_vpllsrc" - , reg_base + 0x10040,
vpll_24mhz_tbl,

drivers/clk/samsung/clk-exynos5250.c +0 -6

-                       ARRAY_SIZE(vpll_24mhz_tbl));
-       } else {
-               pr_warn("%s: valid vpll rate_table missing for\n"
-                       "parent mout_vpllsrc_rate:%lu hz\n", __func__,
-                       mout_vpllsrc_rate);
-               samsung_clk_register_pll36xx("fout_vpll",

"mout_vpllsrc", - reg_base + 0x10040, NULL, 0);

drivers/clk/samsung/clk-exynos5250.c +2 -0

+               vpll_init_data.rate_table =  epll_24mhz_tb;
        }
+       samsung_clk_register_pll(&fout_epll_data, 1);
        samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
                        ARRAY_SIZE(exynos5250_fixed_rate_clks));

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.h index 4780e6c..3b02dc5 100644

drivers/clk/samsung/clk-pll.h +23 -0

@@ -39,6 +39,39 @@ struct samsung_pll_rate_table {
        unsigned int kdiv;
 };
 
+#define PLL(_type, _name, _pname, _lock_reg, _con_reg, _rate_table)   
 \ +       {                                                            
  \ +               .type   =       _type,                             
    \ +               .name   =       _name,                           
      \ +               .parent_name =  _pname,                        
        \ +               .flags  =       CLK_GET_RATE_NOCACHE,        
          \ +               .rate_table =   _rate_table,               
            \ +               .con_reg =      _con_reg,                
              \ +               .lock_reg =     _lock_reg,             
                \ +       }
+
+enum samsung_pll_type {
+       pll_3500,
+       pll_45xx,
+       pll_2550,
+       pll_3600,
+       pll_46xx,
+       pll_2660,
+};
+
+
+struct samsung_pll_init_data {
+       enum            samsung_pll_type type;
+       struct          samsung_pll_rate_table *rate_table;
+       const char      *name;
+       const char      *parent_name;
+       unsigned long   flags;
+       const void __iomem *con_reg;
+       const void __iomem *lock_reg;
+};
+
+extern int  __init samsung_clk_register_pll(struct

samsung_pll_init_data *data, + unsigned
int nr_pll);
+

Regards,
Yadwinder

Quoted 40 lines · On Thursday 13 of June 2013 12:32:05 Yadwinder Singh Brar wrote:

On Thu, Jun 13, 2013 at 3:32 AM, Andrew Bresticker

Quoted 31 lines · <[email protected]> wrote:

Doug,

Quoted 21 lines

Quoted 21 lines

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

Quoted 7 lines

Quoted 3 lines

        samsung_clk_register_pll(exynos4210_pll_clks,

                        ARRAY_SIZE(exynos4210_pll_clks));

Something like that looks like what I was thinking. I'd have to see it actually coded up to see if there's something I'm missing that would prevent us from doing that, but I don't see anything.

Quoted 5 lines

The only issue I see with this is that we may only want to register a rate table with a PLL only if fin_pll is running at a certain rate. On 5250 and 5420, for example, we have EPLL and VPLL rate tables that should only be registered if fin_pll is 24Mhz. We may have to register those separately, but this approach seems fine otherwise.

Quoted 6 lines

As Andrew Bresticker said, we will face problem with different table, and it will give some pain while handling such cases but I think overall code may look better.

Similar thoughts were their in my mind also, but i didn't want to disturb this series :).

Yes, I was thinking the same as well, but now that Exynos5420 doesn't follow the 0x100 register spacing, we have a problem :) .

Quoted 37 lines

Anyways, I think we can do it now only rather going for incremental patches after this series.
I was thinking to make samsung_clk_register_pllxxxx itself little generic instead
of using helper, as we are almost duplicating code for most PLLs.

A rough picture in my mind was,
After implementing generic samung_clk_register_pll(), code may look like below. Its just an idea, please feel free to correct it. Later we can factor out other common clk.ops for PLLs also.

this diff is over this series.
Assuming a generic samung_clk_register_pll() is their(which i think is not impossible) 8<----------------------------------------------------------------------
----

drivers/clk/samsung/clk-exynos5250.c +2 -0

@@ -493,6 +493,20 @@ static __initdata struct samsung_pll_rate_table
 epll_24mhz_tbl[] = {
        PLL_36XX_RATE(32768000, 131, 3, 5, 4719),
 };
 
+struct __initdata samsung_pll_init_data samsung_plls[] = {
+       PLL(pll_3500, "fout_apll", "fin_pll", APLL_LOCK, APLL_CON0,
 NULL), +       PLL(pll_3500, "fout_mpll", "fin_pll", MPLL_LOCK,
 MPLL_CON0, NULL), +       PLL(pll_3500, "fout_bpll",

"fin_pll",BPLL_LOCK, BPLL_CON0, NULL), + PLL(pll_3500,
"fout_gpll", "fin_pll",GPLL_LOCK, GPLL_CON0, NULL), + PLL(pll_3500, "fout_cpll", "fin_pll",BPLL_LOCK, CPLL_CON0, NULL), +};

drivers/clk/samsung/clk-exynos5250.c +3 -0

+
+struct __initdata samsung_pll_init_data epll_init_data =
+       PLL(pll_3600, "fout_epll", "fin_pll", EPLL_LOCK, EPLL_CON0,

NULL); +

drivers/clk/samsung/clk-exynos5250.c +2 -0

+struct __initdata samsung_pll_init_data vpll_init_data =
+       PLL(pll_3600, "fout_epll", "fin_pll", VPLL_LOCK, VPLL_CON0,

Quoted 1 line

NULL); +

This is mostly what I had in my mind. In addition I think I might have a solution for rate tables:

If we create another array

        struct samsung_pll_rate_table *rate_tables_24mhz[] = {
                apll_rate_table_24mhz,
                mpll_rate_table_24mhz, /* can be NULL as well, if no
support for rate change */
                epll_rate_table_24mhz,
                vpll_rate_table_24mhz,
                /* ... */
        };

which lists rate tables for given input frequency. This relies on making rate tables end with a sentinel, to remove the need of passing array sizes.

Quoted 10 lines

/* register exynox5250 clocks */
void __init exynos5250_clk_init(struct device_node *np) {

drivers/clk/samsung/clk-exynos5250.c +1 -1

@@ -519,44 +533,22 @@ void __init exynos5250_clk_init(struct device_node
 *np) samsung_clk_register_mux(exynos5250_pll_pmux_clks,
                        ARRAY_SIZE(exynos5250_pll_pmux_clks));
 
-       fin_pll_rate = _get_rate("fin_pll");
+       samsung_clk_register_pll(samsung_plls,

ARRAY_SIZE(samsung_plls)); +

...and then pass it here like:

        if (fin_pll_rate == 24 * MHZ) {
                samsung_clk_register_pll(samsung_plls,
ARRAY_SIZE(samsung_plls), rate_tables_24mhz);
        } else {
                /* warning or whatever */
                samsung_clk_register_pll(samsung_plls,
ARRAY_SIZE(samsung_plls), NULL);
        }

This way we could specify different rate tables depending on input frequency for a whole group of PLLs.

The only thing I don't like here is having two separate arrays that need to have the same sizes. Feel free to improve (or discard) this idea, though.

Best regards,
Tomasz

Quoted 93 lines

        vpllsrc = __clk_lookup("mout_vpllsrc");
        if (vpllsrc)
                mout_vpllsrc_rate = clk_get_rate(vpllsrc);

drivers/clk/samsung/clk-exynos5250.c +1 -12

-       apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-                       reg_base, NULL, 0);
-       mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-                       reg_base + 0x4000, NULL, 0);
-       bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
-                       reg_base + 0x20010, NULL, 0);
-       gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
-                       reg_base + 0x10050, NULL, 0);
-       cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
-                       reg_base + 0x10020, NULL, 0);
-
+       fin_pll_rate = _get_rate("fin_pll");
        if (fin_pll_rate == (24 * MHZ)) {
-               epll = samsung_clk_register_pll36xx("fout_epll",

"fin_pll", - reg_base + 0x10030,
epll_24mhz_tbl, -
ARRAY_SIZE(epll_24mhz_tbl));

drivers/clk/samsung/clk-exynos5250.c +0 -3

-       } else {
-               pr_warn("%s: valid epll rate_table missing for\n"
-                       "parent fin_pll:%lu hz\n", __func__,

fin_pll_rate); - epll =
samsung_clk_register_pll36xx("fout_epll", "fin_pll", -

             reg_base + 0x10030, NULL, 0);

drivers/clk/samsung/clk-exynos5250.c +2 -1

+               epll_init_data.rate_table =  epll_24mhz_tb;
        }
+       samsung_clk_register_pll(&fout_epll_data, 1);
 
        if (mout_vpllsrc_rate == (24 * MHZ)) {
-               vpll = samsung_clk_register_pll36xx("fout_vpll",

"mout_vpllsrc" - , reg_base + 0x10040,
vpll_24mhz_tbl,

drivers/clk/samsung/clk-exynos5250.c +0 -6

-                       ARRAY_SIZE(vpll_24mhz_tbl));
-       } else {
-               pr_warn("%s: valid vpll rate_table missing for\n"
-                       "parent mout_vpllsrc_rate:%lu hz\n", __func__,
-                       mout_vpllsrc_rate);
-               samsung_clk_register_pll36xx("fout_vpll",

"mout_vpllsrc", - reg_base + 0x10040, NULL, 0);

drivers/clk/samsung/clk-exynos5250.c +2 -0

+               vpll_init_data.rate_table =  epll_24mhz_tb;
        }
+       samsung_clk_register_pll(&fout_epll_data, 1);
        samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
                        ARRAY_SIZE(exynos5250_fixed_rate_clks));

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.h index 4780e6c..3b02dc5 100644

drivers/clk/samsung/clk-pll.h +23 -0

@@ -39,6 +39,39 @@ struct samsung_pll_rate_table {
        unsigned int kdiv;
 };
 
+#define PLL(_type, _name, _pname, _lock_reg, _con_reg, _rate_table)
 \ +       {
  \ +               .type   =       _type,
    \ +               .name   =       _name,
      \ +               .parent_name =  _pname,
        \ +               .flags  =       CLK_GET_RATE_NOCACHE,
          \ +               .rate_table =   _rate_table,
            \ +               .con_reg =      _con_reg,
              \ +               .lock_reg =     _lock_reg,
                \ +       }
+
+enum samsung_pll_type {
+       pll_3500,
+       pll_45xx,
+       pll_2550,
+       pll_3600,
+       pll_46xx,
+       pll_2660,
+};
+
+
+struct samsung_pll_init_data {
+       enum            samsung_pll_type type;
+       struct          samsung_pll_rate_table *rate_table;
+       const char      *name;
+       const char      *parent_name;
+       unsigned long   flags;
+       const void __iomem *con_reg;
+       const void __iomem *lock_reg;
+};
+
+extern int  __init samsung_clk_register_pll(struct

samsung_pll_init_data *data, + unsigned
int nr_pll);
+

Regards,
Yadwinder

On Thu, Jun 13, 2013 at 3:00 PM, Tomasz Figa <[email protected]>

Quoted 114 lines · On Thursday 13 of June 2013 12:32:05 Yadwinder Singh Brar wrote:

Quoted 43 lines

On Thu, Jun 13, 2013 at 3:32 AM, Andrew Bresticker

Quoted 32 lines · <[email protected]> wrote:

Doug,

Quoted 21 lines

Quoted 21 lines

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

Quoted 8 lines

Quoted 3 lines

        samsung_clk_register_pll(exynos4210_pll_clks,
        
                        ARRAY_SIZE(exynos4210_pll_clks));

Something like that looks like what I was thinking. I'd have to see
it actually coded up to see if there's something I'm missing that would prevent us from doing that, but I don't see anything.

Quoted 7 lines

The only issue I see with this is that we may only want to register a
rate table with a PLL only if fin_pll is running at a certain rate. On 5250 and 5420, for example, we have EPLL and VPLL rate tables that
should only be registered if fin_pll is 24Mhz. We may have to register those separately, but this approach seems fine otherwise.

Quoted 6 lines

As Andrew Bresticker said, we will face problem with different table, and it will give some pain while handling such cases but I think overall code may look better.

Similar thoughts were their in my mind also, but i didn't want to disturb this series :).

Yes, I was thinking the same as well, but now that Exynos5420 doesn't follow the 0x100 register spacing, we have a problem :) .

Quoted 40 lines

Anyways, I think we can do it now only rather going for incremental patches after this series.
I was thinking to make samsung_clk_register_pllxxxx itself little generic instead
of using helper, as we are almost duplicating code for most PLLs.

A rough picture in my mind was,
After implementing generic samung_clk_register_pll(), code may look like below. Its just an idea, please feel free to correct it. Later we can factor out other common clk.ops for PLLs also.

this diff is over this series.
Assuming a generic samung_clk_register_pll() is their(which i think is
not impossible) 8<------------------------------------------------------------------- --- ----

drivers/clk/samsung/clk-exynos5250.c +2 -0

@@ -493,6 +493,20 @@ static __initdata struct samsung_pll_rate_table
 epll_24mhz_tbl[] = {
 
        PLL_36XX_RATE(32768000, 131, 3, 5, 4719),
 
 };
 
+struct __initdata samsung_pll_init_data samsung_plls[] = {
+       PLL(pll_3500, "fout_apll", "fin_pll", APLL_LOCK, APLL_CON0,

NULL), + PLL(pll_3500, "fout_mpll", "fin_pll", MPLL_LOCK,
MPLL_CON0, NULL), + PLL(pll_3500, "fout_bpll",
"fin_pll",BPLL_LOCK, BPLL_CON0, NULL), + PLL(pll_3500,
"fout_gpll", "fin_pll",GPLL_LOCK, GPLL_CON0, NULL), + PLL(pll_3500, "fout_cpll", "fin_pll",BPLL_LOCK, CPLL_CON0, NULL), +};

drivers/clk/samsung/clk-exynos5250.c +3 -0

+
+struct __initdata samsung_pll_init_data epll_init_data =
+       PLL(pll_3600, "fout_epll", "fin_pll", EPLL_LOCK, EPLL_CON0,

NULL); +

drivers/clk/samsung/clk-exynos5250.c +2 -0

+struct __initdata samsung_pll_init_data vpll_init_data =
+       PLL(pll_3600, "fout_epll", "fin_pll", VPLL_LOCK, VPLL_CON0,

Quoted 1 line

NULL); +

This is mostly what I had in my mind. In addition I think I might have a solution for rate tables:

If we create another array

        struct samsung_pll_rate_table *rate_tables_24mhz[] = {
        
                apll_rate_table_24mhz,
                mpll_rate_table_24mhz, /* can be NULL as well, if no

support for rate change */

                epll_rate_table_24mhz,
                vpll_rate_table_24mhz,
                /* ... */
        
        };

Quoted 3 lines

which lists rate tables for given input frequency. This relies on making rate tables end with a sentinel, to remove the need of passing array sizes.

I think we may also have to make assumption that entries in the arrays rate_tables_24mhz[] and samsung_plls[] should be in same order in both arrays, and which may not be fair assumption, otherwise we have to use some mechanism to identify which rate_table is for which PLL, which will increase code and complexity.
Am I missed something or you are thinking something else?

Any thoughts from Doug or others ?

Quoted 27 lines

Quoted 11 lines

/* register exynox5250 clocks */
void __init exynos5250_clk_init(struct device_node *np) {

drivers/clk/samsung/clk-exynos5250.c +1 -1

@@ -519,44 +533,22 @@ void __init exynos5250_clk_init(struct
 device_node *np) samsung_clk_register_mux(exynos5250_pll_pmux_clks,
 
                        ARRAY_SIZE(exynos5250_pll_pmux_clks));
 
-       fin_pll_rate = _get_rate("fin_pll");
+       samsung_clk_register_pll(samsung_plls,

Quoted 1 line

ARRAY_SIZE(samsung_plls)); +

...and then pass it here like:

        if (fin_pll_rate == 24 * MHZ) {
        
                samsung_clk_register_pll(samsung_plls,

ARRAY_SIZE(samsung_plls), rate_tables_24mhz);

        } else {
        
                /* warning or whatever */
                samsung_clk_register_pll(samsung_plls,

ARRAY_SIZE(samsung_plls), NULL);

Quoted 4 lines

        }

This way we could specify different rate tables depending on input frequency for a whole group of PLLs.

I think code lines or complexity will be same.
Also all PLLs may not have same parent.

Quoted 3 lines

The only thing I don't like here is having two separate arrays that need to have the same sizes. Feel free to improve (or discard) this idea, though.

Sorry, I didn't get your point. You are talking about which two arrays?

Regards,
Yadwinder

Quoted 121 lines

Best regards,
Tomasz

Quoted 118 lines

        vpllsrc = __clk_lookup("mout_vpllsrc");
        if (vpllsrc)
        
                mout_vpllsrc_rate = clk_get_rate(vpllsrc);

drivers/clk/samsung/clk-exynos5250.c +1 -12

-       apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-                       reg_base, NULL, 0);
-       mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-                       reg_base + 0x4000, NULL, 0);
-       bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
-                       reg_base + 0x20010, NULL, 0);
-       gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
-                       reg_base + 0x10050, NULL, 0);
-       cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
-                       reg_base + 0x10020, NULL, 0);
-
+       fin_pll_rate = _get_rate("fin_pll");
 
        if (fin_pll_rate == (24 * MHZ)) {
 
-               epll = samsung_clk_register_pll36xx("fout_epll",

"fin_pll", - reg_base + 0x10030,
epll_24mhz_tbl, -
ARRAY_SIZE(epll_24mhz_tbl));

drivers/clk/samsung/clk-exynos5250.c +0 -3

-       } else {
-               pr_warn("%s: valid epll rate_table missing for\n"
-                       "parent fin_pll:%lu hz\n", __func__,

fin_pll_rate); - epll =
samsung_clk_register_pll36xx("fout_epll", "fin_pll", -

             reg_base + 0x10030, NULL, 0);

drivers/clk/samsung/clk-exynos5250.c +2 -1

+               epll_init_data.rate_table =  epll_24mhz_tb;
 
        }
 
+       samsung_clk_register_pll(&fout_epll_data, 1);
 
        if (mout_vpllsrc_rate == (24 * MHZ)) {
 
-               vpll = samsung_clk_register_pll36xx("fout_vpll",

"mout_vpllsrc" - , reg_base + 0x10040,
vpll_24mhz_tbl,

drivers/clk/samsung/clk-exynos5250.c +0 -4

-                       ARRAY_SIZE(vpll_24mhz_tbl));
-       } else {
-               pr_warn("%s: valid vpll rate_table missing for\n"
-                       "parent mout_vpllsrc_rate:%lu hz\n",

__func__,

drivers/clk/samsung/clk-exynos5250.c +0 -2

-                       mout_vpllsrc_rate);
-               samsung_clk_register_pll36xx("fout_vpll",

"mout_vpllsrc", - reg_base + 0x10040, NULL, 0);

drivers/clk/samsung/clk-exynos5250.c +2 -0

+               vpll_init_data.rate_table =  epll_24mhz_tb;
 
        }
 
+       samsung_clk_register_pll(&fout_epll_data, 1);
 
        samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
 
                        ARRAY_SIZE(exynos5250_fixed_rate_clks));

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.h index 4780e6c..3b02dc5 100644

drivers/clk/samsung/clk-pll.h +23 -0

@@ -39,6 +39,39 @@ struct samsung_pll_rate_table {
 
        unsigned int kdiv;
 
 };
 
+#define PLL(_type, _name, _pname, _lock_reg, _con_reg, _rate_table)
 \ +       {
 
  \ +               .type   =       _type,
 
    \ +               .name   =       _name,
 
      \ +               .parent_name =  _pname,
 
        \ +               .flags  =       CLK_GET_RATE_NOCACHE,
 
          \ +               .rate_table =   _rate_table,
 
            \ +               .con_reg =      _con_reg,
 
              \ +               .lock_reg =     _lock_reg,
 
                \ +       }
 
+
+enum samsung_pll_type {
+       pll_3500,
+       pll_45xx,
+       pll_2550,
+       pll_3600,
+       pll_46xx,
+       pll_2660,
+};
+
+
+struct samsung_pll_init_data {
+       enum            samsung_pll_type type;
+       struct          samsung_pll_rate_table *rate_table;
+       const char      *name;
+       const char      *parent_name;
+       unsigned long   flags;
+       const void __iomem *con_reg;
+       const void __iomem *lock_reg;
+};
+
+extern int  __init samsung_clk_register_pll(struct

samsung_pll_init_data *data, + unsigned
int nr_pll);
+

Regards,
Yadwinder

Quoted 1 line · On Friday 14 of June 2013 00:05:31 Yadwinder Singh Brar wrote:

On Thu, Jun 13, 2013 at 3:00 PM, Tomasz Figa <[email protected]>

Quoted 118 lines · wrote:

Quoted 114 lines · On Thursday 13 of June 2013 12:32:05 Yadwinder Singh Brar wrote:

Quoted 43 lines

On Thu, Jun 13, 2013 at 3:32 AM, Andrew Bresticker

Quoted 32 lines · <[email protected]> wrote:

Doug,

Quoted 21 lines

Quoted 21 lines

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

Quoted 8 lines

Quoted 3 lines

        samsung_clk_register_pll(exynos4210_pll_clks,

                        ARRAY_SIZE(exynos4210_pll_clks));

Something like that looks like what I was thinking. I'd have to see
it actually coded up to see if there's something I'm missing that would prevent us from doing that, but I don't see anything.

Quoted 7 lines

The only issue I see with this is that we may only want to register a
rate table with a PLL only if fin_pll is running at a certain rate. On 5250 and 5420, for example, we have EPLL and VPLL rate tables that
should only be registered if fin_pll is 24Mhz. We may have to register those separately, but this approach seems fine otherwise.

Quoted 6 lines

As Andrew Bresticker said, we will face problem with different table, and it will give some pain while handling such cases but I think overall code may look better.

Similar thoughts were their in my mind also, but i didn't want to disturb this series :).

Yes, I was thinking the same as well, but now that Exynos5420 doesn't follow the 0x100 register spacing, we have a problem :) .

Quoted 40 lines

Anyways, I think we can do it now only rather going for incremental patches after this series.
I was thinking to make samsung_clk_register_pllxxxx itself little generic instead
of using helper, as we are almost duplicating code for most PLLs.

A rough picture in my mind was,
After implementing generic samung_clk_register_pll(), code may look like below. Its just an idea, please feel free to correct it. Later we can factor out other common clk.ops for PLLs also.

this diff is over this series.
Assuming a generic samung_clk_register_pll() is their(which i think is
not impossible) 8<------------------------------------------------------------------- --- ----

drivers/clk/samsung/clk-exynos5250.c +2 -0

@@ -493,6 +493,20 @@ static __initdata struct samsung_pll_rate_table
 epll_24mhz_tbl[] = {
 
        PLL_36XX_RATE(32768000, 131, 3, 5, 4719),
 
 };
 
+struct __initdata samsung_pll_init_data samsung_plls[] = {
+       PLL(pll_3500, "fout_apll", "fin_pll", APLL_LOCK, APLL_CON0,

NULL), + PLL(pll_3500, "fout_mpll", "fin_pll", MPLL_LOCK,
MPLL_CON0, NULL), + PLL(pll_3500, "fout_bpll",
"fin_pll",BPLL_LOCK, BPLL_CON0, NULL), + PLL(pll_3500,
"fout_gpll", "fin_pll",GPLL_LOCK, GPLL_CON0, NULL), + PLL(pll_3500, "fout_cpll", "fin_pll",BPLL_LOCK, CPLL_CON0, NULL), +};

drivers/clk/samsung/clk-exynos5250.c +3 -0

+
+struct __initdata samsung_pll_init_data epll_init_data =
+       PLL(pll_3600, "fout_epll", "fin_pll", EPLL_LOCK, EPLL_CON0,

NULL); +

drivers/clk/samsung/clk-exynos5250.c +2 -0

+struct __initdata samsung_pll_init_data vpll_init_data =
+       PLL(pll_3600, "fout_epll", "fin_pll", VPLL_LOCK, VPLL_CON0,

Quoted 1 line

NULL); +

This is mostly what I had in my mind. In addition I think I might have a solution for rate tables:

If we create another array

        struct samsung_pll_rate_table *rate_tables_24mhz[] = {

                apll_rate_table_24mhz,
                mpll_rate_table_24mhz, /* can be NULL as well, if no

support for rate change */

                epll_rate_table_24mhz,
                vpll_rate_table_24mhz,
                /* ... */

        };

Quoted 3 lines

which lists rate tables for given input frequency. This relies on making rate tables end with a sentinel, to remove the need of passing array sizes.

Quoted 6 lines

I think we may also have to make assumption that entries in the arrays rate_tables_24mhz[] and samsung_plls[] should be in same order in both arrays, and which may not be fair assumption, otherwise we have to use some mechanism to identify which rate_table is for which PLL, which will increase code and complexity.
Am I missed something or you are thinking something else?

Yes, this is exactly what I thought. The order and size of rate_tables_24mhz[] would have to be the same as of samsung_plls[], which shouldn't be a problem technically, but adds another responsibility to the person who defines them.

Quoted 34 lines

Any thoughts from Doug or others ?

Quoted 31 lines

Quoted 11 lines

/* register exynox5250 clocks */
void __init exynos5250_clk_init(struct device_node *np) {

drivers/clk/samsung/clk-exynos5250.c +1 -1

@@ -519,44 +533,22 @@ void __init exynos5250_clk_init(struct
 device_node *np) samsung_clk_register_mux(exynos5250_pll_pmux_clks,
 
                        ARRAY_SIZE(exynos5250_pll_pmux_clks));
 
-       fin_pll_rate = _get_rate("fin_pll");
+       samsung_clk_register_pll(samsung_plls,

Quoted 1 line

ARRAY_SIZE(samsung_plls)); +

...and then pass it here like:

        if (fin_pll_rate == 24 * MHZ) {

                samsung_clk_register_pll(samsung_plls,

ARRAY_SIZE(samsung_plls), rate_tables_24mhz);

        } else {

                /* warning or whatever */
                samsung_clk_register_pll(samsung_plls,

ARRAY_SIZE(samsung_plls), NULL);

        }

This way we could specify different rate tables depending on input frequency for a whole group of PLLs.

Quoted 2 lines

I think code lines or complexity will be same.
Also all PLLs may not have same parent.

Most of them usually do. Anyway, they can be grouped by the parent, e.g. all that have fin_pll as input can use one set of arrays and other can have their own set.

Quoted 4 lines

Quoted 3 lines

The only thing I don't like here is having two separate arrays that need to have the same sizes. Feel free to improve (or discard) this idea, though.

Quoted 1 line

Sorry, I didn't get your point. You are talking about which two arrays?

I mean that the code would need to assume that

samsung_plls[] and rate_tables_24mhz[]

have the same sizes (and orders), which is not a perfect solution, but I can't think of anything better at the moment.

Best regards,
Tomasz

Quoted 124 lines

Regards,
Yadwinder

Quoted 121 lines

Best regards,
Tomasz

Quoted 118 lines

        vpllsrc = __clk_lookup("mout_vpllsrc");
        if (vpllsrc)

                mout_vpllsrc_rate = clk_get_rate(vpllsrc);

drivers/clk/samsung/clk-exynos5250.c +1 -12

-       apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-                       reg_base, NULL, 0);
-       mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-                       reg_base + 0x4000, NULL, 0);
-       bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
-                       reg_base + 0x20010, NULL, 0);
-       gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
-                       reg_base + 0x10050, NULL, 0);
-       cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
-                       reg_base + 0x10020, NULL, 0);
-
+       fin_pll_rate = _get_rate("fin_pll");
 
        if (fin_pll_rate == (24 * MHZ)) {
 
-               epll = samsung_clk_register_pll36xx("fout_epll",

"fin_pll", - reg_base + 0x10030,
epll_24mhz_tbl, -
ARRAY_SIZE(epll_24mhz_tbl));

drivers/clk/samsung/clk-exynos5250.c +0 -3

-       } else {
-               pr_warn("%s: valid epll rate_table missing for\n"
-                       "parent fin_pll:%lu hz\n", __func__,

fin_pll_rate); - epll =
samsung_clk_register_pll36xx("fout_epll", "fin_pll", -

             reg_base + 0x10030, NULL, 0);

drivers/clk/samsung/clk-exynos5250.c +2 -1

+               epll_init_data.rate_table =  epll_24mhz_tb;
 
        }
 
+       samsung_clk_register_pll(&fout_epll_data, 1);
 
        if (mout_vpllsrc_rate == (24 * MHZ)) {
 
-               vpll = samsung_clk_register_pll36xx("fout_vpll",

"mout_vpllsrc" - , reg_base + 0x10040,
vpll_24mhz_tbl,

drivers/clk/samsung/clk-exynos5250.c +0 -4

-                       ARRAY_SIZE(vpll_24mhz_tbl));
-       } else {
-               pr_warn("%s: valid vpll rate_table missing for\n"
-                       "parent mout_vpllsrc_rate:%lu hz\n",

__func__,

drivers/clk/samsung/clk-exynos5250.c +0 -2

-                       mout_vpllsrc_rate);
-               samsung_clk_register_pll36xx("fout_vpll",

"mout_vpllsrc", - reg_base + 0x10040, NULL, 0);

drivers/clk/samsung/clk-exynos5250.c +2 -0

+               vpll_init_data.rate_table =  epll_24mhz_tb;
 
        }
 
+       samsung_clk_register_pll(&fout_epll_data, 1);
 
        samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
 
                        ARRAY_SIZE(exynos5250_fixed_rate_clks));

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.h index 4780e6c..3b02dc5 100644

drivers/clk/samsung/clk-pll.h +23 -0

@@ -39,6 +39,39 @@ struct samsung_pll_rate_table {
 
        unsigned int kdiv;
 
 };
 
+#define PLL(_type, _name, _pname, _lock_reg, _con_reg, _rate_table)
 \ +       {
 
  \ +               .type   =       _type,
 
    \ +               .name   =       _name,
 
      \ +               .parent_name =  _pname,
 
        \ +               .flags  =       CLK_GET_RATE_NOCACHE,
 
          \ +               .rate_table =   _rate_table,
 
            \ +               .con_reg =      _con_reg,
 
              \ +               .lock_reg =     _lock_reg,
 
                \ +       }
 
+
+enum samsung_pll_type {
+       pll_3500,
+       pll_45xx,
+       pll_2550,
+       pll_3600,
+       pll_46xx,
+       pll_2660,
+};
+
+
+struct samsung_pll_init_data {
+       enum            samsung_pll_type type;
+       struct          samsung_pll_rate_table *rate_table;
+       const char      *name;
+       const char      *parent_name;
+       unsigned long   flags;
+       const void __iomem *con_reg;
+       const void __iomem *lock_reg;
+};
+
+extern int  __init samsung_clk_register_pll(struct

samsung_pll_init_data *data, + unsigned
int nr_pll);
+

Regards,
Yadwinder

This patch defines a common rate_table which will contain recommended p, m, s, k values for supported rates that needs to be changed for changing corresponding PLL's rate.

Reviewed-by Doug Anderson <[email protected]>

Signed-off-by Yadwinder Singh Brar <[email protected]>

---
 drivers/clk/samsung/clk-exynos4.c    |    8 +++---
 drivers/clk/samsung/clk-exynos5250.c |   14 ++++++------
 drivers/clk/samsung/clk-pll.c        |   22 +++++++++++++++++++-
 drivers/clk/samsung/clk-pll.h        |   35 ++++++++++++++++++++++++++++++++-
 4 files changed, 64 insertions(+), 15 deletions(-)

drivers/clk/samsung/clk-exynos4.c +4 -4

index ba33bc6..e02a342 100644
@@ -1028,13 +1028,13 @@ void __init exynos4_clk_init(struct device_node *np, enum exynos4_soc exynos4_so
                                        reg_base + VPLL_CON0, pll_4650c);
        } else {
                apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-                                       reg_base + APLL_LOCK);
+                                       reg_base + APLL_LOCK, NULL, 0);
                mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-                                       reg_base + E4X12_MPLL_LOCK);
+                                       reg_base + E4X12_MPLL_LOCK, NULL, 0);
                epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
-                                       reg_base + EPLL_LOCK);
+                                       reg_base + EPLL_LOCK, NULL, 0);
                vpll = samsung_clk_register_pll36xx("fout_vpll", "fin_pll",
-                                       reg_base + VPLL_LOCK);
+                                       reg_base + VPLL_LOCK, NULL, 0);
        }
 
        samsung_clk_add_lookup(apll, fout_apll);

drivers/clk/samsung/clk-exynos5250.c +7 -7

index 687b580..ddf10ca 100644
@@ -491,19 +491,19 @@ void __init exynos5250_clk_init(struct device_node *np)
                        ext_clk_match);
 
        apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-                       reg_base);
+                       reg_base, NULL, 0);
        mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-                       reg_base + 0x4000);
+                       reg_base + 0x4000, NULL, 0);
        bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
-                       reg_base + 0x20010);
+                       reg_base + 0x20010, NULL, 0);
        gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
-                       reg_base + 0x10050);
+                       reg_base + 0x10050, NULL, 0);
        cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
-                       reg_base + 0x10020);
+                       reg_base + 0x10020, NULL, 0);
        epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
-                       reg_base + 0x10030);
+                       reg_base + 0x10030, NULL, 0);
        vpll = samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc",
-                       reg_base + 0x10040);
+                       reg_base + 0x10040, NULL, 0);
 
        samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
                        ARRAY_SIZE(exynos5250_fixed_rate_clks));

drivers/clk/samsung/clk-pll.c +14 -2

index a7d8ad9..cba73a4 100644
@@ -16,6 +16,8 @@
 struct samsung_clk_pll {
        struct clk_hw           hw;
        const void __iomem      *base;
+       const struct samsung_pll_rate_table *rate_table;
+       unsigned int rate_count;
 };
 
 #define to_clk_pll(_hw) container_of(_hw, struct samsung_clk_pll, hw)

@@ -62,7 +64,9 @@ static const struct clk_ops samsung_pll35xx_clk_ops = {
 };
 
 struct clk * __init samsung_clk_register_pll35xx(const char *name,
-                       const char *pname, const void __iomem *base)
+                       const char *pname, const void __iomem *base,
+                       const struct samsung_pll_rate_table *rate_table,
+                       const unsigned int rate_count)
 {
        struct samsung_clk_pll *pll;
        struct clk *clk;

@@ -80,6 +84,12 @@ struct clk * __init samsung_clk_register_pll35xx(const char *name,
        init.parent_names = &pname;
        init.num_parents = 1;
 
+       if (rate_table && rate_count) {
+               pll->rate_count = rate_count;
+               pll->rate_table = kmemdup(rate_table, rate_count *
+                       sizeof(struct samsung_pll_rate_table), GFP_KERNEL);
+       }
+
        pll->hw.init = &init;
        pll->base = base;
 

@@ -137,7 +147,9 @@ static const struct clk_ops samsung_pll36xx_clk_ops = {
 };
 
 struct clk * __init samsung_clk_register_pll36xx(const char *name,
-                       const char *pname, const void __iomem *base)
+                       const char *pname, const void __iomem *base,
+                       const struct samsung_pll_rate_table *rate_table,
+                       const unsigned int rate_count)
 {
        struct samsung_clk_pll *pll;
        struct clk *clk;

Yadwinder,

On Mon, Jun 3, 2013 at 8:09 AM, Yadwinder Singh Brar

Quoted 6 lines · <[email protected]> wrote:

This patch defines a common rate_table which will contain recommended p, m, s, k values for supported rates that needs to be changed for changing corresponding PLL's rate.

Reviewed-by Doug Anderson <[email protected]>

Signed-off-by Yadwinder Singh Brar <[email protected]>

Quoted 8 lines

---

 drivers/clk/samsung/clk-exynos4.c    |    8 +++---
 drivers/clk/samsung/clk-exynos5250.c |   14 ++++++------
 drivers/clk/samsung/clk-pll.c        |   22 +++++++++++++++++++-
 drivers/clk/samsung/clk-pll.h        |   35

++++++++++++++++++++++++++++++++- 4 files changed, 64 insertions(+), 15 deletions(-)

Using something like patman
<http://git.denx.de/?p=u-boot.git;a=blob;f=tools/patman/README;hb=refs/h
eads/master> would really help here so you could get some version history. I see it in 0/6 but that's a bit of a pain...

Did you and Tomasz ever come to an agreement about whether the fin freq needs to be specified with the PMSK values?

Quoted 164 lines

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-exynos4.c index ba33bc6..e02a342 100644

drivers/clk/samsung/clk-exynos4.c +0 -0

@@ -1028,13 +1028,13 @@ void __init exynos4_clk_init(struct

device_node *np, enum exynos4_soc exynos4_so>

                                        reg_base + VPLL_CON0,
                                        pll_4650c);
        
        } else {
        
                apll = samsung_clk_register_pll35xx("fout_apll",
                "fin_pll",

drivers/clk/samsung/clk-exynos4.c +1 -1

-                                       reg_base + APLL_LOCK);
+                                       reg_base + APLL_LOCK, NULL,

0);

mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",

drivers/clk/samsung/clk-exynos4.c +1 -1

-                                       reg_base + E4X12_MPLL_LOCK);
+                                       reg_base + E4X12_MPLL_LOCK,

NULL, 0);>
epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",

drivers/clk/samsung/clk-exynos4.c +1 -1

-                                       reg_base + EPLL_LOCK);
+                                       reg_base + EPLL_LOCK, NULL,

0);

vpll = samsung_clk_register_pll36xx("fout_vpll", "fin_pll",

drivers/clk/samsung/clk-exynos4.c +1 -1

-                                       reg_base + VPLL_LOCK);
+                                       reg_base + VPLL_LOCK, NULL,

0);

        }
        
        samsung_clk_add_lookup(apll, fout_apll);

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-exynos5250.c index 687b580..ddf10ca 100644

drivers/clk/samsung/clk-exynos5250.c +0 -0

@@ -491,19 +491,19 @@ void __init exynos5250_clk_init(struct

device_node *np)>

                        ext_clk_match);
        
        apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",

drivers/clk/samsung/clk-exynos5250.c +7 -7

-                       reg_base);
+                       reg_base, NULL, 0);
 
        mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
 
-                       reg_base + 0x4000);
+                       reg_base + 0x4000, NULL, 0);
 
        bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
 
-                       reg_base + 0x20010);
+                       reg_base + 0x20010, NULL, 0);
 
        gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
 
-                       reg_base + 0x10050);
+                       reg_base + 0x10050, NULL, 0);
 
        cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
 
-                       reg_base + 0x10020);
+                       reg_base + 0x10020, NULL, 0);
 
        epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
 
-                       reg_base + 0x10030);
+                       reg_base + 0x10030, NULL, 0);
 
        vpll = samsung_clk_register_pll36xx("fout_vpll",
        "mout_vpllsrc",
 
-                       reg_base + 0x10040);
+                       reg_base + 0x10040, NULL, 0);
 
        samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
 
                        ARRAY_SIZE(exynos5250_fixed_rate_clks));

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.c index a7d8ad9..cba73a4 100644

drivers/clk/samsung/clk-pll.c +2 -0

@@ -16,6 +16,8 @@
 
 struct samsung_clk_pll {
 
        struct clk_hw           hw;
        const void __iomem      *base;
 
+       const struct samsung_pll_rate_table *rate_table;
+       unsigned int rate_count;

};

#define to_clk_pll(_hw) container_of(_hw, struct samsung_clk_pll, hw)

drivers/clk/samsung/clk-pll.c +0 -0

@@ -62,7 +64,9 @@ static const struct clk_ops samsung_pll35xx_clk_ops

= {>
};

struct clk * __init samsung_clk_register_pll35xx(const char *name,

drivers/clk/samsung/clk-pll.c +2 -1

-                       const char *pname, const void __iomem *base)
+                       const char *pname, const void __iomem *base,
+                       const struct samsung_pll_rate_table

*rate_table, + const unsigned int rate_count)

{

        struct samsung_clk_pll *pll;
        struct clk *clk;

drivers/clk/samsung/clk-pll.c +0 -0

@@ -80,6 +84,12 @@ struct clk * __init

samsung_clk_register_pll35xx(const char *name,>

        init.parent_names = &pname;
        init.num_parents = 1;

drivers/clk/samsung/clk-pll.c +4 -0

+       if (rate_table && rate_count) {
+               pll->rate_count = rate_count;
+               pll->rate_table = kmemdup(rate_table, rate_count *
+                       sizeof(struct samsung_pll_rate_table),

GFP_KERNEL); + }

drivers/clk/samsung/clk-pll.c +1 -0

+
 
        pll->hw.init = &init;
        pll->base = base;

@@ -137,7 +147,9 @@ static const struct clk_ops

samsung_pll36xx_clk_ops = {>
};

struct clk * __init samsung_clk_register_pll36xx(const char *name,

drivers/clk/samsung/clk-pll.c +2 -1

-                       const char *pname, const void __iomem *base)
+                       const char *pname, const void __iomem *base,
+                       const struct samsung_pll_rate_table

*rate_table, + const unsigned int rate_count)

{

        struct samsung_clk_pll *pll;
        struct clk *clk;

drivers/clk/samsung/clk-pll.c +0 -0

@@ -155,6 +167,12 @@ struct clk * __init

samsung_clk_register_pll36xx(const char *name,>

        init.parent_names = &pname;
        init.num_parents = 1;

drivers/clk/samsung/clk-pll.c +4 -0

+       if (rate_table && rate_count) {
+               pll->rate_count = rate_count;
+               pll->rate_table = kmemdup(rate_table, rate_count *
+                       sizeof(struct samsung_pll_rate_table),

Quoted 1 line

GFP_KERNEL);

To me I'd rather see the tables left as "init", not "init_data" and avoid the strdup(). The small amount of waste from multiple tables doesn't seem worth the extra allocation.

...but I don't really care much either way.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

This patch add set_rate() and round_rate() for PLL35xx

Reviewed-by Doug Anderson <[email protected]>

Signed-off-by Yadwinder Singh Brar <[email protected]>

---
 drivers/clk/samsung/clk-pll.c |  104

++++++++++++++++++++++++++++++++++++++++- 1 files changed, 103 insertions(+), 1 deletions(-)

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.c index cba73a4..319b52b 100644

drivers/clk/samsung/clk-pll.c +29 -0

+{
+	const struct samsung_pll_rate_table  *rate_table = pll-
 rate_table;
+	int i;
+
+	for (i = 0; i < pll->rate_count; i++) {
+		if (rate == rate_table[i].rate)
+			return &rate_table[i];
+	}
+
+	return NULL;
+}
+
+static long samsung_pll_round_rate(struct clk_hw *hw,
+			unsigned long drate, unsigned long *prate)
+{
+	struct samsung_clk_pll *pll = to_clk_pll(hw);
+	const struct samsung_pll_rate_table *rate_table = pll->rate_table;
+	int i;
+
+	/* Assumming rate_table is in descending order */
+	for (i = 0; i < pll->rate_count; i++) {
+		if (drate >= rate_table[i].rate)
+			return rate_table[i].rate;
+	}
+
+	/* return minimum supported value */
+	return rate_table[i - 1].rate;
+}
+
 /*
  * PLL35xx Clock Type

*/

drivers/clk/samsung/clk-pll.c +5 -0

@@ -34,12 +65,17 @@ struct samsung_clk_pll {
 #define PLL35XX_CON0_OFFSET	(0x100)
 #define PLL35XX_CON1_OFFSET	(0x104)
 
+/* Maximum lock time can be 270 * PDIV cycles */
+#define PLL35XX_LOCK_FACTOR	(270)
+
 #define PLL35XX_MDIV_MASK       (0x3FF)
 #define PLL35XX_PDIV_MASK       (0x3F)
 #define PLL35XX_SDIV_MASK       (0x7)
+#define PLL35XX_LOCK_STAT_MASK  (0x1)
 #define PLL35XX_MDIV_SHIFT      (16)
 #define PLL35XX_PDIV_SHIFT      (8)
 #define PLL35XX_SDIV_SHIFT      (0)
+#define PLL35XX_LOCK_STAT_SHIFT	(29)
 
 static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,
 				unsigned long parent_rate)

@@ -59,8 +95,72 @@ static unsigned long

samsung_pll35xx_recalc_rate(struct clk_hw *hw, return (unsigned long)fvco;
}

drivers/clk/samsung/clk-pll.c +12 -0

+static inline bool samsung_pll35xx_mp_change(
+		const struct samsung_pll_rate_table *rate, u32 pll_con)
+{
+	u32 old_mdiv, old_pdiv;
+
+	old_mdiv = (pll_con >> PLL35XX_MDIV_SHIFT) & PLL35XX_MDIV_MASK;
+	old_pdiv = (pll_con >> PLL35XX_PDIV_SHIFT) & PLL35XX_PDIV_MASK;
+
+	return (rate->mdiv != old_mdiv || rate->pdiv != old_pdiv);
+}
+
+static int samsung_pll35xx_set_rate(struct clk_hw *hw, unsigned long

drate, + unsigned long prate)

drivers/clk/samsung/clk-pll.c +22 -1

+		tmp |= (rate->mdiv << PLL35XX_MDIV_SHIFT) |
+				(rate->pdiv << PLL35XX_PDIV_SHIFT) |
+				(rate->sdiv << PLL35XX_SDIV_SHIFT);
+		pll_writel(pll, tmp, PLL35XX_CON0_OFFSET);
+
+		/* wait_lock_time */
+		do {
+			cpu_relax();
+			tmp = pll_readl(pll, PLL35XX_CON0_OFFSET);
+		} while (!(tmp & (PLL35XX_LOCK_STAT_MASK
+				<< PLL35XX_LOCK_STAT_SHIFT)));
+	}
+
+	return 0;
+}
+
 static const struct clk_ops samsung_pll35xx_clk_ops = {
 	.recalc_rate = samsung_pll35xx_recalc_rate,
+	.round_rate = samsung_pll_round_rate,
+	.set_rate = samsung_pll35xx_set_rate,
+};
+
+static const struct clk_ops samsung_pll35xx_clk_min_ops = {
+	.recalc_rate = samsung_pll35xx_recalc_rate,
 };
 
 struct clk * __init samsung_clk_register_pll35xx(const char *name,

@@ -79,7 +179,6 @@ struct clk * __init
 samsung_clk_register_pll35xx(const char *name, }
 
 	init.name = name;
-	init.ops = &samsung_pll35xx_clk_ops;
 	init.flags = CLK_GET_RATE_NOCACHE;
 	init.parent_names = &pname;
 	init.num_parents = 1;

@@ -88,6 +187,9 @@ struct clk * __init

samsung_clk_register_pll35xx(const char *name, pll->rate_count = rate_count;

drivers/clk/samsung/clk-pll.c +3 -0

+		init.ops = &samsung_pll35xx_clk_ops;
+	} else {
+		init.ops = &samsung_pll35xx_clk_min_ops;
 	}
 
 	pll->hw.init = &init;

From: Vikas Sajjan <[email protected]>

This patch adds set_rate and round_rate clk_ops for PLL36xx

Reviewed-by Tomasz Figa <[email protected]>

Reviewed-by Doug Anderson <[email protected]>

Signed-off-by Vikas Sajjan <[email protected]>

---
 drivers/clk/samsung/clk-pll.c |   79

++++++++++++++++++++++++++++++++++++++++- 1 files changed, 78 insertions(+), 1 deletions(-)

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.c index 319b52b..42b60b5 100644

drivers/clk/samsung/clk-pll.c +0 -0

@@ -215,6 +215,9 @@ struct clk * __init

samsung_clk_register_pll35xx(const char *name, #define
PLL36XX_CON0_OFFSET (0x100)
#define PLL36XX_CON1_OFFSET (0x104)

drivers/clk/samsung/clk-pll.c +3 -0

+/* Maximum lock time can be 3000 * PDIV cycles */
+#define PLL36XX_LOCK_FACTOR	(3000)
+
 #define PLL36XX_KDIV_MASK	(0xFFFF)
 #define PLL36XX_MDIV_MASK	(0x1FF)
 #define PLL36XX_PDIV_MASK	(0x3F)

@@ -222,6 +225,8 @@ struct clk * __init

samsung_clk_register_pll35xx(const char *name, #define
PLL36XX_MDIV_SHIFT (16)
#define PLL36XX_PDIV_SHIFT (8)
#define PLL36XX_SDIV_SHIFT (0)

drivers/clk/samsung/clk-pll.c +2 -0

+#define PLL36XX_KDIV_SHIFT	(0)
+#define PLL36XX_LOCK_STAT_SHIFT	(29)
 
 static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,
 				unsigned long parent_rate)

@@ -244,8 +249,78 @@ static unsigned long

samsung_pll36xx_recalc_rate(struct clk_hw *hw, return (unsigned long)fvco;
}

drivers/clk/samsung/clk-pll.c +12 -0

+{
+	u32 old_mdiv, old_pdiv, old_kdiv;
+
+	old_mdiv = (pll_con0 >> PLL36XX_MDIV_SHIFT) & PLL36XX_MDIV_MASK;
+	old_pdiv = (pll_con0 >> PLL36XX_PDIV_SHIFT) & PLL36XX_PDIV_MASK;
+	old_kdiv = (pll_con1 >> PLL36XX_KDIV_SHIFT) & PLL36XX_KDIV_MASK;
+
+	return (rate->mdiv != old_mdiv || rate->pdiv != old_pdiv ||
+		rate->kdiv != old_kdiv);
+}
+
+static int samsung_pll36xx_set_rate(struct clk_hw *hw, unsigned long

drate, + unsigned long parent_rate)

drivers/clk/samsung/clk-pll.c +35 -1

+		return 0;
+	}
+
+	/* Set PLL lock time. */
+	pll_writel(pll, (rate->pdiv * PLL36XX_LOCK_FACTOR),
+			PLL36XX_LOCK_OFFSET);
+
+	 /* Change PLL PMS values */
+	pll_con0 &= ~((PLL36XX_MDIV_MASK << PLL36XX_MDIV_SHIFT) |
+			(PLL36XX_PDIV_MASK << PLL36XX_PDIV_SHIFT) |
+			(PLL36XX_SDIV_MASK << PLL36XX_SDIV_SHIFT));
+	pll_con0 |= (rate->mdiv << PLL36XX_MDIV_SHIFT) |
+			(rate->pdiv << PLL36XX_PDIV_SHIFT) |
+			(rate->sdiv << PLL36XX_SDIV_SHIFT);
+	pll_writel(pll, pll_con0, PLL36XX_CON0_OFFSET);
+
+	pll_con1 &= ~(PLL36XX_KDIV_MASK << PLL36XX_KDIV_SHIFT);
+	pll_con1 |= rate->kdiv << PLL36XX_KDIV_SHIFT;
+	pll_writel(pll, pll_con1, PLL36XX_CON1_OFFSET);
+
+	/* wait_lock_time */
+	do {
+		cpu_relax();
+		tmp = pll_readl(pll, PLL36XX_CON0_OFFSET);
+	} while (!(tmp & (1 << PLL36XX_LOCK_STAT_SHIFT)));
+
+	return 0;
+}
+
 static const struct clk_ops samsung_pll36xx_clk_ops = {
 	.recalc_rate = samsung_pll36xx_recalc_rate,
+	.set_rate = samsung_pll36xx_set_rate,
+	.round_rate = samsung_pll_round_rate,
+};
+
+static const struct clk_ops samsung_pll36xx_clk_min_ops = {
+	.recalc_rate = samsung_pll36xx_recalc_rate,
 };
 
 struct clk * __init samsung_clk_register_pll36xx(const char *name,

@@ -264,7 +339,6 @@ struct clk * __init
 samsung_clk_register_pll36xx(const char *name, }
 
 	init.name = name;
-	init.ops = &samsung_pll36xx_clk_ops;
 	init.flags = CLK_GET_RATE_NOCACHE;
 	init.parent_names = &pname;
 	init.num_parents = 1;

@@ -273,6 +347,9 @@ struct clk * __init

samsung_clk_register_pll36xx(const char *name, pll->rate_count = rate_count;

drivers/clk/samsung/clk-pll.c +3 -0

+		init.ops = &samsung_pll36xx_clk_ops;
+	} else {
+		init.ops = &samsung_pll36xx_clk_min_ops;
 	}
 
 	pll->hw.init = &init;

From: Vikas Sajjan <[email protected]>

While trying to get rate of "mout_vpllsrc" MUX (parent) for registering the "fout_vpll" (child), we found get rate was failing.

So this patch moves the mout_vpllsrc MUX out of the existing common list and registers the mout_vpllsrc MUX before the PLL registrations.

Signed-off-by Vikas Sajjan <[email protected]>

Signed-off-by Yadwinder Singh Brar <[email protected]>

---
 drivers/clk/samsung/clk-exynos5250.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-exynos5250.c index ddf10ca..70cc6cf 100644

drivers/clk/samsung/clk-exynos5250.c +4 -0

@@ -207,6 +207,10 @@ struct samsung_fixed_factor_clock
 exynos5250_fixed_factor_clks[] __initdata = { FFACTOR(none,
 "fout_bplldiv2", "fout_bpll", 1, 2, 0),
 };
 
+struct samsung_mux_clock exynos5250_pll_pmux_clks[] __initdata = {
+	MUX(none, "mout_vpllsrc", mout_vpllsrc_p, SRC_TOP2, 0, 1),
+};
+
 struct samsung_mux_clock exynos5250_mux_clks[] __initdata = {
 	MUX(none, "mout_apll", mout_apll_p, SRC_CPU, 0, 1),

drivers/clk/samsung/clk-exynos5250.c +3 -1

 	MUX(none, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1),

@@ -214,7 +218,6 @@ struct samsung_mux_clock exynos5250_mux_clks[]
 __initdata = { MUX(none, "sclk_mpll", mout_mpll_p, SRC_CORE1, 8, 1),
 	MUX(none, "mout_bpll_fout", mout_bpll_fout_p, PLL_DIV2_SEL, 0, 1),
 	MUX(none, "sclk_bpll", mout_bpll_p, SRC_CDREX, 0, 1),
-	MUX(none, "mout_vpllsrc", mout_vpllsrc_p, SRC_TOP2, 0, 1),
 	MUX(none, "sclk_vpll", mout_vpll_p, SRC_TOP2, 16, 1),
 	MUX(none, "sclk_epll", mout_epll_p, SRC_TOP2, 12, 1),
 	MUX(none, "sclk_cpll", mout_cpll_p, SRC_TOP2, 8, 1),

@@ -490,6 +493,9 @@ void __init exynos5250_clk_init(struct device_node
 *np) ARRAY_SIZE(exynos5250_fixed_rate_ext_clks),
 			ext_clk_match);
 
+	samsung_clk_register_mux(exynos5250_pll_pmux_clks,
+			ARRAY_SIZE(exynos5250_pll_pmux_clks));
+
 	apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
 			reg_base, NULL, 0);
 	mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",

Adds the EPLL and VPLL freq table for exynos5250 SoC.

Signed-off-by Vikas Sajjan <[email protected]>

---
 drivers/clk/samsung/clk-exynos5250.c |   53 ++++++++++++++++++++++++++++++++--
 drivers/clk/samsung/clk.h            |    2 +
 2 files changed, 52 insertions(+), 3 deletions(-)

drivers/clk/samsung/clk-exynos5250.c +28 -1

index 70cc6cf..f98c19d 100644
@@ -472,11 +472,34 @@ static __initdata struct of_device_id ext_clk_match[] = {
        { },
 };
 
+static __initdata struct samsung_pll_rate_table vpll_24mhz_tbl[] = {
+       /* sorted in descending order */
+       /* PLL_36XX_RATE(rate, m, p, s, k) */
+       PLL_36XX_RATE(266000000, 266, 3, 3, 0),
+       /* Not in UM, but need for eDP on snow */
+       PLL_36XX_RATE(70500000, 94, 2, 4, 0),
+};
+
+static __initdata struct samsung_pll_rate_table epll_24mhz_tbl[] = {
+       /* sorted in descending order */
+       /* PLL_36XX_RATE(rate, m, p, s, k) */
+       PLL_36XX_RATE(192000000, 48, 3, 1, 0),
+       PLL_36XX_RATE(180633600, 45, 3, 1, 10381),
+       PLL_36XX_RATE(180000000, 45, 3, 1, 0),
+       PLL_36XX_RATE(73728000, 73, 3, 3, 47710),
+       PLL_36XX_RATE(67737600, 90, 4, 3, 20762),
+       PLL_36XX_RATE(49152000, 49, 3, 3, 9962),
+       PLL_36XX_RATE(45158400, 45, 3, 3, 10381),
+       PLL_36XX_RATE(32768000, 131, 3, 5, 4719),
+};
+
 /* register exynox5250 clocks */
 void __init exynos5250_clk_init(struct device_node *np)
 {
        void __iomem *reg_base;
        struct clk *apll, *mpll, *epll, *vpll, *bpll, *gpll, *cpll;
+       struct clk *vpllsrc;
+       unsigned long fin_pll_rate, mout_vpllsrc_rate = 0;
 
        if (np) {
                reg_base = of_iomap(np, 0);

@@ -496,6 +519,11 @@ void __init exynos5250_clk_init(struct device_node *np)
        samsung_clk_register_mux(exynos5250_pll_pmux_clks,
                        ARRAY_SIZE(exynos5250_pll_pmux_clks));
 
+       fin_pll_rate = _get_rate("fin_pll");
+       vpllsrc = __clk_lookup("mout_vpllsrc");
+       if (vpllsrc)
+               mout_vpllsrc_rate = clk_get_rate(vpllsrc);
+
        apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
                        reg_base, NULL, 0);
        mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",

@@ -506,10 +534,29 @@ void __init exynos5250_clk_init(struct device_node *np)
                        reg_base + 0x10050, NULL, 0);
        cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
                        reg_base + 0x10020, NULL, 0);
-       epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",