qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/12] qemu-sparc queue 20210908
@ 2021-09-08 11:54 Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 01/12] target/sparc: Drop use of gen_io_end() Mark Cave-Ayland
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

The following changes since commit abf7aee72ea66944a62962603e4c2381f5e473e7:

  Merge remote-tracking branch 'remotes/thuth-gitlab/tags/s390x-pull-request-2021-09-07' into staging (2021-09-07 17:46:13 +0100)

are available in the Git repository at:

  git://github.com/mcayland/qemu.git tags/qemu-sparc-20210908

for you to fetch changes up to 15a2a1a4d1eecc74a87e1552f5cc4e3668375715:

  escc: fix STATUS_SYNC bit in R_STATUS register (2021-09-08 11:09:45 +0100)

----------------------------------------------------------------
qemu-sparc queue

----------------------------------------------------------------
Mark Cave-Ayland (10):
      sun4m: fix setting CPU id when more than one CPU is present
      escc: checkpatch fixes
      escc: reset register values to zero in escc_reset()
      escc: introduce escc_soft_reset_chn() for software reset
      escc: introduce escc_hard_reset_chn() for hardware reset
      escc: implement soft reset as described in the datasheet
      escc: implement hard reset as described in the datasheet
      escc: remove register changes from escc_reset_chn()
      escc: re-use escc_reset_chn() for soft reset
      escc: fix STATUS_SYNC bit in R_STATUS register

Peter Maydell (2):
      target/sparc: Drop use of gen_io_end()
      tcg: Drop gen_io_end()

 docs/devel/tcg-icount.rst |   3 -
 hw/char/escc.c            | 263 +++++++++++++++++++++++++++++++---------------
 hw/char/trace-events      |   2 +
 hw/sparc/sun4m.c          |   2 +-
 include/exec/gen-icount.h |  27 ++---
 target/sparc/translate.c  |  25 ++---
 6 files changed, 202 insertions(+), 120 deletions(-)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PULL 01/12] target/sparc: Drop use of gen_io_end()
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 02/12] tcg: Drop gen_io_end() Mark Cave-Ayland
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

From: Peter Maydell <peter.maydell@linaro.org>

The gen_io_end() function is obsolete (as documented in
docs/devel/tcg-icount.rst). Where an instruction is an I/O
operation, the translator frontend should call gen_io_start()
before generating the code which does the I/O, and then
end the TB immediately after this insn.

Remove the calls to gen_io_end() in the SPARC frontend,
and ensure that the insns which were calling it end the
TB if they didn't do so already.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210724134902.7785-2-peter.maydell@linaro.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/sparc/translate.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 11de5a4963..bb70ba17de 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -3401,7 +3401,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                         tcg_temp_free_i32(r_const);
                         gen_store_gpr(dc, rd, cpu_dst);
                         if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-                            gen_io_end();
+                            /* I/O operations in icount mode must end the TB */
+                            dc->base.is_jmp = DISAS_EXIT;
                         }
                     }
                     break;
@@ -3454,7 +3455,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                         tcg_temp_free_i32(r_const);
                         gen_store_gpr(dc, rd, cpu_dst);
                         if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-                            gen_io_end();
+                            /* I/O operations in icount mode must end the TB */
+                            dc->base.is_jmp = DISAS_EXIT;
                         }
                     }
                     break;
@@ -3588,7 +3590,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                         tcg_temp_free_ptr(r_tickptr);
                         tcg_temp_free_i32(r_const);
                         if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-                            gen_io_end();
+                            /* I/O operations in icount mode must end the TB */
+                            dc->base.is_jmp = DISAS_EXIT;
                         }
                     }
                     break;
@@ -4582,7 +4585,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                 }
                                 gen_helper_wrpstate(cpu_env, cpu_tmp0);
                                 if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-                                    gen_io_end();
+                                    /* I/O ops in icount mode must end the TB */
+                                    dc->base.is_jmp = DISAS_EXIT;
                                 }
                                 dc->npc = DYNAMIC_PC;
                                 break;
@@ -4598,7 +4602,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                 }
                                 gen_helper_wrpil(cpu_env, cpu_tmp0);
                                 if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-                                    gen_io_end();
+                                    /* I/O ops in icount mode must end the TB */
+                                    dc->base.is_jmp = DISAS_EXIT;
                                 }
                                 break;
                             case 9: // cwp
@@ -4697,10 +4702,6 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                     gen_helper_tick_set_limit(r_tickptr,
                                                               cpu_hstick_cmpr);
                                     tcg_temp_free_ptr(r_tickptr);
-                                    if (tb_cflags(dc->base.tb) &
-                                           CF_USE_ICOUNT) {
-                                        gen_io_end();
-                                    }
                                     /* End TB to handle timer interrupt */
                                     dc->base.is_jmp = DISAS_EXIT;
                                 }
@@ -5327,9 +5328,6 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                 gen_io_start();
                             }
                             gen_helper_done(cpu_env);
-                            if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-                                gen_io_end();
-                            }
                             goto jmp_insn;
                         case 1:
                             if (!supervisor(dc))
@@ -5340,9 +5338,6 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                 gen_io_start();
                             }
                             gen_helper_retry(cpu_env);
-                            if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-                                gen_io_end();
-                            }
                             goto jmp_insn;
                         default:
                             goto illegal_insn;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 02/12] tcg: Drop gen_io_end()
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 01/12] target/sparc: Drop use of gen_io_end() Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 03/12] sun4m: fix setting CPU id when more than one CPU is present Mark Cave-Ayland
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

From: Peter Maydell <peter.maydell@linaro.org>

Now we have removed all the uses of gen_io_end() from target frontends,
the only callsite is inside gen_tb_start(). Inline the code there,
and remove the reference to it from the documentation.

While we are inlining the code, switch it to use tcg_constant_i32()
so we don't have to manually create and destroy a TCG temporary.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210724134902.7785-3-peter.maydell@linaro.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 docs/devel/tcg-icount.rst |  3 ---
 include/exec/gen-icount.h | 27 ++++++++++-----------------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/docs/devel/tcg-icount.rst b/docs/devel/tcg-icount.rst
index 8d67b6c076..50c8e8dabc 100644
--- a/docs/devel/tcg-icount.rst
+++ b/docs/devel/tcg-icount.rst
@@ -92,6 +92,3 @@ When the translator is handling an instruction of this kind:
     }
 
 * it must end the TB immediately after this instruction
