diff mbox series

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

Message ID 166718254546.5893.5075929684621857903-4@git.sr.ht
State New
Headers show
Series improve hw/timer/imx_epit | expand

Commit Message

~axelheider Oct. 25, 2022, 10:33 a.m. UTC 
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(-)

Comments

Philippe Mathieu-Daudé Oct. 31, 2022, 8:19 a.m. UTC | #1 
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>
Axel Heider Nov. 1, 2022, 9:15 p.m. UTC | #2 
-------- 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
diff mbox series

Patch

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;
 };