[PATCH v4 00/11] ARM: davinci: modernize the timer support

[PATCH v4 00/11] ARM: davinci: modernize the timer support

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This series removes the legacy timer code from mach-davinci in favor of
a new clocksource driver it introduces.

Patch 1 adds a new clocksource driver for davinci.

Patch 2 enables the new driver for device-tree based systems.

Patch 3 adds a WARN_ON() to the machine code of all davinci boards
which is triggered if clk_get() for the timer clock fails. This is needed
as the new driver expects the clock to be functional and doesn't check it.

Patches 4-5 and 7-10 switch the board files to using the new
clocksource driver while patch 6 moves some necessary defines to
a different place since we'll be removing the file that contains them.

Patch 11 removes legacy timer code.

v1 -> v2:

- changed the license to GPL-only
- dropped regmap usage due to the additional code paths making the
  code too slow
- minor code rearrangement
- fixed the da830 timer configuration
- correctly assign the timer halves to clocksource and clockevent
- sqashed the commits adding WARN() when clk_get() fails
- used WARN_ON() and added a return when clk_get() fails to avoid
  entering the timer driver without a valid clock
- request the register range before iomapping it
- simplified the configuration structure (dropped the additional
  cmp struct)

v2 -> v3:

- use readl/writel_relaxed() instead of __raw variants in the clocksource
  driver
- request the memory region before remapping it
- add explanatory comments
- fixed licensing
- used pr_fmt macro
- correctly assign the clocksource name
- use DEFINE_RES_x macros when defining resources to avoid off-by-one
  bugs in resource's size
- tweak the commit messages
- don't delete the comments describing the timer usage
- other minor improvements
- rebased on top of Sekhar's soc branch
- dropped patch fixing the clocksource interrupts in device tree (already
  upstream)

v3 -> v4:
- fixed formatting of error messages
- fixed comments all around the place
- rebased on top of v5.1-rc1

Bartosz Golaszewski (11):
  clocksource: davinci-timer: new driver
  ARM: davinci: enable the clocksource driver for DT mode
  ARM: davinci: WARN_ON() if clk_get() fails
  ARM: davinci: da850: switch to using the clocksource driver
  ARM: davinci: da830: switch to using the clocksource driver
  ARM: davinci: move timer definitions to davinci.h
  ARM: davinci: dm355: switch to using the clocksource driver
  ARM: davinci: dm365: switch to using the clocksource driver
  ARM: davinci: dm644x: switch to using the clocksource driver
  ARM: davinci: dm646x: switch to using the clocksource driver
  ARM: davinci: remove legacy timer support

 arch/arm/Kconfig                            |   1 +
 arch/arm/mach-davinci/Makefile              |   3 +-
 arch/arm/mach-davinci/da830.c               |  45 +-
 arch/arm/mach-davinci/da850.c               |  50 +--
 arch/arm/mach-davinci/davinci.h             |   3 +
 arch/arm/mach-davinci/devices-da8xx.c       |   1 -
 arch/arm/mach-davinci/devices.c             |  19 -
 arch/arm/mach-davinci/dm355.c               |  28 +-
 arch/arm/mach-davinci/dm365.c               |  26 +-
 arch/arm/mach-davinci/dm644x.c              |  28 +-
 arch/arm/mach-davinci/dm646x.c              |  28 +-
 arch/arm/mach-davinci/include/mach/common.h |  17 -
 arch/arm/mach-davinci/include/mach/time.h   |  35 --
 arch/arm/mach-davinci/time.c                | 414 ------------------
 drivers/clocksource/Kconfig                 |   5 +
 drivers/clocksource/Makefile                |   1 +
 drivers/clocksource/timer-davinci.c         | 438 ++++++++++++++++++++
 include/clocksource/timer-davinci.h         |  44 ++
 18 files changed, 598 insertions(+), 588 deletions(-)
 delete mode 100644 arch/arm/mach-davinci/include/mach/time.h
 delete mode 100644 arch/arm/mach-davinci/time.c
 create mode 100644 drivers/clocksource/timer-davinci.c
 create mode 100644 include/clocksource/timer-davinci.h

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 01/11] clocksource: davinci-timer: new driver

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

We don't bother freeing resources on errors in davinci_timer_register()
as the system won't boot without a timer anyway.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 drivers/clocksource/Kconfig         |   5 +
 drivers/clocksource/Makefile        |   1 +
 drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
 include/clocksource/timer-davinci.h |  44 +++
 4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"
+	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 be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
--- /dev/null
+++ b/drivers/clocksource/timer-davinci.c
@@ -0,0 +1,438 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// TI DaVinci clocksource driver
+//
+// Copyright (C) 2019 Texas Instruments
+// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/err.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)
+
+/* Shift depends on timer. */
+#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_PERIODIC		BIT(1)
+
+#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_CLKSRC_BITS		32
+
+#define DAVINCI_TIMER_TGCR_DEFAULT \
+		(DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
+
+enum {
+	DAVINCI_TIMER_MODE_DISABLED = 0,
+	DAVINCI_TIMER_MODE_ONESHOT,
+	DAVINCI_TIMER_MODE_PERIODIC,
+};
+
+struct davinci_timer_data;
+
+typedef void (*davinci_timer_set_period_func)(struct davinci_timer_data *,
+					      unsigned int period);
+
+/**
+ * struct davinci_timer_regs - timer-specific register offsets
+ *
+ * @tim_off: timer counter register
+ * @prd_off: timer period register
+ * @enamode_shift: left bit-shift of the enable register associated
+ *                 with this timer in the TCR register
+ */
+struct davinci_timer_regs {
+	unsigned int tim_off;
+	unsigned int prd_off;
+	unsigned int enamode_shift;
+};
+
+struct davinci_timer_data {
+	void __iomem *base;
+	const struct davinci_timer_regs *regs;
+	unsigned int mode;
+	davinci_timer_set_period_func set_period;
+	unsigned int cmp_off;
+};
+
+struct davinci_timer_clockevent {
+	struct clock_event_device dev;
+	unsigned int tick_rate;
+	struct davinci_timer_data timer;
+};
+
+struct davinci_timer_clocksource {
+	struct clocksource dev;
+	struct davinci_timer_data timer;
+};
+
+static const struct davinci_timer_regs davinci_timer_tim12_regs = {
+	.tim_off		= DAVINCI_TIMER_REG_TIM12,
+	.prd_off		= DAVINCI_TIMER_REG_PRD12,
+	.enamode_shift		= DAVINCI_TIMER_ENAMODE_SHIFT_TIM12,
+};
+
+static const struct davinci_timer_regs davinci_timer_tim34_regs = {
+	.tim_off		= DAVINCI_TIMER_REG_TIM34,
+	.prd_off		= DAVINCI_TIMER_REG_PRD34,
+	.enamode_shift		= DAVINCI_TIMER_ENAMODE_SHIFT_TIM34,
+};
+
+/* Must be global for davinci_timer_read_sched_clock(). */
+static struct davinci_timer_data *davinci_timer_clksrc_timer;
+
+static struct davinci_timer_clockevent *
+to_davinci_timer_clockevent(struct clock_event_device *clockevent)
+{
+	return container_of(clockevent, struct davinci_timer_clockevent, dev);
+}
+
+static struct davinci_timer_clocksource *
+to_davinci_timer_clocksource(struct clocksource *clocksource)
+{
+	return container_of(clocksource, struct davinci_timer_clocksource, dev);
+}
+
+static unsigned int davinci_timer_read(struct davinci_timer_data *timer,
+				       unsigned int reg)
+{
+	return readl_relaxed(timer->base + reg);
+}
+
+static void davinci_timer_write(struct davinci_timer_data *timer,
+				unsigned int reg, unsigned int val)
+{
+	writel_relaxed(val, timer->base + reg);
+}
+
+static void davinci_timer_update(struct davinci_timer_data *timer,
+				 unsigned int reg, unsigned int mask,
+				 unsigned int val)
+{
+	unsigned int new, orig;
+
+	orig = davinci_timer_read(timer, reg);
+	new = orig & ~mask;
+	new |= val & mask;
+
+	davinci_timer_write(timer, reg, new);
+}
+
+static void davinci_timer_set_period(struct davinci_timer_data *timer,
+				     unsigned int period)
+{
+	timer->set_period(timer, period);
+}
+
+static void davinci_timer_set_period_std(struct davinci_timer_data *timer,
+					 unsigned int period)
+{
+	const struct davinci_timer_regs *regs = timer->regs;
+	unsigned int enamode;
+
+	enamode = davinci_timer_read(timer, DAVINCI_TIMER_REG_TCR);
+
+	davinci_timer_update(timer, DAVINCI_TIMER_REG_TCR,
+			DAVINCI_TIMER_ENAMODE_MASK << regs->enamode_shift,
+			DAVINCI_TIMER_ENAMODE_DISABLED << regs->enamode_shift);
+
+	davinci_timer_write(timer, regs->tim_off, 0x0);
+	davinci_timer_write(timer, regs->prd_off, period);
+
+	if (timer->mode == DAVINCI_TIMER_MODE_ONESHOT)
+		enamode = DAVINCI_TIMER_ENAMODE_ONESHOT;
+	else if (timer->mode == DAVINCI_TIMER_MODE_PERIODIC)
+		enamode = DAVINCI_TIMER_ENAMODE_PERIODIC;
+
+	davinci_timer_update(timer, DAVINCI_TIMER_REG_TCR,
+			     DAVINCI_TIMER_ENAMODE_MASK << regs->enamode_shift,
+			     enamode << regs->enamode_shift);
+}
+
+static void davinci_timer_set_period_cmp(struct davinci_timer_data *timer,
+					 unsigned int period)
+{
+	const struct davinci_timer_regs *regs = timer->regs;
+	unsigned int curr_time;
+
+	curr_time = davinci_timer_read(timer, regs->tim_off);
+	davinci_timer_write(timer, timer->cmp_off, curr_time + period);
+}
+
+static irqreturn_t davinci_timer_irq_timer(int irq, void *data)
+{
+	struct davinci_timer_clockevent *clockevent = data;
+
+	clockevent->dev.event_handler(&clockevent->dev);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t davinci_timer_irq_freerun(int irq, void *data)
+{
+	return IRQ_HANDLED;
+}
+
+static u64 notrace davinci_timer_read_sched_clock(void)
+{
+	struct davinci_timer_data *timer;
+	unsigned int val;
+
+	timer = davinci_timer_clksrc_timer;
+	val = davinci_timer_read(timer, timer->regs->tim_off);
+
+	return val;
+}
+
+static u64 davinci_timer_clksrc_read(struct clocksource *dev)
+{
+	struct davinci_timer_clocksource *clocksource;
+	const struct davinci_timer_regs *regs;
+	unsigned int val;
+
+	clocksource = to_davinci_timer_clocksource(dev);
+	regs = clocksource->timer.regs;
+
+	val = davinci_timer_read(&clocksource->timer, regs->tim_off);
+
+	return val;
+}
+
+static int davinci_timer_set_next_event(unsigned long cycles,
+					struct clock_event_device *dev)
+{
+	struct davinci_timer_clockevent *clockevent;
+
+	clockevent = to_davinci_timer_clockevent(dev);
+	davinci_timer_set_period(&clockevent->timer, cycles);
+
+	return 0;
+}
+
+static int davinci_timer_set_state_shutdown(struct clock_event_device *dev)
+{
+	struct davinci_timer_clockevent *clockevent;
+
+	clockevent = to_davinci_timer_clockevent(dev);
+	clockevent->timer.mode = DAVINCI_TIMER_MODE_DISABLED;
+
+	return 0;
+}
+
+static int davinci_timer_set_state_periodic(struct clock_event_device *dev)
+{
+	struct davinci_timer_clockevent *clockevent;
+	unsigned int period;
+
+	clockevent = to_davinci_timer_clockevent(dev);
+	period = clockevent->tick_rate / HZ;
+
+	clockevent->timer.mode = DAVINCI_TIMER_MODE_PERIODIC;
+	davinci_timer_set_period(&clockevent->timer, period);
+
+	return 0;
+}
+
+static int davinci_timer_set_state_oneshot(struct clock_event_device *dev)
+{
+	struct davinci_timer_clockevent *clockevent;
+
+	clockevent = to_davinci_timer_clockevent(dev);
+	clockevent->timer.mode = DAVINCI_TIMER_MODE_ONESHOT;
+
+	return 0;
+}
+
+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_timer_clocksource *clocksource;
+	struct davinci_timer_clockevent *clockevent;
+	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);
+
+	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.set_next_event = davinci_timer_set_next_event;
+	clockevent->dev.set_state_shutdown = davinci_timer_set_state_shutdown;
+	clockevent->dev.set_state_periodic = davinci_timer_set_state_periodic;
+	clockevent->dev.set_state_oneshot = davinci_timer_set_state_oneshot;
+	clockevent->dev.cpumask = cpumask_of(0);
+	clockevent->tick_rate = clk_get_rate(clk);
+	clockevent->timer.mode = DAVINCI_TIMER_MODE_DISABLED;
+	clockevent->timer.base = base;
+	clockevent->timer.regs = &davinci_timer_tim12_regs;
+
+	if (timer_cfg->cmp_off) {
+		clockevent->timer.cmp_off = timer_cfg->cmp_off;
+		clockevent->timer.set_period = davinci_timer_set_period_cmp;
+	} else {
+		clockevent->dev.features |= CLOCK_EVT_FEAT_PERIODIC;
+		clockevent->timer.set_period = davinci_timer_set_period_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,
+					clockevent->tick_rate,
+					DAVINCI_TIMER_MIN_DELTA,
+					DAVINCI_TIMER_MAX_DELTA);
+
+	clocksource = kzalloc(sizeof(*clocksource), GFP_KERNEL);
+	if (!clocksource) {
+		pr_err("Error allocating memory for clocksource data");
+		return -ENOMEM;
+	}
+
+	clocksource->dev.rating = 300;
+	clocksource->dev.read = davinci_timer_clksrc_read;
+	clocksource->dev.mask = CLOCKSOURCE_MASK(DAVINCI_TIMER_CLKSRC_BITS);
+	clocksource->dev.flags = CLOCK_SOURCE_IS_CONTINUOUS;
+	clocksource->timer.set_period = davinci_timer_set_period_std;
+	clocksource->timer.mode = DAVINCI_TIMER_MODE_PERIODIC;
+	clocksource->timer.base = base;
+
+	if (timer_cfg->cmp_off) {
+		clocksource->timer.regs = &davinci_timer_tim12_regs;
+		clocksource->dev.name = "tim12";
+	} else {
+		clocksource->timer.regs = &davinci_timer_tim34_regs;
+		clocksource->dev.name = "tim34";
+	}
+
+	rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKSOURCE_IRQ].start,
+			 davinci_timer_irq_freerun, IRQF_TIMER,
+			 "free-run counter", clocksource);
+	if (rv) {
+		pr_err("Unable to request the clocksource interrupt");
+		return rv;
+	}
+
+	rv = clocksource_register_hz(&clocksource->dev, clockevent->tick_rate);
+	if (rv) {
+		pr_err("Unable to register clocksource");
+		return rv;
+	}
+
+	davinci_timer_clksrc_timer = &clocksource->timer;
+
+	sched_clock_register(davinci_timer_read_sched_clock,
+			     DAVINCI_TIMER_CLKSRC_BITS,
+			     clockevent->tick_rate);
+
+	davinci_timer_set_period(&clockevent->timer,
+				 clockevent->tick_rate / HZ);
+	davinci_timer_set_period(&clocksource->timer, UINT_MAX);
+
+	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.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 02/11] ARM: davinci: enable the clocksource driver for DT mode

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Switch all davinci boards supporting device tree to using the new
clocksource driver: remove the previous OF_TIMER_DECLARE() from
mach-davinci and select davinci-timer for ARCH_DAVINCI.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/Kconfig             |  1 +
 arch/arm/mach-davinci/time.c | 14 --------------
 2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 054ead960f98..f893798e13d3 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -588,6 +588,7 @@ config ARCH_DAVINCI
 	select ARCH_HAS_HOLES_MEMORYMODEL
 	select COMMON_CLK
 	select CPU_ARM926T
