diff mbox

[v2,3/6] clk: samsung: add plls used in s3c2416 and s3c2443

Message ID 201307100059.08621.heiko@sntech.de (mailing list archive)
State New, archived
Headers show

Commit Message

Heiko Stübner July 9, 2013, 10:59 p.m. UTC 
This adds support for pll2126x, pll3000x, pll6552x and pll6553x.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/samsung/clk-pll.c |  280 +++++++++++++++++++++++++++++++++++++++++
 drivers/clk/samsung/clk-pll.h |    8 ++
 2 files changed, 288 insertions(+)

Comments

Tomasz Figa July 11, 2013, 8:16 a.m. UTC | #1 
Hi Heiko,

On Wednesday 10 of July 2013 00:59:08 Heiko Stübner wrote:
> This adds support for pll2126x, pll3000x, pll6552x and pll6553x.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/clk/samsung/clk-pll.c |  280
> +++++++++++++++++++++++++++++++++++++++++ drivers/clk/samsung/clk-pll.h |  
>  8 ++
>  2 files changed, 288 insertions(+)

Generally the patch looks good, but I have some comments to the part related 
to 655xx PLLs.

I had a patch adding support for them too, but we can go with yours, since the 
way of registration has been changed by Yadwinder's patches and mine would 
have to be updated anyway.

> diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
> index 0afaec6..35c15a1 100644
> --- a/drivers/clk/samsung/clk-pll.c
> +++ b/drivers/clk/samsung/clk-pll.c
> @@ -323,6 +323,73 @@ struct clk * __init samsung_clk_register_pll46xx(const
> char *name, }
> 
>  /*
> + * PLL2126x Clock Type
> + */
> +
> +#define PLL2126X_MDIV_MASK	(0xFF)
> +#define PLL2126X_PDIV_MASK	(0x3)
> +#define PLL2126X_SDIV_MASK	(0x3)
> +#define PLL2126X_MDIV_SHIFT	(16)
> +#define PLL2126X_PDIV_SHIFT	(8)
> +#define PLL2126X_SDIV_SHIFT	(0)
> +
> +struct samsung_clk_pll2126x {
> +	struct clk_hw		hw;
> +	const void __iomem	*con_reg;
> +};
> +
> +#define to_clk_pll2126x(_hw) container_of(_hw, struct samsung_clk_pll2126x,
> hw) +
> +static unsigned long samsung_pll2126x_recalc_rate(struct clk_hw *hw,
> +				unsigned long parent_rate)
> +{
> +	struct samsung_clk_pll2126x *pll = to_clk_pll2126x(hw);
> +	u32 pll_con, mdiv, pdiv, sdiv;
> +	u64 fvco = parent_rate;
> +
> +	pll_con = __raw_readl(pll->con_reg);
> +	mdiv = (pll_con >> PLL2126X_MDIV_SHIFT) & PLL2126X_MDIV_MASK;
> +	pdiv = (pll_con >> PLL2126X_PDIV_SHIFT) & PLL2126X_PDIV_MASK;
> +	sdiv = (pll_con >> PLL2126X_SDIV_SHIFT) & PLL2126X_SDIV_MASK;
> +
> +	fvco *= (mdiv + 8);
> +	do_div(fvco, (pdiv + 2) << sdiv);
> +
> +	return (unsigned long)fvco;
> +}
> +
> +static const struct clk_ops samsung_pll2126x_clk_ops = {
> +	.recalc_rate = samsung_pll2126x_recalc_rate,
> +};
> +
> +struct clk * __init samsung_clk_register_pll2126x(const char *name,
> +			const char *pname, const void __iomem *con_reg)
> +{
> +	struct samsung_clk_pll2126x *pll;
> +	struct clk *clk;
> +	struct clk_init_data init;
> +
> +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +	if (!pll)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init.name = name;
> +	init.ops = &samsung_pll2126x_clk_ops;
> +	init.flags = CLK_GET_RATE_NOCACHE;
> +	init.parent_names = &pname;
> +	init.num_parents = 1;
> +
> +	pll->hw.init = &init;
> +	pll->con_reg = con_reg;
> +
> +	clk = samsung_register_pll(&pll->hw);
> +	if (IS_ERR(clk))
> +		kfree(pll);
> +
> +	return clk;
> +}
> +
> +/*
>   * PLL2550x Clock Type
>   */
> 
> @@ -396,3 +463,216 @@ struct clk * __init
> samsung_clk_register_pll2550x(const char *name,
> 
>  	return clk;
>  }
> +
> +/*
> + * PLL3000x Clock Type
> + */
> +
> +#define PLL3000X_MDIV_MASK	(0xFF)
> +#define PLL3000X_PDIV_MASK	(0x3)
> +#define PLL3000X_SDIV_MASK	(0x3)
> +#define PLL3000X_MDIV_SHIFT	(16)
> +#define PLL3000X_PDIV_SHIFT	(8)
> +#define PLL3000X_SDIV_SHIFT	(0)
> +
> +struct samsung_clk_pll3000x {
> +	struct clk_hw		hw;
> +	const void __iomem	*con_reg;
> +};
> +
> +#define to_clk_pll3000x(_hw) container_of(_hw, struct samsung_clk_pll3000x,
> hw) +
> +static unsigned long samsung_pll3000x_recalc_rate(struct clk_hw *hw,
> +				unsigned long parent_rate)
> +{
> +	struct samsung_clk_pll3000x *pll = to_clk_pll3000x(hw);
> +	u32 pll_con, mdiv, pdiv, sdiv;
> +	u64 fvco = parent_rate;
> +
> +	pll_con = __raw_readl(pll->con_reg);
> +	mdiv = (pll_con >> PLL3000X_MDIV_SHIFT) & PLL3000X_MDIV_MASK;
> +	pdiv = (pll_con >> PLL3000X_PDIV_SHIFT) & PLL3000X_PDIV_MASK;
> +	sdiv = (pll_con >> PLL3000X_SDIV_SHIFT) & PLL3000X_SDIV_MASK;
> +
> +	fvco *= (2 * (mdiv + 8));
> +	do_div(fvco, pdiv << sdiv);
> +
> +	return (unsigned long)fvco;
> +}
> +
> +static const struct clk_ops samsung_pll3000x_clk_ops = {
> +	.recalc_rate = samsung_pll3000x_recalc_rate,
> +};
> +
> +struct clk * __init samsung_clk_register_pll3000x(const char *name,
> +			const char *pname, const void __iomem *con_reg)
> +{
> +	struct samsung_clk_pll3000x *pll;
> +	struct clk *clk;
> +	struct clk_init_data init;
> +
> +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +	if (!pll)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init.name = name;
> +	init.ops = &samsung_pll3000x_clk_ops;
> +	init.flags = CLK_GET_RATE_NOCACHE;
> +	init.parent_names = &pname;
> +	init.num_parents = 1;
> +
> +	pll->hw.init = &init;
> +	pll->con_reg = con_reg;
> +
> +	clk = samsung_register_pll(&pll->hw);
> +	if (IS_ERR(clk))
> +		kfree(pll);
> +
> +	return clk;
> +}
> +
> +/*
> + * PLL6552x Clock Type
> + */
> +
> +#define PLL6552X_MDIV_MASK	(0x3FF)
> +#define PLL6552X_PDIV_MASK	(0x3F)
> +#define PLL6552X_SDIV_MASK	(0x7)
> +#define PLL6552X_MDIV_SHIFT	(14)
> +#define PLL6552X_PDIV_SHIFT	(5)
> +#define PLL6552X_SDIV_SHIFT	(0)

