[Qemu-devel] [PATCH 00/17] add serial wacom tablet emulation (gsoc 2016)

[Qemu-devel] [PATCH 00/17] add serial wacom tablet emulation (gsoc 2016)

[Gerd Hoffmann]

  Hi,

Finally found the time to put the wctablet code into shape.  Here comes
the patch series.

Patch #1 is the submission by Anatoli, almost unmodified.  I've only
adapted it to the recent chardev changes in the qemu code base so it
actually builds and works on current qemu.

The following patches improve the code base:
 * various cleanups (codestyle, delete unused stuff).
 * use accept_input insted of a timer.
 * use new input interface to register the driver.
 * rewrite command detection.
 * added some helper functions.
 * added trace events.

Functional changes:
 * handle line speed changes.
 * implement ST and SP commands.

See individual commit messages for more details.

Current state:
 * wacom test application works.
 * windows 3.11 driver works.
 * linux guest fails (inputattach --wacom_iv).  Probably due to '~C'
   not being implemented, that is the last command sent by linux
   before it throws an error.
 * left button acts somewhat strange.  One host side mouse click seems
   to toggle the button state inside the guest (windows 3.11).  Not sure
   whenever that is a bug in the emulation or something the guest driver
   is doing ...

cheers,
  Gerd

Anatoli Huseu1 (1):
  Add wctablet device

Gerd Hoffmann (16):
  wctablet: add wctablet_queue_output helper
  wctablet: save all chars in the query buffer
  wctablet: drop wctablet_commands_names
  wctablet: strip leading \r + \n from buffer
  wctablet: track line speed, reset on speed changes
  wctablet: operate on line speed 9600
  wctablet: drop debug code from wctablet_handler
  wctablet: add wctablet_shift_input
  wctablet: move init/detect sequence
  wctablet: revamp command parser.
  wctablet: drop timer, hook into chr->accept_input instead
  wctablet: drop DPRINTF, add trace events instead
  wctablet: misc cleanups
  wctablet: switch to new input interface
  wctablet: update file comment
  wctablet: implement ST and SP commands

 Makefile.objs            |   1 +
 backends/Makefile.objs   |   2 +-
 backends/trace-events    |  10 ++
 backends/wctablet.c      | 364 +++++++++++++++++++++++++++++++++++++++++++++++
 docs/qdev-device-use.txt |   2 +-
 qapi-schema.json         |   1 +
 qemu-char.c              |   1 +
 7 files changed, 379 insertions(+), 2 deletions(-)
 create mode 100644 backends/trace-events
 create mode 100644 backends/wctablet.c

-- 
1.8.3.1

[Qemu-devel] [PATCH 01/17] Add wctablet device

[Gerd Hoffmann]

From: Anatoli Huseu1 <avg.tolik@gmail.com>

[ kraxel: adapt to chardev changes ]

Signed-off-by: Anatoli Huseu1 <avg.tolik@gmail.com>
---
 backends/Makefile.objs   |   2 +-
 backends/wctablet.c      | 347 +++++++++++++++++++++++++++++++++++++++++++++++
 docs/qdev-device-use.txt |   2 +-
 qapi-schema.json         |   1 +
 qemu-char.c              |   1 +
 5 files changed, 351 insertions(+), 2 deletions(-)
 create mode 100644 backends/wctablet.c

diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 1846998..0e0f156 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -1,7 +1,7 @@
 common-obj-y += rng.o rng-egd.o
 common-obj-$(CONFIG_POSIX) += rng-random.o
 
-common-obj-y += msmouse.o testdev.o
+common-obj-y += msmouse.o wctablet.o testdev.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 baum.o-cflags := $(SDL_CFLAGS)
 
