Add system timer driver for Mediatek SoCs

Add system timer driver for Mediatek SoCs

[Stanley Chu]

This patch adds a new driver for system timer on the Mediatek SoCs.

[PATCH 1/2] dt-bindings: Add mtk-systimer bindings

[Stanley Chu]

Add binding documentation for the System Timer driver of
the Mediatek SoCs.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 .../bindings/timer/mediatek,mtk-systimer.txt       |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt

diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt
new file mode 100644
index 0000000..735a6fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt
@@ -0,0 +1,18 @@
+Mediatek System Timers
+----------------------
+
+Required properties:
+- compatible: should contain
+	"mediatek,sys_timer" for those platforms which support system timer.
+- reg: Mandatory. The location and length for system timer registers.
+- clock-frequency: Optional. If not assigned specifically, default
+	value 13000000 (means 13 MHz) would be applied.
+
+Examples:
+
+	sys_timer@10017000 {
+		compatible = "mediatek,sys_timer";
+		reg = <0 0x10017000 0 0x1000>;
+		interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
+		clock-frequency = <13000000>;
+	};
-- 
1.7.9.5

[PATCH 2/2] clocksource/drivers/mtk_systimer: Add support for Mediatek SoCs

[Stanley Chu]

This patch adds a new clock event for the timer
found on the Mediatek SoCs.

The Mediatek System Timer has several 32-bit timers.
Only one timer is used by this driver as a clock event
supporting oneshot events.

The System Timer can be run with two clocks. A 13 MHz system
clock and the RTC clock running at 32 KHz. This implementation
uses the system clock with no clock source divider.

The interrupts are shared between the different timers.
We just enable one interrupt for the clock event. The clock
event timer is used by all cores.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/clocksource/Kconfig        |   13 ++-
 drivers/clocksource/Makefile       |    1 +
 drivers/clocksource/mtk_systimer.c |  177 ++++++++++++++++++++++++++++++++++++
 3 files changed, 189 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clocksource/mtk_systimer.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index dec0dd8..362c110 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -442,12 +442,21 @@ config SYS_SUPPORTS_SH_CMT
         bool
 
 config MTK_TIMER
-	bool "Mediatek timer driver" if COMPILE_TEST
+	bool "Mediatek general purpose timer driver" if COMPILE_TEST
 	depends on HAS_IOMEM
 	select TIMER_OF
 	select CLKSRC_MMIO
 	help
-	  Support for Mediatek timer driver.
+	  Support for Mediatek General Purpose Timer (GPT) driver.
+
+config MTK_TIMER_SYSTIMER
+	bool "Mediatek system timer driver" if COMPILE_TEST
+	depends on HAS_IOMEM
+	select TIMER_OF
+	select CLKSRC_MMIO
+	help
+	  Support for Mediatek System Timer (sys_timer) driver used as
+	  a clock event supporting oneshot events.
 
 config SPRD_TIMER
 	bool "Spreadtrum timer driver" if EXPERT
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 00caf37..cdd34b2 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_FSL_FTM_TIMER)	+= fsl_ftm_timer.o
 obj-$(CONFIG_VF_PIT_TIMER)	+= vf_pit_timer.o
 obj-$(CONFIG_CLKSRC_QCOM)	+= qcom-timer.o
 obj-$(CONFIG_MTK_TIMER)		+= mtk_timer.o
+obj-$(CONFIG_MTK_TIMER_SYSTIMER)	+= mtk_systimer.o
 obj-$(CONFIG_CLKSRC_PISTACHIO)	+= time-pistachio.o
 obj-$(CONFIG_CLKSRC_TI_32K)	+= timer-ti-32k.o
 obj-$(CONFIG_CLKSRC_NPS)	+= timer-nps.o
