[U-Boot] [PATCH 1/5] imx: gpt: Add 24Mhz OSC clock source support for GPT
Stefano Babic
sbabic at denx.de
Mon Oct 27 12:18:50 CET 2014
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 at 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
=====================================================================
More information about the U-Boot
mailing list