Are you sure about those bitfields?

In S3C6410 User's Manual they are different. You can look at my patch for a 
comparison:

http://thread.gmane.org/gmane.linux.usb.general/87571/focus=88344

> +struct samsung_clk_pll6552x {
> +	struct clk_hw		hw;
> +	const void __iomem	*con_reg;
> +};
> +
> +#define to_clk_pll6552x(_hw) container_of(_hw, struct samsung_clk_pll6552x,
> hw) +
> +static unsigned long samsung_pll6552x_recalc_rate(struct clk_hw *hw,
> +				unsigned long parent_rate)
> +{
> +	struct samsung_clk_pll6552x *pll = to_clk_pll6552x(hw);
> +	u32 pll_con, mdiv, pdiv, sdiv;
> +	u64 fvco = parent_rate;
> +
> +	pll_con = __raw_readl(pll->con_reg);
> +	mdiv = (pll_con >> PLL6552X_MDIV_SHIFT) & PLL6552X_MDIV_MASK;
> +	pdiv = (pll_con >> PLL6552X_PDIV_SHIFT) & PLL6552X_PDIV_MASK;
> +	sdiv = (pll_con >> PLL6552X_SDIV_SHIFT) & PLL6552X_SDIV_MASK;
> +
> +	fvco *= mdiv;
> +	do_div(fvco, (pdiv << sdiv));
> +
> +	return (unsigned long)fvco;
> +}
> +
> +static const struct clk_ops samsung_pll6552x_clk_ops = {
> +	.recalc_rate = samsung_pll6552x_recalc_rate,
> +};
> +
> +struct clk * __init samsung_clk_register_pll6552x(const char *name,
> +			const char *pname, const void __iomem *con_reg)
> +{
> +	struct samsung_clk_pll6552x *pll;
> +	struct clk *clk;
> +	struct clk_init_data init;
> +
> +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +	if (!pll)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init.name = name;
> +	init.ops = &samsung_pll6552x_clk_ops;
> +	init.flags = CLK_GET_RATE_NOCACHE;
> +	init.parent_names = &pname;
> +	init.num_parents = 1;
> +
> +	pll->hw.init = &init;
> +	pll->con_reg = con_reg;
> +
> +	clk = samsung_register_pll(&pll->hw);
> +	if (IS_ERR(clk))
> +		kfree(pll);
> +
> +	return clk;
> +}
> +
> +/*
> + * PLL6553x Clock Type
> + */
> +
> +#define PLL6553X_MDIV_MASK	(0x7F)
> +#define PLL6553X_PDIV_MASK	(0x1F)
> +#define PLL6553X_SDIV_MASK	(0x3)
> +#define PLL6553X_KDIV_MASK	(0xFFFF)
> +#define PLL6553X_MDIV_SHIFT	(16)
> +#define PLL6553X_PDIV_SHIFT	(8)
> +#define PLL6553X_SDIV_SHIFT	(0)

Same about those bitfields. They seem to be different on S3C64xx.

Best regards,
Tomasz
Heiko Stübner July 11, 2013, 8:50 a.m. UTC | #2 
Am Donnerstag, 11. Juli 2013, 10:16:41 schrieb Tomasz Figa:
> Hi Heiko,
> 
> On Wednesday 10 of July 2013 00:59:08 Heiko Stübner wrote:
> > This adds support for pll2126x, pll3000x, pll6552x and pll6553x.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > 
> >  drivers/clk/samsung/clk-pll.c |  280
> > 
> > +++++++++++++++++++++++++++++++++++++++++ drivers/clk/samsung/clk-pll.h |
> > 
> >  8 ++
> >  2 files changed, 288 insertions(+)
> 
> Generally the patch looks good, but I have some comments to the part
> related to 655xx PLLs.
> 
> I had a patch adding support for them too, but we can go with yours, since
> the way of registration has been changed by Yadwinder's patches and mine
> would have to be updated anyway.
> 
> > diff --git a/drivers/clk/samsung/clk-pll.c
> > b/drivers/clk/samsung/clk-pll.c index 0afaec6..35c15a1 100644
> > --- a/drivers/clk/samsung/clk-pll.c
> > +++ b/drivers/clk/samsung/clk-pll.c
> > @@ -323,6 +323,73 @@ struct clk * __init
> > samsung_clk_register_pll46xx(const char *name, }
> > 
> >  /*
> > 
> > + * PLL2126x Clock Type
> > + */
> > +
> > +#define PLL2126X_MDIV_MASK	(0xFF)
> > +#define PLL2126X_PDIV_MASK	(0x3)
> > +#define PLL2126X_SDIV_MASK	(0x3)
> > +#define PLL2126X_MDIV_SHIFT	(16)
> > +#define PLL2126X_PDIV_SHIFT	(8)
> > +#define PLL2126X_SDIV_SHIFT	(0)


