diff mbox series

[1/4] timer: imx-gpt: Add timer support for i.MX SoCs family

Message ID 20210208002437.13500-2-mr.bossman075@gmail.com
State Superseded
Delegated to: Stefano Babic
Headers show
Series timer: imx-gpt: Add timer support for i.MX SoCs family | expand

Commit Message

Jesse T Feb. 8, 2021, 12:24 a.m. UTC 
This timer driver is using GPT Timer (General Purpose Timer) available
on almost all i.MX SoCs family.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
Cc: Stefano Babic <tsbabic@denx.de>
Cc: Giulio Benetti <giulio.benetti@benettiengineering.com>
Cc: Jesse Taube <mr.bossman075@gmail.com>
---
 drivers/timer/Kconfig         |   7 ++
 drivers/timer/Makefile        |   1 +
 drivers/timer/imx-gpt-timer.c | 118 ++++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+)
 create mode 100644 drivers/timer/imx-gpt-timer.c

Comments

Sean Anderson Feb. 8, 2021, 2:35 a.m. UTC | #1 
On 2/7/21 7:24 PM, Jesse Taube wrote:
> This timer driver is using GPT Timer (General Purpose Timer) available
> on almost all i.MX SoCs family.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Cc: Stefano Babic <tsbabic@denx.de>
> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Cc: Jesse Taube <mr.bossman075@gmail.com>
> ---
>   drivers/timer/Kconfig         |   7 ++
>   drivers/timer/Makefile        |   1 +
>   drivers/timer/imx-gpt-timer.c | 118 ++++++++++++++++++++++++++++++++++
>   3 files changed, 126 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..c4e8c12a42
> --- /dev/null
> +++ b/drivers/timer/imx-gpt-timer.c
> @@ -0,0 +1,118 @@
> +// 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
> +
> +struct imx_gpt_timer_regs {

Typical style is to use offsets for this. Though I don't think it
matters.

> +	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 int imx_gpt_timer_get_count(struct udevice *dev, u64 *count)

This signature is incorrect since 8af7bb914f ("timer: Return count from
timer_ops.get_count"). Please rebase onto u-boot/master.

> +{
> +	struct imx_gpt_timer_priv *priv = dev_get_priv(dev);
> +	struct imx_gpt_timer_regs *regs = priv->base;
> +
> +	*count = readl(&regs->cnt);
> +
> +	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 prescaler;
> +	u32 rate, pr;
> +	int ret;
> +
> +	addr = dev_read_addr(dev);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	priv->base = (struct imx_gpt_timer_regs *)addr;
> +
> +	/* TODO: Retrieve clock-source */
> +
> +	/* TODO: Retrieve prescaler */
> +
> +	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);
> +
> +	/* we set timer prescaler to obtain a 1MHz timer counter frequency */
> +//	pr = (rate / CONFIG_SYS_HZ_CLOCK) - 1;

Should this be uncommented?

> +	writel(pr, &regs->pr);
> +
> +	/* Set timer frequency to 1MHz */
> +	uc_priv->clock_rate = rate / prescaler;
> +
> +	/* start timer */
> +//	setbits_le32(&regs->cr1, CR1_CEN);

ditto

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

I believe this clock is present on other i.MX processors as well (not
just RT ones). Though those typically can use the armv7 cp15 clock.

--Sean

> +	{}
> +};
> +
> +U_BOOT_DRIVER(imx_gpt_timer) = {
> +	.name = "imx_gpt_timer",
> +	.id = UCLASS_TIMER,
> +	.of_match = imx_gpt_timer_ids,
> +	.priv_auto_alloc_size = sizeof(struct imx_gpt_timer_priv),
> +	.probe = imx_gpt_timer_probe,
> +	.ops = &imx_gpt_timer_ops,
> +};
> +
>
Giulio Benetti Feb. 8, 2021, 10:20 p.m. UTC | #2 
Hi Jesse,

On 2/8/21 1:24 AM, Jesse Taube wrote:
> This timer driver is using GPT Timer (General Purpose Timer) available
> on almost all i.MX SoCs family.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Cc: Stefano Babic <tsbabic@denx.de>
> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Cc: Jesse Taube <mr.bossman075@gmail.com>

All these ^^^ Cc in commit log are useless in this case, they are useful 
only in the case people are not listed when calling get_maintainers.pl
or because you specifically need them to be in Cc. Your name/e-mail 
should be present as SoB and not as Cc because you're sending the patch, 
so add Signed-off-by instead of Cc for you entry and in case you did 
some modification write it like:

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
Signed-off-by: Jesse Taube <mr.bossman075@gmail.com>
[Jesse: changed this, set this to etc.]

This way authorship is kept, but you write what you have done on this patch.

> ---
>   drivers/timer/Kconfig         |   7 ++
>   drivers/timer/Makefile        |   1 +
>   drivers/timer/imx-gpt-timer.c | 118 ++++++++++++++++++++++++++++++++++
>   3 files changed, 126 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..c4e8c12a42
> --- /dev/null
> +++ b/drivers/timer/imx-gpt-timer.c
> @@ -0,0 +1,118 @@
> +// 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
> +
> +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 int imx_gpt_timer_get_count(struct udevice *dev, u64 *count)
> +{
> +	struct imx_gpt_timer_priv *priv = dev_get_priv(dev);
> +	struct imx_gpt_timer_regs *regs = priv->base;
> +
> +	*count = readl(&regs->cnt);
> +
> +	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 prescaler;
> +	u32 rate, pr;
> +	int ret;
> +
> +	addr = dev_read_addr(dev);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	priv->base = (struct imx_gpt_timer_regs *)addr;
> +
> +	/* TODO: Retrieve clock-source */

This is something that prevents driver from working so a patch like this 
should not be accepted.

> +
> +	/* TODO: Retrieve prescaler */

ditto

> +
> +	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);
> +
> +	/* we set timer prescaler to obtain a 1MHz timer counter frequency */
> +//	pr = (rate / CONFIG_SYS_HZ_CLOCK) - 1;

This ^^^ was another TODO: I've put there, ditto.
Also take care to never leave commented code.

> +	writel(pr, &regs->pr);
> +
> +	/* Set timer frequency to 1MHz */
> +	uc_priv->clock_rate = rate / prescaler;
> +
> +	/* start timer */
> +//	setbits_le32(&regs->cr1, CR1_CEN);

ditto

> +
> +	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_alloc_size = sizeof(struct imx_gpt_timer_priv),

This break builds and makes the patch not bisectable.

> +	.probe = imx_gpt_timer_probe,
> +	.ops = &imx_gpt_timer_ops,
> +};
> +
> 