+	select DAVINCI_TIMER
 	select GENERIC_ALLOCATOR
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_IRQ_CHIP
diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
index 5a6de5368ab0..740410a3bb6a 100644
--- a/arch/arm/mach-davinci/time.c
+++ b/arch/arm/mach-davinci/time.c
@@ -398,17 +398,3 @@ void __init davinci_timer_init(struct clk *timer_clk)
 	for (i=0; i< ARRAY_SIZE(timers); i++)
 		timer32_config(&timers[i]);
 }
-
-static int __init of_davinci_timer_init(struct device_node *np)
-{
-	struct clk *clk;
-
-	clk = of_clk_get(np, 0);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
-
-	davinci_timer_init(clk);
-
-	return 0;
-}
-TIMER_OF_DECLARE(davinci_timer, "ti,da830-timer", of_davinci_timer_init);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 03/11] ARM: davinci: WARN_ON() if clk_get() fails

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Currently the timer code checks if the clock pointer passed to it is
good (!IS_ERR(clk)). The new clocksource driver expects the clock to
be functional and doesn't perform any checks so emit a warning if
clk_get() fails. Apply this to all davinci platforms.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/da830.c  | 4 ++++
 arch/arm/mach-davinci/da850.c  | 4 ++++
 arch/arm/mach-davinci/dm355.c  | 4 ++++
 arch/arm/mach-davinci/dm365.c  | 4 ++++
 arch/arm/mach-davinci/dm644x.c | 4 ++++
 arch/arm/mach-davinci/dm646x.c | 4 ++++
 6 files changed, 24 insertions(+)

diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index 63511f638ce4..d242ce06f7e5 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -750,6 +750,10 @@ void __init da830_init_time(void)
 	da830_pll_init(NULL, pll, NULL);
 
 	clk = clk_get(NULL, "timer0");
+	if (WARN_ON(IS_ERR(clk))) {
+		pr_err("Unable to get the timer clock\n");
+		return;
+	}
 
 	davinci_timer_init(clk);
 }
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 67ab71ba3ad3..72d64d39d42a 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -680,6 +680,10 @@ void __init da850_init_time(void)
 	da850_pll0_init(NULL, pll0, cfgchip);
 
 	clk = clk_get(NULL, "timer0");
+	if (WARN_ON(IS_ERR(clk))) {
+		pr_err("Unable to get the timer clock\n");
+		return;
+	}
 
 	davinci_timer_init(clk);
 }
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index 4a482445b9a2..57c5a660758e 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -742,6 +742,10 @@ void __init dm355_init_time(void)
 	dm355_psc_init(NULL, psc);
 
 	clk = clk_get(NULL, "timer0");
+	if (WARN_ON(IS_ERR(clk))) {
+		pr_err("Unable to get the timer clock\n");
+		return;
+	}
 
 	davinci_timer_init(clk);
 }
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 8e0a77315add..1d82bb630d11 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -783,6 +783,10 @@ void __init dm365_init_time(void)
 	dm365_psc_init(NULL, psc);
 
 	clk = clk_get(NULL, "timer0");
+	if (WARN_ON(IS_ERR(clk))) {
+		pr_err("Unable to get the timer clock\n");
+		return;
+	}
 
 	davinci_timer_init(clk);
 }
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index cecc7ceb8d34..2b0e921aa755 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -678,6 +678,10 @@ void __init dm644x_init_time(void)
 	dm644x_psc_init(NULL, psc);
 
 	clk = clk_get(NULL, "timer0");
+	if (WARN_ON(IS_ERR(clk))) {
+		pr_err("Unable to get the timer clock\n");
+		return;
+	}
 
 	davinci_timer_init(clk);
 }
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index f33392f77a03..7e5af984ed9f 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -662,6 +662,10 @@ void __init dm646x_init_time(unsigned long ref_clk_rate,
 	dm646x_psc_init(NULL, psc);
 
 	clk = clk_get(NULL, "timer0");
+	if (WARN_ON(IS_ERR(clk))) {
+		pr_err("Unable to get the timer clock\n");
+		return;
+	}
 
 	davinci_timer_init(clk);
 }
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 04/11] ARM: davinci: da850: switch to using the clocksource driver

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We now have a proper clocksource driver for davinci. Switch the da850
platform to using it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/da850.c | 46 ++++++++++-------------------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 72d64d39d42a..4e18bdf05ac4 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -34,7 +34,8 @@
 #include <mach/cputype.h>
 #include <mach/da8xx.h>
 #include <mach/pm.h>
-#include <mach/time.h>
+
+#include <clocksource/timer-davinci.h>
 
 #include "irqs.h"
 #include "mux.h"
@@ -332,38 +333,16 @@ static struct davinci_id da850_ids[] = {
 	},
 };
 
-static struct davinci_timer_instance da850_timer_instance[4] = {
-	{
-		.base		= DA8XX_TIMER64P0_BASE,
-		.bottom_irq	= DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT12_0),
-		.top_irq	= DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT34_0),
-	},
-	{
-		.base		= DA8XX_TIMER64P1_BASE,
-		.bottom_irq	= DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT12_1),
-		.top_irq	= DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT34_1),
-	},
-	{
-		.base		= DA850_TIMER64P2_BASE,
-		.bottom_irq	= DAVINCI_INTC_IRQ(IRQ_DA850_TINT12_2),
-		.top_irq	= DAVINCI_INTC_IRQ(IRQ_DA850_TINT34_2),
-	},
-	{
-		.base		= DA850_TIMER64P3_BASE,
-		.bottom_irq	= DAVINCI_INTC_IRQ(IRQ_DA850_TINT12_3),
-		.top_irq	= DAVINCI_INTC_IRQ(IRQ_DA850_TINT34_3),
-	},
-};
-
 /*
- * T0_BOT: Timer 0, bottom		: Used for clock_event
- * T0_TOP: Timer 0, top			: Used for clocksource
- * T1_BOT, T1_TOP: Timer 1, bottom & top: Used for watchdog timer
+ * Bottom half of timer 0 is used for clock_event, top half for
+ * clocksource.
  */
-static struct davinci_timer_info da850_timer_info = {
-	.timers		= da850_timer_instance,
-	.clockevent_id	= T0_BOT,
-	.clocksource_id	= T0_TOP,
+static const struct davinci_timer_cfg da850_timer_cfg = {
+	.reg = DEFINE_RES_IO(DA8XX_TIMER64P0_BASE, SZ_4K),
+	.irq = {
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT12_0)),
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT34_0)),
+	},
 };
 
 #ifdef CONFIG_CPU_FREQ
@@ -634,7 +613,6 @@ static const struct davinci_soc_info davinci_soc_info_da850 = {
 	.pinmux_base		= DA8XX_SYSCFG0_BASE + 0x120,
 	.pinmux_pins		= da850_pins,
 	.pinmux_pins_num	= ARRAY_SIZE(da850_pins),
-	.timer_info		= &da850_timer_info,
 	.emac_pdata		= &da8xx_emac_pdata,
 	.sram_dma		= DA8XX_SHARED_RAM_BASE,
 	.sram_len		= SZ_128K,
@@ -671,6 +649,7 @@ void __init da850_init_time(void)
 	void __iomem *pll0;
 	struct regmap *cfgchip;
 	struct clk *clk;
+	int rv;
 
 	clk_register_fixed_rate(NULL, "ref_clk", NULL, 0, DA850_REF_FREQ);
 
@@ -685,7 +664,8 @@ void __init da850_init_time(void)
 		return;
 	}
 
