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

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

[Hervé Poussineau]

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(-)

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;
             }
         }
-- 
2.1.4

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

[Programmingkid]

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. 

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

[Mark Cave-Ayland]

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.

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

[Hervé Poussineau]

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é

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

[Programmingkid]

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. 

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

[Hervé Poussineau]

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é

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

[Peter Maydell]

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

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

[Hervé Poussineau]

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>
---
 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 3d39368..43d3205 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -396,9 +396,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;
             }
         }
@@ -437,6 +443,7 @@ static void adb_keyboard_event(DeviceState *dev, QemuConsole *src,
     if (qcode >= ARRAY_SIZE(qcode_to_adb_keycode)) {
         return;
     }
+    /* 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 */
         ADB_DPRINTF("Ignoring NO_KEY\n");
@@ -631,8 +638,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;
             }
         }
-- 
2.1.4

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3109 bytes --]

On Tue, Oct 25, 2016 at 09:01:01AM +0200, 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>

Applied to ppc-for-2.8, thanks.

> ---
>  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 3d39368..43d3205 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -396,9 +396,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;
>              }
>          }
> @@ -437,6 +443,7 @@ static void adb_keyboard_event(DeviceState *dev, QemuConsole *src,
>      if (qcode >= ARRAY_SIZE(qcode_to_adb_keycode)) {
>          return;
>      }
> +    /* 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 */
>          ADB_DPRINTF("Ignoring NO_KEY\n");
> @@ -631,8 +638,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;
>              }
>          }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]