* [PATCH v2 01/10] arm: zynq: Use standard timer binding
2013-03-26 17:43 Zynq core changes v2 Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 20:14 ` Steffen Trumtrar
2013-03-26 17:43 ` [PATCH v2 02/10] arm: zynq: Move timer to clocksource interface Michal Simek
` (8 subsequent siblings)
9 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
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@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>;
- };
};
};
};
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@%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
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 01/10] arm: zynq: Use standard timer binding
2013-03-26 17:43 ` [PATCH v2 01/10] arm: zynq: Use standard timer binding Michal Simek
@ 2013-03-26 20:14 ` Steffen Trumtrar
2013-03-27 7:40 ` Michal Simek
0 siblings, 1 reply; 42+ messages in thread
From: Steffen Trumtrar @ 2013-03-26 20:14 UTC (permalink / raw)
To: linux-arm-kernel
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@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 |
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 01/10] arm: zynq: Use standard timer binding
2013-03-26 20:14 ` Steffen Trumtrar
@ 2013-03-27 7:40 ` Michal Simek
2013-03-27 9:04 ` Steffen Trumtrar
0 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-27 7:40 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/26 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> 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@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?
There is a big move because previous DT description was incorrect.
Device-tree should describe hardware and there is no something like
ttc-counter-clocksource or ttc-counter-clockevent compatible driver.
Every ttc counter can be used as clocksource or clockevent.
Changing/Setting compatible properties for linux purpose is not correct.
Just read this thread about.
https://patchwork.kernel.org/patch/2112791/
Utilization of the core or using different timers in the system for clock
is different discussion. We can just try to utilize timers in the arm core
or add bunch of them to PL to see how kernel behaves.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 01/10] arm: zynq: Use standard timer binding
2013-03-27 7:40 ` Michal Simek
@ 2013-03-27 9:04 ` Steffen Trumtrar
2013-03-27 9:25 ` Michal Simek
0 siblings, 1 reply; 42+ messages in thread
From: Steffen Trumtrar @ 2013-03-27 9:04 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 27, 2013 at 08:40:36AM +0100, Michal Simek wrote:
> 2013/3/26 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> > 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@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?
>
> There is a big move because previous DT description was incorrect.
> Device-tree should describe hardware and there is no something like
> ttc-counter-clocksource or ttc-counter-clockevent compatible driver.
> Every ttc counter can be used as clocksource or clockevent.
> Changing/Setting compatible properties for linux purpose is not correct.
>
That is correct. DT describes the HW and the TTC includes three counters that
could be used in any way a HW vendor likes. How would you decide that with
just the driver?
The compatible property is uncommon and might even be wrong. How about adding
for example a mode-property like usb does for otg,peripheral,host?
Suppose I have some obscure board with a Zynq on it, that has ttc1_0 setup to
count some external signal and ttc1_1 counts some signal from the PL. I mean
it is possible to configure the SoC in that way, no?!
I would than have a board-specific dts where I would somehow have to describe
this setup and mark the different ttcs accordingly.
What I'm going for is, what is wrong in having the subnodes in ttc and
then iterate over these subnodes in the driver and registering the first two
ttcs that I find and that support being used as clocksource/event?
> Just read this thread about.
> https://patchwork.kernel.org/patch/2112791/
>
Already did.
> Utilization of the core or using different timers in the system for clock
> is different discussion. We can just try to utilize timers in the arm core
> or add bunch of them to PL to see how kernel behaves.
>
No problem there. I use the smp_twd on the ZedBoard and the kernel just does the
right thing.
Regards,
Steffen
--
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 |
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 01/10] arm: zynq: Use standard timer binding
2013-03-27 9:04 ` Steffen Trumtrar
@ 2013-03-27 9:25 ` Michal Simek
2013-03-27 9:37 ` Steffen Trumtrar
0 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-27 9:25 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/27 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> On Wed, Mar 27, 2013 at 08:40:36AM +0100, Michal Simek wrote:
>> 2013/3/26 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>> > 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@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?
>>
>> There is a big move because previous DT description was incorrect.
>> Device-tree should describe hardware and there is no something like
>> ttc-counter-clocksource or ttc-counter-clockevent compatible driver.
>> Every ttc counter can be used as clocksource or clockevent.
>> Changing/Setting compatible properties for linux purpose is not correct.
>>
>
> That is correct. DT describes the HW and the TTC includes three counters that
> could be used in any way a HW vendor likes. How would you decide that with
> just the driver?
> The compatible property is uncommon and might even be wrong. How about adding
> for example a mode-property like usb does for otg,peripheral,host?
I don't think that this is right thing to do.
Rob: What do you think?
> Suppose I have some obscure board with a Zynq on it, that has ttc1_0 setup to
> count some external signal and ttc1_1 counts some signal from the PL. I mean
> it is possible to configure the SoC in that way, no?!
If you are on ttc1 then it should be fine because ttc0 should be probed first
and ttc1 is not allocated but look below.
> I would than have a board-specific dts where I would somehow have to describe
> this setup and mark the different ttcs accordingly.
I understand your setup and I am not aware if there is any standard
way how to handles
other timers in the system to be able to work with them via standard api.
What do you use? UIO for ttc setup and reading it back?
> What I'm going for is, what is wrong in having the subnodes in ttc and
> then iterate over these subnodes in the driver and registering the first two
> ttcs that I find and that support being used as clocksource/event?
I understand your concern but the question is how to describe this in
dts in a proper way.
Compatible property - no
Based on aliases order - possible but based on my discussion with Rob
probably no
Additional mode options - it is also suggesting Linux specific usage.
What next?
>
>> Just read this thread about.
>> https://patchwork.kernel.org/patch/2112791/
>>
>
> Already did.
>
>> Utilization of the core or using different timers in the system for clock
>> is different discussion. We can just try to utilize timers in the arm core
>> or add bunch of them to PL to see how kernel behaves.
>>
>
> No problem there. I use the smp_twd on the ZedBoard and the kernel just does the
> right thing.
ok.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 01/10] arm: zynq: Use standard timer binding
2013-03-27 9:25 ` Michal Simek
@ 2013-03-27 9:37 ` Steffen Trumtrar
2013-03-27 9:54 ` Michal Simek
0 siblings, 1 reply; 42+ messages in thread
From: Steffen Trumtrar @ 2013-03-27 9:37 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 27, 2013 at 10:25:30AM +0100, Michal Simek wrote:
> 2013/3/27 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> > On Wed, Mar 27, 2013 at 08:40:36AM +0100, Michal Simek wrote:
> >> 2013/3/26 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> >> > 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@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?
> >>
> >> There is a big move because previous DT description was incorrect.
> >> Device-tree should describe hardware and there is no something like
> >> ttc-counter-clocksource or ttc-counter-clockevent compatible driver.
> >> Every ttc counter can be used as clocksource or clockevent.
> >> Changing/Setting compatible properties for linux purpose is not correct.
> >>
> >
> > That is correct. DT describes the HW and the TTC includes three counters that
> > could be used in any way a HW vendor likes. How would you decide that with
> > just the driver?
> > The compatible property is uncommon and might even be wrong. How about adding
> > for example a mode-property like usb does for otg,peripheral,host?
>
> I don't think that this is right thing to do.
> Rob: What do you think?
>
> > Suppose I have some obscure board with a Zynq on it, that has ttc1_0 setup to
> > count some external signal and ttc1_1 counts some signal from the PL. I mean
> > it is possible to configure the SoC in that way, no?!
>
> If you are on ttc1 then it should be fine because ttc0 should be probed first
> and ttc1 is not allocated but look below.
>
> > I would than have a board-specific dts where I would somehow have to describe
> > this setup and mark the different ttcs accordingly.
>
> I understand your setup and I am not aware if there is any standard
> way how to handles
> other timers in the system to be able to work with them via standard api.
> What do you use? UIO for ttc setup and reading it back?
>
Oh, I don't have this setup. I'm just imagining things and how we could then
handle this setup, if it ever occurs. When this binding is fixed it is fixed.
At the moment it might be okay to rework this, as current mainline only works
under certain circumstances and there is probably no one really using it as of yet.
> > What I'm going for is, what is wrong in having the subnodes in ttc and
> > then iterate over these subnodes in the driver and registering the first two
> > ttcs that I find and that support being used as clocksource/event?
>
> I understand your concern but the question is how to describe this in
> dts in a proper way.
> Compatible property - no
Agreed.
> Based on aliases order - possible but based on my discussion with Rob
> probably no
Agreed.
> Additional mode options - it is also suggesting Linux specific usage.
>
Hm, it would specify the capabilities of the ttc. Other users of the DT
might be interested in this info, too. I'm not saying it is the best/only
solution.
> What next?
>
Regards,
Steffen
--
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 |
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 01/10] arm: zynq: Use standard timer binding
2013-03-27 9:37 ` Steffen Trumtrar
@ 2013-03-27 9:54 ` Michal Simek
0 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-27 9:54 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/27 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> On Wed, Mar 27, 2013 at 10:25:30AM +0100, Michal Simek wrote:
>> 2013/3/27 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>> > On Wed, Mar 27, 2013 at 08:40:36AM +0100, Michal Simek wrote:
>> >> 2013/3/26 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>> >> > 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@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?
>> >>
>> >> There is a big move because previous DT description was incorrect.
>> >> Device-tree should describe hardware and there is no something like
>> >> ttc-counter-clocksource or ttc-counter-clockevent compatible driver.
>> >> Every ttc counter can be used as clocksource or clockevent.
>> >> Changing/Setting compatible properties for linux purpose is not correct.
>> >>
>> >
>> > That is correct. DT describes the HW and the TTC includes three counters that
>> > could be used in any way a HW vendor likes. How would you decide that with
>> > just the driver?
>> > The compatible property is uncommon and might even be wrong. How about adding
>> > for example a mode-property like usb does for otg,peripheral,host?
>>
>> I don't think that this is right thing to do.
>> Rob: What do you think?
>>
>> > Suppose I have some obscure board with a Zynq on it, that has ttc1_0 setup to
>> > count some external signal and ttc1_1 counts some signal from the PL. I mean
>> > it is possible to configure the SoC in that way, no?!
>>
>> If you are on ttc1 then it should be fine because ttc0 should be probed first
>> and ttc1 is not allocated but look below.
>>
>> > I would than have a board-specific dts where I would somehow have to describe
>> > this setup and mark the different ttcs accordingly.
>>
>> I understand your setup and I am not aware if there is any standard
>> way how to handles
>> other timers in the system to be able to work with them via standard api.
>> What do you use? UIO for ttc setup and reading it back?
>>
>
> Oh, I don't have this setup. I'm just imagining things and how we could then
> handle this setup, if it ever occurs. When this binding is fixed it is fixed.
> At the moment it might be okay to rework this, as current mainline only works
> under certain circumstances and there is probably no one really using it as of yet.
>
>> > What I'm going for is, what is wrong in having the subnodes in ttc and
>> > then iterate over these subnodes in the driver and registering the first two
>> > ttcs that I find and that support being used as clocksource/event?
>>
>> I understand your concern but the question is how to describe this in
>> dts in a proper way.
>> Compatible property - no
>
> Agreed.
>
>> Based on aliases order - possible but based on my discussion with Rob
>> probably no
>
> Agreed.
>
>> Additional mode options - it is also suggesting Linux specific usage.
>>
>
> Hm, it would specify the capabilities of the ttc. Other users of the DT
> might be interested in this info, too. I'm not saying it is the best/only
> solution.
But the capabilities for all 2 ttc (6 timers) will be the same because
they are the same
and all of them can do the same things.
I am definitely willing to find out any generic solution and the reason
for this patch is that the current dt description is not correct and based
on my discussion with Rob this was the solution which others are also using.
In the valid case you have described above there are some things which can
be easy to handle on zynq - if you know that the first two timers are used
by linux because of the code then changing input to 3rd timer your hw design
should be possible (not sure if ttc can handle input but in generic
case definitely yes).
But I can imagine case where the system has some limitations and for this
purpose you have to use the first counter and currently you have no option
how to describe this in the device-tree.
It means there should be any user input to the system to say - not to
use this timer
for Linux purpose. And DTS is only one user input to the kernel that's why
it should be written there.
Describe this in chosen node?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 02/10] arm: zynq: Move timer to clocksource interface
2013-03-26 17:43 Zynq core changes v2 Michal Simek
2013-03-26 17:43 ` [PATCH v2 01/10] arm: zynq: Use standard timer binding Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 03/10] arm: zynq: Move timer to generic location Michal Simek
` (7 subsequent siblings)
9 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Use clocksource timer initialization.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
v2: Free allocated space for structures
Till this time this was only one timer driver which
could be possible to use on zynq.
---
arch/arm/mach-zynq/common.c | 2 +-
arch/arm/mach-zynq/common.h | 2 --
arch/arm/mach-zynq/timer.c | 43 ++++++++++++++++++++-----------------------
3 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 76493b0..68e0907 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -78,7 +78,7 @@ static void __init xilinx_zynq_timer_init(void)
xilinx_zynq_clocks_init(slcr);
- xttcps_timer_init();
+ clocksource_of_init();
}
/**
diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
index 8b4dbba..5050bb1 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/arch/arm/mach-zynq/common.h
@@ -17,6 +17,4 @@
#ifndef __MACH_ZYNQ_COMMON_H__
#define __MACH_ZYNQ_COMMON_H__
-void __init xttcps_timer_init(void);
-
#endif
diff --git a/arch/arm/mach-zynq/timer.c b/arch/arm/mach-zynq/timer.c
index 82357d9..ab5b839 100644
--- a/arch/arm/mach-zynq/timer.c
+++ b/arch/arm/mach-zynq/timer.c
@@ -22,7 +22,6 @@
#include <linux/of_irq.h>
#include <linux/slab.h>
#include <linux/clk-provider.h>
-#include "common.h"
/*
* This driver configures the 2 16-bit count-up timers as follows:
@@ -260,8 +259,10 @@ static void __init xttc_setup_clocksource(struct clk *clk, void __iomem *base)
ttccs->xttc.clk = clk;
err = clk_prepare_enable(ttccs->xttc.clk);
- if (WARN_ON(err))
+ if (WARN_ON(err)) {
+ kfree(ttccs);
return;
+ }
ttccs->xttc.clk_rate_change_nb.notifier_call =
xttcps_rate_change_clocksource_cb;
@@ -290,9 +291,10 @@ static void __init xttc_setup_clocksource(struct clk *clk, void __iomem *base)
err = clocksource_register_hz(&ttccs->cs,
clk_get_rate(ttccs->xttc.clk) / PRESCALE);
- if (WARN_ON(err))
+ if (WARN_ON(err)) {
+ kfree(ttccs);
return;
-
+ }
}
static int xttcps_rate_change_clockevent_cb(struct notifier_block *nb,
@@ -341,8 +343,10 @@ static void __init xttc_setup_clockevent(struct clk *clk,
ttcce->xttc.clk = clk;
err = clk_prepare_enable(ttcce->xttc.clk);
- if (WARN_ON(err))
+ if (WARN_ON(err)) {
+ kfree(ttcce);
return;
+ }
ttcce->xttc.clk_rate_change_nb.notifier_call =
xttcps_rate_change_clockevent_cb;
@@ -373,8 +377,10 @@ static void __init xttc_setup_clockevent(struct clk *clk,
err = request_irq(irq, xttcps_clock_event_interrupt,
IRQF_DISABLED | IRQF_TIMER,
ttcce->ce.name, ttcce);
- if (WARN_ON(err))
+ if (WARN_ON(err)) {
+ kfree(ttcce);
return;
+ }
clockevents_config_and_register(&ttcce->ce,
clk_get_rate(ttcce->xttc.clk) / PRESCALE, 1, 0xfffe);
@@ -386,11 +392,17 @@ static void __init xttc_setup_clockevent(struct clk *clk,
* 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)
+static void __init xttcps_timer_init(struct device_node *timer)
{
unsigned int irq;
void __iomem *timer_baseaddr;
struct clk *clk;
+ static int initialized;
+
+ if (initialized)
+ return;
+
+ initialized = 1;
/*
* Get the 1st Triple Timer Counter (TTC) block from the device tree
@@ -421,19 +433,4 @@ static void __init xttcps_timer_init_of(struct device_node *timer)
pr_info("%s #0 at %p, irq=%d\n", timer->name, timer_baseaddr, irq);
}
-void __init xttcps_timer_init(void)
-{
- 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);
-}
+CLOCKSOURCE_OF_DECLARE(ttc, "cdns,ttc", xttcps_timer_init);
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 03/10] arm: zynq: Move timer to generic location
2013-03-26 17:43 Zynq core changes v2 Michal Simek
2013-03-26 17:43 ` [PATCH v2 01/10] arm: zynq: Use standard timer binding Michal Simek
2013-03-26 17:43 ` [PATCH v2 02/10] arm: zynq: Move timer to clocksource interface Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time Michal Simek
` (6 subsequent siblings)
9 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Move zynq timer out of mach folder to generic location
and enable it.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
v2: Add binding documentation
Rename zynq_timer.c to cadence_ttc_timer.c
Use CADENCE_TTC_TIMER to be able to enable this driver for others
Arnd: This is the last patch as we discuss about moving
this timer to generic location.
---
.../bindings/timer/cadence,ttc-timer.txt | 17 +
arch/arm/mach-zynq/Kconfig | 1 +
arch/arm/mach-zynq/Makefile | 2 +-
arch/arm/mach-zynq/timer.c | 436 --------------------
drivers/clocksource/Kconfig | 3 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/cadence_ttc_timer.c | 436 ++++++++++++++++++++
7 files changed, 459 insertions(+), 437 deletions(-)
create mode 100644 Documentation/devicetree/bindings/timer/cadence,ttc-timer.txt
delete mode 100644 arch/arm/mach-zynq/timer.c
create mode 100644 drivers/clocksource/cadence_ttc_timer.c
diff --git a/Documentation/devicetree/bindings/timer/cadence,ttc-timer.txt b/Documentation/devicetree/bindings/timer/cadence,ttc-timer.txt
new file mode 100644
index 0000000..993695c
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/cadence,ttc-timer.txt
@@ -0,0 +1,17 @@
+Cadence TTC - Triple Timer Counter
+
+Required properties:
+- compatible : Should be "cdns,ttc".
+- reg : Specifies base physical address and size of the registers.
+- interrupts : A list of 3 interrupts; one per timer channel.
+- clocks: phandle to the source clock
+
+Example:
+
+ttc0: ttc0 at f8001000 {
+ interrupt-parent = <&intc>;
+ interrupts = < 0 10 4 0 11 4 0 12 4 >;
+ compatible = "cdns,ttc";
+ reg = <0xF8001000 0x1000>;
+ clocks = <&cpu_clk 3>;
+};
diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
index adb6c0e..d70651e 100644
--- a/arch/arm/mach-zynq/Kconfig
+++ b/arch/arm/mach-zynq/Kconfig
@@ -9,5 +9,6 @@ config ARCH_ZYNQ
select MIGHT_HAVE_CACHE_L2X0
select USE_OF
select SPARSE_IRQ
+ select CADENCE_TTC_TIMER
help
Support for Xilinx Zynq ARM Cortex A9 Platform
diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
index 397268c..320faed 100644
--- a/arch/arm/mach-zynq/Makefile
+++ b/arch/arm/mach-zynq/Makefile
@@ -3,4 +3,4 @@
#
# Common support
-obj-y := common.o timer.o
+obj-y := common.o
diff --git a/arch/arm/mach-zynq/timer.c b/arch/arm/mach-zynq/timer.c
deleted file mode 100644
index ab5b839..0000000
--- a/arch/arm/mach-zynq/timer.c
+++ /dev/null
@@ -1,436 +0,0 @@
-/*
- * This file contains driver for the Xilinx PS Timer Counter IP.
- *
- * Copyright (C) 2011-2013 Xilinx
- *
- * based on arch/mips/kernel/time.c timer driver
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that 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.
- */
-
-#include <linux/clk.h>
-#include <linux/interrupt.h>
-#include <linux/clockchips.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
-#include <linux/slab.h>
-#include <linux/clk-provider.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
- */
-#define XTTCPS_CLK_CNTRL_OFFSET 0x00 /* Clock Control Reg, RW */
-#define XTTCPS_CNT_CNTRL_OFFSET 0x0C /* Counter Control Reg, RW */
-#define XTTCPS_COUNT_VAL_OFFSET 0x18 /* Counter Value Reg, RO */
-#define XTTCPS_INTR_VAL_OFFSET 0x24 /* Interval Count Reg, RW */
-#define XTTCPS_ISR_OFFSET 0x54 /* Interrupt Status Reg, RO */
-#define XTTCPS_IER_OFFSET 0x60 /* Interrupt Enable Reg, RW */
-
-#define XTTCPS_CNT_CNTRL_DISABLE_MASK 0x1
-
-/*
- * Setup the timers to use pre-scaling, using a fixed value for now that will
- * work across most input frequency, but it may need to be more dynamic
- */
-#define PRESCALE_EXPONENT 11 /* 2 ^ PRESCALE_EXPONENT = PRESCALE */
-#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)
-
-/**
- * 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;
- 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;
-};
-
-#define to_xttcps_timer_clksrc(x) \
- container_of(x, struct xttcps_timer_clocksource, cs)
-
-struct xttcps_timer_clockevent {
- struct xttcps_timer xttc;
- struct clock_event_device ce;
-};
-
-#define to_xttcps_timer_clkevent(x) \
- container_of(x, struct xttcps_timer_clockevent, ce)
-
-/**
- * xttcps_set_interval - Set the timer interval value
- *
- * @timer: Pointer to the timer instance
- * @cycles: Timer interval ticks
- **/
-static void xttcps_set_interval(struct xttcps_timer *timer,
- unsigned long cycles)
-{
- u32 ctrl_reg;
-
- /* Disable the counter, set the counter value and re-enable counter */
- ctrl_reg = __raw_readl(timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
- ctrl_reg |= XTTCPS_CNT_CNTRL_DISABLE_MASK;
- __raw_writel(ctrl_reg, timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
-
- __raw_writel(cycles, timer->base_addr + XTTCPS_INTR_VAL_OFFSET);
-
- /*
- * Reset the counter (0x10) so that it starts from 0, one-shot
- * mode makes this needed for timing to be right.
- */
- ctrl_reg |= CNT_CNTRL_RESET;
- ctrl_reg &= ~XTTCPS_CNT_CNTRL_DISABLE_MASK;
- __raw_writel(ctrl_reg, timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
-}
-
-/**
- * xttcps_clock_event_interrupt - Clock event timer interrupt handler
- *
- * @irq: IRQ number of the Timer
- * @dev_id: void pointer to the xttcps_timer instance
- *
- * returns: Always IRQ_HANDLED - success
- **/
-static irqreturn_t xttcps_clock_event_interrupt(int irq, void *dev_id)
-{
- struct xttcps_timer_clockevent *xttce = dev_id;
- struct xttcps_timer *timer = &xttce->xttc;
-
- /* Acknowledge the interrupt and call event handler */
- __raw_readl(timer->base_addr + XTTCPS_ISR_OFFSET);
-
- xttce->ce.event_handler(&xttce->ce);
-
- return IRQ_HANDLED;
-}
-
-/**
- * __xttc_clocksource_read - Reads the timer counter register
- *
- * returns: Current timer counter register value
- **/
-static cycle_t __xttc_clocksource_read(struct clocksource *cs)
-{
- struct xttcps_timer *timer = &to_xttcps_timer_clksrc(cs)->xttc;
-
- return (cycle_t)__raw_readl(timer->base_addr +
- XTTCPS_COUNT_VAL_OFFSET);
-}
-
-/**
- * xttcps_set_next_event - Sets the time interval for next event
- *
- * @cycles: Timer interval ticks
- * @evt: Address of clock event instance
- *
- * returns: Always 0 - success
- **/
-static int xttcps_set_next_event(unsigned long cycles,
- struct clock_event_device *evt)
-{
- struct xttcps_timer_clockevent *xttce = to_xttcps_timer_clkevent(evt);
- struct xttcps_timer *timer = &xttce->xttc;
-
- xttcps_set_interval(timer, cycles);
- return 0;
-}
-
-/**
- * xttcps_set_mode - Sets the mode of timer
- *
- * @mode: Mode to be set
- * @evt: Address of clock event instance
- **/
-static void xttcps_set_mode(enum clock_event_mode mode,
- struct clock_event_device *evt)
-{
- struct xttcps_timer_clockevent *xttce = to_xttcps_timer_clkevent(evt);
- struct xttcps_timer *timer = &xttce->xttc;
- u32 ctrl_reg;
-
- switch (mode) {
- case CLOCK_EVT_MODE_PERIODIC:
- xttcps_set_interval(timer,
- DIV_ROUND_CLOSEST(clk_get_rate(xttce->xttc.clk),
- PRESCALE * HZ));
- break;
- case CLOCK_EVT_MODE_ONESHOT:
- case CLOCK_EVT_MODE_UNUSED:
- case CLOCK_EVT_MODE_SHUTDOWN:
- ctrl_reg = __raw_readl(timer->base_addr +
- XTTCPS_CNT_CNTRL_OFFSET);
- ctrl_reg |= XTTCPS_CNT_CNTRL_DISABLE_MASK;
- __raw_writel(ctrl_reg,
- timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
- break;
- case CLOCK_EVT_MODE_RESUME:
- ctrl_reg = __raw_readl(timer->base_addr +
- XTTCPS_CNT_CNTRL_OFFSET);
- ctrl_reg &= ~XTTCPS_CNT_CNTRL_DISABLE_MASK;
- __raw_writel(ctrl_reg,
- timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
- break;
- }
-}
-
-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;
- int err;
-
- ttccs = kzalloc(sizeof(*ttccs), GFP_KERNEL);
- if (WARN_ON(!ttccs))
- return;
-
- ttccs->xttc.clk = clk;
-
- err = clk_prepare_enable(ttccs->xttc.clk);
- if (WARN_ON(err)) {
- kfree(ttccs);
- return;
- }
-
- 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->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(ttccs->xttc.clk) / PRESCALE);
- if (WARN_ON(err)) {
- kfree(ttccs);
- return;
- }
-}
-
-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;
-
- ttcce = kzalloc(sizeof(*ttcce), GFP_KERNEL);
- if (WARN_ON(!ttcce))
- return;
-
- ttcce->xttc.clk = clk;
-
- err = clk_prepare_enable(ttcce->xttc.clk);
- if (WARN_ON(err)) {
- kfree(ttcce);
- 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->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;
- ttcce->ce.rating = 200;
- 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_DISABLED | IRQF_TIMER,
- ttcce->ce.name, ttcce);
- if (WARN_ON(err)) {
- kfree(ttcce);
- return;
- }
-
- clockevents_config_and_register(&ttcce->ce,
- clk_get_rate(ttcce->xttc.clk) / PRESCALE, 1, 0xfffe);
-}
-
-/**
- * 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(struct device_node *timer)
-{
- unsigned int irq;
- void __iomem *timer_baseaddr;
- struct clk *clk;
- static int initialized;
-
- if (initialized)
- return;
-
- initialized = 1;
-
- /*
- * 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@%p, irq=%d\n", timer->name, timer_baseaddr, irq);
-}
-
-CLOCKSOURCE_OF_DECLARE(ttc, "cdns,ttc", xttcps_timer_init);
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index e507ab7..3167fda 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -31,6 +31,9 @@ config SUNXI_TIMER
config VT8500_TIMER
bool
+config CADENCE_TTC_TIMER
+ bool
+
config CLKSRC_NOMADIK_MTU
bool
depends on (ARCH_NOMADIK || ARCH_U8500)
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 4d8283a..e74c8ce 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_ARCH_BCM2835) += bcm2835_timer.o
obj-$(CONFIG_SUNXI_TIMER) += sunxi_timer.o
obj-$(CONFIG_ARCH_TEGRA) += tegra20_timer.o
obj-$(CONFIG_VT8500_TIMER) += vt8500_timer.o
+obj-$(CONFIG_CADENCE_TTC_TIMER) += cadence_ttc_timer.o
obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o
obj-$(CONFIG_CLKSRC_METAG_GENERIC) += metag_generic.o
diff --git a/drivers/clocksource/cadence_ttc_timer.c b/drivers/clocksource/cadence_ttc_timer.c
new file mode 100644
index 0000000..ab5b839
--- /dev/null
+++ b/drivers/clocksource/cadence_ttc_timer.c
@@ -0,0 +1,436 @@
+/*
+ * This file contains driver for the Xilinx PS Timer Counter IP.
+ *
+ * Copyright (C) 2011-2013 Xilinx
+ *
+ * based on arch/mips/kernel/time.c timer driver
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/clockchips.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+#include <linux/clk-provider.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
+ */
+#define XTTCPS_CLK_CNTRL_OFFSET 0x00 /* Clock Control Reg, RW */
+#define XTTCPS_CNT_CNTRL_OFFSET 0x0C /* Counter Control Reg, RW */
+#define XTTCPS_COUNT_VAL_OFFSET 0x18 /* Counter Value Reg, RO */
+#define XTTCPS_INTR_VAL_OFFSET 0x24 /* Interval Count Reg, RW */
+#define XTTCPS_ISR_OFFSET 0x54 /* Interrupt Status Reg, RO */
+#define XTTCPS_IER_OFFSET 0x60 /* Interrupt Enable Reg, RW */
+
+#define XTTCPS_CNT_CNTRL_DISABLE_MASK 0x1
+
+/*
+ * Setup the timers to use pre-scaling, using a fixed value for now that will
+ * work across most input frequency, but it may need to be more dynamic
+ */
+#define PRESCALE_EXPONENT 11 /* 2 ^ PRESCALE_EXPONENT = PRESCALE */
+#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)
+
+/**
+ * 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;
+ 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;
+};
+
+#define to_xttcps_timer_clksrc(x) \
+ container_of(x, struct xttcps_timer_clocksource, cs)
+
+struct xttcps_timer_clockevent {
+ struct xttcps_timer xttc;
+ struct clock_event_device ce;
+};
+
+#define to_xttcps_timer_clkevent(x) \
+ container_of(x, struct xttcps_timer_clockevent, ce)
+
+/**
+ * xttcps_set_interval - Set the timer interval value
+ *
+ * @timer: Pointer to the timer instance
+ * @cycles: Timer interval ticks
+ **/
+static void xttcps_set_interval(struct xttcps_timer *timer,
+ unsigned long cycles)
+{
+ u32 ctrl_reg;
+
+ /* Disable the counter, set the counter value and re-enable counter */
+ ctrl_reg = __raw_readl(timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
+ ctrl_reg |= XTTCPS_CNT_CNTRL_DISABLE_MASK;
+ __raw_writel(ctrl_reg, timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
+
+ __raw_writel(cycles, timer->base_addr + XTTCPS_INTR_VAL_OFFSET);
+
+ /*
+ * Reset the counter (0x10) so that it starts from 0, one-shot
+ * mode makes this needed for timing to be right.
+ */
+ ctrl_reg |= CNT_CNTRL_RESET;
+ ctrl_reg &= ~XTTCPS_CNT_CNTRL_DISABLE_MASK;
+ __raw_writel(ctrl_reg, timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
+}
+
+/**
+ * xttcps_clock_event_interrupt - Clock event timer interrupt handler
+ *
+ * @irq: IRQ number of the Timer
+ * @dev_id: void pointer to the xttcps_timer instance
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcps_clock_event_interrupt(int irq, void *dev_id)
+{
+ struct xttcps_timer_clockevent *xttce = dev_id;
+ struct xttcps_timer *timer = &xttce->xttc;
+
+ /* Acknowledge the interrupt and call event handler */
+ __raw_readl(timer->base_addr + XTTCPS_ISR_OFFSET);
+
+ xttce->ce.event_handler(&xttce->ce);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * __xttc_clocksource_read - Reads the timer counter register
+ *
+ * returns: Current timer counter register value
+ **/
+static cycle_t __xttc_clocksource_read(struct clocksource *cs)
+{
+ struct xttcps_timer *timer = &to_xttcps_timer_clksrc(cs)->xttc;
+
+ return (cycle_t)__raw_readl(timer->base_addr +
+ XTTCPS_COUNT_VAL_OFFSET);
+}
+
+/**
+ * xttcps_set_next_event - Sets the time interval for next event
+ *
+ * @cycles: Timer interval ticks
+ * @evt: Address of clock event instance
+ *
+ * returns: Always 0 - success
+ **/
+static int xttcps_set_next_event(unsigned long cycles,
+ struct clock_event_device *evt)
+{
+ struct xttcps_timer_clockevent *xttce = to_xttcps_timer_clkevent(evt);
+ struct xttcps_timer *timer = &xttce->xttc;
+
+ xttcps_set_interval(timer, cycles);
+ return 0;
+}
+
+/**
+ * xttcps_set_mode - Sets the mode of timer
+ *
+ * @mode: Mode to be set
+ * @evt: Address of clock event instance
+ **/
+static void xttcps_set_mode(enum clock_event_mode mode,
+ struct clock_event_device *evt)
+{
+ struct xttcps_timer_clockevent *xttce = to_xttcps_timer_clkevent(evt);
+ struct xttcps_timer *timer = &xttce->xttc;
+ u32 ctrl_reg;
+
+ switch (mode) {
+ case CLOCK_EVT_MODE_PERIODIC:
+ xttcps_set_interval(timer,
+ DIV_ROUND_CLOSEST(clk_get_rate(xttce->xttc.clk),
+ PRESCALE * HZ));
+ break;
+ case CLOCK_EVT_MODE_ONESHOT:
+ case CLOCK_EVT_MODE_UNUSED:
+ case CLOCK_EVT_MODE_SHUTDOWN:
+ ctrl_reg = __raw_readl(timer->base_addr +
+ XTTCPS_CNT_CNTRL_OFFSET);
+ ctrl_reg |= XTTCPS_CNT_CNTRL_DISABLE_MASK;
+ __raw_writel(ctrl_reg,
+ timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
+ break;
+ case CLOCK_EVT_MODE_RESUME:
+ ctrl_reg = __raw_readl(timer->base_addr +
+ XTTCPS_CNT_CNTRL_OFFSET);
+ ctrl_reg &= ~XTTCPS_CNT_CNTRL_DISABLE_MASK;
+ __raw_writel(ctrl_reg,
+ timer->base_addr + XTTCPS_CNT_CNTRL_OFFSET);
+ break;
+ }
+}
+
+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;
+ int err;
+
+ ttccs = kzalloc(sizeof(*ttccs), GFP_KERNEL);
+ if (WARN_ON(!ttccs))
+ return;
+
+ ttccs->xttc.clk = clk;
+
+ err = clk_prepare_enable(ttccs->xttc.clk);
+ if (WARN_ON(err)) {
+ kfree(ttccs);
+ return;
+ }
+
+ 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->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(ttccs->xttc.clk) / PRESCALE);
+ if (WARN_ON(err)) {
+ kfree(ttccs);
+ return;
+ }
+}
+
+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;
+
+ ttcce = kzalloc(sizeof(*ttcce), GFP_KERNEL);
+ if (WARN_ON(!ttcce))
+ return;
+
+ ttcce->xttc.clk = clk;
+
+ err = clk_prepare_enable(ttcce->xttc.clk);
+ if (WARN_ON(err)) {
+ kfree(ttcce);
+ 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->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;
+ ttcce->ce.rating = 200;
+ 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_DISABLED | IRQF_TIMER,
+ ttcce->ce.name, ttcce);
+ if (WARN_ON(err)) {
+ kfree(ttcce);
+ return;
+ }
+
+ clockevents_config_and_register(&ttcce->ce,
+ clk_get_rate(ttcce->xttc.clk) / PRESCALE, 1, 0xfffe);
+}
+
+/**
+ * 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(struct device_node *timer)
+{
+ unsigned int irq;
+ void __iomem *timer_baseaddr;
+ struct clk *clk;
+ static int initialized;
+
+ if (initialized)
+ return;
+
+ initialized = 1;
+
+ /*
+ * 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@%p, irq=%d\n", timer->name, timer_baseaddr, irq);
+}
+
+CLOCKSOURCE_OF_DECLARE(ttc, "cdns,ttc", xttcps_timer_init);
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-26 17:43 Zynq core changes v2 Michal Simek
` (2 preceding siblings ...)
2013-03-26 17:43 ` [PATCH v2 03/10] arm: zynq: Move timer to generic location Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 21:44 ` Arnd Bergmann
2013-03-26 17:43 ` [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file Michal Simek
` (5 subsequent siblings)
9 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Use Cortex a9 cp15 to read scu baseaddress.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
v2: Remove dynamic mapping because it is probably not needed
(code taken from vexpress platsmp code)
Remove scu node from DTS
Use zynq_scu_map_io() instead of scu_init() as Steffen suggested
Add comment to scu_init to be sure that we know what we are doing here
---
arch/arm/mach-zynq/common.c | 34 ++++++++++++++++++++++------------
arch/arm/mach-zynq/common.h | 2 ++
2 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 68e0907..b53c34d 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -33,10 +33,13 @@
#include <asm/mach-types.h>
#include <asm/page.h>
#include <asm/pgtable.h>
+#include <asm/smp_scu.h>
#include <asm/hardware/cache-l2x0.h>
#include "common.h"
+void __iomem *scu_base;
+
static struct of_device_id zynq_of_bus_ids[] __initdata = {
{ .compatible = "simple-bus", },
{}
@@ -56,17 +59,6 @@ static void __init xilinx_init_machine(void)
of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
}
-#define SCU_PERIPH_PHYS 0xF8F00000
-#define SCU_PERIPH_SIZE SZ_8K
-#define SCU_PERIPH_VIRT (VMALLOC_END - SCU_PERIPH_SIZE)
-
-static struct map_desc scu_desc __initdata = {
- .virtual = SCU_PERIPH_VIRT,
- .pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
- .length = SCU_PERIPH_SIZE,
- .type = MT_DEVICE,
-};
-
static void __init xilinx_zynq_timer_init(void)
{
struct device_node *np;
@@ -81,13 +73,31 @@ static void __init xilinx_zynq_timer_init(void)
clocksource_of_init();
}
+static struct map_desc zynq_cortex_a9_scu_map __initdata = {
+ .length = SZ_256,
+ .type = MT_DEVICE,
+};
+
+static void __init zynq_scu_map_io(void)
+{
+ unsigned long base;
+
+ base = scu_a9_get_base();
+ zynq_cortex_a9_scu_map.pfn = __phys_to_pfn(base);
+ /* Expected address is in vmalloc area that's why simple assign here */
+ zynq_cortex_a9_scu_map.virtual = base;
+ iotable_init(&zynq_cortex_a9_scu_map, 1);
+ scu_base = (void __iomem *)base;
+ BUG_ON(!scu_base);
+}
+
/**
* xilinx_map_io() - Create memory mappings needed for early I/O.
*/
static void __init xilinx_map_io(void)
{
debug_ll_io_init();
- iotable_init(&scu_desc, 1);
+ zynq_scu_map_io();
}
static const char *xilinx_dt_match[] = {
diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
index 5050bb1..38727a2 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/arch/arm/mach-zynq/common.h
@@ -17,4 +17,6 @@
#ifndef __MACH_ZYNQ_COMMON_H__
#define __MACH_ZYNQ_COMMON_H__
+extern void __iomem *scu_base;
+
#endif
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-26 17:43 ` [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time Michal Simek
@ 2013-03-26 21:44 ` Arnd Bergmann
2013-03-27 7:49 ` Michal Simek
0 siblings, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2013-03-26 21:44 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 26 March 2013, Michal Simek wrote:
> +void __iomem *scu_base;
> +
This cannot be a global variable in a multiplatform kernel. Can you
make it static?
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-26 21:44 ` Arnd Bergmann
@ 2013-03-27 7:49 ` Michal Simek
2013-03-27 9:29 ` Arnd Bergmann
0 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-27 7:49 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/26 Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 26 March 2013, Michal Simek wrote:
>> +void __iomem *scu_base;
>> +
>
> This cannot be a global variable in a multiplatform kernel. Can you
> make it static?
This global variable is shared because I am using it in smp code
and also it will be used in pm code we have.
What is it another way how to handle this?
Because the same problem could be with zynq_slcr_base.
Or the rule is just to use zynq_ prefix for all global variables?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-27 7:49 ` Michal Simek
@ 2013-03-27 9:29 ` Arnd Bergmann
2013-03-27 9:37 ` Michal Simek
0 siblings, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2013-03-27 9:29 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 27 March 2013, Michal Simek wrote:
> 2013/3/26 Arnd Bergmann <arnd@arndb.de>:
> > On Tuesday 26 March 2013, Michal Simek wrote:
> >> +void __iomem *scu_base;
> >> +
> >
> > This cannot be a global variable in a multiplatform kernel. Can you
> > make it static?
>
> This global variable is shared because I am using it in smp code
> and also it will be used in pm code we have.
>
> What is it another way how to handle this?
>
> Because the same problem could be with zynq_slcr_base.
>
> Or the rule is just to use zynq_ prefix for all global variables?
>
Yes. The best solution is to mke variables local to some context and
only pointed to by other data structures. If that doesn't work, you
should try using a static variable in one file and move all code using
it there. If that doesn't work, you need a global variable, but that
must be uniquely named.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-27 9:29 ` Arnd Bergmann
@ 2013-03-27 9:37 ` Michal Simek
2013-03-27 10:00 ` Arnd Bergmann
0 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-27 9:37 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/27 Arnd Bergmann <arnd@arndb.de>:
> On Wednesday 27 March 2013, Michal Simek wrote:
>> 2013/3/26 Arnd Bergmann <arnd@arndb.de>:
>> > On Tuesday 26 March 2013, Michal Simek wrote:
>> >> +void __iomem *scu_base;
>> >> +
>> >
>> > This cannot be a global variable in a multiplatform kernel. Can you
>> > make it static?
>>
>> This global variable is shared because I am using it in smp code
>> and also it will be used in pm code we have.
>>
>> What is it another way how to handle this?
>>
>> Because the same problem could be with zynq_slcr_base.
>>
>> Or the rule is just to use zynq_ prefix for all global variables?
>>
>
> Yes. The best solution is to mke variables local to some context and
> only pointed to by other data structures. If that doesn't work, you
> should try using a static variable in one file and move all code using
> it there. If that doesn't work, you need a global variable, but that
> must be uniquely named.
FYI: I have looked at some code and I saw that Rob is using scu_base_addr
in highbank. And then pointing to it in cpuidle-calxeda.c.
Moving everything to one file is probably impossible.
And in connection to symbols/functions/variables. It means that all
specific soc functions in mach
should also use specific prefix for everything.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-27 9:37 ` Michal Simek
@ 2013-03-27 10:00 ` Arnd Bergmann
2013-03-27 10:09 ` Michal Simek
0 siblings, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2013-03-27 10:00 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 27 March 2013, Michal Simek wrote:
> FYI: I have looked at some code and I saw that Rob is using scu_base_addr
> in highbank. And then pointing to it in cpuidle-calxeda.c.
Yes, the point is that it works as long as only one person uses that
identifier, so we should either not use it at all, or have a single
global definition shared by all ARM platforms.
> Moving everything to one file is probably impossible.
>
> And in connection to symbols/functions/variables. It means that all
> specific soc functions in mach
> should also use specific prefix for everything.
That is a good rule, although for static symbols it is not a bug
if they don't have a prefix.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-27 10:00 ` Arnd Bergmann
@ 2013-03-27 10:09 ` Michal Simek
2013-03-27 10:43 ` Steffen Trumtrar
2013-03-27 10:45 ` Arnd Bergmann
0 siblings, 2 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-27 10:09 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/27 Arnd Bergmann <arnd@arndb.de>:
> On Wednesday 27 March 2013, Michal Simek wrote:
>> FYI: I have looked at some code and I saw that Rob is using scu_base_addr
>> in highbank. And then pointing to it in cpuidle-calxeda.c.
>
> Yes, the point is that it works as long as only one person uses that
> identifier, so we should either not use it at all, or have a single
> global definition shared by all ARM platforms.
yep.
>> Moving everything to one file is probably impossible.
>>
>> And in connection to symbols/functions/variables. It means that all
>> specific soc functions in mach
>> should also use specific prefix for everything.
>
> That is a good rule, although for static symbols it is not a bug
> if they don't have a prefix.
ok. Let me check all my patches and Add there zynq_ prefix everywhere.
BTW: Have you added to zero day testing system arm multi platform
or is there any arm multi platform config which developer should run
to be sure that armv7 code can be compiled for armv6 and also
that there is no problem with symbols?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-27 10:09 ` Michal Simek
@ 2013-03-27 10:43 ` Steffen Trumtrar
2013-03-27 10:49 ` Michal Simek
2013-03-27 10:45 ` Arnd Bergmann
1 sibling, 1 reply; 42+ messages in thread
From: Steffen Trumtrar @ 2013-03-27 10:43 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 27, 2013 at 11:09:45AM +0100, Michal Simek wrote:
> 2013/3/27 Arnd Bergmann <arnd@arndb.de>:
> > On Wednesday 27 March 2013, Michal Simek wrote:
> >> FYI: I have looked at some code and I saw that Rob is using scu_base_addr
> >> in highbank. And then pointing to it in cpuidle-calxeda.c.
> >
> > Yes, the point is that it works as long as only one person uses that
> > identifier, so we should either not use it at all, or have a single
> > global definition shared by all ARM platforms.
>
> yep.
>
> >> Moving everything to one file is probably impossible.
> >>
> >> And in connection to symbols/functions/variables. It means that all
> >> specific soc functions in mach
> >> should also use specific prefix for everything.
> >
> > That is a good rule, although for static symbols it is not a bug
> > if they don't have a prefix.
>
> ok. Let me check all my patches and Add there zynq_ prefix everywhere.
>
While you're at, how about getting rid of the xilinx in the names and use
just zynq_* ?
I would also suggest s/xttc/ttc/g in your timer patch. It is up to you,
but I think the x doesn't make any sense anymore.
Regards,
Steffen
--
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 |
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-27 10:43 ` Steffen Trumtrar
@ 2013-03-27 10:49 ` Michal Simek
0 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-27 10:49 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/27 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> On Wed, Mar 27, 2013 at 11:09:45AM +0100, Michal Simek wrote:
>> 2013/3/27 Arnd Bergmann <arnd@arndb.de>:
>> > On Wednesday 27 March 2013, Michal Simek wrote:
>> >> FYI: I have looked at some code and I saw that Rob is using scu_base_addr
>> >> in highbank. And then pointing to it in cpuidle-calxeda.c.
>> >
>> > Yes, the point is that it works as long as only one person uses that
>> > identifier, so we should either not use it at all, or have a single
>> > global definition shared by all ARM platforms.
>>
>> yep.
>>
>> >> Moving everything to one file is probably impossible.
>> >>
>> >> And in connection to symbols/functions/variables. It means that all
>> >> specific soc functions in mach
>> >> should also use specific prefix for everything.
>> >
>> > That is a good rule, although for static symbols it is not a bug
>> > if they don't have a prefix.
>>
>> ok. Let me check all my patches and Add there zynq_ prefix everywhere.
>>
>
> While you're at, how about getting rid of the xilinx in the names and use
> just zynq_* ?
yep. That's what I am going to do.
> I would also suggest s/xttc/ttc/g in your timer patch. It is up to you,
> but I think the x doesn't make any sense anymore.
Agree. I don't know the historical reason for this.
Also in xilinx git repo there was PSS instead of PS everywhere.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-27 10:09 ` Michal Simek
2013-03-27 10:43 ` Steffen Trumtrar
@ 2013-03-27 10:45 ` Arnd Bergmann
2013-03-27 10:47 ` Michal Simek
1 sibling, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2013-03-27 10:45 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 27 March 2013, Michal Simek wrote:
> BTW: Have you added to zero day testing system arm multi platform
> or is there any arm multi platform config which developer should run
> to be sure that armv7 code can be compiled for armv6 and also
> that there is no problem with symbols?
I'm only occasionally doing build tested right now, not all the time.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time
2013-03-27 10:45 ` Arnd Bergmann
@ 2013-03-27 10:47 ` Michal Simek
0 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-27 10:47 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/27 Arnd Bergmann <arnd@arndb.de>:
> On Wednesday 27 March 2013, Michal Simek wrote:
>> BTW: Have you added to zero day testing system arm multi platform
>> or is there any arm multi platform config which developer should run
>> to be sure that armv7 code can be compiled for armv6 and also
>> that there is no problem with symbols?
>
> I'm only occasionally doing build tested right now, not all the time.
What about to provide this config to Fengguang to add this to zero day testing
system which will check this all the time?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file
2013-03-26 17:43 Zynq core changes v2 Michal Simek
` (3 preceding siblings ...)
2013-03-26 17:43 ` [PATCH v2 04/10] arm: zynq: Load scu baseaddress at run time Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 21:43 ` Arnd Bergmann
2013-03-27 13:01 ` Philip Balister
2013-03-26 17:43 ` [PATCH v2 06/10] arm: zynq: Add support for system reset Michal Simek
` (4 subsequent siblings)
9 siblings, 2 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Create separate slcr driver instead of pollute common code.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
arch/arm/mach-zynq/Makefile | 2 +-
arch/arm/mach-zynq/common.c | 10 +------
arch/arm/mach-zynq/common.h | 3 ++
arch/arm/mach-zynq/slcr.c | 69 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 74 insertions(+), 10 deletions(-)
create mode 100644 arch/arm/mach-zynq/slcr.c
diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
index 320faed..13ee09b 100644
--- a/arch/arm/mach-zynq/Makefile
+++ b/arch/arm/mach-zynq/Makefile
@@ -3,4 +3,4 @@
#
# Common support
-obj-y := common.o
+obj-y := common.o slcr.o
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index b53c34d..2cb94ab 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -61,15 +61,7 @@ static void __init xilinx_init_machine(void)
static void __init xilinx_zynq_timer_init(void)
{
- struct device_node *np;
- void __iomem *slcr;
-
- np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
- slcr = of_iomap(np, 0);
- WARN_ON(!slcr);
-
- xilinx_zynq_clocks_init(slcr);
-
+ slcr_init();
clocksource_of_init();
}
diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
index 38727a2..e30898a 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/arch/arm/mach-zynq/common.h
@@ -17,6 +17,9 @@
#ifndef __MACH_ZYNQ_COMMON_H__
#define __MACH_ZYNQ_COMMON_H__
+extern int slcr_init(void);
+
+extern void __iomem *zynq_slcr_base;
extern void __iomem *scu_base;
#endif
diff --git a/arch/arm/mach-zynq/slcr.c b/arch/arm/mach-zynq/slcr.c
new file mode 100644
index 0000000..1883b70
--- /dev/null
+++ b/arch/arm/mach-zynq/slcr.c
@@ -0,0 +1,69 @@
+/*
+ * Xilinx SLCR driver
+ *
+ * Copyright (c) 2011-2013 Xilinx Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the Free
+ * Software Foundation, Inc., 675 Mass Ave, Cambridge, MA
+ * 02139, USA.
+ */
+
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/fs.h>
+#include <linux/interrupt.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/uaccess.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/clk/zynq.h>
+#include "common.h"
+
+#define SLCR_UNLOCK_MAGIC 0xDF0D
+#define SLCR_UNLOCK 0x8 /* SCLR unlock register */
+
+void __iomem *zynq_slcr_base;
+
+/**
+ * xslcr_init()
+ * Returns 0 on success, negative errno otherwise.
+ *
+ * Called early during boot from platform code to remap SLCR area.
+ */
+int __init slcr_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
+ if (!np) {
+ pr_err("%s: no slcr node found\n", __func__);
+ BUG();
+ }
+
+ zynq_slcr_base = of_iomap(np, 0);
+ if (!zynq_slcr_base) {
+ pr_err("%s: Unable to map I/O memory\n", __func__);
+ BUG();
+ }
+
+ /* unlock the SLCR so that registers can be changed */
+ writel(SLCR_UNLOCK_MAGIC, zynq_slcr_base + SLCR_UNLOCK);
+
+ pr_info("%s mapped to %p\n", np->name, zynq_slcr_base);
+
+ xilinx_zynq_clocks_init(zynq_slcr_base);
+
+ of_node_put(np);
+
+ return 0;
+}
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file
2013-03-26 17:43 ` [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file Michal Simek
@ 2013-03-26 21:43 ` Arnd Bergmann
2013-03-27 6:55 ` Steffen Trumtrar
2013-03-27 13:01 ` Philip Balister
1 sibling, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2013-03-26 21:43 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 26 March 2013, Michal Simek wrote:
> Create separate slcr driver instead of pollute common code.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Can't you move that code into the zynq_cpu_clk_setup function
instead, and only call of_clk_init(NULL) from platform code?
I would like to eventually add a default call to of_clk_init(NULL)
for all platforms, and that would be a step in that direction.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file
2013-03-26 21:43 ` Arnd Bergmann
@ 2013-03-27 6:55 ` Steffen Trumtrar
2013-03-27 9:31 ` Arnd Bergmann
0 siblings, 1 reply; 42+ messages in thread
From: Steffen Trumtrar @ 2013-03-27 6:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd,
On Tue, Mar 26, 2013 at 09:43:23PM +0000, Arnd Bergmann wrote:
> On Tuesday 26 March 2013, Michal Simek wrote:
> > Create separate slcr driver instead of pollute common code.
> >
> > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>
> Can't you move that code into the zynq_cpu_clk_setup function
> instead, and only call of_clk_init(NULL) from platform code?
>
if you are talking about the slcr function, than moving it into
a separate file is the right move. This should actually become a
real driver. The slcr is master over all clock, reset, pinmux and
ddr registers. And as all those registers can be locked/unlocked
via a slcr register (for whatever reason one would do that), there
should be one master that controls this space.
> I would like to eventually add a default call to of_clk_init(NULL)
> for all platforms, and that would be a step in that direction.
>
Regards,
Steffen
--
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 |
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file
2013-03-27 6:55 ` Steffen Trumtrar
@ 2013-03-27 9:31 ` Arnd Bergmann
2013-03-27 9:41 ` Steffen Trumtrar
0 siblings, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2013-03-27 9:31 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 27 March 2013, Steffen Trumtrar wrote:
> On Tue, Mar 26, 2013 at 09:43:23PM +0000, Arnd Bergmann wrote:
> > On Tuesday 26 March 2013, Michal Simek wrote:
> > > Create separate slcr driver instead of pollute common code.
> > >
> > > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >
> > Can't you move that code into the zynq_cpu_clk_setup function
> > instead, and only call of_clk_init(NULL) from platform code?
> >
> if you are talking about the slcr function, than moving it into
> a separate file is the right move. This should actually become a
> real driver. The slcr is master over all clock, reset, pinmux and
> ddr registers. And as all those registers can be locked/unlocked
> via a slcr register (for whatever reason one would do that), there
> should be one master that controls this space.
Ok, I see. Thanks for the explanation. Should this be using the
drivers/mfd/syscon.c infrastructure then?
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file
2013-03-27 9:31 ` Arnd Bergmann
@ 2013-03-27 9:41 ` Steffen Trumtrar
2013-03-27 14:15 ` Michal Simek
0 siblings, 1 reply; 42+ messages in thread
From: Steffen Trumtrar @ 2013-03-27 9:41 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 27, 2013 at 09:31:26AM +0000, Arnd Bergmann wrote:
> On Wednesday 27 March 2013, Steffen Trumtrar wrote:
> > On Tue, Mar 26, 2013 at 09:43:23PM +0000, Arnd Bergmann wrote:
> > > On Tuesday 26 March 2013, Michal Simek wrote:
> > > > Create separate slcr driver instead of pollute common code.
> > > >
> > > > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > >
> > > Can't you move that code into the zynq_cpu_clk_setup function
> > > instead, and only call of_clk_init(NULL) from platform code?
> > >
> > if you are talking about the slcr function, than moving it into
> > a separate file is the right move. This should actually become a
> > real driver. The slcr is master over all clock, reset, pinmux and
> > ddr registers. And as all those registers can be locked/unlocked
> > via a slcr register (for whatever reason one would do that), there
> > should be one master that controls this space.
>
> Ok, I see. Thanks for the explanation. Should this be using the
> drivers/mfd/syscon.c infrastructure then?
A quick look suggests that this might be the way to go. I wasn't aware of that.
Thanks,
Steffen
--
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 |
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file
2013-03-27 9:41 ` Steffen Trumtrar
@ 2013-03-27 14:15 ` Michal Simek
0 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-27 14:15 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/27 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
> On Wed, Mar 27, 2013 at 09:31:26AM +0000, Arnd Bergmann wrote:
>> On Wednesday 27 March 2013, Steffen Trumtrar wrote:
>> > On Tue, Mar 26, 2013 at 09:43:23PM +0000, Arnd Bergmann wrote:
>> > > On Tuesday 26 March 2013, Michal Simek wrote:
>> > > > Create separate slcr driver instead of pollute common code.
>> > > >
>> > > > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> > >
>> > > Can't you move that code into the zynq_cpu_clk_setup function
>> > > instead, and only call of_clk_init(NULL) from platform code?
>> > >
>> > if you are talking about the slcr function, than moving it into
>> > a separate file is the right move. This should actually become a
>> > real driver. The slcr is master over all clock, reset, pinmux and
>> > ddr registers. And as all those registers can be locked/unlocked
>> > via a slcr register (for whatever reason one would do that), there
>> > should be one master that controls this space.
>>
>> Ok, I see. Thanks for the explanation. Should this be using the
>> drivers/mfd/syscon.c infrastructure then?
>
> A quick look suggests that this might be the way to go. I wasn't aware of that.
I have looked at syscon driver but the problem is when this driver is
ready to use.
It is too late because bus probing is done in init_machine but
we need to unlock slcr before clk and timer init functions are called.
For unlocking we have to also map it.
of_clk_init(NULL) is called from zynq clock. It is no problem to move it
to common code but still we have to map slcr register in the driver
and pass this address to clk driver.
Currently we are passing slcr reg base as xilinx_zynq_clocks_init() parameter
to avoid to have extern in C file.
I see that Rob in highbank clk choose a way with extern.
of_iomap is done in highbank_timer_init and in the same function calls
of_clk_init()
and clocksource_of_init().
Syscon will be nice to use much later but not for this core stuff.
But maybe you know nicer way how to handle it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file
2013-03-26 17:43 ` [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file Michal Simek
2013-03-26 21:43 ` Arnd Bergmann
@ 2013-03-27 13:01 ` Philip Balister
2013-03-27 13:31 ` Michal Simek
1 sibling, 1 reply; 42+ messages in thread
From: Philip Balister @ 2013-03-27 13:01 UTC (permalink / raw)
To: linux-arm-kernel
On 03/26/2013 01:43 PM, Michal Simek wrote:
> Create separate slcr driver instead of pollute common code.
Nitpicking: A native speaker would say "polluting common code".
Philip
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> arch/arm/mach-zynq/Makefile | 2 +-
> arch/arm/mach-zynq/common.c | 10 +------
> arch/arm/mach-zynq/common.h | 3 ++
> arch/arm/mach-zynq/slcr.c | 69 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 74 insertions(+), 10 deletions(-)
> create mode 100644 arch/arm/mach-zynq/slcr.c
>
> diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
> index 320faed..13ee09b 100644
> --- a/arch/arm/mach-zynq/Makefile
> +++ b/arch/arm/mach-zynq/Makefile
> @@ -3,4 +3,4 @@
> #
>
> # Common support
> -obj-y := common.o
> +obj-y := common.o slcr.o
> diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> index b53c34d..2cb94ab 100644
> --- a/arch/arm/mach-zynq/common.c
> +++ b/arch/arm/mach-zynq/common.c
> @@ -61,15 +61,7 @@ static void __init xilinx_init_machine(void)
>
> static void __init xilinx_zynq_timer_init(void)
> {
> - struct device_node *np;
> - void __iomem *slcr;
> -
> - np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
> - slcr = of_iomap(np, 0);
> - WARN_ON(!slcr);
> -
> - xilinx_zynq_clocks_init(slcr);
> -
> + slcr_init();
> clocksource_of_init();
> }
>
> diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
> index 38727a2..e30898a 100644
> --- a/arch/arm/mach-zynq/common.h
> +++ b/arch/arm/mach-zynq/common.h
> @@ -17,6 +17,9 @@
> #ifndef __MACH_ZYNQ_COMMON_H__
> #define __MACH_ZYNQ_COMMON_H__
>
> +extern int slcr_init(void);
> +
> +extern void __iomem *zynq_slcr_base;
> extern void __iomem *scu_base;
>
> #endif
> diff --git a/arch/arm/mach-zynq/slcr.c b/arch/arm/mach-zynq/slcr.c
> new file mode 100644
> index 0000000..1883b70
> --- /dev/null
> +++ b/arch/arm/mach-zynq/slcr.c
> @@ -0,0 +1,69 @@
> +/*
> + * Xilinx SLCR driver
> + *
> + * Copyright (c) 2011-2013 Xilinx Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; if not, write to the Free
> + * Software Foundation, Inc., 675 Mass Ave, Cambridge, MA
> + * 02139, USA.
> + */
> +
> +#include <linux/export.h>
> +#include <linux/io.h>
> +#include <linux/fs.h>
> +#include <linux/interrupt.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/uaccess.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/clk/zynq.h>
> +#include "common.h"
> +
> +#define SLCR_UNLOCK_MAGIC 0xDF0D
> +#define SLCR_UNLOCK 0x8 /* SCLR unlock register */
> +
> +void __iomem *zynq_slcr_base;
> +
> +/**
> + * xslcr_init()
> + * Returns 0 on success, negative errno otherwise.
> + *
> + * Called early during boot from platform code to remap SLCR area.
> + */
> +int __init slcr_init(void)
> +{
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "xlnx,zynq-slcr");
> + if (!np) {
> + pr_err("%s: no slcr node found\n", __func__);
> + BUG();
> + }
> +
> + zynq_slcr_base = of_iomap(np, 0);
> + if (!zynq_slcr_base) {
> + pr_err("%s: Unable to map I/O memory\n", __func__);
> + BUG();
> + }
> +
> + /* unlock the SLCR so that registers can be changed */
> + writel(SLCR_UNLOCK_MAGIC, zynq_slcr_base + SLCR_UNLOCK);
> +
> + pr_info("%s mapped to %p\n", np->name, zynq_slcr_base);
> +
> + xilinx_zynq_clocks_init(zynq_slcr_base);
> +
> + of_node_put(np);
> +
> + return 0;
> +}
>
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file
2013-03-27 13:01 ` Philip Balister
@ 2013-03-27 13:31 ` Michal Simek
0 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-27 13:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Philip,
2013/3/27 Philip Balister <philip@balister.org>:
> On 03/26/2013 01:43 PM, Michal Simek wrote:
>>
>> Create separate slcr driver instead of pollute common code.
>
>
> Nitpicking: A native speaker would say "polluting common code".
Feel free to check my english. :-)
M
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 06/10] arm: zynq: Add support for system reset
2013-03-26 17:43 Zynq core changes v2 Michal Simek
` (4 preceding siblings ...)
2013-03-26 17:43 ` [PATCH v2 05/10] arm: zynq: Move slcr initialization to separate file Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 07/10] arm: zynq: Add support for pmu Michal Simek
` (3 subsequent siblings)
9 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Do system reset via slcr registers.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
v2: Fix comment
use writel instead of __raw_writel
Do not use PSS - use PS instead
---
arch/arm/mach-zynq/common.c | 6 ++++++
arch/arm/mach-zynq/common.h | 1 +
arch/arm/mach-zynq/slcr.c | 28 ++++++++++++++++++++++++++++
3 files changed, 35 insertions(+)
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 2cb94ab..4ad3560 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -92,6 +92,11 @@ static void __init xilinx_map_io(void)
zynq_scu_map_io();
}
+static void xilinx_system_reset(char mode, const char *cmd)
+{
+ slcr_system_reset();
+}
+
static const char *xilinx_dt_match[] = {
"xlnx,zynq-zc702",
"xlnx,zynq-7000",
@@ -104,4 +109,5 @@ MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform")
.init_machine = xilinx_init_machine,
.init_time = xilinx_zynq_timer_init,
.dt_compat = xilinx_dt_match,
+ .restart = xilinx_system_reset,
MACHINE_END
diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
index e30898a..e5628f7 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/arch/arm/mach-zynq/common.h
@@ -18,6 +18,7 @@
#define __MACH_ZYNQ_COMMON_H__
extern int slcr_init(void);
+extern void slcr_system_reset(void);
extern void __iomem *zynq_slcr_base;
extern void __iomem *scu_base;
diff --git a/arch/arm/mach-zynq/slcr.c b/arch/arm/mach-zynq/slcr.c
index 1883b70..3b4695b 100644
--- a/arch/arm/mach-zynq/slcr.c
+++ b/arch/arm/mach-zynq/slcr.c
@@ -32,9 +32,37 @@
#define SLCR_UNLOCK_MAGIC 0xDF0D
#define SLCR_UNLOCK 0x8 /* SCLR unlock register */
+#define SLCR_PS_RST_CTRL_OFFSET 0x200 /* PS Software Reset Control */
+#define SLCR_REBOOT_STATUS 0x258 /* PS Reboot Status */
+
void __iomem *zynq_slcr_base;
/**
+ * xslcr_system_reset - Reset the entire system.
+ *
+ */
+void slcr_system_reset(void)
+{
+ u32 reboot;
+
+ /*
+ * Unlock the SLCR then reset the system.
+ * Note that this seems to require raw i/o
+ * functions or there's a lockup?
+ */
+ writel(SLCR_UNLOCK_MAGIC, zynq_slcr_base + SLCR_UNLOCK);
+
+ /*
+ * Clear 0x0F000000 bits of reboot status register to workaround
+ * the FSBL not loading the bitstream after soft-reboot
+ * This is a temporary solution until we know more.
+ */
+ reboot = readl(zynq_slcr_base + SLCR_REBOOT_STATUS);
+ writel(reboot & 0xF0FFFFFF, zynq_slcr_base + SLCR_REBOOT_STATUS);
+ writel(1, zynq_slcr_base + SLCR_PS_RST_CTRL_OFFSET);
+}
+
+/**
* xslcr_init()
* Returns 0 on success, negative errno otherwise.
*
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 07/10] arm: zynq: Add support for pmu
2013-03-26 17:43 Zynq core changes v2 Michal Simek
` (5 preceding siblings ...)
2013-03-26 17:43 ` [PATCH v2 06/10] arm: zynq: Add support for system reset Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 08/10] arm: zynq: Add smp support Michal Simek
` (2 subsequent siblings)
9 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Zynq is standard PMU with 2 interrupt per core.
There is also access via register which is not used right now.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
arch/arm/boot/dts/zynq-7000.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 51243db..748fc34 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -15,6 +15,13 @@
/ {
compatible = "xlnx,zynq-7000";
+ pmu {
+ compatible = "arm,cortex-a9-pmu";
+ interrupts = <0 5 4>, <0 6 4>;
+ interrupt-parent = <&intc>;
+ reg = < 0xf8891000 0x1000 0xf8893000 0x1000 >;
+ };
+
amba {
compatible = "simple-bus";
#address-cells = <1>;
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 08/10] arm: zynq: Add smp support
2013-03-26 17:43 Zynq core changes v2 Michal Simek
` (6 preceding siblings ...)
2013-03-26 17:43 ` [PATCH v2 07/10] arm: zynq: Add support for pmu Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-27 8:59 ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 09/10] arm: zynq: Add hotplug support Michal Simek
2013-03-26 17:43 ` [PATCH v2 10/10] arm: zynq: Add cpuidle support Michal Simek
9 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Zynq is dual core Cortex A9 which starts always
at zero. Using simple trampoline ensure long jump
to secondary_startup code.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
v2: Remove boot_lock from secondary_init
Incorporate Steffen changes in smp code
Use flush range instead of simple flush
Add HAVE_SMP
Add support for the booting kernels with
non zero starting address
---
arch/arm/mach-zynq/Kconfig | 1 +
arch/arm/mach-zynq/Makefile | 1 +
arch/arm/mach-zynq/common.c | 1 +
arch/arm/mach-zynq/common.h | 11 ++++
arch/arm/mach-zynq/headsmp.S | 24 +++++++
arch/arm/mach-zynq/platsmp.c | 149 ++++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-zynq/slcr.c | 29 ++++++++
7 files changed, 216 insertions(+)
create mode 100644 arch/arm/mach-zynq/headsmp.S
create mode 100644 arch/arm/mach-zynq/platsmp.c
diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
index d70651e..f4a7e63 100644
--- a/arch/arm/mach-zynq/Kconfig
+++ b/arch/arm/mach-zynq/Kconfig
@@ -8,6 +8,7 @@ config ARCH_ZYNQ
select ICST
select MIGHT_HAVE_CACHE_L2X0
select USE_OF
+ select HAVE_SMP
select SPARSE_IRQ
select CADENCE_TTC_TIMER
help
diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
index 13ee09b..b595d22 100644
--- a/arch/arm/mach-zynq/Makefile
+++ b/arch/arm/mach-zynq/Makefile
@@ -4,3 +4,4 @@
# Common support
obj-y := common.o slcr.o
+obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 4ad3560..da93957 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -104,6 +104,7 @@ static const char *xilinx_dt_match[] = {
};
MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform")
+ .smp = smp_ops(zynq_smp_ops),
.map_io = xilinx_map_io,
.init_irq = irqchip_init,
.init_machine = xilinx_init_machine,
diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
index e5628f7..33cdc23 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/arch/arm/mach-zynq/common.h
@@ -19,6 +19,17 @@
extern int slcr_init(void);
extern void slcr_system_reset(void);
+extern void slcr_cpu_stop(int cpu);
+extern void slcr_cpu_start(int cpu);
+
+#ifdef CONFIG_SMP
+extern void secondary_startup(void);
+extern char zynq_secondary_trampoline;
+extern char zynq_secondary_trampoline_jump;
+extern char zynq_secondary_trampoline_end;
+extern int __cpuinit zynq_cpun_start(u32 address, int cpu);
+extern struct smp_operations zynq_smp_ops __initdata;
+#endif
extern void __iomem *zynq_slcr_base;
extern void __iomem *scu_base;
diff --git a/arch/arm/mach-zynq/headsmp.S b/arch/arm/mach-zynq/headsmp.S
new file mode 100644
index 0000000..d183cd2
--- /dev/null
+++ b/arch/arm/mach-zynq/headsmp.S
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ * Copyright (c) 2012-2013 Xilinx
+ *
+ * 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/linkage.h>
+#include <linux/init.h>
+
+ __CPUINIT
+
+ENTRY(zynq_secondary_trampoline)
+ ldr r0, [pc]
+ bx r0
+.globl zynq_secondary_trampoline_jump
+zynq_secondary_trampoline_jump:
+ /* Space for jumping address */
+ .word /* cpu 1 */
+.globl zynq_secondary_trampoline_end
+zynq_secondary_trampoline_end:
+
+ENDPROC(zynq_secondary_trampoline)
diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
new file mode 100644
index 0000000..062b319
--- /dev/null
+++ b/arch/arm/mach-zynq/platsmp.c
@@ -0,0 +1,149 @@
+/*
+ * This file contains Xilinx specific SMP code, used to start up
+ * the second processor.
+ *
+ * Copyright (C) 2011-2013 Xilinx
+ *
+ * based on linux/arch/arm/mach-realview/platsmp.c
+ *
+ * Copyright (C) 2002 ARM Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#include <linux/export.h>
+#include <linux/jiffies.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <asm/cacheflush.h>
+#include <asm/smp_scu.h>
+#include <linux/irqchip/arm-gic.h>
+#include "common.h"
+
+/*
+ * Store number of cores in the system
+ * Because of scu_get_core_count() must be in __init section and can't
+ * be called from zynq_cpun_start() because it is in __cpuinit section.
+ */
+static int ncores;
+
+/* Secondary CPU kernel startup is a 2 step process. The primary CPU
+ * starts the secondary CPU by giving it the address of the kernel and
+ * then sending it an event to wake it up. The secondary CPU then
+ * starts the kernel and tells the primary CPU it's up and running.
+ */
+static void __cpuinit zynq_secondary_init(unsigned int cpu)
+{
+ /*
+ * if any interrupts are already enabled for the primary
+ * core (e.g. timer irq), then they will not have been enabled
+ * for us: do so
+ */
+ gic_secondary_init(0);
+}
+
+int __cpuinit zynq_cpun_start(u32 address, int cpu)
+{
+ u32 trampoline_code_size = &zynq_secondary_trampoline_end -
+ &zynq_secondary_trampoline;
+
+ if (cpu > ncores) {
+ pr_warn("CPU No. is not available in the system\n");
+ return -1;
+ }
+
+ /* MS: Expectation that SLCR are directly map and accessible */
+ /* Not possible to jump to non aligned address */
+ if (!(address & 3) && (!address || (address >= trampoline_code_size))) {
+ /* Store pointer to ioremap area which points to address 0x0 */
+ static u8 __iomem *zero;
+ u32 trampoline_size = &zynq_secondary_trampoline_jump -
+ &zynq_secondary_trampoline;
+
+ slcr_cpu_stop(cpu);
+
+ if (__pa(PAGE_OFFSET)) {
+ zero = ioremap(0, trampoline_code_size);
+ if (!zero) {
+ pr_warn("BOOTUP jump vectors not accessible\n");
+ return -1;
+ }
+ } else {
+ zero = (__force u8 __iomem *)PAGE_OFFSET;
+ }
+
+ /*
+ * This is elegant way how to jump to any address
+ * 0x0: Load address at 0x8 to r0
+ * 0x4: Jump by mov instruction
+ * 0x8: Jumping address
+ */
+ memcpy((__force void *)zero, &zynq_secondary_trampoline,
+ trampoline_size);
+ writel(address, zero + trampoline_size + sizeof(int));
+
+ flush_cache_all();
+ outer_flush_range(0, trampoline_code_size);
+ smp_wmb();
+
+ if (__pa(PAGE_OFFSET))
+ iounmap(zero);
+
+ slcr_cpu_start(cpu);
+
+ return 0;
+ }
+
+ pr_warn("Can't start CPU%d: Wrong starting address %x\n", cpu, address);
+
+ return -1;
+}
+EXPORT_SYMBOL(zynq_cpun_start);
+
+static int __cpuinit zynq_boot_secondary(unsigned int cpu,
+ struct task_struct *idle)
+{
+ return zynq_cpun_start(virt_to_phys(secondary_startup), cpu);
+}
+
+/*
+ * Initialise the CPU possible map early - this describes the CPUs
+ * which may be present or become present in the system.
+ */
+static void __init zynq_smp_init_cpus(void)
+{
+ int i;
+
+ ncores = scu_get_core_count(scu_base);
+
+ for (i = 0; i < ncores && i < CONFIG_NR_CPUS; i++)
+ set_cpu_possible(i, true);
+}
+
+static void __init zynq_smp_prepare_cpus(unsigned int max_cpus)
+{
+ int i;
+
+ /*
+ * Initialise the present map, which describes the set of CPUs
+ * actually populated@the present time.
+ */
+ for (i = 0; i < max_cpus; i++)
+ set_cpu_present(i, true);
+
+ scu_enable(scu_base);
+}
+
+struct smp_operations zynq_smp_ops __initdata = {
+ .smp_init_cpus = zynq_smp_init_cpus,
+ .smp_prepare_cpus = zynq_smp_prepare_cpus,
+ .smp_secondary_init = zynq_secondary_init,
+ .smp_boot_secondary = zynq_boot_secondary,
+};
diff --git a/arch/arm/mach-zynq/slcr.c b/arch/arm/mach-zynq/slcr.c
index 3b4695b..60d6548 100644
--- a/arch/arm/mach-zynq/slcr.c
+++ b/arch/arm/mach-zynq/slcr.c
@@ -33,6 +33,11 @@
#define SLCR_UNLOCK 0x8 /* SCLR unlock register */
#define SLCR_PS_RST_CTRL_OFFSET 0x200 /* PS Software Reset Control */
+
+#define SLCR_A9_CPU_CLKSTOP 0x10
+#define SLCR_A9_CPU_RST 0x1
+
+#define SLCR_A9_CPU_RST_CTRL 0x244 /* CPU Software Reset Control */
#define SLCR_REBOOT_STATUS 0x258 /* PS Reboot Status */
void __iomem *zynq_slcr_base;
@@ -63,6 +68,30 @@ void slcr_system_reset(void)
}
/**
+ * slcr_cpu_start - Start cpu
+ * @cpu: cpu number
+ */
+void slcr_cpu_start(int cpu)
+{
+ /* enable CPUn */
+ writel(SLCR_A9_CPU_CLKSTOP << cpu,
+ zynq_slcr_base + SLCR_A9_CPU_RST_CTRL);
+ /* enable CLK for CPUn */
+ writel(0x0 << cpu, zynq_slcr_base + SLCR_A9_CPU_RST_CTRL);
+}
+
+/**
+ * slcr_cpu_stop - Stop cpu
+ * @cpu: cpu number
+ */
+void slcr_cpu_stop(int cpu)
+{
+ /* stop CLK and reset CPUn */
+ writel((SLCR_A9_CPU_CLKSTOP | SLCR_A9_CPU_RST) << cpu,
+ zynq_slcr_base + SLCR_A9_CPU_RST_CTRL);
+}
+
+/**
* xslcr_init()
* Returns 0 on success, negative errno otherwise.
*
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 08/10] arm: zynq: Add smp support
2013-03-26 17:43 ` [PATCH v2 08/10] arm: zynq: Add smp support Michal Simek
@ 2013-03-27 8:59 ` Michal Simek
0 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-27 8:59 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/26 Michal Simek <michal.simek@xilinx.com>:
> Zynq is dual core Cortex A9 which starts always
> at zero. Using simple trampoline ensure long jump
> to secondary_startup code.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> v2: Remove boot_lock from secondary_init
> Incorporate Steffen changes in smp code
> Use flush range instead of simple flush
> Add HAVE_SMP
> Add support for the booting kernels with
> non zero starting address
> ---
> arch/arm/mach-zynq/Kconfig | 1 +
> arch/arm/mach-zynq/Makefile | 1 +
> arch/arm/mach-zynq/common.c | 1 +
> arch/arm/mach-zynq/common.h | 11 ++++
> arch/arm/mach-zynq/headsmp.S | 24 +++++++
> arch/arm/mach-zynq/platsmp.c | 149 ++++++++++++++++++++++++++++++++++++++++++
> arch/arm/mach-zynq/slcr.c | 29 ++++++++
> 7 files changed, 216 insertions(+)
> create mode 100644 arch/arm/mach-zynq/headsmp.S
> create mode 100644 arch/arm/mach-zynq/platsmp.c
>
> diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
> index d70651e..f4a7e63 100644
> --- a/arch/arm/mach-zynq/Kconfig
> +++ b/arch/arm/mach-zynq/Kconfig
> @@ -8,6 +8,7 @@ config ARCH_ZYNQ
> select ICST
> select MIGHT_HAVE_CACHE_L2X0
> select USE_OF
> + select HAVE_SMP
> select SPARSE_IRQ
> select CADENCE_TTC_TIMER
> help
> diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
> index 13ee09b..b595d22 100644
> --- a/arch/arm/mach-zynq/Makefile
> +++ b/arch/arm/mach-zynq/Makefile
> @@ -4,3 +4,4 @@
>
> # Common support
> obj-y := common.o slcr.o
> +obj-$(CONFIG_SMP) += headsmp.o platsmp.o
> diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> index 4ad3560..da93957 100644
> --- a/arch/arm/mach-zynq/common.c
> +++ b/arch/arm/mach-zynq/common.c
> @@ -104,6 +104,7 @@ static const char *xilinx_dt_match[] = {
> };
>
> MACHINE_START(XILINX_EP107, "Xilinx Zynq Platform")
> + .smp = smp_ops(zynq_smp_ops),
> .map_io = xilinx_map_io,
> .init_irq = irqchip_init,
> .init_machine = xilinx_init_machine,
> diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
> index e5628f7..33cdc23 100644
> --- a/arch/arm/mach-zynq/common.h
> +++ b/arch/arm/mach-zynq/common.h
> @@ -19,6 +19,17 @@
>
> extern int slcr_init(void);
> extern void slcr_system_reset(void);
> +extern void slcr_cpu_stop(int cpu);
> +extern void slcr_cpu_start(int cpu);
> +
> +#ifdef CONFIG_SMP
> +extern void secondary_startup(void);
> +extern char zynq_secondary_trampoline;
> +extern char zynq_secondary_trampoline_jump;
> +extern char zynq_secondary_trampoline_end;
> +extern int __cpuinit zynq_cpun_start(u32 address, int cpu);
> +extern struct smp_operations zynq_smp_ops __initdata;
> +#endif
>
> extern void __iomem *zynq_slcr_base;
> extern void __iomem *scu_base;
> diff --git a/arch/arm/mach-zynq/headsmp.S b/arch/arm/mach-zynq/headsmp.S
> new file mode 100644
> index 0000000..d183cd2
> --- /dev/null
> +++ b/arch/arm/mach-zynq/headsmp.S
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright (c) 2013 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + * Copyright (c) 2012-2013 Xilinx
> + *
> + * 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/linkage.h>
> +#include <linux/init.h>
> +
> + __CPUINIT
> +
> +ENTRY(zynq_secondary_trampoline)
> + ldr r0, [pc]
> + bx r0
> +.globl zynq_secondary_trampoline_jump
> +zynq_secondary_trampoline_jump:
> + /* Space for jumping address */
> + .word /* cpu 1 */
> +.globl zynq_secondary_trampoline_end
> +zynq_secondary_trampoline_end:
> +
> +ENDPROC(zynq_secondary_trampoline)
> diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
> new file mode 100644
> index 0000000..062b319
> --- /dev/null
> +++ b/arch/arm/mach-zynq/platsmp.c
> @@ -0,0 +1,149 @@
> +/*
> + * This file contains Xilinx specific SMP code, used to start up
> + * the second processor.
> + *
> + * Copyright (C) 2011-2013 Xilinx
> + *
> + * based on linux/arch/arm/mach-realview/platsmp.c
> + *
> + * Copyright (C) 2002 ARM Ltd.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + */
> +
> +#include <linux/export.h>
> +#include <linux/jiffies.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <asm/cacheflush.h>
> +#include <asm/smp_scu.h>
> +#include <linux/irqchip/arm-gic.h>
> +#include "common.h"
> +
> +/*
> + * Store number of cores in the system
> + * Because of scu_get_core_count() must be in __init section and can't
> + * be called from zynq_cpun_start() because it is in __cpuinit section.
> + */
> +static int ncores;
> +
> +/* Secondary CPU kernel startup is a 2 step process. The primary CPU
> + * starts the secondary CPU by giving it the address of the kernel and
> + * then sending it an event to wake it up. The secondary CPU then
> + * starts the kernel and tells the primary CPU it's up and running.
> + */
> +static void __cpuinit zynq_secondary_init(unsigned int cpu)
> +{
> + /*
> + * if any interrupts are already enabled for the primary
> + * core (e.g. timer irq), then they will not have been enabled
> + * for us: do so
> + */
> + gic_secondary_init(0);
> +}
> +
> +int __cpuinit zynq_cpun_start(u32 address, int cpu)
> +{
> + u32 trampoline_code_size = &zynq_secondary_trampoline_end -
> + &zynq_secondary_trampoline;
> +
> + if (cpu > ncores) {
> + pr_warn("CPU No. is not available in the system\n");
> + return -1;
> + }
> +
> + /* MS: Expectation that SLCR are directly map and accessible */
> + /* Not possible to jump to non aligned address */
> + if (!(address & 3) && (!address || (address >= trampoline_code_size))) {
> + /* Store pointer to ioremap area which points to address 0x0 */
> + static u8 __iomem *zero;
> + u32 trampoline_size = &zynq_secondary_trampoline_jump -
> + &zynq_secondary_trampoline;
> +
> + slcr_cpu_stop(cpu);
> +
> + if (__pa(PAGE_OFFSET)) {
> + zero = ioremap(0, trampoline_code_size);
> + if (!zero) {
> + pr_warn("BOOTUP jump vectors not accessible\n");
> + return -1;
> + }
> + } else {
> + zero = (__force u8 __iomem *)PAGE_OFFSET;
> + }
> +
> + /*
> + * This is elegant way how to jump to any address
> + * 0x0: Load address at 0x8 to r0
> + * 0x4: Jump by mov instruction
> + * 0x8: Jumping address
> + */
> + memcpy((__force void *)zero, &zynq_secondary_trampoline,
> + trampoline_size);
> + writel(address, zero + trampoline_size + sizeof(int));
Please remove this + sizeof(int). I forget to remove it from this version.
Because I tested Rob's idea to have jump table based on cpu id.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 09/10] arm: zynq: Add hotplug support
2013-03-26 17:43 Zynq core changes v2 Michal Simek
` (7 preceding siblings ...)
2013-03-26 17:43 ` [PATCH v2 08/10] arm: zynq: Add smp support Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 17:43 ` [PATCH v2 10/10] arm: zynq: Add cpuidle support Michal Simek
9 siblings, 0 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
v2: Change platform_cpu_die name function to zynq_platform_cpu_dio
Fix header
Add cpu compilation flags to be able to build on V6 too
---
arch/arm/mach-zynq/Makefile | 3 ++
arch/arm/mach-zynq/common.h | 3 ++
arch/arm/mach-zynq/hotplug.c | 104 ++++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-zynq/platsmp.c | 3 ++
4 files changed, 113 insertions(+)
create mode 100644 arch/arm/mach-zynq/hotplug.c
diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
index b595d22..1b25d92 100644
--- a/arch/arm/mach-zynq/Makefile
+++ b/arch/arm/mach-zynq/Makefile
@@ -4,4 +4,7 @@
# Common support
obj-y := common.o slcr.o
+CFLAGS_REMOVE_hotplug.o =-march=armv6k
+CFLAGS_hotplug.o =-Wa,-march=armv7-a -mcpu=cortex-a9
+obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
index 33cdc23..7a1eaec 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/arch/arm/mach-zynq/common.h
@@ -34,4 +34,7 @@ extern struct smp_operations zynq_smp_ops __initdata;
extern void __iomem *zynq_slcr_base;
extern void __iomem *scu_base;
+/* Hotplug */
+extern void zynq_platform_cpu_die(unsigned int cpu);
+
#endif
diff --git a/arch/arm/mach-zynq/hotplug.c b/arch/arm/mach-zynq/hotplug.c
new file mode 100644
index 0000000..64e3d6ec
--- /dev/null
+++ b/arch/arm/mach-zynq/hotplug.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright (C) 2012-2013 Xilinx
+ *
+ * based on linux/arch/arm/mach-realview/hotplug.c
+ *
+ * Copyright (C) 2002 ARM Ltd.
+ * All Rights Reserved
+ *
+ * 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/kernel.h>
+#include <linux/errno.h>
+#include <linux/smp.h>
+
+#include <asm/cacheflush.h>
+#include <asm/cp15.h>
+#include "common.h"
+
+static inline void cpu_enter_lowpower(void)
+{
+ unsigned int v;
+
+ flush_cache_all();
+ asm volatile(
+ " mcr p15, 0, %1, c7, c5, 0\n"
+ " dsb\n"
+ /*
+ * Turn off coherency
+ */
+ " mrc p15, 0, %0, c1, c0, 1\n"
+ " bic %0, %0, #0x40\n"
+ " mcr p15, 0, %0, c1, c0, 1\n"
+ " mrc p15, 0, %0, c1, c0, 0\n"
+ " bic %0, %0, %2\n"
+ " mcr p15, 0, %0, c1, c0, 0\n"
+ : "=&r" (v)
+ : "r" (0), "Ir" (CR_C)
+ : "cc");
+}
+
+static inline void cpu_leave_lowpower(void)
+{
+ unsigned int v;
+
+ asm volatile(
+ " mrc p15, 0, %0, c1, c0, 0\n"
+ " orr %0, %0, %1\n"
+ " mcr p15, 0, %0, c1, c0, 0\n"
+ " mrc p15, 0, %0, c1, c0, 1\n"
+ " orr %0, %0, #0x40\n"
+ " mcr p15, 0, %0, c1, c0, 1\n"
+ : "=&r" (v)
+ : "Ir" (CR_C)
+ : "cc");
+}
+
+static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
+{
+ /*
+ * there is no power-control hardware on this platform, so all
+ * we can do is put the core into WFI; this is safe as the calling
+ * code will have already disabled interrupts
+ */
+ for (;;) {
+ dsb();
+ wfi();
+
+ /*
+ * Getting here, means that we have come out of WFI without
+ * having been woken up - this shouldn't happen
+ *
+ * Just note it happening - when we're woken, we can report
+ * its occurrence.
+ */
+ (*spurious)++;
+ }
+}
+
+/*
+ * platform-specific code to shutdown a CPU
+ *
+ * Called with IRQs disabled
+ */
+void zynq_platform_cpu_die(unsigned int cpu)
+{
+ int spurious = 0;
+
+ /*
+ * we're ready for shutdown now, so do it
+ */
+ cpu_enter_lowpower();
+ platform_do_lowpower(cpu, &spurious);
+
+ /*
+ * bring this CPU back into the world of cache
+ * coherency, and then restore interrupts
+ */
+ cpu_leave_lowpower();
+
+ if (spurious)
+ pr_warn("CPU%u: %u spurious wakeup calls\n", cpu, spurious);
+}
diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
index 062b319..b1be091 100644
--- a/arch/arm/mach-zynq/platsmp.c
+++ b/arch/arm/mach-zynq/platsmp.c
@@ -146,4 +146,7 @@ struct smp_operations zynq_smp_ops __initdata = {
.smp_prepare_cpus = zynq_smp_prepare_cpus,
.smp_secondary_init = zynq_secondary_init,
.smp_boot_secondary = zynq_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_die = zynq_platform_cpu_die,
+#endif
};
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 10/10] arm: zynq: Add cpuidle support
2013-03-26 17:43 Zynq core changes v2 Michal Simek
` (8 preceding siblings ...)
2013-03-26 17:43 ` [PATCH v2 09/10] arm: zynq: Add hotplug support Michal Simek
@ 2013-03-26 17:43 ` Michal Simek
2013-03-26 21:46 ` Arnd Bergmann
2013-03-27 10:07 ` Daniel Lezcano
9 siblings, 2 replies; 42+ messages in thread
From: Michal Simek @ 2013-03-26 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Add support for cpuidle.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
v2: Fix file header
---
arch/arm/mach-zynq/Makefile | 1 +
arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 134 insertions(+)
create mode 100644 arch/arm/mach-zynq/cpuidle.c
diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
index 1b25d92..238b8f9 100644
--- a/arch/arm/mach-zynq/Makefile
+++ b/arch/arm/mach-zynq/Makefile
@@ -7,4 +7,5 @@ obj-y := common.o slcr.o
CFLAGS_REMOVE_hotplug.o =-march=armv6k
CFLAGS_hotplug.o =-Wa,-march=armv7-a -mcpu=cortex-a9
obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
+obj-$(CONFIG_CPU_IDLE) += cpuidle.o
obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/mach-zynq/cpuidle.c b/arch/arm/mach-zynq/cpuidle.c
new file mode 100644
index 0000000..4ed8855
--- /dev/null
+++ b/arch/arm/mach-zynq/cpuidle.c
@@ -0,0 +1,133 @@
+/*
+ * Copyright (C) 2012-2013 Xilinx
+ *
+ * CPU idle support for Xilinx
+ *
+ * based on arch/arm/mach-at91/cpuidle.c
+ *
+ * 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.
+ *
+ * The cpu idle uses wait-for-interrupt and RAM self refresh in order
+ * to implement two idle states -
+ * #1 wait-for-interrupt
+ * #2 wait-for-interrupt and RAM self refresh
+ *
+ * Note that this code is only intended as a prototype and is not tested
+ * well yet, or tuned for the Xilinx Cortex A9. Also note that for a
+ * tickless kernel, high res timers must not be turned on. The cpuidle
+ * framework must also be turned on in the kernel.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/cpuidle.h>
+#include <linux/io.h>
+#include <linux/export.h>
+#include <linux/clockchips.h>
+#include <linux/cpu_pm.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <asm/proc-fns.h>
+
+#define XILINX_MAX_STATES 1
+
+static DEFINE_PER_CPU(struct cpuidle_device, xilinx_cpuidle_device);
+
+/* Actual code that puts the SoC in different idle states */
+static int xilinx_enter_idle(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index)
+{
+ struct timeval before, after;
+ int idle_time;
+
+ local_irq_disable();
+ do_gettimeofday(&before);
+
+ if (index == 0)
+ /* Wait for interrupt state */
+ cpu_do_idle();
+
+ else if (index == 1) {
+ unsigned int cpu_id = smp_processor_id();
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id);
+
+ /* Devices must be stopped here */
+ cpu_pm_enter();
+
+ /* Add code for DDR self refresh start */
+
+ cpu_do_idle();
+ /*cpu_suspend(foo, bar);*/
+
+ /* Add code for DDR self refresh stop */
+
+ cpu_pm_exit();
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id);
+ }
+
+ do_gettimeofday(&after);
+ local_irq_enable();
+ idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
+ (after.tv_usec - before.tv_usec);
+
+ dev->last_residency = idle_time;
+ return index;
+}
+
+static struct cpuidle_driver xilinx_idle_driver = {
+ .name = "xilinx_idle",
+ .owner = THIS_MODULE,
+ .state_count = XILINX_MAX_STATES,
+ /* Wait for interrupt state */
+ .states[0] = {
+ .enter = xilinx_enter_idle,
+ .exit_latency = 1,
+ .target_residency = 10000,
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .name = "WFI",
+ .desc = "Wait for interrupt",
+ },
+ /* Wait for interrupt and RAM self refresh state */
+ .states[1] = {
+ .enter = xilinx_enter_idle,
+ .exit_latency = 10,
+ .target_residency = 10000,
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .name = "RAM_SR",
+ .desc = "WFI and RAM Self Refresh",
+ },
+};
+
+/* Initialize CPU idle by registering the idle states */
+static int xilinx_init_cpuidle(void)
+{
+ unsigned int cpu;
+ struct cpuidle_device *device;
+ int ret;
+
+ ret = cpuidle_register_driver(&xilinx_idle_driver);
+ if (ret) {
+ pr_err("Registering Xilinx CpuIdle Driver failed.\n");
+ return ret;
+ }
+
+ for_each_possible_cpu(cpu) {
+ device = &per_cpu(xilinx_cpuidle_device, cpu);
+ device->state_count = XILINX_MAX_STATES;
+ device->cpu = cpu;
+ ret = cpuidle_register_device(device);
+ if (ret) {
+ pr_err("xilinx_init_cpuidle: Failed registering\n");
+ return ret;
+ }
+ }
+
+ pr_info("Xilinx CpuIdle Driver started\n");
+ return 0;
+}
+device_initcall(xilinx_init_cpuidle);
--
1.7.9.7
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 10/10] arm: zynq: Add cpuidle support
2013-03-26 17:43 ` [PATCH v2 10/10] arm: zynq: Add cpuidle support Michal Simek
@ 2013-03-26 21:46 ` Arnd Bergmann
2013-03-27 7:56 ` Michal Simek
2013-03-27 10:07 ` Daniel Lezcano
1 sibling, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2013-03-26 21:46 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 26 March 2013, Michal Simek wrote:
> Add support for cpuidle.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> v2: Fix file header
> ---
> arch/arm/mach-zynq/Makefile | 1 +
> arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 134 insertions(+)
> create mode 100644 arch/arm/mach-zynq/cpuidle.c
Can you move that file to drivers/cpuidle instead?
>+/* Initialize CPU idle by registering the idle states */
>+static int xilinx_init_cpuidle(void)
>+{
>+ unsigned int cpu;
>+ struct cpuidle_device *device;
>+ int ret;
>+
>+ ret = cpuidle_register_driver(&xilinx_idle_driver);
>+ if (ret) {
>+ pr_err("Registering Xilinx CpuIdle Driver failed.\n");
>+ return ret;
>+ }
I think you have to check that you actually run on a Zynq system before
registering the driver.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 10/10] arm: zynq: Add cpuidle support
2013-03-26 21:46 ` Arnd Bergmann
@ 2013-03-27 7:56 ` Michal Simek
2013-03-27 9:27 ` Arnd Bergmann
0 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-27 7:56 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/26 Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 26 March 2013, Michal Simek wrote:
>> Add support for cpuidle.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>> v2: Fix file header
>> ---
>> arch/arm/mach-zynq/Makefile | 1 +
>> arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 134 insertions(+)
>> create mode 100644 arch/arm/mach-zynq/cpuidle.c
>
> Can you move that file to drivers/cpuidle instead?
I can't see any problem in it right now.
>>+/* Initialize CPU idle by registering the idle states */
>>+static int xilinx_init_cpuidle(void)
>>+{
>>+ unsigned int cpu;
>>+ struct cpuidle_device *device;
>>+ int ret;
>>+
>>+ ret = cpuidle_register_driver(&xilinx_idle_driver);
>>+ if (ret) {
>>+ pr_err("Registering Xilinx CpuIdle Driver failed.\n");
>>+ return ret;
>>+ }
>
> I think you have to check that you actually run on a Zynq system before
> registering the driver.
Is there any elegant way how to do it?
I see that Rob is checking compatible machine with of_machine_is_compatible().
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 10/10] arm: zynq: Add cpuidle support
2013-03-27 7:56 ` Michal Simek
@ 2013-03-27 9:27 ` Arnd Bergmann
2013-03-27 10:15 ` Daniel Lezcano
0 siblings, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2013-03-27 9:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 27 March 2013, Michal Simek wrote:
> 2013/3/26 Arnd Bergmann <arnd@arndb.de>:
> > On Tuesday 26 March 2013, Michal Simek wrote:
> >> arch/arm/mach-zynq/Makefile | 1 +
> >> arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 134 insertions(+)
> >> create mode 100644 arch/arm/mach-zynq/cpuidle.c
> >
> > Can you move that file to drivers/cpuidle instead?
>
> I can't see any problem in it right now.
Ok.
Adding Daniel Lezcano to CC, he's currently doing a lot of work on
the cpuidle drivers and may have some input as well.
> >>+/* Initialize CPU idle by registering the idle states */
> >>+static int xilinx_init_cpuidle(void)
> >>+{
> >>+ unsigned int cpu;
> >>+ struct cpuidle_device *device;
> >>+ int ret;
> >>+
> >>+ ret = cpuidle_register_driver(&xilinx_idle_driver);
> >>+ if (ret) {
> >>+ pr_err("Registering Xilinx CpuIdle Driver failed.\n");
> >>+ return ret;
> >>+ }
> >
> > I think you have to check that you actually run on a Zynq system before
> > registering the driver.
>
> Is there any elegant way how to do it?
> I see that Rob is checking compatible machine with of_machine_is_compatible().
>
Most drivers use some resource that they can check the presence of, which is
better than checking the global "compatible" property of the system. I don't
see any of that in your driver, but I may be missing something. What is it
specifically that makes this cpuidle driver special to Zynq and different
from other cpuidle drivers? Is that difference something we can describe
using the device tree in more specific terms than the root compatible
property?
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 10/10] arm: zynq: Add cpuidle support
2013-03-27 9:27 ` Arnd Bergmann
@ 2013-03-27 10:15 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2013-03-27 10:15 UTC (permalink / raw)
To: linux-arm-kernel
On 03/27/2013 10:27 AM, Arnd Bergmann wrote:
> On Wednesday 27 March 2013, Michal Simek wrote:
>> 2013/3/26 Arnd Bergmann <arnd@arndb.de>:
>>> On Tuesday 26 March 2013, Michal Simek wrote:
>>>> arch/arm/mach-zynq/Makefile | 1 +
>>>> arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 134 insertions(+)
>>>> create mode 100644 arch/arm/mach-zynq/cpuidle.c
>>>
>>> Can you move that file to drivers/cpuidle instead?
>>
>> I can't see any problem in it right now.
>
> Ok.
>
> Adding Daniel Lezcano to CC, he's currently doing a lot of work on
> the cpuidle drivers and may have some input as well.
Thanks Arnd,
I commented the driver.
I am in favor for moving also this driver to drivers/cpuidle.
I suggest in the future, you nack any new drivers which is not located
in drivers/cpuidle. Let's try to move the current ones out of
arch/arm/mach-* and concentrate the location in a single place.
Maintaining all these drivers by jumping branch to branch like a monkey
is really painful :)
>>>> +/* Initialize CPU idle by registering the idle states */
>>>> +static int xilinx_init_cpuidle(void)
>>>> +{
>>>> + unsigned int cpu;
>>>> + struct cpuidle_device *device;
>>>> + int ret;
>>>> +
>>>> + ret = cpuidle_register_driver(&xilinx_idle_driver);
>>>> + if (ret) {
>>>> + pr_err("Registering Xilinx CpuIdle Driver failed.\n");
>>>> + return ret;
>>>> + }
>>>
>>> I think you have to check that you actually run on a Zynq system before
>>> registering the driver.
>>
>> Is there any elegant way how to do it?
>> I see that Rob is checking compatible machine with of_machine_is_compatible().
>>
>
> Most drivers use some resource that they can check the presence of, which is
> better than checking the global "compatible" property of the system. I don't
> see any of that in your driver, but I may be missing something. What is it
> specifically that makes this cpuidle driver special to Zynq and different
> from other cpuidle drivers? Is that difference something we can describe
> using the device tree in more specific terms than the root compatible
> property?
>
> Arnd
>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 10/10] arm: zynq: Add cpuidle support
2013-03-26 17:43 ` [PATCH v2 10/10] arm: zynq: Add cpuidle support Michal Simek
2013-03-26 21:46 ` Arnd Bergmann
@ 2013-03-27 10:07 ` Daniel Lezcano
2013-03-27 10:31 ` Michal Simek
1 sibling, 1 reply; 42+ messages in thread
From: Daniel Lezcano @ 2013-03-27 10:07 UTC (permalink / raw)
To: linux-arm-kernel
On 03/26/2013 06:43 PM, Michal Simek wrote:
> Add support for cpuidle.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Hi Michal,
at the first glance, the driver won't enter in the self refresh state.
If I am right, by checking:
/sys/devices/system/cpu/cpu0/cpuidle/state1/usage, it should be always zero.
Also, I see there is self refresh but you save the cpu context, so it is
cpu shutdown no ?
In this case, 3 states would be needed, WFI, self-refresh, powerdown, no ?
The comments below applies for this driver based on linux-pm-next.
https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
> ---
> v2: Fix file header
> ---
> arch/arm/mach-zynq/Makefile | 1 +
> arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 134 insertions(+)
> create mode 100644 arch/arm/mach-zynq/cpuidle.c
>
> diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
> index 1b25d92..238b8f9 100644
> --- a/arch/arm/mach-zynq/Makefile
> +++ b/arch/arm/mach-zynq/Makefile
> @@ -7,4 +7,5 @@ obj-y := common.o slcr.o
> CFLAGS_REMOVE_hotplug.o =-march=armv6k
> CFLAGS_hotplug.o =-Wa,-march=armv7-a -mcpu=cortex-a9
> obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
> +obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> obj-$(CONFIG_SMP) += headsmp.o platsmp.o
> diff --git a/arch/arm/mach-zynq/cpuidle.c b/arch/arm/mach-zynq/cpuidle.c
> new file mode 100644
> index 0000000..4ed8855
> --- /dev/null
> +++ b/arch/arm/mach-zynq/cpuidle.c
> @@ -0,0 +1,133 @@
> +/*
> + * Copyright (C) 2012-2013 Xilinx
> + *
> + * CPU idle support for Xilinx
> + *
> + * based on arch/arm/mach-at91/cpuidle.c
> + *
> + * 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.
> + *
> + * The cpu idle uses wait-for-interrupt and RAM self refresh in order
> + * to implement two idle states -
> + * #1 wait-for-interrupt
> + * #2 wait-for-interrupt and RAM self refresh
> + *
> + * Note that this code is only intended as a prototype and is not tested
> + * well yet, or tuned for the Xilinx Cortex A9. Also note that for a
> + * tickless kernel, high res timers must not be turned on. The cpuidle
> + * framework must also be turned on in the kernel.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/cpuidle.h>
> +#include <linux/io.h>
> +#include <linux/export.h>
> +#include <linux/clockchips.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <asm/proc-fns.h>
> +
> +#define XILINX_MAX_STATES 1
You defined XILINX_MAX_STATES as 1, it should be 2.
> +static DEFINE_PER_CPU(struct cpuidle_device, xilinx_cpuidle_device);
> +
> +/* Actual code that puts the SoC in different idle states */
> +static int xilinx_enter_idle(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> +{
> + struct timeval before, after;
> + int idle_time;
> +
> + local_irq_disable();
Remove this line, it is done by the caller.
> + do_gettimeofday(&before);
Remove this line, that will be taken into account by the driver's
en_core_tk_irqen flag.
> +
> + if (index == 0)
> + /* Wait for interrupt state */
> + cpu_do_idle();
Don't check the index, see in the driver initialization comment below
the defaul wfi state. You will have one state entering WFI and the
xilinx_enter_idle function entering with always index 1.
> + else if (index == 1) {
> + unsigned int cpu_id = smp_processor_id();
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id);
Remove this line, CPUIDLE_FLAG_TIMER_STOP will tell the cpuidle
framework to handle that.
> + /* Devices must be stopped here */
> + cpu_pm_enter();
> +
> + /* Add code for DDR self refresh start */
> +
> + cpu_do_idle();
> + /*cpu_suspend(foo, bar);*/
> +
> + /* Add code for DDR self refresh stop */
> +
> + cpu_pm_exit();
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id);
Remove this line, CPUIDLE_FLAG_TIMER_STOP will tell the cpuidle
framework to handle that.
> + }
> +
> + do_gettimeofday(&after);
> + local_irq_enable();
> + idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
> + (after.tv_usec - before.tv_usec);
> +
> + dev->last_residency = idle_time;
Remove these lines about time computation, that is handled by the caller.
> + return index;
> +}
> +
> +static struct cpuidle_driver xilinx_idle_driver = {
> + .name = "xilinx_idle",
> + .owner = THIS_MODULE,
> + .state_count = XILINX_MAX_STATES,
> + /* Wait for interrupt state */
> + .states[0] = {
> + .enter = xilinx_enter_idle,
> + .exit_latency = 1,
> + .target_residency = 10000,
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .name = "WFI",
> + .desc = "Wait for interrupt",
> + },
> + /* Wait for interrupt and RAM self refresh state */
> + .states[1] = {
> + .enter = xilinx_enter_idle,
> + .exit_latency = 10,
> + .target_residency = 10000,
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .name = "RAM_SR",
> + .desc = "WFI and RAM Self Refresh",
> + },
> +};
static struct cpuidle_driver xilinx_idle_driver = {
.name = "xilinx_idle",
.owner = THIS_MODULE,
.states = {
ARM_CPUIDLE_WFI_STATE,
{
.enter = xilinx_enter_idle,
.exit_latency = 10,
.target_residency = 10000,
.flags = CPUIDLE_FLAG_TIME_VALID,
CPUIDLE_FLAG_TIMER_STOP,
.name = "RAM_SR",
.desc = "WFI and RAM Self Refresh",
},
},
.safe_state_index = 0,
.state_count = XILINX_MAX_STATES,
};
> +/* Initialize CPU idle by registering the idle states */
> +static int xilinx_init_cpuidle(void)
> +{
> + unsigned int cpu;
> + struct cpuidle_device *device;
> + int ret;
> +
> + ret = cpuidle_register_driver(&xilinx_idle_driver);
> + if (ret) {
> + pr_err("Registering Xilinx CpuIdle Driver failed.\n");
> + return ret;
> + }
> +
> + for_each_possible_cpu(cpu) {
> + device = &per_cpu(xilinx_cpuidle_device, cpu);
> + device->state_count = XILINX_MAX_STATES;
This is initialization is done from the cpuidle_register_device. As
drv->state_count == device->state_count.
> + device->cpu = cpu;
> + ret = cpuidle_register_device(device);
> + if (ret) {
> + pr_err("xilinx_init_cpuidle: Failed registering\n");
> + return ret;
> + }
> + }
> +
> + pr_info("Xilinx CpuIdle Driver started\n");
> + return 0;
> +}
> +device_initcall(xilinx_init_cpuidle);
>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 10/10] arm: zynq: Add cpuidle support
2013-03-27 10:07 ` Daniel Lezcano
@ 2013-03-27 10:31 ` Michal Simek
2013-03-27 10:37 ` Daniel Lezcano
0 siblings, 1 reply; 42+ messages in thread
From: Michal Simek @ 2013-03-27 10:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Daniel,
2013/3/27 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 03/26/2013 06:43 PM, Michal Simek wrote:
>> Add support for cpuidle.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>
> Hi Michal,
>
> at the first glance, the driver won't enter in the self refresh state.
>
> If I am right, by checking:
>
> /sys/devices/system/cpu/cpu0/cpuidle/state1/usage, it should be always zero.
I have found this some mins ago.
I means that currently it is not zero.
zynq> cat /sys/devices/system/cpu/cpu0/cpuidle/state1/usage
96931
zynq> cat /sys/devices/system/cpu/cpu0/cpuidle/state0/usage
8396
> Also, I see there is self refresh but you save the cpu context, so it is
> cpu shutdown no ?
>
> In this case, 3 states would be needed, WFI, self-refresh, powerdown, no ?
Let me talk to Soren how this should be done on zynq.
> The comments below applies for this driver based on linux-pm-next.
>
> https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
Appreciate that - the origin author of this code is not longer with Xilinx
that's why hard to tell why it was done in this way. This version
was done 2011-02. But that's not excuse just explanation.
It mean that this driver should also go through this tree, right?
>
>> ---
>> v2: Fix file header
>> ---
>> arch/arm/mach-zynq/Makefile | 1 +
>> arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 134 insertions(+)
>> create mode 100644 arch/arm/mach-zynq/cpuidle.c
>>
>> diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
>> index 1b25d92..238b8f9 100644
>> --- a/arch/arm/mach-zynq/Makefile
>> +++ b/arch/arm/mach-zynq/Makefile
>> @@ -7,4 +7,5 @@ obj-y := common.o slcr.o
>> CFLAGS_REMOVE_hotplug.o =-march=armv6k
>> CFLAGS_hotplug.o =-Wa,-march=armv7-a -mcpu=cortex-a9
>> obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
>> +obj-$(CONFIG_CPU_IDLE) += cpuidle.o
>> obj-$(CONFIG_SMP) += headsmp.o platsmp.o
>> diff --git a/arch/arm/mach-zynq/cpuidle.c b/arch/arm/mach-zynq/cpuidle.c
>> new file mode 100644
>> index 0000000..4ed8855
>> --- /dev/null
>> +++ b/arch/arm/mach-zynq/cpuidle.c
>> @@ -0,0 +1,133 @@
>> +/*
>> + * Copyright (C) 2012-2013 Xilinx
>> + *
>> + * CPU idle support for Xilinx
>> + *
>> + * based on arch/arm/mach-at91/cpuidle.c
>> + *
>> + * 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.
>> + *
>> + * The cpu idle uses wait-for-interrupt and RAM self refresh in order
>> + * to implement two idle states -
>> + * #1 wait-for-interrupt
>> + * #2 wait-for-interrupt and RAM self refresh
>> + *
>> + * Note that this code is only intended as a prototype and is not tested
>> + * well yet, or tuned for the Xilinx Cortex A9. Also note that for a
>> + * tickless kernel, high res timers must not be turned on. The cpuidle
>> + * framework must also be turned on in the kernel.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/cpuidle.h>
>> +#include <linux/io.h>
>> +#include <linux/export.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/cpu_pm.h>
>> +#include <linux/clk.h>
>> +#include <linux/err.h>
>> +#include <asm/proc-fns.h>
>> +
>> +#define XILINX_MAX_STATES 1
>
> You defined XILINX_MAX_STATES as 1, it should be 2.
yep. Done.
>
>> +static DEFINE_PER_CPU(struct cpuidle_device, xilinx_cpuidle_device);
>> +
>> +/* Actual code that puts the SoC in different idle states */
>> +static int xilinx_enter_idle(struct cpuidle_device *dev,
>> + struct cpuidle_driver *drv, int index)
>> +{
>> + struct timeval before, after;
>> + int idle_time;
>> +
>> + local_irq_disable();
>
> Remove this line, it is done by the caller.
Where is that caller function? I can trace it but this maybe faster way. :-)
>
>> + do_gettimeofday(&before);
>
> Remove this line, that will be taken into account by the driver's
> en_core_tk_irqen flag.
ok
>
>> +
>> + if (index == 0)
>> + /* Wait for interrupt state */
>> + cpu_do_idle();
>
> Don't check the index, see in the driver initialization comment below
> the defaul wfi state. You will have one state entering WFI and the
> xilinx_enter_idle function entering with always index 1.
ok
>
>> + else if (index == 1) {
>> + unsigned int cpu_id = smp_processor_id();
>> +
>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id);
>
> Remove this line, CPUIDLE_FLAG_TIMER_STOP will tell the cpuidle
> framework to handle that.
ok
>
>> + /* Devices must be stopped here */
>> + cpu_pm_enter();
>> +
>> + /* Add code for DDR self refresh start */
>> +
>> + cpu_do_idle();
>> + /*cpu_suspend(foo, bar);*/
>> +
>> + /* Add code for DDR self refresh stop */
>> +
>> + cpu_pm_exit();
>> +
>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id);
>
> Remove this line, CPUIDLE_FLAG_TIMER_STOP will tell the cpuidle
> framework to handle that.
ok
>
>> + }
>> +
>> + do_gettimeofday(&after);
>> + local_irq_enable();
>> + idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
>> + (after.tv_usec - before.tv_usec);
>> +
>> + dev->last_residency = idle_time;
>
> Remove these lines about time computation, that is handled by the caller.
ok
>
>> + return index;
>> +}
>> +
>> +static struct cpuidle_driver xilinx_idle_driver = {
>> + .name = "xilinx_idle",
>> + .owner = THIS_MODULE,
>> + .state_count = XILINX_MAX_STATES,
>> + /* Wait for interrupt state */
>> + .states[0] = {
>> + .enter = xilinx_enter_idle,
>> + .exit_latency = 1,
>> + .target_residency = 10000,
>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>> + .name = "WFI",
>> + .desc = "Wait for interrupt",
>> + },
>> + /* Wait for interrupt and RAM self refresh state */
>> + .states[1] = {
>> + .enter = xilinx_enter_idle,
>> + .exit_latency = 10,
>> + .target_residency = 10000,
>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>> + .name = "RAM_SR",
>> + .desc = "WFI and RAM Self Refresh",
>> + },
>> +};
>
> static struct cpuidle_driver xilinx_idle_driver = {
> .name = "xilinx_idle",
> .owner = THIS_MODULE,
> .states = {
> ARM_CPUIDLE_WFI_STATE,
> {
> .enter = xilinx_enter_idle,
> .exit_latency = 10,
> .target_residency = 10000,
> .flags = CPUIDLE_FLAG_TIME_VALID,
> CPUIDLE_FLAG_TIMER_STOP,
> .name = "RAM_SR",
> .desc = "WFI and RAM Self Refresh",
> },
> },
> .safe_state_index = 0,
> .state_count = XILINX_MAX_STATES,
> };
>
ok
>
>> +/* Initialize CPU idle by registering the idle states */
>> +static int xilinx_init_cpuidle(void)
>> +{
>> + unsigned int cpu;
>> + struct cpuidle_device *device;
>> + int ret;
>> +
>> + ret = cpuidle_register_driver(&xilinx_idle_driver);
>> + if (ret) {
>> + pr_err("Registering Xilinx CpuIdle Driver failed.\n");
>> + return ret;
>> + }
>> +
>> + for_each_possible_cpu(cpu) {
>> + device = &per_cpu(xilinx_cpuidle_device, cpu);
>> + device->state_count = XILINX_MAX_STATES;
>
> This is initialization is done from the cpuidle_register_device. As
> drv->state_count == device->state_count.
ok. Thanks you very much for your comments.
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v2 10/10] arm: zynq: Add cpuidle support
2013-03-27 10:31 ` Michal Simek
@ 2013-03-27 10:37 ` Daniel Lezcano
0 siblings, 0 replies; 42+ messages in thread
From: Daniel Lezcano @ 2013-03-27 10:37 UTC (permalink / raw)
To: linux-arm-kernel
On 03/27/2013 11:31 AM, Michal Simek wrote:
> Hi Daniel,
>
> 2013/3/27 Daniel Lezcano <daniel.lezcano@linaro.org>:
>> On 03/26/2013 06:43 PM, Michal Simek wrote:
>>> Add support for cpuidle.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>
>> Hi Michal,
>>
>> at the first glance, the driver won't enter in the self refresh state.
>>
>> If I am right, by checking:
>>
>> /sys/devices/system/cpu/cpu0/cpuidle/state1/usage, it should be always zero.
>
> I have found this some mins ago.
>
> I means that currently it is not zero.
>
> zynq> cat /sys/devices/system/cpu/cpu0/cpuidle/state1/usage
> 96931
> zynq> cat /sys/devices/system/cpu/cpu0/cpuidle/state0/usage
> 8396
>
>> Also, I see there is self refresh but you save the cpu context, so it is
>> cpu shutdown no ?
>>
>> In this case, 3 states would be needed, WFI, self-refresh, powerdown, no ?
>
> Let me talk to Soren how this should be done on zynq.
>
>
>> The comments below applies for this driver based on linux-pm-next.
>>
>> https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
>
> Appreciate that - the origin author of this code is not longer with Xilinx
> that's why hard to tell why it was done in this way. This version
> was done 2011-02. But that's not excuse just explanation.
>
> It mean that this driver should also go through this tree, right?
>
>
>>
>>> ---
>>> v2: Fix file header
>>> ---
>>> arch/arm/mach-zynq/Makefile | 1 +
>>> arch/arm/mach-zynq/cpuidle.c | 133 ++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 134 insertions(+)
>>> create mode 100644 arch/arm/mach-zynq/cpuidle.c
>>>
>>> diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
>>> index 1b25d92..238b8f9 100644
>>> --- a/arch/arm/mach-zynq/Makefile
>>> +++ b/arch/arm/mach-zynq/Makefile
>>> @@ -7,4 +7,5 @@ obj-y := common.o slcr.o
>>> CFLAGS_REMOVE_hotplug.o =-march=armv6k
>>> CFLAGS_hotplug.o =-Wa,-march=armv7-a -mcpu=cortex-a9
>>> obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
>>> +obj-$(CONFIG_CPU_IDLE) += cpuidle.o
>>> obj-$(CONFIG_SMP) += headsmp.o platsmp.o
>>> diff --git a/arch/arm/mach-zynq/cpuidle.c b/arch/arm/mach-zynq/cpuidle.c
>>> new file mode 100644
>>> index 0000000..4ed8855
>>> --- /dev/null
>>> +++ b/arch/arm/mach-zynq/cpuidle.c
>>> @@ -0,0 +1,133 @@
>>> +/*
>>> + * Copyright (C) 2012-2013 Xilinx
>>> + *
>>> + * CPU idle support for Xilinx
>>> + *
>>> + * based on arch/arm/mach-at91/cpuidle.c
>>> + *
>>> + * 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.
>>> + *
>>> + * The cpu idle uses wait-for-interrupt and RAM self refresh in order
>>> + * to implement two idle states -
>>> + * #1 wait-for-interrupt
>>> + * #2 wait-for-interrupt and RAM self refresh
>>> + *
>>> + * Note that this code is only intended as a prototype and is not tested
>>> + * well yet, or tuned for the Xilinx Cortex A9. Also note that for a
>>> + * tickless kernel, high res timers must not be turned on. The cpuidle
>>> + * framework must also be turned on in the kernel.
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/init.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/cpuidle.h>
>>> +#include <linux/io.h>
>>> +#include <linux/export.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/cpu_pm.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/err.h>
>>> +#include <asm/proc-fns.h>
>>> +
>>> +#define XILINX_MAX_STATES 1
>>
>> You defined XILINX_MAX_STATES as 1, it should be 2.
>
> yep. Done.
>
>>
>>> +static DEFINE_PER_CPU(struct cpuidle_device, xilinx_cpuidle_device);
>>> +
>>> +/* Actual code that puts the SoC in different idle states */
>>> +static int xilinx_enter_idle(struct cpuidle_device *dev,
>>> + struct cpuidle_driver *drv, int index)
>>> +{
>>> + struct timeval before, after;
>>> + int idle_time;
>>> +
>>> + local_irq_disable();
>>
>> Remove this line, it is done by the caller.
>
> Where is that caller function? I can trace it but this maybe faster way. :-)
In the idle loop arch/arm/kernel/process.c
>>> + do_gettimeofday(&before);
>>
>> Remove this line, that will be taken into account by the driver's
>> en_core_tk_irqen flag.
>
> ok
>
>>
>>> +
>>> + if (index == 0)
>>> + /* Wait for interrupt state */
>>> + cpu_do_idle();
>>
>> Don't check the index, see in the driver initialization comment below
>> the defaul wfi state. You will have one state entering WFI and the
>> xilinx_enter_idle function entering with always index 1.
>
> ok
>
>>
>>> + else if (index == 1) {
>>> + unsigned int cpu_id = smp_processor_id();
>>> +
>>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id);
>>
>> Remove this line, CPUIDLE_FLAG_TIMER_STOP will tell the cpuidle
>> framework to handle that.
>
> ok
>
>>
>>> + /* Devices must be stopped here */
>>> + cpu_pm_enter();
>>> +
>>> + /* Add code for DDR self refresh start */
>>> +
>>> + cpu_do_idle();
>>> + /*cpu_suspend(foo, bar);*/
>>> +
>>> + /* Add code for DDR self refresh stop */
>>> +
>>> + cpu_pm_exit();
>>> +
>>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id);
>>
>> Remove this line, CPUIDLE_FLAG_TIMER_STOP will tell the cpuidle
>> framework to handle that.
>
> ok
>
>>
>>> + }
>>> +
>>> + do_gettimeofday(&after);
>>> + local_irq_enable();
>>> + idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
>>> + (after.tv_usec - before.tv_usec);
>>> +
>>> + dev->last_residency = idle_time;
>>
>> Remove these lines about time computation, that is handled by the caller.
>
> ok
>
>>
>>> + return index;
>>> +}
>>> +
>>> +static struct cpuidle_driver xilinx_idle_driver = {
>>> + .name = "xilinx_idle",
>>> + .owner = THIS_MODULE,
>>> + .state_count = XILINX_MAX_STATES,
>>> + /* Wait for interrupt state */
>>> + .states[0] = {
>>> + .enter = xilinx_enter_idle,
>>> + .exit_latency = 1,
>>> + .target_residency = 10000,
>>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>>> + .name = "WFI",
>>> + .desc = "Wait for interrupt",
>>> + },
>>> + /* Wait for interrupt and RAM self refresh state */
>>> + .states[1] = {
>>> + .enter = xilinx_enter_idle,
>>> + .exit_latency = 10,
>>> + .target_residency = 10000,
>>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>>> + .name = "RAM_SR",
>>> + .desc = "WFI and RAM Self Refresh",
>>> + },
>>> +};
>>
>> static struct cpuidle_driver xilinx_idle_driver = {
>> .name = "xilinx_idle",
>> .owner = THIS_MODULE,
>> .states = {
>> ARM_CPUIDLE_WFI_STATE,
>> {
>> .enter = xilinx_enter_idle,
>> .exit_latency = 10,
>> .target_residency = 10000,
>> .flags = CPUIDLE_FLAG_TIME_VALID,
>> CPUIDLE_FLAG_TIMER_STOP,
>> .name = "RAM_SR",
>> .desc = "WFI and RAM Self Refresh",
>> },
>> },
>> .safe_state_index = 0,
>> .state_count = XILINX_MAX_STATES,
>> };
>>
>
> ok
>
>>
>>> +/* Initialize CPU idle by registering the idle states */
>>> +static int xilinx_init_cpuidle(void)
>>> +{
>>> + unsigned int cpu;
>>> + struct cpuidle_device *device;
>>> + int ret;
>>> +
>>> + ret = cpuidle_register_driver(&xilinx_idle_driver);
>>> + if (ret) {
>>> + pr_err("Registering Xilinx CpuIdle Driver failed.\n");
>>> + return ret;
>>> + }
>>> +
>>> + for_each_possible_cpu(cpu) {
>>> + device = &per_cpu(xilinx_cpuidle_device, cpu);
>>> + device->state_count = XILINX_MAX_STATES;
>>
>> This is initialization is done from the cpuidle_register_device. As
>> drv->state_count == device->state_count.
>
> ok. Thanks you very much for your comments.
> Michal
>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 42+ messages in thread