[1/2] hw/timer/grlib_gptimer.c: Switch to transaction-based ptimer API

Message ID 20191017132351.4762-2-peter.maydell@linaro.org
State Superseded
Headers show
Series
  • Convert sparc devices to new ptimer API
Related show

Commit Message

Peter Maydell Oct. 17, 2019, 1:23 p.m.
Switch the grlib_gptimer 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/grlib_gptimer.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

-- 
2.20.1

Comments

Richard Henderson Oct. 17, 2019, 2:19 p.m. | #1
On 10/17/19 6:23 AM, Peter Maydell wrote:
> Switch the grlib_gptimer 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/grlib_gptimer.c | 28 ++++++++++++++++++++++++----

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


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



r~
Philippe Mathieu-Daudé Oct. 17, 2019, 2:46 p.m. | #2
On 10/17/19 3:23 PM, Peter Maydell wrote:
> Switch the grlib_gptimer 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/grlib_gptimer.c | 28 ++++++++++++++++++++++++----

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

> 

> diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c

> index bb09268ea14..7a9371c0e30 100644

> --- a/hw/timer/grlib_gptimer.c

> +++ b/hw/timer/grlib_gptimer.c

> @@ -29,7 +29,6 @@

>   #include "hw/irq.h"

>   #include "hw/ptimer.h"

>   #include "hw/qdev-properties.h"

> -#include "qemu/main-loop.h"

>   #include "qemu/module.h"

>   

>   #include "trace.h"

> @@ -63,7 +62,6 @@ typedef struct GPTimer     GPTimer;

>   typedef struct GPTimerUnit GPTimerUnit;

>   