-
-Note that some older front-ends call a "gen_io_end()" function:
-this is obsolete and should not be used.
diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index 467529d84c..610cba58fe 100644
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -17,22 +17,6 @@ static inline void gen_io_start(void)
     tcg_temp_free_i32(tmp);
 }
 
-/*
- * cpu->can_do_io is cleared automatically at the beginning of
- * each translation block.  The cost is minimal and only paid
- * for -icount, plus it would be very easy to forget doing it
- * in the translator.  Therefore, backends only need to call
- * gen_io_start.
- */
-static inline void gen_io_end(void)
-{
-    TCGv_i32 tmp = tcg_const_i32(0);
-    tcg_gen_st_i32(tmp, cpu_env,
-                   offsetof(ArchCPU, parent_obj.can_do_io) -
-                   offsetof(ArchCPU, env));
-    tcg_temp_free_i32(tmp);
-}
-
 static inline void gen_tb_start(const TranslationBlock *tb)
 {
     TCGv_i32 count;
@@ -64,7 +48,16 @@ static inline void gen_tb_start(const TranslationBlock *tb)
         tcg_gen_st16_i32(count, cpu_env,
                          offsetof(ArchCPU, neg.icount_decr.u16.low) -
                          offsetof(ArchCPU, env));
-        gen_io_end();
+        /*
+         * cpu->can_do_io is cleared automatically here at the beginning of
+         * each translation block.  The cost is minimal and only paid for
+         * -icount, plus it would be very easy to forget doing it in the
+         * translator. Doing it here means we don't need a gen_io_end() to
+         * go with gen_io_start().
+         */
+        tcg_gen_st_i32(tcg_constant_i32(0), cpu_env,
+                       offsetof(ArchCPU, parent_obj.can_do_io) -
+                       offsetof(ArchCPU, env));
     }
 
     tcg_temp_free_i32(count);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 03/12] sun4m: fix setting CPU id when more than one CPU is present
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 01/12] target/sparc: Drop use of gen_io_end() Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 02/12] tcg: Drop gen_io_end() Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 04/12] escc: checkpatch fixes Mark Cave-Ayland
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

Commit 24f675cd3b ("sparc/sun4m: Use start-powered-off CPUState property") changed
the sun4m CPU reset code to use the start-powered-off property and so split the
creation of the CPU into separate instantiation and realization phases to enable
the new start-powered-off property to be set.

This accidentally broke sun4m machines with more than one CPU present since
sparc_cpu_realizefn() sets a default CPU id, and now that realization occurs after
calling cpu_sparc_set_id() in cpu_devinit() the CPU id gets reset back to the
default instead of being uniquely encoded based upon the CPU number. As soon as
another CPU is brought online, the OS gets confused between them and promptly
panics.

Resolve the issue by moving the cpu_sparc_set_id() call in cpu_devinit() to after
the point where the CPU device has been realized as before.

Fixes: 24f675cd3b ("sparc/sun4m: Use start-powered-off CPUState property")
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210825095100.20180-1-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/sparc/sun4m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 42e139849e..7f3a7c0027 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -803,11 +803,11 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
     cpu = SPARC_CPU(object_new(cpu_type));
     env = &cpu->env;
 
-    cpu_sparc_set_id(env, id);
     qemu_register_reset(sun4m_cpu_reset, cpu);
     object_property_set_bool(OBJECT(cpu), "start-powered-off", id != 0,
                              &error_fatal);
     qdev_realize_and_unref(DEVICE(cpu), NULL, &error_fatal);
+    cpu_sparc_set_id(env, id);
     *cpu_irqs = qemu_allocate_irqs(cpu_set_irq, cpu, MAX_PILS);
     env->prom_addr = prom_addr;
 }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 04/12] escc: checkpatch fixes
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (2 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 03/12] sun4m: fix setting CPU id when more than one CPU is present Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 05/12] escc: reset register values to zero in escc_reset() Mark Cave-Ayland
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

Also fix a couple of spelling mistakes in comments.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-2-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c | 162 +++++++++++++++++++++++++++++--------------------
 1 file changed, 97 insertions(+), 65 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 52e7978287..c87ecd59d8 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -230,20 +230,23 @@ static uint32_t get_queue(void *opaque)
         q->count--;
     }
     trace_escc_get_queue(CHN_C(s), val);
-    if (q->count > 0)
+    if (q->count > 0) {
         serial_receive_byte(s, 0);
+    }
     return val;
 }
 
 static int escc_update_irq_chn(ESCCChannelState *s)
 {
     if ((((s->wregs[W_INTR] & INTR_TXINT) && (s->txint == 1)) ||
-         // tx ints enabled, pending
-         ((((s->wregs[W_INTR] & INTR_RXMODEMSK) == INTR_RXINT1ST) ||
-           ((s->wregs[W_INTR] & INTR_RXMODEMSK) == INTR_RXINTALL)) &&
-          s->rxint == 1) || // rx ints enabled, pending
-         ((s->wregs[W_EXTINT] & EXTINT_BRKINT) &&
-          (s->rregs[R_STATUS] & STATUS_BRK)))) { // break int e&p
+        /* tx ints enabled, pending */
+        ((((s->wregs[W_INTR] & INTR_RXMODEMSK) == INTR_RXINT1ST) ||
+        ((s->wregs[W_INTR] & INTR_RXMODEMSK) == INTR_RXINTALL)) &&
+            s->rxint == 1) ||
+        /* rx ints enabled, pending */
+        ((s->wregs[W_EXTINT] & EXTINT_BRKINT) &&
+            (s->rregs[R_STATUS] & STATUS_BRK)))) {
+        /* break int e&p */
         return 1;
     }
     return 0;
@@ -269,17 +272,22 @@ static void escc_reset_chn(ESCCChannelState *s)
         s->rregs[i] = 0;
         s->wregs[i] = 0;
     }
-    s->wregs[W_TXCTRL1] = TXCTRL1_1STOP; // 1X divisor, 1 stop bit, no parity
+    /* 1X divisor, 1 stop bit, no parity */
+    s->wregs[W_TXCTRL1] = TXCTRL1_1STOP;
     s->wregs[W_MINTR] = MINTR_RST_ALL;
