Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Programmingkid <programmingkidx@gmail.com>]

On Mar 13, 2016, at 11:40 AM, Peter Maydell wrote:

> On 12 March 2016 at 05:40, Programmingkid <programmingkidx@gmail.com> wrote:
>> 
>> On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote:
>> 
>>> 
>>>> +    }
>>>> +    keycode = s->data[s->rptr];
>>>> +    if (++s->rptr == sizeof(s->data)) {
>>>> +        s->rptr = 0;
>>>>    }
>>>> +    s->count--;
>>>> +
>>>> +    obuf[0] = keycode;
>>> 
>>> 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.
>>> 
>>>> +    /* NOTE: could put a second keycode if needed */
>>>> +    obuf[1] = 0xff;
>>>> +    olen = 2;
>>>> +
>>>>    return olen;
>>>> }
>> 
>> Is this ok?
>> 
>>    /* The power key is the only two byte value key, so it is a special case. */
>>    if (keycode == (ADB_KEY_POWER & 0x00ff)) {
>>        obuf[0] = ADB_KEY_POWER & 0x00ff;
>>        obuf[1] = ADB_KEY_POWER & 0xff00 >> 8;
>>        olen = 2;
>>    } else {
>>        obuf[0] = keycode;
>>        /* NOTE: could put a second keycode if needed */
>>        obuf[1] = 0xff;
>>        olen = 2;
>>    }
>> 
>> The keycode value comes from an 8 bit array so holding the
>> full value of the power key is not possible.
> 
> 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 == Q_KEY_CODE_POWER) {
>    /* put power key into buffer */
>    [...]
> } else {
>    keycode = qcode_to_adb_keycode[...];
>    etc;
> }
> and not put it into the array at all.
> 
> 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 = keycode & 0x80.