+#define PLL2126X_PDIV_MASK	(0x3F)

is the correct value.


> > +
> > +struct samsung_clk_pll2126x {
> > +	struct clk_hw		hw;
> > +	const void __iomem	*con_reg;
> > +};
> > +
> > +#define to_clk_pll2126x(_hw) container_of(_hw, struct
> > samsung_clk_pll2126x, hw) +
> > +static unsigned long samsung_pll2126x_recalc_rate(struct clk_hw *hw,
> > +				unsigned long parent_rate)
> > +{
> > +	struct samsung_clk_pll2126x *pll = to_clk_pll2126x(hw);
> > +	u32 pll_con, mdiv, pdiv, sdiv;
> > +	u64 fvco = parent_rate;
> > +
> > +	pll_con = __raw_readl(pll->con_reg);
> > +	mdiv = (pll_con >> PLL2126X_MDIV_SHIFT) & PLL2126X_MDIV_MASK;
> > +	pdiv = (pll_con >> PLL2126X_PDIV_SHIFT) & PLL2126X_PDIV_MASK;
> > +	sdiv = (pll_con >> PLL2126X_SDIV_SHIFT) & PLL2126X_SDIV_MASK;
> > +
> > +	fvco *= (mdiv + 8);
> > +	do_div(fvco, (pdiv + 2) << sdiv);
> > +
> > +	return (unsigned long)fvco;
> > +}
> > +
> > +static const struct clk_ops samsung_pll2126x_clk_ops = {
> > +	.recalc_rate = samsung_pll2126x_recalc_rate,
> > +};
> > +
> > +struct clk * __init samsung_clk_register_pll2126x(const char *name,
> > +			const char *pname, const void __iomem *con_reg)
> > +{
> > +	struct samsung_clk_pll2126x *pll;
> > +	struct clk *clk;
> > +	struct clk_init_data init;
> > +
> > +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> > +	if (!pll)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	init.name = name;
> > +	init.ops = &samsung_pll2126x_clk_ops;
> > +	init.flags = CLK_GET_RATE_NOCACHE;
> > +	init.parent_names = &pname;
> > +	init.num_parents = 1;
> > +
> > +	pll->hw.init = &init;
> > +	pll->con_reg = con_reg;
> > +
> > +	clk = samsung_register_pll(&pll->hw);
> > +	if (IS_ERR(clk))
> > +		kfree(pll);
> > +
> > +	return clk;
> > +}
> > +
> > +/*
> > 
> >   * PLL2550x Clock Type
> >   */
> > 
> > @@ -396,3 +463,216 @@ struct clk * __init
> > samsung_clk_register_pll2550x(const char *name,
> > 
> >  	return clk;
> >  
> >  }
> > 
> > +
> > +/*
> > + * PLL3000x Clock Type
> > + */
> > +
> > +#define PLL3000X_MDIV_MASK	(0xFF)
> > +#define PLL3000X_PDIV_MASK	(0x3)
> > +#define PLL3000X_SDIV_MASK	(0x3)
> > +#define PLL3000X_MDIV_SHIFT	(16)
> > +#define PLL3000X_PDIV_SHIFT	(8)
> > +#define PLL3000X_SDIV_SHIFT	(0)

these are correct.

> > +
> > +struct samsung_clk_pll3000x {
> > +	struct clk_hw		hw;
> > +	const void __iomem	*con_reg;
> > +};
> > +
> > +#define to_clk_pll3000x(_hw) container_of(_hw, struct
> > samsung_clk_pll3000x, hw) +
> > +static unsigned long samsung_pll3000x_recalc_rate(struct clk_hw *hw,
> > +				unsigned long parent_rate)
> > +{
> > +	struct samsung_clk_pll3000x *pll = to_clk_pll3000x(hw);
> > +	u32 pll_con, mdiv, pdiv, sdiv;
> > +	u64 fvco = parent_rate;
> > +
> > +	pll_con = __raw_readl(pll->con_reg);
> > +	mdiv = (pll_con >> PLL3000X_MDIV_SHIFT) & PLL3000X_MDIV_MASK;
> > +	pdiv = (pll_con >> PLL3000X_PDIV_SHIFT) & PLL3000X_PDIV_MASK;
> > +	sdiv = (pll_con >> PLL3000X_SDIV_SHIFT) & PLL3000X_SDIV_MASK;
> > +
> > +	fvco *= (2 * (mdiv + 8));
> > +	do_div(fvco, pdiv << sdiv);
> > +
> > +	return (unsigned long)fvco;
> > +}
> > +
> > +static const struct clk_ops samsung_pll3000x_clk_ops = {
> > +	.recalc_rate = samsung_pll3000x_recalc_rate,
> > +};
> > +
> > +struct clk * __init samsung_clk_register_pll3000x(const char *name,
> > +			const char *pname, const void __iomem *con_reg)
> > +{
> > +	struct samsung_clk_pll3000x *pll;
> > +	struct clk *clk;
> > +	struct clk_init_data init;
> > +
> > +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> > +	if (!pll)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	init.name = name;
> > +	init.ops = &samsung_pll3000x_clk_ops;
> > +	init.flags = CLK_GET_RATE_NOCACHE;
> > +	init.parent_names = &pname;
> > +	init.num_parents = 1;
> > +
> > +	pll->hw.init = &init;
> > +	pll->con_reg = con_reg;
> > +
> > +	clk = samsung_register_pll(&pll->hw);
> > +	if (IS_ERR(clk))
> > +		kfree(pll);
> > +
> > +	return clk;
> > +}
> > +
> > +/*
> > + * PLL6552x Clock Type
> > + */
> > +
> > +#define PLL6552X_MDIV_MASK	(0x3FF)
> > +#define PLL6552X_PDIV_MASK	(0x3F)
> > +#define PLL6552X_SDIV_MASK	(0x7)
> > +#define PLL6552X_MDIV_SHIFT	(14)
> > +#define PLL6552X_PDIV_SHIFT	(5)
> > +#define PLL6552X_SDIV_SHIFT	(0)
> 
> Are you sure about those bitfields?
> 
> In S3C6410 User's Manual they are different. You can look at my patch for a
> comparison:
> 
> http://thread.gmane.org/gmane.linux.usb.general/87571/focus=88344

