[RFC V2 0/8] imx5 clock port to Mike's clkv4

[RFC V2 0/8] imx5 clock port to Mike's clkv4

[Richard Zhao]

Changes in V2:
 - port to Mike's clkv4 base.
 - move clk-div and clk-mux define macro to common header file
 - add driver/clk/imx to hold imx clock drivers and move imx common clock
   and imx51/53 clock to the folder

[RFC V2 1/8] clk: pass parent rate if recalc_rate is NULL
[RFC V2 2/8] clk: Add support for simple dividers
[RFC V2 3/8] clk: Add support for a generic clock multiplexer
[RFC V2 4/8] ARM i.MX: Add generic support for pllv2
[RFC V2 5/8] ARM i.MX: clk: add generic support of gate2b
[RFC V2 6/8] ARM i.MX: prepare common clk support
[RFC V2 7/8] ARM i.MX51/53: reimplement clock support
[RFC V2 8/8] clk/imx: move imx51/imx53 clock to driver/clk/imx

Thanks
Richard

[RFC V2 0/8] imx5 clock port to Mike's clkv4

[Richard Zhao]

Since all the 8 patches are blocked. Attach them.
Sorry for the inconvenient.

Richard

On 14 December 2011 17:23, Richard Zhao <richard.zhao@linaro.org> wrote:
> Changes in V2:
> ?- port to Mike's clkv4 base.
> ?- move clk-div and clk-mux define macro to common header file
> ?- add driver/clk/imx to hold imx clock drivers and move imx common clock
> ? and imx51/53 clock to the folder
>
> [RFC V2 1/8] clk: pass parent rate if recalc_rate is NULL
> [RFC V2 2/8] clk: Add support for simple dividers
> [RFC V2 3/8] clk: Add support for a generic clock multiplexer
> [RFC V2 4/8] ARM i.MX: Add generic support for pllv2
> [RFC V2 5/8] ARM i.MX: clk: add generic support of gate2b
> [RFC V2 6/8] ARM i.MX: prepare common clk support
> [RFC V2 7/8] ARM i.MX51/53: reimplement clock support
> [RFC V2 8/8] clk/imx: move imx51/imx53 clock to driver/clk/imx
>
> Thanks
> Richard
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: imx5_clk_patch.tar.gz
Type: application/x-gzip
Size: 33507 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111214/5693b623/attachment-0001.gz>

[RFC V2 1/8] clk: pass parent rate if recalc_rate is NULL

[Mike Turquette]

On Wed, Dec 14, 2011 at 1:23 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
> ?drivers/clk/clk.c | ? ?4 ++++
> ?1 files changed, 4 insertions(+), 0 deletions(-)

Last time this patch came up I pointed out that Saravana objected to
it at Linaro Connect but there was no response, so I'll try again.

Saravana, do you still dislike this change for your platform?
Otherwise it seems like a sane default and I'll take it in for the
next series.

Regards,
Mike

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index a6ddbb1..30ba22e 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -470,6 +470,8 @@ static void __clk_recalc_rates(struct clk *clk, unsigned long msg)
>
> ? ? ? ?if (clk->ops->recalc_rate)
> ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
> + ? ? ? else if (clk->parent)
> + ? ? ? ? ? ? ? clk->rate = clk->parent->rate;
>
> ? ? ? ?/* ignore return value for POST_RATE_CHANGE & ABORT_RATE_CHANGE */
> ? ? ? ?if (clk->notifier_count)
> @@ -864,6 +866,8 @@ void clk_init(struct device *dev, struct clk *clk)
>
> ? ? ? ?if (clk->ops->recalc_rate)
> ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
> + ? ? ? else if (clk->parent)
> + ? ? ? ? ? ? ? clk->rate = clk->parent->rate;
> ? ? ? ?else
> ? ? ? ? ? ? ? ?clk->rate = 0;
>
> --
> 1.7.5.4
>
>

[RFC V2 4/8] ARM i.MX: Add generic support for pllv2

[Mike Turquette]

