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

Daniel Lezcano
2018-06-26T07:41:12Z

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 |