Hi Stephen,
On Wed, Sep 25, 2013 at 04:16:14PM -0700, Stephen Boyd wrote:
> On 09/25/13 07:03, Maxime Ripard wrote:
> > diff --git
> > a/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
> > b/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
> > new file mode 100644
> > index 0000000..b1f81e9
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
> > @@ -0,0 +1,22 @@
> > +Allwinner SoCs High Speed Timer Controller
> > +
> > +Required properties:
> > +
> > +- compatible : should be "allwinner,sun5i-a13-hstimer" or
> > + "allwinner,sun7i-a20-hstimer"
> > +- reg : Specifies base physical address and size of the registers.
> > +- interrupts : The interrupts of these timers (2 for the sun5i IP, 4
> > for the sun7i
> > + one)
> > +- clocks: phandle to the source clock (usually the AHB clock)
> > +
> > +Example:
> > +
> > +hstimer@01c60000 {
>
> This should just be 'timer@1c60000'
Ok.
> > + compatible = "allwinner,sun7i-a20-hstimer";
> > + reg = <0x01c60000 0x1000>;
> > + interrupts = <0 51 1>,
> > + <0 52 1>,
> > + <0 53 1>,
> > + <0 54 1>;
> > + clocks = <&ahb1_gates 19>;
> > +};
>
> Weird mix of tabs and spaces here.
Right.
> > +
> > +static void __iomem *timer_base;
> > +static u32 ticks_per_jiffy;
> > +
> > +/*
> > + * When we disable a timer, we need to wait at least for 2 cycles of
> > + * the timer source clock. We will use for that the clocksource timer
> > + * that is already setup and runs at the same frequency than the other
> > + * timers, and we never will be disabled.
> > + */
> > +static void sun5i_clkevt_sync(void)
> > +{
> > + u32 old = readl(timer_base + TIMER_CNTVAL_LO_REG(1));
> > +
> > + while ((old - readl(timer_base + TIMER_CNTVAL_LO_REG(1))) < 3)
> > + cpu_relax();
> > +}
> > +
> [...]
> > +
> > +static int sun5i_clkevt_next_event(unsigned long evt,
> > + struct clock_event_device *unused)
> > +{
> > + sun5i_clkevt_time_stop(0);
> > + sun5i_clkevt_time_setup(0, evt);
> > + sun5i_clkevt_time_start(0, false);
>
> I suppose the min delta wants to be 3 instead of 1 because if we program
> an evt one tick in the future first we'll wait for two ticks (or is that
> three?) while we stop the timer and then program the timer to fire one
> tick after that. Perhaps we should subtract two ticks from the evt as
> well when we program it here?
Hmmm, indeed.
> > +
> > + return 0;
> > +}
> > +
> > +static struct clock_event_device sun5i_clockevent = {
> > + .name = "sun5i_tick",
> > + .rating = 350,
> > + .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> > + .set_mode = sun5i_clkevt_mode,
> > + .set_next_event = sun5i_clkevt_next_event,
> > +};
> > +
> > +
> > +static irqreturn_t sun5i_timer_interrupt(int irq, void *dev_id)
> > +{
> > + struct clock_event_device *evt = (struct clock_event_device *)dev_id;
> > +
> > + writel(0x1, timer_base + TIMER_IRQ_ST_REG);
> > + evt->event_handler(evt);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static struct irqaction sun5i_timer_irq = {
> > + .name = "sun5i_timer0",
> > + .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
>
> IRQF_DISABLED is a no-op and can be dropped.
Right.
> > +
> > +static u32 sun5i_timer_sched_read(void)
> > +{
> > + return ~readl(timer_base + TIMER_CNTVAL_LO_REG(1));
> > +}
> > +
> > +static void __init sun5i_timer_init(struct device_node *node)
> > +{
> [...]
> > +
> > + sun5i_clockevent.cpumask = cpumask_of(0);
>
> Can this timer interrupt any CPU or is it hardwired to CPU0? If the
> interrupt can go to any CPU this should be cpu_possible_mask instead.
This is an interrupt that can interrupt any CPU available on the system,
so yes, cpu_possible_mask would make sense.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
