Re: [Qemu-devel] [PATCH v2] Split adb.c into adb.c, adb-mouse.c and adb-kbd.c

[Laurent Vivier]

Le 20/12/2017 à 14:16, Philippe Mathieu-Daudé a écrit :
> On Wed, Dec 20, 2017 at 9:14 AM, Laurent Vivier <address@hidden> wrote:
>> It makes the code clearer to separate the bus implementation
>> from the devices one.
>>
>> Replace ADB_DPRINTF() with trace events (and adding new ones in adb-kbd.c).
>> Some minor changes to make checkpatch.pl happy.
>>
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>> v2: move internal declarations to adb-internal.h
>>     replace ADB_DPRINTF() with trace event.
>>
>>  hw/input/Makefile.objs  |   2 +-
>>  hw/input/adb-internal.h |  49 ++++
>>  hw/input/adb-kbd.c      | 400 +++++++++++++++++++++++++++++++
>>  hw/input/adb-mouse.c    | 254 ++++++++++++++++++++
>>  hw/input/adb.c          | 622 
>> +-----------------------------------------------
>>  hw/input/trace-events   |   8 +
>>  6 files changed, 714 insertions(+), 621 deletions(-)
>>  create mode 100644 hw/input/adb-internal.h
>>  create mode 100644 hw/input/adb-kbd.c
>>  create mode 100644 hw/input/adb-mouse.c
>>
...
>> diff --git a/hw/input/trace-events b/hw/input/trace-events
>> index 88150ef7a6..a8d46cb766 100644
>> --- a/hw/input/trace-events
>> +++ b/hw/input/trace-events
>> @@ -1,5 +1,13 @@
>>  # See docs/devel/tracing.txt for syntax documentation.
>>
>> +# hw/input/adb-kbd.c
>> +adb_kbd_no_key(void) "Ignoring NO_KEY"
>> +adb_kbd_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> +adb_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 
>> 0x%2.2x obuf[1] 0x%2.2x"
> 
> Minor aesthetical change:
> adb_kbd_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf
> [0x%2.2x, 0x%2.2x]"
> 
> Anyway, regardless this:
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> 
>> +# hw/input/adb-mouse.c
>> +adb_mouse_writereg(int reg, uint8_t val) "reg %d val 0x%2.2x"
>> +adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf[0] 
>> 0x%2.2x obuf[1] 0x%2.2x"
> 
> Ditto:
> adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg %d obuf
> [0x%2.2x, 0x%2.2x]"
> 

I will in case of a v3: I don't think it is worth another round only for
that.

Thanks,
Laurent