The numbers where taken from the previous pll code, but I now again checked 
them against the datasheet of the s3c2416 and the s3c2450.

When comparing with your patch, it really seems that the bit offsets in the 
register are different for the pdiv and mdiv - the above values are correct 
according the the datasheet (and also produce the expected results in the 
clock tree).



> > +struct samsung_clk_pll6552x {
> > +	struct clk_hw		hw;
> > +	const void __iomem	*con_reg;
> > +};
> > +
> > +#define to_clk_pll6552x(_hw) container_of(_hw, struct
> > samsung_clk_pll6552x, hw) +
> > +static unsigned long samsung_pll6552x_recalc_rate(struct clk_hw *hw,
> > +				unsigned long parent_rate)
> > +{
> > +	struct samsung_clk_pll6552x *pll = to_clk_pll6552x(hw);
> > +	u32 pll_con, mdiv, pdiv, sdiv;
> > +	u64 fvco = parent_rate;
> > +
> > +	pll_con = __raw_readl(pll->con_reg);
> > +	mdiv = (pll_con >> PLL6552X_MDIV_SHIFT) & PLL6552X_MDIV_MASK;
> > +	pdiv = (pll_con >> PLL6552X_PDIV_SHIFT) & PLL6552X_PDIV_MASK;
> > +	sdiv = (pll_con >> PLL6552X_SDIV_SHIFT) & PLL6552X_SDIV_MASK;
> > +
> > +	fvco *= mdiv;
> > +	do_div(fvco, (pdiv << sdiv));
> > +
> > +	return (unsigned long)fvco;
> > +}
> > +
> > +static const struct clk_ops samsung_pll6552x_clk_ops = {
> > +	.recalc_rate = samsung_pll6552x_recalc_rate,
> > +};
> > +
> > +struct clk * __init samsung_clk_register_pll6552x(const char *name,
> > +			const char *pname, const void __iomem *con_reg)
> > +{
> > +	struct samsung_clk_pll6552x *pll;
> > +	struct clk *clk;
> > +	struct clk_init_data init;
> > +
> > +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> > +	if (!pll)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	init.name = name;
> > +	init.ops = &samsung_pll6552x_clk_ops;
> > +	init.flags = CLK_GET_RATE_NOCACHE;
> > +	init.parent_names = &pname;
> > +	init.num_parents = 1;
> > +
> > +	pll->hw.init = &init;
> > +	pll->con_reg = con_reg;
> > +
> > +	clk = samsung_register_pll(&pll->hw);
> > +	if (IS_ERR(clk))
> > +		kfree(pll);
> > +
> > +	return clk;
> > +}
> > +
> > +/*
> > + * PLL6553x Clock Type
> > + */
> > +
> > +#define PLL6553X_MDIV_MASK	(0x7F)
> > +#define PLL6553X_PDIV_MASK	(0x1F)
> > +#define PLL6553X_SDIV_MASK	(0x3)
> > +#define PLL6553X_KDIV_MASK	(0xFFFF)
> > +#define PLL6553X_MDIV_SHIFT	(16)
> > +#define PLL6553X_PDIV_SHIFT	(8)
> > +#define PLL6553X_SDIV_SHIFT	(0)
> 
> Same about those bitfields. They seem to be different on S3C64xx.

Here it seems the values were off in the original code. According to the 
datasheet the values in your patch are correct. Thanks for the catch.

+#define PLL6553X_MDIV_MASK	(0xFF)
+#define PLL6553X_PDIV_MASK	(0x3F)
+#define PLL6553X_SDIV_MASK	(0x7)


This leaves the problem on what to do with the 6552X and its different bit 
offsets. Is the pll in question really a 6552X? In the s3c2416 manual its name 
is explicitly stated.


