linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/3] move gpt per clk parent from ipg_per to OSC
@ 2014-09-05  3:26 Anson Huang
  2014-09-05  3:26 ` [PATCH V2 1/3] ARM: imx: add gpt_3m clk for i.mx6qdl Anson Huang
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Anson Huang @ 2014-09-05  3:26 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, gpt timer's clock is from ipg_per, and ipg_per clock is from ipg
on most of i.MX6 series SOCs, but ipg's rate may be scaled when system enters
low bus mode for saving power, then gpt timer's clock rate will be scaled as
well, as system timer should be kept stable and NOT drift, better to keep gpt
timer's clk at fixed rate, on i.MX6Q TO > 1.0, i.MX6DL and i.MX6SX, there is
OSC clk available for gpt timer, we should enable this feature, the hardware
design is as below:

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.

As we still need to make it work on i.MX6Q TO1.0 which has no OSC clock source
for gpt per, so we add a soc_per clock for i.MX6Q, other SoCs all have OSC
clock source available for gpt per, so we can just replace the original gpt
per clk with gpt_3m clock.

Anson Huang (3):
  ARM: imx: add gpt_3m clk for i.mx6qdl
  ARM: dts: imx6: make gpt per clock can be from OSC
  ARM: imx: source gpt per clk from OSC for system timer

 arch/arm/boot/dts/imx6qdl.dtsi            |    5 ++--
 arch/arm/boot/dts/imx6sx.dtsi             |    2 +-
 arch/arm/mach-imx/clk-imx6q.c             |    1 +
 arch/arm/mach-imx/time.c                  |   41 ++++++++++++++++++++++++-----
 include/dt-bindings/clock/imx6qdl-clock.h |    3 ++-
 5 files changed, 42 insertions(+), 10 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 1/3] ARM: imx: add gpt_3m clk for i.mx6qdl
  2014-09-05  3:26 [PATCH V2 0/3] move gpt per clk parent from ipg_per to OSC Anson Huang
@ 2014-09-05  3:26 ` Anson Huang
  2014-09-05  3:26 ` [PATCH V2 2/3] ARM: dts: imx6: make gpt per clock can be from OSC Anson Huang
  2014-09-05  3:26 ` [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer Anson Huang
  2 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2014-09-05  3:26 UTC (permalink / raw)
  To: linux-arm-kernel

Add gpt_3m clock for i.mx6qdl, as gpt can source clock
from OSC, some i.MX6 series SOCs has fixed divider of
8 for gpt clock, so here add a fix clk of gpt_3m.

Signed-off-by: Anson Huang <b20788@freescale.com>
---
 arch/arm/mach-imx/clk-imx6q.c             |    1 +
 include/dt-bindings/clock/imx6qdl-clock.h |    3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 2edcebf..7e6b3dd 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -194,6 +194,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[IMX6QDL_CLK_PLL3_80M]  = imx_clk_fixed_factor("pll3_80m",  "pll3_usb_otg",   1, 6);
 	clk[IMX6QDL_CLK_PLL3_60M]  = imx_clk_fixed_factor("pll3_60m",  "pll3_usb_otg",   1, 8);
 	clk[IMX6QDL_CLK_TWD]       = imx_clk_fixed_factor("twd",       "arm",            1, 2);
+	clk[IMX6QDL_CLK_GPT_3M]    = imx_clk_fixed_factor("gpt_3m",    "osc",            1, 8);
 	if (cpu_is_imx6dl()) {
 		clk[IMX6QDL_CLK_GPU2D_AXI] = imx_clk_fixed_factor("gpu2d_axi", "mmdc_ch0_axi_podf", 1, 1);
 		clk[IMX6QDL_CLK_GPU3D_AXI] = imx_clk_fixed_factor("gpu3d_axi", "mmdc_ch0_axi_podf", 1, 1);
diff --git a/include/dt-bindings/clock/imx6qdl-clock.h b/include/dt-bindings/clock/imx6qdl-clock.h
index 323e865..9bc2e07 100644
--- a/include/dt-bindings/clock/imx6qdl-clock.h
+++ b/include/dt-bindings/clock/imx6qdl-clock.h
@@ -220,6 +220,7 @@
 #define IMX6QDL_CLK_LVDS2_GATE			207
 #define IMX6QDL_CLK_ESAI_IPG			208
 #define IMX6QDL_CLK_ESAI_MEM			209
-#define IMX6QDL_CLK_END				210
+#define IMX6QDL_CLK_GPT_3M			210
+#define IMX6QDL_CLK_END				211
 
 #endif /* __DT_BINDINGS_CLOCK_IMX6QDL_H */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 2/3] ARM: dts: imx6: make gpt per clock can be from OSC
  2014-09-05  3:26 [PATCH V2 0/3] move gpt per clk parent from ipg_per to OSC Anson Huang
  2014-09-05  3:26 ` [PATCH V2 1/3] ARM: imx: add gpt_3m clk for i.mx6qdl Anson Huang
@ 2014-09-05  3:26 ` Anson Huang
  2014-09-05  3:26 ` [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer Anson Huang
  2 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2014-09-05  3:26 UTC (permalink / raw)
  To: linux-arm-kernel

Original gpt per clk parent is from ipg_per clk which
may be scaled when system enter low bus mode, as ipg
clk will be lower in low bus mode, to keep system clk
NOT drift, select gpt per clk parent from OSC which
is at fixed freq always.

On i.mx6qdl, add a osc_per clk source for i.mx6q
TO > 1.0 and all i.MX6dl SoC.

On i.mx6sx, just make gpt per clk from OSC.

Signed-off-by: Anson Huang <b20788@freescale.com>
---
change log from v1 -> v2
	on i.mx6q TO1.0, there is no OSC clk source for
	gpt per, so I have to add a new clk source of
	osc_per for gpt to cover i.mx6q TO1.0.

 arch/arm/boot/dts/imx6qdl.dtsi |    5 +++--
 arch/arm/boot/dts/imx6sx.dtsi  |    2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 70d7207..8de2801 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -401,8 +401,9 @@
 				reg = <0x02098000 0x4000>;
 				interrupts = <0 55 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_GPT_IPG>,
-					 <&clks IMX6QDL_CLK_GPT_IPG_PER>;
-				clock-names = "ipg", "per";
+					 <&clks IMX6QDL_CLK_GPT_IPG_PER>,
+					 <&clks IMX6QDL_CLK_GPT_3M>;
+				clock-names = "ipg", "per", "osc_per";
 			};
 
 			gpio1: gpio at 0209c000 {
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 63d9d82..b5d8252 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -421,7 +421,7 @@
 				reg = <0x02098000 0x4000>;
 				interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_GPT_BUS>,
-					 <&clks IMX6SX_CLK_GPT_SERIAL>;
+					 <&clks IMX6SX_CLK_GPT_3M>;
 				clock-names = "ipg", "per";
 			};
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
  2014-09-05  3:26 [PATCH V2 0/3] move gpt per clk parent from ipg_per to OSC Anson Huang
  2014-09-05  3:26 ` [PATCH V2 1/3] ARM: imx: add gpt_3m clk for i.mx6qdl Anson Huang
  2014-09-05  3:26 ` [PATCH V2 2/3] ARM: dts: imx6: make gpt per clock can be from OSC Anson Huang
@ 2014-09-05  3:26 ` Anson Huang
  2014-09-05 12:09   ` Fabio Estevam
  2014-09-10  7:36   ` Shawn Guo
  2 siblings, 2 replies; 12+ messages in thread
From: Anson Huang @ 2014-09-05  3:26 UTC (permalink / raw)
  To: linux-arm-kernel

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(-)

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);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
  2014-09-05  3:26 ` [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer Anson Huang
@ 2014-09-05 12:09   ` Fabio Estevam
  2014-09-05 12:58     ` Anson.Huang at freescale.com
  2014-09-10  7:36   ` Shawn Guo
  1 sibling, 1 reply; 12+ messages in thread
From: Fabio Estevam @ 2014-09-05 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

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?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
  2014-09-05 12:09   ` Fabio Estevam
@ 2014-09-05 12:58     ` Anson.Huang at freescale.com
  2014-09-10  7:33       ` Shawn Guo
  0 siblings, 1 reply; 12+ messages in thread
From: Anson.Huang at freescale.com @ 2014-09-05 12:58 UTC (permalink / raw)
  To: linux-arm-kernel



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?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
  2014-09-05 12:58     ` Anson.Huang at freescale.com
@ 2014-09-10  7:33       ` Shawn Guo
  2014-09-10  7:43         ` Anson.Huang at freescale.com
  0 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2014-09-10  7:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 05, 2014 at 12:58:28PM +0000, Anson.Huang at freescale.com wrote:
> >> @@ -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?

[The lines should be wrapped around 70 columns]

I'm also a bit concerned by the readability of the code.  Can we
reasonably assume it must be V2_TCTL_CLK_OSC_DIV8 case if
clk_get_rate(clk_per) returns 3000000?  In that case, the code can be
simplified a bit, something like below.  Is it going to work?

Shawn

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 4ee6e77a0fdf..3f0401e27b38 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -245,6 +245,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
        clk[IMX6QDL_CLK_PLL3_80M]  = imx_clk_fixed_factor("pll3_80m",  "pll3_usb_otg",   1, 6);
        clk[IMX6QDL_CLK_PLL3_60M]  = imx_clk_fixed_factor("pll3_60m",  "pll3_usb_otg",   1, 8);
        clk[IMX6QDL_CLK_TWD]       = imx_clk_fixed_factor("twd",       "arm",            1, 2);
+       clk[IMX6QDL_CLK_GPT_3M]    = imx_clk_fixed_factor("gpt_3m",    "osc",            1, 8);
        if (cpu_is_imx6dl()) {
                clk[IMX6QDL_CLK_GPU2D_AXI] = imx_clk_fixed_factor("gpu2d_axi", "mmdc_ch0_axi_podf", 1, 1);
                clk[IMX6QDL_CLK_GPU3D_AXI] = imx_clk_fixed_factor("gpu3d_axi", "mmdc_ch0_axi_podf", 1, 1);
@@ -469,6 +470,13 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)

        clk_register_clkdev(clk[IMX6QDL_CLK_ENET_REF], "enet_ref", NULL);

+       /*
+        * The gpt_3m clock is not available on i.MX6Q TO1.0.  Let's point it
+        * to clock gpt_ipg_per to ease the gpt driver code.
+        */
+       if (cpu_is_imx6q() && imx_get_soc_revision() == IMX_CHIP_REVISION_1_0)
+               clk[IMX6QDL_CLK_GPT_3M] = clk[IMX6QDL_CLK_GPT_IPG_PER];
+
        if ((imx_get_soc_revision() != IMX_CHIP_REVISION_1_0) ||
            cpu_is_imx6dl()) {
                clk_set_parent(clk[IMX6QDL_CLK_LDB_DI0_SEL], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV]);

diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c
index bf92e5a351c0..c0ad839516b0 100644
--- a/arch/arm/mach-imx/time.c
+++ b/arch/arm/mach-imx/time.c
@@ -312,10 +317,21 @@ 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()) {
+               tctl_val = V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN;
+               if (clk_get_rate(clk_per) == V2_TIMER_RATE_OSC_DIV8) {
			/* Is the assumption corrrect ??? */
+                       tctl_val |= V2_TCTL_CLK_OSC_DIV8;
+                       if (cpu_is_imx6dl() || cpu_is_imx6sx()) {
+                               /* 24 / 8 = 3 MHz */
+                               __raw_writel(7 << V2_TPRER_PRE24M, timer_base + MXC_TPRER);
+                               tctl_val |= V2_TCTL_24MEN;
+                       }
+               } else {
+                       tctl_val |= V2_TCTL_CLK_PER;
+               }
+       } else {
                tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN;
+       }

        __raw_writel(tctl_val, timer_base + MXC_TCTL);

