[PATCH 02/16] timer: imx-gpt: Add timer support for i.MX SoCs family

Giulio Benetti giulio.benetti at benettiengineering.com
Thu Apr 8 05:05:51 CEST 2021


Hi Jesse,

please use plain-text e-mail instead of html,

On 4/8/21 3:06 AM, Jesse T wrote:
>     > +     if (rate == 24000000UL) {
>     > +             /* Set timer frequency if using 24M clock source */
>     > +             if (prescaler > GPT_PR_PRESCALER24M_MAX)
>     > +                     return -EINVAL;
>     > +
> 
> In the datasheet for the imxrt1050 the 24Mhz scaler output goes into the 
> normal prescaler.The width of the 24Mhz prescaler is only 4 bits it 
> seems to be meant to divide the 24Mhz to something more reasonable for 
> the other prescaler.

You're right, I've missed RM Figure 51-2 against 51-1, I was sure that 
Prescaler 24M was directly feeding the Timer Counter. Anyway I was 
trying to follow what they do in Linux kernel:
https://elixir.bootlin.com/linux/latest/source/drivers/clocksource/timer-imx-gpt.c#L314

What they try to do as I understand is to avoid using PLL if 3Mhz is the 
wished rate and the same I'm trying to do here. Then yes, you're right 
that main prescaler can divide again.

> I would do something like this...
> 
> +     if (rate == 24000000UL) {
> +             /* Set timer frequency if using 24M clock source */
> +             if (prescaler > (GPT_PR_PRESCALER24M_MAX  <<  12) | 
> GPT_PR_PRESCALER_MAX)

I can't understand this check ^^^

> +                     return -EINVAL;
> +
> +               /* Set 24M prescaler */
> +               writel(( (prescaler >> 12) << 
> GPT_PR_PRESCALER24M_SHIFT), &regs->pr);
> +               writel(( (prescaler & GPT_PR_PRESCALER_MASK) << 
> GPT_PR_PRESCALER_SHIFT), &regs->pr);

This way ^^^ you set the 2 in-series prescalers with the same value if 
it's what you mean.

> 
> Ideally we would have the lengths in bits for GPT_PR_PRESCALER_MASK to 
> replace the 12, but we can also do...
> 
> +     if (rate == 24000000UL) {
> +             /* Set timer frequency if using 24M clock source */
> +             if (prescaler > (GPT_PR_PRESCALER24M_MASK | 
> GPT_PR_PRESCALER_MAX))

Still don't understand this ^^^ with the 2 or'ed macros.

That gets expanded like:
```
if (prescaler > (GPT_PR_PRESCALER24M_MASK | ((GPT_PR_PRESCALER24M_MASK 
 >> GPT_PR_PRESCALER24M_SHIFT)))
```
and to:
```
if (prescaler > (0x0000F000 | (0x0000F000 >> 12)))
```
that results in:
```
if (prescaler > (0x0000F000 | 0xF))
```

so it doesn't make sense to me, maybe did you mean something different 
or I'm wrong while expanding?

> +                     return -EINVAL;
> +
> +               /* Set 24M prescaler */
> +               writel(prescaler , &regs->pr);
> 
> Side note while debugging this i added `KCFLAGS=-DDEBUG` and the boot 
> would hang with it but without it would boot normally. The cause for the 
> hang is this loop will never exit, not this only happens while booting 
> u-boot and not during the spl stage.
>   /drivers/serial/serial_lpuart.c
> @@ -356,7 +356,9 @@ static void _lpuart32_serial_putc(struct 
> lpuart_serial_plat *plat,
> 
>    while (true) {
>    lpuart_read32(plat->flags, &base->stat, &stat);
> 
>    if ((stat & STAT_TDRE))
>    break;

Thanks for recalling me this, I still didn't check it, so I'm going 
through this and fix it.

> Sorry if im being dumb

You're not being dumb

im new to this stuff and thanks for every one for
> walking me through this.

You're welcome :-) and thank you for helping me with this driver and 
reviewing! Reviewing is a very important part of the process of 
upstreaming patches.

Kind regards
-- 
Giulio Benetti
Benetti Engineering sas

> 
> On Wed, Apr 7, 2021 at 3:20 PM Giulio Benetti 
> <giulio.benetti at benettiengineering.com 
> <mailto:giulio.benetti at benettiengineering.com>> wrote:
> 
>     On 4/7/21 9:02 PM, Giulio Benetti wrote:
>      > This timer driver is using GPT Timer (General Purpose Timer)
>     available
>      > on almost all i.MX SoCs family. Since this driver is only meant to
>      > provide u-boot's timer and counter, and most of the i.MX* SoCs use a
>      > 24Mhz crystal, let's only deal with that specific source.
> 
>     Sorry, it's not true we deal 24Mhz only, also peripheral clock is dealt.
>     So commit log should be:
>     ```
>     This timer driver is using GPT Timer (General Purpose Timer) available
>     on almost all i.MX SoCs family. This driver deals with both 24Mhz
>     oscillator as well as peripheral clock.
>     ```
> 
>     Let me know if I need to re-send.
> 
>     Best regards
>     -- 
>     Giulio Benetti
>     Benetti Engineering sas
> 
>      > Signed-off-by: Giulio Benetti
>     <giulio.benetti at benettiengineering.com
>     <mailto:giulio.benetti at benettiengineering.com>>
>      > [Giulio: added the driver's stub and handled peripheral clock
>     prescaler
>      > setting]
>      > Signed-off-by: Jesse Taube <mr.bossman075 at gmail.com
>     <mailto:mr.bossman075 at gmail.com>>
>      > [Jesse: added init, setting prescaler for 24Mhz support and enabling
>      > timer]
>      > ---
>      >   drivers/timer/Kconfig         |   7 ++
>      >   drivers/timer/Makefile        |   1 +
>      >   drivers/timer/imx-gpt-timer.c | 162
>     ++++++++++++++++++++++++++++++++++
>      >   3 files changed, 170 insertions(+)
>      >   create mode 100644 drivers/timer/imx-gpt-timer.c
>      >
>      > diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
>      > index 80743a2551..ee81dfa776 100644
>      > --- a/drivers/timer/Kconfig
>      > +++ b/drivers/timer/Kconfig
>      > @@ -227,4 +227,11 @@ config MCHP_PIT64B_TIMER
>      >         Select this to enable support for Microchip 64-bit periodic
>      >         interval timer.
>      >
>      > +config IMX_GPT_TIMER
>      > +     bool "NXP i.MX GPT timer support"
>      > +     depends on TIMER
>      > +     help
>      > +       Select this to enable support for the timer found on
>      > +       NXP i.MX devices.
>      > +
>      >   endmenu
>      > diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
>      > index eb5c48cc6c..e214ba7268 100644
>      > --- a/drivers/timer/Makefile
>      > +++ b/drivers/timer/Makefile
>      > @@ -25,3 +25,4 @@ obj-$(CONFIG_STM32_TIMER)   += stm32_timer.o
>      >   obj-$(CONFIG_X86_TSC_TIMER) += tsc_timer.o
>      >   obj-$(CONFIG_MTK_TIMER)             += mtk_timer.o
>      >   obj-$(CONFIG_MCHP_PIT64B_TIMER)     += mchp-pit64b-timer.o
>      > +obj-$(CONFIG_IMX_GPT_TIMER)  += imx-gpt-timer.o
>      > diff --git a/drivers/timer/imx-gpt-timer.c
>     b/drivers/timer/imx-gpt-timer.c
>      > new file mode 100644
>      > index 0000000000..a498f2e21c
>      > --- /dev/null
>      > +++ b/drivers/timer/imx-gpt-timer.c
>      > @@ -0,0 +1,162 @@
>      > +// SPDX-License-Identifier: GPL-2.0+
>      > +/*
>      > + * Copyright (C) 2021
>      > + * Author(s): Giulio Benetti
>     <giulio.benetti at benettiengineering.com
>     <mailto:giulio.benetti at benettiengineering.com>>
>      > + */
>      > +
>      > +#include <common.h>
>      > +#include <clk.h>
>      > +#include <dm.h>
>      > +#include <fdtdec.h>
>      > +#include <timer.h>
>      > +#include <dm/device_compat.h>
>      > +
>      > +#include <asm/io.h>
>      > +
>      > +#define GPT_CR_EN                    BIT(0)
>      > +#define GPT_CR_FRR                   BIT(9)
>      > +#define GPT_CR_EN_24M                        BIT(10)
>      > +#define GPT_CR_SWR                   BIT(15)
>      > +
>      > +#define GPT_PR_PRESCALER24M_MASK     0x0000F000
>      > +#define GPT_PR_PRESCALER24M_SHIFT    12
>      > +#define GPT_PR_PRESCALER24M_MAX      (GPT_PR_PRESCALER24M_MASK
>      >> GPT_PR_PRESCALER24M_SHIFT)
>      > +#define GPT_PR_PRESCALER_MASK                0x00000FFF
>      > +#define GPT_PR_PRESCALER_SHIFT               0
>      > +#define GPT_PR_PRESCALER_MAX         (GPT_PR_PRESCALER_MASK >>
>     GPT_PR_PRESCALER_SHIFT)
>      > +
>      > +#define GPT_CLKSRC_IPG_CLK           (1 << 6)
>      > +#define GPT_CLKSRC_IPG_CLK_24M               (5 << 6)
>      > +
>      > +/* If CONFIG_SYS_HZ_CLOCK not specified et's default to 3Mhz */
>      > +#ifndef CONFIG_SYS_HZ_CLOCK
>      > +#define CONFIG_SYS_HZ_CLOCK          3000000
>      > +#endif
>      > +
>      > +struct imx_gpt_timer_regs {
>      > +     u32 cr;
>      > +     u32 pr;
>      > +     u32 sr;
>      > +     u32 ir;
>      > +     u32 ocr1;
>      > +     u32 ocr2;
>      > +     u32 ocr3;
>      > +     u32 icr1;
>      > +     u32 icr2;
>      > +     u32 cnt;
>      > +};
>      > +
>      > +struct imx_gpt_timer_priv {
>      > +     struct imx_gpt_timer_regs *base;
>      > +};
>      > +
>      > +static u64 imx_gpt_timer_get_count(struct udevice *dev)
>      > +{
>      > +     struct imx_gpt_timer_priv *priv = dev_get_priv(dev);
>      > +     struct imx_gpt_timer_regs *regs = priv->base;
>      > +
>      > +     return readl(&regs->cnt);
>      > +}
>      > +
>      > +static int imx_gpt_setup(struct imx_gpt_timer_regs *regs, u32 rate)
>      > +{
>      > +     u32 prescaler = (rate / CONFIG_SYS_HZ_CLOCK) - 1;
>      > +
>      > +     /* Reset the timer */
>      > +     setbits_le32(&regs->cr, GPT_CR_SWR);
>      > +
>      > +     /* Wait for timer to finish reset */
>      > +     while (readl(&regs->cr) & GPT_CR_SWR)
>      > +             ;
>      > +
>      > +     if (rate == 24000000UL) {
>      > +             /* Set timer frequency if using 24M clock source */
>      > +             if (prescaler > GPT_PR_PRESCALER24M_MAX)
>      > +                     return -EINVAL;
>      > +
>      > +             /* Set 24M prescaler */
>      > +             writel((prescaler << GPT_PR_PRESCALER24M_SHIFT),
>     &regs->pr);
>      > +             /* Set Oscillator as clock source, enable 24M input
>     and set gpt
>      > +              * in free-running mode
>      > +              */
>      > +             writel(GPT_CLKSRC_IPG_CLK_24M | GPT_CR_EN_24M |
>     GPT_CR_FRR, &regs->cr);
>      > +     } else {
>      > +             if (prescaler > GPT_PR_PRESCALER_MAX)
>      > +                     return -EINVAL;
>      > +
>      > +             /* Set prescaler */
>      > +             writel((prescaler << GPT_PR_PRESCALER_SHIFT),
>     &regs->pr);
>      > +             /* Set Peripheral as clock source and set gpt in
>     free-running
>      > +              * mode
>      > +              */
>      > +             writel(GPT_CLKSRC_IPG_CLK | GPT_CR_FRR, &regs->cr);
>      > +     }
>      > +
>      > +     /* Start timer */
>      > +     setbits_le32(&regs->cr, GPT_CR_EN);
>      > +
>      > +     return 0;
>      > +}
>      > +
>      > +static int imx_gpt_timer_probe(struct udevice *dev)
>      > +{
>      > +     struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>      > +     struct imx_gpt_timer_priv *priv = dev_get_priv(dev);
>      > +     struct imx_gpt_timer_regs *regs;
>      > +     struct clk clk;
>      > +     fdt_addr_t addr;
>      > +     u32 clk_rate;
>      > +     int ret;
>      > +
>      > +     addr = dev_read_addr(dev);
>      > +     if (addr == FDT_ADDR_T_NONE)
>      > +             return -EINVAL;
>      > +
>      > +     priv->base = (struct imx_gpt_timer_regs *)addr;
>      > +     regs = priv->base;
>      > +
>      > +     ret = clk_get_by_index(dev, 0, &clk);
>      > +     if (ret < 0)
>      > +             return ret;
>      > +
>      > +     ret = clk_enable(&clk);
>      > +     if (ret) {
>      > +             dev_err(dev, "Failed to enable clock\n");
>      > +             return ret;
>      > +     }
>      > +
>      > +     /* Get timer clock rate */
>      > +     clk_rate = clk_get_rate(&clk);
>      > +     if (clk_rate <= 0) {
>      > +             dev_err(dev, "Could not get clock rate...\n");
>      > +             return -EINVAL;
>      > +     }
>      > +
>      > +     ret = imx_gpt_setup(regs, clk_rate);
>      > +     if (ret) {
>      > +             dev_err(dev, "Could not setup timer\n");
>      > +             return ret;
>      > +     }
>      > +
>      > +     uc_priv->clock_rate = CONFIG_SYS_HZ_CLOCK;
>      > +
>      > +     return 0;
>      > +}
>      > +
>      > +static const struct timer_ops imx_gpt_timer_ops = {
>      > +     .get_count = imx_gpt_timer_get_count,
>      > +};
>      > +
>      > +static const struct udevice_id imx_gpt_timer_ids[] = {
>      > +     { .compatible = "fsl,imxrt-gpt" },
>      > +     {}
>      > +};
>      > +
>      > +U_BOOT_DRIVER(imx_gpt_timer) = {
>      > +     .name = "imx_gpt_timer",
>      > +     .id = UCLASS_TIMER,
>      > +     .of_match = imx_gpt_timer_ids,
>      > +     .priv_auto = sizeof(struct imx_gpt_timer_priv),
>      > +     .probe = imx_gpt_timer_probe,
>      > +     .ops = &imx_gpt_timer_ops,
>      > +};
>      >
> 



More information about the U-Boot mailing list