On Thu, 17 Oct 2019 at 16:01, Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
>
> Hi Peter,
>
> On 10/17/19 3:21 PM, Peter Maydell wrote:
> > Switch the xilinx_axidma code away from bottom-half based ptimers to
> > the new transaction-based ptimer API. This just requires adding
> > begin/commit calls around the various places that modify the ptimer
> > state, and using the new ptimer_init() function to create the timer.
> >
> > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> > ---
> > hw/dma/xilinx_axidma.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> > index e035d1f7504..fb3a978e282 100644
> > --- a/hw/dma/xilinx_axidma.c
> > +++ b/hw/dma/xilinx_axidma.c
> > @@ -31,7 +31,6 @@
> > #include "hw/ptimer.h"
> > #include "hw/qdev-properties.h"
> > #include "qemu/log.h"
> > -#include "qemu/main-loop.h"
> > #include "qemu/module.h"
> >
> > #include "hw/stream.h"
> > @@ -104,7 +103,6 @@ enum {
> > };
> >
> > struct Stream {
> > - QEMUBH *bh;
> > ptimer_state *ptimer;
> > qemu_irq irq;
> >
> > @@ -242,6 +240,7 @@ static void stream_complete(struct Stream *s)
> > unsigned int comp_delay;
> >
> > /* Start the delayed timer. */
> > + ptimer_transaction_begin(s->ptimer);
> > comp_delay = s->regs[R_DMACR] >> 24;
> > if (comp_delay) {
> > ptimer_stop(s->ptimer);
> > @@ -255,6 +254,7 @@ static void stream_complete(struct Stream *s)
> > s->regs[R_DMASR] |= DMASR_IOC_IRQ;
> > stream_reload_complete_cnt(s);
> > }
> > + ptimer_transaction_commit(s->ptimer);
>
> I'd restrict the transaction here within the if() statement:
>
> -- >8 --
> @@ -244,9 +244,11 @@ static void stream_complete(struct Stream *s)
> /* Start the delayed timer. */
> comp_delay = s->regs[R_DMACR] >> 24;
> if (comp_delay) {
> + ptimer_transaction_begin(s->ptimer);
> ptimer_stop(s->ptimer);
> ptimer_set_count(s->ptimer, comp_delay);
> ptimer_run(s->ptimer, 1);
> + ptimer_transaction_commit(s->ptimer);
> }
>
> s->complete_cnt--;
The timer_hit callback function itself writes to
s->complete_cnt, so we don't want to allow it to
be called (via the commit()) before stream_complete()
is done with changing that state.
thanks
-- PMM
