qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Patch to fix malfunctioning of T2-T5 timers on the STM32 platform


From: Lucjan Bryndza
Subject: Patch to fix malfunctioning of T2-T5 timers on the STM32 platform
Date: Fri, 27 Oct 2023 21:37:05 +0200


Current implementation of T2 - T5 times on the STM32 platform does not work properly. 
After configuring the timer-counter circuit to report interrupts every 10ms, in reality the first interrupt is reported 
only once after a few seconds, while subsequent interrupts do not come. 
The current code also does not properly emulate the operation of even the basic functions of the TIME-BASE unit.  
This patch contains fixes that reimplements the basic functionality
of the time base unit such as up-counting down-counting , and alternate-mode up-down counting. 
The ptimer() API is used to emulate timers. 
After applying the patch, STM32 timer works correctly in its basic functionality.
The ISIX-RTOS test unit was used to test the patch.
Links and instructions can be found below:



qemu-system-arm -M olimex-stm32-h405  -kernel build/tests/libisix/isixunittests.binary -nographic
unittests_entry.cpp:146|ISIX VERSION pub/ep0319-157-gb239b35f-dirty
unittests_entry.cpp:83|Exceptions pretest. OK
51 selected tests
[   RUN    ] 01_base_00 TimeBase timer vs systick
[  1001ms  ] ...
[   RUN    ] 01_base_01 Basic heap allocator
[   1ms    ] ...


Best Regards
Lucjan Bryndza


From 3ccfe70979d1b263d4fa22104ecf42ac5a628554 Mon Sep 17 00:00:00 2001
From: Lucjan Bryndza <lbryndza.oss@icloud.com>
Date: Thu, 26 Oct 2023 22:45:26 +0200
Subject: [PATCH] Fixing the basic functionality of STM32 timers

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

Signed-off-by: Lucjan Bryndza <lbryndza.oss@icloud.com>
---
 hw/arm/stm32f405_soc.c             |   2 +-
 hw/timer/stm32f2xx_timer.c         | 291 ++++++++++++++++++-----------
 include/hw/timer/stm32f2xx_timer.h |  23 ++-
 3 files changed, 202 insertions(+), 114 deletions(-)

diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
index cef23d7ee4..69316181b3 100644
--- a/hw/arm/stm32f405_soc.c
+++ b/hw/arm/stm32f405_soc.c
@@ -183,7 +183,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
     /* Timer 2 to 5 */
     for (i = 0; i < STM_NUM_TIMERS; i++) {
         dev = DEVICE(&(s->timer[i]));
-        qdev_prop_set_uint64(dev, "clock-frequency", 1000000000);
+       qdev_prop_set_uint64(dev, "clock-frequency", 48000000);
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->timer[i]), errp)) {
             return;
         }
diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
index ba8694dcd3..65f3287125 100644
--- a/hw/timer/stm32f2xx_timer.c
+++ b/hw/timer/stm32f2xx_timer.c
@@ -29,11 +29,18 @@
 #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
 #endif
 
+/* PCLK /4 */
+#define CLOCK_FREQUENCY 48000000ULL
+
 #define DB_PRINT_L(lvl, fmt, args...) do { \
     if (STM_TIMER_ERR_DEBUG >= lvl) { \
         qemu_log("%s: " fmt, __func__, ## args); \
@@ -42,63 +49,87 @@
 
 #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)
+static uint32_t stm32f2xx_timer_get_count(STM32F2XXTimerState *s)
 {
-    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)));
+    uint64_t cnt = ptimer_get_count(s->timer);
+    if (s->count_mode == TIMER_UP_COUNT) {
+        return s->tim_arr - (cnt & 0xffff);
+    } else {
+        return cnt & 0xffff;
     }
 }
 
-static inline int64_t stm32f2xx_ns_to_ticks(STM32F2XXTimerState *s, int64_t t)
+
+static void stm32f2xx_timer_set_count(STM32F2XXTimerState *s, uint32_t cnt)
 {
-    return muldiv64(t, s->freq_hz, 1000000000ULL) / (s->tim_psc + 1);
+    if (s->count_mode == TIMER_UP_COUNT) {
+        ptimer_set_count(s->timer, s->tim_arr - (cnt & 0xffff));
+    } else {
+        ptimer_set_count(s->timer, cnt & 0xffff);
+    }
 }
 
