[RFC v2 0/2] clocksource: davinci-timer: new driver

[RFC v2 0/2] clocksource: davinci-timer: new driver

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Hi Daniel,

this is another try at the davinci clocksource driver. Changes in
regard to v1 listed below. As before, the driver is split into two
parts (one for clockevent and one for clocksource).

v1 -> v2:
- changed the format of the copyright notice
- removed all mentiones of the periodic timer setting
- added caching of the TCR register value so that its updating doesn't
  require a read
- split the timer configuration for clock events into the
  set_state_oneshot() and set_state_shutdown() callbacks

Bartosz Golaszewski (2):
  clocksource: davinci-timer: add support for clockevents
  clocksource: timer-davinci: add support for clocksource

 drivers/clocksource/Kconfig         |   5 +
 drivers/clocksource/Makefile        |   1 +
 drivers/clocksource/timer-davinci.c | 355 ++++++++++++++++++++++++++++
 include/clocksource/timer-davinci.h |  44 ++++
 4 files changed, 405 insertions(+)
 create mode 100644 drivers/clocksource/timer-davinci.c
 create mode 100644 include/clocksource/timer-davinci.h

-- 
2.21.0

[RFC v2 1/2] clocksource: davinci-timer: add support for clockevents

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Currently the clocksource and clockevent support for davinci platforms
lives in mach-davinci. It hard-codes many things, uses global variables,
implements functionalities unused by any platform and has code fragments
scattered across many (often unrelated) files.

Implement a new, modern and simplified timer driver and put it into
drivers/clocksource. We still need to support legacy board files so
export a config structure and a function that allows machine code to
register the timer.

The timer we're using is 64-bit but can be programmed in dual 32-bit
mode (both chained and unchained). We're using dual 32-bit mode to
have separate counters for clockevents and clocksource.

This patch contains the core code and support for clockevent. The
clocksource code will be included in a subsequent patch.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/clocksource/Kconfig         |   5 +
 drivers/clocksource/Makefile        |   1 +
 drivers/clocksource/timer-davinci.c | 285 ++++++++++++++++++++++++++++
 include/clocksource/timer-davinci.h |  44 +++++
 4 files changed, 335 insertions(+)
 create mode 100644 drivers/clocksource/timer-davinci.c
 create mode 100644 include/clocksource/timer-davinci.h

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 6bcaa4e2e72c..32dee6abd54a 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -42,6 +42,11 @@ config BCM_KONA_TIMER
 	help
 	  Enables the support for the BCM Kona mobile timer driver.
 
+config DAVINCI_TIMER
+	bool "Texas Instruments DaVinci timer driver" if COMPILE_TEST
+	help
+	  Enables the support for the TI DaVinci timer driver.
+
 config DIGICOLOR_TIMER
 	bool "Digicolor timer driver" if COMPILE_TEST
 	select CLKSRC_MMIO
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 236858fa7fbf..021831bcc567 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
 obj-$(CONFIG_EM_TIMER_STI)	+= em_sti.o
 obj-$(CONFIG_CLKBLD_I8253)	+= i8253.o
 obj-$(CONFIG_CLKSRC_MMIO)	+= mmio.o
+obj-$(CONFIG_DAVINCI_TIMER)	+= timer-davinci.o
 obj-$(CONFIG_DIGICOLOR_TIMER)	+= timer-digicolor.o
 obj-$(CONFIG_OMAP_DM_TIMER)	+= timer-ti-dm.o
 obj-$(CONFIG_DW_APB_TIMER)	+= dw_apb_timer.o
diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
new file mode 100644
index 000000000000..a8fc7b3805c9
--- /dev/null
+++ b/drivers/clocksource/timer-davinci.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * TI DaVinci clocksource driver
+ *
+ * Copyright (C) 2019 Texas Instruments
+ * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+ * (with tiny parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/sched_clock.h>
+
+#include <clocksource/timer-davinci.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "%s: " fmt "\n", __func__
+
+#define DAVINCI_TIMER_REG_TIM12			0x10
+#define DAVINCI_TIMER_REG_TIM34			0x14
+#define DAVINCI_TIMER_REG_PRD12			0x18
+#define DAVINCI_TIMER_REG_PRD34			0x1c
+#define DAVINCI_TIMER_REG_TCR			0x20
+#define DAVINCI_TIMER_REG_TGCR			0x24
+
+#define DAVINCI_TIMER_TIMMODE_MASK		GENMASK(3, 2)
+#define DAVINCI_TIMER_RESET_MASK		GENMASK(1, 0)
+#define DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED	BIT(2)
+#define DAVINCI_TIMER_UNRESET			GENMASK(1, 0)
+
+#define DAVINCI_TIMER_ENAMODE_MASK		GENMASK(1, 0)
+#define DAVINCI_TIMER_ENAMODE_DISABLED		0x00
+#define DAVINCI_TIMER_ENAMODE_ONESHOT		BIT(0)
+
+#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM12	6
+#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM34	22
+
+#define DAVINCI_TIMER_MIN_DELTA			0x01
+#define DAVINCI_TIMER_MAX_DELTA			0xfffffffe
+
+#define DAVINCI_TIMER_TGCR_DEFAULT \
+		(DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
+
+/* Cache the TCR register value. */
+static unsigned int davinci_tcr;
+
+struct davinci_clockevent {
+	struct clock_event_device dev;
+	void __iomem *base;
+
+	unsigned int tim_off;
+	unsigned int prd_off;
+	unsigned int cmp_off;
+
+	unsigned int enamode_disabled;
+	unsigned int enamode_oneshot;
+	unsigned int enamode_mask;
+};
+
+static struct davinci_clockevent *
+to_davinci_clockevent(struct clock_event_device *clockevent)
+{
+	return container_of(clockevent, struct davinci_clockevent, dev);
+}
+
+static unsigned int
+davinci_clockevent_read(struct davinci_clockevent *clockevent,
+			unsigned int reg)
+{
+	return readl_relaxed(clockevent->base + reg);
+}
+
+static void davinci_clockevent_write(struct davinci_clockevent *clockevent,
+				     unsigned int reg, unsigned int val)
+{
+	writel_relaxed(val, clockevent->base + reg);
+}
+
+static void davinci_tcr_update(void __iomem *base,
+			       unsigned int mask, unsigned int val)
+{
+	davinci_tcr &= ~mask;
+	davinci_tcr |= val & mask;
+
+	writel_relaxed(davinci_tcr, base + DAVINCI_TIMER_REG_TCR);
+}
+
+static int davinci_clockevent_shutdown(struct clock_event_device *dev)
+{
+	struct davinci_clockevent *clockevent;
+
+	clockevent = to_davinci_clockevent(dev);
+
+	davinci_tcr_update(clockevent->base,
+			   clockevent->enamode_mask,
+			   clockevent->enamode_disabled);
+
+	return 0;
+}
+
+static int davinci_clockevent_set_oneshot(struct clock_event_device *dev)
+{
+	struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
+
+	davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
+
+	davinci_tcr_update(clockevent->base,
+			   clockevent->enamode_mask,
+			   clockevent->enamode_oneshot);
+
+	return 0;
+}
+
+static int
+davinci_clockevent_set_next_event_std(unsigned long cycles,
+				      struct clock_event_device *dev)
+{
+	struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
+
+	davinci_clockevent_shutdown(dev);
+
+	davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
+	davinci_clockevent_write(clockevent, clockevent->prd_off, cycles);
+
+	davinci_clockevent_set_oneshot(dev);
+
+	return 0;
+}
+
+static int
+davinci_clockevent_set_next_event_cmp(unsigned long cycles,
+				      struct clock_event_device *dev)
+{
+	struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
+	unsigned int curr_time;
+
+	curr_time = davinci_clockevent_read(clockevent, clockevent->tim_off);
+	davinci_clockevent_write(clockevent,
+				 clockevent->cmp_off, curr_time + cycles);
+
+	return 0;
+}
+
+static irqreturn_t davinci_timer_irq_timer(int irq, void *data)
+{
+	struct davinci_clockevent *clockevent = data;
+
+	if (!clockevent_state_oneshot(&clockevent->dev))
+		davinci_tcr_update(clockevent,
+				   clockevent->enamode_mask,
+				   clockevent->enamode_disabled);
+
+	clockevent->dev.event_handler(&clockevent->dev);
+
+	return IRQ_HANDLED;
+}
+
+static void davinci_timer_init(void __iomem *base)
+{
+	/* Set clock to internal mode and disable it. */
+	writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TCR);
+	/*
+	 * Reset both 32-bit timers, set no prescaler for timer 34, set the
+	 * timer to dual 32-bit unchained mode, unreset both 32-bit timers.
+	 */
+	writel_relaxed(DAVINCI_TIMER_TGCR_DEFAULT,
+		       base + DAVINCI_TIMER_REG_TGCR);
+	/* Init both counters to zero. */
+	writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM12);
+	writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM34);
+}
+
+int __init davinci_timer_register(struct clk *clk,
+				  const struct davinci_timer_cfg *timer_cfg)
+{
+	struct davinci_clockevent *clockevent;
+	unsigned int tick_rate, shift;
+	void __iomem *base;
+	int rv;
+
+	rv = clk_prepare_enable(clk);
+	if (rv) {
+		pr_err("Unable to prepare and enable the timer clock");
+		return rv;
+	}
+
+	base = request_mem_region(timer_cfg->reg.start,
+				  resource_size(&timer_cfg->reg),
+				  "davinci-timer");
+	if (!base) {
+		pr_err("Unable to request memory region");
+		return -EBUSY;
+	}
+
+	base = ioremap(timer_cfg->reg.start, resource_size(&timer_cfg->reg));
+	if (!base) {
+		pr_err("Unable to map the register range");
+		return -ENOMEM;
+	}
+
+	davinci_timer_init(base);
+	tick_rate = clk_get_rate(clk);
+
+	clockevent = kzalloc(sizeof(*clockevent), GFP_KERNEL);
+	if (!clockevent) {
+		pr_err("Error allocating memory for clockevent data");
+		return -ENOMEM;
+	}
+
+	clockevent->dev.name = "tim12";
+	clockevent->dev.features = CLOCK_EVT_FEAT_ONESHOT;
+	clockevent->dev.cpumask = cpumask_of(0);
+
+	clockevent->base = base;
+	clockevent->tim_off = DAVINCI_TIMER_REG_TIM12;
+	clockevent->prd_off = DAVINCI_TIMER_REG_PRD12;
+
+	shift = DAVINCI_TIMER_ENAMODE_SHIFT_TIM12;
+	clockevent->enamode_disabled = DAVINCI_TIMER_ENAMODE_DISABLED << shift;
+	clockevent->enamode_oneshot = DAVINCI_TIMER_ENAMODE_ONESHOT << shift;
+	clockevent->enamode_mask = DAVINCI_TIMER_ENAMODE_MASK << shift;
+
+	clockevent->dev.set_state_shutdown = davinci_clockevent_shutdown;
+	clockevent->dev.set_state_oneshot = davinci_clockevent_set_oneshot;
+
+	if (timer_cfg->cmp_off) {
+		clockevent->cmp_off = timer_cfg->cmp_off;
+		clockevent->dev.set_next_event =
+				davinci_clockevent_set_next_event_cmp;
+	} else {
+		clockevent->dev.set_next_event =
+				davinci_clockevent_set_next_event_std;
+	}
+
+	rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKEVENT_IRQ].start,
+			 davinci_timer_irq_timer, IRQF_TIMER,
+			 "clockevent", clockevent);
+	if (rv) {
+		pr_err("Unable to request the clockevent interrupt");
+		return rv;
+	}
+
+	clockevents_config_and_register(&clockevent->dev, tick_rate,
+					DAVINCI_TIMER_MIN_DELTA,
+					DAVINCI_TIMER_MAX_DELTA);
+
+	return 0;
+}
+
+static int __init of_davinci_timer_register(struct device_node *np)
+{
+	struct davinci_timer_cfg timer_cfg = { };
+	struct clk *clk;
+	int rv;
+
+	rv = of_address_to_resource(np, 0, &timer_cfg.reg);
+	if (rv) {
+		pr_err("Unable to get the register range for timer");
+		return rv;
+	}
+
+	rv = of_irq_to_resource_table(np, timer_cfg.irq,
+				      DAVINCI_TIMER_NUM_IRQS);
+	if (rv != DAVINCI_TIMER_NUM_IRQS) {
+		pr_err("Unable to get the interrupts for timer");
+		return rv;
+	}
+
+	clk = of_clk_get(np, 0);
+	if (IS_ERR(clk)) {
+		pr_err("Unable to get the timer clock");
+		return PTR_ERR(clk);
+	}
+
+	rv = davinci_timer_register(clk, &timer_cfg);
+	if (rv)
+		clk_put(clk);
+
+	return rv;
+}
+TIMER_OF_DECLARE(davinci_timer, "ti,da830-timer", of_davinci_timer_register);
diff --git a/include/clocksource/timer-davinci.h b/include/clocksource/timer-davinci.h
new file mode 100644
index 000000000000..1dcc1333fbc8
--- /dev/null
+++ b/include/clocksource/timer-davinci.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * TI DaVinci clocksource driver
+ *
+ * Copyright (C) 2019 Texas Instruments
+ * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+ */
+
+#ifndef __TIMER_DAVINCI_H__
+#define __TIMER_DAVINCI_H__
+
+#include <linux/clk.h>
+#include <linux/ioport.h>
+
+enum {
+	DAVINCI_TIMER_CLOCKEVENT_IRQ,
+	DAVINCI_TIMER_CLOCKSOURCE_IRQ,
+	DAVINCI_TIMER_NUM_IRQS,
+};
+
+/**
+ * struct davinci_timer_cfg - davinci clocksource driver configuration struct
+ * @reg:        register range resource
+ * @irq:        clockevent and clocksource interrupt resources
+ * @cmp_off:    if set - it specifies the compare register used for clockevent
+ *
+ * Note: if the compare register is specified, the driver will use the bottom
+ * clock half for both clocksource and clockevent and the compare register
+ * to generate event irqs. The user must supply the correct compare register
+ * interrupt number.
+ *
+ * This is only used by da830 the DSP of which uses the top half. The timer
+ * driver still configures the top half to run in free-run mode.
+ */
+struct davinci_timer_cfg {
+	struct resource reg;
+	struct resource irq[DAVINCI_TIMER_NUM_IRQS];
+	unsigned int cmp_off;
+};
+
+int __init davinci_timer_register(struct clk *clk,
+				  const struct davinci_timer_cfg *data);
+
+#endif /* __TIMER_DAVINCI_H__ */
-- 
2.21.0

