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

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

[Mark Cave-Ayland]

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>


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               | 212 +++++++++++++++++--
 hw/input/trace-events        |  27 ++-
 hw/misc/mac_via.c            | 399 ++++++++++++++++++++++-------------
 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, 608 insertions(+), 285 deletions(-)

-- 
2.20.1

[PATCH 01/22] adb: coding style update to fix checkpatch errors

[Mark Cave-Ayland]

This will help ensure that style guidelines are being maintained during
subsequent changes.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index b1ac4a3852..bf1bc30d19 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -44,14 +44,14 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
 
     cmd = buf[0] & 0xf;
     if (cmd == ADB_BUSRESET) {
-        for(i = 0; i < s->nb_devices; i++) {
+        for (i = 0; i < s->nb_devices; i++) {
             d = s->devices[i];
             adb_device_reset(d);
         }
         return 0;
     }
     devaddr = buf[0] >> 4;
-    for(i = 0; i < s->nb_devices; i++) {
+    for (i = 0; i < s->nb_devices; i++) {
         d = s->devices[i];
         if (d->devaddr == devaddr) {
             ADBDeviceClass *adc = ADB_DEVICE_GET_CLASS(d);
@@ -69,9 +69,10 @@ int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
     uint8_t buf[1];
 
     olen = 0;
-    for(i = 0; i < s->nb_devices; i++) {
-        if (s->poll_index >= s->nb_devices)
+    for (i = 0; i < s->nb_devices; i++) {
+        if (s->poll_index >= s->nb_devices) {
             s->poll_index = 0;
+        }
         d = s->devices[s->poll_index];
         if ((1 << d->devaddr) & poll_mask) {
             buf[0] = ADB_READREG | (d->devaddr << 4);
-- 
2.20.1

[PATCH 02/22] adb: fix adb-mouse read length and revert disable-reg3-direct-writes workaround

[Mark Cave-Ayland]

Commit 84051eb400 "adb: add property to disable direct reg 3 writes" introduced
a workaround for spurious writes to ADB register 3 when MacOS 9 enables
autopoll on the mouse device. Further analysis shows that the problem is that
only a partial request is sent, and since the len parameter is ignored then
stale data from the previous request is used causing the incorrect address
assignment.

Remove the disable-reg3-direct-writes workaround and instead check the length
parameter when the write is attempted, discarding the invalid request.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb-kbd.c     | 26 +++++++++++------------
 hw/input/adb-mouse.c   | 48 ++++++++++++++++++++++++------------------
 hw/input/adb.c         |  7 ------
 hw/ppc/mac_newworld.c  |  2 --
 include/hw/input/adb.h |  1 -
 5 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c
index a6d5c9b7c9..027dd3e531 100644
--- a/hw/input/adb-kbd.c
+++ b/hw/input/adb-kbd.c
@@ -259,21 +259,19 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
                 trace_adb_kbd_request_change_addr(d->devaddr);
                 break;
             default:
-                if (!d->disable_direct_reg3_writes) {
-                    d->devaddr = buf[1] & 0xf;
-
-                    /* we support handlers:
-                     * 1: Apple Standard Keyboard
-                     * 2: Apple Extended Keyboard (LShift = RShift)
-                     * 3: Apple Extended Keyboard (LShift != RShift)
-                     */
-                    if (buf[2] == 1 || buf[2] == 2 || buf[2] == 3) {
-                        d->handler = buf[2];
-                    }
-
-                    trace_adb_kbd_request_change_addr_and_handler(d->devaddr,
-                                                                  d->handler);
+                d->devaddr = buf[1] & 0xf;
+                /*
+                 * we support handlers:
+                 * 1: Apple Standard Keyboard
+                 * 2: Apple Extended Keyboard (LShift = RShift)
+                 * 3: Apple Extended Keyboard (LShift != RShift)
+                 */
+                if (buf[2] == 1 || buf[2] == 2 || buf[2] == 3) {
+                    d->handler = buf[2];
                 }
+
+                trace_adb_kbd_request_change_addr_and_handler(d->devaddr,
+                                                              d->handler);
                 break;
             }
         }
diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
index aeba41bddd..78b6f5030c 100644
--- a/hw/input/adb-mouse.c
+++ b/hw/input/adb-mouse.c
@@ -135,6 +135,16 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
         case 2:
             break;
         case 3:
+            /*
+             * MacOS 9 has a bug in its ADB driver whereby after configuring
+             * the ADB bus devices it sends another write of invalid length
+             * to reg 3. Make sure we ignore it to prevent an address clash
+             * with the previous device.
+             */
+            if (len != 3) {
+                return 0;
+            }
+
             switch (buf[2]) {
             case ADB_CMD_SELF_TEST:
                 break;
@@ -145,27 +155,25 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
                 trace_adb_mouse_request_change_addr(d->devaddr);
                 break;
             default:
-                if (!d->disable_direct_reg3_writes) {
-                    d->devaddr = buf[1] & 0xf;
-
-                    /* we support handlers:
-                     * 0x01: Classic Apple Mouse Protocol / 100 cpi operations
-                     * 0x02: Classic Apple Mouse Protocol / 200 cpi operations
-                     * we don't support handlers (at least):
-                     * 0x03: Mouse systems A3 trackball
-                     * 0x04: Extended Apple Mouse Protocol
-                     * 0x2f: Microspeed mouse
-                     * 0x42: Macally
-                     * 0x5f: Microspeed mouse
-                     * 0x66: Microspeed mouse
-                     */
-                    if (buf[2] == 1 || buf[2] == 2) {
-                        d->handler = buf[2];
-                    }
-
-                    trace_adb_mouse_request_change_addr_and_handler(
-                        d->devaddr, d->handler);
+                d->devaddr = buf[1] & 0xf;
+                /*
+                 * we support handlers:
+                 * 0x01: Classic Apple Mouse Protocol / 100 cpi operations
+                 * 0x02: Classic Apple Mouse Protocol / 200 cpi operations
+                 * we don't support handlers (at least):
+                 * 0x03: Mouse systems A3 trackball
+                 * 0x04: Extended Apple Mouse Protocol
+                 * 0x2f: Microspeed mouse
+                 * 0x42: Macally
+                 * 0x5f: Microspeed mouse
+                 * 0x66: Microspeed mouse
+                 */
+                if (buf[2] == 1 || buf[2] == 2) {
+                    d->handler = buf[2];
                 }
+
+                trace_adb_mouse_request_change_addr_and_handler(d->devaddr,
+                                                                d->handler);
                 break;
             }
         }
diff --git a/hw/input/adb.c b/hw/input/adb.c
index bf1bc30d19..d85278a7b7 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -118,18 +118,11 @@ static void adb_device_realizefn(DeviceState *dev, Error **errp)
     bus->devices[bus->nb_devices++] = d;
 }
 
-static Property adb_device_properties[] = {
-    DEFINE_PROP_BOOL("disable-direct-reg3-writes", ADBDevice,
-                     disable_direct_reg3_writes, false),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void adb_device_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     dc->realize = adb_device_realizefn;
-    device_class_set_props(dc, adb_device_properties);
     dc->bus_type = TYPE_ADB_BUS;
 }
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 3507f26f6e..12a2864341 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -404,11 +404,9 @@ static void ppc_core99_init(MachineState *machine)
 
         adb_bus = qdev_get_child_bus(dev, "adb.0");
         dev = qdev_create(adb_bus, TYPE_ADB_KEYBOARD);
-        qdev_prop_set_bit(dev, "disable-direct-reg3-writes", true);
         qdev_init_nofail(dev);
 
         dev = qdev_create(adb_bus, TYPE_ADB_MOUSE);
-        qdev_prop_set_bit(dev, "disable-direct-reg3-writes", true);
         qdev_init_nofail(dev);
     }
 
diff --git a/include/hw/input/adb.h b/include/hw/input/adb.h
index b7b32e2b16..4d2c565f54 100644
--- a/include/hw/input/adb.h
+++ b/include/hw/input/adb.h
@@ -49,7 +49,6 @@ struct ADBDevice {
 
     int devaddr;
     int handler;
-    bool disable_direct_reg3_writes;
 };
 
 #define ADB_DEVICE_CLASS(cls) \
-- 
2.20.1

[PATCH 03/22] cuda: convert ADB autopoll timer from ns to ms

[Mark Cave-Ayland]

This is in preparation for consolidating all of the ADB autopoll management
in one place.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/cuda.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index e0cc0aac5d..a407f2abc8 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -208,8 +208,9 @@ static void cuda_adb_poll(void *opaque)
         obuf[1] = 0x40; /* polled data */
         cuda_send_packet_to_host(s, obuf, olen + 2);
     }
-    timer_mod(s->adb_poll_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-              (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
+
+    timer_mod(s->adb_poll_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+              s->autopoll_rate_ms);
 }
 
 /* description of commands */
@@ -236,8 +237,8 @@ static bool cuda_cmd_autopoll(CUDAState *s,
         s->autopoll = autopoll;
         if (autopoll) {
             timer_mod(s->adb_poll_timer,
-                      qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-                      (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
+                      qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+                      s->autopoll_rate_ms);
         } else {
             timer_del(s->adb_poll_timer);
         }
@@ -262,8 +263,8 @@ static bool cuda_cmd_set_autorate(CUDAState *s,
     s->autopoll_rate_ms = in_data[0];
     if (s->autopoll) {
         timer_mod(s->adb_poll_timer,
-                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-                  (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
+                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+                  s->autopoll_rate_ms);
     }
     return true;
 }
@@ -539,7 +540,7 @@ static void cuda_realize(DeviceState *dev, Error **errp)
     s->sr_delay_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_set_sr_int, s);
     s->sr_delay_ns = 20 * SCALE_US;
 
-    s->adb_poll_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_adb_poll, s);
+    s->adb_poll_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, cuda_adb_poll, s);
     s->adb_poll_mask = 0xffff;
     s->autopoll_rate_ms = 20;
 }
-- 
2.20.1

[PATCH 04/22] pmu: fix duplicate autopoll mask variable

[Mark Cave-Ayland]

It seems that during the initial work to introduce the via-pmu ADB support a
duplicate autopoll mask variable was accidentally left in place.

Remove the duplicate autopoll_mask variable and switch everything over to
use adb_poll_mask instead.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/pmu.c         | 15 +++++++--------
 include/hw/misc/macio/pmu.h |  1 -
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 9a9cd427e1..3af4ef1a04 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -173,11 +173,11 @@ static void pmu_cmd_set_adb_autopoll(PMUState *s, uint16_t mask)
 {
     trace_pmu_cmd_set_adb_autopoll(mask);
 
-    if (s->autopoll_mask == mask) {
+    if (s->adb_poll_mask == mask) {
         return;
     }
 
-    s->autopoll_mask = mask;
+    s->adb_poll_mask = mask;
     if (mask) {
         timer_mod(s->adb_poll_timer,
                   qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 30);
@@ -272,9 +272,9 @@ static void pmu_cmd_adb_poll_off(PMUState *s,
         return;
     }
 
-    if (s->has_adb && s->autopoll_mask) {
+    if (s->has_adb && s->adb_poll_mask) {
         timer_del(s->adb_poll_timer);
-        s->autopoll_mask = false;
+        s->adb_poll_mask = 0;
     }
 }
 
@@ -696,8 +696,8 @@ static const VMStateDescription vmstate_pmu_adb = {
 
 static const VMStateDescription vmstate_pmu = {
     .name = "pmu",
-    .version_id = 0,
-    .minimum_version_id = 0,
+    .version_id = 1,
+    .minimum_version_id = 1,
     .fields = (VMStateField[]) {
         VMSTATE_STRUCT(mos6522_pmu.parent_obj, PMUState, 0, vmstate_mos6522,
                        MOS6522State),
@@ -713,7 +713,6 @@ static const VMStateDescription vmstate_pmu = {
         VMSTATE_UINT8(intbits, PMUState),
         VMSTATE_UINT8(intmask, PMUState),
         VMSTATE_UINT8(autopoll_rate_ms, PMUState),
-        VMSTATE_UINT8(autopoll_mask, PMUState),
         VMSTATE_UINT32(tick_offset, PMUState),
         VMSTATE_TIMER_PTR(one_sec_timer, PMUState),
         VMSTATE_INT64(one_sec_target, PMUState),
@@ -733,7 +732,7 @@ static void pmu_reset(DeviceState *dev)
     s->intbits = 0;
 
     s->cmd_state = pmu_state_idle;
-    s->autopoll_mask = 0;
+    s->adb_poll_mask = 0;
 }
 
 static void pmu_realize(DeviceState *dev, Error **errp)
diff --git a/include/hw/misc/macio/pmu.h b/include/hw/misc/macio/pmu.h
index 7ef83dee4c..4f34b6f9e7 100644
--- a/include/hw/misc/macio/pmu.h
+++ b/include/hw/misc/macio/pmu.h
@@ -220,7 +220,6 @@ typedef struct PMUState {
     ADBBusState adb_bus;
     uint16_t adb_poll_mask;
     uint8_t autopoll_rate_ms;
-    uint8_t autopoll_mask;
     QEMUTimer *adb_poll_timer;
     uint8_t adb_reply_size;
     uint8_t adb_reply[ADB_MAX_OUT_LEN];
-- 
2.20.1

[PATCH 05/22] pmu: honour autopoll_rate_ms when rearming the ADB autopoll timer

[Mark Cave-Ayland]

Don't use a fixed value but instead use the default value from the ADB bus
state.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 3af4ef1a04..edb061e417 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -104,7 +104,7 @@ static void pmu_adb_poll(void *opaque)
     }
 
     timer_mod(s->adb_poll_timer,
-              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 30);
+              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + s->autopoll_rate_ms);
 }
 
 static void pmu_one_sec_timer(void *opaque)
@@ -180,7 +180,7 @@ static void pmu_cmd_set_adb_autopoll(PMUState *s, uint16_t mask)
     s->adb_poll_mask = mask;
     if (mask) {
         timer_mod(s->adb_poll_timer,
-                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 30);
+                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + s->autopoll_rate_ms);
     } else {
         timer_del(s->adb_poll_timer);
     }
-- 
2.20.1

[PATCH 06/22] adb: introduce realize/unrealize and VMStateDescription for ADB bus

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index d85278a7b7..21a9b3aa96 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -89,10 +89,42 @@ int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
     return olen;
 }
 
+static const VMStateDescription vmstate_adb_bus = {
+    .name = "adb_bus",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void adb_bus_realize(BusState *qbus, Error **errp)
+{
+    ADBBusState *adb_bus = ADB_BUS(qbus);
+
+    vmstate_register(NULL, -1, &vmstate_adb_bus, adb_bus);
+}
+
+static void adb_bus_unrealize(BusState *qbus)
+{
+    ADBBusState *adb_bus = ADB_BUS(qbus);
+
+    vmstate_unregister(NULL, &vmstate_adb_bus, adb_bus);
+}
+
+static void adb_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->realize = adb_bus_realize;
+    k->unrealize = adb_bus_unrealize;
+}
+
 static const TypeInfo adb_bus_type_info = {
     .name = TYPE_ADB_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(ADBBusState),
+    .class_init = adb_bus_class_init,
 };
 
 const VMStateDescription vmstate_adb_device = {
-- 
2.20.1

[PATCH 07/22] adb: create autopoll variables directly within ADBBusState

[Mark Cave-Ayland]

Rather than each ADB implementation requiring its own functions to manage
autopoll state, timers, and autopoll masks prepare to move this information
directly into ADBBusState.

Add external functions within adb.h to allow each ADB implementation to
manage the new autopoll variables.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c         | 77 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/input/adb.h | 13 +++++++
 2 files changed, 90 insertions(+)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index 21a9b3aa96..bb36ce6fad 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -27,6 +27,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
+#include "qemu/timer.h"
 #include "adb-internal.h"
 
 /* error codes */
@@ -89,19 +90,92 @@ int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
     return olen;
 }
 
+void adb_set_autopoll_enabled(ADBBusState *s, bool enabled)
+{
+    if (s->autopoll_enabled != enabled) {
+        s->autopoll_enabled = enabled;
+        if (s->autopoll_enabled) {
+            timer_mod(s->autopoll_timer,
+                      qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+                      s->autopoll_rate_ms);
+        } else {
+            timer_del(s->autopoll_timer);
+        }
+    }
+}
+
+void adb_set_autopoll_rate_ms(ADBBusState *s, int rate_ms)
+{
+    s->autopoll_rate_ms = rate_ms;
+
+    if (s->autopoll_enabled) {
+        timer_mod(s->autopoll_timer,
+                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+                  s->autopoll_rate_ms);
+    }
+}
+
+void adb_set_autopoll_mask(ADBBusState *s, uint16_t mask)
+{
+    if (s->autopoll_mask != mask) {
+        s->autopoll_mask = mask;
+        if (s->autopoll_enabled && s->autopoll_mask) {
+            timer_mod(s->autopoll_timer,
+                      qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+                      s->autopoll_rate_ms);
+        } else {
+            timer_del(s->autopoll_timer);
+        }
+    }
+}
+
+static void adb_autopoll(void *opaque)
+{
+    ADBBusState *s = opaque;
+
+    s->autopoll_cb(s->autopoll_cb_opaque);
+
+    timer_mod(s->autopoll_timer,
+              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+              s->autopoll_rate_ms);
+}
+
+void adb_register_autopoll_callback(ADBBusState *s, void (*cb)(void *opaque),
+                                    void *opaque)
+{
+    s->autopoll_cb = cb;
+    s->autopoll_cb_opaque = opaque;
+}
+
 static const VMStateDescription vmstate_adb_bus = {
     .name = "adb_bus",
     .version_id = 0,
     .minimum_version_id = 0,
     .fields = (VMStateField[]) {
+        VMSTATE_TIMER_PTR(autopoll_timer, ADBBusState),
+        VMSTATE_BOOL(autopoll_enabled, ADBBusState),
+        VMSTATE_UINT8(autopoll_rate_ms, ADBBusState),
+        VMSTATE_UINT16(autopoll_mask, ADBBusState),
         VMSTATE_END_OF_LIST()
     }
 };
 
+static void adb_bus_reset(BusState *qbus)
+{
+    ADBBusState *adb_bus = ADB_BUS(qbus);
+
+    adb_bus->autopoll_enabled = false;
+    adb_bus->autopoll_mask = 0xffff;
+    adb_bus->autopoll_rate_ms = 20;
+}
+
 static void adb_bus_realize(BusState *qbus, Error **errp)
 {
     ADBBusState *adb_bus = ADB_BUS(qbus);
 
+    adb_bus->autopoll_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, adb_autopoll,
+                                           adb_bus);
+
     vmstate_register(NULL, -1, &vmstate_adb_bus, adb_bus);
 }
 
@@ -109,6 +183,8 @@ static void adb_bus_unrealize(BusState *qbus)
 {
     ADBBusState *adb_bus = ADB_BUS(qbus);
 
+    timer_del(adb_bus->autopoll_timer);
+
     vmstate_unregister(NULL, &vmstate_adb_bus, adb_bus);
 }
 
@@ -118,6 +194,7 @@ static void adb_bus_class_init(ObjectClass *klass, void *data)
 
     k->realize = adb_bus_realize;
     k->unrealize = adb_bus_unrealize;
+    k->reset = adb_bus_reset;
 }
 
 static const TypeInfo adb_bus_type_info = {
diff --git a/include/hw/input/adb.h b/include/hw/input/adb.h
index 4d2c565f54..15b1874a3d 100644
--- a/include/hw/input/adb.h
+++ b/include/hw/input/adb.h
@@ -75,12 +75,25 @@ struct ADBBusState {
     ADBDevice *devices[MAX_ADB_DEVICES];
     int nb_devices;
     int poll_index;
+
+    QEMUTimer *autopoll_timer;
+    bool autopoll_enabled;
+    uint8_t autopoll_rate_ms;
+    uint16_t autopoll_mask;
+    void (*autopoll_cb)(void *opaque);
+    void *autopoll_cb_opaque;
 };
 
 int adb_request(ADBBusState *s, uint8_t *buf_out,
                 const uint8_t *buf, int len);
 int adb_poll(ADBBusState *s, uint8_t *buf_out, uint16_t poll_mask);
 
+void adb_set_autopoll_enabled(ADBBusState *s, bool enabled);
+void adb_set_autopoll_rate_ms(ADBBusState *s, int rate_ms);
+void adb_set_autopoll_mask(ADBBusState *s, uint16_t mask);
+void adb_register_autopoll_callback(ADBBusState *s, void (*cb)(void *opaque),
+                                    void *opaque);
+
 #define TYPE_ADB_KEYBOARD "adb-keyboard"
 #define TYPE_ADB_MOUSE "adb-mouse"
 
-- 
2.20.1

[PATCH 08/22] cuda: convert to use ADBBusState internal autopoll variables

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/cuda.c         | 56 +++++++++++++++---------------------
 include/hw/misc/macio/cuda.h |  4 ---
 2 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index a407f2abc8..716866ea34 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -199,18 +199,16 @@ static void cuda_send_packet_to_host(CUDAState *s,
 static void cuda_adb_poll(void *opaque)
 {
     CUDAState *s = opaque;
+    ADBBusState *adb_bus = &s->adb_bus;
     uint8_t obuf[ADB_MAX_OUT_LEN + 2];
     int olen;
 
-    olen = adb_poll(&s->adb_bus, obuf + 2, s->adb_poll_mask);
+    olen = adb_poll(adb_bus, obuf + 2, adb_bus->autopoll_mask);
     if (olen > 0) {
         obuf[0] = ADB_PACKET;
         obuf[1] = 0x40; /* polled data */
         cuda_send_packet_to_host(s, obuf, olen + 2);
     }
-
-    timer_mod(s->adb_poll_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
-              s->autopoll_rate_ms);
 }
 
 /* description of commands */
@@ -226,23 +224,16 @@ static bool cuda_cmd_autopoll(CUDAState *s,
                               const uint8_t *in_data, int in_len,
                               uint8_t *out_data, int *out_len)
 {
-    int autopoll;
+    ADBBusState *adb_bus = &s->adb_bus;
+    bool autopoll;
 
     if (in_len != 1) {
         return false;
     }
 
-    autopoll = (in_data[0] != 0);
-    if (autopoll != s->autopoll) {
-        s->autopoll = autopoll;
-        if (autopoll) {
-            timer_mod(s->adb_poll_timer,
-                      qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
-                      s->autopoll_rate_ms);
-        } else {
-            timer_del(s->adb_poll_timer);
-        }
-    }
+    autopoll = (in_data[0] != 0) ? true : false;
+
+    adb_set_autopoll_enabled(adb_bus, autopoll);
     return true;
 }
 
@@ -250,6 +241,8 @@ static bool cuda_cmd_set_autorate(CUDAState *s,
                                   const uint8_t *in_data, int in_len,
                                   uint8_t *out_data, int *out_len)
 {
+    ADBBusState *adb_bus = &s->adb_bus;
+
     if (in_len != 1) {
         return false;
     }
@@ -260,12 +253,7 @@ static bool cuda_cmd_set_autorate(CUDAState *s,
         return false;
     }
 
-    s->autopoll_rate_ms = in_data[0];
-    if (s->autopoll) {
-        timer_mod(s->adb_poll_timer,
-                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
-                  s->autopoll_rate_ms);
-    }
+    adb_set_autopoll_rate_ms(adb_bus, in_data[0]);
     return true;
 }
 
@@ -273,11 +261,16 @@ static bool cuda_cmd_set_device_list(CUDAState *s,
                                      const uint8_t *in_data, int in_len,
                                      uint8_t *out_data, int *out_len)
 {
+    ADBBusState *adb_bus = &s->adb_bus;
+    uint16_t mask;
+
     if (in_len != 2) {
         return false;
     }
 
-    s->adb_poll_mask = (((uint16_t)in_data[0]) << 8) | in_data[1];
+    mask = (((uint16_t)in_data[0]) << 8) | in_data[1];
+
+    adb_set_autopoll_mask(adb_bus, mask);
     return true;
 }
 
@@ -488,8 +481,8 @@ static const MemoryRegionOps mos6522_cuda_ops = {
 
 static const VMStateDescription vmstate_cuda = {
     .name = "cuda",
-    .version_id = 5,
-    .minimum_version_id = 5,
+    .version_id = 6,
+    .minimum_version_id = 6,
     .fields = (VMStateField[]) {
         VMSTATE_STRUCT(mos6522_cuda.parent_obj, CUDAState, 0, vmstate_mos6522,
                        MOS6522State),
@@ -498,13 +491,9 @@ static const VMStateDescription vmstate_cuda = {
         VMSTATE_INT32(data_in_size, CUDAState),
         VMSTATE_INT32(data_in_index, CUDAState),
         VMSTATE_INT32(data_out_index, CUDAState),
-        VMSTATE_UINT8(autopoll, CUDAState),
-        VMSTATE_UINT8(autopoll_rate_ms, CUDAState),
-        VMSTATE_UINT16(adb_poll_mask, CUDAState),
         VMSTATE_BUFFER(data_in, CUDAState),
         VMSTATE_BUFFER(data_out, CUDAState),
         VMSTATE_UINT32(tick_offset, CUDAState),
-        VMSTATE_TIMER_PTR(adb_poll_timer, CUDAState),
         VMSTATE_TIMER_PTR(sr_delay_timer, CUDAState),
         VMSTATE_END_OF_LIST()
     }
@@ -513,11 +502,13 @@ static const VMStateDescription vmstate_cuda = {
 static void cuda_reset(DeviceState *dev)
 {
     CUDAState *s = CUDA(dev);
+    ADBBusState *adb_bus = &s->adb_bus;
 
     s->data_in_size = 0;
     s->data_in_index = 0;
     s->data_out_index = 0;
-    s->autopoll = 0;
+
+    adb_set_autopoll_enabled(adb_bus, false);
 }
 
 static void cuda_realize(DeviceState *dev, Error **errp)
@@ -526,6 +517,7 @@ static void cuda_realize(DeviceState *dev, Error **errp)
     SysBusDevice *sbd;
     MOS6522State *ms;
     DeviceState *d;
+    ADBBusState *adb_bus = &s->adb_bus;
     struct tm tm;
 
     /* Pass IRQ from 6522 */
@@ -540,9 +532,7 @@ static void cuda_realize(DeviceState *dev, Error **errp)
     s->sr_delay_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_set_sr_int, s);
     s->sr_delay_ns = 20 * SCALE_US;
 
-    s->adb_poll_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, cuda_adb_poll, s);
-    s->adb_poll_mask = 0xffff;
-    s->autopoll_rate_ms = 20;
+    adb_register_autopoll_callback(adb_bus, cuda_adb_poll, s);
 }
 
 static void cuda_init(Object *obj)
diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
index 5768075ac5..a8cf0be1ec 100644
--- a/include/hw/misc/macio/cuda.h
+++ b/include/hw/misc/macio/cuda.h
@@ -95,12 +95,8 @@ typedef struct CUDAState {
     int data_out_index;
 
     qemu_irq irq;
-    uint16_t adb_poll_mask;
-    uint8_t autopoll_rate_ms;
-    uint8_t autopoll;
     uint8_t data_in[128];
     uint8_t data_out[16];
-    QEMUTimer *adb_poll_timer;
 } CUDAState;
 
 #endif /* CUDA_H */
-- 
2.20.1

[PATCH 09/22] pmu: convert to use ADBBusState internal autopoll variables

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/pmu.c         | 39 ++++++++++++++-----------------------
 include/hw/misc/macio/pmu.h |  3 ---
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index edb061e417..e2291cc9f6 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -90,10 +90,11 @@ static void pmu_update_extirq(PMUState *s)
 static void pmu_adb_poll(void *opaque)
 {
     PMUState *s = opaque;
+    ADBBusState *adb_bus = &s->adb_bus;
     int olen;
 
     if (!(s->intbits & PMU_INT_ADB)) {
-        olen = adb_poll(&s->adb_bus, s->adb_reply, s->adb_poll_mask);
+        olen = adb_poll(adb_bus, s->adb_reply, adb_bus->autopoll_mask);
         trace_pmu_adb_poll(olen);
 
         if (olen > 0) {
@@ -102,9 +103,6 @@ static void pmu_adb_poll(void *opaque)
             pmu_update_extirq(s);
         }
     }
-
-    timer_mod(s->adb_poll_timer,
-              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + s->autopoll_rate_ms);
 }
 
 static void pmu_one_sec_timer(void *opaque)
@@ -171,18 +169,15 @@ static void pmu_cmd_set_int_mask(PMUState *s,
 
 static void pmu_cmd_set_adb_autopoll(PMUState *s, uint16_t mask)
 {
-    trace_pmu_cmd_set_adb_autopoll(mask);
+    ADBBusState *adb_bus = &s->adb_bus;
 
-    if (s->adb_poll_mask == mask) {
-        return;
-    }
+    trace_pmu_cmd_set_adb_autopoll(mask);
 
-    s->adb_poll_mask = mask;
     if (mask) {
-        timer_mod(s->adb_poll_timer,
-                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + s->autopoll_rate_ms);
+        adb_set_autopoll_mask(adb_bus, mask);
+        adb_set_autopoll_enabled(adb_bus, true);
     } else {
-        timer_del(s->adb_poll_timer);
+        adb_set_autopoll_enabled(adb_bus, false);
     }
 }
 
@@ -265,6 +260,8 @@ static void pmu_cmd_adb_poll_off(PMUState *s,
                                  const uint8_t *in_data, uint8_t in_len,
                                  uint8_t *out_data, uint8_t *out_len)
 {
+    ADBBusState *adb_bus = &s->adb_bus;
+
     if (in_len != 0) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "PMU: ADB POLL OFF command, invalid len: %d want: 0\n",
@@ -272,9 +269,8 @@ static void pmu_cmd_adb_poll_off(PMUState *s,
         return;
     }
 
-    if (s->has_adb && s->adb_poll_mask) {
-        timer_del(s->adb_poll_timer);
-        s->adb_poll_mask = 0;
+    if (s->has_adb) {
+        adb_set_autopoll_enabled(adb_bus, false);
     }
 }
 
@@ -682,12 +678,10 @@ static bool pmu_adb_state_needed(void *opaque)
 
 static const VMStateDescription vmstate_pmu_adb = {
     .name = "pmu/adb",
-    .version_id = 0,
-    .minimum_version_id = 0,
+    .version_id = 1,
+    .minimum_version_id = 1,
     .needed = pmu_adb_state_needed,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT16(adb_poll_mask, PMUState),
-        VMSTATE_TIMER_PTR(adb_poll_timer, PMUState),
         VMSTATE_UINT8(adb_reply_size, PMUState),
         VMSTATE_BUFFER(adb_reply, PMUState),
         VMSTATE_END_OF_LIST()
@@ -712,7 +706,6 @@ static const VMStateDescription vmstate_pmu = {
         VMSTATE_BUFFER(cmd_rsp, PMUState),
         VMSTATE_UINT8(intbits, PMUState),
         VMSTATE_UINT8(intmask, PMUState),
-        VMSTATE_UINT8(autopoll_rate_ms, PMUState),
         VMSTATE_UINT32(tick_offset, PMUState),
         VMSTATE_TIMER_PTR(one_sec_timer, PMUState),
         VMSTATE_INT64(one_sec_target, PMUState),
@@ -732,7 +725,6 @@ static void pmu_reset(DeviceState *dev)
     s->intbits = 0;
 
     s->cmd_state = pmu_state_idle;
-    s->adb_poll_mask = 0;
 }
 
 static void pmu_realize(DeviceState *dev, Error **errp)
@@ -741,6 +733,7 @@ static void pmu_realize(DeviceState *dev, Error **errp)
     SysBusDevice *sbd;
     MOS6522State *ms;
     DeviceState *d;
+    ADBBusState *adb_bus = &s->adb_bus;
     struct tm tm;
 
     /* Pass IRQ from 6522 */
@@ -758,9 +751,7 @@ static void pmu_realize(DeviceState *dev, Error **errp)
     if (s->has_adb) {
         qbus_create_inplace(&s->adb_bus, sizeof(s->adb_bus), TYPE_ADB_BUS,
                             dev, "adb.0");
-        s->adb_poll_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, pmu_adb_poll, s);
-        s->adb_poll_mask = 0xffff;
-        s->autopoll_rate_ms = 20;
+        adb_register_autopoll_callback(adb_bus, pmu_adb_poll, s);
     }
 }
 
diff --git a/include/hw/misc/macio/pmu.h b/include/hw/misc/macio/pmu.h
index 4f34b6f9e7..72f75612b6 100644
--- a/include/hw/misc/macio/pmu.h
+++ b/include/hw/misc/macio/pmu.h
@@ -218,9 +218,6 @@ typedef struct PMUState {
     /* ADB */
     bool has_adb;
     ADBBusState adb_bus;
-    uint16_t adb_poll_mask;
-    uint8_t autopoll_rate_ms;
-    QEMUTimer *adb_poll_timer;
     uint8_t adb_reply_size;
     uint8_t adb_reply[ADB_MAX_OUT_LEN];
 
-- 
2.20.1

[PATCH 10/22] mac_via: convert to use ADBBusState internal autopoll variables

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/mac_via.c         | 22 ++++++++++------------
 include/hw/misc/mac_via.h |  1 -
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index e05623d730..669fdca4c4 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -601,6 +601,8 @@ static void via1_rtc_update(MacVIAState *m)
 
 static int adb_via_poll(MacVIAState *s, int state, uint8_t *data)
 {
+    ADBBusState *adb_bus = &s->adb_bus;
+
     if (state != ADB_STATE_IDLE) {
         return 0;
     }
@@ -615,7 +617,8 @@ static int adb_via_poll(MacVIAState *s, int state, uint8_t *data)
 
     s->adb_data_in_index = 0;
     s->adb_data_out_index = 0;
-    s->adb_data_in_size = adb_poll(&s->adb_bus, s->adb_data_in, 0xffff);
+    s->adb_data_in_size = adb_poll(adb_bus, s->adb_data_in,
+                                   adb_bus->autopoll_mask);
 
     if (s->adb_data_in_size) {
         *data = s->adb_data_in[s->adb_data_in_index++];
@@ -768,10 +771,6 @@ static void via_adb_poll(void *opaque)
             s->b &= ~VIA1B_vADBInt;
         }
     }
-
-    timer_mod(m->adb_poll_timer,
-              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-              (NANOSECONDS_PER_SECOND / VIA_ADB_POLL_FREQ));
 }
 
 static uint64_t mos6522_q800_via1_read(void *opaque, hwaddr addr, unsigned size)
@@ -854,10 +853,9 @@ static void mac_via_reset(DeviceState *dev)
 {
     MacVIAState *m = MAC_VIA(dev);
     MOS6522Q800VIA1State *v1s = &m->mos6522_via1;
+    ADBBusState *adb_bus = &m->adb_bus;
 
-    timer_mod(m->adb_poll_timer,
-              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-              (NANOSECONDS_PER_SECOND / VIA_ADB_POLL_FREQ));
+    adb_set_autopoll_enabled(adb_bus, true);
 
     timer_del(v1s->VBL_timer);
     v1s->next_VBL = 0;
@@ -872,6 +870,7 @@ static void mac_via_realize(DeviceState *dev, Error **errp)
 {
     MacVIAState *m = MAC_VIA(dev);
     MOS6522State *ms;
+    ADBBusState *adb_bus = &m->adb_bus;
     struct tm tm;
     int ret;
 
@@ -904,7 +903,7 @@ static void mac_via_realize(DeviceState *dev, Error **errp)
     qemu_get_timedate(&tm, 0);
     m->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
 
-    m->adb_poll_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, via_adb_poll, m);
+    adb_register_autopoll_callback(adb_bus, via_adb_poll, m);
     m->adb_data_ready = qdev_get_gpio_in_named(dev, "via1-irq",
                                                VIA1_IRQ_ADB_READY_BIT);
 
@@ -977,8 +976,8 @@ static int mac_via_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_mac_via = {
     .name = "mac-via",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .post_load = mac_via_post_load,
     .fields = (VMStateField[]) {
         /* VIAs */
@@ -1002,7 +1001,6 @@ static const VMStateDescription vmstate_mac_via = {
         VMSTATE_INT32(wprotect, MacVIAState),
         VMSTATE_INT32(alt, MacVIAState),
         /* ADB */
-        VMSTATE_TIMER_PTR(adb_poll_timer, MacVIAState),
         VMSTATE_INT32(adb_data_in_size, MacVIAState),
         VMSTATE_INT32(adb_data_in_index, MacVIAState),
         VMSTATE_INT32(adb_data_out_index, MacVIAState),
diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index e74f85be0f..2aaf9e27bf 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -106,7 +106,6 @@ typedef struct MacVIAState {
 
     /* ADB */
     ADBBusState adb_bus;
-    QEMUTimer *adb_poll_timer;
     qemu_irq adb_data_ready;
     int adb_data_in_size;
     int adb_data_in_index;
-- 
2.20.1

[PATCH 11/22] adb: introduce new ADBDeviceHasData method to ADBDeviceClass

[Mark Cave-Ayland]

This is required later to allow devices to assert a service request (SRQ)
signal to indicate that it has data to send, without having to consume it.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb-kbd.c     | 8 ++++++++
 hw/input/adb-mouse.c   | 9 +++++++++
 include/hw/input/adb.h | 3 +++
 3 files changed, 20 insertions(+)

diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c
index 027dd3e531..23760ecf7b 100644
--- a/hw/input/adb-kbd.c
+++ b/hw/input/adb-kbd.c
@@ -300,6 +300,13 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
     return olen;
 }
 
+static bool adb_kbd_has_data(ADBDevice *d)
+{
+    KBDState *s = ADB_KEYBOARD(d);
+
+    return s->count > 0;
+}
+
 /* This is where keyboard events enter this file */
 static void adb_keyboard_event(DeviceState *dev, QemuConsole *src,
                                InputEvent *evt)
@@ -382,6 +389,7 @@ static void adb_kbd_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 
     adc->devreq = adb_kbd_request;
+    adc->devhasdata = adb_kbd_has_data;
     dc->reset = adb_kbd_reset;
     dc->vmsd = &vmstate_adb_kbd;
 }
diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
index 78b6f5030c..e2359fd74d 100644
--- a/hw/input/adb-mouse.c
+++ b/hw/input/adb-mouse.c
@@ -197,6 +197,14 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
     return olen;
 }
 
+static bool adb_mouse_has_data(ADBDevice *d)
+{
+    MouseState *s = ADB_MOUSE(d);
+
+    return !(s->last_buttons_state == s->buttons_state &&
+             s->dx == 0 && s->dy == 0);
+}
+
 static void adb_mouse_reset(DeviceState *dev)
 {
     ADBDevice *d = ADB_DEVICE(dev);
@@ -252,6 +260,7 @@ static void adb_mouse_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 
     adc->devreq = adb_mouse_request;
+    adc->devhasdata = adb_mouse_has_data;
     dc->reset = adb_mouse_reset;
     dc->vmsd = &vmstate_adb_mouse;
 }
diff --git a/include/hw/input/adb.h b/include/hw/input/adb.h
index 15b1874a3d..9b80204e43 100644
--- a/include/hw/input/adb.h
+++ b/include/hw/input/adb.h
@@ -39,6 +39,8 @@ typedef struct ADBDevice ADBDevice;
 typedef int ADBDeviceRequest(ADBDevice *d, uint8_t *buf_out,
                               const uint8_t *buf, int len);
 
+typedef bool ADBDeviceHasData(ADBDevice *d);
+
 #define TYPE_ADB_DEVICE "adb-device"
 #define ADB_DEVICE(obj) OBJECT_CHECK(ADBDevice, (obj), TYPE_ADB_DEVICE)
 
@@ -62,6 +64,7 @@ typedef struct ADBDeviceClass {
     /*< public >*/
 
     ADBDeviceRequest *devreq;
+    ADBDeviceHasData *devhasdata;
 } ADBDeviceClass;
 
 #define TYPE_ADB_BUS "apple-desktop-bus"
-- 
2.20.1

[PATCH 12/22] adb: keep track of devices with pending data

[Mark Cave-Ayland]

Add a new pending variable to ADBBusState which is a bitmask indicating which
ADB devices have data to send. Update the bitmask every time that an ADB
request is executed.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c         | 16 +++++++++++++++-
 include/hw/input/adb.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index bb36ce6fad..c1adb21e6b 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -41,6 +41,7 @@ static void adb_device_reset(ADBDevice *d)
 int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
 {
     ADBDevice *d;
+    ADBDeviceClass *adc;
     int devaddr, cmd, i;
 
     cmd = buf[0] & 0xf;
@@ -51,14 +52,27 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
         }
         return 0;
     }
+
+    s->pending = 0;
+    for (i = 0; i < s->nb_devices; i++) {
+        d = s->devices[i];
+        adc = ADB_DEVICE_GET_CLASS(d);
+
+        if (adc->devhasdata(d)) {
+            s->pending |= (1 << d->devaddr);
+        }
+    }
+
     devaddr = buf[0] >> 4;
     for (i = 0; i < s->nb_devices; i++) {
         d = s->devices[i];
+        adc = ADB_DEVICE_GET_CLASS(d);
+
         if (d->devaddr == devaddr) {
-            ADBDeviceClass *adc = ADB_DEVICE_GET_CLASS(d);
             return adc->devreq(d, obuf, buf, len);
         }
     }
+
     return ADB_RET_NOTPRESENT;
 }
 
diff --git a/include/hw/input/adb.h b/include/hw/input/adb.h
index 9b80204e43..f1bc358d8e 100644
--- a/include/hw/input/adb.h
+++ b/include/hw/input/adb.h
@@ -76,6 +76,7 @@ struct ADBBusState {
     /*< public >*/
 
     ADBDevice *devices[MAX_ADB_DEVICES];
+    uint16_t pending;
     int nb_devices;
     int poll_index;
 
-- 
2.20.1

[PATCH 13/22] adb: add status field for holding information about the last ADB request

[Mark Cave-Ayland]

Currently only 2 bits are defined: one to indicate if the request timed out (no
reply) and another to indicate whether the request was the result of an autopoll
operation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c         | 14 +++++++++++---
 include/hw/input/adb.h |  4 ++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index c1adb21e6b..a7a482fdfa 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -42,7 +42,7 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
 {
     ADBDevice *d;
     ADBDeviceClass *adc;
-    int devaddr, cmd, i;
+    int devaddr, cmd, olen, i;
 
     cmd = buf[0] & 0xf;
     if (cmd == ADB_BUSRESET) {
@@ -50,6 +50,7 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
             d = s->devices[i];
             adb_device_reset(d);
         }
+        s->status = 0;
         return 0;
     }
 
@@ -63,16 +64,22 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
         }
     }
 
+    s->status = 0;
     devaddr = buf[0] >> 4;
     for (i = 0; i < s->nb_devices; i++) {
         d = s->devices[i];
         adc = ADB_DEVICE_GET_CLASS(d);
 
         if (d->devaddr == devaddr) {
-            return adc->devreq(d, obuf, buf, len);
+            olen = adc->devreq(d, obuf, buf, len);
+            if (!olen) {
+                s->status |= ADB_STATUS_BUSTIMEOUT;
+            }
+            return olen;
         }
     }
 
+    s->status |= ADB_STATUS_BUSTIMEOUT;
     return ADB_RET_NOTPRESENT;
 }
 
@@ -94,9 +101,10 @@ int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
             olen = adb_request(s, obuf + 1, buf, 1);
             /* if there is data, we poll again the same device */
             if (olen > 0) {
+                s->status |= ADB_STATUS_POLLREPLY;
                 obuf[0] = buf[0];
                 olen++;
-                break;
+                return olen;
             }
         }
         s->poll_index++;
diff --git a/include/hw/input/adb.h b/include/hw/input/adb.h
index f1bc358d8e..cff264739c 100644
--- a/include/hw/input/adb.h
+++ b/include/hw/input/adb.h
@@ -70,6 +70,9 @@ typedef struct ADBDeviceClass {
 #define TYPE_ADB_BUS "apple-desktop-bus"
 #define ADB_BUS(obj) OBJECT_CHECK(ADBBusState, (obj), TYPE_ADB_BUS)
 
+#define ADB_STATUS_BUSTIMEOUT  0x1
+#define ADB_STATUS_POLLREPLY   0x2
+
 struct ADBBusState {
     /*< private >*/
     BusState parent_obj;
@@ -79,6 +82,7 @@ struct ADBBusState {
     uint16_t pending;
     int nb_devices;
     int poll_index;
+    uint8_t status;
 
     QEMUTimer *autopoll_timer;
     bool autopoll_enabled;
-- 
2.20.1

[PATCH 14/22] adb: use adb_request() only for explicit requests

[Mark Cave-Ayland]

Currently adb_request() is called both for explicit ADB requests and internal
autopoll requests via adb_poll().

Move the current functionality into do_adb_request() to be used internally and
add a simple adb_request() wrapper for explicit requests.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index a7a482fdfa..b3ad7c5fca 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -38,7 +38,8 @@ static void adb_device_reset(ADBDevice *d)
     qdev_reset_all(DEVICE(d));
 }
 
-int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
+static int do_adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf,
+                          int len)
 {
     ADBDevice *d;
     ADBDeviceClass *adc;
@@ -83,6 +84,11 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
     return ADB_RET_NOTPRESENT;
 }
 
+int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
+{
+    return do_adb_request(s, obuf, buf, len);
+}
+
 /* XXX: move that to cuda ? */
 int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
 {
@@ -98,7 +104,7 @@ int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
         d = s->devices[s->poll_index];
         if ((1 << d->devaddr) & poll_mask) {
             buf[0] = ADB_READREG | (d->devaddr << 4);
-            olen = adb_request(s, obuf + 1, buf, 1);
+            olen = do_adb_request(s, obuf + 1, buf, 1);
             /* if there is data, we poll again the same device */
             if (olen > 0) {
                 s->status |= ADB_STATUS_POLLREPLY;
-- 
2.20.1

[PATCH 15/22] adb: add autopoll_blocked variable to block autopoll

[Mark Cave-Ayland]

Whilst autopoll is enabled it is necessary to prevent the ADB buffer contents
from being overwritten until the host has read back the response in its
entirety.

Add adb_autopoll_block() and adb_autopoll_unblock() functions in preparation
for ensuring that the ADB buffer contents are protected for explicit ADB
requests.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c         | 21 +++++++++++++++++++++
 include/hw/input/adb.h |  4 ++++
 2 files changed, 25 insertions(+)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index b3ad7c5fca..70aa1f4570 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -157,6 +157,26 @@ void adb_set_autopoll_mask(ADBBusState *s, uint16_t mask)
     }
 }
 
+void adb_autopoll_block(ADBBusState *s)
+{
+    s->autopoll_blocked = true;
+
+    if (s->autopoll_enabled) {
+        timer_del(s->autopoll_timer);
+    }
+}
+
+void adb_autopoll_unblock(ADBBusState *s)
+{
+    s->autopoll_blocked = false;
+
+    if (s->autopoll_enabled) {
+        timer_mod(s->autopoll_timer,
+                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+                  s->autopoll_rate_ms);
+    }
+}
+
 static void adb_autopoll(void *opaque)
 {
     ADBBusState *s = opaque;
@@ -184,6 +204,7 @@ static const VMStateDescription vmstate_adb_bus = {
         VMSTATE_BOOL(autopoll_enabled, ADBBusState),
         VMSTATE_UINT8(autopoll_rate_ms, ADBBusState),
         VMSTATE_UINT16(autopoll_mask, ADBBusState),
+        VMSTATE_BOOL(autopoll_blocked, ADBBusState),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/include/hw/input/adb.h b/include/hw/input/adb.h
index cff264739c..bb75a7b1e3 100644
--- a/include/hw/input/adb.h
+++ b/include/hw/input/adb.h
@@ -86,6 +86,7 @@ struct ADBBusState {
 
     QEMUTimer *autopoll_timer;
     bool autopoll_enabled;
+    bool autopoll_blocked;
     uint8_t autopoll_rate_ms;
     uint16_t autopoll_mask;
     void (*autopoll_cb)(void *opaque);
@@ -96,6 +97,9 @@ int adb_request(ADBBusState *s, uint8_t *buf_out,
                 const uint8_t *buf, int len);
 int adb_poll(ADBBusState *s, uint8_t *buf_out, uint16_t poll_mask);
 
+void adb_autopoll_block(ADBBusState *s);
+void adb_autopoll_unblock(ADBBusState *s);
+
 void adb_set_autopoll_enabled(ADBBusState *s, bool enabled);
 void adb_set_autopoll_rate_ms(ADBBusState *s, int rate_ms);
 void adb_set_autopoll_mask(ADBBusState *s, uint16_t mask);
-- 
2.20.1

[PATCH 16/22] cuda: add adb_autopoll_block() and adb_autopoll_unblock() functions

[Mark Cave-Ayland]

Ensure that the CUDA buffer is protected from autopoll requests overwriting
its contents whilst existing CUDA requests are in progress.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/cuda.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 716866ea34..3e46ab6864 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -114,6 +114,7 @@ static void cuda_update(CUDAState *s)
 {
     MOS6522CUDAState *mcs = &s->mos6522_cuda;
     MOS6522State *ms = MOS6522(mcs);
+    ADBBusState *adb_bus = &s->adb_bus;
     int packet_received, len;
 
     packet_received = 0;
@@ -124,6 +125,9 @@ static void cuda_update(CUDAState *s)
             /* data output */
             if ((ms->b & (TACK | TIP)) != (s->last_b & (TACK | TIP))) {
                 if (s->data_out_index < sizeof(s->data_out)) {
+                    if (s->data_out_index == 0) {
+                        adb_autopoll_block(adb_bus);
+                    }
                     trace_cuda_data_send(ms->sr);
                     s->data_out[s->data_out_index++] = ms->sr;
                     cuda_delay_set_sr_int(s);
@@ -138,6 +142,7 @@ static void cuda_update(CUDAState *s)
                     /* indicate end of transfer */
                     if (s->data_in_index >= s->data_in_size) {
                         ms->b = (ms->b | TREQ);
+                        adb_autopoll_unblock(adb_bus);
                     }
                     cuda_delay_set_sr_int(s);
                 }
-- 
2.20.1

[PATCH 17/22] pmu: add adb_autopoll_block() and adb_autopoll_unblock() functions

[Mark Cave-Ayland]

Ensure that the PMU buffer is protected from autopoll requests overwriting
its contents whilst existing PMU requests are in progress.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/macio/pmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index e2291cc9f6..833dea4f71 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -515,6 +515,7 @@ static void pmu_update(PMUState *s)
 {
     MOS6522PMUState *mps = &s->mos6522_pmu;
     MOS6522State *ms = MOS6522(mps);
+    ADBBusState *adb_bus = &s->adb_bus;
 
     /* Only react to changes in reg B */
     if (ms->b == s->last_b) {
@@ -576,6 +577,7 @@ static void pmu_update(PMUState *s)
         s->cmd_rsp_pos = 0;
         s->cmd_state = pmu_state_cmd;
 
+        adb_autopoll_block(adb_bus);
         trace_pmu_debug_protocol_cmd(s->cmd, s->cmdlen, s->rsplen);
         break;
 
@@ -634,6 +636,7 @@ static void pmu_update(PMUState *s)
     if (s->cmd_state == pmu_state_rsp && s->rsplen == s->cmd_rsp_pos) {
         trace_pmu_debug_protocol_cmd_resp_complete(ms->ier);
 
+        adb_autopoll_unblock(adb_bus);
         s->cmd_state = pmu_state_idle;
     }
 }
-- 
2.20.1

[PATCH 18/22] mac_via: move VIA1 portB write logic into mos6522_q800_via1_write()

[Mark Cave-Ayland]

Currently the logic is split between the mos6522 portB_write() callback and
the memory region used to capture the VIA1 MMIO accesses. Move everything
into the latter mos6522_q800_via1_write() function to keep all the logic in
one place to make it easier to follow.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/mac_via.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 669fdca4c4..4779236f95 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -801,11 +801,21 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val,
                                     unsigned size)
 {
     MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(opaque);
+    MacVIAState *m = container_of(v1s, MacVIAState, mos6522_via1);
     MOS6522State *ms = MOS6522(v1s);
 
     addr = (addr >> 9) & 0xf;
     mos6522_write(ms, addr, val, size);
 
+    switch (addr) {
+    case VIA_REG_B:
+        via1_rtc_update(m);
+        via1_adb_update(m);
+
+        v1s->last_b = ms->b;
+        break;
+    }
+
     via1_one_second_update(v1s);
     via1_VBL_update(v1s);
 }
@@ -1034,18 +1044,6 @@ static TypeInfo mac_via_info = {
 };
 
 /* VIA 1 */
-static void mos6522_q800_via1_portB_write(MOS6522State *s)
-{
-    MOS6522Q800VIA1State *v1s = container_of(s, MOS6522Q800VIA1State,
-                                             parent_obj);
-    MacVIAState *m = container_of(v1s, MacVIAState, mos6522_via1);
-
-    via1_rtc_update(m);
-    via1_adb_update(m);
-
-    v1s->last_b = s->b;
-}
-
 static void mos6522_q800_via1_reset(DeviceState *dev)
 {
     MOS6522State *ms = MOS6522(dev);
@@ -1068,10 +1066,8 @@ static void mos6522_q800_via1_init(Object *obj)
 static void mos6522_q800_via1_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    MOS6522DeviceClass *mdc = MOS6522_DEVICE_CLASS(oc);
 
     dc->reset = mos6522_q800_via1_reset;
-    mdc->portB_write = mos6522_q800_via1_portB_write;
 }
 
 static const TypeInfo mos6522_q800_via1_type_info = {
-- 
2.20.1

[PATCH 19/22] mac_via: rework ADB state machine to be compatible with both MacOS and Linux

[Mark Cave-Ayland]

The existing ADB state machine is designed to work with Linux which has a different
interpretation of the state machine detailed in "Guide to the Macintosh Family
Hardware". In particular the current Linux implementation includes an extra change
to IDLE state when switching the VIA between send and receive modes which does not
occur in MacOS, and omitting this transition causes the current mac_via ADB state
machine to fail.

Rework the ADB state machine accordingly so that it can enumerate and autopoll the
ADB under both Linux and MacOS, including the addition of the new adb_autopoll_block()
and adb_autopoll_unblock() functions.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/misc/mac_via.c         | 363 +++++++++++++++++++++++++-------------
 hw/misc/trace-events      |   3 +
 include/hw/misc/mac_via.h |   1 +
 3 files changed, 246 insertions(+), 121 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 4779236f95..7676545474 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -599,176 +599,297 @@ static void via1_rtc_update(MacVIAState *m)
     m->cmd = REG_EMPTY;
 }
 
-static int adb_via_poll(MacVIAState *s, int state, uint8_t *data)
+static void adb_via_poll(void *opaque)
 {
-    ADBBusState *adb_bus = &s->adb_bus;
+    MacVIAState *m = opaque;
+    MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(&m->mos6522_via1);
+    MOS6522State *s = MOS6522(v1s);
+    ADBBusState *adb_bus = &m->adb_bus;
+    uint8_t obuf[9];
+    uint8_t *data = &s->sr;
+    int olen;
+    uint16_t pending;
 
-    if (state != ADB_STATE_IDLE) {
-        return 0;
+    /*
+     * Setting vADBInt below indicates that an autopoll reply has been
+     * received, however we must block autopoll until the point where
+     * the entire reply has been read back to the host
+     */
+    if (adb_bus->autopoll_blocked) {
+        return;
+    } else {
+        adb_autopoll_block(adb_bus);
     }
 
-    if (s->adb_data_in_size < s->adb_data_in_index) {
-        return 0;
-    }
+    m->adb_data_in_index = 0;
+    m->adb_data_out_index = 0;
+    olen = adb_poll(adb_bus, obuf, adb_bus->autopoll_mask);
+
+    if (olen > 0) {
+        /* Autopoll response */
+        *data = obuf[0];
+        olen--;
+        memcpy(m->adb_data_in, &obuf[1], olen);
+        m->adb_data_in_size = olen;
+
+        s->b &= ~VIA1B_vADBInt;
+        qemu_irq_raise(m->adb_data_ready);
+    } else if (olen < 0) {
+        /* Bus timeout (device does not exist) */
+        *data = 0xff;
+        s->b |= VIA1B_vADBInt;
+        adb_autopoll_unblock(adb_bus);
+    } else {
+        pending = adb_bus->pending & ~(1 << (m->adb_autopoll_cmd >> 4));
+
+        if (pending) {
+            /*
+             * Bus timeout (device exists but another device has data). Block
+             * autopoll so the OS can read out the first EVEN and first ODD
+             * byte to determine bus timeout and SRQ status
+             */
+            *data = m->adb_autopoll_cmd;
+            s->b &= ~VIA1B_vADBInt;
 
-    if (s->adb_data_out_index != 0) {
-        return 0;
-    }
+            obuf[0] = 0xff;
+            obuf[1] = 0xff;
+            olen = 2;
 
-    s->adb_data_in_index = 0;
-    s->adb_data_out_index = 0;
-    s->adb_data_in_size = adb_poll(adb_bus, s->adb_data_in,
-                                   adb_bus->autopoll_mask);
+            memcpy(m->adb_data_in, obuf, olen);
+            m->adb_data_in_size = olen;
 
-    if (s->adb_data_in_size) {
-        *data = s->adb_data_in[s->adb_data_in_index++];
-        qemu_irq_raise(s->adb_data_ready);
+            qemu_irq_raise(m->adb_data_ready);
+        } else {
+            /* Bus timeout (device exists but no other device has data) */
+            *data = 0;
+            s->b |= VIA1B_vADBInt;
+            adb_autopoll_unblock(adb_bus);
+        }
     }
 
-    return s->adb_data_in_size;
+    trace_via1_adb_poll(*data, (s->b & VIA1B_vADBInt) ? "+" : "-",
+                        adb_bus->status, m->adb_data_in_index, olen);
 }
 
-static int adb_via_send(MacVIAState *s, int state, uint8_t data)
+static int adb_via_send_len(uint8_t data)
 {
-    switch (state) {
-    case ADB_STATE_NEW:
-        s->adb_data_out_index = 0;
-        break;
-    case ADB_STATE_EVEN:
-        if ((s->adb_data_out_index & 1) == 0) {
-            return 0;
-        }
-        break;
-    case ADB_STATE_ODD:
-        if (s->adb_data_out_index & 1) {
-            return 0;
+    /* Determine the send length from the given ADB command */
+    uint8_t cmd = data & 0xc;
+    uint8_t reg = data & 0x3;
+
+    switch (cmd) {
+    case 0x8:
+        /* Listen command */
+        switch (reg) {
+        case 2:
+            /* Register 2 is only used for the keyboard */
+            return 3;
+        case 3:
+            /*
+             * Fortunately our devices only implement writes
+             * to register 3 which is fixed at 2 bytes
+             */
+            return 3;
+        default:
+            qemu_log_mask(LOG_UNIMP, "ADB unknown length for register %d\n",
+                          reg);
+            return 1;
         }
-        break;
-    case ADB_STATE_IDLE:
-        return 0;
+    default:
+        /* Talk, BusReset */
+        return 1;
     }
-
-    assert(s->adb_data_out_index < sizeof(s->adb_data_out) - 1);
-
-    s->adb_data_out[s->adb_data_out_index++] = data;
-    qemu_irq_raise(s->adb_data_ready);
-    return 1;
 }
 
-static int adb_via_receive(MacVIAState *s, int state, uint8_t *data)
+static void adb_via_send(MacVIAState *s, int state, uint8_t data)
 {
+    MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(&s->mos6522_via1);
+    MOS6522State *ms = MOS6522(v1s);
+    ADBBusState *adb_bus = &s->adb_bus;
+    uint16_t autopoll_mask;
+
     switch (state) {
     case ADB_STATE_NEW:
-        return 0;
-
-    case ADB_STATE_EVEN:
-        if (s->adb_data_in_size <= 0) {
-            qemu_irq_raise(s->adb_data_ready);
-            return 0;
-        }
-
-        if (s->adb_data_in_index >= s->adb_data_in_size) {
-            *data = 0;
-            qemu_irq_raise(s->adb_data_ready);
-            return 1;
-        }
-
-        if ((s->adb_data_in_index & 1) == 0) {
-            return 0;
+        /*
+         * Command byte: vADBInt tells host autopoll data already present
+         * in VIA shift register and ADB transceiver
+         */
+        adb_autopoll_block(adb_bus);
+
+        if (adb_bus->status & ADB_STATUS_POLLREPLY) {
+            /* Tell the host the existing data is from autopoll */
+            ms->b &= ~VIA1B_vADBInt;
+        } else {
+            ms->b |= VIA1B_vADBInt;
+            s->adb_data_out_index = 0;
+            s->adb_data_out[s->adb_data_out_index++] = data;
         }
 
+        trace_via1_adb_send(" NEW", data, (ms->b & VIA1B_vADBInt) ? "+" : "-");
+        qemu_irq_raise(s->adb_data_ready);
         break;
 
+    case ADB_STATE_EVEN:
     case ADB_STATE_ODD:
-        if (s->adb_data_in_size <= 0) {
-            qemu_irq_raise(s->adb_data_ready);
-            return 0;
-        }
-
-        if (s->adb_data_in_index >= s->adb_data_in_size) {
-            *data = 0;
-            qemu_irq_raise(s->adb_data_ready);
-            return 1;
-        }
-
-        if (s->adb_data_in_index & 1) {
-            return 0;
-        }
+        ms->b |= VIA1B_vADBInt;
+        s->adb_data_out[s->adb_data_out_index++] = data;
 
+        trace_via1_adb_send(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
+                            data, (ms->b & VIA1B_vADBInt) ? "+" : "-");
+        qemu_irq_raise(s->adb_data_ready);
         break;
 
     case ADB_STATE_IDLE:
-        if (s->adb_data_out_index == 0) {
-            return 0;
-        }
+        return;
+    }
 
-        s->adb_data_in_size = adb_request(&s->adb_bus, s->adb_data_in,
+    /* If the command is complete, execute it */
+    if (s->adb_data_out_index == adb_via_send_len(s->adb_data_out[0])) {
+        s->adb_data_in_size = adb_request(adb_bus, s->adb_data_in,
                                           s->adb_data_out,
                                           s->adb_data_out_index);
-        s->adb_data_out_index = 0;
         s->adb_data_in_index = 0;
-        if (s->adb_data_in_size < 0) {
-            *data = 0xff;
-            qemu_irq_raise(s->adb_data_ready);
-            return -1;
-        }
-
-        if (s->adb_data_in_size == 0) {
-            return 0;
-        }
-
-        break;
-    }
 
-    assert(s->adb_data_in_index < sizeof(s->adb_data_in) - 1);
+        /*
+         * If last command is TALK, store it for use by autopoll and adjust
+         * the autopoll mask accordingly
+         */
+        if ((s->adb_data_out[0] & 0xc) == 0xc) {
+            s->adb_autopoll_cmd = s->adb_data_out[0];
 
-    *data = s->adb_data_in[s->adb_data_in_index++];
-    qemu_irq_raise(s->adb_data_ready);
-    if (*data == 0xff || *data == 0) {
-        return 0;
+            autopoll_mask = 1 << (s->adb_autopoll_cmd >> 4);
+            adb_set_autopoll_mask(adb_bus, autopoll_mask);
+        }
     }
-    return 1;
 }
 
-static void via1_adb_update(MacVIAState *m)
+static void adb_via_receive(MacVIAState *s, int state, uint8_t *data)
 {
-    MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(&m->mos6522_via1);
-    MOS6522State *s = MOS6522(v1s);
-    int state;
-    int ret;
+    MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(&s->mos6522_via1);
+    MOS6522State *ms = MOS6522(v1s);
+    ADBBusState *adb_bus = &s->adb_bus;
+    uint16_t pending;
 
-    state = (s->b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift;
+    switch (state) {
+    case ADB_STATE_NEW:
+        ms->b |= VIA1B_vADBInt;
+        return;
 
-    if (s->acr & VIA1ACR_vShiftOut) {
-        /* output mode */
-        ret = adb_via_send(m, state, s->sr);
-        if (ret > 0) {
-            s->b &= ~VIA1B_vADBInt;
+    case ADB_STATE_IDLE:
+        /*
+         * Since adb_request() will have already consumed the data from the
+         * device, we must detect this extra state change and re-inject the
+         * reponse as either a "fake" autopoll reply or bus timeout
+         * accordingly
+         */
+        if (s->adb_data_in_index == 0) {
+            if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
+                *data = 0xff;
+                ms->b |= VIA1B_vADBInt;
+                s->adb_data_in[0] = 0xff;
+                s->adb_data_in[1] = 0xff;
+                s->adb_data_in_size = 2;
+                qemu_irq_raise(s->adb_data_ready);
+            } else if (s->adb_data_in_size > 0) {
+                adb_bus->status = ADB_STATUS_POLLREPLY;
+                *data = s->adb_autopoll_cmd;
+                ms->b &= ~VIA1B_vADBInt;
+                qemu_irq_raise(s->adb_data_ready);
+            }
         } else {
-            s->b |= VIA1B_vADBInt;
+            ms->b |= VIA1B_vADBInt;
+            adb_autopoll_unblock(adb_bus);
         }
-    } else {
-        /* input mode */
-        ret = adb_via_receive(m, state, &s->sr);
-        if (ret > 0 && s->sr != 0xff) {
-            s->b &= ~VIA1B_vADBInt;
-        } else {
-            s->b |= VIA1B_vADBInt;
+
+        trace_via1_adb_receive("IDLE", *data,
+                        (ms->b & VIA1B_vADBInt) ? "+" : "-", adb_bus->status,
+                        s->adb_data_in_index, s->adb_data_in_size);
+
+        break;
+
+    case ADB_STATE_EVEN:
+    case ADB_STATE_ODD:
+        switch (s->adb_data_in_index) {
+        case 0:
+            /* First EVEN byte: vADBInt indicates bus timeout */
+            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
+                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : "-",
+                                   adb_bus->status, s->adb_data_in_index,
+                                   s->adb_data_in_size);
+
+            *data = s->adb_data_in[s->adb_data_in_index++];
+            if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
+                ms->b &= ~VIA1B_vADBInt;
+            } else {
+                ms->b |= VIA1B_vADBInt;
+            }
+            break;
+
+        case 1:
+            /* First ODD byte: vADBInt indicates SRQ */
+            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
+                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : "-",
+                                   adb_bus->status, s->adb_data_in_index,
+                                   s->adb_data_in_size);
+
+            *data = s->adb_data_in[s->adb_data_in_index++];
+            pending = adb_bus->pending & ~(1 << (s->adb_autopoll_cmd >> 4));
+            if (pending) {
+                ms->b &= ~VIA1B_vADBInt;
+            } else {
+                ms->b |= VIA1B_vADBInt;
+            }
+            break;
+
+        default:
+            /*
+             * Otherwise vADBInt indicates end of data. Note that Linux
+             * specifically checks for the sequence 0x0 0xff to confirm the
+             * end of the request, so provide these extra bytes below to keep
+             * it happy
+             */
+            trace_via1_adb_receive(state == ADB_STATE_EVEN ? "EVEN" : " ODD",
+                                   *data, (ms->b & VIA1B_vADBInt) ? "+" : "-",
+                                   adb_bus->status, s->adb_data_in_index,
+                                   s->adb_data_in_size);
+
+            if (s->adb_data_in_index < s->adb_data_in_size) {
+                *data = s->adb_data_in[s->adb_data_in_index++];
+                ms->b |= VIA1B_vADBInt;
+            } else if (s->adb_data_in_index == s->adb_data_in_size) {
+                *data = 0;
+                s->adb_data_in_index++;
+                ms->b &= ~VIA1B_vADBInt;
+            } else {
+                *data = 0xff;
+                ms->b &= ~VIA1B_vADBInt;
+                adb_bus->status = 0;
+                adb_autopoll_unblock(adb_bus);
+            }
+            break;
         }
+
+        qemu_irq_raise(s->adb_data_ready);
+        break;
     }
 }
 
-static void via_adb_poll(void *opaque)
+static void via1_adb_update(MacVIAState *m)
 {
-    MacVIAState *m = opaque;
     MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(&m->mos6522_via1);
     MOS6522State *s = MOS6522(v1s);
-    int state;
+    int oldstate, state;
 
-    if (s->b & VIA1B_vADBInt) {
-        state = (s->b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift;
-        if (adb_via_poll(m, state, &s->sr)) {
-            s->b &= ~VIA1B_vADBInt;
+    oldstate = (v1s->last_b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift;
+    state = (s->b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift;
+
+    if (state != oldstate) {
+        if (s->acr & VIA1ACR_vShiftOut) {
+            /* output mode */
+            adb_via_send(m, state, s->sr);
+        } else {
+            /* input mode */
+            adb_via_receive(m, state, &s->sr);
         }
     }
 }
@@ -913,7 +1034,7 @@ static void mac_via_realize(DeviceState *dev, Error **errp)
     qemu_get_timedate(&tm, 0);
     m->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
 
-    adb_register_autopoll_callback(adb_bus, via_adb_poll, m);
+    adb_register_autopoll_callback(adb_bus, adb_via_poll, m);
     m->adb_data_ready = qdev_get_gpio_in_named(dev, "via1-irq",
                                                VIA1_IRQ_ADB_READY_BIT);
 
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 5561746866..68a6d9f2ab 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -202,6 +202,9 @@ via1_rtc_cmd_pram_read(int addr, int value) "addr=%u value=0x%02x"
 via1_rtc_cmd_pram_write(int addr, int value) "addr=%u value=0x%02x"
 via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
 via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
+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"
 
 # grlib_ahb_apb_pnp.c
 grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read addr:0x%03"PRIx64" data:0x%08x"
diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index 2aaf9e27bf..0be05d649b 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -112,6 +112,7 @@ typedef struct MacVIAState {
     int adb_data_out_index;
     uint8_t adb_data_in[128];
     uint8_t adb_data_out[16];
+    uint8_t adb_autopoll_cmd;
 } MacVIAState;
 
 #endif
-- 
2.20.1

[PATCH 20/22] adb: only call autopoll callbacks when autopoll is not blocked

[Mark Cave-Ayland]

Handle this at the ADB bus level so that individual implementations do not need
to handle this themselves.

Finally add an assert() into adb_request() to prevent developers from accidentally
making an explicit ADB request without blocking autopoll.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c    | 7 +++++--
 hw/misc/mac_via.c | 6 +-----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index 70aa1f4570..fe0f6c7ef3 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -86,10 +86,11 @@ static int do_adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf,
 
 int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
 {
+    assert(s->autopoll_blocked);
+
     return do_adb_request(s, obuf, buf, len);
 }
 
-/* XXX: move that to cuda ? */
 int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
 {
     ADBDevice *d;
@@ -181,7 +182,9 @@ static void adb_autopoll(void *opaque)
 {
     ADBBusState *s = opaque;
 
-    s->autopoll_cb(s->autopoll_cb_opaque);
+    if (!s->autopoll_blocked) {
+        s->autopoll_cb(s->autopoll_cb_opaque);
+    }
 
     timer_mod(s->autopoll_timer,
               qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 7676545474..efe96138d0 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -615,11 +615,7 @@ static void adb_via_poll(void *opaque)
      * received, however we must block autopoll until the point where
      * the entire reply has been read back to the host
      */
-    if (adb_bus->autopoll_blocked) {
-        return;
-    } else {
-        adb_autopoll_block(adb_bus);
-    }
+    adb_autopoll_block(adb_bus);
 
     m->adb_data_in_index = 0;
     m->adb_data_out_index = 0;
-- 
2.20.1

[PATCH 21/22] adb: use adb_device prefix for ADB device trace events

[Mark Cave-Ayland]

This is to allow us to distinguish between ADB device events and ADB
bus events separately.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb-kbd.c    | 12 ++++++------
 hw/input/adb-mouse.c  | 12 ++++++------
 hw/input/trace-events | 20 ++++++++++----------
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c
index 23760ecf7b..3cfb6a7a20 100644
--- a/hw/input/adb-kbd.c
+++ b/hw/input/adb-kbd.c
@@ -243,7 +243,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
     olen = 0;
     switch (cmd) {
     case ADB_WRITEREG:
-        trace_adb_kbd_writereg(reg, buf[1]);
+        trace_adb_device_kbd_writereg(reg, buf[1]);
         switch (reg) {
         case 2:
             /* LED status */
@@ -256,7 +256,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
             case ADB_CMD_CHANGE_ID_AND_ACT:
             case ADB_CMD_CHANGE_ID_AND_ENABLE:
                 d->devaddr = buf[1] & 0xf;
-                trace_adb_kbd_request_change_addr(d->devaddr);
+                trace_adb_device_kbd_request_change_addr(d->devaddr);
                 break;
             default:
                 d->devaddr = buf[1] & 0xf;
@@ -270,8 +270,8 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
                     d->handler = buf[2];
                 }
 
-                trace_adb_kbd_request_change_addr_and_handler(d->devaddr,
-                                                              d->handler);
+                trace_adb_device_kbd_request_change_addr_and_handler(
+                    d->devaddr, d->handler);
                 break;
             }
         }
@@ -294,7 +294,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
             olen = 2;
             break;
         }
-        trace_adb_kbd_readreg(reg, obuf[0], obuf[1]);
+        trace_adb_device_kbd_readreg(reg, obuf[0], obuf[1]);
         break;
     }
     return olen;
@@ -321,7 +321,7 @@ static void adb_keyboard_event(DeviceState *dev, QemuConsole *src,
     /* FIXME: take handler into account when translating qcode */
     keycode = qcode_to_adb_keycode[qcode];
     if (keycode == NO_KEY) {  /* We don't want to send this to the guest */
-        trace_adb_kbd_no_key();
+        trace_adb_device_kbd_no_key();
         return;
     }
     if (evt->u.key.data->down == false) { /* if key release event */
diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
index e2359fd74d..577a38ff2e 100644
--- a/hw/input/adb-mouse.c
+++ b/hw/input/adb-mouse.c
@@ -121,7 +121,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
         s->dx = 0;
         s->dy = 0;
         s->dz = 0;
-        trace_adb_mouse_flush();
+        trace_adb_device_mouse_flush();
         return 0;
     }
 
@@ -130,7 +130,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
     olen = 0;
     switch (cmd) {
     case ADB_WRITEREG:
-        trace_adb_mouse_writereg(reg, buf[1]);
+        trace_adb_device_mouse_writereg(reg, buf[1]);
         switch (reg) {
         case 2:
             break;
@@ -152,7 +152,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
             case ADB_CMD_CHANGE_ID_AND_ACT:
             case ADB_CMD_CHANGE_ID_AND_ENABLE:
                 d->devaddr = buf[1] & 0xf;
-                trace_adb_mouse_request_change_addr(d->devaddr);
+                trace_adb_device_mouse_request_change_addr(d->devaddr);
                 break;
             default:
                 d->devaddr = buf[1] & 0xf;
@@ -172,8 +172,8 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
                     d->handler = buf[2];
                 }
 
-                trace_adb_mouse_request_change_addr_and_handler(d->devaddr,
-                                                                d->handler);
+                trace_adb_device_mouse_request_change_addr_and_handler(
+                    d->devaddr, d->handler);
                 break;
             }
         }
@@ -191,7 +191,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
             olen = 2;
             break;
         }
-        trace_adb_mouse_readreg(reg, obuf[0], obuf[1]);
+        trace_adb_device_mouse_readreg(reg, obuf[0], obuf[1]);
         break;
     }
     return olen;
diff --git a/hw/input/trace-events b/hw/input/trace-events
index a2888fd10c..6f0d78241c 100644
--- a/hw/input/trace-events
+++ b/hw/input/trace-events
@@ -1,18 +1,18 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
 # adb-kbd.c
-adb_kbd_no_key(void) "Ignoring NO_KEY"
-adb_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
-adb_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
-adb_kbd_request_change_addr(int devaddr) "change addr to 0x%x"
-adb_kbd_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
+adb_device_kbd_no_key(void) "Ignoring NO_KEY"
+adb_device_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
+adb_device_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
+adb_device_kbd_request_change_addr(int devaddr) "change addr to 0x%x"
+adb_device_kbd_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
 
 # adb-mouse.c
-adb_mouse_flush(void) "flush"
-adb_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
-adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
-adb_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
-adb_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
+adb_device_mouse_flush(void) "flush"
+adb_device_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
+adb_device_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
+adb_device_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
+adb_device_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
 
 # pckbd.c
 pckbd_kbd_read_data(uint32_t val) "0x%02x"
-- 
2.20.1

[PATCH 22/22] adb: add ADB bus trace events

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb.c        | 23 ++++++++++++++++++++++-
 hw/input/trace-events |  7 +++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index fe0f6c7ef3..4976f52c36 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -29,10 +29,18 @@
 #include "qemu/module.h"
 #include "qemu/timer.h"
 #include "adb-internal.h"
+#include "trace.h"
 
 /* error codes */
 #define ADB_RET_NOTPRESENT (-2)
 
+static const char *adb_commands[] = {
+    "RESET", "FLUSH", "(Reserved 0x2)", "(Reserved 0x3)",
+    "Reserved (0x4)", "(Reserved 0x5)", "(Reserved 0x6)", "(Reserved 0x7)",
+    "LISTEN r0", "LISTEN r1", "LISTEN r2", "LISTEN r3",
+    "TALK r0", "TALK r1", "TALK r2", "TALK r3",
+};
+
 static void adb_device_reset(ADBDevice *d)
 {
     qdev_reset_all(DEVICE(d));
@@ -86,9 +94,16 @@ static int do_adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf,
 
 int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
 {
+    int ret;
+
+    trace_adb_bus_request(buf[0] >> 4, adb_commands[buf[0] & 0xf], len);
+
     assert(s->autopoll_blocked);
 
-    return do_adb_request(s, obuf, buf, len);
+    ret = do_adb_request(s, obuf, buf, len);
+
+    trace_adb_bus_request_done(buf[0] >> 4, adb_commands[buf[0] & 0xf], ret);
+    return ret;
 }
 
 int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
@@ -160,6 +175,8 @@ void adb_set_autopoll_mask(ADBBusState *s, uint16_t mask)
 
 void adb_autopoll_block(ADBBusState *s)
 {
+    trace_adb_bus_autopoll_block("autopoll BLOCKED");
+
     s->autopoll_blocked = true;
 
     if (s->autopoll_enabled) {
@@ -169,6 +186,8 @@ void adb_autopoll_block(ADBBusState *s)
 
 void adb_autopoll_unblock(ADBBusState *s)
 {
+    trace_adb_bus_autopoll_block("autopoll UNBLOCKED");
+
     s->autopoll_blocked = false;
 
     if (s->autopoll_enabled) {
@@ -183,7 +202,9 @@ static void adb_autopoll(void *opaque)
     ADBBusState *s = opaque;
 
     if (!s->autopoll_blocked) {
+        trace_adb_bus_autopoll_cb(s->autopoll_mask);
         s->autopoll_cb(s->autopoll_cb_opaque);
+        trace_adb_bus_autopoll_cb_done(s->autopoll_mask);
     }
 
     timer_mod(s->autopoll_timer,
diff --git a/hw/input/trace-events b/hw/input/trace-events
index 6f0d78241c..119d1ce2bd 100644
--- a/hw/input/trace-events
+++ b/hw/input/trace-events
@@ -14,6 +14,13 @@ adb_device_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x
 adb_device_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
 adb_device_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
 
+# adb.c
+adb_bus_request(uint8_t addr, const char *cmd, int size) "device 0x%x %s cmdsize=%d"
+adb_bus_request_done(uint8_t addr, const char *cmd, int size) "device 0x%x %s replysize=%d"
+adb_bus_autopoll_block(const char *s) "%s"
+adb_bus_autopoll_cb(uint16_t mask) "executing autopoll_cb with autopoll mask 0x%x"
+adb_bus_autopoll_cb_done(uint16_t mask) "done executing autopoll_cb with autopoll mask 0x%x"
+
 # pckbd.c
 pckbd_kbd_read_data(uint32_t val) "0x%02x"
 pckbd_kbd_read_status(int status) "0x%02x"
-- 
2.20.1

Re: [PATCH 01/22] adb: coding style update to fix checkpatch errors

[Philippe Mathieu-Daudé]

On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
> This will help ensure that style guidelines are being maintained during
> subsequent changes.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/input/adb.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/input/adb.c b/hw/input/adb.c
> index b1ac4a3852..bf1bc30d19 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -44,14 +44,14 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
>  
>      cmd = buf[0] & 0xf;
>      if (cmd == ADB_BUSRESET) {
> -        for(i = 0; i < s->nb_devices; i++) {
> +        for (i = 0; i < s->nb_devices; i++) {
>              d = s->devices[i];
>              adb_device_reset(d);
>          }
>          return 0;
>      }
>      devaddr = buf[0] >> 4;
> -    for(i = 0; i < s->nb_devices; i++) {
> +    for (i = 0; i < s->nb_devices; i++) {
>          d = s->devices[i];
>          if (d->devaddr == devaddr) {
>              ADBDeviceClass *adc = ADB_DEVICE_GET_CLASS(d);
> @@ -69,9 +69,10 @@ int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
>      uint8_t buf[1];
>  
>      olen = 0;
> -    for(i = 0; i < s->nb_devices; i++) {
> -        if (s->poll_index >= s->nb_devices)
> +    for (i = 0; i < s->nb_devices; i++) {
> +        if (s->poll_index >= s->nb_devices) {
>              s->poll_index = 0;
> +        }
>          d = s->devices[s->poll_index];
>          if ((1 << d->devaddr) & poll_mask) {
>              buf[0] = ADB_READREG | (d->devaddr << 4);
> 

Re: [PATCH 03/22] cuda: convert ADB autopoll timer from ns to ms

[Philippe Mathieu-Daudé]

On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
> This is in preparation for consolidating all of the ADB autopoll management
> in one place.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/misc/macio/cuda.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index e0cc0aac5d..a407f2abc8 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -208,8 +208,9 @@ static void cuda_adb_poll(void *opaque)
>          obuf[1] = 0x40; /* polled data */
>          cuda_send_packet_to_host(s, obuf, olen + 2);
>      }
> -    timer_mod(s->adb_poll_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> -              (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
> +
> +    timer_mod(s->adb_poll_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
> +              s->autopoll_rate_ms);
>  }
>  
>  /* description of commands */
> @@ -236,8 +237,8 @@ static bool cuda_cmd_autopoll(CUDAState *s,
>          s->autopoll = autopoll;
>          if (autopoll) {
>              timer_mod(s->adb_poll_timer,
> -                      qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> -                      (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
> +                      qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
> +                      s->autopoll_rate_ms);
>          } else {
>              timer_del(s->adb_poll_timer);
>          }
> @@ -262,8 +263,8 @@ static bool cuda_cmd_set_autorate(CUDAState *s,
>      s->autopoll_rate_ms = in_data[0];
>      if (s->autopoll) {
>          timer_mod(s->adb_poll_timer,
> -                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> -                  (NANOSECONDS_PER_SECOND / (1000 / s->autopoll_rate_ms)));
> +                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
> +                  s->autopoll_rate_ms);
>      }
>      return true;
>  }
> @@ -539,7 +540,7 @@ static void cuda_realize(DeviceState *dev, Error **errp)
>      s->sr_delay_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_set_sr_int, s);
>      s->sr_delay_ns = 20 * SCALE_US;
>  
> -    s->adb_poll_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_adb_poll, s);
> +    s->adb_poll_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, cuda_adb_poll, s);
>      s->adb_poll_mask = 0xffff;
>      s->autopoll_rate_ms = 20;
>  }
> 

Re: [PATCH 05/22] pmu: honour autopoll_rate_ms when rearming the ADB autopoll timer

[Philippe Mathieu-Daudé]

On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
> Don't use a fixed value but instead use the default value from the ADB bus
> state.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/misc/macio/pmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
> index 3af4ef1a04..edb061e417 100644
> --- a/hw/misc/macio/pmu.c
> +++ b/hw/misc/macio/pmu.c
> @@ -104,7 +104,7 @@ static void pmu_adb_poll(void *opaque)
>      }
>  
>      timer_mod(s->adb_poll_timer,
> -              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 30);
> +              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + s->autopoll_rate_ms);
>  }
>  
>  static void pmu_one_sec_timer(void *opaque)
> @@ -180,7 +180,7 @@ static void pmu_cmd_set_adb_autopoll(PMUState *s, uint16_t mask)
>      s->adb_poll_mask = mask;
>      if (mask) {
>          timer_mod(s->adb_poll_timer,
> -                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 30);
> +                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + s->autopoll_rate_ms);
>      } else {
>          timer_del(s->adb_poll_timer);
>      }
> 

Re: [PATCH 18/22] mac_via: move VIA1 portB write logic into mos6522_q800_via1_write()

[Philippe Mathieu-Daudé]

On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
> Currently the logic is split between the mos6522 portB_write() callback and
> the memory region used to capture the VIA1 MMIO accesses. Move everything
> into the latter mos6522_q800_via1_write() function to keep all the logic in
> one place to make it easier to follow.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/misc/mac_via.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index 669fdca4c4..4779236f95 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -801,11 +801,21 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val,
>                                      unsigned size)
>  {
>      MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(opaque);
> +    MacVIAState *m = container_of(v1s, MacVIAState, mos6522_via1);

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>      MOS6522State *ms = MOS6522(v1s);
>  
>      addr = (addr >> 9) & 0xf;
>      mos6522_write(ms, addr, val, size);
>  
> +    switch (addr) {
> +    case VIA_REG_B:
> +        via1_rtc_update(m);
> +        via1_adb_update(m);
> +
> +        v1s->last_b = ms->b;
> +        break;
> +    }
> +
>      via1_one_second_update(v1s);
>      via1_VBL_update(v1s);
>  }
> @@ -1034,18 +1044,6 @@ static TypeInfo mac_via_info = {
>  };
>  
>  /* VIA 1 */
> -static void mos6522_q800_via1_portB_write(MOS6522State *s)
> -{
> -    MOS6522Q800VIA1State *v1s = container_of(s, MOS6522Q800VIA1State,
> -                                             parent_obj);
> -    MacVIAState *m = container_of(v1s, MacVIAState, mos6522_via1);
> -
> -    via1_rtc_update(m);
> -    via1_adb_update(m);
> -
> -    v1s->last_b = s->b;
> -}
> -
>  static void mos6522_q800_via1_reset(DeviceState *dev)
>  {
>      MOS6522State *ms = MOS6522(dev);
> @@ -1068,10 +1066,8 @@ static void mos6522_q800_via1_init(Object *obj)
>  static void mos6522_q800_via1_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> -    MOS6522DeviceClass *mdc = MOS6522_DEVICE_CLASS(oc);
>  
>      dc->reset = mos6522_q800_via1_reset;
> -    mdc->portB_write = mos6522_q800_via1_portB_write;
>  }
>  
>  static const TypeInfo mos6522_q800_via1_type_info = {
> 

Re: [PATCH 22/22] adb: add ADB bus trace events

[Philippe Mathieu-Daudé]

Hi Mark,

On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/input/adb.c        | 23 ++++++++++++++++++++++-
>  hw/input/trace-events |  7 +++++++
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/input/adb.c b/hw/input/adb.c
> index fe0f6c7ef3..4976f52c36 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -29,10 +29,18 @@
>  #include "qemu/module.h"
>  #include "qemu/timer.h"
>  #include "adb-internal.h"
> +#include "trace.h"
>  
>  /* error codes */
>  #define ADB_RET_NOTPRESENT (-2)
>  
> +static const char *adb_commands[] = {
> +    "RESET", "FLUSH", "(Reserved 0x2)", "(Reserved 0x3)",
> +    "Reserved (0x4)", "(Reserved 0x5)", "(Reserved 0x6)", "(Reserved 0x7)",
> +    "LISTEN r0", "LISTEN r1", "LISTEN r2", "LISTEN r3",
> +    "TALK r0", "TALK r1", "TALK r2", "TALK r3",
> +};
> +
>  static void adb_device_reset(ADBDevice *d)
>  {
>      qdev_reset_all(DEVICE(d));
> @@ -86,9 +94,16 @@ static int do_adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf,
>  
>  int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
>  {
> +    int ret;
> +
> +    trace_adb_bus_request(buf[0] >> 4, adb_commands[buf[0] & 0xf], len);
> +
>      assert(s->autopoll_blocked);
>  
> -    return do_adb_request(s, obuf, buf, len);
> +    ret = do_adb_request(s, obuf, buf, len);
> +
> +    trace_adb_bus_request_done(buf[0] >> 4, adb_commands[buf[0] & 0xf], ret);
> +    return ret;
>  }
>  
>  int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
> @@ -160,6 +175,8 @@ void adb_set_autopoll_mask(ADBBusState *s, uint16_t mask)
>  
>  void adb_autopoll_block(ADBBusState *s)
>  {
> +    trace_adb_bus_autopoll_block("autopoll BLOCKED");

Regarding how trace backends work, in this case it is better
to use a boolean value and let the backend do the formatting:

       trace_adb_bus_autopoll_block(true);

The rationale is it is easier for backends to filter on a
bool (register) arg rather than fetching memory for strcmp.

So format can be:

adb_bus_autopoll_block(bool state) "autopoll is_blocked:%u"

Anyway if you want to keep as it, it is cleaner to change the
format as "autopoll %s".

> +
>      s->autopoll_blocked = true;

This can also be:

       trace_adb_bus_autopoll_block(s->autopoll_blocked);

>  
>      if (s->autopoll_enabled) {
> @@ -169,6 +186,8 @@ void adb_autopoll_block(ADBBusState *s)
>  
>  void adb_autopoll_unblock(ADBBusState *s)
>  {
> +    trace_adb_bus_autopoll_block("autopoll UNBLOCKED");
> +
>      s->autopoll_blocked = false;

Ditto:

       trace_adb_bus_autopoll_block(s->autopoll_blocked);

>  
>      if (s->autopoll_enabled) {
> @@ -183,7 +202,9 @@ static void adb_autopoll(void *opaque)
>      ADBBusState *s = opaque;
>  
>      if (!s->autopoll_blocked) {
> +        trace_adb_bus_autopoll_cb(s->autopoll_mask);
>          s->autopoll_cb(s->autopoll_cb_opaque);
> +        trace_adb_bus_autopoll_cb_done(s->autopoll_mask);
>      }
>  
>      timer_mod(s->autopoll_timer,
> diff --git a/hw/input/trace-events b/hw/input/trace-events
> index 6f0d78241c..119d1ce2bd 100644
> --- a/hw/input/trace-events
> +++ b/hw/input/trace-events
> @@ -14,6 +14,13 @@ adb_device_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x
>  adb_device_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
>  adb_device_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
>  
> +# adb.c
> +adb_bus_request(uint8_t addr, const char *cmd, int size) "device 0x%x %s cmdsize=%d"
> +adb_bus_request_done(uint8_t addr, const char *cmd, int size) "device 0x%x %s replysize=%d"
> +adb_bus_autopoll_block(const char *s) "%s"
> +adb_bus_autopoll_cb(uint16_t mask) "executing autopoll_cb with autopoll mask 0x%x"
> +adb_bus_autopoll_cb_done(uint16_t mask) "done executing autopoll_cb with autopoll mask 0x%x"
> +
>  # pckbd.c
>  pckbd_kbd_read_data(uint32_t val) "0x%02x"
>  pckbd_kbd_read_status(int status) "0x%02x"
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 21/22] adb: use adb_device prefix for ADB device trace events

[Philippe Mathieu-Daudé]

On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
> This is to allow us to distinguish between ADB device events and ADB
> bus events separately.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/input/adb-kbd.c    | 12 ++++++------
>  hw/input/adb-mouse.c  | 12 ++++++------
>  hw/input/trace-events | 20 ++++++++++----------
>  3 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c
> index 23760ecf7b..3cfb6a7a20 100644
> --- a/hw/input/adb-kbd.c
> +++ b/hw/input/adb-kbd.c
> @@ -243,7 +243,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>      olen = 0;
>      switch (cmd) {
>      case ADB_WRITEREG:
> -        trace_adb_kbd_writereg(reg, buf[1]);
> +        trace_adb_device_kbd_writereg(reg, buf[1]);
>          switch (reg) {
>          case 2:
>              /* LED status */
> @@ -256,7 +256,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>              case ADB_CMD_CHANGE_ID_AND_ACT:
>              case ADB_CMD_CHANGE_ID_AND_ENABLE:
>                  d->devaddr = buf[1] & 0xf;
> -                trace_adb_kbd_request_change_addr(d->devaddr);
> +                trace_adb_device_kbd_request_change_addr(d->devaddr);
>                  break;
>              default:
>                  d->devaddr = buf[1] & 0xf;
> @@ -270,8 +270,8 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>                      d->handler = buf[2];
>                  }
>  
> -                trace_adb_kbd_request_change_addr_and_handler(d->devaddr,
> -                                                              d->handler);
> +                trace_adb_device_kbd_request_change_addr_and_handler(
> +                    d->devaddr, d->handler);
>                  break;
>              }
>          }
> @@ -294,7 +294,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>              olen = 2;
>              break;
>          }
> -        trace_adb_kbd_readreg(reg, obuf[0], obuf[1]);
> +        trace_adb_device_kbd_readreg(reg, obuf[0], obuf[1]);
>          break;
>      }
>      return olen;
> @@ -321,7 +321,7 @@ static void adb_keyboard_event(DeviceState *dev, QemuConsole *src,
>      /* FIXME: take handler into account when translating qcode */
>      keycode = qcode_to_adb_keycode[qcode];
>      if (keycode == NO_KEY) {  /* We don't want to send this to the guest */
> -        trace_adb_kbd_no_key();
> +        trace_adb_device_kbd_no_key();
>          return;
>      }
>      if (evt->u.key.data->down == false) { /* if key release event */
> diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
> index e2359fd74d..577a38ff2e 100644
> --- a/hw/input/adb-mouse.c
> +++ b/hw/input/adb-mouse.c
> @@ -121,7 +121,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>          s->dx = 0;
>          s->dy = 0;
>          s->dz = 0;
> -        trace_adb_mouse_flush();
> +        trace_adb_device_mouse_flush();
>          return 0;
>      }
>  
> @@ -130,7 +130,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>      olen = 0;
>      switch (cmd) {
>      case ADB_WRITEREG:
> -        trace_adb_mouse_writereg(reg, buf[1]);
> +        trace_adb_device_mouse_writereg(reg, buf[1]);
>          switch (reg) {
>          case 2:
>              break;
> @@ -152,7 +152,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>              case ADB_CMD_CHANGE_ID_AND_ACT:
>              case ADB_CMD_CHANGE_ID_AND_ENABLE:
>                  d->devaddr = buf[1] & 0xf;
> -                trace_adb_mouse_request_change_addr(d->devaddr);
> +                trace_adb_device_mouse_request_change_addr(d->devaddr);
>                  break;
>              default:
>                  d->devaddr = buf[1] & 0xf;
> @@ -172,8 +172,8 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>                      d->handler = buf[2];
>                  }
>  
> -                trace_adb_mouse_request_change_addr_and_handler(d->devaddr,
> -                                                                d->handler);
> +                trace_adb_device_mouse_request_change_addr_and_handler(
> +                    d->devaddr, d->handler);
>                  break;
>              }
>          }
> @@ -191,7 +191,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>              olen = 2;
>              break;
>          }
> -        trace_adb_mouse_readreg(reg, obuf[0], obuf[1]);
> +        trace_adb_device_mouse_readreg(reg, obuf[0], obuf[1]);
>          break;
>      }
>      return olen;
> diff --git a/hw/input/trace-events b/hw/input/trace-events
> index a2888fd10c..6f0d78241c 100644
> --- a/hw/input/trace-events
> +++ b/hw/input/trace-events
> @@ -1,18 +1,18 @@
>  # See docs/devel/tracing.txt for syntax documentation.
>  
>  # adb-kbd.c
> -adb_kbd_no_key(void) "Ignoring NO_KEY"
> -adb_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
> -adb_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
> -adb_kbd_request_change_addr(int devaddr) "change addr to 0x%x"
> -adb_kbd_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
> +adb_device_kbd_no_key(void) "Ignoring NO_KEY"
> +adb_device_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
> +adb_device_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
> +adb_device_kbd_request_change_addr(int devaddr) "change addr to 0x%x"
> +adb_device_kbd_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
>  
>  # adb-mouse.c
> -adb_mouse_flush(void) "flush"
> -adb_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
> -adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
> -adb_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
> -adb_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
> +adb_device_mouse_flush(void) "flush"

For the following: ...

> +adb_device_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
> +adb_device_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
> +adb_device_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
> +adb_device_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"

... you could use a common trace, using a device_type argument, i.e.:

adb_device_writereg(const char *device_type, int reg, uint8_t val) "%s
reg %d val 0x%2.2x"

But then you can not filter traces for a particular device.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>  
>  # pckbd.c
>  pckbd_kbd_read_data(uint32_t val) "0x%02x"
> 

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

[Finn Thain]

Tested-by: Finn Thain <fthain@telegraphics.com.au>

Thanks for all your work on this.

I've just noticed a discrepancy between the traces from an ADB bus scan on 
Laurent's Apple Quadra and an ADB bus scan on your patched QEMU machine.

Apple Q800:

[C1f][s   ][Rff-][Rff ][rff-]
[C2f][s   ][R61 ][R05 ][r00-]
[C3f][s   ][R79 ][R01 ][r00-]
[C4f][s   ][Rff-][Rff ][rff-]
[C5f][s   ][Rff-][Rff ][rff-]
[C6f][s   ][Rff-][Rff ][rff-]
[C7f][s   ][Rff-][Rff ][rff-]
[C8f][s   ][Rff-][Rff ][rff-]
[C9f][s   ][Rff-][Rff ][rff-]
[Caf][s   ][Rff-][Rff ][rff-]
[Cbf][s   ][Rff-][Rff ][rff-]
[Ccf][s   ][Rff-][Rff ][rff-]
[Cdf][s   ][Rff-][Rff ][rff-]
[Cef][s   ][Rff-][Rff ][rff-]
[Cff][s   ][Rff-][Rff ][rff-]

QEMU Q800:

[C1f][s   ][Rff-][Rff ][rff-]
[C2f][s   ][R02 ][R01 ][r00-]
[C3f][s   ][R03 ][R02 ][r00-]
[C4f][s   ][R03-][R02 ][rff-]
[C5f][s   ][R03-][R02 ][rff-]
[C6f][s   ][R03-][R02 ][rff-]
[C7f][s   ][R03-][R02 ][rff-]
[C8f][s   ][R03-][R02 ][rff-]
[C9f][s   ][R03-][R02 ][rff-]
[Caf][s   ][R03-][R02 ][rff-]
[Cbf][s   ][R03-][R02 ][rff-]
[Ccf][s   ][R03-][R02 ][rff-]
[Cdf][s   ][R03-][R02 ][rff-]
[Cef][s   ][R03-][R02 ][rff-]
[Cff][s   ][R03-][R02 ][rff-]

I think this could be easy to fix; it's probably just an uninitialized 
packet buffer. When you come to submit v2, you may want to look into this.

Re: [PATCH 22/22] adb: add ADB bus trace events

[Mark Cave-Ayland]

On 14/06/2020 18:16, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/input/adb.c        | 23 ++++++++++++++++++++++-
>>  hw/input/trace-events |  7 +++++++
>>  2 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/input/adb.c b/hw/input/adb.c
>> index fe0f6c7ef3..4976f52c36 100644
>> --- a/hw/input/adb.c
>> +++ b/hw/input/adb.c
>> @@ -29,10 +29,18 @@
>>  #include "qemu/module.h"
>>  #include "qemu/timer.h"
>>  #include "adb-internal.h"
>> +#include "trace.h"
>>  
>>  /* error codes */
>>  #define ADB_RET_NOTPRESENT (-2)
>>  
>> +static const char *adb_commands[] = {
>> +    "RESET", "FLUSH", "(Reserved 0x2)", "(Reserved 0x3)",
>> +    "Reserved (0x4)", "(Reserved 0x5)", "(Reserved 0x6)", "(Reserved 0x7)",
>> +    "LISTEN r0", "LISTEN r1", "LISTEN r2", "LISTEN r3",
>> +    "TALK r0", "TALK r1", "TALK r2", "TALK r3",
>> +};
>> +
>>  static void adb_device_reset(ADBDevice *d)
>>  {
>>      qdev_reset_all(DEVICE(d));
>> @@ -86,9 +94,16 @@ static int do_adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf,
>>  
>>  int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)
>>  {
>> +    int ret;
>> +
>> +    trace_adb_bus_request(buf[0] >> 4, adb_commands[buf[0] & 0xf], len);
>> +
>>      assert(s->autopoll_blocked);
>>  
>> -    return do_adb_request(s, obuf, buf, len);
>> +    ret = do_adb_request(s, obuf, buf, len);
>> +
>> +    trace_adb_bus_request_done(buf[0] >> 4, adb_commands[buf[0] & 0xf], ret);
>> +    return ret;
>>  }
>>  
>>  int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
>> @@ -160,6 +175,8 @@ void adb_set_autopoll_mask(ADBBusState *s, uint16_t mask)
>>  
>>  void adb_autopoll_block(ADBBusState *s)
>>  {
>> +    trace_adb_bus_autopoll_block("autopoll BLOCKED");
> 
> Regarding how trace backends work, in this case it is better
> to use a boolean value and let the backend do the formatting:
> 
>        trace_adb_bus_autopoll_block(true);
> 
> The rationale is it is easier for backends to filter on a
> bool (register) arg rather than fetching memory for strcmp.
> 
> So format can be:
> 
> adb_bus_autopoll_block(bool state) "autopoll is_blocked:%u"
> 
> Anyway if you want to keep as it, it is cleaner to change the
> format as "autopoll %s".
> 
>> +
>>      s->autopoll_blocked = true;
> 
> This can also be:
> 
>        trace_adb_bus_autopoll_block(s->autopoll_blocked);
> 
>>  
>>      if (s->autopoll_enabled) {
>> @@ -169,6 +186,8 @@ void adb_autopoll_block(ADBBusState *s)
>>  
>>  void adb_autopoll_unblock(ADBBusState *s)
>>  {
>> +    trace_adb_bus_autopoll_block("autopoll UNBLOCKED");
>> +
>>      s->autopoll_blocked = false;
> 
> Ditto:
> 
>        trace_adb_bus_autopoll_block(s->autopoll_blocked);
> 
>>  
>>      if (s->autopoll_enabled) {
>> @@ -183,7 +202,9 @@ static void adb_autopoll(void *opaque)
>>      ADBBusState *s = opaque;
>>  
>>      if (!s->autopoll_blocked) {
>> +        trace_adb_bus_autopoll_cb(s->autopoll_mask);
>>          s->autopoll_cb(s->autopoll_cb_opaque);
>> +        trace_adb_bus_autopoll_cb_done(s->autopoll_mask);
>>      }
>>  
>>      timer_mod(s->autopoll_timer,
>> diff --git a/hw/input/trace-events b/hw/input/trace-events
>> index 6f0d78241c..119d1ce2bd 100644
>> --- a/hw/input/trace-events
>> +++ b/hw/input/trace-events
>> @@ -14,6 +14,13 @@ adb_device_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x
>>  adb_device_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
>>  adb_device_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
>>  
>> +# adb.c
>> +adb_bus_request(uint8_t addr, const char *cmd, int size) "device 0x%x %s cmdsize=%d"
>> +adb_bus_request_done(uint8_t addr, const char *cmd, int size) "device 0x%x %s replysize=%d"
>> +adb_bus_autopoll_block(const char *s) "%s"
>> +adb_bus_autopoll_cb(uint16_t mask) "executing autopoll_cb with autopoll mask 0x%x"
>> +adb_bus_autopoll_cb_done(uint16_t mask) "done executing autopoll_cb with autopoll mask 0x%x"
>> +
>>  # pckbd.c
>>  pckbd_kbd_read_data(uint32_t val) "0x%02x"
>>  pckbd_kbd_read_status(int status) "0x%02x"
>>
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks for the review: I think you're right in that the boolean makes things easier
for other trace backends, so I'll implement your changes in a v2.


ATB,

Mark.

Re: [PATCH 21/22] adb: use adb_device prefix for ADB device trace events

[Mark Cave-Ayland]

On 14/06/2020 18:20, Philippe Mathieu-Daudé wrote:

> On 6/14/20 4:28 PM, Mark Cave-Ayland wrote:
>> This is to allow us to distinguish between ADB device events and ADB
>> bus events separately.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/input/adb-kbd.c    | 12 ++++++------
>>  hw/input/adb-mouse.c  | 12 ++++++------
>>  hw/input/trace-events | 20 ++++++++++----------
>>  3 files changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c
>> index 23760ecf7b..3cfb6a7a20 100644
>> --- a/hw/input/adb-kbd.c
>> +++ b/hw/input/adb-kbd.c
>> @@ -243,7 +243,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>>      olen = 0;
>>      switch (cmd) {
>>      case ADB_WRITEREG:
>> -        trace_adb_kbd_writereg(reg, buf[1]);
>> +        trace_adb_device_kbd_writereg(reg, buf[1]);
>>          switch (reg) {
>>          case 2:
>>              /* LED status */
>> @@ -256,7 +256,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>>              case ADB_CMD_CHANGE_ID_AND_ACT:
>>              case ADB_CMD_CHANGE_ID_AND_ENABLE:
>>                  d->devaddr = buf[1] & 0xf;
>> -                trace_adb_kbd_request_change_addr(d->devaddr);
>> +                trace_adb_device_kbd_request_change_addr(d->devaddr);
>>                  break;
>>              default:
>>                  d->devaddr = buf[1] & 0xf;
>> @@ -270,8 +270,8 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>>                      d->handler = buf[2];
>>                  }
>>  
>> -                trace_adb_kbd_request_change_addr_and_handler(d->devaddr,
>> -                                                              d->handler);
>> +                trace_adb_device_kbd_request_change_addr_and_handler(
>> +                    d->devaddr, d->handler);
>>                  break;
>>              }
>>          }
>> @@ -294,7 +294,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
>>              olen = 2;
>>              break;
>>          }
>> -        trace_adb_kbd_readreg(reg, obuf[0], obuf[1]);
>> +        trace_adb_device_kbd_readreg(reg, obuf[0], obuf[1]);
>>          break;
>>      }
>>      return olen;
>> @@ -321,7 +321,7 @@ static void adb_keyboard_event(DeviceState *dev, QemuConsole *src,
>>      /* FIXME: take handler into account when translating qcode */
>>      keycode = qcode_to_adb_keycode[qcode];
>>      if (keycode == NO_KEY) {  /* We don't want to send this to the guest */
>> -        trace_adb_kbd_no_key();
>> +        trace_adb_device_kbd_no_key();
>>          return;
>>      }
>>      if (evt->u.key.data->down == false) { /* if key release event */
>> diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
>> index e2359fd74d..577a38ff2e 100644
>> --- a/hw/input/adb-mouse.c
>> +++ b/hw/input/adb-mouse.c
>> @@ -121,7 +121,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>          s->dx = 0;
>>          s->dy = 0;
>>          s->dz = 0;
>> -        trace_adb_mouse_flush();
>> +        trace_adb_device_mouse_flush();
>>          return 0;
>>      }
>>  
>> @@ -130,7 +130,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>      olen = 0;
>>      switch (cmd) {
>>      case ADB_WRITEREG:
>> -        trace_adb_mouse_writereg(reg, buf[1]);
>> +        trace_adb_device_mouse_writereg(reg, buf[1]);
>>          switch (reg) {
>>          case 2:
>>              break;
>> @@ -152,7 +152,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>              case ADB_CMD_CHANGE_ID_AND_ACT:
>>              case ADB_CMD_CHANGE_ID_AND_ENABLE:
>>                  d->devaddr = buf[1] & 0xf;
>> -                trace_adb_mouse_request_change_addr(d->devaddr);
>> +                trace_adb_device_mouse_request_change_addr(d->devaddr);
>>                  break;
>>              default:
>>                  d->devaddr = buf[1] & 0xf;
>> @@ -172,8 +172,8 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>                      d->handler = buf[2];
>>                  }
>>  
>> -                trace_adb_mouse_request_change_addr_and_handler(d->devaddr,
>> -                                                                d->handler);
>> +                trace_adb_device_mouse_request_change_addr_and_handler(
>> +                    d->devaddr, d->handler);
>>                  break;
>>              }
>>          }
>> @@ -191,7 +191,7 @@ static int adb_mouse_request(ADBDevice *d, uint8_t *obuf,
>>              olen = 2;
>>              break;
>>          }
>> -        trace_adb_mouse_readreg(reg, obuf[0], obuf[1]);
>> +        trace_adb_device_mouse_readreg(reg, obuf[0], obuf[1]);
>>          break;
>>      }
>>      return olen;
>> diff --git a/hw/input/trace-events b/hw/input/trace-events
>> index a2888fd10c..6f0d78241c 100644
>> --- a/hw/input/trace-events
>> +++ b/hw/input/trace-events
>> @@ -1,18 +1,18 @@
>>  # See docs/devel/tracing.txt for syntax documentation.
>>  
>>  # adb-kbd.c
>> -adb_kbd_no_key(void) "Ignoring NO_KEY"
>> -adb_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> -adb_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
>> -adb_kbd_request_change_addr(int devaddr) "change addr to 0x%x"
>> -adb_kbd_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
>> +adb_device_kbd_no_key(void) "Ignoring NO_KEY"
>> +adb_device_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> +adb_device_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
>> +adb_device_kbd_request_change_addr(int devaddr) "change addr to 0x%x"
>> +adb_device_kbd_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
>>  
>>  # adb-mouse.c
>> -adb_mouse_flush(void) "flush"
>> -adb_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> -adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
>> -adb_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
>> -adb_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
>> +adb_device_mouse_flush(void) "flush"
> 
> For the following: ...
> 
>> +adb_device_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> +adb_device_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 0x%2.2x obuf[1] 0x%2.2x"
>> +adb_device_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
>> +adb_device_mouse_request_change_addr_and_handler(int devaddr, int handler) "change addr and handler to 0x%x, 0x%x"
> 
> ... you could use a common trace, using a device_type argument, i.e.:
> 
> adb_device_writereg(const char *device_type, int reg, uint8_t val) "%s
> reg %d val 0x%2.2x"
> 
> But then you can not filter traces for a particular device.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Yeah that's right. Generally whilst working on this I've tended to want to see either
everything or individual devices, so I'll leave this one as it is for now. Thanks for
the review though!


ATB,

Mark.

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

[Mark Cave-Ayland]

On 16/06/2020 11:24, Finn Thain wrote:

> Tested-by: Finn Thain <fthain@telegraphics.com.au>
> 
> Thanks for all your work on this.
> 
> I've just noticed a discrepancy between the traces from an ADB bus scan on 
> Laurent's Apple Quadra and an ADB bus scan on your patched QEMU machine.
> 
> Apple Q800:
> 
> [C1f][s   ][Rff-][Rff ][rff-]
> [C2f][s   ][R61 ][R05 ][r00-]
> [C3f][s   ][R79 ][R01 ][r00-]
> [C4f][s   ][Rff-][Rff ][rff-]
> [C5f][s   ][Rff-][Rff ][rff-]
> [C6f][s   ][Rff-][Rff ][rff-]
> [C7f][s   ][Rff-][Rff ][rff-]
> [C8f][s   ][Rff-][Rff ][rff-]
> [C9f][s   ][Rff-][Rff ][rff-]
> [Caf][s   ][Rff-][Rff ][rff-]
> [Cbf][s   ][Rff-][Rff ][rff-]
> [Ccf][s   ][Rff-][Rff ][rff-]
> [Cdf][s   ][Rff-][Rff ][rff-]
> [Cef][s   ][Rff-][Rff ][rff-]
> [Cff][s   ][Rff-][Rff ][rff-]
> 
> QEMU Q800:
> 
> [C1f][s   ][Rff-][Rff ][rff-]
> [C2f][s   ][R02 ][R01 ][r00-]
> [C3f][s   ][R03 ][R02 ][r00-]
> [C4f][s   ][R03-][R02 ][rff-]
> [C5f][s   ][R03-][R02 ][rff-]
> [C6f][s   ][R03-][R02 ][rff-]
> [C7f][s   ][R03-][R02 ][rff-]
> [C8f][s   ][R03-][R02 ][rff-]
> [C9f][s   ][R03-][R02 ][rff-]
> [Caf][s   ][R03-][R02 ][rff-]
> [Cbf][s   ][R03-][R02 ][rff-]
> [Ccf][s   ][R03-][R02 ][rff-]
> [Cdf][s   ][R03-][R02 ][rff-]
> [Cef][s   ][R03-][R02 ][rff-]
> [Cff][s   ][R03-][R02 ][rff-]
> 
> I think this could be easy to fix; it's probably just an uninitialized 
> packet buffer. When you come to submit v2, you may want to look into this.

Thanks for testing again! I think the issue here is simply that I've been doing my
testing on mailine which still has to extra switch to IDLE state before reading the
response.

Can you test with the following patch on top of the series to see if this works for you?


diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index b3fb710f7f..362878ca06 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -745,6 +745,11 @@ static void adb_via_send(MacVIAState *s, int state, uint8_t data)
                                           s->adb_data_out,
                                           s->adb_data_out_index);
         s->adb_data_in_index = 0;
+        if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
+            s->adb_data_in[0] = 0xff;
+            s->adb_data_in[1] = 0xff;
+            s->adb_data_in_size = 2;
+        }

         /*
          * If last command is TALK, store it for use by autopoll and adjust
@@ -782,9 +787,6 @@ static void adb_via_receive(MacVIAState *s, int state, uint8_t *data)
             if (adb_bus->status & ADB_STATUS_BUSTIMEOUT) {
                 *data = 0xff;
                 ms->b |= VIA1B_vADBInt;
-                s->adb_data_in[0] = 0xff;
-                s->adb_data_in[1] = 0xff;
-                s->adb_data_in_size = 2;
                 qemu_irq_raise(s->adb_data_ready);
             } else if (s->adb_data_in_size > 0) {
                 adb_bus->status = ADB_STATUS_POLLREPLY;


This should correctly initialise the buffer in both cases. If it works then I'll
squash it into the original patch for v2.


ATB,

Mark.