-	davinci_timer_init(clk);
+	rv = davinci_timer_register(clk, &da850_timer_cfg);
+	WARN(rv, "Unable to register the timer: %d\n", rv);
 }
 
 static struct resource da850_pll1_resources[] = {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 05/11] ARM: davinci: da830: switch to using the clocksource driver

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We now have a proper clocksource driver for davinci. Switch the da830
platform to using it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/da830.c | 41 ++++++++++++-----------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index d242ce06f7e5..3b2c9454828c 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -20,7 +20,8 @@
 #include <mach/common.h>
 #include <mach/cputype.h>
 #include <mach/da8xx.h>
-#include <mach/time.h>
+
+#include <clocksource/timer-davinci.h>
 
 #include "irqs.h"
 #include "mux.h"
@@ -675,32 +676,17 @@ int __init da830_register_gpio(void)
 	return da8xx_register_gpio(&da830_gpio_platform_data);
 }
 
-static struct davinci_timer_instance da830_timer_instance[2] = {
-	{
-		.base		= DA8XX_TIMER64P0_BASE,
-		.bottom_irq	= DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT12_0),
-		.top_irq	= DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT34_0),
-		.cmp_off	= DA830_CMP12_0,
-		.cmp_irq	= DAVINCI_INTC_IRQ(IRQ_DA830_T12CMPINT0_0),
-	},
-	{
-		.base		= DA8XX_TIMER64P1_BASE,
-		.bottom_irq	= DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT12_1),
-		.top_irq	= DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT34_1),
-		.cmp_off	= DA830_CMP12_0,
-		.cmp_irq	= DAVINCI_INTC_IRQ(IRQ_DA830_T12CMPINT0_1),
-	},
-};
-
 /*
- * T0_BOT: Timer 0, bottom		: Used for clock_event & clocksource
- * T0_TOP: Timer 0, top			: Used by DSP
- * T1_BOT, T1_TOP: Timer 1, bottom & top: Used for watchdog timer
+ * Bottom half of timer0 is used both for clock even and clocksource.
+ * Top half is used by DSP.
  */
-static struct davinci_timer_info da830_timer_info = {
-	.timers		= da830_timer_instance,
-	.clockevent_id	= T0_BOT,
-	.clocksource_id	= T0_BOT,
+static const struct davinci_timer_cfg da830_timer_cfg = {
+	.reg = DEFINE_RES_IO(DA8XX_TIMER64P0_BASE, SZ_4K),
+	.irq = {
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_DA830_T12CMPINT0_0)),
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_DA8XX_TINT12_0)),
+	},
+	.cmp_off = DA830_CMP12_0,
 };
 
 static const struct davinci_soc_info davinci_soc_info_da830 = {
@@ -712,7 +698,6 @@ static const struct davinci_soc_info davinci_soc_info_da830 = {
 	.pinmux_base		= DA8XX_SYSCFG0_BASE + 0x120,
 	.pinmux_pins		= da830_pins,
 	.pinmux_pins_num	= ARRAY_SIZE(da830_pins),
-	.timer_info		= &da830_timer_info,
 	.emac_pdata		= &da8xx_emac_pdata,
 };
 
@@ -742,6 +727,7 @@ void __init da830_init_time(void)
 {
 	void __iomem *pll;
 	struct clk *clk;
+	int rv;
 
 	clk_register_fixed_rate(NULL, "ref_clk", NULL, 0, DA830_REF_FREQ);
 
@@ -755,7 +741,8 @@ void __init da830_init_time(void)
 		return;
 	}
 
-	davinci_timer_init(clk);
+	rv = davinci_timer_register(clk, &da830_timer_cfg);
+	WARN(rv, "Unable to register the timer: %d\n", rv);
 }
 
 static struct resource da830_psc0_resources[] = {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 06/11] ARM: davinci: move timer definitions to davinci.h

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Boards from the dm* family rely on register offset definitions from
arch/arm/mach-davinci/include/mach/time.h. We'll be removing this file
soon, so move the required defines to davinci.h where the rest of such
constants live.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/davinci.h           | 3 +++
 arch/arm/mach-davinci/include/mach/time.h | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-davinci/davinci.h b/arch/arm/mach-davinci/davinci.h
index 56c1835c42e5..208d7a4d3597 100644
--- a/arch/arm/mach-davinci/davinci.h
+++ b/arch/arm/mach-davinci/davinci.h
@@ -60,6 +60,9 @@ void davinci_map_sysmod(void);
 #define DAVINCI_GPIO_BASE 0x01C67000
 int davinci_gpio_register(struct resource *res, int size, void *pdata);
 
+#define DAVINCI_TIMER0_BASE		(IO_PHYS + 0x21400)
+#define DAVINCI_WDOG_BASE		(IO_PHYS + 0x21C00)
+
 /* DM355 base addresses */
 #define DM355_ASYNC_EMIF_CONTROL_BASE	0x01e10000
 #define DM355_ASYNC_EMIF_DATA_CE0_BASE	0x02000000
diff --git a/arch/arm/mach-davinci/include/mach/time.h b/arch/arm/mach-davinci/include/mach/time.h
index 1c971d8d8ba8..ba913736990f 100644
--- a/arch/arm/mach-davinci/include/mach/time.h
+++ b/arch/arm/mach-davinci/include/mach/time.h
@@ -11,9 +11,7 @@
 #ifndef __ARCH_ARM_MACH_DAVINCI_TIME_H
 #define __ARCH_ARM_MACH_DAVINCI_TIME_H
 
-#define DAVINCI_TIMER0_BASE		(IO_PHYS + 0x21400)
 #define DAVINCI_TIMER1_BASE		(IO_PHYS + 0x21800)
-#define DAVINCI_WDOG_BASE		(IO_PHYS + 0x21C00)
 
 enum {
 	T0_BOT,
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 07/11] ARM: davinci: dm355: switch to using the clocksource driver

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We now have a proper clocksource driver for davinci. Switch the dm355
platform to using it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/dm355.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index 57c5a660758e..d871172f74c2 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -29,7 +29,8 @@
 #include <mach/cputype.h>
 #include <mach/mux.h>
 #include <mach/serial.h>
-#include <mach/time.h>
+
+#include <clocksource/timer-davinci.h>
 
 #include "asp.h"
 #include "davinci.h"
@@ -619,15 +620,15 @@ static struct davinci_id dm355_ids[] = {
 };
 
 /*
- * T0_BOT: Timer 0, bottom:  clockevent source for hrtimers
- * T0_TOP: Timer 0, top   :  clocksource for generic timekeeping
- * T1_BOT: Timer 1, bottom:  (used by DSP in TI DSPLink code)
- * T1_TOP: Timer 1, top   :  <unused>
+ * Bottom half of timer0 is used for clockevent, top half is used for
+ * clocksource.
  */
-static struct davinci_timer_info dm355_timer_info = {
-	.timers		= davinci_timer_instance,
-	.clockevent_id	= T0_BOT,
-	.clocksource_id	= T0_TOP,
+static const struct davinci_timer_cfg dm355_timer_cfg = {
+	.reg = DEFINE_RES_IO(DAVINCI_TIMER0_BASE, SZ_4K),
+	.irq = {
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_TINT0_TINT12)),
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_TINT0_TINT34)),
+	},
 };
 
 static struct plat_serial8250_port dm355_serial0_platform_data[] = {
@@ -705,7 +706,6 @@ static const struct davinci_soc_info davinci_soc_info_dm355 = {
 	.pinmux_base		= DAVINCI_SYSTEM_MODULE_BASE,
 	.pinmux_pins		= dm355_pins,
 	.pinmux_pins_num	= ARRAY_SIZE(dm355_pins),
-	.timer_info		= &dm355_timer_info,
 	.sram_dma		= 0x00010000,
 	.sram_len		= SZ_32K,
 };
@@ -732,6 +732,7 @@ void __init dm355_init_time(void)
 {
 	void __iomem *pll1, *psc;
 	struct clk *clk;
+	int rv;
 
 	clk_register_fixed_rate(NULL, "ref_clk", NULL, 0, DM355_REF_FREQ);
 
@@ -747,7 +748,8 @@ void __init dm355_init_time(void)
 		return;
 	}
 
-	davinci_timer_init(clk);
+	rv = davinci_timer_register(clk, &dm355_timer_cfg);
+	WARN(rv, "Unable to register the timer: %d\n", rv);
 }
 
 static struct resource dm355_pll2_resources[] = {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 08/11] ARM: davinci: dm365: switch to using the clocksource driver

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We now have a proper clocksource driver for davinci. Switch the dm365
platform to using it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/dm365.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 1d82bb630d11..5e93e0b6410b 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -34,7 +34,8 @@
 #include <mach/cputype.h>
 #include <mach/mux.h>
 #include <mach/serial.h>
-#include <mach/time.h>
+
+#include <clocksource/timer-davinci.h>
 
 #include "asp.h"
 #include "davinci.h"
@@ -659,10 +660,16 @@ static struct davinci_id dm365_ids[] = {
 	},
 };
 
-static struct davinci_timer_info dm365_timer_info = {
-	.timers		= davinci_timer_instance,
-	.clockevent_id	= T0_BOT,
-	.clocksource_id	= T0_TOP,
+/*
+ * Bottom half of timer0 is used for clockevent, top half is used for
+ * clocksource.
+ */
+static const struct davinci_timer_cfg dm365_timer_cfg = {
+	.reg = DEFINE_RES_IO(DAVINCI_TIMER0_BASE, SZ_4K),
+	.irq = {
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_TINT0_TINT12)),
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_TINT0_TINT34)),
+	},
 };
 
 #define DM365_UART1_BASE	(IO_PHYS + 0x106000)
@@ -722,7 +729,6 @@ static const struct davinci_soc_info davinci_soc_info_dm365 = {
 	.pinmux_base		= DAVINCI_SYSTEM_MODULE_BASE,
 	.pinmux_pins		= dm365_pins,
 	.pinmux_pins_num	= ARRAY_SIZE(dm365_pins),
-	.timer_info		= &dm365_timer_info,
 	.emac_pdata		= &dm365_emac_pdata,
 	.sram_dma		= 0x00010000,
 	.sram_len		= SZ_32K,
@@ -770,6 +776,7 @@ void __init dm365_init_time(void)
 {
 	void __iomem *pll1, *pll2, *psc;
 	struct clk *clk;
+	int rv;
 
 	clk_register_fixed_rate(NULL, "ref_clk", NULL, 0, DM365_REF_FREQ);
 
@@ -788,7 +795,8 @@ void __init dm365_init_time(void)
 		return;
 	}
 
-	davinci_timer_init(clk);
+	rv = davinci_timer_register(clk, &dm365_timer_cfg);
+	WARN(rv, "Unable to register the timer: %d\n", rv);
 }
 
 void __init dm365_register_clocks(void)
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 09/11] ARM: davinci: dm644x: switch to using the clocksource driver

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We now have a proper clocksource driver for davinci. Switch the dm644x
platform to using it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/dm644x.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 2b0e921aa755..9f7c348d8efe 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -26,7 +26,8 @@
 #include <mach/cputype.h>
 #include <mach/mux.h>
 #include <mach/serial.h>
-#include <mach/time.h>
+
+#include <clocksource/timer-davinci.h>
 
 #include "asp.h"
 #include "davinci.h"
@@ -560,15 +561,15 @@ static struct davinci_id dm644x_ids[] = {
 };
 
 /*
- * T0_BOT: Timer 0, bottom:  clockevent source for hrtimers
- * T0_TOP: Timer 0, top   :  clocksource for generic timekeeping
- * T1_BOT: Timer 1, bottom:  (used by DSP in TI DSPLink code)
- * T1_TOP: Timer 1, top   :  <unused>
+ * Bottom half of timer0 is used for clockevent, top half is used for
+ * clocksource.
  */