Re: [RFC v2 1/2] clocksource: davinci-timer: add support for clockevents

[Daniel Lezcano]

On 23/05/2019 14:58, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Currently the clocksource and clockevent support for davinci platforms
> lives in mach-davinci. It hard-codes many things, uses global variables,
> implements functionalities unused by any platform and has code fragments
> scattered across many (often unrelated) files.
> 
> Implement a new, modern and simplified timer driver and put it into
> drivers/clocksource. We still need to support legacy board files so
> export a config structure and a function that allows machine code to
> register the timer.
> 
> The timer we're using is 64-bit but can be programmed in dual 32-bit
> mode (both chained and unchained). We're using dual 32-bit mode to
> have separate counters for clockevents and clocksource.
> 
> This patch contains the core code and support for clockevent. The
> clocksource code will be included in a subsequent patch.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/clocksource/Kconfig         |   5 +
>  drivers/clocksource/Makefile        |   1 +
>  drivers/clocksource/timer-davinci.c | 285 ++++++++++++++++++++++++++++
>  include/clocksource/timer-davinci.h |  44 +++++
>  4 files changed, 335 insertions(+)
>  create mode 100644 drivers/clocksource/timer-davinci.c
>  create mode 100644 include/clocksource/timer-davinci.h
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 6bcaa4e2e72c..32dee6abd54a 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -42,6 +42,11 @@ config BCM_KONA_TIMER
>  	help
>  	  Enables the support for the BCM Kona mobile timer driver.
>  
> +config DAVINCI_TIMER
> +	bool "Texas Instruments DaVinci timer driver" if COMPILE_TEST
> +	help
> +	  Enables the support for the TI DaVinci timer driver.
> +
>  config DIGICOLOR_TIMER
>  	bool "Digicolor timer driver" if COMPILE_TEST
>  	select CLKSRC_MMIO
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 236858fa7fbf..021831bcc567 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
>  obj-$(CONFIG_EM_TIMER_STI)	+= em_sti.o
>  obj-$(CONFIG_CLKBLD_I8253)	+= i8253.o
>  obj-$(CONFIG_CLKSRC_MMIO)	+= mmio.o
> +obj-$(CONFIG_DAVINCI_TIMER)	+= timer-davinci.o
>  obj-$(CONFIG_DIGICOLOR_TIMER)	+= timer-digicolor.o
>  obj-$(CONFIG_OMAP_DM_TIMER)	+= timer-ti-dm.o
>  obj-$(CONFIG_DW_APB_TIMER)	+= dw_apb_timer.o
> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> new file mode 100644
> index 000000000000..a8fc7b3805c9
> --- /dev/null
> +++ b/drivers/clocksource/timer-davinci.c
> @@ -0,0 +1,285 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * TI DaVinci clocksource driver
> + *
> + * Copyright (C) 2019 Texas Instruments
> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> + * (with tiny parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/sched_clock.h>
> +
> +#include <clocksource/timer-davinci.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "%s: " fmt "\n", __func__
> +
> +#define DAVINCI_TIMER_REG_TIM12			0x10
> +#define DAVINCI_TIMER_REG_TIM34			0x14
> +#define DAVINCI_TIMER_REG_PRD12			0x18
> +#define DAVINCI_TIMER_REG_PRD34			0x1c
> +#define DAVINCI_TIMER_REG_TCR			0x20
> +#define DAVINCI_TIMER_REG_TGCR			0x24
> +
> +#define DAVINCI_TIMER_TIMMODE_MASK		GENMASK(3, 2)
> +#define DAVINCI_TIMER_RESET_MASK		GENMASK(1, 0)
> +#define DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED	BIT(2)
> +#define DAVINCI_TIMER_UNRESET			GENMASK(1, 0)
> +
> +#define DAVINCI_TIMER_ENAMODE_MASK		GENMASK(1, 0)
> +#define DAVINCI_TIMER_ENAMODE_DISABLED		0x00
> +#define DAVINCI_TIMER_ENAMODE_ONESHOT		BIT(0)
> +
> +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM12	6
> +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM34	22
> +
> +#define DAVINCI_TIMER_MIN_DELTA			0x01
> +#define DAVINCI_TIMER_MAX_DELTA			0xfffffffe
> +
> +#define DAVINCI_TIMER_TGCR_DEFAULT \
> +		(DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> +
> +/* Cache the TCR register value. */
> +static unsigned int davinci_tcr;
> +
> +struct davinci_clockevent {
> +	struct clock_event_device dev;
> +	void __iomem *base;
> +
> +	unsigned int tim_off;
> +	unsigned int prd_off;
> +	unsigned int cmp_off;
> +
> +	unsigned int enamode_disabled;
> +	unsigned int enamode_oneshot;
> +	unsigned int enamode_mask;
> +};
> +
> +static struct davinci_clockevent *
> +to_davinci_clockevent(struct clock_event_device *clockevent)
> +{
> +	return container_of(clockevent, struct davinci_clockevent, dev);
> +}
> +
> +static unsigned int
> +davinci_clockevent_read(struct davinci_clockevent *clockevent,
> +			unsigned int reg)
> +{
> +	return readl_relaxed(clockevent->base + reg);
> +}
> +
> +static void davinci_clockevent_write(struct davinci_clockevent *clockevent,
> +				     unsigned int reg, unsigned int val)
> +{
> +	writel_relaxed(val, clockevent->base + reg);
> +}
> +
> +static void davinci_tcr_update(void __iomem *base,
> +			       unsigned int mask, unsigned int val)
> +{
> +	davinci_tcr &= ~mask;
> +	davinci_tcr |= val & mask;


I don't see when the davinci_tcr is initialized.

> +	writel_relaxed(davinci_tcr, base + DAVINCI_TIMER_REG_TCR);
> +}
> +
> +static int davinci_clockevent_shutdown(struct clock_event_device *dev)
> +{
> +	struct davinci_clockevent *clockevent;
> +
> +	clockevent = to_davinci_clockevent(dev);
> +
> +	davinci_tcr_update(clockevent->base,
> +			   clockevent->enamode_mask,
> +			   clockevent->enamode_disabled);
> +
> +	return 0;
> +}
> +
> +static int davinci_clockevent_set_oneshot(struct clock_event_device *dev)
> +{
> +	struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> +
> +	davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
> +
> +	davinci_tcr_update(clockevent->base,
> +			   clockevent->enamode_mask,
> +			   clockevent->enamode_oneshot);
> +
> +	return 0;
> +}
> +
> +static int
> +davinci_clockevent_set_next_event_std(unsigned long cycles,
> +				      struct clock_event_device *dev)
> +{
> +	struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> +
> +	davinci_clockevent_shutdown(dev);
> +
> +	davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
> +	davinci_clockevent_write(clockevent, clockevent->prd_off, cycles);
> +
> +	davinci_clockevent_set_oneshot(dev);
> +
> +	return 0;
> +}
> +
> +static int
> +davinci_clockevent_set_next_event_cmp(unsigned long cycles,
> +				      struct clock_event_device *dev)
> +{
> +	struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> +	unsigned int curr_time;
> +
> +	curr_time = davinci_clockevent_read(clockevent, clockevent->tim_off);
> +	davinci_clockevent_write(clockevent,
> +				 clockevent->cmp_off, curr_time + cycles);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t davinci_timer_irq_timer(int irq, void *data)
> +{
> +	struct davinci_clockevent *clockevent = data;
> +
> +	if (!clockevent_state_oneshot(&clockevent->dev))
> +		davinci_tcr_update(clockevent,
> +				   clockevent->enamode_mask,
> +				   clockevent->enamode_disabled);
> +
> +	clockevent->dev.event_handler(&clockevent->dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void davinci_timer_init(void __iomem *base)
> +{
> +	/* Set clock to internal mode and disable it. */
> +	writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TCR);
> +	/*
> +	 * Reset both 32-bit timers, set no prescaler for timer 34, set the
> +	 * timer to dual 32-bit unchained mode, unreset both 32-bit timers.
> +	 */
> +	writel_relaxed(DAVINCI_TIMER_TGCR_DEFAULT,
> +		       base + DAVINCI_TIMER_REG_TGCR);
> +	/* Init both counters to zero. */
> +	writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM12);
> +	writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM34);
> +}
> +
> +int __init davinci_timer_register(struct clk *clk,
> +				  const struct davinci_timer_cfg *timer_cfg)
> +{
> +	struct davinci_clockevent *clockevent;
> +	unsigned int tick_rate, shift;
> +	void __iomem *base;
> +	int rv;
> +
> +	rv = clk_prepare_enable(clk);
> +	if (rv) {
> +		pr_err("Unable to prepare and enable the timer clock");
> +		return rv;
> +	}
> +
> +	base = request_mem_region(timer_cfg->reg.start,
> +				  resource_size(&timer_cfg->reg),
> +				  "davinci-timer");
> +	if (!base) {
> +		pr_err("Unable to request memory region");
> +		return -EBUSY;
> +	}
> +
> +	base = ioremap(timer_cfg->reg.start, resource_size(&timer_cfg->reg));
> +	if (!base) {
> +		pr_err("Unable to map the register range");
> +		return -ENOMEM;
> +	}
> +
> +	davinci_timer_init(base);
> +	tick_rate = clk_get_rate(clk);
> +
> +	clockevent = kzalloc(sizeof(*clockevent), GFP_KERNEL);
> +	if (!clockevent) {
> +		pr_err("Error allocating memory for clockevent data");
> +		return -ENOMEM;
> +	}
> +
> +	clockevent->dev.name = "tim12";
> +	clockevent->dev.features = CLOCK_EVT_FEAT_ONESHOT;
> +	clockevent->dev.cpumask = cpumask_of(0);
> +
> +	clockevent->base = base;
> +	clockevent->tim_off = DAVINCI_TIMER_REG_TIM12;
> +	clockevent->prd_off = DAVINCI_TIMER_REG_PRD12;
> +
> +	shift = DAVINCI_TIMER_ENAMODE_SHIFT_TIM12;
> +	clockevent->enamode_disabled = DAVINCI_TIMER_ENAMODE_DISABLED << shift;
> +	clockevent->enamode_oneshot = DAVINCI_TIMER_ENAMODE_ONESHOT << shift;
> +	clockevent->enamode_mask = DAVINCI_TIMER_ENAMODE_MASK << shift;
> +
> +	clockevent->dev.set_state_shutdown = davinci_clockevent_shutdown;
> +	clockevent->dev.set_state_oneshot = davinci_clockevent_set_oneshot;
> +
> +	if (timer_cfg->cmp_off) {
> +		clockevent->cmp_off = timer_cfg->cmp_off;
> +		clockevent->dev.set_next_event =
> +				davinci_clockevent_set_next_event_cmp;
> +	} else {
> +		clockevent->dev.set_next_event =
> +				davinci_clockevent_set_next_event_std;
> +	}
> +
> +	rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKEVENT_IRQ].start,
> +			 davinci_timer_irq_timer, IRQF_TIMER,
> +			 "clockevent", clockevent);
> +	if (rv) {
> +		pr_err("Unable to request the clockevent interrupt");
> +		return rv;
> +	}
> +
> +	clockevents_config_and_register(&clockevent->dev, tick_rate,
> +					DAVINCI_TIMER_MIN_DELTA,
> +					DAVINCI_TIMER_MAX_DELTA);
> +
> +	return 0;
> +}
> +
> +static int __init of_davinci_timer_register(struct device_node *np)
> +{
> +	struct davinci_timer_cfg timer_cfg = { };
> +	struct clk *clk;
> +	int rv;
> +
> +	rv = of_address_to_resource(np, 0, &timer_cfg.reg);
> +	if (rv) {
> +		pr_err("Unable to get the register range for timer");
> +		return rv;
> +	}
> +
> +	rv = of_irq_to_resource_table(np, timer_cfg.irq,
> +				      DAVINCI_TIMER_NUM_IRQS);
> +	if (rv != DAVINCI_TIMER_NUM_IRQS) {
> +		pr_err("Unable to get the interrupts for timer");
> +		return rv;
> +	}
> +
> +	clk = of_clk_get(np, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("Unable to get the timer clock");
> +		return PTR_ERR(clk);
> +	}
> +
> +	rv = davinci_timer_register(clk, &timer_cfg);
> +	if (rv)
> +		clk_put(clk);
> +
> +	return rv;
> +}
> +TIMER_OF_DECLARE(davinci_timer, "ti,da830-timer", of_davinci_timer_register);
> diff --git a/include/clocksource/timer-davinci.h b/include/clocksource/timer-davinci.h
> new file mode 100644
> index 000000000000..1dcc1333fbc8
> --- /dev/null
> +++ b/include/clocksource/timer-davinci.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * TI DaVinci clocksource driver
> + *
> + * Copyright (C) 2019 Texas Instruments
> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> + */
> +
> +#ifndef __TIMER_DAVINCI_H__
> +#define __TIMER_DAVINCI_H__
> +
> +#include <linux/clk.h>
> +#include <linux/ioport.h>
> +
> +enum {
> +	DAVINCI_TIMER_CLOCKEVENT_IRQ,
> +	DAVINCI_TIMER_CLOCKSOURCE_IRQ,
> +	DAVINCI_TIMER_NUM_IRQS,
> +};
> +
> +/**
> + * struct davinci_timer_cfg - davinci clocksource driver configuration struct
> + * @reg:        register range resource
> + * @irq:        clockevent and clocksource interrupt resources
> + * @cmp_off:    if set - it specifies the compare register used for clockevent
> + *
> + * Note: if the compare register is specified, the driver will use the bottom
> + * clock half for both clocksource and clockevent and the compare register
> + * to generate event irqs. The user must supply the correct compare register
> + * interrupt number.
> + *
> + * This is only used by da830 the DSP of which uses the top half. The timer
> + * driver still configures the top half to run in free-run mode.
> + */
> +struct davinci_timer_cfg {
> +	struct resource reg;
> +	struct resource irq[DAVINCI_TIMER_NUM_IRQS];
> +	unsigned int cmp_off;
> +};
> +
> +int __init davinci_timer_register(struct clk *clk,
> +				  const struct davinci_timer_cfg *data);
> +
> +#endif /* __TIMER_DAVINCI_H__ */
> 


