From: Bai Ping <[email protected]>
Add the binding doc for nxp system counter timer module.
Signed-off-by: Bai Ping <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
change v1->v2
- remove the blank line at EOF
change v2->v3
- update the binding example based on the driver change
change v3->v4
- no change
---
.../bindings/timer/nxp,sysctr-timer.txt | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
new file mode 100644
index 000000000000..d57659996d62
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -0,0 +1,25 @@
+NXP System Counter Module(sys_ctr)
+
+The system counter(sys_ctr) is a programmable system counter which provides
+a shared time base to Cortex A15, A7, A53, A73, etc. it is intended for use in
+applications where the counter is always powered and support multiple,
+unrelated clocks. The compare frame inside can be used for timer purpose.
+
+Required properties:
+
+- compatible : should be "nxp,sysctr-timer"
+- reg : Specifies the base physical address and size of the comapre
+ frame and the counter control, read & compare.
+- interrupts : should be the first compare frames' interrupt
+- clocks : Specifies the counter clock.
+- clock-names: Specifies the clock's name of this module
+
+Example:
+
+ system_counter: timer@306a0000 {
+ compatible = "nxp,sysctr-timer";
+ reg = <0x306a0000 0x20000>;/* system-counter-rd & compare */
+ clocks = <&clk_8m>;
+ clock-names = "per";
+ interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+ };
--
2.21.0
From: Bai Ping <[email protected]>
The system counter (sys_ctr) is a programmable system counter
which provides a shared time base to the Cortex A15, A7, A53 etc cores.
It is intended for use in applications where the counter is always
powered on and supports multiple, unrelated clocks. The sys_ctr hardware
supports:
- 56-bit counter width (roll-over time greater than 40 years)
- compare frame(64-bit compare value) contains programmable interrupt
generation when compare value <= counter value.
Signed-off-by: Bai Ping <[email protected]>
---
change v1->v2:
- no change
change v2->v3:
- remove the clocksource, we only need to use this module for timer purpose,
so register it as clockevent is enough.
- use the timer_of_init to init the irq, clock, etc.
- remove some unnecessary comments.
change v3->v4:
- use cached value for CMPCR,
- remove unnecessary timer enabe from set_state_oneshot function.
---
drivers/clocksource/Kconfig | 7 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-imx-sysctr.c | 146 +++++++++++++++++++++++++
3 files changed, 154 insertions(+)
create mode 100644 drivers/clocksource/timer-imx-sysctr.c
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 6bcaa4e2e72c..ee48620a4561 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -616,6 +616,13 @@ config CLKSRC_IMX_TPM
Enable this option to use IMX Timer/PWM Module (TPM) timer as
clocksource.
+config TIMER_IMX_SYS_CTR
+ bool "i.MX system counter timer" if COMPILE_TEST
+ depends on ARCH_MXC
+ select TIMER_OF
+ help
+ Enable this option to use i.MX system counter timer for clockevent.
+
config CLKSRC_ST_LPC
bool "Low power clocksource found in the LPC" if COMPILE_TEST
select TIMER_OF if OF
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 236858fa7fbf..5fba39e81a40 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_CLKSRC_MIPS_GIC) += mips-gic-timer.o
obj-$(CONFIG_CLKSRC_TANGO_XTAL) += timer-tango-xtal.o
obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o
obj-$(CONFIG_CLKSRC_IMX_TPM) += timer-imx-tpm.o
+obj-$(CONFIG_TIMER_IMX_SYS_CTR) += timer-imx-sysctr.o
obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o
obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o
obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
new file mode 100644
index 000000000000..d0428d3189f8
--- /dev/null
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright 2017-2019 NXP
+
+#include <linux/interrupt.h>
+#include <linux/clockchips.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#include "timer-of.h"
+
+#define CMP_OFFSET 0x10000
+
+#define CNTCV_LO 0x8
+#define CNTCV_HI 0xc
+#define CMPCV_LO (CMP_OFFSET + 0x20)
+#define CMPCV_HI (CMP_OFFSET + 0x24)
+#define CMPCR (CMP_OFFSET + 0x2c)
+
+#define SYS_CTR_EN 0x1
+#define SYS_CTR_IRQ_MASK 0x2
+
+static void __iomem *sys_ctr_base;
+static u32 cmpcr;
+
+static void sysctr_timer_enable(bool enable)
+{
+ cmpcr &= ~SYS_CTR_EN;
+ if (enable)
+ cmpcr |= SYS_CTR_EN;
+
+ writel(cmpcr, sys_ctr_base + CMPCR);
+}
+
+static void sysctr_irq_acknowledge(void)
+{
+ /*
+ * clear the enable bit(EN =0) will clear
+ * the status bit(ISTAT = 0), then the interrupt
+ * signal will be negated(acknowledged).
+ */
+ sysctr_timer_enable(false);
+}
+
+static inline u64 sysctr_read_counter(void)
+{
+ u32 cnt_hi, tmp_hi, cnt_lo;
+
+ do {
+ cnt_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
+ cnt_lo = readl_relaxed(sys_ctr_base + CNTCV_LO);
+ tmp_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
+ } while (tmp_hi != cnt_hi);
+
+ return ((u64) cnt_hi << 32) | cnt_lo;
+}
+
+static int sysctr_set_next_event(unsigned long delta,
+ struct clock_event_device *evt)
+{
+ u32 cmp_hi, cmp_lo;
+ u64 next;
+
+ sysctr_timer_enable(false);
+
+ next = sysctr_read_counter();
+
+ next += delta;
+
+ cmp_hi = (next >> 32) & 0x00fffff;
+ cmp_lo = next & 0xffffffff;
+
+ writel_relaxed(cmp_hi, sys_ctr_base + CMPCV_HI);
+ writel_relaxed(cmp_lo, sys_ctr_base + CMPCV_LO);
+
+ sysctr_timer_enable(true);
+
+ return 0;
+}
+
+static int sysctr_set_state_oneshot(struct clock_event_device *evt)
+{
+ return 0;
+}
+
+static int sysctr_set_state_shutdown(struct clock_event_device *evt)
+{
+ sysctr_timer_enable(false);
+
+ return 0;
+}
+
+static irqreturn_t sysctr_timer_interrupt(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = dev_id;
+
+ sysctr_irq_acknowledge();
+
+ evt->event_handler(evt);
+
+ return IRQ_HANDLED;
+}
+
+static struct timer_of to_sysctr = {
+ .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
+ .clkevt = {
+ .name = "i.MX system counter timer",
+ .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ,
+ .set_state_oneshot = sysctr_set_state_oneshot,
+ .set_next_event = sysctr_set_next_event,
+ .set_state_shutdown = sysctr_set_state_shutdown,
+ .rating = 200,
+ },
+ .of_irq = {
+ .handler = sysctr_timer_interrupt,
+ .flags = IRQF_TIMER | IRQF_IRQPOLL,
+ },
+ .of_clk = {
+ .name = "per",
+ },
+};
+
+static void __init sysctr_clockevent_init(void)
+{
+ to_sysctr.clkevt.cpumask = cpumask_of(0);
+
+ clockevents_config_and_register(&to_sysctr.clkevt, timer_of_rate(&to_sysctr),
+ 0xff, 0x7fffffff);
+}
+
+static int __init sysctr_timer_init(struct device_node *np)
+{
+ int ret = 0;
+
+ ret = timer_of_init(np, &to_sysctr);
+ if (ret)
+ return ret;
+
+ sys_ctr_base = timer_of_base(&to_sysctr);
+ cmpcr = readl(sys_ctr_base + CMPCR);
+
+ sysctr_clockevent_init();
+
+ return 0;
+}
+TIMER_OF_DECLARE(sysctr_timer, "nxp,sysctr-timer", sysctr_timer_init);
--
2.21.0
On 21/05/2019 09:18, Jacky Bai wrote:
> From: Bai Ping <[email protected]>
>
> The system counter (sys_ctr) is a programmable system counter
> which provides a shared time base to the Cortex A15, A7, A53 etc cores.
> It is intended for use in applications where the counter is always
> powered on and supports multiple, unrelated clocks. The sys_ctr hardware
> supports:
> - 56-bit counter width (roll-over time greater than 40 years)
The benefit of using more than 32bits on a 32bits system is not proven.
The function to read and build the 56bits value can have a very
significant impact on the performance of your platform.
Using a 32bits counter can be enough if it does not wrap too fast.
Can you consider a 32 bits counter ?
> - compare frame(64-bit compare value) contains programmable interrupt
> generation when compare value <= counter value.
>
> Signed-off-by: Bai Ping <[email protected]>
> ---
> change v1->v2:
> - no change
> change v2->v3:
> - remove the clocksource, we only need to use this module for timer purpose,
> so register it as clockevent is enough.
> - use the timer_of_init to init the irq, clock, etc.
> - remove some unnecessary comments.
> change v3->v4:
> - use cached value for CMPCR,
> - remove unnecessary timer enabe from set_state_oneshot function.
> ---
> drivers/clocksource/Kconfig | 7 ++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-imx-sysctr.c | 146 +++++++++++++++++++++++++
> 3 files changed, 154 insertions(+)
> create mode 100644 drivers/clocksource/timer-imx-sysctr.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 6bcaa4e2e72c..ee48620a4561 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -616,6 +616,13 @@ config CLKSRC_IMX_TPM
> Enable this option to use IMX Timer/PWM Module (TPM) timer as
> clocksource.
>
> +config TIMER_IMX_SYS_CTR
> + bool "i.MX system counter timer" if COMPILE_TEST
> + depends on ARCH_MXC
Do you really need this dep?
> + select TIMER_OF
> + help
> + Enable this option to use i.MX system counter timer for clockevent.
> +
> config CLKSRC_ST_LPC
> bool "Low power clocksource found in the LPC" if COMPILE_TEST
> select TIMER_OF if OF
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 236858fa7fbf..5fba39e81a40 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -74,6 +74,7 @@ obj-$(CONFIG_CLKSRC_MIPS_GIC) += mips-gic-timer.o
> obj-$(CONFIG_CLKSRC_TANGO_XTAL) += timer-tango-xtal.o
> obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o
> obj-$(CONFIG_CLKSRC_IMX_TPM) += timer-imx-tpm.o
> +obj-$(CONFIG_TIMER_IMX_SYS_CTR) += timer-imx-sysctr.o
> obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o
> obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o
> obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
> diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
> new file mode 100644
> index 000000000000..d0428d3189f8
> --- /dev/null
> +++ b/drivers/clocksource/timer-imx-sysctr.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Copyright 2017-2019 NXP
> +
> +#include <linux/interrupt.h>
> +#include <linux/clockchips.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +#include "timer-of.h"
> +
> +#define CMP_OFFSET 0x10000
> +
> +#define CNTCV_LO 0x8
> +#define CNTCV_HI 0xc
> +#define CMPCV_LO (CMP_OFFSET + 0x20)
> +#define CMPCV_HI (CMP_OFFSET + 0x24)
> +#define CMPCR (CMP_OFFSET + 0x2c)
> +
> +#define SYS_CTR_EN 0x1
> +#define SYS_CTR_IRQ_MASK 0x2
> +
> +static void __iomem *sys_ctr_base;
> +static u32 cmpcr;
> +
> +static void sysctr_timer_enable(bool enable)
> +{
> + cmpcr &= ~SYS_CTR_EN;
Do the computation after reading the value in the init function...
> + if (enable)
> + cmpcr |= SYS_CTR_EN;
... then
writel(enable ? cmpcr | SYS_CTR_EN : cmpcr, sys_ctr_base);
> + writel(cmpcr, sys_ctr_base + CMPCR);
> +}
> +
> +static void sysctr_irq_acknowledge(void)
> +{
> + /*
> + * clear the enable bit(EN =0) will clear
> + * the status bit(ISTAT = 0), then the interrupt
> + * signal will be negated(acknowledged).
> + */
> + sysctr_timer_enable(false);
> +}
> +
> +static inline u64 sysctr_read_counter(void)
> +{
> + u32 cnt_hi, tmp_hi, cnt_lo;
> +
> + do {
> + cnt_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
> + cnt_lo = readl_relaxed(sys_ctr_base + CNTCV_LO);
> + tmp_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
> + } while (tmp_hi != cnt_hi);
> +
> + return ((u64) cnt_hi << 32) | cnt_lo;
> +}
> +
> +static int sysctr_set_next_event(unsigned long delta,
> + struct clock_event_device *evt)
> +{
> + u32 cmp_hi, cmp_lo;
> + u64 next;
> +
> + sysctr_timer_enable(false);
> +
> + next = sysctr_read_counter();
> +
> + next += delta;
> +
> + cmp_hi = (next >> 32) & 0x00fffff;
> + cmp_lo = next & 0xffffffff;
> +
> + writel_relaxed(cmp_hi, sys_ctr_base + CMPCV_HI);
> + writel_relaxed(cmp_lo, sys_ctr_base + CMPCV_LO);
> +
> + sysctr_timer_enable(true);
> +
> + return 0;
> +}
> +
> +static int sysctr_set_state_oneshot(struct clock_event_device *evt)
> +{
> + return 0;
> +}
> +
> +static int sysctr_set_state_shutdown(struct clock_event_device *evt)
> +{
> + sysctr_timer_enable(false);
> +
> + return 0;
> +}
> +
> +static irqreturn_t sysctr_timer_interrupt(int irq, void *dev_id)
> +{
> + struct clock_event_device *evt = dev_id;
> +
> + sysctr_irq_acknowledge();
> +
> + evt->event_handler(evt);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static struct timer_of to_sysctr = {
> + .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
> + .clkevt = {
> + .name = "i.MX system counter timer",
> + .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ,
> + .set_state_oneshot = sysctr_set_state_oneshot,
> + .set_next_event = sysctr_set_next_event,
> + .set_state_shutdown = sysctr_set_state_shutdown,
> + .rating = 200,
> + },
> + .of_irq = {
> + .handler = sysctr_timer_interrupt,
> + .flags = IRQF_TIMER | IRQF_IRQPOLL,
> + },
> + .of_clk = {
> + .name = "per",
> + },
> +};
> +
> +static void __init sysctr_clockevent_init(void)
> +{
> + to_sysctr.clkevt.cpumask = cpumask_of(0);
> +
> + clockevents_config_and_register(&to_sysctr.clkevt, timer_of_rate(&to_sysctr),
> + 0xff, 0x7fffffff);
> +}
> +
> +static int __init sysctr_timer_init(struct device_node *np)
> +{
> + int ret = 0;
> +
> + ret = timer_of_init(np, &to_sysctr);
> + if (ret)
> + return ret;
> +
> + sys_ctr_base = timer_of_base(&to_sysctr);
> + cmpcr = readl(sys_ctr_base + CMPCR);
> +
> + sysctr_clockevent_init();
> +
> + return 0;
> +}
> +TIMER_OF_DECLARE(sysctr_timer, "nxp,sysctr-timer", sysctr_timer_init);
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
> -----Original Message-----
> From: Daniel Lezcano [mailto:[email protected]]
> Sent: Tuesday, May 21, 2019 6:08 PM
> To: Jacky Bai <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; Aisheng Dong
> <[email protected]>
> Cc: [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v4 2/2] driver: clocksource: Add nxp system counter timer
> driver support
>
> On 21/05/2019 09:18, Jacky Bai wrote:
> > From: Bai Ping <[email protected]>
> >
> > The system counter (sys_ctr) is a programmable system counter which
> > provides a shared time base to the Cortex A15, A7, A53 etc cores.
> > It is intended for use in applications where the counter is always
> > powered on and supports multiple, unrelated clocks. The sys_ctr
> > hardware
> > supports:
> > - 56-bit counter width (roll-over time greater than 40 years)
>
> The benefit of using more than 32bits on a 32bits system is not proven.
>
It is mainly used on 64bit ARMv8 system.
> The function to read and build the 56bits value can have a very significant
> impact on the performance of your platform.
>
> Using a 32bits counter can be enough if it does not wrap too fast.
>
> Can you consider a 32 bits counter ?
this counter is ARMv8 arch timer's counter source. As it also has timer function, so I choose it
to act as a broadcast timer for cpuidle. The timer interrupt can only be triggered when 'compare[55:0] <= counter[55:0]'.
So you mean that only use the lower 32bit to implement this timer? If so, I can change to use only the lower 32bit.
>
> > - compare frame(64-bit compare value) contains programmable interrupt
> > generation when compare value <= counter value.
> >
> > Signed-off-by: Bai Ping <[email protected]>
> > ---
> > change v1->v2:
> > - no change
> > change v2->v3:
> > - remove the clocksource, we only need to use this module for timer
> purpose,
> > so register it as clockevent is enough.
> > - use the timer_of_init to init the irq, clock, etc.
> > - remove some unnecessary comments.
> > change v3->v4:
> > - use cached value for CMPCR,
> > - remove unnecessary timer enabe from set_state_oneshot function.
> > ---
> > drivers/clocksource/Kconfig | 7 ++
> > drivers/clocksource/Makefile | 1 +
> > drivers/clocksource/timer-imx-sysctr.c | 146
> > +++++++++++++++++++++++++
> > 3 files changed, 154 insertions(+)
> > create mode 100644 drivers/clocksource/timer-imx-sysctr.c
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 6bcaa4e2e72c..ee48620a4561 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -616,6 +616,13 @@ config CLKSRC_IMX_TPM
> > Enable this option to use IMX Timer/PWM Module (TPM) timer as
> > clocksource.
> >
> > +config TIMER_IMX_SYS_CTR
> > + bool "i.MX system counter timer" if COMPILE_TEST
> > + depends on ARCH_MXC
>
> Do you really need this dep?
>
Not really necessary, I can remove it.
> > + select TIMER_OF
> > + help
> > + Enable this option to use i.MX system counter timer for clockevent.
> > +
> > config CLKSRC_ST_LPC
> > bool "Low power clocksource found in the LPC" if COMPILE_TEST
> > select TIMER_OF if OF
> > diff --git a/drivers/clocksource/Makefile
> > b/drivers/clocksource/Makefile index 236858fa7fbf..5fba39e81a40 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -74,6 +74,7 @@ obj-$(CONFIG_CLKSRC_MIPS_GIC) +=
> mips-gic-timer.o
> > obj-$(CONFIG_CLKSRC_TANGO_XTAL) += timer-tango-xtal.o
> > obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o
> > obj-$(CONFIG_CLKSRC_IMX_TPM) += timer-imx-tpm.o
> > +obj-$(CONFIG_TIMER_IMX_SYS_CTR) += timer-imx-sysctr.o
> > obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o
> > obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o
> > obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
> > diff --git a/drivers/clocksource/timer-imx-sysctr.c
> > b/drivers/clocksource/timer-imx-sysctr.c
> > new file mode 100644
> > index 000000000000..d0428d3189f8
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-imx-sysctr.c
> > @@ -0,0 +1,146 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +//
> > +// Copyright 2017-2019 NXP
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +
> > +#include "timer-of.h"
> > +
> > +#define CMP_OFFSET 0x10000
> > +
> > +#define CNTCV_LO 0x8
> > +#define CNTCV_HI 0xc
> > +#define CMPCV_LO (CMP_OFFSET + 0x20)
> > +#define CMPCV_HI (CMP_OFFSET + 0x24)
> > +#define CMPCR (CMP_OFFSET + 0x2c)
> > +
> > +#define SYS_CTR_EN 0x1
> > +#define SYS_CTR_IRQ_MASK 0x2
> > +
> > +static void __iomem *sys_ctr_base;
> > +static u32 cmpcr;
> > +
> > +static void sysctr_timer_enable(bool enable) {
> > + cmpcr &= ~SYS_CTR_EN;
>
> Do the computation after reading the value in the init function...
>
> > + if (enable)
> > + cmpcr |= SYS_CTR_EN;
>
> ... then
>
> writel(enable ? cmpcr | SYS_CTR_EN : cmpcr, sys_ctr_base);
>
Ok, thank you.
BR
Jacky Bai
> > + writel(cmpcr, sys_ctr_base + CMPCR); }
> > +
> > +static void sysctr_irq_acknowledge(void) {
> > + /*
> > + * clear the enable bit(EN =0) will clear
> > + * the status bit(ISTAT = 0), then the interrupt
> > + * signal will be negated(acknowledged).
> > + */
> > + sysctr_timer_enable(false);
> > +}
> > +
> > +static inline u64 sysctr_read_counter(void) {
> > + u32 cnt_hi, tmp_hi, cnt_lo;
> > +
> > + do {
> > + cnt_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
> > + cnt_lo = readl_relaxed(sys_ctr_base + CNTCV_LO);
> > + tmp_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
> > + } while (tmp_hi != cnt_hi);
> > +
> > + return ((u64) cnt_hi << 32) | cnt_lo; }
> > +
> > +static int sysctr_set_next_event(unsigned long delta,
> > + struct clock_event_device *evt)
> > +{
> > + u32 cmp_hi, cmp_lo;
> > + u64 next;
> > +
> > + sysctr_timer_enable(false);
> > +
> > + next = sysctr_read_counter();
> > +
> > + next += delta;
> > +
> > + cmp_hi = (next >> 32) & 0x00fffff;
> > + cmp_lo = next & 0xffffffff;
> > +
> > + writel_relaxed(cmp_hi, sys_ctr_base + CMPCV_HI);
> > + writel_relaxed(cmp_lo, sys_ctr_base + CMPCV_LO);
> > +
> > + sysctr_timer_enable(true);
> > +
> > + return 0;
> > +}
> > +
> > +static int sysctr_set_state_oneshot(struct clock_event_device *evt) {
> > + return 0;
> > +}
> > +
> > +static int sysctr_set_state_shutdown(struct clock_event_device *evt)
> > +{
> > + sysctr_timer_enable(false);
> > +
> > + return 0;
> > +}
> > +
> > +static irqreturn_t sysctr_timer_interrupt(int irq, void *dev_id) {
> > + struct clock_event_device *evt = dev_id;
> > +
> > + sysctr_irq_acknowledge();
> > +
> > + evt->event_handler(evt);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static struct timer_of to_sysctr = {
> > + .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
> > + .clkevt = {
> > + .name = "i.MX system counter timer",
> > + .features = CLOCK_EVT_FEAT_ONESHOT |
> CLOCK_EVT_FEAT_DYNIRQ,
> > + .set_state_oneshot = sysctr_set_state_oneshot,
> > + .set_next_event = sysctr_set_next_event,
> > + .set_state_shutdown = sysctr_set_state_shutdown,
> > + .rating = 200,
> > + },
> > + .of_irq = {
> > + .handler = sysctr_timer_interrupt,
> > + .flags = IRQF_TIMER | IRQF_IRQPOLL,
> > + },
> > + .of_clk = {
> > + .name = "per",
> > + },
> > +};
> > +
> > +static void __init sysctr_clockevent_init(void) {
> > + to_sysctr.clkevt.cpumask = cpumask_of(0);
> > +
> > + clockevents_config_and_register(&to_sysctr.clkevt,
> timer_of_rate(&to_sysctr),
> > + 0xff, 0x7fffffff);
> > +}
> > +
> > +static int __init sysctr_timer_init(struct device_node *np) {
> > + int ret = 0;
> > +
> > + ret = timer_of_init(np, &to_sysctr);
> > + if (ret)
> > + return ret;
> > +
> > + sys_ctr_base = timer_of_base(&to_sysctr);
> > + cmpcr = readl(sys_ctr_base + CMPCR);
> > +
> > + sysctr_clockevent_init();
> > +
> > + return 0;
> > +}
> > +TIMER_OF_DECLARE(sysctr_timer, "nxp,sysctr-timer",
> > +sysctr_timer_init);
> >
>
>
> --
On 21/05/2019 14:01, Jacky Bai wrote:
>
>> -----Original Message-----
>> From: Daniel Lezcano [mailto:[email protected]]
>> Sent: Tuesday, May 21, 2019 6:08 PM
>> To: Jacky Bai <[email protected]>; [email protected]; [email protected];
>> [email protected]; [email protected]; Aisheng Dong
>> <[email protected]>
>> Cc: [email protected]; [email protected]; dl-linux-imx
>> <[email protected]>
>> Subject: Re: [PATCH v4 2/2] driver: clocksource: Add nxp system counter timer
>> driver support
>>
>> On 21/05/2019 09:18, Jacky Bai wrote:
>>> From: Bai Ping <[email protected]>
>>>
>>> The system counter (sys_ctr) is a programmable system counter which
>>> provides a shared time base to the Cortex A15, A7, A53 etc cores.
>>> It is intended for use in applications where the counter is always
>>> powered on and supports multiple, unrelated clocks. The sys_ctr
>>> hardware
>>> supports:
>>> - 56-bit counter width (roll-over time greater than 40 years)
>>
>> The benefit of using more than 32bits on a 32bits system is not proven.
>>
>
> It is mainly used on 64bit ARMv8 system.
Oh, ok. Fair enough.
>
>> The function to read and build the 56bits value can have a very significant
>> impact on the performance of your platform.
>>
>> Using a 32bits counter can be enough if it does not wrap too fast.
>>
>> Can you consider a 32 bits counter ?
>
> this counter is ARMv8 arch timer's counter source. As it also has timer function, so I choose it
> to act as a broadcast timer for cpuidle. The timer interrupt can only be triggered when 'compare[55:0] <= counter[55:0]'.
> So you mean that only use the lower 32bit to implement this timer? If so, I can change to use only the lower 32bit.
IMO it is preferable but you decide (probably compare with how long it
takes to wrap when 32bits).
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
> -----Original Message-----
> From: Daniel Lezcano [mailto:[email protected]]
> Sent: Tuesday, May 21, 2019 8:20 PM
> To: Jacky Bai <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; Aisheng Dong
> <[email protected]>
> Cc: [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v4 2/2] driver: clocksource: Add nxp system counter timer
> driver support
>
> On 21/05/2019 14:01, Jacky Bai wrote:
> >
> >> -----Original Message-----
> >> From: Daniel Lezcano [mailto:[email protected]]
> >> Sent: Tuesday, May 21, 2019 6:08 PM
> >> To: Jacky Bai <[email protected]>; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> Aisheng Dong <[email protected]>
> >> Cc: [email protected]; [email protected];
> >> dl-linux-imx <[email protected]>
> >> Subject: Re: [PATCH v4 2/2] driver: clocksource: Add nxp system
> >> counter timer driver support
> >>
> >> On 21/05/2019 09:18, Jacky Bai wrote:
> >>> From: Bai Ping <[email protected]>
> >>>
> >>> The system counter (sys_ctr) is a programmable system counter which
> >>> provides a shared time base to the Cortex A15, A7, A53 etc cores.
> >>> It is intended for use in applications where the counter is always
> >>> powered on and supports multiple, unrelated clocks. The sys_ctr
> >>> hardware
> >>> supports:
> >>> - 56-bit counter width (roll-over time greater than 40 years)
> >>
> >> The benefit of using more than 32bits on a 32bits system is not proven.
> >>
> >
> > It is mainly used on 64bit ARMv8 system.
>
> Oh, ok. Fair enough.
>
> >
> >> The function to read and build the 56bits value can have a very
> >> significant impact on the performance of your platform.
> >>
> >> Using a 32bits counter can be enough if it does not wrap too fast.
> >>
> >> Can you consider a 32 bits counter ?
> >
> > this counter is ARMv8 arch timer's counter source. As it also has
> > timer function, so I choose it to act as a broadcast timer for cpuidle. The
> timer interrupt can only be triggered when 'compare[55:0] <= counter[55:0]'.
> > So you mean that only use the lower 32bit to implement this timer? If so, I
> can change to use only the lower 32bit.
>
> IMO it is preferable but you decide (probably compare with how long it takes
> to wrap when 32bits).
>
Normally, it is driven by fixed 8MHz clock. 32bit has risk, when the lower 32bit wrapped,
then the 'counter value >= compare value' is true if only use 32bit for timer, then IRQ will
Pending all the time. I will keep use the whole 56bit.
>
>