All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/1] timer: imx-gpt: Add timer support for i.MX SoCs family
@ 2021-02-12  1:11 Jesse
  2021-02-12  1:11 ` [PATCH v3 1/1] " Jesse
  2021-02-12  9:50 ` [PATCH v3 0/1] " Giulio Benetti
  0 siblings, 2 replies; 6+ messages in thread
From: Jesse @ 2021-02-12  1:11 UTC (permalink / raw)
  To: u-boot

From: Jesse Taube <mr.bossman075@gmail.com>

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.

Jesse Taube (1):
  timer: imx-gpt: Add timer support for i.MX SoCs family

 drivers/timer/Kconfig         |   7 ++
 drivers/timer/Makefile        |   1 +
 drivers/timer/imx-gpt-timer.c | 149 ++++++++++++++++++++++++++++++++++
 3 files changed, 157 insertions(+)
 create mode 100644 drivers/timer/imx-gpt-timer.c
---
V1->V2:
* Fixed indentation
* Fixed capitals
* Made timer work on only 24MHz clock
---
---
V1->V3:
* Fixed indentation
* Made implementation imatate the Linux kernel
* Fix wrong definition 
---
-- 
2.30.0

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/1] timer: imx-gpt: Add timer support for i.MX SoCs family
  2021-02-12  1:11 [PATCH v3 0/1] timer: imx-gpt: Add timer support for i.MX SoCs family Jesse
@ 2021-02-12  1:11 ` Jesse
  2021-02-12  1:29   ` Jesse T
  2021-02-12 10:13   ` Giulio Benetti
  2021-02-12  9:50 ` [PATCH v3 0/1] " Giulio Benetti
  1 sibling, 2 replies; 6+ messages in thread
From: Jesse @ 2021-02-12  1:11 UTC (permalink / raw)
  To: u-boot

From: Jesse Taube <mr.bossman075@gmail.com>

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.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
Signed-off-by: Jesse Taube <mr.bossman075@gmail.com>
---
 drivers/timer/Kconfig         |   7 ++
 drivers/timer/Makefile        |   1 +
 drivers/timer/imx-gpt-timer.c | 149 ++++++++++++++++++++++++++++++++++
 3 files changed, 157 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..62db6e663a
--- /dev/null
+++ b/drivers/timer/imx-gpt-timer.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020
+ * Author(s): Giulio Benetti <giulio.benetti@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_SWR		0x00008000
+#define GPT_CR_CLKSRC		0x000001C0
+#define GPT_CR_EN_24M		0x00000400
+#define GPT_CR_EN		0x00000001
+#define GPT_PR_PRESCALER	0x00000FFF
+#define GPT_PR_PRESCALER24M	0x0000F000
+
+#define NO_CLOCK		(0)
+#define IPG_CLK			(1 << 6)
+#define IPG_CLK_HF		(2 << 6)
+#define IPG_EXT			(3 << 6)
+#define IPG_CLK_32K		(4 << 6)
+#define IPG_CLK_24M		(5 << 6)
+
+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);
+}
+
+u32 imxrt_gpt_setup_ctrl(u32 rate)
+{
+	u32 ctlr = 0;
+
+	if (rate == 24000000UL) {
+		ctlr |= IPG_CLK_24M;
+		ctlr |= GPT_CR_EN_24M;
+	} else {
+		ctlr |= IPG_CLK;
+	}
+
+	return ctlr;
+}
+
+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 prescaler;
+	u32 rate;
+	u32 ctlr;
+	int ret;
+
+	addr = dev_read_addr(dev);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	priv->base = (struct imx_gpt_timer_regs *)addr;
+
+	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;
+	}
+
+	regs = priv->base;
+
+	/* Reset the timer */
+	setbits_le32(&regs->cr, GPT_CR_SWR);
+
+	/* Wait for timer to finish reset */
+	while (readl(&regs->cr) & GPT_CR_SWR)
+		;
+
+	/* Get timer clock rate */
+	rate = clk_get_rate(&clk);
+	if (rate <= 0) {
+		dev_err(dev, "Could not get clock rate...\n");
+		return -EINVAL;
+	}
+
+	/* Set timer frequency to 1MHz */
+	uc_priv->clock_rate = CONFIG_SYS_HZ_CLOCK;
+	prescaler = (rate / CONFIG_SYS_HZ_CLOCK) - 1;
+
+	if (prescaler > 0xfff) {
+		dev_err(dev, "Could not set prescaler...\n");
+		return -EINVAL;
+	}
+	/* We set timer prescaler to obtain a 1MHz timer counter frequency */
+	writel(prescaler, &regs->pr);
+
+	ctlr = imxrt_gpt_setup_ctrl(rate);
+
+	/* Start timer */
+	writel(ctlr, &regs->cr);
+	setbits_le32(&regs->cr, GPT_CR_EN);
+
+	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,
+};
-- 
2.30.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3 1/1] timer: imx-gpt: Add timer support for i.MX SoCs family
  2021-02-12  1:11 ` [PATCH v3 1/1] " Jesse
@ 2021-02-12  1:29   ` Jesse T
  2021-02-12  9:49     ` Giulio Benetti
  2021-02-12 10:13   ` Giulio Benetti
  1 sibling, 1 reply; 6+ messages in thread