-    s->wregs[W_CLOCK] = CLOCK_TRXC; // Synch mode tx clock = TRxC
-    s->wregs[W_MISC2] = MISC2_PLLDIS; // PLL disabled
+    /* Synch mode tx clock = TRxC */
+    s->wregs[W_CLOCK] = CLOCK_TRXC;
+    /* PLL disabled */
+    s->wregs[W_MISC2] = MISC2_PLLDIS;
+    /* Enable most interrupts */
     s->wregs[W_EXTINT] = EXTINT_DCD | EXTINT_SYNCINT | EXTINT_CTSINT |
-        EXTINT_TXUNDRN | EXTINT_BRKINT; // Enable most interrupts
-    if (s->disabled)
+                         EXTINT_TXUNDRN | EXTINT_BRKINT;
+    if (s->disabled) {
         s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_DCD | STATUS_SYNC |
-            STATUS_CTS | STATUS_TXUNDRN;
-    else
+                             STATUS_CTS | STATUS_TXUNDRN;
+    } else {
         s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_TXUNDRN;
+    }
     s->rregs[R_SPEC] = SPEC_BITS8 | SPEC_ALLSENT;
 
     s->rx = s->tx = 0;
@@ -300,21 +308,25 @@ static void escc_reset(DeviceState *d)
 static inline void set_rxint(ESCCChannelState *s)
 {
     s->rxint = 1;
-    /* XXX: missing daisy chainnig: escc_chn_b rx should have a lower priority
-       than chn_a rx/tx/special_condition service*/
+    /*
+     * XXX: missing daisy chaining: escc_chn_b rx should have a lower priority
+     * than chn_a rx/tx/special_condition service
+     */
     s->rxint_under_svc = 1;
     if (s->chn == escc_chn_a) {
         s->rregs[R_INTR] |= INTR_RXINTA;
-        if (s->wregs[W_MINTR] & MINTR_STATUSHI)
+        if (s->wregs[W_MINTR] & MINTR_STATUSHI) {
             s->otherchn->rregs[R_IVEC] = IVEC_HIRXINTA;
-        else
+        } else {
             s->otherchn->rregs[R_IVEC] = IVEC_LORXINTA;
+        }
     } else {
         s->otherchn->rregs[R_INTR] |= INTR_RXINTB;
-        if (s->wregs[W_MINTR] & MINTR_STATUSHI)
+        if (s->wregs[W_MINTR] & MINTR_STATUSHI) {
             s->rregs[R_IVEC] = IVEC_HIRXINTB;
-        else
+        } else {
             s->rregs[R_IVEC] = IVEC_LORXINTB;
+        }
     }
     escc_update_irq(s);
 }
@@ -328,17 +340,18 @@ static inline void set_txint(ESCCChannelState *s)
             if (s->wregs[W_INTR] & INTR_TXINT) {
                 s->rregs[R_INTR] |= INTR_TXINTA;
             }
-            if (s->wregs[W_MINTR] & MINTR_STATUSHI)
+            if (s->wregs[W_MINTR] & MINTR_STATUSHI) {
                 s->otherchn->rregs[R_IVEC] = IVEC_HITXINTA;
-            else
+            } else {
                 s->otherchn->rregs[R_IVEC] = IVEC_LOTXINTA;
+            }
         } else {
             s->rregs[R_IVEC] = IVEC_TXINTB;
             if (s->wregs[W_INTR] & INTR_TXINT) {
                 s->otherchn->rregs[R_INTR] |= INTR_TXINTB;
             }
         }
-    escc_update_irq(s);
+        escc_update_irq(s);
     }
 }
 
@@ -347,20 +360,23 @@ static inline void clr_rxint(ESCCChannelState *s)
     s->rxint = 0;
     s->rxint_under_svc = 0;
     if (s->chn == escc_chn_a) {
-        if (s->wregs[W_MINTR] & MINTR_STATUSHI)
+        if (s->wregs[W_MINTR] & MINTR_STATUSHI) {
             s->otherchn->rregs[R_IVEC] = IVEC_HINOINT;
-        else
+        } else {
             s->otherchn->rregs[R_IVEC] = IVEC_LONOINT;
+        }
         s->rregs[R_INTR] &= ~INTR_RXINTA;
     } else {
-        if (s->wregs[W_MINTR] & MINTR_STATUSHI)
+        if (s->wregs[W_MINTR] & MINTR_STATUSHI) {
             s->rregs[R_IVEC] = IVEC_HINOINT;
-        else
+        } else {
             s->rregs[R_IVEC] = IVEC_LONOINT;
+        }
         s->otherchn->rregs[R_INTR] &= ~INTR_RXINTB;
     }
-    if (s->txint)
+    if (s->txint) {
         set_txint(s);
+    }
     escc_update_irq(s);
 }
 
@@ -369,21 +385,24 @@ static inline void clr_txint(ESCCChannelState *s)
     s->txint = 0;
     s->txint_under_svc = 0;
     if (s->chn == escc_chn_a) {
-        if (s->wregs[W_MINTR] & MINTR_STATUSHI)
+        if (s->wregs[W_MINTR] & MINTR_STATUSHI) {
             s->otherchn->rregs[R_IVEC] = IVEC_HINOINT;
-        else
+        } else {
             s->otherchn->rregs[R_IVEC] = IVEC_LONOINT;
+        }
         s->rregs[R_INTR] &= ~INTR_TXINTA;
     } else {
         s->otherchn->rregs[R_INTR] &= ~INTR_TXINTB;
-        if (s->wregs[W_MINTR] & MINTR_STATUSHI)
+        if (s->wregs[W_MINTR] & MINTR_STATUSHI) {
             s->rregs[R_IVEC] = IVEC_HINOINT;
-        else
+        } else {
             s->rregs[R_IVEC] = IVEC_LONOINT;
+        }
         s->otherchn->rregs[R_INTR] &= ~INTR_TXINTB;
     }
-    if (s->rxint)
+    if (s->rxint) {
         set_rxint(s);
+    }
     escc_update_irq(s);
 }
 
@@ -392,21 +411,24 @@ static void escc_update_parameters(ESCCChannelState *s)
     int speed, parity, data_bits, stop_bits;
     QEMUSerialSetParams ssp;
 
-    if (!qemu_chr_fe_backend_connected(&s->chr) || s->type != escc_serial)
+    if (!qemu_chr_fe_backend_connected(&s->chr) || s->type != escc_serial) {
         return;
+    }
 
     if (s->wregs[W_TXCTRL1] & TXCTRL1_PAREN) {
-        if (s->wregs[W_TXCTRL1] & TXCTRL1_PAREV)
+        if (s->wregs[W_TXCTRL1] & TXCTRL1_PAREV) {
             parity = 'E';
-        else
+        } else {
             parity = 'O';
+        }
     } else {
         parity = 'N';
     }
