| Message ID | 20230909094827.33871-16-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | q800: add support for booting MacOS Classic - part 2 | expand |
Le 09/09/2023 à 11:48, Mark Cave-Ayland a écrit : > NetBSD assumes it can send its first ADB command after sending the ADB_BUSRESET > command in ADB_STATE_NEW without changing the state back to ADB_STATE_IDLE > first as detailed in the ADB protocol. > > Add a workaround to detect this condition at the start of ADB enumeration > and send the next command written to SR after a ADB_BUSRESET onto the bus > regardless, even if we don't detect a state transition to ADB_STATE_NEW. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/misc/mac_via.c | 34 ++++++++++++++++++++++++++++++++++ > hw/misc/trace-events | 1 + > 2 files changed, 35 insertions(+) > > diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c > index 766a32a95d..208216aed3 100644 > --- a/hw/misc/mac_via.c > +++ b/hw/misc/mac_via.c > @@ -1001,6 +1001,8 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val, > { > MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(opaque); > MOS6522State *ms = MOS6522(v1s); > + int oldstate, state; > + int oldsr = ms->sr; > > addr = (addr >> 9) & 0xf; > > @@ -1016,6 +1018,38 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val, > > v1s->last_b = ms->b; > break; > + > + case VIA_REG_SR: > + { > + /* > + * NetBSD assumes it can send its first ADB command after sending > + * the ADB_BUSRESET command in ADB_STATE_NEW without changing the > + * state back to ADB_STATE_IDLE first as detailed in the ADB > + * protocol. > + * > + * Add a workaround to detect this condition at the start of ADB > + * enumeration and send the next command written to SR after a > + * ADB_BUSRESET onto the bus regardless, even if we don't detect a > + * state transition to ADB_STATE_NEW. > + * > + * Note that in my tests the NetBSD state machine takes one ADB > + * operation to recover which means the probe for an ADB device at > + * address 1 always fails. However since the first device is at > + * address 2 then this will work fine, without having to come up > + * with a more complicated and invasive solution. > + */ > + oldstate = (v1s->last_b & VIA1B_vADB_StateMask) >> > + VIA1B_vADB_StateShift; > + state = (ms->b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift; > + > + if (oldstate == ADB_STATE_NEW && state == ADB_STATE_NEW && > + (ms->acr & VIA1ACR_vShiftOut) && > + oldsr == 0 /* ADB_BUSRESET */) { > + trace_via1_adb_netbsd_enum_hack(); > + adb_via_send(v1s, state, ms->sr); > + } > + } > + break; > } > } > > diff --git a/hw/misc/trace-events b/hw/misc/trace-events > index 0c9762fdf6..db8bb2d28a 100644 > --- a/hw/misc/trace-events > +++ b/hw/misc/trace-events > @@ -271,6 +271,7 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "secto > via1_adb_send(const char *state, uint8_t data, const char *vadbint) "state %s data=0x%02x vADBInt=%s" > via1_adb_receive(const char *state, uint8_t data, const char *vadbint, int status, int index, int size) "state %s data=0x%02x vADBInt=%s status=0x%x index=%d size=%d" > via1_adb_poll(uint8_t data, const char *vadbint, int status, int index, int size) "data=0x%02x vADBInt=%s status=0x%x index=%d size=%d" > +via1_adb_netbsd_enum_hack(void) "using NetBSD enum hack" > via1_auxmode(int mode) "setting auxmode to %d" > via1_timer_hack_state(int state) "setting timer_hack_state to %d" > Did you ask NetBSD to fix their code? Reviewed-by: Laurent Vivier <laurent@vivier.eu>
On 26/09/2023 09:04, Laurent Vivier wrote: > Le 09/09/2023 à 11:48, Mark Cave-Ayland a écrit : >> NetBSD assumes it can send its first ADB command after sending the ADB_BUSRESET >> command in ADB_STATE_NEW without changing the state back to ADB_STATE_IDLE >> first as detailed in the ADB protocol. >> >> Add a workaround to detect this condition at the start of ADB enumeration >> and send the next command written to SR after a ADB_BUSRESET onto the bus >> regardless, even if we don't detect a state transition to ADB_STATE_NEW. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/misc/mac_via.c | 34 ++++++++++++++++++++++++++++++++++ >> hw/misc/trace-events | 1 + >> 2 files changed, 35 insertions(+) >> >> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c >> index 766a32a95d..208216aed3 100644 >> --- a/hw/misc/mac_via.c >> +++ b/hw/misc/mac_via.c >> @@ -1001,6 +1001,8 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr >> addr, uint64_t val, >> { >> MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(opaque); >> MOS6522State *ms = MOS6522(v1s); >> + int oldstate, state; >> + int oldsr = ms->sr; >> addr = (addr >> 9) & 0xf; >> @@ -1016,6 +1018,38 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr >> addr, uint64_t val, >> v1s->last_b = ms->b; >> break; >> + >> + case VIA_REG_SR: >> + { >> + /* >> + * NetBSD assumes it can send its first ADB command after sending >> + * the ADB_BUSRESET command in ADB_STATE_NEW without changing the >> + * state back to ADB_STATE_IDLE first as detailed in the ADB >> + * protocol. >> + * >> + * Add a workaround to detect this condition at the start of ADB >> + * enumeration and send the next command written to SR after a >> + * ADB_BUSRESET onto the bus regardless, even if we don't detect a >> + * state transition to ADB_STATE_NEW. >> + * >> + * Note that in my tests the NetBSD state machine takes one ADB >> + * operation to recover which means the probe for an ADB device at >> + * address 1 always fails. However since the first device is at >> + * address 2 then this will work fine, without having to come up >> + * with a more complicated and invasive solution. >> + */ >> + oldstate = (v1s->last_b & VIA1B_vADB_StateMask) >> >> + VIA1B_vADB_StateShift; >> + state = (ms->b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift; >> + >> + if (oldstate == ADB_STATE_NEW && state == ADB_STATE_NEW && >> + (ms->acr & VIA1ACR_vShiftOut) && >> + oldsr == 0 /* ADB_BUSRESET */) { >> + trace_via1_adb_netbsd_enum_hack(); >> + adb_via_send(v1s, state, ms->sr); >> + } >> + } >> + break; >> } >> } >> diff --git a/hw/misc/trace-events b/hw/misc/trace-events >> index 0c9762fdf6..db8bb2d28a 100644 >> --- a/hw/misc/trace-events >> +++ b/hw/misc/trace-events >> @@ -271,6 +271,7 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, >> int value) "secto >> via1_adb_send(const char *state, uint8_t data, const char *vadbint) "state %s >> data=0x%02x vADBInt=%s" >> via1_adb_receive(const char *state, uint8_t data, const char *vadbint, int >> status, int index, int size) "state %s data=0x%02x vADBInt=%s status=0x%x index=%d >> size=%d" >> via1_adb_poll(uint8_t data, const char *vadbint, int status, int index, int size) >> "data=0x%02x vADBInt=%s status=0x%x index=%d size=%d" >> +via1_adb_netbsd_enum_hack(void) "using NetBSD enum hack" >> via1_auxmode(int mode) "setting auxmode to %d" >> via1_timer_hack_state(int state) "setting timer_hack_state to %d" > > Did you ask NetBSD to fix their code? Not yet. I'm fairly sure this is one of those undocumented behaviour edge cases which happens to work since the introduction of an MCU to interface onto the ADB bus. It may be that the MCU detects the incorrect write sequence and manages the bus reset on behalf of the host for some invalid sequences, but I don't have a way to easily find this out unfortunately. > Reviewed-by: Laurent Vivier <laurent@vivier.eu> ATB, Mark.
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c index 766a32a95d..208216aed3 100644 --- a/hw/misc/mac_via.c +++ b/hw/misc/mac_via.c @@ -1001,6 +1001,8 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val, { MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(opaque); MOS6522State *ms = MOS6522(v1s); + int oldstate, state; + int oldsr = ms->sr; addr = (addr >> 9) & 0xf; @@ -1016,6 +1018,38 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val, v1s->last_b = ms->b; break; + + case VIA_REG_SR: + { + /* + * NetBSD assumes it can send its first ADB command after sending + * the ADB_BUSRESET command in ADB_STATE_NEW without changing the + * state back to ADB_STATE_IDLE first as detailed in the ADB + * protocol. + * + * Add a workaround to detect this condition at the start of ADB + * enumeration and send the next command written to SR after a + * ADB_BUSRESET onto the bus regardless, even if we don't detect a + * state transition to ADB_STATE_NEW. + * + * Note that in my tests the NetBSD state machine takes one ADB + * operation to recover which means the probe for an ADB device at + * address 1 always fails. However since the first device is at + * address 2 then this will work fine, without having to come up + * with a more complicated and invasive solution. + */ + oldstate = (v1s->last_b & VIA1B_vADB_StateMask) >> + VIA1B_vADB_StateShift; + state = (ms->b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift; + + if (oldstate == ADB_STATE_NEW && state == ADB_STATE_NEW && + (ms->acr & VIA1ACR_vShiftOut) && + oldsr == 0 /* ADB_BUSRESET */) { + trace_via1_adb_netbsd_enum_hack(); + adb_via_send(v1s, state, ms->sr); + } + } + break; } } diff --git a/hw/misc/trace-events b/hw/misc/trace-events index 0c9762fdf6..db8bb2d28a 100644 --- a/hw/misc/trace-events +++ b/hw/misc/trace-events @@ -271,6 +271,7 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "secto via1_adb_send(const char *state, uint8_t data, const char *vadbint) "state %s data=0x%02x vADBInt=%s" via1_adb_receive(const char *state, uint8_t data, const char *vadbint, int status, int index, int size) "state %s data=0x%02x vADBInt=%s status=0x%x index=%d size=%d" via1_adb_poll(uint8_t data, const char *vadbint, int status, int index, int size) "data=0x%02x vADBInt=%s status=0x%x index=%d size=%d" +via1_adb_netbsd_enum_hack(void) "using NetBSD enum hack" via1_auxmode(int mode) "setting auxmode to %d" via1_timer_hack_state(int state) "setting timer_hack_state to %d"
NetBSD assumes it can send its first ADB command after sending the ADB_BUSRESET command in ADB_STATE_NEW without changing the state back to ADB_STATE_IDLE first as detailed in the ADB protocol. Add a workaround to detect this condition at the start of ADB enumeration and send the next command written to SR after a ADB_BUSRESET onto the bus regardless, even if we don't detect a state transition to ADB_STATE_NEW. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/misc/mac_via.c | 34 ++++++++++++++++++++++++++++++++++ hw/misc/trace-events | 1 + 2 files changed, 35 insertions(+)