From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33274) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afbZg-0002i8-Vq for qemu-devel@nongnu.org; Mon, 14 Mar 2016 19:06:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afbZd-0003Og-3w for qemu-devel@nongnu.org; Mon, 14 Mar 2016 19:06:56 -0400 Received: from mail-ig0-x241.google.com ([2607:f8b0:4001:c05::241]:35760) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afbZc-0003OX-Ur for qemu-devel@nongnu.org; Mon, 14 Mar 2016 19:06:53 -0400 Received: by mail-ig0-x241.google.com with SMTP id ww10so260219igb.2 for ; Mon, 14 Mar 2016 16:06:52 -0700 (PDT) Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Programmingkid In-Reply-To: Date: Mon, 14 Mar 2016 19:06:45 -0400 Content-Transfer-Encoding: quoted-printable Message-Id: <77582ECD-0206-4FD9-AE2C-3B31E78FA44F@gmail.com> References: <8768A4C2-CEBF-411A-9B43-9F43EA755238@gmail.com> <41EC2072-FB9D-4003-A0CB-1DBFA0B2A27D@gmail.com> Subject: Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Gerd Hoffmann , qemu-devel qemu-devel On Mar 13, 2016, at 11:40 AM, Peter Maydell wrote: > On 12 March 2016 at 05:40, Programmingkid = wrote: >>=20 >> On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote: >>=20 >>>=20 >>>> + } >>>> + keycode =3D s->data[s->rptr]; >>>> + if (++s->rptr =3D=3D sizeof(s->data)) { >>>> + s->rptr =3D 0; >>>> } >>>> + s->count--; >>>> + >>>> + obuf[0] =3D keycode; >>>=20 >>> You are still trying to put a two byte keycode (ADB_KEY_POWER) >>> into this one-byte array slot. I don't know what the right way to >>> send a two-byte keycode is but this is obviously not it, as >>> I said before. >>>=20 >>>> + /* NOTE: could put a second keycode if needed */ >>>> + obuf[1] =3D 0xff; >>>> + olen =3D 2; >>>> + >>>> return olen; >>>> } >>=20 >> Is this ok? >>=20 >> /* The power key is the only two byte value key, so it is a = special case. */ >> if (keycode =3D=3D (ADB_KEY_POWER & 0x00ff)) { >> obuf[0] =3D ADB_KEY_POWER & 0x00ff; >> obuf[1] =3D ADB_KEY_POWER & 0xff00 >> 8; >> olen =3D 2; >> } else { >> obuf[0] =3D keycode; >> /* NOTE: could put a second keycode if needed */ >> obuf[1] =3D 0xff; >> olen =3D 2; >> } >>=20 >> The keycode value comes from an 8 bit array so holding the >> full value of the power key is not possible. >=20 > Ah, I hadn't noticed that -- that is not a good approach. > You should either: > (a) deal with the fact that ADB_KEY values may be 16 bits > all the way through (including having that array be uint16_t > rather than uint8_t) > (b) have the power key be a completely special case which > is handled by > if (qcode =3D=3D Q_KEY_CODE_POWER) { > /* put power key into buffer */ > [...] > } else { > keycode =3D qcode_to_adb_keycode[...]; > etc; > } > and not put it into the array at all. >=20 > Also, you need to handle the power-key key-release > scancode; as far as I can tell (by looking for info via > google about the ADB protocol) power-key-down is > 0x7f 0x7f, and power-key-up is 0xff 0xff. (This is > kind of weird and suggests we should just take option > (a) and special case the power key completely.) The power-key-up (break) keycode is 0xff 0xff? Could you send me your = source for this information? I always thought the up (break) code was = the keycode AND with 0x80. Or this if code is easier to understand: break_keycode =3D keycode & 0x80.=