-static struct davinci_timer_info dm644x_timer_info = {
-	.timers		= davinci_timer_instance,
-	.clockevent_id	= T0_BOT,
-	.clocksource_id	= T0_TOP,
+static const struct davinci_timer_cfg dm644x_timer_cfg = {
+	.reg = DEFINE_RES_IO(DAVINCI_TIMER0_BASE, SZ_4K),
+	.irq = {
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_TINT0_TINT12)),
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_TINT0_TINT34)),
+	},
 };
 
 static struct plat_serial8250_port dm644x_serial0_platform_data[] = {
@@ -646,7 +647,6 @@ static const struct davinci_soc_info davinci_soc_info_dm644x = {
 	.pinmux_base		= DAVINCI_SYSTEM_MODULE_BASE,
 	.pinmux_pins		= dm644x_pins,
 	.pinmux_pins_num	= ARRAY_SIZE(dm644x_pins),
-	.timer_info		= &dm644x_timer_info,
 	.emac_pdata		= &dm644x_emac_pdata,
 	.sram_dma		= 0x00008000,
 	.sram_len		= SZ_16K,
@@ -668,6 +668,7 @@ void __init dm644x_init_time(void)
 {
 	void __iomem *pll1, *psc;
 	struct clk *clk;
+	int rv;
 
 	clk_register_fixed_rate(NULL, "ref_clk", NULL, 0, DM644X_REF_FREQ);
 
@@ -683,7 +684,8 @@ void __init dm644x_init_time(void)
 		return;
 	}
 
-	davinci_timer_init(clk);
+	rv = davinci_timer_register(clk, &dm644x_timer_cfg);
+	WARN(rv, "Unable to register the timer: %d\n", rv);
 }
 
 static struct resource dm644x_pll2_resources[] = {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 10/11] ARM: davinci: dm646x: switch to using the clocksource driver

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We now have a proper clocksource driver for davinci. Switch the dm646x
platform to using it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/dm646x.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 7e5af984ed9f..281e778ba19e 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -27,7 +27,8 @@
 #include <mach/cputype.h>
 #include <mach/mux.h>
 #include <mach/serial.h>
-#include <mach/time.h>
+
+#include <clocksource/timer-davinci.h>
 
 #include "asp.h"
 #include "davinci.h"
@@ -500,15 +501,15 @@ static struct davinci_id dm646x_ids[] = {
 };
 
 /*
- * T0_BOT: Timer 0, bottom:  clockevent source for hrtimers
- * T0_TOP: Timer 0, top   :  clocksource for generic timekeeping
- * T1_BOT: Timer 1, bottom:  (used by DSP in TI DSPLink code)
- * T1_TOP: Timer 1, top   :  <unused>
+ * Bottom half of timer0 is used for clockevent, top half is used for
+ * clocksource.
  */
-static struct davinci_timer_info dm646x_timer_info = {
-	.timers		= davinci_timer_instance,
-	.clockevent_id	= T0_BOT,
-	.clocksource_id	= T0_TOP,
+static const struct davinci_timer_cfg dm646x_timer_cfg = {
+	.reg = DEFINE_RES_IO(DAVINCI_TIMER0_BASE, SZ_4K),
+	.irq = {
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_TINT0_TINT12)),
+		DEFINE_RES_IRQ(DAVINCI_INTC_IRQ(IRQ_TINT0_TINT34)),
+	},
 };
 
 static struct plat_serial8250_port dm646x_serial0_platform_data[] = {
@@ -586,7 +587,6 @@ static const struct davinci_soc_info davinci_soc_info_dm646x = {
 	.pinmux_base		= DAVINCI_SYSTEM_MODULE_BASE,
 	.pinmux_pins		= dm646x_pins,
 	.pinmux_pins_num	= ARRAY_SIZE(dm646x_pins),
-	.timer_info		= &dm646x_timer_info,
 	.emac_pdata		= &dm646x_emac_pdata,
 	.sram_dma		= 0x10010000,
 	.sram_len		= SZ_32K,
@@ -651,6 +651,7 @@ void __init dm646x_init_time(unsigned long ref_clk_rate,
 {
 	void __iomem *pll1, *psc;
 	struct clk *clk;
+	int rv;
 
 	clk_register_fixed_rate(NULL, "ref_clk", NULL, 0, ref_clk_rate);
 	clk_register_fixed_rate(NULL, "aux_clkin", NULL, 0, aux_clkin_rate);
@@ -667,7 +668,8 @@ void __init dm646x_init_time(unsigned long ref_clk_rate,
 		return;
 	}
 
-	davinci_timer_init(clk);
+	rv = davinci_timer_register(clk, &dm646x_timer_cfg);
+	WARN(rv, "Unable to register the timer: %d\n", rv);
 }
 
 static struct resource dm646x_pll2_resources[] = {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 11/11] ARM: davinci: remove legacy timer support

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

All platforms have now been switched to the new clocksource driver.
Remove the old code and various no longer needed bits and pieces.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/Makefile              |   3 +-
 arch/arm/mach-davinci/devices-da8xx.c       |   1 -
 arch/arm/mach-davinci/devices.c             |  19 -
 arch/arm/mach-davinci/include/mach/common.h |  17 -
 arch/arm/mach-davinci/include/mach/time.h   |  33 --
 arch/arm/mach-davinci/time.c                | 400 --------------------
 6 files changed, 1 insertion(+), 472 deletions(-)
 delete mode 100644 arch/arm/mach-davinci/include/mach/time.h
 delete mode 100644 arch/arm/mach-davinci/time.c

diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile
index f76a8482784f..d9af8c0c0b87 100644
--- a/arch/arm/mach-davinci/Makefile
+++ b/arch/arm/mach-davinci/Makefile
@@ -5,8 +5,7 @@
 #
 
 # Common objects
-obj-y 					:= time.o serial.o usb.o \
-					   common.o sram.o
+obj-y 					:= serial.o usb.o common.o sram.o
 
 obj-$(CONFIG_DAVINCI_MUX)		+= mux.o
 
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index b8dc674e06bc..5c6508821de4 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -24,7 +24,6 @@
 #include <mach/common.h>
 #include <mach/cputype.h>
 #include <mach/da8xx.h>
-#include <mach/time.h>
 
 #include "asp.h"
 #include "cpuidle.h"
diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c
index 40bd8029e457..3844433c9deb 100644
--- a/arch/arm/mach-davinci/devices.c
+++ b/arch/arm/mach-davinci/devices.c
@@ -21,7 +21,6 @@
 #include <mach/hardware.h>
 #include <mach/cputype.h>
 #include <mach/mux.h>
-#include <mach/time.h>
 
 #include "davinci.h"
 #include "irqs.h"
@@ -307,21 +306,3 @@ int davinci_gpio_register(struct resource *res, int size, void *pdata)
 	davinci_gpio_device.dev.platform_data = pdata;
 	return platform_device_register(&davinci_gpio_device);
 }
-
-/*-------------------------------------------------------------------------*/
-
-/*-------------------------------------------------------------------------*/
-
-struct davinci_timer_instance davinci_timer_instance[2] = {
-	{
-		.base		= DAVINCI_TIMER0_BASE,
-		.bottom_irq	= DAVINCI_INTC_IRQ(IRQ_TINT0_TINT12),
-		.top_irq	= DAVINCI_INTC_IRQ(IRQ_TINT0_TINT34),
-	},
-	{
-		.base		= DAVINCI_TIMER1_BASE,
-		.bottom_irq	= DAVINCI_INTC_IRQ(IRQ_TINT1_TINT12),
-		.top_irq	= DAVINCI_INTC_IRQ(IRQ_TINT1_TINT34),
-	},
-};
-
diff --git a/arch/arm/mach-davinci/include/mach/common.h b/arch/arm/mach-davinci/include/mach/common.h
index 9526e5da0d33..139b83de011d 100644
--- a/arch/arm/mach-davinci/include/mach/common.h
+++ b/arch/arm/mach-davinci/include/mach/common.h
@@ -22,22 +22,6 @@
 #define DAVINCI_INTC_START		NR_IRQS
 #define DAVINCI_INTC_IRQ(_irqnum)	(DAVINCI_INTC_START + (_irqnum))
 
-void davinci_timer_init(struct clk *clk);
-
-struct davinci_timer_instance {
-	u32		base;
-	u32		bottom_irq;
-	u32		top_irq;
-	unsigned long	cmp_off;
-	unsigned int	cmp_irq;
-};
-
-struct davinci_timer_info {
-	struct davinci_timer_instance	*timers;
-	unsigned int			clockevent_id;
-	unsigned int			clocksource_id;
-};
-
 struct davinci_gpio_controller;
 
 /*
@@ -58,7 +42,6 @@ struct davinci_soc_info {
 	u32				pinmux_base;
 	const struct mux_config		*pinmux_pins;
 	unsigned long			pinmux_pins_num;
-	struct davinci_timer_info	*timer_info;
 	int				gpio_type;
 	u32				gpio_base;
 	unsigned			gpio_num;
diff --git a/arch/arm/mach-davinci/include/mach/time.h b/arch/arm/mach-davinci/include/mach/time.h
deleted file mode 100644
index ba913736990f..000000000000
--- a/arch/arm/mach-davinci/include/mach/time.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Local header file for DaVinci time code.
- *
- * Author: Kevin Hilman, MontaVista Software, Inc. <source@mvista.com>
- *
- * 2007 (c) MontaVista Software, Inc. This file is licensed under
- * the terms of the GNU General Public License version 2. This program
- * is licensed "as is" without any warranty of any kind, whether express
- * or implied.
- */
-#ifndef __ARCH_ARM_MACH_DAVINCI_TIME_H
-#define __ARCH_ARM_MACH_DAVINCI_TIME_H
-
-#define DAVINCI_TIMER1_BASE		(IO_PHYS + 0x21800)
-
-enum {
-	T0_BOT,
-	T0_TOP,
-	T1_BOT,
-	T1_TOP,
-	NUM_TIMERS
-};
-
-#define IS_TIMER1(id)		(id & 0x2)
-#define IS_TIMER0(id)		(!IS_TIMER1(id))
-#define IS_TIMER_TOP(id)	((id & 0x1))
-#define IS_TIMER_BOT(id)	(!IS_TIMER_TOP(id))
-
-#define ID_TO_TIMER(id)		(IS_TIMER1(id) != 0)
-
-extern struct davinci_timer_instance davinci_timer_instance[];
-
-#endif /* __ARCH_ARM_MACH_DAVINCI_TIME_H */
diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
deleted file mode 100644
index 740410a3bb6a..000000000000
--- a/arch/arm/mach-davinci/time.c
+++ /dev/null
@@ -1,400 +0,0 @@
-/*
- * DaVinci timer subsystem
- *
- * Author: Kevin Hilman, MontaVista Software, Inc. <source@mvista.com>
- *
- * 2007 (c) MontaVista Software, Inc. This file is licensed under
- * the terms of the GNU General Public License version 2. This program
- * is licensed "as is" without any warranty of any kind, whether express
- * or implied.
- */
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/types.h>
-#include <linux/interrupt.h>
-#include <linux/clocksource.h>
-#include <linux/clockchips.h>
-#include <linux/io.h>
-#include <linux/clk.h>
-#include <linux/err.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/sched_clock.h>
-
-#include <asm/mach/irq.h>
-#include <asm/mach/time.h>
-
-#include <mach/cputype.h>
-#include <mach/hardware.h>
-#include <mach/time.h>
-
-static struct clock_event_device clockevent_davinci;
-static unsigned int davinci_clock_tick_rate;
-
-/*
- * This driver configures the 2 64-bit count-up timers as 4 independent
- * 32-bit count-up timers used as follows:
- */
-
-enum {
-	TID_CLOCKEVENT,
-	TID_CLOCKSOURCE,
-};
-
-/* Timer register offsets */
-#define PID12			0x0
-#define TIM12			0x10
-#define TIM34			0x14
-#define PRD12			0x18
-#define PRD34			0x1c
-#define TCR			0x20
-#define TGCR			0x24
-#define WDTCR			0x28
-
-/* Offsets of the 8 compare registers */
-#define	CMP12_0			0x60
-#define	CMP12_1			0x64
-#define	CMP12_2			0x68
-#define	CMP12_3			0x6c
-#define	CMP12_4			0x70
-#define	CMP12_5			0x74
-#define	CMP12_6			0x78
-#define	CMP12_7			0x7c
-
-/* Timer register bitfields */
-#define TCR_ENAMODE_DISABLE          0x0
-#define TCR_ENAMODE_ONESHOT          0x1
-#define TCR_ENAMODE_PERIODIC         0x2
-#define TCR_ENAMODE_MASK             0x3
-
-#define TGCR_TIMMODE_SHIFT           2
-#define TGCR_TIMMODE_64BIT_GP        0x0
-#define TGCR_TIMMODE_32BIT_UNCHAINED 0x1
-#define TGCR_TIMMODE_64BIT_WDOG      0x2
-#define TGCR_TIMMODE_32BIT_CHAINED   0x3
-
-#define TGCR_TIM12RS_SHIFT           0
-#define TGCR_TIM34RS_SHIFT           1
-#define TGCR_RESET                   0x0
-#define TGCR_UNRESET                 0x1
-#define TGCR_RESET_MASK              0x3
-
-struct timer_s {
-	char *name;
-	unsigned int id;
-	unsigned long period;
-	unsigned long opts;
-	unsigned long flags;
-	void __iomem *base;
-	unsigned long tim_off;
-	unsigned long prd_off;
-	unsigned long enamode_shift;
-	struct irqaction irqaction;
-};
-static struct timer_s timers[];
-
-/* values for 'opts' field of struct timer_s */
-#define TIMER_OPTS_DISABLED		0x01
-#define TIMER_OPTS_ONESHOT		0x02
-#define TIMER_OPTS_PERIODIC		0x04
-#define TIMER_OPTS_STATE_MASK		0x07
-
-#define TIMER_OPTS_USE_COMPARE		0x80000000
-#define USING_COMPARE(t)		((t)->opts & TIMER_OPTS_USE_COMPARE)
-
-static char *id_to_name[] = {
-	[T0_BOT]	= "timer0_0",
-	[T0_TOP]	= "timer0_1",
-	[T1_BOT]	= "timer1_0",
-	[T1_TOP]	= "timer1_1",
-};
-
-static int timer32_config(struct timer_s *t)
-{
-	u32 tcr;
-	struct davinci_soc_info *soc_info = &davinci_soc_info;
-
-	if (USING_COMPARE(t)) {
-		struct davinci_timer_instance *dtip =
-				soc_info->timer_info->timers;
-		int event_timer = ID_TO_TIMER(timers[TID_CLOCKEVENT].id);
-
-		/*
-		 * Next interrupt should be the current time reg value plus
-		 * the new period (using 32-bit unsigned addition/wrapping
-		 * to 0 on overflow).  This assumes that the clocksource
-		 * is setup to count to 2^32-1 before wrapping around to 0.
-		 */
-		__raw_writel(__raw_readl(t->base + t->tim_off) + t->period,
-			t->base + dtip[event_timer].cmp_off);
-	} else {
-		tcr = __raw_readl(t->base + TCR);
-
-		/* disable timer */
-		tcr &= ~(TCR_ENAMODE_MASK << t->enamode_shift);
-		__raw_writel(tcr, t->base + TCR);
-
-		/* reset counter to zero, set new period */
-		__raw_writel(0, t->base + t->tim_off);
-		__raw_writel(t->period, t->base + t->prd_off);
-
-		/* Set enable mode */
-		if (t->opts & TIMER_OPTS_ONESHOT)
-			tcr |= TCR_ENAMODE_ONESHOT << t->enamode_shift;
-		else if (t->opts & TIMER_OPTS_PERIODIC)
-			tcr |= TCR_ENAMODE_PERIODIC << t->enamode_shift;
-
-		__raw_writel(tcr, t->base + TCR);
-	}
-	return 0;
-}
-
-static inline u32 timer32_read(struct timer_s *t)
-{
-	return __raw_readl(t->base + t->tim_off);
-}
-
-static irqreturn_t timer_interrupt(int irq, void *dev_id)
-{
-	struct clock_event_device *evt = &clockevent_davinci;
-
-	evt->event_handler(evt);
-	return IRQ_HANDLED;
-}
-
-/* called when 32-bit counter wraps */
-static irqreturn_t freerun_interrupt(int irq, void *dev_id)
-{
-	return IRQ_HANDLED;
-}
-
-static struct timer_s timers[] = {
-	[TID_CLOCKEVENT] = {
-		.name      = "clockevent",
-		.opts      = TIMER_OPTS_DISABLED,
-		.irqaction = {
-			.flags   = IRQF_TIMER,
-			.handler = timer_interrupt,
-		}
-	},
-	[TID_CLOCKSOURCE] = {
-		.name       = "free-run counter",
-		.period     = ~0,
-		.opts       = TIMER_OPTS_PERIODIC,
-		.irqaction = {
-			.flags   = IRQF_TIMER,
-			.handler = freerun_interrupt,
-		}
-	},
-};
-
-static void __init timer_init(void)
-{
-	struct davinci_soc_info *soc_info = &davinci_soc_info;
-	struct davinci_timer_instance *dtip = soc_info->timer_info->timers;
-	void __iomem *base[2];
-	int i;
-
-	/* Global init of each 64-bit timer as a whole */
-	for(i=0; i<2; i++) {
-		u32 tgcr;
-
-		base[i] = ioremap(dtip[i].base, SZ_4K);
-		if (WARN_ON(!base[i]))
-			continue;
-
-		/* Disabled, Internal clock source */
-		__raw_writel(0, base[i] + TCR);
-
-		/* reset both timers, no pre-scaler for timer34 */
-		tgcr = 0;
-		__raw_writel(tgcr, base[i] + TGCR);
-
-		/* Set both timers to unchained 32-bit */
-		tgcr = TGCR_TIMMODE_32BIT_UNCHAINED << TGCR_TIMMODE_SHIFT;
-		__raw_writel(tgcr, base[i] + TGCR);
-
-		/* Unreset timers */
-		tgcr |= (TGCR_UNRESET << TGCR_TIM12RS_SHIFT) |
-			(TGCR_UNRESET << TGCR_TIM34RS_SHIFT);
-		__raw_writel(tgcr, base[i] + TGCR);
-
-		/* Init both counters to zero */
-		__raw_writel(0, base[i] + TIM12);
-		__raw_writel(0, base[i] + TIM34);
-	}
-
-	/* Init of each timer as a 32-bit timer */
-	for (i=0; i< ARRAY_SIZE(timers); i++) {
-		struct timer_s *t = &timers[i];
-		int timer = ID_TO_TIMER(t->id);
-		u32 irq;
-
-		t->base = base[timer];
-		if (!t->base)
-			continue;
-
-		if (IS_TIMER_BOT(t->id)) {
-			t->enamode_shift = 6;
-			t->tim_off = TIM12;
-			t->prd_off = PRD12;
-			irq = dtip[timer].bottom_irq;
-		} else {
-			t->enamode_shift = 22;
-			t->tim_off = TIM34;
-			t->prd_off = PRD34;
-			irq = dtip[timer].top_irq;
-		}
-
-		/* Register interrupt */
-		t->irqaction.name = t->name;
-		t->irqaction.dev_id = (void *)t;
-
-		if (t->irqaction.handler != NULL) {
-			irq = USING_COMPARE(t) ? dtip[i].cmp_irq : irq;
-			setup_irq(irq, &t->irqaction);
-		}
-	}
-}
-
-/*
- * clocksource
- */
-static u64 read_cycles(struct clocksource *cs)
-{
-	struct timer_s *t = &timers[TID_CLOCKSOURCE];
-
-	return (cycles_t)timer32_read(t);
-}
-
-static struct clocksource clocksource_davinci = {
-	.rating		= 300,
-	.read		= read_cycles,
-	.mask		= CLOCKSOURCE_MASK(32),
-	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
-};
-
-/*
- * Overwrite weak default sched_clock with something more precise
- */
-static u64 notrace davinci_read_sched_clock(void)
-{
-	return timer32_read(&timers[TID_CLOCKSOURCE]);
-}
-
-/*
- * clockevent
- */
-static int davinci_set_next_event(unsigned long cycles,
-				  struct clock_event_device *evt)
-{
-	struct timer_s *t = &timers[TID_CLOCKEVENT];
-
-	t->period = cycles;
-	timer32_config(t);
-	return 0;
-}
-
-static int davinci_shutdown(struct clock_event_device *evt)
-{
-	struct timer_s *t = &timers[TID_CLOCKEVENT];
-
-	t->opts &= ~TIMER_OPTS_STATE_MASK;
-	t->opts |= TIMER_OPTS_DISABLED;
-	return 0;
-}
-
-static int davinci_set_oneshot(struct clock_event_device *evt)
-{
-	struct timer_s *t = &timers[TID_CLOCKEVENT];
-
-	t->opts &= ~TIMER_OPTS_STATE_MASK;
-	t->opts |= TIMER_OPTS_ONESHOT;
-	return 0;
-}
-
-static int davinci_set_periodic(struct clock_event_device *evt)
-{
-	struct timer_s *t = &timers[TID_CLOCKEVENT];
-
-	t->period = davinci_clock_tick_rate / (HZ);
-	t->opts &= ~TIMER_OPTS_STATE_MASK;
-	t->opts |= TIMER_OPTS_PERIODIC;
-	timer32_config(t);
-	return 0;
-}
-
-static struct clock_event_device clockevent_davinci = {
-	.features		= CLOCK_EVT_FEAT_PERIODIC |
-				  CLOCK_EVT_FEAT_ONESHOT,
-	.set_next_event		= davinci_set_next_event,
-	.set_state_shutdown	= davinci_shutdown,
-	.set_state_periodic	= davinci_set_periodic,
-	.set_state_oneshot	= davinci_set_oneshot,
-};
-
-void __init davinci_timer_init(struct clk *timer_clk)
-{
-	struct davinci_soc_info *soc_info = &davinci_soc_info;
-	unsigned int clockevent_id;
-	unsigned int clocksource_id;
-	int i;
-
-	clockevent_id = soc_info->timer_info->clockevent_id;
-	clocksource_id = soc_info->timer_info->clocksource_id;
-
-	timers[TID_CLOCKEVENT].id = clockevent_id;
-	timers[TID_CLOCKSOURCE].id = clocksource_id;
-
-	/*
-	 * If using same timer for both clock events & clocksource,
-	 * a compare register must be used to generate an event interrupt.
-	 * This is equivalent to a oneshot timer only (not periodic).
-	 */
-	if (clockevent_id == clocksource_id) {
-		struct davinci_timer_instance *dtip =
-				soc_info->timer_info->timers;
-		int event_timer = ID_TO_TIMER(clockevent_id);
-
-		/* Only bottom timers can use compare regs */
-		if (IS_TIMER_TOP(clockevent_id))
-			pr_warn("%s: Invalid use of system timers.  Results unpredictable.\n",
-				__func__);
-		else if ((dtip[event_timer].cmp_off == 0)
-				|| (dtip[event_timer].cmp_irq == 0))
-			pr_warn("%s: Invalid timer instance setup.  Results unpredictable.\n",
-				__func__);
-		else {
-			timers[TID_CLOCKEVENT].opts |= TIMER_OPTS_USE_COMPARE;
-			clockevent_davinci.features = CLOCK_EVT_FEAT_ONESHOT;
-		}
-	}
-
-	BUG_ON(IS_ERR(timer_clk));
-	clk_prepare_enable(timer_clk);
-
-	/* init timer hw */
-	timer_init();
-
-	davinci_clock_tick_rate = clk_get_rate(timer_clk);
-
-	/* setup clocksource */
-	clocksource_davinci.name = id_to_name[clocksource_id];
-	if (clocksource_register_hz(&clocksource_davinci,
-				    davinci_clock_tick_rate))
-		pr_err("%s: can't register clocksource!\n",
-		       clocksource_davinci.name);
-
-	sched_clock_register(davinci_read_sched_clock, 32,
-			  davinci_clock_tick_rate);
-
-	/* setup clockevent */
-	clockevent_davinci.name = id_to_name[timers[TID_CLOCKEVENT].id];
-
-	clockevent_davinci.cpumask = cpumask_of(0);
-	clockevents_config_and_register(&clockevent_davinci,
-					davinci_clock_tick_rate, 1, 0xfffffffe);
-
-	for (i=0; i< ARRAY_SIZE(timers); i++)
-		timer32_config(&timers[i]);
-}
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 00/11] ARM: davinci: modernize the timer support

[Sekhar Nori]

Hi Daniel, Thomas,

On 18/03/19 5:40 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> This series removes the legacy timer code from mach-davinci in favor of
> a new clocksource driver it introduces.
> 
> Patch 1 adds a new clocksource driver for davinci.
> 
> Patch 2 enables the new driver for device-tree based systems.
> 
> Patch 3 adds a WARN_ON() to the machine code of all davinci boards
> which is triggered if clk_get() for the timer clock fails. This is needed
> as the new driver expects the clock to be functional and doesn't check it.
> 
> Patches 4-5 and 7-10 switch the board files to using the new
> clocksource driver while patch 6 moves some necessary defines to
> a different place since we'll be removing the file that contains them.
> 
> Patch 11 removes legacy timer code.

The series looks good to me. With your ack on 1/11, I would like to
merge the series through ARM SoC tree.

Thanks,
Sekhar

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 00/11] ARM: davinci: modernize the timer support