On Wed, Dec 14, 2011 at 1:23 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
>
> The pllv2 is found on i.MX5 SoCs.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
> ?arch/arm/plat-mxc/Kconfig ? ? ? ? ? ? ?| ? ?4 +
> ?arch/arm/plat-mxc/Makefile ? ? ? ? ? ? | ? ?1 +
> ?arch/arm/plat-mxc/clk-pllv2.c ? ? ? ? ?| ?221 ++++++++++++++++++++++++++++++++
> ?arch/arm/plat-mxc/include/mach/clock.h | ? 26 ++++
> ?4 files changed, 252 insertions(+), 0 deletions(-)
> ?create mode 100644 arch/arm/plat-mxc/clk-pllv2.c
>
> diff --git a/arch/arm/plat-mxc/Kconfig b/arch/arm/plat-mxc/Kconfig
> index b3a1f2b..903c15e 100644
> --- a/arch/arm/plat-mxc/Kconfig
> +++ b/arch/arm/plat-mxc/Kconfig
> @@ -106,4 +106,8 @@ config IRAM_ALLOC
> ? ? ? ?bool
> ? ? ? ?select GENERIC_ALLOCATOR
>
> +config IMX_CLK_PLLV2
> + ? ? ? bool
> + ? ? ? depends on GENERIC_CLK
> +
> ?endif
> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> index b9f0f5f..96d45dc 100644
> --- a/arch/arm/plat-mxc/Makefile
> +++ b/arch/arm/plat-mxc/Makefile
> @@ -6,6 +6,7 @@
> ?obj-y := clock.o time.o devices.o cpu.o system.o irq-common.o
>
> ?obj-$(CONFIG_ARM_GIC) += gic.o
> +obj-$(CONFIG_IMX_CLK_PLLV2) += clk-pllv2.o
> ?obj-$(CONFIG_MXC_TZIC) += tzic.o
> ?obj-$(CONFIG_MXC_AVIC) += avic.o
>
> diff --git a/arch/arm/plat-mxc/clk-pllv2.c b/arch/arm/plat-mxc/clk-pllv2.c
> new file mode 100644
> index 0000000..ee94c60
> --- /dev/null
> +++ b/arch/arm/plat-mxc/clk-pllv2.c
> @@ -0,0 +1,221 @@
> +#include <linux/kernel.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/errno.h>
> +#include <linux/delay.h>
> +
> +#include <asm/div64.h>
> +
> +#include <mach/clock.h>
> +
> +#define to_clk_pllv2(ck) (container_of(clk, struct clk_pllv2, clk))
> +
> +/* PLL Register Offsets */
> +#define MXC_PLL_DP_CTL ? ? ? ? ? ? ? ? 0x00
> +#define MXC_PLL_DP_CONFIG ? ? ? ? ? ? ?0x04
> +#define MXC_PLL_DP_OP ? ? ? ? ? ? ? ? ?0x08
> +#define MXC_PLL_DP_MFD ? ? ? ? ? ? ? ? 0x0C
> +#define MXC_PLL_DP_MFN ? ? ? ? ? ? ? ? 0x10
> +#define MXC_PLL_DP_MFNMINUS ? ? ? ? ? ?0x14
> +#define MXC_PLL_DP_MFNPLUS ? ? ? ? ? ? 0x18
> +#define MXC_PLL_DP_HFS_OP ? ? ? ? ? ? ?0x1C
> +#define MXC_PLL_DP_HFS_MFD ? ? ? ? ? ? 0x20
> +#define MXC_PLL_DP_HFS_MFN ? ? ? ? ? ? 0x24
> +#define MXC_PLL_DP_MFN_TOGC ? ? ? ? ? ?0x28
> +#define MXC_PLL_DP_DESTAT ? ? ? ? ? ? ?0x2c
> +
> +/* PLL Register Bit definitions */
> +#define MXC_PLL_DP_CTL_MUL_CTRL ? ? ? ? ? ? ? ?0x2000
> +#define MXC_PLL_DP_CTL_DPDCK0_2_EN ? ? 0x1000
> +#define MXC_PLL_DP_CTL_DPDCK0_2_OFFSET 12
> +#define MXC_PLL_DP_CTL_ADE ? ? ? ? ? ? 0x800
> +#define MXC_PLL_DP_CTL_REF_CLK_DIV ? ? 0x400
> +#define MXC_PLL_DP_CTL_REF_CLK_SEL_MASK ? ? ? ?(3 << 8)
> +#define MXC_PLL_DP_CTL_REF_CLK_SEL_OFFSET ? ? ?8
> +#define MXC_PLL_DP_CTL_HFSM ? ? ? ? ? ?0x80
> +#define MXC_PLL_DP_CTL_PRE ? ? ? ? ? ? 0x40
> +#define MXC_PLL_DP_CTL_UPEN ? ? ? ? ? ?0x20
> +#define MXC_PLL_DP_CTL_RST ? ? ? ? ? ? 0x10
> +#define MXC_PLL_DP_CTL_RCP ? ? ? ? ? ? 0x8
> +#define MXC_PLL_DP_CTL_PLM ? ? ? ? ? ? 0x4
> +#define MXC_PLL_DP_CTL_BRM0 ? ? ? ? ? ?0x2
> +#define MXC_PLL_DP_CTL_LRF ? ? ? ? ? ? 0x1
> +
> +#define MXC_PLL_DP_CONFIG_BIST ? ? ? ? 0x8
> +#define MXC_PLL_DP_CONFIG_SJC_CE ? ? ? 0x4
> +#define MXC_PLL_DP_CONFIG_AREN ? ? ? ? 0x2
> +#define MXC_PLL_DP_CONFIG_LDREQ ? ? ? ? ? ? ? ?0x1
> +
> +#define MXC_PLL_DP_OP_MFI_OFFSET ? ? ? 4
> +#define MXC_PLL_DP_OP_MFI_MASK ? ? ? ? (0xF << 4)
> +#define MXC_PLL_DP_OP_PDF_OFFSET ? ? ? 0
> +#define MXC_PLL_DP_OP_PDF_MASK ? ? ? ? 0xF
> +
> +#define MXC_PLL_DP_MFD_OFFSET ? ? ? ? ?0
> +#define MXC_PLL_DP_MFD_MASK ? ? ? ? ? ?0x07FFFFFF
> +
> +#define MXC_PLL_DP_MFN_OFFSET ? ? ? ? ?0x0
> +#define MXC_PLL_DP_MFN_MASK ? ? ? ? ? ?0x07FFFFFF
> +
> +#define MXC_PLL_DP_MFN_TOGC_TOG_DIS ? ?(1 << 17)
> +#define MXC_PLL_DP_MFN_TOGC_TOG_EN ? ? (1 << 16)
> +#define MXC_PLL_DP_MFN_TOGC_CNT_OFFSET 0x0
> +#define MXC_PLL_DP_MFN_TOGC_CNT_MASK ? 0xFFFF
> +
> +#define MXC_PLL_DP_DESTAT_TOG_SEL ? ? ?(1 << 31)
> +#define MXC_PLL_DP_DESTAT_MFN ? ? ? ? ?0x07FFFFFF
> +
> +#define MAX_DPLL_WAIT_TRIES ? ?1000 /* 1000 * udelay(1) = 1ms */
> +
> +static unsigned long clk_pllv2_get_rate(struct clk *clk)

