Re: [PATCH 6/8] hw/watchdog/etraxfs_timer.c: Switch to transaction-based ptimer API

[Philippe Mathieu-Daudé]

On 10/17/19 3:29 PM, Peter Maydell wrote:
> Switch the etraxfs_timer 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 <address@hidden>
> ---
>   hw/timer/etraxfs_timer.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/hw/timer/etraxfs_timer.c b/hw/timer/etraxfs_timer.c
> index ab27fe1895b..afe3d30a8ea 100644
> --- a/hw/timer/etraxfs_timer.c
> +++ b/hw/timer/etraxfs_timer.c
> @@ -26,7 +26,6 @@
>   #include "hw/sysbus.h"
>   #include "sysemu/reset.h"
>   #include "sysemu/runstate.h"
> -#include "qemu/main-loop.h"
>   #include "qemu/module.h"
>   #include "qemu/timer.h"
>   #include "hw/irq.h"
> @@ -59,9 +58,6 @@ typedef struct ETRAXTimerState {
>       qemu_irq irq;
>       qemu_irq nmi;
>   
> -    QEMUBH *bh_t0;
> -    QEMUBH *bh_t1;
> -    QEMUBH *bh_wd;
>       ptimer_state *ptimer_t0;
>       ptimer_state *ptimer_t1;
>       ptimer_state *ptimer_wd;
> @@ -155,6 +151,7 @@ static void update_ctrl(ETRAXTimerState *t, int tnum)
>       }
>   
>       D(printf ("freq_hz=%d div=%d\n", freq_hz, div));
> +    ptimer_transaction_begin(timer);
>       ptimer_set_freq(timer, freq_hz);
>       ptimer_set_limit(timer, div, 0);
>   
> @@ -176,6 +173,7 @@ static void update_ctrl(ETRAXTimerState *t, int tnum)
>               abort();
>               break;
>       }
> +    ptimer_transaction_commit(timer);
>   }
>   
>   static void timer_update_irq(ETRAXTimerState *t)
> @@ -240,6 +238,7 @@ static inline void timer_watchdog_update(ETRAXTimerState 
> *t, uint32_t value)
>   
>       t->wd_hits = 0;
>   
> +    ptimer_transaction_begin(t->ptimer_wd);
>       ptimer_set_freq(t->ptimer_wd, 760);
>       if (wd_cnt == 0)
>           wd_cnt = 256;
> @@ -250,6 +249,7 @@ static inline void timer_watchdog_update(ETRAXTimerState 
> *t, uint32_t value)
>           ptimer_stop(t->ptimer_wd);
>   
>       t->rw_wd_ctrl = value;
> +    ptimer_transaction_commit(t->ptimer_wd);
>   }
>   
>   static void
> @@ -311,9 +311,15 @@ static void etraxfs_timer_reset(void *opaque)
>   {
>       ETRAXTimerState *t = opaque;
>   
> +    ptimer_transaction_begin(t->ptimer_t0);
>       ptimer_stop(t->ptimer_t0);
> +    ptimer_transaction_commit(t->ptimer_t0);
> +    ptimer_transaction_begin(t->ptimer_t1);
>       ptimer_stop(t->ptimer_t1);
> +    ptimer_transaction_commit(t->ptimer_t1);
> +    ptimer_transaction_begin(t->ptimer_wd);
>       ptimer_stop(t->ptimer_wd);
> +    ptimer_transaction_commit(t->ptimer_wd);
>       t->rw_wd_ctrl = 0;
>       t->r_intr = 0;
>       t->rw_intr_mask = 0;
> @@ -325,12 +331,9 @@ static void etraxfs_timer_realize(DeviceState *dev, Error 
> **errp)
>       ETRAXTimerState *t = ETRAX_TIMER(dev);
>       SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>   
> -    t->bh_t0 = qemu_bh_new(timer0_hit, t);
> -    t->bh_t1 = qemu_bh_new(timer1_hit, t);
> -    t->bh_wd = qemu_bh_new(watchdog_hit, t);
> -    t->ptimer_t0 = ptimer_init_with_bh(t->bh_t0, PTIMER_POLICY_DEFAULT);
> -    t->ptimer_t1 = ptimer_init_with_bh(t->bh_t1, PTIMER_POLICY_DEFAULT);
> -    t->ptimer_wd = ptimer_init_with_bh(t->bh_wd, PTIMER_POLICY_DEFAULT);
> +    t->ptimer_t0 = ptimer_init(timer0_hit, t, PTIMER_POLICY_DEFAULT);
> +    t->ptimer_t1 = ptimer_init(timer1_hit, t, PTIMER_POLICY_DEFAULT);
> +    t->ptimer_wd = ptimer_init(watchdog_hit, t, PTIMER_POLICY_DEFAULT);
>   
>       sysbus_init_irq(sbd, &t->irq);
>       sysbus_init_irq(sbd, &t->nmi);

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