diff --git a/drivers/clocksource/mtk_systimer.c b/drivers/clocksource/mtk_systimer.c
new file mode 100644
index 0000000..06e74ce
--- /dev/null
+++ b/drivers/clocksource/mtk_systimer.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+//  Copyright (C) 2018 MediaTek Inc.
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/jiffies.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <clocksource/arm_arch_timer.h>
+
+#define STMR_CLKEVT_DEFAULT_RATE  (13000000)
+
+/* registers */
+#define STMR_CON                  (0x0)
+#define STMR_VAL                  (0x4)
+
+/* STMR_CON */
+#define STMR_CON_EN               BIT(0)
+#define STMR_CON_IRQ_EN           BIT(1)
+#define STMR_CON_IRQ_CLR          BIT(4)
+
+struct mtk_stmr_clkevt_device {
+	void __iomem *base;
+	struct clock_event_device dev;
+};
+
+static inline struct mtk_stmr_clkevt_device*
+	to_mtk_clkevt_device(struct clock_event_device *c)
+{
+	return container_of(c, struct mtk_stmr_clkevt_device, dev);
+}
+
+static void mtk_stmr_reset(struct mtk_stmr_clkevt_device *evt)
+{
+	/* Clear IRQ */
+	writel(STMR_CON_IRQ_CLR | STMR_CON_EN, evt->base + STMR_CON);
+
+	/* Reset counter */
+	writel(0, evt->base + STMR_VAL);
+
+	/* Disable timer */
+	writel(0, evt->base + STMR_CON);
+}
+
+static void mtk_stmr_ack_irq(struct mtk_stmr_clkevt_device *evt)
+{
+	mtk_stmr_reset(evt);
+}
+
+static irqreturn_t mtk_stmr_handler(int irq, void *priv)
+{
+	struct mtk_stmr_clkevt_device *evt = priv;
+
+	mtk_stmr_ack_irq(evt);
+	evt->dev.event_handler(&evt->dev);
+
+	return IRQ_HANDLED;
+}
+
+static int mtk_stmr_clkevt_next_event(unsigned long ticks,
+				      struct clock_event_device *dev)
+{
+	struct mtk_stmr_clkevt_device *evt = to_mtk_clkevt_device(dev);
+
+	/*
+	 * reset timer first because we do not expect interrupt is triggered
+	 * by old compare value.
+	 */
+	mtk_stmr_reset(evt);
+
+	writel(STMR_CON_EN, evt->base + STMR_CON);
+
+	writel(ticks, evt->base + STMR_VAL);
+
+	writel(STMR_CON_EN | STMR_CON_IRQ_EN, evt->base + STMR_CON);
+
+	return 0;
+}
+
+static int mtk_stmr_clkevt_shutdown(struct clock_event_device *dev)
+{
+	struct mtk_stmr_clkevt_device *evt = to_mtk_clkevt_device(dev);
+
+	mtk_stmr_reset(evt);
+
+	return 0;
+}
+
+static int mtk_stmr_clkevt_resume(struct clock_event_device *clk)
+{
+	return mtk_stmr_clkevt_shutdown(clk);
+}
+
+static int mtk_stmr_clkevt_oneshot(struct clock_event_device *clk)
+{
+	return 0;
+}
+
+static int __init mtk_stmr_init(struct device_node *node)
+{
+	struct mtk_stmr_clkevt_device *evt;
+	struct resource res;
+	u32 freq;
+
+	evt = kzalloc(sizeof(*evt), GFP_KERNEL);
+	if (!evt)
+		return -ENOMEM;
+
+	evt->dev.name = "mtk-clkevt";
+	evt->dev.shift = 32;
+	evt->dev.rating = 300;
+	evt->dev.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
+	evt->dev.set_state_shutdown = mtk_stmr_clkevt_shutdown;
+	evt->dev.set_state_oneshot = mtk_stmr_clkevt_oneshot;
+	evt->dev.tick_resume = mtk_stmr_clkevt_resume;
+	evt->dev.set_next_event = mtk_stmr_clkevt_next_event;
+	evt->dev.cpumask = cpu_possible_mask;
+
+	evt->base = of_iomap(node, 0);
+	if (IS_ERR(evt->base)) {
+		pr_err("Can't get resource\n");
+		goto err_kzalloc;
+	}
+
+	mtk_stmr_reset(evt);
+
+	evt->dev.irq = irq_of_parse_and_map(node, 0);
+	if (evt->dev.irq <= 0) {
+		pr_err("Can't parse IRQ\n");
+		goto err_mem;
+	}
+
+	if (of_property_read_u32(node, "clock-frequency", &freq)) {
+		pr_err("Can't get clk rate\n");
+		freq = STMR_CLKEVT_DEFAULT_RATE;
+	}
+
+	evt->dev.mult = div_sc(freq, NSEC_PER_SEC, evt->dev.shift);
+	evt->dev.max_delta_ns = clockevent_delta2ns(0xffffffff, &evt->dev);
+	evt->dev.min_delta_ns = clockevent_delta2ns(3, &evt->dev);
+
+	clockevents_register_device(&evt->dev);
+
+	if (request_irq(evt->dev.irq, mtk_stmr_handler,
+			IRQF_TIMER | IRQF_IRQPOLL |
+			IRQF_TRIGGER_HIGH | IRQF_PERCPU,
+			"mtk-clkevt", evt)) {
+		pr_err("failed to setup irq %d\n", evt->dev.irq);
+		goto err_mem;
+	}
+
+	pr_info("mtk_stmr: base=0x%lx, irq=%d, freq=%d, hz=%d\n",
+		(unsigned long)evt->base, evt->dev.irq, freq, HZ);
+
+	return 0;
+
+err_mem:
+	iounmap(evt->base);
+	of_address_to_resource(node, 0, &res);
+	release_mem_region(res.start, resource_size(&res));
+err_kzalloc:
+	kfree(evt);
+
+	return -EINVAL;
+}
+
+TIMER_OF_DECLARE(mtk_systimer, "mediatek,sys_timer", mtk_stmr_init);
-- 
1.7.9.5

