* [PATCH v3 13/13] clocksource/drivers/rockchip_timer: Prevent ftrace recursion
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Currently rockchip_timer can be used as a scheduler clock. We properly
marked rk_timer_sched_clock_read() as notrace but we then call another
function rk_timer_counter_read() that _wasn't_ notrace.
Having a traceable function in the sched_clock() path leads to a recursion
within ftrace and a kernel crash.
Fix this by adding an extra notrace function to keep other users of
rk_timer_counter_read() traceable.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/clocksource/rockchip_timer.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index 1af80a0..a127822 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -87,7 +87,7 @@ static void rk_timer_update_counter(u64 cycles, struct rk_timer *timer)
writel_relaxed(upper, timer->base + TIMER_LOAD_COUNT1);
}
-static u64 rk_timer_counter_read(struct rk_timer *timer)
+static u64 notrace _rk_timer_counter_read(struct rk_timer *timer)
{
u64 counter;
u32 lower;
@@ -106,6 +106,11 @@ static u64 rk_timer_counter_read(struct rk_timer *timer)
return counter;
}
+static u64 rk_timer_counter_read(struct rk_timer *timer)
+{
+ return _rk_timer_counter_read(timer);
+}
+
static void rk_timer_interrupt_clear(struct rk_timer *timer)
{
writel_relaxed(1, timer->base + TIMER_INT_STATUS);
@@ -168,7 +173,7 @@ static u64 notrace rk_timer_sched_clock_read(void)
{
struct rk_clocksource *_cs = &cs_timer;
- return ~rk_timer_counter_read(&_cs->timer);
+ return ~_rk_timer_counter_read(&_cs->timer);
}
static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 12/13] clocksource/drivers/rockchip_timer: implement clocksource timer
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
The clock supplying the arm-global-timer on the rk3188 is coming from the
the cpu clock itself and thus changes its rate everytime cpufreq adjusts
the cpu frequency making this timer unsuitable as a stable clocksource.
The rk3188, rk3288 and following socs share a separate timer block already
handled by the rockchip-timer driver. Therefore adapt this driver to also
be able to act as clocksource on rk3188.
In order to test clocksource you can run following commands and check
how much time it take in real. On rk3188 it take about ~45 seconds.
Such error cannot be fixed using NTP. Haven't test clocksource
on rk3288 and onwards. Guess they can also have unstable clocksource.
cpufreq-set -f 1.6GHZ
date; sleep 60; date
In order to use the patch you need to declare two timers in the dts
file. The first timer will be initialized as clockevent provider
and the second one as clocksource. The clockevent must be from
alive subsystem as it used as backup for the local timers at sleep
time.
In order to resolve ambiguity between timers in the device tree,
it is possible to correctly number the timers using "aliases" node.
The patch does not break compatibility with older device tree files.
The older device tree files contain only one timer. The timer
will be initialized as clockevent, as expected.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/clocksource/rockchip_timer.c | 101 ++++++++++++++++++++++++++++------
1 file changed, 85 insertions(+), 16 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index 6224aa9..1af80a0 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -11,6 +11,7 @@
#include <linux/clockchips.h>
#include <linux/init.h>
#include <linux/interrupt.h>
+#include <linux/sched_clock.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
@@ -42,7 +43,19 @@ struct rk_clock_event_device {
struct rk_timer timer;
};
+struct rk_clocksource {
+ struct clocksource cs;
+ struct rk_timer timer;
+};
+
+enum {
+ ROCKCHIP_CLKSRC_CLOCKEVENT = 0,
+ ROCKCHIP_CLKSRC_CLOCKSOURCE = 1,
+};
+
static struct rk_clock_event_device bc_timer;
+static struct rk_clocksource cs_timer;
+static int rk_next_clksrc = ROCKCHIP_CLKSRC_CLOCKEVENT;
static inline struct rk_clock_event_device*
rk_clock_event_device(struct clock_event_device *ce)
@@ -143,13 +156,46 @@ static irqreturn_t rk_timer_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static cycle_t rk_timer_clocksource_read(struct clocksource *cs)
+{
+ struct rk_clocksource *_cs =
+ container_of(cs, struct rk_clocksource, cs);
+
+ return ~rk_timer_counter_read(&_cs->timer);
+}
+
+static u64 notrace rk_timer_sched_clock_read(void)
+{
+ struct rk_clocksource *_cs = &cs_timer;
+
+ return ~rk_timer_counter_read(&_cs->timer);
+}
+
static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
{
- struct clock_event_device *ce = &bc_timer.ce;
- struct rk_timer *timer = &bc_timer.timer;
+ struct clock_event_device *ce = NULL;
+ struct clocksource *cs = NULL;
+ struct rk_timer *timer = NULL;
struct clk *timer_clk;
struct clk *pclk;
int ret = -EINVAL, irq;
+ int clksrc;
+
+ clksrc = rk_next_clksrc;
+ rk_next_clksrc++;
+
+ switch (clksrc) {
+ case ROCKCHIP_CLKSRC_CLOCKEVENT:
+ ce = &bc_timer.ce;
+ timer = &bc_timer.timer;
+ break;
+ case ROCKCHIP_CLKSRC_CLOCKSOURCE:
+ cs = &cs_timer.cs;
+ timer = &cs_timer.timer;
+ break;
+ default:
+ return -ENODEV;
+ }
timer->base = of_iomap(np, 0);
if (!timer->base) {
@@ -193,26 +239,49 @@ static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
goto out_irq;
}
- ce->name = TIMER_NAME;
- ce->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
- CLOCK_EVT_FEAT_DYNIRQ;
- ce->set_next_event = rk_timer_set_next_event;
- ce->set_state_shutdown = rk_timer_shutdown;
- ce->set_state_periodic = rk_timer_set_periodic;
- ce->irq = irq;
- ce->cpumask = cpu_possible_mask;
- ce->rating = 250;
+ if (ce) {
+ ce->name = TIMER_NAME;
+ ce->features = CLOCK_EVT_FEAT_PERIODIC |
+ CLOCK_EVT_FEAT_ONESHOT |
+ CLOCK_EVT_FEAT_DYNIRQ;
+ ce->set_next_event = rk_timer_set_next_event;
+ ce->set_state_shutdown = rk_timer_shutdown;
+ ce->set_state_periodic = rk_timer_set_periodic;
+ ce->irq = irq;
+ ce->cpumask = cpu_possible_mask;
+ ce->rating = 250;
+ }
+
+ if (cs) {
+ cs->name = TIMER_NAME;
+ cs->flags = CLOCK_SOURCE_IS_CONTINUOUS;
+ cs->mask = CLOCKSOURCE_MASK(64);
+ cs->read = rk_timer_clocksource_read;
+ cs->rating = 250;
+ }
rk_timer_interrupt_clear(timer);
rk_timer_disable(timer);
- ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER, TIMER_NAME, ce);
- if (ret) {
- pr_err("Failed to initialize '%s': %d\n", TIMER_NAME, ret);
- goto out_irq;
+ if (ce) {
+ ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER,
+ TIMER_NAME, ce);
+ if (ret) {
+ pr_err("Failed to initialize '%s': %d\n",
+ TIMER_NAME, ret);
+ goto out_irq;
+ }
+
+ clockevents_config_and_register(ce, timer->freq, 1, UINT_MAX);
}
- clockevents_config_and_register(ce, timer->freq, 1, UINT_MAX);
+ if (cs) {
+ rk_timer_update_counter(U64_MAX, timer);
+ rk_timer_enable(timer, 0);
+ clocksource_register_hz(cs, timer->freq);
+ sched_clock_register(rk_timer_sched_clock_read, 64,
+ timer->freq);
+ }
return 0;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 11/13] clocksource/drivers/rockchip_timer: implement reading 64bit value from timer
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/clocksource/rockchip_timer.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index c2b0454..6224aa9 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -19,6 +19,8 @@
#define TIMER_LOAD_COUNT0 0x00
#define TIMER_LOAD_COUNT1 0x04
+#define TIMER_CURRENT_VALUE0 0x08
+#define TIMER_CURRENT_VALUE1 0x0C
#define TIMER_CONTROL_REG3288 0x10
#define TIMER_CONTROL_REG3399 0x1c
#define TIMER_INT_STATUS 0x18
@@ -72,6 +74,25 @@ static void rk_timer_update_counter(u64 cycles, struct rk_timer *timer)
writel_relaxed(upper, timer->base + TIMER_LOAD_COUNT1);
}
+static u64 rk_timer_counter_read(struct rk_timer *timer)
+{
+ u64 counter;
+ u32 lower;
+ u32 upper, old_upper;
+
+ upper = readl_relaxed(timer->base + TIMER_CURRENT_VALUE1);
+ do {
+ old_upper = upper;
+ lower = readl_relaxed(timer->base + TIMER_CURRENT_VALUE0);
+ upper = readl_relaxed(timer->base + TIMER_CURRENT_VALUE1);
+ } while (upper != old_upper);
+
+ counter = upper;
+ counter <<= 32;
+ counter |= lower;
+ return counter;
+}
+
static void rk_timer_interrupt_clear(struct rk_timer *timer)
{
writel_relaxed(1, timer->base + TIMER_INT_STATUS);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 10/13] clocksource/drivers/rockchip_timer: implement loading 64bit value into timer
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/clocksource/rockchip_timer.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index 61c3bb1..c2b0454 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -63,11 +63,13 @@ static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
-static void rk_timer_update_counter(unsigned long cycles,
- struct rk_timer *timer)
+static void rk_timer_update_counter(u64 cycles, struct rk_timer *timer)
{
- writel_relaxed(cycles, timer->base + TIMER_LOAD_COUNT0);
- writel_relaxed(0, timer->base + TIMER_LOAD_COUNT1);
+ u32 lower = cycles & 0xFFFFFFFF;
+ u32 upper = (cycles >> 32) & 0xFFFFFFFF;
+
+ writel_relaxed(lower, timer->base + TIMER_LOAD_COUNT0);
+ writel_relaxed(upper, timer->base + TIMER_LOAD_COUNT1);
}
static void rk_timer_interrupt_clear(struct rk_timer *timer)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 09/13] clocksource/drivers/rockchip_timer: move TIMER_INT_UNMASK out of rk_timer_enable()
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This allow to enable timer without enabling interrupts from it.
As that mode will be used in clocksource implementation.
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/clocksource/rockchip_timer.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index a17dc61..61c3bb1 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -60,8 +60,7 @@ static inline void rk_timer_disable(struct rk_timer *timer)
static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
{
- writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
- timer->ctrl);
+ writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
static void rk_timer_update_counter(unsigned long cycles,
@@ -83,7 +82,8 @@ static inline int rk_timer_set_next_event(unsigned long cycles,
rk_timer_disable(timer);
rk_timer_update_counter(cycles, timer);
- rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT);
+ rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT |
+ TIMER_INT_UNMASK);
return 0;
}
@@ -101,7 +101,7 @@ static int rk_timer_set_periodic(struct clock_event_device *ce)
rk_timer_disable(timer);
rk_timer_update_counter(timer->freq / HZ - 1, timer);
- rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING);
+ rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING | TIMER_INT_UNMASK);
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 08/13] clocksource/drivers/rockchip_timer: drop unused rk_base() and rk_ctrl()
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Use of functions has been ceased by previous commit.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/clocksource/rockchip_timer.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index aa9ccd1..a17dc61 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -53,16 +53,6 @@ static inline struct rk_timer *rk_timer(struct clock_event_device *ce)
return &rk_clock_event_device(ce)->timer;
}
-static inline void __iomem *rk_base(struct clock_event_device *ce)
-{
- return rk_timer(ce)->base;
-}
-
-static inline void __iomem *rk_ctrl(struct clock_event_device *ce)
-{
- return rk_timer(ce)->ctrl;
-}
-
static inline void rk_timer_disable(struct rk_timer *timer)
{
writel_relaxed(TIMER_DISABLE, timer->ctrl);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 07/13] clocksource/drivers/rockchip_timer: low level routines take rk_timer as parameter
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel, devicetree, linux-arm-kernel, linux-rockchip
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet@gmail.com>
Pass rk_timer instead of clock_event_device to low lever timer routines.
So that code could be reused by clocksource implementation.
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
drivers/clocksource/rockchip_timer.c | 47 +++++++++++++++++++---------------
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index 6d68d4c..aa9ccd1 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -63,60 +63,67 @@ static inline void __iomem *rk_ctrl(struct clock_event_device *ce)
return rk_timer(ce)->ctrl;
}
-static inline void rk_timer_disable(struct clock_event_device *ce)
+static inline void rk_timer_disable(struct rk_timer *timer)
{
- writel_relaxed(TIMER_DISABLE, rk_ctrl(ce));
+ writel_relaxed(TIMER_DISABLE, timer->ctrl);
}
-static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
+static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
{
writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
- rk_ctrl(ce));
+ timer->ctrl);
}
static void rk_timer_update_counter(unsigned long cycles,
- struct clock_event_device *ce)
+ struct rk_timer *timer)
{
- writel_relaxed(cycles, rk_base(ce) + TIMER_LOAD_COUNT0);
- writel_relaxed(0, rk_base(ce) + TIMER_LOAD_COUNT1);
+ writel_relaxed(cycles, timer->base + TIMER_LOAD_COUNT0);
+ writel_relaxed(0, timer->base + TIMER_LOAD_COUNT1);
}
-static void rk_timer_interrupt_clear(struct clock_event_device *ce)
+static void rk_timer_interrupt_clear(struct rk_timer *timer)
{
- writel_relaxed(1, rk_base(ce) + TIMER_INT_STATUS);
+ writel_relaxed(1, timer->base + TIMER_INT_STATUS);
}
static inline int rk_timer_set_next_event(unsigned long cycles,
struct clock_event_device *ce)
{
- rk_timer_disable(ce);
- rk_timer_update_counter(cycles, ce);
- rk_timer_enable(ce, TIMER_MODE_USER_DEFINED_COUNT);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
+ rk_timer_update_counter(cycles, timer);
+ rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT);
return 0;
}
static int rk_timer_shutdown(struct clock_event_device *ce)
{
- rk_timer_disable(ce);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
return 0;
}
static int rk_timer_set_periodic(struct clock_event_device *ce)
{
- rk_timer_disable(ce);
- rk_timer_update_counter(rk_timer(ce)->freq / HZ - 1, ce);
- rk_timer_enable(ce, TIMER_MODE_FREE_RUNNING);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
+ rk_timer_update_counter(timer->freq / HZ - 1, timer);
+ rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING);
return 0;
}
static irqreturn_t rk_timer_interrupt(int irq, void *dev_id)
{
struct clock_event_device *ce = dev_id;
+ struct rk_timer *timer = rk_timer(ce);
- rk_timer_interrupt_clear(ce);
+ rk_timer_interrupt_clear(timer);
if (clockevent_state_oneshot(ce))
- rk_timer_disable(ce);
+ rk_timer_disable(timer);
ce->event_handler(ce);
@@ -183,8 +190,8 @@ static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
ce->cpumask = cpu_possible_mask;
ce->rating = 250;
- rk_timer_interrupt_clear(ce);
- rk_timer_disable(ce);
+ rk_timer_interrupt_clear(timer);
+ rk_timer_disable(timer);
ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER, TIMER_NAME, ce);
if (ret) {
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 06/13] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
The patch move ce field out of struct bc_timer into struct
rk_clock_event_device and rename struct bc_timer to struct rk_timer.
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/clocksource/rockchip_timer.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index 23e267a..6d68d4c 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -29,18 +29,28 @@
#define TIMER_MODE_USER_DEFINED_COUNT (1 << 1)
#define TIMER_INT_UNMASK (1 << 2)
-struct bc_timer {
- struct clock_event_device ce;
+struct rk_timer {
void __iomem *base;
void __iomem *ctrl;
u32 freq;
};
-static struct bc_timer bc_timer;
+struct rk_clock_event_device {
+ struct clock_event_device ce;
+ struct rk_timer timer;
+};
+
+static struct rk_clock_event_device bc_timer;
+
+static inline struct rk_clock_event_device*
+rk_clock_event_device(struct clock_event_device *ce)
+{
+ return container_of(ce, struct rk_clock_event_device, ce);
+}
-static inline struct bc_timer *rk_timer(struct clock_event_device *ce)
+static inline struct rk_timer *rk_timer(struct clock_event_device *ce)
{
- return container_of(ce, struct bc_timer, ce);
+ return &rk_clock_event_device(ce)->timer;
}
static inline void __iomem *rk_base(struct clock_event_device *ce)
@@ -116,16 +126,17 @@ static irqreturn_t rk_timer_interrupt(int irq, void *dev_id)
static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
{
struct clock_event_device *ce = &bc_timer.ce;
+ struct rk_timer *timer = &bc_timer.timer;
struct clk *timer_clk;
struct clk *pclk;
int ret = -EINVAL, irq;
- bc_timer.base = of_iomap(np, 0);
- if (!bc_timer.base) {
+ timer->base = of_iomap(np, 0);
+ if (!timer->base) {
pr_err("Failed to get base address for '%s'\n", TIMER_NAME);
return -ENXIO;
}
- bc_timer.ctrl = bc_timer.base + ctrl_reg;
+ timer->ctrl = timer->base + ctrl_reg;
pclk = of_clk_get_by_name(np, "pclk");
if (IS_ERR(pclk)) {
@@ -153,7 +164,7 @@ static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
goto out_timer_clk;
}
- bc_timer.freq = clk_get_rate(timer_clk);
+ timer->freq = clk_get_rate(timer_clk);
irq = irq_of_parse_and_map(np, 0);
if (!irq) {
@@ -181,7 +192,7 @@ static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
goto out_irq;
}
- clockevents_config_and_register(ce, bc_timer.freq, 1, UINT_MAX);
+ clockevents_config_and_register(ce, timer->freq, 1, UINT_MAX);
return 0;
@@ -190,7 +201,7 @@ out_irq:
out_timer_clk:
clk_disable_unprepare(pclk);
out_unmap:
- iounmap(bc_timer.base);
+ iounmap(timer->base);
return ret;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 05/13] ARM: dts: rockchip: disable arm-global-timer for rk3188
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
arm-global-timer can provide clockevents, clocksource and shed_clock. But
on rk3188 platform it provide only clocksource and shed_clock. clockevents
from arm-global-timer is not used by kernel because there is another
clockevent provider with higher rating (smp-twd).
My commit from the series implement clocksource and shed_clock using
rockchip_timer. But sched clock from rk_timer is not selected by kernel
due to lower frequency than arm-global-timer, and clocksource from
rk_timer is not selected by kernel due to lower rating than
arm-global-timer. And I don't want to increase clocksource rating
because ratings greater than 300 used for high frequency clocksources.
clocksource and shed_clock is quite unstable, because their rate depends
on cpu frequency. So disable arm-global-timer and use clocksource and
sched_clock from rockchip_timer.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
arch/arm/boot/dts/rk3188.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 0dc52fe..44da3d42 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -546,6 +546,7 @@
&global_timer {
interrupts = <GIC_PPI 11 0xf04>;
+ status = "disabled";
};
&local_timer {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 04/13] ARM: dts: rockchip: add timer entries to rk3188 dtsi
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
The patch add two timers to all rk3188 based boards.
The first timer is from alive subsystem and it act as a backup
for the local timers at sleep time. It act the same as timers
on other rockchip chips already present in kernel.
The second timer is from CPU subsystem and act as replacement
for the arm-global-timer clocksource. It run as stable frequency
24MHz.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
arch/arm/boot/dts/rk3188.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 31f81b2..0dc52fe 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -106,6 +106,22 @@
};
};
+ timer3: timer@2000e000 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x2000e000 0x20>;
+ interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru SCLK_TIMER3>, <&cru PCLK_TIMER3>;
+ clock-names = "timer", "pclk";
+ };
+
+ timer6: timer@200380a0 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x200380a0 0x20>;
+ interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru SCLK_TIMER6>, <&cru PCLK_TIMER0>;
+ clock-names = "timer", "pclk";
+ };
+
i2s0: i2s@1011a000 {
compatible = "rockchip,rk3188-i2s", "rockchip,rk3066-i2s";
reg = <0x1011a000 0x2000>;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 03/13] ARM: dts: rockchip: update compatible property for rk3229 timer
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Property set to '"rockchip,rk3229-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
arch/arm/boot/dts/rk3229-evb.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3229-evb.dts b/arch/arm/boot/dts/rk3229-evb.dts
index b6a1203..6629769 100644
--- a/arch/arm/boot/dts/rk3229-evb.dts
+++ b/arch/arm/boot/dts/rk3229-evb.dts
@@ -88,3 +88,7 @@
&uart2 {
status = "okay";
};
+
+&timer {
+ compatible = "rockchip,rk3229-timer", "rockchip,rk3288-timer";
+};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 02/13] ARM: dts: rockchip: update compatible property for rk3228 timer
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Property set to '"rockchip,rk3228-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
arch/arm/boot/dts/rk3228-evb.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3228-evb.dts b/arch/arm/boot/dts/rk3228-evb.dts
index 904668e..38eab87 100644
--- a/arch/arm/boot/dts/rk3228-evb.dts
+++ b/arch/arm/boot/dts/rk3228-evb.dts
@@ -70,3 +70,7 @@
&uart2 {
status = "okay";
};
+
+&timer {
+ compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
+};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 01/13] dt-bindings: clarify compatible property for rockchip timers
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480427118-5126-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Make all properties description in form '"rockchip,<chip>-timer",
"rockchip,rk3288-timer"' for all chips found in linux kernel.
Suggested-by: Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
.../bindings/timer/rockchip,rk-timer.txt | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
index a41b184..16a5f45 100644
--- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
+++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
@@ -1,9 +1,15 @@
Rockchip rk timer
Required properties:
-- compatible: shall be one of:
- "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368
- "rockchip,rk3399-timer" - for rk3399
+- compatible: should be:
+ "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
+ "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
+ "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
+ "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
+ "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
+ "rockchip,rk3288-timer": for Rockchip RK3288
+ "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
+ "rockchip,rk3399-timer": for Rockchip RK3399
- reg: base address of the timer register starting with TIMERS CONTROL register
- interrupts: should contain the interrupts for Timer0
- clocks : must contain an entry for each entry in clock-names
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 00/13] Implement clocksource for rockchip SoC using rockchip timer
From: Alexander Kochetkov @ 2016-11-29 13:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Gleixner, Heiko Stuebner, Mark Rutland, Rob Herring,
Russell King, Caesar Wang, Huang Tao, Daniel Lezcano,
Alexander Kochetkov
In-Reply-To: <1480343486-25539-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hello,
This patch series contain:
- devicetree bindings clarification for rockchip timers
- dts files fixes for rk3228-evb, rk3229-evb and rk3188
- implementation of clocksource for rockchip SoC
The clock supplying the arm-global-timer on the rk3188 is coming from the
the cpu clock itself and thus changes its rate everytime cpufreq adjusts
the cpu frequency making this timer unsuitable as a stable clocksource.
The rk3188, rk3288 and following socs share a separate timer block already
handled by the rockchip-timer driver. Therefore adapt this driver to also
be able to act as clocksource on rk3188.
In order to test clocksource you can run following commands and check
how much time it take in real. On rk3188 it take about ~45 seconds.
Such error cannot be fixed using NTP. Haven't test clocksource
on rk3288 and onwards. Guess they can also have unstable clocksource.
cpufreq-set -f 1.6GHZ
date; sleep 60; date
Regards,
Alexander.
Changes in v3:
added patches:
ARM: dts: rockchip: disable arm-global-timer for rk3188
clocksource/drivers/rockchip_timer: Prevent ftrace recursion
This is try 3. Please discard all v1 patches:
devicetree:
https://patchwork.ozlabs.org/patch/699019/
https://patchwork.ozlabs.org/patch/699020/
kernel:
https://patchwork.kernel.org/patch/9443975/
https://patchwork.kernel.org/patch/9443971/
https://patchwork.kernel.org/patch/9443959/
https://patchwork.kernel.org/patch/9443963/
https://patchwork.kernel.org/patch/9443979/
https://patchwork.kernel.org/patch/9443989/
https://patchwork.kernel.org/patch/9443987/
https://patchwork.kernel.org/patch/9443977/
https://patchwork.kernel.org/patch/9443991/
Old thread:
http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013147.html
Alexander Kochetkov (13):
dt-bindings: clarify compatible property for rockchip timers
ARM: dts: rockchip: update compatible property for rk3228 timer
ARM: dts: rockchip: update compatible property for rk3229 timer
ARM: dts: rockchip: add timer entries to rk3188 dtsi
ARM: dts: rockchip: disable arm-global-timer for rk3188
clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and
rk_clock_event_device
clocksource/drivers/rockchip_timer: low level routines take rk_timer
as parameter
clocksource/drivers/rockchip_timer: drop unused rk_base() and
rk_ctrl()
clocksource/drivers/rockchip_timer: move TIMER_INT_UNMASK out of
rk_timer_enable()
clocksource/drivers/rockchip_timer: implement loading 64bit value
into timer
clocksource/drivers/rockchip_timer: implement reading 64bit value
from timer
clocksource/drivers/rockchip_timer: implement clocksource timer
clocksource/drivers/rockchip_timer: Prevent ftrace recursion
.../bindings/timer/rockchip,rk-timer.txt | 12 +-
arch/arm/boot/dts/rk3188.dtsi | 17 ++
arch/arm/boot/dts/rk3228-evb.dts | 4 +
arch/arm/boot/dts/rk3229-evb.dts | 4 +
drivers/clocksource/rockchip_timer.c | 207 +++++++++++++++-----
5 files changed, 190 insertions(+), 54 deletions(-)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 13:11 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <c06f9906-6089-c145-3b36-c410d88c786d-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 15:11 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 29/11/2016 13:08, Pantelis Antoniou wrote:
>> There’s no difference; you can cpp -IFOO_VALUE=foo in.dts | dtc | cat >/config/foo/dtb in a nutshell.
>>
>> And have foo-property = FOO_VALUE; in in.dts.
> In a boot loader?
>
Ah, that why you don’t do that in the bootloader :)
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 13:11 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <C5CD81E3-A9FF-4C23-A7A5-7E2A4E80E193-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 29/11/2016 13:08, Pantelis Antoniou wrote:
> There’s no difference; you can cpp -IFOO_VALUE=foo in.dts | dtc | cat >/config/foo/dtb in a nutshell.
>
> And have foo-property = FOO_VALUE; in in.dts.
In a boot loader?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Overlays and boolean properties
From: Pantelis Antoniou @ 2016-11-29 13:10 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, devicetree-u79uwXL29TY76Z2rM5mHXA,
Devicetree Compiler
In-Reply-To: <a02487bb-6f79-fe4d-8180-86375b9413b9-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 15:06 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> Boolean properties are defined as being properties with no content, that
> are true if present and false if absent. They pose a problem for DT
> overlays, since the proposed (and widely used) overlay mechanism does
> not allow for properties (or nodes) to be deleted; overlays can only
> make a false property true, so boolean properties are effectively
> monostable - once true they become immutable.
>
> The standard DT syntax includes /delete-property/ and /delete-node/
> directives that do what you would expect from their names, but that
> facility is not available to overlays. There is no FDT node that
> represents the deletion - the directives are acted on immediately - so
> we would need some extra markup - say __delete_property__ and
> __delete_node__ - to hold the names of items to be deleted.
>
> Before I take this further, does anybody have any thoughts on the idea?
>
The original patchset did support removing properties (by prefixing them with -).
I can revive that if we have consensus about the format/method.
> Phil
>
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 13:08 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <dbcfc090-43e2-d6f8-6f35-2761bc4d3da1-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
> On Nov 29, 2016, at 15:05 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 29/11/2016 13:00, Pantelis Antoniou wrote:
>> An alias is the standard way to refer to nodes symbolically. They can be overwritten at
>> runtime without triggering an error.
> Can you give me a concrete example of how this would look?
Maybe later in the day, kinda busy right now.
>> Speaking of which, since these overlays are applied at runtime, why not build them with a script
>> and have a #define passed to the c preprocessor before compiling them?
> Because the parameters are applied at run time, not compile time.
>
There’s no difference; you can cpp -IFOO_VALUE=foo in.dts | dtc | cat >/config/foo/dtb in a nutshell.
And have foo-property = FOO_VALUE; in in.dts.
> Phil
>
Regards
— Pantelis
^ permalink raw reply
* Overlays and boolean properties
From: Phil Elwell @ 2016-11-29 13:06 UTC (permalink / raw)
To: David Gibson, Pantelis Antoniou,
devicetree-u79uwXL29TY76Z2rM5mHXA, Devicetree Compiler
Boolean properties are defined as being properties with no content, that
are true if present and false if absent. They pose a problem for DT
overlays, since the proposed (and widely used) overlay mechanism does
not allow for properties (or nodes) to be deleted; overlays can only
make a false property true, so boolean properties are effectively
monostable - once true they become immutable.
The standard DT syntax includes /delete-property/ and /delete-node/
directives that do what you would expect from their names, but that
facility is not available to overlays. There is no FDT node that
represents the deletion - the directives are acted on immediately - so
we would need some extra markup - say __delete_property__ and
__delete_node__ - to hold the names of items to be deleted.
Before I take this further, does anybody have any thoughts on the idea?
Phil
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 13:05 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <27651F03-6E8F-4C76-A0E4-0DFBEC40277C-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 29/11/2016 13:00, Pantelis Antoniou wrote:
> An alias is the standard way to refer to nodes symbolically. They can be overwritten at
> runtime without triggering an error.
Can you give me a concrete example of how this would look?
> Speaking of which, since these overlays are applied at runtime, why not build them with a script
> and have a #define passed to the c preprocessor before compiling them?
Because the parameters are applied at run time, not compile time.
Phil
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 13:00 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <a1ba4783-2a3b-eefd-9c41-2f33524472fe-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 14:57 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 29/11/2016 12:24, Pantelis Antoniou wrote:
>> First note is that this is exactly what the portable connector is supposed to
>> do; abstract away the SoC differences.
>>
>> Second note is that it’s not the overlay application that’s having problems, it’s
>> your parameter patching method.
>>
>> The 'spimaxfrequency = <&can0>,"spi-max-frequency:0”’ form could more easily be
>> done by targeting aliases instead of node labels.
>>
>> I.e. You can apply the overlay, set an alias to the node and instead of referencing
>> the label, reference the alias.
> But the patching is done before the overlays are applied, in isolation
> from the base tree, so that they can still be used with the kernel
> configfs overlay mechanism. How do aliases (which associated symbols
> with absolute paths) help?
>
An alias is the standard way to refer to nodes symbolically. They can be overwritten at
runtime without triggering an error.
>> Again, this is a stop-gap until the portable connector is done, but what I take out
>> of this is the need for a parameterization step in which an overlay is modified before
>> it is applied according to an external parameter.
> Yes, absolutely.
>
Speaking of which, since these overlays are applied at runtime, why not build them with a script
and have a #define passed to the c preprocessor before compiling them?
It doesn’t appear to be a problem doing it this way.
> Phil
> —
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 12:57 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <96BE1B80-0843-4981-AA2A-E89EA6A02600-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 29/11/2016 12:24, Pantelis Antoniou wrote:
> First note is that this is exactly what the portable connector is supposed to
> do; abstract away the SoC differences.
>
> Second note is that it’s not the overlay application that’s having problems, it’s
> your parameter patching method.
>
> The 'spimaxfrequency = <&can0>,"spi-max-frequency:0”’ form could more easily be
> done by targeting aliases instead of node labels.
>
> I.e. You can apply the overlay, set an alias to the node and instead of referencing
> the label, reference the alias.
But the patching is done before the overlays are applied, in isolation
from the base tree, so that they can still be used with the kernel
configfs overlay mechanism. How do aliases (which associated symbols
with absolute paths) help?
> Again, this is a stop-gap until the portable connector is done, but what I take out
> of this is the need for a parameterization step in which an overlay is modified before
> it is applied according to an external parameter.
Yes, absolutely.
Phil
^ permalink raw reply
* Re: [PATCH V5 00/10] PM / OPP: Multiple regulator support
From: Rafael J. Wysocki @ 2016-11-29 12:48 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Nishanth Menon, Stephen Boyd, Lists linaro-kernel,
Linux PM, Linux Kernel Mailing List, Vincent Guittot, Rob Herring,
Dave Gerlach, Mark Brown, devicetree@vger.kernel.org
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>
On Tue, Nov 29, 2016 at 7:36 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi,
>
> Some platforms (like TI) have complex DVFS configuration for CPU
> devices, where multiple regulators are required to be configured to
> change DVFS state of the device. This was explained well by Nishanth
> earlier [1].
>
> One of the major complaints around multiple regulators case was that the
> DT isn't responsible in any way to represent the order in which multiple
> supplies need to be programmed, before or after a frequency change. It
> was considered in this patch and such information is left for the
> platform specific OPP driver now, which can register its own
> opp_set_rate() callback with the OPP core and the OPP core will then
> call it during DVFS.
>
> The patches are tested on Exynos5250 (Dual A15). I have hacked around DT
> and code to pass values for multiple regulators and verified that they
> are all properly read by the kernel (using debugfs interface).
>
> Dave Gerlach has already tested [2] it on the real TI platforms and it
> works well for him.
>
> This is rebased over: linux-next branch in the PM tree.
>
> V4->V5:
> - Stephen boyd had some minor review comments and gave his Reviewed-by
> tag for the rest. Only 2 patches don't have his RBY tag.
> - Individual patches contain the version history from V4 to V5.
Cool.
I'd still like to see that everyone agrees with patch [6/10] in particular.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 12:24 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ba8e2ed3-9798-3074-1167-3f6851321a25-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 14:11 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> Pantelis,
>
> On 29/11/2016 10:55, Pantelis Antoniou wrote:
>> Manually adding symbols by targeting __symbols__ is just bad. There is absolutely
>> no guarantee that the symbol/fixup node(s) will still be there in following iterations
>> of the patches.
> Remember that this is now part of the Linux kernel - it isn't something
> you can just change at will.
Since I’m the one that put it there, I’m sure I have a little bit of leverage.
>> I am thinking of parsing them, recording the information in kernel structures and then
>> deleting them altogether.
>>
>>> How does your patch handle duplicate symbols?
>>>
>> It doesn’t. Having duplicate global symbols is bad.
>>
>> It appears you want scoping rules instead. Care to paste a concrete example?
>
> Concrete non-trivial examples are hard to come by. There are some simple
> cases where we've attached labels to __overlay__ nodes so that the
> contents can be patched by our overlay parameter mechanism - they could
> just be given unique names instead of just "frag0", "frag1" etc. I'm
> more concerned about parameterised macro-expanded overlays.
>
> Consider an overlay that defines a CAN controller on an SPI bus. We
> currently have two such overlays in the RPi tree, one for SPI 0.0 and
> one for SPI 0.1. Here's an extract from one of them:
>
> /* the interrupt pin of the can-controller */
> fragment@2 {
> target = <&gpio>;
> __overlay__ {
> can0_pins: can0_pins {
> brcm,pins = <25>;
> brcm,function = <0>; /* input */
> };
> };
> };
> ...
> fragment@4 {
> target = <&spi0>;
> __overlay__ {
> /* needed to avoid dtc warning */
> #address-cells = <1>;
> #size-cells = <0>;
> can0: mcp2515@0 {
> reg = <0>;
> compatible = "microchip,mcp2515";
> pinctrl-names = "default";
> pinctrl-0 = <&can0_pins>;
> spi-max-frequency = <10000000>;
> interrupt-parent = <&gpio>;
> interrupts = <25 0x2>;
> clocks = <&can0_osc>;
> };
> };
> };
>
> One day I'd like to merge these into a single parameterised version that
> could target any CS line on any SPI controller. This requires that any
> created node names are unique with the scope of the parent ("mcp2515@0",
> "can0_pins"), and that the name of the target label (spi0) is patched to
> select the correct SPI bus. Our existing, limited overlay parameter
> mechanism uses labels to identify properties to patch:
>
> spimaxfrequency = <&can0>,"spi-max-frequency:0";
>
> (I would have attached labels to the properties themselves, but that
> doesn't seem to work, contrary to the ePAPR spec.)
>
> If the labels that locate the node, property and label names to change
> also themselves have to be made unique then that adds an extra level of
> complexity.
>
> The parameter application is a pre-processing step before the overlay is
> merged, so there is nothing preventing me from filtering the symbols
> node before passing it on based on rules of my own choosing, but I
> wanted to make more people aware of this change.
>
First note is that this is exactly what the portable connector is supposed to
do; abstract away the SoC differences.
Second note is that it’s not the overlay application that’s having problems, it’s
your parameter patching method.
The 'spimaxfrequency = <&can0>,"spi-max-frequency:0”’ form could more easily be
done by targeting aliases instead of node labels.
I.e. You can apply the overlay, set an alias to the node and instead of referencing
the label, reference the alias.
Again, this is a stop-gap until the portable connector is done, but what I take out
of this is the need for a parameterization step in which an overlay is modified before
it is applied according to an external parameter.
> Phil
>
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 12:11 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1F9EDF06-98B1-4270-AA58-1A9D9A9F9803-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
Pantelis,
On 29/11/2016 10:55, Pantelis Antoniou wrote:
> Manually adding symbols by targeting __symbols__ is just bad. There is absolutely
> no guarantee that the symbol/fixup node(s) will still be there in following iterations
> of the patches.
Remember that this is now part of the Linux kernel - it isn't something
you can just change at will.
> I am thinking of parsing them, recording the information in kernel structures and then
> deleting them altogether.
>
>> How does your patch handle duplicate symbols?
>>
> It doesn’t. Having duplicate global symbols is bad.
>
> It appears you want scoping rules instead. Care to paste a concrete example?
Concrete non-trivial examples are hard to come by. There are some simple
cases where we've attached labels to __overlay__ nodes so that the
contents can be patched by our overlay parameter mechanism - they could
just be given unique names instead of just "frag0", "frag1" etc. I'm
more concerned about parameterised macro-expanded overlays.
Consider an overlay that defines a CAN controller on an SPI bus. We
currently have two such overlays in the RPi tree, one for SPI 0.0 and
one for SPI 0.1. Here's an extract from one of them:
/* the interrupt pin of the can-controller */
fragment@2 {
target = <&gpio>;
__overlay__ {
can0_pins: can0_pins {
brcm,pins = <25>;
brcm,function = <0>; /* input */
};
};
};
...
fragment@4 {
target = <&spi0>;
__overlay__ {
/* needed to avoid dtc warning */
#address-cells = <1>;
#size-cells = <0>;
can0: mcp2515@0 {
reg = <0>;
compatible = "microchip,mcp2515";
pinctrl-names = "default";
pinctrl-0 = <&can0_pins>;
spi-max-frequency = <10000000>;
interrupt-parent = <&gpio>;
interrupts = <25 0x2>;
clocks = <&can0_osc>;
};
};
};
One day I'd like to merge these into a single parameterised version that
could target any CS line on any SPI controller. This requires that any
created node names are unique with the scope of the parent ("mcp2515@0",
"can0_pins"), and that the name of the target label (spi0) is patched to
select the correct SPI bus. Our existing, limited overlay parameter
mechanism uses labels to identify properties to patch:
spimaxfrequency = <&can0>,"spi-max-frequency:0";
(I would have attached labels to the properties themselves, but that
doesn't seem to work, contrary to the ePAPR spec.)
If the labels that locate the node, property and label names to change
also themselves have to be made unique then that adds an extra level of
complexity.
The parameter application is a pre-processing step before the overlay is
merged, so there is nothing preventing me from filtering the symbols
node before passing it on based on rules of my own choosing, but I
wanted to make more people aware of this change.
Phil
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox