Re: [PATCH v2 0/8] Common Clock Framework support for Samsung S3C64xx

Re: [PATCH v2 0/8] Common Clock Framework support for Samsung S3C64xx

[Kukjin Kim]

On 08/20/13 16:09, Kukjin Kim wrote:
> Mike Turquette wrote:
>>
>
> [...]
>
>>> OK, if new branch is ready, I will replace with that or if re-merge is
>>> required, I will. Either way, I'm fine and can handle. Mike, let me know
>>> your choice :-)
>>
>> Since I have already published it let's just go with the delta patch.  I
>> can create another stable branch named clk-next-s3c64xx-delta that just
>> has this patch on top of clk-next-s3c64xx OR I can apply it on top of
>> the existing clk-next-s3c64xx and re-merge it.
>>
> Sounds good to me. If the branch for the delta is ready, let me know.
>
Mike, I'm waiting for your delta branch which includes following from 
Tomasz.

[PATCH] clk: samsung: pll: Use new registration method for PLL6552 and 
PLL6553

I couldn't send some branches to arm-soc for upcoming merge window yet 
because of build compilation breakage of common-clk-s3c64xx branch which 
has many dependencies...

- Kukjin

>> I'm trying to think on whether there are any weird git corner cases with
>> re-merging clk-next-s3c64xx. Let me know if re-merging is somehow unsafe
>> (makes history weird, or whatever).
>>
> I don't think it causes some problem.
>
>> Let me know what option is better for you. I'll publish as soon as I get
>> the delta patch. Apologies again for creating some extra work!
>>
> No problem.

Re: [PATCH] clk: samsung: pll: Use new registration method for PLL6552 and PLL6553

[Mike Turquette]

Quoting Tomasz Figa (2013-08-20 17:33:21)
> This patch modifies PLL6552 and PLL6553 clock drivers to use recently
> added common Samsung PLL registration method.
> 
> Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>

Sigh. This change won't apply on top of clk-next-s3c64xx since it
requires Yadwinder's change (amongst) others.

The best I could do was spin out another topic branch with the relevant
changes and apply this patch on top. It is a subset of clk-next but it
is almost entirely Samsung patches:

git://git.linaro.org/people/mturquette/linux.git clk-next-s3c64xx-delta

Kukjin & Tomasz, let me know if this OK for you to add as a dependency
to the Samsung tree. If it is I will merge it into clk-next, but I want
your OK that this branch is OK for you to take.

Regards,
Mike