-- 
 <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: [RFC v2 1/2] clocksource: davinci-timer: add support for clockevents

[Bartosz Golaszewski]

czw., 23 maj 2019 o 18:38 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>
> On 23/05/2019 14:58, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Currently the clocksource and clockevent support for davinci platforms
> > lives in mach-davinci. It hard-codes many things, uses global variables,
> > implements functionalities unused by any platform and has code fragments
> > scattered across many (often unrelated) files.
> >
> > Implement a new, modern and simplified timer driver and put it into
> > drivers/clocksource. We still need to support legacy board files so
> > export a config structure and a function that allows machine code to
> > register the timer.
> >
> > The timer we're using is 64-bit but can be programmed in dual 32-bit
> > mode (both chained and unchained). We're using dual 32-bit mode to
> > have separate counters for clockevents and clocksource.
> >
> > This patch contains the core code and support for clockevent. The
> > clocksource code will be included in a subsequent patch.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> >  drivers/clocksource/Kconfig         |   5 +
> >  drivers/clocksource/Makefile        |   1 +
> >  drivers/clocksource/timer-davinci.c | 285 ++++++++++++++++++++++++++++
> >  include/clocksource/timer-davinci.h |  44 +++++
> >  4 files changed, 335 insertions(+)
> >  create mode 100644 drivers/clocksource/timer-davinci.c
> >  create mode 100644 include/clocksource/timer-davinci.h
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 6bcaa4e2e72c..32dee6abd54a 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -42,6 +42,11 @@ config BCM_KONA_TIMER
> >       help
> >         Enables the support for the BCM Kona mobile timer driver.
> >
> > +config DAVINCI_TIMER
> > +     bool "Texas Instruments DaVinci timer driver" if COMPILE_TEST
> > +     help
> > +       Enables the support for the TI DaVinci timer driver.
> > +
> >  config DIGICOLOR_TIMER
> >       bool "Digicolor timer driver" if COMPILE_TEST
> >       select CLKSRC_MMIO
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index 236858fa7fbf..021831bcc567 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -15,6 +15,7 @@ obj-$(CONFIG_SH_TIMER_TMU)  += sh_tmu.o
> >  obj-$(CONFIG_EM_TIMER_STI)   += em_sti.o
> >  obj-$(CONFIG_CLKBLD_I8253)   += i8253.o
> >  obj-$(CONFIG_CLKSRC_MMIO)    += mmio.o
> > +obj-$(CONFIG_DAVINCI_TIMER)  += timer-davinci.o
> >  obj-$(CONFIG_DIGICOLOR_TIMER)        += timer-digicolor.o
> >  obj-$(CONFIG_OMAP_DM_TIMER)  += timer-ti-dm.o
> >  obj-$(CONFIG_DW_APB_TIMER)   += dw_apb_timer.o
> > diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> > new file mode 100644
> > index 000000000000..a8fc7b3805c9
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-davinci.c
> > @@ -0,0 +1,285 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * TI DaVinci clocksource driver
> > + *
> > + * Copyright (C) 2019 Texas Instruments
> > + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > + * (with tiny parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/kernel.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/sched_clock.h>
> > +
> > +#include <clocksource/timer-davinci.h>
> > +
> > +#undef pr_fmt
> > +#define pr_fmt(fmt) "%s: " fmt "\n", __func__
> > +
> > +#define DAVINCI_TIMER_REG_TIM12                      0x10
> > +#define DAVINCI_TIMER_REG_TIM34                      0x14
> > +#define DAVINCI_TIMER_REG_PRD12                      0x18
> > +#define DAVINCI_TIMER_REG_PRD34                      0x1c
> > +#define DAVINCI_TIMER_REG_TCR                        0x20
> > +#define DAVINCI_TIMER_REG_TGCR                       0x24
> > +
> > +#define DAVINCI_TIMER_TIMMODE_MASK           GENMASK(3, 2)
> > +#define DAVINCI_TIMER_RESET_MASK             GENMASK(1, 0)
> > +#define DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED        BIT(2)
> > +#define DAVINCI_TIMER_UNRESET                        GENMASK(1, 0)
> > +
> > +#define DAVINCI_TIMER_ENAMODE_MASK           GENMASK(1, 0)
> > +#define DAVINCI_TIMER_ENAMODE_DISABLED               0x00
> > +#define DAVINCI_TIMER_ENAMODE_ONESHOT                BIT(0)
> > +
> > +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM12    6
> > +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM34    22
> > +
> > +#define DAVINCI_TIMER_MIN_DELTA                      0x01
> > +#define DAVINCI_TIMER_MAX_DELTA                      0xfffffffe
> > +
> > +#define DAVINCI_TIMER_TGCR_DEFAULT \
> > +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> > +
> > +/* Cache the TCR register value. */
> > +static unsigned int davinci_tcr;
> > +
> > +struct davinci_clockevent {
> > +     struct clock_event_device dev;
> > +     void __iomem *base;
> > +
> > +     unsigned int tim_off;
> > +     unsigned int prd_off;
> > +     unsigned int cmp_off;
> > +
> > +     unsigned int enamode_disabled;
> > +     unsigned int enamode_oneshot;
> > +     unsigned int enamode_mask;
> > +};
> > +
> > +static struct davinci_clockevent *
> > +to_davinci_clockevent(struct clock_event_device *clockevent)
> > +{
> > +     return container_of(clockevent, struct davinci_clockevent, dev);
> > +}
> > +
> > +static unsigned int
> > +davinci_clockevent_read(struct davinci_clockevent *clockevent,
> > +                     unsigned int reg)
> > +{
> > +     return readl_relaxed(clockevent->base + reg);
> > +}
> > +
> > +static void davinci_clockevent_write(struct davinci_clockevent *clockevent,
> > +                                  unsigned int reg, unsigned int val)
> > +{
> > +     writel_relaxed(val, clockevent->base + reg);
> > +}
> > +
> > +static void davinci_tcr_update(void __iomem *base,
> > +                            unsigned int mask, unsigned int val)
> > +{
> > +     davinci_tcr &= ~mask;
> > +     davinci_tcr |= val & mask;
>
>
> I don't see when the davinci_tcr is initialized.
>

It's set to 0x0 by the compiler and we're setting the register to 0x0
in davinci_timer_init().

Bart

> > +     writel_relaxed(davinci_tcr, base + DAVINCI_TIMER_REG_TCR);
> > +}
> > +
> > +static int davinci_clockevent_shutdown(struct clock_event_device *dev)
> > +{
> > +     struct davinci_clockevent *clockevent;
> > +
> > +     clockevent = to_davinci_clockevent(dev);
> > +
> > +     davinci_tcr_update(clockevent->base,
> > +                        clockevent->enamode_mask,
> > +                        clockevent->enamode_disabled);
> > +
> > +     return 0;
> > +}
> > +
> > +static int davinci_clockevent_set_oneshot(struct clock_event_device *dev)
> > +{
> > +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> > +
> > +     davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
> > +
> > +     davinci_tcr_update(clockevent->base,
> > +                        clockevent->enamode_mask,
> > +                        clockevent->enamode_oneshot);
> > +
> > +     return 0;
> > +}
> > +
> > +static int
> > +davinci_clockevent_set_next_event_std(unsigned long cycles,
> > +                                   struct clock_event_device *dev)
> > +{
> > +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> > +
> > +     davinci_clockevent_shutdown(dev);
> > +
> > +     davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
> > +     davinci_clockevent_write(clockevent, clockevent->prd_off, cycles);
> > +
> > +     davinci_clockevent_set_oneshot(dev);
> > +
> > +     return 0;
> > +}
> > +
> > +static int
> > +davinci_clockevent_set_next_event_cmp(unsigned long cycles,
> > +                                   struct clock_event_device *dev)
> > +{
> > +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> > +     unsigned int curr_time;
> > +
> > +     curr_time = davinci_clockevent_read(clockevent, clockevent->tim_off);
> > +     davinci_clockevent_write(clockevent,
> > +                              clockevent->cmp_off, curr_time + cycles);
> > +
> > +     return 0;
> > +}
> > +
> > +static irqreturn_t davinci_timer_irq_timer(int irq, void *data)
> > +{
> > +     struct davinci_clockevent *clockevent = data;
> > +
> > +     if (!clockevent_state_oneshot(&clockevent->dev))
> > +             davinci_tcr_update(clockevent,
> > +                                clockevent->enamode_mask,
> > +                                clockevent->enamode_disabled);
> > +
> > +     clockevent->dev.event_handler(&clockevent->dev);
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +static void davinci_timer_init(void __iomem *base)
> > +{
> > +     /* Set clock to internal mode and disable it. */
> > +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TCR);
> > +     /*
> > +      * Reset both 32-bit timers, set no prescaler for timer 34, set the
> > +      * timer to dual 32-bit unchained mode, unreset both 32-bit timers.
> > +      */
> > +     writel_relaxed(DAVINCI_TIMER_TGCR_DEFAULT,
> > +                    base + DAVINCI_TIMER_REG_TGCR);
> > +     /* Init both counters to zero. */
> > +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM12);
> > +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM34);
> > +}
> > +
> > +int __init davinci_timer_register(struct clk *clk,
> > +                               const struct davinci_timer_cfg *timer_cfg)
> > +{
> > +     struct davinci_clockevent *clockevent;
> > +     unsigned int tick_rate, shift;
> > +     void __iomem *base;
> > +     int rv;
> > +
> > +     rv = clk_prepare_enable(clk);
> > +     if (rv) {
> > +             pr_err("Unable to prepare and enable the timer clock");
> > +             return rv;
> > +     }
> > +
> > +     base = request_mem_region(timer_cfg->reg.start,
> > +                               resource_size(&timer_cfg->reg),
> > +                               "davinci-timer");
> > +     if (!base) {
> > +             pr_err("Unable to request memory region");
> > +             return -EBUSY;
> > +     }
> > +
> > +     base = ioremap(timer_cfg->reg.start, resource_size(&timer_cfg->reg));
> > +     if (!base) {
> > +             pr_err("Unable to map the register range");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     davinci_timer_init(base);
> > +     tick_rate = clk_get_rate(clk);
> > +
> > +     clockevent = kzalloc(sizeof(*clockevent), GFP_KERNEL);
> > +     if (!clockevent) {
> > +             pr_err("Error allocating memory for clockevent data");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     clockevent->dev.name = "tim12";
> > +     clockevent->dev.features = CLOCK_EVT_FEAT_ONESHOT;
> > +     clockevent->dev.cpumask = cpumask_of(0);
> > +
> > +     clockevent->base = base;
> > +     clockevent->tim_off = DAVINCI_TIMER_REG_TIM12;
> > +     clockevent->prd_off = DAVINCI_TIMER_REG_PRD12;
> > +
> > +     shift = DAVINCI_TIMER_ENAMODE_SHIFT_TIM12;
> > +     clockevent->enamode_disabled = DAVINCI_TIMER_ENAMODE_DISABLED << shift;
> > +     clockevent->enamode_oneshot = DAVINCI_TIMER_ENAMODE_ONESHOT << shift;
> > +     clockevent->enamode_mask = DAVINCI_TIMER_ENAMODE_MASK << shift;
> > +
> > +     clockevent->dev.set_state_shutdown = davinci_clockevent_shutdown;
> > +     clockevent->dev.set_state_oneshot = davinci_clockevent_set_oneshot;
> > +
> > +     if (timer_cfg->cmp_off) {
> > +             clockevent->cmp_off = timer_cfg->cmp_off;
> > +             clockevent->dev.set_next_event =
> > +                             davinci_clockevent_set_next_event_cmp;
> > +     } else {
> > +             clockevent->dev.set_next_event =
> > +                             davinci_clockevent_set_next_event_std;
> > +     }
> > +
> > +     rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKEVENT_IRQ].start,
> > +                      davinci_timer_irq_timer, IRQF_TIMER,
> > +                      "clockevent", clockevent);
> > +     if (rv) {
> > +             pr_err("Unable to request the clockevent interrupt");
> > +             return rv;
> > +     }
> > +
> > +     clockevents_config_and_register(&clockevent->dev, tick_rate,
> > +                                     DAVINCI_TIMER_MIN_DELTA,
> > +                                     DAVINCI_TIMER_MAX_DELTA);
> > +
> > +     return 0;
> > +}
> > +
> > +static int __init of_davinci_timer_register(struct device_node *np)
> > +{
> > +     struct davinci_timer_cfg timer_cfg = { };
> > +     struct clk *clk;
> > +     int rv;
> > +
> > +     rv = of_address_to_resource(np, 0, &timer_cfg.reg);
> > +     if (rv) {
> > +             pr_err("Unable to get the register range for timer");
> > +             return rv;
> > +     }
> > +
> > +     rv = of_irq_to_resource_table(np, timer_cfg.irq,
> > +                                   DAVINCI_TIMER_NUM_IRQS);
> > +     if (rv != DAVINCI_TIMER_NUM_IRQS) {
> > +             pr_err("Unable to get the interrupts for timer");
> > +             return rv;
> > +     }
> > +
> > +     clk = of_clk_get(np, 0);
> > +     if (IS_ERR(clk)) {
> > +             pr_err("Unable to get the timer clock");
> > +             return PTR_ERR(clk);
> > +     }
> > +
> > +     rv = davinci_timer_register(clk, &timer_cfg);
> > +     if (rv)
> > +             clk_put(clk);
> > +
> > +     return rv;
> > +}
> > +TIMER_OF_DECLARE(davinci_timer, "ti,da830-timer", of_davinci_timer_register);
> > diff --git a/include/clocksource/timer-davinci.h b/include/clocksource/timer-davinci.h
> > new file mode 100644
> > index 000000000000..1dcc1333fbc8
> > --- /dev/null
> > +++ b/include/clocksource/timer-davinci.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * TI DaVinci clocksource driver
> > + *
> > + * Copyright (C) 2019 Texas Instruments
> > + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > + */
> > +
> > +#ifndef __TIMER_DAVINCI_H__
> > +#define __TIMER_DAVINCI_H__
> > +
> > +#include <linux/clk.h>
> > +#include <linux/ioport.h>
> > +
> > +enum {
> > +     DAVINCI_TIMER_CLOCKEVENT_IRQ,
> > +     DAVINCI_TIMER_CLOCKSOURCE_IRQ,
> > +     DAVINCI_TIMER_NUM_IRQS,
> > +};
> > +
> > +/**
> > + * struct davinci_timer_cfg - davinci clocksource driver configuration struct
> > + * @reg:        register range resource
> > + * @irq:        clockevent and clocksource interrupt resources
> > + * @cmp_off:    if set - it specifies the compare register used for clockevent
> > + *
> > + * Note: if the compare register is specified, the driver will use the bottom
> > + * clock half for both clocksource and clockevent and the compare register
> > + * to generate event irqs. The user must supply the correct compare register
> > + * interrupt number.
> > + *
> > + * This is only used by da830 the DSP of which uses the top half. The timer
> > + * driver still configures the top half to run in free-run mode.
> > + */
> > +struct davinci_timer_cfg {
> > +     struct resource reg;
> > +     struct resource irq[DAVINCI_TIMER_NUM_IRQS];
> > +     unsigned int cmp_off;
> > +};
> > +
> > +int __init davinci_timer_register(struct clk *clk,
> > +                               const struct davinci_timer_cfg *data);
> > +
> > +#endif /* __TIMER_DAVINCI_H__ */
> >
>
>
> --
>  <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: [RFC v2 1/2] clocksource: davinci-timer: add support for clockevents

