ptimer: Rename PTIMER_POLICY_DEFAULT to PTIMER_POLICY_LEGACY
authorPeter Maydell <peter.maydell@linaro.org>
Mon, 16 May 2022 10:30:58 +0000 (11:30 +0100)
committerPeter Maydell <peter.maydell@linaro.org>
Thu, 19 May 2022 15:19:03 +0000 (16:19 +0100)
The traditional ptimer behaviour includes a collection of weird edge
case behaviours.  In 2016 we improved the ptimer implementation to
fix these and generally make the behaviour more flexible, with
ptimers opting in to the new behaviour by passing an appropriate set
of policy flags to ptimer_init().  For backwards-compatibility, we
defined PTIMER_POLICY_DEFAULT (which sets no flags) to give the old
weird behaviour.

This turns out to be a poor choice of name, because people writing
new devices which use ptimers are misled into thinking that the
default is probably a sensible choice of flags, when in fact it is
almost always not what you want.  Rename PTIMER_POLICY_DEFAULT to
PTIMER_POLICY_LEGACY and beef up the comment to more clearly say that
new devices should not be using it.

The code-change part of this commit was produced by
  sed -i -e 's/PTIMER_POLICY_DEFAULT/PTIMER_POLICY_LEGACY/g' $(git grep -l PTIMER_POLICY_DEFAULT)
with the exception of a test name string change in
tests/unit/ptimer-test.c which was added manually.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220516103058.162280-1-peter.maydell@linaro.org

25 files changed:
hw/arm/musicpal.c
hw/dma/xilinx_axidma.c
hw/dma/xlnx_csu_dma.c
hw/m68k/mcf5206.c
hw/m68k/mcf5208.c
hw/net/can/xlnx-zynqmp-can.c
hw/net/fsl_etsec/etsec.c
hw/net/lan9118.c
hw/rtc/exynos4210_rtc.c
hw/timer/allwinner-a10-pit.c
hw/timer/altera_timer.c
hw/timer/arm_timer.c
hw/timer/digic-timer.c
hw/timer/etraxfs_timer.c
hw/timer/exynos4210_mct.c
hw/timer/exynos4210_pwm.c
hw/timer/grlib_gptimer.c
hw/timer/imx_epit.c
hw/timer/imx_gpt.c
hw/timer/mss-timer.c
hw/timer/sh_timer.c
hw/timer/slavio_timer.c
hw/timer/xilinx_timer.c
include/hw/ptimer.h
tests/unit/ptimer-test.c

index 7c840fb4283ecaf0ad768e933d623a06a88d0a93..b65c020115a63e2ed6806fa6ad120ea2e14754b0 100644 (file)
@@ -464,7 +464,7 @@ static void mv88w8618_timer_init(SysBusDevice *dev, mv88w8618_timer_state *s,
     sysbus_init_irq(dev, &s->irq);
     s->freq = freq;
 
-    s->ptimer = ptimer_init(mv88w8618_timer_tick, s, PTIMER_POLICY_DEFAULT);
+    s->ptimer = ptimer_init(mv88w8618_timer_tick, s, PTIMER_POLICY_LEGACY);
 }
 
 static uint64_t mv88w8618_pit_read(void *opaque, hwaddr offset,
index bc383f53cca3294a9c3088225ec44650f913f5d7..cbb8f0f16963993b057544c32d699be0ac18aae8 100644 (file)
@@ -552,7 +552,7 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
 
         st->dma = s;
         st->nr = i;
-        st->ptimer = ptimer_init(timer_hit, st, PTIMER_POLICY_DEFAULT);
+        st->ptimer = ptimer_init(timer_hit, st, PTIMER_POLICY_LEGACY);
         ptimer_transaction_begin(st->ptimer);
         ptimer_set_freq(st->ptimer, s->freqhz);
         ptimer_transaction_commit(st->ptimer);
index 60ada3286b43b7b5af55f386ed18ef584ee62cac..1ce52ea5a2ba5109b571a2fb4685674808f8345e 100644 (file)
@@ -666,7 +666,7 @@ static void xlnx_csu_dma_realize(DeviceState *dev, Error **errp)
     sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
 
     s->src_timer = ptimer_init(xlnx_csu_dma_src_timeout_hit,
-                               s, PTIMER_POLICY_DEFAULT);
+                               s, PTIMER_POLICY_LEGACY);
 
     s->attr = MEMTXATTRS_UNSPECIFIED;
 
