[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

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

[Benjamin Herrenschmidt]

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

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

[BALATON Zoltan]

On Tue, 9 Aug 2016, Benjamin Herrenschmidt wrote:
> 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.

I don't know much about this but I've read here 
<http://mcosre.sourceforge.net/docs/apple_io.html> that there are three 
different kind of chips: CUDA, PMU99 and PMU. Confusingly both PMU-s are 
called via-pmu by Apple. I think we want PMU99 which is found on desktop 
machines and not PMU found in Powerbooks (unless we want to emulate that) 
because PMU99 has less features and more similar to CUDA. But it has some 
differences such as a different interrupt mentioned in this page and maybe 
others. However, desktop machines have no ADB so I'm not sure why we have 
one still in QEMU. If USB is working then we don't need ADB with PMU99 and 
that could resolve all the problems with it. We can keep it to old world 
beige G3 with CUDA where it works. I think only some Powerbooks have PMU 
and ADB but we are not targeting that. Here's an (untested) patch for 
switching to using USB keyboard and mouse instead of ADB unless USB is 
disabled. (The device tree should be changed accordingly in OpenBIOS.) Not 
sure if this is helpful.

--
Regards, 
BALATON Zoltan

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 7d25106..bf8ad9b 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -418,24 +418,21 @@ static void ppc_core99_init(MachineState *machine)
                                                          "ide[1]"));
      macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]);

-    dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda"));
-    adb_bus = qdev_get_child_bus(dev, "adb.0");
-    dev = qdev_create(adb_bus, TYPE_ADB_KEYBOARD);
-    qdev_init_nofail(dev);
-    dev = qdev_create(adb_bus, TYPE_ADB_MOUSE);
-    qdev_init_nofail(dev);
-
      if (machine->usb) {
          pci_create_simple(pci_bus, -1, "pci-ohci");

-        /* U3 needs to use USB for input because Linux doesn't support via-cuda
-        on PPC64 */
-        if (machine_arch == ARCH_MAC99_U3) {
-            USBBus *usb_bus = usb_bus_find(-1);
+        /* New world machines have USB instead of ADB */
+        USBBus *usb_bus = usb_bus_find(-1);

-            usb_create_simple(usb_bus, "usb-kbd");
-            usb_create_simple(usb_bus, "usb-mouse");
-        }
+        usb_create_simple(usb_bus, "usb-kbd");
+        usb_create_simple(usb_bus, "usb-mouse");
+    } else {
+        dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda"));
+        adb_bus = qdev_get_child_bus(dev, "adb.0");
+        dev = qdev_create(adb_bus, TYPE_ADB_KEYBOARD);
+        qdev_init_nofail(dev);
+        dev = qdev_create(adb_bus, TYPE_ADB_MOUSE);
+        qdev_init_nofail(dev);
      }

      pci_vga_init(pci_bus);

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

[Benjamin Herrenschmidt]

On Tue, 2016-08-09 at 02:11 +0200, BALATON Zoltan wrote:
> I don't know much about this but I've read here 
> > <http://mcosre.sourceforge.net/docs/apple_io.html> that there are three 
> different kind of chips: CUDA, PMU99 and PMU. Confusingly both PMU-s are 
> > called via-pmu by Apple.

And there's Egret ... ;-)

No really we don't care. There's more than 2 kinds in fact, it's just
depends whatever microcontroller's in there. But it's the same
protocol. It's always connected to the VIA.

>  I think we want PMU99 which is found on desktop 
> machines and not PMU found in Powerbooks (unless we want to emulate that) 
> > because PMU99 has less features and more similar to CUDA.

PMU99 doesn't have "less" features, it's the same in the desktop G4 and
a PowerBook G4. I will eventually add some of the powerbook'ish
features so we can support sleep ...

>  But it has some 
> differences such as a different interrupt mentioned in this page and maybe 
> > others.

The different interrupt is just a hack they did with KeyLargo ASIC,
instead of using the VIA CB1, it uses a GPIO to signal that the OS
needs to fetch interrupt conditions, but that's about it. The basic VIA
SR interrupt is still used for the basic shifting.

>  However, desktop machines have no ADB so I'm not sure why we have 
> > one still in QEMU.

Because PowerBooks do (or rather a PMU-simulation of ADB) and MacOS
doesn't care. If ADB is in the device-tree, it will use it. It makes
things easier to support multiple combinations especially when
"comparing" things for debug.

Additionally, USB doesn't work well in OpenBIOS at this point ;-)

Also, I have some evil plan to change the way ADB autopoll works in
Qemu so that the devices signal the PMU when they want to talk. That
will avoid having yet another 30-something HZ timer ticking in qemu,
and in that regard will probably be more efficient (read: slows down
the emulator less) than OHCI.

>  If USB is working then we don't need ADB with PMU99 and 
> > that could resolve all the problems with it.

I have solved the ADB problems so there's nothing left to solve here :-
) But see above why I think it's a good idea to keep ADB as an option.

>  We can keep it to old world 
> beige G3 with CUDA where it works. I think only some Powerbooks have PMU 
> > and ADB but we are not targeting that.

