This patchset implements COMMON_CLK support for the zynq. At this
point, only the basic fundamental clocks are modelled, and only
passively; for rate calculation. of_clk bindings are implemented to
allow specifying clock/peripheral relationships in the device tree.
Patch 1 and 2 are a followup to my early patch: "ARM: zynq: move ttc
timer code to drivers/clocksource". Patch 1 moves the definition
sys_timer definition out of the ttc code, and into the zynq common code.
Patch 2 is the actual rename, and makefile cleanup.
Patch 3 adds a description of the second uart to zynq-ep107.dts. I did
this pre-split (patch 4), because I felt it might make reviewing easier.
Patch 4 uses zynq-ep107.dts as a reference to create zynq-7000.dtsi,
which is intended to be a common dtsi snippet for inclusion in
describing zynq-7000 based boards. zynq-zc702.dts is created as an
example consumer. The zynq-ep107.dts file is removed entirely (it
describes, presumably, a board not available to consumers).
Patch 5 is the real meat; it adds an implementation of the clk models
for the PLLs, the CPU clock network, and basic (simplified) clk models
for the essential peripherals (UART and the TTC).
Patch 6 removes CONFIG_OF conditional code from the xilinx uart driver.
The zynq kernel requires CONFIG_OF, and this hardware is not currently
used on any other non CONFIG_OF platform.
Patch 7 adds support to the xilinx_uartps driver to allow getting clock
rate information form the device tree.
Patch 8 implements DT support for the ttc, including pulling clock tree
info.
---
There are some specific concerns that I had that I would like some
guidance on:
Two identical timers on the board have historically been statically
allocated to act as the system clocksource, and the clockevent_device.
With patch 8, this distinction is done in the device tree by tweaking
with the compatible properties of which of the timers you want used for
what purpose.
I feel, however, that this is an abuse of the device tree, which should
only be used to describe hardware, not to layout a policy on how the
hardware is used.
So, if it's not in the device tree, then where? Do I go back to the
static allocation routine, such that the first matching ttc node in the
tree becomes the clockevent_device, and the second one a clocksource?
That seems like a hack.
Is it somehow possible to have all of the timers registered as both a
clocksource and a clockevent_device, and have some higher level logic
make the policy decision as to which timer is used for what?
An additional question regarding of_clk bindings:
my_clock {
#clock-cells = <0>;
clock-output-names = "my_out_clock";
};
node_a {
clocks = <&clk>;
clock-names = "my_clock";
clock-ranges;
node_b {
/* ... */
};
};
In this scenario, should I be expecting of_clk_get(node_b, 0) to
retrieve a handle to parent's consumed clock (due to clock-ranges)? I
could make this work using of_clk_get_by_name(node_b, "my_clock"), but I
was somewhat surprised the former didn't work.
Thanks (and sorry for the novel),
Josh
---
Josh Cartwright (8):
ARM: zynq: move arm-specific sys_timer out of ttc
ARM: zynq: move ttc timer code to drivers/clocksource
ARM: zynq: dts: add description of the second uart
ARM: zynq: dts: split up device tree
ARM: zynq: add COMMON_CLK support
serial: xilinx_uartps: kill CONFIG_OF conditional
serial: xilinx_uartps: get clock rate info from dts
clocksource: xilinx_ttc: add OF_CLK support
.../devicetree/bindings/clock/zynq-7000.txt | 55 ++++
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/zynq-7000.dtsi | 166 ++++++++++
arch/arm/boot/dts/zynq-ep107.dts | 63 ----
arch/arm/boot/dts/zynq-zc702.dts | 44 +++
arch/arm/mach-zynq/Makefile | 2 +-
arch/arm/mach-zynq/common.c | 29 +-
arch/arm/mach-zynq/timer.c | 298 -----------------
drivers/clk/Makefile | 1 +
drivers/clk/clk-zynq.c | 355 +++++++++++++++++++++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/xilinx_ttc.c | 326 +++++++++++++++++++
drivers/tty/serial/xilinx_uartps.c | 39 +--
include/linux/clk/zynq.h | 24 ++
.../common.h => include/linux/xilinx_ttc.h | 8 +-
16 files changed, 1022 insertions(+), 391 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/zynq-7000.txt
create mode 100644 arch/arm/boot/dts/zynq-7000.dtsi
delete mode 100644 arch/arm/boot/dts/zynq-ep107.dts
create mode 100644 arch/arm/boot/dts/zynq-zc702.dts
delete mode 100644 arch/arm/mach-zynq/timer.c
create mode 100644 drivers/clk/clk-zynq.c
create mode 100644 drivers/clocksource/xilinx_ttc.c
create mode 100644 include/linux/clk/zynq.h
rename arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h (81%)
--
1.8.0
The zynq-7000 has an additional UART at 0xE0001000. Describe it in the
device tree.
Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/boot/dts/zynq-ep107.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-ep107.dts
index 574bc04..5caf100 100644
--- a/arch/arm/boot/dts/zynq-ep107.dts
+++ b/arch/arm/boot/dts/zynq-ep107.dts
@@ -59,5 +59,12 @@
interrupts = <0 27 4>;
clock = <50000000>;
};
+
+ uart1: uart@e0001000 {
+ compatible = "xlnx,xuartps";
+ reg = <0xE0001000 0x1000>;
+ interrupts = <0 50 4>;
+ clock = <50000000>;
+ };
};
};
--
1.8.0
The Zynq platform requires the use of CONFIG_OF. Remove the #ifdef
conditionals in the uartps driver.
Signed-off-by: Josh Cartwright <[email protected]>
---
drivers/tty/serial/xilinx_uartps.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index b627363..23efe17 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -946,15 +946,11 @@ static int __devinit xuartps_probe(struct platform_device *pdev)
struct resource *res, *res2;
int clk = 0;
-#ifdef CONFIG_OF
const unsigned int *prop;
prop = of_get_property(pdev->dev.of_node, "clock", NULL);
if (prop)
clk = be32_to_cpup(prop);
-#else
- clk = *((unsigned int *)(pdev->dev.platform_data));
-#endif
if (!clk) {
dev_err(&pdev->dev, "no clock specified\n");
return -ENODEV;
@@ -1044,16 +1040,11 @@ static int xuartps_resume(struct platform_device *pdev)
}
/* Match table for of_platform binding */
-
-#ifdef CONFIG_OF
static struct of_device_id xuartps_of_match[] __devinitdata = {
{ .compatible = "xlnx,xuartps", },
{}
};
MODULE_DEVICE_TABLE(of, xuartps_of_match);
-#else
-#define xuartps_of_match NULL
-#endif
static struct platform_driver xuartps_platform_driver = {
.probe = xuartps_probe, /* Probe method */
--
1.8.0
Add support for retrieving TTC configuration from device tree. This
includes the ability to pull information about the driving clocks from
the of_clk bindings.
Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/boot/dts/zynq-7000.dtsi | 53 ++++++++
arch/arm/boot/dts/zynq-zc702.dts | 10 ++
drivers/clocksource/xilinx_ttc.c | 273 ++++++++++++++++++++++-----------------
3 files changed, 218 insertions(+), 118 deletions(-)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 5fb763f..9a2442c 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -109,5 +109,58 @@
};
};
};
+
+ ttc0: ttc0@f8001000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "xlnx,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: ttc0@f8002000 {
+ #interrupt-parent = <&intc>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "xlnx,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 86f44d5..c772942 100644
--- a/arch/arm/boot/dts/zynq-zc702.dts
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -32,3 +32,13 @@
&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/drivers/clocksource/xilinx_ttc.c b/drivers/clocksource/xilinx_ttc.c
index ff38b3e..a4718f7 100644
--- a/drivers/clocksource/xilinx_ttc.c
+++ b/drivers/clocksource/xilinx_ttc.c
@@ -23,30 +23,14 @@
#include <linux/clocksource.h>
#include <linux/clockchips.h>
#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+#include <linux/clk-provider.h>
#include <mach/zynq_soc.h>
-#define IRQ_TIMERCOUNTER0 42
-
-/*
- * 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 will be 200MHz. With the
- * pre-scaler of 32, the timers are clocked at 6.25MHz (160ns resolution).
- */
-#define XTTCPSS_CLOCKSOURCE 0 /* Timer 1 as a generic timekeeping */
-#define XTTCPSS_CLOCKEVENT 1 /* Timer 2 as a clock event */
-
-#define XTTCPSS_TIMER_BASE TTC0_BASE
-#define XTTCPCC_EVENT_TIMER_IRQ (IRQ_TIMERCOUNTER0 + 1)
/*
* Timer Register Offset Definitions of Timer 1, Increment base address by 4
* and use same offsets for Timer 2
@@ -63,9 +47,14 @@
#define XTTCPSS_CNT_CNTRL_DISABLE_MASK 0x1
-/* Setup the timers to use pre-scaling */
-
-#define TIMER_RATE (PERIPHERAL_CLOCK_RATE / 32)
+/* 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 xttcpss_timer - This definition defines local timer structure
@@ -73,11 +62,25 @@
* @base_addr: Base address of timer
**/
struct xttcpss_timer {
- void __iomem *base_addr;
+ void __iomem *base_addr;
+};
+
+struct xttcpss_timer_clocksource {
+ struct xttcpss_timer xttc;
+ struct clocksource cs;
};
-static struct xttcpss_timer timers[2];
-static struct clock_event_device xttcpss_clockevent;
+#define to_xttcpss_timer_clksrc(x) \
+ container_of(x, struct xttcpss_timer_clocksource, cs)
+
+struct xttcpss_timer_clockevent {
+ struct xttcpss_timer xttc;
+ struct clock_event_device ce;
+ struct clk *clk;
+};
+
+#define to_xttcpss_timer_clkevent(x) \
+ container_of(x, struct xttcpss_timer_clockevent, ce)
/**
* xttcpss_set_interval - Set the timer interval value
@@ -99,7 +102,7 @@ static void xttcpss_set_interval(struct xttcpss_timer *timer,
/* Reset the counter (0x10) so that it starts from 0, one-shot
mode makes this needed for timing to be right. */
- ctrl_reg |= 0x10;
+ ctrl_reg |= CNT_CNTRL_RESET;
ctrl_reg &= ~XTTCPSS_CNT_CNTRL_DISABLE_MASK;
__raw_writel(ctrl_reg, timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET);
}
@@ -114,90 +117,31 @@ static void xttcpss_set_interval(struct xttcpss_timer *timer,
**/
static irqreturn_t xttcpss_clock_event_interrupt(int irq, void *dev_id)
{
- struct clock_event_device *evt = &xttcpss_clockevent;
- struct xttcpss_timer *timer = dev_id;
+ struct xttcpss_timer_clockevent *xttce = dev_id;
+ struct xttcpss_timer *timer = &xttce->xttc;
/* Acknowledge the interrupt and call event handler */
__raw_writel(__raw_readl(timer->base_addr + XTTCPSS_ISR_OFFSET),
timer->base_addr + XTTCPSS_ISR_OFFSET);
- evt->event_handler(evt);
+ xttce->ce.event_handler(&xttce->ce);
return IRQ_HANDLED;
}
-static struct irqaction event_timer_irq = {
- .name = "xttcpss clockevent",
- .flags = IRQF_DISABLED | IRQF_TIMER,
- .handler = xttcpss_clock_event_interrupt,
-};
-
-/**
- * xttcpss_timer_hardware_init - Initialize the timer hardware
- *
- * Initialize the hardware to start the clock source, get the clock
- * event timer ready to use, and hook up the interrupt.
- **/
-static void __init xttcpss_timer_hardware_init(void)
-{
- /* 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.
- */
- timers[XTTCPSS_CLOCKSOURCE].base_addr = XTTCPSS_TIMER_BASE;
-
- __raw_writel(0x0, timers[XTTCPSS_CLOCKSOURCE].base_addr +
- XTTCPSS_IER_OFFSET);
- __raw_writel(0x9, timers[XTTCPSS_CLOCKSOURCE].base_addr +
- XTTCPSS_CLK_CNTRL_OFFSET);
- __raw_writel(0x10, timers[XTTCPSS_CLOCKSOURCE].base_addr +
- XTTCPSS_CNT_CNTRL_OFFSET);
-
- /* Setup the clock event timer to be an interval timer which
- * is prescaled by 32 using the interval interrupt. Leave it
- * disabled for now.
- */
-
- timers[XTTCPSS_CLOCKEVENT].base_addr = XTTCPSS_TIMER_BASE + 4;
-
- __raw_writel(0x23, timers[XTTCPSS_CLOCKEVENT].base_addr +
- XTTCPSS_CNT_CNTRL_OFFSET);
- __raw_writel(0x9, timers[XTTCPSS_CLOCKEVENT].base_addr +
- XTTCPSS_CLK_CNTRL_OFFSET);
- __raw_writel(0x1, timers[XTTCPSS_CLOCKEVENT].base_addr +
- XTTCPSS_IER_OFFSET);
-
- /* Setup IRQ the clock event timer */
- event_timer_irq.dev_id = &timers[XTTCPSS_CLOCKEVENT];
- setup_irq(XTTCPCC_EVENT_TIMER_IRQ, &event_timer_irq);
-}
-
/**
- * __raw_readl_cycles - Reads the timer counter register
+ * __xttc_clocksource_read - Reads the timer counter register
*
* returns: Current timer counter register value
**/
-static cycle_t __raw_readl_cycles(struct clocksource *cs)
+static cycle_t __xttc_clocksource_read(struct clocksource *cs)
{
- struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKSOURCE];
+ struct xttcpss_timer *timer = &to_xttcpss_timer_clksrc(cs)->xttc;
return (cycle_t)__raw_readl(timer->base_addr +
XTTCPSS_COUNT_VAL_OFFSET);
}
-
-/*
- * Instantiate and initialize the clock source structure
- */
-static struct clocksource clocksource_xttcpss = {
- .name = "xttcpss_timer1",
- .rating = 200, /* Reasonable clock source */
- .read = __raw_readl_cycles,
- .mask = CLOCKSOURCE_MASK(16),
- .flags = CLOCK_SOURCE_IS_CONTINUOUS,
-};
-
-
/**
* xttcpss_set_next_event - Sets the time interval for next event
*
@@ -209,7 +153,8 @@ static struct clocksource clocksource_xttcpss = {
static int xttcpss_set_next_event(unsigned long cycles,
struct clock_event_device *evt)
{
- struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+ struct xttcpss_timer_clockevent *xttce = to_xttcpss_timer_clkevent(evt);
+ struct xttcpss_timer *timer = &xttce->xttc;
xttcpss_set_interval(timer, cycles);
return 0;
@@ -224,12 +169,14 @@ static int xttcpss_set_next_event(unsigned long cycles,
static void xttcpss_set_mode(enum clock_event_mode mode,
struct clock_event_device *evt)
{
- struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+ struct xttcpss_timer_clockevent *xttce = to_xttcpss_timer_clkevent(evt);
+ struct xttcpss_timer *timer = &xttce->xttc;
u32 ctrl_reg;
switch (mode) {
case CLOCK_EVT_MODE_PERIODIC:
- xttcpss_set_interval(timer, TIMER_RATE / HZ);
+ xttcpss_set_interval(timer,
+ clk_get_rate(xttce->clk) / PRESCALE);
break;
case CLOCK_EVT_MODE_ONESHOT:
case CLOCK_EVT_MODE_UNUSED:
@@ -250,15 +197,99 @@ static void xttcpss_set_mode(enum clock_event_mode mode,
}
}
-/*
- * Instantiate and initialize the clock event structure
- */
-static struct clock_event_device xttcpss_clockevent = {
- .name = "xttcpss_timer2",
- .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
- .set_next_event = xttcpss_set_next_event,
- .set_mode = xttcpss_set_mode,
- .rating = 200,
+static int __init zynq_ttc_setup_clocksource(struct device_node *np,
+ void __iomem *base)
+{
+ struct xttcpss_timer_clocksource *ttccs;
+ struct clk *clk;
+ int err;
+ u32 reg;
+
+ ttccs = kzalloc(sizeof(*ttccs), GFP_KERNEL);
+ WARN_ON(!ttccs);
+
+ err = of_property_read_u32(np, "reg", ®);
+ WARN_ON(err);
+
+ clk = of_clk_get_by_name(np, "cpu_1x");
+ WARN_ON(IS_ERR(clk));
+
+ err = clk_prepare_enable(clk);
+ WARN_ON(err);
+
+ ttccs->xttc.base_addr = base + reg * 4;
+
+ ttccs->cs.name = np->name;
+ ttccs->cs.rating = 200;
+ ttccs->cs.read = __xttc_clocksource_read;
+ ttccs->cs.mask = CLOCKSOURCE_MASK(16);
+ ttccs->cs.flags = CLOCK_SOURCE_IS_CONTINUOUS;
+
+ __raw_writel(0x0, ttccs->xttc.base_addr + XTTCPSS_IER_OFFSET);
+ __raw_writel(CLK_CNTRL_PRESCALE | CLK_CNTRL_PRESCALE_EN,
+ ttccs->xttc.base_addr + XTTCPSS_CLK_CNTRL_OFFSET);
+ __raw_writel(CNT_CNTRL_RESET,
+ ttccs->xttc.base_addr + XTTCPSS_CNT_CNTRL_OFFSET);
+
+ err = clocksource_register_hz(&ttccs->cs, clk_get_rate(clk) / PRESCALE);
+ WARN_ON(err);
+
+ return 0;
+}
+
+static int __init zynq_ttc_setup_clockevent(struct device_node *np,
+ void __iomem *base)
+{
+ struct xttcpss_timer_clockevent *ttcce;
+ int err, irq;
+ u32 reg;
+
+ ttcce = kzalloc(sizeof(*ttcce), GFP_KERNEL);
+ WARN_ON(!ttcce);
+
+ err = of_property_read_u32(np, "reg", ®);
+ WARN_ON(err);
+
+ ttcce->xttc.base_addr = base + reg * 4;
+
+ ttcce->clk = of_clk_get_by_name(np, "cpu_1x");
+ WARN_ON(IS_ERR(ttcce->clk));
+
+ err = clk_prepare_enable(ttcce->clk);
+ WARN_ON(err);
+
+ irq = irq_of_parse_and_map(np, 0);
+ WARN_ON(!irq);
+
+ ttcce->ce.name = np->name;
+ ttcce->ce.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+ ttcce->ce.set_next_event = xttcpss_set_next_event;
+ ttcce->ce.set_mode = xttcpss_set_mode;
+ ttcce->ce.rating = 200;
+ ttcce->ce.irq = irq;
+
+ __raw_writel(0x23, ttcce->xttc.base_addr + XTTCPSS_CNT_CNTRL_OFFSET);
+ __raw_writel(CLK_CNTRL_PRESCALE | CLK_CNTRL_PRESCALE_EN,
+ ttcce->xttc.base_addr + XTTCPSS_CLK_CNTRL_OFFSET);
+ __raw_writel(0x1, ttcce->xttc.base_addr + XTTCPSS_IER_OFFSET);
+
+ err = request_irq(irq, xttcpss_clock_event_interrupt, IRQF_TIMER,
+ np->name, ttcce);
+ WARN_ON(err);
+
+ clockevents_config_and_register(&ttcce->ce,
+ clk_get_rate(ttcce->clk) / PRESCALE,
+ 1, 0xfffe);
+
+ return 0;
+}
+
+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, },
+ {}
};
/**
@@ -269,21 +300,27 @@ static struct clock_event_device xttcpss_clockevent = {
**/
void __init xttcpss_timer_init(void)
{
- xttcpss_timer_hardware_init();
- clocksource_register_hz(&clocksource_xttcpss, TIMER_RATE);
+ struct device_node *np;
+
+ for_each_compatible_node(np, NULL, "xlnx,ttc") {
+ struct device_node *np_chld;
+ void __iomem *base;
- /* Calculate the parameters to allow the clockevent to operate using
- integer math
- */
- clockevents_calc_mult_shift(&xttcpss_clockevent, TIMER_RATE, 4);
+ base = of_iomap(np, 0);
+ WARN_ON(!base);
- xttcpss_clockevent.max_delta_ns =
- clockevent_delta2ns(0xfffe, &xttcpss_clockevent);
- xttcpss_clockevent.min_delta_ns =
- clockevent_delta2ns(1, &xttcpss_clockevent);
+ for_each_available_child_of_node(np, np_chld) {
+ int (*cb)(struct device_node *np, void __iomem *base);
+ const struct of_device_id *match;
- /* Indicate that clock event is on 1st CPU as SMP boot needs it */
+ match = of_match_node(zynq_ttc_match, np_chld);
+ if (match) {
+ int err;
- xttcpss_clockevent.cpumask = cpumask_of(0);
- clockevents_register_device(&xttcpss_clockevent);
+ cb = match->data;
+ err = cb(np_chld, base);
+ WARN_ON(err);
+ }
+ }
+ }
}
--
1.8.0
Add support for COMMON_CLK, and provide simplified models for the
necessary clocks on the zynq-7000. Currently, the PLLs, the CPU clock
network, and the basic peripheral clock networks (for SDIO, SMC, SPI,
QSPI, UART) are modelled.
Signed-off-by: Josh Cartwright <[email protected]>
---
.../devicetree/bindings/clock/zynq-7000.txt | 55 ++++
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/zynq-7000.dtsi | 56 ++++
arch/arm/boot/dts/zynq-zc702.dts | 4 +
arch/arm/mach-zynq/common.c | 11 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-zynq.c | 355 +++++++++++++++++++++
include/linux/clk/zynq.h | 24 ++
8 files changed, 507 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/zynq-7000.txt
create mode 100644 drivers/clk/clk-zynq.c
create mode 100644 include/linux/clk/zynq.h
diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt b/Documentation/devicetree/bindings/clock/zynq-7000.txt
new file mode 100644
index 0000000..2e21fc1
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
@@ -0,0 +1,55 @@
+Device Tree Clock bindings for the Zynq 7000 EPP
+
+The Zynq EPP has several different clk providers, each with there own bindings.
+The purpose of this document is to document their usage.
+
+See clock_bindings.txt for more information on the generic clock bindings.
+See Chapter 25 of Zynq TRM for more information about Zynq clocks.
+
+== PLLs ==
+
+Used to describe the ARM_PLL, DDR_PLL, and IO_PLL.
+
+Required properties:
+- #clock-cells : shall be 0 (only one clock is output from this node)
+- compatible : "xlnx,zynq-pll"
+- reg : pair of u32 values, which are the address offsets within the SLCR
+ of the relevant PLL_CTRL register and PLL_CFG register respectively
+- clocks : phandle for parent clock. should be the phandle for ps_clk
+
+Optional properties:
+- clock-output-names : name of the output clock
+
+Example:
+ armpll: armpll {
+ #clock-cells = <0>;
+ compatible = "xlnx,zynq-pll";
+ clocks = <&ps_clk>;
+ reg = <0x100 0x110>;
+ clock-output-names = "armpll";
+ };
+
+== Peripheral clocks ==
+
+Describes clock node for the SDIO, SMC, SPI, QSPI, and UART clocks.
+
+Required properties:
+- #clock-cells : shall be 1
+- compatible : "xlnx,zynq-periph-clock"
+- reg : a single u32 value, describing the offset within the SLCR where
+ the CLK_CTRL register is found for this peripheral
+- clocks : phandle for parent clocks. should hold phandles for
+ the IO_PLL, ARM_PLL, and DDR_PLL in order
+
+Optional properties:
+- clock-output-names : name of the output clock
+
+Example:
+ uart_clk: uart_clk {
+ #clock-cells = <1>;
+ compatible = "xlnx,zynq-periph-clock";
+ clocks = <&iopll &armpll &ddrpll>;
+ reg = <0x154>;
+ clock-output-names = "uart0_ref_clk",
+ "uart1_ref_clk";
+ };
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 21ed87b..ccfe0ab 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -959,6 +959,7 @@ config ARCH_ZYNQ
bool "Xilinx Zynq ARM Cortex A9 Platform"
select ARM_AMBA
select ARM_GIC
+ select COMMON_CLK
select CPU_V7
select GENERIC_CLOCKEVENTS
select ICST
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 8b30e59..bb3085c 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -53,5 +53,61 @@
interrupts = <0 50 4>;
clock = <50000000>;
};
+
+ slcr: slcr@f8000000 {
+ compatible = "xlnx,zynq-slcr";
+ reg = <0xF8000000 0x1000>;
+
+ clocks {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ps_clk: ps_clk {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ /* clock-frequency set in board-specific file */
+ clock-output-names = "ps_clk";
+ };
+ armpll: armpll {
+ #clock-cells = <0>;
+ compatible = "xlnx,zynq-pll";
+ clocks = <&ps_clk>;
+ reg = <0x100 0x110>;
+ clock-output-names = "armpll";
+ };
+ ddrpll: ddrpll {
+ #clock-cells = <0>;
+ compatible = "xlnx,zynq-pll";
+ clocks = <&ps_clk>;
+ reg = <0x104 0x114>;
+ clock-output-names = "ddrpll";
+ };
+ iopll: iopll {
+ #clock-cells = <0>;
+ compatible = "xlnx,zynq-pll";
+ clocks = <&ps_clk>;
+ reg = <0x108 0x118>;
+ clock-output-names = "iopll";
+ };
+ uart_clk: uart_clk {
+ #clock-cells = <1>;
+ compatible = "xlnx,zynq-periph-clock";
+ clocks = <&iopll &armpll &ddrpll>;
+ reg = <0x154>;
+ clock-output-names = "uart0_ref_clk",
+ "uart1_ref_clk";
+ };
+ cpu_clk: cpu_clk {
+ #clock-cells = <1>;
+ compatible = "xlnx,zynq-cpu-clock";
+ clocks = <&iopll &armpll &ddrpll>;
+ reg = <0x120 0x1C4>;
+ clock-output-names = "cpu_6x4x",
+ "cpu_3x2x",
+ "cpu_2x",
+ "cpu_1x";
+ };
+ };
+ };
};
};
diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
index e25a307..86f44d5 100644
--- a/arch/arm/boot/dts/zynq-zc702.dts
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -28,3 +28,7 @@
};
};
+
+&ps_clk {
+ clock-frequency = <33333330>;
+};
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 447904b..f0cb3e4 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -19,6 +19,8 @@
#include <linux/cpumask.h>
#include <linux/platform_device.h>
#include <linux/clk.h>
+#include <linux/clk/zynq.h>
+#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/of.h>
@@ -96,6 +98,15 @@ static struct map_desc io_desc[] __initdata = {
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);
+
xttcpss_timer_init();
}
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 71a25b9..d35a34c 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -19,6 +19,7 @@ endif
obj-$(CONFIG_MACH_LOONGSON1) += clk-ls1x.o
obj-$(CONFIG_ARCH_U8500) += ux500/
obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
+obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o
# Chip specific
obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
diff --git a/drivers/clk/clk-zynq.c b/drivers/clk/clk-zynq.c
new file mode 100644
index 0000000..2afa84d
--- /dev/null
+++ b/drivers/clk/clk-zynq.c
@@ -0,0 +1,355 @@
+/*
+ * Copyright (c) 2012 National Instruments
+ *
+ * Josh Cartwright <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/clk-provider.h>
+
+static void __iomem *slcr_base;
+
+struct zynq_pll_clk {
+ struct clk_hw hw;
+ void __iomem *pll_ctrl;
+ void __iomem *pll_cfg;
+};
+
+#define to_zynq_pll_clk(hw) container_of(hw, struct zynq_pll_clk, hw)
+
+#define CTRL_PLL_FDIV(x) ((x)>>12)
+
+static unsigned long zynq_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct zynq_pll_clk *pll = to_zynq_pll_clk(hw);
+ return parent_rate * CTRL_PLL_FDIV(ioread32(pll->pll_ctrl));
+}
+
+static const struct clk_ops zynq_pll_clk_ops = {
+ .recalc_rate = zynq_pll_recalc_rate,
+};
+
+static void __init zynq_pll_clk_setup(struct device_node *np)
+{
+ struct clk_init_data init;
+ struct zynq_pll_clk *pll;
+ const char *parent_name;
+ struct clk *clk;
+ u32 regs[2];
+ int ret;
+
+ ret = of_property_read_u32_array(np, "reg", regs, ARRAY_SIZE(regs));
+ WARN_ON(ret);
+
+ pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+ WARN_ON(!pll);
+
+ pll->pll_ctrl = slcr_base + regs[0];
+ pll->pll_cfg = slcr_base + regs[1];
+
+ of_property_read_string(np, "clock-output-names", &init.name);
+
+ init.ops = &zynq_pll_clk_ops;
+ parent_name = of_clk_get_parent_name(np, 0);
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ pll->hw.init = &init;
+
+ clk = clk_register(NULL, &pll->hw);
+ WARN_ON(IS_ERR(clk));
+
+ ret = of_clk_add_provider(np, of_clk_src_simple_get, clk);
+ WARN_ON(ret);
+}
+
+struct zynq_periph_clk {
+ struct clk_hw hw;
+ struct clk_onecell_data onecell_data;
+ struct clk *gates[2];
+ void __iomem *clk_ctrl;
+ spinlock_t clkact_lock;
+};
+
+#define to_zynq_periph_clk(hw) container_of(hw, struct zynq_periph_clk, hw)
+
+static const u8 periph_clk_parent_map[] = {
+ 0, 0, 1, 2
+};
+#define PERIPH_CLK_CTRL_SRC(x) (periph_clk_parent_map[((x)&3)>>4])
+#define PERIPH_CLK_CTRL_DIV(x) (((x)&0x3F00)>>8)
+
+static unsigned long zynq_periph_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct zynq_periph_clk *periph = to_zynq_periph_clk(hw);
+ return parent_rate / PERIPH_CLK_CTRL_DIV(ioread32(periph->clk_ctrl));
+}
+
+static u8 zynq_periph_get_parent(struct clk_hw *hw)
+{
+ struct zynq_periph_clk *periph = to_zynq_periph_clk(hw);
+ return PERIPH_CLK_CTRL_SRC(ioread32(periph->clk_ctrl));
+}
+
+static const struct clk_ops zynq_periph_clk_ops = {
+ .recalc_rate = zynq_periph_recalc_rate,
+ .get_parent = zynq_periph_get_parent,
+};
+
+static void __init zynq_periph_clk_setup(struct device_node *np)
+{
+ struct zynq_periph_clk *periph;
+ const char *parent_names[3];
+ struct clk_init_data init;
+ struct clk *clk;
+ int err;
+ u32 reg;
+ int i;
+
+ err = of_property_read_u32(np, "reg", ®);
+ WARN_ON(err);
+
+ periph = kzalloc(sizeof(*periph), GFP_KERNEL);
+ WARN_ON(!periph);
+
+ periph->clk_ctrl = slcr_base + reg;
+ spin_lock_init(&periph->clkact_lock);
+
+ init.name = np->name;
+ init.ops = &zynq_periph_clk_ops;
+ for (i = 0; i < ARRAY_SIZE(parent_names); i++)
+ parent_names[i] = of_clk_get_parent_name(np, i);
+ init.parent_names = parent_names;
+ init.num_parents = ARRAY_SIZE(parent_names);
+
+ periph->hw.init = &init;
+
+ clk = clk_register(NULL, &periph->hw);
+ WARN_ON(IS_ERR(clk));
+
+ err = of_clk_add_provider(np, of_clk_src_simple_get, clk);
+ WARN_ON(err);
+
+ for (i = 0; i < 2; i++) {
+ const char *name;
+
+ err = of_property_read_string_index(np, "clock-output-names", i,
+ &name);
+ WARN_ON(err);
+
+ periph->gates[i] = clk_register_gate(NULL, name, np->name, 0,
+ periph->clk_ctrl, i, 0,
+ &periph->clkact_lock);
+ WARN_ON(IS_ERR(periph->gates[i]));
+ }
+
+ periph->onecell_data.clks = periph->gates;
+ periph->onecell_data.clk_num = i;
+
+ err = of_clk_add_provider(np, of_clk_src_onecell_get,
+ &periph->onecell_data);
+ WARN_ON(err);
+}
+
+/* CPU Clock domain is modelled as a mux with 4 children subclks, whose
+ * derivative rates depend on CLK_621_TRUE
+ */
+
+struct zynq_cpu_clk {
+ struct clk_hw hw;
+ struct clk_onecell_data onecell_data;
+ struct clk *subclks[4];
+ void __iomem *clk_ctrl;
+ spinlock_t clkact_lock;
+};
+
+#define to_zynq_cpu_clk(hw) container_of(hw, struct zynq_cpu_clk, hw)
+
+static const u8 zynq_cpu_clk_parent_map[] = {
+ 1, 1, 2, 0
+};
+#define CPU_CLK_SRCSEL(x) (zynq_cpu_clk_parent_map[(((x)&0x30)>>4)])
+#define CPU_CLK_CTRL_DIV(x) (((x)&0x3F00)>>8)
+
+static u8 zynq_cpu_clk_get_parent(struct clk_hw *hw)
+{
+ struct zynq_cpu_clk *cpuclk = to_zynq_cpu_clk(hw);
+ return CPU_CLK_SRCSEL(ioread32(cpuclk->clk_ctrl));
+}
+
+static unsigned long zynq_cpu_clk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct zynq_cpu_clk *cpuclk = to_zynq_cpu_clk(hw);
+ return parent_rate / CPU_CLK_CTRL_DIV(ioread32(cpuclk->clk_ctrl));
+}
+
+static const struct clk_ops zynq_cpu_clk_ops = {
+ .get_parent = zynq_cpu_clk_get_parent,
+ .recalc_rate = zynq_cpu_clk_recalc_rate,
+};
+
+struct zynq_cpu_subclk {
+ struct clk_hw hw;
+ void __iomem *clk_621;
+ enum {
+ CPU_SUBCLK_6X4X,
+ CPU_SUBCLK_3X2X,
+ CPU_SUBCLK_2X,
+ CPU_SUBCLK_1X,
+ } which;
+};
+
+#define CLK_621_TRUE(x) ((x)&1)
+
+#define to_zynq_cpu_subclk(hw) container_of(hw, struct zynq_cpu_subclk, hw);
+
+static unsigned long zynq_cpu_subclk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ unsigned long uninitialized_var(rate);
+ struct zynq_cpu_subclk *subclk;
+ bool is_621;
+
+ subclk = to_zynq_cpu_subclk(hw)
+ is_621 = CLK_621_TRUE(ioread32(subclk->clk_621));
+
+ switch (subclk->which) {
+ case CPU_SUBCLK_6X4X:
+ rate = parent_rate;
+ break;
+ case CPU_SUBCLK_3X2X:
+ rate = parent_rate / 2;
+ break;
+ case CPU_SUBCLK_2X:
+ rate = parent_rate / (is_621 ? 3 : 2);
+ break;
+ case CPU_SUBCLK_1X:
+ rate = parent_rate / (is_621 ? 6 : 4);
+ break;
+ };
+
+ return rate;
+}
+
+static const struct clk_ops zynq_cpu_subclk_ops = {
+ .recalc_rate = zynq_cpu_subclk_recalc_rate,
+};
+
+static struct clk *zynq_cpu_subclk_setup(struct device_node *np, u8 which,
+ void __iomem *clk_621)
+{
+ struct zynq_cpu_subclk *subclk;
+ struct clk_init_data init;
+ struct clk *clk;
+ int err;
+
+ err = of_property_read_string_index(np, "clock-output-names",
+ which, &init.name);
+ if (WARN_ON(err))
+ goto err_read_output_name;
+
+ subclk = kzalloc(sizeof(*subclk), GFP_KERNEL);
+ if (!subclk)
+ goto err_subclk_alloc;
+
+ subclk->clk_621 = clk_621;
+ subclk->which = which;
+
+ init.ops = &zynq_cpu_subclk_ops;
+ init.parent_names = &np->name;
+ init.num_parents = 1;
+
+ subclk->hw.init = &init;
+
+ clk = clk_register(NULL, &subclk->hw);
+ if (WARN_ON(IS_ERR(clk)))
+ goto err_clk_register;
+
+ return clk;
+
+err_clk_register:
+err_subclk_alloc:
+err_read_output_name:
+ return NULL;
+}
+
+static void __init zynq_cpu_clk_setup(struct device_node *np)
+{
+ struct zynq_cpu_clk *cpuclk;
+ const char *parent_names[3];
+ struct clk_init_data init;
+ void __iomem *clk_621;
+ struct clk *clk;
+ u32 reg[2];
+ int err;
+ int i;
+
+ err = of_property_read_u32_array(np, "reg", reg, ARRAY_SIZE(reg));
+ WARN_ON(err);
+
+ cpuclk = kzalloc(sizeof(*cpuclk), GFP_KERNEL);
+ WARN_ON(!cpuclk);
+
+ cpuclk->clk_ctrl = slcr_base + reg[0];
+ clk_621 = slcr_base + reg[1];
+ spin_lock_init(&cpuclk->clkact_lock);
+
+ init.name = np->name;
+ init.ops = &zynq_cpu_clk_ops;
+ for (i = 0; i < ARRAY_SIZE(parent_names); i++)
+ parent_names[i] = of_clk_get_parent_name(np, i);
+ init.parent_names = parent_names;
+ init.num_parents = ARRAY_SIZE(parent_names);
+
+ cpuclk->hw.init = &init;
+
+ clk = clk_register(NULL, &cpuclk->hw);
+ WARN_ON(IS_ERR(clk));
+
+ err = of_clk_add_provider(np, of_clk_src_simple_get, clk);
+ WARN_ON(err);
+
+ for (i = 0; i < 4; i++) {
+ cpuclk->subclks[i] = zynq_cpu_subclk_setup(np, i, clk_621);
+ WARN_ON(IS_ERR(cpuclk->subclks[i]));
+ }
+
+ cpuclk->onecell_data.clks = cpuclk->subclks;
+ cpuclk->onecell_data.clk_num = i;
+
+ err = of_clk_add_provider(np, of_clk_src_onecell_get,
+ &cpuclk->onecell_data);
+ WARN_ON(err);
+}
+
+static const __initconst struct of_device_id zynq_clk_match[] = {
+ { .compatible = "fixed-clock", .data = of_fixed_clk_setup, },
+ { .compatible = "xlnx,zynq-pll", .data = zynq_pll_clk_setup, },
+ { .compatible = "xlnx,zynq-periph-clock",
+ .data = zynq_periph_clk_setup, },
+ { .compatible = "xlnx,zynq-cpu-clock", .data = zynq_cpu_clk_setup, },
+ {}
+};
+
+void __init xilinx_zynq_clocks_init(void __iomem *slcr)
+{
+ slcr_base = slcr;
+ of_clk_init(zynq_clk_match);
+}
diff --git a/include/linux/clk/zynq.h b/include/linux/clk/zynq.h
new file mode 100644
index 0000000..56be7cd
--- /dev/null
+++ b/include/linux/clk/zynq.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2012 National Instruments
+ *
+ * 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.
+ *
+ * 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.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef __LINUX_CLK_ZYNQ_H_
+#define __LINUX_CLK_ZYNQ_H_
+
+void __init xilinx_zynq_clocks_init(void __iomem *slcr);
+
+#endif
--
1.8.0
Suggested cleanup by Arnd Bergmann. Move the ttc timer.c code to
drivers/clocksource, and out of the mach-zynq directory.
The common.h (which only held the timer declaration) was renamed to
xilinx_ttc.h and moved into include/linux.
Signed-off-by: Josh Cartwright <[email protected]>
Cc: Arnd Bergmann <[email protected]>
---
arch/arm/mach-zynq/Makefile | 2 +-
arch/arm/mach-zynq/common.c | 2 +-
drivers/clocksource/Makefile | 1 +
arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c | 1 -
arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h | 4 ++--
5 files changed, 5 insertions(+), 5 deletions(-)
rename arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c (99%)
rename arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h (91%)
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/common.c b/arch/arm/mach-zynq/common.c
index 6f058258..0279ea7 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -22,6 +22,7 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/of.h>
+#include <linux/xilinx_ttc.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
@@ -32,7 +33,6 @@
#include <asm/hardware/cache-l2x0.h>
#include <mach/zynq_soc.h>
-#include "common.h"
static struct of_device_id zynq_of_bus_ids[] __initdata = {
{ .compatible = "simple-bus", },
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 603be36..f27c7b1 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_DW_APB_TIMER_OF) += dw_apb_timer_of.o
obj-$(CONFIG_CLKSRC_DBX500_PRCMU) += clksrc-dbx500-prcmu.o
obj-$(CONFIG_ARMADA_370_XP_TIMER) += time-armada-370-xp.o
obj-$(CONFIG_ARCH_BCM2835) += bcm2835_timer.o
+obj-$(CONFIG_ARCH_ZYNQ) += xilinx_ttc.o
obj-$(CONFIG_CLKSRC_ARM_GENERIC) += arm_generic.o
diff --git a/arch/arm/mach-zynq/timer.c b/drivers/clocksource/xilinx_ttc.c
similarity index 99%
rename from arch/arm/mach-zynq/timer.c
rename to drivers/clocksource/xilinx_ttc.c
index c93cbe5..ff38b3e 100644
--- a/arch/arm/mach-zynq/timer.c
+++ b/drivers/clocksource/xilinx_ttc.c
@@ -25,7 +25,6 @@
#include <linux/io.h>
#include <mach/zynq_soc.h>
-#include "common.h"
#define IRQ_TIMERCOUNTER0 42
diff --git a/arch/arm/mach-zynq/common.h b/include/linux/xilinx_ttc.h
similarity index 91%
rename from arch/arm/mach-zynq/common.h
rename to include/linux/xilinx_ttc.h
index 954b91c..303a3fd 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/include/linux/xilinx_ttc.h
@@ -14,8 +14,8 @@
* GNU General Public License for more details.
*/
-#ifndef __MACH_ZYNQ_COMMON_H__
-#define __MACH_ZYNQ_COMMON_H__
+#ifndef __XILINX_TTC_H__
+#define __XILINX_TTC_H__
void __init xttcpss_timer_init(void);
--
1.8.0
Add support for specifying clock information for the uart clk via the
device tree. This eliminates the need to hardcode rates in the device
tree.
Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/boot/dts/zynq-7000.dtsi | 4 ++--
drivers/tty/serial/xilinx_uartps.c | 30 +++++++++++++++++-------------
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index bb3085c..5fb763f 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -44,14 +44,14 @@
compatible = "xlnx,xuartps";
reg = <0xE0000000 0x1000>;
interrupts = <0 27 4>;
- clock = <50000000>;
+ clocks = <&uart_clk 0>;
};
uart1: uart@e0001000 {
compatible = "xlnx,xuartps";
reg = <0xE0001000 0x1000>;
interrupts = <0 50 4>;
- clock = <50000000>;
+ clocks = <&uart_clk 0>;
};
slcr: slcr@f8000000 {
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 23efe17..adfecbc 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -17,6 +17,7 @@
#include <linux/tty.h>
#include <linux/tty_flip.h>
#include <linux/console.h>
+#include <linux/clk.h>
#include <linux/irq.h>
#include <linux/io.h>
#include <linux/of.h>
@@ -944,18 +945,20 @@ static int __devinit xuartps_probe(struct platform_device *pdev)
int rc;
struct uart_port *port;
struct resource *res, *res2;
- int clk = 0;
+ struct clk *clk;
- const unsigned int *prop;
-
- prop = of_get_property(pdev->dev.of_node, "clock", NULL);
- if (prop)
- clk = be32_to_cpup(prop);
+ clk = of_clk_get(pdev->dev.of_node, 0);
if (!clk) {
dev_err(&pdev->dev, "no clock specified\n");
return -ENODEV;
}
+ rc = clk_prepare_enable(clk);
+ if (rc) {
+ dev_err(&pdev->dev, "could not enable clock\n");
+ return -EBUSY;
+ }
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -ENODEV;
@@ -978,7 +981,8 @@ static int __devinit xuartps_probe(struct platform_device *pdev)
port->mapbase = res->start;
port->irq = res2->start;
port->dev = &pdev->dev;
- port->uartclk = clk;
+ port->uartclk = clk_get_rate(clk);
+ port->private_data = clk;
dev_set_drvdata(&pdev->dev, port);
rc = uart_add_one_port(&xuartps_uart_driver, port);
if (rc) {
@@ -1000,14 +1004,14 @@ static int __devinit xuartps_probe(struct platform_device *pdev)
static int __devexit xuartps_remove(struct platform_device *pdev)
{
struct uart_port *port = dev_get_drvdata(&pdev->dev);
- int rc = 0;
+ struct clk *clk = port->private_data;
+ int rc;
/* Remove the xuartps port from the serial core */
- if (port) {
- rc = uart_remove_one_port(&xuartps_uart_driver, port);
- dev_set_drvdata(&pdev->dev, NULL);
- port->mapbase = 0;
- }
+ rc = uart_remove_one_port(&xuartps_uart_driver, port);
+ dev_set_drvdata(&pdev->dev, NULL);
+ port->mapbase = 0;
+ clk_disable_unprepare(clk);
return rc;
}
--
1.8.0
Move the sys_timer definition out of ttc driver and make it part of the
common zynq code. This is preparation for renaming and COMMON_CLK
support.
Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/mach-zynq/common.c | 13 +++++++++++++
arch/arm/mach-zynq/common.h | 4 +---
arch/arm/mach-zynq/timer.c | 10 +---------
3 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ba8d14f..6f058258 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -25,6 +25,7 @@
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
+#include <asm/mach/time.h>
#include <asm/mach-types.h>
#include <asm/page.h>
#include <asm/hardware/gic.h>
@@ -93,6 +94,18 @@ static struct map_desc io_desc[] __initdata = {
};
+static void __init xilinx_zynq_timer_init(void)
+{
+ xttcpss_timer_init();
+}
+
+/*
+ * Instantiate and initialize the system timer structure
+ */
+static struct sys_timer xttcpss_sys_timer = {
+ .init = xilinx_zynq_timer_init,
+};
+
/**
* xilinx_map_io() - Create memory mappings needed for early I/O.
*/
diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
index a009644..954b91c 100644
--- a/arch/arm/mach-zynq/common.h
+++ b/arch/arm/mach-zynq/common.h
@@ -17,8 +17,6 @@
#ifndef __MACH_ZYNQ_COMMON_H__
#define __MACH_ZYNQ_COMMON_H__
-#include <asm/mach/time.h>
-
-extern struct sys_timer xttcpss_sys_timer;
+void __init xttcpss_timer_init(void);
#endif
diff --git a/arch/arm/mach-zynq/timer.c b/arch/arm/mach-zynq/timer.c
index c2c96cc..c93cbe5 100644
--- a/arch/arm/mach-zynq/timer.c
+++ b/arch/arm/mach-zynq/timer.c
@@ -24,7 +24,6 @@
#include <linux/clockchips.h>
#include <linux/io.h>
-#include <asm/mach/time.h>
#include <mach/zynq_soc.h>
#include "common.h"
@@ -269,7 +268,7 @@ static struct clock_event_device xttcpss_clockevent = {
* Initializes the timer hardware and register the clock source and clock event
* timers with Linux kernal timer framework
**/
-static void __init xttcpss_timer_init(void)
+void __init xttcpss_timer_init(void)
{
xttcpss_timer_hardware_init();
clocksource_register_hz(&clocksource_xttcpss, TIMER_RATE);
@@ -289,10 +288,3 @@ static void __init xttcpss_timer_init(void)
xttcpss_clockevent.cpumask = cpumask_of(0);
clockevents_register_device(&xttcpss_clockevent);
}
-
-/*
- * Instantiate and initialize the system timer structure
- */
-struct sys_timer xttcpss_sys_timer = {
- .init = xttcpss_timer_init,
-};
--
1.8.0
The purpose of the created zynq-7000.dtsi file is to describe the
hardware common to all Zynq 7000-based boards. Also, get rid of the
zynq-ep107 device tree, since it is not hardware anyone can purchase.
Add a zc702 dts file based on the zynq-7000.dtsi. Add it to the
dts/Makefile so it is built with the 'dtbs' target.
Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/boot/dts/Makefile | 1 +
.../boot/dts/{zynq-ep107.dts => zynq-7000.dtsi} | 19 +++-----------
arch/arm/boot/dts/zynq-zc702.dts | 30 ++++++++++++++++++++++
arch/arm/mach-zynq/common.c | 3 ++-
4 files changed, 36 insertions(+), 17 deletions(-)
rename arch/arm/boot/dts/{zynq-ep107.dts => zynq-7000.dtsi} (79%)
create mode 100644 arch/arm/boot/dts/zynq-zc702.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index f37cf9f..76ed11e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -103,5 +103,6 @@ dtb-$(CONFIG_ARCH_VEXPRESS) += vexpress-v2p-ca5s.dtb \
dtb-$(CONFIG_ARCH_VT8500) += vt8500-bv07.dtb \
wm8505-ref.dtb \
wm8650-mid.dtb
+dtb-$(CONFIG_ARCH_ZYNQ) += zynq-zc702.dtb
endif
diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-7000.dtsi
similarity index 79%
rename from arch/arm/boot/dts/zynq-ep107.dts
rename to arch/arm/boot/dts/zynq-7000.dtsi
index 5caf100..8b30e59 100644
--- a/arch/arm/boot/dts/zynq-ep107.dts
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -10,29 +10,16 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
+/include/ "skeleton.dtsi"
-/dts-v1/;
/ {
- model = "Xilinx Zynq EP107";
- compatible = "xlnx,zynq-ep107";
- #address-cells = <1>;
- #size-cells = <1>;
- interrupt-parent = <&intc>;
-
- memory {
- device_type = "memory";
- reg = <0x0 0x10000000>;
- };
-
- chosen {
- bootargs = "console=ttyPS0,9600 root=/dev/ram rw initrd=0x800000,8M earlyprintk";
- linux,stdout-path = &uart0;
- };
+ compatible = "xlnx,zynq-7000";
amba {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
+ interrupt-parent = <&intc>;
ranges;
intc: interrupt-controller@f8f01000 {
diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
new file mode 100644
index 0000000..e25a307
--- /dev/null
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2011 Xilinx
+ * Copyright (C) 2012 National Instruments Corp.
+ *
+ * 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.
+ */
+/dts-v1/;
+/include/ "zynq-7000.dtsi"
+
+/ {
+ model = "Zynq ZC702 Development Board";
+ compatible = "xlnx,zynq-zc702", "xlnx,zynq-7000";
+
+ memory {
+ device_type = "memory";
+ reg = <0x0 0x40000000>;
+ };
+
+ chosen {
+ bootargs = "console=ttyPS1,115200 earlyprintk";
+ };
+
+};
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 0279ea7..447904b 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -115,7 +115,8 @@ static void __init xilinx_map_io(void)
}
static const char *xilinx_dt_match[] = {
- "xlnx,zynq-ep107",
+ "xlnx,zynq-zc702",
+ "xlnx,zynq-7000",
NULL
};
--
1.8.0
On Wed, Oct 31, 2012 at 01:56:14PM -0600, Josh Cartwright wrote:
> Add support for retrieving TTC configuration from device tree. This
> includes the ability to pull information about the driving clocks from
> the of_clk bindings.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> diff --git a/drivers/clocksource/xilinx_ttc.c b/drivers/clocksource/xilinx_ttc.c
> index ff38b3e..a4718f7 100644
> --- a/drivers/clocksource/xilinx_ttc.c
> +++ b/drivers/clocksource/xilinx_ttc.c
> @@ -209,7 +153,8 @@ static struct clocksource clocksource_xttcpss = {
> static int xttcpss_set_next_event(unsigned long cycles,
> struct clock_event_device *evt)
> {
> - struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
> + struct xttcpss_timer_clockevent *xttce = to_xttcpss_timer_clkevent(evt);
> + struct xttcpss_timer *timer = &xttce->xttc;
>
> xttcpss_set_interval(timer, cycles);
> return 0;
> @@ -224,12 +169,14 @@ static int xttcpss_set_next_event(unsigned long cycles,
> static void xttcpss_set_mode(enum clock_event_mode mode,
> struct clock_event_device *evt)
> {
> - struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
> + struct xttcpss_timer_clockevent *xttce = to_xttcpss_timer_clkevent(evt);
> + struct xttcpss_timer *timer = &xttce->xttc;
> u32 ctrl_reg;
>
> switch (mode) {
> case CLOCK_EVT_MODE_PERIODIC:
> - xttcpss_set_interval(timer, TIMER_RATE / HZ);
> + xttcpss_set_interval(timer,
> + clk_get_rate(xttce->clk) / PRESCALE);
I discovered with further testing that the above calculation is broken;
calculated interval also needs to be divided by HZ.
(I'll post a v2; just wanted to get this out there in the slim chance
anyone's testing this ;)
Josh
On 10/31/2012 08:28 PM, Josh Cartwright wrote:
> Add support for specifying clock information for the uart clk via the
> device tree. This eliminates the need to hardcode rates in the device
> tree.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 4 ++--
> drivers/tty/serial/xilinx_uartps.c | 30 +++++++++++++++++-------------
> 2 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index bb3085c..5fb763f 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -44,14 +44,14 @@
> compatible = "xlnx,xuartps";
> reg = <0xE0000000 0x1000>;
> interrupts = <0 27 4>;
> - clock = <50000000>;
> + clocks = <&uart_clk 0>;
> };
>
> uart1: uart@e0001000 {
> compatible = "xlnx,xuartps";
> reg = <0xE0001000 0x1000>;
> interrupts = <0 50 4>;
> - clock = <50000000>;
> + clocks = <&uart_clk 0>;
Shouldn't this be <&uart_clk 1>?
> };
>
> slcr: slcr@f8000000 {
On 10/31/2012 07:58 PM, Josh Cartwright wrote:
> [...]
> +#define PERIPH_CLK_CTRL_SRC(x) (periph_clk_parent_map[((x)&3)>>4])
> +#define PERIPH_CLK_CTRL_DIV(x) (((x)&0x3F00)>>8)
A few more spaces wouldn't hurt ;)
> [...]
> +static void __init zynq_periph_clk_setup(struct device_node *np)
> +{
> + struct zynq_periph_clk *periph;
> + const char *parent_names[3];
> + struct clk_init_data init;
> + struct clk *clk;
> + int err;
> + u32 reg;
> + int i;
> +
> + err = of_property_read_u32(np, "reg", ®);
> + WARN_ON(err);
Shouldn't the function abort if a error happens somewhere? Continuing here
will lead to undefined behavior. Same is probably true for the other WARN_ONs.
> +
> + periph = kzalloc(sizeof(*periph), GFP_KERNEL);
> + WARN_ON(!periph);
> +
> + periph->clk_ctrl = slcr_base + reg;
> + spin_lock_init(&periph->clkact_lock);
> +
> + init.name = np->name;
> + init.ops = &zynq_periph_clk_ops;
> + for (i = 0; i < ARRAY_SIZE(parent_names); i++)
> + parent_names[i] = of_clk_get_parent_name(np, i);
> + init.parent_names = parent_names;
> + init.num_parents = ARRAY_SIZE(parent_names);
> +
> + periph->hw.init = &init;
> +
> + clk = clk_register(NULL, &periph->hw);
> + WARN_ON(IS_ERR(clk));
> +
> + err = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> + WARN_ON(err);
> +
> + for (i = 0; i < 2; i++) {
Not all of the peripheral clock generators have two output clocks. I think
it makes sense to use the number entries in clock-output-names here.
> + const char *name;
> +
> + err = of_property_read_string_index(np, "clock-output-names", i,
> + &name);
> + WARN_ON(err);
> +
> + periph->gates[i] = clk_register_gate(NULL, name, np->name, 0,
> + periph->clk_ctrl, i, 0,
> + &periph->clkact_lock);
> + WARN_ON(IS_ERR(periph->gates[i]));
> + }
> +
> + periph->onecell_data.clks = periph->gates;
> + periph->onecell_data.clk_num = i;
> +
> + err = of_clk_add_provider(np, of_clk_src_onecell_get,
> + &periph->onecell_data);
> + WARN_ON(err);
> +}
> [...]
Thanks for the review.
On Fri, Nov 02, 2012 at 10:33:44AM +0100, Lars-Peter Clausen wrote:
> On 10/31/2012 07:58 PM, Josh Cartwright wrote:
> > [...]
> > +#define PERIPH_CLK_CTRL_SRC(x) (periph_clk_parent_map[((x)&3)>>4])
> > +#define PERIPH_CLK_CTRL_DIV(x) (((x)&0x3F00)>>8)
>
> A few more spaces wouldn't hurt ;)
Okay, sure.
> > [...]
> > +static void __init zynq_periph_clk_setup(struct device_node *np)
> > +{
> > + struct zynq_periph_clk *periph;
> > + const char *parent_names[3];
> > + struct clk_init_data init;
> > + struct clk *clk;
> > + int err;
> > + u32 reg;
> > + int i;
> > +
> > + err = of_property_read_u32(np, "reg", ®);
> > + WARN_ON(err);
>
> Shouldn't the function abort if a error happens somewhere? Continuing here
> will lead to undefined behavior. Same is probably true for the other WARN_ONs.
The way I see it is: the kernel is will be left in a bad state in the
case of any failure, regardless of if we bail out or continue. AFAICT,
there is no clean way to recover from a failure this early.
Given that, it seems simpler (albeit marginally so) just to continue; so
that's what I chose to do. I'm not opposed to bailing out, just not
convinced it does anything for us.
> > +
> > + periph = kzalloc(sizeof(*periph), GFP_KERNEL);
> > + WARN_ON(!periph);
> > +
> > + periph->clk_ctrl = slcr_base + reg;
> > + spin_lock_init(&periph->clkact_lock);
> > +
> > + init.name = np->name;
> > + init.ops = &zynq_periph_clk_ops;
> > + for (i = 0; i < ARRAY_SIZE(parent_names); i++)
> > + parent_names[i] = of_clk_get_parent_name(np, i);
> > + init.parent_names = parent_names;
> > + init.num_parents = ARRAY_SIZE(parent_names);
> > +
> > + periph->hw.init = &init;
> > +
> > + clk = clk_register(NULL, &periph->hw);
> > + WARN_ON(IS_ERR(clk));
> > +
> > + err = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> > + WARN_ON(err);
> > +
> > + for (i = 0; i < 2; i++) {
>
> Not all of the peripheral clock generators have two output clocks. I think
> it makes sense to use the number entries in clock-output-names here.
Yes, I agree. I'll also update the bindings documentation.
Thanks again,
Josh
On Fri, Nov 02, 2012 at 10:20:33AM +0100, Lars-Peter Clausen wrote:
> On 10/31/2012 08:28 PM, Josh Cartwright wrote:
> > Add support for specifying clock information for the uart clk via the
> > device tree. This eliminates the need to hardcode rates in the device
> > tree.
> >
> > Signed-off-by: Josh Cartwright <[email protected]>
> > ---
> > arch/arm/boot/dts/zynq-7000.dtsi | 4 ++--
> > drivers/tty/serial/xilinx_uartps.c | 30 +++++++++++++++++-------------
> > 2 files changed, 19 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> > index bb3085c..5fb763f 100644
> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> > @@ -44,14 +44,14 @@
> > compatible = "xlnx,xuartps";
> > reg = <0xE0000000 0x1000>;
> > interrupts = <0 27 4>;
> > - clock = <50000000>;
> > + clocks = <&uart_clk 0>;
> > };
> >
> > uart1: uart@e0001000 {
> > compatible = "xlnx,xuartps";
> > reg = <0xE0001000 0x1000>;
> > interrupts = <0 50 4>;
> > - clock = <50000000>;
> > + clocks = <&uart_clk 0>;
>
> Shouldn't this be <&uart_clk 1>?
Yes, indeed.
Thanks,
Josh
On 11/02/2012 02:38 PM, Josh Cartwright wrote:
> Thanks for the review.
>
> On Fri, Nov 02, 2012 at 10:33:44AM +0100, Lars-Peter Clausen wrote:
>> On 10/31/2012 07:58 PM, Josh Cartwright wrote:
>>> [...]
>>> +#define PERIPH_CLK_CTRL_SRC(x) (periph_clk_parent_map[((x)&3)>>4])
>>> +#define PERIPH_CLK_CTRL_DIV(x) (((x)&0x3F00)>>8)
>>
>> A few more spaces wouldn't hurt ;)
>
> Okay, sure.
>
>>> [...]
>>> +static void __init zynq_periph_clk_setup(struct device_node *np)
>>> +{
>>> + struct zynq_periph_clk *periph;
>>> + const char *parent_names[3];
>>> + struct clk_init_data init;
>>> + struct clk *clk;
>>> + int err;
>>> + u32 reg;
>>> + int i;
>>> +
>>> + err = of_property_read_u32(np, "reg", ®);
>>> + WARN_ON(err);
>>
>> Shouldn't the function abort if a error happens somewhere? Continuing here
>> will lead to undefined behavior. Same is probably true for the other WARN_ONs.
>
> The way I see it is: the kernel is will be left in a bad state in the
> case of any failure, regardless of if we bail out or continue. AFAICT,
> there is no clean way to recover from a failure this early.
>
> Given that, it seems simpler (albeit marginally so) just to continue; so
> that's what I chose to do. I'm not opposed to bailing out, just not
> convinced it does anything for us.
>
The issue with this approach is that, while you get a warning, unexpected
seemingly unrelated side-effects may happen later on. E.g. if no reg
property for the clock is specified the reg variable will be uninitialized
and contain whatever was on the stack before. The clock will be registered
nonetheless and the boot process continues. Now if the clock is enabled a
bit in a random register will be modified, which could result in strange and
abnormal behavior, which can be very hard to track down.
Also if for example just one clock has its reg property missing the system
will continue to boot if we bail out here. It is just that the peripherals
using that clock won't work. Which will certainly be easier to diagnose than
random abnormal behavior.
>>> +
>>> + periph = kzalloc(sizeof(*periph), GFP_KERNEL);
>>> + WARN_ON(!periph);
>>> +
>>> + periph->clk_ctrl = slcr_base + reg;
>>> + spin_lock_init(&periph->clkact_lock);
>>> +
>>> + init.name = np->name;
>>> + init.ops = &zynq_periph_clk_ops;
>>> + for (i = 0; i < ARRAY_SIZE(parent_names); i++)
>>> + parent_names[i] = of_clk_get_parent_name(np, i);
>>> + init.parent_names = parent_names;
>>> + init.num_parents = ARRAY_SIZE(parent_names);
>>> +
>>> + periph->hw.init = &init;
>>> +
>>> + clk = clk_register(NULL, &periph->hw);
>>> + WARN_ON(IS_ERR(clk));
>>> +
>>> + err = of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> + WARN_ON(err);
>>> +
>>> + for (i = 0; i < 2; i++) {
>>
>> Not all of the peripheral clock generators have two output clocks. I think
>> it makes sense to use the number entries in clock-output-names here.
>
> Yes, I agree. I'll also update the bindings documentation.
>
> Thanks again,
> Josh
On Fri, Nov 02, 2012 at 04:12:21PM +0100, Lars-Peter Clausen wrote:
> On 11/02/2012 02:38 PM, Josh Cartwright wrote:
> > On Fri, Nov 02, 2012 at 10:33:44AM +0100, Lars-Peter Clausen wrote:
> >> On 10/31/2012 07:58 PM, Josh Cartwright wrote:
[...]
> >>> +static void __init zynq_periph_clk_setup(struct device_node *np)
> >>> +{
> >>> + struct zynq_periph_clk *periph;
> >>> + const char *parent_names[3];
> >>> + struct clk_init_data init;
> >>> + struct clk *clk;
> >>> + int err;
> >>> + u32 reg;
> >>> + int i;
> >>> +
> >>> + err = of_property_read_u32(np, "reg", ®);
> >>> + WARN_ON(err);
> >>
> >> Shouldn't the function abort if a error happens somewhere? Continuing here
> >> will lead to undefined behavior. Same is probably true for the other WARN_ONs.
> >
> > The way I see it is: the kernel is will be left in a bad state in the
> > case of any failure, regardless of if we bail out or continue. AFAICT,
> > there is no clean way to recover from a failure this early.
> >
> > Given that, it seems simpler (albeit marginally so) just to continue; so
> > that's what I chose to do. I'm not opposed to bailing out, just not
> > convinced it does anything for us.
> >
> The issue with this approach is that, while you get a warning, unexpected
> seemingly unrelated side-effects may happen later on. E.g. if no reg
> property for the clock is specified the reg variable will be uninitialized
> and contain whatever was on the stack before. The clock will be registered
> nonetheless and the boot process continues. Now if the clock is enabled a
> bit in a random register will be modified, which could result in strange and
> abnormal behavior, which can be very hard to track down.
Okay.....but any reasonable person would start their debugging quest at
the source of the WARN_ON. If someone sees the WARN_ON message but
stupidly chooses to ignore it, they deserves to spend the time trying to
track down abnormal behavior, so I'm still not convinced.
Josh
2012/10/31 Josh Cartwright <[email protected]>:
> Move the sys_timer definition out of ttc driver and make it part of the
> common zynq code. This is preparation for renaming and COMMON_CLK
> support.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/mach-zynq/common.c | 13 +++++++++++++
> arch/arm/mach-zynq/common.h | 4 +---
> arch/arm/mach-zynq/timer.c | 10 +---------
> 3 files changed, 15 insertions(+), 12 deletions(-)
>
Tested-by: Michal Simek <[email protected]>
Looks good to me. I have added it to my testing branch
and will provide path to mainline through xilinx arm-next branch
Thanks,
Michal
2012/10/29 Josh Cartwright <[email protected]>:
> Suggested cleanup by Arnd Bergmann. Move the ttc timer.c code to
> drivers/clocksource, and out of the mach-zynq directory.
>
> The common.h (which only held the timer declaration) was renamed to
> xilinx_ttc.h and moved into include/linux.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> ---
> arch/arm/mach-zynq/Makefile | 2 +-
> arch/arm/mach-zynq/common.c | 2 +-
> drivers/clocksource/Makefile | 1 +
> arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c | 1 -
> arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h | 4 ++--
> 5 files changed, 5 insertions(+), 5 deletions(-)
> rename arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c (99%)
> rename arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h (91%)
Not going to apply this patch till there is clean way how to move all
drivers there.
Especially I don't like to add xilinx_ttc.h to include/linux folder.
Thanks,
Michal
2012/10/31 Josh Cartwright <[email protected]>:
> The purpose of the created zynq-7000.dtsi file is to describe the
> hardware common to all Zynq 7000-based boards. Also, get rid of the
> zynq-ep107 device tree, since it is not hardware anyone can purchase.
>
> Add a zc702 dts file based on the zynq-7000.dtsi. Add it to the
> dts/Makefile so it is built with the 'dtbs' target.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/boot/dts/Makefile | 1 +
> .../boot/dts/{zynq-ep107.dts => zynq-7000.dtsi} | 19 +++-----------
> arch/arm/boot/dts/zynq-zc702.dts | 30 ++++++++++++++++++++++
> arch/arm/mach-zynq/common.c | 3 ++-
> 4 files changed, 36 insertions(+), 17 deletions(-)
> rename arch/arm/boot/dts/{zynq-ep107.dts => zynq-7000.dtsi} (79%)
> create mode 100644 arch/arm/boot/dts/zynq-zc702.dts
Not going to apply this. We need to finish our discussion in
"[PATCH v4 1/5] zynq: use GIC device tree bindings" before.
Definitely I like idea to use "xlnx,zynq-7000" generic model name.
Thanks,
Michal
2012/10/31 Josh Cartwright <[email protected]>:
> The zynq-7000 has an additional UART at 0xE0001000. Describe it in the
> device tree.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/boot/dts/zynq-ep107.dts | 7 +++++++
> 1 file changed, 7 insertions(+)
Applied to my testing branch.
Thanks,
Michal
2012/10/30 Josh Cartwright <[email protected]>:
> The Zynq platform requires the use of CONFIG_OF. Remove the #ifdef
> conditionals in the uartps driver.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> drivers/tty/serial/xilinx_uartps.c | 9 ---------
> 1 file changed, 9 deletions(-)
Please send this separately out of this patchset.
Also if you do this change which is understandable, you should also
add
depends on OF
Thanks,
Michal
2012/11/2 Josh Cartwright <[email protected]>:
> On Fri, Nov 02, 2012 at 04:12:21PM +0100, Lars-Peter Clausen wrote:
>> On 11/02/2012 02:38 PM, Josh Cartwright wrote:
>> > On Fri, Nov 02, 2012 at 10:33:44AM +0100, Lars-Peter Clausen wrote:
>> >> On 10/31/2012 07:58 PM, Josh Cartwright wrote:
> [...]
>> >>> +static void __init zynq_periph_clk_setup(struct device_node *np)
>> >>> +{
>> >>> + struct zynq_periph_clk *periph;
>> >>> + const char *parent_names[3];
>> >>> + struct clk_init_data init;
>> >>> + struct clk *clk;
>> >>> + int err;
>> >>> + u32 reg;
>> >>> + int i;
>> >>> +
>> >>> + err = of_property_read_u32(np, "reg", ®);
>> >>> + WARN_ON(err);
>> >>
>> >> Shouldn't the function abort if a error happens somewhere? Continuing here
>> >> will lead to undefined behavior. Same is probably true for the other WARN_ONs.
>> >
>> > The way I see it is: the kernel is will be left in a bad state in the
>> > case of any failure, regardless of if we bail out or continue. AFAICT,
>> > there is no clean way to recover from a failure this early.
>> >
>> > Given that, it seems simpler (albeit marginally so) just to continue; so
>> > that's what I chose to do. I'm not opposed to bailing out, just not
>> > convinced it does anything for us.
>> >
>> The issue with this approach is that, while you get a warning, unexpected
>> seemingly unrelated side-effects may happen later on. E.g. if no reg
>> property for the clock is specified the reg variable will be uninitialized
>> and contain whatever was on the stack before. The clock will be registered
>> nonetheless and the boot process continues. Now if the clock is enabled a
>> bit in a random register will be modified, which could result in strange and
>> abnormal behavior, which can be very hard to track down.
>
> Okay.....but any reasonable person would start their debugging quest at
> the source of the WARN_ON. If someone sees the WARN_ON message but
> stupidly chooses to ignore it, they deserves to spend the time trying to
> track down abnormal behavior, so I'm still not convinced.
I am with Lars. You would be surprised how many people do no read bootlog.
It should be handled better.
Thanks,
Michal
2012/10/31 Josh Cartwright <[email protected]>:
> Add support for retrieving TTC configuration from device tree. This
> includes the ability to pull information about the driving clocks from
> the of_clk bindings.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 53 ++++++++
> arch/arm/boot/dts/zynq-zc702.dts | 10 ++
> drivers/clocksource/xilinx_ttc.c | 273 ++++++++++++++++++++++-----------------
> 3 files changed, 218 insertions(+), 118 deletions(-)
>
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index 5fb763f..9a2442c 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -109,5 +109,58 @@
> };
> };
> };
> +
> + ttc0: ttc0@f8001000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "xlnx,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: ttc0@f8002000 {
Also this is ttc1: ttc1.
These type of faults can be simple removed by proper dts node generation.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
On 11/05/2012 05:22 AM, Michal Simek wrote:
> 2012/10/29 Josh Cartwright <[email protected]>:
>> Suggested cleanup by Arnd Bergmann. Move the ttc timer.c code to
>> drivers/clocksource, and out of the mach-zynq directory.
>>
>> The common.h (which only held the timer declaration) was renamed to
>> xilinx_ttc.h and moved into include/linux.
>>
>> Signed-off-by: Josh Cartwright <[email protected]>
>> Cc: Arnd Bergmann <[email protected]>
>> ---
>> arch/arm/mach-zynq/Makefile | 2 +-
>> arch/arm/mach-zynq/common.c | 2 +-
>> drivers/clocksource/Makefile | 1 +
>> arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c | 1 -
>> arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h | 4 ++--
>> 5 files changed, 5 insertions(+), 5 deletions(-)
>> rename arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c (99%)
>> rename arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h (91%)
>
> Not going to apply this patch till there is clean way how to move all
> drivers there.
> Especially I don't like to add xilinx_ttc.h to include/linux folder.
>
A cleaner way is how we are doing irqchips [1]. This needs a single
function (or one each for clksrc and clkevt) that has a DT match list of
all known timers and calls their init function. It should be a bit
simpler than irqchips init function because you don't need to worry
about hierarchy init ordering. That doesn't solve non-DT though and if
there are any extra functions like we have with irqchips, you still need
the header in include/linux.
Rob
[1] http://www.spinics.net/lists/arm-kernel/msg203687.html
On Mon, Nov 05, 2012 at 12:22:55PM +0100, Michal Simek wrote:
> 2012/10/29 Josh Cartwright <[email protected]>:
> > Suggested cleanup by Arnd Bergmann. Move the ttc timer.c code to
> > drivers/clocksource, and out of the mach-zynq directory.
> >
> > The common.h (which only held the timer declaration) was renamed to
> > xilinx_ttc.h and moved into include/linux.
> >
> > Signed-off-by: Josh Cartwright <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > ---
> > arch/arm/mach-zynq/Makefile | 2 +-
> > arch/arm/mach-zynq/common.c | 2 +-
> > drivers/clocksource/Makefile | 1 +
> > arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c | 1 -
> > arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h | 4 ++--
> > 5 files changed, 5 insertions(+), 5 deletions(-)
> > rename arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c (99%)
> > rename arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h (91%)
>
> Not going to apply this patch till there is clean way how to move all
> drivers there. Especially I don't like to add xilinx_ttc.h to
> include/linux folder.
Okay; I think it's best to defer the moving of the ttc driver from this
patchset. It is not a dependency of the clk driver support stuff.
If you agree, I can spin up a v2 of the patchset w/o this change, and
without the serial CONFIG_OF stuff. Should v2 contain the patches
you've already pulled into testing?
I'll give Rob's irqchip-like suggestion a spin and see how that works
out in parallel.
Thanks,
Josh
2012/11/5 Josh Cartwright <[email protected]>:
> On Mon, Nov 05, 2012 at 12:22:55PM +0100, Michal Simek wrote:
>> 2012/10/29 Josh Cartwright <[email protected]>:
>> > Suggested cleanup by Arnd Bergmann. Move the ttc timer.c code to
>> > drivers/clocksource, and out of the mach-zynq directory.
>> >
>> > The common.h (which only held the timer declaration) was renamed to
>> > xilinx_ttc.h and moved into include/linux.
>> >
>> > Signed-off-by: Josh Cartwright <[email protected]>
>> > Cc: Arnd Bergmann <[email protected]>
>> > ---
>> > arch/arm/mach-zynq/Makefile | 2 +-
>> > arch/arm/mach-zynq/common.c | 2 +-
>> > drivers/clocksource/Makefile | 1 +
>> > arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c | 1 -
>> > arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h | 4 ++--
>> > 5 files changed, 5 insertions(+), 5 deletions(-)
>> > rename arch/arm/mach-zynq/timer.c => drivers/clocksource/xilinx_ttc.c (99%)
>> > rename arch/arm/mach-zynq/common.h => include/linux/xilinx_ttc.h (91%)
>>
>> Not going to apply this patch till there is clean way how to move all
>> drivers there. Especially I don't like to add xilinx_ttc.h to
>> include/linux folder.
>
> Okay; I think it's best to defer the moving of the ttc driver from this
> patchset. It is not a dependency of the clk driver support stuff.
> If you agree, I can spin up a v2 of the patchset w/o this change, and
> without the serial CONFIG_OF stuff. Should v2 contain the patches
> you've already pulled into testing?
Sure and I see you have done which is good.
Thanks,
Michal