Re: [PATCH v2 3/3] ARM: EXYNOS4: Add EPLL clock operations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Every one,

On 18 July 2011 11:22, Kukjin Kim <kgene.kim@xxxxxxxxxxx> wrote:
> Naveen Krishna Chatradhi wrote:
>>
>> S5PV210 and EXYNOS4 uses similar PLL(PLL46XX) for EPLL.
>> So, The EPLL set rate function is duplicated.
>>
>> Note: Moved common code to plat-s5p, as commented by Kukjin Kim.
>>
> Since if you want to keep this in git log, this should be moved after below
> '---' :(
Thanks for pointing.
>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@xxxxxxxxxxx>
>> ---
>>  arch/arm/mach-exynos4/clock.c        |    1 +
>>  arch/arm/mach-s5pv210/clock.c        |   78
> +---------------------------------
>>  arch/arm/plat-s5p/clock.c            |   77
>> +++++++++++++++++++++++++++++++++
>>  arch/arm/plat-s5p/include/plat/pll.h |    3 +
>>  4 files changed, 82 insertions(+), 77 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos4/clock.c b/arch/arm/mach-exynos4/clock.c
>> index feeb27e..7687087 100644
>> --- a/arch/arm/mach-exynos4/clock.c
>> +++ b/arch/arm/mach-exynos4/clock.c
>> @@ -1294,6 +1294,7 @@ void __init_or_cpufreq exynos4_setup_clocks(void)
>>                               __raw_readl(S5P_VPLL_CON1), pll_4650);
>>
>>       clk_fout_apll.ops = &exynos4_fout_apll_ops;
>> +     clk_fout_epll.ops = &pll46xx_epll_ops;
>>       clk_fout_mpll.rate = mpll;
>>       clk_fout_epll.rate = epll;
>>       clk_fout_vpll.rate = vpll;
>> diff --git a/arch/arm/mach-s5pv210/clock.c b/arch/arm/mach-s5pv210/clock.c
>> index ae72f87..dd77c2c 100644
>> --- a/arch/arm/mach-s5pv210/clock.c
>> +++ b/arch/arm/mach-s5pv210/clock.c
>> @@ -979,82 +979,6 @@ static struct clksrc_clk *sysclks[] = {
>>       &clk_sclk_spdif,
>>  };
>>
>> -static u32 epll_div[][6] = {
>> -     {  48000000, 0, 48, 3, 3, 0 },
>> -     {  96000000, 0, 48, 3, 2, 0 },
>> -     { 144000000, 1, 72, 3, 2, 0 },
>> -     { 192000000, 0, 48, 3, 1, 0 },
>> -     { 288000000, 1, 72, 3, 1, 0 },
>> -     {  32750000, 1, 65, 3, 4, 35127 },
>> -     {  32768000, 1, 65, 3, 4, 35127 },
>> -     {  45158400, 0, 45, 3, 3, 10355 },
>> -     {  45000000, 0, 45, 3, 3, 10355 },
>> -     {  45158000, 0, 45, 3, 3, 10355 },
>> -     {  49125000, 0, 49, 3, 3, 9961 },
>> -     {  49152000, 0, 49, 3, 3, 9961 },
>> -     {  67737600, 1, 67, 3, 3, 48366 },
>> -     {  67738000, 1, 67, 3, 3, 48366 },
>> -     {  73800000, 1, 73, 3, 3, 47710 },
>> -     {  73728000, 1, 73, 3, 3, 47710 },
>> -     {  36000000, 1, 32, 3, 4, 0 },
>> -     {  60000000, 1, 60, 3, 3, 0 },
>> -     {  72000000, 1, 72, 3, 3, 0 },
>> -     {  80000000, 1, 80, 3, 3, 0 },
>> -     {  84000000, 0, 42, 3, 2, 0 },
>> -     {  50000000, 0, 50, 3, 3, 0 },
>> -};
>> -
>> -static int s5pv210_epll_set_rate(struct clk *clk, unsigned long rate)
>> -{
>> -     unsigned int epll_con, epll_con_k;
>> -     unsigned int i;
>> -
>> -     /* Return if nothing changed */
>> -     if (clk->rate == rate)
>> -             return 0;
>> -
>> -     epll_con = __raw_readl(S5P_EPLL_CON);
>> -     epll_con_k = __raw_readl(S5P_EPLL_CON1);
>> -
>> -     epll_con_k &= ~PLL46XX_KDIV_MASK;
>> -     epll_con &= ~(1 << 27 |
>> -                     PLL46XX_MDIV_MASK << PLL46XX_MDIV_SHIFT |
>> -                     PLL46XX_PDIV_MASK << PLL46XX_PDIV_SHIFT |
>> -                     PLL46XX_SDIV_MASK << PLL46XX_SDIV_SHIFT);
>> -
>> -     for (i = 0; i < ARRAY_SIZE(epll_div); i++) {
>> -             if (epll_div[i][0] == rate) {
>> -                     epll_con_k |= epll_div[i][5] << 0;
>> -                     epll_con |= (epll_div[i][1] << 27 |
>> -                                     epll_div[i][2] <<
>> PLL46XX_MDIV_SHIFT |
>> -                                     epll_div[i][3] <<
>> PLL46XX_PDIV_SHIFT |
>> -                                     epll_div[i][4] <<
>> PLL46XX_SDIV_SHIFT);
>> -                     break;
>> -             }
>> -     }
>> -
>> -     if (i == ARRAY_SIZE(epll_div)) {
>> -             printk(KERN_ERR "%s: Invalid Clock EPLL Frequency\n",
>> -                             __func__);
>> -             return -EINVAL;
>> -     }
>> -
>> -     __raw_writel(epll_con, S5P_EPLL_CON);
>> -     __raw_writel(epll_con_k, S5P_EPLL_CON1);
>> -
>> -     printk(KERN_WARNING "EPLL Rate changes from %lu to %lu\n",
>> -                     clk->rate, rate);
>> -
>> -     clk->rate = rate;
>> -
>> -     return 0;
>> -}
>> -
>> -static struct clk_ops s5pv210_epll_ops = {
>> -     .set_rate = s5pv210_epll_set_rate,
>> -     .get_rate = s5p_epll_get_rate,
>> -};
>> -
>>  void __init_or_cpufreq s5pv210_setup_clocks(void)
>>  {
>>       struct clk *xtal_clk;
>> @@ -1075,7 +999,7 @@ void __init_or_cpufreq s5pv210_setup_clocks(void)
>>
>>       /* Set functions for clk_fout_epll */
>>       clk_fout_epll.enable = s5p_epll_enable;
>> -     clk_fout_epll.ops = &s5pv210_epll_ops;
>> +     clk_fout_epll.ops = &pll46xx_epll_ops;
>>
>>       printk(KERN_DEBUG "%s: registering clocks\n", __func__);
>>
>> diff --git a/arch/arm/plat-s5p/clock.c b/arch/arm/plat-s5p/clock.c
>> index 02af235..2a4678d 100644
>> --- a/arch/arm/plat-s5p/clock.c
>> +++ b/arch/arm/plat-s5p/clock.c
>> @@ -24,6 +24,7 @@
>>  #include <mach/regs-clock.h>
>>
>>  #include <plat/clock.h>
>> +#include <plat/pll.h>
>>  #include <plat/clock-clksrc.h>
>>  #include <plat/s5p-clock.h>
>>
>> @@ -203,6 +204,82 @@ struct clk_ops s5p_sclk_spdif_ops = {
>>       .get_rate       = s5p_spdif_get_rate,
>>  };
>>
>> +static u32 epll_div[][6] = {
>> +     {  48000000, 0, 48, 3, 3, 0 },
>> +     {  96000000, 0, 48, 3, 2, 0 },
>> +     { 144000000, 1, 72, 3, 2, 0 },
>> +     { 192000000, 0, 48, 3, 1, 0 },
>> +     { 288000000, 1, 72, 3, 1, 0 },
>> +     {  32750000, 1, 65, 3, 4, 35127 },
>> +     {  32768000, 1, 65, 3, 4, 35127 },
>> +     {  45158400, 0, 45, 3, 3, 10355 },
>> +     {  45000000, 0, 45, 3, 3, 10355 },
>> +     {  45158000, 0, 45, 3, 3, 10355 },
>> +     {  49125000, 0, 49, 3, 3, 9961 },
>> +     {  49152000, 0, 49, 3, 3, 9961 },
>> +     {  67737600, 1, 67, 3, 3, 48366 },
>> +     {  67738000, 1, 67, 3, 3, 48366 },
>> +     {  73800000, 1, 73, 3, 3, 47710 },
>> +     {  73728000, 1, 73, 3, 3, 47710 },
>> +     {  36000000, 1, 32, 3, 4, 0 },
>> +     {  60000000, 1, 60, 3, 3, 0 },
>> +     {  72000000, 1, 72, 3, 3, 0 },
>> +     {  80000000, 1, 80, 3, 3, 0 },
>> +     {  84000000, 0, 42, 3, 2, 0 },
>> +     {  50000000, 0, 50, 3, 3, 0 },
>> +};
>
> Hmm, ok for now. But as Seungwhan Youn said, you have to know it can be
> changed according to input clock for EPLL when you add this into common
> plat-s5p.

As mentioned earlier, I've not found any generic formula for using
across boards.
So, I dropped the plan for moving it to plat-s5p for all boards.

This patch only applies only for S5PV210 and EXYNOS4,
The epll_div values are same  as per the latest User Manuals for
S5PV210 and EXYNOS4.

Sorry for the mess, attaching a patch for RFC have caused.
>
>> +
>> +int pll46xx_epll_set_rate(struct clk *clk, unsigned long rate)
>
> How about to move this in plat-s5p/include/plat/pll.h like other pll helper
> functions?
Isn't it wrong for the helper functions to use raw_readl and
raw_writel functions ??
>
>> +{
>> +     unsigned int epll_con, epll_con_k;
>> +     unsigned int i;
>> +
>> +     /* Return if nothing changed */
>> +     if (clk->rate == rate)
>> +             return 0;
>> +
>> +     epll_con = __raw_readl(S5P_EPLL_CON);
>> +     epll_con_k = __raw_readl(S5P_EPLL_CON1);
>> +
>> +     epll_con_k &= ~PLL46XX_KDIV_MASK;
>> +     epll_con &= ~(1 << 27 |
>> +                     PLL46XX_MDIV_MASK << PLL46XX_MDIV_SHIFT |
>> +                     PLL46XX_PDIV_MASK << PLL46XX_PDIV_SHIFT |
>> +                     PLL46XX_SDIV_MASK << PLL46XX_SDIV_SHIFT);
>> +
>> +     for (i = 0; i < ARRAY_SIZE(epll_div); i++) {
>> +             if (epll_div[i][0] == rate) {
>> +                     epll_con_k |= epll_div[i][5] << 0;
>> +                     epll_con |= (epll_div[i][1] << 27 |
>> +                                     epll_div[i][2] <<
>> PLL46XX_MDIV_SHIFT |
>> +                                     epll_div[i][3] <<
>> PLL46XX_PDIV_SHIFT |
>> +                                     epll_div[i][4] <<
>> PLL46XX_SDIV_SHIFT);
>> +                     break;
>> +             }
>> +     }
>> +
>> +     if (i == ARRAY_SIZE(epll_div)) {
>> +             printk(KERN_ERR "%s: Invalid Clock EPLL Frequency\n",
>> +                             __func__);
>> +             return -EINVAL;
>> +     }
>> +
>> +     __raw_writel(epll_con, S5P_EPLL_CON);
>> +     __raw_writel(epll_con_k, S5P_EPLL_CON1);
>
> Basically, need to add check of pll locking after changing PLL value.
Yes, i understand.
It was a simple code movement, Will apply locking in next version of patch set.
>
>> +
>> +     printk(KERN_WARNING "EPLL Rate changes from %lu to %lu\n",
>> +                     clk->rate, rate);
>> +
>> +     clk->rate = rate;
>> +
>> +     return 0;
>> +}
>> +
>> +struct clk_ops pll46xx_epll_ops = {
>> +     .set_rate = pll46xx_epll_set_rate,
>> +     .get_rate = s5p_epll_get_rate,
>
> I'm not sure we need .get_rate here.
> Could you please test without this?
I've followed the legacy code, get_rate is never used,. yes will
remove in the next version.
>
>> +};
>> +
>>  static struct clk *s5p_clks[] __initdata = {
>>       &clk_ext_xtal_mux,
>>       &clk_48m,
>> diff --git a/arch/arm/plat-s5p/include/plat/pll.h
> b/arch/arm/plat-s5p/include/plat/pll.h
>> index bf28fad..911a20e 100644
>> --- a/arch/arm/plat-s5p/include/plat/pll.h
>> +++ b/arch/arm/plat-s5p/include/plat/pll.h
>> @@ -94,6 +94,9 @@ static inline unsigned long s5p_get_pll46xx(unsigned
> long
>> baseclk,
>>       return result;
>>  }
>>
>> +extern int pll46xx_epll_set_rate(struct clk *clk, unsigned long rate);
>> +extern struct clk_ops pll46xx_epll_ops;
>> +
>>  #define PLL90XX_MDIV_MASK    (0xFF)
>>  #define PLL90XX_PDIV_MASK    (0x3F)
>>  #define PLL90XX_SDIV_MASK    (0x7)
>> --
>> 1.7.2.3
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
> --
> 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
>



-- 
Shine bright,
(: Nav :)
--
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


[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux