[PATCH qemu.git 04/11] hw/timer/imx_epit: remove explicit fields cnt and freq

[PATCH qemu.git 04/11] hw/timer/imx_epit: remove explicit fields cnt and freq

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

The CNT register is a read-only register. There is no need to
store it's value, it can be calculated on demand.
The calculated frequency is needed temporarily only.

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c         | 42 +++++++++++++++----------------------
 include/hw/timer/imx_epit.h |  2 --
 2 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index a79f58c963..37b04a1b53 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -77,23 +77,25 @@ static void imx_epit_update_int(IMXEPITState *s)
  * Must be called from within a ptimer_transaction_begin/commit block
  * for both s->timer_cmp and s->timer_reload.
  */
-static void imx_epit_set_freq(IMXEPITState *s)
+static uint32_t imx_epit_set_freq(IMXEPITState *s)
 {
     uint32_t clksrc;
     uint32_t prescaler;
+    uint32_t freq;
 
     clksrc = extract32(s->cr, CR_CLKSRC_SHIFT, 2);
     prescaler = 1 + extract32(s->cr, CR_PRESCALE_SHIFT, 12);
 
-    s->freq = imx_ccm_get_clock_frequency(s->ccm,
+    freq = imx_ccm_get_clock_frequency(s->ccm,
                                 imx_epit_clocks[clksrc]) / prescaler;
 
-    DPRINTF("Setting ptimer frequency to %u\n", s->freq);
+    DPRINTF("Setting ptimer frequency to %u\n", freq);
 
-    if (s->freq) {
-        ptimer_set_freq(s->timer_reload, s->freq);
-        ptimer_set_freq(s->timer_cmp, s->freq);
+    if (freq) {
+        ptimer_set_freq(s->timer_reload, freq);
+        ptimer_set_freq(s->timer_cmp, freq);
     }
+    return freq;
 }
 
 static void imx_epit_reset(DeviceState *dev)
@@ -107,18 +109,17 @@ static void imx_epit_reset(DeviceState *dev)
     s->sr = 0;
     s->lr = EPIT_TIMER_MAX;
     s->cmp = 0;
-    s->cnt = 0;
     ptimer_transaction_begin(s->timer_cmp);
     ptimer_transaction_begin(s->timer_reload);
     /* stop both timers */
     ptimer_stop(s->timer_cmp);
     ptimer_stop(s->timer_reload);
     /* compute new frequency */
-    imx_epit_set_freq(s);
+    uint32_t freq = imx_epit_set_freq(s);
     /* init both timers to EPIT_TIMER_MAX */
     ptimer_set_limit(s->timer_cmp, EPIT_TIMER_MAX, 1);
     ptimer_set_limit(s->timer_reload, EPIT_TIMER_MAX, 1);
-    if (s->freq && (s->cr & CR_EN)) {
+    if (freq && (s->cr & CR_EN)) {
         /* if the timer is still enabled, restart it */
         ptimer_run(s->timer_reload, 0);
     }
@@ -126,13 +127,6 @@ static void imx_epit_reset(DeviceState *dev)
     ptimer_transaction_commit(s->timer_reload);
 }
 
-static uint32_t imx_epit_update_count(IMXEPITState *s)
-{
-    s->cnt = ptimer_get_count(s->timer_reload);
-
-    return s->cnt;
-}
-
 static uint64_t imx_epit_read(void *opaque, hwaddr offset, unsigned size)
 {
     IMXEPITState *s = IMX_EPIT(opaque);
@@ -156,8 +150,7 @@ static uint64_t imx_epit_read(void *opaque, hwaddr offset, unsigned size)
         break;
 
     case 4: /* CNT */
-        imx_epit_update_count(s);
-        reg_value = s->cnt;
+        reg_value = ptimer_get_count(s->timer_reload);
         break;
 
     default:
@@ -176,7 +169,7 @@ static void imx_epit_reload_compare_timer(IMXEPITState *s)
 {
     if ((s->cr & (CR_EN | CR_OCIEN)) == (CR_EN | CR_OCIEN))  {
         /* if the compare feature is on and timers are running */
-        uint32_t tmp = imx_epit_update_count(s);
+        uint32_t tmp = ptimer_get_count(s->timer_reload);
         uint64_t next;
         if (tmp > s->cmp) {
             /* It'll fire in this round of the timer */
@@ -190,6 +183,7 @@ static void imx_epit_reload_compare_timer(IMXEPITState *s)
 
 static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
 {
+    uint32_t freq = 0;
     uint32_t oldcr = s->cr;
     s->cr = value & 0x03ffffff;
     if (s->cr & CR_SWR) {
@@ -205,10 +199,10 @@ static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
     ptimer_transaction_begin(s->timer_reload);
 
     if (!(s->cr & CR_SWR)) {
-        imx_epit_set_freq(s);
+        freq = imx_epit_set_freq(s);
     }
 
-    if (s->freq && (s->cr & CR_EN) && !(oldcr & CR_EN)) {
+    if (freq && (s->cr & CR_EN) && !(oldcr & CR_EN)) {
         if (s->cr & CR_ENMOD) {
             if (s->cr & CR_RLD) {
                 ptimer_set_limit(s->timer_reload, s->lr, 1);
@@ -342,15 +336,13 @@ static const MemoryRegionOps imx_epit_ops = {
 
 static const VMStateDescription vmstate_imx_timer_epit = {
     .name = TYPE_IMX_EPIT,
-    .version_id = 2,
-    .minimum_version_id = 2,
+    .version_id = 3,
+    .minimum_version_id = 3,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(cr, IMXEPITState),
         VMSTATE_UINT32(sr, IMXEPITState),
         VMSTATE_UINT32(lr, IMXEPITState),
         VMSTATE_UINT32(cmp, IMXEPITState),
-        VMSTATE_UINT32(cnt, IMXEPITState),
-        VMSTATE_UINT32(freq, IMXEPITState),
         VMSTATE_PTIMER(timer_reload, IMXEPITState),
         VMSTATE_PTIMER(timer_cmp, IMXEPITState),
         VMSTATE_END_OF_LIST()
diff --git a/include/hw/timer/imx_epit.h b/include/hw/timer/imx_epit.h
index 2acc41e982..2219a426ab 100644
--- a/include/hw/timer/imx_epit.h
+++ b/include/hw/timer/imx_epit.h
@@ -72,9 +72,7 @@ struct IMXEPITState {
     uint32_t sr;
     uint32_t lr;
     uint32_t cmp;
-    uint32_t cnt;
 
-    uint32_t freq;
     qemu_irq irq;
 };
 
-- 
2.34.5

[PATCH qemu.git 05/11] hw/timer/imx_epit: simplify interrupt logic

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 37b04a1b53..d21cbf16f6 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -61,18 +61,6 @@ static const IMXClk imx_epit_clocks[] =  {
     CLK_32k,       /* 11 ipg_clk_32k -- ~32kHz */
 };
 
-/*
- * Update interrupt status
- */
-static void imx_epit_update_int(IMXEPITState *s)
-{
-    if (s->sr && (s->cr & CR_OCIEN) && (s->cr & CR_EN)) {
-        qemu_irq_raise(s->irq);
-    } else {
-        qemu_irq_lower(s->irq);
-    }
-}
-
 /*
  * Must be called from within a ptimer_transaction_begin/commit block
  * for both s->timer_cmp and s->timer_reload.
@@ -242,7 +230,7 @@ static void imx_epit_write_sr(IMXEPITState *s, uint32_t value)
     /* writing 1 to OCIF clears the OCIF bit */
     if (value & 0x01) {
         s->sr = 0;
-        imx_epit_update_int(s);
+        qemu_irq_lower(s->irq);
     }
 }
 
@@ -320,7 +308,14 @@ static void imx_epit_cmp(void *opaque)
     DPRINTF("sr was %d\n", s->sr);
 
     s->sr = 1;
-    imx_epit_update_int(s);
+
+    /*
+     * An interrupt is generated only if both the peripheral is enabled and the
+     * interrupt generation is enabled.
+     */
+    if ((s->cr & (CR_EN | CR_OCIEN)) == (CR_EN | CR_OCIEN)) {
+        qemu_irq_raise(s->irq);
+    }
 }
 
 static void imx_epit_reload(void *opaque)
-- 
2.34.5

[PATCH qemu.git 01/11] hw/timer/imx_epit: fix typo in comment

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index ec0fa440d7..06785fe6f6 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -254,7 +254,7 @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
         break;
 
     case 1: /* SR - ACK*/
-        /* writing 1 to OCIF clear the OCIF bit */
+        /* writing 1 to OCIF clears the OCIF bit */
         if (value & 0x01) {
             s->sr = 0;
             imx_epit_update_int(s);
-- 
2.34.5

[PATCH qemu.git 02/11] hw/timer/imx_epit: improve comments

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 06785fe6f6..b6c013292f 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -352,8 +352,18 @@ static void imx_epit_realize(DeviceState *dev, Error **errp)
                           0x00001000);
     sysbus_init_mmio(sbd, &s->iomem);
 
+    /*
+     * The reload timer keeps running when the peripheral is enabled. It is a
+     * kind of wall clock that does not generate any interrupts. The callback
+     * needs to be provided, but it does nothing as the ptimer already supports
+     * all necessary reloading functionality.
+     */
     s->timer_reload = ptimer_init(imx_epit_reload, s, PTIMER_POLICY_LEGACY);
 
+    /*
+     * The compare timer is running only when the peripheral configuration is
+     * in a state that will generate compare interrupts.
+     */
     s->timer_cmp = ptimer_init(imx_epit_cmp, s, PTIMER_POLICY_LEGACY);
 }
 
-- 
2.34.5

[PATCH qemu.git 07/11] hw/timer/imx_epit: do not persist CR.SWR bit

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 2e4ff89613..bba9c87cd4 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -175,9 +175,12 @@ static void imx_epit_reload_compare_timer(IMXEPITState *s)
 static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
 {
     uint32_t freq = 0;
+
+    /* SWR bit is never persisted, it clears itself once reset is done */
     uint32_t oldcr = s->cr;
-    s->cr = value & 0x03ffffff;
-    if (s->cr & CR_SWR) {
+    s->cr = (value & ~CR_SWR) & 0x03ffffff;
+
+    if (value & CR_SWR) {
         /* handle the reset */
         imx_epit_reset(DEVICE(s));
         /*
@@ -189,7 +192,7 @@ static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
     ptimer_transaction_begin(s->timer_cmp);
     ptimer_transaction_begin(s->timer_reload);
 
-    if (!(s->cr & CR_SWR)) {
+    if (!(value & CR_SWR)) {
         freq = imx_epit_set_freq(s);
     }
 
-- 
2.34.5

[PATCH qemu.git 06/11] hw/timer/imx_epit: software reset clears the interrupt

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

---
 hw/timer/imx_epit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index d21cbf16f6..2e4ff89613 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -97,6 +97,9 @@ static void imx_epit_reset(DeviceState *dev)
     s->sr = 0;
     s->lr = EPIT_TIMER_MAX;
     s->cmp = 0;
+    /* clear the interrupt */
+    qemu_irq_lower(s->irq);
+
     ptimer_transaction_begin(s->timer_cmp);
     ptimer_transaction_begin(s->timer_reload);
     /* stop both timers */
-- 
2.34.5

[PATCH qemu.git 03/11] hw/timer/imx_epit: factor out register write handlers

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c | 189 ++++++++++++++++++++++++--------------------
 1 file changed, 103 insertions(+), 86 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index b6c013292f..a79f58c963 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -188,111 +188,128 @@ static void imx_epit_reload_compare_timer(IMXEPITState *s)
     }
 }
 
-static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
-                           unsigned size)
+static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
 {
-    IMXEPITState *s = IMX_EPIT(opaque);
-    uint64_t oldcr;
-
-    DPRINTF("(%s, value = 0x%08x)\n", imx_epit_reg_name(offset >> 2),
-            (uint32_t)value);
-
-    switch (offset >> 2) {
-    case 0: /* CR */
-
-        oldcr = s->cr;
-        s->cr = value & 0x03ffffff;
-        if (s->cr & CR_SWR) {
-            /* handle the reset */
-            imx_epit_reset(DEVICE(s));
-            /*
-             * TODO: could we 'break' here? following operations appear
-             * to duplicate the work imx_epit_reset() already did.
-             */
-        }
-
-        ptimer_transaction_begin(s->timer_cmp);
-        ptimer_transaction_begin(s->timer_reload);
+    uint32_t oldcr = s->cr;
+    s->cr = value & 0x03ffffff;
+    if (s->cr & CR_SWR) {
+        /* handle the reset */
+        imx_epit_reset(DEVICE(s));
+        /*
+         * TODO: could we 'break' here? following operations appear
+         * to duplicate the work imx_epit_reset() already did.
+         */
+    }
 
-        if (!(s->cr & CR_SWR)) {
-            imx_epit_set_freq(s);
-        }
+    ptimer_transaction_begin(s->timer_cmp);
+    ptimer_transaction_begin(s->timer_reload);
 
-        if (s->freq && (s->cr & CR_EN) && !(oldcr & CR_EN)) {
-            if (s->cr & CR_ENMOD) {
-                if (s->cr & CR_RLD) {
-                    ptimer_set_limit(s->timer_reload, s->lr, 1);
-                    ptimer_set_limit(s->timer_cmp, s->lr, 1);
-                } else {
-                    ptimer_set_limit(s->timer_reload, EPIT_TIMER_MAX, 1);
-                    ptimer_set_limit(s->timer_cmp, EPIT_TIMER_MAX, 1);
-                }
-            }
+    if (!(s->cr & CR_SWR)) {
+        imx_epit_set_freq(s);
+    }
 
-            imx_epit_reload_compare_timer(s);
-            ptimer_run(s->timer_reload, 0);
-            if (s->cr & CR_OCIEN) {
-                ptimer_run(s->timer_cmp, 0);
+    if (s->freq && (s->cr & CR_EN) && !(oldcr & CR_EN)) {
+        if (s->cr & CR_ENMOD) {
+            if (s->cr & CR_RLD) {
+                ptimer_set_limit(s->timer_reload, s->lr, 1);
+                ptimer_set_limit(s->timer_cmp, s->lr, 1);
             } else {
-                ptimer_stop(s->timer_cmp);
-            }
-        } else if (!(s->cr & CR_EN)) {
-            /* stop both timers */
-            ptimer_stop(s->timer_reload);
-            ptimer_stop(s->timer_cmp);
-        } else  if (s->cr & CR_OCIEN) {
-            if (!(oldcr & CR_OCIEN)) {
-                imx_epit_reload_compare_timer(s);
-                ptimer_run(s->timer_cmp, 0);
+                ptimer_set_limit(s->timer_reload, EPIT_TIMER_MAX, 1);
+                ptimer_set_limit(s->timer_cmp, EPIT_TIMER_MAX, 1);
             }
+        }
+
+        imx_epit_reload_compare_timer(s);
+        ptimer_run(s->timer_reload, 0);
+        if (s->cr & CR_OCIEN) {
+            ptimer_run(s->timer_cmp, 0);
         } else {
             ptimer_stop(s->timer_cmp);
         }
+    } else if (!(s->cr & CR_EN)) {
+        /* stop both timers */
+        ptimer_stop(s->timer_reload);
+        ptimer_stop(s->timer_cmp);
+    } else if (s->cr & CR_OCIEN) {
+        if (!(oldcr & CR_OCIEN)) {
+            imx_epit_reload_compare_timer(s);
+            ptimer_run(s->timer_cmp, 0);
+        }
+    } else {
+        ptimer_stop(s->timer_cmp);
+    }
+
+    ptimer_transaction_commit(s->timer_cmp);
+    ptimer_transaction_commit(s->timer_reload);
+}
+
+static void imx_epit_write_sr(IMXEPITState *s, uint32_t value)
+{
+    /* writing 1 to OCIF clears the OCIF bit */
+    if (value & 0x01) {
+        s->sr = 0;
+        imx_epit_update_int(s);
+    }
+}
+
+static void imx_epit_write_lr(IMXEPITState *s, uint32_t value)
+{
+    s->lr = value;
+
+    ptimer_transaction_begin(s->timer_cmp);
+    ptimer_transaction_begin(s->timer_reload);
+    if (s->cr & CR_RLD) {
+        /* Also set the limit if the LRD bit is set */
+        /* If IOVW bit is set then set the timer value */
+        ptimer_set_limit(s->timer_reload, s->lr, s->cr & CR_IOVW);
+        ptimer_set_limit(s->timer_cmp, s->lr, 0);
+    } else if (s->cr & CR_IOVW) {
+        /* If IOVW bit is set then set the timer value */
+        ptimer_set_count(s->timer_reload, s->lr);
+    }
+    /*
+     * Commit the change to s->timer_reload, so it can propagate. Otherwise
+     * the timer interrupt may not fire properly. The commit must happen
+     * before calling imx_epit_reload_compare_timer(), which reads
+     * s->timer_reload internally again.
+     */
+    ptimer_transaction_commit(s->timer_reload);
+    imx_epit_reload_compare_timer(s);
+    ptimer_transaction_commit(s->timer_cmp);
+}
+
+static void imx_epit_write_cmp(IMXEPITState *s, uint32_t value)
+{
+    s->cmp = value;
+
+    ptimer_transaction_begin(s->timer_cmp);
+    imx_epit_reload_compare_timer(s);
+    ptimer_transaction_commit(s->timer_cmp);
+}
 
-        ptimer_transaction_commit(s->timer_cmp);
-        ptimer_transaction_commit(s->timer_reload);
+static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
+                           unsigned size)
+{
+    IMXEPITState *s = IMX_EPIT(opaque);
+
+    DPRINTF("(%s, value = 0x%08x)\n", imx_epit_reg_name(offset >> 2),
+            (uint32_t)value);
+
+    switch (offset >> 2) {
+    case 0: /* CR */
+        imx_epit_write_cr(s, (uint32_t)value);
         break;
 
     case 1: /* SR - ACK*/
-        /* writing 1 to OCIF clears the OCIF bit */
-        if (value & 0x01) {
-            s->sr = 0;
-            imx_epit_update_int(s);
-        }
+        imx_epit_write_sr(s, (uint32_t)value);
         break;
 
     case 2: /* LR - set ticks */
-        s->lr = value;
-
-        ptimer_transaction_begin(s->timer_cmp);
-        ptimer_transaction_begin(s->timer_reload);
-        if (s->cr & CR_RLD) {
-            /* Also set the limit if the LRD bit is set */
-            /* If IOVW bit is set then set the timer value */
-            ptimer_set_limit(s->timer_reload, s->lr, s->cr & CR_IOVW);
-            ptimer_set_limit(s->timer_cmp, s->lr, 0);
-        } else if (s->cr & CR_IOVW) {
-            /* If IOVW bit is set then set the timer value */
-            ptimer_set_count(s->timer_reload, s->lr);
-        }
-        /*
-         * Commit the change to s->timer_reload, so it can propagate. Otherwise
-         * the timer interrupt may not fire properly. The commit must happen
-         * before calling imx_epit_reload_compare_timer(), which reads
-         * s->timer_reload internally again.
-         */
-        ptimer_transaction_commit(s->timer_reload);
-        imx_epit_reload_compare_timer(s);
-        ptimer_transaction_commit(s->timer_cmp);
+        imx_epit_write_lr(s, (uint32_t)value);
         break;
 
     case 3: /* CMP */
-        s->cmp = value;
-
-        ptimer_transaction_begin(s->timer_cmp);
-        imx_epit_reload_compare_timer(s);
-        ptimer_transaction_commit(s->timer_cmp);
-
+        imx_epit_write_cmp(s, (uint32_t)value);
         break;
 
     default:
-- 
2.34.5

[PATCH qemu.git 08/11] hw/timer/imx_epit: simplify CR.ENMOD handling

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index bba9c87cd4..5915d4b3d4 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -198,13 +198,10 @@ static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
 
     if (freq && (s->cr & CR_EN) && !(oldcr & CR_EN)) {
         if (s->cr & CR_ENMOD) {
-            if (s->cr & CR_RLD) {
-                ptimer_set_limit(s->timer_reload, s->lr, 1);
-                ptimer_set_limit(s->timer_cmp, s->lr, 1);
-            } else {
-                ptimer_set_limit(s->timer_reload, EPIT_TIMER_MAX, 1);
-                ptimer_set_limit(s->timer_cmp, EPIT_TIMER_MAX, 1);
-            }
+            uint64_t limit = (s->cr & CR_RLD) ? s->lr : EPIT_TIMER_MAX;
+            /* set new limit and also set timer to this value right now */
+            ptimer_set_limit(s->timer_reload, limit, 1);
+            ptimer_set_limit(s->timer_cmp, limit, 1);
         }
 
         imx_epit_reload_compare_timer(s);
-- 
2.34.5

[PATCH qemu.git 09/11] hw/timer/imx_epit: cleanup CR defines

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

remove unused defines, add needed defines

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c         | 4 ++--
 include/hw/timer/imx_epit.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 5915d4b3d4..196fc67c30 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -71,8 +71,8 @@ static uint32_t imx_epit_set_freq(IMXEPITState *s)
     uint32_t prescaler;
     uint32_t freq;
 
-    clksrc = extract32(s->cr, CR_CLKSRC_SHIFT, 2);
-    prescaler = 1 + extract32(s->cr, CR_PRESCALE_SHIFT, 12);
+    clksrc = extract32(s->cr, CR_CLKSRC_SHIFT, CR_CLKSRC_BITS);
+    prescaler = 1 + extract32(s->cr, CR_PRESCALE_SHIFT, CR_PRESCALE_BITS);
 
     freq = imx_ccm_get_clock_frequency(s->ccm,
                                 imx_epit_clocks[clksrc]) / prescaler;
diff --git a/include/hw/timer/imx_epit.h b/include/hw/timer/imx_epit.h
index 2219a426ab..f6d41be7e1 100644
--- a/include/hw/timer/imx_epit.h
+++ b/include/hw/timer/imx_epit.h
@@ -43,7 +43,7 @@
 #define CR_OCIEN    (1 << 2)
 #define CR_RLD      (1 << 3)
 #define CR_PRESCALE_SHIFT (4)
-#define CR_PRESCALE_MASK  (0xfff)
+#define CR_PRESCALE_BITS  (12)
 #define CR_SWR      (1 << 16)
 #define CR_IOVW     (1 << 17)
 #define CR_DBGEN    (1 << 18)
@@ -51,7 +51,7 @@
 #define CR_DOZEN    (1 << 20)
 #define CR_STOPEN   (1 << 21)
 #define CR_CLKSRC_SHIFT (24)
-#define CR_CLKSRC_MASK  (0x3 << CR_CLKSRC_SHIFT)
+#define CR_CLKSRC_BITS  (2)
 
 #define EPIT_TIMER_MAX  0XFFFFFFFFUL
 
-- 
2.34.5

[PATCH qemu.git 10/11] hw/timer/imx_epit: fix compare timer update

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

The compare timer will never fire if the reload value is less
than the compare value, so it must be disabled in this case.
The compare time fire exactly once when the compare value is
less than the current value, but the reload values is less
than the compare value.

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c | 112 +++++++++++++++++++++++++-------------------
 1 file changed, 64 insertions(+), 48 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 196fc67c30..7dd9f54c9c 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -61,29 +61,12 @@ static const IMXClk imx_epit_clocks[] =  {
     CLK_32k,       /* 11 ipg_clk_32k -- ~32kHz */
 };
 
-/*
- * Must be called from within a ptimer_transaction_begin/commit block
- * for both s->timer_cmp and s->timer_reload.
- */
-static uint32_t imx_epit_set_freq(IMXEPITState *s)
+static uint32_t imx_epit_get_freq(IMXEPITState *s)
 {
-    uint32_t clksrc;
-    uint32_t prescaler;
-    uint32_t freq;
-
-    clksrc = extract32(s->cr, CR_CLKSRC_SHIFT, CR_CLKSRC_BITS);
-    prescaler = 1 + extract32(s->cr, CR_PRESCALE_SHIFT, CR_PRESCALE_BITS);
-
-    freq = imx_ccm_get_clock_frequency(s->ccm,
-                                imx_epit_clocks[clksrc]) / prescaler;
-
-    DPRINTF("Setting ptimer frequency to %u\n", freq);
-
-    if (freq) {
-        ptimer_set_freq(s->timer_reload, freq);
-        ptimer_set_freq(s->timer_cmp, freq);
-    }
-    return freq;
+    uint32_t clksrc = extract32(s->cr, CR_CLKSRC_SHIFT, CR_CLKSRC_BITS);
+    uint32_t prescaler = 1 + extract32(s->cr, CR_PRESCALE_SHIFT, CR_PRESCALE_BITS);
+    uint32_t f_in = imx_ccm_get_clock_frequency(s->ccm, imx_epit_clocks[clksrc]);
+    return f_in / prescaler;
 }
 
 static void imx_epit_reset(DeviceState *dev)
@@ -106,7 +89,12 @@ static void imx_epit_reset(DeviceState *dev)
     ptimer_stop(s->timer_cmp);
     ptimer_stop(s->timer_reload);
     /* compute new frequency */
-    uint32_t freq = imx_epit_set_freq(s);
+    uint32_t freq = imx_epit_get_freq(s);
+    DPRINTF("Setting ptimer frequency to %u\n", freq);
+    if (freq) {
+        ptimer_set_freq(s->timer_reload, freq);
+        ptimer_set_freq(s->timer_cmp, freq);
+    }
     /* init both timers to EPIT_TIMER_MAX */
     ptimer_set_limit(s->timer_cmp, EPIT_TIMER_MAX, 1);
     ptimer_set_limit(s->timer_reload, EPIT_TIMER_MAX, 1);
@@ -155,21 +143,51 @@ static uint64_t imx_epit_read(void *opaque, hwaddr offset, unsigned size)
     return reg_value;
 }
 
-/* Must be called from ptimer_transaction_begin/commit block for s->timer_cmp */
-static void imx_epit_reload_compare_timer(IMXEPITState *s)
+/*
+ * Must be called from a ptimer_transaction_begin/commit block for
+ * s->timer_cmp, but outside of a transaction block of s->timer_reload,
+ * so the proper counter value is read.
+ */
+static void imx_epit_update_compare_timer(IMXEPITState *s)
 {
-    if ((s->cr & (CR_EN | CR_OCIEN)) == (CR_EN | CR_OCIEN))  {
-        /* if the compare feature is on and timers are running */
-        uint32_t tmp = ptimer_get_count(s->timer_reload);
-        uint64_t next;
-        if (tmp > s->cmp) {
-            /* It'll fire in this round of the timer */
-            next = tmp - s->cmp;
-        } else { /* catch it next time around */
-            next = tmp - s->cmp + ((s->cr & CR_RLD) ? EPIT_TIMER_MAX : s->lr);
+    int is_oneshot = 0;
+
+    /*
+     * The compare time will only be active when the EPIT timer is enabled
+     * (CR_EN), the compare interrupt generation is enabled (CR_OCIEN) and
+     *  the input clock if not off.
+     */
+    uint32_t freq = imx_epit_get_freq(s);
+    if (!freq || ((s->cr & (CR_EN | CR_OCIEN)) != (CR_EN | CR_OCIEN))) {
+        ptimer_stop(s->timer_cmp);
+        return;
+    }
+
+    /* calculate the next timeout for the compare timer. */
+    uint64_t tmp = ptimer_get_count(s->timer_reload);
+    uint64_t max = (s->cr & CR_RLD) ? EPIT_TIMER_MAX : s->lr;
+    if (s->cmp <= tmp) {
+        /* fire in this round */
+        tmp -= s->cmp;
+        /* if the reload value is less than the compare value, the timer will
+         * only fire once
+         */
+        is_oneshot = (s->cmp > max);
+    } else {
+        /*
+         * fire after a reload - but only if the reload value is equal
+         * or higher than the compare value.
+         */
+        if (s->cmp > max) {
+            ptimer_stop(s->timer_cmp);
+            return;
         }
-        ptimer_set_count(s->timer_cmp, next);
+        tmp += max - s->cmp;
     }
+
+    /* re-initialize the compare timer and run it */
+    ptimer_set_count(s->timer_cmp, tmp);
+    ptimer_run(s->timer_cmp, is_oneshot);
 }
 
 static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
@@ -193,7 +211,12 @@ static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
     ptimer_transaction_begin(s->timer_reload);
 
     if (!(value & CR_SWR)) {
-        freq = imx_epit_set_freq(s);
+        uint32_t freq = imx_epit_get_freq(s);
+        DPRINTF("Setting ptimer frequency to %u\n", freq);
+        if (freq) {
+            ptimer_set_freq(s->timer_reload, freq);
+            ptimer_set_freq(s->timer_cmp, freq);
+        }
     }
 
     if (freq && (s->cr & CR_EN) && !(oldcr & CR_EN)) {
@@ -203,22 +226,15 @@ static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
             ptimer_set_limit(s->timer_reload, limit, 1);
             ptimer_set_limit(s->timer_cmp, limit, 1);
         }
-
-        imx_epit_reload_compare_timer(s);
         ptimer_run(s->timer_reload, 0);
-        if (s->cr & CR_OCIEN) {
-            ptimer_run(s->timer_cmp, 0);
-        } else {
-            ptimer_stop(s->timer_cmp);
-        }
+        imx_epit_update_compare_timer(s);
     } else if (!(s->cr & CR_EN)) {
         /* stop both timers */
         ptimer_stop(s->timer_reload);
         ptimer_stop(s->timer_cmp);
     } else if (s->cr & CR_OCIEN) {
         if (!(oldcr & CR_OCIEN)) {
-            imx_epit_reload_compare_timer(s);
-            ptimer_run(s->timer_cmp, 0);
+            imx_epit_update_compare_timer(s);
         }
     } else {
         ptimer_stop(s->timer_cmp);
@@ -255,11 +271,11 @@ static void imx_epit_write_lr(IMXEPITState *s, uint32_t value)
     /*
      * Commit the change to s->timer_reload, so it can propagate. Otherwise
      * the timer interrupt may not fire properly. The commit must happen
-     * before calling imx_epit_reload_compare_timer(), which reads
+     * before calling imx_epit_update_compare_timer(), which reads
      * s->timer_reload internally again.
      */
     ptimer_transaction_commit(s->timer_reload);
-    imx_epit_reload_compare_timer(s);
+    imx_epit_update_compare_timer(s);
     ptimer_transaction_commit(s->timer_cmp);
 }
 
@@ -268,7 +284,7 @@ static void imx_epit_write_cmp(IMXEPITState *s, uint32_t value)
     s->cmp = value;
 
     ptimer_transaction_begin(s->timer_cmp);
-    imx_epit_reload_compare_timer(s);
+    imx_epit_update_compare_timer(s);
     ptimer_transaction_commit(s->timer_cmp);
 }
 
-- 
2.34.5

[PATCH qemu.git 11/11] hw/timer/imx_epit: rework CR write handling

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

- simplify code, improve comments
- fix https://gitlab.com/qemu-project/qemu/-/issues/1263

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 hw/timer/imx_epit.c | 139 +++++++++++++++++++++-----------------------
 1 file changed, 65 insertions(+), 74 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 7dd9f54c9c..4a6326b1de 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -69,43 +69,6 @@ static uint32_t imx_epit_get_freq(IMXEPITState *s)
     return f_in / prescaler;
 }
 
-static void imx_epit_reset(DeviceState *dev)
-{
-    IMXEPITState *s = IMX_EPIT(dev);
-
-    /*
-     * Soft reset doesn't touch some bits; hard reset clears them
-     */
-    s->cr &= (CR_EN|CR_ENMOD|CR_STOPEN|CR_DOZEN|CR_WAITEN|CR_DBGEN);
-    s->sr = 0;
-    s->lr = EPIT_TIMER_MAX;
-    s->cmp = 0;
-    /* clear the interrupt */
-    qemu_irq_lower(s->irq);
-
-    ptimer_transaction_begin(s->timer_cmp);
-    ptimer_transaction_begin(s->timer_reload);
-    /* stop both timers */
-    ptimer_stop(s->timer_cmp);
-    ptimer_stop(s->timer_reload);
-    /* compute new frequency */
-    uint32_t freq = imx_epit_get_freq(s);
-    DPRINTF("Setting ptimer frequency to %u\n", freq);
-    if (freq) {
-        ptimer_set_freq(s->timer_reload, freq);
-        ptimer_set_freq(s->timer_cmp, freq);
-    }
-    /* init both timers to EPIT_TIMER_MAX */
-    ptimer_set_limit(s->timer_cmp, EPIT_TIMER_MAX, 1);
-    ptimer_set_limit(s->timer_reload, EPIT_TIMER_MAX, 1);
-    if (freq && (s->cr & CR_EN)) {
-        /* if the timer is still enabled, restart it */
-        ptimer_run(s->timer_reload, 0);
-    }
-    ptimer_transaction_commit(s->timer_cmp);
-    ptimer_transaction_commit(s->timer_reload);
-}
-
 static uint64_t imx_epit_read(void *opaque, hwaddr offset, unsigned size)
 {
     IMXEPITState *s = IMX_EPIT(opaque);
@@ -192,56 +155,75 @@ static void imx_epit_update_compare_timer(IMXEPITState *s)
 
 static void imx_epit_write_cr(IMXEPITState *s, uint32_t value)
 {
-    uint32_t freq = 0;
+    ptimer_transaction_begin(s->timer_cmp);
+    ptimer_transaction_begin(s->timer_reload);
 
-    /* SWR bit is never persisted, it clears itself once reset is done */
     uint32_t oldcr = s->cr;
     s->cr = (value & ~CR_SWR) & 0x03ffffff;
 
     if (value & CR_SWR) {
-        /* handle the reset */
-        imx_epit_reset(DEVICE(s));
         /*
-         * TODO: could we 'break' here? following operations appear
-         * to duplicate the work imx_epit_reset() already did.
+         * Soft reset doesn't touch some bits, just a hard reset clears all
+         * of them. Clearing CLKSRC disables the input clock, which will
+         * happen when we re-init of the timer frequency below.
+         */
+        s->cr &= (CR_EN|CR_ENMOD|CR_STOPEN|CR_DOZEN|CR_WAITEN|CR_DBGEN);
+        /*
+         * We have applied the new CR value and then cleared most bits,
+         * thus some bits from the write request are now lost. The TRM
+         * is not clear about the behavior, maybe these bits are to be
+         * applied after the reset (e.g. for selecting a new clock
+         * source). However, it seem this is undefined behavior and a
+         * it's assumed a reset does not try to do anything else.
          */
+        s->sr = 0;
+        s->lr = EPIT_TIMER_MAX;
+        s->cmp = 0;
+        /* turn interrupt off since SR and the OCIEN bit is cleared */
+        qemu_irq_lower(s->irq);
+        /* reset timer limits, set timer values to the limits */
+        ptimer_set_limit(s->timer_cmp, EPIT_TIMER_MAX, 1);
+        ptimer_set_limit(s->timer_reload, EPIT_TIMER_MAX, 1);
     }
 
-    ptimer_transaction_begin(s->timer_cmp);
-    ptimer_transaction_begin(s->timer_reload);
-
-    if (!(value & CR_SWR)) {
-        uint32_t freq = imx_epit_get_freq(s);
+    /* re-initialize frequency, or turn off timers if input clock is off */
+    uint32_t freq = imx_epit_get_freq(s);
+    if (freq) {
         DPRINTF("Setting ptimer frequency to %u\n", freq);
-        if (freq) {
-            ptimer_set_freq(s->timer_reload, freq);
-            ptimer_set_freq(s->timer_cmp, freq);
-        }
+        ptimer_set_freq(s->timer_reload, freq);
+        ptimer_set_freq(s->timer_cmp, freq);
     }
 
-    if (freq && (s->cr & CR_EN) && !(oldcr & CR_EN)) {
-        if (s->cr & CR_ENMOD) {
-            uint64_t limit = (s->cr & CR_RLD) ? s->lr : EPIT_TIMER_MAX;
-            /* set new limit and also set timer to this value right now */
-            ptimer_set_limit(s->timer_reload, limit, 1);
-            ptimer_set_limit(s->timer_cmp, limit, 1);
-        }
-        ptimer_run(s->timer_reload, 0);
-        imx_epit_update_compare_timer(s);
-    } else if (!(s->cr & CR_EN)) {
-        /* stop both timers */
-        ptimer_stop(s->timer_reload);
+    if (!freq || !(s->cr & CR_EN)) {
+        /*
+         * The EPIT timer is effectively disabled if it is not enabled or
+         * the input clock is off. In this case we can stop the ptimers.
+         */
         ptimer_stop(s->timer_cmp);
-    } else if (s->cr & CR_OCIEN) {
-        if (!(oldcr & CR_OCIEN)) {
-            imx_epit_update_compare_timer(s);
-        }
+        ptimer_stop(s->timer_reload);
     } else {
-        ptimer_stop(s->timer_cmp);
+        /* The EPIT timer is active. */
+        if (!(oldcr & CR_EN)) {
+            /* The EPI timer has just been enabled, initialize and start it. */
+            if (s->cr & CR_ENMOD) {
+                uint64_t limit = (s->cr & CR_RLD) ? s->lr : EPIT_TIMER_MAX;
+                /* set new limit and also set timer to this value right now */
+                ptimer_set_limit(s->timer_reload, limit, 1);
+                ptimer_set_limit(s->timer_cmp, limit, 1);
+            }
+            ptimer_run(s->timer_reload, 0);
+        }
     }
+    /*
+     * Commit the change to s->timer_reload, so it can propagate and the
+     * updated value will be read in imx_epit_update_compare_timer(),
+     * Otherwise a stale value will be seen and the compare interrupt is
+     * set up wrongly.
+     */
+    ptimer_transaction_commit(s->timer_reload);
+    imx_epit_update_compare_timer(s);
 
     ptimer_transaction_commit(s->timer_cmp);
-    ptimer_transaction_commit(s->timer_reload);
 }
 
 static void imx_epit_write_sr(IMXEPITState *s, uint32_t value)
@@ -269,10 +251,10 @@ static void imx_epit_write_lr(IMXEPITState *s, uint32_t value)
         ptimer_set_count(s->timer_reload, s->lr);
     }
     /*
-     * Commit the change to s->timer_reload, so it can propagate. Otherwise
-     * the timer interrupt may not fire properly. The commit must happen
-     * before calling imx_epit_update_compare_timer(), which reads
-     * s->timer_reload internally again.
+     * Commit the change to s->timer_reload, so it can propagate and the
+     * updated value will be read in imx_epit_update_compare_timer(),
+     * Otherwise a stale value will be seen and the compare interrupt is
+     * set up wrongly.
      */
     ptimer_transaction_commit(s->timer_reload);
     imx_epit_update_compare_timer(s);
@@ -390,6 +372,15 @@ static void imx_epit_realize(DeviceState *dev, Error **errp)
     s->timer_cmp = ptimer_init(imx_epit_cmp, s, PTIMER_POLICY_LEGACY);
 }
 
+static void imx_epit_reset(DeviceState *dev)
+{
+    IMXEPITState *s = IMX_EPIT(dev);
+
+    /* initialize CR and perform a software reset */
+    s->cr = 0;
+    imx_epit_write_cr(s, CR_SWR);
+}
+
 static void imx_epit_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc  = DEVICE_CLASS(klass);
-- 
2.34.5

[PATCH qemu.git 00/11] improve hw/timer/imx_epit

[~axelheider]

This is a follow-up on my initial work to fix https://gitlab.com/qemu-
project/qemu/-/issues/1263.
- fix #1263 for CR writes
- fix compare timer update
- software reset clears the interrupt
- do not persist CR.SWR bit
- general code cleanup and comment improvements

Axel Heider (11):
  hw/timer/imx_epit: fix typo in comment
  hw/timer/imx_epit: improve comments
  hw/timer/imx_epit: factor out register write handlers
  hw/timer/imx_epit: remove explicit fields cnt and freq
  hw/timer/imx_epit: simplify interrupt logic
  hw/timer/imx_epit: software reset clears the interrupt
  hw/timer/imx_epit: do not persist CR.SWR bit
  hw/timer/imx_epit: simplify CR.ENMOD handling
  hw/timer/imx_epit: cleanup CR defines
  hw/timer/imx_epit: fix compare timer update
  hw/timer/imx_epit: rework CR write handling

 hw/timer/imx_epit.c         | 372 +++++++++++++++++++-----------------
 include/hw/timer/imx_epit.h |   6 +-
 2 files changed, 200 insertions(+), 178 deletions(-)

-- 
2.34.5

Re: [PATCH qemu.git 04/11] hw/timer/imx_epit: remove explicit fields cnt and freq

[Philippe Mathieu-Daudé]

On 25/10/22 12:33, ~axelheider wrote:
> From: Axel Heider <axel.heider@hensoldt.net>
> 
> The CNT register is a read-only register. There is no need to
> store it's value, it can be calculated on demand.
> The calculated frequency is needed temporarily only.
> 
> Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
> ---
>   hw/timer/imx_epit.c         | 42 +++++++++++++++----------------------
>   include/hw/timer/imx_epit.h |  2 --
>   2 files changed, 17 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
> index a79f58c963..37b04a1b53 100644
> --- a/hw/timer/imx_epit.c
> +++ b/hw/timer/imx_epit.c
> @@ -77,23 +77,25 @@ static void imx_epit_update_int(IMXEPITState *s)
>    * Must be called from within a ptimer_transaction_begin/commit block
>    * for both s->timer_cmp and s->timer_reload.
>    */
> -static void imx_epit_set_freq(IMXEPITState *s)
> +static uint32_t imx_epit_set_freq(IMXEPITState *s)

Maybe rename as imx_epit_get_freq() or simply imx_epit_freq(),
otherwise:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH qemu.git 02/11] hw/timer/imx_epit: improve comments

[Philippe Mathieu-Daudé]

On 25/10/22 17:33, ~axelheider wrote:
> From: Axel Heider <axel.heider@hensoldt.net>
> 
> Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
> ---
>   hw/timer/imx_epit.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH qemu.git 06/11] hw/timer/imx_epit: software reset clears the interrupt

[Philippe Mathieu-Daudé]

On 25/10/22 20:32, ~axelheider wrote:
> From: Axel Heider <axel.heider@hensoldt.net>
> 
> ---
>   hw/timer/imx_epit.c | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH qemu.git 08/11] hw/timer/imx_epit: simplify CR.ENMOD handling

[Philippe Mathieu-Daudé]

On 29/10/22 18:41, ~axelheider wrote:
> From: Axel Heider <axel.heider@hensoldt.net>
> 
> Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
> ---
>   hw/timer/imx_epit.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH qemu.git 09/11] hw/timer/imx_epit: cleanup CR defines

[Philippe Mathieu-Daudé]

On 31/10/22 00:59, ~axelheider wrote:
> From: Axel Heider <axel.heider@hensoldt.net>
> 
> remove unused defines, add needed defines
> 
> Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
> ---
>   hw/timer/imx_epit.c         | 4 ++--
>   include/hw/timer/imx_epit.h | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH qemu.git 11/11] hw/timer/imx_epit: rework CR write handling

[Philippe Mathieu-Daudé]

On 31/10/22 01:28, ~axelheider wrote:
> From: Axel Heider <axel.heider@hensoldt.net>
> 
> - simplify code, improve comments
> - fix https://gitlab.com/qemu-project/qemu/-/issues/1263

This doesn't match GitLab issues closing pattern:
https://docs.gitlab.com/ee/user/project/issues/managing_issues.html#default-closing-pattern

> 
> Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
> ---
>   hw/timer/imx_epit.c | 139 +++++++++++++++++++++-----------------------
>   1 file changed, 65 insertions(+), 74 deletions(-)

Re: [PATCH qemu.git 04/11] hw/timer/imx_epit: remove explicit fields cnt and freq

[Axel Heider]

-------- Original Message --------
> From: Philippe Mathieu-Daudé [mailto:philmd@linaro.org]> diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c>
> index a79f58c963..37b04a1b53 100644
> --- a/hw/timer/imx_epit.c
> +++ b/hw/timer/imx_epit.c
> @@ -77,23 +77,25 @@ static void imx_epit_update_int(IMXEPITState *s)
>    * Must be called from within a ptimer_transaction_begin/commit block
>    * for both s->timer_cmp and s->timer_reload.
>    */
> -static void imx_epit_set_freq(IMXEPITState *s)
> +static uint32_t imx_epit_set_freq(IMXEPITState *s)
>
> Maybe rename as imx_epit_get_freq() or simply imx_epit_freq(),
>

There will be an update of the whole patchset, so I will change
the name to imx_epit_freq(). That makes the name and signature in
sync. Note that the next commit in this patchset does more
refactoring anyway, so this is just a intermediate change that
is not really visible.


Axel

Re: [PATCH qemu.git 11/11] hw/timer/imx_epit: rework CR write handling

[Axel Heider]

-------- Original Message --------
> From: Philippe Mathieu-Daudé [mailto:philmd@linaro.org]
>
>> - simplify code, improve comments
>> - fix https://gitlab.com/qemu-project/qemu/-/issues/1263
>
> This doesn't match GitLab issues closing pattern:
> https://docs.gitlab.com/ee/user/project/issues/managing_issues.html#default-closing-pattern

I will change this to use "fix #1263" then. But the issue git closed already
from an earlier patch that got merged a few days ago.

Axel