Hi Pankaj, Rahul, Arun,
On Friday 06 of December 2013 21:26:29 Rahul Sharma wrote:
> From: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
>
> exynos5260 use pll2520xx and it has different bitfields
> for P,M,S values as compared to pll2550xx. Support for
> pll2520xx is added here.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
> Signed-off-by: Rahul Sharma <rahul.sharma@xxxxxxxxxxx>
> Signed-off-by: Arun Kumar K <arun.kk@xxxxxxxxxxx>
> ---
> drivers/clk/samsung/clk-pll.c | 107 +++++++++++++++++++++++++++++++++++++++++
> drivers/clk/samsung/clk-pll.h | 1 +
> 2 files changed, 108 insertions(+)
>
> diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
> index e8e8953..237a889 100644
> --- a/drivers/clk/samsung/clk-pll.c
> +++ b/drivers/clk/samsung/clk-pll.c
> @@ -710,6 +710,107 @@ struct clk * __init samsung_clk_register_pll2550x(const char *name,
> return clk;
> }
>
> +/*
> + * PLL2550xx Clock Type
> + */
> +
> +/* Maximum lock time can be 270 * PDIV cycles */
> +#define PLL2550XX_LOCK_FACTOR (270)
> +
> +#define PLL2550XX_MDIV_MASK (0x3FF)
> +#define PLL2550XX_PDIV_MASK (0x3F)
> +#define PLL2550XX_SDIV_MASK (0x7)
> +#define PLL2550XX_LOCK_STAT_MASK (0x1)
> +#define PLL2550XX_MDIV_SHIFT (9)
> +#define PLL2550XX_PDIV_SHIFT (3)
> +#define PLL2550XX_SDIV_SHIFT (0)
> +#define PLL2550XX_LOCK_STAT_SHIFT (21)
> +
> +static unsigned long samsung_pll2550xx_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct samsung_clk_pll *pll = to_clk_pll(hw);
> + u32 mdiv, pdiv, sdiv, pll_con;
> + u64 fvco = parent_rate;
> +
> + pll_con = __raw_readl(pll->con_reg);
> + mdiv = (pll_con >> PLL2550XX_MDIV_SHIFT) & PLL2550XX_MDIV_MASK;
> + pdiv = (pll_con >> PLL2550XX_PDIV_SHIFT) & PLL2550XX_PDIV_MASK;
> + sdiv = (pll_con >> PLL2550XX_SDIV_SHIFT) & PLL2550XX_SDIV_MASK;
> +
> + fvco *= mdiv;
> + do_div(fvco, (pdiv << sdiv));
> +
> + return (unsigned long)fvco;
> +}
> +
> +static inline bool samsung_pll2550xx_mp_change(u32 mdiv, u32 pdiv, u32 pll_con)
> +{
> + if ((mdiv != ((pll_con >> PLL2550XX_MDIV_SHIFT) &
> + PLL2550XX_MDIV_MASK)) ||
> + (pdiv != ((pll_con >> PLL2550XX_PDIV_SHIFT) &
> + PLL2550XX_PDIV_MASK)))
> + return 1;
> + else
> + return 0;
This doesn't look too good. Can you make this consistent with
implementations of this helper for other PLLs, such as
samsung_pll35xx_mp_change()?
> +}
> +
> +static int samsung_pll2550xx_set_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;
> + u32 tmp;
> +
> + /* Get required rate settings from table */
> + rate = samsung_get_pll_settings(pll, drate);
> + if (!rate) {
> + pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__,
> + drate, __clk_get_name(hw->clk));
> + return -EINVAL;
> + }
> +
> + tmp = __raw_readl(pll->con_reg);
> +
> + if (!(samsung_pll2550xx_mp_change(rate->mdiv, rate->pdiv, tmp))) {
> + /* If only s change, change just s value only*/
> + tmp &= ~(PLL2550XX_SDIV_MASK << PLL2550XX_SDIV_SHIFT);
> + tmp |= rate->sdiv << PLL2550XX_SDIV_SHIFT;
> + __raw_writel(tmp, pll->con_reg);
> + } else {
Please make coding style of this function consistent with implementations
of this operation for other PLLs, such as samsung_pll35xx_set_rate().
Otherwise the patch looks fine.
Best regards,
Tomasz
--
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
