On Aug 15, 2016, at 11:55 PM, David Gibson wrote:
> On Mon, Aug 15, 2016 at 03:53:02PM -0400, Programmingkid wrote:
>>
>> On Aug 15, 2016, at 8:19 AM, David Gibson wrote:
>>
>>> On Fri, Aug 12, 2016 at 08:10:03PM -0400, John Arbuckle wrote:
>>>> Add support for the power key.
>>>>
>>>> Signed-off-by: John Arbuckle <programmingk...@gmail.com>
>>>> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>>>> ---
>>>> v4 changes
>>>> Removed double-quote from end of comment.
>>>> Removed debug printf statement.
>>>>
>>>> v3 change
>>>> Add several suggested comments.
>>>> Moved the location of an else statement in the adb_keyboard_event()
>>>> function.
>>>>
>>>> hw/input/adb.c | 43 +++++++++++++++++++++++++++++++++----------
>>>> 1 file changed, 33 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/hw/input/adb.c b/hw/input/adb.c
>>>> index 2042903..3998490 100644
>>>> --- a/hw/input/adb.c
>>>> +++ b/hw/input/adb.c
>>>> @@ -340,10 +340,25 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
>>>> s->rptr = 0;
>>>> }
>>>> s->count--;
>>>> - obuf[0] = keycode;
>>>> - /* NOTE: could put a second keycode if needed */
>>>> - obuf[1] = 0xff;
>>>> - olen = 2;
>>>> + /*
>>>> + * The power key is the only two byte value key, so it is a special
>>>> case.
>>>> + * Since 0x7f is not a used keycode for ADB we overload it to
>>>> indicate the
>>>> + * power button when we're storing keycodes in our internal buffer,
>>>> and
>>>> + * expand it out to two bytes when we send to the guest.
>>>> + */
>>>> + if (keycode == 0x7f) {
>>>> + obuf[0] = 0x7f;
>>>> + obuf[1] = 0x7f;
>>>> + olen = 2;
>>>> + } else {
>>>> + obuf[0] = keycode;
>>>> + /* NOTE: the power key key-up is the two byte sequence 0xff 0xff;
>>>> + * otherwise we could in theory send a second keycode in the
>>>> second
>>>> + * byte, but choose not to bother.
>>>> + */
>>>> + obuf[1] = 0xff;
>>>> + olen = 2;
>>>> + }
>>>>
>>>> return olen;
>>>> }
>>>> @@ -421,14 +436,22 @@ static void adb_keyboard_event(DeviceState *dev,
>>>> QemuConsole *src,
>>>> return;
>>>> }
>>>> keycode = qcode_to_adb_keycode[qcode];
>>>> - if (keycode == NO_KEY) { /* We don't want to send this to the guest
>>>> */
>>>> +
>>>> + /* The power button is a special case because it is a 16-bit value */
>>>> + if (qcode == Q_KEY_CODE_POWER) {
>>>> + if (evt->u.key.data->down == true) { /* Power button pushed
>>>> keycode */
>>>> + adb_kbd_put_keycode(s, 0x7f);
>>>> + } else { /* Power button released
>>>> keycode */
>>>> + adb_kbd_put_keycode(s, 0xff);
>>>> + }
>>>> + } else if (keycode == NO_KEY) { /* NO_KEY shouldn't be sent to guest
>>>> */
>>>> return;
>>>> + } else { /* For all non-power keys - safe for 8-bit keycodes */
>>>> + if (evt->u.key.data->down == false) { /* if key release event */
>>>> + keycode = keycode | 0x80; /* create keyboard break code */
>>>> + }
>>>> + adb_kbd_put_keycode(s, keycode);
>>>
>>> The logic in the CODE_POWER and else cases doesn't actually seem to be
>>> different AFAICT.
>>
>> The power key is a 16 bit value.
>
> I assume you're meaning the adb code here, rather than the qkey code,
> yes?
Yes.
> Oh.. btw.. your initial adb-keys.h commit has ADB_KEY_POWER commented
> out, and I didn't see that removed in any of the intervening patches.
I completely forgot about that.
It does look like I can use the else case in the adb_keyboard_event() for the
Power key.
>
>> It needs its own special case or the operating
>> system would not see it. After disabling the "if (qcode ==
>> Q_KEY_CODE_POWER)" condition
>> so the power key would go thru the else statement, the guest stopped
>> detecting the
>> power key.
>
> Well.. I don't think your earlier patches actually put a mapping in
> the table for Q_KEY_CODE_POWER, so I guess it would map to NO_KEY,
> which would explain that. But I don't see a reason you need special
> logic here rather than just adding the mapping to the table.
>
> The Q_KEY_CODE_POWER case here still only invokes a single
> adb_kbd_put_keycode() with an 8-bit value. So I'm not really seeing
> where the 16-bit value comes into play.
>
>>>> }
>>>> - if (evt->u.key.data->down == false) { /* if key release event */
>>>> - keycode = keycode | 0x80; /* create keyboard break code */
>>>> - }
>>>> -
>>>> - adb_kbd_put_keycode(s, keycode);
>>>> }
>>>>
>>>> static const VMStateDescription vmstate_adb_kbd = {
Ok I think I have a plan for my next set of patches:
patch 1: adb-keys.h initial commit
- with Power key uncommented
patch 2: add support for QKeyCode
- minus the Power key if-condition in adb_keyboard_event()
- with NO_KEY mapping added to beginning of the qcode_to_adb_keycode array
- with NO_KEY if-condition added to the adb_keyboard_event() function
- with the ADB_DPRINTF() code added to the adb_keyboard_event() (see below)
if (keycode == NO_KEY) { /* NO_KEY shouldn't be sent to guest */
ADB_DPRINTF("Ignoring NO_KEY\n");
return;
- with added commit message that states "just a mechanical substitution, which
a number of broken mappings left in."
- with qemu_input_handler_register() call moved to the realized() function
- code from power key patch merged with this patch
patch 3: correct several key assignments
- Will still fix the broken mappings left in
If this looks right I will make my next set of patches.