-    if ((s->wregs[W_TXCTRL1] & TXCTRL1_STPMSK) == TXCTRL1_2STOP)
+    if ((s->wregs[W_TXCTRL1] & TXCTRL1_STPMSK) == TXCTRL1_2STOP) {
         stop_bits = 2;
-    else
+    } else {
         stop_bits = 1;
+    }
     switch (s->wregs[W_TXCTRL2] & TXCTRL2_BITMSK) {
     case TXCTRL2_5BITS:
         data_bits = 5;
@@ -523,10 +545,11 @@ static void escc_mem_write(void *opaque, hwaddr addr,
         default:
             break;
         }
-        if (s->reg == 0)
+        if (s->reg == 0) {
             s->reg = newreg;
-        else
+        } else {
             s->reg = 0;
+        }
         break;
     case SERIAL_DATA:
         trace_escc_mem_writeb_data(CHN_C(s), val);
@@ -538,17 +561,19 @@ static void escc_mem_write(void *opaque, hwaddr addr,
         s->txint = 0;
         escc_update_irq(s);
         s->tx = val;
-        if (s->wregs[W_TXCTRL2] & TXCTRL2_TXEN) { // tx enabled
+        if (s->wregs[W_TXCTRL2] & TXCTRL2_TXEN) { /* tx enabled */
             if (qemu_chr_fe_backend_connected(&s->chr)) {
-                /* XXX this blocks entire thread. Rewrite to use
-                 * qemu_chr_fe_write and background I/O callbacks */
+                /*
+                 * XXX this blocks entire thread. Rewrite to use
+                 * qemu_chr_fe_write and background I/O callbacks
+                 */
                 qemu_chr_fe_write_all(&s->chr, &s->tx, 1);
             } else if (s->type == escc_kbd && !s->disabled) {
                 handle_kbd_command(s, val);
             }
         }
-        s->rregs[R_STATUS] |= STATUS_TXEMPTY; // Tx buffer empty
-        s->rregs[R_SPEC] |= SPEC_ALLSENT; // All sent
+        s->rregs[R_STATUS] |= STATUS_TXEMPTY; /* Tx buffer empty */
+        s->rregs[R_SPEC] |= SPEC_ALLSENT; /* All sent */
         set_txint(s);
         break;
     default:
@@ -606,12 +631,13 @@ static int serial_can_receive(void *opaque)
     ESCCChannelState *s = opaque;
     int ret;
 
-    if (((s->wregs[W_RXCTRL] & RXCTRL_RXEN) == 0) // Rx not enabled
-        || ((s->rregs[R_STATUS] & STATUS_RXAV) == STATUS_RXAV))
-        // char already available
+    if (((s->wregs[W_RXCTRL] & RXCTRL_RXEN) == 0) /* Rx not enabled */
+        || ((s->rregs[R_STATUS] & STATUS_RXAV) == STATUS_RXAV)) {
+        /* char already available */
         ret = 0;
-    else
+    } else {
         ret = 1;
+    }
     return ret;
 }
 
@@ -638,12 +664,13 @@ static void serial_receive1(void *opaque, const uint8_t *buf, int size)
 static void serial_event(void *opaque, QEMUChrEvent event)
 {
     ESCCChannelState *s = opaque;
-    if (event == CHR_EVENT_BREAK)
+    if (event == CHR_EVENT_BREAK) {
         serial_receive_break(s);
+    }
 }
 
 static const VMStateDescription vmstate_escc_chn = {
-    .name ="escc_chn",
+    .name = "escc_chn",
     .version_id = 2,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -662,7 +689,7 @@ static const VMStateDescription vmstate_escc_chn = {
 };
 
 static const VMStateDescription vmstate_escc = {
-    .name ="escc",
+    .name = "escc",
     .version_id = 2,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -734,21 +761,21 @@ static QemuInputHandler sunkbd_handler = {
 static void handle_kbd_command(ESCCChannelState *s, int val)
 {
     trace_escc_kbd_command(val);
-    if (s->led_mode) { // Ignore led byte
+    if (s->led_mode) { /* Ignore led byte */
         s->led_mode = 0;
         return;
     }
     switch (val) {
-    case 1: // Reset, return type code
+    case 1: /* Reset, return type code */
         clear_queue(s);
         put_queue(s, 0xff);
-        put_queue(s, 4); // Type 4
+        put_queue(s, 4); /* Type 4 */
         put_queue(s, 0x7f);
         break;
-    case 0xe: // Set leds
+    case 0xe: /* Set leds */
         s->led_mode = 1;
         break;
-    case 7: // Query layout
+    case 7: /* Query layout */
     case 0xf:
         clear_queue(s);
         put_queue(s, 0xfe);
@@ -768,34 +795,39 @@ static void sunmouse_event(void *opaque,
     trace_escc_sunmouse_event(dx, dy, buttons_state);
     ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
 
-    if (buttons_state & MOUSE_EVENT_LBUTTON)
+    if (buttons_state & MOUSE_EVENT_LBUTTON) {
         ch ^= 0x4;
-    if (buttons_state & MOUSE_EVENT_MBUTTON)
+    }
+    if (buttons_state & MOUSE_EVENT_MBUTTON) {
         ch ^= 0x2;
-    if (buttons_state & MOUSE_EVENT_RBUTTON)
+    }
+    if (buttons_state & MOUSE_EVENT_RBUTTON) {
         ch ^= 0x1;
+    }
 
     put_queue(s, ch);
 
     ch = dx;
 
-    if (ch > 127)
+    if (ch > 127) {
         ch = 127;
-    else if (ch < -127)
+    } else if (ch < -127) {
         ch = -127;
+    }
 
     put_queue(s, ch & 0xff);
 
     ch = -dy;
 
-    if (ch > 127)
+    if (ch > 127) {
         ch = 127;
-    else if (ch < -127)
+    } else if (ch < -127) {
         ch = -127;
+    }
 
     put_queue(s, ch & 0xff);
 
-    // MSC protocol specify two extra motion bytes
+    /* MSC protocol specifies two extra motion bytes */
 
     put_queue(s, 0);
     put_queue(s, 0);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 05/12] escc: reset register values to zero in escc_reset()
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (3 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 04/12] escc: checkpatch fixes Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 06/12] escc: introduce escc_soft_reset_chn() for software reset Mark Cave-Ayland
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

This is to ensure that a device reset always returns the ESCC to a known state.

Note that this is currently redundant with the same code in escc_reset_chn()
but that will change shortly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-3-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index c87ecd59d8..b0d3b92dc1 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -300,9 +300,24 @@ static void escc_reset_chn(ESCCChannelState *s)
 static void escc_reset(DeviceState *d)
 {
     ESCCState *s = ESCC(d);
+    int i, j;
 
-    escc_reset_chn(&s->chn[0]);
-    escc_reset_chn(&s->chn[1]);
+    for (i = 0; i < 2; i++) {
+        ESCCChannelState *cs = &s->chn[i];
+
+        /*
+         * According to the ESCC datasheet "Miscellaneous Questions" section
+         * on page 384, the values of the ESCC registers are not guaranteed on
+         * power-on until an explicit hardware or software reset has been
+         * issued. For now we zero the registers so that a device reset always
+         * returns the emulated device to a fixed state.
+         */
+        for (j = 0; j < ESCC_SERIAL_REGS; j++) {
+            cs->rregs[j] = 0;
+            cs->wregs[j] = 0;
+        }
+        escc_reset_chn(cs);
+    }
 }
 
 static inline void set_rxint(ESCCChannelState *s)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 06/12] escc: introduce escc_soft_reset_chn() for software reset
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (4 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 05/12] escc: reset register values to zero in escc_reset() Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 07/12] escc: introduce escc_hard_reset_chn() for hardware reset Mark Cave-Ayland
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

This new software reset function is to be called when the appropriate channel
software reset bit is written to register WR9. Its initial implementation is
the same as the existing escc_reset_chn() function used for device reset.

Add a new trace event when the guest initiates a soft reset via the WR9 register
to help diagnose guest reset issues.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-4-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c       | 40 ++++++++++++++++++++++++++++++++++++++--
 hw/char/trace-events |  1 +
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index b0d3b92dc1..697f15f383 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -297,6 +297,40 @@ static void escc_reset_chn(ESCCChannelState *s)
     clear_queue(s);
 }
 
+static void escc_soft_reset_chn(ESCCChannelState *s)
+{
+    int i;
+
+    s->reg = 0;
+    for (i = 0; i < ESCC_SERIAL_REGS; i++) {
+        s->rregs[i] = 0;
+        s->wregs[i] = 0;
+    }
+    /* 1X divisor, 1 stop bit, no parity */
+    s->wregs[W_TXCTRL1] = TXCTRL1_1STOP;
+    s->wregs[W_MINTR] = MINTR_RST_ALL;
+    /* Synch mode tx clock = TRxC */
+    s->wregs[W_CLOCK] = CLOCK_TRXC;
+    /* PLL disabled */
+    s->wregs[W_MISC2] = MISC2_PLLDIS;
+    /* Enable most interrupts */
+    s->wregs[W_EXTINT] = EXTINT_DCD | EXTINT_SYNCINT | EXTINT_CTSINT |
+                         EXTINT_TXUNDRN | EXTINT_BRKINT;
+    if (s->disabled) {
+        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_DCD | STATUS_SYNC |
+                             STATUS_CTS | STATUS_TXUNDRN;
+    } else {
+        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_TXUNDRN;
+    }
+    s->rregs[R_SPEC] = SPEC_BITS8 | SPEC_ALLSENT;
+
+    s->rx = s->tx = 0;
+    s->rxint = s->txint = 0;
+    s->rxint_under_svc = s->txint_under_svc = 0;
+    s->e0_mode = s->led_mode = s->caps_lock_mode = s->num_lock_mode = 0;
+    clear_queue(s);
+}
+
 static void escc_reset(DeviceState *d)
 {
     ESCCState *s = ESCC(d);
@@ -547,10 +581,12 @@ static void escc_mem_write(void *opaque, hwaddr addr,
             default:
                 break;
             case MINTR_RST_B:
-                escc_reset_chn(&serial->chn[0]);
+                trace_escc_soft_reset_chn(CHN_C(&serial->chn[0]));
+                escc_soft_reset_chn(&serial->chn[0]);
                 return;
             case MINTR_RST_A:
-                escc_reset_chn(&serial->chn[1]);
+                trace_escc_soft_reset_chn(CHN_C(&serial->chn[1]));
+                escc_soft_reset_chn(&serial->chn[1]);
                 return;
             case MINTR_RST_ALL:
                 escc_reset(DEVICE(serial));
diff --git a/hw/char/trace-events b/hw/char/trace-events
index 1436fb462d..073f98ebe8 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -36,6 +36,7 @@ grlib_apbuart_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" va
 grlib_apbuart_readl_unknown(uint64_t addr) "addr 0x%"PRIx64
 
 # escc.c
+escc_soft_reset_chn(char channel) "soft reset channel %c"
 escc_put_queue(char channel, int b) "channel %c put: 0x%02x"
 escc_get_queue(char channel, int val) "channel %c get 0x%02x"
 escc_update_irq(int irq) "IRQ = %d"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 07/12] escc: introduce escc_hard_reset_chn() for hardware reset
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (5 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 06/12] escc: introduce escc_soft_reset_chn() for software reset Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 08/12] escc: implement soft reset as described in the datasheet Mark Cave-Ayland
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

This new hardware reset function is to be called for both channels when the
hardware reset bit is written to register WR9. Its initial implementation is
the same as the existing escc_reset_chn() function used for device reset.

Add a new trace event when the guest initiates a hard reset via the WR9 register
to help diagnose guest reset issues.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-5-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c       | 38 +++++++++++++++++++++++++++++++++++++-
 hw/char/trace-events |  1 +
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 697f15f383..806f593738 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -331,6 +331,40 @@ static void escc_soft_reset_chn(ESCCChannelState *s)
     clear_queue(s);
 }
 
+static void escc_hard_reset_chn(ESCCChannelState *s)
+{
+    int i;
+
+    s->reg = 0;
+    for (i = 0; i < ESCC_SERIAL_REGS; i++) {
+        s->rregs[i] = 0;
+        s->wregs[i] = 0;
+    }
+    /* 1X divisor, 1 stop bit, no parity */
+    s->wregs[W_TXCTRL1] = TXCTRL1_1STOP;
+    s->wregs[W_MINTR] = MINTR_RST_ALL;
+    /* Synch mode tx clock = TRxC */
+    s->wregs[W_CLOCK] = CLOCK_TRXC;
+    /* PLL disabled */
+    s->wregs[W_MISC2] = MISC2_PLLDIS;
+    /* Enable most interrupts */
+    s->wregs[W_EXTINT] = EXTINT_DCD | EXTINT_SYNCINT | EXTINT_CTSINT |
+                         EXTINT_TXUNDRN | EXTINT_BRKINT;
+    if (s->disabled) {
+        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_DCD | STATUS_SYNC |
+                             STATUS_CTS | STATUS_TXUNDRN;
+    } else {
+        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_TXUNDRN;
+    }
+    s->rregs[R_SPEC] = SPEC_BITS8 | SPEC_ALLSENT;
+
+    s->rx = s->tx = 0;
+    s->rxint = s->txint = 0;
+    s->rxint_under_svc = s->txint_under_svc = 0;
+    s->e0_mode = s->led_mode = s->caps_lock_mode = s->num_lock_mode = 0;
+    clear_queue(s);
+}
+
 static void escc_reset(DeviceState *d)
 {
     ESCCState *s = ESCC(d);
@@ -589,7 +623,9 @@ static void escc_mem_write(void *opaque, hwaddr addr,
                 escc_soft_reset_chn(&serial->chn[1]);
                 return;
             case MINTR_RST_ALL:
-                escc_reset(DEVICE(serial));
+                trace_escc_hard_reset();
+                escc_hard_reset_chn(&serial->chn[0]);
+                escc_hard_reset_chn(&serial->chn[1]);
                 return;
             }
             break;
diff --git a/hw/char/trace-events b/hw/char/trace-events
index 073f98ebe8..b774832af4 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -36,6 +36,7 @@ grlib_apbuart_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" va
 grlib_apbuart_readl_unknown(uint64_t addr) "addr 0x%"PRIx64
 
 # escc.c
+escc_hard_reset(void) "hard reset"
 escc_soft_reset_chn(char channel) "soft reset channel %c"
 escc_put_queue(char channel, int b) "channel %c put: 0x%02x"
 escc_get_queue(char channel, int val) "channel %c get 0x%02x"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 08/12] escc: implement soft reset as described in the datasheet
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (6 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 07/12] escc: introduce escc_hard_reset_chn() for hardware reset Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 09/12] escc: implement hard " Mark Cave-Ayland
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

The software reset differs from a device reset in that it only changes the contents
of specific registers. Remove the code that resets all the registers to zero during
soft reset and implement the default values listed in the table in the "Z85C30 Reset"
section.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-6-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c | 48 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 806f593738..d5c7136e97 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -86,9 +86,11 @@
 #define W_INTR    1
 #define INTR_INTALL    0x01
 #define INTR_TXINT     0x02
+#define INTR_PAR_SPEC  0x04
 #define INTR_RXMODEMSK 0x18
 #define INTR_RXINT1ST  0x08
 #define INTR_RXINTALL  0x10
+#define INTR_WTRQ_TXRX 0x20
 #define W_IVEC    2
 #define W_RXCTRL  3
 #define RXCTRL_RXEN    0x01
@@ -105,6 +107,7 @@
 #define TXCTRL1_CLK64X 0xc0
 #define TXCTRL1_CLKMSK 0xc0
 #define W_TXCTRL2 5
+#define TXCTRL2_TXCRC  0x01
 #define TXCTRL2_TXEN   0x08
 #define TXCTRL2_BITMSK 0x60
 #define TXCTRL2_5BITS  0x00
@@ -116,16 +119,24 @@
 #define W_TXBUF   8
 #define W_MINTR   9
 #define MINTR_STATUSHI 0x10
+#define MINTR_SOFTIACK 0x20
 #define MINTR_RST_MASK 0xc0
 #define MINTR_RST_B    0x40
 #define MINTR_RST_A    0x80
 #define MINTR_RST_ALL  0xc0
 #define W_MISC1  10
+#define MISC1_ENC_MASK 0x60
 #define W_CLOCK  11
 #define CLOCK_TRXC     0x08
 #define W_BRGLO  12
 #define W_BRGHI  13
 #define W_MISC2  14
+#define MISC2_BRG_EN   0x01
+#define MISC2_BRG_SRC  0x02
+#define MISC2_LCL_LOOP 0x10
+#define MISC2_PLLCMD0  0x20
+#define MISC2_PLLCMD1  0x40
+#define MISC2_PLLCMD2  0x80
 #define MISC2_PLLDIS   0x30
 #define W_EXTINT 15
 #define EXTINT_DCD     0x08
@@ -170,6 +181,7 @@
 #define R_RXBUF   8
 #define R_RXCTRL  9
 #define R_MISC   10
+#define MISC_2CLKMISS  0x40
 #define R_MISC1  11
 #define R_BRGLO  12
 #define R_BRGHI  13
@@ -299,30 +311,32 @@ static void escc_reset_chn(ESCCChannelState *s)
 
 static void escc_soft_reset_chn(ESCCChannelState *s)
 {
-    int i;
-
     s->reg = 0;
-    for (i = 0; i < ESCC_SERIAL_REGS; i++) {
-        s->rregs[i] = 0;
-        s->wregs[i] = 0;
-    }
-    /* 1X divisor, 1 stop bit, no parity */
-    s->wregs[W_TXCTRL1] = TXCTRL1_1STOP;
-    s->wregs[W_MINTR] = MINTR_RST_ALL;
-    /* Synch mode tx clock = TRxC */
-    s->wregs[W_CLOCK] = CLOCK_TRXC;
+    s->wregs[W_CMD] = 0;
+    s->wregs[W_INTR] &= INTR_PAR_SPEC | INTR_WTRQ_TXRX;
+    s->wregs[W_RXCTRL] &= ~RXCTRL_RXEN;
+    /* 1 stop bit */
+    s->wregs[W_TXCTRL1] |= TXCTRL1_1STOP;
+    s->wregs[W_TXCTRL2] &= TXCTRL2_TXCRC | TXCTRL2_8BITS;
+    s->wregs[W_MINTR] &= ~MINTR_SOFTIACK;
+    s->wregs[W_MISC1] &= MISC1_ENC_MASK;
     /* PLL disabled */
-    s->wregs[W_MISC2] = MISC2_PLLDIS;
+    s->wregs[W_MISC2] &= MISC2_BRG_EN | MISC2_BRG_SRC |
+                         MISC2_PLLCMD1 | MISC2_PLLCMD2;
+    s->wregs[W_MISC2] |= MISC2_PLLCMD0;
     /* Enable most interrupts */
     s->wregs[W_EXTINT] = EXTINT_DCD | EXTINT_SYNCINT | EXTINT_CTSINT |
                          EXTINT_TXUNDRN | EXTINT_BRKINT;
+
+    s->rregs[R_STATUS] &= STATUS_DCD | STATUS_SYNC | STATUS_CTS | STATUS_BRK;
+    s->rregs[R_STATUS] |= STATUS_TXEMPTY | STATUS_TXUNDRN;
     if (s->disabled) {
-        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_DCD | STATUS_SYNC |
-                             STATUS_CTS | STATUS_TXUNDRN;
-    } else {
-        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_TXUNDRN;
+        s->rregs[R_STATUS] |= STATUS_DCD | STATUS_SYNC | STATUS_CTS;
     }
-    s->rregs[R_SPEC] = SPEC_BITS8 | SPEC_ALLSENT;
+    s->rregs[R_SPEC] &= SPEC_ALLSENT;
+    s->rregs[R_SPEC] |= SPEC_BITS8;
+    s->rregs[R_INTR] = 0;
+    s->rregs[R_MISC] &= MISC_2CLKMISS;
 
     s->rx = s->tx = 0;
     s->rxint = s->txint = 0;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 09/12] escc: implement hard reset as described in the datasheet
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (7 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 08/12] escc: implement soft reset as described in the datasheet Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 10/12] escc: remove register changes from escc_reset_chn() Mark Cave-Ayland
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

The hardware reset differs from a device reset in that it only changes the contents
of specific registers. Remove the code that resets all the registers to zero during
hardware reset and implement the default values using the existing soft reset code
with the additional changes listed in the table in the "Z85C30 Reset" section.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-7-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c | 41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index d5c7136e97..80f1d1b8fc 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -118,6 +118,8 @@
 #define W_SYNC2   7
 #define W_TXBUF   8
 #define W_MINTR   9
+#define MINTR_VIS      0x01
+#define MINTR_NV       0x02
 #define MINTR_STATUSHI 0x10
 #define MINTR_SOFTIACK 0x20
 #define MINTR_RST_MASK 0xc0
@@ -347,36 +349,19 @@ static void escc_soft_reset_chn(ESCCChannelState *s)
 
 static void escc_hard_reset_chn(ESCCChannelState *s)
 {
-    int i;
+    escc_soft_reset_chn(s);
 
-    s->reg = 0;
-    for (i = 0; i < ESCC_SERIAL_REGS; i++) {
-        s->rregs[i] = 0;
-        s->wregs[i] = 0;
-    }
-    /* 1X divisor, 1 stop bit, no parity */
-    s->wregs[W_TXCTRL1] = TXCTRL1_1STOP;
-    s->wregs[W_MINTR] = MINTR_RST_ALL;
-    /* Synch mode tx clock = TRxC */
+    /*
+     * Hard reset is almost identical to soft reset above, except that the
+     * values of WR9 (W_MINTR), WR10 (W_MISC1), WR11 (W_CLOCK) and WR14
+     * (W_MISC2) have extra bits forced to 0/1
+     */
+    s->wregs[W_MINTR] &= MINTR_VIS | MINTR_NV;
+    s->wregs[W_MINTR] |= MINTR_RST_B | MINTR_RST_A;
+    s->wregs[W_MISC1] = 0;
     s->wregs[W_CLOCK] = CLOCK_TRXC;
-    /* PLL disabled */
-    s->wregs[W_MISC2] = MISC2_PLLDIS;
-    /* Enable most interrupts */
-    s->wregs[W_EXTINT] = EXTINT_DCD | EXTINT_SYNCINT | EXTINT_CTSINT |
-                         EXTINT_TXUNDRN | EXTINT_BRKINT;
-    if (s->disabled) {
-        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_DCD | STATUS_SYNC |
-                             STATUS_CTS | STATUS_TXUNDRN;
-    } else {
-        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_TXUNDRN;
-    }
-    s->rregs[R_SPEC] = SPEC_BITS8 | SPEC_ALLSENT;
-
-    s->rx = s->tx = 0;
-    s->rxint = s->txint = 0;
-    s->rxint_under_svc = s->txint_under_svc = 0;
-    s->e0_mode = s->led_mode = s->caps_lock_mode = s->num_lock_mode = 0;
-    clear_queue(s);
+    s->wregs[W_MISC2] &= MISC2_PLLCMD1 | MISC2_PLLCMD2;
+    s->wregs[W_MISC2] |= MISC2_LCL_LOOP | MISC2_PLLCMD0;
 }
 
 static void escc_reset(DeviceState *d)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 10/12] escc: remove register changes from escc_reset_chn()
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (8 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 09/12] escc: implement hard " Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 11/12] escc: re-use escc_reset_chn() for soft reset Mark Cave-Ayland
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