index 6d93d761a5e2ba056a2f83199af43ae0a805f4ee..2ab1b4f0593b8252fcb7ca1c3a3f9b7c9378809e 100644 (file)
@@ -152,7 +152,7 @@ static m5206_timer_state *m5206_timer_init(qemu_irq irq)
     m5206_timer_state *s;
 
     s = g_new0(m5206_timer_state, 1);
-    s->timer = ptimer_init(m5206_timer_trigger, s, PTIMER_POLICY_DEFAULT);
+    s->timer = ptimer_init(m5206_timer_trigger, s, PTIMER_POLICY_LEGACY);
     s->irq = irq;
     m5206_timer_reset(s);
     return s;
index 655207e393dc558c777712c3b9a0ad95f0105f12..be1033f84f87d86acaf6e409cd5cafa2c85cbe99 100644 (file)
@@ -197,7 +197,7 @@ static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic)
     /* Timers.  */
     for (i = 0; i < 2; i++) {
         s = g_new0(m5208_timer_state, 1);
-        s->timer = ptimer_init(m5208_timer_trigger, s, PTIMER_POLICY_DEFAULT);
+        s->timer = ptimer_init(m5208_timer_trigger, s, PTIMER_POLICY_LEGACY);
         memory_region_init_io(&s->iomem, NULL, &m5208_timer_ops, s,
                               "m5208-timer", 0x00004000);
         memory_region_add_subregion(address_space, 0xfc080000 + 0x4000 * i,
index 22bb8910fa8c1ffaf5a44631b835938eff83d775..82ac48cee2406076ace331ea9eb54f190d999240 100644 (file)
@@ -1079,7 +1079,7 @@ static void xlnx_zynqmp_can_realize(DeviceState *dev, Error **errp)
 
     /* Allocate a new timer. */
     s->can_timer = ptimer_init(xlnx_zynqmp_can_ptimer_cb, s,
-                               PTIMER_POLICY_DEFAULT);
+                               PTIMER_POLICY_LEGACY);
 
     ptimer_transaction_begin(s->can_timer);
 
index 6d50c395439a9592c660dd89458e063a3344e385..4e6cc708dec0b24afc5efb81499ce7cfce84f99c 100644 (file)
@@ -393,7 +393,7 @@ static void etsec_realize(DeviceState *dev, Error **errp)
                               object_get_typename(OBJECT(dev)), dev->id, etsec);
     qemu_format_nic_info_str(qemu_get_queue(etsec->nic), etsec->conf.macaddr.a);
 
-    etsec->ptimer = ptimer_init(etsec_timer_hit, etsec, PTIMER_POLICY_DEFAULT);
+    etsec->ptimer = ptimer_init(etsec_timer_hit, etsec, PTIMER_POLICY_LEGACY);
     ptimer_transaction_begin(etsec->ptimer);
     ptimer_set_freq(etsec->ptimer, 100);
     ptimer_transaction_commit(etsec->ptimer);
index 6aff424cbe54d33c4a0ee35a95c4ca005454d360..456ae38107b9504b8e1e39463442c7f1d7cf8685 100644 (file)
@@ -1363,7 +1363,7 @@ static void lan9118_realize(DeviceState *dev, Error **errp)
     s->pmt_ctrl = 1;
     s->txp = &s->tx_packet;
 
-    s->timer = ptimer_init(lan9118_tick, s, PTIMER_POLICY_DEFAULT);
+    s->timer = ptimer_init(lan9118_tick, s, PTIMER_POLICY_LEGACY);
     ptimer_transaction_begin(s->timer);
     ptimer_set_freq(s->timer, 10000);
     ptimer_set_limit(s->timer, 0xffff, 1);
index ae67641de66832bf407b3b4c49a2b7a76369b7fd..d1620c7a2ace17afd1cae57f7d8bf18ffe2cf549 100644 (file)
@@ -564,14 +564,14 @@ static void exynos4210_rtc_init(Object *obj)
     Exynos4210RTCState *s = EXYNOS4210_RTC(obj);
     SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
-    s->ptimer = ptimer_init(exynos4210_rtc_tick, s, PTIMER_POLICY_DEFAULT);
+    s->ptimer = ptimer_init(exynos4210_rtc_tick, s, PTIMER_POLICY_LEGACY);
     ptimer_transaction_begin(s->ptimer);
     ptimer_set_freq(s->ptimer, RTC_BASE_FREQ);
     exynos4210_rtc_update_freq(s, 0);
     ptimer_transaction_commit(s->ptimer);
 
     s->ptimer_1Hz = ptimer_init(exynos4210_rtc_1Hz_tick,
-                                s, PTIMER_POLICY_DEFAULT);
+                                s, PTIMER_POLICY_LEGACY);
     ptimer_transaction_begin(s->ptimer_1Hz);
     ptimer_set_freq(s->ptimer_1Hz, RTC_BASE_FREQ);
     ptimer_transaction_commit(s->ptimer_1Hz);
index c3fc2a4daaf51cc1a3c14ef1c9834594c041735f..971f78462ab40b5d631f62c63e5066b15980dbdc 100644 (file)
@@ -275,7 +275,7 @@ static void a10_pit_init(Object *obj)
 
         tc->container = s;
         tc->index = i;
-        s->timer[i] = ptimer_init(a10_pit_timer_cb, tc, PTIMER_POLICY_DEFAULT);
+        s->timer[i] = ptimer_init(a10_pit_timer_cb, tc, PTIMER_POLICY_LEGACY);
     }
 }
 
index c6e02d2b5a7c8b5fe8efb88d5e359b33bf189337..0f1f54206a3634fef5f904769041524ad9e0eed6 100644 (file)
@@ -185,7 +185,7 @@ static void altera_timer_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    t->ptimer = ptimer_init(timer_hit, t, PTIMER_POLICY_DEFAULT);
+    t->ptimer = ptimer_init(timer_hit, t, PTIMER_POLICY_LEGACY);
     ptimer_transaction_begin(t->ptimer);
     ptimer_set_freq(t->ptimer, t->freq_hz);
     ptimer_transaction_commit(t->ptimer);
index 84cf2726bb3913165b059b5b087179626e30f2a9..69c886347222ddcf80806becfbe908e8393115f0 100644 (file)
@@ -180,7 +180,7 @@ static arm_timer_state *arm_timer_init(uint32_t freq)
     s->freq = freq;
     s->control = TIMER_CTRL_IE;
 
-    s->timer = ptimer_init(arm_timer_tick, s, PTIMER_POLICY_DEFAULT);
+    s->timer = ptimer_init(arm_timer_tick, s, PTIMER_POLICY_LEGACY);
     vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, &vmstate_arm_timer, s);
     return s;
 }
index e3aae4a45a48b39a84c228bacfc5a730bddce57c..d5186f445494973b88754d63b92f7a7efba2d3c4 100644 (file)
@@ -139,7 +139,7 @@ static void digic_timer_init(Object *obj)
 {
     DigicTimerState *s = DIGIC_TIMER(obj);
 
-    s->ptimer = ptimer_init(digic_timer_tick, NULL, PTIMER_POLICY_DEFAULT);
+    s->ptimer = ptimer_init(digic_timer_tick, NULL, PTIMER_POLICY_LEGACY);
 
     /*
      * FIXME: there is no documentation on Digic timer
index 139e5b86a4471bced7348baab4aa18c574715148..ecc2831bafb0e1875fa55015d9d57bf06aca3426 100644 (file)
@@ -370,9 +370,9 @@ static void etraxfs_timer_realize(DeviceState *dev, Error **errp)
     ETRAXTimerState *t = ETRAX_TIMER(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
-    t->ptimer_t0 = ptimer_init(timer0_hit, t, PTIMER_POLICY_DEFAULT);
-    t->ptimer_t1 = ptimer_init(timer1_hit, t, PTIMER_POLICY_DEFAULT);
-    t->ptimer_wd = ptimer_init(watchdog_hit, t, PTIMER_POLICY_DEFAULT);
+    t->ptimer_t0 = ptimer_init(timer0_hit, t, PTIMER_POLICY_LEGACY);
+    t->ptimer_t1 = ptimer_init(timer1_hit, t, PTIMER_POLICY_LEGACY);
+    t->ptimer_wd = ptimer_init(watchdog_hit, t, PTIMER_POLICY_LEGACY);
 
     sysbus_init_irq(sbd, &t->irq);
     sysbus_init_irq(sbd, &t->nmi);
index d0e53439968025bb9e9e65577cbef07e6f7ea4ff..e175a9f5b9b31c2f6d0369831a193410cfae80a4 100644 (file)
@@ -1503,17 +1503,17 @@ static void exynos4210_mct_init(Object *obj)
 
     /* Global timer */
     s->g_timer.ptimer_frc = ptimer_init(exynos4210_gfrc_event, s,
-                                        PTIMER_POLICY_DEFAULT);
+                                        PTIMER_POLICY_LEGACY);
     memset(&s->g_timer.reg, 0, sizeof(struct gregs));
 
     /* Local timers */
     for (i = 0; i < 2; i++) {
         s->l_timer[i].tick_timer.ptimer_tick =
             ptimer_init(exynos4210_ltick_event, &s->l_timer[i],
-                        PTIMER_POLICY_DEFAULT);
+                        PTIMER_POLICY_LEGACY);
         s->l_timer[i].ptimer_frc =
             ptimer_init(exynos4210_lfrc_event, &s->l_timer[i],
-                        PTIMER_POLICY_DEFAULT);
+                        PTIMER_POLICY_LEGACY);
         s->l_timer[i].id = i;
     }
 
