From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39491) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W370D-0007ds-EM for qemu-devel@nongnu.org; Tue, 14 Jan 2014 11:38:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W3707-00080c-CH for qemu-devel@nongnu.org; Tue, 14 Jan 2014 11:38:09 -0500 Received: from mail-qe0-x229.google.com ([2607:f8b0:400d:c02::229]:40690) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3707-00080K-7D for qemu-devel@nongnu.org; Tue, 14 Jan 2014 11:38:03 -0500 Received: by mail-qe0-f41.google.com with SMTP id i11so24530qej.28 for ; Tue, 14 Jan 2014 08:38:02 -0800 (PST) Sender: Paolo Bonzini Message-ID: <52D567E4.6070209@redhat.com> Date: Tue, 14 Jan 2014 17:37:56 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1389652389-22241-1-git-send-email-romain.naour@openwide.fr> <1389652389-22241-2-git-send-email-romain.naour@openwide.fr> <52D46C6D.8070300@redhat.com> <52D47B02.6080905@openwide.fr> In-Reply-To: <52D47B02.6080905@openwide.fr> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/1] genius: add genius serial mouse emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Romain Naour Cc: qemu-devel@nongnu.org Il 14/01/2014 00:47, Romain Naour ha scritto: > Hi, > > Thanks for review. > > Le 13/01/2014 23:45, Eric Blake a écrit : >> On 01/13/2014 03:33 PM, Romain Naour wrote: >>> This patch adds the emulation for a serial Genius mouse using >>> Mouse Systems protocol (5bytes). >>> This protocol is compatible with most 3-button serial mouse. >>> >>> Signed-off-by: Romain Naour >>> --- >>> backends/Makefile.objs | 2 +- >>> backends/gnmouse.c | 339 >>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>> include/sysemu/char.h | 3 + >>> qapi-schema.json | 1 + >>> qemu-char.c | 4 + >>> qemu-options.hx | 10 ++ >>> 6 files changed, 358 insertions(+), 1 deletion(-) >>> create mode 100644 backends/gnmouse.c >>> +++ b/backends/gnmouse.c >>> @@ -0,0 +1,339 @@ >>> +/* >>> + * QEMU Genius GM-6 serial mouse emulation >>> + * >>> + * Adapted from msmouse >>> + * >>> + * Copyright (c) 2012 Romain Naour >> It is now 2014. > Fixed >> >> >>> +++ b/qapi-schema.json >>> @@ -3617,6 +3617,7 @@ >>> 'null' : 'ChardevDummy', >>> 'mux' : 'ChardevMux', >>> 'msmouse': 'ChardevDummy', >>> + 'gnmouse': 'ChardevDummy', >>> 'braille': 'ChardevDummy', >>> 'stdio' : 'ChardevStdio', >>> 'console': 'ChardevDummy', >> Ideally, you should also document that this branch was not always >> available in the union (I was expecting to see a doc line with '(since >> 2.0)' somewhere in the patch); but looking at the existing schema, >> you're just copying bad practice of adding to an already >> under-documented union. >> > > You mean something like that ? > ## > # @ChardevBackendKind: > # > # A union referencing different chardev backend configuration' info. > # > # Since: 1.4 > # > # gnmouse backend since: 2.0 > ## > { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile', > [snip] > The standard is more like # A union referencing different chardev backend configuration' info. # # @gnmouse: Send mouse data using the Genius protocol (since 2.0). # # Since: 1.4 ## but that format would require doucmenting all items. :( You can leave this out of v2. However, I'd be happy if you changed the documentation like this @item -chardev msmouse,id=@var{id} Forward events from QEMU's emulated mouse to the guest using the Microsoft protocol. @option{msmouse} does not take any options. @item -chardev gnmouse ,id=@var{id} Forward events from QEMU's emulated mouse to the guest using the Genius (Mouse Systems) protocol. @option{gnmouse} does not take any options.