Re: Add system timer driver for Mediatek SoCs

[Daniel Lezcano]

On 25/06/2018 09:09, Stanley Chu wrote:
> This patch adds a new driver for system timer on the Mediatek SoCs.

Please elaborate why we need yet another timer.

Is this timer present on all Mediatek platform ? Does it always co-exist
with the existing one ?




-- 
 <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

Re: Add system timer driver for Mediatek SoCs

[Stanley Chu]

On Mon, 2018-06-25 at 09:34 +0200, Daniel Lezcano wrote:
> On 25/06/2018 09:09, Stanley Chu wrote:
> > This patch adds a new driver for system timer on the Mediatek SoCs.
> 
> Please elaborate why we need yet another timer.
> 
> Is this timer present on all Mediatek platform ? Does it always co-exist
> with the existing one ?
> 
> 
> 
> 
Hi Daniel,

This new "system timer" only exists on recent Mediatek platforms,
for example, MT6765. We will upstream driver first, and then update
device tree on MT6765 later.

System timer is designed and optimized as a SoC timer for
tick-broadcasting. Besides timer IP has simpler register manipulation
and friendly low-power design.

There is no plan to remove existed "General Purpose Timer" now.

Thanks.
Stanley Chu

Re: [PATCH 1/2] dt-bindings: Add mtk-systimer bindings

[Daniel Lezcano]

On 25/06/2018 09:10, Stanley Chu wrote:
> Add binding documentation for the System Timer driver of
> the Mediatek SoCs.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
>  .../bindings/timer/mediatek,mtk-systimer.txt       |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt
> 
> diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt
> new file mode 100644
> index 0000000..735a6fd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt
> @@ -0,0 +1,18 @@
> +Mediatek System Timers
> +----------------------
> +
> +Required properties:
> +- compatible: should contain
> +	"mediatek,sys_timer" for those platforms which support system timer.
> +- reg: Mandatory. The location and length for system timer registers.
> +- clock-frequency: Optional. If not assigned specifically, default
> +	value 13000000 (means 13 MHz) would be applied.

The driver tells there is an error if it is not defined, so it can't be
optional.

> +Examples:
> +
> +	sys_timer@10017000 {
> +		compatible = "mediatek,sys_timer";
> +		reg = <0 0x10017000 0 0x1000>;
> +		interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
> +		clock-frequency = <13000000>;

Replace it by a phandle to a 'fixed-clock'.

eg.

clocks = <&myclk>;


> +	};

myclk: myclk {
	#clock-cells = <0>;
	compatible = "fixed-clock";
        clock-frequency = <13000000>;
};

Thanks

  -- DAniel




-- 
 <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

Re: [PATCH 1/2] dt-bindings: Add mtk-systimer bindings

[Stanley Chu]

On Mon, 2018-06-25 at 23:53 +0200, Daniel Lezcano wrote:
> On 25/06/2018 09:10, Stanley Chu wrote:
> > Add binding documentation for the System Timer driver of
> > the Mediatek SoCs.
> > 
> > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> > ---
> >  .../bindings/timer/mediatek,mtk-systimer.txt       |   18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt
> > new file mode 100644
> > index 0000000..735a6fd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-systimer.txt
> > @@ -0,0 +1,18 @@
> > +Mediatek System Timers
> > +----------------------
> > +
> > +Required properties:
> > +- compatible: should contain
> > +	"mediatek,sys_timer" for those platforms which support system timer.
> > +- reg: Mandatory. The location and length for system timer registers.
> > +- clock-frequency: Optional. If not assigned specifically, default
> > +	value 13000000 (means 13 MHz) would be applied.
> 
> The driver tells there is an error if it is not defined, so it can't be
> optional.
> 
> > +Examples:
> > +
> > +	sys_timer@10017000 {
> > +		compatible = "mediatek,sys_timer";
> > +		reg = <0 0x10017000 0 0x1000>;
> > +		interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
> > +		clock-frequency = <13000000>;
> 
> Replace it by a phandle to a 'fixed-clock'.
> 
> eg.
> 
> clocks = <&myclk>;
> 
> 
> > +	};
> 
> myclk: myclk {
> 	#clock-cells = <0>;
> 	compatible = "fixed-clock";
>         clock-frequency = <13000000>;
> };
> 
> Thanks
> 
>   -- DAniel
> 
> 
> 
> 
Hi Daniel,