Thanks
Heiko
Tomasz Figa July 11, 2013, 9:25 a.m. UTC | #3 
On Thursday 11 of July 2013 10:50:39 Heiko Stübner wrote:
> Am Donnerstag, 11. Juli 2013, 10:16:41 schrieb Tomasz Figa:
> > Hi Heiko,
> > 
> > On Wednesday 10 of July 2013 00:59:08 Heiko Stübner wrote:
> > > This adds support for pll2126x, pll3000x, pll6552x and pll6553x.
> > > 
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > > 
> > >  drivers/clk/samsung/clk-pll.c |  280
> > > 
> > > +++++++++++++++++++++++++++++++++++++++++ drivers/clk/samsung/clk-pll.h
> > > |
> > > 
> > >  8 ++
> > >  2 files changed, 288 insertions(+)
> > 
> > Generally the patch looks good, but I have some comments to the part
> > related to 655xx PLLs.
> > 
> > I had a patch adding support for them too, but we can go with yours, since
> > the way of registration has been changed by Yadwinder's patches and mine
> > would have to be updated anyway.
> > 
> > > diff --git a/drivers/clk/samsung/clk-pll.c
> > > b/drivers/clk/samsung/clk-pll.c index 0afaec6..35c15a1 100644
> > > --- a/drivers/clk/samsung/clk-pll.c
> > > +++ b/drivers/clk/samsung/clk-pll.c
> > > @@ -323,6 +323,73 @@ struct clk * __init
> > > samsung_clk_register_pll46xx(const char *name, }
> > > 
> > >  /*
> > > 
> > > + * PLL2126x Clock Type
> > > + */
> > > +
> > > +#define PLL2126X_MDIV_MASK	(0xFF)
> > > +#define PLL2126X_PDIV_MASK	(0x3)
> > > +#define PLL2126X_SDIV_MASK	(0x3)
> > > +#define PLL2126X_MDIV_SHIFT	(16)
> > > +#define PLL2126X_PDIV_SHIFT	(8)
> > > +#define PLL2126X_SDIV_SHIFT	(0)
> 
> +#define PLL2126X_PDIV_MASK	(0x3F)
> 
> is the correct value.
> 
> > > +
> > > +struct samsung_clk_pll2126x {
> > > +	struct clk_hw		hw;
> > > +	const void __iomem	*con_reg;
> > > +};
> > > +
> > > +#define to_clk_pll2126x(_hw) container_of(_hw, struct
> > > samsung_clk_pll2126x, hw) +
> > > +static unsigned long samsung_pll2126x_recalc_rate(struct clk_hw *hw,
> > > +				unsigned long parent_rate)
> > > +{
> > > +	struct samsung_clk_pll2126x *pll = to_clk_pll2126x(hw);
> > > +	u32 pll_con, mdiv, pdiv, sdiv;
> > > +	u64 fvco = parent_rate;
> > > +
> > > +	pll_con = __raw_readl(pll->con_reg);
> > > +	mdiv = (pll_con >> PLL2126X_MDIV_SHIFT) & PLL2126X_MDIV_MASK;
> > > +	pdiv = (pll_con >> PLL2126X_PDIV_SHIFT) & PLL2126X_PDIV_MASK;
> > > +	sdiv = (pll_con >> PLL2126X_SDIV_SHIFT) & PLL2126X_SDIV_MASK;
> > > +
> > > +	fvco *= (mdiv + 8);
> > > +	do_div(fvco, (pdiv + 2) << sdiv);
> > > +
> > > +	return (unsigned long)fvco;
> > > +}
> > > +
> > > +static const struct clk_ops samsung_pll2126x_clk_ops = {
> > > +	.recalc_rate = samsung_pll2126x_recalc_rate,
> > > +};
> > > +
> > > +struct clk * __init samsung_clk_register_pll2126x(const char *name,
> > > +			const char *pname, const void __iomem *con_reg)
> > > +{
> > > +	struct samsung_clk_pll2126x *pll;
> > > +	struct clk *clk;
> > > +	struct clk_init_data init;
> > > +
> > > +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> > > +	if (!pll)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	init.name = name;
> > > +	init.ops = &samsung_pll2126x_clk_ops;
> > > +	init.flags = CLK_GET_RATE_NOCACHE;
> > > +	init.parent_names = &pname;
> > > +	init.num_parents = 1;
> > > +
> > > +	pll->hw.init = &init;
> > > +	pll->con_reg = con_reg;
> > > +
> > > +	clk = samsung_register_pll(&pll->hw);
> > > +	if (IS_ERR(clk))
> > > +		kfree(pll);
> > > +
> > > +	return clk;
> > > +}
> > > +
> > > +/*
> > > 
> > >   * PLL2550x Clock Type
> > >   */
> > > 
> > > @@ -396,3 +463,216 @@ struct clk * __init
> > > samsung_clk_register_pll2550x(const char *name,
> > > 
> > >  	return clk;
> > >  
> > >  }
> > > 
> > > +
> > > +/*
> > > + * PLL3000x Clock Type
> > > + */
> > > +
> > > +#define PLL3000X_MDIV_MASK	(0xFF)
> > > +#define PLL3000X_PDIV_MASK	(0x3)
> > > +#define PLL3000X_SDIV_MASK	(0x3)
> > > +#define PLL3000X_MDIV_SHIFT	(16)
> > > +#define PLL3000X_PDIV_SHIFT	(8)
> > > +#define PLL3000X_SDIV_SHIFT	(0)
> 
> these are correct.
> 
> > > +
> > > +struct samsung_clk_pll3000x {
> > > +	struct clk_hw		hw;
> > > +	const void __iomem	*con_reg;
> > > +};
> > > +
> > > +#define to_clk_pll3000x(_hw) container_of(_hw, struct
> > > samsung_clk_pll3000x, hw) +
> > > +static unsigned long samsung_pll3000x_recalc_rate(struct clk_hw *hw,
> > > +				unsigned long parent_rate)
> > > +{
> > > +	struct samsung_clk_pll3000x *pll = to_clk_pll3000x(hw);
> > > +	u32 pll_con, mdiv, pdiv, sdiv;
> > > +	u64 fvco = parent_rate;
> > > +
> > > +	pll_con = __raw_readl(pll->con_reg);
> > > +	mdiv = (pll_con >> PLL3000X_MDIV_SHIFT) & PLL3000X_MDIV_MASK;
> > > +	pdiv = (pll_con >> PLL3000X_PDIV_SHIFT) & PLL3000X_PDIV_MASK;
> > > +	sdiv = (pll_con >> PLL3000X_SDIV_SHIFT) & PLL3000X_SDIV_MASK;
> > > +
> > > +	fvco *= (2 * (mdiv + 8));
> > > +	do_div(fvco, pdiv << sdiv);
> > > +
> > > +	return (unsigned long)fvco;
> > > +}
> > > +
> > > +static const struct clk_ops samsung_pll3000x_clk_ops = {
> > > +	.recalc_rate = samsung_pll3000x_recalc_rate,
> > > +};
> > > +
> > > +struct clk * __init samsung_clk_register_pll3000x(const char *name,
> > > +			const char *pname, const void __iomem *con_reg)
> > > +{
> > > +	struct samsung_clk_pll3000x *pll;
> > > +	struct clk *clk;
> > > +	struct clk_init_data init;
> > > +
> > > +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> > > +	if (!pll)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	init.name = name;
> > > +	init.ops = &samsung_pll3000x_clk_ops;
> > > +	init.flags = CLK_GET_RATE_NOCACHE;
> > > +	init.parent_names = &pname;
> > > +	init.num_parents = 1;
> > > +
> > > +	pll->hw.init = &init;
> > > +	pll->con_reg = con_reg;
> > > +
> > > +	clk = samsung_register_pll(&pll->hw);
> > > +	if (IS_ERR(clk))
> > > +		kfree(pll);
> > > +
> > > +	return clk;
> > > +}
> > > +
> > > +/*
> > > + * PLL6552x Clock Type
> > > + */
> > > +
> > > +#define PLL6552X_MDIV_MASK	(0x3FF)
> > > +#define PLL6552X_PDIV_MASK	(0x3F)
> > > +#define PLL6552X_SDIV_MASK	(0x7)
> > > +#define PLL6552X_MDIV_SHIFT	(14)
> > > +#define PLL6552X_PDIV_SHIFT	(5)
> > > +#define PLL6552X_SDIV_SHIFT	(0)
> > 
> > Are you sure about those bitfields?
> > 
> > In S3C6410 User's Manual they are different. You can look at my patch for
> > a
> > comparison:
> > 
> > http://thread.gmane.org/gmane.linux.usb.general/87571/focus=88344
> 
> The numbers where taken from the previous pll code, but I now again checked
> them against the datasheet of the s3c2416 and the s3c2450.
> 
> When comparing with your patch, it really seems that the bit offsets in the
> register are different for the pdiv and mdiv - the above values are correct
> according the the datasheet (and also produce the expected results in the
> clock tree).