From: Jesse T @ 2021-02-12  1:29 UTC (permalink / raw)
  To: u-boot

I'm very sorry for my email issues I some how mess it up each time.

On Thu, Feb 11, 2021, 8:11 PM Jesse <mr.bossman075@gmail.com> wrote:

> From: Jesse Taube <mr.bossman075@gmail.com>
>
> 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.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Signed-off-by: Jesse Taube <mr.bossman075@gmail.com>
> ---
>  drivers/timer/Kconfig         |   7 ++
>  drivers/timer/Makefile        |   1 +
>  drivers/timer/imx-gpt-timer.c | 149 ++++++++++++++++++++++++++++++++++
>  3 files changed, 157 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..62db6e663a
> --- /dev/null
> +++ b/drivers/timer/imx-gpt-timer.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020
> + * Author(s): Giulio Benetti <giulio.benetti@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_SWR             0x00008000
> +#define GPT_CR_CLKSRC          0x000001C0
> +#define GPT_CR_EN_24M          0x00000400
> +#define GPT_CR_EN              0x00000001
> +#define GPT_PR_PRESCALER       0x00000FFF
> +#define GPT_PR_PRESCALER24M    0x0000F000
> +
> +#define NO_CLOCK               (0)
> +#define IPG_CLK                        (1 << 6)
> +#define IPG_CLK_HF             (2 << 6)
> +#define IPG_EXT                        (3 << 6)
> +#define IPG_CLK_32K            (4 << 6)
> +#define IPG_CLK_24M            (5 << 6)
> +
> +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);
> +}
> +
> +u32 imxrt_gpt_setup_ctrl(u32 rate)
> +{
> +       u32 ctlr = 0;
> +
> +       if (rate == 24000000UL) {
> +               ctlr |= IPG_CLK_24M;
> +               ctlr |= GPT_CR_EN_24M;
> +       } else {
> +               ctlr |= IPG_CLK;
> +       }
> +
> +       return ctlr;
> +}
> +
> +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 prescaler;
> +       u32 rate;
> +       u32 ctlr;
> +       int ret;
> +
> +       addr = dev_read_addr(dev);
> +       if (addr == FDT_ADDR_T_NONE)
> +               return -EINVAL;
> +
> +       priv->base = (struct imx_gpt_timer_regs *)addr;
> +
> +       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;
> +       }
> +
> +       regs = priv->base;
> +
> +       /* Reset the timer */
> +       setbits_le32(&regs->cr, GPT_CR_SWR);
> +
> +       /* Wait for timer to finish reset */
> +       while (readl(&regs->cr) & GPT_CR_SWR)
> +               ;
> +
> +       /* Get timer clock rate */
> +       rate = clk_get_rate(&clk);
> +       if (rate <= 0) {
> +               dev_err(dev, "Could not get clock rate...\n");
> +               return -EINVAL;
> +       }
> +
> +       /* Set timer frequency to 1MHz */
> +       uc_priv->clock_rate = CONFIG_SYS_HZ_CLOCK;
> +       prescaler = (rate / CONFIG_SYS_HZ_CLOCK) - 1;
> +
> +       if (prescaler > 0xfff) {
> +               dev_err(dev, "Could not set prescaler...\n");
> +               return -EINVAL;
> +       }
> +       /* We set timer prescaler to obtain a 1MHz timer counter frequency
> */
> +       writel(prescaler, &regs->pr);
> +
> +       ctlr = imxrt_gpt_setup_ctrl(rate);
> +
> +       /* Start timer */
> +       writel(ctlr, &regs->cr);
> +       setbits_le32(&regs->cr, GPT_CR_EN);
> +
> +       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,
> +};
> --
> 2.30.0
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/1] timer: imx-gpt: Add timer support for i.MX SoCs family
  2021-02-12  1:29   ` Jesse T
@ 2021-02-12  9:49     ` Giulio Benetti
  0 siblings, 0 replies; 6+ messages in thread
