diff mbox

[v5,2/2] clocksource: driver for Conexant Digicolor SoC timer

Message ID ad84f7155286c45401ff7f6bc6506407cc3beb61.1422272518.git.baruch@tkos.co.il (mailing list archive)
State New, archived
Headers show

Commit Message

Baruch Siach Jan. 26, 2015, 11:57 a.m. UTC
Add clocksource driver to the Conexant CX92755 SoC, part of the Digicolor SoCs
series. Hardware provides 8 timers, A to H. Timer A is dedicated to a future
watchdog driver so we don't use it here. Use timer B for sched_clock, and timer
C for clock_event.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/clocksource/Makefile          |   1 +
 drivers/clocksource/timer-digicolor.c | 199 ++++++++++++++++++++++++++++++++++
 2 files changed, 200 insertions(+)
 create mode 100644 drivers/clocksource/timer-digicolor.c

Comments

Daniel Lezcano Jan. 26, 2015, 1:44 p.m. UTC | #1
On 01/26/2015 12:57 PM, Baruch Siach wrote:
> Add clocksource driver to the Conexant CX92755 SoC, part of the Digicolor SoCs
> series. Hardware provides 8 timers, A to H. Timer A is dedicated to a future
> watchdog driver so we don't use it here. Use timer B for sched_clock, and timer
> C for clock_event.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   drivers/clocksource/Makefile          |   1 +
>   drivers/clocksource/timer-digicolor.c | 199 ++++++++++++++++++++++++++++++++++
>   2 files changed, 200 insertions(+)
>   create mode 100644 drivers/clocksource/timer-digicolor.c
>
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 94d90b24b56b..a993c108be67 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -10,6 +10,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_ARCH_DIGICOLOR)	+= timer-digicolor.o

Ah, one minor change I forgot to mention in the last review (sorry about 
that).

Don't depend on the ARCH, add and select a TIMER_DIGICOLOR (or whatever 
the name you prefer) in the mach-digicolor/Kconfig and use it here.

Other than that, it looks good for me.

Thanks

   -- Daniel