> > > +struct samsung_clk_pll6552x {
> > > +	struct clk_hw		hw;
> > > +	const void __iomem	*con_reg;
> > > +};
> > > +
> > > +#define to_clk_pll6552x(_hw) container_of(_hw, struct
> > > samsung_clk_pll6552x, hw) +
> > > +static unsigned long samsung_pll6552x_recalc_rate(struct clk_hw *hw,
> > > +				unsigned long parent_rate)
> > > +{
> > > +	struct samsung_clk_pll6552x *pll = to_clk_pll6552x(hw);
> > > +	u32 pll_con, mdiv, pdiv, sdiv;
> > > +	u64 fvco = parent_rate;
> > > +
> > > +	pll_con = __raw_readl(pll->con_reg);
> > > +	mdiv = (pll_con >> PLL6552X_MDIV_SHIFT) & PLL6552X_MDIV_MASK;
> > > +	pdiv = (pll_con >> PLL6552X_PDIV_SHIFT) & PLL6552X_PDIV_MASK;
> > > +	sdiv = (pll_con >> PLL6552X_SDIV_SHIFT) & PLL6552X_SDIV_MASK;
> > > +
> > > +	fvco *= mdiv;
> > > +	do_div(fvco, (pdiv << sdiv));
> > > +
> > > +	return (unsigned long)fvco;
> > > +}
> > > +
> > > +static const struct clk_ops samsung_pll6552x_clk_ops = {
> > > +	.recalc_rate = samsung_pll6552x_recalc_rate,
> > > +};
> > > +
> > > +struct clk * __init samsung_clk_register_pll6552x(const char *name,
> > > +			const char *pname, const void __iomem *con_reg)
> > > +{
> > > +	struct samsung_clk_pll6552x *pll;
> > > +	struct clk *clk;
> > > +	struct clk_init_data init;
> > > +
> > > +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> > > +	if (!pll)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	init.name = name;
> > > +	init.ops = &samsung_pll6552x_clk_ops;
> > > +	init.flags = CLK_GET_RATE_NOCACHE;
> > > +	init.parent_names = &pname;
> > > +	init.num_parents = 1;
> > > +
> > > +	pll->hw.init = &init;
> > > +	pll->con_reg = con_reg;
> > > +
> > > +	clk = samsung_register_pll(&pll->hw);
> > > +	if (IS_ERR(clk))
> > > +		kfree(pll);
> > > +
> > > +	return clk;
> > > +}
> > > +
> > > +/*
> > > + * PLL6553x Clock Type
> > > + */
> > > +
> > > +#define PLL6553X_MDIV_MASK	(0x7F)
> > > +#define PLL6553X_PDIV_MASK	(0x1F)
> > > +#define PLL6553X_SDIV_MASK	(0x3)
> > > +#define PLL6553X_KDIV_MASK	(0xFFFF)
> > > +#define PLL6553X_MDIV_SHIFT	(16)
> > > +#define PLL6553X_PDIV_SHIFT	(8)
> > > +#define PLL6553X_SDIV_SHIFT	(0)
> > 
> > Same about those bitfields. They seem to be different on S3C64xx.
> 
> Here it seems the values were off in the original code. According to the
> datasheet the values in your patch are correct. Thanks for the catch.
> 
> +#define PLL6553X_MDIV_MASK	(0xFF)
> +#define PLL6553X_PDIV_MASK	(0x3F)
> +#define PLL6553X_SDIV_MASK	(0x7)
> 
> 
> This leaves the problem on what to do with the 6552X and its different bit
> offsets. Is the pll in question really a 6552X? In the s3c2416 manual its
> name is explicitly stated.

