All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: peter.maydell@linaro.org, laurent@vivier.eu,
	david@gibson.dropbear.id.au, qemu-devel@nongnu.org,
	qemu-ppc@nongnu.org
Subject: [PULL 02/22] adb: fix adb-mouse read length and revert disable-reg3-direct-writes workaround
Date: Fri, 26 Jun 2020 10:22:57 +0100	[thread overview]
Message-ID: <20200626092317.3875-3-mark.cave-ayland@ilande.co.uk> (raw)
In-Reply-To: <20200626092317.3875-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>
Tested-by: Finn Thain <fthain@telegraphics.com.au>
Acked-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200623204936.24064-3-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 5f3a028e6a..828c5992ae 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_new(TYPE_ADB_KEYBOARD);
-        qdev_prop_set_bit(dev, "disable-direct-reg3-writes", true);
         qdev_realize_and_unref(dev, adb_bus, &error_fatal);
 
         dev = qdev_new(TYPE_ADB_MOUSE);
-        qdev_prop_set_bit(dev, "disable-direct-reg3-writes", true);
         qdev_realize_and_unref(dev, adb_bus, &error_fatal);
     }
 
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-26  9:24 UTC|newest]

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

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=20200626092317.3875-3-mark.cave-ayland@ilande.co.uk \
    --to=mark.cave-ayland@ilande.co.uk \
    --cc=david@gibson.dropbear.id.au \
    --cc=laurent@vivier.eu \
    --cc=peter.maydell@linaro.org \
    --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.