Now that register values at reset are handled elsewhere for all of device reset,
soft reset and hard reset, escc_reset_chn() only needs to handle initialisation
of internal device state.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-8-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 80f1d1b8fc..22c97414a1 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -139,7 +139,6 @@
 #define MISC2_PLLCMD0  0x20
 #define MISC2_PLLCMD1  0x40
 #define MISC2_PLLCMD2  0x80
-#define MISC2_PLLDIS   0x30
 #define W_EXTINT 15
 #define EXTINT_DCD     0x08
 #define EXTINT_SYNCINT 0x10
@@ -279,31 +278,7 @@ static void escc_update_irq(ESCCChannelState *s)
 
 static void escc_reset_chn(ESCCChannelState *s)
 {
-    int i;
-
     s->reg = 0;
-    for (i = 0; i < ESCC_SERIAL_REGS; i++) {
-        s->rregs[i] = 0;
-        s->wregs[i] = 0;
-    }
-    /* 1X divisor, 1 stop bit, no parity */
-    s->wregs[W_TXCTRL1] = TXCTRL1_1STOP;
-    s->wregs[W_MINTR] = MINTR_RST_ALL;
-    /* Synch mode tx clock = TRxC */
-    s->wregs[W_CLOCK] = CLOCK_TRXC;
-    /* PLL disabled */
-    s->wregs[W_MISC2] = MISC2_PLLDIS;
-    /* Enable most interrupts */
-    s->wregs[W_EXTINT] = EXTINT_DCD | EXTINT_SYNCINT | EXTINT_CTSINT |
-                         EXTINT_TXUNDRN | EXTINT_BRKINT;
-    if (s->disabled) {
-        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_DCD | STATUS_SYNC |
-                             STATUS_CTS | STATUS_TXUNDRN;
-    } else {
-        s->rregs[R_STATUS] = STATUS_TXEMPTY | STATUS_TXUNDRN;
-    }
-    s->rregs[R_SPEC] = SPEC_BITS8 | SPEC_ALLSENT;
-
     s->rx = s->tx = 0;
     s->rxint = s->txint = 0;
     s->rxint_under_svc = s->txint_under_svc = 0;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 11/12] escc: re-use escc_reset_chn() for soft reset
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (9 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 10/12] escc: remove register changes from escc_reset_chn() Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-08 11:54 ` [PULL 12/12] escc: fix STATUS_SYNC bit in R_STATUS register Mark Cave-Ayland
  2021-09-10 10:08 ` [PULL 00/12] qemu-sparc queue 20210908 Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