Why not ?
>  Here's an (untested) patch for
> switching to using USB keyboard and mouse instead of ADB unless USB is 
> disabled. (The device tree should be changed accordingly in OpenBIOS.) Not 
> sure if this is helpful.

Don't bother just yet, as I said, I am reworking all of that code. I will
probably just create various -machine options so you can chose what bits
and pieces you want to put togeher, ie, CUDA, PMU, with or without ADB,
which machine model string to expose to the OS etc...

Cheers,
Ben.

> > --

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

[BALATON Zoltan]

On Tue, 9 Aug 2016, Benjamin Herrenschmidt wrote:
> On Tue, 2016-08-09 at 02:11 +0200, BALATON Zoltan wrote:
>> I don't know much about this but I've read here 
>>> <http://mcosre.sourceforge.net/docs/apple_io.html> that there are three 
>> different kind of chips: CUDA, PMU99 and PMU. Confusingly both PMU-s are 
>>> called via-pmu by Apple.
>
> And there's Egret ... ;-)

Right, for completeness, but that's only in very old machines we don't 
emulate in QEMU.

>>  However, desktop machines have no ADB so I'm not sure why we have 
>>> one still in QEMU.
>
> Because PowerBooks do (or rather a PMU-simulation of ADB) and MacOS
> doesn't care. If ADB is in the device-tree, it will use it. It makes
> things easier to support multiple combinations especially when
> "comparing" things for debug.
>
> Additionally, USB doesn't work well in OpenBIOS at this point ;-)

In what way? Keyboard works. What else is needed?

> Also, I have some evil plan to change the way ADB autopoll works in
> Qemu so that the devices signal the PMU when they want to talk. That
> will avoid having yet another 30-something HZ timer ticking in qemu,
> and in that regard will probably be more efficient (read: slows down
> the emulator less) than OHCI.

OK, this is a really nice thing and justifies having an ADB bus. (This 
would also likely fix mouse problems seen by others.)

>>  If USB is working then we don't need ADB with PMU99 and 
>>> that could resolve all the problems with it.
>
> I have solved the ADB problems so there's nothing left to solve here :-
> ) But see above why I think it's a good idea to keep ADB as an option.
>
>>  We can keep it to old world 
>> beige G3 with CUDA where it works. I think only some Powerbooks have PMU 
>>> and ADB but we are not targeting that.
>
> Why not ?

Just thought emulating all the additional details for Powerbook power 
management might be difficult. But if you're willing to go there I won't 
stop you. :-) Obviously you understand this very well so maybe it's not 
that difficult for you.

>>  Here's an (untested) patch for
>> switching to using USB keyboard and mouse instead of ADB unless USB is 
>> disabled. (The device tree should be changed accordingly in OpenBIOS.) Not 
>> sure if this is helpful.
>
> Don't bother just yet, as I said, I am reworking all of that code. I will
> probably just create various -machine options so you can chose what bits
> and pieces you want to put togeher, ie, CUDA, PMU, with or without ADB,
> which machine model string to expose to the OS etc...

Cool, looking forward to it. Thanks for all the great stuff you did for 
this and keep it up.

Regards,
BALATON Zoltan

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

[Benjamin Herrenschmidt]

On Tue, 2016-08-09 at 03:31 +0200, BALATON Zoltan wrote:
> 
> > Because PowerBooks do (or rather a PMU-simulation of ADB) and MacOS
> > doesn't care. If ADB is in the device-tree, it will use it. It makes
> > things easier to support multiple combinations especially when
> > "comparing" things for debug.
> > 
> > Additionally, USB doesn't work well in OpenBIOS at this point ;-)
> 
> In what way? Keyboard works. What else is needed?

Doesnt' work for me half of the time, I haven't dug into why yet.

> > Also, I have some evil plan to change the way ADB autopoll works in
> > Qemu so that the devices signal the PMU when they want to talk. That
> > will avoid having yet another 30-something HZ timer ticking in qemu,
> > and in that regard will probably be more efficient (read: slows down
> > the emulator less) than OHCI.
> 
> OK, this is a really nice thing and justifies having an ADB bus. (This 
> would also likely fix mouse problems seen by others.)

Possibly ;-) The tracking in OS 9 at least is still done by a timer
inside MacOS itself. With my latest ndrv it's running at 30Hz. I'm
looking at maybe doing some kind of paravirt hack to make it adaptative
on whether there is actual movement on the cursor, but that's for later.

> > Why not ?
> 
> Just thought emulating all the additional details for Powerbook power 
> management might be difficult. But if you're willing to go there I won't 
> stop you. :-) Obviously you understand this very well so maybe it's not 
> that difficult for you.

Well, I wrote most of the corresponding Linux code so it's mostly a matter
of swapping that knowledge back into my brain from lossy long term storage
and finding all the MacOS 9 bugs along the way :-)

> > > 
> > >  Here's an (untested) patch for
> > > switching to using USB keyboard and mouse instead of ADB unless USB is 
> > > disabled. (The device tree should be changed accordingly in OpenBIOS.) Not 
> > > sure if this is helpful.
> > 
> > Don't bother just yet, as I said, I am reworking all of that code. I will
> > probably just create various -machine options so you can chose what bits
> > and pieces you want to put togeher, ie, CUDA, PMU, with or without ADB,
> > which machine model string to expose to the OS etc...
> 
> Cool, looking forward to it. Thanks for all the great stuff you did for 
> this and keep it up.

Cheers,
Ben.

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

[Howard Spoelstra]

On Tue, Aug 9, 2016 at 6:06 AM, Benjamin Herrenschmidt <
benh@kernel.crashing.org> wrote:

> On Tue, 2016-08-09 at 03:31 +0200, BALATON Zoltan wrote:
> >
> > > Because PowerBooks do (or rather a PMU-simulation of ADB) and MacOS
> > > doesn't care. If ADB is in the device-tree, it will use it. It makes
> > > things easier to support multiple combinations especially when
> > > "comparing" things for debug.
> > >
> > > Additionally, USB doesn't work well in OpenBIOS at this point ;-)
> >
> > In what way? Keyboard works. What else is needed?
>
> Doesnt' work for me half of the time, I haven't dug into why yet.
>
> > > Also, I have some evil plan to change the way ADB autopoll works in
> > > Qemu so that the devices signal the PMU when they want to talk. That
> > > will avoid having yet another 30-something HZ timer ticking in qemu,
> > > and in that regard will probably be more efficient (read: slows down
> > > the emulator less) than OHCI.
> >
> > OK, this is a really nice thing and justifies having an ADB bus. (This
> > would also likely fix mouse problems seen by others.)
>
> Possibly ;-) The tracking in OS 9 at least is still done by a timer
> inside MacOS itself. With my latest ndrv it's running at 30Hz. I'm
> looking at maybe doing some kind of paravirt hack to make it adaptative
> on whether there is actual movement on the cursor, but that's for later.
>
> > > Why not ?
> >
> > Just thought emulating all the additional details for Powerbook power
> > management might be difficult. But if you're willing to go there I won't
> > stop you. :-) Obviously you understand this very well so maybe it's not
> > that difficult for you.
>
> Well, I wrote most of the corresponding Linux code so it's mostly a matter
> of swapping that knowledge back into my brain from lossy long term storage
> and finding all the MacOS 9 bugs along the way :-)
>
> > > >
> > > >  Here's an (untested) patch for
> > > > switching to using USB keyboard and mouse instead of ADB unless USB
> is
> > > > disabled. (The device tree should be changed accordingly in
> OpenBIOS.) Not
> > > > sure if this is helpful.
> > >
> > > Don't bother just yet, as I said, I am reworking all of that code. I
> will
> > > probably just create various -machine options so you can chose what
> bits
> > > and pieces you want to put togeher, ie, CUDA, PMU, with or without ADB,
> > > which machine model string to expose to the OS etc...
> >
> > Cool, looking forward to it. Thanks for all the great stuff you did for
> > this and keep it up.
>
> Cheers,
> Ben.
>
>
Openbios has problems with the USB keyboard when one specifically defines
e.g., -device ich9-usb-uhci1,id=newusb -device usb-mouse,bus=newusb.0
(which is needed to get mouse/keyboard going in OSX 10.5)

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

[BALATON Zoltan]

On Tue, 9 Aug 2016, Howard Spoelstra wrote:
> Openbios has problems with the USB keyboard when one specifically defines
> e.g., -device ich9-usb-uhci1,id=newusb -device usb-mouse,bus=newusb.0
> (which is needed to get mouse/keyboard going in OSX 10.5)

I think it does not have a UHCI driver, only minimal HID support on OHCI. 
I could not reproduce a problem with this command on the master HEAD 
version, only if I create a usb-kbd explicitely with -device is when it 
does not work. Creating only a usb-mouse on either pci-ohci or 
ich9-usb-uhci1 (does that make sense on a PPC Mac?) seems working for me 
and still have keyboard in OpenBIOS but adding a keyboard this way indeed 
has a problem.

Regards,
BALATON Zoltan

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

[Benjamin Herrenschmidt]

On Tue, 2016-08-09 at 12:26 +0200, BALATON Zoltan wrote:
> On Tue, 9 Aug 2016, Howard Spoelstra wrote:
> > 
> > Openbios has problems with the USB keyboard when one specifically defines
> > e.g., -device ich9-usb-uhci1,id=newusb -device usb-mouse,bus=newusb.0
> > (which is needed to get mouse/keyboard going in OSX 10.5)
> 
> I think it does not have a UHCI driver, only minimal HID support on OHCI. 
> I could not reproduce a problem with this command on the master HEAD 
> version, only if I create a usb-kbd explicitely with -device is when it 
> does not work. Creating only a usb-mouse on either pci-ohci or 
> ich9-usb-uhci1 (does that make sense on a PPC Mac?) seems working for me 
> and still have keyboard in OpenBIOS but adding a keyboard this way indeed 
> has a problem.

UHCI definitely doesn't make sense on a ppc mac. OHCI and EHCI do.

Cheers,
Ben.

[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 --]