[Daniel Lezcano]

Hi Sekhar,


On 27/03/2019 16:36, Sekhar Nori wrote:
> Hi Daniel, Thomas,
> 
> On 18/03/19 5:40 PM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> This series removes the legacy timer code from mach-davinci in favor of
>> a new clocksource driver it introduces.
>>
>> Patch 1 adds a new clocksource driver for davinci.
>>
>> Patch 2 enables the new driver for device-tree based systems.
>>
>> Patch 3 adds a WARN_ON() to the machine code of all davinci boards
>> which is triggered if clk_get() for the timer clock fails. This is needed
>> as the new driver expects the clock to be functional and doesn't check it.
>>
>> Patches 4-5 and 7-10 switch the board files to using the new
>> clocksource driver while patch 6 moves some necessary defines to
>> a different place since we'll be removing the file that contains them.
>>
>> Patch 11 removes legacy timer code.
> 
> The series looks good to me. With your ack on 1/11, I would like to
> merge the series through ARM SoC tree.

ok, will do the review asap.

 -- Daniel

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver

[Daniel Lezcano]

On 18/03/2019 13:10, 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.
> 
> We don't bother freeing resources on errors in davinci_timer_register()
> as the system won't boot without a timer anyway.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Reviewed-by: David Lechner <david@lechnology.com>
> ---
>  drivers/clocksource/Kconfig         |   5 +
>  drivers/clocksource/Makefile        |   1 +
>  drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
>  include/clocksource/timer-davinci.h |  44 +++
>  4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"
> +	help
> +	  Enables the support for the TI DaVinci timer driver.
> +

