On 5 March 2016 at 11:29, Andrew Jeffery <and...@aj.id.au> wrote: > Implement basic AST2400 timer functionality: Up to 8 timers can > independently be configured, enabled, reset and disabled. A couple of > hardware features are not implemented, namely clock value matching and > pulse generation, but the implementation is enough to boot the Linux > kernel configured with aspeed_defconfig. > > Signed-off-by: Andrew Jeffery <and...@aj.id.au> > --- > +/** > + * Avoid mutual references between AspeedTimerCtrlState and AspeedTimer > + * structs, as it's a waste of memory and it makes implementing > + * VMStateDescription a little clunky.
Not sure what you have in mind with the reference to VMStateDescription here. The vmstate struct only has to list the fields which contain actual volatile state -- things like backreference pointers to other structs aren't volatile state so don't appear. > The ptimer BH callback needs to know > + * whether a specific AspeedTimer is enabled, but this information is held in > + * AspeedTimerCtrlState. So, provide a helper to hoist ourselves from an > + * arbitrary AspeedTimer to AspeedTimerCtrlState. > + */ > +static inline struct AspeedTimerCtrlState *timer_to_ctrl(AspeedTimer *t) > +{ > + AspeedTimer (*timers)[] = (void *)t - (t->id * sizeof(*t)); > + return container_of(timers, AspeedTimerCtrlState, timers); > +} > +static void aspeed_timer_expire(void *opaque) > +{ > + AspeedTimer *t = opaque; > + > + /* Only support interrupts on match values of zero for the moment - this > is > + * sufficient to boot an aspeed_defconfig Linux kernel. Non-zero match > + * values need some further consideration given the current ptimer API. > + * Maybe run multiple ptimers? > + */ See hw/timer/a9gtimer.c for an example of a timer with a comparator that can fire when the timer hits an arbitrary comparator value (it doesn't use ptimers but the principle is the same -- you set the timer to fire at the next interesting event, and then in the timer-fired handler you reset the timer to fire whenever the next event after that is, if any.) In any case this is probably ok for now. > + bool match = !(t->match[0] && t->match[1]); > + bool interrupt = timer_overflow_interrupt(t) || match; > + if (timer_enabled(t) && interrupt) { > + t->level = !t->level; > + qemu_set_irq(t->irq, t->level); > + } > +} > + Otherwise Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> thanks -- PMM