From: Giulio Benetti @ 2021-02-12  9:49 UTC (permalink / raw)
  To: u-boot

Hi Jesse,

On 2/12/21 2:29 AM, Jesse T wrote:
> I'm very sorry for my email issues I some how mess it up each time.

You can try git send-mail with --to "Jesse T <mr.bossman075@gmail.com>".
This way you can check the result and only when sure you send to 
everybody and ML.

Kind regards
-- 
Giulio Benetti
Benetti Engineering sas

> On Thu, Feb 11, 2021, 8:11 PM Jesse <mr.bossman075@gmail.com 
> <mailto:mr.bossman075@gmail.com>> wrote:
> 
>     From: Jesse Taube <mr.bossman075@gmail.com
>     <mailto:mr.bossman075@gmail.com>>
> 
>     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.
> 
>     Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com
>     <mailto:giulio.benetti@benettiengineering.com>>
>     Signed-off-by: Jesse Taube <mr.bossman075@gmail.com
>     <mailto:mr.bossman075@gmail.com>>
>     ---
>      ?drivers/timer/Kconfig? ? ? ? ?|? ?7 ++
>      ?drivers/timer/Makefile? ? ? ? |? ?1 +
>      ?drivers/timer/imx-gpt-timer.c | 149 ++++++++++++++++++++++++++++++++++
>      ?3 files changed, 157 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..62db6e663a
>     --- /dev/null
>     +++ b/drivers/timer/imx-gpt-timer.c
>     @@ -0,0 +1,149 @@
>     +// SPDX-License-Identifier: GPL-2.0+
>     +/*
>     + * Copyright (C) 2020
>     + * Author(s): Giulio Benetti <giulio.benetti@benettiengineering.com
>     <mailto:giulio.benetti@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_SWR? ? ? ? ? ? ?0x00008000
>     +#define GPT_CR_CLKSRC? ? ? ? ? 0x000001C0
>     +#define GPT_CR_EN_24M? ? ? ? ? 0x00000400
>     +#define GPT_CR_EN? ? ? ? ? ? ? 0x00000001
>     +#define GPT_PR_PRESCALER? ? ? ?0x00000FFF
>     +#define GPT_PR_PRESCALER24M? ? 0x0000F000
>     +
>     +#define NO_CLOCK? ? ? ? ? ? ? ?(0)
>     +#define IPG_CLK? ? ? ? ? ? ? ? ? ? ? ? (1 << 6)
>     +#define IPG_CLK_HF? ? ? ? ? ? ?(2 << 6)
>     +#define IPG_EXT? ? ? ? ? ? ? ? ? ? ? ? (3 << 6)
>     +#define IPG_CLK_32K? ? ? ? ? ? (4 << 6)
>     +#define IPG_CLK_24M? ? ? ? ? ? (5 << 6)
>     +
>     +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);
>     +}
>     +
>     +u32 imxrt_gpt_setup_ctrl(u32 rate)
>     +{
>     +? ? ? ?u32 ctlr = 0;
>     +
>     +? ? ? ?if (rate == 24000000UL) {
>     +? ? ? ? ? ? ? ?ctlr |= IPG_CLK_24M;
>     +? ? ? ? ? ? ? ?ctlr |= GPT_CR_EN_24M;
>     +? ? ? ?} else {
>     +? ? ? ? ? ? ? ?ctlr |= IPG_CLK;
>     +? ? ? ?}
>     +
>     +? ? ? ?return ctlr;
>     +}
>     +
>     +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 prescaler;
>     +? ? ? ?u32 rate;
>     +? ? ? ?u32 ctlr;
>     +? ? ? ?int ret;
>     +
>     +? ? ? ?addr = dev_read_addr(dev);
>     +? ? ? ?if (addr == FDT_ADDR_T_NONE)
>     +? ? ? ? ? ? ? ?return -EINVAL;
>     +
>     +? ? ? ?priv->base = (struct imx_gpt_timer_regs *)addr;
>     +
>     +? ? ? ?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;
>     +? ? ? ?}
>     +
>     +? ? ? ?regs = priv->base;
>     +
>     +? ? ? ?/* Reset the timer */
>     +? ? ? ?setbits_le32(&regs->cr, GPT_CR_SWR);
>     +
>     +? ? ? ?/* Wait for timer to finish reset */
>     +? ? ? ?while (readl(&regs->cr) & GPT_CR_SWR)
>     +? ? ? ? ? ? ? ?;
>     +
>     +? ? ? ?/* Get timer clock rate */
>     +? ? ? ?rate = clk_get_rate(&clk);
>     +? ? ? ?if (rate <= 0) {
>     +? ? ? ? ? ? ? ?dev_err(dev, "Could not get clock rate...\n");
>     +? ? ? ? ? ? ? ?return -EINVAL;
>     +? ? ? ?}
>     +
>     +? ? ? ?/* Set timer frequency to 1MHz */
>     +? ? ? ?uc_priv->clock_rate = CONFIG_SYS_HZ_CLOCK;
>     +? ? ? ?prescaler = (rate / CONFIG_SYS_HZ_CLOCK) - 1;
>     +
>     +? ? ? ?if (prescaler > 0xfff) {
>     +? ? ? ? ? ? ? ?dev_err(dev, "Could not set prescaler...\n");
>     +? ? ? ? ? ? ? ?return -EINVAL;
>     +? ? ? ?}
>     +? ? ? ?/* We set timer prescaler to obtain a 1MHz timer counter
>     frequency */
>     +? ? ? ?writel(prescaler, &regs->pr);
>     +
>     +? ? ? ?ctlr = imxrt_gpt_setup_ctrl(rate);
>     +
>     +? ? ? ?/* Start timer */
>     +? ? ? ?writel(ctlr, &regs->cr);
>     +? ? ? ?setbits_le32(&regs->cr, GPT_CR_EN);
>     +
>     +? ? ? ?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,
>     +};
>     -- 
>     2.30.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 0/1] timer: imx-gpt: Add timer support for i.MX SoCs family
  2021-02-12  1:11 [PATCH v3 0/1] timer: imx-gpt: Add timer support for i.MX SoCs family Jesse
  2021-02-12  1:11 ` [PATCH v3 1/1] " Jesse
@ 2021-02-12  9:50 ` Giulio Benetti
  1 sibling, 0 replies; 6+ messages in thread