>   obj-$(CONFIG_DW_APB_TIMER)	+= dw_apb_timer.o
>   obj-$(CONFIG_DW_APB_TIMER_OF)	+= dw_apb_timer_of.o
>   obj-$(CONFIG_CLKSRC_NOMADIK_MTU)	+= nomadik-mtu.o
> diff --git a/drivers/clocksource/timer-digicolor.c b/drivers/clocksource/timer-digicolor.c
> new file mode 100644
> index 000000000000..7f8388cfa810
> --- /dev/null
> +++ b/drivers/clocksource/timer-digicolor.c
> @@ -0,0 +1,199 @@
> +/*
> + * Conexant Digicolor timer driver
> + *
> + * Author: Baruch Siach <baruch@tkos.co.il>
> + *
> + * Copyright (C) 2014 Paradox Innovation Ltd.
> + *
> + * Based on:
> + *	Allwinner SoCs hstimer driver
> + *
> + * Copyright (C) 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * 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.
> + */
> +
> +/*
> + * Conexant Digicolor SoCs have 8 configurable timers, named from "Timer A" to
> + * "Timer H". Timer A is the only one with watchdog support, so it is dedicated
> + * to the watchdog driver. This driver uses Timer B for sched_clock(), and
> + * Timer C for clockevents.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqreturn.h>
> +#include <linux/sched_clock.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +enum {
> +	TIMER_A,
> +	TIMER_B,
> +	TIMER_C,
> +	TIMER_D,
> +	TIMER_E,
> +	TIMER_F,
> +	TIMER_G,
> +	TIMER_H,
> +};
> +
> +#define CONTROL(t)	((t)*8)
> +#define COUNT(t)	((t)*8 + 4)
> +
> +#define CONTROL_DISABLE		0
> +#define CONTROL_ENABLE		BIT(0)
> +#define CONTROL_MODE(m)		((m) << 4)
> +#define CONTROL_MODE_ONESHOT	CONTROL_MODE(1)
> +#define CONTROL_MODE_PERIODIC	CONTROL_MODE(2)
> +
> +struct digicolor_timer {
> +	struct clock_event_device ce;
> +	void __iomem *base;
> +	u32 ticks_per_jiffy;
> +	int timer_id; /* one of TIMER_* */
> +};
> +
> +struct digicolor_timer *dc_timer(struct clock_event_device *ce)
> +{
> +	return container_of(ce, struct digicolor_timer, ce);
> +}
> +
> +static inline void dc_timer_disable(struct clock_event_device *ce)
> +{
> +	struct digicolor_timer *dt = dc_timer(ce);
> +	writeb(CONTROL_DISABLE, dt->base + CONTROL(dt->timer_id));
> +}
> +
> +static inline void dc_timer_enable(struct clock_event_device *ce, u32 mode)
> +{
> +	struct digicolor_timer *dt = dc_timer(ce);
> +	writeb(CONTROL_ENABLE | mode, dt->base + CONTROL(dt->timer_id));
> +}
> +
> +static inline void dc_timer_set_count(struct clock_event_device *ce,
> +				      unsigned long count)
> +{
> +	struct digicolor_timer *dt = dc_timer(ce);
> +	writel(count, dt->base + COUNT(dt->timer_id));
> +}
> +
> +static void digicolor_clkevt_mode(enum clock_event_mode mode,
> +				  struct clock_event_device *ce)
> +{
> +	struct digicolor_timer *dt = dc_timer(ce);
> +
> +	switch (mode) {
> +	case CLOCK_EVT_MODE_PERIODIC:
> +		dc_timer_disable(ce);
> +		dc_timer_set_count(ce, dt->ticks_per_jiffy);
> +		dc_timer_enable(ce, CONTROL_MODE_PERIODIC);
> +		break;
> +	case CLOCK_EVT_MODE_ONESHOT:
> +		dc_timer_disable(ce);
> +		dc_timer_enable(ce, CONTROL_MODE_ONESHOT);
> +		break;
> +	case CLOCK_EVT_MODE_UNUSED:
> +	case CLOCK_EVT_MODE_SHUTDOWN:
> +	default:
> +		dc_timer_disable(ce);
> +		break;
> +	}
> +}
> +
> +static int digicolor_clkevt_next_event(unsigned long evt,
> +				       struct clock_event_device *ce)
> +{
> +	dc_timer_disable(ce);
> +	dc_timer_set_count(ce, evt);
> +	dc_timer_enable(ce, CONTROL_MODE_ONESHOT);
> +
> +	return 0;
> +}
> +
> +static struct digicolor_timer dc_timer_dev = {
> +	.ce = {
> +		.name = "digicolor_tick",
> +		.rating = 340,
> +		.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> +		.set_mode = digicolor_clkevt_mode,
> +		.set_next_event = digicolor_clkevt_next_event,
> +	},
> +	.timer_id = TIMER_C,
> +};
> +
> +static irqreturn_t digicolor_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct clock_event_device *evt = dev_id;
> +
> +	evt->event_handler(evt);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static u64 digicolor_timer_sched_read(void)
> +{
> +	return ~readl(dc_timer_dev.base + COUNT(TIMER_B));
> +}
> +
> +static void __init digicolor_timer_init(struct device_node *node)
> +{
> +	unsigned long rate;
> +	struct clk *clk;
> +	int ret, irq;
> +
> +	/*
> +	 * timer registers are shared with the watchdog timer;
> +	 * don't map exclusively
> +	 */
> +	dc_timer_dev.base = of_iomap(node, 0);
> +	if (!dc_timer_dev.base) {
> +		pr_err("Can't map registers");
> +		return;
> +	}
> +
> +	irq = irq_of_parse_and_map(node, dc_timer_dev.timer_id);
> +	if (irq <= 0) {
> +		pr_err("Can't parse IRQ");
> +		return;
> +	}
> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("Can't get timer clock");
> +		return;
> +	}
> +	clk_prepare_enable(clk);
> +	rate = clk_get_rate(clk);
> +	dc_timer_dev.ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
> +
> +	writeb(CONTROL_DISABLE, dc_timer_dev.base + CONTROL(TIMER_B));
> +	writel(UINT_MAX, dc_timer_dev.base + COUNT(TIMER_B));
> +	writeb(CONTROL_ENABLE, dc_timer_dev.base + CONTROL(TIMER_B));
> +
> +	sched_clock_register(digicolor_timer_sched_read, 32, rate);
> +	clocksource_mmio_init(dc_timer_dev.base + COUNT(TIMER_B), node->name,
> +			      rate, 340, 32, clocksource_mmio_readl_down);
> +
> +	ret = request_irq(irq, digicolor_timer_interrupt,
> +			  IRQF_TIMER | IRQF_IRQPOLL, "digicolor_timerC",
> +			  &dc_timer_dev.ce);
> +	if (ret)
> +		pr_warn("request of timer irq %d failed (%d)\n", irq, ret);
> +
> +	dc_timer_dev.ce.cpumask = cpu_possible_mask;
> +	dc_timer_dev.ce.irq = irq;
> +
> +	clockevents_config_and_register(&dc_timer_dev.ce, rate, 0, 0xffffffff);
> +}
> +CLOCKSOURCE_OF_DECLARE(conexant_digicolor, "cnxt,cx92755-timer",
> +		       digicolor_timer_init);
>
Baruch Siach Jan. 26, 2015, 1:47 p.m. UTC | #2
Hi Daniel,

On Mon, Jan 26, 2015 at 02:44:15PM +0100, Daniel Lezcano wrote:
> >diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> >index 94d90b24b56b..a993c108be67 100644
> >--- a/drivers/clocksource/Makefile
> >+++ b/drivers/clocksource/Makefile
> >@@ -10,6 +10,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_ARCH_DIGICOLOR)	+= timer-digicolor.o
> 
> Ah, one minor change I forgot to mention in the last review (sorry about
> that).
> 
> Don't depend on the ARCH, add and select a TIMER_DIGICOLOR (or whatever the
> name you prefer) in the mach-digicolor/Kconfig and use it here.

OK. I'll make the change. I'm just curious: what is the advantage of having a 
separate config symbol for the timer?

baruch
Daniel Lezcano Jan. 26, 2015, 1:58 p.m. UTC | #3
On 01/26/2015 02:47 PM, Baruch Siach wrote:
> Hi Daniel,
>
> On Mon, Jan 26, 2015 at 02:44:15PM +0100, Daniel Lezcano wrote:
>>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>>> index 94d90b24b56b..a993c108be67 100644
>>> --- a/drivers/clocksource/Makefile
>>> +++ b/drivers/clocksource/Makefile
>>> @@ -10,6 +10,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_ARCH_DIGICOLOR)	+= timer-digicolor.o
>>
>> Ah, one minor change I forgot to mention in the last review (sorry about
>> that).
>>
>> Don't depend on the ARCH, add and select a TIMER_DIGICOLOR (or whatever the
>> name you prefer) in the mach-digicolor/Kconfig and use it here.
>
> OK. I'll make the change. I'm just curious: what is the advantage of having a
> separate config symbol for the timer?

It is because some timers could be used in different platforms, so the 
policy of the house is to let the platform Kconfig to select the timer.

Also, that let one platform to not select the timer if it is not needed. 
For example one SoC uses the architected timers and there is not power 
management on the board, hence no cpu is powered down and no backup 
timer is needed.

Even if the timer is expected to be used on only one platform, we try to 
keep the rule consistent across the different drivers (even if I admit 
some cleanups should be done in the Kconfig file).
diff mbox

Patch

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 94d90b24b56b..a993c108be67 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -10,6 +10,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_ARCH_DIGICOLOR)	+= timer-digicolor.o
 obj-$(CONFIG_DW_APB_TIMER)	+= dw_apb_timer.o
 obj-$(CONFIG_DW_APB_TIMER_OF)	+= dw_apb_timer_of.o
 obj-$(CONFIG_CLKSRC_NOMADIK_MTU)	+= nomadik-mtu.o
diff --git a/drivers/clocksource/timer-digicolor.c b/drivers/clocksource/timer-digicolor.c
new file mode 100644
index 000000000000..7f8388cfa810
--- /dev/null
+++ b/drivers/clocksource/timer-digicolor.c
@@ -0,0 +1,199 @@ 
+/*
+ * Conexant Digicolor timer driver
+ *
+ * Author: Baruch Siach <baruch@tkos.co.il>
+ *
+ * Copyright (C) 2014 Paradox Innovation Ltd.
+ *
+ * Based on:
+ *	Allwinner SoCs hstimer driver
+ *
+ * Copyright (C) 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * 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.
+ */
+
+/*
+ * Conexant Digicolor SoCs have 8 configurable timers, named from "Timer A" to
+ * "Timer H". Timer A is the only one with watchdog support, so it is dedicated
+ * to the watchdog driver. This driver uses Timer B for sched_clock(), and
+ * Timer C for clockevents.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqreturn.h>
+#include <linux/sched_clock.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+enum {
+	TIMER_A,
+	TIMER_B,
+	TIMER_C,
+	TIMER_D,
+	TIMER_E,
+	TIMER_F,
+	TIMER_G,
+	TIMER_H,
+};
+
+#define CONTROL(t)	((t)*8)
+#define COUNT(t)	((t)*8 + 4)
+
+#define CONTROL_DISABLE		0
+#define CONTROL_ENABLE		BIT(0)
+#define CONTROL_MODE(m)		((m) << 4)
+#define CONTROL_MODE_ONESHOT	CONTROL_MODE(1)
+#define CONTROL_MODE_PERIODIC	CONTROL_MODE(2)
+
+struct digicolor_timer {
+	struct clock_event_device ce;
+	void __iomem *base;
+	u32 ticks_per_jiffy;
+	int timer_id; /* one of TIMER_* */
+};
+
+struct digicolor_timer *dc_timer(struct clock_event_device *ce)
+{
+	return container_of(ce, struct digicolor_timer, ce);
+}
+
+static inline void dc_timer_disable(struct clock_event_device *ce)
+{
+	struct digicolor_timer *dt = dc_timer(ce);
+	writeb(CONTROL_DISABLE, dt->base + CONTROL(dt->timer_id));
+}
+
+static inline void dc_timer_enable(struct clock_event_device *ce, u32 mode)
+{
+	struct digicolor_timer *dt = dc_timer(ce);
+	writeb(CONTROL_ENABLE | mode, dt->base + CONTROL(dt->timer_id));
+}
+
+static inline void dc_timer_set_count(struct clock_event_device *ce,
+				      unsigned long count)
+{
+	struct digicolor_timer *dt = dc_timer(ce);
+	writel(count, dt->base + COUNT(dt->timer_id));
+}
+
+static void digicolor_clkevt_mode(enum clock_event_mode mode,
+				  struct clock_event_device *ce)
+{
+	struct digicolor_timer *dt = dc_timer(ce);
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		dc_timer_disable(ce);
+		dc_timer_set_count(ce, dt->ticks_per_jiffy);
+		dc_timer_enable(ce, CONTROL_MODE_PERIODIC);
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+		dc_timer_disable(ce);
+		dc_timer_enable(ce, CONTROL_MODE_ONESHOT);
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	default:
+		dc_timer_disable(ce);
+		break;
+	}
+}
+
+static int digicolor_clkevt_next_event(unsigned long evt,
+				       struct clock_event_device *ce)
+{
+	dc_timer_disable(ce);
+	dc_timer_set_count(ce, evt);
+	dc_timer_enable(ce, CONTROL_MODE_ONESHOT);
+
+	return 0;
+}
+
+static struct digicolor_timer dc_timer_dev = {
+	.ce = {
+		.name = "digicolor_tick",
+		.rating = 340,
+		.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+		.set_mode = digicolor_clkevt_mode,
+		.set_next_event = digicolor_clkevt_next_event,
+	},
+	.timer_id = TIMER_C,
+};
+
+static irqreturn_t digicolor_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static u64 digicolor_timer_sched_read(void)
+{
+	return ~readl(dc_timer_dev.base + COUNT(TIMER_B));
+}
+
+static void __init digicolor_timer_init(struct device_node *node)
+{
+	unsigned long rate;
+	struct clk *clk;
+	int ret, irq;
+
+	/*
+	 * timer registers are shared with the watchdog timer;
+	 * don't map exclusively
+	 */
+	dc_timer_dev.base = of_iomap(node, 0);
+	if (!dc_timer_dev.base) {
+		pr_err("Can't map registers");
+		return;
+	}
+
+	irq = irq_of_parse_and_map(node, dc_timer_dev.timer_id);
+	if (irq <= 0) {
+		pr_err("Can't parse IRQ");
+		return;
+	}
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("Can't get timer clock");
+		return;
+	}
+	clk_prepare_enable(clk);
+	rate = clk_get_rate(clk);
+	dc_timer_dev.ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
+
+	writeb(CONTROL_DISABLE, dc_timer_dev.base + CONTROL(TIMER_B));
+	writel(UINT_MAX, dc_timer_dev.base + COUNT(TIMER_B));
+	writeb(CONTROL_ENABLE, dc_timer_dev.base + CONTROL(TIMER_B));
+
+	sched_clock_register(digicolor_timer_sched_read, 32, rate);
+	clocksource_mmio_init(dc_timer_dev.base + COUNT(TIMER_B), node->name,
+			      rate, 340, 32, clocksource_mmio_readl_down);
+
+	ret = request_irq(irq, digicolor_timer_interrupt,
+			  IRQF_TIMER | IRQF_IRQPOLL, "digicolor_timerC",
+			  &dc_timer_dev.ce);
+	if (ret)
+		pr_warn("request of timer irq %d failed (%d)\n", irq, ret);
+
+	dc_timer_dev.ce.cpumask = cpu_possible_mask;
+	dc_timer_dev.ce.irq = irq;
+
+	clockevents_config_and_register(&dc_timer_dev.ce, rate, 0, 0xffffffff);
+}
+CLOCKSOURCE_OF_DECLARE(conexant_digicolor, "cnxt,cx92755-timer",
+		       digicolor_timer_init);