index 220088120eef3b922082736009023ac1ca92d6e2..02924a9e5b37bec43a7daf5dc753caa0fa0865a0 100644 (file)
@@ -400,7 +400,7 @@ static void exynos4210_pwm_init(Object *obj)
         sysbus_init_irq(dev, &s->timer[i].irq);
         s->timer[i].ptimer = ptimer_init(exynos4210_pwm_tick,
                                          &s->timer[i],
-                                         PTIMER_POLICY_DEFAULT);
+                                         PTIMER_POLICY_LEGACY);
         s->timer[i].id = i;
         s->timer[i].parent = s;
     }
index d51189010975091f064091789c4f8e792a7411f5..5c4923c1e09c1dfaa24a8e3909dcfc99903edde9 100644 (file)
@@ -383,7 +383,7 @@ static void grlib_gptimer_realize(DeviceState *dev, Error **errp)
 
         timer->unit   = unit;
         timer->ptimer = ptimer_init(grlib_gptimer_hit, timer,
-                                    PTIMER_POLICY_DEFAULT);
+                                    PTIMER_POLICY_LEGACY);
         timer->id     = i;
 
         /* One IRQ line for each timer */
index ebd58254d15fd9d8ca37bbc50592717c2a8b4a1f..2bf8c754b21d0e4484454ad2dc62f4b2a2adb053 100644 (file)
@@ -347,9 +347,9 @@ static void imx_epit_realize(DeviceState *dev, Error **errp)
                           0x00001000);
     sysbus_init_mmio(sbd, &s->iomem);
 
-    s->timer_reload = ptimer_init(imx_epit_reload, s, PTIMER_POLICY_DEFAULT);
+    s->timer_reload = ptimer_init(imx_epit_reload, s, PTIMER_POLICY_LEGACY);
 
-    s->timer_cmp = ptimer_init(imx_epit_cmp, s, PTIMER_POLICY_DEFAULT);
+    s->timer_cmp = ptimer_init(imx_epit_cmp, s, PTIMER_POLICY_LEGACY);
 }
 
 static void imx_epit_class_init(ObjectClass *klass, void *data)
index 5c0d9a269ceb044bfd7c348876e31c945173ad84..80b830263991638cb59b0e961bf71f68017adb46 100644 (file)
@@ -505,7 +505,7 @@ static void imx_gpt_realize(DeviceState *dev, Error **errp)
                           0x00001000);
     sysbus_init_mmio(sbd, &s->iomem);
 
-    s->timer = ptimer_init(imx_gpt_timeout, s, PTIMER_POLICY_DEFAULT);
+    s->timer = ptimer_init(imx_gpt_timeout, s, PTIMER_POLICY_LEGACY);
 }
 
 static void imx_gpt_class_init(ObjectClass *klass, void *data)
