Re: [PATCH 2/3] escc: fix R_STATUS channel reset value

[Peter Maydell]

On Sun, 29 Aug 2021 at 11:07, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> According to the "Z80X30 Register Reset Values" table in the ESCC datasheet
> bits 2 and 6 are set whilst bits 0 and 1 are cleared during channel reset.
> All other bits should be left unaltered.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/char/escc.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/escc.c b/hw/char/escc.c
> index 63e0f15dfa..0f6957ba8b 100644
> --- a/hw/char/escc.c
> +++ b/hw/char/escc.c
> @@ -281,12 +281,11 @@ static void escc_reset_chn(ESCCChannelState *s)
>      s->wregs[W_MISC2] = MISC2_PLLDIS;
>      /* Enable most interrupts */
>      s->wregs[W_EXTINT] = EXTINT_DCD | EXTINT_SYNCINT | EXTINT_CTSINT |
> -        EXTINT_TXUNDRN | EXTINT_BRKINT;
> +                         EXTINT_TXUNDRN | EXTINT_BRKINT;

This indentation fix should probably have been in the
coding-style-fixes patch.

> +    s->rregs[R_STATUS] &= ~(STATUS_RXAV | STATUS_ZERO);
> +    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_CTS;
>      }
>      s->rregs[R_SPEC] = SPEC_BITS8 | SPEC_ALLSENT;

We seem to use this function for both 'channel reset' and
'hardware reset' -- escc_reset just calls escc_reset_chn for
each channel, and is used as the DeviceState::reset as well
as for the 'hardware reset' you get by writing to WR9 with
both D6 and D7 set to 1.

Because we want this software-triggered 'hardware reset' to not
reset all registers to fixed values, I think we need to disentangle
the power-on reset DeviceState::reset so that power-on reset always
brings the device back to exactly the state that it has when QEMU
first starts.

thanks
-- PMM