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

[Mark Cave-Ayland]

On 29/08/2021 14:17, Peter Maydell wrote:

> 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.

Okay I can do that.

> > +    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.

I'm currently struggling to understand what the initial power-on values are for the 
ESCC registers. According to the reset table both 'hardware reset' and 'software 
reset' don't change the values of all bits which implies the power-on state is 
different, but I can't seem to find a description of it anywhere? Could we assume 
that all ESCC registers are initialised to zero, and the guest will perform a 
suitable reset during driver initialisation?


ATB,

Mark.