[PATCH v5 0/3] Introduce clocksource driver for Keystone platform

[PATCH v5 0/3] Introduce clocksource driver for Keystone platform

[Ivan Khoronzhuk]

Add a broadcast timer64 based clockevent driver for keystone arch.
This driver uses timer in 64-bit general purpose mode as clock event
device.

Documentation:
    http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf

Based on
git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git
keystone/master

v4..v5:
	used __iowmb() insted of wmb()

v3..v4:
	rebased on latest of linux-keystone.git keystone/master

v2..v3:
- clocksource: timer-keystone: introduce clocksource driver for
	changed "u64" type to "unsigned long" for hz_period as more appropriate
	hz_period rounded up by DIV_ROUND_UP(rate, HZ)
	corrected comments

v1..v2:
- clocksource: timer-keystone: introduce clocksource driver for
	renamed timer on "timer-keystone"
	in keystone_timer_interrupt() evet pointer is passed via "dev_id"
	used __relaxed variants of writel/readl and added explicit barriers
	added "keystone_timer_disable()" for using in keystone_set_mode()
	keystone_timer_config() is not used for disabling the timer any more
	in case of an unsupported mode the keystone_timer_config() returns -1.
	used request_irq() instead of setup_irq()
	assigned irq for event_device in event_dev->irq
	calculated timer.hz_period for CLOCK_EVT_MODE_PERIODIC at init
	deleted spare call of keystone_timer_config() in keystone_timer_init()

Ivan Khoronzhuk (3):
  clocksource: timer-keystone: introduce clocksource driver for Keystone
  clocksource: keystone: add bindings for keystone timer
  arm: dts: keystone: add keystone timer entry

 .../bindings/timer/ti,keystone-timer.txt           |  29 +++
 arch/arm/boot/dts/keystone-clocks.dtsi             |  10 +
 arch/arm/boot/dts/keystone.dtsi                    |   7 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-keystone.c               | 233 +++++++++++++++++++++
 5 files changed, 280 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
 create mode 100644 drivers/clocksource/timer-keystone.c

-- 
1.8.3.2

[PATCH v5 0/3] Introduce clocksource driver for Keystone platform

[Ivan Khoronzhuk]

Add a broadcast timer64 based clockevent driver for keystone arch.
This driver uses timer in 64-bit general purpose mode as clock event
device.

Documentation:
    http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf

Based on
git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git
keystone/master

v4..v5:
	used __iowmb() insted of wmb()

v3..v4:
	rebased on latest of linux-keystone.git keystone/master

v2..v3:
- clocksource: timer-keystone: introduce clocksource driver for
	changed "u64" type to "unsigned long" for hz_period as more appropriate
	hz_period rounded up by DIV_ROUND_UP(rate, HZ)
	corrected comments

v1..v2:
- clocksource: timer-keystone: introduce clocksource driver for
	renamed timer on "timer-keystone"
	in keystone_timer_interrupt() evet pointer is passed via "dev_id"
	used __relaxed variants of writel/readl and added explicit barriers
	added "keystone_timer_disable()" for using in keystone_set_mode()
	keystone_timer_config() is not used for disabling the timer any more
	in case of an unsupported mode the keystone_timer_config() returns -1.
	used request_irq() instead of setup_irq()
	assigned irq for event_device in event_dev->irq
	calculated timer.hz_period for CLOCK_EVT_MODE_PERIODIC at init
	deleted spare call of keystone_timer_config() in keystone_timer_init()

Ivan Khoronzhuk (3):
  clocksource: timer-keystone: introduce clocksource driver for Keystone
  clocksource: keystone: add bindings for keystone timer
  arm: dts: keystone: add keystone timer entry

 .../bindings/timer/ti,keystone-timer.txt           |  29 +++
 arch/arm/boot/dts/keystone-clocks.dtsi             |  10 +
 arch/arm/boot/dts/keystone.dtsi                    |   7 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-keystone.c               | 233 +++++++++++++++++++++
 5 files changed, 280 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
 create mode 100644 drivers/clocksource/timer-keystone.c

-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 0/3] Introduce clocksource driver for Keystone platform

[Ivan Khoronzhuk]

Add a broadcast timer64 based clockevent driver for keystone arch.
This driver uses timer in 64-bit general purpose mode as clock event
device.

Documentation:
    http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf

Based on
git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git
keystone/master

v4..v5:
	used __iowmb() insted of wmb()

v3..v4:
	rebased on latest of linux-keystone.git keystone/master

v2..v3:
- clocksource: timer-keystone: introduce clocksource driver for
	changed "u64" type to "unsigned long" for hz_period as more appropriate
	hz_period rounded up by DIV_ROUND_UP(rate, HZ)
	corrected comments

v1..v2:
- clocksource: timer-keystone: introduce clocksource driver for
	renamed timer on "timer-keystone"
	in keystone_timer_interrupt() evet pointer is passed via "dev_id"
	used __relaxed variants of writel/readl and added explicit barriers
	added "keystone_timer_disable()" for using in keystone_set_mode()
	keystone_timer_config() is not used for disabling the timer any more
	in case of an unsupported mode the keystone_timer_config() returns -1.
	used request_irq() instead of setup_irq()
	assigned irq for event_device in event_dev->irq
	calculated timer.hz_period for CLOCK_EVT_MODE_PERIODIC at init
	deleted spare call of keystone_timer_config() in keystone_timer_init()

Ivan Khoronzhuk (3):
  clocksource: timer-keystone: introduce clocksource driver for Keystone
  clocksource: keystone: add bindings for keystone timer
  arm: dts: keystone: add keystone timer entry

 .../bindings/timer/ti,keystone-timer.txt           |  29 +++
 arch/arm/boot/dts/keystone-clocks.dtsi             |  10 +
 arch/arm/boot/dts/keystone.dtsi                    |   7 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-keystone.c               | 233 +++++++++++++++++++++
 5 files changed, 280 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
 create mode 100644 drivers/clocksource/timer-keystone.c

-- 
1.8.3.2

[PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

Add broadcast clock-event device for the Keystone arch.

The timer can be configured as a general-purpose 64-bit timer,
dual general-purpose 32-bit timers. When configured as dual 32-bit
timers, each half can operate in conjunction (chain mode) or
independently (unchained mode) of each other.

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
 2 files changed, 234 insertions(+)
 create mode 100644 drivers/clocksource/timer-keystone.c

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index c7ca50a..4abe5aa 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
 obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
+obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
new file mode 100644
index 0000000..2299666
--- /dev/null
+++ b/drivers/clocksource/timer-keystone.c
@@ -0,0 +1,233 @@
+/*
+ * Keystone broadcast clock-event
+ *
+ * Copyright 2013 Texas Instruments, Inc.
+ *
+ * Author: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define TIMER_NAME			"timer-keystone"
+
+/* Timer register offsets */
+#define TIM12				0x10
+#define TIM34				0x14
+#define PRD12				0x18
+#define PRD34				0x1c
+#define TCR				0x20
+#define TGCR				0x24
+#define INTCTLSTAT			0x44
+
+/* Timer register bitfields */
+#define TCR_ENAMODE_MASK		0xC0
+#define TCR_ENAMODE_ONESHOT_MASK	0x40
+#define TCR_ENAMODE_PERIODIC_MASK	0x80
+
+#define TGCR_TIM_UNRESET_MASK		0x03
+#define INTCTLSTAT_ENINT_MASK		0x01
+
+/**
+ * struct keystone_timer: holds timer's data
+ * @base: timer memory base address
+ * @hz_period: cycles per HZ period
+ * @event_dev: event device based on timer
+ */
+static struct keystone_timer {
+	void __iomem *base;
+	unsigned long hz_period;
+	struct clock_event_device event_dev;
+} timer;
+
+static inline u32 keystone_timer_readl(unsigned long rg)
+{
+	return readl_relaxed(timer.base + rg);
+}
+
+static inline void keystone_timer_writel(u32 val, unsigned long rg)
+{
+	writel_relaxed(val, timer.base + rg);
+}
+
+/**
+ * keystone_timer_config: configures timer to work in oneshot/periodic modes.
+ * @ mode: mode to configure
+ * @ period: cycles number to configure for
+ */
+static int keystone_timer_config(u64 period, enum clock_event_mode mode)
+{
+	u32 tcr;
+	u32 off;
+
+	tcr = keystone_timer_readl(TCR);
+	off = tcr & ~(TCR_ENAMODE_MASK);
+
+	/* set enable mode */
+	switch (mode) {
+	case CLOCK_EVT_MODE_ONESHOT:
+		tcr |= TCR_ENAMODE_ONESHOT_MASK;
+		break;
+	case CLOCK_EVT_MODE_PERIODIC:
+		tcr |= TCR_ENAMODE_PERIODIC_MASK;
+		break;
+	default:
+		return -1;
+	}
+
+	/* disable timer */
+	keystone_timer_writel(off, TCR);
+	/* here we have to be sure the timer has been disabled */
+	__iowmb();
+
+	/* reset counter to zero, set new period */
+	keystone_timer_writel(0, TIM12);
+	keystone_timer_writel(0, TIM34);
+	keystone_timer_writel(period & 0xffffffff, PRD12);
+	keystone_timer_writel(period >> 32, PRD34);
+
+	/*
+	 * enable timer
+	 * here we have to be sure that CNTLO, CNTHI, PRDLO, PRDHI registers
+	 * have been written.
+	 */
+	__iowmb();
+	keystone_timer_writel(tcr, TCR);
+	return 0;
+}
+
+static void keystone_timer_disable(void)
+{
+	u32 tcr;
+
+	tcr = keystone_timer_readl(TCR);
+
+	/* disable timer */
+	tcr &= ~(TCR_ENAMODE_MASK);
+	keystone_timer_writel(tcr, TCR);
+}
+
+static irqreturn_t keystone_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	evt->event_handler(evt);
+	return IRQ_HANDLED;
+}
+
+static int keystone_set_next_event(unsigned long cycles,
+				  struct clock_event_device *evt)
+{
+	return keystone_timer_config(cycles, evt->mode);
+}
+
+static void keystone_set_mode(enum clock_event_mode mode,
+			     struct clock_event_device *evt)
+{
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		keystone_timer_config(timer.hz_period, CLOCK_EVT_MODE_PERIODIC);
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	case CLOCK_EVT_MODE_ONESHOT:
+		keystone_timer_disable();
+		break;
+	default:
+		break;
+	}
+}
+
+static void __init keystone_timer_init(struct device_node *np)
+{
+	struct clock_event_device *event_dev = &timer.event_dev;
+	unsigned long rate;
+	struct clk *clk;
+	int irq, error;
+	u32 tgcr;
+
+	irq  = irq_of_parse_and_map(np, 0);
+	if (irq == NO_IRQ) {
+		pr_err("%s: failed to map interrupts\n", __func__);
+		return;
+	}
+
+	timer.base = of_iomap(np, 0);
+	if (!timer.base) {
+		pr_err("%s: failed to map registers\n", __func__);
+		return;
+	}
+
+	clk = of_clk_get(np, 0);
+	if (!clk) {
+		pr_err("%s: failed to get clock\n", __func__);
+		iounmap(timer.base);
+		return;
+	}
+
+	error = clk_prepare_enable(clk);
+	if (error) {
+		pr_err("%s: failed to enable clock\n", __func__);
+		goto err;
+	}
+
+	rate = clk_get_rate(clk);
+
+	/* disable, use internal clock source */
+	keystone_timer_writel(0, TCR);
+	/* here we have to be sure the timer has been disabled */
+	__iowmb();
+
+	/* reset timer as 64-bit, no pre-scaler, plus features are disabled */
+	tgcr = 0;
+	keystone_timer_writel(0, TGCR);
+
+	/* unreset timer */
+	tgcr |= TGCR_TIM_UNRESET_MASK;
+	keystone_timer_writel(tgcr, TGCR);
+
+	/* init counter to zero */
+	keystone_timer_writel(0, TIM12);
+	keystone_timer_writel(0, TIM34);
+
+	timer.hz_period = DIV_ROUND_UP(rate, HZ);
+
+	/* enable timer interrupts */
+	keystone_timer_writel(INTCTLSTAT_ENINT_MASK, INTCTLSTAT);
+
+	error = request_irq(irq, keystone_timer_interrupt, IRQF_TIMER,
+			    TIMER_NAME, event_dev);
+	if (error) {
+		pr_err("%s: failed to setup irq\n", __func__);
+		goto err;
+	}
+
+	/* setup clockevent */
+	event_dev->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+	event_dev->set_next_event = keystone_set_next_event;
+	event_dev->set_mode = keystone_set_mode;
+	event_dev->cpumask = cpu_all_mask;
+	event_dev->owner = THIS_MODULE;
+	event_dev->name = TIMER_NAME;
+	event_dev->irq = irq;
+
+	clockevents_config_and_register(event_dev, rate, 1, ULONG_MAX);
+
+	pr_info("keystone timer clock @%lu Hz\n", rate);
+	return;
+err:
+	clk_put(clk);
+	iounmap(timer.base);
+}
+
+CLOCKSOURCE_OF_DECLARE(keystone_timer, "ti,keystone-timer",
+					keystone_timer_init);
-- 
1.8.3.2

[PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

Add broadcast clock-event device for the Keystone arch.

The timer can be configured as a general-purpose 64-bit timer,
dual general-purpose 32-bit timers. When configured as dual 32-bit
timers, each half can operate in conjunction (chain mode) or
independently (unchained mode) of each other.

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
 2 files changed, 234 insertions(+)
 create mode 100644 drivers/clocksource/timer-keystone.c

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index c7ca50a..4abe5aa 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
 obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
+obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
new file mode 100644
index 0000000..2299666
--- /dev/null
+++ b/drivers/clocksource/timer-keystone.c
@@ -0,0 +1,233 @@
+/*
+ * Keystone broadcast clock-event
+ *
+ * Copyright 2013 Texas Instruments, Inc.
+ *
+ * Author: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define TIMER_NAME			"timer-keystone"
+
+/* Timer register offsets */
+#define TIM12				0x10
+#define TIM34				0x14
+#define PRD12				0x18
+#define PRD34				0x1c
+#define TCR				0x20
+#define TGCR				0x24
+#define INTCTLSTAT			0x44
+
+/* Timer register bitfields */
+#define TCR_ENAMODE_MASK		0xC0
+#define TCR_ENAMODE_ONESHOT_MASK	0x40
+#define TCR_ENAMODE_PERIODIC_MASK	0x80
+
+#define TGCR_TIM_UNRESET_MASK		0x03
+#define INTCTLSTAT_ENINT_MASK		0x01
+
+/**
+ * struct keystone_timer: holds timer's data
+ * @base: timer memory base address
+ * @hz_period: cycles per HZ period
+ * @event_dev: event device based on timer
+ */
+static struct keystone_timer {
+	void __iomem *base;
+	unsigned long hz_period;
+	struct clock_event_device event_dev;
+} timer;
+
+static inline u32 keystone_timer_readl(unsigned long rg)
+{
+	return readl_relaxed(timer.base + rg);
+}
+
+static inline void keystone_timer_writel(u32 val, unsigned long rg)
+{
+	writel_relaxed(val, timer.base + rg);
+}
+
+/**
+ * keystone_timer_config: configures timer to work in oneshot/periodic modes.
+ * @ mode: mode to configure
+ * @ period: cycles number to configure for
+ */
+static int keystone_timer_config(u64 period, enum clock_event_mode mode)
+{
+	u32 tcr;
+	u32 off;
+
+	tcr = keystone_timer_readl(TCR);
+	off = tcr & ~(TCR_ENAMODE_MASK);
+
+	/* set enable mode */
+	switch (mode) {
+	case CLOCK_EVT_MODE_ONESHOT:
+		tcr |= TCR_ENAMODE_ONESHOT_MASK;
+		break;
+	case CLOCK_EVT_MODE_PERIODIC:
+		tcr |= TCR_ENAMODE_PERIODIC_MASK;
+		break;
+	default:
+		return -1;
+	}
+
+	/* disable timer */
+	keystone_timer_writel(off, TCR);
+	/* here we have to be sure the timer has been disabled */
+	__iowmb();
+
+	/* reset counter to zero, set new period */
+	keystone_timer_writel(0, TIM12);
+	keystone_timer_writel(0, TIM34);
+	keystone_timer_writel(period & 0xffffffff, PRD12);
+	keystone_timer_writel(period >> 32, PRD34);
+
+	/*
+	 * enable timer
+	 * here we have to be sure that CNTLO, CNTHI, PRDLO, PRDHI registers
+	 * have been written.
+	 */
+	__iowmb();
+	keystone_timer_writel(tcr, TCR);
+	return 0;
+}
+
+static void keystone_timer_disable(void)
+{
+	u32 tcr;
+
+	tcr = keystone_timer_readl(TCR);
+
+	/* disable timer */
+	tcr &= ~(TCR_ENAMODE_MASK);
+	keystone_timer_writel(tcr, TCR);
+}
+
+static irqreturn_t keystone_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	evt->event_handler(evt);
+	return IRQ_HANDLED;
+}
+
+static int keystone_set_next_event(unsigned long cycles,
+				  struct clock_event_device *evt)
+{
+	return keystone_timer_config(cycles, evt->mode);
+}
+
+static void keystone_set_mode(enum clock_event_mode mode,
+			     struct clock_event_device *evt)
+{
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		keystone_timer_config(timer.hz_period, CLOCK_EVT_MODE_PERIODIC);
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	case CLOCK_EVT_MODE_ONESHOT:
+		keystone_timer_disable();
+		break;
+	default:
+		break;
+	}
+}
+
+static void __init keystone_timer_init(struct device_node *np)
+{
+	struct clock_event_device *event_dev = &timer.event_dev;
+	unsigned long rate;
+	struct clk *clk;
+	int irq, error;
+	u32 tgcr;
+
+	irq  = irq_of_parse_and_map(np, 0);
+	if (irq == NO_IRQ) {
+		pr_err("%s: failed to map interrupts\n", __func__);
+		return;
+	}
+
+	timer.base = of_iomap(np, 0);
+	if (!timer.base) {
+		pr_err("%s: failed to map registers\n", __func__);
+		return;
+	}
+
+	clk = of_clk_get(np, 0);
+	if (!clk) {
+		pr_err("%s: failed to get clock\n", __func__);
+		iounmap(timer.base);
+		return;
+	}
+
+	error = clk_prepare_enable(clk);
+	if (error) {
+		pr_err("%s: failed to enable clock\n", __func__);
+		goto err;
+	}
+
+	rate = clk_get_rate(clk);
+
+	/* disable, use internal clock source */
+	keystone_timer_writel(0, TCR);
+	/* here we have to be sure the timer has been disabled */
+	__iowmb();
+
+	/* reset timer as 64-bit, no pre-scaler, plus features are disabled */
+	tgcr = 0;
+	keystone_timer_writel(0, TGCR);
+
+	/* unreset timer */
+	tgcr |= TGCR_TIM_UNRESET_MASK;
+	keystone_timer_writel(tgcr, TGCR);
+
+	/* init counter to zero */
+	keystone_timer_writel(0, TIM12);
+	keystone_timer_writel(0, TIM34);
+
+	timer.hz_period = DIV_ROUND_UP(rate, HZ);
+
+	/* enable timer interrupts */
+	keystone_timer_writel(INTCTLSTAT_ENINT_MASK, INTCTLSTAT);
+
+	error = request_irq(irq, keystone_timer_interrupt, IRQF_TIMER,
+			    TIMER_NAME, event_dev);
+	if (error) {
+		pr_err("%s: failed to setup irq\n", __func__);
+		goto err;
+	}
+
+	/* setup clockevent */
+	event_dev->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+	event_dev->set_next_event = keystone_set_next_event;
+	event_dev->set_mode = keystone_set_mode;
+	event_dev->cpumask = cpu_all_mask;
+	event_dev->owner = THIS_MODULE;
+	event_dev->name = TIMER_NAME;
+	event_dev->irq = irq;
+
+	clockevents_config_and_register(event_dev, rate, 1, ULONG_MAX);
+
+	pr_info("keystone timer clock @%lu Hz\n", rate);
+	return;
+err:
+	clk_put(clk);
+	iounmap(timer.base);
+}
+
+CLOCKSOURCE_OF_DECLARE(keystone_timer, "ti,keystone-timer",
+					keystone_timer_init);
-- 
1.8.3.2

[PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

Add broadcast clock-event device for the Keystone arch.

The timer can be configured as a general-purpose 64-bit timer,
dual general-purpose 32-bit timers. When configured as dual 32-bit
timers, each half can operate in conjunction (chain mode) or
independently (unchained mode) of each other.

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
 2 files changed, 234 insertions(+)
 create mode 100644 drivers/clocksource/timer-keystone.c

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index c7ca50a..4abe5aa 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
 obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
+obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
new file mode 100644
index 0000000..2299666
--- /dev/null
+++ b/drivers/clocksource/timer-keystone.c
@@ -0,0 +1,233 @@
+/*
+ * Keystone broadcast clock-event
+ *
+ * Copyright 2013 Texas Instruments, Inc.
+ *
+ * Author: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define TIMER_NAME			"timer-keystone"
+
+/* Timer register offsets */
+#define TIM12				0x10
+#define TIM34				0x14
+#define PRD12				0x18
+#define PRD34				0x1c
+#define TCR				0x20
+#define TGCR				0x24
+#define INTCTLSTAT			0x44
+
+/* Timer register bitfields */
+#define TCR_ENAMODE_MASK		0xC0
+#define TCR_ENAMODE_ONESHOT_MASK	0x40
+#define TCR_ENAMODE_PERIODIC_MASK	0x80
+
+#define TGCR_TIM_UNRESET_MASK		0x03
+#define INTCTLSTAT_ENINT_MASK		0x01
+
+/**
+ * struct keystone_timer: holds timer's data
+ * @base: timer memory base address
+ * @hz_period: cycles per HZ period
+ * @event_dev: event device based on timer
+ */
+static struct keystone_timer {
+	void __iomem *base;
+	unsigned long hz_period;
+	struct clock_event_device event_dev;
+} timer;
+
+static inline u32 keystone_timer_readl(unsigned long rg)
+{
+	return readl_relaxed(timer.base + rg);
+}
+
+static inline void keystone_timer_writel(u32 val, unsigned long rg)
+{
+	writel_relaxed(val, timer.base + rg);
+}
+
+/**
+ * keystone_timer_config: configures timer to work in oneshot/periodic modes.
+ * @ mode: mode to configure
+ * @ period: cycles number to configure for
+ */
+static int keystone_timer_config(u64 period, enum clock_event_mode mode)
+{
+	u32 tcr;
+	u32 off;
+
+	tcr = keystone_timer_readl(TCR);
+	off = tcr & ~(TCR_ENAMODE_MASK);
+
+	/* set enable mode */
+	switch (mode) {
+	case CLOCK_EVT_MODE_ONESHOT:
+		tcr |= TCR_ENAMODE_ONESHOT_MASK;
+		break;
+	case CLOCK_EVT_MODE_PERIODIC:
+		tcr |= TCR_ENAMODE_PERIODIC_MASK;
+		break;
+	default:
+		return -1;
+	}
+
+	/* disable timer */
+	keystone_timer_writel(off, TCR);
+	/* here we have to be sure the timer has been disabled */
+	__iowmb();
+
+	/* reset counter to zero, set new period */
+	keystone_timer_writel(0, TIM12);
+	keystone_timer_writel(0, TIM34);
+	keystone_timer_writel(period & 0xffffffff, PRD12);
+	keystone_timer_writel(period >> 32, PRD34);
+
+	/*
+	 * enable timer
+	 * here we have to be sure that CNTLO, CNTHI, PRDLO, PRDHI registers
+	 * have been written.
+	 */
+	__iowmb();
+	keystone_timer_writel(tcr, TCR);
+	return 0;
+}
+
+static void keystone_timer_disable(void)
+{
+	u32 tcr;
+
+	tcr = keystone_timer_readl(TCR);
+
+	/* disable timer */
+	tcr &= ~(TCR_ENAMODE_MASK);
+	keystone_timer_writel(tcr, TCR);
+}
+
+static irqreturn_t keystone_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	evt->event_handler(evt);
+	return IRQ_HANDLED;
+}
+
+static int keystone_set_next_event(unsigned long cycles,
+				  struct clock_event_device *evt)
+{
+	return keystone_timer_config(cycles, evt->mode);
+}
+
+static void keystone_set_mode(enum clock_event_mode mode,
+			     struct clock_event_device *evt)
+{
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		keystone_timer_config(timer.hz_period, CLOCK_EVT_MODE_PERIODIC);
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	case CLOCK_EVT_MODE_ONESHOT:
+		keystone_timer_disable();
+		break;
+	default:
+		break;
+	}
+}
+
+static void __init keystone_timer_init(struct device_node *np)
+{
+	struct clock_event_device *event_dev = &timer.event_dev;
+	unsigned long rate;
+	struct clk *clk;
+	int irq, error;
+	u32 tgcr;
+
+	irq  = irq_of_parse_and_map(np, 0);
+	if (irq == NO_IRQ) {
+		pr_err("%s: failed to map interrupts\n", __func__);
+		return;
+	}
+
+	timer.base = of_iomap(np, 0);
+	if (!timer.base) {
+		pr_err("%s: failed to map registers\n", __func__);
+		return;
+	}
+
+	clk = of_clk_get(np, 0);
+	if (!clk) {
+		pr_err("%s: failed to get clock\n", __func__);
+		iounmap(timer.base);
+		return;
+	}
+
+	error = clk_prepare_enable(clk);
+	if (error) {
+		pr_err("%s: failed to enable clock\n", __func__);
+		goto err;
+	}
+
+	rate = clk_get_rate(clk);
+
+	/* disable, use internal clock source */
+	keystone_timer_writel(0, TCR);
+	/* here we have to be sure the timer has been disabled */
+	__iowmb();
+
+	/* reset timer as 64-bit, no pre-scaler, plus features are disabled */
+	tgcr = 0;
+	keystone_timer_writel(0, TGCR);
+
+	/* unreset timer */
+	tgcr |= TGCR_TIM_UNRESET_MASK;
+	keystone_timer_writel(tgcr, TGCR);
+
+	/* init counter to zero */
+	keystone_timer_writel(0, TIM12);
+	keystone_timer_writel(0, TIM34);
+
+	timer.hz_period = DIV_ROUND_UP(rate, HZ);
+
+	/* enable timer interrupts */
+	keystone_timer_writel(INTCTLSTAT_ENINT_MASK, INTCTLSTAT);
+
+	error = request_irq(irq, keystone_timer_interrupt, IRQF_TIMER,
+			    TIMER_NAME, event_dev);
+	if (error) {
+		pr_err("%s: failed to setup irq\n", __func__);
+		goto err;
+	}
+
+	/* setup clockevent */
+	event_dev->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+	event_dev->set_next_event = keystone_set_next_event;
+	event_dev->set_mode = keystone_set_mode;
+	event_dev->cpumask = cpu_all_mask;
+	event_dev->owner = THIS_MODULE;
+	event_dev->name = TIMER_NAME;
+	event_dev->irq = irq;
+
+	clockevents_config_and_register(event_dev, rate, 1, ULONG_MAX);
+
+	pr_info("keystone timer clock @%lu Hz\n", rate);
+	return;
+err:
+	clk_put(clk);
+	iounmap(timer.base);
+}
+
+CLOCKSOURCE_OF_DECLARE(keystone_timer, "ti,keystone-timer",
+					keystone_timer_init);
-- 
1.8.3.2

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

This patch provides bindings for the 64-bit timer in the KeyStone
architecture devices. The timer can be configured as a general-purpose 64-bit
timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
timers, each half can operate in conjunction (chain mode) or independently
(unchained mode) of each other.

It is global timer is a free running up-counter and can generate interrupt
when the counter reaches preset counter values.

Documentation:
http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf

Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 .../bindings/timer/ti,keystone-timer.txt           | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
new file mode 100644
index 0000000..5fbe361
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
@@ -0,0 +1,29 @@
+* Device tree bindings for Texas instruments Keystone timer
+
+This document provides bindings for the 64-bit timer in the KeyStone
+architecture devices. The timer can be configured as a general-purpose 64-bit
+timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
+timers, each half can operate in conjunction (chain mode) or independently
+(unchained mode) of each other.
+
+It is global timer is a free running up-counter and can generate interrupt
+when the counter reaches preset counter values.
+
+Documentation:
+http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
+
+Required properties:
+
+- compatible : should be "ti,keystone-timer".
+- reg : specifies base physical address and count of the registers.
+- interrupts : interrupt generated by the timer.
+- clocks : the clock feeding the timer clock.
+
+Example:
+
+timer@22f0000 {
+	compatible = "ti,keystone-timer";
+	reg = <0x022f0000 0x80>;
+	interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
+	clocks = <&clktimer15>;
+};
-- 
1.8.3.2

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

This patch provides bindings for the 64-bit timer in the KeyStone
architecture devices. The timer can be configured as a general-purpose 64-bit
timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
timers, each half can operate in conjunction (chain mode) or independently
(unchained mode) of each other.

It is global timer is a free running up-counter and can generate interrupt
when the counter reaches preset counter values.

Documentation:
http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf

Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 .../bindings/timer/ti,keystone-timer.txt           | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
new file mode 100644
index 0000000..5fbe361
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
@@ -0,0 +1,29 @@
+* Device tree bindings for Texas instruments Keystone timer
+
+This document provides bindings for the 64-bit timer in the KeyStone
+architecture devices. The timer can be configured as a general-purpose 64-bit
+timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
+timers, each half can operate in conjunction (chain mode) or independently
+(unchained mode) of each other.
+
+It is global timer is a free running up-counter and can generate interrupt
+when the counter reaches preset counter values.
+
+Documentation:
+http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
+
+Required properties:
+
+- compatible : should be "ti,keystone-timer".
+- reg : specifies base physical address and count of the registers.
+- interrupts : interrupt generated by the timer.
+- clocks : the clock feeding the timer clock.
+
+Example:
+
+timer@22f0000 {
+	compatible = "ti,keystone-timer";
+	reg = <0x022f0000 0x80>;
+	interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
+	clocks = <&clktimer15>;
+};
-- 
1.8.3.2

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

This patch provides bindings for the 64-bit timer in the KeyStone
architecture devices. The timer can be configured as a general-purpose 64-bit
timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
timers, each half can operate in conjunction (chain mode) or independently
(unchained mode) of each other.

It is global timer is a free running up-counter and can generate interrupt
when the counter reaches preset counter values.

Documentation:
http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf

Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 .../bindings/timer/ti,keystone-timer.txt           | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
new file mode 100644
index 0000000..5fbe361
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
@@ -0,0 +1,29 @@
+* Device tree bindings for Texas instruments Keystone timer
+
+This document provides bindings for the 64-bit timer in the KeyStone
+architecture devices. The timer can be configured as a general-purpose 64-bit
+timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
+timers, each half can operate in conjunction (chain mode) or independently
+(unchained mode) of each other.
+
+It is global timer is a free running up-counter and can generate interrupt
+when the counter reaches preset counter values.
+
+Documentation:
+http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
+
+Required properties:
+
+- compatible : should be "ti,keystone-timer".
+- reg : specifies base physical address and count of the registers.
+- interrupts : interrupt generated by the timer.
+- clocks : the clock feeding the timer clock.
+
+Example:
+
+timer at 22f0000 {
+	compatible = "ti,keystone-timer";
+	reg = <0x022f0000 0x80>;
+	interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
+	clocks = <&clktimer15>;
+};
-- 
1.8.3.2

[PATCH v5 3/3] arm: dts: keystone: add keystone timer entry

[Ivan Khoronzhuk]

Add keystone timer entry to keystone device tree.
This 64-bit timer is used as backup clock event device.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 arch/arm/boot/dts/keystone-clocks.dtsi | 10 ++++++++++
 arch/arm/boot/dts/keystone.dtsi        |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/keystone-clocks.dtsi b/arch/arm/boot/dts/keystone-clocks.dtsi
index 2363593..16d2aba 100644
--- a/arch/arm/boot/dts/keystone-clocks.dtsi
+++ b/arch/arm/boot/dts/keystone-clocks.dtsi
@@ -737,6 +737,16 @@ clocks {
 		domain-id = <0>;
 	};
 
+	clktimer15: clktimer15 {
+		#clock-cells = <0>;
+		compatible = "ti,keystone,psc-clock";
+		clocks = <&clkmodrst0>;
+		clock-output-names = "timer15";
+		reg = <0x02350000 0xb00>, <0x02350000 0x400>;
+		reg-names = "control", "domain";
+		domain-id = <0>;
+	};
+
 	clkuart0: clkuart0 {
 		#clock-cells = <0>;
 		compatible = "ti,keystone,psc-clock";
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index b420290..cac9841 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -208,5 +208,12 @@
 				usb-phy = <&usb_phy>, <&usb_phy>;
 			};
 		};
+
+		clock_event: timer@22f0000 {
+			compatible = "ti,keystone-timer";
+			reg = <0x022f0000 0x80>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&clktimer15>;
+		};
 	};
 };
-- 
1.8.3.2

[PATCH v5 3/3] arm: dts: keystone: add keystone timer entry

[Ivan Khoronzhuk]

Add keystone timer entry to keystone device tree.
This 64-bit timer is used as backup clock event device.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 arch/arm/boot/dts/keystone-clocks.dtsi | 10 ++++++++++
 arch/arm/boot/dts/keystone.dtsi        |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/keystone-clocks.dtsi b/arch/arm/boot/dts/keystone-clocks.dtsi
index 2363593..16d2aba 100644
--- a/arch/arm/boot/dts/keystone-clocks.dtsi
+++ b/arch/arm/boot/dts/keystone-clocks.dtsi
@@ -737,6 +737,16 @@ clocks {
 		domain-id = <0>;
 	};
 
