Re: [Qemu-devel] [Qemu-ppc] [PATCH] adb: change handler only when recognized

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Sat, 2016-03-12 at 14:38 +0100, Hervé Poussineau wrote:
> ADB devices must take new handler into account only when they
> recognize it.
> This lets operating systems probe for valid/invalid handles, to know
> device capabilities.
> 
> Add a FIXME in keyboard handler, which should use a different
> translation
> table depending of the selected handler.

Ah interesting ! I was just debugging why my new via-pmu model in Qemu
makes the ADB mouse emulation not work, while I tracked it down to
problems in that area and started re-inventing ... your patch :-)

The other issue is we shouldn't let the device change address unless
it's one of the "special" handler IDs. MacOS 9 with a PMU tries to
send an oddball 3-bytes write to register 3 during boot to the mouse
(probably some Trackpad related magic) with "2" in the address field,
if we accept the address change, things go very wrong.

We should add support for handler 4 for the mouse at some point too
while we are at it (different protocol though reg 0 though).

I'll send a fixup patch to correctly ignore the address change for
now but I'll wait for you to rebase your patch for the rest.

Cheers,
Ben.

> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
> 
> This conflicts with some in-list patches, but may explain why
> translation tables are not
> correct, or don't work in all situations.
> I have another patch to add 3-button mouse support, but I'll wait for
> 2.7 merge window.
> 
>  hw/input/adb.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/input/adb.c b/hw/input/adb.c
> index f0ad0d4..82bfb05 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -237,6 +237,7 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t
> *obuf)
>          if (keycode == 0xe0) {
>              ext_keycode = 1;
>          } else {
> +            /* FIXME: take handler into account when translating
> keycode */
>              if (ext_keycode)
>                  adb_keycode =  pc_to_adb_keycode[keycode | 0x80];
>              else
> @@ -283,9 +284,15 @@ static int adb_kbd_request(ADBDevice *d, uint8_t
> *obuf,
>                  d->devaddr = buf[1] & 0xf;
>                  break;
>              default:
> -                /* XXX: check this */
>                  d->devaddr = buf[1] & 0xf;
> -                d->handler = buf[2];
> +                /* 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];
> +                }
>                  break;
>              }
>          }
> @@ -492,8 +499,21 @@ static int adb_mouse_request(ADBDevice *d,
> uint8_t *obuf,
>                  d->devaddr = buf[1] & 0xf;
>                  break;
>              default:
> -                /* XXX: check this */
>                  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];
> +                }
>                  break;
>              }
>          }