So is it in S3C6410 User's Manual.

If you look at the old plat/pll.h header, you can see that none of S3C2416_PLL 
(which I believe corresponds to your 6552X) and S3C6400_PLL (which is the 
PLL6552x that can be found on S3C64xx) is labelled as PLL6552x explicitly.

Best regards,
Tomasz
diff mbox

Patch

diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
index 0afaec6..35c15a1 100644
--- a/drivers/clk/samsung/clk-pll.c
+++ b/drivers/clk/samsung/clk-pll.c
@@ -323,6 +323,73 @@  struct clk * __init samsung_clk_register_pll46xx(const char *name,
 }
 
 /*
+ * PLL2126x Clock Type
+ */
+
+#define PLL2126X_MDIV_MASK	(0xFF)
+#define PLL2126X_PDIV_MASK	(0x3)
+#define PLL2126X_SDIV_MASK	(0x3)
+#define PLL2126X_MDIV_SHIFT	(16)
+#define PLL2126X_PDIV_SHIFT	(8)
+#define PLL2126X_SDIV_SHIFT	(0)
+
+struct samsung_clk_pll2126x {
+	struct clk_hw		hw;
+	const void __iomem	*con_reg;
+};
+
+#define to_clk_pll2126x(_hw) container_of(_hw, struct samsung_clk_pll2126x, hw)
+
+static unsigned long samsung_pll2126x_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct samsung_clk_pll2126x *pll = to_clk_pll2126x(hw);
+	u32 pll_con, mdiv, pdiv, sdiv;
+	u64 fvco = parent_rate;
+
+	pll_con = __raw_readl(pll->con_reg);
+	mdiv = (pll_con >> PLL2126X_MDIV_SHIFT) & PLL2126X_MDIV_MASK;
+	pdiv = (pll_con >> PLL2126X_PDIV_SHIFT) & PLL2126X_PDIV_MASK;
+	sdiv = (pll_con >> PLL2126X_SDIV_SHIFT) & PLL2126X_SDIV_MASK;
+
+	fvco *= (mdiv + 8);
+	do_div(fvco, (pdiv + 2) << sdiv);
+
+	return (unsigned long)fvco;
+}
+
+static const struct clk_ops samsung_pll2126x_clk_ops = {
+	.recalc_rate = samsung_pll2126x_recalc_rate,
+};
+
+struct clk * __init samsung_clk_register_pll2126x(const char *name,
+			const char *pname, const void __iomem *con_reg)
+{
+	struct samsung_clk_pll2126x *pll;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &samsung_pll2126x_clk_ops;
+	init.flags = CLK_GET_RATE_NOCACHE;
+	init.parent_names = &pname;
+	init.num_parents = 1;
+
+	pll->hw.init = &init;
+	pll->con_reg = con_reg;
+
+	clk = samsung_register_pll(&pll->hw);
+	if (IS_ERR(clk))
+		kfree(pll);
+
+	return clk;
+}
+
+/*
  * PLL2550x Clock Type
  */
 
@@ -396,3 +463,216 @@  struct clk * __init samsung_clk_register_pll2550x(const char *name,
 
 	return clk;
 }