diff --git a/backends/wctablet.c b/backends/wctablet.c
new file mode 100644
index 0000000..0aee6fe
--- /dev/null
+++ b/backends/wctablet.c
@@ -0,0 +1,347 @@
+/*
+ * QEMU Wacome serial tablet emulation
+ *
+ * Copyright (c) 2008 Lubomir Rintel
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include <stdlib.h>
+#include <string.h>
+#include <sys/time.h>
+#include <time.h>
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "sysemu/char.h"
+#include "ui/console.h"
+#include "ui/input.h"
+
+
+#define DEBUG_WCTABLET_MOUSE
+
+#ifdef DEBUG_WCTABLET_MOUSE
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do {} while (0)
+#endif
+
+#define WC_COMMANDS_COUNT 30
+
+#define WC_BUSY_STATE 1
+#define WC_BUSY_WITH_CODES 3
+#define WC_WAITING_STATE 2
+#define WC_OUTPUT_BUF_MAX_LEN 512
+#define WC_COMMAND_MAX_LEN 60
+
+#define WC_L7(n) ((n) & 127)
+#define WC_M7(n) (((n) >> 7) & 127)
+#define WC_H2(n) ((n) >> 14)
+
+#define WC_L4(n) ((n) & 15)
+#define WC_H4(n) (((n) >> 4) & 15)
+
+// Avaliable commands
+uint8_t wctablet_commands[WC_COMMANDS_COUNT][7] = {
+    {0x0a, 0x53, 0x50, 0x0a, 0},                // \nSP\n
+    {0x7e, 0x23, 0},                            // ~#
+    {0x0a, 0x54, 0x45, 0x0a, 0},                // \nTE\n
+    {0x52, 0x45, 0x0a, 0},                      // RE\n
+    {0x41, 0x53, 0x31, 0x0a, 0},                // AS1\n
+    {0x49, 0x43, 0x31, 0x0a, 0},                // IC1\n
+    {0x4f, 0x43, 0x31, 0x0a, 0},                // OC1\n
+    {0x49, 0x54, 0x88, 0x88, 0},                // IT3\r
+    {0x53, 0x55, 0x88, 0x88, 0},                // SU3\n
+    {0x50, 0x48, 0x31, 0x0a, 0},                // PH1\n
+    {0x0d, 0x53, 0x54, 0x0d, 0},                // \rST\n
+    {0x0d, 0x53, 0x50, 0x0d, 0},                // \rSP\r
+    {0x54, 0x45, 0x0d, 0},                      // TE\r
+    {0x53, 0x50, 0x88, 0},                      // SP\n
+    {0x23, 0x41, 0x4c, 0x31, 0x0d, 0},          // #AL1\r
+    {0x53, 0x54, 0x88, 0},                      // ST\n
+    {0x0d, 0x54, 0x53, 0x88, 0xd, 0},           // \rTS&\r
+    {0x0d, 0x0a, 0x53, 0x50, 0x0d, 0x0a, 0},    // \r\nSP\r\n
+    {0x7e, 0x23, 0x0d, 0}                       // ~#\r
+};
+
+// Char strings with avaliable commands
+char wctablet_commands_names[WC_COMMANDS_COUNT][12] = {
+    "\\nSP\\n",
+    "~#",
+    "\\nTE\\n",
+    "RE\\n",
+    "AS1\\n",
+    "IC1\\n",
+    "OC1\\n",
+    "IT3\\r",
+    "SU3\\n",
+    "PH1\\n",
+    "\\rST\\n",
+    "\\rSP\\r",
+    "TE\\r",
+    "SP\\n",
+    "#AL1\\r",
+    "ST\\n",
+    "\\rTS&\\r",
+    "\\r\\nSP\\r\\n",
+    "~#\\r"
+};
+
+// Model string and config string
+uint8_t *WC_MODEL_STRING = (uint8_t *) "~#CT-0045R,V1.3-5,";
+size_t WC_MODEL_STRING_LENGTH = 18;
+uint8_t *WC_CONFIG_STRING = (uint8_t *) "96,N,8,0";
+size_t WC_CONFIG_STRING_LENGTH = 8;
+uint8_t WC_FULL_CONFIG_STRING[61] = {
+    0x5c, 0x39, 0x36, 0x2c, 0x4e, 0x2c, 0x38, 0x2c,
+    0x31, 0x28, 0x01, 0x24, 0x57, 0x41, 0x43, 0x30,
+    0x30, 0x34, 0x35, 0x5c, 0x5c, 0x50, 0x45, 0x4e, 0x5c,
+    0x57, 0x41, 0x43, 0x30, 0x30, 0x30, 0x30, 0x5c,
+    0x54, 0x61, 0x62, 0x6c, 0x65, 0x74, 0x0d, 0x0a,
+    0x43, 0x54, 0x2d, 0x30, 0x30, 0x34, 0x35, 0x52,
+    0x2c, 0x56, 0x31, 0x2e, 0x33, 0x2d, 0x35, 0x0d,
+    0x0a, 0x45, 0x37, 0x29
+};
+size_t WC_FULL_CONFIG_STRING_LENGTH = 61;
+int count = 0;
+int COMMON_SPEAD = 900 * 1000;
+
+
+// This structure is used to save private info for Wacom Tablet.
+typedef struct {
+    struct QEMUTimer *transmit_timer;
+    /* QEMU timer */
+    uint64_t transmit_time;
+    /* time to transmit a char in ticks */
+    uint8_t query[100];
+    int query_index;
+    /* Query string from serial */
+    uint8_t outbuf[WC_OUTPUT_BUF_MAX_LEN];
+    int outlen;
+    /* Command to be sent to serial port */
+} TabletState;
+
+static int wctablet_memcmp(uint8_t *a1, uint8_t *a2, int count)
+{
+    int i;
+
+    for (i = 0; i < count; i++) {
+        if (a1[i] != a2[i] && a2[i] != 0x88) {
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static int wctablet_check_command(uint8_t *arr, int count)
+{
+    int i;
+
+    for (i = 0; i < WC_COMMANDS_COUNT; i++) {
+        if (wctablet_memcmp(arr, wctablet_commands[i], count) == 0 &&
+            wctablet_commands[i][count] == 0) {
+            return i;
+        }
+    }
+
+    return -1;
+}
+
+static void wctablet_event(void *opaque, int x,
+                           int y, int dz, int buttons_state)
+{
+    CharDriverState *chr = (CharDriverState *) opaque;
+    TabletState *tablet = (TabletState *) chr->opaque;
+    uint8_t codes[8] = { 0xe0, 0, 0, 0, 0, 0, 0 };
+    // uint8_t codes[8] = { 0xa0, 0x0e, 0x06, 0x00, 0x13, 0x3b, 0x00 };
+    // uint8_t codes[8] = { 0xe0, 0x05, 0x6a, 0x00, 0x06, 0x64, 0x40 };
+    // uint8_t codes[8] = { 0xa0, 0x1c, 0x29, 0x00, 0x19, 0x1c, 0x00 };
+
+    DPRINTF("x= %d; y= %d; buttons=%x\n", x, y, buttons_state);
+    int newX = x * 0.1537;
+    int nexY = y * 0.1152;
+
+    codes[0] = codes[0] | WC_H2(newX);
+    codes[1] = codes[1] | WC_M7(newX);
+    codes[2] = codes[2] | WC_L7(newX);
+
+    codes[3] = codes[3] | WC_H2(nexY);
+    codes[4] = codes[4] | WC_M7(nexY);
+    codes[5] = codes[5] | WC_L7(nexY);
+
+    if (buttons_state == 0x01) {
+        codes[0] = 0xa0;
+    }
+
+    if (tablet->outlen + 7 < WC_OUTPUT_BUF_MAX_LEN) {
+        memcpy(tablet->outbuf + tablet->outlen, codes, 7);
+        tablet->outlen += 7;
+    }
+}
+
+static void wctablet_handler(void *opaque)
+{
+    CharDriverState *chr = (CharDriverState *) opaque;
+    TabletState *tablet = (TabletState *) chr->opaque;
+    int len, canWrite; // , i;
+
+    canWrite = qemu_chr_be_can_write(chr);
+    len = canWrite;
+    if (len > tablet->outlen) {
+        len = tablet->outlen;
+    }
+
+    if (len) {
+        // DPRINTF("-------- Write %2d: ", canWrite);
+        // for (i = 0; i < len; i++) {
+        //     DPRINTF(" %02x", tablet->outbuf[i]);
+        // }
+        // DPRINTF("\n");
+
+        qemu_chr_be_write(chr, tablet->outbuf, len);
+        tablet->outlen -= len;
+        if (tablet->outlen) {
+            memmove(tablet->outbuf, tablet->outbuf + len, tablet->outlen);
+        }
+    }
+
+    timer_mod(tablet->transmit_timer,
+              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + tablet->transmit_time);
+}
+
+static int wctablet_chr_write (struct CharDriverState *s,
+                               const uint8_t *buf, int len)
+{
+    TabletState *tablet = (TabletState *) s->opaque;
+    uint8_t c = buf[0];
+    uint8_t input;
+
+    if (c == 0x40) {
+        return len;
+    }
+
+    tablet->query[tablet->query_index++] = c;
+
+    // DPRINTF("Receive: %.2x\n", c);
+
+    int comm = wctablet_check_command(tablet->query, tablet->query_index);
+
+    if (comm == 1) {
+        count++;
+    }
+
+    if (comm != -1) {
+        if (comm == 1 && count == 2) {
+            memcpy(tablet->outbuf + tablet->outlen, WC_MODEL_STRING, WC_MODEL_STRING_LENGTH);
+            tablet->outlen += WC_MODEL_STRING_LENGTH;
+        }
+
+        if (comm == 3) {
+            memcpy(tablet->outbuf + tablet->outlen, WC_CONFIG_STRING, WC_CONFIG_STRING_LENGTH);
+            tablet->outlen += WC_CONFIG_STRING_LENGTH;
+        }
+
+        if (comm == 18) {
+            memcpy(tablet->outbuf + tablet->outlen, WC_FULL_CONFIG_STRING, WC_FULL_CONFIG_STRING_LENGTH);
+            tablet->outlen += WC_FULL_CONFIG_STRING_LENGTH;
+        }
+
+        if (comm == 16) {
+            input = tablet->query[3];
+            uint8_t codes[7] = {
+                0xa3,
+                0x88,
+                0x88,
+                0x03,
+                0x7f,
+                0x7f,
+                0x00
+            };
+            codes[1] = ((input & 0x80) == 0) ? 0x7e : 0x7f;
+            codes[2] = ( ( ( WC_H4(input) & 0x7 ) ^ 0x5) << 4 ) | (WC_L4(input) ^ 0x7);
+
+            memcpy(tablet->outbuf + tablet->outlen, codes, 7);
+            tablet->outlen += 7;
+        }
+
+        // DPRINTF("-------- Command: %s\n", wctablet_commands_names[comm]);
+
+        tablet->query_index = 0;
+    }
+
+    return len;
+}
+
+static void wctablet_chr_free(struct CharDriverState *chr)
+{
+    g_free (chr->opaque);
+    g_free (chr);
+}
+
+static CharDriverState *qemu_chr_open_wctablet(const char *id,
+                                              ChardevBackend *backend,
+                                              ChardevReturn *ret,
+                                              bool *be_opened,
+                                              Error **errp)
+{
+    ChardevCommon *common = backend->u.wctablet.data;
+    CharDriverState *chr;
+    TabletState *tablet;
+
+    chr = qemu_chr_alloc(common, errp);
+    tablet = g_malloc0(sizeof(TabletState));
+    if (!chr) {
+        return NULL;
+    }
+    chr->chr_write = wctablet_chr_write;
+    chr->chr_free = wctablet_chr_free;
+    *be_opened = true;
+
+    /* create a new QEMU's timer with wctablet_handler() as timeout handler. */
+    tablet->transmit_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+                                       (QEMUTimerCB *) wctablet_handler, chr);
+
+    tablet->transmit_time = COMMON_SPEAD;
+
+    timer_mod(tablet->transmit_timer,
+              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + tablet->transmit_time);
+
+
+    /* init state machine */
+    memcpy(tablet->outbuf, WC_FULL_CONFIG_STRING, WC_FULL_CONFIG_STRING_LENGTH);
+    tablet->outlen = WC_FULL_CONFIG_STRING_LENGTH;
+    tablet->query_index = 0;
+
+    chr->opaque = tablet;
+
+    qemu_add_mouse_event_handler(wctablet_event, chr, 1,
+                                 "QEMU Wacome Pen Tablet");
+
+    return chr;
+}
+
+static void register_types(void)
+{
+    register_char_driver("wctablet", CHARDEV_BACKEND_KIND_WCTABLET, NULL,
+                         qemu_chr_open_wctablet);
+}
+
+type_init(register_types);
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index 136d271..b059405 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -200,7 +200,7 @@ LEGACY-CHARDEV translates to -chardev HOST-OPTS... as follows:
 
 * null becomes -chardev null
 
-* pty, msmouse, braille, stdio likewise
+* pty, msmouse, wctablet, braille, stdio likewise
 
 * vc:WIDTHxHEIGHT becomes -chardev vc,width=WIDTH,height=HEIGHT
 
diff --git a/qapi-schema.json b/qapi-schema.json
index a0d3b5d..40e76ad 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3879,6 +3879,7 @@
                                        'null'   : 'ChardevCommon',
                                        'mux'    : 'ChardevMux',
                                        'msmouse': 'ChardevCommon',
+                                       'wctablet' : 'ChardevCommon',
                                        'braille': 'ChardevCommon',
                                        'testdev': 'ChardevCommon',
                                        'stdio'  : 'ChardevStdio',
diff --git a/qemu-char.c b/qemu-char.c
index 2c9940c..162df2f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3734,6 +3734,7 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
     if (strcmp(filename, "null")    == 0 ||
         strcmp(filename, "pty")     == 0 ||
         strcmp(filename, "msmouse") == 0 ||
+        strcmp(filename, "wctablet") == 0 ||
         strcmp(filename, "braille") == 0 ||
         strcmp(filename, "testdev") == 0 ||
         strcmp(filename, "stdio")   == 0) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/17] wctablet: add wctablet_queue_output helper

[Gerd Hoffmann]

Add helper function to place data in the output queue, so we don't
duplicate the code and also check the available space every time.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 0aee6fe..758c9e8 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -165,6 +165,16 @@ static int wctablet_check_command(uint8_t *arr, int count)
     return -1;
 }
 
+static void wctablet_queue_output(TabletState *tablet, uint8_t *buf, int count)
+{
+    if (tablet->outlen + count > sizeof(tablet->outbuf)) {
+        return;
+    }
+
+    memcpy(tablet->outbuf + tablet->outlen, buf, count);
+    tablet->outlen += count;
+}
+
 static void wctablet_event(void *opaque, int x,
                            int y, int dz, int buttons_state)
 {
@@ -191,10 +201,7 @@ static void wctablet_event(void *opaque, int x,
         codes[0] = 0xa0;
     }
 
-    if (tablet->outlen + 7 < WC_OUTPUT_BUF_MAX_LEN) {
-        memcpy(tablet->outbuf + tablet->outlen, codes, 7);
-        tablet->outlen += 7;
-    }
+    wctablet_queue_output(tablet, codes, 7);
 }
 
 static void wctablet_handler(void *opaque)
@@ -250,18 +257,18 @@ static int wctablet_chr_write (struct CharDriverState *s,
 
     if (comm != -1) {
         if (comm == 1 && count == 2) {
-            memcpy(tablet->outbuf + tablet->outlen, WC_MODEL_STRING, WC_MODEL_STRING_LENGTH);
-            tablet->outlen += WC_MODEL_STRING_LENGTH;
+            wctablet_queue_output(tablet, WC_MODEL_STRING,
+                                  WC_MODEL_STRING_LENGTH);
         }
 
         if (comm == 3) {
-            memcpy(tablet->outbuf + tablet->outlen, WC_CONFIG_STRING, WC_CONFIG_STRING_LENGTH);
-            tablet->outlen += WC_CONFIG_STRING_LENGTH;
+            wctablet_queue_output(tablet, WC_CONFIG_STRING,
+                                  WC_CONFIG_STRING_LENGTH);
         }
 
         if (comm == 18) {
-            memcpy(tablet->outbuf + tablet->outlen, WC_FULL_CONFIG_STRING, WC_FULL_CONFIG_STRING_LENGTH);
-            tablet->outlen += WC_FULL_CONFIG_STRING_LENGTH;
+            wctablet_queue_output(tablet, WC_FULL_CONFIG_STRING,
+                                  WC_FULL_CONFIG_STRING_LENGTH);
         }
 
         if (comm == 16) {
@@ -278,8 +285,7 @@ static int wctablet_chr_write (struct CharDriverState *s,
             codes[1] = ((input & 0x80) == 0) ? 0x7e : 0x7f;
             codes[2] = ( ( ( WC_H4(input) & 0x7 ) ^ 0x5) << 4 ) | (WC_L4(input) ^ 0x7);
 
-            memcpy(tablet->outbuf + tablet->outlen, codes, 7);
-            tablet->outlen += 7;
+            wctablet_queue_output(tablet, codes, 7);
         }
 
         // DPRINTF("-------- Command: %s\n", wctablet_commands_names[comm]);
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/17] wctablet: save all chars in the query buffer

[Gerd Hoffmann]

... instead of storing the first character only, to avoid things break
in case multiple chars are passed in a single wctablet_chr_write call.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 758c9e8..787da49 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -234,21 +234,25 @@ static void wctablet_handler(void *opaque)
               qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + tablet->transmit_time);
 }
 