13 MHz clock is necessary. We will fix it in v2 by using system's
fixed-clock.

Thanks.
Stanley Chu

Re: Add system timer driver for Mediatek SoCs

[Daniel Lezcano]

On 25/06/2018 11:07, Stanley Chu wrote:
> On Mon, 2018-06-25 at 09:34 +0200, Daniel Lezcano wrote:
>> On 25/06/2018 09:09, Stanley Chu wrote:
>>> This patch adds a new driver for system timer on the Mediatek SoCs.
>>
>> Please elaborate why we need yet another timer.
>>
>> Is this timer present on all Mediatek platform ? Does it always co-exist
>> with the existing one ?
>>
>>
>>
>>
> Hi Daniel,
> 
> This new "system timer" only exists on recent Mediatek platforms,
> for example, MT6765. We will upstream driver first, and then update
> device tree on MT6765 later.
> 
> System timer is designed and optimized as a SoC timer for
> tick-broadcasting. Besides timer IP has simpler register manipulation
> and friendly low-power design.
> 
> There is no plan to remove existed "General Purpose Timer" now.

It is unclear what "General Purpose Timer" and "System Timer" differ
except they are different timers running on different platforms.

Merge them into a single timer-mediatek.c file.

-- 
 <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

Re: [PATCH 2/2] clocksource/drivers/mtk_systimer: Add support for Mediatek SoCs

[Daniel Lezcano]

On 25/06/2018 09:10, Stanley Chu wrote:
> This patch adds a new clock event for the timer
> found on the Mediatek SoCs.
> 
> The Mediatek System Timer has several 32-bit timers.
> Only one timer is used by this driver as a clock event
> supporting oneshot events.
> 
> The System Timer can be run with two clocks. A 13 MHz system
> clock and the RTC clock running at 32 KHz. This implementation
> uses the system clock with no clock source divider.
> 
> The interrupts are shared between the different timers.
> We just enable one interrupt for the clock event. The clock
> event timer is used by all cores.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
>  drivers/clocksource/Kconfig        |   13 ++-
>  drivers/clocksource/Makefile       |    1 +
>  drivers/clocksource/mtk_systimer.c |  177 ++++++++++++++++++++++++++++++++++++
>

>  3 files changed, 189 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/clocksource/mtk_systimer.c
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index dec0dd8..362c110 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -442,12 +442,21 @@ config SYS_SUPPORTS_SH_CMT
>          bool
>  
>  config MTK_TIMER
> -	bool "Mediatek timer driver" if COMPILE_TEST
> +	bool "Mediatek general purpose timer driver" if COMPILE_TEST
>  	depends on HAS_IOMEM
>  	select TIMER_OF
>  	select CLKSRC_MMIO
>  	help
> -	  Support for Mediatek timer driver.
> +	  Support for Mediatek General Purpose Timer (GPT) driver.
> +
> +config MTK_TIMER_SYSTIMER
> +	bool "Mediatek system timer driver" if COMPILE_TEST
> +	depends on HAS_IOMEM
> +	select TIMER_OF
> +	select CLKSRC_MMIO
> +	help
> +	  Support for Mediatek System Timer (sys_timer) driver used as
> +	  a clock event supporting oneshot events.
>  
>  config SPRD_TIMER
>  	bool "Spreadtrum timer driver" if EXPERT
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 00caf37..cdd34b2 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_FSL_FTM_TIMER)	+= fsl_ftm_timer.o
>  obj-$(CONFIG_VF_PIT_TIMER)	+= vf_pit_timer.o
>  obj-$(CONFIG_CLKSRC_QCOM)	+= qcom-timer.o
>  obj-$(CONFIG_MTK_TIMER)		+= mtk_timer.o
> +obj-$(CONFIG_MTK_TIMER_SYSTIMER)	+= mtk_systimer.o
>  obj-$(CONFIG_CLKSRC_PISTACHIO)	+= time-pistachio.o
>  obj-$(CONFIG_CLKSRC_TI_32K)	+= timer-ti-32k.o
>  obj-$(CONFIG_CLKSRC_NPS)	+= timer-nps.o
> diff --git a/drivers/clocksource/mtk_systimer.c b/drivers/clocksource/mtk_systimer.c
> new file mode 100644
> index 0000000..06e74ce
> --- /dev/null
> +++ b/drivers/clocksource/mtk_systimer.c
> @@ -0,0 +1,177 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +//  Copyright (C) 2018 MediaTek Inc.
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
> +#include <linux/jiffies.h>
> +#include <linux/clockchips.h>
> +#include <linux/clocksource.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <clocksource/arm_arch_timer.h>