>   struct GPTimer {

> -    QEMUBH *bh;

>       struct ptimer_state *ptimer;

>   

>       qemu_irq     irq;

> @@ -93,6 +91,17 @@ struct GPTimerUnit {

>       uint32_t config;

>   };

>   

> +static void grlib_gptimer_tx_begin(GPTimer *timer)

> +{

> +    ptimer_transaction_begin(timer->ptimer);

> +}

> +

> +static void grlib_gptimer_tx_commit(GPTimer *timer)

> +{

> +    ptimer_transaction_commit(timer->ptimer);

> +}

> +

> +/* Must be called within grlib_gptimer_tx_begin/commit block */

>   static void grlib_gptimer_enable(GPTimer *timer)

>   {

>       assert(timer != NULL);

> @@ -115,6 +124,7 @@ static void grlib_gptimer_enable(GPTimer *timer)

>       ptimer_run(timer->ptimer, 1);

>   }

>   

> +/* Must be called within grlib_gptimer_tx_begin/commit block */

>   static void grlib_gptimer_restart(GPTimer *timer)

>   {

>       assert(timer != NULL);

> @@ -141,7 +151,9 @@ static void grlib_gptimer_set_scaler(GPTimerUnit *unit, uint32_t scaler)

>       trace_grlib_gptimer_set_scaler(scaler, value);

>   

>       for (i = 0; i < unit->nr_timers; i++) {

> +        ptimer_transaction_begin(unit->timers[i].ptimer);

>           ptimer_set_freq(unit->timers[i].ptimer, value);

> +        ptimer_transaction_commit(unit->timers[i].ptimer);

>       }

>   }

>   

> @@ -266,8 +278,10 @@ static void grlib_gptimer_write(void *opaque, hwaddr addr,

>           switch (timer_addr) {

>           case COUNTER_OFFSET:

>               trace_grlib_gptimer_writel(id, addr, value);

> +            grlib_gptimer_tx_begin(&unit->timers[id]);

>               unit->timers[id].counter = value;

>               grlib_gptimer_enable(&unit->timers[id]);

> +            grlib_gptimer_tx_commit(&unit->timers[id]);

>               return;

>   

>           case COUNTER_RELOAD_OFFSET:

> @@ -291,6 +305,7 @@ static void grlib_gptimer_write(void *opaque, hwaddr addr,

>               /* gptimer_restart calls gptimer_enable, so if "enable" and "load"

>                  bits are present, we just have to call restart. */

>   

> +            grlib_gptimer_tx_begin(&unit->timers[id]);

>               if (value & GPTIMER_LOAD) {

>                   grlib_gptimer_restart(&unit->timers[id]);

>               } else if (value & GPTIMER_ENABLE) {

> @@ -301,6 +316,7 @@ static void grlib_gptimer_write(void *opaque, hwaddr addr,

>               value &= ~(GPTIMER_LOAD & GPTIMER_DEBUG_HALT);

>   

>               unit->timers[id].config = value;

> +            grlib_gptimer_tx_commit(&unit->timers[id]);

>               return;

>   

>           default:

> @@ -344,9 +360,11 @@ static void grlib_gptimer_reset(DeviceState *d)

>           timer->counter = 0;

>           timer->reload = 0;

>           timer->config = 0;

> +        ptimer_transaction_begin(timer->ptimer);

>           ptimer_stop(timer->ptimer);

>           ptimer_set_count(timer->ptimer, 0);

>           ptimer_set_freq(timer->ptimer, unit->freq_hz);

> +        ptimer_transaction_commit(timer->ptimer);

>       }

>   }

>   

> @@ -365,14 +383,16 @@ static void grlib_gptimer_realize(DeviceState *dev, Error **errp)

>           GPTimer *timer = &unit->timers[i];

>   

>           timer->unit   = unit;

> -        timer->bh     = qemu_bh_new(grlib_gptimer_hit, timer);

> -        timer->ptimer = ptimer_init_with_bh(timer->bh, PTIMER_POLICY_DEFAULT);

> +        timer->ptimer = ptimer_init(grlib_gptimer_hit, timer,

> +                                    PTIMER_POLICY_DEFAULT);

>           timer->id     = i;

>   

>           /* One IRQ line for each timer */

>           sysbus_init_irq(sbd, &timer->irq);

>   

> +        ptimer_transaction_begin(timer->ptimer);

>           ptimer_set_freq(timer->ptimer, unit->freq_hz);

> +        ptimer_transaction_commit(timer->ptimer);

>       }

>   

>       memory_region_init_io(&unit->iomem, OBJECT(unit), &grlib_gptimer_ops,

> 


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

Patch

diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c
index bb09268ea14..7a9371c0e30 100644
--- a/hw/timer/grlib_gptimer.c
+++ b/hw/timer/grlib_gptimer.c
@@ -29,7 +29,6 @@ 
 #include "hw/irq.h"
 #include "hw/ptimer.h"
 #include "hw/qdev-properties.h"
-#include "qemu/main-loop.h"
 #include "qemu/module.h"
 
 #include "trace.h"
@@ -63,7 +62,6 @@  typedef struct GPTimer     GPTimer;
 typedef struct GPTimerUnit GPTimerUnit;
 
 struct GPTimer {
-    QEMUBH *bh;
     struct ptimer_state *ptimer;
 
     qemu_irq     irq;
@@ -93,6 +91,17 @@  struct GPTimerUnit {
     uint32_t config;
 };
 
+static void grlib_gptimer_tx_begin(GPTimer *timer)
+{
+    ptimer_transaction_begin(timer->ptimer);
+}
+
+static void grlib_gptimer_tx_commit(GPTimer *timer)
+{
+    ptimer_transaction_commit(timer->ptimer);
+}
+
+/* Must be called within grlib_gptimer_tx_begin/commit block */
 static void grlib_gptimer_enable(GPTimer *timer)
 {
     assert(timer != NULL);
@@ -115,6 +124,7 @@  static void grlib_gptimer_enable(GPTimer *timer)
     ptimer_run(timer->ptimer, 1);
 }
 
+/* Must be called within grlib_gptimer_tx_begin/commit block */
 static void grlib_gptimer_restart(GPTimer *timer)
 {
     assert(timer != NULL);
@@ -141,7 +151,9 @@  static void grlib_gptimer_set_scaler(GPTimerUnit *unit, uint32_t scaler)
     trace_grlib_gptimer_set_scaler(scaler, value);
 
     for (i = 0; i < unit->nr_timers; i++) {
+        ptimer_transaction_begin(unit->timers[i].ptimer);
         ptimer_set_freq(unit->timers[i].ptimer, value);
+        ptimer_transaction_commit(unit->timers[i].ptimer);
     }
 }
 
@@ -266,8 +278,10 @@  static void grlib_gptimer_write(void *opaque, hwaddr addr,
         switch (timer_addr) {
         case COUNTER_OFFSET:
             trace_grlib_gptimer_writel(id, addr, value);
+            grlib_gptimer_tx_begin(&unit->timers[id]);
             unit->timers[id].counter = value;
             grlib_gptimer_enable(&unit->timers[id]);
+            grlib_gptimer_tx_commit(&unit->timers[id]);
             return;
 
         case COUNTER_RELOAD_OFFSET:
@@ -291,6 +305,7 @@  static void grlib_gptimer_write(void *opaque, hwaddr addr,
             /* gptimer_restart calls gptimer_enable, so if "enable" and "load"
                bits are present, we just have to call restart. */
 
+            grlib_gptimer_tx_begin(&unit->timers[id]);
             if (value & GPTIMER_LOAD) {
                 grlib_gptimer_restart(&unit->timers[id]);
             } else if (value & GPTIMER_ENABLE) {
@@ -301,6 +316,7 @@  static void grlib_gptimer_write(void *opaque, hwaddr addr,
             value &= ~(GPTIMER_LOAD & GPTIMER_DEBUG_HALT);
 
             unit->timers[id].config = value;
+            grlib_gptimer_tx_commit(&unit->timers[id]);
             return;
 
         default:
@@ -344,9 +360,11 @@  static void grlib_gptimer_reset(DeviceState *d)
         timer->counter = 0;
         timer->reload = 0;
         timer->config = 0;
+        ptimer_transaction_begin(timer->ptimer);
         ptimer_stop(timer->ptimer);
         ptimer_set_count(timer->ptimer, 0);
         ptimer_set_freq(timer->ptimer, unit->freq_hz);
+        ptimer_transaction_commit(timer->ptimer);
     }
 }
 
@@ -365,14 +383,16 @@  static void grlib_gptimer_realize(DeviceState *dev, Error **errp)
         GPTimer *timer = &unit->timers[i];
 
         timer->unit   = unit;
-        timer->bh     = qemu_bh_new(grlib_gptimer_hit, timer);
-        timer->ptimer = ptimer_init_with_bh(timer->bh, PTIMER_POLICY_DEFAULT);
+        timer->ptimer = ptimer_init(grlib_gptimer_hit, timer,
+                                    PTIMER_POLICY_DEFAULT);
         timer->id     = i;
 
         /* One IRQ line for each timer */
         sysbus_init_irq(sbd, &timer->irq);
 
+        ptimer_transaction_begin(timer->ptimer);
         ptimer_set_freq(timer->ptimer, unit->freq_hz);
+        ptimer_transaction_commit(timer->ptimer);
     }
 
     memory_region_init_io(&unit->iomem, OBJECT(unit), &grlib_gptimer_ops,