-static void stm32f2xx_timer_set_alarm(STM32F2XXTimerState *s, int64_t now)
+static void stm32f2xx_timer_update(STM32F2XXTimerState *s)
 {
-    uint64_t ticks;
-    int64_t now_ticks;
+    if (s->tim_cr1 & TIM_CR1_DIR) {
+        s->count_mode = TIMER_DOWN_COUNT;
+    } else {
+        s->count_mode = TIMER_UP_COUNT;
+    }
 
-    if (s->tim_arr == 0) {
-        return;
+    if (s->tim_cr1 & TIM_CR1_CMS) {
+        s->count_mode = TIMER_UP_COUNT;
     }
 
-    DB_PRINT("Alarm set at: 0x%x\n", s->tim_cr1);
+    if (s->tim_cr1 & TIM_CR1_CEN) {
+        DB_PRINT("Enabling timer\n");
+        ptimer_set_freq(s->timer, s->freq_hz);
+        ptimer_run(s->timer, !(s->tim_cr1 & 0x04));
+    } else {
+        DB_PRINT("Disabling timer\n");
+        ptimer_stop(s->timer);
+    }
+}
 
-    now_ticks = stm32f2xx_ns_to_ticks(s, now);
-    ticks = s->tim_arr - (now_ticks - s->tick_offset);
+static void stm32f2xx_timer_update_uif(STM32F2XXTimerState *s, uint8_t value)
+{
+    s->tim_sr &= ~TIM_SR1_UIF;
+    s->tim_sr |= (value & TIM_SR1_UIF);
+    qemu_set_irq(s->irq, value);
+}
 
-    DB_PRINT("Alarm set in %d ticks\n", (int) ticks);
+static void stm32f2xx_timer_tick(void *opaque)
+{
+    STM32F2XXTimerState *s = (STM32F2XXTimerState *)opaque;
+    DB_PRINT("Alarm raised\n");
+    stm32f2xx_timer_update_uif(s, 1);
+
+    if (s->count_mode == TIMER_UP_COUNT) {
+        stm32f2xx_timer_set_count(s, 0);
+    } else {
+        stm32f2xx_timer_set_count(s, s->tim_arr);
+    }
 
-    s->hit_time = muldiv64((ticks + (uint64_t) now_ticks) * (s->tim_psc + 1),
-                               1000000000ULL, s->freq_hz);
+    if (s->tim_cr1 & TIM_CR1_CMS) {
+        if (s->count_mode == TIMER_UP_COUNT) {
+            s->count_mode = TIMER_DOWN_COUNT;
+        } else {
+            s->count_mode = TIMER_UP_COUNT;
+        }
+    }
 
-    timer_mod(s->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->hit_time);
-    DB_PRINT("Wait Time: %" PRId64 " ticks\n", s->hit_time);
+    if (s->tim_cr1 & TIM_CR1_OPM) {
+        s->tim_cr1 &= ~TIM_CR1_CEN;
+    } else {
+        stm32f2xx_timer_update(s);
+    }
 }
 
+
 static void stm32f2xx_timer_reset(DeviceState *dev)
 {
     STM32F2XXTimerState *s = STM32F2XXTIMER(dev);
-    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-
     s->tim_cr1 = 0;
     s->tim_cr2 = 0;
     s->tim_smcr = 0;
@@ -117,8 +148,6 @@ static void stm32f2xx_timer_reset(DeviceState *dev)
     s->tim_dcr = 0;
     s->tim_dmar = 0;
     s->tim_or = 0;
-
-    s->tick_offset = stm32f2xx_ns_to_ticks(s, now);
 }
 
 static uint64_t stm32f2xx_timer_read(void *opaque, hwaddr offset,
@@ -132,15 +161,18 @@ static uint64_t stm32f2xx_timer_read(void *opaque, hwaddr offset,
     case TIM_CR1:
         return s->tim_cr1;
     case TIM_CR2:
-        return s->tim_cr2;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: CR2 not supported");
+        return 0;
     case TIM_SMCR:
-        return s->tim_smcr;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: SMCR not supported");
+        return 0;
     case TIM_DIER:
         return s->tim_dier;
     case TIM_SR:
         return s->tim_sr;
     case TIM_EGR:
-        return s->tim_egr;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: EGR write only");
+        return 0;
     case TIM_CCMR1:
         return s->tim_ccmr1;
     case TIM_CCMR2:
@@ -148,8 +180,7 @@ static uint64_t stm32f2xx_timer_read(void *opaque, hwaddr offset,
     case TIM_CCER:
         return s->tim_ccer;
     case TIM_CNT:
-        return stm32f2xx_ns_to_ticks(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) -
-               s->tick_offset;
+        return stm32f2xx_timer_get_count(s);
     case TIM_PSC:
         return s->tim_psc;
     case TIM_ARR:
@@ -163,105 +194,152 @@ static uint64_t stm32f2xx_timer_read(void *opaque, hwaddr offset,
     case TIM_CCR4:
         return s->tim_ccr4;
     case TIM_DCR:
-        return s->tim_dcr;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: DCR not supported");
+        return 0;
     case TIM_DMAR:
-        return s->tim_dmar;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: CR2 not supported");
+        return 0;
     case TIM_OR:
         return s->tim_or;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, offset);
     }
-
     return 0;
 }
 
+static void stm32f2xx_update_cr1(STM32F2XXTimerState *s, uint64_t value)
+{
+    s->tim_cr1 = value & 0x3FF;
+    ptimer_transaction_begin(s->timer);
+    stm32f2xx_timer_update(s);
+    ptimer_transaction_commit(s->timer);
+    DB_PRINT("write cr1 = %x\n", s->tim_cr1);
+}
+
+static void stm32f2xx_update_sr(STM32F2XXTimerState *s, uint64_t value)
+{
+    s->tim_sr ^= (value ^ 0xFFFF);
+    s->tim_sr &= 0x1eFF;
+    ptimer_transaction_begin(s->timer);
+    stm32f2xx_timer_update_uif(s, s->tim_sr & 0x1);
+    ptimer_transaction_commit(s->timer);
+    DB_PRINT("write sr = %x\n", s->tim_sr);
+}
+
+static void stm32f2xx_update_psc(STM32F2XXTimerState *s, uint64_t value)
+{
+    s->tim_psc = value & 0xffff;
+    ptimer_transaction_begin(s->timer);
+    ptimer_set_freq(s->timer, s->freq_hz);
+    ptimer_transaction_commit(s->timer);
+    DB_PRINT("write psc = %x\n", s->tim_psc);
+}
+
+static void stm32f2xx_update_egr(STM32F2XXTimerState *s, uint64_t value)
+{
+    s->tim_egr = value & 0x1E;
+    if (value & TIM_EGR_TG) {
+        s->tim_sr |= TIM_EGR_TG;
+    }
+    if (value & TIM_EGR_UG) {
+        /* UG bit - reload */
+        ptimer_transaction_begin(s->timer);
+        ptimer_set_limit(s->timer, s->tim_arr, 1);
+        ptimer_transaction_commit(s->timer);
+    }
+    DB_PRINT("write EGR = %x\n", s->tim_egr);
+}
+
+static void stm32f2xx_update_cnt(STM32F2XXTimerState *s, uint64_t value)
+{
+    ptimer_transaction_begin(s->timer);
+    stm32f2xx_timer_set_count(s, value & 0xffff);
+    ptimer_transaction_commit(s->timer);
+    DB_PRINT("write cnt = %x\n", stm32f2xx_timer_get_count(s));
+}
+
+static void stm32f2xx_update_arr(STM32F2XXTimerState *s, uint64_t value)
+{
+    s->tim_arr = value & 0xffff;
+    ptimer_transaction_begin(s->timer);
+    ptimer_set_limit(s->timer, s->tim_arr, 1);
+    ptimer_transaction_commit(s->timer);
+    DB_PRINT("write arr = %x\n", s->tim_arr);
+}
+
 static void stm32f2xx_timer_write(void *opaque, hwaddr offset,
-                        uint64_t val64, unsigned size)
+                        uint64_t value, unsigned size)
 {
     STM32F2XXTimerState *s = opaque;
-    uint32_t value = val64;
-    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    uint32_t timer_val = 0;
-
-    DB_PRINT("Write 0x%x, 0x%"HWADDR_PRIx"\n", value, offset);
 
     switch (offset) {
     case TIM_CR1:
-        s->tim_cr1 = value;
-        return;
+       stm32f2xx_update_cr1(s, value);
+       break;
     case TIM_CR2:
-        s->tim_cr2 = value;
-        return;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: CR2 not supported");
+        break;
     case TIM_SMCR:
-        s->tim_smcr = value;
-        return;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: SCMR not supported");
+        break;
     case TIM_DIER:
-        s->tim_dier = value;
-        return;
+        s->tim_dier = value & 0x5F5F;
+        DB_PRINT("write dier = %x\n", s->tim_dier);
+        break;
     case TIM_SR:
-        /* This is set by hardware and cleared by software */
-        s->tim_sr &= value;
-        return;
+        stm32f2xx_update_sr(s, value);
+        break;
     case TIM_EGR:
-        s->tim_egr = value;
-        if (s->tim_egr & TIM_EGR_UG) {
-            timer_val = 0;
-            break;
-        }
-        return;
+        stm32f2xx_update_egr(s, value);
+        break;
     case TIM_CCMR1:
-        s->tim_ccmr1 = value;
-        return;
+        s->tim_ccmr1 = value & 0xffff;
+        DB_PRINT("write ccmr1 = %x\n", s->tim_ccmr1);
+        break;
     case TIM_CCMR2:
-        s->tim_ccmr2 = value;
-        return;
+        s->tim_ccmr2 = value & 0xffff;
+        DB_PRINT("write ccmr2 = %x\n", s->tim_ccmr2);
+        break;
     case TIM_CCER:
-        s->tim_ccer = value;
-        return;
+        s->tim_ccer = value & 0x3333;
+        DB_PRINT("write ccer = %x\n", s->tim_ccer);
+        break;
     case TIM_PSC:
-        timer_val = stm32f2xx_ns_to_ticks(s, now) - s->tick_offset;
-        s->tim_psc = value & 0xFFFF;
+        stm32f2xx_update_psc(s, value);
         break;
     case TIM_CNT:
-        timer_val = value;
+        stm32f2xx_update_cnt(s, value);
         break;
     case TIM_ARR:
-        s->tim_arr = value;
-        stm32f2xx_timer_set_alarm(s, now);
-        return;
+        stm32f2xx_update_arr(s, value);
+        break;
     case TIM_CCR1:
-        s->tim_ccr1 = value;
-        return;
+        s->tim_ccr1 = value & 0xffff;
+        break;
     case TIM_CCR2:
-        s->tim_ccr2 = value;
-        return;
+        s->tim_ccr2 = value & 0xffff;
+        break;
     case TIM_CCR3:
-        s->tim_ccr3 = value;
-        return;
+        s->tim_ccr3 = value & 0xffff;
+        break;
     case TIM_CCR4:
-        s->tim_ccr4 = value;
-        return;
+        s->tim_ccr4 = value & 0xffff;
+        break;
     case TIM_DCR:
-        s->tim_dcr = value;
-        return;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: DCR not supported");
+        break;
     case TIM_DMAR:
-        s->tim_dmar = value;
-        return;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: DMAR not supported");
+        break;
     case TIM_OR:
-        s->tim_or = value;
-        return;
+        qemu_log_mask(LOG_GUEST_ERROR, "stm32_timer: OR not supported");
+        break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, offset);
-        return;
+        break;
     }
-
-    /* This means that a register write has affected the timer in a way that
-     * requires a refresh of both tick_offset and the alarm.
-     */
-    s->tick_offset = stm32f2xx_ns_to_ticks(s, now) - timer_val;
-    stm32f2xx_timer_set_alarm(s, now);
 }
 
 static const MemoryRegionOps stm32f2xx_timer_ops = {
@@ -275,7 +353,7 @@ static const VMStateDescription vmstate_stm32f2xx_timer = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_INT64(tick_offset, STM32F2XXTimerState),
+        VMSTATE_INT32(count_mode, STM32F2XXTimerState),
         VMSTATE_UINT32(tim_cr1, STM32F2XXTimerState),
         VMSTATE_UINT32(tim_cr2, STM32F2XXTimerState),
         VMSTATE_UINT32(tim_smcr, STM32F2XXTimerState),
@@ -300,25 +378,24 @@ static const VMStateDescription vmstate_stm32f2xx_timer = {
 
 static Property stm32f2xx_timer_properties[] = {
     DEFINE_PROP_UINT64("clock-frequency", struct STM32F2XXTimerState,
-                       freq_hz, 1000000000),
+                       freq_hz, CLOCK_FREQUENCY),
     DEFINE_PROP_END_OF_LIST(),
 };
 
 static void stm32f2xx_timer_init(Object *obj)
 {
     STM32F2XXTimerState *s = STM32F2XXTIMER(obj);
-
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
-
     memory_region_init_io(&s->iomem, obj, &stm32f2xx_timer_ops, s,
                           "stm32f2xx_timer", 0x400);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+
 }
 
 static void stm32f2xx_timer_realize(DeviceState *dev, Error **errp)
 {
     STM32F2XXTimerState *s = STM32F2XXTIMER(dev);
-    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, stm32f2xx_timer_interrupt, s);
+    s->timer = ptimer_init(stm32f2xx_timer_tick, s, PTIMER_POLICY_LEGACY);
 }
 
 static void stm32f2xx_timer_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/timer/stm32f2xx_timer.h b/include/hw/timer/stm32f2xx_timer.h