Nitpick: can you name this clk_pllv2_recalc_rate?  That's because this
function should be called by clk_recalc_rate, whereas clk_get_rate
just returns the cached value in clk->rate.  Improves readability I
think.

> +{
> + ? ? ? long mfi, mfn, mfd, pdf, ref_clk, mfn_abs;
> + ? ? ? unsigned long dp_op, dp_mfd, dp_mfn, dp_ctl, pll_hfsm, dbl;
> + ? ? ? void __iomem *pllbase;
> + ? ? ? s64 temp;
> + ? ? ? unsigned long parent_rate;
> + ? ? ? struct clk_pllv2 *pll = to_clk_pllv2(clk);
> +
> + ? ? ? parent_rate = clk->parent->rate;
> +
> + ? ? ? pllbase = pll->base;
> +
> + ? ? ? dp_ctl = __raw_readl(pllbase + MXC_PLL_DP_CTL);
> + ? ? ? pll_hfsm = dp_ctl & MXC_PLL_DP_CTL_HFSM;
> + ? ? ? dbl = dp_ctl & MXC_PLL_DP_CTL_DPDCK0_2_EN;
> +
> + ? ? ? if (pll_hfsm == 0) {
> + ? ? ? ? ? ? ? dp_op = __raw_readl(pllbase + MXC_PLL_DP_OP);
> + ? ? ? ? ? ? ? dp_mfd = __raw_readl(pllbase + MXC_PLL_DP_MFD);
> + ? ? ? ? ? ? ? dp_mfn = __raw_readl(pllbase + MXC_PLL_DP_MFN);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? dp_op = __raw_readl(pllbase + MXC_PLL_DP_HFS_OP);
> + ? ? ? ? ? ? ? dp_mfd = __raw_readl(pllbase + MXC_PLL_DP_HFS_MFD);
> + ? ? ? ? ? ? ? dp_mfn = __raw_readl(pllbase + MXC_PLL_DP_HFS_MFN);
> + ? ? ? }
> + ? ? ? pdf = dp_op & MXC_PLL_DP_OP_PDF_MASK;
> + ? ? ? mfi = (dp_op & MXC_PLL_DP_OP_MFI_MASK) >> MXC_PLL_DP_OP_MFI_OFFSET;
> + ? ? ? mfi = (mfi <= 5) ? 5 : mfi;
> + ? ? ? mfd = dp_mfd & MXC_PLL_DP_MFD_MASK;
> + ? ? ? mfn = mfn_abs = dp_mfn & MXC_PLL_DP_MFN_MASK;
> + ? ? ? /* Sign extend to 32-bits */
> + ? ? ? if (mfn >= 0x04000000) {
> + ? ? ? ? ? ? ? mfn |= 0xFC000000;
> + ? ? ? ? ? ? ? mfn_abs = -mfn;
> + ? ? ? }
> +
> + ? ? ? ref_clk = 2 * parent_rate;
> + ? ? ? if (dbl != 0)
> + ? ? ? ? ? ? ? ref_clk *= 2;
> +
> + ? ? ? ref_clk /= (pdf + 1);
> + ? ? ? temp = (u64) ref_clk * mfn_abs;
> + ? ? ? do_div(temp, mfd + 1);
> + ? ? ? if (mfn < 0)
> + ? ? ? ? ? ? ? temp = -temp;
> + ? ? ? temp = (ref_clk * mfi) + temp;
> +
> + ? ? ? return temp;
> +}
> +
> +static int clk_pllv2_set_rate(struct clk *clk, unsigned long rate)
> +{
> + ? ? ? u32 reg;
> + ? ? ? void __iomem *pllbase;
> + ? ? ? struct clk_pllv2 *pll = to_clk_pllv2(clk);
> +
> + ? ? ? long mfi, pdf, mfn, mfd = 999999;
> + ? ? ? s64 temp64;
> + ? ? ? unsigned long quad_parent_rate;
> + ? ? ? unsigned long pll_hfsm, dp_ctl;
> + ? ? ? unsigned long parent_rate;
> +
> + ? ? ? parent_rate = clk->parent->rate;
> +
> + ? ? ? pllbase = pll->base;
> +
> + ? ? ? quad_parent_rate = 4 * parent_rate;
> + ? ? ? pdf = mfi = -1;
> + ? ? ? while (++pdf < 16 && mfi < 5)
> + ? ? ? ? ? ? ? mfi = rate * (pdf+1) / quad_parent_rate;
> + ? ? ? if (mfi > 15)
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? pdf--;
> +
> + ? ? ? temp64 = rate * (pdf+1) - quad_parent_rate * mfi;
> + ? ? ? do_div(temp64, quad_parent_rate/1000000);
> + ? ? ? mfn = (long)temp64;
> +
> + ? ? ? dp_ctl = __raw_readl(pllbase + MXC_PLL_DP_CTL);
> + ? ? ? /* use dpdck0_2 */
> + ? ? ? __raw_writel(dp_ctl | 0x1000L, pllbase + MXC_PLL_DP_CTL);
> + ? ? ? pll_hfsm = dp_ctl & MXC_PLL_DP_CTL_HFSM;
> + ? ? ? if (pll_hfsm == 0) {
> + ? ? ? ? ? ? ? reg = mfi << 4 | pdf;
> + ? ? ? ? ? ? ? __raw_writel(reg, pllbase + MXC_PLL_DP_OP);
> + ? ? ? ? ? ? ? __raw_writel(mfd, pllbase + MXC_PLL_DP_MFD);
> + ? ? ? ? ? ? ? __raw_writel(mfn, pllbase + MXC_PLL_DP_MFN);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? reg = mfi << 4 | pdf;
> + ? ? ? ? ? ? ? __raw_writel(reg, pllbase + MXC_PLL_DP_HFS_OP);
> + ? ? ? ? ? ? ? __raw_writel(mfd, pllbase + MXC_PLL_DP_HFS_MFD);
> + ? ? ? ? ? ? ? __raw_writel(mfn, pllbase + MXC_PLL_DP_HFS_MFN);
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +
> +static long clk_pllv2_round_rate(struct clk *clk, unsigned long rate,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *prate)
> +{
> + ? ? ? *prate = 0;

Should check if prate != NULL.  For instance clk_round_rate doesn't
pass in a real parent_rate:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=drivers/clk/clk.c;h=8cadadd744967cdf90df864f8b250a248033790a;hb=f9f6f555d44749912b5f55d5077604d6ad1ae840#l241

Regards,
Mike

[RFC V2 4/8] ARM i.MX: Add generic support for pllv2

[Richard Zhao]

On Fri, Dec 16, 2011 at 03:17:02PM -0800, Mike Turquette wrote:
> On Wed, Dec 14, 2011 at 1:23 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> >
> > The pllv2 is found on i.MX5 SoCs.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > ---
> > ?arch/arm/plat-mxc/Kconfig ? ? ? ? ? ? ?| ? ?4 +
> > ?arch/arm/plat-mxc/Makefile ? ? ? ? ? ? | ? ?1 +
> > ?arch/arm/plat-mxc/clk-pllv2.c ? ? ? ? ?| ?221 ++++++++++++++++++++++++++++++++
> > ?arch/arm/plat-mxc/include/mach/clock.h | ? 26 ++++
> > ?4 files changed, 252 insertions(+), 0 deletions(-)
> > ?create mode 100644 arch/arm/plat-mxc/clk-pllv2.c
> >
> > diff --git a/arch/arm/plat-mxc/Kconfig b/arch/arm/plat-mxc/Kconfig
> > index b3a1f2b..903c15e 100644
> > --- a/arch/arm/plat-mxc/Kconfig
> > +++ b/arch/arm/plat-mxc/Kconfig
> > @@ -106,4 +106,8 @@ config IRAM_ALLOC
> > ? ? ? ?bool
> > ? ? ? ?select GENERIC_ALLOCATOR
> >
> > +config IMX_CLK_PLLV2
> > + ? ? ? bool
> > + ? ? ? depends on GENERIC_CLK
> > +
> > ?endif
> > diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile
> > index b9f0f5f..96d45dc 100644
> > --- a/arch/arm/plat-mxc/Makefile
> > +++ b/arch/arm/plat-mxc/Makefile
> > @@ -6,6 +6,7 @@
> > ?obj-y := clock.o time.o devices.o cpu.o system.o irq-common.o
> >
> > ?obj-$(CONFIG_ARM_GIC) += gic.o
> > +obj-$(CONFIG_IMX_CLK_PLLV2) += clk-pllv2.o
> > ?obj-$(CONFIG_MXC_TZIC) += tzic.o
> > ?obj-$(CONFIG_MXC_AVIC) += avic.o
> >
> > diff --git a/arch/arm/plat-mxc/clk-pllv2.c b/arch/arm/plat-mxc/clk-pllv2.c
> > new file mode 100644
> > index 0000000..ee94c60
> > --- /dev/null
> > +++ b/arch/arm/plat-mxc/clk-pllv2.c
> > @@ -0,0 +1,221 @@
> > +#include <linux/kernel.h>
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/errno.h>
> > +#include <linux/delay.h>
> > +
> > +#include <asm/div64.h>
> > +
> > +#include <mach/clock.h>
> > +
> > +#define to_clk_pllv2(ck) (container_of(clk, struct clk_pllv2, clk))
> > +
> > +/* PLL Register Offsets */
> > +#define MXC_PLL_DP_CTL ? ? ? ? ? ? ? ? 0x00
> > +#define MXC_PLL_DP_CONFIG ? ? ? ? ? ? ?0x04
> > +#define MXC_PLL_DP_OP ? ? ? ? ? ? ? ? ?0x08
> > +#define MXC_PLL_DP_MFD ? ? ? ? ? ? ? ? 0x0C
> > +#define MXC_PLL_DP_MFN ? ? ? ? ? ? ? ? 0x10
> > +#define MXC_PLL_DP_MFNMINUS ? ? ? ? ? ?0x14
> > +#define MXC_PLL_DP_MFNPLUS ? ? ? ? ? ? 0x18
> > +#define MXC_PLL_DP_HFS_OP ? ? ? ? ? ? ?0x1C
> > +#define MXC_PLL_DP_HFS_MFD ? ? ? ? ? ? 0x20
> > +#define MXC_PLL_DP_HFS_MFN ? ? ? ? ? ? 0x24
> > +#define MXC_PLL_DP_MFN_TOGC ? ? ? ? ? ?0x28
> > +#define MXC_PLL_DP_DESTAT ? ? ? ? ? ? ?0x2c
> > +
> > +/* PLL Register Bit definitions */
> > +#define MXC_PLL_DP_CTL_MUL_CTRL ? ? ? ? ? ? ? ?0x2000
> > +#define MXC_PLL_DP_CTL_DPDCK0_2_EN ? ? 0x1000
> > +#define MXC_PLL_DP_CTL_DPDCK0_2_OFFSET 12
> > +#define MXC_PLL_DP_CTL_ADE ? ? ? ? ? ? 0x800
> > +#define MXC_PLL_DP_CTL_REF_CLK_DIV ? ? 0x400
> > +#define MXC_PLL_DP_CTL_REF_CLK_SEL_MASK ? ? ? ?(3 << 8)
> > +#define MXC_PLL_DP_CTL_REF_CLK_SEL_OFFSET ? ? ?8
> > +#define MXC_PLL_DP_CTL_HFSM ? ? ? ? ? ?0x80
> > +#define MXC_PLL_DP_CTL_PRE ? ? ? ? ? ? 0x40
> > +#define MXC_PLL_DP_CTL_UPEN ? ? ? ? ? ?0x20
> > +#define MXC_PLL_DP_CTL_RST ? ? ? ? ? ? 0x10
> > +#define MXC_PLL_DP_CTL_RCP ? ? ? ? ? ? 0x8
> > +#define MXC_PLL_DP_CTL_PLM ? ? ? ? ? ? 0x4
> > +#define MXC_PLL_DP_CTL_BRM0 ? ? ? ? ? ?0x2
> > +#define MXC_PLL_DP_CTL_LRF ? ? ? ? ? ? 0x1
> > +
> > +#define MXC_PLL_DP_CONFIG_BIST ? ? ? ? 0x8
> > +#define MXC_PLL_DP_CONFIG_SJC_CE ? ? ? 0x4
> > +#define MXC_PLL_DP_CONFIG_AREN ? ? ? ? 0x2
> > +#define MXC_PLL_DP_CONFIG_LDREQ ? ? ? ? ? ? ? ?0x1
> > +
> > +#define MXC_PLL_DP_OP_MFI_OFFSET ? ? ? 4
> > +#define MXC_PLL_DP_OP_MFI_MASK ? ? ? ? (0xF << 4)
> > +#define MXC_PLL_DP_OP_PDF_OFFSET ? ? ? 0
> > +#define MXC_PLL_DP_OP_PDF_MASK ? ? ? ? 0xF
> > +
> > +#define MXC_PLL_DP_MFD_OFFSET ? ? ? ? ?0
> > +#define MXC_PLL_DP_MFD_MASK ? ? ? ? ? ?0x07FFFFFF
> > +
> > +#define MXC_PLL_DP_MFN_OFFSET ? ? ? ? ?0x0
> > +#define MXC_PLL_DP_MFN_MASK ? ? ? ? ? ?0x07FFFFFF
> > +
> > +#define MXC_PLL_DP_MFN_TOGC_TOG_DIS ? ?(1 << 17)
> > +#define MXC_PLL_DP_MFN_TOGC_TOG_EN ? ? (1 << 16)
> > +#define MXC_PLL_DP_MFN_TOGC_CNT_OFFSET 0x0
> > +#define MXC_PLL_DP_MFN_TOGC_CNT_MASK ? 0xFFFF
> > +
> > +#define MXC_PLL_DP_DESTAT_TOG_SEL ? ? ?(1 << 31)
> > +#define MXC_PLL_DP_DESTAT_MFN ? ? ? ? ?0x07FFFFFF
> > +
> > +#define MAX_DPLL_WAIT_TRIES ? ?1000 /* 1000 * udelay(1) = 1ms */
> > +
> > +static unsigned long clk_pllv2_get_rate(struct clk *clk)
> 
> Nitpick: can you name this clk_pllv2_recalc_rate?  That's because this
> function should be called by clk_recalc_rate, whereas clk_get_rate
> just returns the cached value in clk->rate.  Improves readability I
> think.
ok, thanks.
> 
> > +{
> > + ? ? ? long mfi, mfn, mfd, pdf, ref_clk, mfn_abs;
> > + ? ? ? unsigned long dp_op, dp_mfd, dp_mfn, dp_ctl, pll_hfsm, dbl;
> > + ? ? ? void __iomem *pllbase;
> > + ? ? ? s64 temp;
> > + ? ? ? unsigned long parent_rate;
> > + ? ? ? struct clk_pllv2 *pll = to_clk_pllv2(clk);
> > +
> > + ? ? ? parent_rate = clk->parent->rate;
> > +
> > + ? ? ? pllbase = pll->base;
> > +
> > + ? ? ? dp_ctl = __raw_readl(pllbase + MXC_PLL_DP_CTL);
> > + ? ? ? pll_hfsm = dp_ctl & MXC_PLL_DP_CTL_HFSM;
> > + ? ? ? dbl = dp_ctl & MXC_PLL_DP_CTL_DPDCK0_2_EN;
> > +
> > + ? ? ? if (pll_hfsm == 0) {
> > + ? ? ? ? ? ? ? dp_op = __raw_readl(pllbase + MXC_PLL_DP_OP);
> > + ? ? ? ? ? ? ? dp_mfd = __raw_readl(pllbase + MXC_PLL_DP_MFD);
> > + ? ? ? ? ? ? ? dp_mfn = __raw_readl(pllbase + MXC_PLL_DP_MFN);
> > + ? ? ? } else {
> > + ? ? ? ? ? ? ? dp_op = __raw_readl(pllbase + MXC_PLL_DP_HFS_OP);
> > + ? ? ? ? ? ? ? dp_mfd = __raw_readl(pllbase + MXC_PLL_DP_HFS_MFD);
> > + ? ? ? ? ? ? ? dp_mfn = __raw_readl(pllbase + MXC_PLL_DP_HFS_MFN);
> > + ? ? ? }
> > + ? ? ? pdf = dp_op & MXC_PLL_DP_OP_PDF_MASK;
> > + ? ? ? mfi = (dp_op & MXC_PLL_DP_OP_MFI_MASK) >> MXC_PLL_DP_OP_MFI_OFFSET;
> > + ? ? ? mfi = (mfi <= 5) ? 5 : mfi;
> > + ? ? ? mfd = dp_mfd & MXC_PLL_DP_MFD_MASK;
> > + ? ? ? mfn = mfn_abs = dp_mfn & MXC_PLL_DP_MFN_MASK;
> > + ? ? ? /* Sign extend to 32-bits */
> > + ? ? ? if (mfn >= 0x04000000) {
> > + ? ? ? ? ? ? ? mfn |= 0xFC000000;
> > + ? ? ? ? ? ? ? mfn_abs = -mfn;
> > + ? ? ? }
> > +
> > + ? ? ? ref_clk = 2 * parent_rate;
> > + ? ? ? if (dbl != 0)
> > + ? ? ? ? ? ? ? ref_clk *= 2;
> > +
> > + ? ? ? ref_clk /= (pdf + 1);
> > + ? ? ? temp = (u64) ref_clk * mfn_abs;
> > + ? ? ? do_div(temp, mfd + 1);
> > + ? ? ? if (mfn < 0)
> > + ? ? ? ? ? ? ? temp = -temp;
> > + ? ? ? temp = (ref_clk * mfi) + temp;
> > +
> > + ? ? ? return temp;
> > +}
> > +
> > +static int clk_pllv2_set_rate(struct clk *clk, unsigned long rate)
> > +{
> > + ? ? ? u32 reg;
> > + ? ? ? void __iomem *pllbase;
> > + ? ? ? struct clk_pllv2 *pll = to_clk_pllv2(clk);
> > +
> > + ? ? ? long mfi, pdf, mfn, mfd = 999999;
> > + ? ? ? s64 temp64;
> > + ? ? ? unsigned long quad_parent_rate;
> > + ? ? ? unsigned long pll_hfsm, dp_ctl;
> > + ? ? ? unsigned long parent_rate;
> > +
> > + ? ? ? parent_rate = clk->parent->rate;
> > +
> > + ? ? ? pllbase = pll->base;
> > +
> > + ? ? ? quad_parent_rate = 4 * parent_rate;
> > + ? ? ? pdf = mfi = -1;
> > + ? ? ? while (++pdf < 16 && mfi < 5)
> > + ? ? ? ? ? ? ? mfi = rate * (pdf+1) / quad_parent_rate;
> > + ? ? ? if (mfi > 15)
> > + ? ? ? ? ? ? ? return -EINVAL;
> > + ? ? ? pdf--;
> > +
> > + ? ? ? temp64 = rate * (pdf+1) - quad_parent_rate * mfi;
> > + ? ? ? do_div(temp64, quad_parent_rate/1000000);
> > + ? ? ? mfn = (long)temp64;
> > +
> > + ? ? ? dp_ctl = __raw_readl(pllbase + MXC_PLL_DP_CTL);
> > + ? ? ? /* use dpdck0_2 */
> > + ? ? ? __raw_writel(dp_ctl | 0x1000L, pllbase + MXC_PLL_DP_CTL);
> > + ? ? ? pll_hfsm = dp_ctl & MXC_PLL_DP_CTL_HFSM;
> > + ? ? ? if (pll_hfsm == 0) {
> > + ? ? ? ? ? ? ? reg = mfi << 4 | pdf;
> > + ? ? ? ? ? ? ? __raw_writel(reg, pllbase + MXC_PLL_DP_OP);
> > + ? ? ? ? ? ? ? __raw_writel(mfd, pllbase + MXC_PLL_DP_MFD);
> > + ? ? ? ? ? ? ? __raw_writel(mfn, pllbase + MXC_PLL_DP_MFN);
> > + ? ? ? } else {
> > + ? ? ? ? ? ? ? reg = mfi << 4 | pdf;
> > + ? ? ? ? ? ? ? __raw_writel(reg, pllbase + MXC_PLL_DP_HFS_OP);
> > + ? ? ? ? ? ? ? __raw_writel(mfd, pllbase + MXC_PLL_DP_HFS_MFD);
> > + ? ? ? ? ? ? ? __raw_writel(mfn, pllbase + MXC_PLL_DP_HFS_MFN);
> > + ? ? ? }
> > +
> > + ? ? ? return 0;
> > +}
> > +
> > +static long clk_pllv2_round_rate(struct clk *clk, unsigned long rate,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *prate)
> > +{
> > + ? ? ? *prate = 0;
> 
> Should check if prate != NULL.  For instance clk_round_rate doesn't
> pass in a real parent_rate:
> http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=drivers/clk/clk.c;h=8cadadd744967cdf90df864f8b250a248033790a;hb=f9f6f555d44749912b5f55d5077604d6ad1ae840#l241
right, thanks.

Richard
> 
> Regards,
> Mike

[RFC V2 5/8] ARM i.MX: clk: add generic support of gate2b

[Mike Turquette]

On Wed, Dec 14, 2011 at 1:23 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> +static int clk_gate2b_enable(struct clk *clk)
> +{
> + ? ? ? struct clk_gate2b *gate2b = to_clk_gate2b(clk);
> + ? ? ? unsigned long flags;
> + ? ? ? u32 reg;
> +
> + ? ? ? if (gate2b->lock)
> + ? ? ? ? ? ? ? spin_lock_irqsave(gate2b->lock, flags);

Are their bits in this register used by other clocks which aren't
protected by the enable_lock spinlock?

> +int clk_gate2b_set_val(struct clk *clk, int en, int dis)
> +{
> + ? ? ? struct clk_gate2b *gate2b = to_clk_gate2b(clk);
> + ? ? ? unsigned long flags;
> + ? ? ? u32 reg, val;
> +
> + ? ? ? en &= 0x3;
> + ? ? ? dis &= 0x3;
> +
> + ? ? ? if (gate2b->lock)
> + ? ? ? ? ? ? ? spin_lock_irqsave(gate2b->lock, flags);
> +
> + ? ? ? reg = __raw_readl(gate2b->reg);
> + ? ? ? val = (reg >> gate2b->shift) & 0x3;
> + ? ? ? reg &= ~(0x3 << gate2b->shift);
> + ? ? ? if (val == gate2b->val_en && val != en)
> + ? ? ? ? ? ? ? reg |= en << gate2b->shift;
> + ? ? ? else if (val == gate2b->val_dis && val != dis)
> + ? ? ? ? ? ? ? reg |= dis << gate2b->shift;
> + ? ? ? __raw_writel(reg, gate2b->reg);
> + ? ? ? gate2b->val_en = en;
> + ? ? ? gate2b->val_dis = dis;
> +
> + ? ? ? if (gate2b->lock)
> + ? ? ? ? ? ? ? spin_unlock_irqrestore(gate2b->lock, flags);
> +
> + ? ? ? return 0;
> +}
> +EXPORT_SYMBOL_GPL(clk_gate2b_set_val);

Who calls clk_gate2b_set_val, and why?

Regards,
Mike

[RFC V2 5/8] ARM i.MX: clk: add generic support of gate2b

[Richard Zhao]

On Fri, Dec 16, 2011 at 03:25:53PM -0800, Mike Turquette wrote:
> On Wed, Dec 14, 2011 at 1:23 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > +static int clk_gate2b_enable(struct clk *clk)
> > +{
> > + ? ? ? struct clk_gate2b *gate2b = to_clk_gate2b(clk);
> > + ? ? ? unsigned long flags;
> > + ? ? ? u32 reg;
> > +
> > + ? ? ? if (gate2b->lock)
> > + ? ? ? ? ? ? ? spin_lock_irqsave(gate2b->lock, flags);
> 
> Are their bits in this register used by other clocks which aren't
> protected by the enable_lock spinlock?
For now, no. If I remove the lock, it'll limit the gate2b usage.
> 
> > +int clk_gate2b_set_val(struct clk *clk, int en, int dis)
> > +{
> > + ? ? ? struct clk_gate2b *gate2b = to_clk_gate2b(clk);
> > + ? ? ? unsigned long flags;
> > + ? ? ? u32 reg, val;
> > +
> > + ? ? ? en &= 0x3;
> > + ? ? ? dis &= 0x3;
> > +
> > + ? ? ? if (gate2b->lock)
> > + ? ? ? ? ? ? ? spin_lock_irqsave(gate2b->lock, flags);
> > +
> > + ? ? ? reg = __raw_readl(gate2b->reg);
> > + ? ? ? val = (reg >> gate2b->shift) & 0x3;
> > + ? ? ? reg &= ~(0x3 << gate2b->shift);
> > + ? ? ? if (val == gate2b->val_en && val != en)
> > + ? ? ? ? ? ? ? reg |= en << gate2b->shift;
> > + ? ? ? else if (val == gate2b->val_dis && val != dis)
> > + ? ? ? ? ? ? ? reg |= dis << gate2b->shift;
> > + ? ? ? __raw_writel(reg, gate2b->reg);
> > + ? ? ? gate2b->val_en = en;
> > + ? ? ? gate2b->val_dis = dis;
> > +
> > + ? ? ? if (gate2b->lock)
> > + ? ? ? ? ? ? ? spin_unlock_irqrestore(gate2b->lock, flags);
> > +
> > + ? ? ? return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(clk_gate2b_set_val);
> 
> Who calls clk_gate2b_set_val, and why?
It's an interface for busfreq. When I enable a gate2b, I may want it to be
active when cpu is running but disabled when wfi. I may also want it to be
active all the time, if some dma device also need it.
So the value I set for enable/disable change at runtime.

Thanks
Richard
> 
> Regards,
> Mike

[RFC V2 6/8] ARM i.MX: prepare common clk support

[Mike Turquette]

On Wed, Dec 14, 2011 at 1:23 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
>
> Add static clock help macros, clock register spinlock.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
> ?arch/arm/plat-mxc/clock.c ? ? ? ? ? ? ?| ? ?8 ++++++
> ?arch/arm/plat-mxc/include/mach/clock.h | ? 42 +++++++++++++++++++++++++++++--
> ?2 files changed, 47 insertions(+), 3 deletions(-)
>
...
>
> +extern spinlock_t imx_ccm_lock;
> +
> +#define DEFINE_CLK_GATE2B(_name, _parent, _reg, _shift) \
> + ? ? ? struct clk_gate2b _name = { \
> + ? ? ? ? ? ? ? .clk = { \
> + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_gate2b_ops, \
> + ? ? ? ? ? ? ? ? ? ? ? .parent = _parent, \
> + ? ? ? ? ? ? ? }, \
> + ? ? ? ? ? ? ? .reg = (_reg), \
> + ? ? ? ? ? ? ? .shift = (_shift) * 2, \
> + ? ? ? ? ? ? ? .val_en = 0x3, \
> + ? ? ? ? ? ? ? .val_dis = 0x0, \
> + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> + ? ? ? }
> +
> +#define IMX_DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width) \
> + ? ? ? DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width, \
> + ? ? ? ? ? ? ? ? ? ? ? &imx_ccm_lock)
> +
> +#define IMX_DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks) \
> + ? ? ? DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks, &imx_ccm_lock)

gate2b, basic div and basic mux clks all use the imx_ccm_lock.  How
many registers are we talking about here?  If the number is very high
then maybe per-register locking makes sense for you?

Also, do you need locking added to the basic gated clk type, or any of
the others?  I'll probably add it anyways but just curious...

Regards,
Mike

[RFC V2 6/8] ARM i.MX: prepare common clk support

[Richard Zhao]

On Fri, Dec 16, 2011 at 03:34:14PM -0800, Mike Turquette wrote:
> On Wed, Dec 14, 2011 at 1:23 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> >
> > Add static clock help macros, clock register spinlock.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > ---
> > ?arch/arm/plat-mxc/clock.c ? ? ? ? ? ? ?| ? ?8 ++++++
> > ?arch/arm/plat-mxc/include/mach/clock.h | ? 42 +++++++++++++++++++++++++++++--
> > ?2 files changed, 47 insertions(+), 3 deletions(-)
> >
> ...
> >
> > +extern spinlock_t imx_ccm_lock;
> > +
> > +#define DEFINE_CLK_GATE2B(_name, _parent, _reg, _shift) \
> > + ? ? ? struct clk_gate2b _name = { \
> > + ? ? ? ? ? ? ? .clk = { \
> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_gate2b_ops, \
> > + ? ? ? ? ? ? ? ? ? ? ? .parent = _parent, \
> > + ? ? ? ? ? ? ? }, \
> > + ? ? ? ? ? ? ? .reg = (_reg), \
> > + ? ? ? ? ? ? ? .shift = (_shift) * 2, \
> > + ? ? ? ? ? ? ? .val_en = 0x3, \
> > + ? ? ? ? ? ? ? .val_dis = 0x0, \
> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> > + ? ? ? }
> > +
> > +#define IMX_DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width) \
> > + ? ? ? DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width, \
> > + ? ? ? ? ? ? ? ? ? ? ? &imx_ccm_lock)
> > +
> > +#define IMX_DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks) \
> > + ? ? ? DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks, &imx_ccm_lock)
> 
> gate2b, basic div and basic mux clks all use the imx_ccm_lock.  How
> many registers are we talking about here?  If the number is very high
> then maybe per-register locking makes sense for you?
per-register/per-register group locking make me feel complicated. How do I group the
registers? and register layout may change for different SoCs.
Considering prepare_lock and enable_lock, only gate register that share with other
non-gate clocks needs a lock. Keeping the rule in mind is strange, and easy to come
with faults.
Another way is to use a global lock. Do normal register read/write in the lock. but for
status busy wait checking, it's read only, we can release the lock temporarily.

Thanks
Richard
> 
> Also, do you need locking added to the basic gated clk type, or any of
> the others?  I'll probably add it anyways but just curious...
I think yes. Give usr an option to use the lock.

Thanks
Richard
> 
> Regards,
> Mike