Please make it a silence option only visible with COMPILE_TEST or
EXPERT, examples here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/Kconfig#n45

or second format:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/Kconfig#n459

>  config DIGICOLOR_TIMER
>  	bool "Digicolor timer driver" if COMPILE_TEST
>  	select CLKSRC_MMIO
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
> --- /dev/null
> +++ b/drivers/clocksource/timer-davinci.c
> @@ -0,0 +1,438 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// TI DaVinci clocksource driver
> +//
> +// Copyright (C) 2019 Texas Instruments
> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/clocksource.h>
> +#include <linux/err.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)
> +
> +/* Shift depends on timer. */
> +#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_PERIODIC		BIT(1)
> +
> +#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_CLKSRC_BITS		32
> +
> +#define DAVINCI_TIMER_TGCR_DEFAULT \
> +		(DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> +
> +enum {
> +	DAVINCI_TIMER_MODE_DISABLED = 0,
> +	DAVINCI_TIMER_MODE_ONESHOT,
> +	DAVINCI_TIMER_MODE_PERIODIC,
> +};
> +
> +struct davinci_timer_data;
> +
> +typedef void (*davinci_timer_set_period_func)(struct davinci_timer_data *,
> +					      unsigned int period);
> +
> +/**
> + * struct davinci_timer_regs - timer-specific register offsets
> + *
> + * @tim_off: timer counter register
> + * @prd_off: timer period register
> + * @enamode_shift: left bit-shift of the enable register associated
> + *                 with this timer in the TCR register
> + */
> +struct davinci_timer_regs {
> +	unsigned int tim_off;
> +	unsigned int prd_off;
> +	unsigned int enamode_shift;
> +};
> +
> +struct davinci_timer_data {
> +	void __iomem *base;
> +	const struct davinci_timer_regs *regs;
> +	unsigned int mode;
> +	davinci_timer_set_period_func set_period;
> +	unsigned int cmp_off;
> +};
> +
> +struct davinci_timer_clockevent {
> +	struct clock_event_device dev;
> +	unsigned int tick_rate;
> +	struct davinci_timer_data timer;
> +};

The timer-of API provides the functions and the common structures for
the usual operations. Please use them instead of redefining your own
structures.



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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver

[Bartosz Golaszewski]

wt., 2 kwi 2019 o 11:21 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>
> On 18/03/2019 13:10, 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.
> >
> > We don't bother freeing resources on errors in davinci_timer_register()
> > as the system won't boot without a timer anyway.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Reviewed-by: David Lechner <david@lechnology.com>
> > ---
> >  drivers/clocksource/Kconfig         |   5 +
> >  drivers/clocksource/Makefile        |   1 +
> >  drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
> >  include/clocksource/timer-davinci.h |  44 +++
> >  4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"
> > +     help
> > +       Enables the support for the TI DaVinci timer driver.
> > +
>
> Please make it a silence option only visible with COMPILE_TEST or
> EXPERT, examples here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/Kconfig#n45
>
> or second format:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/Kconfig#n459
>
> >  config DIGICOLOR_TIMER
> >       bool "Digicolor timer driver" if COMPILE_TEST
> >       select CLKSRC_MMIO
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-davinci.c
> > @@ -0,0 +1,438 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// TI DaVinci clocksource driver
> > +//
> > +// Copyright (C) 2019 Texas Instruments
> > +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> > +#include <linux/clk.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/err.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)
> > +
> > +/* Shift depends on timer. */
> > +#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_PERIODIC               BIT(1)
> > +
> > +#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_CLKSRC_BITS            32
> > +
> > +#define DAVINCI_TIMER_TGCR_DEFAULT \
> > +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> > +
> > +enum {
> > +     DAVINCI_TIMER_MODE_DISABLED = 0,
> > +     DAVINCI_TIMER_MODE_ONESHOT,
> > +     DAVINCI_TIMER_MODE_PERIODIC,
> > +};
> > +
> > +struct davinci_timer_data;
> > +
> > +typedef void (*davinci_timer_set_period_func)(struct davinci_timer_data *,
> > +                                           unsigned int period);
> > +
> > +/**
> > + * struct davinci_timer_regs - timer-specific register offsets
> > + *
> > + * @tim_off: timer counter register
> > + * @prd_off: timer period register
> > + * @enamode_shift: left bit-shift of the enable register associated
> > + *                 with this timer in the TCR register
> > + */
> > +struct davinci_timer_regs {
> > +     unsigned int tim_off;
> > +     unsigned int prd_off;
> > +     unsigned int enamode_shift;
> > +};
> > +
> > +struct davinci_timer_data {
> > +     void __iomem *base;
> > +     const struct davinci_timer_regs *regs;
> > +     unsigned int mode;
> > +     davinci_timer_set_period_func set_period;
> > +     unsigned int cmp_off;
> > +};
> > +
> > +struct davinci_timer_clockevent {
> > +     struct clock_event_device dev;
> > +     unsigned int tick_rate;
> > +     struct davinci_timer_data timer;
> > +};
>
> The timer-of API provides the functions and the common structures for
> the usual operations. Please use them instead of redefining your own
> structures.
>

Hi Daniel,

do you have any suggestion about where to put the information about
register offsets I'm now storing in struct davinci_timer_regs? I
thought about having specialized wrappers around all relevant
functions that access the registers, but that seems like an overkill.
For instance we'd have:

davinci_timer_set_period_std()

and two specialized variants:

davinci_timer_set_period_std_tim12() and davinci_timer_set_period_std_tim34()

The former would take struct davinci_timer_regs as argument while two
latter routines would fit the callback format. We'd then need the same
for three more routines.

Alternatively struct timer_of could be packed together with struct
davinci_timer_regs in another structure and accessed in callbacks via
container_of.

Do you have a preference or a different idea?

Bartosz

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver

[Bartosz Golaszewski]

wt., 2 kwi 2019 o 11:21 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>
> On 18/03/2019 13:10, 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.
> >
> > We don't bother freeing resources on errors in davinci_timer_register()
> > as the system won't boot without a timer anyway.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Reviewed-by: David Lechner <david@lechnology.com>
> > ---
> >  drivers/clocksource/Kconfig         |   5 +
> >  drivers/clocksource/Makefile        |   1 +
> >  drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
> >  include/clocksource/timer-davinci.h |  44 +++
> >  4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"
> > +     help
> > +       Enables the support for the TI DaVinci timer driver.
> > +
>
> Please make it a silence option only visible with COMPILE_TEST or
> EXPERT, examples here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/Kconfig#n45
>
> or second format:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/Kconfig#n459
>
> >  config DIGICOLOR_TIMER
> >       bool "Digicolor timer driver" if COMPILE_TEST
> >       select CLKSRC_MMIO
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-davinci.c
> > @@ -0,0 +1,438 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// TI DaVinci clocksource driver
> > +//
> > +// Copyright (C) 2019 Texas Instruments
> > +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> > +#include <linux/clk.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/err.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)
> > +
> > +/* Shift depends on timer. */
> > +#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_PERIODIC               BIT(1)
> > +
> > +#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_CLKSRC_BITS            32
> > +
> > +#define DAVINCI_TIMER_TGCR_DEFAULT \
> > +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> > +
> > +enum {
> > +     DAVINCI_TIMER_MODE_DISABLED = 0,
> > +     DAVINCI_TIMER_MODE_ONESHOT,
> > +     DAVINCI_TIMER_MODE_PERIODIC,
> > +};
> > +
> > +struct davinci_timer_data;
> > +
> > +typedef void (*davinci_timer_set_period_func)(struct davinci_timer_data *,
> > +                                           unsigned int period);
> > +
> > +/**
> > + * struct davinci_timer_regs - timer-specific register offsets
> > + *
> > + * @tim_off: timer counter register
> > + * @prd_off: timer period register
> > + * @enamode_shift: left bit-shift of the enable register associated
> > + *                 with this timer in the TCR register
> > + */
> > +struct davinci_timer_regs {
> > +     unsigned int tim_off;
> > +     unsigned int prd_off;
> > +     unsigned int enamode_shift;
> > +};
> > +
> > +struct davinci_timer_data {
> > +     void __iomem *base;
> > +     const struct davinci_timer_regs *regs;
> > +     unsigned int mode;
> > +     davinci_timer_set_period_func set_period;
> > +     unsigned int cmp_off;
> > +};
> > +
> > +struct davinci_timer_clockevent {
> > +     struct clock_event_device dev;
> > +     unsigned int tick_rate;
> > +     struct davinci_timer_data timer;
> > +};
>
> The timer-of API provides the functions and the common structures for
> the usual operations. Please use them instead of redefining your own
> structures.
>

After giving it another thought, I don't think we can use timer-of in
this driver easily.

It's meant to work both for board files and device-tree. The solution
with the least code duplication is to take the device_node on DT
systems and put all relevant properties into the custom structure used
by board files users. Timer-of maps the memory, enables the clock and
requests interrupts directly from the device_node. I'd have to do the
same for board files manually anyway. I think it's better to have a
single point of entry for the initialization code.

Even if I were just to reuse the timer-of structures without reusing
the actual code - they are still missing certain fields. We're using
32-bit "halves" of timers that are actually 64 bits - the 32-bit
control register deals with both by using the same layout of bit
fields but shifted. This is what the struct davinci_timer_regs is for.
Reusing those structures would also be confusing because of the of_
prefix in the naming.

I'd prefer to leave it as it is. Let me know what you think.

Bart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver

[Daniel Lezcano]

On 08/04/2019 15:49, Bartosz Golaszewski wrote:
> wt., 2 kwi 2019 o 11:21 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>>
>> On 18/03/2019 13:10, 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.
>>>
>>> We don't bother freeing resources on errors in davinci_timer_register()
>>> as the system won't boot without a timer anyway.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> Reviewed-by: David Lechner <david@lechnology.com>
>>> ---
>>>  drivers/clocksource/Kconfig         |   5 +
>>>  drivers/clocksource/Makefile        |   1 +
>>>  drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
>>>  include/clocksource/timer-davinci.h |  44 +++
>>>  4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"
>>> +     help
>>> +       Enables the support for the TI DaVinci timer driver.
>>> +
>>
>> Please make it a silence option only visible with COMPILE_TEST or
>> EXPERT, examples here:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/Kconfig#n45
>>
>> or second format:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/Kconfig#n459
>>
>>>  config DIGICOLOR_TIMER
>>>       bool "Digicolor timer driver" if COMPILE_TEST
>>>       select CLKSRC_MMIO
>>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>>> index be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
>>> --- /dev/null
>>> +++ b/drivers/clocksource/timer-davinci.c
>>> @@ -0,0 +1,438 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +//
>>> +// TI DaVinci clocksource driver
>>> +//
>>> +// Copyright (C) 2019 Texas Instruments
>>> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
>>> +#include <linux/clk.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/clocksource.h>
>>> +#include <linux/err.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)
>>> +
>>> +/* Shift depends on timer. */
>>> +#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_PERIODIC               BIT(1)
>>> +
>>> +#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_CLKSRC_BITS            32
>>> +
>>> +#define DAVINCI_TIMER_TGCR_DEFAULT \
>>> +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
>>> +
>>> +enum {
>>> +     DAVINCI_TIMER_MODE_DISABLED = 0,
>>> +     DAVINCI_TIMER_MODE_ONESHOT,
>>> +     DAVINCI_TIMER_MODE_PERIODIC,
>>> +};
>>> +
>>> +struct davinci_timer_data;
>>> +
>>> +typedef void (*davinci_timer_set_period_func)(struct davinci_timer_data *,
>>> +                                           unsigned int period);
>>> +
>>> +/**
>>> + * struct davinci_timer_regs - timer-specific register offsets
>>> + *
>>> + * @tim_off: timer counter register
>>> + * @prd_off: timer period register
>>> + * @enamode_shift: left bit-shift of the enable register associated
>>> + *                 with this timer in the TCR register
>>> + */
>>> +struct davinci_timer_regs {
>>> +     unsigned int tim_off;
>>> +     unsigned int prd_off;
>>> +     unsigned int enamode_shift;
>>> +};
>>> +
>>> +struct davinci_timer_data {
>>> +     void __iomem *base;
>>> +     const struct davinci_timer_regs *regs;
>>> +     unsigned int mode;
>>> +     davinci_timer_set_period_func set_period;
>>> +     unsigned int cmp_off;
>>> +};
>>> +
>>> +struct davinci_timer_clockevent {
>>> +     struct clock_event_device dev;
>>> +     unsigned int tick_rate;
>>> +     struct davinci_timer_data timer;
>>> +};
>>
>> The timer-of API provides the functions and the common structures for
>> the usual operations. Please use them instead of redefining your own
>> structures.
>>
> 
> After giving it another thought, I don't think we can use timer-of in
> this driver easily.
> 
> It's meant to work both for board files and device-tree. The solution
> with the least code duplication is to take the device_node on DT
> systems and put all relevant properties into the custom structure used
> by board files users. Timer-of maps the memory, enables the clock and
> requests interrupts directly from the device_node. I'd have to do the
> same for board files manually anyway. I think it's better to have a
> single point of entry for the initialization code.
> 
> Even if I were just to reuse the timer-of structures without reusing
> the actual code - they are still missing certain fields. We're using
> 32-bit "halves" of timers that are actually 64 bits - the 32-bit
> control register deals with both by using the same layout of bit
> fields but shifted. This is what the struct davinci_timer_regs is for.
> Reusing those structures would also be confusing because of the of_
> prefix in the naming.
> 
> I'd prefer to leave it as it is. Let me know what you think.

Sorry for not being responsive, I've been OoO last week. Give me some
days to digest the emails stack I have in my mailbox and I'll tell you
after reviewing how works this timer.


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver

[Daniel Lezcano]

On 18/03/2019 13:10, 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.
> 
> We don't bother freeing resources on errors in davinci_timer_register()
> as the system won't boot without a timer anyway.

Can you give a quick description of the timer hardware and how it works?

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Reviewed-by: David Lechner <david@lechnology.com>
> ---
>  drivers/clocksource/Kconfig         |   5 +
>  drivers/clocksource/Makefile        |   1 +
>  drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
>  include/clocksource/timer-davinci.h |  44 +++
>  4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"

Make the option silent (eg. 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 be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
> --- /dev/null
> +++ b/drivers/clocksource/timer-davinci.c
> @@ -0,0 +1,438 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// TI DaVinci clocksource driver
> +//
> +// Copyright (C) 2019 Texas Instruments
> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/clocksource.h>
> +#include <linux/err.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)
> +
> +/* Shift depends on timer. */
> +#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_PERIODIC		BIT(1)
> +
> +#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_CLKSRC_BITS		32
> +
> +#define DAVINCI_TIMER_TGCR_DEFAULT \
> +		(DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> +
> +enum {
> +	DAVINCI_TIMER_MODE_DISABLED = 0,
> +	DAVINCI_TIMER_MODE_ONESHOT,
> +	DAVINCI_TIMER_MODE_PERIODIC,
> +};

This is replicating what is already available. Right after set_periodic
or set_oneshot are called, the timer state is changed to respectively
CLOCK_EVT_STATE_PERIODIC and CLOCK_EVT_STATE_ONESHOT. So it is useless
to define those enum again as what you want is to check the timer mode.


[ ... ]

> +
> +	clocksource = kzalloc(sizeof(*clocksource), GFP_KERNEL);
> +	if (!clocksource) {
> +		pr_err("Error allocating memory for clocksource data");
> +		return -ENOMEM;
> +	}
> +
> +	clocksource->dev.rating = 300;
> +	clocksource->dev.read = davinci_timer_clksrc_read;
> +	clocksource->dev.mask = CLOCKSOURCE_MASK(DAVINCI_TIMER_CLKSRC_BITS);
> +	clocksource->dev.flags = CLOCK_SOURCE_IS_CONTINUOUS;

>>>

> +	clocksource->timer.set_period = davinci_timer_set_period_std;
> +	clocksource->timer.mode = DAVINCI_TIMER_MODE_PERIODIC;
> +	clocksource->timer.base = base;

What for?

<<<

> +	if (timer_cfg->cmp_off) {
> +		clocksource->timer.regs = &davinci_timer_tim12_regs;
> +		clocksource->dev.name = "tim12";
> +	} else {
> +		clocksource->timer.regs = &davinci_timer_tim34_regs;
> +		clocksource->dev.name = "tim34";
> +	}
> +
> +	rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKSOURCE_IRQ].start,
> +			 davinci_timer_irq_freerun, IRQF_TIMER,
> +			 "free-run counter", clocksource);
> +	if (rv) {
> +		pr_err("Unable to request the clocksource interrupt");
> +		return rv;
> +	}

Why do you have to request an interrupt to do nothing? Isn't possible to
let the timer run and wrap without generating interrupts?

[ ... ]




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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver

[Bartosz Golaszewski]

pon., 15 kwi 2019 o 14:36 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>
> On 18/03/2019 13:10, 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.
> >
> > We don't bother freeing resources on errors in davinci_timer_register()
> > as the system won't boot without a timer anyway.
>
> Can you give a quick description of the timer hardware and how it works?
>

Will do.

> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Reviewed-by: David Lechner <david@lechnology.com>
> > ---
> >  drivers/clocksource/Kconfig         |   5 +
> >  drivers/clocksource/Makefile        |   1 +
> >  drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
> >  include/clocksource/timer-davinci.h |  44 +++
> >  4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"
>
> Make the option silent (eg. if COMPILE_TEST)
>

Sure, I already did it locally after your last review.

> > +     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 be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-davinci.c
> > @@ -0,0 +1,438 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// TI DaVinci clocksource driver
> > +//
> > +// Copyright (C) 2019 Texas Instruments
> > +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> > +
> > +#include <linux/clk.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/err.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)
> > +
> > +/* Shift depends on timer. */
> > +#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_PERIODIC               BIT(1)
> > +
> > +#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_CLKSRC_BITS            32
> > +
> > +#define DAVINCI_TIMER_TGCR_DEFAULT \
> > +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> > +
> > +enum {
> > +     DAVINCI_TIMER_MODE_DISABLED = 0,
> > +     DAVINCI_TIMER_MODE_ONESHOT,
> > +     DAVINCI_TIMER_MODE_PERIODIC,
> > +};
>
> This is replicating what is already available. Right after set_periodic
> or set_oneshot are called, the timer state is changed to respectively
> CLOCK_EVT_STATE_PERIODIC and CLOCK_EVT_STATE_ONESHOT. So it is useless
> to define those enum again as what you want is to check the timer mode.
>

I did it like this because I'm reusing the code in
davinci_timer_set_period_std() for both clocksource and clockevent
timers. If you prefer it be split to reuse the clockevent accessors, I
can do this (see below).

>
> [ ... ]
>
> > +
> > +     clocksource = kzalloc(sizeof(*clocksource), GFP_KERNEL);
> > +     if (!clocksource) {
> > +             pr_err("Error allocating memory for clocksource data");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     clocksource->dev.rating = 300;
> > +     clocksource->dev.read = davinci_timer_clksrc_read;
> > +     clocksource->dev.mask = CLOCKSOURCE_MASK(DAVINCI_TIMER_CLKSRC_BITS);
> > +     clocksource->dev.flags = CLOCK_SOURCE_IS_CONTINUOUS;
>
> >>>
>
> > +     clocksource->timer.set_period = davinci_timer_set_period_std;
> > +     clocksource->timer.mode = DAVINCI_TIMER_MODE_PERIODIC;
> > +     clocksource->timer.base = base;
>
> What for?
>

What am I assigning the timer for? In order to call
davinci_timer_set_period() on it at the bottom of the init function.
I'm not sure if it is a problem you're pointing out, but as I said
above - I can configure the clocksource timer by hand in the init
function, drop the davinci_timer_clocksource structure and use the
clockevent accessors for checking the clock mode if you prefer it that
way. Would that be fine?

> <<<
>
> > +     if (timer_cfg->cmp_off) {
> > +             clocksource->timer.regs = &davinci_timer_tim12_regs;
> > +             clocksource->dev.name = "tim12";
> > +     } else {
> > +             clocksource->timer.regs = &davinci_timer_tim34_regs;
> > +             clocksource->dev.name = "tim34";
> > +     }
> > +
> > +     rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKSOURCE_IRQ].start,
> > +                      davinci_timer_irq_freerun, IRQF_TIMER,
> > +                      "free-run counter", clocksource);
> > +     if (rv) {
> > +             pr_err("Unable to request the clocksource interrupt");
> > +             return rv;
> > +     }
>
> Why do you have to request an interrupt to do nothing? Isn't possible to
> let the timer run and wrap without generating interrupts?
>