@@ -349,9 +365,13 @@ static void __init mxc_timer_init_dt(struct device_node *np)
        WARN_ON(!timer_base);
        irq = irq_of_parse_and_map(np, 0);

-       clk_per = of_clk_get_by_name(np, "per");
        clk_ipg = of_clk_get_by_name(np, "ipg");

+       /* Try osc_per clock first, and fall back to per clock otherwise */
+       clk_per = of_clk_get_by_name(np, "osc_per");
+       if (!IS_ERR(clk_per))
+               clk_per = of_clk_get_by_name(np, "per");
+
        _mxc_timer_init(irq, clk_per, clk_ipg);
 }
 CLOCKSOURCE_OF_DECLARE(mx1_timer, "fsl,imx1-gpt", mxc_timer_init_dt);

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
  2014-09-05  3:26 ` [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer Anson Huang
  2014-09-05 12:09   ` Fabio Estevam
@ 2014-09-10  7:36   ` Shawn Guo
  2014-09-10  7:45     ` Anson.Huang at freescale.com
  1 sibling, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2014-09-10  7:36 UTC (permalink / raw)
  To: linux-arm-kernel

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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
  2014-09-10  7:33       ` Shawn Guo
@ 2014-09-10  7:43         ` Anson.Huang at freescale.com
  2014-09-10 11:49           ` Shawn Guo
  0 siblings, 1 reply; 12+ messages in thread
From: Anson.Huang at freescale.com @ 2014-09-10  7:43 UTC (permalink / raw)
  To: linux-arm-kernel

Hi, Shawn
	Please see below response:

Best regards!
Anson Huang


-----Original Message-----
From: Shawn Guo [mailto:shawn.guo at linaro.org] 
Sent: 2014-09-10 3:34 PM
To: Huang Yongcai-B20788
Cc: Fabio Estevam; Sascha Hauer; devicetree at vger.kernel.org; linux-arm-kernel at 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 12:58:28PM +0000, Anson.Huang at freescale.com wrote:
> >> @@ -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?

[The lines should be wrapped around 70 columns]

I'm also a bit concerned by the readability of the code.  Can we reasonably assume it must be V2_TCTL_CLK_OSC_DIV8 case if
clk_get_rate(clk_per) returns 3000000?  In that case, the code can be simplified a bit, something like below.  Is it going to work?

Shawn

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index 4ee6e77a0fdf..3f0401e27b38 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -245,6 +245,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
        clk[IMX6QDL_CLK_PLL3_80M]  = imx_clk_fixed_factor("pll3_80m",  "pll3_usb_otg",   1, 6);
        clk[IMX6QDL_CLK_PLL3_60M]  = imx_clk_fixed_factor("pll3_60m",  "pll3_usb_otg",   1, 8);
        clk[IMX6QDL_CLK_TWD]       = imx_clk_fixed_factor("twd",       "arm",            1, 2);
+       clk[IMX6QDL_CLK_GPT_3M]    = imx_clk_fixed_factor("gpt_3m",    "osc",            1, 8);
        if (cpu_is_imx6dl()) {
                clk[IMX6QDL_CLK_GPU2D_AXI] = imx_clk_fixed_factor("gpu2d_axi", "mmdc_ch0_axi_podf", 1, 1);
                clk[IMX6QDL_CLK_GPU3D_AXI] = imx_clk_fixed_factor("gpu3d_axi", "mmdc_ch0_axi_podf", 1, 1); @@ -469,6 +470,13 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)

        clk_register_clkdev(clk[IMX6QDL_CLK_ENET_REF], "enet_ref", NULL);

+       /*
+        * The gpt_3m clock is not available on i.MX6Q TO1.0.  Let's point it
+        * to clock gpt_ipg_per to ease the gpt driver code.
+        */
+       if (cpu_is_imx6q() && imx_get_soc_revision() == IMX_CHIP_REVISION_1_0)
+               clk[IMX6QDL_CLK_GPT_3M] = clk[IMX6QDL_CLK_GPT_IPG_PER];
+
        if ((imx_get_soc_revision() != IMX_CHIP_REVISION_1_0) ||
            cpu_is_imx6dl()) {
                clk_set_parent(clk[IMX6QDL_CLK_LDB_DI0_SEL], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV]);

diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c index bf92e5a351c0..c0ad839516b0 100644
--- a/arch/arm/mach-imx/time.c
+++ b/arch/arm/mach-imx/time.c
@@ -312,10 +317,21 @@ 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()) {
+               tctl_val = V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN;
+               if (clk_get_rate(clk_per) == V2_TIMER_RATE_OSC_DIV8) {
			/* Is the assumption corrrect ??? */
[Anson] I am afraid that i.MX6Q TO1.0 can NOT meet it, as there is no gpt_3m available for i.MX6Q TO1.0, but we have it in dtb and clk driver. 

+                       tctl_val |= V2_TCTL_CLK_OSC_DIV8;
+                       if (cpu_is_imx6dl() || cpu_is_imx6sx()) {
+                               /* 24 / 8 = 3 MHz */
+                               __raw_writel(7 << V2_TPRER_PRE24M, timer_base + MXC_TPRER);
+                               tctl_val |= V2_TCTL_24MEN;
+                       }
+               } else {
+                       tctl_val |= V2_TCTL_CLK_PER;
+               }
+       } else {
                tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN;
+       }

        __raw_writel(tctl_val, timer_base + MXC_TCTL);

@@ -349,9 +365,13 @@ static void __init mxc_timer_init_dt(struct device_node *np)
        WARN_ON(!timer_base);
        irq = irq_of_parse_and_map(np, 0);

-       clk_per = of_clk_get_by_name(np, "per");
        clk_ipg = of_clk_get_by_name(np, "ipg");

+       /* Try osc_per clock first, and fall back to per clock otherwise */
+       clk_per = of_clk_get_by_name(np, "osc_per");
+       if (!IS_ERR(clk_per))
+               clk_per = of_clk_get_by_name(np, "per");
+
[Anson] For i.MX6Q TO1.0, there is no GPT_3M clock for GPT, but in dtb, there is osc_per, so clk_per will be initialized by osc_per, and the clk rate read in _mxc_timer_init function will be 3000000 and go to the GPT_3M clk source path, which will NOT work for i.MX6Q TO1.0.

        _mxc_timer_init(irq, clk_per, clk_ipg);  }  CLOCKSOURCE_OF_DECLARE(mx1_timer, "fsl,imx1-gpt", mxc_timer_init_dt);

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
  2014-09-10  7:36   ` Shawn Guo
