[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT

[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT

[Ye.Li]

Introduce a new configuration "CONFIG_MXC_GPT_HCLK". When it is set,
the GPT will use 24Mhz OSC as clock source. Otherwise, the GPT will
use 32Khz OSC as clock source.

Since only the GPT on iMX6 series provide the clock source option for
24Mhz OSC. For other series(MX5), if the configuration is set, the
perclk will be selected as clock source.

MX6Q/D Rev 1.0 and MX6SL can't use the 24Mhz OSC clock source option,
so select the perclk for them. For MX6SL, we will set the OSC 24Mhz to
perclk in CCM, so eventually the clock comes from OSC 24Mhz.

Signed-off-by: Ye.Li <B37916@freescale.com>
---
 arch/arm/imx-common/timer.c |   76 +++++++++++++++++++++++++++++++++++++------
 1 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
index c63f78f..f7e49bd 100644
--- a/arch/arm/imx-common/timer.c
+++ b/arch/arm/imx-common/timer.c
@@ -2,7 +2,7 @@
  * (C) Copyright 2007
  * Sascha Hauer, Pengutronix
  *
- * (C) Copyright 2009 Freescale Semiconductor, Inc.
+ * (C) Copyright 2009-2014 Freescale Semiconductor, Inc.
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
@@ -12,6 +12,7 @@
 #include <div64.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
+#include <asm/arch/sys_proto.h>
 
 /* General purpose timers registers */
 struct mxc_gpt {
@@ -26,23 +27,60 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR;
 
 /* General purpose timers bitfields */
 #define GPTCR_SWR		(1 << 15)	/* Software reset */
+#define GPTCR_24MEN	    (1 << 10)	/* Enable 24MHz clock input from crystal */
 #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
-#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
+#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source 32khz */
+#define GPTCR_CLKSOURCE_OSC	(5 << 6)	/* Clock source OSC */
+#define GPTCR_CLKSOURCE_PRE	(1 << 6)	/* Clock source PRECLK */
+#define GPTCR_CLKSOURCE_MASK (0x7 << 6)
 #define GPTCR_TEN		1		/* Timer enable */
 
+#define GPTPR_PRESCALER24M_SHIFT 12
+#define GPTPR_PRESCALER24M_MASK (0xF << GPTPR_PRESCALER24M_SHIFT)
+
 DECLARE_GLOBAL_DATA_PTR;
 
+static inline int gpt_has_clk_source_osc(void)
+{
+#if defined(CONFIG_MX6)
+	if (((is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
+		&& (is_soc_rev(CHIP_REV_1_0) > 0))
+		|| is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
+		|| is_cpu_type(MXC_CPU_MX6SX))
+		return 1;
+
+	return 0;
+#else
+	return 0;
+#endif
+}
+
+static inline ulong gpt_get_clk(void)
+{
+#ifdef CONFIG_MXC_GPT_HCLK
+	if (gpt_has_clk_source_osc())
+		return MXC_HCLK >> 3;
+	else
+		return mxc_get_clock(MXC_IPG_PERCLK);
+#else
+	return MXC_CLK32;
+#endif
+}
 static inline unsigned long long tick_to_time(unsigned long long tick)
 {
+	ulong gpt_clk = gpt_get_clk();
+
 	tick *= CONFIG_SYS_HZ;
-	do_div(tick, MXC_CLK32);
+	do_div(tick, gpt_clk);
 
 	return tick;
 }
 
 static inline unsigned long long us_to_tick(unsigned long long usec)
 {
-	usec = usec * MXC_CLK32 + 999999;
+	ulong gpt_clk = gpt_get_clk();
+
+	usec = usec * gpt_clk + 999999;
 	do_div(usec, 1000000);
 
 	return usec;
@@ -59,11 +97,29 @@ int timer_init(void)
 	for (i = 0; i < 100; i++)
 		__raw_writel(0, &cur_gpt->control);
 
-	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
-
-	/* Freerun Mode, PERCLK1 input */
 	i = __raw_readl(&cur_gpt->control);
-	__raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, &cur_gpt->control);
+	i &= ~GPTCR_CLKSOURCE_MASK;
+
+#ifdef CONFIG_MXC_GPT_HCLK
+	if (gpt_has_clk_source_osc()) {
+		i |= GPTCR_CLKSOURCE_OSC | GPTCR_TEN;
+
+		/* For DL/S, SX, they has 24M OSC Enable bit and need to set prescaler */
+		if (is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
+			|| is_cpu_type(MXC_CPU_MX6SX)) {
+			i |= GPTCR_24MEN;
+
+			/* Produce 3Mhz clock */
+			__raw_writel((7 << GPTPR_PRESCALER24M_SHIFT), &cur_gpt->prescaler);
+		}
+	} else {
+		i |= GPTCR_CLKSOURCE_PRE | GPTCR_TEN;
+	}
+#else
+	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
+	i |= GPTCR_CLKSOURCE_32 | GPTCR_TEN;
+#endif
+	__raw_writel(i, &cur_gpt->control);
 
 	gd->arch.tbl = __raw_readl(&cur_gpt->counter);
 	gd->arch.tbu = 0;
@@ -86,7 +142,7 @@ ulong get_timer_masked(void)
 {
 	/*
 	 * get_ticks() returns a long long (64 bit), it wraps in
-	 * 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
+	 * 2^64 / GPT_CLK = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
 	 * 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in
 	 * 5 * 10^6 days - long enough.
 	 */
@@ -117,5 +173,5 @@ void __udelay(unsigned long usec)
  */
 ulong get_tbclk(void)
 {
-	return MXC_CLK32;
+	return gpt_get_clk();
 }
-- 
1.7.4.1

[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT

[Ye.Li]

Introduce a new configuration "CONFIG_MXC_GPT_HCLK". When it is set,
the GPT will use 24Mhz OSC as clock source. Otherwise, the GPT will
use 32Khz OSC as clock source.

Since only the GPT on iMX6 series provide the clock source option for
24Mhz OSC. For other series(MX5), if the configuration is set, the
perclk will be selected as clock source.

MX6Q/D Rev 1.0 and MX6SL can't use the 24Mhz OSC clock source option,
so select the perclk for them. For MX6SL, we will set the OSC 24Mhz to
perclk in CCM, so eventually the clock comes from OSC 24Mhz.

Signed-off-by: Ye.Li <B37916@freescale.com>
---
 arch/arm/imx-common/timer.c |   76 +++++++++++++++++++++++++++++++++++++------
 1 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
index c63f78f..f7e49bd 100644
--- a/arch/arm/imx-common/timer.c
+++ b/arch/arm/imx-common/timer.c
@@ -2,7 +2,7 @@
  * (C) Copyright 2007
  * Sascha Hauer, Pengutronix
  *
- * (C) Copyright 2009 Freescale Semiconductor, Inc.
+ * (C) Copyright 2009-2014 Freescale Semiconductor, Inc.
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
@@ -12,6 +12,7 @@
 #include <div64.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
+#include <asm/arch/sys_proto.h>
 
 /* General purpose timers registers */
 struct mxc_gpt {
@@ -26,23 +27,60 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR;
 
 /* General purpose timers bitfields */
 #define GPTCR_SWR		(1 << 15)	/* Software reset */
+#define GPTCR_24MEN	    (1 << 10)	/* Enable 24MHz clock input from crystal */
 #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
-#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
+#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source 32khz */
+#define GPTCR_CLKSOURCE_OSC	(5 << 6)	/* Clock source OSC */
+#define GPTCR_CLKSOURCE_PRE	(1 << 6)	/* Clock source PRECLK */
+#define GPTCR_CLKSOURCE_MASK (0x7 << 6)
 #define GPTCR_TEN		1		/* Timer enable */
 
+#define GPTPR_PRESCALER24M_SHIFT 12
+#define GPTPR_PRESCALER24M_MASK (0xF << GPTPR_PRESCALER24M_SHIFT)
+
 DECLARE_GLOBAL_DATA_PTR;
 
+static inline int gpt_has_clk_source_osc(void)
+{
+#if defined(CONFIG_MX6)
+	if (((is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
+		&& (is_soc_rev(CHIP_REV_1_0) > 0))
+		|| is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
+		|| is_cpu_type(MXC_CPU_MX6SX))
+		return 1;
+
+	return 0;
+#else
+	return 0;
+#endif
+}
+
+static inline ulong gpt_get_clk(void)
+{
+#ifdef CONFIG_MXC_GPT_HCLK
+	if (gpt_has_clk_source_osc())
+		return MXC_HCLK >> 3;
+	else
+		return mxc_get_clock(MXC_IPG_PERCLK);
+#else
+	return MXC_CLK32;
+#endif
+}
 static inline unsigned long long tick_to_time(unsigned long long tick)
 {
+	ulong gpt_clk = gpt_get_clk();
+
 	tick *= CONFIG_SYS_HZ;
-	do_div(tick, MXC_CLK32);
+	do_div(tick, gpt_clk);
 
 	return tick;
 }
 
 static inline unsigned long long us_to_tick(unsigned long long usec)
 {
-	usec = usec * MXC_CLK32 + 999999;
+	ulong gpt_clk = gpt_get_clk();
+
+	usec = usec * gpt_clk + 999999;
 	do_div(usec, 1000000);
 
 	return usec;
@@ -59,11 +97,29 @@ int timer_init(void)
 	for (i = 0; i < 100; i++)
 		__raw_writel(0, &cur_gpt->control);
 
-	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
-
-	/* Freerun Mode, PERCLK1 input */
 	i = __raw_readl(&cur_gpt->control);
-	__raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, &cur_gpt->control);
+	i &= ~GPTCR_CLKSOURCE_MASK;
+
+#ifdef CONFIG_MXC_GPT_HCLK
+	if (gpt_has_clk_source_osc()) {
+		i |= GPTCR_CLKSOURCE_OSC | GPTCR_TEN;
+
+		/* For DL/S, SX, they has 24M OSC Enable bit and need to set prescaler */
+		if (is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
+			|| is_cpu_type(MXC_CPU_MX6SX)) {
+			i |= GPTCR_24MEN;
+
+			/* Produce 3Mhz clock */
+			__raw_writel((7 << GPTPR_PRESCALER24M_SHIFT), &cur_gpt->prescaler);
+		}
+	} else {
+		i |= GPTCR_CLKSOURCE_PRE | GPTCR_TEN;
+	}
+#else
+	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
+	i |= GPTCR_CLKSOURCE_32 | GPTCR_TEN;
+#endif
+	__raw_writel(i, &cur_gpt->control);
 
 	gd->arch.tbl = __raw_readl(&cur_gpt->counter);
 	gd->arch.tbu = 0;
@@ -86,7 +142,7 @@ ulong get_timer_masked(void)
 {
 	/*
 	 * get_ticks() returns a long long (64 bit), it wraps in
-	 * 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
+	 * 2^64 / GPT_CLK = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
 	 * 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in
 	 * 5 * 10^6 days - long enough.
 	 */
@@ -117,5 +173,5 @@ void __udelay(unsigned long usec)
  */
 ulong get_tbclk(void)
 {
-	return MXC_CLK32;
+	return gpt_get_clk();
 }
-- 
1.7.4.1

[U-Boot] [PATCH 2/5] imx: mx6sl: Add perclk_clk_sel bit define in CCM

[Ye.Li]

The MX6SL has the perclk_clk_sel to select the perclk source. Add
its define in CCM

Signed-off-by: Ye.Li <B37916@freescale.com>
---
 arch/arm/include/asm/arch-mx6/crm_regs.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/arch-mx6/crm_regs.h b/arch/arm/include/asm/arch-mx6/crm_regs.h
index e67b5b9..53ffb3d 100644
--- a/arch/arm/include/asm/arch-mx6/crm_regs.h
+++ b/arch/arm/include/asm/arch-mx6/crm_regs.h
@@ -228,6 +228,8 @@ struct mxc_ccm_reg {
 #ifdef CONFIG_MX6SX
 #define MXC_CCM_CSCMR1_QSPI1_CLK_SEL_MASK		(0x7 << 7)
 #define MXC_CCM_CSCMR1_QSPI1_CLK_SEL_OFFSET		7
+#endif
+#if (defined(CONFIG_MX6SL) || defined(CONFIG_MX6SX))
 #define MXC_CCM_CSCMR1_PER_CLK_SEL_MASK			(1 << 6)
 #define MXC_CCM_CSCMR1_PER_CLK_SEL_OFFSET		6
 #endif
-- 
1.7.4.1

[U-Boot] [PATCH 3/5] imx: mx6: Change the get_ipg_per_clk for OSC 24Mhz source

[Ye.Li]

For MX6SL and MX6SX, the perclk can come from OSC 24Mhz source. Fix
the get_ipg_per_clk function to support it.

Signed-off-by: Ye.Li <B37916@freescale.com>
---
 arch/arm/cpu/armv7/mx6/clock.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
index d200531..6c9c78c 100644
--- a/arch/arm/cpu/armv7/mx6/clock.c
+++ b/arch/arm/cpu/armv7/mx6/clock.c
@@ -312,6 +312,10 @@ static u32 get_ipg_per_clk(void)
 	u32 reg, perclk_podf;
 
 	reg = __raw_readl(&imx_ccm->cscmr1);
+#if (defined(CONFIG_MX6SL) || defined(CONFIG_MX6SX))
+	if (reg & MXC_CCM_CSCMR1_PER_CLK_SEL_MASK)
+		return MXC_HCLK; /* OSC 24Mhz */
+#endif
 	perclk_podf = reg & MXC_CCM_CSCMR1_PERCLK_PODF_MASK;
 
 	return get_ipg_clk() / (perclk_podf + 1);
-- 
1.7.4.1

[U-Boot] [PATCH 4/5] imx: mx6sl: Set the preclk clock source to OSC 24Mhz

[Ye.Li]

For MX6SL, uses the OSC 24Mhz as the preclk source in CCM. Align the
preclk setting with kernel.

Signed-off-by: Ye.Li <B37916@freescale.com>
---
 arch/arm/cpu/armv7/mx6/soc.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
index 6dc2600..c0bb431 100644
--- a/arch/arm/cpu/armv7/mx6/soc.c
+++ b/arch/arm/cpu/armv7/mx6/soc.c
@@ -239,6 +239,18 @@ static void clear_mmdc_ch_mask(void)
 	writel(0, &mxc_ccm->ccdr);
 }
 
+#ifdef CONFIG_MX6SL
+static void set_preclk_from_osc(void)
+{
+	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
+	u32 reg;
+
+	reg = readl(&mxc_ccm->cscmr1);
+	reg |= MXC_CCM_CSCMR1_PER_CLK_SEL_MASK;
+	writel(reg, &mxc_ccm->cscmr1);
+}
+#endif
+
 int arch_cpu_init(void)
 {
 	init_aips();
@@ -254,6 +266,11 @@ int arch_cpu_init(void)
 	if (mxc_get_clock(MXC_ARM_CLK) == 396000000)
 		set_ahb_rate(132000000);
 
+		/* Set perclk to source from OSC 24MHz */
+#if defined(CONFIG_MX6SL)
+	set_preclk_from_osc();
+#endif
+
 	imx_set_wdog_powerdown(false); /* Disable PDE bit of WMCR register */
 
 #ifdef CONFIG_APBH_DMA
-- 
1.7.4.1

[U-Boot] [PATCH 5/5] imx: mx6: Enable 24Mhz OSC for GPT clock source

[Ye.Li]

Set the CONFIG_MXC_GPT_HCLK configuration in mx6_common.h, so that
enable the 24Mhz OSC GPT on all MX6 platforms.

Signed-off-by: Ye.Li <B37916@freescale.com>
---
 include/configs/mx6_common.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h
index 135a3f5..e0528ce 100644
--- a/include/configs/mx6_common.h
+++ b/include/configs/mx6_common.h
@@ -29,5 +29,6 @@
 #endif
 
 #define CONFIG_MP
+#define CONFIG_MXC_GPT_HCLK
 
 #endif
-- 
1.7.4.1

[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT

[Stefano Babic]

Hi Ye,

On 24/10/2014 14:32, Ye.Li wrote:
> Introduce a new configuration "CONFIG_MXC_GPT_HCLK". When it is set,
> the GPT will use 24Mhz OSC as clock source. Otherwise, the GPT will
> use 32Khz OSC as clock source.
> 
> Since only the GPT on iMX6 series provide the clock source option for
> 24Mhz OSC. For other series(MX5), if the configuration is set, the
> perclk will be selected as clock source.

But if the source option is possible only for i.MX6, why should be
possible to set it for MX5 ? It should be hidden in the SOC
configuration and not in board configuration.

> 
> MX6Q/D Rev 1.0 and MX6SL can't use the 24Mhz OSC clock source option,
> so select the perclk for them. For MX6SL, we will set the OSC 24Mhz to
> perclk in CCM, so eventually the clock comes from OSC 24Mhz.
> 

I am trying to understand. The explanation here does not reflect in the
implementation.  From the implementation, it is possible to set it even
with wrong revision.

Anyway, I do not think it is correct to put it as a configuration
option. This makes that we need different binaries for different
revisions of the SOC. It should be checked at runtime if the revision is
correct to set this clock as source. gpt_has_clk_source_osc has a check,
but does it make sense that CONFIG_MXC_GPT_HCLK can be set in any case ?

> Signed-off-by: Ye.Li <B37916@freescale.com>
> ---

Is this a second version of the patchset ? What are the changes ? Please
add version number to your patchset and write a history about changes. I
can suggest to use Simon's patman to post your patches.

>  arch/arm/imx-common/timer.c |   76 +++++++++++++++++++++++++++++++++++++------
>  1 files changed, 66 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
> index c63f78f..f7e49bd 100644
> --- a/arch/arm/imx-common/timer.c
> +++ b/arch/arm/imx-common/timer.c
> @@ -2,7 +2,7 @@
>   * (C) Copyright 2007
>   * Sascha Hauer, Pengutronix
>   *
> - * (C) Copyright 2009 Freescale Semiconductor, Inc.
> + * (C) Copyright 2009-2014 Freescale Semiconductor, Inc.

I have already complain about this. There are a lot of commits after the
file was merged into u-boot, and a lot of them not directly by
Freescale. I do not think it is legally correct to change the Copyright.

>   *
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
> @@ -12,6 +12,7 @@
>  #include <div64.h>
>  #include <asm/arch/imx-regs.h>
>  #include <asm/arch/clock.h>
> +#include <asm/arch/sys_proto.h>
>  
>  /* General purpose timers registers */
>  struct mxc_gpt {
> @@ -26,23 +27,60 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR;
>  
>  /* General purpose timers bitfields */
>  #define GPTCR_SWR		(1 << 15)	/* Software reset */
> +#define GPTCR_24MEN	    (1 << 10)	/* Enable 24MHz clock input from crystal */
>  #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
> -#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
> +#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source 32khz */
> +#define GPTCR_CLKSOURCE_OSC	(5 << 6)	/* Clock source OSC */
> +#define GPTCR_CLKSOURCE_PRE	(1 << 6)	/* Clock source PRECLK */
> +#define GPTCR_CLKSOURCE_MASK (0x7 << 6)
>  #define GPTCR_TEN		1		/* Timer enable */
>  
> +#define GPTPR_PRESCALER24M_SHIFT 12
> +#define GPTPR_PRESCALER24M_MASK (0xF << GPTPR_PRESCALER24M_SHIFT)
> +
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +static inline int gpt_has_clk_source_osc(void)
> +{
> +#if defined(CONFIG_MX6)
> +	if (((is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
> +		&& (is_soc_rev(CHIP_REV_1_0) > 0))
> +		|| is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
> +		|| is_cpu_type(MXC_CPU_MX6SX))
> +		return 1;
> +
> +	return 0;
> +#else
> +	return 0;
> +#endif

We are generally trying to get rid of #ifdef, but it seems we are not
going in the right direction. is_cpu_type() should return false if the
CPU is not what we expect - any chance for having this function for all
i.MXes ? We can then get rid of a lot of nasty #ifdef. What about moving
this function centrally, maybe in arch/arm/include/asm/imx-common/ ?

> +}
> +
> +static inline ulong gpt_get_clk(void)
> +{
> +#ifdef CONFIG_MXC_GPT_HCLK
> +	if (gpt_has_clk_source_osc())
> +		return MXC_HCLK >> 3;
> +	else
> +		return mxc_get_clock(MXC_IPG_PERCLK);
> +#else
> +	return MXC_CLK32;
> +#endif
> +}
>  static inline unsigned long long tick_to_time(unsigned long long tick)
>  {
> +	ulong gpt_clk = gpt_get_clk();
> +
>  	tick *= CONFIG_SYS_HZ;
> -	do_div(tick, MXC_CLK32);
> +	do_div(tick, gpt_clk);
>  
>  	return tick;
>  }
>  
>  static inline unsigned long long us_to_tick(unsigned long long usec)
>  {
> -	usec = usec * MXC_CLK32 + 999999;
> +	ulong gpt_clk = gpt_get_clk();
> +
> +	usec = usec * gpt_clk + 999999;
>  	do_div(usec, 1000000);
>  
>  	return usec;
> @@ -59,11 +97,29 @@ int timer_init(void)
>  	for (i = 0; i < 100; i++)
>  		__raw_writel(0, &cur_gpt->control);
>  
> -	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
> -
> -	/* Freerun Mode, PERCLK1 input */
>  	i = __raw_readl(&cur_gpt->control);
> -	__raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, &cur_gpt->control);
> +	i &= ~GPTCR_CLKSOURCE_MASK;
> +
> +#ifdef CONFIG_MXC_GPT_HCLK
> +	if (gpt_has_clk_source_osc()) {
> +		i |= GPTCR_CLKSOURCE_OSC | GPTCR_TEN;
> +
> +		/* For DL/S, SX, they has 24M OSC Enable bit and need to set prescaler */
> +		if (is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
> +			|| is_cpu_type(MXC_CPU_MX6SX)) {
> +			i |= GPTCR_24MEN;
> +
> +			/* Produce 3Mhz clock */
> +			__raw_writel((7 << GPTPR_PRESCALER24M_SHIFT), &cur_gpt->prescaler);
> +		}
> +	} else {
> +		i |= GPTCR_CLKSOURCE_PRE | GPTCR_TEN;
> +	}
> +#else
> +	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
> +	i |= GPTCR_CLKSOURCE_32 | GPTCR_TEN;
> +#endif
> +	__raw_writel(i, &cur_gpt->control);
>  
>  	gd->arch.tbl = __raw_readl(&cur_gpt->counter);
>  	gd->arch.tbu = 0;
> @@ -86,7 +142,7 @@ ulong get_timer_masked(void)
>  {
>  	/*
>  	 * get_ticks() returns a long long (64 bit), it wraps in
> -	 * 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
> +	 * 2^64 / GPT_CLK = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
>  	 * 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in
>  	 * 5 * 10^6 days - long enough.
>  	 */
> @@ -117,5 +173,5 @@ void __udelay(unsigned long usec)
>   */
>  ulong get_tbclk(void)
>  {
> -	return MXC_CLK32;
> +	return gpt_get_clk();
>  }
> 


Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT

[Li Ye-B37916]

Hi Stefano,

On 10/24/2014 9:18 PM, Stefano Babic wrote:
> Hi Ye,
>
> On 24/10/2014 14:32, Ye.Li wrote:
>> Introduce a new configuration "CONFIG_MXC_GPT_HCLK". When it is set,
>> the GPT will use 24Mhz OSC as clock source. Otherwise, the GPT will
>> use 32Khz OSC as clock source.
>>
>> Since only the GPT on iMX6 series provide the clock source option for
>> 24Mhz OSC. For other series(MX5), if the configuration is set, the
>> perclk will be selected as clock source.
> But if the source option is possible only for i.MX6, why should be
> possible to set it for MX5 ? It should be hidden in the SOC
> configuration and not in board configuration.

The patch is used to add a choice for GPT clock source to provide high frequency clock.  The configuration "CONFIG_MXC_GPT_HCLK" is not dependent on the chip version. Even it is i.MX28, it is ok to set the configuration.

The implementation has handled the differences between the chips.
Most of i.MX6 series supports to use 24Mhz OSC as its high clock source (except MX6Q/D Rev 1.0 and MX6SL).  So for i.MX6, the implementation uses the 24Mhz OSC.
For others (the time.c only compiles for i.MX5 and i.MX6, so the other is i.MX5),  they don't have 24Mhz OSC source for GPT. When the configuration is set, we use perclk instead.

I feel the patch subject need to modify, like "add HCLK clock source for GPT", then people may not confuse.

>> MX6Q/D Rev 1.0 and MX6SL can't use the 24Mhz OSC clock source option,
>> so select the perclk for them. For MX6SL, we will set the OSC 24Mhz to
>> perclk in CCM, so eventually the clock comes from OSC 24Mhz.
>>
> I am trying to understand. The explanation here does not reflect in the
> implementation.  From the implementation, it is possible to set it even
> with wrong revision.

As explained above, the implementation has handled the differences. Users does not need to care the revision. For example, when the image runs on a  MX6Q/D Rev 1.0 chip, the perclk will be selected not the 24Mhz OSC.

> Anyway, I do not think it is correct to put it as a configuration
> option. This makes that we need different binaries for different
> revisions of the SOC. It should be checked at runtime if the revision is
> correct to set this clock as source. gpt_has_clk_source_osc has a check,
> but does it make sense that CONFIG_MXC_GPT_HCLK can be set in any case ?
Only i.MX5 and i.MX6 uses the GPT driver. I think the CONFIG_MXC_GPT_HCLK can be set in any case.

>> Signed-off-by: Ye.Li <B37916@freescale.com>
>> ---
> Is this a second version of the patchset ? What are the changes ? Please
> add version number to your patchset and write a history about changes. I
> can suggest to use Simon's patman to post your patches.

I met a email problem last Friday. I can't get any email from u-boot at lists.denx.de for a long time. So I mistakenly thought
the first patches are failed and send out second.

>>  arch/arm/imx-common/timer.c |   76 +++++++++++++++++++++++++++++++++++++------
>>  1 files changed, 66 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
>> index c63f78f..f7e49bd 100644
>> --- a/arch/arm/imx-common/timer.c
>> +++ b/arch/arm/imx-common/timer.c
>> @@ -2,7 +2,7 @@
>>   * (C) Copyright 2007
>>   * Sascha Hauer, Pengutronix
>>   *
>> - * (C) Copyright 2009 Freescale Semiconductor, Inc.
>> + * (C) Copyright 2009-2014 Freescale Semiconductor, Inc.
> I have already complain about this. There are a lot of commits after the
> file was merged into u-boot, and a lot of them not directly by
> Freescale. I do not think it is legally correct to change the Copyright.

Will fix this.

>>   *
>>   * SPDX-License-Identifier:	GPL-2.0+
>>   */
>> @@ -12,6 +12,7 @@
>>  #include <div64.h>
>>  #include <asm/arch/imx-regs.h>
>>  #include <asm/arch/clock.h>
>> +#include <asm/arch/sys_proto.h>
>>  
>>  /* General purpose timers registers */
>>  struct mxc_gpt {
>> @@ -26,23 +27,60 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR;
>>  
>>  /* General purpose timers bitfields */
>>  #define GPTCR_SWR		(1 << 15)	/* Software reset */
>> +#define GPTCR_24MEN	    (1 << 10)	/* Enable 24MHz clock input from crystal */
>>  #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
>> -#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
>> +#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source 32khz */
>> +#define GPTCR_CLKSOURCE_OSC	(5 << 6)	/* Clock source OSC */
>> +#define GPTCR_CLKSOURCE_PRE	(1 << 6)	/* Clock source PRECLK */
>> +#define GPTCR_CLKSOURCE_MASK (0x7 << 6)
>>  #define GPTCR_TEN		1		/* Timer enable */
>>  
>> +#define GPTPR_PRESCALER24M_SHIFT 12
>> +#define GPTPR_PRESCALER24M_MASK (0xF << GPTPR_PRESCALER24M_SHIFT)
>> +
>>  DECLARE_GLOBAL_DATA_PTR;
>>  
>> +static inline int gpt_has_clk_source_osc(void)
>> +{
>> +#if defined(CONFIG_MX6)
>> +	if (((is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
>> +		&& (is_soc_rev(CHIP_REV_1_0) > 0))
>> +		|| is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
>> +		|| is_cpu_type(MXC_CPU_MX6SX))
>> +		return 1;
>> +
>> +	return 0;
>> +#else
>> +	return 0;
>> +#endif
> We are generally trying to get rid of #ifdef, but it seems we are not
> going in the right direction. is_cpu_type() should return false if the
> CPU is not what we expect - any chance for having this function for all
> i.MXes ? We can then get rid of a lot of nasty #ifdef. What about moving
> this function centrally, maybe in arch/arm/include/asm/imx-common/ ?

This actually is another topic, not relate with this patch much.  It is a good suggestion to reduce the #ifdef. But i think it need more work to do.

>> +}
>> +
>> +static inline ulong gpt_get_clk(void)
>> +{
>> +#ifdef CONFIG_MXC_GPT_HCLK
>> +	if (gpt_has_clk_source_osc())
>> +		return MXC_HCLK >> 3;
>> +	else
>> +		return mxc_get_clock(MXC_IPG_PERCLK);
>> +#else
>> +	return MXC_CLK32;
>> +#endif
>> +}
>>  static inline unsigned long long tick_to_time(unsigned long long tick)
>>  {
>> +	ulong gpt_clk = gpt_get_clk();
>> +
>>  	tick *= CONFIG_SYS_HZ;
>> -	do_div(tick, MXC_CLK32);
>> +	do_div(tick, gpt_clk);
>>  
>>  	return tick;
>>  }
>>  
>>  static inline unsigned long long us_to_tick(unsigned long long usec)
>>  {
>> -	usec = usec * MXC_CLK32 + 999999;
>> +	ulong gpt_clk = gpt_get_clk();
>> +
>> +	usec = usec * gpt_clk + 999999;
>>  	do_div(usec, 1000000);
>>  
>>  	return usec;
>> @@ -59,11 +97,29 @@ int timer_init(void)
>>  	for (i = 0; i < 100; i++)
>>  		__raw_writel(0, &cur_gpt->control);
>>  
>> -	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
>> -
>> -	/* Freerun Mode, PERCLK1 input */
>>  	i = __raw_readl(&cur_gpt->control);
>> -	__raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, &cur_gpt->control);
>> +	i &= ~GPTCR_CLKSOURCE_MASK;
>> +
>> +#ifdef CONFIG_MXC_GPT_HCLK
>> +	if (gpt_has_clk_source_osc()) {
>> +		i |= GPTCR_CLKSOURCE_OSC | GPTCR_TEN;
>> +
>> +		/* For DL/S, SX, they has 24M OSC Enable bit and need to set prescaler */
>> +		if (is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
>> +			|| is_cpu_type(MXC_CPU_MX6SX)) {
>> +			i |= GPTCR_24MEN;
>> +
>> +			/* Produce 3Mhz clock */
>> +			__raw_writel((7 << GPTPR_PRESCALER24M_SHIFT), &cur_gpt->prescaler);
>> +		}
>> +	} else {
>> +		i |= GPTCR_CLKSOURCE_PRE | GPTCR_TEN;
>> +	}
>> +#else
>> +	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
>> +	i |= GPTCR_CLKSOURCE_32 | GPTCR_TEN;
>> +#endif
>> +	__raw_writel(i, &cur_gpt->control);
>>  
>>  	gd->arch.tbl = __raw_readl(&cur_gpt->counter);
>>  	gd->arch.tbu = 0;
>> @@ -86,7 +142,7 @@ ulong get_timer_masked(void)
>>  {
>>  	/*
>>  	 * get_ticks() returns a long long (64 bit), it wraps in
>> -	 * 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
>> +	 * 2^64 / GPT_CLK = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
>>  	 * 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in
>>  	 * 5 * 10^6 days - long enough.
>>  	 */
>> @@ -117,5 +173,5 @@ void __udelay(unsigned long usec)
>>   */
>>  ulong get_tbclk(void)
>>  {
>> -	return MXC_CLK32;
>> +	return gpt_get_clk();
>>  }
>>
>
> Best regards,
> Stefano Babic
>

[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT

[Stefano Babic]

Hi Ye,

On 27/10/2014 05:10, Li Ye-B37916 wrote:
> 
> The patch is used to add a choice for GPT clock source to provide high frequency clock.  The configuration "CONFIG_MXC_GPT_HCLK" is not dependent on the chip version. Even it is i.MX28, it is ok to set the configuration.

Ok, thanks for clarification.

> 
> The implementation has handled the differences between the chips.
> Most of i.MX6 series supports to use 24Mhz OSC as its high clock source (except MX6Q/D Rev 1.0 and MX6SL).  So for i.MX6, the implementation uses the 24Mhz OSC.
> For others (the time.c only compiles for i.MX5 and i.MX6, so the other is i.MX5),  they don't have 24Mhz OSC source for GPT. When the configuration is set, we use perclk instead.

It should be not bad if you check for MX5 and CONFIG_MXC_GPT_HCLK and
raise an error at compile time. This configuration is wrong and the
error should be reported and not hidden at runtime.

> 
> I feel the patch subject need to modify, like "add HCLK clock source for GPT", then people may not confuse.

Agree, do it.

> 
>>> MX6Q/D Rev 1.0 and MX6SL can't use the 24Mhz OSC clock source option,
>>> so select the perclk for them. For MX6SL, we will set the OSC 24Mhz to
>>> perclk in CCM, so eventually the clock comes from OSC 24Mhz.
>>>
>> I am trying to understand. The explanation here does not reflect in the
>> implementation.  From the implementation, it is possible to set it even
>> with wrong revision.
> 
> As explained above, the implementation has handled the differences. Users does not need to care the revision. For example, when the image runs on a  MX6Q/D Rev 1.0 chip, the perclk will be selected not the 24Mhz OSC.
> 
>> Anyway, I do not think it is correct to put it as a configuration
>> option. This makes that we need different binaries for different
>> revisions of the SOC. It should be checked at runtime if the revision is
>> correct to set this clock as source. gpt_has_clk_source_osc has a check,
>> but does it make sense that CONFIG_MXC_GPT_HCLK can be set in any case ?
> Only i.MX5 and i.MX6 uses the GPT driver. I think the CONFIG_MXC_GPT_HCLK can be set in any case.

Well, but if does not make sense for i.MX5, why do we have to accept it ?

> 
>>> Signed-off-by: Ye.Li <B37916@freescale.com>
>>> ---
>> Is this a second version of the patchset ? What are the changes ? Please
>> add version number to your patchset and write a history about changes. I
>> can suggest to use Simon's patman to post your patches.
> 
> I met a email problem last Friday. I can't get any email from u-boot at lists.denx.de for a long time. So I mistakenly thought
> the first patches are failed and send out second.

Never mind ;-)

> 
>>>  arch/arm/imx-common/timer.c |   76 +++++++++++++++++++++++++++++++++++++------
>>>  1 files changed, 66 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
>>> index c63f78f..f7e49bd 100644
>>> --- a/arch/arm/imx-common/timer.c
>>> +++ b/arch/arm/imx-common/timer.c
>>> @@ -2,7 +2,7 @@
>>>   * (C) Copyright 2007
>>>   * Sascha Hauer, Pengutronix
>>>   *
>>> - * (C) Copyright 2009 Freescale Semiconductor, Inc.
>>> + * (C) Copyright 2009-2014 Freescale Semiconductor, Inc.
>> I have already complain about this. There are a lot of commits after the
>> file was merged into u-boot, and a lot of them not directly by
>> Freescale. I do not think it is legally correct to change the Copyright.
> 
> Will fix this.

Thanks

> 
>>>   *
>>>   * SPDX-License-Identifier:	GPL-2.0+
>>>   */
>>> @@ -12,6 +12,7 @@
>>>  #include <div64.h>
>>>  #include <asm/arch/imx-regs.h>
>>>  #include <asm/arch/clock.h>
>>> +#include <asm/arch/sys_proto.h>
>>>  
>>>  /* General purpose timers registers */
>>>  struct mxc_gpt {
>>> @@ -26,23 +27,60 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR;
>>>  
>>>  /* General purpose timers bitfields */
>>>  #define GPTCR_SWR		(1 << 15)	/* Software reset */
>>> +#define GPTCR_24MEN	    (1 << 10)	/* Enable 24MHz clock input from crystal */
>>>  #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
>>> -#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
>>> +#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source 32khz */
>>> +#define GPTCR_CLKSOURCE_OSC	(5 << 6)	/* Clock source OSC */
>>> +#define GPTCR_CLKSOURCE_PRE	(1 << 6)	/* Clock source PRECLK */
>>> +#define GPTCR_CLKSOURCE_MASK (0x7 << 6)
>>>  #define GPTCR_TEN		1		/* Timer enable */
>>>  
>>> +#define GPTPR_PRESCALER24M_SHIFT 12
>>> +#define GPTPR_PRESCALER24M_MASK (0xF << GPTPR_PRESCALER24M_SHIFT)
>>> +
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>  
>>> +static inline int gpt_has_clk_source_osc(void)
>>> +{
>>> +#if defined(CONFIG_MX6)
>>> +	if (((is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
>>> +		&& (is_soc_rev(CHIP_REV_1_0) > 0))
>>> +		|| is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
>>> +		|| is_cpu_type(MXC_CPU_MX6SX))
>>> +		return 1;
>>> +
>>> +	return 0;
>>> +#else
>>> +	return 0;
>>> +#endif
>> We are generally trying to get rid of #ifdef, but it seems we are not
>> going in the right direction. is_cpu_type() should return false if the
>> CPU is not what we expect - any chance for having this function for all
>> i.MXes ? We can then get rid of a lot of nasty #ifdef. What about moving
>> this function centrally, maybe in arch/arm/include/asm/imx-common/ ?
> 
> This actually is another topic, not relate with this patch much.  It is a good suggestion to reduce the #ifdef. But i think it need more work to do.

ok - agree this is a more general topic, and can be addressed later.

> 
>>> +}
>>> +
>>> +static inline ulong gpt_get_clk(void)
>>> +{
>>> +#ifdef CONFIG_MXC_GPT_HCLK
>>> +	if (gpt_has_clk_source_osc())
>>> +		return MXC_HCLK >> 3;
>>> +	else
>>> +		return mxc_get_clock(MXC_IPG_PERCLK);
>>> +#else
>>> +	return MXC_CLK32;
>>> +#endif
>>> +}
>>>  static inline unsigned long long tick_to_time(unsigned long long tick)
>>>  {
>>> +	ulong gpt_clk = gpt_get_clk();
>>> +
>>>  	tick *= CONFIG_SYS_HZ;
>>> -	do_div(tick, MXC_CLK32);
>>> +	do_div(tick, gpt_clk);
>>>  
>>>  	return tick;
>>>  }
>>>  
>>>  static inline unsigned long long us_to_tick(unsigned long long usec)
>>>  {
>>> -	usec = usec * MXC_CLK32 + 999999;
>>> +	ulong gpt_clk = gpt_get_clk();
>>> +
>>> +	usec = usec * gpt_clk + 999999;
>>>  	do_div(usec, 1000000);
>>>  
>>>  	return usec;
>>> @@ -59,11 +97,29 @@ int timer_init(void)
>>>  	for (i = 0; i < 100; i++)
>>>  		__raw_writel(0, &cur_gpt->control);
>>>  
>>> -	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
>>> -
>>> -	/* Freerun Mode, PERCLK1 input */
>>>  	i = __raw_readl(&cur_gpt->control);
>>> -	__raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, &cur_gpt->control);
>>> +	i &= ~GPTCR_CLKSOURCE_MASK;
>>> +
>>> +#ifdef CONFIG_MXC_GPT_HCLK
>>> +	if (gpt_has_clk_source_osc()) {
>>> +		i |= GPTCR_CLKSOURCE_OSC | GPTCR_TEN;
>>> +
>>> +		/* For DL/S, SX, they has 24M OSC Enable bit and need to set prescaler */
>>> +		if (is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
>>> +			|| is_cpu_type(MXC_CPU_MX6SX)) {
>>> +			i |= GPTCR_24MEN;
>>> +
>>> +			/* Produce 3Mhz clock */
>>> +			__raw_writel((7 << GPTPR_PRESCALER24M_SHIFT), &cur_gpt->prescaler);
>>> +		}
>>> +	} else {
>>> +		i |= GPTCR_CLKSOURCE_PRE | GPTCR_TEN;
>>> +	}
>>> +#else
>>> +	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
>>> +	i |= GPTCR_CLKSOURCE_32 | GPTCR_TEN;
>>> +#endif
>>> +	__raw_writel(i, &cur_gpt->control);
>>>  
>>>  	gd->arch.tbl = __raw_readl(&cur_gpt->counter);
>>>  	gd->arch.tbu = 0;
>>> @@ -86,7 +142,7 @@ ulong get_timer_masked(void)
>>>  {
>>>  	/*
>>>  	 * get_ticks() returns a long long (64 bit), it wraps in
>>> -	 * 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
>>> +	 * 2^64 / GPT_CLK = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
>>>  	 * 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in
>>>  	 * 5 * 10^6 days - long enough.
>>>  	 */
>>> @@ -117,5 +173,5 @@ void __udelay(unsigned long usec)
>>>   */
>>>  ulong get_tbclk(void)
>>>  {
>>> -	return MXC_CLK32;
>>> +	return gpt_get_clk();
>>>  }
>>>
>>

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT

[Li Ye-B37916]

Hi Stefano,

On 10/27/2014 7:18 PM, Stefano Babic wrote:
> Hi Ye,
>
> On 27/10/2014 05:10, Li Ye-B37916 wrote:
>> The patch is used to add a choice for GPT clock source to provide high frequency clock.  The configuration "CONFIG_MXC_GPT_HCLK" is not dependent on the chip version. Even it is i.MX28, it is ok to set the configuration.
> Ok, thanks for clarification.
>
>> The implementation has handled the differences between the chips.
>> Most of i.MX6 series supports to use 24Mhz OSC as its high clock source (except MX6Q/D Rev 1.0 and MX6SL).  So for i.MX6, the implementation uses the 24Mhz OSC.
>> For others (the time.c only compiles for i.MX5 and i.MX6, so the other is i.MX5),  they don't have 24Mhz OSC source for GPT. When the configuration is set, we use perclk instead.
> It should be not bad if you check for MX5 and CONFIG_MXC_GPT_HCLK and
> raise an error at compile time. This configuration is wrong and the
> error should be reported and not hidden at runtime.

No, this configuration is not wrong for MX5. When it is set on MX5, the implementation uses preclk as the clock source to generate a high frequency GPT.  

>> I feel the patch subject need to modify, like "add HCLK clock source for GPT", then people may not confuse.
> Agree, do it.
>
>>>> MX6Q/D Rev 1.0 and MX6SL can't use the 24Mhz OSC clock source option,
>>>> so select the perclk for them. For MX6SL, we will set the OSC 24Mhz to
>>>> perclk in CCM, so eventually the clock comes from OSC 24Mhz.
>>>>
>>> I am trying to understand. The explanation here does not reflect in the
>>> implementation.  From the implementation, it is possible to set it even
>>> with wrong revision.
>> As explained above, the implementation has handled the differences. Users does not need to care the revision. For example, when the image runs on a  MX6Q/D Rev 1.0 chip, the perclk will be selected not the 24Mhz OSC.
>>
>>> Anyway, I do not think it is correct to put it as a configuration
>>> option. This makes that we need different binaries for different
>>> revisions of the SOC. It should be checked at runtime if the revision is
>>> correct to set this clock as source. gpt_has_clk_source_osc has a check,
>>> but does it make sense that CONFIG_MXC_GPT_HCLK can be set in any case ?
>> Only i.MX5 and i.MX6 uses the GPT driver. I think the CONFIG_MXC_GPT_HCLK can be set in any case.
> Well, but if does not make sense for i.MX5, why do we have to accept it ?

The MX5 can also have a high frequency GPT. The difference is it uses another clock source "preclk" than the "24Mhz OSC" used for iMX6.


>>>> Signed-off-by: Ye.Li <B37916@freescale.com>
>>>> ---
>>> Is this a second version of the patchset ? What are the changes ? Please
>>> add version number to your patchset and write a history about changes. I
>>> can suggest to use Simon's patman to post your patches.
>> I met a email problem last Friday. I can't get any email from u-boot at lists.denx.de for a long time. So I mistakenly thought
>> the first patches are failed and send out second.
> Never mind ;-)
>
>>>>  arch/arm/imx-common/timer.c |   76 +++++++++++++++++++++++++++++++++++++------
>>>>  1 files changed, 66 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
>>>> index c63f78f..f7e49bd 100644
>>>> --- a/arch/arm/imx-common/timer.c
>>>> +++ b/arch/arm/imx-common/timer.c
>>>> @@ -2,7 +2,7 @@
>>>>   * (C) Copyright 2007
>>>>   * Sascha Hauer, Pengutronix
>>>>   *
>>>> - * (C) Copyright 2009 Freescale Semiconductor, Inc.
>>>> + * (C) Copyright 2009-2014 Freescale Semiconductor, Inc.
>>> I have already complain about this. There are a lot of commits after the
>>> file was merged into u-boot, and a lot of them not directly by
>>> Freescale. I do not think it is legally correct to change the Copyright.
>> Will fix this.
> Thanks
>
>>>>   *
>>>>   * SPDX-License-Identifier:	GPL-2.0+
>>>>   */
>>>> @@ -12,6 +12,7 @@
>>>>  #include <div64.h>
>>>>  #include <asm/arch/imx-regs.h>
>>>>  #include <asm/arch/clock.h>
>>>> +#include <asm/arch/sys_proto.h>
>>>>  
>>>>  /* General purpose timers registers */
>>>>  struct mxc_gpt {
>>>> @@ -26,23 +27,60 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR;
>>>>  
>>>>  /* General purpose timers bitfields */
>>>>  #define GPTCR_SWR		(1 << 15)	/* Software reset */
>>>> +#define GPTCR_24MEN	    (1 << 10)	/* Enable 24MHz clock input from crystal */
>>>>  #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
>>>> -#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
>>>> +#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source 32khz */
>>>> +#define GPTCR_CLKSOURCE_OSC	(5 << 6)	/* Clock source OSC */
>>>> +#define GPTCR_CLKSOURCE_PRE	(1 << 6)	/* Clock source PRECLK */
>>>> +#define GPTCR_CLKSOURCE_MASK (0x7 << 6)
>>>>  #define GPTCR_TEN		1		/* Timer enable */
>>>>  
>>>> +#define GPTPR_PRESCALER24M_SHIFT 12
>>>> +#define GPTPR_PRESCALER24M_MASK (0xF << GPTPR_PRESCALER24M_SHIFT)
>>>> +
>>>>  DECLARE_GLOBAL_DATA_PTR;
>>>>  
>>>> +static inline int gpt_has_clk_source_osc(void)
>>>> +{
>>>> +#if defined(CONFIG_MX6)
>>>> +	if (((is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
>>>> +		&& (is_soc_rev(CHIP_REV_1_0) > 0))
>>>> +		|| is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
>>>> +		|| is_cpu_type(MXC_CPU_MX6SX))
>>>> +		return 1;
>>>> +
>>>> +	return 0;
>>>> +#else
>>>> +	return 0;
>>>> +#endif
>>> We are generally trying to get rid of #ifdef, but it seems we are not
>>> going in the right direction. is_cpu_type() should return false if the
>>> CPU is not what we expect - any chance for having this function for all
>>> i.MXes ? We can then get rid of a lot of nasty #ifdef. What about moving
>>> this function centrally, maybe in arch/arm/include/asm/imx-common/ ?
>> This actually is another topic, not relate with this patch much.  It is a good suggestion to reduce the #ifdef. But i think it need more work to do.
> ok - agree this is a more general topic, and can be addressed later.
>
>>>> +}
>>>> +
>>>> +static inline ulong gpt_get_clk(void)
>>>> +{
>>>> +#ifdef CONFIG_MXC_GPT_HCLK
>>>> +	if (gpt_has_clk_source_osc())
>>>> +		return MXC_HCLK >> 3;
>>>> +	else
>>>> +		return mxc_get_clock(MXC_IPG_PERCLK);
>>>> +#else
>>>> +	return MXC_CLK32;
>>>> +#endif
>>>> +}
>>>>  static inline unsigned long long tick_to_time(unsigned long long tick)
>>>>  {
>>>> +	ulong gpt_clk = gpt_get_clk();
>>>> +
>>>>  	tick *= CONFIG_SYS_HZ;
>>>> -	do_div(tick, MXC_CLK32);
>>>> +	do_div(tick, gpt_clk);
>>>>  
>>>>  	return tick;
>>>>  }
>>>>  
>>>>  static inline unsigned long long us_to_tick(unsigned long long usec)
>>>>  {
>>>> -	usec = usec * MXC_CLK32 + 999999;
>>>> +	ulong gpt_clk = gpt_get_clk();
>>>> +
>>>> +	usec = usec * gpt_clk + 999999;
>>>>  	do_div(usec, 1000000);
>>>>  
>>>>  	return usec;
>>>> @@ -59,11 +97,29 @@ int timer_init(void)
>>>>  	for (i = 0; i < 100; i++)
>>>>  		__raw_writel(0, &cur_gpt->control);
>>>>  
>>>> -	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
>>>> -
>>>> -	/* Freerun Mode, PERCLK1 input */
>>>>  	i = __raw_readl(&cur_gpt->control);
>>>> -	__raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, &cur_gpt->control);
>>>> +	i &= ~GPTCR_CLKSOURCE_MASK;
>>>> +
>>>> +#ifdef CONFIG_MXC_GPT_HCLK
>>>> +	if (gpt_has_clk_source_osc()) {
>>>> +		i |= GPTCR_CLKSOURCE_OSC | GPTCR_TEN;
>>>> +
>>>> +		/* For DL/S, SX, they has 24M OSC Enable bit and need to set prescaler */
>>>> +		if (is_cpu_type(MXC_CPU_MX6DL) || is_cpu_type(MXC_CPU_MX6SOLO)
>>>> +			|| is_cpu_type(MXC_CPU_MX6SX)) {
>>>> +			i |= GPTCR_24MEN;
>>>> +
>>>> +			/* Produce 3Mhz clock */
>>>> +			__raw_writel((7 << GPTPR_PRESCALER24M_SHIFT), &cur_gpt->prescaler);
>>>> +		}
>>>> +	} else {
>>>> +		i |= GPTCR_CLKSOURCE_PRE | GPTCR_TEN;
>>>> +	}
>>>> +#else
>>>> +	__raw_writel(0, &cur_gpt->prescaler); /* 32Khz */
>>>> +	i |= GPTCR_CLKSOURCE_32 | GPTCR_TEN;
>>>> +#endif
>>>> +	__raw_writel(i, &cur_gpt->control);
>>>>  
>>>>  	gd->arch.tbl = __raw_readl(&cur_gpt->counter);
>>>>  	gd->arch.tbu = 0;
>>>> @@ -86,7 +142,7 @@ ulong get_timer_masked(void)
>>>>  {
>>>>  	/*
>>>>  	 * get_ticks() returns a long long (64 bit), it wraps in
>>>> -	 * 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
>>>> +	 * 2^64 / GPT_CLK = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~
>>>>  	 * 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in
>>>>  	 * 5 * 10^6 days - long enough.
>>>>  	 */
>>>> @@ -117,5 +173,5 @@ void __udelay(unsigned long usec)
>>>>   */
>>>>  ulong get_tbclk(void)
>>>>  {
>>>> -	return MXC_CLK32;
>>>> +	return gpt_get_clk();
>>>>  }
>>>>
> Best regards,
> Stefano Babic
>
>

Best regards,
Ye Li

[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT

[Stefano Babic]

Hi Ye,

On 27/10/2014 14:58, Li Ye-B37916 wrote:
> Hi Stefano,
> 
> On 10/27/2014 7:18 PM, Stefano Babic wrote:
>> Hi Ye,
>>
>> On 27/10/2014 05:10, Li Ye-B37916 wrote:
>>> The patch is used to add a choice for GPT clock source to provide high frequency clock.  The configuration "CONFIG_MXC_GPT_HCLK" is not dependent on the chip version. Even it is i.MX28, it is ok to set the configuration.
>> Ok, thanks for clarification.
>>
>>> The implementation has handled the differences between the chips.
>>> Most of i.MX6 series supports to use 24Mhz OSC as its high clock source (except MX6Q/D Rev 1.0 and MX6SL).  So for i.MX6, the implementation uses the 24Mhz OSC.
>>> For others (the time.c only compiles for i.MX5 and i.MX6, so the other is i.MX5),  they don't have 24Mhz OSC source for GPT. When the configuration is set, we use perclk instead.
>> It should be not bad if you check for MX5 and CONFIG_MXC_GPT_HCLK and
>> raise an error at compile time. This configuration is wrong and the
>> error should be reported and not hidden at runtime.
> 
> No, this configuration is not wrong for MX5. When it is set on MX5, the implementation uses preclk as the clock source to generate a high frequency GPT.  

I see - in this case you defined GPTCR_CLKSOURCE_PRE. Ok, got it.

> 
>>> I feel the patch subject need to modify, like "add HCLK clock source for GPT", then people may not confuse.
>> Agree, do it.
>>
>>>>> MX6Q/D Rev 1.0 and MX6SL can't use the 24Mhz OSC clock source option,
>>>>> so select the perclk for them. For MX6SL, we will set the OSC 24Mhz to
>>>>> perclk in CCM, so eventually the clock comes from OSC 24Mhz.
>>>>>
>>>> I am trying to understand. The explanation here does not reflect in the
>>>> implementation.  From the implementation, it is possible to set it even
>>>> with wrong revision.
>>> As explained above, the implementation has handled the differences. Users does not need to care the revision. For example, when the image runs on a  MX6Q/D Rev 1.0 chip, the perclk will be selected not the 24Mhz OSC.
>>>
>>>> Anyway, I do not think it is correct to put it as a configuration
>>>> option. This makes that we need different binaries for different
>>>> revisions of the SOC. It should be checked at runtime if the revision is
>>>> correct to set this clock as source. gpt_has_clk_source_osc has a check,
>>>> but does it make sense that CONFIG_MXC_GPT_HCLK can be set in any case ?
>>> Only i.MX5 and i.MX6 uses the GPT driver. I think the CONFIG_MXC_GPT_HCLK can be set in any case.
>> Well, but if does not make sense for i.MX5, why do we have to accept it ?
> 
> The MX5 can also have a high frequency GPT. The difference is it uses another clock source "preclk" than the "24Mhz OSC" used for iMX6.

Fine.


Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================