All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, laurent@vivier.eu,
	fthain@telegraphics.com.au
Subject: [PATCH 02/22] adb: fix adb-mouse read length and revert disable-reg3-direct-writes workaround
Date: Sun, 14 Jun 2020 15:28:20 +0100	[thread overview]
Message-ID: <20200614142840.10245-3-mark.cave-ayland@ilande.co.uk> (raw)
In-Reply-To: <20200614142840.10245-1-mark.cave-ayland@ilande.co.uk>

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



  parent reply	other threads:[~2020-06-14 14:32 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-14 14:28 [PATCH 00/22] ADB: fix autopoll issues and rework mac_via state machine Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 01/22] adb: coding style update to fix checkpatch errors Mark Cave-Ayland
2020-06-14 16:49   ` Philippe Mathieu-Daudé
2020-06-14 14:28 ` Mark Cave-Ayland [this message]
2020-06-14 14:28 ` [PATCH 03/22] cuda: convert ADB autopoll timer from ns to ms Mark Cave-Ayland
2020-06-14 16:50   ` Philippe Mathieu-Daudé
2020-06-14 14:28 ` [PATCH 04/22] pmu: fix duplicate autopoll mask variable Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 05/22] pmu: honour autopoll_rate_ms when rearming the ADB autopoll timer Mark Cave-Ayland
2020-06-14 16:50   ` Philippe Mathieu-Daudé
2020-06-14 14:28 ` [PATCH 06/22] adb: introduce realize/unrealize and VMStateDescription for ADB bus Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 07/22] adb: create autopoll variables directly within ADBBusState Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 08/22] cuda: convert to use ADBBusState internal autopoll variables Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 09/22] pmu: " Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 10/22] mac_via: " Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 11/22] adb: introduce new ADBDeviceHasData method to ADBDeviceClass Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 12/22] adb: keep track of devices with pending data Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 13/22] adb: add status field for holding information about the last ADB request Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 14/22] adb: use adb_request() only for explicit requests Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 15/22] adb: add autopoll_blocked variable to block autopoll Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 16/22] cuda: add adb_autopoll_block() and adb_autopoll_unblock() functions Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 17/22] pmu: " Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 18/22] mac_via: move VIA1 portB write logic into mos6522_q800_via1_write() Mark Cave-Ayland
2020-06-14 17:03   ` Philippe Mathieu-Daudé
2020-06-14 14:28 ` [PATCH 19/22] mac_via: rework ADB state machine to be compatible with both MacOS and Linux Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 20/22] adb: only call autopoll callbacks when autopoll is not blocked Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 21/22] adb: use adb_device prefix for ADB device trace events Mark Cave-Ayland
2020-06-14 17:20   ` Philippe Mathieu-Daudé
2020-06-20 12:01     ` Mark Cave-Ayland
2020-06-14 14:28 ` [PATCH 22/22] adb: add ADB bus " Mark Cave-Ayland
2020-06-14 17:16   ` Philippe Mathieu-Daudé
2020-06-20 11:59     ` Mark Cave-Ayland
2020-06-16 10:24 ` [PATCH 00/22] ADB: fix autopoll issues and rework mac_via state machine Finn Thain
2020-06-20 12:10   ` Mark Cave-Ayland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200614142840.10245-3-mark.cave-ayland@ilande.co.uk \
    --to=mark.cave-ayland@ilande.co.uk \
    --cc=fthain@telegraphics.com.au \
    --cc=laurent@vivier.eu \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.