[PATCH qemu v3 02/20] Fixing the basic functionality of STM32 timers

~lbryndza posted 20 patches 12 months ago
Maintainers: Alistair Francis <alistair@alistair23.me>, Peter Maydell <peter.maydell@linaro.org>
[PATCH qemu v3 02/20] Fixing the basic functionality of STM32 timers
Posted by ~lbryndza 12 months ago
From: Lucjan Bryndza <lbryndza.oss@icloud.com>

The current implementation of timers does not work properly
even in basic functionality. A counter configured to report
an interrupt every 10ms reports the first interrupts after a
few seconds.  There are also no properly implemented count up an
count down modes. This commit fixes bugs with interrupt
reporting and implements the basic modes of the counter's
time-base block.

Remove wrong qemu timer implementation

Signed-off-by: Lucjan Bryndza <lbryndza.oss@icloud.com>
---
 hw/timer/stm32f2xx_timer.c | 55 ++++----------------------------------
 1 file changed, 5 insertions(+), 50 deletions(-)

diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
index ba8694dcd3..f03f594a17 100644
--- a/hw/timer/stm32f2xx_timer.c
+++ b/hw/timer/stm32f2xx_timer.c
@@ -23,12 +23,17 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/timer/stm32f2xx_timer.h"
 #include "migration/vmstate.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qemu/typedefs.h"
+#include "qemu/timer.h"
+#include "qemu/main-loop.h"
+#include "sysemu/dma.h"
 
 #ifndef STM_TIMER_ERR_DEBUG
 #define STM_TIMER_ERR_DEBUG 0
@@ -42,57 +47,7 @@
 
 #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
 
-static void stm32f2xx_timer_set_alarm(STM32F2XXTimerState *s, int64_t now);
 
-static void stm32f2xx_timer_interrupt(void *opaque)
-{
-    STM32F2XXTimerState *s = opaque;
-
-    DB_PRINT("Interrupt\n");
-
-    if (s->tim_dier & TIM_DIER_UIE && s->tim_cr1 & TIM_CR1_CEN) {
-        s->tim_sr |= 1;
-        qemu_irq_pulse(s->irq);
-        stm32f2xx_timer_set_alarm(s, s->hit_time);
-    }
-
-    if (s->tim_ccmr1 & (TIM_CCMR1_OC2M2 | TIM_CCMR1_OC2M1) &&
-        !(s->tim_ccmr1 & TIM_CCMR1_OC2M0) &&
-        s->tim_ccmr1 & TIM_CCMR1_OC2PE &&
-        s->tim_ccer & TIM_CCER_CC2E) {
-        /* PWM 2 - Mode 1 */
-        DB_PRINT("PWM2 Duty Cycle: %d%%\n",
-                s->tim_ccr2 / (100 * (s->tim_psc + 1)));
-    }
-}
-
-static inline int64_t stm32f2xx_ns_to_ticks(STM32F2XXTimerState *s, int64_t t)
-{
-    return muldiv64(t, s->freq_hz, 1000000000ULL) / (s->tim_psc + 1);
-}
-
-static void stm32f2xx_timer_set_alarm(STM32F2XXTimerState *s, int64_t now)
-{
-    uint64_t ticks;
-    int64_t now_ticks;
-
-    if (s->tim_arr == 0) {
-        return;
-    }
-
-    DB_PRINT("Alarm set at: 0x%x\n", s->tim_cr1);
-
-    now_ticks = stm32f2xx_ns_to_ticks(s, now);
-    ticks = s->tim_arr - (now_ticks - s->tick_offset);
-
-    DB_PRINT("Alarm set in %d ticks\n", (int) ticks);
-
-    s->hit_time = muldiv64((ticks + (uint64_t) now_ticks) * (s->tim_psc + 1),
-                               1000000000ULL, s->freq_hz);
-
-    timer_mod(s->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->hit_time);
-    DB_PRINT("Wait Time: %" PRId64 " ticks\n", s->hit_time);
-}
 
 static void stm32f2xx_timer_reset(DeviceState *dev)
 {
-- 
2.38.5
Re: [PATCH qemu v3 02/20] Fixing the basic functionality of STM32 timers
Posted by Alex Bennée 11 months, 3 weeks ago
~lbryndza <lbryndza@git.sr.ht> writes:

> From: Lucjan Bryndza <lbryndza.oss@icloud.com>
>
> The current implementation of timers does not work properly
> even in basic functionality. A counter configured to report
> an interrupt every 10ms reports the first interrupts after a
> few seconds.  There are also no properly implemented count up an
> count down modes. This commit fixes bugs with interrupt
> reporting and implements the basic modes of the counter's
> time-base block.
>
> Remove wrong qemu timer implementation

I suspect this breaks bisectability of the series. Each point in the
series should still be able to compile and at least function as well as
it did before. So in this case I think this patch needs to be merged
with the patch that brings in the replacement functionality.

>
> Signed-off-by: Lucjan Bryndza <lbryndza.oss@icloud.com>
> ---
>  hw/timer/stm32f2xx_timer.c | 55 ++++----------------------------------
>  1 file changed, 5 insertions(+), 50 deletions(-)
>
> diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
> index ba8694dcd3..f03f594a17 100644
> --- a/hw/timer/stm32f2xx_timer.c
> +++ b/hw/timer/stm32f2xx_timer.c
> @@ -23,12 +23,17 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/timer/stm32f2xx_timer.h"
>  #include "migration/vmstate.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> +#include "qemu/typedefs.h"
> +#include "qemu/timer.h"
> +#include "qemu/main-loop.h"
> +#include "sysemu/dma.h"

Seems odd to increase the includes needed when the rest of the patch
just deletes code.

<snip>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro