clocksource: em_sti: Emma Mobile STI driver
Subsystems (5)
Magnus Damm
maintainerPATCH 1/1 clocksource: em_sti: Emma Mobile STI driver
From: Magnus Damm <[email protected]>
This is V1 of the Emma Mobile STI timer driver.
The STI hardware is based on a single 48-bit 32kHz counter that together with two individual compare registers can generate interrupts. There are no timer operating modes selectable which means that the timer can not clear on match.
This driver is providing clocksource support for the 48-bit counter. Clockevents are also supported using the same timer in periodic or oneshot modes.
---
arch/arm/mach-shmobile/Kconfig | 6 drivers/clocksource/Makefile | 1 drivers/clocksource/em_sti.c | 450 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 457 insertions(+)
arch/arm/mach-shmobile/Kconfig +6 -0
@@ -168,6 +168,12 @@ config SH_TIMER_TMU help This enables build of the TMU timer driver. +config EM_TIMER_STI + bool "STI timer driver" + default y + help + This enables build of the STI timer driver. + endmenu config SH_CLK_CPG
drivers/clocksource/Makefile +1 -0
@@ -6,6 +6,7 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC) += obj-$(CONFIG_SH_TIMER_CMT) += sh_cmt.o obj-$(CONFIG_SH_TIMER_MTU2) += sh_mtu2.o obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o +obj-$(CONFIG_EM_TIMER_STI) += em_sti.o obj-$(CONFIG_CLKBLD_I8253) += i8253.o obj-$(CONFIG_CLKSRC_MMIO) += mmio.o obj-$(CONFIG_DW_APB_TIMER) += dw_apb_timer.o
drivers/clocksource/em_sti.c new file +450 -0
450 lines not on this page · raw message
Quoted 10 lines · Magnus Damm
From: Magnus Damm <[email protected]>
This is V1 of the Emma Mobile STI timer driver.
The STI hardware is based on a single 48-bit 32kHz counter that together with two individual compare registers can generate interrupts. There are no timer operating modes selectable which means that the timer can not clear on match.
Quoted 5 lines
This driver is providing clocksource support for the 48-bit counter. Clockevents are also supported using the same timer in periodic or oneshot modes.
Signed-off-by Magnus Damm <[email protected]>
drivers/clocksource/em_sti.c line 184
Quoted 13 lines · Magnus Damm
drivers/clocksource/em_sti.c new file +13
-0+/* private flags */ +#define FLAG_CLOCKEVENT (1 << 0) +#define FLAG_CLOCKSOURCE (1 << 1) + +static int em_sti_start (struct em_sti_priv *p,unsigned long flag) +{ +int ret =0 ; +unsigned long flags; + +spin_lock_irqsave (&p->lock, flags); + +if (!(p->flags & (FLAG_CLOCKEVENT |FLAG_CLOCKSOURCE ))) + ret =em_sti_enable (p);
That's confusing. You seem to enable both CLOCKEVENT and CLOCKSOURCE independent of "flag" value.
drivers/clocksource/em_sti.c line 187
Quoted 3 lines
drivers/clocksource/em_sti.c new file +3
-0+ +if (!ret) + p->flags |= flag;
And then just or "flag" ??????
Quoted 7 lines
drivers/clocksource/em_sti.c new file +7
-0+spin_unlock_irqrestore (&p->lock, flags); + +return ret; +} + +static void em_sti_stop (struct em_sti_priv *p,unsigned long flag) +{
drivers/clocksource/em_sti.c line 205
Quoted 10 lines
drivers/clocksource/em_sti.c new file +10
-0+unsigned long flags; +unsigned long f; + +spin_lock_irqsave (&p->lock, flags); + + f = p->flags & (FLAG_CLOCKEVENT |FLAG_CLOCKSOURCE ); + p->flags &= ~flag; + +if (f && !(p->flags & (FLAG_CLOCKEVENT |FLAG_CLOCKSOURCE ))) +em_sti_disable (p);
Huch? If the caller wants to disable the clockevent, you stop the clocksource as well because em_sti_disable() stops the clock.
/me is confused.
drivers/clocksource/em_sti.c line 277
Quoted 10 lines
drivers/clocksource/em_sti.c new file +10
-0+static void em_sti_clock_event_start (struct em_sti_priv *p) +{ +struct clock_event_device *ced = &p->ced; + +em_sti_start (p,FLAG_CLOCKEVENT ); + +/* TODO: calculate good shift from rate and counter bit width */ + + ced->shift =32 ; + ced->mult =div_sc (p->rate,NSEC_PER_SEC , ced->shift);
IIRC, we have a generic function to do that :)
Thanks,
tglx
Magnus Damm
maintainerHi Thomas,
Quoted 17 lines · Thomas Gleixner
drivers/clocksource/em_sti.c line 184
Quoted 13 lines · Magnus Damm
drivers/clocksource/em_sti.c new file +13
-0+/* private flags */ +#define FLAG_CLOCKEVENT (1 << 0) +#define FLAG_CLOCKSOURCE (1 << 1) + +static int em_sti_start (struct em_sti_priv *p,unsigned long flag) +{ + int ret = 0; + unsigned long flags; + + spin_lock_irqsave(&p->lock, flags); + + if (!(p->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) + ret = em_sti_enable(p);That's confusing. You seem to enable both CLOCKEVENT and CLOCKSOURCE independent of "flag" value.
Hm, I believe the idea is to check if it has been enabled already...
Quoted 5 lines
Quoted 3 lines
+
+ if (!ret)
+p->flags|= flag;And then just or "flag" ??????
... but it certainly is overly complicated. I'll rework it into something that is easier to digest. =)
Quoted 21 lines
Quoted 7 lines
+
spin_unlock_irqrestore(&p->lock, flags);drivers/clocksource/em_sti.c new file +6
-0+ + return ret; +} + +static void em_sti_stop (struct em_sti_priv *p,unsigned long flag) +{drivers/clocksource/em_sti.c line 205
Quoted 10 lines
drivers/clocksource/em_sti.c new file +10
-0+ unsigned long flags; + unsigned long f; + + spin_lock_irqsave(&p->lock, flags); + + f = p->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE); + p->flags &= ~flag; + + if (f && !(p->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) + em_sti_disable(p);Huch? If the caller wants to disable the clockevent, you stop the clocksource as well because
em_sti_disable()stops the clock.
Quoted 1 line
/me is confused.
I believe the idea is that if the timer is currently enabled (f) and if we are the last user thenthen stop.
A regular usage counter probably makes much more sense!
Quoted 4 lines
Quoted 4 lines
drivers/clocksource/em_sti.c new file +4
-0+static void em_sti_clock_event_start (struct em_sti_priv *p) +{ + struct clock_event_device *ced = &p->ced; +
Quoted 8 lines
drivers/clocksource/em_sti.c line 277
Quoted 6 lines
+
em_sti_start(p, FLAG_CLOCKEVENT);drivers/clocksource/em_sti.c new file +5
-0+ + /* TODO: calculate good shift from rate and counter bit width */ + + ced->shift = 32; + ced->mult = div_sc(p->rate, NSEC_PER_SEC, ced->shift);IIRC, we have a generic function to do that :)
Ok, thanks for pointing that out. Need to look it up!
Will update and post a V2. Thanks for your help!
Cheers,
/ magnus