[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), ®s->pr);
> + writel(( (prescaler & GPT_PR_PRESCALER_MASK) <<
> GPT_PR_PRESCALER_SHIFT), ®s->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 , ®s->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(®s->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(®s->cr, GPT_CR_SWR);
> > +
> > + /* Wait for timer to finish reset */
> > + while (readl(®s->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),
> ®s->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, ®s->cr);
> > + } else {
> > + if (prescaler > GPT_PR_PRESCALER_MAX)
> > + return -EINVAL;
> > +
> > + /* Set prescaler */
> > + writel((prescaler << GPT_PR_PRESCALER_SHIFT),
> ®s->pr);
> > + /* Set Peripheral as clock source and set gpt in
> free-running
> > + * mode
> > + */
> > + writel(GPT_CLKSRC_IPG_CLK | GPT_CR_FRR, ®s->cr);
> > + }
> > +
> > + /* Start timer */
> > + setbits_le32(®s->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