[Daniel Lezcano]

On 24/05/2019 09:28, Bartosz Golaszewski wrote:
> czw., 23 maj 2019 o 18:38 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>>
>> On 23/05/2019 14:58, Bartosz Golaszewski wrote:
>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>
>>> Currently the clocksource and clockevent support for davinci platforms
>>> lives in mach-davinci. It hard-codes many things, uses global variables,
>>> implements functionalities unused by any platform and has code fragments
>>> scattered across many (often unrelated) files.
>>>
>>> Implement a new, modern and simplified timer driver and put it into
>>> drivers/clocksource. We still need to support legacy board files so
>>> export a config structure and a function that allows machine code to
>>> register the timer.
>>>
>>> The timer we're using is 64-bit but can be programmed in dual 32-bit
>>> mode (both chained and unchained). We're using dual 32-bit mode to
>>> have separate counters for clockevents and clocksource.
>>>
>>> This patch contains the core code and support for clockevent. The
>>> clocksource code will be included in a subsequent patch.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> ---
>>>  drivers/clocksource/Kconfig         |   5 +
>>>  drivers/clocksource/Makefile        |   1 +
>>>  drivers/clocksource/timer-davinci.c | 285 ++++++++++++++++++++++++++++
>>>  include/clocksource/timer-davinci.h |  44 +++++
>>>  4 files changed, 335 insertions(+)
>>>  create mode 100644 drivers/clocksource/timer-davinci.c
>>>  create mode 100644 include/clocksource/timer-davinci.h
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index 6bcaa4e2e72c..32dee6abd54a 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -42,6 +42,11 @@ config BCM_KONA_TIMER
>>>       help
>>>         Enables the support for the BCM Kona mobile timer driver.
>>>
>>> +config DAVINCI_TIMER
>>> +     bool "Texas Instruments DaVinci timer driver" if COMPILE_TEST
>>> +     help
>>> +       Enables the support for the TI DaVinci timer driver.
>>> +
>>>  config DIGICOLOR_TIMER
>>>       bool "Digicolor timer driver" if COMPILE_TEST
>>>       select CLKSRC_MMIO
>>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>>> index 236858fa7fbf..021831bcc567 100644
>>> --- a/drivers/clocksource/Makefile
>>> +++ b/drivers/clocksource/Makefile
>>> @@ -15,6 +15,7 @@ obj-$(CONFIG_SH_TIMER_TMU)  += sh_tmu.o
>>>  obj-$(CONFIG_EM_TIMER_STI)   += em_sti.o
>>>  obj-$(CONFIG_CLKBLD_I8253)   += i8253.o
>>>  obj-$(CONFIG_CLKSRC_MMIO)    += mmio.o
>>> +obj-$(CONFIG_DAVINCI_TIMER)  += timer-davinci.o
>>>  obj-$(CONFIG_DIGICOLOR_TIMER)        += timer-digicolor.o
>>>  obj-$(CONFIG_OMAP_DM_TIMER)  += timer-ti-dm.o
>>>  obj-$(CONFIG_DW_APB_TIMER)   += dw_apb_timer.o
>>> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
>>> new file mode 100644
>>> index 000000000000..a8fc7b3805c9
>>> --- /dev/null
>>> +++ b/drivers/clocksource/timer-davinci.c
>>> @@ -0,0 +1,285 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * TI DaVinci clocksource driver
>>> + *
>>> + * Copyright (C) 2019 Texas Instruments
>>> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> + * (with tiny parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/sched_clock.h>
>>> +
>>> +#include <clocksource/timer-davinci.h>
>>> +
>>> +#undef pr_fmt
>>> +#define pr_fmt(fmt) "%s: " fmt "\n", __func__
>>> +
>>> +#define DAVINCI_TIMER_REG_TIM12                      0x10
>>> +#define DAVINCI_TIMER_REG_TIM34                      0x14
>>> +#define DAVINCI_TIMER_REG_PRD12                      0x18
>>> +#define DAVINCI_TIMER_REG_PRD34                      0x1c
>>> +#define DAVINCI_TIMER_REG_TCR                        0x20
>>> +#define DAVINCI_TIMER_REG_TGCR                       0x24
>>> +
>>> +#define DAVINCI_TIMER_TIMMODE_MASK           GENMASK(3, 2)
>>> +#define DAVINCI_TIMER_RESET_MASK             GENMASK(1, 0)
>>> +#define DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED        BIT(2)
>>> +#define DAVINCI_TIMER_UNRESET                        GENMASK(1, 0)
>>> +
>>> +#define DAVINCI_TIMER_ENAMODE_MASK           GENMASK(1, 0)
>>> +#define DAVINCI_TIMER_ENAMODE_DISABLED               0x00
>>> +#define DAVINCI_TIMER_ENAMODE_ONESHOT                BIT(0)
>>> +
>>> +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM12    6
>>> +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM34    22
>>> +
>>> +#define DAVINCI_TIMER_MIN_DELTA                      0x01
>>> +#define DAVINCI_TIMER_MAX_DELTA                      0xfffffffe
>>> +
>>> +#define DAVINCI_TIMER_TGCR_DEFAULT \
>>> +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
>>> +
>>> +/* Cache the TCR register value. */
>>> +static unsigned int davinci_tcr;
>>> +
>>> +struct davinci_clockevent {
>>> +     struct clock_event_device dev;
>>> +     void __iomem *base;
>>> +
>>> +     unsigned int tim_off;
>>> +     unsigned int prd_off;
>>> +     unsigned int cmp_off;
>>> +
>>> +     unsigned int enamode_disabled;
>>> +     unsigned int enamode_oneshot;
>>> +     unsigned int enamode_mask;
>>> +};
>>> +
>>> +static struct davinci_clockevent *
>>> +to_davinci_clockevent(struct clock_event_device *clockevent)
>>> +{
>>> +     return container_of(clockevent, struct davinci_clockevent, dev);
>>> +}
>>> +
>>> +static unsigned int
>>> +davinci_clockevent_read(struct davinci_clockevent *clockevent,
>>> +                     unsigned int reg)
>>> +{
>>> +     return readl_relaxed(clockevent->base + reg);
>>> +}
>>> +
>>> +static void davinci_clockevent_write(struct davinci_clockevent *clockevent,
>>> +                                  unsigned int reg, unsigned int val)
>>> +{
>>> +     writel_relaxed(val, clockevent->base + reg);
>>> +}
>>> +
>>> +static void davinci_tcr_update(void __iomem *base,
>>> +                            unsigned int mask, unsigned int val)
>>> +{
>>> +     davinci_tcr &= ~mask;
>>> +     davinci_tcr |= val & mask;
>>
>>
>> I don't see when the davinci_tcr is initialized.
>>
> 
> It's set to 0x0 by the compiler and we're setting the register to 0x0
> in davinci_timer_init().

Why did you need to readl before in the previous version? The idea of
caching the value was to save an extra readl.

If it is always zero, then we don't need this variable neither the read,
just doing:

writel_relaxed(val & mask, base + DAVINCI_TIMER_REG_TCR);

should work no ?


>>> +     writel_relaxed(davinci_tcr, base + DAVINCI_TIMER_REG_TCR);
>>> +}
>>> +
>>> +static int davinci_clockevent_shutdown(struct clock_event_device *dev)
>>> +{
>>> +     struct davinci_clockevent *clockevent;
>>> +
>>> +     clockevent = to_davinci_clockevent(dev);
>>> +
>>> +     davinci_tcr_update(clockevent->base,
>>> +                        clockevent->enamode_mask,
>>> +                        clockevent->enamode_disabled);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int davinci_clockevent_set_oneshot(struct clock_event_device *dev)
>>> +{
>>> +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
>>> +
>>> +     davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
>>> +
>>> +     davinci_tcr_update(clockevent->base,
>>> +                        clockevent->enamode_mask,
>>> +                        clockevent->enamode_oneshot);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int
>>> +davinci_clockevent_set_next_event_std(unsigned long cycles,
>>> +                                   struct clock_event_device *dev)
>>> +{
>>> +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
>>> +
>>> +     davinci_clockevent_shutdown(dev);
>>> +
>>> +     davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
>>> +     davinci_clockevent_write(clockevent, clockevent->prd_off, cycles);
>>> +
>>> +     davinci_clockevent_set_oneshot(dev);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int
>>> +davinci_clockevent_set_next_event_cmp(unsigned long cycles,
>>> +                                   struct clock_event_device *dev)
>>> +{
>>> +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
>>> +     unsigned int curr_time;
>>> +
>>> +     curr_time = davinci_clockevent_read(clockevent, clockevent->tim_off);
>>> +     davinci_clockevent_write(clockevent,
>>> +                              clockevent->cmp_off, curr_time + cycles);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static irqreturn_t davinci_timer_irq_timer(int irq, void *data)
>>> +{
>>> +     struct davinci_clockevent *clockevent = data;
>>> +
>>> +     if (!clockevent_state_oneshot(&clockevent->dev))
>>> +             davinci_tcr_update(clockevent,
>>> +                                clockevent->enamode_mask,
>>> +                                clockevent->enamode_disabled);
>>> +
>>> +     clockevent->dev.event_handler(&clockevent->dev);
>>> +
>>> +     return IRQ_HANDLED;
>>> +}
>>> +
>>> +static void davinci_timer_init(void __iomem *base)
>>> +{
>>> +     /* Set clock to internal mode and disable it. */
>>> +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TCR);
>>> +     /*
>>> +      * Reset both 32-bit timers, set no prescaler for timer 34, set the
>>> +      * timer to dual 32-bit unchained mode, unreset both 32-bit timers.
>>> +      */
>>> +     writel_relaxed(DAVINCI_TIMER_TGCR_DEFAULT,
>>> +                    base + DAVINCI_TIMER_REG_TGCR);
>>> +     /* Init both counters to zero. */
>>> +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM12);
>>> +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM34);
>>> +}
>>> +
>>> +int __init davinci_timer_register(struct clk *clk,
>>> +                               const struct davinci_timer_cfg *timer_cfg)
>>> +{
>>> +     struct davinci_clockevent *clockevent;
>>> +     unsigned int tick_rate, shift;
>>> +     void __iomem *base;
>>> +     int rv;
>>> +
>>> +     rv = clk_prepare_enable(clk);
>>> +     if (rv) {
>>> +             pr_err("Unable to prepare and enable the timer clock");
>>> +             return rv;
>>> +     }
>>> +
>>> +     base = request_mem_region(timer_cfg->reg.start,
>>> +                               resource_size(&timer_cfg->reg),
>>> +                               "davinci-timer");
>>> +     if (!base) {
>>> +             pr_err("Unable to request memory region");
>>> +             return -EBUSY;
>>> +     }
>>> +
>>> +     base = ioremap(timer_cfg->reg.start, resource_size(&timer_cfg->reg));
>>> +     if (!base) {
>>> +             pr_err("Unable to map the register range");
>>> +             return -ENOMEM;
>>> +     }
>>> +
>>> +     davinci_timer_init(base);
>>> +     tick_rate = clk_get_rate(clk);
>>> +
>>> +     clockevent = kzalloc(sizeof(*clockevent), GFP_KERNEL);
>>> +     if (!clockevent) {
>>> +             pr_err("Error allocating memory for clockevent data");
>>> +             return -ENOMEM;
>>> +     }
>>> +
>>> +     clockevent->dev.name = "tim12";
>>> +     clockevent->dev.features = CLOCK_EVT_FEAT_ONESHOT;
>>> +     clockevent->dev.cpumask = cpumask_of(0);
>>> +
>>> +     clockevent->base = base;
>>> +     clockevent->tim_off = DAVINCI_TIMER_REG_TIM12;
>>> +     clockevent->prd_off = DAVINCI_TIMER_REG_PRD12;
>>> +
>>> +     shift = DAVINCI_TIMER_ENAMODE_SHIFT_TIM12;
>>> +     clockevent->enamode_disabled = DAVINCI_TIMER_ENAMODE_DISABLED << shift;
>>> +     clockevent->enamode_oneshot = DAVINCI_TIMER_ENAMODE_ONESHOT << shift;
>>> +     clockevent->enamode_mask = DAVINCI_TIMER_ENAMODE_MASK << shift;
>>> +
>>> +     clockevent->dev.set_state_shutdown = davinci_clockevent_shutdown;
>>> +     clockevent->dev.set_state_oneshot = davinci_clockevent_set_oneshot;
>>> +
>>> +     if (timer_cfg->cmp_off) {
>>> +             clockevent->cmp_off = timer_cfg->cmp_off;
>>> +             clockevent->dev.set_next_event =
>>> +                             davinci_clockevent_set_next_event_cmp;
>>> +     } else {
>>> +             clockevent->dev.set_next_event =
>>> +                             davinci_clockevent_set_next_event_std;
>>> +     }
>>> +
>>> +     rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKEVENT_IRQ].start,
>>> +                      davinci_timer_irq_timer, IRQF_TIMER,
>>> +                      "clockevent", clockevent);
>>> +     if (rv) {
>>> +             pr_err("Unable to request the clockevent interrupt");
>>> +             return rv;
>>> +     }
>>> +
>>> +     clockevents_config_and_register(&clockevent->dev, tick_rate,
>>> +                                     DAVINCI_TIMER_MIN_DELTA,
>>> +                                     DAVINCI_TIMER_MAX_DELTA);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int __init of_davinci_timer_register(struct device_node *np)
>>> +{
>>> +     struct davinci_timer_cfg timer_cfg = { };
>>> +     struct clk *clk;
>>> +     int rv;
>>> +
>>> +     rv = of_address_to_resource(np, 0, &timer_cfg.reg);
>>> +     if (rv) {
>>> +             pr_err("Unable to get the register range for timer");
>>> +             return rv;
>>> +     }
>>> +
>>> +     rv = of_irq_to_resource_table(np, timer_cfg.irq,
>>> +                                   DAVINCI_TIMER_NUM_IRQS);
>>> +     if (rv != DAVINCI_TIMER_NUM_IRQS) {
>>> +             pr_err("Unable to get the interrupts for timer");
>>> +             return rv;
>>> +     }
>>> +
>>> +     clk = of_clk_get(np, 0);
>>> +     if (IS_ERR(clk)) {
>>> +             pr_err("Unable to get the timer clock");
>>> +             return PTR_ERR(clk);
>>> +     }
>>> +
>>> +     rv = davinci_timer_register(clk, &timer_cfg);
>>> +     if (rv)
>>> +             clk_put(clk);
>>> +
>>> +     return rv;
>>> +}
>>> +TIMER_OF_DECLARE(davinci_timer, "ti,da830-timer", of_davinci_timer_register);
>>> diff --git a/include/clocksource/timer-davinci.h b/include/clocksource/timer-davinci.h
>>> new file mode 100644
>>> index 000000000000..1dcc1333fbc8
>>> --- /dev/null
>>> +++ b/include/clocksource/timer-davinci.h
>>> @@ -0,0 +1,44 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * TI DaVinci clocksource driver
>>> + *
>>> + * Copyright (C) 2019 Texas Instruments
>>> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> + */
>>> +
>>> +#ifndef __TIMER_DAVINCI_H__
>>> +#define __TIMER_DAVINCI_H__
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/ioport.h>
>>> +
>>> +enum {
>>> +     DAVINCI_TIMER_CLOCKEVENT_IRQ,
>>> +     DAVINCI_TIMER_CLOCKSOURCE_IRQ,
>>> +     DAVINCI_TIMER_NUM_IRQS,
>>> +};
>>> +
>>> +/**
>>> + * struct davinci_timer_cfg - davinci clocksource driver configuration struct
>>> + * @reg:        register range resource
>>> + * @irq:        clockevent and clocksource interrupt resources
>>> + * @cmp_off:    if set - it specifies the compare register used for clockevent
>>> + *
>>> + * Note: if the compare register is specified, the driver will use the bottom
>>> + * clock half for both clocksource and clockevent and the compare register
>>> + * to generate event irqs. The user must supply the correct compare register
>>> + * interrupt number.
>>> + *
>>> + * This is only used by da830 the DSP of which uses the top half. The timer
>>> + * driver still configures the top half to run in free-run mode.
>>> + */
>>> +struct davinci_timer_cfg {
>>> +     struct resource reg;
>>> +     struct resource irq[DAVINCI_TIMER_NUM_IRQS];
>>> +     unsigned int cmp_off;
>>> +};
>>> +
>>> +int __init davinci_timer_register(struct clk *clk,
>>> +                               const struct davinci_timer_cfg *data);
>>> +
>>> +#endif /* __TIMER_DAVINCI_H__ */
>>>
>>
>>
>> --
>>  <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
>>


-- 
 <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: [RFC v2 1/2] clocksource: davinci-timer: add support for clockevents

[Bartosz Golaszewski]

pt., 24 maj 2019 o 10:59 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>
> On 24/05/2019 09:28, Bartosz Golaszewski wrote:
> > czw., 23 maj 2019 o 18:38 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
> >>
> >> On 23/05/2019 14:58, Bartosz Golaszewski wrote:
> >>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>>
> >>> Currently the clocksource and clockevent support for davinci platforms
> >>> lives in mach-davinci. It hard-codes many things, uses global variables,
> >>> implements functionalities unused by any platform and has code fragments
> >>> scattered across many (often unrelated) files.
> >>>
> >>> Implement a new, modern and simplified timer driver and put it into
> >>> drivers/clocksource. We still need to support legacy board files so
> >>> export a config structure and a function that allows machine code to
> >>> register the timer.
> >>>
> >>> The timer we're using is 64-bit but can be programmed in dual 32-bit
> >>> mode (both chained and unchained). We're using dual 32-bit mode to
> >>> have separate counters for clockevents and clocksource.
> >>>
> >>> This patch contains the core code and support for clockevent. The
> >>> clocksource code will be included in a subsequent patch.
> >>>
> >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>> ---
> >>>  drivers/clocksource/Kconfig         |   5 +
> >>>  drivers/clocksource/Makefile        |   1 +
> >>>  drivers/clocksource/timer-davinci.c | 285 ++++++++++++++++++++++++++++
> >>>  include/clocksource/timer-davinci.h |  44 +++++
> >>>  4 files changed, 335 insertions(+)
> >>>  create mode 100644 drivers/clocksource/timer-davinci.c
> >>>  create mode 100644 include/clocksource/timer-davinci.h
> >>>
> >>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> >>> index 6bcaa4e2e72c..32dee6abd54a 100644
> >>> --- a/drivers/clocksource/Kconfig
> >>> +++ b/drivers/clocksource/Kconfig
> >>> @@ -42,6 +42,11 @@ config BCM_KONA_TIMER
> >>>       help
> >>>         Enables the support for the BCM Kona mobile timer driver.
> >>>
> >>> +config DAVINCI_TIMER
> >>> +     bool "Texas Instruments DaVinci timer driver" if COMPILE_TEST
> >>> +     help
> >>> +       Enables the support for the TI DaVinci timer driver.
> >>> +
> >>>  config DIGICOLOR_TIMER
> >>>       bool "Digicolor timer driver" if COMPILE_TEST
> >>>       select CLKSRC_MMIO
> >>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> >>> index 236858fa7fbf..021831bcc567 100644
> >>> --- a/drivers/clocksource/Makefile
> >>> +++ b/drivers/clocksource/Makefile
> >>> @@ -15,6 +15,7 @@ obj-$(CONFIG_SH_TIMER_TMU)  += sh_tmu.o
> >>>  obj-$(CONFIG_EM_TIMER_STI)   += em_sti.o
> >>>  obj-$(CONFIG_CLKBLD_I8253)   += i8253.o
> >>>  obj-$(CONFIG_CLKSRC_MMIO)    += mmio.o
> >>> +obj-$(CONFIG_DAVINCI_TIMER)  += timer-davinci.o
> >>>  obj-$(CONFIG_DIGICOLOR_TIMER)        += timer-digicolor.o
> >>>  obj-$(CONFIG_OMAP_DM_TIMER)  += timer-ti-dm.o
> >>>  obj-$(CONFIG_DW_APB_TIMER)   += dw_apb_timer.o
> >>> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> >>> new file mode 100644
> >>> index 000000000000..a8fc7b3805c9
> >>> --- /dev/null
> >>> +++ b/drivers/clocksource/timer-davinci.c
> >>> @@ -0,0 +1,285 @@
> >>> +// SPDX-License-Identifier: GPL-2.0-only
> >>> +/*
> >>> + * TI DaVinci clocksource driver
> >>> + *
> >>> + * Copyright (C) 2019 Texas Instruments
> >>> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>> + * (with tiny parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> >>> + */
> >>> +
> >>> +#include <linux/clk.h>
> >>> +#include <linux/clockchips.h>
> >>> +#include <linux/interrupt.h>
> >>> +#include <linux/kernel.h>
> >>> +#include <linux/of_address.h>
> >>> +#include <linux/of_irq.h>
> >>> +#include <linux/sched_clock.h>
> >>> +
> >>> +#include <clocksource/timer-davinci.h>
> >>> +
> >>> +#undef pr_fmt
> >>> +#define pr_fmt(fmt) "%s: " fmt "\n", __func__
> >>> +
> >>> +#define DAVINCI_TIMER_REG_TIM12                      0x10
> >>> +#define DAVINCI_TIMER_REG_TIM34                      0x14
> >>> +#define DAVINCI_TIMER_REG_PRD12                      0x18
> >>> +#define DAVINCI_TIMER_REG_PRD34                      0x1c
> >>> +#define DAVINCI_TIMER_REG_TCR                        0x20
> >>> +#define DAVINCI_TIMER_REG_TGCR                       0x24
> >>> +
> >>> +#define DAVINCI_TIMER_TIMMODE_MASK           GENMASK(3, 2)
> >>> +#define DAVINCI_TIMER_RESET_MASK             GENMASK(1, 0)
> >>> +#define DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED        BIT(2)
> >>> +#define DAVINCI_TIMER_UNRESET                        GENMASK(1, 0)
> >>> +
> >>> +#define DAVINCI_TIMER_ENAMODE_MASK           GENMASK(1, 0)
> >>> +#define DAVINCI_TIMER_ENAMODE_DISABLED               0x00
> >>> +#define DAVINCI_TIMER_ENAMODE_ONESHOT                BIT(0)
> >>> +
> >>> +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM12    6
> >>> +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM34    22
> >>> +
> >>> +#define DAVINCI_TIMER_MIN_DELTA                      0x01
> >>> +#define DAVINCI_TIMER_MAX_DELTA                      0xfffffffe
> >>> +
> >>> +#define DAVINCI_TIMER_TGCR_DEFAULT \
> >>> +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> >>> +
> >>> +/* Cache the TCR register value. */
> >>> +static unsigned int davinci_tcr;
> >>> +
> >>> +struct davinci_clockevent {
> >>> +     struct clock_event_device dev;
> >>> +     void __iomem *base;
> >>> +
> >>> +     unsigned int tim_off;
> >>> +     unsigned int prd_off;
> >>> +     unsigned int cmp_off;
> >>> +
> >>> +     unsigned int enamode_disabled;
> >>> +     unsigned int enamode_oneshot;
> >>> +     unsigned int enamode_mask;
> >>> +};
> >>> +
> >>> +static struct davinci_clockevent *
> >>> +to_davinci_clockevent(struct clock_event_device *clockevent)
> >>> +{
> >>> +     return container_of(clockevent, struct davinci_clockevent, dev);
> >>> +}
> >>> +
> >>> +static unsigned int
> >>> +davinci_clockevent_read(struct davinci_clockevent *clockevent,
> >>> +                     unsigned int reg)
> >>> +{
> >>> +     return readl_relaxed(clockevent->base + reg);
> >>> +}
> >>> +
> >>> +static void davinci_clockevent_write(struct davinci_clockevent *clockevent,
> >>> +                                  unsigned int reg, unsigned int val)
> >>> +{
> >>> +     writel_relaxed(val, clockevent->base + reg);
> >>> +}
> >>> +
> >>> +static void davinci_tcr_update(void __iomem *base,
> >>> +                            unsigned int mask, unsigned int val)
> >>> +{
> >>> +     davinci_tcr &= ~mask;
> >>> +     davinci_tcr |= val & mask;
> >>
> >>
> >> I don't see when the davinci_tcr is initialized.
> >>
> >
> > It's set to 0x0 by the compiler and we're setting the register to 0x0
> > in davinci_timer_init().
>
> Why did you need to readl before in the previous version? The idea of
> caching the value was to save an extra readl.
>
> If it is always zero, then we don't need this variable neither the read,
> just doing:
>
> writel_relaxed(val & mask, base + DAVINCI_TIMER_REG_TCR);
>
> should work no ?

It's not always zero. Its reset value is zero and we write 0 to it at
init time just to make sure, but then we modify it according to the
configuration. The single TCR register controls both halves of the
timer, so we do need an actual update, not a simple write.

Bart

>
>
> >>> +     writel_relaxed(davinci_tcr, base + DAVINCI_TIMER_REG_TCR);
> >>> +}
> >>> +
> >>> +static int davinci_clockevent_shutdown(struct clock_event_device *dev)
> >>> +{
> >>> +     struct davinci_clockevent *clockevent;
> >>> +
> >>> +     clockevent = to_davinci_clockevent(dev);
> >>> +
> >>> +     davinci_tcr_update(clockevent->base,
> >>> +                        clockevent->enamode_mask,
> >>> +                        clockevent->enamode_disabled);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int davinci_clockevent_set_oneshot(struct clock_event_device *dev)
> >>> +{
> >>> +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> >>> +
> >>> +     davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
> >>> +
> >>> +     davinci_tcr_update(clockevent->base,
> >>> +                        clockevent->enamode_mask,
> >>> +                        clockevent->enamode_oneshot);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int
> >>> +davinci_clockevent_set_next_event_std(unsigned long cycles,
> >>> +                                   struct clock_event_device *dev)
> >>> +{
> >>> +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> >>> +
> >>> +     davinci_clockevent_shutdown(dev);
> >>> +
> >>> +     davinci_clockevent_write(clockevent, clockevent->tim_off, 0x0);
> >>> +     davinci_clockevent_write(clockevent, clockevent->prd_off, cycles);
> >>> +
> >>> +     davinci_clockevent_set_oneshot(dev);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int
> >>> +davinci_clockevent_set_next_event_cmp(unsigned long cycles,
> >>> +                                   struct clock_event_device *dev)
> >>> +{
> >>> +     struct davinci_clockevent *clockevent = to_davinci_clockevent(dev);
> >>> +     unsigned int curr_time;
> >>> +
> >>> +     curr_time = davinci_clockevent_read(clockevent, clockevent->tim_off);
> >>> +     davinci_clockevent_write(clockevent,
> >>> +                              clockevent->cmp_off, curr_time + cycles);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static irqreturn_t davinci_timer_irq_timer(int irq, void *data)
> >>> +{
> >>> +     struct davinci_clockevent *clockevent = data;
> >>> +
> >>> +     if (!clockevent_state_oneshot(&clockevent->dev))
> >>> +             davinci_tcr_update(clockevent,
> >>> +                                clockevent->enamode_mask,
> >>> +                                clockevent->enamode_disabled);
> >>> +
> >>> +     clockevent->dev.event_handler(&clockevent->dev);
> >>> +
> >>> +     return IRQ_HANDLED;
> >>> +}
> >>> +
> >>> +static void davinci_timer_init(void __iomem *base)
> >>> +{
> >>> +     /* Set clock to internal mode and disable it. */
> >>> +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TCR);
> >>> +     /*
> >>> +      * Reset both 32-bit timers, set no prescaler for timer 34, set the
> >>> +      * timer to dual 32-bit unchained mode, unreset both 32-bit timers.
> >>> +      */
> >>> +     writel_relaxed(DAVINCI_TIMER_TGCR_DEFAULT,
> >>> +                    base + DAVINCI_TIMER_REG_TGCR);
> >>> +     /* Init both counters to zero. */
> >>> +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM12);
> >>> +     writel_relaxed(0x0, base + DAVINCI_TIMER_REG_TIM34);
> >>> +}
> >>> +
> >>> +int __init davinci_timer_register(struct clk *clk,
> >>> +                               const struct davinci_timer_cfg *timer_cfg)
> >>> +{
> >>> +     struct davinci_clockevent *clockevent;
> >>> +     unsigned int tick_rate, shift;
> >>> +     void __iomem *base;
> >>> +     int rv;
> >>> +
> >>> +     rv = clk_prepare_enable(clk);
> >>> +     if (rv) {
> >>> +             pr_err("Unable to prepare and enable the timer clock");
> >>> +             return rv;
> >>> +     }
> >>> +
> >>> +     base = request_mem_region(timer_cfg->reg.start,
> >>> +                               resource_size(&timer_cfg->reg),
> >>> +                               "davinci-timer");
> >>> +     if (!base) {
> >>> +             pr_err("Unable to request memory region");
> >>> +             return -EBUSY;
> >>> +     }
> >>> +
> >>> +     base = ioremap(timer_cfg->reg.start, resource_size(&timer_cfg->reg));
> >>> +     if (!base) {
> >>> +             pr_err("Unable to map the register range");
> >>> +             return -ENOMEM;
> >>> +     }
> >>> +
> >>> +     davinci_timer_init(base);
> >>> +     tick_rate = clk_get_rate(clk);
> >>> +
> >>> +     clockevent = kzalloc(sizeof(*clockevent), GFP_KERNEL);
> >>> +     if (!clockevent) {
> >>> +             pr_err("Error allocating memory for clockevent data");
> >>> +             return -ENOMEM;
> >>> +     }
> >>> +
> >>> +     clockevent->dev.name = "tim12";
> >>> +     clockevent->dev.features = CLOCK_EVT_FEAT_ONESHOT;
> >>> +     clockevent->dev.cpumask = cpumask_of(0);
> >>> +
> >>> +     clockevent->base = base;
> >>> +     clockevent->tim_off = DAVINCI_TIMER_REG_TIM12;
> >>> +     clockevent->prd_off = DAVINCI_TIMER_REG_PRD12;
> >>> +
> >>> +     shift = DAVINCI_TIMER_ENAMODE_SHIFT_TIM12;
> >>> +     clockevent->enamode_disabled = DAVINCI_TIMER_ENAMODE_DISABLED << shift;
> >>> +     clockevent->enamode_oneshot = DAVINCI_TIMER_ENAMODE_ONESHOT << shift;
> >>> +     clockevent->enamode_mask = DAVINCI_TIMER_ENAMODE_MASK << shift;
> >>> +
> >>> +     clockevent->dev.set_state_shutdown = davinci_clockevent_shutdown;
> >>> +     clockevent->dev.set_state_oneshot = davinci_clockevent_set_oneshot;
> >>> +
> >>> +     if (timer_cfg->cmp_off) {
> >>> +             clockevent->cmp_off = timer_cfg->cmp_off;
> >>> +             clockevent->dev.set_next_event =
> >>> +                             davinci_clockevent_set_next_event_cmp;
> >>> +     } else {
> >>> +             clockevent->dev.set_next_event =
> >>> +                             davinci_clockevent_set_next_event_std;
> >>> +     }
> >>> +
> >>> +     rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKEVENT_IRQ].start,
> >>> +                      davinci_timer_irq_timer, IRQF_TIMER,
> >>> +                      "clockevent", clockevent);
> >>> +     if (rv) {
> >>> +             pr_err("Unable to request the clockevent interrupt");
> >>> +             return rv;
> >>> +     }
> >>> +
> >>> +     clockevents_config_and_register(&clockevent->dev, tick_rate,
> >>> +                                     DAVINCI_TIMER_MIN_DELTA,
> >>> +                                     DAVINCI_TIMER_MAX_DELTA);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int __init of_davinci_timer_register(struct device_node *np)
> >>> +{
> >>> +     struct davinci_timer_cfg timer_cfg = { };
> >>> +     struct clk *clk;
> >>> +     int rv;
> >>> +
> >>> +     rv = of_address_to_resource(np, 0, &timer_cfg.reg);
> >>> +     if (rv) {
> >>> +             pr_err("Unable to get the register range for timer");
> >>> +             return rv;
> >>> +     }
> >>> +
> >>> +     rv = of_irq_to_resource_table(np, timer_cfg.irq,
> >>> +                                   DAVINCI_TIMER_NUM_IRQS);
> >>> +     if (rv != DAVINCI_TIMER_NUM_IRQS) {
> >>> +             pr_err("Unable to get the interrupts for timer");
> >>> +             return rv;
> >>> +     }
> >>> +
> >>> +     clk = of_clk_get(np, 0);
> >>> +     if (IS_ERR(clk)) {
> >>> +             pr_err("Unable to get the timer clock");
> >>> +             return PTR_ERR(clk);
> >>> +     }
> >>> +
> >>> +     rv = davinci_timer_register(clk, &timer_cfg);
> >>> +     if (rv)
> >>> +             clk_put(clk);
> >>> +
> >>> +     return rv;
> >>> +}
> >>> +TIMER_OF_DECLARE(davinci_timer, "ti,da830-timer", of_davinci_timer_register);
> >>> diff --git a/include/clocksource/timer-davinci.h b/include/clocksource/timer-davinci.h
> >>> new file mode 100644
> >>> index 000000000000..1dcc1333fbc8
> >>> --- /dev/null
> >>> +++ b/include/clocksource/timer-davinci.h
> >>> @@ -0,0 +1,44 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0-only */
> >>> +/*
> >>> + * TI DaVinci clocksource driver
> >>> + *
> >>> + * Copyright (C) 2019 Texas Instruments
> >>> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>> + */
> >>> +
> >>> +#ifndef __TIMER_DAVINCI_H__
> >>> +#define __TIMER_DAVINCI_H__
> >>> +
> >>> +#include <linux/clk.h>
> >>> +#include <linux/ioport.h>
> >>> +
> >>> +enum {
> >>> +     DAVINCI_TIMER_CLOCKEVENT_IRQ,
> >>> +     DAVINCI_TIMER_CLOCKSOURCE_IRQ,
> >>> +     DAVINCI_TIMER_NUM_IRQS,
> >>> +};
> >>> +
> >>> +/**
> >>> + * struct davinci_timer_cfg - davinci clocksource driver configuration struct
> >>> + * @reg:        register range resource
> >>> + * @irq:        clockevent and clocksource interrupt resources
> >>> + * @cmp_off:    if set - it specifies the compare register used for clockevent
> >>> + *
> >>> + * Note: if the compare register is specified, the driver will use the bottom
> >>> + * clock half for both clocksource and clockevent and the compare register
> >>> + * to generate event irqs. The user must supply the correct compare register
> >>> + * interrupt number.
> >>> + *
> >>> + * This is only used by da830 the DSP of which uses the top half. The timer
> >>> + * driver still configures the top half to run in free-run mode.
> >>> + */
> >>> +struct davinci_timer_cfg {
> >>> +     struct resource reg;
> >>> +     struct resource irq[DAVINCI_TIMER_NUM_IRQS];
> >>> +     unsigned int cmp_off;
> >>> +};
> >>> +
> >>> +int __init davinci_timer_register(struct clk *clk,
> >>> +                               const struct davinci_timer_cfg *data);
> >>> +
> >>> +#endif /* __TIMER_DAVINCI_H__ */
> >>>
> >>
> >>
> >> --
> >>  <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
> >>
>
>
> --
>  <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: [RFC v2 1/2] clocksource: davinci-timer: add support for clockevents

[Daniel Lezcano]

On 24/05/2019 13:53, Bartosz Golaszewski wrote:
> pt., 24 maj 2019 o 10:59 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>>
>> On 24/05/2019 09:28, Bartosz Golaszewski wrote:
>>> czw., 23 maj 2019 o 18:38 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>>>>
>>>> On 23/05/2019 14:58, Bartosz Golaszewski wrote:
>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>
>>>>> Currently the clocksource and clockevent support for davinci platforms
>>>>> lives in mach-davinci. It hard-codes many things, uses global variables,
>>>>> implements functionalities unused by any platform and has code fragments
>>>>> scattered across many (often unrelated) files.
>>>>>
>>>>> Implement a new, modern and simplified timer driver and put it into
>>>>> drivers/clocksource. We still need to support legacy board files so
>>>>> export a config structure and a function that allows machine code to
>>>>> register the timer.
>>>>>
>>>>> The timer we're using is 64-bit but can be programmed in dual 32-bit
>>>>> mode (both chained and unchained). We're using dual 32-bit mode to
>>>>> have separate counters for clockevents and clocksource.
>>>>>
>>>>> This patch contains the core code and support for clockevent. The
>>>>> clocksource code will be included in a subsequent patch.
>>>>>

[ ... ]

>>>>> +static unsigned int
>>>>> +davinci_clockevent_read(struct davinci_clockevent *clockevent,
>>>>> +                     unsigned int reg)
>>>>> +{
>>>>> +     return readl_relaxed(clockevent->base + reg);
>>>>> +}
>>>>> +
>>>>> +static void davinci_clockevent_write(struct davinci_clockevent *clockevent,
>>>>> +                                  unsigned int reg, unsigned int val)
>>>>> +{
>>>>> +     writel_relaxed(val, clockevent->base + reg);
>>>>> +}
>>>>> +
>>>>> +static void davinci_tcr_update(void __iomem *base,
>>>>> +                            unsigned int mask, unsigned int val)
>>>>> +{
>>>>> +     davinci_tcr &= ~mask;
>>>>> +     davinci_tcr |= val & mask;
>>>>
>>>>
>>>> I don't see when the davinci_tcr is initialized.
>>>>
>>>
>>> It's set to 0x0 by the compiler and we're setting the register to 0x0
>>> in davinci_timer_init().
>>
>> Why did you need to readl before in the previous version? The idea of
>> caching the value was to save an extra readl.
>>
>> If it is always zero, then we don't need this variable neither the read,
>> just doing:
>>
>> writel_relaxed(val & mask, base + DAVINCI_TIMER_REG_TCR);
>>
>> should work no ?
> 
> It's not always zero. Its reset value is zero and we write 0 to it at
> init time just to make sure, but then we modify it according to the
> configuration. The single TCR register controls both halves of the
> timer, so we do need an actual update, not a simple write.

Ok but the driver can be oneshot or disabled in the code (mutually
exclusive), no ?

So doing

 - writel(oneshot, base);
 - writel(disabled, base);

works without any mask computation, no?

Well the above assumes other part of the register aren't changed by
other subsystems (or by the timer itself).











-- :
 <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: [RFC v2 1/2] clocksource: davinci-timer: add support for clockevents

[Bartosz Golaszewski]

sob., 25 maj 2019 o 16:16 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>
> On 24/05/2019 13:53, Bartosz Golaszewski wrote:
> > pt., 24 maj 2019 o 10:59 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
> >>
> >> On 24/05/2019 09:28, Bartosz Golaszewski wrote:
> >>> czw., 23 maj 2019 o 18:38 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
> >>>>
> >>>> On 23/05/2019 14:58, Bartosz Golaszewski wrote:
> >>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>>>>
> >>>>> Currently the clocksource and clockevent support for davinci platforms
> >>>>> lives in mach-davinci. It hard-codes many things, uses global variables,
> >>>>> implements functionalities unused by any platform and has code fragments
> >>>>> scattered across many (often unrelated) files.
> >>>>>
> >>>>> Implement a new, modern and simplified timer driver and put it into
> >>>>> drivers/clocksource. We still need to support legacy board files so
> >>>>> export a config structure and a function that allows machine code to
> >>>>> register the timer.
> >>>>>
> >>>>> The timer we're using is 64-bit but can be programmed in dual 32-bit
> >>>>> mode (both chained and unchained). We're using dual 32-bit mode to
> >>>>> have separate counters for clockevents and clocksource.
> >>>>>
> >>>>> This patch contains the core code and support for clockevent. The
> >>>>> clocksource code will be included in a subsequent patch.
> >>>>>
>
> [ ... ]
>
> >>>>> +static unsigned int
> >>>>> +davinci_clockevent_read(struct davinci_clockevent *clockevent,
> >>>>> +                     unsigned int reg)
> >>>>> +{
> >>>>> +     return readl_relaxed(clockevent->base + reg);
> >>>>> +}
> >>>>> +
> >>>>> +static void davinci_clockevent_write(struct davinci_clockevent *clockevent,
> >>>>> +                                  unsigned int reg, unsigned int val)
> >>>>> +{
> >>>>> +     writel_relaxed(val, clockevent->base + reg);
> >>>>> +}
> >>>>> +
> >>>>> +static void davinci_tcr_update(void __iomem *base,
> >>>>> +                            unsigned int mask, unsigned int val)
> >>>>> +{
> >>>>> +     davinci_tcr &= ~mask;
> >>>>> +     davinci_tcr |= val & mask;
> >>>>
> >>>>
> >>>> I don't see when the davinci_tcr is initialized.
> >>>>
> >>>
> >>> It's set to 0x0 by the compiler and we're setting the register to 0x0
> >>> in davinci_timer_init().
> >>
> >> Why did you need to readl before in the previous version? The idea of
> >> caching the value was to save an extra readl.
> >>
> >> If it is always zero, then we don't need this variable neither the read,
> >> just doing:
> >>
> >> writel_relaxed(val & mask, base + DAVINCI_TIMER_REG_TCR);
> >>
> >> should work no ?
> >
> > It's not always zero. Its reset value is zero and we write 0 to it at
> > init time just to make sure, but then we modify it according to the
> > configuration. The single TCR register controls both halves of the
> > timer, so we do need an actual update, not a simple write.
>
> Ok but the driver can be oneshot or disabled in the code (mutually
> exclusive), no ?
>
> So doing
>
>  - writel(oneshot, base);
>  - writel(disabled, base);
>
> works without any mask computation, no?
>
> Well the above assumes other part of the register aren't changed by
> other subsystems (or by the timer itself).
>
>

I'm not sure I understand. You can be using two timers. Both
controlled by a single TCR register. In your example oneshot can equal
(0x00, or 0x01) and either be shifted left by 6 or 22 for TIM12 and
TIM34 respectively. If you do writel(oneshot-for-time12, base) you'll
set tim34 to disabled.

Bart

>
>
>
>
>
>
>
>
>
> -- :
>  <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: [RFC v2 1/2] clocksource: davinci-timer: add support for clockevents

[Bartosz Golaszewski]

niedz., 26 maj 2019 o 10:16 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> sob., 25 maj 2019 o 16:16 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
> >
> > On 24/05/2019 13:53, Bartosz Golaszewski wrote:
> > > pt., 24 maj 2019 o 10:59 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
> > >>
> > >> On 24/05/2019 09:28, Bartosz Golaszewski wrote:
> > >>> czw., 23 maj 2019 o 18:38 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
> > >>>>
> > >>>> On 23/05/2019 14:58, Bartosz Golaszewski wrote:
> > >>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >>>>>
> > >>>>> Currently the clocksource and clockevent support for davinci platforms
> > >>>>> lives in mach-davinci. It hard-codes many things, uses global variables,
> > >>>>> implements functionalities unused by any platform and has code fragments
> > >>>>> scattered across many (often unrelated) files.
> > >>>>>
> > >>>>> Implement a new, modern and simplified timer driver and put it into
> > >>>>> drivers/clocksource. We still need to support legacy board files so
> > >>>>> export a config structure and a function that allows machine code to
> > >>>>> register the timer.
> > >>>>>
> > >>>>> The timer we're using is 64-bit but can be programmed in dual 32-bit
> > >>>>> mode (both chained and unchained). We're using dual 32-bit mode to
> > >>>>> have separate counters for clockevents and clocksource.
> > >>>>>
> > >>>>> This patch contains the core code and support for clockevent. The
> > >>>>> clocksource code will be included in a subsequent patch.
> > >>>>>
> >
> > [ ... ]
> >
> > >>>>> +static unsigned int
> > >>>>> +davinci_clockevent_read(struct davinci_clockevent *clockevent,
> > >>>>> +                     unsigned int reg)
> > >>>>> +{
> > >>>>> +     return readl_relaxed(clockevent->base + reg);
> > >>>>> +}
> > >>>>> +
> > >>>>> +static void davinci_clockevent_write(struct davinci_clockevent *clockevent,
> > >>>>> +                                  unsigned int reg, unsigned int val)
> > >>>>> +{
> > >>>>> +     writel_relaxed(val, clockevent->base + reg);
> > >>>>> +}
> > >>>>> +
> > >>>>> +static void davinci_tcr_update(void __iomem *base,
> > >>>>> +                            unsigned int mask, unsigned int val)
> > >>>>> +{
> > >>>>> +     davinci_tcr &= ~mask;
> > >>>>> +     davinci_tcr |= val & mask;
> > >>>>
> > >>>>
> > >>>> I don't see when the davinci_tcr is initialized.
> > >>>>
> > >>>
> > >>> It's set to 0x0 by the compiler and we're setting the register to 0x0
> > >>> in davinci_timer_init().
> > >>
> > >> Why did you need to readl before in the previous version? The idea of
> > >> caching the value was to save an extra readl.
> > >>
> > >> If it is always zero, then we don't need this variable neither the read,
> > >> just doing:
> > >>
> > >> writel_relaxed(val & mask, base + DAVINCI_TIMER_REG_TCR);
> > >>
> > >> should work no ?
> > >
> > > It's not always zero. Its reset value is zero and we write 0 to it at
> > > init time just to make sure, but then we modify it according to the
> > > configuration. The single TCR register controls both halves of the
> > > timer, so we do need an actual update, not a simple write.
> >
> > Ok but the driver can be oneshot or disabled in the code (mutually
> > exclusive), no ?
> >
> > So doing
> >
> >  - writel(oneshot, base);
> >  - writel(disabled, base);
> >
> > works without any mask computation, no?
> >
> > Well the above assumes other part of the register aren't changed by
> > other subsystems (or by the timer itself).
> >
> >
>
> I'm not sure I understand. You can be using two timers. Both
> controlled by a single TCR register. In your example oneshot can equal
> (0x00, or 0x01) and either be shifted left by 6 or 22 for TIM12 and
> TIM34 respectively. If you do writel(oneshot-for-time12, base) you'll
> set tim34 to disabled.
>
> Bart

Gentle ping.

Bart

[RFC v2 2/2] clocksource: timer-davinci: add support for clocksource

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Extend the davinci-timer driver to also register a clock source.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/clocksource/timer-davinci.c | 70 +++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
index a8fc7b3805c9..c478991da253 100644
--- a/drivers/clocksource/timer-davinci.c
+++ b/drivers/clocksource/timer-davinci.c
@@ -42,6 +42,8 @@
 #define DAVINCI_TIMER_MIN_DELTA			0x01
 #define DAVINCI_TIMER_MAX_DELTA			0xfffffffe
 
+#define DAVINCI_TIMER_CLKSRC_BITS		32
+
 #define DAVINCI_TIMER_TGCR_DEFAULT \
 		(DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
 
@@ -61,6 +63,16 @@ struct davinci_clockevent {
 	unsigned int enamode_mask;
 };
 
+/*
+ * This must be globally accessible by davinci_timer_read_sched_clock(), so
+ * let's keep it here.
+ */
+static struct {
+	struct clocksource dev;
+	void __iomem *base;
+	unsigned int tim_off;
+} davinci_clocksource;
+
 static struct davinci_clockevent *
 to_davinci_clockevent(struct clock_event_device *clockevent)
 {
@@ -159,6 +171,32 @@ static irqreturn_t davinci_timer_irq_timer(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static u64 notrace davinci_timer_read_sched_clock(void)
+{
+	return readl_relaxed(davinci_clocksource.base +
+			     davinci_clocksource.tim_off);
+}
+
+static u64 davinci_clocksource_read(struct clocksource *dev)
+{
+	return davinci_timer_read_sched_clock();
+}
+
+static void davinci_clocksource_init(void __iomem *base, unsigned int tim_off,
+				     unsigned int prd_off, unsigned int shift)
+{
+	davinci_tcr_update(base,
+			   DAVINCI_TIMER_ENAMODE_MASK << shift,
+			   DAVINCI_TIMER_ENAMODE_DISABLED << shift);
+
+	writel_relaxed(0x0, base + tim_off);
+	writel_relaxed(UINT_MAX, base + prd_off);
+
+	davinci_tcr_update(base,
+			   DAVINCI_TIMER_ENAMODE_MASK << shift,
+			   DAVINCI_TIMER_ENAMODE_ONESHOT << shift);
+}
+
 static void davinci_timer_init(void __iomem *base)
 {
 	/* Set clock to internal mode and disable it. */
@@ -248,6 +286,38 @@ int __init davinci_timer_register(struct clk *clk,
 					DAVINCI_TIMER_MIN_DELTA,
 					DAVINCI_TIMER_MAX_DELTA);
 
+	davinci_clocksource.dev.rating = 300;
+	davinci_clocksource.dev.read = davinci_clocksource_read;
+	davinci_clocksource.dev.mask =
+			CLOCKSOURCE_MASK(DAVINCI_TIMER_CLKSRC_BITS);
+	davinci_clocksource.dev.flags = CLOCK_SOURCE_IS_CONTINUOUS;
+	davinci_clocksource.base = base;
+
+	if (timer_cfg->cmp_off) {
+		davinci_clocksource.dev.name = "tim12";
+		davinci_clocksource.tim_off = DAVINCI_TIMER_REG_TIM12;
+		davinci_clocksource_init(base,
+					 DAVINCI_TIMER_REG_TIM12,
+					 DAVINCI_TIMER_REG_PRD12,
+					 DAVINCI_TIMER_ENAMODE_SHIFT_TIM12);
+	} else {
+		davinci_clocksource.dev.name = "tim34";
+		davinci_clocksource.tim_off = DAVINCI_TIMER_REG_TIM34;
+		davinci_clocksource_init(base,
+					 DAVINCI_TIMER_REG_TIM34,
+					 DAVINCI_TIMER_REG_PRD34,
+					 DAVINCI_TIMER_ENAMODE_SHIFT_TIM34);
+	}
+
+	rv = clocksource_register_hz(&davinci_clocksource.dev, tick_rate);
+	if (rv) {
+		pr_err("Unable to register clocksource");
+		return rv;
+	}
+
+	sched_clock_register(davinci_timer_read_sched_clock,
+			     DAVINCI_TIMER_CLKSRC_BITS, tick_rate);
+
 	return 0;
 }
 
-- 
2.21.0