[2/8] hw/timer/sh_timer: Switch to transaction-based ptimer API

Message ID 20191017132905.5604-3-peter.maydell@linaro.org
State Superseded
Headers show
Series
  • Convert misc-arch devices to new ptimer API
Related show

Commit Message

Peter Maydell Oct. 17, 2019, 1:28 p.m.
Switch the sh_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 <peter.maydell@linaro.org>

---
 hw/timer/sh_timer.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.20.1

Comments

Richard Henderson Oct. 17, 2019, 2:49 p.m. | #1
On 10/17/19 6:28 AM, Peter Maydell wrote:
> Switch the sh_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 <peter.maydell@linaro.org>

> ---

>  hw/timer/sh_timer.c | 13 +++++++++----

>  1 file changed, 9 insertions(+), 4 deletions(-)



Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



> @@ -168,12 +173,14 @@ static void sh_timer_start_stop(void *opaque, int enable)

>      printf("sh_timer_start_stop %d (%d)\n", enable, s->enabled);

>  #endif

>  

> +    ptimer_transaction_begin(s->timer);

>      if (s->enabled && !enable) {

>          ptimer_stop(s->timer);

>      }

>      if (!s->enabled && enable) {

>          ptimer_run(s->timer, 0);

>      }

> +    ptimer_transaction_commit(s->timer);

>      s->enabled = !!enable;


Ew.  Future cleanup should perhaps be

- static void sh_timer_start_stop(void *opaque, int enable)
+ static void sh_timer_start_stop(void *opaque, bool enable)

    if (s->enabled != enable) {
        s->enabled = enable;
        ptimer_transaction_begin(s->timer);
        if (enable) {
            ptimer_run(s->timer, 0);
        } else {
            ptimer_stop(s->timer);
        }
        ptimer_transaction_commit(s->timer);
    }


r~
Philippe Mathieu-Daudé Oct. 17, 2019, 4:01 p.m. | #2
On 10/17/19 3:28 PM, Peter Maydell wrote:
> Switch the sh_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 <peter.maydell@linaro.org>

> ---

>   hw/timer/sh_timer.c | 13 +++++++++----

>   1 file changed, 9 insertions(+), 4 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Patch

diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c
index 48a81b4dc79..13c4051808f 100644
--- a/hw/timer/sh_timer.c
+++ b/hw/timer/sh_timer.c
@@ -13,7 +13,6 @@ 
 #include "hw/irq.h"
 #include "hw/sh4/sh.h"
 #include "qemu/timer.h"
-#include "qemu/main-loop.h"
 #include "hw/ptimer.h"
 
 //#define DEBUG_TIMER
@@ -91,13 +90,18 @@  static void sh_timer_write(void *opaque, hwaddr offset,
     switch (offset >> 2) {
     case OFFSET_TCOR:
         s->tcor = value;
+        ptimer_transaction_begin(s->timer);
         ptimer_set_limit(s->timer, s->tcor, 0);
+        ptimer_transaction_commit(s->timer);
         break;
     case OFFSET_TCNT:
         s->tcnt = value;
+        ptimer_transaction_begin(s->timer);
         ptimer_set_count(s->timer, s->tcnt);
+        ptimer_transaction_commit(s->timer);
         break;
     case OFFSET_TCR:
+        ptimer_transaction_begin(s->timer);
         if (s->enabled) {
             /* Pause the timer if it is running.  This may cause some
                inaccuracy dure to rounding, but avoids a whole lot of other
@@ -148,6 +152,7 @@  static void sh_timer_write(void *opaque, hwaddr offset,
             /* Restart the timer if still enabled.  */
             ptimer_run(s->timer, 0);
         }
+        ptimer_transaction_commit(s->timer);
         break;
     case OFFSET_TCPR:
         if (s->feat & TIMER_FEAT_CAPT) {
@@ -168,12 +173,14 @@  static void sh_timer_start_stop(void *opaque, int enable)
     printf("sh_timer_start_stop %d (%d)\n", enable, s->enabled);
 #endif
 
+    ptimer_transaction_begin(s->timer);
     if (s->enabled && !enable) {
         ptimer_stop(s->timer);
     }
     if (!s->enabled && enable) {
         ptimer_run(s->timer, 0);
     }
+    ptimer_transaction_commit(s->timer);
     s->enabled = !!enable;
 
 #ifdef DEBUG_TIMER
@@ -191,7 +198,6 @@  static void sh_timer_tick(void *opaque)
 static void *sh_timer_init(uint32_t freq, int feat, qemu_irq irq)
 {
     sh_timer_state *s;
-    QEMUBH *bh;
 
     s = (sh_timer_state *)g_malloc0(sizeof(sh_timer_state));
     s->freq = freq;
@@ -203,8 +209,7 @@  static void *sh_timer_init(uint32_t freq, int feat, qemu_irq irq)
     s->enabled = 0;
     s->irq = irq;
 
-    bh = qemu_bh_new(sh_timer_tick, s);
-    s->timer = ptimer_init_with_bh(bh, PTIMER_POLICY_DEFAULT);
+    s->timer = ptimer_init(sh_timer_tick, s, PTIMER_POLICY_DEFAULT);
 
     sh_timer_write(s, OFFSET_TCOR >> 2, s->tcor);
     sh_timer_write(s, OFFSET_TCNT >> 2, s->tcnt);