diff mbox

[v2,1/2] clocksource: Add Oxford Semiconductor RPS Dual Timer

Message ID 1465315452-20599-2-git-send-email-narmstrong@baylibre.com (mailing list archive)
State New, archived
Headers show

Commit Message

Neil Armstrong June 7, 2016, 4:04 p.m. UTC
Add clocksource and clockevent driver from dual RPS timer.
The HW provides a dual one-shot or periodic 24bit timers,
the drivers set the first one as tick event source and the
second as a continuous scheduler clock source.
The timer can use 1, 16 or 256 as pre-dividers, thus the
clocksource uses 16 by default.

CC: Ma Haijun <mahaijuns@gmail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/clocksource/Kconfig           |   6 +
 drivers/clocksource/Makefile          |   1 +
 drivers/clocksource/timer-oxnas-rps.c | 270 ++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/clocksource/timer-oxnas-rps.c

Comments

Daniel Lezcano June 13, 2016, 2:35 p.m. UTC | #1
On Tue, Jun 07, 2016 at 06:04:11PM +0200, Neil Armstrong wrote:
> Add clocksource and clockevent driver from dual RPS timer.
> The HW provides a dual one-shot or periodic 24bit timers,
> the drivers set the first one as tick event source and the
> second as a continuous scheduler clock source.
> The timer can use 1, 16 or 256 as pre-dividers, thus the
> clocksource uses 16 by default.
> 
> CC: Ma Haijun <mahaijuns@gmail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/clocksource/Kconfig           |   6 +
>  drivers/clocksource/Makefile          |   1 +
>  drivers/clocksource/timer-oxnas-rps.c | 270 ++++++++++++++++++++++++++++++++++
>  3 files changed, 277 insertions(+)
>  create mode 100644 drivers/clocksource/timer-oxnas-rps.c
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 47352d2..7e382c5 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -293,6 +293,12 @@ config VF_PIT_TIMER
>  	help
>  	  Support for Period Interrupt Timer on Freescale Vybrid Family SoCs.
>  
> +config OXNAS_RPS_TIMER

   config OXNAS_RPS_TIMER "bla bla" if COMPILE_TEST

> +	bool
> +	select CLKSRC_MMIO
> +	help
> +	  This enables support for the Oxford Semiconductor OXNAS RPS timers.
> +
> @@ -0,0 +1,270 @@
> +/*
> + * drivers/clocksource/timer-oxnas-rps.c
> + *
> + * Copyright (C) 2009 Oxford Semiconductor Ltd
> + * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>

What are these two copyrights ?

[ ... ]

> +static void __init oxnas_rps_timer_init(struct device_node *np)
> +{
> +	struct oxnas_rps_timer *rps;
> +	void __iomem *base;
> +	int ret;
> +
> +	rps = kzalloc(sizeof(*rps), GFP_KERNEL);
> +	if (WARN_ON(!rps))

It is pointless to add a WARN_ON, kzalloc already does that on failure.

> +		return;
> +
> +	rps->clk = of_clk_get(np, 0);
> +	if (WARN_ON(IS_ERR(rps->clk)))
> +		return;
> +
> +	if (WARN_ON(clk_prepare_enable(rps->clk)))
> +		goto err;
> +
> +	base = of_iomap(np, 0);
> +	if (WARN_ON(!base))
> +		goto err;
> +
> +
> +	rps->irq = irq_of_parse_and_map(np, 0);
> +	if (WARN_ON(rps->irq < 0))
> +		goto err;
> +
> +	rps->clkevt_base = base + TIMER1_REG_OFFSET;
> +	rps->clksrc_base = base + TIMER2_REG_OFFSET;
> +
> +	/* Disable timers */
> +	writel_relaxed(0, rps->clkevt_base + TIMER_CTRL_REG);
> +	writel_relaxed(0, rps->clksrc_base + TIMER_CTRL_REG);
> +	writel_relaxed(0, rps->clkevt_base + TIMER_LOAD_REG);
> +	writel_relaxed(0, rps->clksrc_base + TIMER_LOAD_REG);
> +	writel_relaxed(0, rps->clkevt_base + TIMER_CLRINT_REG);
> +	writel_relaxed(0, rps->clksrc_base + TIMER_CLRINT_REG);
> +
> +	ret = request_irq(rps->irq, oxnas_rps_timer_irq,
> +			  IRQF_TIMER | IRQF_IRQPOLL,
> +			  "rps-timer", rps);
> +	if (WARN_ON(ret))
> +		goto err;
> +
> +	oxnas_rps_clockevent_init(rps);
> +	oxnas_rps_clocksource_init(rps);
> +
> +	return;

err_iounmap:
	iounmap(base);

err_clk_unprepare:
	clk_unprepare(rps->clk)

err_clk_put:
> +	clk_put(rps->clk);
> +	kfree(rps);
> +}

Regarding the current work I am doing to change the init function to return 
an error in case of failure, can you do proper error handling in this 
function and rollback ?

1. replace WARN_ON by a pr_err
2. make oxnas_rps_clockevent_init and oxnas_rps_clocksource_init to return 
an error code
3. rollback clockevent or clocksource if one fails.

Thanks !

  -- Daniel

> +
> +CLOCKSOURCE_OF_DECLARE(ox810se_rps,
> +		       "oxsemi,ox810se-rps-timer", oxnas_rps_timer_init);
> -- 
> 1.9.1
>
Neil Armstrong June 14, 2016, 9:52 a.m. UTC | #2
Hi Daniel,

On 06/13/2016 04:35 PM, Daniel Lezcano wrote:
> On Tue, Jun 07, 2016 at 06:04:11PM +0200, Neil Armstrong wrote:
>> Add clocksource and clockevent driver from dual RPS timer.
>> The HW provides a dual one-shot or periodic 24bit timers,
>> the drivers set the first one as tick event source and the
>> second as a continuous scheduler clock source.
>> The timer can use 1, 16 or 256 as pre-dividers, thus the
>> clocksource uses 16 by default.
>>
>> CC: Ma Haijun <mahaijuns@gmail.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/clocksource/Kconfig           |   6 +
>>  drivers/clocksource/Makefile          |   1 +
>>  drivers/clocksource/timer-oxnas-rps.c | 270 ++++++++++++++++++++++++++++++++++
>>  3 files changed, 277 insertions(+)
>>  create mode 100644 drivers/clocksource/timer-oxnas-rps.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index 47352d2..7e382c5 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -293,6 +293,12 @@ config VF_PIT_TIMER
>>  	help
>>  	  Support for Period Interrupt Timer on Freescale Vybrid Family SoCs.
>>  
>> +config OXNAS_RPS_TIMER
> 
>    config OXNAS_RPS_TIMER "bla bla" if COMPILE_TEST
> 
OK

Shoud I also add CLKSRC_OF ?

>> +	bool
>> +	select CLKSRC_MMIO
>> +	help
>> +	  This enables support for the Oxford Semiconductor OXNAS RPS timers.
>> +
>> @@ -0,0 +1,270 @@
>> +/*
>> + * drivers/clocksource/timer-oxnas-rps.c
>> + *
>> + * Copyright (C) 2009 Oxford Semiconductor Ltd
>> + * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
> 
> What are these two copyrights ?
> 
> [ ... ]

This driver is based from a driver from the OX820 sdk from Oxford and modified by Ma Haijun, thus the copyrights.

>> +static void __init oxnas_rps_timer_init(struct device_node *np)
>> +{
>> +	struct oxnas_rps_timer *rps;
>> +	void __iomem *base;
>> +	int ret;
>> +
>> +	rps = kzalloc(sizeof(*rps), GFP_KERNEL);
>> +	if (WARN_ON(!rps))
> 
> It is pointless to add a WARN_ON, kzalloc already does that on failure.
> 
>> +		return;
>> +
>> +	rps->clk = of_clk_get(np, 0);
>> +	if (WARN_ON(IS_ERR(rps->clk)))
>> +		return;

[....]

>> +		goto err;
>> +
>> +	oxnas_rps_clockevent_init(rps);
>> +	oxnas_rps_clocksource_init(rps);
>> +
>> +	return;
> 
> err_iounmap:
> 	iounmap(base);
> 
> err_clk_unprepare:
> 	clk_unprepare(rps->clk)
> 
> err_clk_put:
>> +	clk_put(rps->clk);
>> +	kfree(rps);
>> +}
> 
> Regarding the current work I am doing to change the init function to return 
> an error in case of failure, can you do proper error handling in this 
> function and rollback ?
> 
> 1. replace WARN_ON by a pr_err
> 2. make oxnas_rps_clockevent_init and oxnas_rps_clocksource_init to return 
> an error code
It can be done.

> 3. rollback clockevent or clocksource if one fails.
Sure, but as for 4.6, it seems neither sched_clock_register, clocksource_mmio_init or clockevents_config_and_register can be reverted !
What should I do ?

> 
> Thanks !
> 
>   -- Daniel
> 
Neil
Daniel Lezcano June 14, 2016, 11:48 a.m. UTC | #3
On 06/14/2016 11:52 AM, Neil Armstrong wrote:
> Hi Daniel,

Hi Neil,

[ ... ]

>>> +config OXNAS_RPS_TIMER
>>
>>     config OXNAS_RPS_TIMER "bla bla" if COMPILE_TEST
>>
> OK
>
> Shoud I also add CLKSRC_OF ?

Yes, right.

>>> +	bool
>>> +	select CLKSRC_MMIO
>>> +	help
>>> +	  This enables support for the Oxford Semiconductor OXNAS RPS timers.
>>> +
>>> @@ -0,0 +1,270 @@
>>> +/*
>>> + * drivers/clocksource/timer-oxnas-rps.c
>>> + *
>>> + * Copyright (C) 2009 Oxford Semiconductor Ltd
>>> + * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
>>
>> What are these two copyrights ?
>>
>> [ ... ]
>
> This driver is based from a driver from the OX820 sdk from Oxford and modified by Ma Haijun, thus the copyrights.

Ok.

>>> +static void __init oxnas_rps_timer_init(struct device_node *np)
>>> +{
>>> +	struct oxnas_rps_timer *rps;
>>> +	void __iomem *base;
>>> +	int ret;
>>> +
>>> +	rps = kzalloc(sizeof(*rps), GFP_KERNEL);
>>> +	if (WARN_ON(!rps))
>>
>> It is pointless to add a WARN_ON, kzalloc already does that on failure.
>>
>>> +		return;
>>> +
>>> +	rps->clk = of_clk_get(np, 0);
>>> +	if (WARN_ON(IS_ERR(rps->clk)))
>>> +		return;
>
> [....]
>
>>> +		goto err;
>>> +
>>> +	oxnas_rps_clockevent_init(rps);
>>> +	oxnas_rps_clocksource_init(rps);
>>> +
>>> +	return;
>>
>> err_iounmap:
>> 	iounmap(base);
>>
>> err_clk_unprepare:
>> 	clk_unprepare(rps->clk)
>>
>> err_clk_put:
>>> +	clk_put(rps->clk);
>>> +	kfree(rps);
>>> +}
>>
>> Regarding the current work I am doing to change the init function to return
>> an error in case of failure, can you do proper error handling in this
>> function and rollback ?
>>
>> 1. replace WARN_ON by a pr_err
>> 2. make oxnas_rps_clockevent_init and oxnas_rps_clocksource_init to return
>> an error code
> It can be done.
>
>> 3. rollback clockevent or clocksource if one fails.
> Sure, but as for 4.6, it seems neither sched_clock_register, clocksource_mmio_init or clockevents_config_and_register can be reverted !
> What should I do ?

Just try to do the best effort.

eg.

ret = oxnas_rps_clockevent_init(rps);
if (ret)
	goto err_...

ret = oxnas_rps_clocksource_init(rps);
if (ret)
	goto err_...

If the function themselves just return 0, it is fine for me.

I initiated a process to cleanup and factor out the clocksource / 
clockevents, so any changes being able to rollback will help in this 
process.

   -- Daniel
diff mbox

Patch

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 47352d2..7e382c5 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -293,6 +293,12 @@  config VF_PIT_TIMER
 	help
 	  Support for Period Interrupt Timer on Freescale Vybrid Family SoCs.
 
+config OXNAS_RPS_TIMER
+	bool
+	select CLKSRC_MMIO
+	help
+	  This enables support for the Oxford Semiconductor OXNAS RPS timers.
+
 config SYS_SUPPORTS_SH_CMT
         bool
 
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 473974f..bc66981 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -48,6 +48,7 @@  obj-$(CONFIG_MTK_TIMER)		+= mtk_timer.o
 obj-$(CONFIG_CLKSRC_PISTACHIO)	+= time-pistachio.o
 obj-$(CONFIG_CLKSRC_TI_32K)	+= timer-ti-32k.o
 obj-$(CONFIG_CLKSRC_NPS)	+= timer-nps.o
+obj-$(CONFIG_OXNAS_RPS_TIMER)	+= timer-oxnas-rps.o
 
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
diff --git a/drivers/clocksource/timer-oxnas-rps.c b/drivers/clocksource/timer-oxnas-rps.c
new file mode 100644
index 0000000..c9b613b
--- /dev/null
+++ b/drivers/clocksource/timer-oxnas-rps.c
@@ -0,0 +1,270 @@ 
+/*
+ * drivers/clocksource/timer-oxnas-rps.c
+ *
+ * Copyright (C) 2009 Oxford Semiconductor Ltd
+ * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
+ * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/clockchips.h>
+#include <linux/sched_clock.h>
+
+/* TIMER1 used as tick
+ * TIMER2 used as clocksource
+ */
+
+/* Registers definitions */
+
+#define TIMER_LOAD_REG		0x0
+#define TIMER_CURR_REG		0x4
+#define TIMER_CTRL_REG		0x8
+#define TIMER_CLRINT_REG	0xC
+
+#define TIMER_BITS		24
+
+#define TIMER_MAX_VAL		(BIT(TIMER_BITS) - 1)
+
+#define TIMER_PERIODIC		BIT(6)
+#define TIMER_ENABLE		BIT(7)
+
+#define TIMER_DIV1		(0)
+#define TIMER_DIV16		(1 << 2)
+#define TIMER_DIV256		(2 << 2)
+
+#define TIMER1_REG_OFFSET	0
+#define TIMER2_REG_OFFSET	0x20
+
+/* Clockevent & Clocksource data */
+
+struct oxnas_rps_timer {
+	struct clock_event_device clkevent;
+	void __iomem *clksrc_base;
+	void __iomem *clkevt_base;
+	unsigned long timer_period;
+	unsigned int timer_prescaler;
+	struct clk *clk;
+	int irq;
+};
+
+static irqreturn_t oxnas_rps_timer_irq(int irq, void *dev_id)
+{
+	struct oxnas_rps_timer *rps = dev_id;
+
+	writel_relaxed(0, rps->clkevt_base + TIMER_CLRINT_REG);
+
+	rps->clkevent.event_handler(&rps->clkevent);
+
+	return IRQ_HANDLED;
+}
+
+static void oxnas_rps_timer_config(struct oxnas_rps_timer *rps,
+				   unsigned long period,
+				   unsigned int periodic)
+{
+	uint32_t cfg = rps->timer_prescaler;
+
+	if (period)
+		cfg |= TIMER_ENABLE;
+
+	if (periodic)
+		cfg |= TIMER_PERIODIC;
+
+	writel_relaxed(period, rps->clkevt_base + TIMER_LOAD_REG);
+	writel_relaxed(cfg, rps->clkevt_base + TIMER_CTRL_REG);
+}
+
+static int oxnas_rps_timer_shutdown(struct clock_event_device *evt)
+{
+	struct oxnas_rps_timer *rps =
+		container_of(evt, struct oxnas_rps_timer, clkevent);
+
+	oxnas_rps_timer_config(rps, 0, 0);
+
+	return 0;
+}
+
+static int oxnas_rps_timer_set_periodic(struct clock_event_device *evt)
+{
+	struct oxnas_rps_timer *rps =
+		container_of(evt, struct oxnas_rps_timer, clkevent);
+
+	oxnas_rps_timer_config(rps, rps->timer_period, 1);
+
+	return 0;
+}
+
+static int oxnas_rps_timer_set_oneshot(struct clock_event_device *evt)
+{
+	struct oxnas_rps_timer *rps =
+		container_of(evt, struct oxnas_rps_timer, clkevent);
+
+	oxnas_rps_timer_config(rps, rps->timer_period, 0);
+
+	return 0;
+}
+
+static int oxnas_rps_timer_next_event(unsigned long delta,
+				struct clock_event_device *evt)
+{
+	struct oxnas_rps_timer *rps =
+		container_of(evt, struct oxnas_rps_timer, clkevent);
+
+	oxnas_rps_timer_config(rps, delta, 0);
+
+	return 0;
+}
+
+static void __init oxnas_rps_clockevent_init(struct oxnas_rps_timer *rps)
+{
+	ulong clk_rate = clk_get_rate(rps->clk);
+	ulong timer_rate;
+
+	/* Start with prescaler 1 */
+	rps->timer_prescaler = TIMER_DIV1;
+	rps->timer_period = DIV_ROUND_UP(clk_rate, HZ);
+	timer_rate = clk_rate;
+
+	if (rps->timer_period > TIMER_MAX_VAL) {
+		rps->timer_prescaler = TIMER_DIV16;
+		timer_rate = clk_rate / 16;
+		rps->timer_period = DIV_ROUND_UP(timer_rate, HZ);
+	}
+	if (rps->timer_period > TIMER_MAX_VAL) {
+		rps->timer_prescaler = TIMER_DIV256;
+		timer_rate = clk_rate / 256;
+		rps->timer_period = DIV_ROUND_UP(timer_rate, HZ);
+	}
+
+	rps->clkevent.name = "oxnas-rps";
+	rps->clkevent.features = CLOCK_EVT_FEAT_PERIODIC |
+				 CLOCK_EVT_FEAT_ONESHOT |
+				 CLOCK_EVT_FEAT_DYNIRQ;
+	rps->clkevent.tick_resume = oxnas_rps_timer_shutdown;
+	rps->clkevent.set_state_shutdown = oxnas_rps_timer_shutdown;
+	rps->clkevent.set_state_periodic = oxnas_rps_timer_set_periodic;
+	rps->clkevent.set_state_oneshot = oxnas_rps_timer_set_oneshot;
+	rps->clkevent.set_next_event = oxnas_rps_timer_next_event;
+	rps->clkevent.rating = 200;
+	rps->clkevent.cpumask = cpu_possible_mask;
+	rps->clkevent.irq = rps->irq;
+	clockevents_config_and_register(&rps->clkevent,
+					timer_rate,
+					1,
+					TIMER_MAX_VAL);
+
+	pr_info("Registered clock event rate %luHz prescaler %x period %lu\n",
+			clk_rate,
+			rps->timer_prescaler,
+			rps->timer_period);
+}
+
+/* Clocksource */
+
+static void __iomem *timer_sched_base;
+
+static u64 notrace oxnas_rps_read_sched_clock(void)
+{
+	return ~readl_relaxed(timer_sched_base);
+}
+
+static void __init oxnas_rps_clocksource_init(struct oxnas_rps_timer *rps)
+{
+	ulong clk_rate = clk_get_rate(rps->clk);
+	int ret;
+
+	/* use prescale 16 */
+	clk_rate = clk_rate / 16;
+
+	writel_relaxed(TIMER_MAX_VAL, rps->clksrc_base + TIMER_LOAD_REG);
+	writel_relaxed(TIMER_PERIODIC | TIMER_ENABLE | TIMER_DIV16,
+			rps->clksrc_base + TIMER_CTRL_REG);
+
+	timer_sched_base = rps->clksrc_base + TIMER_CURR_REG;
+	sched_clock_register(oxnas_rps_read_sched_clock,
+			     TIMER_BITS, clk_rate);
+	ret = clocksource_mmio_init(timer_sched_base,
+				    "oxnas_rps_clocksource_timer",
+				    clk_rate, 250, TIMER_BITS,
+				    clocksource_mmio_readl_down);
+	if (WARN_ON(ret))
+		pr_err("can't register clocksource\n");
+
+	pr_info("Registered clocksource rate %luHz\n", clk_rate);
+}
+
+static void __init oxnas_rps_timer_init(struct device_node *np)
+{
+	struct oxnas_rps_timer *rps;
+	void __iomem *base;
+	int ret;
+
+	rps = kzalloc(sizeof(*rps), GFP_KERNEL);
+	if (WARN_ON(!rps))
+		return;
+
+	rps->clk = of_clk_get(np, 0);
+	if (WARN_ON(IS_ERR(rps->clk)))
+		return;
+
+	if (WARN_ON(clk_prepare_enable(rps->clk)))
+		goto err;
+
+	base = of_iomap(np, 0);
+	if (WARN_ON(!base))
+		goto err;
+
+
+	rps->irq = irq_of_parse_and_map(np, 0);
+	if (WARN_ON(rps->irq < 0))
+		goto err;
+
+	rps->clkevt_base = base + TIMER1_REG_OFFSET;
+	rps->clksrc_base = base + TIMER2_REG_OFFSET;
+
+	/* Disable timers */
+	writel_relaxed(0, rps->clkevt_base + TIMER_CTRL_REG);
+	writel_relaxed(0, rps->clksrc_base + TIMER_CTRL_REG);
+	writel_relaxed(0, rps->clkevt_base + TIMER_LOAD_REG);
+	writel_relaxed(0, rps->clksrc_base + TIMER_LOAD_REG);
+	writel_relaxed(0, rps->clkevt_base + TIMER_CLRINT_REG);
+	writel_relaxed(0, rps->clksrc_base + TIMER_CLRINT_REG);
+
+	ret = request_irq(rps->irq, oxnas_rps_timer_irq,
+			  IRQF_TIMER | IRQF_IRQPOLL,
+			  "rps-timer", rps);
+	if (WARN_ON(ret))
+		goto err;
+
+	oxnas_rps_clockevent_init(rps);
+	oxnas_rps_clocksource_init(rps);
+
+	return;
+err:
+	clk_put(rps->clk);
+	kfree(rps);
+}
+
+CLOCKSOURCE_OF_DECLARE(ox810se_rps,
+		       "oxsemi,ox810se-rps-timer", oxnas_rps_timer_init);