> ---
>  drivers/clk/samsung/clk-pll.c | 105 +++++-------------------------------------
>  drivers/clk/samsung/clk-pll.h |   6 +--
>  2 files changed, 13 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
> index 0775554..7572d1d 100644
> --- a/drivers/clk/samsung/clk-pll.c
> +++ b/drivers/clk/samsung/clk-pll.c
> @@ -441,9 +441,6 @@ struct clk * __init samsung_clk_register_pll46xx(const char *name,
>   * PLL6552 Clock Type
>   */
>  
> -#define PLL6552_LOCK_REG       0x00
> -#define PLL6552_CON_REG                0x0c
> -
>  #define PLL6552_MDIV_MASK      0x3ff
>  #define PLL6552_PDIV_MASK      0x3f
>  #define PLL6552_SDIV_MASK      0x7
> @@ -451,21 +448,14 @@ struct clk * __init samsung_clk_register_pll46xx(const char *name,
>  #define PLL6552_PDIV_SHIFT     8
>  #define PLL6552_SDIV_SHIFT     0
>  
> -struct samsung_clk_pll6552 {
> -       struct clk_hw hw;
> -       void __iomem *reg_base;
> -};
> -
> -#define to_clk_pll6552(_hw) container_of(_hw, struct samsung_clk_pll6552, hw)
> -
>  static unsigned long samsung_pll6552_recalc_rate(struct clk_hw *hw,
>                                                 unsigned long parent_rate)
>  {
> -       struct samsung_clk_pll6552 *pll = to_clk_pll6552(hw);
> +       struct samsung_clk_pll *pll = to_clk_pll(hw);
>         u32 mdiv, pdiv, sdiv, pll_con;
>         u64 fvco = parent_rate;
>  
> -       pll_con = __raw_readl(pll->reg_base + PLL6552_CON_REG);
> +       pll_con = __raw_readl(pll->con_reg);
>         mdiv = (pll_con >> PLL6552_MDIV_SHIFT) & PLL6552_MDIV_MASK;
>         pdiv = (pll_con >> PLL6552_PDIV_SHIFT) & PLL6552_PDIV_MASK;
>         sdiv = (pll_con >> PLL6552_SDIV_SHIFT) & PLL6552_SDIV_MASK;
> @@ -480,48 +470,10 @@ static const struct clk_ops samsung_pll6552_clk_ops = {
>         .recalc_rate = samsung_pll6552_recalc_rate,
>  };
>  
> -struct clk * __init samsung_clk_register_pll6552(const char *name,
> -                                       const char *pname, void __iomem *base)
> -{
> -       struct samsung_clk_pll6552 *pll;
> -       struct clk *clk;
> -       struct clk_init_data init;
> -
> -       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> -       if (!pll) {
> -               pr_err("%s: could not allocate pll clk %s\n", __func__, name);
> -               return NULL;
> -       }
> -
> -       init.name = name;
> -       init.ops = &samsung_pll6552_clk_ops;
> -       init.parent_names = &pname;
> -       init.num_parents = 1;
> -
> -       pll->hw.init = &init;
> -       pll->reg_base = base;
> -
> -       clk = clk_register(NULL, &pll->hw);
> -       if (IS_ERR(clk)) {
> -               pr_err("%s: failed to register pll clock %s\n", __func__,
> -                               name);
> -               kfree(pll);
> -       }
> -
> -       if (clk_register_clkdev(clk, name, NULL))
> -               pr_err("%s: failed to register lookup for %s", __func__, name);
> -
> -       return clk;
> -}
> -
>  /*
>   * PLL6553 Clock Type
>   */
>  
> -#define PLL6553_LOCK_REG       0x00
> -#define PLL6553_CON0_REG       0x0c
> -#define PLL6553_CON1_REG       0x10
> -
>  #define PLL6553_MDIV_MASK      0xff
>  #define PLL6553_PDIV_MASK      0x3f
>  #define PLL6553_SDIV_MASK      0x7
> @@ -531,22 +483,15 @@ struct clk * __init samsung_clk_register_pll6552(const char *name,
>  #define PLL6553_SDIV_SHIFT     0
>  #define PLL6553_KDIV_SHIFT     0
>  
> -struct samsung_clk_pll6553 {
> -       struct clk_hw hw;
> -       void __iomem *reg_base;
> -};
> -
> -#define to_clk_pll6553(_hw) container_of(_hw, struct samsung_clk_pll6553, hw)
> -
>  static unsigned long samsung_pll6553_recalc_rate(struct clk_hw *hw,
>                                                 unsigned long parent_rate)
>  {
> -       struct samsung_clk_pll6553 *pll = to_clk_pll6553(hw);
> +       struct samsung_clk_pll *pll = to_clk_pll(hw);
>         u32 mdiv, pdiv, sdiv, kdiv, pll_con0, pll_con1;
>         u64 fvco = parent_rate;
>  
> -       pll_con0 = __raw_readl(pll->reg_base + PLL6553_CON0_REG);
> -       pll_con1 = __raw_readl(pll->reg_base + PLL6553_CON1_REG);
> +       pll_con0 = __raw_readl(pll->con_reg);
> +       pll_con1 = __raw_readl(pll->con_reg + 0x4);
>         mdiv = (pll_con0 >> PLL6553_MDIV_SHIFT) & PLL6553_MDIV_MASK;
>         pdiv = (pll_con0 >> PLL6553_PDIV_SHIFT) & PLL6553_PDIV_MASK;
>         sdiv = (pll_con0 >> PLL6553_SDIV_SHIFT) & PLL6553_SDIV_MASK;
> @@ -563,40 +508,6 @@ static const struct clk_ops samsung_pll6553_clk_ops = {
>         .recalc_rate = samsung_pll6553_recalc_rate,
>  };
>  
> -struct clk * __init samsung_clk_register_pll6553(const char *name,
> -                                       const char *pname, void __iomem *base)
> -{
> -       struct samsung_clk_pll6553 *pll;
> -       struct clk *clk;
> -       struct clk_init_data init;
> -
> -       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> -       if (!pll) {
> -               pr_err("%s: could not allocate pll clk %s\n", __func__, name);
> -               return NULL;
> -       }
> -
> -       init.name = name;
> -       init.ops = &samsung_pll6553_clk_ops;
> -       init.parent_names = &pname;
> -       init.num_parents = 1;
> -
> -       pll->hw.init = &init;
> -       pll->reg_base = base;
> -
> -       clk = clk_register(NULL, &pll->hw);
> -       if (IS_ERR(clk)) {
> -               pr_err("%s: failed to register pll clock %s\n", __func__,
> -                               name);
> -               kfree(pll);
> -       }
> -
> -       if (clk_register_clkdev(clk, name, NULL))
> -               pr_err("%s: failed to register lookup for %s", __func__, name);
> -
> -       return clk;
> -}
> -
>  /*
>   * PLL2550x Clock Type
>   */
> @@ -732,6 +643,12 @@ static void __init _samsung_clk_register_pll(struct samsung_pll_clock *pll_clk,
>                 else
>                         init.ops = &samsung_pll36xx_clk_ops;
>                 break;
> +       case pll_6552:
> +               init.ops = &samsung_pll6552_clk_ops;
> +               break;
> +       case pll_6553:
> +               init.ops = &samsung_pll6553_clk_ops;
> +               break;
>         default:
>                 pr_warn("%s: Unknown pll type for pll clk %s\n",
>                         __func__, pll_clk->name);
> diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h
> index 2f70e88..cd11037 100644
> --- a/drivers/clk/samsung/clk-pll.h
> +++ b/drivers/clk/samsung/clk-pll.h
> @@ -17,6 +17,8 @@ enum samsung_pll_type {
>         pll_36xx,
>         pll_2550,
>         pll_2650,
> +       pll_6552,
> +       pll_6553,
>  };
>  
>  #define PLL_35XX_RATE(_rate, _m, _p, _s)                       \
> @@ -64,10 +66,6 @@ extern struct clk * __init samsung_clk_register_pll45xx(const char *name,
>  extern struct clk * __init samsung_clk_register_pll46xx(const char *name,
>                         const char *pname, const void __iomem *con_reg,
>                         enum pll46xx_type type);
> -extern struct clk *samsung_clk_register_pll6552(const char *name,
> -                       const char *pname, void __iomem *base);
> -extern struct clk *samsung_clk_register_pll6553(const char *name,
> -                       const char *pname, void __iomem *base);
>  extern struct clk * __init samsung_clk_register_pll2550x(const char *name,
>                         const char *pname, const void __iomem *reg_base,
>                         const unsigned long offset);
> -- 
> 1.8.3.2

Re: [PATCH] clk: samsung: pll: Use new registration method for PLL6552 and PLL6553

[Kukjin Kim]

On 08/27/13 10:14, Mike Turquette wrote:
> Quoting Tomasz Figa (2013-08-20 17:33:21)
>> This patch modifies PLL6552 and PLL6553 clock drivers to use recently
>> added common Samsung PLL registration method.
>>
>> Signed-off-by: Tomasz Figa<tomasz.figa@gmail.com>
>
> Sigh. This change won't apply on top of clk-next-s3c64xx since it
> requires Yadwinder's change (amongst) others.
>
> The best I could do was spin out another topic branch with the relevant
> changes and apply this patch on top. It is a subset of clk-next but it
> is almost entirely Samsung patches:
>
> git://git.linaro.org/people/mturquette/linux.git clk-next-s3c64xx-delta
>
> Kukjin&  Tomasz, let me know if this OK for you to add as a dependency
> to the Samsung tree. If it is I will merge it into clk-next, but I want
> your OK that this branch is OK for you to take.
>
Hi Mike,

I merged the branch clk-next-s3c64xx-delta you provided on top of 
previous clk-next-s3c64xx for common-clk-s3c64xx and dt-s3c64xx and it 
looks fine to me.

Thanks,
Kukjin

Re: [PATCH] clk: samsung: pll: Use new registration method for PLL6552 and PLL6553

[Mike Turquette]

Quoting Kukjin Kim (2013-08-27 10:16:14)
> On 08/27/13 10:14, Mike Turquette wrote:
> > Quoting Tomasz Figa (2013-08-20 17:33:21)
> >> This patch modifies PLL6552 and PLL6553 clock drivers to use recently
> >> added common Samsung PLL registration method.
> >>
> >> Signed-off-by: Tomasz Figa<tomasz.figa@gmail.com>
> >
> > Sigh. This change won't apply on top of clk-next-s3c64xx since it
> > requires Yadwinder's change (amongst) others.
> >
> > The best I could do was spin out another topic branch with the relevant
> > changes and apply this patch on top. It is a subset of clk-next but it
> > is almost entirely Samsung patches:
> >
> > git://git.linaro.org/people/mturquette/linux.git clk-next-s3c64xx-delta
> >
> > Kukjin&  Tomasz, let me know if this OK for you to add as a dependency
> > to the Samsung tree. If it is I will merge it into clk-next, but I want
> > your OK that this branch is OK for you to take.
> >
> Hi Mike,
> 
> I merged the branch clk-next-s3c64xx-delta you provided on top of 
> previous clk-next-s3c64xx for common-clk-s3c64xx and dt-s3c64xx and it 
> looks fine to me.

Cool. I've merged it into clk-next as well.

Regards,
Mike

> 
> Thanks,
> Kukjin