Best regards
diff mbox series

Patch

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..c4e8c12a42
--- /dev/null
+++ b/drivers/timer/imx-gpt-timer.c
@@ -0,0 +1,118 @@ 
+// 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
+
+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 int imx_gpt_timer_get_count(struct udevice *dev, u64 *count)
+{
+	struct imx_gpt_timer_priv *priv = dev_get_priv(dev);
+	struct imx_gpt_timer_regs *regs = priv->base;
+
+	*count = readl(&regs->cnt);
+
+	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 prescaler;
+	u32 rate, pr;
+	int ret;
+
+	addr = dev_read_addr(dev);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	priv->base = (struct imx_gpt_timer_regs *)addr;
+
+	/* TODO: Retrieve clock-source */
+
+	/* TODO: Retrieve prescaler */
+
+	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);
+
+	/* we set timer prescaler to obtain a 1MHz timer counter frequency */
+//	pr = (rate / CONFIG_SYS_HZ_CLOCK) - 1;
+	writel(pr, &regs->pr);
+
+	/* Set timer frequency to 1MHz */
+	uc_priv->clock_rate = rate / prescaler;
+
+	/* start timer */
+//	setbits_le32(&regs->cr1, CR1_CEN);
+
+	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_alloc_size = sizeof(struct imx_gpt_timer_priv),
+	.probe = imx_gpt_timer_probe,
+	.ops = &imx_gpt_timer_ops,
+};
+