+	clktimer15: clktimer15 {
+		#clock-cells = <0>;
+		compatible = "ti,keystone,psc-clock";
+		clocks = <&clkmodrst0>;
+		clock-output-names = "timer15";
+		reg = <0x02350000 0xb00>, <0x02350000 0x400>;
+		reg-names = "control", "domain";
+		domain-id = <0>;
+	};
+
 	clkuart0: clkuart0 {
 		#clock-cells = <0>;
 		compatible = "ti,keystone,psc-clock";
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index b420290..cac9841 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -208,5 +208,12 @@
 				usb-phy = <&usb_phy>, <&usb_phy>;
 			};
 		};
+
+		clock_event: timer@22f0000 {
+			compatible = "ti,keystone-timer";
+			reg = <0x022f0000 0x80>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&clktimer15>;
+		};
 	};
 };
-- 
1.8.3.2

[PATCH v5 3/3] arm: dts: keystone: add keystone timer entry

[Ivan Khoronzhuk]

Add keystone timer entry to keystone device tree.
This 64-bit timer is used as backup clock event device.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 arch/arm/boot/dts/keystone-clocks.dtsi | 10 ++++++++++
 arch/arm/boot/dts/keystone.dtsi        |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/keystone-clocks.dtsi b/arch/arm/boot/dts/keystone-clocks.dtsi
index 2363593..16d2aba 100644
--- a/arch/arm/boot/dts/keystone-clocks.dtsi
+++ b/arch/arm/boot/dts/keystone-clocks.dtsi
@@ -737,6 +737,16 @@ clocks {
 		domain-id = <0>;
 	};
 
+	clktimer15: clktimer15 {
+		#clock-cells = <0>;
+		compatible = "ti,keystone,psc-clock";
+		clocks = <&clkmodrst0>;
+		clock-output-names = "timer15";
+		reg = <0x02350000 0xb00>, <0x02350000 0x400>;
+		reg-names = "control", "domain";
+		domain-id = <0>;
+	};
+
 	clkuart0: clkuart0 {
 		#clock-cells = <0>;
 		compatible = "ti,keystone,psc-clock";
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index b420290..cac9841 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -208,5 +208,12 @@
 				usb-phy = <&usb_phy>, <&usb_phy>;
 			};
 		};
+
+		clock_event: timer at 22f0000 {
+			compatible = "ti,keystone-timer";
+			reg = <0x022f0000 0x80>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&clktimer15>;
+		};
 	};
 };
-- 
1.8.3.2

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
> This patch provides bindings for the 64-bit timer in the KeyStone
> architecture devices. The timer can be configured as a general-purpose 64-bit
> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
> timers, each half can operate in conjunction (chain mode) or independently
> (unchained mode) of each other.

This is software configurable or h/w design time configurations?

Rob

>
> It is global timer is a free running up-counter and can generate interrupt
> when the counter reaches preset counter values.
>
> Documentation:
> http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  .../bindings/timer/ti,keystone-timer.txt           | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
> new file mode 100644
> index 0000000..5fbe361
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
> @@ -0,0 +1,29 @@
> +* Device tree bindings for Texas instruments Keystone timer
> +
> +This document provides bindings for the 64-bit timer in the KeyStone
> +architecture devices. The timer can be configured as a general-purpose 64-bit
> +timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
> +timers, each half can operate in conjunction (chain mode) or independently
> +(unchained mode) of each other.
> +
> +It is global timer is a free running up-counter and can generate interrupt
> +when the counter reaches preset counter values.
> +
> +Documentation:
> +http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
> +
> +Required properties:
> +
> +- compatible : should be "ti,keystone-timer".
> +- reg : specifies base physical address and count of the registers.
> +- interrupts : interrupt generated by the timer.
> +- clocks : the clock feeding the timer clock.
> +
> +Example:
> +
> +timer@22f0000 {
> +       compatible = "ti,keystone-timer";
> +       reg = <0x022f0000 0x80>;
> +       interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
> +       clocks = <&clktimer15>;
> +};
> --
> 1.8.3.2
>

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
> This patch provides bindings for the 64-bit timer in the KeyStone
> architecture devices. The timer can be configured as a general-purpose 64-bit
> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
> timers, each half can operate in conjunction (chain mode) or independently
> (unchained mode) of each other.

This is software configurable or h/w design time configurations?

Rob

>
> It is global timer is a free running up-counter and can generate interrupt
> when the counter reaches preset counter values.
>
> Documentation:
> http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  .../bindings/timer/ti,keystone-timer.txt           | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
> new file mode 100644
> index 0000000..5fbe361
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
> @@ -0,0 +1,29 @@
> +* Device tree bindings for Texas instruments Keystone timer
> +
> +This document provides bindings for the 64-bit timer in the KeyStone
> +architecture devices. The timer can be configured as a general-purpose 64-bit
> +timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
> +timers, each half can operate in conjunction (chain mode) or independently
> +(unchained mode) of each other.
> +
> +It is global timer is a free running up-counter and can generate interrupt
> +when the counter reaches preset counter values.
> +
> +Documentation:
> +http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
> +
> +Required properties:
> +
> +- compatible : should be "ti,keystone-timer".
> +- reg : specifies base physical address and count of the registers.
> +- interrupts : interrupt generated by the timer.
> +- clocks : the clock feeding the timer clock.
> +
> +Example:
> +
> +timer@22f0000 {
> +       compatible = "ti,keystone-timer";
> +       reg = <0x022f0000 0x80>;
> +       interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
> +       clocks = <&clktimer15>;
> +};
> --
> 1.8.3.2
>

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
> This patch provides bindings for the 64-bit timer in the KeyStone
> architecture devices. The timer can be configured as a general-purpose 64-bit
> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
> timers, each half can operate in conjunction (chain mode) or independently
> (unchained mode) of each other.

This is software configurable or h/w design time configurations?

Rob

>
> It is global timer is a free running up-counter and can generate interrupt
> when the counter reaches preset counter values.
>
> Documentation:
> http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  .../bindings/timer/ti,keystone-timer.txt           | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
> new file mode 100644
> index 0000000..5fbe361
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
> @@ -0,0 +1,29 @@
> +* Device tree bindings for Texas instruments Keystone timer
> +
> +This document provides bindings for the 64-bit timer in the KeyStone
> +architecture devices. The timer can be configured as a general-purpose 64-bit
> +timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
> +timers, each half can operate in conjunction (chain mode) or independently
> +(unchained mode) of each other.
> +
> +It is global timer is a free running up-counter and can generate interrupt
> +when the counter reaches preset counter values.
> +
> +Documentation:
> +http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
> +
> +Required properties:
> +
> +- compatible : should be "ti,keystone-timer".
> +- reg : specifies base physical address and count of the registers.
> +- interrupts : interrupt generated by the timer.
> +- clocks : the clock feeding the timer clock.
> +
> +Example:
> +
> +timer at 22f0000 {
> +       compatible = "ti,keystone-timer";
> +       reg = <0x022f0000 0x80>;
> +       interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
> +       clocks = <&clktimer15>;
> +};
> --
> 1.8.3.2
>

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/05/2014 04:39 PM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> This patch provides bindings for the 64-bit timer in the KeyStone
>> architecture devices. The timer can be configured as a general-purpose 64-bit
>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>> timers, each half can operate in conjunction (chain mode) or independently
>> (unchained mode) of each other.
> This is software configurable or h/w design time configurations?
>
> Rob
>

This is h/w design time configurations

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/05/2014 04:39 PM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> This patch provides bindings for the 64-bit timer in the KeyStone
>> architecture devices. The timer can be configured as a general-purpose 64-bit
>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>> timers, each half can operate in conjunction (chain mode) or independently
>> (unchained mode) of each other.
> This is software configurable or h/w design time configurations?
>
> Rob
>

This is h/w design time configurations

-- 
Regards,
Ivan Khoronzhuk

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/05/2014 04:39 PM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> This patch provides bindings for the 64-bit timer in the KeyStone
>> architecture devices. The timer can be configured as a general-purpose 64-bit
>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>> timers, each half can operate in conjunction (chain mode) or independently
>> (unchained mode) of each other.
> This is software configurable or h/w design time configurations?
>
> Rob
>

This is h/w design time configurations

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>
> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>
>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> wrote:
>>>
>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>> architecture devices. The timer can be configured as a general-purpose
>>> 64-bit
>>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>>> timers, each half can operate in conjunction (chain mode) or
>>> independently
>>> (unchained mode) of each other.
>>
>> This is software configurable or h/w design time configurations?
>>
>> Rob
>>
>
> This is h/w design time configurations