Do a cleanup with the headers. They are not all necessary.

> +#define STMR_CLKEVT_DEFAULT_RATE  (13000000)
> +
> +/* registers */
> +#define STMR_CON                  (0x0)
> +#define STMR_VAL                  (0x4)
> +
> +/* STMR_CON */
> +#define STMR_CON_EN               BIT(0)
> +#define STMR_CON_IRQ_EN           BIT(1)
> +#define STMR_CON_IRQ_CLR          BIT(4)
> +
> +struct mtk_stmr_clkevt_device {
> +	void __iomem *base;
> +	struct clock_event_device dev;
> +};

Use the timer_of structure, that will save you some lines of code as
well as redundant structure definition.

> +static inline struct mtk_stmr_clkevt_device*
> +	to_mtk_clkevt_device(struct clock_event_device *c)> +{
> +	return container_of(c, struct mtk_stmr_clkevt_device, dev);
> +}
> +
> +static void mtk_stmr_reset(struct mtk_stmr_clkevt_device *evt)
> +{
> +	/* Clear IRQ */
> +	writel(STMR_CON_IRQ_CLR | STMR_CON_EN, evt->base + STMR_CON);
> +
> +	/* Reset counter */
> +	writel(0, evt->base + STMR_VAL);
> +
> +	/* Disable timer */
> +	writel(0, evt->base + STMR_CON);
> +}
> +
> +static void mtk_stmr_ack_irq(struct mtk_stmr_clkevt_device *evt)
> +{
> +	mtk_stmr_reset(evt);
> +}
> +
> +static irqreturn_t mtk_stmr_handler(int irq, void *priv)
> +{
> +	struct mtk_stmr_clkevt_device *evt = priv;
> +
> +	mtk_stmr_ack_irq(evt);
> +	evt->dev.event_handler(&evt->dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mtk_stmr_clkevt_next_event(unsigned long ticks,
> +				      struct clock_event_device *dev)

Indent.

> +{
> +	struct mtk_stmr_clkevt_device *evt = to_mtk_clkevt_device(dev);
> +
> +	/*
> +	 * reset timer first because we do not expect interrupt is triggered
> +	 * by old compare value.
> +	 */
> +	mtk_stmr_reset(evt);
> +
> +	writel(STMR_CON_EN, evt->base + STMR_CON);
> +
> +	writel(ticks, evt->base + STMR_VAL);
> +
> +	writel(STMR_CON_EN | STMR_CON_IRQ_EN, evt->base + STMR_CON);
> +
> +	return 0;
> +}
> +
> +static int mtk_stmr_clkevt_shutdown(struct clock_event_device *dev)
> +{
> +	struct mtk_stmr_clkevt_device *evt = to_mtk_clkevt_device(dev);
> +
> +	mtk_stmr_reset(evt);
> +
> +	return 0;
> +}
> +
> +static int mtk_stmr_clkevt_resume(struct clock_event_device *clk)
> +{
> +	return mtk_stmr_clkevt_shutdown(clk);
> +}
> +
> +static int mtk_stmr_clkevt_oneshot(struct clock_event_device *clk)
> +{
> +	return 0;
> +}
> +
> +static int __init mtk_stmr_init(struct device_node *node)
> +{
> +	struct mtk_stmr_clkevt_device *evt;
> +	struct resource res;
> +	u32 freq;
> +
> +	evt = kzalloc(sizeof(*evt), GFP_KERNEL);
> +	if (!evt)
> +		return -ENOMEM;
> +
> +	evt->dev.name = "mtk-clkevt";
> +	evt->dev.shift = 32;
> +	evt->dev.rating = 300;
> +	evt->dev.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
> +	evt->dev.set_state_shutdown = mtk_stmr_clkevt_shutdown;
> +	evt->dev.set_state_oneshot = mtk_stmr_clkevt_oneshot;
> +	evt->dev.tick_resume = mtk_stmr_clkevt_resume;
> +	evt->dev.set_next_event = mtk_stmr_clkevt_next_event;
> +	evt->dev.cpumask = cpu_possible_mask;
> +
> +	evt->base = of_iomap(node, 0);
> +	if (IS_ERR(evt->base)) {
> +		pr_err("Can't get resource\n");
> +		goto err_kzalloc;
> +	}
> +
> +	mtk_stmr_reset(evt);
> +
> +	evt->dev.irq = irq_of_parse_and_map(node, 0);
> +	if (evt->dev.irq <= 0) {
> +		pr_err("Can't parse IRQ\n");
> +		goto err_mem;
> +	}
> +
> +	if (of_property_read_u32(node, "clock-frequency", &freq)) {
> +		pr_err("Can't get clk rate\n");
> +		freq = STMR_CLKEVT_DEFAULT_RATE;
> +	}
> +
> +	evt->dev.mult = div_sc(freq, NSEC_PER_SEC, evt->dev.shift);
> +	evt->dev.max_delta_ns = clockevent_delta2ns(0xffffffff, &evt->dev);
> +	evt->dev.min_delta_ns = clockevent_delta2ns(3, &evt->dev);
> +
> +	clockevents_register_device(&evt->dev);
> +
> +	if (request_irq(evt->dev.irq, mtk_stmr_handler,
> +			IRQF_TIMER | IRQF_IRQPOLL |
> +			IRQF_TRIGGER_HIGH | IRQF_PERCPU,
> +			"mtk-clkevt", evt)) {
> +		pr_err("failed to setup irq %d\n", evt->dev.irq);
> +		goto err_mem;
> +	}
> +

Remove all the above by using:

        ret = timer_of_init(node, &to);
        if (ret)
                return ret;


> +	pr_info("mtk_stmr: base=0x%lx, irq=%d, freq=%d, hz=%d\n",
> +		(unsigned long)evt->base, evt->dev.irq, freq, HZ);
> +
> +	return 0;
> +
> +err_mem:
> +	iounmap(evt->base);
> +	of_address_to_resource(node, 0, &res);
> +	release_mem_region(res.start, resource_size(&res));
> +err_kzalloc:
> +	kfree(evt);
> +
> +	return -EINVAL;
> +}
> +
> +TIMER_OF_DECLARE(mtk_systimer, "mediatek,sys_timer", mtk_stmr_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

Re: Add system timer driver for Mediatek SoCs

[Stanley Chu]

On Tue, 2018-06-26 at 09:31 +0200, Daniel Lezcano wrote:
> On 25/06/2018 11:07, Stanley Chu wrote:
> > On Mon, 2018-06-25 at 09:34 +0200, Daniel Lezcano wrote:
> >> On 25/06/2018 09:09, Stanley Chu wrote:
> >>> This patch adds a new driver for system timer on the Mediatek SoCs.
> >>
> >> Please elaborate why we need yet another timer.
> >>
> >> Is this timer present on all Mediatek platform ? Does it always co-exist
> >> with the existing one ?
> >>
> >>
> >>
> >>
> > Hi Daniel,
> > 
> > This new "system timer" only exists on recent Mediatek platforms,
> > for example, MT6765. We will upstream driver first, and then update
> > device tree on MT6765 later.
> > 
> > System timer is designed and optimized as a SoC timer for
> > tick-broadcasting. Besides timer IP has simpler register manipulation
> > and friendly low-power design.
> > 
> > There is no plan to remove existed "General Purpose Timer" now.
> 
> It is unclear what "General Purpose Timer" and "System Timer" differ
> except they are different timers running on different platforms.
> 
> Merge them into a single timer-mediatek.c file.
> 
Hi Daniel,

Yes, they are different timers running on different platforms.

"System Timer" can replace "General Purpose Timer" in recent platforms,
for example, MT6765.

Thanks.
Stanley Chu

Re: [PATCH 2/2] clocksource/drivers/mtk_systimer: Add support for Mediatek SoCs

[Stanley Chu]

On Tue, 2018-06-26 at 09:41 +0200, Daniel Lezcano wrote:
> On 25/06/2018 09:10, Stanley Chu wrote:
> > This patch adds a new clock event for the timer
> > found on the Mediatek SoCs.
> > 
> > The Mediatek System Timer has several 32-bit timers.
> > Only one timer is used by this driver as a clock event
> > supporting oneshot events.
> > 
> > The System Timer can be run with two clocks. A 13 MHz system
> > clock and the RTC clock running at 32 KHz. This implementation
> > uses the system clock with no clock source divider.
> > 
> > The interrupts are shared between the different timers.
> > We just enable one interrupt for the clock event. The clock
> > event timer is used by all cores.
> > 
> > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> > ---
> >  drivers/clocksource/Kconfig        |   13 ++-
> >  drivers/clocksource/Makefile       |    1 +
> >  drivers/clocksource/mtk_systimer.c |  177 ++++++++++++++++++++++++++++++++++++
> >
> 
> >  3 files changed, 189 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/clocksource/mtk_systimer.c
> > 
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index dec0dd8..362c110 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -442,12 +442,21 @@ config SYS_SUPPORTS_SH_CMT
> >          bool
> >  
> >  config MTK_TIMER
> > -	bool "Mediatek timer driver" if COMPILE_TEST
> > +	bool "Mediatek general purpose timer driver" if COMPILE_TEST
> >  	depends on HAS_IOMEM
> >  	select TIMER_OF
> >  	select CLKSRC_MMIO
> >  	help
> > -	  Support for Mediatek timer driver.
> > +	  Support for Mediatek General Purpose Timer (GPT) driver.
> > +
> > +config MTK_TIMER_SYSTIMER
> > +	bool "Mediatek system timer driver" if COMPILE_TEST
> > +	depends on HAS_IOMEM
> > +	select TIMER_OF
> > +	select CLKSRC_MMIO
> > +	help
> > +	  Support for Mediatek System Timer (sys_timer) driver used as
> > +	  a clock event supporting oneshot events.
> >  
> >  config SPRD_TIMER
> >  	bool "Spreadtrum timer driver" if EXPERT
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index 00caf37..cdd34b2 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -50,6 +50,7 @@ obj-$(CONFIG_FSL_FTM_TIMER)	+= fsl_ftm_timer.o
> >  obj-$(CONFIG_VF_PIT_TIMER)	+= vf_pit_timer.o
> >  obj-$(CONFIG_CLKSRC_QCOM)	+= qcom-timer.o
> >  obj-$(CONFIG_MTK_TIMER)		+= mtk_timer.o
> > +obj-$(CONFIG_MTK_TIMER_SYSTIMER)	+= mtk_systimer.o
> >  obj-$(CONFIG_CLKSRC_PISTACHIO)	+= time-pistachio.o
> >  obj-$(CONFIG_CLKSRC_TI_32K)	+= timer-ti-32k.o
> >  obj-$(CONFIG_CLKSRC_NPS)	+= timer-nps.o
> > diff --git a/drivers/clocksource/mtk_systimer.c b/drivers/clocksource/mtk_systimer.c
> > new file mode 100644
> > index 0000000..06e74ce
> > --- /dev/null
> > +++ b/drivers/clocksource/mtk_systimer.c
> > @@ -0,0 +1,177 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +//
> > +//  Copyright (C) 2018 MediaTek Inc.
> > +
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/irq.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irqreturn.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <clocksource/arm_arch_timer.h>
> 
> Do a cleanup with the headers. They are not all necessary.
> 
> > +#define STMR_CLKEVT_DEFAULT_RATE  (13000000)
> > +
> > +/* registers */
> > +#define STMR_CON                  (0x0)
> > +#define STMR_VAL                  (0x4)
> > +
> > +/* STMR_CON */
> > +#define STMR_CON_EN               BIT(0)
> > +#define STMR_CON_IRQ_EN           BIT(1)
> > +#define STMR_CON_IRQ_CLR          BIT(4)
> > +
> > +struct mtk_stmr_clkevt_device {
> > +	void __iomem *base;
> > +	struct clock_event_device dev;
> > +};
> 
> Use the timer_of structure, that will save you some lines of code as
> well as redundant structure definition.
> 
> > +static inline struct mtk_stmr_clkevt_device*
> > +	to_mtk_clkevt_device(struct clock_event_device *c)> +{
> > +	return container_of(c, struct mtk_stmr_clkevt_device, dev);
> > +}
> > +
> > +static void mtk_stmr_reset(struct mtk_stmr_clkevt_device *evt)
> > +{
> > +	/* Clear IRQ */
> > +	writel(STMR_CON_IRQ_CLR | STMR_CON_EN, evt->base + STMR_CON);
> > +
> > +	/* Reset counter */
> > +	writel(0, evt->base + STMR_VAL);
> > +
> > +	/* Disable timer */
> > +	writel(0, evt->base + STMR_CON);
> > +}
> > +
> > +static void mtk_stmr_ack_irq(struct mtk_stmr_clkevt_device *evt)
> > +{
> > +	mtk_stmr_reset(evt);
> > +}
> > +
> > +static irqreturn_t mtk_stmr_handler(int irq, void *priv)
> > +{
> > +	struct mtk_stmr_clkevt_device *evt = priv;
> > +
> > +	mtk_stmr_ack_irq(evt);
> > +	evt->dev.event_handler(&evt->dev);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int mtk_stmr_clkevt_next_event(unsigned long ticks,
> > +				      struct clock_event_device *dev)
> 
> Indent.
> 
> > +{
> > +	struct mtk_stmr_clkevt_device *evt = to_mtk_clkevt_device(dev);
> > +
> > +	/*
> > +	 * reset timer first because we do not expect interrupt is triggered
> > +	 * by old compare value.
> > +	 */
> > +	mtk_stmr_reset(evt);
> > +
> > +	writel(STMR_CON_EN, evt->base + STMR_CON);
> > +
> > +	writel(ticks, evt->base + STMR_VAL);
> > +
> > +	writel(STMR_CON_EN | STMR_CON_IRQ_EN, evt->base + STMR_CON);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_stmr_clkevt_shutdown(struct clock_event_device *dev)
> > +{
> > +	struct mtk_stmr_clkevt_device *evt = to_mtk_clkevt_device(dev);
> > +
> > +	mtk_stmr_reset(evt);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_stmr_clkevt_resume(struct clock_event_device *clk)
> > +{
> > +	return mtk_stmr_clkevt_shutdown(clk);
> > +}
> > +
> > +static int mtk_stmr_clkevt_oneshot(struct clock_event_device *clk)
> > +{
> > +	return 0;
> > +}
> > +
> > +static int __init mtk_stmr_init(struct device_node *node)
> > +{
> > +	struct mtk_stmr_clkevt_device *evt;
> > +	struct resource res;
> > +	u32 freq;
> > +
> > +	evt = kzalloc(sizeof(*evt), GFP_KERNEL);
> > +	if (!evt)
> > +		return -ENOMEM;
> > +
> > +	evt->dev.name = "mtk-clkevt";
> > +	evt->dev.shift = 32;
> > +	evt->dev.rating = 300;
> > +	evt->dev.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
> > +	evt->dev.set_state_shutdown = mtk_stmr_clkevt_shutdown;
> > +	evt->dev.set_state_oneshot = mtk_stmr_clkevt_oneshot;
> > +	evt->dev.tick_resume = mtk_stmr_clkevt_resume;
> > +	evt->dev.set_next_event = mtk_stmr_clkevt_next_event;
> > +	evt->dev.cpumask = cpu_possible_mask;
> > +
> > +	evt->base = of_iomap(node, 0);
> > +	if (IS_ERR(evt->base)) {
> > +		pr_err("Can't get resource\n");
> > +		goto err_kzalloc;
> > +	}
> > +
> > +	mtk_stmr_reset(evt);
> > +
> > +	evt->dev.irq = irq_of_parse_and_map(node, 0);
> > +	if (evt->dev.irq <= 0) {
> > +		pr_err("Can't parse IRQ\n");
> > +		goto err_mem;
> > +	}
> > +
> > +	if (of_property_read_u32(node, "clock-frequency", &freq)) {
> > +		pr_err("Can't get clk rate\n");
> > +		freq = STMR_CLKEVT_DEFAULT_RATE;
> > +	}
> > +
> > +	evt->dev.mult = div_sc(freq, NSEC_PER_SEC, evt->dev.shift);
> > +	evt->dev.max_delta_ns = clockevent_delta2ns(0xffffffff, &evt->dev);
> > +	evt->dev.min_delta_ns = clockevent_delta2ns(3, &evt->dev);
> > +
> > +	clockevents_register_device(&evt->dev);
> > +
> > +	if (request_irq(evt->dev.irq, mtk_stmr_handler,
> > +			IRQF_TIMER | IRQF_IRQPOLL |
> > +			IRQF_TRIGGER_HIGH | IRQF_PERCPU,
> > +			"mtk-clkevt", evt)) {
> > +		pr_err("failed to setup irq %d\n", evt->dev.irq);
> > +		goto err_mem;
> > +	}
> > +
> 
> Remove all the above by using:
> 
>         ret = timer_of_init(node, &to);
>         if (ret)
>                 return ret;
> 
> 
> > +	pr_info("mtk_stmr: base=0x%lx, irq=%d, freq=%d, hz=%d\n",
> > +		(unsigned long)evt->base, evt->dev.irq, freq, HZ);
> > +
> > +	return 0;
> > +
> > +err_mem:
> > +	iounmap(evt->base);
> > +	of_address_to_resource(node, 0, &res);
> > +	release_mem_region(res.start, resource_size(&res));
> > +err_kzalloc:
> > +	kfree(evt);
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +TIMER_OF_DECLARE(mtk_systimer, "mediatek,sys_timer", mtk_stmr_init);
> > 
> 
> 
Hi Daniel,

Good suggestion for timer_of structure! We will use it and fix above all
in v2.

Thanks.
Stanley Chu