Re: [PATCH v2 00/22] ADB: fix autopoll issues and rework mac_via state machine

[Laurent Vivier]

Le 23/06/2020 à 22:49, Mark Cave-Ayland a écrit :
> This patchset is something I have been chipping away at for a while since
> spending some time over the Christmas holidays trying to boot the MacOS
> toolbox ROM on the new q800 machine.
> 
> Initially I discovered that there were some problems when the MacOS ROM was
> enumerating ADB devices due to multiple meanings of the vADBInt bit. After
> fixing this there were still issues with keys being dropped during autopoll
> which were eventually traced back to the autopoll timer re-firing before
> the host had managed to read back the previous response.
> 
> At this point I noticed that CUDA/PMU/mac_via all had their own 
> implementations
> of ADB autopoll, and that it would make sense to consolidate the autopoll 
> timer,
> mask, interval and locking into the ADB bus. This would allow the logic to be
> removed from each separate device and managed in just one place.
> 
> Finally I updated the trace-events to allow separate tracing of bus requests
> and device responses which makes it easier to follow the ADB enumeration 
> process.
> 
> The breakdown of the patchset is as follows:
> 
> - Patch 1 keeps checkpatch happy for the remainder of the patchset whilst 
> patch
>   2 is the proper fix for a spurious ADB register 3 write during enumeration
>   caused by ignoring the request length which I had tried to work around 
> earlier.
> 
> - Patches 3 to 10 are part of the autopoll consolidation process which moves 
> the
>   separate autopoll implementations into a single implementation within
>   ADBBusState.
> 
> - Patches 11 to 13 update the ADB implementation to hold a status variable
>   indicating the result of the last request and allow devices to indicate
>   whether they have data to send. This extra information is required by the
>   upcoming mac_via state machine changes.
> 
> - Patches 14 to 17 add a variable and functions to block and unblock ADB
>   autopoll at bus level, adding the functions at the correct places within
>   CUDA and PMU.
> 
> - Patches 18 and 19 rework the mac_via ADB state machine so that the bus
>   can be enumerated correctly, and both explicit and autopoll requests work
>   under both MacOS and Linux.
> 
> - Patch 20 enforces the blocking and unblocking of autopoll at the ADB
>   level, including adding an assert() to prevent developers from trying to
>   make an ADB request whilst autopoll is in progress.
>   
> - Patches 21 and 22 update the trace-events to separate out ADB device and
>   ADB bus events.
> 
> The patch has been tested by myself and a couple of others during the 
> development
> process across the PPC g3beige/mac99 and 68K q800 machine so it should be 
> quite
> solid.
> 
> One thing to indicate is that the patchset bumps the VMState versions for the
> affected devices but does not allow older versions to load. This is a 
> conscious
> decision given that for the mac_via device used in the q800 machine it would 
> be
> just about impossible to map this in a way that would work for all cases. 
> Similarly
> for the Mac PPC machines migration is already hit/miss due to timebase issues 
> so
> I don't see this as being a big loss.
> 
> To finish off I'd also like to say a big thank-you to both Laurent Vivier and
> Finn Thain who both took time to answer my questions, dump information from a
> real q800, and analyse it in very fine detail. Without them this patchset 
> would
> still be several months away.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> 
> v2:
> - Rebased onto master
> - Added R-B tags from Philippe
> - Fixed byte discrepency at end of bus timeout spotted by Finn
> - Added Tested-by tag from Finn
> 
> 
> Mark Cave-Ayland (22):
>   adb: coding style update to fix checkpatch errors
>   adb: fix adb-mouse read length and revert disable-reg3-direct-writes
>     workaround
>   cuda: convert ADB autopoll timer from ns to ms
>   pmu: fix duplicate autopoll mask variable
>   pmu: honour autopoll_rate_ms when rearming the ADB autopoll timer
>   adb: introduce realize/unrealize and VMStateDescription for ADB bus
>   adb: create autopoll variables directly within ADBBusState
>   cuda: convert to use ADBBusState internal autopoll variables
>   pmu: convert to use ADBBusState internal autopoll variables
>   mac_via: convert to use ADBBusState internal autopoll variables
>   adb: introduce new ADBDeviceHasData method to ADBDeviceClass
>   adb: keep track of devices with pending data
>   adb: add status field for holding information about the last ADB
>     request
>   adb: use adb_request() only for explicit requests
>   adb: add autopoll_blocked variable to block autopoll
>   cuda: add adb_autopoll_block() and adb_autopoll_unblock() functions
>   pmu: add adb_autopoll_block() and adb_autopoll_unblock() functions
>   mac_via: move VIA1 portB write logic into mos6522_q800_via1_write()
>   mac_via: rework ADB state machine to be compatible with both MacOS and
>     Linux
>   adb: only call autopoll callbacks when autopoll is not blocked
>   adb: use adb_device prefix for ADB device trace events
>   adb: add ADB bus trace events
> 
>  hw/input/adb-kbd.c           |  42 ++--
>  hw/input/adb-mouse.c         |  65 ++++--
>  hw/input/adb.c               | 210 ++++++++++++++++--
>  hw/input/trace-events        |  27 ++-
>  hw/misc/mac_via.c            | 411 +++++++++++++++++++++++------------
>  hw/misc/macio/cuda.c         |  60 +++--
>  hw/misc/macio/pmu.c          |  47 ++--
>  hw/misc/trace-events         |   3 +
>  hw/ppc/mac_newworld.c        |   2 -
>  include/hw/input/adb.h       |  26 ++-
>  include/hw/misc/mac_via.h    |   2 +-
>  include/hw/misc/macio/cuda.h |   4 -
>  include/hw/misc/macio/pmu.h  |   4 -
>  13 files changed, 620 insertions(+), 283 deletions(-)
> 

Acked-by: Laurent Vivier <laurent@vivier.eu>