Yes it is, but the already existing DT bindings define two interrupts.
It's true that nobody uses it, but I thought I'd stick with what was
done before. If you prefer that, I can use a single interrupt - and
just ignore the second one defined in DT (and also remove it from the
config structure for board files).

Thanks,
Bartosz

> [ ... ]
>
>
>
>
> --
>  <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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver

[Bartosz Golaszewski]

wt., 16 kwi 2019 o 15:44 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> pon., 15 kwi 2019 o 14:36 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
> >
> > On 18/03/2019 13:10, 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.
> > >
> > > We don't bother freeing resources on errors in davinci_timer_register()
> > > as the system won't boot without a timer anyway.
> >
> > Can you give a quick description of the timer hardware and how it works?
> >
>
> Will do.
>
> > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > Reviewed-by: David Lechner <david@lechnology.com>
> > > ---
> > >  drivers/clocksource/Kconfig         |   5 +
> > >  drivers/clocksource/Makefile        |   1 +
> > >  drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
> > >  include/clocksource/timer-davinci.h |  44 +++
> > >  4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"
> >
> > Make the option silent (eg. if COMPILE_TEST)
> >
>
> Sure, I already did it locally after your last review.
>
> > > +     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 be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
> > > --- /dev/null
> > > +++ b/drivers/clocksource/timer-davinci.c
> > > @@ -0,0 +1,438 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +//
> > > +// TI DaVinci clocksource driver
> > > +//
> > > +// Copyright (C) 2019 Texas Instruments
> > > +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/clockchips.h>
> > > +#include <linux/clocksource.h>
> > > +#include <linux/err.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)
> > > +
> > > +/* Shift depends on timer. */
> > > +#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_PERIODIC               BIT(1)
> > > +
> > > +#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_CLKSRC_BITS            32
> > > +
> > > +#define DAVINCI_TIMER_TGCR_DEFAULT \
> > > +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> > > +
> > > +enum {
> > > +     DAVINCI_TIMER_MODE_DISABLED = 0,
> > > +     DAVINCI_TIMER_MODE_ONESHOT,
> > > +     DAVINCI_TIMER_MODE_PERIODIC,
> > > +};
> >
> > This is replicating what is already available. Right after set_periodic
> > or set_oneshot are called, the timer state is changed to respectively
> > CLOCK_EVT_STATE_PERIODIC and CLOCK_EVT_STATE_ONESHOT. So it is useless
> > to define those enum again as what you want is to check the timer mode.
> >
>
> I did it like this because I'm reusing the code in
> davinci_timer_set_period_std() for both clocksource and clockevent
> timers. If you prefer it be split to reuse the clockevent accessors, I
> can do this (see below).
>
> >
> > [ ... ]
> >
> > > +
> > > +     clocksource = kzalloc(sizeof(*clocksource), GFP_KERNEL);
> > > +     if (!clocksource) {
> > > +             pr_err("Error allocating memory for clocksource data");
> > > +             return -ENOMEM;
> > > +     }
> > > +
> > > +     clocksource->dev.rating = 300;
> > > +     clocksource->dev.read = davinci_timer_clksrc_read;
> > > +     clocksource->dev.mask = CLOCKSOURCE_MASK(DAVINCI_TIMER_CLKSRC_BITS);
> > > +     clocksource->dev.flags = CLOCK_SOURCE_IS_CONTINUOUS;
> >
> > >>>
> >
> > > +     clocksource->timer.set_period = davinci_timer_set_period_std;
> > > +     clocksource->timer.mode = DAVINCI_TIMER_MODE_PERIODIC;
> > > +     clocksource->timer.base = base;
> >
> > What for?
> >
>
> What am I assigning the timer for? In order to call
> davinci_timer_set_period() on it at the bottom of the init function.
> I'm not sure if it is a problem you're pointing out, but as I said
> above - I can configure the clocksource timer by hand in the init
> function, drop the davinci_timer_clocksource structure and use the
> clockevent accessors for checking the clock mode if you prefer it that
> way. Would that be fine?
>
> > <<<
> >
> > > +     if (timer_cfg->cmp_off) {
> > > +             clocksource->timer.regs = &davinci_timer_tim12_regs;
> > > +             clocksource->dev.name = "tim12";
> > > +     } else {
> > > +             clocksource->timer.regs = &davinci_timer_tim34_regs;
> > > +             clocksource->dev.name = "tim34";
> > > +     }
> > > +