This removes duplication of the internal device state initialisation between
device reset and soft reset.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-9-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 22c97414a1..9283ed70a6 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -288,7 +288,8 @@ static void escc_reset_chn(ESCCChannelState *s)
 
 static void escc_soft_reset_chn(ESCCChannelState *s)
 {
-    s->reg = 0;
+    escc_reset_chn(s);
+
     s->wregs[W_CMD] = 0;
     s->wregs[W_INTR] &= INTR_PAR_SPEC | INTR_WTRQ_TXRX;
     s->wregs[W_RXCTRL] &= ~RXCTRL_RXEN;
@@ -314,12 +315,6 @@ static void escc_soft_reset_chn(ESCCChannelState *s)
     s->rregs[R_SPEC] |= SPEC_BITS8;
     s->rregs[R_INTR] = 0;
     s->rregs[R_MISC] &= MISC_2CLKMISS;
-
-    s->rx = s->tx = 0;
-    s->rxint = s->txint = 0;
-    s->rxint_under_svc = s->txint_under_svc = 0;
-    s->e0_mode = s->led_mode = s->caps_lock_mode = s->num_lock_mode = 0;
-    clear_queue(s);
 }
 
 static void escc_hard_reset_chn(ESCCChannelState *s)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 12/12] escc: fix STATUS_SYNC bit in R_STATUS register
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (10 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 11/12] escc: re-use escc_reset_chn() for soft reset Mark Cave-Ayland
@ 2021-09-08 11:54 ` Mark Cave-Ayland
  2021-09-10 10:08 ` [PULL 00/12] qemu-sparc queue 20210908 Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2021-09-08 11:54 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