index 90f40f1746..c83f7b0d6f 100644
--- a/include/hw/timer/stm32f2xx_timer.h
+++ b/include/hw/timer/stm32f2xx_timer.h
@@ -28,6 +28,7 @@
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "qom/object.h"
+#include "hw/ptimer.h"
 
 #define TIM_CR1      0x00
 #define TIM_CR2      0x04
@@ -49,9 +50,15 @@
 #define TIM_DMAR     0x4C
 #define TIM_OR       0x50
 
-#define TIM_CR1_CEN   1
+#define TIM_CR1_CEN 0x0001
+#define TIM_CR1_DIR 0x0010
+#define TIM_CR1_CMS 0x0060
+#define TIM_CR1_OPM 0x0008
 
-#define TIM_EGR_UG 1
+#define TIM_SR1_UIF 0x0001
+
+#define TIM_EGR_UG 0x0001
+#define TIM_EGR_TG 0x0040
 
 #define TIM_CCER_CC2E   (1 << 4)
 #define TIM_CCMR1_OC2M2 (1 << 14)
@@ -61,6 +68,7 @@
 
 #define TIM_DIER_UIE  1
 
+
 #define TYPE_STM32F2XX_TIMER "stm32f2xx-timer"
 typedef struct STM32F2XXTimerState STM32F2XXTimerState;
 DECLARE_INSTANCE_CHECKER(STM32F2XXTimerState, STM32F2XXTIMER,
@@ -72,12 +80,10 @@ struct STM32F2XXTimerState {
 
     /* <public> */
     MemoryRegion iomem;
-    QEMUTimer *timer;
+    ptimer_state *timer;
     qemu_irq irq;
-
-    int64_t tick_offset;
-    uint64_t hit_time;
     uint64_t freq_hz;
+    int count_mode;
 
     uint32_t tim_cr1;
     uint32_t tim_cr2;
@@ -99,4 +105,9 @@ struct STM32F2XXTimerState {
     uint32_t tim_or;
 };
 
+enum {
+    TIMER_UP_COUNT     = 0,
+    TIMER_DOWN_COUNT   = 1
+};
+
 #endif /* HW_STM32F2XX_TIMER_H */
-- 
2.39.3 (Apple Git-145)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]