| Message ID | 1457789937-30923-1-git-send-email-hpoussin@reactos.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mar 12, 2016, at 8:38 AM, 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. > > 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. I spent some time trying to add two button mouse support to adb. I would really like to try out your three button mouse patch please.
On 12/03/16 13:38, 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. > > Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> Interesting. Can you explain a bit more about which OSs this patch affects and the symptoms it alleviates? ATB, Mark.
Le 12/03/2016 16:24, Mark Cave-Ayland a écrit : > On 12/03/16 13:38, 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. >> >> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> > > Interesting. Can you explain a bit more about which OSs this patch > affects and the symptoms it alleviates? Here is a small list of handlers requested by some operating systems without the patch HelenOS kbd=1 mouse=2 MacOS 9 kbd=1 mouse=0xc (what is 0xc?) Linux kbd=3 mouse=4 Here is a small list of handlers requested by some operating systems with the patch HelenOS kbd=1 mouse=2 MacOS 9 kbd=1 mouse=2 Linux kbd=3 mouse=2 I have no example of current problem with the keyboard part. However, I suspect it may be related some problems John is seeing on some operating systems, as handler 1 and 2/3 must not use the same translation table. Note that MacOS 9 uses handler 1 (Apple Standard Keyboard), while Linux uses handler 3 (Apple Extended Keyboard LShift != RShift). On mouse part, operating systems (like MacOS or Linux) try to probe the mouse model by testing different handlers, and see which ones are accepted. On Linux, the handler 1 and 2 have a 3 bytes protocol, while the handler 4 has a 4 bytes protocol. Correctly supporting protocol 4 will be required to handle 3-button mice. Hervé
On Mar 12, 2016, at 12:44 PM, Hervé Poussineau wrote: > Le 12/03/2016 16:24, Mark Cave-Ayland a écrit : >> On 12/03/16 13:38, 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. >>> >>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >> >> Interesting. Can you explain a bit more about which OSs this patch >> affects and the symptoms it alleviates? > > Here is a small list of handlers requested by some operating systems without the patch > HelenOS kbd=1 mouse=2 > MacOS 9 kbd=1 mouse=0xc (what is 0xc?) > Linux kbd=3 mouse=4 > > Here is a small list of handlers requested by some operating systems with the patch > HelenOS kbd=1 mouse=2 > MacOS 9 kbd=1 mouse=2 > Linux kbd=3 mouse=2 > > > I have no example of current problem with the keyboard part. However, I suspect it may be related some > problems John is seeing on some operating systems, as handler 1 and 2/3 must not use the same > translation table. > Note that MacOS 9 uses handler 1 (Apple Standard Keyboard), while Linux uses handler 3 (Apple Extended Keyboard LShift != RShift). > > On mouse part, operating systems (like MacOS or Linux) try to probe the mouse model by testing > different handlers, and see which ones are accepted. > On Linux, the handler 1 and 2 have a 3 bytes protocol, while the handler 4 has a 4 bytes protocol. > Correctly supporting protocol 4 will be required to handle 3-button mice. > > Hervé Very interesting. Thank you for this information. So it sounds like the keys should change in the keyboard array with respect to the value of kbd. It would have to happen during runtime. Do you have any documentation related to your handler code? Particularly what keys are and not available for a particular handler/keyboard.
Le 12/03/2016 19:13, Programmingkid a écrit : > > On Mar 12, 2016, at 12:44 PM, Hervé Poussineau wrote: > >> Le 12/03/2016 16:24, Mark Cave-Ayland a écrit : >>> On 12/03/16 13:38, 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. >>>> >>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >>> >>> Interesting. Can you explain a bit more about which OSs this patch >>> affects and the symptoms it alleviates? >> >> Here is a small list of handlers requested by some operating systems without the patch >> HelenOS kbd=1 mouse=2 >> MacOS 9 kbd=1 mouse=0xc (what is 0xc?) >> Linux kbd=3 mouse=4 >> >> Here is a small list of handlers requested by some operating systems with the patch >> HelenOS kbd=1 mouse=2 >> MacOS 9 kbd=1 mouse=2 >> Linux kbd=3 mouse=2 >> >> >> I have no example of current problem with the keyboard part. However, I suspect it may be related some >> problems John is seeing on some operating systems, as handler 1 and 2/3 must not use the same >> translation table. >> Note that MacOS 9 uses handler 1 (Apple Standard Keyboard), while Linux uses handler 3 (Apple Extended Keyboard LShift != RShift). >> >> On mouse part, operating systems (like MacOS or Linux) try to probe the mouse model by testing >> different handlers, and see which ones are accepted. >> On Linux, the handler 1 and 2 have a 3 bytes protocol, while the handler 4 has a 4 bytes protocol. >> Correctly supporting protocol 4 will be required to handle 3-button mice. >> >> Hervé > > Very interesting. Thank you for this information. So it sounds like the keys should change in the keyboard array with respect to the value of kbd. It would have to happen during runtime. Do you have any documentation related to your handler code? Particularly what keys are and not available for a particular handler/keyboard. Of course, I've no real documentation for Standard Keyboard vs Extended Keyboard... However, on a side note, while checking Linux code (for example https://www.kernel.org/pub/linux/kernel/people/marcelo/linux-2.4/drivers/macintosh/adbhid.c ) I saw that keys Mute/VolumeUp/VolumeDown/EjectCD/BrightnessIncrease/BrightnessDecrease are not on the keyboard, but on another ADB device with category "misc". Might be interesting if we want to add support for these keys one day in QEMU. Hervé
On 12 March 2016 at 20:31, Hervé Poussineau <hpoussin@reactos.org> wrote: > Of course, I've no real documentation for Standard Keyboard vs Extended > Keyboard... Try http://www.archive.org/stream/apple-guide-macintosh-family-hardware/Apple_Guide_to_the_Macintosh_Family_Hardware_2e#page/n345/mode/2up pages 306 and 308 ? thanks -- PMM
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; > } > }
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; } }
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. 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(-)