After an SDLC "Enter hunt" command has been sent the STATUS_SYNC bit should remain
high until the flag byte has been detected. Whilst the ESCC device doesn't yet
implement SDLC mode, without this change the active low STATUS_SYNC is constantly
asserted causing the MacOS OpenTransport extension to hang on startup as it thinks
it is constantly receiving LocalTalk responses during its initial negotiation
phase.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210903113223.19551-10-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/char/escc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 9283ed70a6..0fce4f6324 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -94,6 +94,7 @@
 #define W_IVEC    2
 #define W_RXCTRL  3
 #define RXCTRL_RXEN    0x01
+#define RXCTRL_HUNT    0x10
 #define W_TXCTRL1 4
 #define TXCTRL1_PAREN  0x01
 #define TXCTRL1_PAREV  0x02
@@ -561,7 +562,13 @@ static void escc_mem_write(void *opaque, hwaddr addr,
                 break;
             }
             break;
-        case W_INTR ... W_RXCTRL:
+        case W_RXCTRL:
+            s->wregs[s->reg] = val;
+            if (val & RXCTRL_HUNT) {
+                s->rregs[R_STATUS] |= STATUS_SYNC;
+            }
+            break;
+        case W_INTR ... W_IVEC:
         case W_SYNC1 ... W_TXBUF:
         case W_MISC1 ... W_CLOCK:
         case W_MISC2 ... W_EXTINT:
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PULL 00/12] qemu-sparc queue 20210908
  2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
                   ` (11 preceding siblings ...)
  2021-09-08 11:54 ` [PULL 12/12] escc: fix STATUS_SYNC bit in R_STATUS register Mark Cave-Ayland