@ 2014-09-10  7:45     ` Anson.Huang at freescale.com
  0 siblings, 0 replies; 12+ messages in thread
From: Anson.Huang at freescale.com @ 2014-09-10  7:45 UTC (permalink / raw)
  To: linux-arm-kernel



Best regards!
Anson Huang


-----Original Message-----
From: Shawn Guo [mailto:shawn.guo at linaro.org] 
Sent: 2014-09-10 3:37 PM
To: Huang Yongcai-B20788
Cc: festevam at gmail.com; kernel at pengutronix.de; devicetree at vger.kernel.org; linux-arm-kernel at 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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
  2014-09-10  7:43         ` Anson.Huang at freescale.com
@ 2014-09-10 11:49           ` Shawn Guo
       [not found]             ` <8DDD46F6-AA96-4E14-ADD6-45A1AD12423C@freescale.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2014-09-10 11:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 10, 2014 at 07:43:41AM +0000, Anson.Huang at freescale.com wrote:
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index 4ee6e77a0fdf..3f0401e27b38 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -245,6 +245,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
>         clk[IMX6QDL_CLK_PLL3_80M]  = imx_clk_fixed_factor("pll3_80m",  "pll3_usb_otg",   1, 6);
>         clk[IMX6QDL_CLK_PLL3_60M]  = imx_clk_fixed_factor("pll3_60m",  "pll3_usb_otg",   1, 8);
>         clk[IMX6QDL_CLK_TWD]       = imx_clk_fixed_factor("twd",       "arm",            1, 2);
> +       clk[IMX6QDL_CLK_GPT_3M]    = imx_clk_fixed_factor("gpt_3m",    "osc",            1, 8);
>         if (cpu_is_imx6dl()) {
>                 clk[IMX6QDL_CLK_GPU2D_AXI] = imx_clk_fixed_factor("gpu2d_axi", "mmdc_ch0_axi_podf", 1, 1);
>                 clk[IMX6QDL_CLK_GPU3D_AXI] = imx_clk_fixed_factor("gpu3d_axi", "mmdc_ch0_axi_podf", 1, 1); @@ -469,6 +470,13 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
> 
>         clk_register_clkdev(clk[IMX6QDL_CLK_ENET_REF], "enet_ref", NULL);
> 
> +       /*
> +        * The gpt_3m clock is not available on i.MX6Q TO1.0.  Let's point it
> +        * to clock gpt_ipg_per to ease the gpt driver code.
> +        */
> +       if (cpu_is_imx6q() && imx_get_soc_revision() == IMX_CHIP_REVISION_1_0)
> +               clk[IMX6QDL_CLK_GPT_3M] = clk[IMX6QDL_CLK_GPT_IPG_PER];
> +
>         if ((imx_get_soc_revision() != IMX_CHIP_REVISION_1_0) ||
>             cpu_is_imx6dl()) {
>                 clk_set_parent(clk[IMX6QDL_CLK_LDB_DI0_SEL], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV]);
> 
> diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c index bf92e5a351c0..c0ad839516b0 100644
> --- a/arch/arm/mach-imx/time.c
> +++ b/arch/arm/mach-imx/time.c
> @@ -312,10 +317,21 @@ 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()) {
> +               tctl_val = V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN;
> +               if (clk_get_rate(clk_per) == V2_TIMER_RATE_OSC_DIV8) {
> 			/* Is the assumption corrrect ??? */
> [Anson] I am afraid that i.MX6Q TO1.0 can NOT meet it, as there is no gpt_3m available for i.MX6Q TO1.0, but we have it in dtb and clk driver. 
> 
> +                       tctl_val |= V2_TCTL_CLK_OSC_DIV8;
> +                       if (cpu_is_imx6dl() || cpu_is_imx6sx()) {
> +                               /* 24 / 8 = 3 MHz */
> +                               __raw_writel(7 << V2_TPRER_PRE24M, timer_base + MXC_TPRER);
> +                               tctl_val |= V2_TCTL_24MEN;
> +                       }
> +               } else {
> +                       tctl_val |= V2_TCTL_CLK_PER;
> +               }
> +       } else {
>                 tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN;
> +       }
> 
>         __raw_writel(tctl_val, timer_base + MXC_TCTL);
> 
> @@ -349,9 +365,13 @@ static void __init mxc_timer_init_dt(struct device_node *np)
>         WARN_ON(!timer_base);
>         irq = irq_of_parse_and_map(np, 0);
> 
> -       clk_per = of_clk_get_by_name(np, "per");
>         clk_ipg = of_clk_get_by_name(np, "ipg");
> 
> +       /* Try osc_per clock first, and fall back to per clock otherwise */
> +       clk_per = of_clk_get_by_name(np, "osc_per");
> +       if (!IS_ERR(clk_per))
> +               clk_per = of_clk_get_by_name(np, "per");
> +
> [Anson] For i.MX6Q TO1.0, there is no GPT_3M clock for GPT, but in dtb, there is osc_per, so clk_per will be initialized by osc_per, and the clk rate read in _mxc_timer_init function will be 3000000 and go to the GPT_3M clk source path, which will NOT work for i.MX6Q TO1.0.
> 