-static int wctablet_chr_write (struct CharDriverState *s,
-                               const uint8_t *buf, int len)
+static int wctablet_chr_write(struct CharDriverState *s,
+                              const uint8_t *buf, int len)
 {
     TabletState *tablet = (TabletState *) s->opaque;
-    uint8_t c = buf[0];
-    uint8_t input;
+    uint8_t i, input;
 
-    if (c == 0x40) {
+    for (i = 0; i < len && tablet->query_index < sizeof(tablet->query) - 1; i++) {
+        tablet->query[tablet->query_index++] = buf[i];
+    }
+    tablet->query[tablet->query_index] = 0;
+
+    while (tablet->query_index > 0 && tablet->query[0] == '@') {
+        memmove(tablet->query, tablet->query + 1, tablet->query_index);
+        tablet->query_index--;
+    }
+    if (!tablet->query_index) {
         return len;
     }
 
-    tablet->query[tablet->query_index++] = c;
-
-    // DPRINTF("Receive: %.2x\n", c);
-
     int comm = wctablet_check_command(tablet->query, tablet->query_index);
 
     if (comm == 1) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 04/17] wctablet: drop wctablet_commands_names

[Gerd Hoffmann]

It is unused, except in a commented debug printf which is dropped too.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 787da49..00498e1 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -81,29 +81,6 @@ uint8_t wctablet_commands[WC_COMMANDS_COUNT][7] = {
     {0x7e, 0x23, 0x0d, 0}                       // ~#\r
 };
 
-// Char strings with avaliable commands
-char wctablet_commands_names[WC_COMMANDS_COUNT][12] = {
-    "\\nSP\\n",
-    "~#",
-    "\\nTE\\n",
-    "RE\\n",
-    "AS1\\n",
-    "IC1\\n",
-    "OC1\\n",
-    "IT3\\r",
-    "SU3\\n",
-    "PH1\\n",
-    "\\rST\\n",
-    "\\rSP\\r",
-    "TE\\r",
-    "SP\\n",
-    "#AL1\\r",
-    "ST\\n",
-    "\\rTS&\\r",
-    "\\r\\nSP\\r\\n",
-    "~#\\r"
-};
-
 // Model string and config string
 uint8_t *WC_MODEL_STRING = (uint8_t *) "~#CT-0045R,V1.3-5,";
 size_t WC_MODEL_STRING_LENGTH = 18;
@@ -292,8 +269,6 @@ static int wctablet_chr_write(struct CharDriverState *s,
             wctablet_queue_output(tablet, codes, 7);
         }
 
-        // DPRINTF("-------- Command: %s\n", wctablet_commands_names[comm]);
-
         tablet->query_index = 0;
     }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/17] wctablet: strip leading \r + \n from buffer

[Gerd Hoffmann]

