Re: [PATCH 2/3] hw/timer/xilinx_timer.c: Switch to transaction-based ptimer API

[Philippe Mathieu-Daudé]

On 10/17/19 5:03 PM, Peter Maydell wrote:
> On Thu, 17 Oct 2019 at 15:57, Philippe Mathieu-Daudé <address@hidden> wrote:
> > Hi Peter,
> >
> > On 10/17/19 3:21 PM, Peter Maydell wrote:
> > > +/* Must be called inside ptimer transaction block */
> > >    static void timer_enable(struct xlx_timer *xt)
> > >    {
> > >        uint64_t count;
> > > @@ -174,8 +173,11 @@ timer_write(void *opaque, hwaddr addr,
> > >                    value &= ~TCSR_TINT;
> > >
> > >                xt->regs[addr] = value & 0x7ff;
> > > -            if (value & TCSR_ENT)
> > > +            if (value & TCSR_ENT) {
> > > +                ptimer_transaction_begin(xt->ptimer);
> > >                    timer_enable(xt);
> > > +                ptimer_transaction_commit(xt->ptimer);
> >
> > Why not move these inside timer_enable()?
>
> Because timer_enable() is called from the callback
> function timer_hit(). Since callback functions are called
> from within a begin/commit block, if we called begin
> again in timer_enable() it would assert().

Yes, now I understood the point of this new API ;)

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>