I had the following change for clk-imx6q.c as above.  Shouldn't it
handle the i.MX6Q TO1.0 case well?

	if (cpu_is_imx6q() && imx_get_soc_revision() == IMX_CHIP_REVISION_1_0)
		clk[IMX6QDL_CLK_GPT_3M] = clk[IMX6QDL_CLK_GPT_IPG_PER];

Shawn

>         _mxc_timer_init(irq, clk_per, clk_ipg);  }  CLOCKSOURCE_OF_DECLARE(mx1_timer, "fsl,imx1-gpt", mxc_timer_init_dt);
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer
       [not found]             ` <8DDD46F6-AA96-4E14-ADD6-45A1AD12423C@freescale.com>
@ 2014-09-10 13:01               ` Shawn Guo
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2014-09-10 13:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 10, 2014 at 12:31:48PM +0000, Anson.Huang at freescale.com wrote:
> I think it should be OK now, but for the "try osc_per clk first ..." the check
> should be as below?
> 
> If (IS_ERR(clk_per))
>       clk_per = of_clk_get_by_name(np, "per");
> 

Ah, yes.  That was a mistake in my example code.

Shawn

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-09-10 13:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-05  3:26 [PATCH V2 0/3] move gpt per clk parent from ipg_per to OSC Anson Huang
2014-09-05  3:26 ` [PATCH V2 1/3] ARM: imx: add gpt_3m clk for i.mx6qdl Anson Huang
2014-09-05  3:26 ` [PATCH V2 2/3] ARM: dts: imx6: make gpt per clock can be from OSC Anson Huang
2014-09-05  3:26 ` [PATCH V2 3/3] ARM: imx: source gpt per clk from OSC for system timer Anson Huang
2014-09-05 12:09   ` Fabio Estevam
2014-09-05 12:58     ` Anson.Huang at freescale.com
2014-09-10  7:33       ` Shawn Guo
2014-09-10  7:43         ` Anson.Huang at freescale.com
2014-09-10 11:49           ` Shawn Guo
     [not found]             ` <8DDD46F6-AA96-4E14-ADD6-45A1AD12423C@freescale.com>
2014-09-10 13:01               ` Shawn Guo
2014-09-10  7:36   ` Shawn Guo
2014-09-10  7:45     ` Anson.Huang at freescale.com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).