index fe0ca905f3ccbdbc967d8d23ef5246c159610af7..ee7438f1684fb2828fdcf0fb5e112fc2ec6d87ec 100644 (file)
@@ -232,7 +232,7 @@ static void mss_timer_init(Object *obj)
     for (i = 0; i < NUM_TIMERS; i++) {
         struct Msf2Timer *st = &t->timers[i];
 
-        st->ptimer = ptimer_init(timer_hit, st, PTIMER_POLICY_DEFAULT);
+        st->ptimer = ptimer_init(timer_hit, st, PTIMER_POLICY_LEGACY);
         ptimer_transaction_begin(st->ptimer);
         ptimer_set_freq(st->ptimer, t->freq_hz);
         ptimer_transaction_commit(st->ptimer);
index c72c327bfafa5ce952dc3d0ee32cb8e10b48e57d..77889397669a9af3e4c89f07d5061298158097b4 100644 (file)
@@ -239,7 +239,7 @@ static void *sh_timer_init(uint32_t freq, int feat, qemu_irq irq)
     s->enabled = 0;
     s->irq = irq;
 
-    s->timer = ptimer_init(sh_timer_tick, s, PTIMER_POLICY_DEFAULT);
+    s->timer = ptimer_init(sh_timer_tick, s, PTIMER_POLICY_LEGACY);
 
     sh_timer_write(s, OFFSET_TCOR >> 2, s->tcor);
     sh_timer_write(s, OFFSET_TCNT >> 2, s->tcnt);
index 90fdce4c442b312876e435afea2a1c846509e569..8c4f6eb06b6c40ddb056171939cea34cf1c75a5a 100644 (file)
@@ -405,7 +405,7 @@ static void slavio_timer_init(Object *obj)
         tc->timer_index = i;
 
         s->cputimer[i].timer = ptimer_init(slavio_timer_irq, tc,
-                                           PTIMER_POLICY_DEFAULT);
+                                           PTIMER_POLICY_LEGACY);
         ptimer_transaction_begin(s->cputimer[i].timer);
         ptimer_set_period(s->cputimer[i].timer, TIMER_PERIOD);
         ptimer_transaction_commit(s->cputimer[i].timer);
index 1eb927eb84730a43022406f2d78bf90d90065c97..c7f17cd646098a6f457e530912ee16d062a4186f 100644 (file)
@@ -223,7 +223,7 @@ static void xilinx_timer_realize(DeviceState *dev, Error **errp)
 
         xt->parent = t;
         xt->nr = i;
-        xt->ptimer = ptimer_init(timer_hit, xt, PTIMER_POLICY_DEFAULT);
+        xt->ptimer = ptimer_init(timer_hit, xt, PTIMER_POLICY_LEGACY);
         ptimer_transaction_begin(xt->ptimer);
         ptimer_set_freq(xt->ptimer, t->freq_hz);
         ptimer_transaction_commit(xt->ptimer);
index c443218475bbe53bbf7537ce5967d10820618adc..4dc02b0de472ed2a14a8e8fa03cb0ec69f1cf16d 100644 (file)
  * to stderr when the guest attempts to enable the timer.
  */
 
-/* The default ptimer policy retains backward compatibility with the legacy
- * timers. Custom policies are adjusting the default one. Consider providing
- * a correct policy for your timer.
+/*
+ * The 'legacy' ptimer policy retains backward compatibility with the
+ * traditional ptimer behaviour from before policy flags were introduced.
+ * It has several weird behaviours which don't match typical hardware
+ * timer behaviour. For a new device using ptimers, you should not
+ * use PTIMER_POLICY_LEGACY, but instead check the actual behaviour
+ * that you need and specify the right set of policy flags to get that.
+ *
+ * If you are overhauling an existing device that uses PTIMER_POLICY_LEGACY
+ * and are in a position to check or test the real hardware behaviour,
+ * consider updating it to specify the right policy flags.
  *
  * The rough edges of the default policy:
  *  - Starting to run with a period = 0 emits error message and stops the
@@ -54,7 +62,7 @@
  *    since the last period, effectively restarting the timer with a
  *    counter = counter value at the moment of change (.i.e. one less).
  */
-#define PTIMER_POLICY_DEFAULT               0
+#define PTIMER_POLICY_LEGACY                0
 
 /* Periodic timer counter stays with "0" for a one period before wrapping
  * around.  */
index 9176b96c1ce219a510626fa1689708e7d3f7731a..a80ef5aff4c188f45906a4828d2d54be9a1d619e 100644 (file)
@@ -768,8 +768,8 @@ static void add_ptimer_tests(uint8_t policy)
     char policy_name[256] = "";
     char *tmp;
 
-    if (policy == PTIMER_POLICY_DEFAULT) {
-        g_sprintf(policy_name, "default");
+    if (policy == PTIMER_POLICY_LEGACY) {
+        g_sprintf(policy_name, "legacy");
     }
 
     if (policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD) {
@@ -862,7 +862,7 @@ static void add_ptimer_tests(uint8_t policy)
 static void add_all_ptimer_policies_comb_tests(void)
 {
     int last_policy = PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT;
-    int policy = PTIMER_POLICY_DEFAULT;
+    int policy = PTIMER_POLICY_LEGACY;
 
     for (; policy < (last_policy << 1); policy++) {
         if ((policy & PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT) &&