+
+/*
+ * PLL3000x Clock Type
+ */
+
+#define PLL3000X_MDIV_MASK	(0xFF)
+#define PLL3000X_PDIV_MASK	(0x3)
+#define PLL3000X_SDIV_MASK	(0x3)
+#define PLL3000X_MDIV_SHIFT	(16)
+#define PLL3000X_PDIV_SHIFT	(8)
+#define PLL3000X_SDIV_SHIFT	(0)
+
+struct samsung_clk_pll3000x {
+	struct clk_hw		hw;
+	const void __iomem	*con_reg;
+};
+
+#define to_clk_pll3000x(_hw) container_of(_hw, struct samsung_clk_pll3000x, hw)
+
+static unsigned long samsung_pll3000x_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct samsung_clk_pll3000x *pll = to_clk_pll3000x(hw);
+	u32 pll_con, mdiv, pdiv, sdiv;
+	u64 fvco = parent_rate;
+
+	pll_con = __raw_readl(pll->con_reg);
+	mdiv = (pll_con >> PLL3000X_MDIV_SHIFT) & PLL3000X_MDIV_MASK;
+	pdiv = (pll_con >> PLL3000X_PDIV_SHIFT) & PLL3000X_PDIV_MASK;
+	sdiv = (pll_con >> PLL3000X_SDIV_SHIFT) & PLL3000X_SDIV_MASK;
+
+	fvco *= (2 * (mdiv + 8));
+	do_div(fvco, pdiv << sdiv);
+
+	return (unsigned long)fvco;
+}
+
+static const struct clk_ops samsung_pll3000x_clk_ops = {
+	.recalc_rate = samsung_pll3000x_recalc_rate,
+};
+
+struct clk * __init samsung_clk_register_pll3000x(const char *name,
+			const char *pname, const void __iomem *con_reg)
+{
+	struct samsung_clk_pll3000x *pll;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &samsung_pll3000x_clk_ops;
+	init.flags = CLK_GET_RATE_NOCACHE;
+	init.parent_names = &pname;
+	init.num_parents = 1;
+
+	pll->hw.init = &init;
+	pll->con_reg = con_reg;
+
+	clk = samsung_register_pll(&pll->hw);
+	if (IS_ERR(clk))
+		kfree(pll);
+
+	return clk;
+}
+
+/*
+ * PLL6552x Clock Type
+ */
+
+#define PLL6552X_MDIV_MASK	(0x3FF)
+#define PLL6552X_PDIV_MASK	(0x3F)
+#define PLL6552X_SDIV_MASK	(0x7)
+#define PLL6552X_MDIV_SHIFT	(14)
+#define PLL6552X_PDIV_SHIFT	(5)
+#define PLL6552X_SDIV_SHIFT	(0)
+
+struct samsung_clk_pll6552x {
+	struct clk_hw		hw;
+	const void __iomem	*con_reg;
+};
+
+#define to_clk_pll6552x(_hw) container_of(_hw, struct samsung_clk_pll6552x, hw)
+
+static unsigned long samsung_pll6552x_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct samsung_clk_pll6552x *pll = to_clk_pll6552x(hw);
+	u32 pll_con, mdiv, pdiv, sdiv;
+	u64 fvco = parent_rate;
+
+	pll_con = __raw_readl(pll->con_reg);
+	mdiv = (pll_con >> PLL6552X_MDIV_SHIFT) & PLL6552X_MDIV_MASK;
+	pdiv = (pll_con >> PLL6552X_PDIV_SHIFT) & PLL6552X_PDIV_MASK;
+	sdiv = (pll_con >> PLL6552X_SDIV_SHIFT) & PLL6552X_SDIV_MASK;
+
+	fvco *= mdiv;
+	do_div(fvco, (pdiv << sdiv));
+
+	return (unsigned long)fvco;
+}
+
+static const struct clk_ops samsung_pll6552x_clk_ops = {
+	.recalc_rate = samsung_pll6552x_recalc_rate,
+};
+
+struct clk * __init samsung_clk_register_pll6552x(const char *name,
+			const char *pname, const void __iomem *con_reg)
+{
+	struct samsung_clk_pll6552x *pll;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &samsung_pll6552x_clk_ops;
+	init.flags = CLK_GET_RATE_NOCACHE;
+	init.parent_names = &pname;
+	init.num_parents = 1;
+
+	pll->hw.init = &init;
+	pll->con_reg = con_reg;
+
+	clk = samsung_register_pll(&pll->hw);
+	if (IS_ERR(clk))
+		kfree(pll);
+
+	return clk;
+}
+
+/*
+ * PLL6553x Clock Type
+ */
+
+#define PLL6553X_MDIV_MASK	(0x7F)
+#define PLL6553X_PDIV_MASK	(0x1F)
+#define PLL6553X_SDIV_MASK	(0x3)
+#define PLL6553X_KDIV_MASK	(0xFFFF)
+#define PLL6553X_MDIV_SHIFT	(16)
+#define PLL6553X_PDIV_SHIFT	(8)
+#define PLL6553X_SDIV_SHIFT	(0)
+
+struct samsung_clk_pll6553x {
+	struct clk_hw		hw;
+	const void __iomem	*con_reg;
+};
+
+#define to_clk_pll6553x(_hw) container_of(_hw, struct samsung_clk_pll6553x, hw)
+
+static unsigned long samsung_pll6553x_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct samsung_clk_pll6553x *pll = to_clk_pll6553x(hw);
+	u32 pll_con0, pll_con1, mdiv, pdiv, sdiv, kdiv;
+	u64 fvco = parent_rate;
+
+	pll_con0 = __raw_readl(pll->con_reg);
+	pll_con1 = __raw_readl(pll->con_reg + 4);
+	mdiv = (pll_con0 >> PLL6553X_MDIV_SHIFT) & PLL6553X_MDIV_MASK;
+	pdiv = (pll_con0 >> PLL6553X_PDIV_SHIFT) & PLL6553X_PDIV_MASK;
+	sdiv = (pll_con0 >> PLL6553X_SDIV_SHIFT) & PLL6553X_SDIV_MASK;
+	kdiv = pll_con1 & PLL6553X_KDIV_MASK;
+
+	/*
+	 * We need to multiple parent_rate by mdiv (the integer part) and kdiv
+	 * which is in 2^16ths, so shift mdiv up (does not overflow) and
+	 * add kdiv before multiplying. The use of tmp is to avoid any
+	 * overflows before shifting bac down into result when multipling
+	 * by the mdiv and kdiv pair.
+	 */
+
+	fvco *= (mdiv << 16) + kdiv;
+	do_div(fvco, (pdiv << sdiv));
+	fvco >>= 16;
+
+	return (unsigned long)fvco;
+}
+
+static const struct clk_ops samsung_pll6553x_clk_ops = {
+	.recalc_rate = samsung_pll6553x_recalc_rate,
+};
+
+struct clk * __init samsung_clk_register_pll6553x(const char *name,
+			const char *pname, const void __iomem *con_reg)
+{
+	struct samsung_clk_pll6553x *pll;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &samsung_pll6553x_clk_ops;
+	init.flags = CLK_GET_RATE_NOCACHE;
+	init.parent_names = &pname;
+	init.num_parents = 1;
+
+	pll->hw.init = &init;
+	pll->con_reg = con_reg;
+
+	clk = samsung_register_pll(&pll->hw);
+	if (IS_ERR(clk))
+		kfree(pll);
+
+	return clk;
+}
diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h
index f33786e..465ee6f 100644
--- a/drivers/clk/samsung/clk-pll.h
+++ b/drivers/clk/samsung/clk-pll.h
@@ -34,8 +34,16 @@  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 * __init samsung_clk_register_pll2126x(const char *name,
+			const char *pname, const void __iomem *con_reg);
 extern struct clk * __init samsung_clk_register_pll2550x(const char *name,
 			const char *pname, const void __iomem *reg_base,
 			const unsigned long offset);
+extern struct clk * __init samsung_clk_register_pll3000x(const char *name,
+			const char *pname, const void __iomem *con_reg);
+extern struct clk * __init samsung_clk_register_pll6552x(const char *name,
+			const char *pname, const void __iomem *con_reg);
+extern struct clk * __init samsung_clk_register_pll6553x(const char *name,
+			const char *pname, const void __iomem *con_reg);
 
 #endif /* __SAMSUNG_CLK_PLL_H */