Hello!

Problem: function imx_gpt_reset is used for soft (requested by guest) 
and hard resets. But soft and hard resets should have different 
behaviour (hard reset should clear all registers, while soft reset 
should preserve some bits).

Patch changelog:
v1 -> v2: use different approach, patch completely rewritten
v2 -> v3: in software reset add preserving of CLKSRC bit as manual says

---
Software reset function clears CR bits that should not be cleared and
preserve bits that should be cleared.

Signed-off-by: Kurban Mallachiev <mallachiev@ispras.ru>
---
  hw/timer/imx_gpt.c | 27 +++++++++++++++++++++------
  1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c
index 010ccbf207..3ea18ff5de 100644
--- a/hw/timer/imx_gpt.c
+++ b/hw/timer/imx_gpt.c
@@ -296,18 +296,21 @@ static uint64_t imx_gpt_read(void *opaque, hwaddr 
offset, unsigned size)
      return reg_value;
  }

-static void imx_gpt_reset(DeviceState *dev)
+static void imx_gpt_reset_common(IMXGPTState *s, int is_soft_reset)
  {
-    IMXGPTState *s = IMX_GPT(dev);
-
      /* stop timer */
      ptimer_stop(s->timer);

      /*
       * Soft reset doesn't touch some bits; hard reset clears them
       */
-    s->cr &= ~(GPT_CR_EN|GPT_CR_ENMOD|GPT_CR_STOPEN|GPT_CR_DOZEN|
-               GPT_CR_WAITEN|GPT_CR_DBGEN);
+    if (is_soft_reset) {
+        s->cr &= GPT_CR_EN|GPT_CR_ENMOD|GPT_CR_STOPEN|GPT_CR_DOZEN|
+                   GPT_CR_WAITEN|GPT_CR_DBGEN|
+                   GPT_CR_CLKSRC_MASK<<GPT_CR_CLKSRC_SHIFT;
+    } else {
+        s->cr = 0;
+    }
      s->sr = 0;
      s->pr = 0;
      s->ir = 0;
@@ -333,6 +336,18 @@ static void imx_gpt_reset(DeviceState *dev)
      }
  }

+static void imx_gpt_soft_reset(DeviceState *dev)
+{
+    IMXGPTState *s = IMX_GPT(dev);
+    imx_gpt_reset_common(s, 1);
+}
+
+static void imx_gpt_reset(DeviceState *dev)
+{
+    IMXGPTState *s = IMX_GPT(dev);
+    imx_gpt_reset_common(s, 0);
+}
+
  static void imx_gpt_write(void *opaque, hwaddr offset, uint64_t value,
                            unsigned size)
  {
@@ -348,7 +363,7 @@ static void imx_gpt_write(void *opaque, hwaddr 
offset, uint64_t value,
          s->cr = value & ~0x7c14;
          if (s->cr & GPT_CR_SWR) { /* force reset */
              /* handle the reset */
-            imx_gpt_reset(DEVICE(s));
+            imx_gpt_soft_reset(DEVICE(s));
          } else {
              /* set our freq, as the source might have changed */
              imx_gpt_set_freq(s);
-- 
2.11.1

On 19 February 2017 at 19:00, Kurban Mallachiev <mallachiev@ispras.ru> wrote:
> Hello!
>
> Problem: function imx_gpt_reset is used for soft (requested by guest) and
> hard resets. But soft and hard resets should have different behaviour (hard
> reset should clear all registers, while soft reset should preserve some
> bits).
>
> Patch changelog:
> v1 -> v2: use different approach, patch completely rewritten
> v2 -> v3: in software reset add preserving of CLKSRC bit as manual says
>
> ---
> Software reset function clears CR bits that should not be cleared and
> preserve bits that should be cleared.
>
> Signed-off-by: Kurban Mallachiev <mallachiev@ispras.ru>

Thanks for this patch -- I have added it to my target-arm queue
(with a few tweaks).

Unfortunately your mail client mangled the patch a bit, and I had
to apply it manually. I'm happy to do that for occasional contributions,
but if you're planning to submit a lot of patches in the near future
you might like to look at your patch submission workflow (we usually
recommend 'git send-email').

thanks
-- PMM