Re: [PATCH V1 1/2] hw/rtc: Fixed loongson rtc emulation errors

[Bibo Mao]

On 2025/6/13 上午9:31, Xianglai Li wrote:
The expire time is sent to the timer only
when the expire Time is greater than 0 or
greater than now. Otherwise, the timer
will trigger interruption continuously.
Again, one line of the comments should no more than 75 bytes :)

I think it should be split into two patches, one is fixup with irq pulse 
type setting, the other is fixup with expired time out calculation in 
rtc emulation driver.

Regards
Bibo Mao

Timer interrupts are sent using pulse functions.


Signed-off-by: Xianglai Li <lixiang...@loongson.cn>
---
Cc: Bibo Mao <maob...@loongson.cn>
Cc: Song Gao <gaos...@loongson.cn>
Cc: Jiaxun Yang <jiaxun.y...@flygoat.com>
Cc: Xianglai Li <lixiang...@loongson.cn>

  hw/loongarch/virt-fdt-build.c | 11 +++++++++--
  hw/rtc/ls7a_rtc.c             | 26 +++++++++++++++++---------
  2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/hw/loongarch/virt-fdt-build.c b/hw/loongarch/virt-fdt-build.c
index 728ce46699..c613131a07 100644
--- a/hw/loongarch/virt-fdt-build.c
+++ b/hw/loongarch/virt-fdt-build.c
@@ -17,6 +17,11 @@
  #include "system/reset.h"
  #include "target/loongarch/cpu.h"
  
+#define FDT_IRQ_FLAGS_EDGE_LO_HI 1
+#define FDT_IRQ_FLAGS_EDGE_HI_LO 2
+#define FDT_IRQ_FLAGS_LEVEL_HI 4
+#define FDT_IRQ_FLAGS_LEVEL_LO 8
+
  static void create_fdt(LoongArchVirtMachineState *lvms)
  {
      MachineState *ms = MACHINE(lvms);
@@ -416,7 +421,8 @@ static void fdt_add_uart_node(LoongArchVirtMachineState 
*lvms,
      if (chosen) {
          qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
      }
-    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq,
+                           FDT_IRQ_FLAGS_LEVEL_HI);
      qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
                            *pch_pic_phandle);
      g_free(nodename);
@@ -436,7 +442,8 @@ static void fdt_add_rtc_node(LoongArchVirtMachineState 
*lvms,
                              "loongson,ls7a-rtc");
      qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg", 2, base, 2, size);
      qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
-                           VIRT_RTC_IRQ - VIRT_GSI_BASE , 0x4);
+                           VIRT_RTC_IRQ - VIRT_GSI_BASE ,
+                           FDT_IRQ_FLAGS_EDGE_LO_HI);
      qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
                            *pch_pic_phandle);
      g_free(nodename);
diff --git a/hw/rtc/ls7a_rtc.c b/hw/rtc/ls7a_rtc.c
index 10097b2db7..7eca75a42a 100644
--- a/hw/rtc/ls7a_rtc.c
+++ b/hw/rtc/ls7a_rtc.c
@@ -145,20 +145,24 @@ static void toymatch_write(LS7ARtcState *s, uint64_t val, 
int num)
          now = qemu_clock_get_ms(rtc_clock);
          toymatch_val_to_time(s, val, &tm);
          expire_time = now + (qemu_timedate_diff(&tm) - s->offset_toy) * 1000;
-        timer_mod(s->toy_timer[num], expire_time);
+        if (expire_time > now) {
+            timer_mod(s->toy_timer[num], expire_time);
+        }
      }
  }
  
  static void rtcmatch_write(LS7ARtcState *s, uint64_t val, int num)
  {
-    uint64_t expire_ns;
+    int64_t expire_ns;
  
      /* it do not support write when toy disabled */
      if (rtc_enabled(s)) {
          s->rtcmatch[num] = val;
          /* calculate expire time */
          expire_ns = ticks_to_ns(val) - ticks_to_ns(s->offset_rtc);
-        timer_mod_ns(s->rtc_timer[num], expire_ns);
+        if (expire_ns > 0) {
+            timer_mod_ns(s->rtc_timer[num], expire_ns);
+        }
      }
  }
  
@@ -185,7 +189,7 @@ static void ls7a_rtc_stop(LS7ARtcState *s)
  static void ls7a_toy_start(LS7ARtcState *s)
  {
      int i;
-    uint64_t expire_time, now;
+    int64_t expire_time, now;
      struct tm tm = {};
  
      now = qemu_clock_get_ms(rtc_clock);
@@ -194,19 +198,23 @@ static void ls7a_toy_start(LS7ARtcState *s)
      for (i = 0; i < TIMER_NUMS; i++) {
          toymatch_val_to_time(s, s->toymatch[i], &tm);
          expire_time = now + (qemu_timedate_diff(&tm) - s->offset_toy) * 1000;
-        timer_mod(s->toy_timer[i], expire_time);
+        if (expire_time > now) {
+            timer_mod(s->toy_timer[i], expire_time);
+        }
      }
  }
  
  static void ls7a_rtc_start(LS7ARtcState *s)
  {
      int i;
-    uint64_t expire_time;
+    int64_t expire_time;
  
      /* recalculate expire time and enable timer */
      for (i = 0; i < TIMER_NUMS; i++) {
          expire_time = ticks_to_ns(s->rtcmatch[i]) - 
ticks_to_ns(s->offset_rtc);
-        timer_mod_ns(s->rtc_timer[i], expire_time);
+        if (expire_time > 0) {
+            timer_mod_ns(s->rtc_timer[i], expire_time);
+        }
      }
  }
  
@@ -370,7 +378,7 @@ static void toy_timer_cb(void *opaque)
      LS7ARtcState *s = opaque;
  
      if (toy_enabled(s)) {
-        qemu_irq_raise(s->irq);
+        qemu_irq_pulse(s->irq);
      }
  }
  
@@ -379,7 +387,7 @@ static void rtc_timer_cb(void *opaque)
      LS7ARtcState *s = opaque;
  
      if (rtc_enabled(s)) {
-        qemu_irq_raise(s->irq);
+        qemu_irq_pulse(s->irq);
      }
  }