| Message ID | 1409887606-22388-4-git-send-email-b20788@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 5, 2014 at 12:26 AM, Anson Huang <b20788@freescale.com> wrote: > On i.MX6Q TO > 1.0, i.MX6DL and i.MX6SX, gpt per clock > can be from OSC instead of ipg_per, as ipg_per's rate > may be scaled when system enter low bus mode, to keep > system timer NOT drift, better to make gpt per clock > at fixed rate, here add support for gpt per clock to > be from OSC which is at fixed rate always. > > There are some difference on this implementation of > gpt per clock source, see below for details: > > i.MX6Q TO > 1.0: GPT_CR_CLKSRC, 3b'101 selects fix clock > of OSC / 8 for gpt per clk; You mean "5b'101 selects fix clock..." > i.MX6DL and i.MX6SX: GPT_CR_CLKSRC, 3b'101 selects OSC Same here: "5b'101" > @@ -293,7 +299,7 @@ static int __init mxc_clockevent_init(struct clk *timer_clk) > static void __init _mxc_timer_init(int irq, > struct clk *clk_per, struct clk *clk_ipg) > { > - uint32_t tctl_val; > + uint32_t tctl_val, tprer_val; > > if (IS_ERR(clk_per)) { > pr_err("i.MX timer: unable to get clk\n"); > @@ -312,10 +318,26 @@ static void __init _mxc_timer_init(int irq, > __raw_writel(0, timer_base + MXC_TCTL); > __raw_writel(0, timer_base + MXC_TPRER); /* see datasheet note */ > > - if (timer_is_v2()) > - tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN; > - else > + if (timer_is_v2()) { > + if (((cpu_is_imx6q() && imx_get_soc_revision() > > + IMX_CHIP_REVISION_1_0) || cpu_is_imx6dl() || > + cpu_is_imx6sx()) && (clk_get_rate(clk_per) == > + V2_TIMER_RATE_OSC_DIV8)) { > + tctl_val = V2_TCTL_CLK_OSC_DIV8 | V2_TCTL_FRR | > + V2_TCTL_WAITEN | MXC_TCTL_TEN; > + if (cpu_is_imx6dl() || cpu_is_imx6sx()) { > + /* 24 / 8 = 3 MHz */ > + tprer_val = 7 << V2_TPRER_PRE24M; > + __raw_writel(tprer_val, timer_base + MXC_TPRER); > + tctl_val |= V2_TCTL_24MEN; > + } > + } else { > + tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | > + V2_TCTL_WAITEN | MXC_TCTL_TEN; > + } > + } else { > tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN; > + } Can this block be rearranged a bit so that it becomes easier to read?
Sent from my iPad ? 2014-9-5?20:09?"Fabio Estevam" <festevam@gmail.com> ??? > On Fri, Sep 5, 2014 at 12:26 AM, Anson Huang <b20788@freescale.com> wrote: >> On i.MX6Q TO > 1.0, i.MX6DL and i.MX6SX, gpt per clock >> can be from OSC instead of ipg_per, as ipg_per's rate >> may be scaled when system enter low bus mode, to keep >> system timer NOT drift, better to make gpt per clock >> at fixed rate, here add support for gpt per clock to >> be from OSC which is at fixed rate always. >> >> There are some difference on this implementation of >> gpt per clock source, see below for details: >> >> i.MX6Q TO > 1.0: GPT_CR_CLKSRC, 3b'101 selects fix clock >> of OSC / 8 for gpt per clk; > > You mean "5b'101 selects fix clock..." > >> i.MX6DL and i.MX6SX: GPT_CR_CLKSRC, 3b'101 selects OSC > > Same here: "5b'101" [Anson] I thought 3b'101 means 3 bits, value 101... Maybe my understanding is incorrect, will correct it n v3. > >> @@ -293,7 +299,7 @@ static int __init mxc_clockevent_init(struct clk *timer_clk) >> static void __init _mxc_timer_init(int irq, >> struct clk *clk_per, struct clk *clk_ipg) >> { >> - uint32_t tctl_val; >> + uint32_t tctl_val, tprer_val; >> >> if (IS_ERR(clk_per)) { >> pr_err("i.MX timer: unable to get clk\n"); >> @@ -312,10 +318,26 @@ static void __init _mxc_timer_init(int irq, >> __raw_writel(0, timer_base + MXC_TCTL); >> __raw_writel(0, timer_base + MXC_TPRER); /* see datasheet note */ >> >> - if (timer_is_v2()) >> - tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN; >> - else >> + if (timer_is_v2()) { >> + if (((cpu_is_imx6q() && imx_get_soc_revision() > >> + IMX_CHIP_REVISION_1_0) || cpu_is_imx6dl() || >> + cpu_is_imx6sx()) && (clk_get_rate(clk_per) == >> + V2_TIMER_RATE_OSC_DIV8)) { >> + tctl_val = V2_TCTL_CLK_OSC_DIV8 | V2_TCTL_FRR | >> + V2_TCTL_WAITEN | MXC_TCTL_TEN; >> + if (cpu_is_imx6dl() || cpu_is_imx6sx()) { >> + /* 24 / 8 = 3 MHz */ >> + tprer_val = 7 << V2_TPRER_PRE24M; >> + __raw_writel(tprer_val, timer_base + MXC_TPRER); >> + tctl_val |= V2_TCTL_24MEN; >> + } >> + } else { >> + tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | >> + V2_TCTL_WAITEN | MXC_TCTL_TEN; >> + } >> + } else { >> tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN; >> + } > > Can this block be rearranged a bit so that it becomes easier to read? I have to consider v1, v2, and on v2, MX6Q's implementation is different from MX6DL and MX6SX, MX6SL has its special implementation, and MX6Q has difference between TO1.0 and other TOs, also, we have to consider the old dtb case. So, there are more than 6 different cases we need to consider, I thought it was the best way I can figure out, could you advice if you have better idea?
On Fri, Sep 05, 2014 at 11:26:46AM +0800, Anson Huang wrote: > diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c > index bf92e5a..a3ecb4a 100644 > --- a/arch/arm/mach-imx/time.c > +++ b/arch/arm/mach-imx/time.c > @@ -60,17 +60,23 @@ > #define MX2_TSTAT_CAPT (1 << 1) > #define MX2_TSTAT_COMP (1 << 0) > > -/* MX31, MX35, MX25, MX5 */ > +/* MX31, MX35, MX25, MX5, MX6 */ > #define V2_TCTL_WAITEN (1 << 3) /* Wait enable mode */ > #define V2_TCTL_CLK_IPG (1 << 6) > #define V2_TCTL_CLK_PER (2 << 6) > +#define V2_TCTL_CLK_OSC_DIV8 (5 << 6) > +#define V2_TCTL_CLK_OSC (7 << 6) This one is unused? > #define V2_TCTL_FRR (1 << 9) > +#define V2_TCTL_24MEN (1 << 10) > +#define V2_TPRER_PRE24M 12 > #define V2_IR 0x0c > #define V2_TSTAT 0x08 > #define V2_TSTAT_OF1 (1 << 0) > #define V2_TCN 0x24 > #define V2_TCMP 0x10 > > +#define V2_TIMER_RATE_OSC_DIV8 3000000 A space instead of tab should be used right after '#define'. > + > #define timer_is_v1() (cpu_is_mx1() || cpu_is_mx21() || cpu_is_mx27()) > #define timer_is_v2() (!timer_is_v1()) > > @@ -293,7 +299,7 @@ static int __init mxc_clockevent_init(struct clk *timer_clk) > static void __init _mxc_timer_init(int irq, > struct clk *clk_per, struct clk *clk_ipg) > { > - uint32_t tctl_val; > + uint32_t tctl_val, tprer_val; The variable tprer_val does not seem really needed. Shawn > > if (IS_ERR(clk_per)) { > pr_err("i.MX timer: unable to get clk\n"); > @@ -312,10 +318,26 @@ static void __init _mxc_timer_init(int irq, > __raw_writel(0, timer_base + MXC_TCTL); > __raw_writel(0, timer_base + MXC_TPRER); /* see datasheet note */ > > - if (timer_is_v2()) > - tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN; > - else > + if (timer_is_v2()) { > + if (((cpu_is_imx6q() && imx_get_soc_revision() > > + IMX_CHIP_REVISION_1_0) || cpu_is_imx6dl() || > + cpu_is_imx6sx()) && (clk_get_rate(clk_per) == > + V2_TIMER_RATE_OSC_DIV8)) { > + tctl_val = V2_TCTL_CLK_OSC_DIV8 | V2_TCTL_FRR | > + V2_TCTL_WAITEN | MXC_TCTL_TEN; > + if (cpu_is_imx6dl() || cpu_is_imx6sx()) { > + /* 24 / 8 = 3 MHz */ > + tprer_val = 7 << V2_TPRER_PRE24M; > + __raw_writel(tprer_val, timer_base + MXC_TPRER); > + tctl_val |= V2_TCTL_24MEN; > + } > + } else { > + tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | > + V2_TCTL_WAITEN | MXC_TCTL_TEN; > + } > + } else { > tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN; > + } > > __raw_writel(tctl_val, timer_base + MXC_TCTL); > > @@ -339,7 +361,7 @@ void __init mxc_timer_init(void __iomem *base, int irq) > > static void __init mxc_timer_init_dt(struct device_node *np) > { > - struct clk *clk_per, *clk_ipg; > + struct clk *clk_per, *clk_ipg, *clk_osc_per; > int irq; > > if (timer_base) > @@ -352,6 +374,13 @@ static void __init mxc_timer_init_dt(struct device_node *np) > clk_per = of_clk_get_by_name(np, "per"); > clk_ipg = of_clk_get_by_name(np, "ipg"); > > + if ((cpu_is_imx6q() && imx_get_soc_revision() > > + IMX_CHIP_REVISION_1_0) || cpu_is_imx6dl()) { > + clk_osc_per = of_clk_get_by_name(np, "osc_per"); > + if (!IS_ERR(clk_osc_per)) > + clk_per = clk_osc_per; > + } > + > _mxc_timer_init(irq, clk_per, clk_ipg); > } > CLOCKSOURCE_OF_DECLARE(mx1_timer, "fsl,imx1-gpt", mxc_timer_init_dt); > -- > 1.7.9.5 >
Best regards! Anson Huang -----Original Message----- From: Shawn Guo [mailto:shawn.guo@linaro.org] Sent: 2014-09-10 3:37 PM To: Huang Yongcai-B20788 Cc: festevam@gmail.com; kernel@pengutronix.de; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer On Fri, Sep 05, 2014 at 11:26:46AM +0800, Anson Huang wrote: > diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c index > bf92e5a..a3ecb4a 100644 > --- a/arch/arm/mach-imx/time.c > +++ b/arch/arm/mach-imx/time.c > @@ -60,17 +60,23 @@ > #define MX2_TSTAT_CAPT (1 << 1) > #define MX2_TSTAT_COMP (1 << 0) > > -/* MX31, MX35, MX25, MX5 */ > +/* MX31, MX35, MX25, MX5, MX6 */ > #define V2_TCTL_WAITEN (1 << 3) /* Wait enable mode */ > #define V2_TCTL_CLK_IPG (1 << 6) > #define V2_TCTL_CLK_PER (2 << 6) > +#define V2_TCTL_CLK_OSC_DIV8 (5 << 6) > +#define V2_TCTL_CLK_OSC (7 << 6) This one is unused? [Anson] Yes, will remove it in V3. > #define V2_TCTL_FRR (1 << 9) > +#define V2_TCTL_24MEN (1 << 10) > +#define V2_TPRER_PRE24M 12 > #define V2_IR 0x0c > #define V2_TSTAT 0x08 > #define V2_TSTAT_OF1 (1 << 0) > #define V2_TCN 0x24 > #define V2_TCMP 0x10 > > +#define V2_TIMER_RATE_OSC_DIV8 3000000 A space instead of tab should be used right after '#define'. [Anson] Accepted. > + > #define timer_is_v1() (cpu_is_mx1() || cpu_is_mx21() || cpu_is_mx27()) > #define timer_is_v2() (!timer_is_v1()) > > @@ -293,7 +299,7 @@ static int __init mxc_clockevent_init(struct clk > *timer_clk) static void __init _mxc_timer_init(int irq, > struct clk *clk_per, struct clk *clk_ipg) { > - uint32_t tctl_val; > + uint32_t tctl_val, tprer_val; The variable tprer_val does not seem really needed. [Anson] Will remove it and write the value directly to the register. Shawn > > if (IS_ERR(clk_per)) { > pr_err("i.MX timer: unable to get clk\n"); > @@ -312,10 +318,26 @@ static void __init _mxc_timer_init(int irq, > __raw_writel(0, timer_base + MXC_TCTL); > __raw_writel(0, timer_base + MXC_TPRER); /* see datasheet note */ > > - if (timer_is_v2()) > - tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN; > - else > + if (timer_is_v2()) { > + if (((cpu_is_imx6q() && imx_get_soc_revision() > > + IMX_CHIP_REVISION_1_0) || cpu_is_imx6dl() || > + cpu_is_imx6sx()) && (clk_get_rate(clk_per) == > + V2_TIMER_RATE_OSC_DIV8)) { > + tctl_val = V2_TCTL_CLK_OSC_DIV8 | V2_TCTL_FRR | > + V2_TCTL_WAITEN | MXC_TCTL_TEN; > + if (cpu_is_imx6dl() || cpu_is_imx6sx()) { > + /* 24 / 8 = 3 MHz */ > + tprer_val = 7 << V2_TPRER_PRE24M; > + __raw_writel(tprer_val, timer_base + MXC_TPRER); > + tctl_val |= V2_TCTL_24MEN; > + } > + } else { > + tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | > + V2_TCTL_WAITEN | MXC_TCTL_TEN; > + } > + } else { > tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN; > + } > > __raw_writel(tctl_val, timer_base + MXC_TCTL); > > @@ -339,7 +361,7 @@ void __init mxc_timer_init(void __iomem *base, int irq) > > static void __init mxc_timer_init_dt(struct device_node *np) > { > - struct clk *clk_per, *clk_ipg; > + struct clk *clk_per, *clk_ipg, *clk_osc_per; > int irq; > > if (timer_base) > @@ -352,6 +374,13 @@ static void __init mxc_timer_init_dt(struct device_node *np) > clk_per = of_clk_get_by_name(np, "per"); > clk_ipg = of_clk_get_by_name(np, "ipg"); > > + if ((cpu_is_imx6q() && imx_get_soc_revision() > > + IMX_CHIP_REVISION_1_0) || cpu_is_imx6dl()) { > + clk_osc_per = of_clk_get_by_name(np, "osc_per"); > + if (!IS_ERR(clk_osc_per)) > + clk_per = clk_osc_per; > + } > + > _mxc_timer_init(irq, clk_per, clk_ipg); > } > CLOCKSOURCE_OF_DECLARE(mx1_timer, "fsl,imx1-gpt", mxc_timer_init_dt); > -- > 1.7.9.5 >
diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c index bf92e5a..a3ecb4a 100644 --- a/arch/arm/mach-imx/time.c +++ b/arch/arm/mach-imx/time.c @@ -60,17 +60,23 @@ #define MX2_TSTAT_CAPT (1 << 1) #define MX2_TSTAT_COMP (1 << 0) -/* MX31, MX35, MX25, MX5 */ +/* MX31, MX35, MX25, MX5, MX6 */ #define V2_TCTL_WAITEN (1 << 3) /* Wait enable mode */ #define V2_TCTL_CLK_IPG (1 << 6) #define V2_TCTL_CLK_PER (2 << 6) +#define V2_TCTL_CLK_OSC_DIV8 (5 << 6) +#define V2_TCTL_CLK_OSC (7 << 6) #define V2_TCTL_FRR (1 << 9) +#define V2_TCTL_24MEN (1 << 10) +#define V2_TPRER_PRE24M 12 #define V2_IR 0x0c #define V2_TSTAT 0x08 #define V2_TSTAT_OF1 (1 << 0) #define V2_TCN 0x24 #define V2_TCMP 0x10 +#define V2_TIMER_RATE_OSC_DIV8 3000000 + #define timer_is_v1() (cpu_is_mx1() || cpu_is_mx21() || cpu_is_mx27()) #define timer_is_v2() (!timer_is_v1()) @@ -293,7 +299,7 @@ static int __init mxc_clockevent_init(struct clk *timer_clk) static void __init _mxc_timer_init(int irq, struct clk *clk_per, struct clk *clk_ipg) { - uint32_t tctl_val; + uint32_t tctl_val, tprer_val; if (IS_ERR(clk_per)) { pr_err("i.MX timer: unable to get clk\n"); @@ -312,10 +318,26 @@ static void __init _mxc_timer_init(int irq, __raw_writel(0, timer_base + MXC_TCTL); __raw_writel(0, timer_base + MXC_TPRER); /* see datasheet note */ - if (timer_is_v2()) - tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN; - else + if (timer_is_v2()) { + if (((cpu_is_imx6q() && imx_get_soc_revision() > + IMX_CHIP_REVISION_1_0) || cpu_is_imx6dl() || + cpu_is_imx6sx()) && (clk_get_rate(clk_per) == + V2_TIMER_RATE_OSC_DIV8)) { + tctl_val = V2_TCTL_CLK_OSC_DIV8 | V2_TCTL_FRR | + V2_TCTL_WAITEN | MXC_TCTL_TEN; + if (cpu_is_imx6dl() || cpu_is_imx6sx()) { + /* 24 / 8 = 3 MHz */ + tprer_val = 7 << V2_TPRER_PRE24M; + __raw_writel(tprer_val, timer_base + MXC_TPRER); + tctl_val |= V2_TCTL_24MEN; + } + } else { + tctl_val = V2_TCTL_CLK_PER | V2_TCTL_FRR | + V2_TCTL_WAITEN | MXC_TCTL_TEN; + } + } else { tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN; + } __raw_writel(tctl_val, timer_base + MXC_TCTL); @@ -339,7 +361,7 @@ void __init mxc_timer_init(void __iomem *base, int irq) static void __init mxc_timer_init_dt(struct device_node *np) { - struct clk *clk_per, *clk_ipg; + struct clk *clk_per, *clk_ipg, *clk_osc_per; int irq; if (timer_base) @@ -352,6 +374,13 @@ static void __init mxc_timer_init_dt(struct device_node *np) clk_per = of_clk_get_by_name(np, "per"); clk_ipg = of_clk_get_by_name(np, "ipg"); + if ((cpu_is_imx6q() && imx_get_soc_revision() > + IMX_CHIP_REVISION_1_0) || cpu_is_imx6dl()) { + clk_osc_per = of_clk_get_by_name(np, "osc_per"); + if (!IS_ERR(clk_osc_per)) + clk_per = clk_osc_per; + } + _mxc_timer_init(irq, clk_per, clk_ipg); } CLOCKSOURCE_OF_DECLARE(mx1_timer, "fsl,imx1-gpt", mxc_timer_init_dt);
On i.MX6Q TO > 1.0, i.MX6DL and i.MX6SX, gpt per clock can be from OSC instead of ipg_per, as ipg_per's rate may be scaled when system enter low bus mode, to keep system timer NOT drift, better to make gpt per clock at fixed rate, here add support for gpt per clock to be from OSC which is at fixed rate always. There are some difference on this implementation of gpt per clock source, see below for details: i.MX6Q TO > 1.0: GPT_CR_CLKSRC, 3b'101 selects fix clock of OSC / 8 for gpt per clk; i.MX6DL and i.MX6SX: GPT_CR_CLKSRC, 3b'101 selects OSC for gpt per clk, and we must enable GPT_CR_24MEM to enable OSC clk source for gpt per, GPT_PR_PRESCALER24M is for pre-scaling of this OSC clk, here set it to 8 to make gpt per clk is 3MHz; i.MX6SL: ipg_per can be from OSC directly, so no need to implement this new clk source for gpt per. Signed-off-by: Anson Huang <b20788@freescale.com> --- change logs v1 -> v2: add gpt per clk rate check before setting gpt's clk source, this is to cover all i.MX6 SoCs and make it work with old dtb. arch/arm/mach-imx/time.c | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-)