Then it seems like the binding should provide for describing those
differences either with a property or different compatible strings.

Rob

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>
> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>
>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> wrote:
>>>
>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>> architecture devices. The timer can be configured as a general-purpose
>>> 64-bit
>>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>>> timers, each half can operate in conjunction (chain mode) or
>>> independently
>>> (unchained mode) of each other.
>>
>> This is software configurable or h/w design time configurations?
>>
>> Rob
>>
>
> This is h/w design time configurations

Then it seems like the binding should provide for describing those
differences either with a property or different compatible strings.

Rob

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>
> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>
>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> wrote:
>>>
>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>> architecture devices. The timer can be configured as a general-purpose
>>> 64-bit
>>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>>> timers, each half can operate in conjunction (chain mode) or
>>> independently
>>> (unchained mode) of each other.
>>
>> This is software configurable or h/w design time configurations?
>>
>> Rob
>>
>
> This is h/w design time configurations

Then it seems like the binding should provide for describing those
differences either with a property or different compatible strings.

Rob

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/05/2014 07:41 PM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> wrote:
>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>> architecture devices. The timer can be configured as a general-purpose
>>>> 64-bit
>>>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>>>> timers, each half can operate in conjunction (chain mode) or
>>>> independently
>>>> (unchained mode) of each other.
>>> This is software configurable or h/w design time configurations?
>>>
>>> Rob
>>>
>> This is h/w design time configurations
> Then it seems like the binding should provide for describing those
> differences either with a property or different compatible strings.
>
> Rob
Oh..sorry, seems I didn't catch, this is configurable by software.
These configurations are like modes in which timer can work
and they are not different hardware IPs. It depends on driver in
which mode it should work.

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/05/2014 07:41 PM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> wrote:
>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>> architecture devices. The timer can be configured as a general-purpose
>>>> 64-bit
>>>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>>>> timers, each half can operate in conjunction (chain mode) or
>>>> independently
>>>> (unchained mode) of each other.
>>> This is software configurable or h/w design time configurations?
>>>
>>> Rob
>>>
>> This is h/w design time configurations
> Then it seems like the binding should provide for describing those
> differences either with a property or different compatible strings.
>
> Rob
Oh..sorry, seems I didn't catch, this is configurable by software.
These configurations are like modes in which timer can work
and they are not different hardware IPs. It depends on driver in
which mode it should work.

-- 
Regards,
Ivan Khoronzhuk

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/05/2014 07:41 PM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> wrote:
>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>> architecture devices. The timer can be configured as a general-purpose
>>>> 64-bit
>>>> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
>>>> timers, each half can operate in conjunction (chain mode) or
>>>> independently
>>>> (unchained mode) of each other.
>>> This is software configurable or h/w design time configurations?
>>>
>>> Rob
>>>
>> This is h/w design time configurations
> Then it seems like the binding should provide for describing those
> differences either with a property or different compatible strings.
>
> Rob
Oh..sorry, seems I didn't catch, this is configurable by software.
These configurations are like modes in which timer can work
and they are not different hardware IPs. It depends on driver in
which mode it should work.

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Thomas Gleixner]

On Wed, 5 Feb 2014, Ivan Khoronzhuk wrote:
> +	/* here we have to be sure the timer has been disabled */

Sigh. This is not a proper explanation for a barrier, really. You want
to explain what it serializes against what. i.e. you want to explain
why you are using the relaxed functions and avoid a separate non
relaxed variant in favour of an explicit barrier.

> +	__iowmb();

The proper thing is to have an inline function key_stone_barrier() and
a full explanation of the issue in exactly that place instead of
handwaving comments here and there.

Thanks,

	tglx

[PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Thomas Gleixner]

On Wed, 5 Feb 2014, Ivan Khoronzhuk wrote:
> +	/* here we have to be sure the timer has been disabled */

Sigh. This is not a proper explanation for a barrier, really. You want
to explain what it serializes against what. i.e. you want to explain
why you are using the relaxed functions and avoid a separate non
relaxed variant in favour of an explicit barrier.

> +	__iowmb();

The proper thing is to have an inline function key_stone_barrier() and
a full explanation of the issue in exactly that place instead of
handwaving comments here and there.

Thanks,

	tglx

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 12:52 PM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>
> On 02/05/2014 07:41 PM, Rob Herring wrote:
>>
>> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> wrote:
>>>
>>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>>>
>>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>> wrote:
>>>>>
>>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>>> architecture devices. The timer can be configured as a general-purpose
>>>>> 64-bit
>>>>> timer, dual general-purpose 32-bit timers. When configured as dual
>>>>> 32-bit
>>>>> timers, each half can operate in conjunction (chain mode) or
>>>>> independently
>>>>> (unchained mode) of each other.
>>>>
>>>> This is software configurable or h/w design time configurations?
>>>>
>>>> Rob
>>>>
>>> This is h/w design time configurations
>>
>> Then it seems like the binding should provide for describing those
>> differences either with a property or different compatible strings.
>>
>> Rob
>
> Oh..sorry, seems I didn't catch, this is configurable by software.
> These configurations are like modes in which timer can work
> and they are not different hardware IPs. It depends on driver in
> which mode it should work.

In that case,

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 12:52 PM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>
> On 02/05/2014 07:41 PM, Rob Herring wrote:
>>
>> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> wrote:
>>>
>>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>>>
>>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>> wrote:
>>>>>
>>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>>> architecture devices. The timer can be configured as a general-purpose
>>>>> 64-bit
>>>>> timer, dual general-purpose 32-bit timers. When configured as dual
>>>>> 32-bit
>>>>> timers, each half can operate in conjunction (chain mode) or
>>>>> independently
>>>>> (unchained mode) of each other.
>>>>
>>>> This is software configurable or h/w design time configurations?
>>>>
>>>> Rob
>>>>
>>> This is h/w design time configurations
>>
>> Then it seems like the binding should provide for describing those
>> differences either with a property or different compatible strings.
>>
>> Rob
>
> Oh..sorry, seems I didn't catch, this is configurable by software.
> These configurations are like modes in which timer can work
> and they are not different hardware IPs. It depends on driver in
> which mode it should work.

In that case,

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Rob Herring]

On Wed, Feb 5, 2014 at 12:52 PM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>
> On 02/05/2014 07:41 PM, Rob Herring wrote:
>>
>> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> wrote:
>>>
>>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>>>
>>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>> wrote:
>>>>>
>>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>>> architecture devices. The timer can be configured as a general-purpose
>>>>> 64-bit
>>>>> timer, dual general-purpose 32-bit timers. When configured as dual
>>>>> 32-bit
>>>>> timers, each half can operate in conjunction (chain mode) or
>>>>> independently
>>>>> (unchained mode) of each other.
>>>>
>>>> This is software configurable or h/w design time configurations?
>>>>
>>>> Rob
>>>>
>>> This is h/w design time configurations
>>
>> Then it seems like the binding should provide for describing those
>> differences either with a property or different compatible strings.
>>
>> Rob
>
> Oh..sorry, seems I didn't catch, this is configurable by software.
> These configurations are like modes in which timer can work
> and they are not different hardware IPs. It depends on driver in
> which mode it should work.

In that case,

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Josh Cartwright]

Hey Ivan-

On Wed, Feb 05, 2014 at 03:47:38PM +0200, Ivan Khoronzhuk wrote:
> Add broadcast clock-event device for the Keystone arch.
> 
> The timer can be configured as a general-purpose 64-bit timer,
> dual general-purpose 32-bit timers. When configured as dual 32-bit
> timers, each half can operate in conjunction (chain mode) or
> independently (unchained mode) of each other.
> 
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
> Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  drivers/clocksource/Makefile         |   1 +
>  drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
>  2 files changed, 234 insertions(+)
>  create mode 100644 drivers/clocksource/timer-keystone.c
> 
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index c7ca50a..4abe5aa 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
>  obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
>  obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
>  obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
> +obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
> diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
> new file mode 100644
> index 0000000..2299666
> --- /dev/null
> +++ b/drivers/clocksource/timer-keystone.c
> +static void __init keystone_timer_init(struct device_node *np)
> +{
> +	struct clock_event_device *event_dev = &timer.event_dev;
> +	unsigned long rate;
> +	struct clk *clk;
> +	int irq, error;
> +	u32 tgcr;
> +
> +	irq  = irq_of_parse_and_map(np, 0);
> +	if (irq == NO_IRQ) {
> +		pr_err("%s: failed to map interrupts\n", __func__);
> +		return;
> +	}
> +
> +	timer.base = of_iomap(np, 0);
> +	if (!timer.base) {
> +		pr_err("%s: failed to map registers\n", __func__);
> +		return;
> +	}
> +
> +	clk = of_clk_get(np, 0);
> +	if (!clk) {

This condition should be IS_ERR(clk).

> +		pr_err("%s: failed to get clock\n", __func__);
> +		iounmap(timer.base);
> +		return;
> +	}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Josh Cartwright]

Hey Ivan-

On Wed, Feb 05, 2014 at 03:47:38PM +0200, Ivan Khoronzhuk wrote:
> Add broadcast clock-event device for the Keystone arch.
> 
> The timer can be configured as a general-purpose 64-bit timer,
> dual general-purpose 32-bit timers. When configured as dual 32-bit
> timers, each half can operate in conjunction (chain mode) or
> independently (unchained mode) of each other.
> 
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
> Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  drivers/clocksource/Makefile         |   1 +
>  drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
>  2 files changed, 234 insertions(+)
>  create mode 100644 drivers/clocksource/timer-keystone.c
> 
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index c7ca50a..4abe5aa 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
>  obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
>  obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
>  obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
> +obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
> diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
> new file mode 100644
> index 0000000..2299666
> --- /dev/null
> +++ b/drivers/clocksource/timer-keystone.c
> +static void __init keystone_timer_init(struct device_node *np)
> +{
> +	struct clock_event_device *event_dev = &timer.event_dev;
> +	unsigned long rate;
> +	struct clk *clk;
> +	int irq, error;
> +	u32 tgcr;
> +
> +	irq  = irq_of_parse_and_map(np, 0);
> +	if (irq == NO_IRQ) {
> +		pr_err("%s: failed to map interrupts\n", __func__);
> +		return;
> +	}
> +
> +	timer.base = of_iomap(np, 0);
> +	if (!timer.base) {
> +		pr_err("%s: failed to map registers\n", __func__);
> +		return;
> +	}
> +
> +	clk = of_clk_get(np, 0);
> +	if (!clk) {

This condition should be IS_ERR(clk).

> +		pr_err("%s: failed to get clock\n", __func__);
> +		iounmap(timer.base);
> +		return;
> +	}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

On 02/05/2014 10:27 PM, Thomas Gleixner wrote:
> On Wed, 5 Feb 2014, Ivan Khoronzhuk wrote:
>> +	/* here we have to be sure the timer has been disabled */
> Sigh. This is not a proper explanation for a barrier, really. You want
> to explain what it serializes against what. i.e. you want to explain
> why you are using the relaxed functions and avoid a separate non
> relaxed variant in favour of an explicit barrier.
>
>> +	__iowmb();
> The proper thing is to have an inline function key_stone_barrier() and
> a full explanation of the issue in exactly that place instead of
> handwaving comments here and there.
>
> Thanks,
>
> 	tglx

I can add new inline function like:

/**
  * keystone_timer_barrier: write memory barrier
  * use explicit barrier to avoid using readl/writel non relaxed function
  * variants, because in our case relaxed variants hide the true places
  * where barrier is needed.
  */
static inline void keystone_timer_barrier(void)
{
     __iowmb();
}

and use it where it is needed.
Are you OK with it?

And I propose to leave comments under the barriers in order to be
able to understand why they are used.

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

On 02/05/2014 10:27 PM, Thomas Gleixner wrote:
> On Wed, 5 Feb 2014, Ivan Khoronzhuk wrote:
>> +	/* here we have to be sure the timer has been disabled */
> Sigh. This is not a proper explanation for a barrier, really. You want
> to explain what it serializes against what. i.e. you want to explain
> why you are using the relaxed functions and avoid a separate non
> relaxed variant in favour of an explicit barrier.
>
>> +	__iowmb();
> The proper thing is to have an inline function key_stone_barrier() and
> a full explanation of the issue in exactly that place instead of
> handwaving comments here and there.
>
> Thanks,
>
> 	tglx

I can add new inline function like:

/**
  * keystone_timer_barrier: write memory barrier
  * use explicit barrier to avoid using readl/writel non relaxed function
  * variants, because in our case relaxed variants hide the true places
  * where barrier is needed.
  */
static inline void keystone_timer_barrier(void)
{
     __iowmb();
}

and use it where it is needed.
Are you OK with it?

And I propose to leave comments under the barriers in order to be
able to understand why they are used.

-- 
Regards,
Ivan Khoronzhuk

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

On 02/05/2014 10:27 PM, Thomas Gleixner wrote:
> On Wed, 5 Feb 2014, Ivan Khoronzhuk wrote:
>> +	/* here we have to be sure the timer has been disabled */
> Sigh. This is not a proper explanation for a barrier, really. You want
> to explain what it serializes against what. i.e. you want to explain
> why you are using the relaxed functions and avoid a separate non
> relaxed variant in favour of an explicit barrier.
>
>> +	__iowmb();
> The proper thing is to have an inline function key_stone_barrier() and
> a full explanation of the issue in exactly that place instead of
> handwaving comments here and there.
>
> Thanks,
>
> 	tglx

I can add new inline function like:

/**
  * keystone_timer_barrier: write memory barrier
  * use explicit barrier to avoid using readl/writel non relaxed function
  * variants, because in our case relaxed variants hide the true places
  * where barrier is needed.
  */
static inline void keystone_timer_barrier(void)
{
     __iowmb();
}

and use it where it is needed.
Are you OK with it?

And I propose to leave comments under the barriers in order to be
able to understand why they are used.

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/06/2014 01:36 AM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 12:52 PM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> On 02/05/2014 07:41 PM, Rob Herring wrote:
>>> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> wrote:
>>>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>>> wrote:
>>>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>>>> architecture devices. The timer can be configured as a general-purpose
>>>>>> 64-bit
>>>>>> timer, dual general-purpose 32-bit timers. When configured as dual
>>>>>> 32-bit
>>>>>> timers, each half can operate in conjunction (chain mode) or
>>>>>> independently
>>>>>> (unchained mode) of each other.
>>>>> This is software configurable or h/w design time configurations?
>>>>>
>>>>> Rob
>>>>>
>>>> This is h/w design time configurations
>>> Then it seems like the binding should provide for describing those
>>> differences either with a property or different compatible strings.
>>>
>>> Rob
>> Oh..sorry, seems I didn't catch, this is configurable by software.
>> These configurations are like modes in which timer can work
>> and they are not different hardware IPs. It depends on driver in
>> which mode it should work.
> In that case,
>
> Acked-by: Rob Herring <robh@kernel.org>

Thanks

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/06/2014 01:36 AM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 12:52 PM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> On 02/05/2014 07:41 PM, Rob Herring wrote:
>>> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> wrote:
>>>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>>> wrote:
>>>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>>>> architecture devices. The timer can be configured as a general-purpose
>>>>>> 64-bit
>>>>>> timer, dual general-purpose 32-bit timers. When configured as dual
>>>>>> 32-bit
>>>>>> timers, each half can operate in conjunction (chain mode) or
>>>>>> independently
>>>>>> (unchained mode) of each other.
>>>>> This is software configurable or h/w design time configurations?
>>>>>
>>>>> Rob
>>>>>
>>>> This is h/w design time configurations
>>> Then it seems like the binding should provide for describing those
>>> differences either with a property or different compatible strings.
>>>
>>> Rob
>> Oh..sorry, seems I didn't catch, this is configurable by software.
>> These configurations are like modes in which timer can work
>> and they are not different hardware IPs. It depends on driver in
>> which mode it should work.
> In that case,
>
> Acked-by: Rob Herring <robh@kernel.org>

Thanks

-- 
Regards,
Ivan Khoronzhuk

[PATCH v5 2/3] clocksource: keystone: add bindings for keystone timer

[Ivan Khoronzhuk]

On 02/06/2014 01:36 AM, Rob Herring wrote:
> On Wed, Feb 5, 2014 at 12:52 PM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> wrote:
>> On 02/05/2014 07:41 PM, Rob Herring wrote:
>>> On Wed, Feb 5, 2014 at 10:18 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> wrote:
>>>> On 02/05/2014 04:39 PM, Rob Herring wrote:
>>>>> On Wed, Feb 5, 2014 at 7:47 AM, Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>>> wrote:
>>>>>> This patch provides bindings for the 64-bit timer in the KeyStone
>>>>>> architecture devices. The timer can be configured as a general-purpose
>>>>>> 64-bit
>>>>>> timer, dual general-purpose 32-bit timers. When configured as dual
>>>>>> 32-bit
>>>>>> timers, each half can operate in conjunction (chain mode) or
>>>>>> independently
>>>>>> (unchained mode) of each other.
>>>>> This is software configurable or h/w design time configurations?
>>>>>
>>>>> Rob
>>>>>
>>>> This is h/w design time configurations
>>> Then it seems like the binding should provide for describing those
>>> differences either with a property or different compatible strings.
>>>
>>> Rob
>> Oh..sorry, seems I didn't catch, this is configurable by software.
>> These configurations are like modes in which timer can work
>> and they are not different hardware IPs. It depends on driver in
>> which mode it should work.
> In that case,
>
> Acked-by: Rob Herring <robh@kernel.org>

Thanks

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

On 02/06/2014 02:35 AM, Josh Cartwright wrote:
> Hey Ivan-
>
> On Wed, Feb 05, 2014 at 03:47:38PM +0200, Ivan Khoronzhuk wrote:
>> Add broadcast clock-event device for the Keystone arch.
>>
>> The timer can be configured as a general-purpose 64-bit timer,
>> dual general-purpose 32-bit timers. When configured as dual 32-bit
>> timers, each half can operate in conjunction (chain mode) or
>> independently (unchained mode) of each other.
>>
>> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
>> Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>>   drivers/clocksource/Makefile         |   1 +
>>   drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 234 insertions(+)
>>   create mode 100644 drivers/clocksource/timer-keystone.c
>>
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index c7ca50a..4abe5aa 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
>>   obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
>>   obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
>>   obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
>> +obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
>> diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
>> new file mode 100644
>> index 0000000..2299666
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-keystone.c
>> +static void __init keystone_timer_init(struct device_node *np)
>> +{
>> +	struct clock_event_device *event_dev = &timer.event_dev;
>> +	unsigned long rate;
>> +	struct clk *clk;
>> +	int irq, error;
>> +	u32 tgcr;
>> +
>> +	irq  = irq_of_parse_and_map(np, 0);
>> +	if (irq == NO_IRQ) {
>> +		pr_err("%s: failed to map interrupts\n", __func__);
>> +		return;
>> +	}
>> +
>> +	timer.base = of_iomap(np, 0);
>> +	if (!timer.base) {
>> +		pr_err("%s: failed to map registers\n", __func__);
>> +		return;
>> +	}
>> +
>> +	clk = of_clk_get(np, 0);
>> +	if (!clk) {
> This condition should be IS_ERR(clk).

Thanks Josh,
I'll fix it.

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

On 02/06/2014 02:35 AM, Josh Cartwright wrote:
> Hey Ivan-
>
> On Wed, Feb 05, 2014 at 03:47:38PM +0200, Ivan Khoronzhuk wrote:
>> Add broadcast clock-event device for the Keystone arch.
>>
>> The timer can be configured as a general-purpose 64-bit timer,
>> dual general-purpose 32-bit timers. When configured as dual 32-bit
>> timers, each half can operate in conjunction (chain mode) or
>> independently (unchained mode) of each other.
>>
>> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
>> Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>>   drivers/clocksource/Makefile         |   1 +
>>   drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 234 insertions(+)
>>   create mode 100644 drivers/clocksource/timer-keystone.c
>>
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index c7ca50a..4abe5aa 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
>>   obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
>>   obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
>>   obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
>> +obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
>> diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
>> new file mode 100644
>> index 0000000..2299666
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-keystone.c
>> +static void __init keystone_timer_init(struct device_node *np)
>> +{
>> +	struct clock_event_device *event_dev = &timer.event_dev;
>> +	unsigned long rate;
>> +	struct clk *clk;
>> +	int irq, error;
>> +	u32 tgcr;
>> +
>> +	irq  = irq_of_parse_and_map(np, 0);
>> +	if (irq == NO_IRQ) {
>> +		pr_err("%s: failed to map interrupts\n", __func__);
>> +		return;
>> +	}
>> +
>> +	timer.base = of_iomap(np, 0);
>> +	if (!timer.base) {
>> +		pr_err("%s: failed to map registers\n", __func__);
>> +		return;
>> +	}
>> +
>> +	clk = of_clk_get(np, 0);
>> +	if (!clk) {
> This condition should be IS_ERR(clk).

Thanks Josh,
I'll fix it.

-- 
Regards,
Ivan Khoronzhuk

[PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Ivan Khoronzhuk]

On 02/06/2014 02:35 AM, Josh Cartwright wrote:
> Hey Ivan-
>
> On Wed, Feb 05, 2014 at 03:47:38PM +0200, Ivan Khoronzhuk wrote:
>> Add broadcast clock-event device for the Keystone arch.
>>
>> The timer can be configured as a general-purpose 64-bit timer,
>> dual general-purpose 32-bit timers. When configured as dual 32-bit
>> timers, each half can operate in conjunction (chain mode) or
>> independently (unchained mode) of each other.
>>
>> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
>> Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>>   drivers/clocksource/Makefile         |   1 +
>>   drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 234 insertions(+)
>>   create mode 100644 drivers/clocksource/timer-keystone.c
>>
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index c7ca50a..4abe5aa 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
>>   obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
>>   obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
>>   obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
>> +obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
>> diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
>> new file mode 100644
>> index 0000000..2299666
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-keystone.c
>> +static void __init keystone_timer_init(struct device_node *np)
>> +{
>> +	struct clock_event_device *event_dev = &timer.event_dev;
>> +	unsigned long rate;
>> +	struct clk *clk;
>> +	int irq, error;
>> +	u32 tgcr;
>> +
>> +	irq  = irq_of_parse_and_map(np, 0);
>> +	if (irq == NO_IRQ) {
>> +		pr_err("%s: failed to map interrupts\n", __func__);
>> +		return;
>> +	}
>> +
>> +	timer.base = of_iomap(np, 0);
>> +	if (!timer.base) {
>> +		pr_err("%s: failed to map registers\n", __func__);
>> +		return;
>> +	}
>> +
>> +	clk = of_clk_get(np, 0);
>> +	if (!clk) {
> This condition should be IS_ERR(clk).

Thanks Josh,
I'll fix it.

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Thomas Gleixner]

On Thu, 6 Feb 2014, Ivan Khoronzhuk wrote:
> On 02/05/2014 10:27 PM, Thomas Gleixner wrote:
> > On Wed, 5 Feb 2014, Ivan Khoronzhuk wrote:
> > > +	/* here we have to be sure the timer has been disabled */
> > Sigh. This is not a proper explanation for a barrier, really. You want
> > to explain what it serializes against what. i.e. you want to explain
> > why you are using the relaxed functions and avoid a separate non
> > relaxed variant in favour of an explicit barrier.
> > 
> > > +	__iowmb();
> > The proper thing is to have an inline function key_stone_barrier() and
> > a full explanation of the issue in exactly that place instead of
> > handwaving comments here and there.
> > 
> > Thanks,
> > 
> > 	tglx
> 
> I can add new inline function like:
> 
> /**
>  * keystone_timer_barrier: write memory barrier
>  * use explicit barrier to avoid using readl/writel non relaxed function
>  * variants, because in our case relaxed variants hide the true places
>  * where barrier is needed.
>  */
> static inline void keystone_timer_barrier(void)
> {
>     __iowmb();
> }
> 
> and use it where it is needed.
> Are you OK with it?
> 
> And I propose to leave comments under the barriers in order to be
> able to understand why they are used.

Sounds good.

[PATCH v5 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Thomas Gleixner]

On Thu, 6 Feb 2014, Ivan Khoronzhuk wrote:
> On 02/05/2014 10:27 PM, Thomas Gleixner wrote:
> > On Wed, 5 Feb 2014, Ivan Khoronzhuk wrote:
> > > +	/* here we have to be sure the timer has been disabled */
> > Sigh. This is not a proper explanation for a barrier, really. You want
> > to explain what it serializes against what. i.e. you want to explain
> > why you are using the relaxed functions and avoid a separate non
> > relaxed variant in favour of an explicit barrier.
> > 
> > > +	__iowmb();
> > The proper thing is to have an inline function key_stone_barrier() and
> > a full explanation of the issue in exactly that place instead of
> > handwaving comments here and there.
> > 
> > Thanks,
> > 
> > 	tglx
> 
> I can add new inline function like:
> 
> /**
>  * keystone_timer_barrier: write memory barrier
>  * use explicit barrier to avoid using readl/writel non relaxed function
>  * variants, because in our case relaxed variants hide the true places
>  * where barrier is needed.
>  */
> static inline void keystone_timer_barrier(void)
> {
>     __iowmb();
> }
> 
> and use it where it is needed.
> Are you OK with it?
> 
> And I propose to leave comments under the barriers in order to be
> able to understand why they are used.

Sounds good.