pnx8xxx: move to clocksource
This patch converts PNX8XXX system timer to clocksource.
arch/mips/philips/pnx8550/common/time.c | 109 +++++++++++++++++++++-----------
1 files changed, 72 insertions(+), 37 deletions(-)
arch/mips/philips/pnx8550/common/time.c +0 -0
@@ -22,7 +22,6 @@
#include <linux/kernel_stat.h>
#include <linux/spinlock.h>
#include <linux/interrupt.h>
-#include <linux/module.h>
#include <asm/bootinfo.h>
#include <asm/cpu.h>
@@ -41,11 +40,60 @@ static cycle_t hpt_read(void)
return read_c0_count2(); }
+static struct clocksource pnx_clocksource = {
+ .name = "pnx8xxx",
+ .rating = 200,
+ .read = hpt_read,
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
static void timer_ack(void)
{
write_c0_compare(cpj);
}+static irqreturn_t pnx8xxx_timer_interrupt(int irq, void *dev_id) +{
+ struct clock_event_device *c = dev_id;
+
+ /* clear MATCH, signal the event */
+ c->event_handler(c);
+
+ return IRQ_HANDLED;
+}
+
+static struct irqaction pnx8xxx_timer_irq = {
+ .handler = pnx8xxx_timer_interrupt,
+ .flags = IRQF_DISABLED | IRQF_PERCPU,
+ .name = "pnx8xxx_timer",
+};
+
+static irqreturn_t monotonic_interrupt(int irq, void *dev_id) +{
+ /* Timer 2 clear interrupt */
+ write_c0_compare2(-1);
+ return IRQ_HANDLED;
+}
+
+static struct irqaction monotonic_irqaction = {
+ .handler = monotonic_interrupt,
+ .flags = IRQF_DISABLED,
+ .name = "Monotonic timer",
+};
+
+static int pnx8xxx_set_next_event(unsigned long delta,
+ struct clock_event_device *evt)
+{
+ write_c0_compare(delta);
+ return 0;
+}
+
+static struct clock_event_device pnx8xxx_clockevent = {
+ .name = "pnx8xxx_clockevent",
+ .features = CLOCK_EVT_FEAT_ONESHOT,
+ .set_next_event = pnx8xxx_set_next_event,
+};
+
/*
* plat_time_init() - it does the following things:
*
@@ -58,11 +106,34 @@ static void timer_ack(void)
__init void plat_time_init(void)
{
+ unsigned int configPR; unsigned int n; unsigned int m; unsigned int p; unsigned int pow2p;
+ clockevents_register_device(&pnx8xxx_clockevent);
+ clocksource_register(&pnx_clocksource);
+
+ setup_irq(PNX8550_INT_TIMER1, &pnx8xxx_timer_irq);
+ setup_irq(PNX8550_INT_TIMER2, &monotonic_irqaction);
+
+ /* Timer 1 start */
+ configPR = read_c0_config7();
+ configPR &= ~0x00000008;
+ write_c0_config7(configPR);
+
+ /* Timer 2 start */
+ configPR = read_c0_config7();
+ configPR &= ~0x00000010;
+ write_c0_config7(configPR);
+
+ /* Timer 3 stop */
+ configPR = read_c0_config7();
+ configPR |= 0x00000020;
+ write_c0_config7(configPR);
+
+
/* PLL0 sets MIPS clock (PLL1 <=> TM1, PLL6 <=> TM2, PLL5 <=> mem) */
/* (but only if CLK_MIPS_CTL select value [bits 3:1] is 1: FIXME) */@@ -87,42 +158,6 @@ __init void plat_time_init(void)
write_c0_count2(0); write_c0_compare2(0xffffffff);
- clocksource_mips.read = hpt_read;
- mips_timer_ack = timer_ack;
-}
-
-static irqreturn_t monotonic_interrupt(int irq, void *dev_id) -{
- /* Timer 2 clear interrupt */
- write_c0_compare2(-1);
- return IRQ_HANDLED;
}
-static struct irqaction monotonic_irqaction = {
- .handler = monotonic_interrupt,
- .flags = IRQF_DISABLED,
- .name = "Monotonic timer",
-};
-void __init plat_timer_setup(struct irqaction *irq) -{
- int configPR;
-
- setup_irq(PNX8550_INT_TIMER1, irq);
- setup_irq(PNX8550_INT_TIMER2, &monotonic_irqaction);
-
- /* Timer 1 start */
- configPR = read_c0_config7();
- configPR &= ~0x00000008;
- write_c0_config7(configPR);
-
- /* Timer 2 start */
- configPR = read_c0_config7();
- configPR &= ~0x00000010;
- write_c0_config7(configPR);
-
- /* Timer 3 stop */
- configPR = read_c0_config7();
- configPR |= 0x00000020;
- write_c0_config7(configPR);
-}
Quoted 1 line · Vitaly Wool
This patch converts PNX8XXX system timer to clocksource.
Checkpatch had a little to moan on your patch:
$ scripts/checkpatch.pl /tmp/pend ERROR: use tabs not spaces #79: FILE: arch/mips/philips/pnx8550/common/time.c:57: + struct clock_event_device *c = dev_id;$
ERROR: use tabs not spaces
#81: FILE: arch/mips/philips/pnx8550/common/time.c:59:
+ /* clear MATCH, signal the event */$
ERROR: use tabs not spaces
#82: FILE: arch/mips/philips/pnx8550/common/time.c:60:
+ c->event_handler(c);$
Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.
I fixed those, applied. Thanks!
Ralf
Hello.
Quoted 1 line · Vitaly Wool
This patch converts PNX8XXX system timer to clocksource.
Well, this patch has been already committed but nevertheless...
Quoted 19 lines
arch/mips/philips/pnx8550/common/time.c| 109
+++++++++++++++++++++-----------1 files changed, 72 insertions(+), 37 deletions(-)Signed-off-by Vitaly Wool <[email protected]>Index:
linux-2.6/arch/mips/philips/pnx8550/common/time.c
===================================================================arch/mips/philips/pnx8550/common/time.c +0
-1@@ -22,7 +22,6 @@#include <linux/kernel_stat.h> #include <linux/spinlock.h> #include <linux/interrupt.h> -#include <linux/module.h> #include <asm/bootinfo.h> #include <asm/cpu.h>
arch/mips/philips/pnx8550/common/time.c static cycle_t hpt_read(void) · line 48
Quoted 9 lines
arch/mips/philips/pnx8550/common/time.c +6
-0@@ -41,11 +40,60 @@ static cycle_t hpt_read(void)return read_c0_count2 (); } +static struct clocksource pnx_clocksource = { + .name ="pnx8xxx" , + .rating =200 , + .read = hpt_read, + .flags =CLOCK_SOURCE_IS_CONTINUOUS , +};
Something probably have converted tabs to 8 spaces...
I would have done it otherwise -- using timer 1 as a generic MIPS
clocksource (just hooking the IRQ to reload the comparator to all ones), and timer 2 as clockevent...
arch/mips/philips/pnx8550/common/time.c static cycle_t hpt_read(void) · line 52
Quoted 4 lines
arch/mips/philips/pnx8550/common/time.c +0
-0static void timer_ack (void ) {write_c0_compare (cpj); }
Do we still need this function? I don't think so -- mips_timer_ack() is
dead...
[...]
arch/mips/philips/pnx8550/common/time.c static cycle_t hpt_read(void) · line 55
Quoted 3 lines
arch/mips/philips/pnx8550/common/time.c +3
-0+static struct clock_event_device pnx8xxx_clockevent = { + .name ="pnx8xxx_clockevent" , + .features =CLOCK_EVT_FEAT_ONESHOT ,
Aren't PNX8550 timers actually periodic in nature?
arch/mips/philips/pnx8550/common/time.c static void timer_ack(void) · line 109
Quoted 11 lines
arch/mips/philips/pnx8550/common/time.c +4
-0+ .set_next_event = pnx8xxx_set_next_event, +}; +/* * plat_time_init() - it does the following things: * @@ -58,11 +106,34 @@ static void timer_ack(void)__init void plat_time_init (void ) { +unsigned int configPR;
Something has definitely spoilt all the tabs in the patch...
Quoted 15 lines
arch/mips/philips/pnx8550/common/time.c +10
-0unsigned int n;unsigned int m;unsigned int p;unsigned int pow2p; +clockevents_register_device (&pnx8xxx_clockevent); +clocksource_register (&pnx_clocksource); + +setup_irq (PNX8550_INT_TIMER1 , &pnx8xxx_timer_irq); +setup_irq (PNX8550_INT_TIMER2 , &monotonic_irqaction); + +/* Timer 1 start */ + configPR =read_c0_config7 (); + configPR &= ~0x00000008 ; +write_c0_config7 (configPR);
arch/mips/philips/pnx8550/common/time.c static void timer_ack(void) · line 134
Quoted 10 lines
arch/mips/philips/pnx8550/common/time.c +10
-0+ +/* Timer 2 start */ + configPR =read_c0_config7 (); + configPR &= ~0x00000010 ; +write_c0_config7 (configPR); + +/* Timer 3 stop */ + configPR =read_c0_config7 (); + configPR |=0x00000020 ; +write_c0_config7 (configPR);
Enabling timers before they are actually set up? :-|
Quoted 7 lines
arch/mips/philips/pnx8550/common/time.c +2
-0+ +/* PLL0 sets MIPS clock (PLL1 <=> TM1, PLL6 <=> TM2, PLL5 <=> mem) */
/* (but only if CLK_MIPS_CTL select value [bits 3:1] is 1:FIXME) */
WBR, Sergei
Quoted 16 lines · Sergei Shtylyov
Quoted 15 lines
Index:
linux-2.6/arch/mips/philips/pnx8550/common/time.c
===================================================================arch/mips/philips/pnx8550/common/time.c +0
-1@@ -22,7 +22,6 @@#include <linux/kernel_stat.h> #include <linux/spinlock.h> #include <linux/interrupt.h> -#include <linux/module.h> #include <asm/bootinfo.h> #include <asm/cpu.h> @@ -41,11 +40,60 @@ static cycle_t hpt_read(void)return read_c0_count2 (); }
Quoted 8 lines
arch/mips/philips/pnx8550/common/time.c static cycle_t hpt_read(void) · line 47
Quoted 6 lines
arch/mips/philips/pnx8550/common/time.c +6
-0+static struct clocksource pnx_clocksource = { + .name ="pnx8xxx" , + .rating =200 , + .read = hpt_read, + .flags =CLOCK_SOURCE_IS_CONTINUOUS , +};Something probably have converted tabs to 8 spaces...
Only 3 of them?
Quoted 11 lines
I would have done it otherwise -- using timer 1 as a generic MIPS
clocksource (just hooking the IRQ to reload the comparator to all ones), and timer 2 as clockevent...arch/mips/philips/pnx8550/common/time.c static cycle_t hpt_read(void) · line 51
Quoted 4 lines
arch/mips/philips/pnx8550/common/time.c +0
-0static void timer_ack (void ) {write_c0_compare (cpj); }Do we still need this function? I don't think so --
mips_timer_ack()is
dead...
It's only used on initialization.
Quoted 7 lines
[...]
arch/mips/philips/pnx8550/common/time.c
Quoted 3 lines
arch/mips/philips/pnx8550/common/time.c +3
-0+static struct clock_event_device pnx8xxx_clockevent = { + .name ="pnx8xxx_clockevent" , + .features =CLOCK_EVT_FEAT_ONESHOT ,Aren't PNX8550 timers actually periodic in nature?
All I recall is they were odd ;-)
The hardware nature of timers and how to declare them to the Linux timer code is not always the same. CLOCK_EVT_FEAT_ONESHOT be used if the time to the next shot can be programmed.
Quoted 6 lines
Quoted 4 lines
__initvoidplat_time_init(void)
{
+ unsigned int configPR;Something has definitely spoilt all the tabs in the patch...
I fixed the three checkpatch.pl was bitching about.
Quoted 19 lines
Quoted 19 lines
unsigned int n; unsigned int m; unsigned int p; unsigned int pow2p;arch/mips/philips/pnx8550/common/time.c +14
-0+clockevents_register_device (&pnx8xxx_clockevent); +clocksource_register (&pnx_clocksource); + +setup_irq (PNX8550_INT_TIMER1 , &pnx8xxx_timer_irq); +setup_irq (PNX8550_INT_TIMER2 , &monotonic_irqaction); + +/* Timer 1 start */ + configPR =read_c0_config7 (); + configPR &= ~0x00000008 ; +write_c0_config7 (configPR); + +/* Timer 2 start */ + configPR =read_c0_config7 (); + configPR &= ~0x00000010 ;
Quoted 8 lines
arch/mips/philips/pnx8550/common/time.c
Quoted 6 lines
arch/mips/philips/pnx8550/common/time.c +6
-0+write_c0_config7 (configPR); + +/* Timer 3 stop */ + configPR =read_c0_config7 (); + configPR |=0x00000020 ; +write_c0_config7 (configPR);Enabling timers before they are actually set up? :-|
Are the additional timers used at all?
Ralf
Quoted 1 line
Quoted 1 line
Something probably have converted tabs to 8 spaces...
No, it's just the lines of original code moved.
Quoted 6 lines
Quoted 6 lines
[...]
Quoted 3 lines
(quoted diff) +3
-0+static struct clock_event_device pnx8xxx_clockevent = { + .name = "pnx8xxx_clockevent", + .features = CLOCK_EVT_FEAT_ONESHOT,
Quoted 7 lines
Quoted 1 line
Aren't PNX8550 timers actually periodic in nature?
All I recall is they were odd ;-)
The hardware nature of timers and how to declare them to the Linux timer code is not always the same.
CLOCK_EVT_FEAT_ONESHOTbe used if the time to the next shot can be programmed.
Absolutely :)
Quoted 3 lines
Quoted 1 line
Enabling timers before they are actually set up? :-|
Are the additional timers used at all?
Heh, that also was the original code which was just moved from one place in the file to the other.
Sergei, I decided not to mix whitespace cleanups and the clocksource changes. I'll come up with the whitespace/tab fixups soon arranging it as a separate patch.
Vitaly
Quoted 3 lines · Vitaly Wool
Sergei, I decided not to mix whitespace cleanups and the clocksource changes. I'll come up with the whitespace/tab fixups soon arranging it as a separate patch.
Generally that's a good approach - but for lines which are modified anyway I see no reason not to clean the whitespace stuff anyway.
Ralf
Quoted 27 lines · Ralf Baechle
Quoted 16 lines · Sergei Shtylyov
Quoted 15 lines
Index:
linux-2.6/arch/mips/philips/pnx8550/common/time.c
===================================================================arch/mips/philips/pnx8550/common/time.c +0
-1@@ -22,7 +22,6 @@#include <linux/kernel_stat.h> #include <linux/spinlock.h> #include <linux/interrupt.h> -#include <linux/module.h> #include <asm/bootinfo.h> #include <asm/cpu.h> @@ -41,11 +40,60 @@ static cycle_t hpt_read(void)return read_c0_count2 (); }Quoted 8 lines
arch/mips/philips/pnx8550/common/time.c static cycle_t hpt_read(void) · line 47
Quoted 6 lines
arch/mips/philips/pnx8550/common/time.c +6
-0+static struct clocksource pnx_clocksource = { + .name ="pnx8xxx" , + .rating =200 , + .read = hpt_read, + .flags =CLOCK_SOURCE_IS_CONTINUOUS , +};Something probably have converted tabs to 8 spaces...
Quoted 1 line
Only 3 of them?
Erm, it's format=flowed that spoils the tabs for Mozilla which renders
them to 4 spaces, so it's actually hard to see which tabs are actually tabs and which are not... :-/
Quoted 8 lines
Quoted 7 lines
I would have done it otherwise -- using timer 1 as a generic MIPS
clocksource (just hooking the IRQ to reload the comparator to all ones), and timer 2 as clockevent...Quoted 4 lines
arch/mips/philips/pnx8550/common/time.c +0
-0static void timer_ack (void ) {write_c0_compare (cpj); }
Quoted 4 lines
Quoted 2 lines
Do we still need this function? I don't think so --
mips_timer_ack()is
dead...It's only used on initialization.
Could have put the call inline, or loaded the comparator with all ones
just like timer 2 -- we don't need 'cpj' variable any more as well...
Quoted 6 lines
Quoted 5 lines
[...]
Quoted 3 lines
arch/mips/philips/pnx8550/common/time.c +3
-0+static struct clock_event_device pnx8xxx_clockevent = { + .name ="pnx8xxx_clockevent" , + .features =CLOCK_EVT_FEAT_ONESHOT ,
Quoted 7 lines
Quoted 1 line
Aren't PNX8550 timers actually periodic in nature?
All I recall is they were odd ;-)
The hardware nature of timers and how to declare them to the Linux timer code is not always the same.
CLOCK_EVT_FEAT_ONESHOTbe used if the time to the next shot can be programmed.
I meant that both modes should have been indicated by the flags.
And actually, shouldn't we disable the timer after expiry if in one-shot mode.
Writing to the comparator doesn't clear the counter, AFAICS -- so, isn't the explicit counter clearing to 0 needed in set_next_event() method?
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
__initvoidplat_time_init(void)
{
+ unsigned int configPR;Something has definitely spoilt all the tabs in the patch...
I fixed the three checkpatch.pl was bitching about.
All the others were due to the way format=flowed is rendered by Mozilla it
seems...
Quoted 28 lines
Quoted 19 lines
Quoted 19 lines
unsigned int n; unsigned int m; unsigned int p; unsigned int pow2p;arch/mips/philips/pnx8550/common/time.c +14
-0+clockevents_register_device (&pnx8xxx_clockevent); +clocksource_register (&pnx_clocksource); + +setup_irq (PNX8550_INT_TIMER1 , &pnx8xxx_timer_irq); +setup_irq (PNX8550_INT_TIMER2 , &monotonic_irqaction); + +/* Timer 1 start */ + configPR =read_c0_config7 (); + configPR &= ~0x00000008 ; +write_c0_config7 (configPR); + +/* Timer 2 start */ + configPR =read_c0_config7 (); + configPR &= ~0x00000010 ;Quoted 8 lines
arch/mips/philips/pnx8550/common/time.c
Quoted 6 lines
arch/mips/philips/pnx8550/common/time.c +6
-0+write_c0_config7 (configPR); + +/* Timer 3 stop */ + configPR =read_c0_config7 (); + configPR |=0x00000020 ; +write_c0_config7 (configPR);Enabling timers before they are actually set up? :-|
Quoted 1 line
Are the additional timers used at all?
Additional == timer 3? It's not used, only 1 and 2 are -- and their
count/compare registers are initialized further in this function...
Quoted 1 line
Ralf
WBR, Sergei