From: Giulio Benetti @ 2021-02-12  9:50 UTC (permalink / raw)
  To: u-boot



On 2/12/21 2:11 AM, Jesse wrote:
> From: Jesse Taube <mr.bossman075@gmail.com>
> 
> 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.
> 
> Jesse Taube (1):
>    timer: imx-gpt: Add timer support for i.MX SoCs family
> 
>   drivers/timer/Kconfig         |   7 ++
>   drivers/timer/Makefile        |   1 +
>   drivers/timer/imx-gpt-timer.c | 149 ++++++++++++++++++++++++++++++++++
>   3 files changed, 157 insertions(+)
>   create mode 100644 drivers/timer/imx-gpt-timer.c
> ---
> V1->V2:
> * Fixed indentation
> * Fixed capitals
> * Made timer work on only 24MHz clock
> ---
> ---

Don't need these 2 dashes lines, only first and last

> V1->V3:

Typo V1(V2)

> * Fixed indentation
> * Made implementation imatate the Linux kernel
> * Fix wrong definition
> ---
> 

For the rest it seems good :-)

-- 
Giulio Benetti
Benetti Engineering sas

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/1] timer: imx-gpt: Add timer support for i.MX SoCs family
  2021-02-12  1:11 ` [PATCH v3 1/1] " Jesse
  2021-02-12  1:29   ` Jesse T
@ 2021-02-12 10:13   ` Giulio Benetti
  1 sibling, 0 replies; 6+ messages in thread
From: Giulio Benetti @ 2021-02-12 10:13 UTC (permalink / raw)
  To: u-boot

+Cc Sean

On 2/12/21 2:11 AM, Jesse wrote:
> From: Jesse Taube <mr.bossman075@gmail.com>
> 
> 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.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Signed-off-by: Jesse Taube <mr.bossman075@gmail.com>
> ---

Here it should go what you've listed in cover letter. Changes must be 
listed patch per patch. So here you add:

V1->V2:
...
V2->3:
...

---

Note that lines between triple dashes are ignored when applying patch, 
so that doesn't impact the final result of the patch.

>   drivers/timer/Kconfig         |   7 ++
>   drivers/timer/Makefile        |   1 +
>   drivers/timer/imx-gpt-timer.c | 149 ++++++++++++++++++++++++++++++++++
>   3 files changed, 157 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..62db6e663a
> --- /dev/null
> +++ b/drivers/timer/imx-gpt-timer.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020
> + * Author(s): Giulio Benetti <giulio.benetti@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_SWR		0x00008000
> +#define GPT_CR_CLKSRC		0x000001C0
> +#define GPT_CR_EN_24M		0x00000400
> +#define GPT_CR_EN		0x00000001
> +#define GPT_PR_PRESCALER	0x00000FFF
> +#define GPT_PR_PRESCALER24M	0x0000F000
> +
> +#define NO_CLOCK		(0)
> +#define IPG_CLK			(1 << 6)

this ^^^ and

> +#define IPG_CLK_HF		(2 << 6)
> +#define IPG_EXT			(3 << 6)

this ^^^ have wrong tab number. Check editor settings to tabs=8 spaces.
I use mceditor and there tabs and spaces are highlighted, try to check 
with that for example.

> +#define IPG_CLK_32K		(4 << 6)
> +#define IPG_CLK_24M		(5 << 6)

Here ^^^ it would be better to add a prefix like GPT_CLKSRC_ to any 
value. Also, please don't list macros that are not used. So you should 
only define GPT_CLKSRC_IPG_CLK and GPT_CLKSRC_IPG_CLK_24M.

> +
> +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);
> +}
> +
> +u32 imxrt_gpt_setup_ctrl(u32 rate)

imxrt_gpt_ should be imx_gpt_timer_ and must be static since it's private.

> +{
> +	u32 ctlr = 0;
> +
> +	if (rate == 24000000UL) {
> +		ctlr |= IPG_CLK_24M;
> +		ctlr |= GPT_CR_EN_24M;

This should be a one line OR. And that way you can remove brackets since 
there are only 1 statement per case.

> +	} else {
> +		ctlr |= IPG_CLK;
> +	}

see above for brackets

> +
> +	return ctlr;
> +}
> +
> +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 prescaler;
> +	u32 rate;
> +	u32 ctlr;
> +	int ret;
> +
> +	addr = dev_read_addr(dev);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	priv->base = (struct imx_gpt_timer_regs *)addr;
> +
> +	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;
> +	}
> +
> +	regs = priv->base;
> +
> +	/* Reset the timer */
> +	setbits_le32(&regs->cr, GPT_CR_SWR);
> +
> +	/* Wait for timer to finish reset */
> +	while (readl(&regs->cr) & GPT_CR_SWR)
> +		;
> +
> +	/* Get timer clock rate */
> +	rate = clk_get_rate(&clk);
> +	if (rate <= 0) {
> +		dev_err(dev, "Could not get clock rate...\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Set timer frequency to 1MHz */
> +	uc_priv->clock_rate = CONFIG_SYS_HZ_CLOCK;
> +	prescaler = (rate / CONFIG_SYS_HZ_CLOCK) - 1;
> +
> +	if (prescaler > 0xfff) {
> +		dev_err(dev, "Could not set prescaler...\n");
> +		return -EINVAL;
> +	}
> +	/* We set timer prescaler to obtain a 1MHz timer counter frequency */
> +	writel(prescaler, &regs->pr);

prescaler setting regards only !24M way. So you should check for 
rate!=24M and set 24M prescaler otherwise. Obviously it's better to 
create a function for doing this. So...

> +	ctlr = imxrt_gpt_setup_ctrl(rate);
> +
> +	/* Start timer */
> +	writel(ctlr, &regs->cr);
> +	setbits_le32(&regs->cr, GPT_CR_EN);

I would create a function imx_gpt_timer_setup(). Inside that function I 
would call 2 different functions named imx_gpt_timer_setup_24M() and 
imx_gpt_timer_setup_per(). Inside them I would set registers according 
to the 2 different cases. Of course all common registers writing, like 
when you Start timer, I would list them before and after checking rate 
and calling specific functions, something like:

int imx_gpt_timer_setup()
{
	<Reset Timer regs write code>

	if (rate == 24000000UL)
		imx_gpt_timer_setup_24M();
	else
		imx_gpt_timer_setup_per();

	<Start Timer regs write code>
}

and call it from probe where now you directly set registers.

What about this?

Best regards
-- 
Giulio Benetti
Benetti Engineering sas

> +	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,
> +};
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-12 10:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-12  1:11 [PATCH v3 0/1] timer: imx-gpt: Add timer support for i.MX SoCs family Jesse
2021-02-12  1:11 ` [PATCH v3 1/1] " Jesse
2021-02-12  1:29   ` Jesse T
2021-02-12  9:49     ` Giulio Benetti
2021-02-12 10:13   ` Giulio Benetti
2021-02-12  9:50 ` [PATCH v3 0/1] " Giulio Benetti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.