Should make command detection more robust.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 00498e1..767887e 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -60,9 +60,9 @@ do {} while (0)
 
 // Avaliable commands
 uint8_t wctablet_commands[WC_COMMANDS_COUNT][7] = {
-    {0x0a, 0x53, 0x50, 0x0a, 0},                // \nSP\n
+    {0x53, 0x50, 0x0a, 0},                      // SP\n
     {0x7e, 0x23, 0},                            // ~#
-    {0x0a, 0x54, 0x45, 0x0a, 0},                // \nTE\n
+    {0x54, 0x45, 0x0a, 0},                      // TE\n
     {0x52, 0x45, 0x0a, 0},                      // RE\n
     {0x41, 0x53, 0x31, 0x0a, 0},                // AS1\n
     {0x49, 0x43, 0x31, 0x0a, 0},                // IC1\n
@@ -70,14 +70,14 @@ uint8_t wctablet_commands[WC_COMMANDS_COUNT][7] = {
     {0x49, 0x54, 0x88, 0x88, 0},                // IT3\r
     {0x53, 0x55, 0x88, 0x88, 0},                // SU3\n
     {0x50, 0x48, 0x31, 0x0a, 0},                // PH1\n
-    {0x0d, 0x53, 0x54, 0x0d, 0},                // \rST\n
-    {0x0d, 0x53, 0x50, 0x0d, 0},                // \rSP\r
+    {0x53, 0x54, 0x0d, 0},                      // ST\n
+    {0x53, 0x50, 0x0d, 0},                      // SP\r
     {0x54, 0x45, 0x0d, 0},                      // TE\r
     {0x53, 0x50, 0x88, 0},                      // SP\n
     {0x23, 0x41, 0x4c, 0x31, 0x0d, 0},          // #AL1\r
     {0x53, 0x54, 0x88, 0},                      // ST\n
-    {0x0d, 0x54, 0x53, 0x88, 0xd, 0},           // \rTS&\r
-    {0x0d, 0x0a, 0x53, 0x50, 0x0d, 0x0a, 0},    // \r\nSP\r\n
+    {0x54, 0x53, 0x88, 0xd, 0},                 // TS&\r
+    {0x53, 0x50, 0x0d, 0x0a, 0},                // SP\r\n
     {0x7e, 0x23, 0x0d, 0}                       // ~#\r
 };
 
@@ -222,7 +222,9 @@ static int wctablet_chr_write(struct CharDriverState *s,
     }
     tablet->query[tablet->query_index] = 0;
 
-    while (tablet->query_index > 0 && tablet->query[0] == '@') {
+    while (tablet->query_index > 0 && (tablet->query[0] == '@'  ||
+                                       tablet->query[0] == '\r' ||
+                                       tablet->query[0] == '\n')) {
         memmove(tablet->query, tablet->query + 1, tablet->query_index);
         tablet->query_index--;
     }
@@ -253,7 +255,7 @@ static int wctablet_chr_write(struct CharDriverState *s,
         }
 
         if (comm == 16) {
-            input = tablet->query[3];
+            input = tablet->query[2];
             uint8_t codes[7] = {
                 0xa3,
                 0x88,
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/17] wctablet: track line speed, reset on speed changes

[Gerd Hoffmann]

Hook up ioctl callback to get notified when the guest reconfigures the
serial port.  Keep track of the line speed, also reset the device (just
flush buffers) on line speed changes.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 767887e..4d14e96 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -113,6 +113,7 @@ typedef struct {
     uint8_t outbuf[WC_OUTPUT_BUF_MAX_LEN];
     int outlen;
     /* Command to be sent to serial port */
+    int line_speed;
 } TabletState;
 
 static int wctablet_memcmp(uint8_t *a1, uint8_t *a2, int count)
@@ -152,6 +153,13 @@ static void wctablet_queue_output(TabletState *tablet, uint8_t *buf, int count)
     tablet->outlen += count;
 }
 
+static void wctablet_reset(TabletState *tablet)
+{
+    /* clear buffers */
+    tablet->query_index = 0;
+    tablet->outlen = 0;
+}
+
 static void wctablet_event(void *opaque, int x,
                            int y, int dz, int buttons_state)
 {
@@ -277,6 +285,25 @@ static int wctablet_chr_write(struct CharDriverState *s,
     return len;
 }
 
+static int wctablet_chr_ioctl(CharDriverState *s, int cmd, void *arg)
+{
+    TabletState *tablet = (TabletState *) s->opaque;
+    QEMUSerialSetParams *ssp;
+
+    switch (cmd) {
+    case CHR_IOCTL_SERIAL_SET_PARAMS:
+        ssp = arg;
+        if (tablet->line_speed != ssp->speed) {
+            wctablet_reset(tablet);
+            tablet->line_speed = ssp->speed;
+        }
+        break;
+    default:
+        return -ENOTSUP;
+    }
+    return 0;
+}
+
 static void wctablet_chr_free(struct CharDriverState *chr)
 {
     g_free (chr->opaque);
@@ -299,6 +326,7 @@ static CharDriverState *qemu_chr_open_wctablet(const char *id,
         return NULL;
     }
     chr->chr_write = wctablet_chr_write;
+    chr->chr_ioctl = wctablet_chr_ioctl;
     chr->chr_free = wctablet_chr_free;
     *be_opened = true;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 07/17] wctablet: operate on line speed 9600

[Gerd Hoffmann]

Ignore all input unless line speed is 9600.  This makes line speed
detection work correctly and alot more reliable.

Also drop the reset counter, this served as workaround for the lack of
proper line speed handling and is not needed any more.

The guest sends the reset sequence multiple times with different speeds
to figure what the line speed is.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 4d14e96..84fb485 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -97,10 +97,8 @@ uint8_t WC_FULL_CONFIG_STRING[61] = {
     0x0a, 0x45, 0x37, 0x29
 };
 size_t WC_FULL_CONFIG_STRING_LENGTH = 61;
-int count = 0;
 int COMMON_SPEAD = 900 * 1000;
 
-
 // This structure is used to save private info for Wacom Tablet.
 typedef struct {
     struct QEMUTimer *transmit_timer;
@@ -170,6 +168,10 @@ static void wctablet_event(void *opaque, int x,
     // uint8_t codes[8] = { 0xe0, 0x05, 0x6a, 0x00, 0x06, 0x64, 0x40 };
     // uint8_t codes[8] = { 0xa0, 0x1c, 0x29, 0x00, 0x19, 0x1c, 0x00 };
 
+    if (tablet->line_speed != 9600) {
+        return;
+    }
+
     DPRINTF("x= %d; y= %d; buttons=%x\n", x, y, buttons_state);
     int newX = x * 0.1537;
     int nexY = y * 0.1152;
@@ -225,6 +227,9 @@ static int wctablet_chr_write(struct CharDriverState *s,
     TabletState *tablet = (TabletState *) s->opaque;
     uint8_t i, input;
 
+    if (tablet->line_speed != 9600) {
+        return len;
+    }
     for (i = 0; i < len && tablet->query_index < sizeof(tablet->query) - 1; i++) {
         tablet->query[tablet->query_index++] = buf[i];
     }
@@ -242,12 +247,8 @@ static int wctablet_chr_write(struct CharDriverState *s,
 
     int comm = wctablet_check_command(tablet->query, tablet->query_index);
 
-    if (comm == 1) {
-        count++;
-    }
-
     if (comm != -1) {
-        if (comm == 1 && count == 2) {
+        if (comm == 1) {
             wctablet_queue_output(tablet, WC_MODEL_STRING,
                                   WC_MODEL_STRING_LENGTH);
         }
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/17] wctablet: drop debug code from wctablet_handler

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 84fb485..d0ee0e4 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -204,12 +204,6 @@ static void wctablet_handler(void *opaque)
     }
 
     if (len) {
-        // DPRINTF("-------- Write %2d: ", canWrite);
-        // for (i = 0; i < len; i++) {
-        //     DPRINTF(" %02x", tablet->outbuf[i]);
-        // }
-        // DPRINTF("\n");
-
         qemu_chr_be_write(chr, tablet->outbuf, len);
         tablet->outlen -= len;
         if (tablet->outlen) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/17] wctablet: add wctablet_shift_input

[Gerd Hoffmann]

Helper function to drop leading chars from the input buffer.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index d0ee0e4..2b14241 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -141,6 +141,13 @@ static int wctablet_check_command(uint8_t *arr, int count)
     return -1;
 }
 
+static void wctablet_shift_input(TabletState *tablet, int count)
+{
+    tablet->query_index -= count;
+    memmove(tablet->query, tablet->query + count, tablet->query_index);
+    tablet->query[tablet->query_index] = 0;
+}
+
 static void wctablet_queue_output(TabletState *tablet, uint8_t *buf, int count)
 {
     if (tablet->outlen + count > sizeof(tablet->outbuf)) {
@@ -232,8 +239,7 @@ static int wctablet_chr_write(struct CharDriverState *s,
     while (tablet->query_index > 0 && (tablet->query[0] == '@'  ||
                                        tablet->query[0] == '\r' ||
                                        tablet->query[0] == '\n')) {
-        memmove(tablet->query, tablet->query + 1, tablet->query_index);
-        tablet->query_index--;
+        wctablet_shift_input(tablet, 1);
     }
     if (!tablet->query_index) {
         return len;
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/17] wctablet: move init/detect sequence

[Gerd Hoffmann]

Move init/detect detection, handle it as special case.
It is the only sequence which doesn't end with a newline.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 2b14241..5c41bc4 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -245,14 +245,17 @@ static int wctablet_chr_write(struct CharDriverState *s,
         return len;
     }
 
+    if (strncmp((char *)tablet->query, "~#", 2) == 0) {
+        /* init / detect sequence */
+        wctablet_shift_input(tablet, 2);
+        wctablet_queue_output(tablet, WC_MODEL_STRING,
+                              WC_MODEL_STRING_LENGTH);
+        return len;
+    }
+
     int comm = wctablet_check_command(tablet->query, tablet->query_index);
 
     if (comm != -1) {
-        if (comm == 1) {
-            wctablet_queue_output(tablet, WC_MODEL_STRING,
-                                  WC_MODEL_STRING_LENGTH);
-        }
-
         if (comm == 3) {
             wctablet_queue_output(tablet, WC_CONFIG_STRING,
                                   WC_CONFIG_STRING_LENGTH);
-- 
1.8.3.1

[Qemu-devel] [PATCH 11/17] wctablet: revamp command parser.

[Gerd Hoffmann]

Split incoming data into lines.  Then use simple string compare
to match commands.  Drop command table and special compare functions.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 113 +++++++++++++++-------------------------------------
 1 file changed, 33 insertions(+), 80 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 5c41bc4..f21b1f4 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -43,8 +43,6 @@ do { fprintf(stderr, fmt , ## __VA_ARGS__); } while (0)
 do {} while (0)
 #endif
 
-#define WC_COMMANDS_COUNT 30
-
 #define WC_BUSY_STATE 1
 #define WC_BUSY_WITH_CODES 3
 #define WC_WAITING_STATE 2
@@ -58,29 +56,6 @@ do {} while (0)
 #define WC_L4(n) ((n) & 15)
 #define WC_H4(n) (((n) >> 4) & 15)
 
-// Avaliable commands
-uint8_t wctablet_commands[WC_COMMANDS_COUNT][7] = {
-    {0x53, 0x50, 0x0a, 0},                      // SP\n
-    {0x7e, 0x23, 0},                            // ~#
-    {0x54, 0x45, 0x0a, 0},                      // TE\n
-    {0x52, 0x45, 0x0a, 0},                      // RE\n
-    {0x41, 0x53, 0x31, 0x0a, 0},                // AS1\n
-    {0x49, 0x43, 0x31, 0x0a, 0},                // IC1\n
-    {0x4f, 0x43, 0x31, 0x0a, 0},                // OC1\n
-    {0x49, 0x54, 0x88, 0x88, 0},                // IT3\r
-    {0x53, 0x55, 0x88, 0x88, 0},                // SU3\n
-    {0x50, 0x48, 0x31, 0x0a, 0},                // PH1\n
-    {0x53, 0x54, 0x0d, 0},                      // ST\n
-    {0x53, 0x50, 0x0d, 0},                      // SP\r
-    {0x54, 0x45, 0x0d, 0},                      // TE\r
-    {0x53, 0x50, 0x88, 0},                      // SP\n
-    {0x23, 0x41, 0x4c, 0x31, 0x0d, 0},          // #AL1\r
-    {0x53, 0x54, 0x88, 0},                      // ST\n
-    {0x54, 0x53, 0x88, 0xd, 0},                 // TS&\r
-    {0x53, 0x50, 0x0d, 0x0a, 0},                // SP\r\n
-    {0x7e, 0x23, 0x0d, 0}                       // ~#\r
-};
-
 // Model string and config string
 uint8_t *WC_MODEL_STRING = (uint8_t *) "~#CT-0045R,V1.3-5,";
 size_t WC_MODEL_STRING_LENGTH = 18;
@@ -114,33 +89,6 @@ typedef struct {
     int line_speed;
 } TabletState;
 
-static int wctablet_memcmp(uint8_t *a1, uint8_t *a2, int count)
-{
-    int i;
-
-    for (i = 0; i < count; i++) {
-        if (a1[i] != a2[i] && a2[i] != 0x88) {
-            return -1;
-        }
-    }
-
-    return 0;
-}
-
-static int wctablet_check_command(uint8_t *arr, int count)
-{
-    int i;
-
-    for (i = 0; i < WC_COMMANDS_COUNT; i++) {
-        if (wctablet_memcmp(arr, wctablet_commands[i], count) == 0 &&
-            wctablet_commands[i][count] == 0) {
-            return i;
-        }
-    }
-
-    return -1;
-}
-
 static void wctablet_shift_input(TabletState *tablet, int count)
 {
     tablet->query_index -= count;
@@ -226,7 +174,8 @@ static int wctablet_chr_write(struct CharDriverState *s,
                               const uint8_t *buf, int len)
 {
     TabletState *tablet = (TabletState *) s->opaque;
-    uint8_t i, input;
+    unsigned int i, clen;
+    char *pos;
 
     if (tablet->line_speed != 9600) {
         return len;
@@ -253,37 +202,41 @@ static int wctablet_chr_write(struct CharDriverState *s,
         return len;
     }
 
-    int comm = wctablet_check_command(tablet->query, tablet->query_index);
+    /* detect line */
+    pos = strchr((char *)tablet->query, '\r');
+    if (!pos) {
+        pos = strchr((char *)tablet->query, '\n');
+    }
+    if (!pos) {
+        return len;
+    }
+    clen = pos - (char *)tablet->query;
 
-    if (comm != -1) {
-        if (comm == 3) {
-            wctablet_queue_output(tablet, WC_CONFIG_STRING,
-                                  WC_CONFIG_STRING_LENGTH);
-        }
+    /* process commands */
+    if (strncmp((char *)tablet->query, "RE", 2) == 0 &&
+        clen == 2) {
+        wctablet_shift_input(tablet, 3);
+        wctablet_queue_output(tablet, WC_CONFIG_STRING,
+                              WC_CONFIG_STRING_LENGTH);
 
-        if (comm == 18) {
-            wctablet_queue_output(tablet, WC_FULL_CONFIG_STRING,
-                                  WC_FULL_CONFIG_STRING_LENGTH);
-        }
+    } else if (strncmp((char *)tablet->query, "TS", 2) == 0 &&
+               clen == 3) {
+        unsigned int input = tablet->query[2];
+        uint8_t codes[7] = {
+            0xa3,
+            ((input & 0x80) == 0) ? 0x7e : 0x7f,
+            (((WC_H4(input) & 0x7) ^ 0x5) << 4) | (WC_L4(input) ^ 0x7),
+            0x03,
+            0x7f,
+            0x7f,
+            0x00,
+        };
+        wctablet_shift_input(tablet, 4);
+        wctablet_queue_output(tablet, codes, 7);
 
-        if (comm == 16) {
-            input = tablet->query[2];
-            uint8_t codes[7] = {
-                0xa3,
-                0x88,
-                0x88,
-                0x03,
-                0x7f,
-                0x7f,
-                0x00
-            };
-            codes[1] = ((input & 0x80) == 0) ? 0x7e : 0x7f;
-            codes[2] = ( ( ( WC_H4(input) & 0x7 ) ^ 0x5) << 4 ) | (WC_L4(input) ^ 0x7);
+    } else {
+        wctablet_shift_input(tablet, clen + 1);
 
-            wctablet_queue_output(tablet, codes, 7);
-        }
-
-        tablet->query_index = 0;
     }
 
     return len;
-- 
1.8.3.1

[Qemu-devel] [PATCH 12/17] wctablet: drop timer, hook into chr->accept_input instead

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index f21b1f4..28daf26 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -76,10 +76,7 @@ int COMMON_SPEAD = 900 * 1000;
 
 // This structure is used to save private info for Wacom Tablet.
 typedef struct {
-    struct QEMUTimer *transmit_timer;
-    /* QEMU timer */
-    uint64_t transmit_time;
-    /* time to transmit a char in ticks */
+    CharDriverState *chr;
     uint8_t query[100];
     int query_index;
     /* Query string from serial */
@@ -89,6 +86,8 @@ typedef struct {
     int line_speed;
 } TabletState;
 
+static void wctablet_chr_accept_input(CharDriverState *chr);
+
 static void wctablet_shift_input(TabletState *tablet, int count)
 {
     tablet->query_index -= count;
@@ -104,6 +103,7 @@ static void wctablet_queue_output(TabletState *tablet, uint8_t *buf, int count)
 
     memcpy(tablet->outbuf + tablet->outlen, buf, count);
     tablet->outlen += count;
+    wctablet_chr_accept_input(tablet->chr);
 }
 
 static void wctablet_reset(TabletState *tablet)
@@ -146,9 +146,8 @@ static void wctablet_event(void *opaque, int x,
     wctablet_queue_output(tablet, codes, 7);
 }
 
-static void wctablet_handler(void *opaque)
+static void wctablet_chr_accept_input(CharDriverState *chr)
 {
-    CharDriverState *chr = (CharDriverState *) opaque;
     TabletState *tablet = (TabletState *) chr->opaque;
     int len, canWrite; // , i;
 
@@ -165,9 +164,6 @@ static void wctablet_handler(void *opaque)
             memmove(tablet->outbuf, tablet->outbuf + len, tablet->outlen);
         }
     }
-
-    timer_mod(tablet->transmit_timer,
-              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + tablet->transmit_time);
 }
 
 static int wctablet_chr_write(struct CharDriverState *s,
@@ -285,24 +281,16 @@ static CharDriverState *qemu_chr_open_wctablet(const char *id,
     chr->chr_write = wctablet_chr_write;
     chr->chr_ioctl = wctablet_chr_ioctl;
     chr->chr_free = wctablet_chr_free;
+    chr->chr_accept_input = wctablet_chr_accept_input;
     *be_opened = true;
 
-    /* create a new QEMU's timer with wctablet_handler() as timeout handler. */
-    tablet->transmit_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-                                       (QEMUTimerCB *) wctablet_handler, chr);
-
-    tablet->transmit_time = COMMON_SPEAD;
-
-    timer_mod(tablet->transmit_timer,
-              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + tablet->transmit_time);
-
-
     /* init state machine */
     memcpy(tablet->outbuf, WC_FULL_CONFIG_STRING, WC_FULL_CONFIG_STRING_LENGTH);
     tablet->outlen = WC_FULL_CONFIG_STRING_LENGTH;
     tablet->query_index = 0;
 
     chr->opaque = tablet;
+    tablet->chr = chr;
 
     qemu_add_mouse_event_handler(wctablet_event, chr, 1,
                                  "QEMU Wacome Pen Tablet");
-- 
1.8.3.1

[Qemu-devel] [PATCH 13/17] wctablet: drop DPRINTF, add trace events instead

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.objs         |  1 +
 backends/trace-events |  8 ++++++++
 backends/wctablet.c   | 18 +++++++-----------
 3 files changed, 16 insertions(+), 11 deletions(-)
 create mode 100644 backends/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index 51c36a4..df7d0cb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -124,6 +124,7 @@ trace-events-y += crypto/trace-events
 trace-events-y += io/trace-events
 trace-events-y += migration/trace-events
 trace-events-y += block/trace-events
+trace-events-y += backends/trace-events
 trace-events-y += hw/block/trace-events
 trace-events-y += hw/char/trace-events
 trace-events-y += hw/intc/trace-events
diff --git a/backends/trace-events b/backends/trace-events
new file mode 100644
index 0000000..f8a2e2b
--- /dev/null
+++ b/backends/trace-events
@@ -0,0 +1,8 @@
+# See docs/tracing.txt for syntax documentation.
+
+# backends/wctablet.c
+wct_init(void) ""
+wct_cmd_re(void) ""
+wct_cmd_ts(int input) "0x%02x"
+wct_cmd_other(const char *cmd) "%s"
+wct_speed(int speed) "%d"
diff --git a/backends/wctablet.c b/backends/wctablet.c
index 28daf26..07d2a7e 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -31,18 +31,9 @@
 #include "sysemu/char.h"
 #include "ui/console.h"
 #include "ui/input.h"
+#include "trace.h"
 
 
-#define DEBUG_WCTABLET_MOUSE
-
-#ifdef DEBUG_WCTABLET_MOUSE
-#define DPRINTF(fmt, ...) \
-do { fprintf(stderr, fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-do {} while (0)
-#endif
-
 #define WC_BUSY_STATE 1
 #define WC_BUSY_WITH_CODES 3
 #define WC_WAITING_STATE 2
@@ -127,7 +118,6 @@ static void wctablet_event(void *opaque, int x,
         return;
     }
 
-    DPRINTF("x= %d; y= %d; buttons=%x\n", x, y, buttons_state);
     int newX = x * 0.1537;
     int nexY = y * 0.1152;
 
@@ -192,6 +182,7 @@ static int wctablet_chr_write(struct CharDriverState *s,
 
     if (strncmp((char *)tablet->query, "~#", 2) == 0) {
         /* init / detect sequence */
+        trace_wct_init();
         wctablet_shift_input(tablet, 2);
         wctablet_queue_output(tablet, WC_MODEL_STRING,
                               WC_MODEL_STRING_LENGTH);
@@ -211,6 +202,7 @@ static int wctablet_chr_write(struct CharDriverState *s,
     /* process commands */
     if (strncmp((char *)tablet->query, "RE", 2) == 0 &&
         clen == 2) {
+        trace_wct_cmd_re();
         wctablet_shift_input(tablet, 3);
         wctablet_queue_output(tablet, WC_CONFIG_STRING,
                               WC_CONFIG_STRING_LENGTH);
@@ -227,10 +219,13 @@ static int wctablet_chr_write(struct CharDriverState *s,
             0x7f,
             0x00,
         };
+        trace_wct_cmd_ts(input);
         wctablet_shift_input(tablet, 4);
         wctablet_queue_output(tablet, codes, 7);
 
     } else {
+        tablet->query[clen] = 0; /* terminate line for printing */
+        trace_wct_cmd_other((char *)tablet->query);
         wctablet_shift_input(tablet, clen + 1);
 
     }
@@ -247,6 +242,7 @@ static int wctablet_chr_ioctl(CharDriverState *s, int cmd, void *arg)
     case CHR_IOCTL_SERIAL_SET_PARAMS:
         ssp = arg;
         if (tablet->line_speed != ssp->speed) {
+            trace_wct_speed(ssp->speed);
             wctablet_reset(tablet);
             tablet->line_speed = ssp->speed;
         }
-- 
1.8.3.1

[Qemu-devel] [PATCH 14/17] wctablet: misc cleanups

[Gerd Hoffmann]

Some codestyle fixes, place comments first,
turn lengts into #defines, delete unused stuff.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 07d2a7e..29b75b4 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -34,9 +34,6 @@
 #include "trace.h"
 
 
-#define WC_BUSY_STATE 1
-#define WC_BUSY_WITH_CODES 3
-#define WC_WAITING_STATE 2
 #define WC_OUTPUT_BUF_MAX_LEN 512
 #define WC_COMMAND_MAX_LEN 60
 
@@ -47,12 +44,15 @@
 #define WC_L4(n) ((n) & 15)
 #define WC_H4(n) (((n) >> 4) & 15)
 
-// Model string and config string
-uint8_t *WC_MODEL_STRING = (uint8_t *) "~#CT-0045R,V1.3-5,";
-size_t WC_MODEL_STRING_LENGTH = 18;
-uint8_t *WC_CONFIG_STRING = (uint8_t *) "96,N,8,0";
-size_t WC_CONFIG_STRING_LENGTH = 8;
-uint8_t WC_FULL_CONFIG_STRING[61] = {
+/* Model string and config string */
+#define WC_MODEL_STRING_LENGTH 18
+uint8_t WC_MODEL_STRING[WC_MODEL_STRING_LENGTH + 1] = "~#CT-0045R,V1.3-5,";
+
+#define WC_CONFIG_STRING_LENGTH 8
+uint8_t WC_CONFIG_STRING[WC_CONFIG_STRING_LENGTH + 1] = "96,N,8,0";
+
+#define WC_FULL_CONFIG_STRING_LENGTH 61
+uint8_t WC_FULL_CONFIG_STRING[WC_FULL_CONFIG_STRING_LENGTH + 1] = {
     0x5c, 0x39, 0x36, 0x2c, 0x4e, 0x2c, 0x38, 0x2c,
     0x31, 0x28, 0x01, 0x24, 0x57, 0x41, 0x43, 0x30,
     0x30, 0x34, 0x35, 0x5c, 0x5c, 0x50, 0x45, 0x4e, 0x5c,
@@ -62,18 +62,19 @@ uint8_t WC_FULL_CONFIG_STRING[61] = {
     0x2c, 0x56, 0x31, 0x2e, 0x33, 0x2d, 0x35, 0x0d,
     0x0a, 0x45, 0x37, 0x29
 };
-size_t WC_FULL_CONFIG_STRING_LENGTH = 61;
-int COMMON_SPEAD = 900 * 1000;
 
-// This structure is used to save private info for Wacom Tablet.
+/* This structure is used to save private info for Wacom Tablet. */
 typedef struct {
     CharDriverState *chr;
+
+    /* Query string from serial */
     uint8_t query[100];
     int query_index;
-    /* Query string from serial */
+
+    /* Command to be sent to serial port */
     uint8_t outbuf[WC_OUTPUT_BUF_MAX_LEN];
     int outlen;
-    /* Command to be sent to serial port */
+
     int line_speed;
 } TabletState;
 
@@ -110,9 +111,6 @@ static void wctablet_event(void *opaque, int x,
     CharDriverState *chr = (CharDriverState *) opaque;
     TabletState *tablet = (TabletState *) chr->opaque;
     uint8_t codes[8] = { 0xe0, 0, 0, 0, 0, 0, 0 };
-    // uint8_t codes[8] = { 0xa0, 0x0e, 0x06, 0x00, 0x13, 0x3b, 0x00 };
-    // uint8_t codes[8] = { 0xe0, 0x05, 0x6a, 0x00, 0x06, 0x64, 0x40 };
-    // uint8_t codes[8] = { 0xa0, 0x1c, 0x29, 0x00, 0x19, 0x1c, 0x00 };
 
     if (tablet->line_speed != 9600) {
         return;
@@ -139,7 +137,7 @@ static void wctablet_event(void *opaque, int x,
 static void wctablet_chr_accept_input(CharDriverState *chr)
 {
     TabletState *tablet = (TabletState *) chr->opaque;
-    int len, canWrite; // , i;
+    int len, canWrite;
 
     canWrite = qemu_chr_be_can_write(chr);
     len = canWrite;
-- 
1.8.3.1

[Qemu-devel] [PATCH 15/17] wctablet: switch to new input interface

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 65 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 29b75b4..7263671 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -66,6 +66,7 @@ uint8_t WC_FULL_CONFIG_STRING[WC_FULL_CONFIG_STRING_LENGTH + 1] = {
 /* This structure is used to save private info for Wacom Tablet. */
 typedef struct {
     CharDriverState *chr;
+    QemuInputHandlerState *hs;
 
     /* Query string from serial */
     uint8_t query[100];
@@ -76,6 +77,9 @@ typedef struct {
     int outlen;
 
     int line_speed;
+    int axis[INPUT_AXIS__MAX];
+    bool btns[INPUT_BUTTON__MAX];
+
 } TabletState;
 
 static void wctablet_chr_accept_input(CharDriverState *chr);
@@ -105,19 +109,16 @@ static void wctablet_reset(TabletState *tablet)
     tablet->outlen = 0;
 }
 
-static void wctablet_event(void *opaque, int x,
-                           int y, int dz, int buttons_state)
+static void wctablet_queue_event(TabletState *tablet)
 {
-    CharDriverState *chr = (CharDriverState *) opaque;
-    TabletState *tablet = (TabletState *) chr->opaque;
     uint8_t codes[8] = { 0xe0, 0, 0, 0, 0, 0, 0 };
 
     if (tablet->line_speed != 9600) {
         return;
     }
 
-    int newX = x * 0.1537;
-    int nexY = y * 0.1152;
+    int newX = tablet->axis[INPUT_AXIS_X] * 0.1537;
+    int nexY = tablet->axis[INPUT_AXIS_Y] * 0.1152;
 
     codes[0] = codes[0] | WC_H2(newX);
     codes[1] = codes[1] | WC_M7(newX);
@@ -127,13 +128,51 @@ static void wctablet_event(void *opaque, int x,
     codes[4] = codes[4] | WC_M7(nexY);
     codes[5] = codes[5] | WC_L7(nexY);
 
-    if (buttons_state == 0x01) {
+    if (tablet->btns[INPUT_BUTTON_LEFT]) {
         codes[0] = 0xa0;
     }
 
     wctablet_queue_output(tablet, codes, 7);
 }
 
+static void wctablet_input_event(DeviceState *dev, QemuConsole *src,
+                                InputEvent *evt)
+{
+    TabletState *tablet = (TabletState *)dev;
+    InputMoveEvent *move;
+    InputBtnEvent *btn;
+
+    switch (evt->type) {
+    case INPUT_EVENT_KIND_ABS:
+        move = evt->u.abs.data;
+        tablet->axis[move->axis] = move->value;
+        break;
+
+    case INPUT_EVENT_KIND_BTN:
+        btn = evt->u.btn.data;
+        tablet->btns[btn->button] = btn->down;
+        break;
+
+    default:
+        /* keep gcc happy */
+        break;
+    }
+}
+
+static void wctablet_input_sync(DeviceState *dev)
+{
+    TabletState *tablet = (TabletState *)dev;
+
+    wctablet_queue_event(tablet);
+}
+
+static QemuInputHandler wctablet_handler = {
+    .name  = "QEMU Wacome Pen Tablet",
+    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_ABS,
+    .event = wctablet_input_event,
+    .sync  = wctablet_input_sync,
+};
+
 static void wctablet_chr_accept_input(CharDriverState *chr)
 {
     TabletState *tablet = (TabletState *) chr->opaque;
@@ -253,8 +292,11 @@ static int wctablet_chr_ioctl(CharDriverState *s, int cmd, void *arg)
 
 static void wctablet_chr_free(struct CharDriverState *chr)
 {
-    g_free (chr->opaque);
-    g_free (chr);
+    TabletState *tablet = (TabletState *) chr->opaque;
+
+    qemu_input_handler_unregister(tablet->hs);
+    g_free(chr->opaque);
+    g_free(chr);
 }
 
 static CharDriverState *qemu_chr_open_wctablet(const char *id,
@@ -286,9 +328,8 @@ static CharDriverState *qemu_chr_open_wctablet(const char *id,
     chr->opaque = tablet;
     tablet->chr = chr;
 
-    qemu_add_mouse_event_handler(wctablet_event, chr, 1,
-                                 "QEMU Wacome Pen Tablet");
-
+    tablet->hs = qemu_input_handler_register((DeviceState *)tablet,
+                                             &wctablet_handler);
     return chr;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 16/17] wctablet: update file comment

[Gerd Hoffmann]

Be more specific which tablet we emulate.
Add spec link.  Fix copyright.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/wctablet.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/backends/wctablet.c b/backends/wctablet.c
index 7263671..97f1996 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -1,7 +1,11 @@
 /*
- * QEMU Wacome serial tablet emulation
+ * QEMU Wacom Penpartner serial tablet emulation
  *
- * Copyright (c) 2008 Lubomir Rintel
+ * some protocol details:
+ *   http://linuxwacom.sourceforge.net/wiki/index.php/Serial_Protocol_IV
+ *
+ * Copyright (c) 2016 Anatoli Huseu1
+ * Copyright (c) 2016 Gerd Hoffmann
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to
-- 
1.8.3.1

[Qemu-devel] [PATCH 17/17] wctablet: implement ST and SP commands

[Gerd Hoffmann]

ST == start sending events
Sp == stop sending events

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/trace-events |  2 ++
 backends/wctablet.c   | 20 +++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/backends/trace-events b/backends/trace-events
index f8a2e2b..8c3289a 100644
--- a/backends/trace-events
+++ b/backends/trace-events
@@ -3,6 +3,8 @@
 # backends/wctablet.c
 wct_init(void) ""
 wct_cmd_re(void) ""
+wct_cmd_st(void) ""
+wct_cmd_sp(void) ""
 wct_cmd_ts(int input) "0x%02x"
 wct_cmd_other(const char *cmd) "%s"
 wct_speed(int speed) "%d"
diff --git a/backends/wctablet.c b/backends/wctablet.c
index 97f1996..f9f4c80 100644
--- a/backends/wctablet.c
+++ b/backends/wctablet.c
@@ -81,6 +81,7 @@ typedef struct {
     int outlen;
 
     int line_speed;
+    bool send_events;
     int axis[INPUT_AXIS__MAX];
     bool btns[INPUT_BUTTON__MAX];
 
@@ -111,6 +112,8 @@ static void wctablet_reset(TabletState *tablet)
     /* clear buffers */
     tablet->query_index = 0;
     tablet->outlen = 0;
+    /* reset state */
+    tablet->send_events = false;
 }
 
 static void wctablet_queue_event(TabletState *tablet)
@@ -167,7 +170,9 @@ static void wctablet_input_sync(DeviceState *dev)
 {
     TabletState *tablet = (TabletState *)dev;
 
-    wctablet_queue_event(tablet);
+    if (tablet->send_events) {
+        wctablet_queue_event(tablet);
+    }
 }
 
 static QemuInputHandler wctablet_handler = {
@@ -248,6 +253,19 @@ static int wctablet_chr_write(struct CharDriverState *s,
         wctablet_queue_output(tablet, WC_CONFIG_STRING,
                               WC_CONFIG_STRING_LENGTH);
 
+    } else if (strncmp((char *)tablet->query, "ST", 2) == 0 &&
+               clen == 2) {
+        trace_wct_cmd_st();
+        wctablet_shift_input(tablet, 3);
+        tablet->send_events = true;
+        wctablet_queue_event(tablet);
+
+    } else if (strncmp((char *)tablet->query, "SP", 2) == 0 &&
+               clen == 2) {
+        trace_wct_cmd_sp();
+        wctablet_shift_input(tablet, 3);
+        tablet->send_events = false;
+
     } else if (strncmp((char *)tablet->query, "TS", 2) == 0 &&
                clen == 3) {
         unsigned int input = tablet->query[2];
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 00/17] add serial wacom tablet emulation (gsoc 2016)

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 00/17] add serial wacom tablet emulation (gsoc 2016)
Type: series
Message-id: 1483692945-9866-1-git-send-email-kraxel@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1483692945-9866-1-git-send-email-kraxel@redhat.com -> patchew/1483692945-9866-1-git-send-email-kraxel@redhat.com
Switched to a new branch 'test'
ab71fe2 wctablet: implement ST and SP commands
cfaa382 wctablet: update file comment
0bc544e wctablet: switch to new input interface
ee75c1b wctablet: misc cleanups
378ba82 wctablet: drop DPRINTF, add trace events instead
f316457 wctablet: drop timer, hook into chr->accept_input instead
06940e8 wctablet: revamp command parser.
3754fc0 wctablet: move init/detect sequence
40d2abf wctablet: add wctablet_shift_input
796d649 wctablet: drop debug code from wctablet_handler
ceaea4f wctablet: operate on line speed 9600
055056b wctablet: track line speed, reset on speed changes
56034dc wctablet: strip leading \r + \n from buffer
7b59593 wctablet: drop wctablet_commands_names
ea61171 wctablet: save all chars in the query buffer
8b3113b wctablet: add wctablet_queue_output helper
de92a0f Add wctablet device

=== OUTPUT BEGIN ===
Checking PATCH 1/17: Add wctablet device...
ERROR: do not use C99 // comments
#90: FILE: backends/wctablet.c:61:
+// Avaliable commands

ERROR: do not use C99 // comments
#92: FILE: backends/wctablet.c:63:
+    {0x0a, 0x53, 0x50, 0x0a, 0},                // \nSP\n

ERROR: do not use C99 // comments
#93: FILE: backends/wctablet.c:64:
+    {0x7e, 0x23, 0},                            // ~#

ERROR: do not use C99 // comments
#94: FILE: backends/wctablet.c:65:
+    {0x0a, 0x54, 0x45, 0x0a, 0},                // \nTE\n

ERROR: do not use C99 // comments
#95: FILE: backends/wctablet.c:66:
+    {0x52, 0x45, 0x0a, 0},                      // RE\n

ERROR: do not use C99 // comments
#96: FILE: backends/wctablet.c:67:
+    {0x41, 0x53, 0x31, 0x0a, 0},                // AS1\n

ERROR: do not use C99 // comments
#97: FILE: backends/wctablet.c:68:
+    {0x49, 0x43, 0x31, 0x0a, 0},                // IC1\n

ERROR: do not use C99 // comments
#98: FILE: backends/wctablet.c:69:
+    {0x4f, 0x43, 0x31, 0x0a, 0},                // OC1\n

ERROR: do not use C99 // comments
#99: FILE: backends/wctablet.c:70:
+    {0x49, 0x54, 0x88, 0x88, 0},                // IT3\r

ERROR: do not use C99 // comments
#100: FILE: backends/wctablet.c:71:
+    {0x53, 0x55, 0x88, 0x88, 0},                // SU3\n

ERROR: do not use C99 // comments
#101: FILE: backends/wctablet.c:72:
+    {0x50, 0x48, 0x31, 0x0a, 0},                // PH1\n

ERROR: do not use C99 // comments
#102: FILE: backends/wctablet.c:73:
+    {0x0d, 0x53, 0x54, 0x0d, 0},                // \rST\n

ERROR: do not use C99 // comments
#103: FILE: backends/wctablet.c:74:
+    {0x0d, 0x53, 0x50, 0x0d, 0},                // \rSP\r

ERROR: do not use C99 // comments
#104: FILE: backends/wctablet.c:75:
+    {0x54, 0x45, 0x0d, 0},                      // TE\r

ERROR: do not use C99 // comments
#105: FILE: backends/wctablet.c:76:
+    {0x53, 0x50, 0x88, 0},                      // SP\n

ERROR: do not use C99 // comments
#106: FILE: backends/wctablet.c:77:
+    {0x23, 0x41, 0x4c, 0x31, 0x0d, 0},          // #AL1\r

ERROR: do not use C99 // comments
#107: FILE: backends/wctablet.c:78:
+    {0x53, 0x54, 0x88, 0},                      // ST\n

ERROR: do not use C99 // comments
#108: FILE: backends/wctablet.c:79:
+    {0x0d, 0x54, 0x53, 0x88, 0xd, 0},           // \rTS&\r

ERROR: do not use C99 // comments
#109: FILE: backends/wctablet.c:80:
+    {0x0d, 0x0a, 0x53, 0x50, 0x0d, 0x0a, 0},    // \r\nSP\r\n

ERROR: do not use C99 // comments
#110: FILE: backends/wctablet.c:81:
+    {0x7e, 0x23, 0x0d, 0}                       // ~#\r

ERROR: do not use C99 // comments
#113: FILE: backends/wctablet.c:84:
+// Char strings with avaliable commands

ERROR: do not use C99 // comments
#136: FILE: backends/wctablet.c:107:
+// Model string and config string

ERROR: do not initialise globals to 0 or NULL
#152: FILE: backends/wctablet.c:123:
+int count = 0;

ERROR: do not use C99 // comments
#156: FILE: backends/wctablet.c:127:
+// This structure is used to save private info for Wacom Tablet.

ERROR: do not use C99 // comments
#203: FILE: backends/wctablet.c:174:
+    // uint8_t codes[8] = { 0xa0, 0x0e, 0x06, 0x00, 0x13, 0x3b, 0x00 };

ERROR: do not use C99 // comments
#204: FILE: backends/wctablet.c:175:
+    // uint8_t codes[8] = { 0xe0, 0x05, 0x6a, 0x00, 0x06, 0x64, 0x40 };

ERROR: do not use C99 // comments
#205: FILE: backends/wctablet.c:176:
+    // uint8_t codes[8] = { 0xa0, 0x1c, 0x29, 0x00, 0x19, 0x1c, 0x00 };

ERROR: do not use C99 // comments
#233: FILE: backends/wctablet.c:204:
+    int len, canWrite; // , i;

ERROR: do not use C99 // comments
#242: FILE: backends/wctablet.c:213:
+        // DPRINTF("-------- Write %2d: ", canWrite);

ERROR: do not use C99 // comments
#243: FILE: backends/wctablet.c:214:
+        // for (i = 0; i < len; i++) {

ERROR: do not use C99 // comments
#244: FILE: backends/wctablet.c:215:
+        //     DPRINTF(" %02x", tablet->outbuf[i]);

ERROR: do not use C99 // comments
#245: FILE: backends/wctablet.c:216:
+        // }

ERROR: do not use C99 // comments
#246: FILE: backends/wctablet.c:217:
+        // DPRINTF("\n");

ERROR: space prohibited between function name and open parenthesis '('
#259: FILE: backends/wctablet.c:230:
+static int wctablet_chr_write (struct CharDriverState *s,

ERROR: do not use C99 // comments
#272: FILE: backends/wctablet.c:243:
+    // DPRINTF("Receive: %.2x\n", c);

ERROR: line over 90 characters
#282: FILE: backends/wctablet.c:253:
+            memcpy(tablet->outbuf + tablet->outlen, WC_MODEL_STRING, WC_MODEL_STRING_LENGTH);

ERROR: line over 90 characters
#287: FILE: backends/wctablet.c:258:
+            memcpy(tablet->outbuf + tablet->outlen, WC_CONFIG_STRING, WC_CONFIG_STRING_LENGTH);

ERROR: line over 90 characters
#292: FILE: backends/wctablet.c:263:
+            memcpy(tablet->outbuf + tablet->outlen, WC_FULL_CONFIG_STRING, WC_FULL_CONFIG_STRING_LENGTH);

WARNING: line over 80 characters
#308: FILE: backends/wctablet.c:279:
+            codes[2] = ( ( ( WC_H4(input) & 0x7 ) ^ 0x5) << 4 ) | (WC_L4(input) ^ 0x7);

ERROR: space prohibited after that open parenthesis '('
#308: FILE: backends/wctablet.c:279:
+            codes[2] = ( ( ( WC_H4(input) & 0x7 ) ^ 0x5) << 4 ) | (WC_L4(input) ^ 0x7);

ERROR: space prohibited before that close parenthesis ')'
#308: FILE: backends/wctablet.c:279:
+            codes[2] = ( ( ( WC_H4(input) & 0x7 ) ^ 0x5) << 4 ) | (WC_L4(input) ^ 0x7);

ERROR: do not use C99 // comments
#314: FILE: backends/wctablet.c:285:
+        // DPRINTF("-------- Command: %s\n", wctablet_commands_names[comm]);

ERROR: space prohibited between function name and open parenthesis '('
#324: FILE: backends/wctablet.c:295:
+    g_free (chr->opaque);

ERROR: space prohibited between function name and open parenthesis '('
#325: FILE: backends/wctablet.c:296:
+    g_free (chr);

total: 43 errors, 1 warnings, 377 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/17: wctablet: add wctablet_queue_output helper...
Checking PATCH 3/17: wctablet: save all chars in the query buffer...
WARNING: line over 80 characters
#32: FILE: backends/wctablet.c:243:
+    for (i = 0; i < len && tablet->query_index < sizeof(tablet->query) - 1; i++) {

total: 0 errors, 1 warnings, 33 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/17: wctablet: drop wctablet_commands_names...
Checking PATCH 5/17: wctablet: strip leading \r + \n from buffer...
ERROR: do not use C99 // comments
#20: FILE: backends/wctablet.c:63:
+    {0x53, 0x50, 0x0a, 0},                      // SP\n

ERROR: do not use C99 // comments
#23: FILE: backends/wctablet.c:65:
+    {0x54, 0x45, 0x0a, 0},                      // TE\n

ERROR: do not use C99 // comments
#33: FILE: backends/wctablet.c:73:
+    {0x53, 0x54, 0x0d, 0},                      // ST\n

ERROR: do not use C99 // comments
#34: FILE: backends/wctablet.c:74:
+    {0x53, 0x50, 0x0d, 0},                      // SP\r

ERROR: do not use C99 // comments
#41: FILE: backends/wctablet.c:79:
+    {0x54, 0x53, 0x88, 0xd, 0},                 // TS&\r

ERROR: do not use C99 // comments
#42: FILE: backends/wctablet.c:80:
+    {0x53, 0x50, 0x0d, 0x0a, 0},                // SP\r\n

total: 6 errors, 0 warnings, 47 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 6/17: wctablet: track line speed, reset on speed changes...
Checking PATCH 7/17: wctablet: operate on line speed 9600...
Checking PATCH 8/17: wctablet: drop debug code from wctablet_handler...
Checking PATCH 9/17: wctablet: add wctablet_shift_input...
Checking PATCH 10/17: wctablet: move init/detect sequence...
Checking PATCH 11/17: wctablet: revamp command parser....
Checking PATCH 12/17: wctablet: drop timer, hook into chr->accept_input instead...
Checking PATCH 13/17: wctablet: drop DPRINTF, add trace events instead...
Checking PATCH 14/17: wctablet: misc cleanups...
Checking PATCH 15/17: wctablet: switch to new input interface...
Checking PATCH 16/17: wctablet: update file comment...
Checking PATCH 17/17: wctablet: implement ST and SP commands...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH 01/17] Add wctablet device

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 5855 bytes --]

On 01/06/2017 02:55 AM, Gerd Hoffmann wrote:
> From: Anatoli Huseu1 <avg.tolik@gmail.com>
> 
> [ kraxel: adapt to chardev changes ]

More chardev changes are in the pipeline, thanks to:
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00606.html

Will be some interesting merge conflicts to resolve between these two.

I see that this code does not match our current style very well, and
that later patches in the series aim to clean that up. Please point it
out in the commit message that this was done intentionally, or else
squash the cleanups directly into this patch.

> 
> Signed-off-by: Anatoli Huseu1 <avg.tolik@gmail.com>
> ---
>  backends/Makefile.objs   |   2 +-
>  backends/wctablet.c      | 347 +++++++++++++++++++++++++++++++++++++++++++++++
>  docs/qdev-device-use.txt |   2 +-
>  qapi-schema.json         |   1 +
>  qemu-char.c              |   1 +
>  5 files changed, 351 insertions(+), 2 deletions(-)
>  create mode 100644 backends/wctablet.c
> 

> +++ b/backends/wctablet.c
> @@ -0,0 +1,347 @@
> +/*
> + * QEMU Wacome serial tablet emulation
> + *
> + * Copyright (c) 2008 Lubomir Rintel

Wow, that's a long time for this code to be waiting.


> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/time.h>
> +#include <time.h>

Please don't include system headers until after osdep.h has been
included; and some (all?) of these headers are already covered by osdep.h.

> +
> +#include "qemu/osdep.h"

See HACKING for rationale why this should be first.

> +#include "qemu-common.h"
> +#include "sysemu/char.h"
> +#include "ui/console.h"
> +#include "ui/input.h"
> +
> +
> +#define DEBUG_WCTABLET_MOUSE
> +
> +#ifdef DEBUG_WCTABLET_MOUSE
> +#define DPRINTF(fmt, ...) \
> +do { fprintf(stderr, fmt , ## __VA_ARGS__); } while (0)

No space after fmt

> +#else
> +#define DPRINTF(fmt, ...) \
> +do {} while (0)

Eww. This is prone to bitrot. If we stick with prints instead of trace
points, please rewrite it in a form that lets the compiler always
validate the arguments:

#ifdef DEBUG_WCTABLET_MOUSE
# define DEBUG_PRINT 1
#else
# define DEBUG_PRINT 0
#endif
#define DPRINTF(fmt, ...) \
  do { \
    if (DEBUG_PRINT) { \
      fprintf(stderr, fmt, ## __VA_ARGS__); \
    } \
  while (0)


> +
> +// Avaliable commands

s/Avaliable/Available/
Prefer C89 /* */ comments. Has this passed our scripts/checkpatch.pl?

> +uint8_t wctablet_commands[WC_COMMANDS_COUNT][7] = {

const?

> +    {0x0a, 0x53, 0x50, 0x0a, 0},                // \nSP\n

Again, comment style.

> +    {0x7e, 0x23, 0},                            // ~#
> +    {0x0a, 0x54, 0x45, 0x0a, 0},                // \nTE\n
> +    {0x52, 0x45, 0x0a, 0},                      // RE\n
> +    {0x41, 0x53, 0x31, 0x0a, 0},                // AS1\n
> +    {0x49, 0x43, 0x31, 0x0a, 0},                // IC1\n
> +    {0x4f, 0x43, 0x31, 0x0a, 0},                // OC1\n
> +    {0x49, 0x54, 0x88, 0x88, 0},                // IT3\r
> +    {0x53, 0x55, 0x88, 0x88, 0},                // SU3\n
> +    {0x50, 0x48, 0x31, 0x0a, 0},                // PH1\n
> +    {0x0d, 0x53, 0x54, 0x0d, 0},                // \rST\n
> +    {0x0d, 0x53, 0x50, 0x0d, 0},                // \rSP\r
> +    {0x54, 0x45, 0x0d, 0},                      // TE\r
> +    {0x53, 0x50, 0x88, 0},                      // SP\n
> +    {0x23, 0x41, 0x4c, 0x31, 0x0d, 0},          // #AL1\r
> +    {0x53, 0x54, 0x88, 0},                      // ST\n
> +    {0x0d, 0x54, 0x53, 0x88, 0xd, 0},           // \rTS&\r
> +    {0x0d, 0x0a, 0x53, 0x50, 0x0d, 0x0a, 0},    // \r\nSP\r\n
> +    {0x7e, 0x23, 0x0d, 0}                       // ~#\r
> +};
> +
> +// Char strings with avaliable commands

s/avaliable/available/, and comment style (I'll quit pointing that out)

> +char wctablet_commands_names[WC_COMMANDS_COUNT][12] = {

const?

> +// This structure is used to save private info for Wacom Tablet.
> +typedef struct {
> +    struct QEMUTimer *transmit_timer;
> +    /* QEMU timer */
> +    uint64_t transmit_time;
> +    /* time to transmit a char in ticks */
> +    uint8_t query[100];

Magic number?


> +static void wctablet_event(void *opaque, int x,
> +                           int y, int dz, int buttons_state)
> +{
> +    CharDriverState *chr = (CharDriverState *) opaque;
> +    TabletState *tablet = (TabletState *) chr->opaque;
> +    uint8_t codes[8] = { 0xe0, 0, 0, 0, 0, 0, 0 };
> +    // uint8_t codes[8] = { 0xa0, 0x0e, 0x06, 0x00, 0x13, 0x3b, 0x00 };
> +    // uint8_t codes[8] = { 0xe0, 0x05, 0x6a, 0x00, 0x06, 0x64, 0x40 };
> +    // uint8_t codes[8] = { 0xa0, 0x1c, 0x29, 0x00, 0x19, 0x1c, 0x00 };

Why the dead-code comments?

> +static void wctablet_handler(void *opaque)
> +{
> +    CharDriverState *chr = (CharDriverState *) opaque;
> +    TabletState *tablet = (TabletState *) chr->opaque;
> +    int len, canWrite; // , i;

and again

> +
> +    canWrite = qemu_chr_be_can_write(chr);
> +    len = canWrite;
> +    if (len > tablet->outlen) {
> +        len = tablet->outlen;
> +    }
> +
> +    if (len) {
> +        // DPRINTF("-------- Write %2d: ", canWrite);
> +        // for (i = 0; i < len; i++) {
> +        //     DPRINTF(" %02x", tablet->outbuf[i]);
> +        // }
> +        // DPRINTF("\n");

and again

> +++ b/qapi-schema.json
> @@ -3879,6 +3879,7 @@
>                                         'null'   : 'ChardevCommon',
>                                         'mux'    : 'ChardevMux',
>                                         'msmouse': 'ChardevCommon',
> +                                       'wctablet' : 'ChardevCommon',

Missing documentation with a Since 2.9 tag.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 08/17] wctablet: drop debug code from wctablet_handler

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 920 bytes --]

On 01/06/2017 02:55 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  backends/wctablet.c | 6 ------
>  1 file changed, 6 deletions(-)

Why not just squash this into 1/17?

> 
> diff --git a/backends/wctablet.c b/backends/wctablet.c
> index 84fb485..d0ee0e4 100644
> --- a/backends/wctablet.c
> +++ b/backends/wctablet.c
> @@ -204,12 +204,6 @@ static void wctablet_handler(void *opaque)
>      }
>  
>      if (len) {
> -        // DPRINTF("-------- Write %2d: ", canWrite);
> -        // for (i = 0; i < len; i++) {
> -        //     DPRINTF(" %02x", tablet->outbuf[i]);
> -        // }
> -        // DPRINTF("\n");
> -
>          qemu_chr_be_write(chr, tablet->outbuf, len);
>          tablet->outlen -= len;
>          if (tablet->outlen) {
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 13/17] wctablet: drop DPRINTF, add trace events instead

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

On 01/06/2017 02:55 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  Makefile.objs         |  1 +
>  backends/trace-events |  8 ++++++++
>  backends/wctablet.c   | 18 +++++++-----------
>  3 files changed, 16 insertions(+), 11 deletions(-)
>  create mode 100644 backends/trace-events

Is this worth squashing into 1/17? It would be less churn, but then
diverges more from the original author's patch.


>  
> -#define DEBUG_WCTABLET_MOUSE
> -
> -#ifdef DEBUG_WCTABLET_MOUSE
> -#define DPRINTF(fmt, ...) \
> -do { fprintf(stderr, fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> -do {} while (0)
> -#endif

I ask because I wonder if we should even (if only temporarily) have this
poor style of DPRINTF in the tree, compared to a version not prone to
bitrot (see my comments on patch 1).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 14/17] wctablet: misc cleanups

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

On 01/06/2017 02:55 AM, Gerd Hoffmann wrote:
> Some codestyle fixes, place comments first,
> turn lengts into #defines, delete unused stuff.

s/lengts/lengths/

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  backends/wctablet.c | 34 ++++++++++++++++------------------
>  1 file changed, 16 insertions(+), 18 deletions(-)

Again, should this be squashed into 1/17?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 16/17] wctablet: update file comment

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

On 01/06/2017 02:55 AM, Gerd Hoffmann wrote:
> Be more specific which tablet we emulate.
> Add spec link.  Fix copyright.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  backends/wctablet.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/wctablet.c b/backends/wctablet.c
> index 7263671..97f1996 100644
> --- a/backends/wctablet.c
> +++ b/backends/wctablet.c
> @@ -1,7 +1,11 @@
>  /*
> - * QEMU Wacome serial tablet emulation
> + * QEMU Wacom Penpartner serial tablet emulation
>   *
> - * Copyright (c) 2008 Lubomir Rintel
> + * some protocol details:
> + *   http://linuxwacom.sourceforge.net/wiki/index.php/Serial_Protocol_IV
> + *
> + * Copyright (c) 2016 Anatoli Huseu1
> + * Copyright (c) 2016 Gerd Hoffmann

What about 2017? Should this be squashed into 1/17?

>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a copy
>   * of this software and associated documentation files (the "Software"), to
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 08/17] wctablet: drop debug code from wctablet_handler

[Gerd Hoffmann]

On Fr, 2017-01-06 at 07:17 -0600, Eric Blake wrote:
> On 01/06/2017 02:55 AM, Gerd Hoffmann wrote:
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  backends/wctablet.c | 6 ------
> >  1 file changed, 6 deletions(-)
> 
> Why not just squash this into 1/17?

My intention is to keep the submission by Anatoli separate from the
cleanups and improvements I did.  Also in case I introduced any
regressions it is easier to figure which change broke things this way.

For the actual merge it probably makes sense to just squash the whole
series into a single patch.

cheers,
  Gerd