@ 2021-09-10 10:08 ` Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2021-09-10 10:08 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: QEMU Developers

On Wed, 8 Sept 2021 at 12:55, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The following changes since commit abf7aee72ea66944a62962603e4c2381f5e473e7:
>
>   Merge remote-tracking branch 'remotes/thuth-gitlab/tags/s390x-pull-request-2021-09-07' into staging (2021-09-07 17:46:13 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/mcayland/qemu.git tags/qemu-sparc-20210908
>
> for you to fetch changes up to 15a2a1a4d1eecc74a87e1552f5cc4e3668375715:
>
>   escc: fix STATUS_SYNC bit in R_STATUS register (2021-09-08 11:09:45 +0100)
>
> ----------------------------------------------------------------
> qemu-sparc queue
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.2
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-09-10 10:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 11:54 [PULL 00/12] qemu-sparc queue 20210908 Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 01/12] target/sparc: Drop use of gen_io_end() Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 02/12] tcg: Drop gen_io_end() Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 03/12] sun4m: fix setting CPU id when more than one CPU is present Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 04/12] escc: checkpatch fixes Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 05/12] escc: reset register values to zero in escc_reset() Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 06/12] escc: introduce escc_soft_reset_chn() for software reset Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 07/12] escc: introduce escc_hard_reset_chn() for hardware reset Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 08/12] escc: implement soft reset as described in the datasheet Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 09/12] escc: implement hard " Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 10/12] escc: remove register changes from escc_reset_chn() Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 11/12] escc: re-use escc_reset_chn() for soft reset Mark Cave-Ayland
2021-09-08 11:54 ` [PULL 12/12] escc: fix STATUS_SYNC bit in R_STATUS register Mark Cave-Ayland
2021-09-10 10:08 ` [PULL 00/12] qemu-sparc queue 20210908 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).