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

Peter Maydell posted 2 patches 6 years ago
Maintainers: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Fabien Chouteau <chouteau@adacore.com>, KONRAD Frederic <frederic.konrad@adacore.com>
There is a newer version of this series
[PATCH 1/2] hw/timer/grlib_gptimer.c: Switch to transaction-based ptimer API
Posted by Peter Maydell 6 years ago
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,
-- 
2.20.1


Re: [PATCH 1/2] hw/timer/grlib_gptimer.c: Switch to transaction-based ptimer API
Posted by Philippe Mathieu-Daudé 6 years ago

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>


Re: [PATCH 1/2] hw/timer/grlib_gptimer.c: Switch to transaction-based ptimer API
Posted by Richard Henderson 6 years ago
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~