Just to clarify for the above: I'll still need to account for these
lines here if I go that way. This again leads to code duplication IMO.

Bart

> > > +     rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKSOURCE_IRQ].start,
> > > +                      davinci_timer_irq_freerun, IRQF_TIMER,
> > > +                      "free-run counter", clocksource);
> > > +     if (rv) {
> > > +             pr_err("Unable to request the clocksource interrupt");
> > > +             return rv;
> > > +     }
> >
> > Why do you have to request an interrupt to do nothing? Isn't possible to
> > let the timer run and wrap without generating interrupts?
> >
>
> Yes it is, but the already existing DT bindings define two interrupts.
> It's true that nobody uses it, but I thought I'd stick with what was
> done before. If you prefer that, I can use a single interrupt - and
> just ignore the second one defined in DT (and also remove it from the
> config structure for board files).
>
> Thanks,
> Bartosz
>
> > [ ... ]
> >
> >
> >
> >
> > --
> >  <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
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver

[Daniel Lezcano]

Hi Bartosz,


On 16/04/2019 15:44, Bartosz Golaszewski wrote:
> pon., 15 kwi 2019 o 14:36 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a):
>>
>> On 18/03/2019 13:10, 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.
>>>
>>> We don't bother freeing resources on errors in davinci_timer_register()
>>> as the system won't boot without a timer anyway.
>>
>> Can you give a quick description of the timer hardware and how it works?
>>
> 
> Will do.
> 
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> Reviewed-by: David Lechner <david@lechnology.com>
>>> ---
>>>  drivers/clocksource/Kconfig         |   5 +
>>>  drivers/clocksource/Makefile        |   1 +
>>>  drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
>>>  include/clocksource/timer-davinci.h |  44 +++
>>>  4 files changed, 488 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 171502a356aa..08b1f539cfc4 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"
>>
>> Make the option silent (eg. if COMPILE_TEST)
>>
> 
> Sure, I already did it locally after your last review.
> 
>>> +     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 be6e0fbc7489..3c73d0e58b45 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..46dfc4d457fc
>>> --- /dev/null
>>> +++ b/drivers/clocksource/timer-davinci.c
>>> @@ -0,0 +1,438 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +//
>>> +// TI DaVinci clocksource driver
>>> +//
>>> +// Copyright (C) 2019 Texas Instruments
>>> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/clocksource.h>
>>> +#include <linux/err.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)
>>> +
>>> +/* Shift depends on timer. */
>>> +#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_PERIODIC               BIT(1)
>>> +
>>> +#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_CLKSRC_BITS            32
>>> +
>>> +#define DAVINCI_TIMER_TGCR_DEFAULT \
>>> +             (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
>>> +
>>> +enum {
>>> +     DAVINCI_TIMER_MODE_DISABLED = 0,
>>> +     DAVINCI_TIMER_MODE_ONESHOT,
>>> +     DAVINCI_TIMER_MODE_PERIODIC,
>>> +};
>>
>> This is replicating what is already available. Right after set_periodic
>> or set_oneshot are called, the timer state is changed to respectively
>> CLOCK_EVT_STATE_PERIODIC and CLOCK_EVT_STATE_ONESHOT. So it is useless
>> to define those enum again as what you want is to check the timer mode.
>>
> 
> I did it like this because I'm reusing the code in
> davinci_timer_set_period_std() for both clocksource and clockevent
> timers. If you prefer it be split to reuse the clockevent accessors, I
> can do this (see below).

I see. It is too much complicated for the purpose. The values are
computed around every time with this flag. At the first glance, the
driver can be simpler.

Please do the following rework:

1. One patch providing the clockevents code only

2. One patch providing the clocksource code only and taking account the
comments below.

In order to not review the entire series every time, just send those two
patches until we agree on it and then the rest of the series.

The computation in the function davinci_timer_set_period_std() can be
replaced with constant put in the field instead of bit shifting everytime.


>> [ ... ]
>>
>>> +
>>> +     clocksource = kzalloc(sizeof(*clocksource), GFP_KERNEL);
>>> +     if (!clocksource) {
>>> +             pr_err("Error allocating memory for clocksource data");
>>> +             return -ENOMEM;
>>> +     }
>>> +
>>> +     clocksource->dev.rating = 300;
>>> +     clocksource->dev.read = davinci_timer_clksrc_read;
>>> +     clocksource->dev.mask = CLOCKSOURCE_MASK(DAVINCI_TIMER_CLKSRC_BITS);
>>> +     clocksource->dev.flags = CLOCK_SOURCE_IS_CONTINUOUS;
>>
>>>>>
>>
>>> +     clocksource->timer.set_period = davinci_timer_set_period_std;
>>> +     clocksource->timer.mode = DAVINCI_TIMER_MODE_PERIODIC;
>>> +     clocksource->timer.base = base;
>>
>> What for?
>>
> 
> What am I assigning the timer for? In order to call
> davinci_timer_set_period() on it at the bottom of the init function.
> I'm not sure if it is a problem you're pointing out, but as I said
> above - I can configure the clocksource timer by hand in the init
> function, drop the davinci_timer_clocksource structure and use the
> clockevent accessors for checking the clock mode if you prefer it that
> way. Would that be fine?

Yes but without checking the clock mode.

>>> +     if (timer_cfg->cmp_off) {
>>> +             clocksource->timer.regs = &davinci_timer_tim12_regs;
>>> +             clocksource->dev.name = "tim12";
>>> +     } else {
>>> +             clocksource->timer.regs = &davinci_timer_tim34_regs;
>>> +             clocksource->dev.name = "tim34";
>>> +     }
>>> +
>>> +     rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKSOURCE_IRQ].start,
>>> +                      davinci_timer_irq_freerun, IRQF_TIMER,
>>> +                      "free-run counter", clocksource);
>>> +     if (rv) {
>>> +             pr_err("Unable to request the clocksource interrupt");
>>> +             return rv;
>>> +     }
>>
>> Why do you have to request an interrupt to do nothing? Isn't possible to
>> let the timer run and wrap without generating interrupts?
>>
> 
> Yes it is, but the already existing DT bindings define two interrupts.
> It's true that nobody uses it, but I thought I'd stick with what was
> done before. If you prefer that, I can use a single interrupt - and
> just ignore the second one defined in DT (and also remove it from the
> config structure for board files).

Yes please, remove the interrupt handling for the clocksource but keep
in the config structure the interrupt in case we want in the future swap
the clocksource and the clockevent.



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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel