[PATCH v2 01/10] arm: zynq: Use standard timer binding
Steffen Trumtrar
s.trumtrar at pengutronix.de
Tue Mar 26 16:14:01 EDT 2013
On Tue, Mar 26, 2013 at 06:43:33PM +0100, Michal Simek wrote:
> Use cdns,ttc because this driver is Cadence Rev06
> Triple Timer Counter and everybody can use it
> without xilinx specific function name or probing.
>
> Also use standard dt description for timer
> and also prepare for moving to clocksource
> initialization.
>
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
> v2: Update year in copyright
> Fix typo fault
> Remove all zynq specific names
>
> Steffen: I have done this change based on our discussion.
> Moving to generic location will be done in the next patch.
>
> Josh: We have talked about this change at ELC.
> Using standard dt binding as other timers.
>
> I have also discussed this with Rob some time ago.
> https://patchwork.kernel.org/patch/2112791/
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 45 +------
> arch/arm/boot/dts/zynq-zc702.dts | 10 --
> arch/arm/mach-zynq/common.c | 1 +
> arch/arm/mach-zynq/timer.c | 261 +++++++++++++++++++++++++++-----------
> 4 files changed, 195 insertions(+), 122 deletions(-)
>
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index 5914b56..51243db 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -111,56 +111,23 @@
> };
>
> ttc0: ttc0 at f8001000 {
> - #address-cells = <1>;
> - #size-cells = <0>;
> - compatible = "xlnx,ttc";
> + interrupt-parent = <&intc>;
> + interrupts = < 0 10 4 0 11 4 0 12 4 >;
> + compatible = "cdns,ttc";
> reg = <0xF8001000 0x1000>;
> clocks = <&cpu_clk 3>;
> clock-names = "cpu_1x";
> clock-ranges;
> -
> - ttc0_0: ttc0.0 {
> - status = "disabled";
> - reg = <0>;
> - interrupts = <0 10 4>;
> - };
> - ttc0_1: ttc0.1 {
> - status = "disabled";
> - reg = <1>;
> - interrupts = <0 11 4>;
> - };
> - ttc0_2: ttc0.2 {
> - status = "disabled";
> - reg = <2>;
> - interrupts = <0 12 4>;
> - };
> };
>
> ttc1: ttc1 at f8002000 {
> - #interrupt-parent = <&intc>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> - compatible = "xlnx,ttc";
> + interrupt-parent = <&intc>;
> + interrupts = < 0 37 4 0 38 4 0 39 4 >;
> + compatible = "cdns,ttc";
> reg = <0xF8002000 0x1000>;
> clocks = <&cpu_clk 3>;
> clock-names = "cpu_1x";
> clock-ranges;
> -
> - ttc1_0: ttc1.0 {
> - status = "disabled";
> - reg = <0>;
> - interrupts = <0 37 4>;
> - };
> - ttc1_1: ttc1.1 {
> - status = "disabled";
> - reg = <1>;
> - interrupts = <0 38 4>;
> - };
> - ttc1_2: ttc1.2 {
> - status = "disabled";
> - reg = <2>;
> - interrupts = <0 39 4>;
> - };
> };
> };
> };
Hi Michal!
Thought about this a little more. I'm not seeing the improvment this gives us.
The ttcs give us 6 counters that could be used however one likes. Linux wants
a clocksource and clockevent provider, but I could use the Cortex timer as
clocksource and would only have to sacrifice one of the 6 counters.
With this patch I have to sacrifice 3 counters and would only use 2 of them.
Then there is pinmuxing. That can be handled by one driver, so I think that is
doable with both versions and I think I'm okay with that.
So what am I missing? Why would I want it like this and not the way it is right
now?
Regards,
Steffen
> diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
> index c772942..86f44d5 100644
> --- a/arch/arm/boot/dts/zynq-zc702.dts
> +++ b/arch/arm/boot/dts/zynq-zc702.dts
> @@ -32,13 +32,3 @@
> &ps_clk {
> clock-frequency = <33333330>;
> };
> -
> -&ttc0_0 {
> - status = "ok";
> - compatible = "xlnx,ttc-counter-clocksource";
> -};
> -
> -&ttc0_1 {
> - status = "ok";
> - compatible = "xlnx,ttc-counter-clockevent";
> -};
> diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> index 5c89832..76493b0 100644
> --- a/arch/arm/mach-zynq/common.c
> +++ b/arch/arm/mach-zynq/common.c
> @@ -20,6 +20,7 @@
> #include <linux/platform_device.h>
> #include <linux/clk.h>
> #include <linux/clk/zynq.h>
> +#include <linux/clocksource.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> diff --git a/arch/arm/mach-zynq/timer.c b/arch/arm/mach-zynq/timer.c
> index f9fbc9c..82357d9 100644
> --- a/arch/arm/mach-zynq/timer.c
> +++ b/arch/arm/mach-zynq/timer.c
> @@ -1,7 +1,7 @@
> /*
> * This file contains driver for the Xilinx PS Timer Counter IP.
> *
> - * Copyright (C) 2011 Xilinx
> + * Copyright (C) 2011-2013 Xilinx
> *
> * based on arch/mips/kernel/time.c timer driver
> *
> @@ -15,6 +15,7 @@
> * GNU General Public License for more details.
> */
>
> +#include <linux/clk.h>
> #include <linux/interrupt.h>
> #include <linux/clockchips.h>
> #include <linux/of_address.h>
> @@ -24,6 +25,21 @@
> #include "common.h"
>
> /*
> + * This driver configures the 2 16-bit count-up timers as follows:
> + *
> + * T1: Timer 1, clocksource for generic timekeeping
> + * T2: Timer 2, clockevent source for hrtimers
> + * T3: Timer 3, <unused>
> + *
> + * The input frequency to the timer module for emulation is 2.5MHz which is
> + * common to all the timer channels (T1, T2, and T3). With a pre-scaler of 32,
> + * the timers are clocked at 78.125KHz (12.8 us resolution).
> +
> + * The input frequency to the timer module in silicon is configurable and
> + * obtained from device tree. The pre-scaler of 32 is used.
> + */
> +
> +/*
> * Timer Register Offset Definitions of Timer 1, Increment base address by 4
> * and use same offsets for Timer 2
> */
> @@ -44,17 +60,24 @@
> #define PRESCALE 2048 /* The exponent must match this */
> #define CLK_CNTRL_PRESCALE ((PRESCALE_EXPONENT - 1) << 1)
> #define CLK_CNTRL_PRESCALE_EN 1
> -#define CNT_CNTRL_RESET (1<<4)
> +#define CNT_CNTRL_RESET (1 << 4)
>
> /**
> * struct xttcps_timer - This definition defines local timer structure
> *
> * @base_addr: Base address of timer
> - **/
> + * @clk: Associated clock source
> + * @clk_rate_change_nb Notifier block for clock rate changes
> + */
> struct xttcps_timer {
> - void __iomem *base_addr;
> + void __iomem *base_addr;
> + struct clk *clk;
> + struct notifier_block clk_rate_change_nb;
> };
>
> +#define to_xttcps_timer(x) \
> + container_of(x, struct xttcps_timer, clk_rate_change_nb)
> +
> struct xttcps_timer_clocksource {
> struct xttcps_timer xttc;
> struct clocksource cs;
> @@ -66,7 +89,6 @@ struct xttcps_timer_clocksource {
> struct xttcps_timer_clockevent {
> struct xttcps_timer xttc;
> struct clock_event_device ce;
> - struct clk *clk;
> };
>
> #define to_xttcps_timer_clkevent(x) \
> @@ -167,8 +189,8 @@ static void xttcps_set_mode(enum clock_event_mode mode,
> switch (mode) {
> case CLOCK_EVT_MODE_PERIODIC:
> xttcps_set_interval(timer,
> - DIV_ROUND_CLOSEST(clk_get_rate(xttce->clk),
> - PRESCALE * HZ));
> + DIV_ROUND_CLOSEST(clk_get_rate(xttce->xttc.clk),
> + PRESCALE * HZ));
> break;
> case CLOCK_EVT_MODE_ONESHOT:
> case CLOCK_EVT_MODE_UNUSED:
> @@ -189,79 +211,148 @@ static void xttcps_set_mode(enum clock_event_mode mode,
> }
> }
>
> -static void __init zynq_ttc_setup_clocksource(struct device_node *np,
> - void __iomem *base)
> +static int xttcps_rate_change_clocksource_cb(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct clk_notifier_data *ndata = data;
> + struct xttcps_timer *xttcps = to_xttcps_timer(nb);
> + struct xttcps_timer_clocksource *xttccs = container_of(xttcps,
> + struct xttcps_timer_clocksource, xttc);
> +
> + switch (event) {
> + case POST_RATE_CHANGE:
> + /*
> + * Do whatever is necessary to maintain a proper time base
> + *
> + * I cannot find a way to adjust the currently used clocksource
> + * to the new frequency. __clocksource_updatefreq_hz() sounds
> + * good, but does not work. Not sure what's that missing.
> + *
> + * This approach works, but triggers two clocksource switches.
> + * The first after unregister to clocksource jiffies. And
> + * another one after the register to the newly registered timer.
> + *
> + * Alternatively we could 'waste' another HW timer to ping pong
> + * between clock sources. That would also use one register and
> + * one unregister call, but only trigger one clocksource switch
> + * for the cost of another HW timer used by the OS.
> + */
> + clocksource_unregister(&xttccs->cs);
> + clocksource_register_hz(&xttccs->cs,
> + ndata->new_rate / PRESCALE);
> + /* fall through */
> + case PRE_RATE_CHANGE:
> + case ABORT_RATE_CHANGE:
> + default:
> + return NOTIFY_DONE;
> + }
> +}
> +
> +static void __init xttc_setup_clocksource(struct clk *clk, void __iomem *base)
> {
> struct xttcps_timer_clocksource *ttccs;
> - struct clk *clk;
> int err;
> - u32 reg;
>
> ttccs = kzalloc(sizeof(*ttccs), GFP_KERNEL);
> if (WARN_ON(!ttccs))
> return;
>
> - err = of_property_read_u32(np, "reg", ®);
> - if (WARN_ON(err))
> - return;
> + ttccs->xttc.clk = clk;
>
> - clk = of_clk_get_by_name(np, "cpu_1x");
> - if (WARN_ON(IS_ERR(clk)))
> - return;
> -
> - err = clk_prepare_enable(clk);
> + err = clk_prepare_enable(ttccs->xttc.clk);
> if (WARN_ON(err))
> return;
>
> - ttccs->xttc.base_addr = base + reg * 4;
> + ttccs->xttc.clk_rate_change_nb.notifier_call =
> + xttcps_rate_change_clocksource_cb;
> + ttccs->xttc.clk_rate_change_nb.next = NULL;
> + if (clk_notifier_register(ttccs->xttc.clk,
> + &ttccs->xttc.clk_rate_change_nb))
> + pr_warn("Unable to register clock notifier.\n");
>
> - ttccs->cs.name = np->name;
> + ttccs->xttc.base_addr = base;
> + ttccs->cs.name = "xttcps_clocksource";
> ttccs->cs.rating = 200;
> ttccs->cs.read = __xttc_clocksource_read;
> ttccs->cs.mask = CLOCKSOURCE_MASK(16);
> ttccs->cs.flags = CLOCK_SOURCE_IS_CONTINUOUS;
>
> + /*
> + * Setup the clock source counter to be an incrementing counter
> + * with no interrupt and it rolls over at 0xFFFF. Pre-scale
> + * it by 32 also. Let it start running now.
> + */
> __raw_writel(0x0, ttccs->xttc.base_addr + XTTCPS_IER_OFFSET);
> __raw_writel(CLK_CNTRL_PRESCALE | CLK_CNTRL_PRESCALE_EN,
> ttccs->xttc.base_addr + XTTCPS_CLK_CNTRL_OFFSET);
> __raw_writel(CNT_CNTRL_RESET,
> ttccs->xttc.base_addr + XTTCPS_CNT_CNTRL_OFFSET);
>
> - err = clocksource_register_hz(&ttccs->cs, clk_get_rate(clk) / PRESCALE);
> + err = clocksource_register_hz(&ttccs->cs,
> + clk_get_rate(ttccs->xttc.clk) / PRESCALE);
> if (WARN_ON(err))
> return;
> +
> }
>
> -static void __init zynq_ttc_setup_clockevent(struct device_node *np,
> - void __iomem *base)
> +static int xttcps_rate_change_clockevent_cb(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct clk_notifier_data *ndata = data;
> + struct xttcps_timer *xttcps = to_xttcps_timer(nb);
> + struct xttcps_timer_clockevent *xttcce = container_of(xttcps,
> + struct xttcps_timer_clockevent, xttc);
> +
> + switch (event) {
> + case POST_RATE_CHANGE:
> + {
> + unsigned long flags;
> +
> + /*
> + * clockevents_update_freq should be called with IRQ disabled on
> + * the CPU the timer provides events for. The timer we use is
> + * common to both CPUs, not sure if we need to run on both
> + * cores.
> + */
> + local_irq_save(flags);
> + clockevents_update_freq(&xttcce->ce,
> + ndata->new_rate / PRESCALE);
> + local_irq_restore(flags);
> +
> + /* fall through */
> + }
> + case PRE_RATE_CHANGE:
> + case ABORT_RATE_CHANGE:
> + default:
> + return NOTIFY_DONE;
> + }
> +}
> +
> +static void __init xttc_setup_clockevent(struct clk *clk,
> + void __iomem *base, u32 irq)
> {
> struct xttcps_timer_clockevent *ttcce;
> - int err, irq;
> - u32 reg;
> + int err;
>
> ttcce = kzalloc(sizeof(*ttcce), GFP_KERNEL);
> if (WARN_ON(!ttcce))
> return;
>
> - err = of_property_read_u32(np, "reg", ®);
> - if (WARN_ON(err))
> - return;
> + ttcce->xttc.clk = clk;
>
> - ttcce->xttc.base_addr = base + reg * 4;
> -
> - ttcce->clk = of_clk_get_by_name(np, "cpu_1x");
> - if (WARN_ON(IS_ERR(ttcce->clk)))
> - return;
> -
> - err = clk_prepare_enable(ttcce->clk);
> + err = clk_prepare_enable(ttcce->xttc.clk);
> if (WARN_ON(err))
> return;
>
> - irq = irq_of_parse_and_map(np, 0);
> - if (WARN_ON(!irq))
> - return;
> + ttcce->xttc.clk_rate_change_nb.notifier_call =
> + xttcps_rate_change_clockevent_cb;
> + ttcce->xttc.clk_rate_change_nb.next = NULL;
> + if (clk_notifier_register(ttcce->xttc.clk,
> + &ttcce->xttc.clk_rate_change_nb))
> + pr_warn("Unable to register clock notifier.\n");
>
> - ttcce->ce.name = np->name;
> + ttcce->xttc.base_addr = base;
> + ttcce->ce.name = "xttcps_clockevent";
> ttcce->ce.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> ttcce->ce.set_next_event = xttcps_set_next_event;
> ttcce->ce.set_mode = xttcps_set_mode;
> @@ -269,56 +360,80 @@ static void __init zynq_ttc_setup_clockevent(struct device_node *np,
> ttcce->ce.irq = irq;
> ttcce->ce.cpumask = cpu_possible_mask;
>
> + /*
> + * Setup the clock event timer to be an interval timer which
> + * is prescaled by 32 using the interval interrupt. Leave it
> + * disabled for now.
> + */
> __raw_writel(0x23, ttcce->xttc.base_addr + XTTCPS_CNT_CNTRL_OFFSET);
> __raw_writel(CLK_CNTRL_PRESCALE | CLK_CNTRL_PRESCALE_EN,
> ttcce->xttc.base_addr + XTTCPS_CLK_CNTRL_OFFSET);
> __raw_writel(0x1, ttcce->xttc.base_addr + XTTCPS_IER_OFFSET);
>
> - err = request_irq(irq, xttcps_clock_event_interrupt, IRQF_TIMER,
> - np->name, ttcce);
> + err = request_irq(irq, xttcps_clock_event_interrupt,
> + IRQF_DISABLED | IRQF_TIMER,
> + ttcce->ce.name, ttcce);
> if (WARN_ON(err))
> return;
>
> clockevents_config_and_register(&ttcce->ce,
> - clk_get_rate(ttcce->clk) / PRESCALE,
> - 1, 0xfffe);
> + clk_get_rate(ttcce->xttc.clk) / PRESCALE, 1, 0xfffe);
> }
>
> -static const __initconst struct of_device_id zynq_ttc_match[] = {
> - { .compatible = "xlnx,ttc-counter-clocksource",
> - .data = zynq_ttc_setup_clocksource, },
> - { .compatible = "xlnx,ttc-counter-clockevent",
> - .data = zynq_ttc_setup_clockevent, },
> - {}
> -};
> -
> /**
> * xttcps_timer_init - Initialize the timer
> *
> * Initializes the timer hardware and register the clock source and clock event
> * timers with Linux kernal timer framework
> - **/
> + */
> +static void __init xttcps_timer_init_of(struct device_node *timer)
> +{
> + unsigned int irq;
> + void __iomem *timer_baseaddr;
> + struct clk *clk;
> +
> + /*
> + * Get the 1st Triple Timer Counter (TTC) block from the device tree
> + * and use it. Note that the event timer uses the interrupt and it's the
> + * 2nd TTC hence the irq_of_parse_and_map(,1)
> + */
> + timer_baseaddr = of_iomap(timer, 0);
> + if (!timer_baseaddr) {
> + pr_err("ERROR: invalid timer base address\n");
> + BUG();
> + }
> +
> + irq = irq_of_parse_and_map(timer, 1);
> + if (irq <= 0) {
> + pr_err("ERROR: invalid interrupt number\n");
> + BUG();
> + }
> +
> + clk = of_clk_get_by_name(timer, "cpu_1x");
> + if (IS_ERR(clk)) {
> + pr_err("ERROR: timer input clock not found\n");
> + BUG();
> + }
> +
> + xttc_setup_clocksource(clk, timer_baseaddr);
> + xttc_setup_clockevent(clk, timer_baseaddr + 4, irq);
> +
> + pr_info("%s #0 at %p, irq=%d\n", timer->name, timer_baseaddr, irq);
> +}
> +
> void __init xttcps_timer_init(void)
> {
> - struct device_node *np;
> -
> - for_each_compatible_node(np, NULL, "xlnx,ttc") {
> - struct device_node *np_chld;
> - void __iomem *base;
> -
> - base = of_iomap(np, 0);
> - if (WARN_ON(!base))
> - return;
> -
> - for_each_available_child_of_node(np, np_chld) {
> - int (*cb)(struct device_node *np, void __iomem *base);
> - const struct of_device_id *match;
> -
> - match = of_match_node(zynq_ttc_match, np_chld);
> - if (match) {
> - cb = match->data;
> - cb(np_chld, base);
> - }
> - }
> + const char * const timer_list[] = {
> + "cdns,ttc",
> + NULL
> + };
> + struct device_node *timer;
> +
> + timer = of_find_compatible_node(NULL, NULL, timer_list[0]);
> + if (!timer) {
> + pr_err("ERROR: no compatible timer found\n");
> + BUG();
> }
> +
> + xttcps_timer_init_of(timer);
> }
> --
> 1.7.9.7
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
More information about the linux-arm-kernel
mailing list