| Message ID | 1568123236-767-8-git-send-email-claudiu.beznea@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add support for clocksource/clockevent DT selection | expand |
On Tue, Sep 10, 2019 at 2:50 PM Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > From: Alexandre Belloni <alexandre.belloni@bootlin.com> > > The driver currently uses aliases to know whether the timer is the > clocksource or the clockevent. OK maybe that wasn't the most elegant solution. > Add the /chosen/linux,clocksource and > /chosen/linux,clockevent parsing while keeping backward compatibility. This is not how I would solve this today. I would simply remove/comment out the IRQ from the timer that cannot be used for clockevent from the device tree (apparently it doesn't work anyway), and make the code only pick a timer with a valid interrupt assigned as clock event, while a timer without interrupt can be used for clock source. This has the upside of not needing any special aliases or chosen things. Yours, Linus Walleij
On 11.09.2019 02:48, Linus Walleij wrote: > External E-Mail > > > On Tue, Sep 10, 2019 at 2:50 PM Claudiu Beznea > <claudiu.beznea@microchip.com> wrote: >> From: Alexandre Belloni <alexandre.belloni@bootlin.com> >> >> The driver currently uses aliases to know whether the timer is the >> clocksource or the clockevent. > > OK maybe that wasn't the most elegant solution. > >> Add the /chosen/linux,clocksource and >> /chosen/linux,clockevent parsing while keeping backward compatibility. > > This is not how I would solve this today. > > I would simply remove/comment out the IRQ from the timer > that cannot be used for clockevent from the device tree > (apparently it doesn't work anyway), and make the code only > pick a timer with a valid interrupt assigned as clock event, > while a timer without interrupt can be used for clock source. This could also be used but it will not be compatible with old device trees. There are different ideas implemented in timer drivers with regards to this issue. Some of them are registering 1st timer to work as clocksource/clockevent and the 2nd one to work as clockevent/clocksource. Some are using different compatibles, one for clocksource, one for clockevent although these compatible seems to be for the same hardware type (I can point drivers/clocksource/timer-sprd.c). The idea with this series was, at it has been proposed in [1] to have one single mechanism for this kind of situations. Thank you, Claudiu Beznea [1] https://lore.kernel.org/lkml/2f831f1b-c87d-48bd-cf02-2ebb334b964c@linaro.org/ > > This has the upside of not needing any special aliases or > chosen things. > > Yours, > Linus Walleij >
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index a642c23b2fba..e1742c0abb03 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -240,6 +240,7 @@ config KEYSTONE_TIMER config INTEGRATOR_AP_TIMER bool "Integrator-ap timer driver" if COMPILE_TEST select CLKSRC_MMIO + select TIMER_OF help Enables support for the Integrator-ap timer. diff --git a/drivers/clocksource/timer-integrator-ap.c b/drivers/clocksource/timer-integrator-ap.c index 8d6f814ace36..78af89e73125 100644 --- a/drivers/clocksource/timer-integrator-ap.c +++ b/drivers/clocksource/timer-integrator-ap.c @@ -14,6 +14,7 @@ #include <linux/interrupt.h> #include <linux/sched_clock.h> +#include "timer-of.h" #include "timer-sp.h" static void __iomem * sched_clk_base; @@ -160,6 +161,12 @@ static int integrator_clockevent_init(unsigned long inrate, return 0; } +static struct timer_of to[] = { + { .flags = TIMER_OF_TYPE_CS, }, + { .flags = TIMER_OF_TYPE_CE, }, + { /* sentinel */ } +}; + static int __init integrator_ap_timer_init_of(struct device_node *node) { const char *path; @@ -169,6 +176,7 @@ static int __init integrator_ap_timer_init_of(struct device_node *node) struct clk *clk; unsigned long rate; struct device_node *alias_node; + struct timer_of *to = node->data; base = of_io_request_and_map(node, 0, "integrator-timer"); if (IS_ERR(base)) @@ -183,6 +191,17 @@ static int __init integrator_ap_timer_init_of(struct device_node *node) rate = clk_get_rate(clk); writel(0, base + TIMER_CTRL); + if (timer_of_is_clocksource(to)) + /* The primary timer lacks IRQ, use as clocksource */ + return integrator_clocksource_init(rate, base); + + if (timer_of_is_clockevent(to)) { + /* The secondary timer will drive the clock event */ + irq = irq_of_parse_and_map(node, 0); + return integrator_clockevent_init(rate, base, irq); + } + + /* DT ABI compatibility below */ err = of_property_read_string(of_aliases, "arm,timer-primary", &path); if (err) { @@ -227,4 +246,4 @@ static int __init integrator_ap_timer_init_of(struct device_node *node) } TIMER_OF_DECLARE(integrator_ap_timer, "arm,integrator-timer", - integrator_ap_timer_init_of, NULL); + integrator_ap_timer_init_of, to);