[PATCH 0/7] XP-PEN Deco Pro S support (for-5.20/uclogic)

José Expósito

unread,

Jul 10, 2022, 1:51:06 PM7/10/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Hello everyone,

This patchset adds support for the XP-PEN Deco Pro S.

Like the Deco L (for-5.20/uclogic) this tablet is a UGEE v2 tablet.

The first patch is not related to this tablet. It fixes some naming
conventions on the KUnit tests.

Patches 2 and 3 just move code to functions and add more unit tests.

Patch 4 parses the tablet string descriptor to get its frame type. The
values discovered so far are: only buttons, buttons + dial or
buttons + dial + mouse.

Patches 5 and 6 add the descriptors for the new frame types.

The last patch uses all of this to add support for the tablet.

I don't have access to the tablet. Credit for testing the patches and
providing the required recordings goes to Jouke Witteveen.
This fixes an issue reported in libinput:
https://gitlab.freedesktop.org/libinput/libinput/-/issues/738

Best wishes,
José Expósito

José Expósito (7):
HID: uclogic: KUnit best practices and naming conventions
HID: uclogic: Refactor UGEE v2 string descriptor parsing
HID: uclogic: Refactor UGEE v2 frame initialization
HID: uclogic: Parse the UGEE v2 frame type
HID: uclogic: Add support for UGEE v2 dial frames
HID: uclogic: Add support for UGEE v2 mouse frames
HID: uclogic: Add support for XP-PEN Deco Pro S

drivers/hid/Kconfig | 2 +-
drivers/hid/Makefile | 4 +-
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-uclogic-core.c | 2 +
drivers/hid/hid-uclogic-params-test.c | 192 ++++++++++++++++++++++
drivers/hid/hid-uclogic-params.c | 225 ++++++++++++++++++++++----
drivers/hid/hid-uclogic-params.h | 10 ++
drivers/hid/hid-uclogic-rdesc-test.c | 22 +--
drivers/hid/hid-uclogic-rdesc.c | 74 +++++++++
drivers/hid/hid-uclogic-rdesc.h | 8 +
10 files changed, 494 insertions(+), 46 deletions(-)
create mode 100644 drivers/hid/hid-uclogic-params-test.c

base-commit: 0cb1fc0988e32bda84c2b7218e0c761af1430baf
--
2.25.1

José Expósito

unread,

Jul 10, 2022, 1:51:09 PM7/10/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The KUnit documentation [1] suggests allowing build tests as a module.

In addition, it is recommended [2] to use snake case names for
kunit_suite and test cases.

Change the Kconfig entry from bool to tristate and stick to the naming
conventions to avoid style issues with future tests.

Link: https://docs.kernel.org/dev-tools/kunit/style.html#test-kconfig-entries [1]
Link: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html [2]
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---
drivers/hid/Kconfig | 2 +-
drivers/hid/Makefile | 3 ++-
drivers/hid/hid-uclogic-rdesc-test.c | 22 +++++++++++-----------
3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 6ce92830b5d1..36a17958493f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1307,7 +1307,7 @@ config HID_MCP2221
will be called hid-mcp2221.ko.

config HID_KUNIT_TEST
- bool "KUnit tests for HID" if !KUNIT_ALL_TESTS
+ tristate "KUnit tests for HID" if !KUNIT_ALL_TESTS
depends on KUNIT=y
depends on HID_UCLOGIC
default KUNIT_ALL_TESTS
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index b0bef8098139..82d8fd97d96c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -144,8 +144,9 @@ obj-$(CONFIG_HID_WIIMOTE) += hid-wiimote.o
obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o

-obj-$(CONFIG_HID_KUNIT_TEST) += hid-uclogic-rdesc.o \
+hid-uclogic-test-objs := hid-uclogic-rdesc.o \
hid-uclogic-rdesc-test.o
+obj-$(CONFIG_HID_KUNIT_TEST) += hid-uclogic-test.o

obj-$(CONFIG_USB_HID) += usbhid/
obj-$(CONFIG_USB_MOUSE) += usbhid/
diff --git a/drivers/hid/hid-uclogic-rdesc-test.c b/drivers/hid/hid-uclogic-rdesc-test.c
index ebebffef5f8a..3971a0854c3e 100644
--- a/drivers/hid/hid-uclogic-rdesc-test.c
+++ b/drivers/hid/hid-uclogic-rdesc-test.c
@@ -97,7 +97,7 @@ static const __u8 template_params_none[] = {

static struct uclogic_template_case uclogic_template_cases[] = {
{
- .name = "Empty template",
+ .name = "empty_template",
.template = template_empty,
.template_size = sizeof(template_empty),
.param_list = params_pen_all,
@@ -105,7 +105,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = template_empty,
},
{
- .name = "Template smaller than the placeholder",
+ .name = "template_smaller_than_the_placeholder",
.template = template_small,
.template_size = sizeof(template_small),
.param_list = params_pen_all,
@@ -113,7 +113,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = template_small,
},
{
- .name = "No placeholder",
+ .name = "no_placeholder",
.template = template_no_ph,
.template_size = sizeof(template_no_ph),
.param_list = params_pen_all,
@@ -121,7 +121,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = template_no_ph,
},
{
- .name = "Pen placeholder at the end, without ID",
+ .name = "pen_placeholder_at_the_end_without_id",
.template = template_pen_ph_end,
.template_size = sizeof(template_pen_ph_end),
.param_list = params_pen_all,
@@ -129,7 +129,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = template_pen_ph_end,
},
{
- .name = "Frame button placeholder at the end, without ID",
+ .name = "frame_button_placeholder_at_the_end_without_id",
.template = template_btn_ph_end,
.template_size = sizeof(template_btn_ph_end),
.param_list = params_frame_all,
@@ -137,7 +137,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = template_btn_ph_end,
},
{
- .name = "All params present in the pen template",
+ .name = "all_params_present_in_the_pen_template",
.template = template_pen_all_params,
.template_size = sizeof(template_pen_all_params),
.param_list = params_pen_all,
@@ -145,7 +145,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = expected_pen_all_params,
},
{
- .name = "All params present in the frame template",
+ .name = "all_params_present_in_the_frame_template",
.template = template_frame_all_params,
.template_size = sizeof(template_frame_all_params),
.param_list = params_frame_all,
@@ -153,7 +153,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = expected_frame_all_params,
},
{
- .name = "Some params present in the pen template (complete param list)",
+ .name = "some_params_present_in_the_pen_template_with_complete_param_list",
.template = template_pen_some_params,
.template_size = sizeof(template_pen_some_params),
.param_list = params_pen_all,
@@ -161,7 +161,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = expected_pen_some_params,
},
{
- .name = "Some params present in the pen template (incomplete param list)",
+ .name = "some_params_present_in_the_pen_template_with_incomplete_param_list",
.template = template_pen_some_params,
.template_size = sizeof(template_pen_some_params),
.param_list = params_pen_some,
@@ -169,7 +169,7 @@ static struct uclogic_template_case uclogic_template_cases[] = {
.expected = expected_pen_some_params,
},
{
- .name = "No params present in the template",
+ .name = "no_params_present_in_the_template",
.template = template_params_none,
.template_size = sizeof(template_params_none),
.param_list = params_pen_some,
@@ -208,7 +208,7 @@ static struct kunit_case hid_uclogic_rdesc_test_cases[] = {
};

static struct kunit_suite hid_uclogic_rdesc_test_suite = {
- .name = "hid-uclogic-rdesc-test",
+ .name = "hid_uclogic_rdesc_test",
.test_cases = hid_uclogic_rdesc_test_cases,
};

--
2.25.1

José Expósito

unread,

Jul 10, 2022, 1:51:11 PM7/10/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

At the moment, the driver only supports UGEE v2 devices that have
buttons in their frames.

In order to support other types of frames in the future, move the code
used to initialize this kind of frames to its own function.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 55 +++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index ab54f0125e05..7b9e1892aa91 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1108,6 +1108,44 @@ static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,
return 0;
}

+/**
+ * uclogic_params_ugee_v2_init_frame_buttons() - initialize a UGEE v2 frame with
+ * buttons.
+ * @p: Parameters to fill in, cannot be NULL.
+ * @desc_params: Device description params list.
+ * @desc_params_size: Size of the description params list.
+ *
+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */
+static int uclogic_params_ugee_v2_init_frame_buttons(struct uclogic_params *p,
+ const s32 *desc_params,
+ size_t desc_params_size)
+{
+ __u8 *rdesc_frame = NULL;
+ int rc = 0;
+
+ if (!p || desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+
+ rdesc_frame = uclogic_rdesc_template_apply(
+ uclogic_rdesc_ugee_v2_frame_btn_template_arr,
+ uclogic_rdesc_ugee_v2_frame_btn_template_size,
+ desc_params, UCLOGIC_RDESC_PH_ID_NUM);
+ if (!rdesc_frame)
+ return -ENOMEM;
+
+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[0],
+ rdesc_frame,
+ uclogic_rdesc_ugee_v2_frame_btn_template_size,
+ UCLOGIC_RDESC_V1_FRAME_ID);
+ kfree(rdesc_frame);
+ if (rc)
+ return rc;
+
+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.
@@ -1136,7 +1174,6 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
const int str_desc_len = 12;
__u8 *str_desc = NULL;
__u8 *rdesc_pen = NULL;
- __u8 *rdesc_frame = NULL;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
@@ -1205,20 +1242,8 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
p.pen.subreport_list[0].id = UCLOGIC_RDESC_V1_FRAME_ID;

/* Initialize the frame interface */
- rdesc_frame = uclogic_rdesc_template_apply(
- uclogic_rdesc_ugee_v2_frame_btn_template_arr,
- uclogic_rdesc_ugee_v2_frame_btn_template_size,
- desc_params, ARRAY_SIZE(desc_params));
- if (!rdesc_frame) {
- rc = -ENOMEM;
- goto cleanup;
- }
-
- rc = uclogic_params_frame_init_with_desc(&p.frame_list[0],
- rdesc_frame,
- uclogic_rdesc_ugee_v2_frame_btn_template_size,
- UCLOGIC_RDESC_V1_FRAME_ID);
- kfree(rdesc_frame);
+ rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
+ ARRAY_SIZE(desc_params));
if (rc) {
uclogic_params_init_invalid(&p);
goto output;
--
2.25.1

José Expósito

unread,

Jul 10, 2022, 1:51:11 PM7/10/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The UGEE v2 tablets expose a string descriptor with their capabilities.

Move the code used to parse the descriptors and generate a parameter
list from it to its own function and add KUnit tests to validate the
parser.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/Makefile | 1 +
drivers/hid/hid-uclogic-params-test.c | 159 ++++++++++++++++++++++++++
drivers/hid/hid-uclogic-params.c | 86 ++++++++++----
3 files changed, 226 insertions(+), 20 deletions(-)
create mode 100644 drivers/hid/hid-uclogic-params-test.c

diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 82d8fd97d96c..fe69dece2a46 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -145,6 +145,7 @@ obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o

hid-uclogic-test-objs := hid-uclogic-rdesc.o \
+ hid-uclogic-params.o \
hid-uclogic-rdesc-test.o
obj-$(CONFIG_HID_KUNIT_TEST) += hid-uclogic-test.o

diff --git a/drivers/hid/hid-uclogic-params-test.c b/drivers/hid/hid-uclogic-params-test.c
new file mode 100644
index 000000000000..9f043f2ab387
--- /dev/null
+++ b/drivers/hid/hid-uclogic-params-test.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * HID driver for UC-Logic devices not fully compliant with HID standard
+ *
+ * Copyright (c) 2022 José Expósito <jose.ex...@gmail.com>
+ */
+
+#include <kunit/test.h>
+#include "./hid-uclogic-rdesc.h"
+
+#define MAX_STR_DESC_SIZE 14
+
+struct uclogic_parse_ugee_v2_desc_case {
+ const char *name;
+ int res;
+ const __u8 str_desc[MAX_STR_DESC_SIZE];
+ size_t str_desc_size;
+ const s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+};
+
+static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[] = {
+ {
+ .name = "invalid_str_desc",
+ .res = -EINVAL,
+ .str_desc = {},
+ .str_desc_size = 0,
+ .desc_params = {},
+ },
+ {
+ .name = "resolution_with_value_0",
+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,
+ 0x70, 0xB2,
+ 0x10, 0x77,
+ 0x08,
+ 0x00,
+ 0xFF, 0x1F,
+ 0x00, 0x00,
+ },
+ .str_desc_size = 12,
+ .desc_params = {
+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB270,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7710,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0,
+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },
+ },
+ /* XP-PEN Deco L str_desc: Frame with 8 buttons */
+ {
+ .name = "frame_type_buttons",
+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,
+ 0x70, 0xB2,
+ 0x10, 0x77,
+ 0x08,
+ 0x00,
+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {
+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB270,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2320,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7710,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x1770,
+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },
+ },
+ /* PARBLO A610 PRO str_desc: Frame with 9 buttons and dial */
+ {
+ .name = "frame_type_dial",
+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,
+ 0x96, 0xC7,
+ 0xF9, 0x7C,
+ 0x09,
+ 0x01,
+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {
+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xC796,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2749,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7CF9,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x1899,
+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x09,
+ },
+ },
+};
+
+static void uclogic_parse_ugee_v2_desc_case_desc(struct uclogic_parse_ugee_v2_desc_case *t,
+ char *desc)
+{
+ strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(uclogic_parse_ugee_v2_desc, uclogic_parse_ugee_v2_desc_cases,
+ uclogic_parse_ugee_v2_desc_case_desc);
+
+static void uclogic_parse_ugee_v2_desc_test(struct kunit *test)
+{
+ int res;
+ s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+ const struct uclogic_parse_ugee_v2_desc_case *params = test->param_value;
+
+ res = uclogic_params_parse_ugee_v2_desc(params->str_desc,
+ params->str_desc_size,
+ desc_params,
+ ARRAY_SIZE(desc_params));
+ KUNIT_ASSERT_EQ(test, res, params->res);
+
+ if (res)
+ return;
+
+ KUNIT_EXPECT_EQ(test,
+ params->desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_LM],
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_LM]);
+ KUNIT_EXPECT_EQ(test,
+ params->desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_PM],
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_PM]);
+ KUNIT_EXPECT_EQ(test,
+ params->desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_LM],
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_LM]);
+ KUNIT_EXPECT_EQ(test,
+ params->desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_PM],
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_PM]);
+ KUNIT_EXPECT_EQ(test,
+ params->desc_params[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM],
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM]);
+ KUNIT_EXPECT_EQ(test,
+ params->desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM],
+ desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM]);
+}
+
+static struct kunit_case hid_uclogic_params_test_cases[] = {
+ KUNIT_CASE_PARAM(uclogic_parse_ugee_v2_desc_test,
+ uclogic_parse_ugee_v2_desc_gen_params),
+ {}
+};
+
+static struct kunit_suite hid_uclogic_params_test_suite = {
+ .name = "hid_uclogic_params_test",
+ .test_cases = hid_uclogic_params_test_cases,
+};
+
+kunit_test_suite(hid_uclogic_params_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for the UC-Logic driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("José Expósito <jose.ex...@gmail.com>");
diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index f24a4aca7920..ab54f0125e05 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1052,6 +1052,62 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,
return rc;
}

+/**
+ * uclogic_params_parse_ugee_v2_desc - parse the string descriptor containing
+ * pen and frame parameters returned by UGEE v2 devices.
+ *
+ * @str_desc: String descriptor, cannot be NULL.
+ * @str_desc_size: Size of the string descriptor.
+ * @desc_params: Output description params list.
+ * @desc_params_size: Size of the output description params list.

+ *
+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,
+ size_t str_desc_size,
+ s32 *desc_params,
+ size_t desc_params_size)
+{
+ s32 pen_x_lm, pen_y_lm;
+ s32 pen_x_pm, pen_y_pm;
+ s32 pen_pressure_lm;
+ s32 frame_num_buttons;
+ s32 resolution;
+
+ /* Minimum descriptor length required, maximum seen so far is 14 */
+ const int min_str_desc_size = 12;
+
+ if (!str_desc || str_desc_size < min_str_desc_size)
+ return -EINVAL;
+
+ if (desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+
+ pen_x_lm = get_unaligned_le16(str_desc + 2);
+ pen_y_lm = get_unaligned_le16(str_desc + 4);
+ frame_num_buttons = str_desc[6];
+ pen_pressure_lm = get_unaligned_le16(str_desc + 8);
+
+ resolution = get_unaligned_le16(str_desc + 10);
+ if (resolution == 0) {
+ pen_x_pm = 0;
+ pen_y_pm = 0;
+ } else {
+ pen_x_pm = pen_x_lm * 1000 / resolution;
+ pen_y_pm = pen_y_lm * 1000 / resolution;
+ }
+
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_LM] = pen_x_lm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_PM] = pen_x_pm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = pen_y_lm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = pen_y_pm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = pen_pressure_lm;
+ desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM] = frame_num_buttons;

+
+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1082,7 +1138,6 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
__u8 *rdesc_pen = NULL;

__u8 *rdesc_frame = NULL;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];

- s32 resolution;

__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

@@ -1124,25 +1179,12 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
goto output;
}

- desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_LM] =
- get_unaligned_le16(str_desc + 2);
- desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_LM] =
- get_unaligned_le16(str_desc + 4);
- desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM] = str_desc[6];
- desc_params[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] =
- get_unaligned_le16(str_desc + 8);
- resolution = get_unaligned_le16(str_desc + 10);
- if (resolution == 0) {
- desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0;
- desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0;
- } else {
- desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_PM] =
- desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_LM] * 1000 /
- resolution;
- desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_PM] =
- desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_LM] * 1000 /
- resolution;
- }
+ rc = uclogic_params_parse_ugee_v2_desc(str_desc, str_desc_len,
+ desc_params,
+ ARRAY_SIZE(desc_params));
+ if (rc)
+ goto cleanup;
+
kfree(str_desc);
str_desc = NULL;

@@ -1513,3 +1555,7 @@ int uclogic_params_init(struct uclogic_params *params,
uclogic_params_cleanup(&p);
return rc;
}
+
+#ifdef CONFIG_HID_KUNIT_TEST
+#include "hid-uclogic-params-test.c"
+#endif
--
2.25.1

José Expósito

unread,

Jul 10, 2022, 1:51:12 PM7/10/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The string descriptor returned by UGEE v2 devices contains a byte
indicating the device frame type.

The values discovered so far are:

- 0: Frame with buttons, present in the XP-PEN Deco L.
- 1: Frame with buttons and dial, present in the PARBLO A610 PRO.
- 2: Frame with buttons and a mouse, shaped as a dial + touchpad.
Present in the XP-PEN Deco Pro S.

Parse the frame type and add KUnit tests.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params-test.c | 35 ++++++++++++++++++++++++++-
drivers/hid/hid-uclogic-params.c | 19 ++++++++++++---
drivers/hid/hid-uclogic-params.h | 10 ++++++++
3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-uclogic-params-test.c b/drivers/hid/hid-uclogic-params-test.c
index 9f043f2ab387..57ef5d3e4b74 100644
--- a/drivers/hid/hid-uclogic-params-test.c
+++ b/drivers/hid/hid-uclogic-params-test.c
@@ -7,6 +7,7 @@
*/

#include <kunit/test.h>
+#include "./hid-uclogic-params.h"
#include "./hid-uclogic-rdesc.h"

#define MAX_STR_DESC_SIZE 14
@@ -17,6 +18,7 @@ struct uclogic_parse_ugee_v2_desc_case {
const __u8 str_desc[MAX_STR_DESC_SIZE];
size_t str_desc_size;
const s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+ enum uclogic_params_frame_type frame_type;
};

static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[] = {
@@ -26,6 +28,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
.str_desc = {},
.str_desc_size = 0,
.desc_params = {},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},
{
.name = "resolution_with_value_0",
@@ -48,6 +51,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},

/* XP-PEN Deco L str_desc: Frame with 8 buttons */

{
@@ -71,6 +75,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},

/* PARBLO A610 PRO str_desc: Frame with 9 buttons and dial */

{
@@ -94,6 +99,31 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x09,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_DIAL,
+ },
+ /* XP-PEN Deco Pro S str_desc: Frame with 8 buttons and mouse */
+ {
+ .name = "frame_type_mouse",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0xC8, 0xB3,
+ 0x34, 0x65,
+ 0x08,
+ 0x02,

+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB3C8,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2363,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x6534,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x13EC,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },

+ .frame_type = UCLOGIC_PARAMS_FRAME_MOUSE,
},
};

@@ -110,12 +140,14 @@ static void uclogic_parse_ugee_v2_desc_test(struct kunit *test)
{
int res;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+ enum uclogic_params_frame_type frame_type;

const struct uclogic_parse_ugee_v2_desc_case *params = test->param_value;

res = uclogic_params_parse_ugee_v2_desc(params->str_desc,
params->str_desc_size,
desc_params,
- ARRAY_SIZE(desc_params));
+ ARRAY_SIZE(desc_params),
+ &frame_type);
KUNIT_ASSERT_EQ(test, res, params->res);

if (res)
@@ -139,6 +171,7 @@ static void uclogic_parse_ugee_v2_desc_test(struct kunit *test)
KUNIT_EXPECT_EQ(test,
params->desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM],
desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM]);
+ KUNIT_EXPECT_EQ(test, params->frame_type, frame_type);
}

static struct kunit_case hid_uclogic_params_test_cases[] = {
diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 7b9e1892aa91..b232125f6972 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1060,6 +1060,7 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,

* @str_desc_size: Size of the string descriptor.

* @desc_params: Output description params list.

* @desc_params_size: Size of the output description params list.

+ * @frame_type: Output frame type.
*
* Returns:

* Zero, if successful. A negative errno code on error.

@@ -1067,7 +1068,8 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,

static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,

size_t str_desc_size,
s32 *desc_params,
- size_t desc_params_size)
+ size_t desc_params_size,
+ enum uclogic_params_frame_type *frame_type)
{
s32 pen_x_lm, pen_y_lm;
s32 pen_x_pm, pen_y_pm;
@@ -1087,6 +1089,7 @@ static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,

pen_x_lm = get_unaligned_le16(str_desc + 2);

pen_y_lm = get_unaligned_le16(str_desc + 4);

frame_num_buttons = str_desc[6];
+ *frame_type = str_desc[7];

pen_pressure_lm = get_unaligned_le16(str_desc + 8);

resolution = get_unaligned_le16(str_desc + 10);

@@ -1175,6 +1178,7 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
__u8 *str_desc = NULL;
__u8 *rdesc_pen = NULL;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+ enum uclogic_params_frame_type frame_type;

__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

@@ -1218,7 +1222,8 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

rc = uclogic_params_parse_ugee_v2_desc(str_desc, str_desc_len,
desc_params,
- ARRAY_SIZE(desc_params));
+ ARRAY_SIZE(desc_params),
+ &frame_type);
if (rc)
goto cleanup;

@@ -1242,8 +1247,14 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

p.pen.subreport_list[0].id = UCLOGIC_RDESC_V1_FRAME_ID;

/* Initialize the frame interface */

- rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
- ARRAY_SIZE(desc_params));
+ switch (frame_type) {
+ case UCLOGIC_PARAMS_FRAME_BUTTONS:
+ default:

+ rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
+ ARRAY_SIZE(desc_params));

+ break;
+ }
+

if (rc) {
uclogic_params_init_invalid(&p);
goto output;

diff --git a/drivers/hid/hid-uclogic-params.h b/drivers/hid/hid-uclogic-params.h
index 5bef8daaa607..a97477c02ff8 100644
--- a/drivers/hid/hid-uclogic-params.h
+++ b/drivers/hid/hid-uclogic-params.h
@@ -29,6 +29,16 @@ enum uclogic_params_pen_inrange {
UCLOGIC_PARAMS_PEN_INRANGE_NONE,
};

+/* Types of frames */
+enum uclogic_params_frame_type {
+ /* Frame with buttons */
+ UCLOGIC_PARAMS_FRAME_BUTTONS = 0,
+ /* Frame with buttons and a dial */
+ UCLOGIC_PARAMS_FRAME_DIAL,
+ /* Frame with buttons and a mouse (shaped as a dial + touchpad) */
+ UCLOGIC_PARAMS_FRAME_MOUSE,
+};
+
/*
* Pen report's subreport data.
*/
--
2.25.1

José Expósito

unread,

Jul 10, 2022, 1:51:13 PM7/10/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Add the required HID descriptors and the initialization function for
UGEE v2 frames with a bitmap dial.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 44 ++++++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.c | 40 +++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.h | 4 +++
3 files changed, 88 insertions(+)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index b232125f6972..8ac7caeda535 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1149,6 +1149,45 @@ static int uclogic_params_ugee_v2_init_frame_buttons(struct uclogic_params *p,
return 0;
}

+/**
+ * uclogic_params_ugee_v2_init_frame_dial() - initialize a UGEE v2 frame with a
+ * bitmap dial.
+ * @p: Parameters to fill in, cannot be NULL.
+ * @desc_params: Device description params list.
+ * @desc_params_size: Size of the description params list.
+ *
+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */
+static int uclogic_params_ugee_v2_init_frame_dial(struct uclogic_params *p,
+ const s32 *desc_params,
+ size_t desc_params_size)
+{

+ __u8 *rdesc_frame = NULL;
+ int rc = 0;
+

+ if (!p || desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+
+ rdesc_frame = uclogic_rdesc_template_apply(
+ uclogic_rdesc_ugee_v2_frame_dial_template_arr,
+ uclogic_rdesc_ugee_v2_frame_dial_template_size,

+ desc_params, UCLOGIC_RDESC_PH_ID_NUM);
+ if (!rdesc_frame)
+ return -ENOMEM;
+
+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[0],
+ rdesc_frame,

+ uclogic_rdesc_ugee_v2_frame_dial_template_size,

+ UCLOGIC_RDESC_V1_FRAME_ID);
+ kfree(rdesc_frame);
+ if (rc)
+ return rc;
+

+ p->frame_list[0].bitmap_dial_byte = 7;

+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1248,6 +1287,11 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

/* Initialize the frame interface */

switch (frame_type) {
+ case UCLOGIC_PARAMS_FRAME_DIAL:
+ case UCLOGIC_PARAMS_FRAME_MOUSE:
+ rc = uclogic_params_ugee_v2_init_frame_dial(&p, desc_params,
+ ARRAY_SIZE(desc_params));
+ break;
case UCLOGIC_PARAMS_FRAME_BUTTONS:
default:
rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index 3d68e8b0784d..22429df693fc 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c
@@ -961,6 +961,46 @@ const __u8 uclogic_rdesc_ugee_v2_frame_btn_template_arr[] = {
const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size =
sizeof(uclogic_rdesc_ugee_v2_frame_btn_template_arr);

+/* Fixed report descriptor template for UGEE v2 frame reports (dial) */
+const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[] = {
+ 0x05, 0x01, /* Usage Page (Desktop), */
+ 0x09, 0x07, /* Usage (Keypad), */
+ 0xA1, 0x01, /* Collection (Application), */
+ 0x85, UCLOGIC_RDESC_V1_FRAME_ID,
+ /* Report ID, */
+ 0x05, 0x0D, /* Usage Page (Digitizer), */
+ 0x09, 0x39, /* Usage (Tablet Function Keys), */
+ 0xA0, /* Collection (Physical), */
+ 0x75, 0x01, /* Report Size (1), */
+ 0x95, 0x08, /* Report Count (8), */
+ 0x81, 0x01, /* Input (Constant), */
+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x19, 0x01, /* Usage Minimum (01h), */
+ UCLOGIC_RDESC_FRAME_PH_BTN,
+ /* Usage Maximum (PLACEHOLDER), */
+ 0x95, 0x0A, /* Report Count (10), */
+ 0x14, /* Logical Minimum (0), */
+ 0x25, 0x01, /* Logical Maximum (1), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x95, 0x06, /* Report Count (6), */
+ 0x81, 0x01, /* Input (Constant), */
+ 0x75, 0x08, /* Report Size (8), */
+ 0x95, 0x03, /* Report Count (3), */
+ 0x81, 0x01, /* Input (Constant), */
+ 0x05, 0x01, /* Usage Page (Desktop), */
+ 0x09, 0x38, /* Usage (Wheel), */
+ 0x95, 0x01, /* Report Count (1), */
+ 0x15, 0xFF, /* Logical Minimum (-1), */
+ 0x25, 0x01, /* Logical Maximum (1), */
+ 0x81, 0x06, /* Input (Variable, Relative), */
+ 0x95, 0x02, /* Report Count (2), */
+ 0x81, 0x01, /* Input (Constant), */
+ 0xC0, /* End Collection, */
+ 0xC0 /* End Collection */
+};
+const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size =
+ sizeof(uclogic_rdesc_ugee_v2_frame_dial_template_arr);
+
/* Fixed report descriptor for Ugee EX07 frame */
const __u8 uclogic_rdesc_ugee_ex07_frame_arr[] = {
0x05, 0x01, /* Usage Page (Desktop), */
diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h
index 86e64a9ee6bd..1a2d658bad3a 100644
--- a/drivers/hid/hid-uclogic-rdesc.h
+++ b/drivers/hid/hid-uclogic-rdesc.h
@@ -169,6 +169,10 @@ extern const size_t uclogic_rdesc_ugee_v2_pen_template_size;
extern const __u8 uclogic_rdesc_ugee_v2_frame_btn_template_arr[];
extern const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size;

+/* Fixed report descriptor template for UGEE v2 frame reports (dial) */
+extern const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[];
+extern const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size;
+
/* Fixed report descriptor for Ugee EX07 frame */
extern const __u8 uclogic_rdesc_ugee_ex07_frame_arr[];
extern const size_t uclogic_rdesc_ugee_ex07_frame_size;
--
2.25.1

José Expósito

unread,

Jul 10, 2022, 1:51:14 PM7/10/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Add the required HID descriptors and the initialization function for

UGEE v2 frames with a mouse in the frame.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 31 +++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.c | 34 ++++++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.h | 4 ++++
3 files changed, 69 insertions(+)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 8ac7caeda535..f25c483b794b 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1188,6 +1188,31 @@ static int uclogic_params_ugee_v2_init_frame_dial(struct uclogic_params *p,
return 0;
}

+/**
+ * uclogic_params_ugee_v2_init_frame_mouse() - initialize a UGEE v2 frame with a
+ * mouse.

+ * @p: Parameters to fill in, cannot be NULL.
+ *

+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_ugee_v2_init_frame_mouse(struct uclogic_params *p)
+{

+ int rc = 0;
+

+ if (!p)
+ return -EINVAL;
+
+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[1],
+ uclogic_rdesc_ugee_v2_frame_mouse_template_arr,
+ uclogic_rdesc_ugee_v2_frame_dial_template_size,
+ UCLOGIC_RDESC_V1_FRAME_ID);

+ if (rc)
+ return rc;
+

+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1231,6 +1256,12 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

iface = to_usb_interface(hdev->dev.parent);
bInterfaceNumber = iface->cur_altsetting->desc.bInterfaceNumber;
+
+ if (bInterfaceNumber == 0) {
+ uclogic_params_ugee_v2_init_frame_mouse(&p);
+ goto output;
+ }
+
if (bInterfaceNumber != 2) {
uclogic_params_init_invalid(&p);
goto output;
diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index 22429df693fc..fb9ca69f72c2 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c
@@ -1001,6 +1001,40 @@ const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[] = {
const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size =
sizeof(uclogic_rdesc_ugee_v2_frame_dial_template_arr);

+/* Fixed report descriptor template for UGEE v2 frame reports (mouse) */
+const __u8 uclogic_rdesc_ugee_v2_frame_mouse_template_arr[] = {

+ 0x05, 0x01, /* Usage Page (Desktop), */

+ 0x09, 0x02, /* Usage (Mouse), */

+ 0xA1, 0x01, /* Collection (Application), */

+ 0x85, 0x01, /* Report ID (1), */
+ 0x05, 0x01, /* Usage Page (Pointer), */

+ 0xA0, /* Collection (Physical), */
+ 0x75, 0x01, /* Report Size (1), */

+ 0x95, 0x02, /* Report Count (2), */

+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x19, 0x01, /* Usage Minimum (01h), */

+ 0x29, 0x02, /* Usage Maximum (02h), */

+ 0x14, /* Logical Minimum (0), */
+ 0x25, 0x01, /* Logical Maximum (1), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x95, 0x06, /* Report Count (6), */
+ 0x81, 0x01, /* Input (Constant), */

+ 0x05, 0x01, /* Usage Page (Generic Desktop), */
+ 0x09, 0x30, /* Usage (X), */
+ 0x09, 0x31, /* Usage (Y), */
+ 0x75, 0x10, /* Report Size (16), */

+ 0x95, 0x02, /* Report Count (2), */

+ 0x16, 0x00, 0x80, /* Logical Minimum (-32768), */
+ 0x26, 0xFF, 0x7F, /* Logical Maximum (32767), */

+ 0x81, 0x06, /* Input (Variable, Relative), */

+ 0x95, 0x01, /* Report Count (1), */

+ 0x81, 0x01, /* Input (Constant), */
+ 0xC0, /* End Collection, */
+ 0xC0 /* End Collection */
+};

+const size_t uclogic_rdesc_ugee_v2_frame_mouse_template_size =
+ sizeof(uclogic_rdesc_ugee_v2_frame_mouse_template_arr);

+
/* Fixed report descriptor for Ugee EX07 frame */
const __u8 uclogic_rdesc_ugee_ex07_frame_arr[] = {
0x05, 0x01, /* Usage Page (Desktop), */
diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h

index 1a2d658bad3a..0502a0656496 100644
--- a/drivers/hid/hid-uclogic-rdesc.h
+++ b/drivers/hid/hid-uclogic-rdesc.h
@@ -173,6 +173,10 @@ extern const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size;
extern const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[];
extern const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size;

+/* Fixed report descriptor template for UGEE v2 frame reports (mouse) */
+extern const __u8 uclogic_rdesc_ugee_v2_frame_mouse_template_arr[];
+extern const size_t uclogic_rdesc_ugee_v2_frame_mouse_template_size;

José Expósito

unread,

Jul 10, 2022, 1:51:14 PM7/10/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The XP-PEN Deco Pro S is a UGEE v2 device with a frame with 8 buttons,
a bitmap dial and a mouse. Its pen has 2 buttons, supports tilt and
pressure.

All the pieces to support it are already in place. Add its ID in order
to support the device.

The required Wireshark traces were captured by Jouke Witteveen.
For more information check [1].

Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/738 [1]

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-uclogic-core.c | 2 ++
drivers/hid/hid-uclogic-params.c | 2 ++
3 files changed, 5 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 139910034c17..ee6d27a5dd1c 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1279,6 +1279,7 @@
#define USB_DEVICE_ID_UGEE_XPPEN_TABLET_G640 0x0094
#define USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO01 0x0042
#define USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_L 0x0935
+#define USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_PRO_S 0x0909
#define USB_DEVICE_ID_UGEE_XPPEN_TABLET_STAR06 0x0078
#define USB_DEVICE_ID_UGEE_TABLET_G5 0x0074
#define USB_DEVICE_ID_UGEE_TABLET_EX07S 0x0071
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index 47a17375c7fc..6fcdb141acec 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -523,6 +523,8 @@ static const struct hid_device_id uclogic_devices[] = {
USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO01) },
{ HID_USB_DEVICE(USB_VENDOR_ID_UGEE,
USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_L) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_UGEE,
+ USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_PRO_S) },
{ HID_USB_DEVICE(USB_VENDOR_ID_UGEE,
USB_DEVICE_ID_UGEE_XPPEN_TABLET_STAR06) },
{ }
diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index f25c483b794b..2407e927d1bf 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1583,6 +1583,8 @@ int uclogic_params_init(struct uclogic_params *params,
break;
case VID_PID(USB_VENDOR_ID_UGEE,
USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_L):
+ case VID_PID(USB_VENDOR_ID_UGEE,
+ USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_PRO_S):
rc = uclogic_params_ugee_v2_init(&p, hdev);
if (rc != 0)
goto cleanup;
--
2.25.1

Daniel Latypov

unread,

Jul 11, 2022, 10:42:05 AM7/11/22

to José Expósito, ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com

On Sun, Jul 10, 2022 at 10:51 AM José Expósito
<jose.ex...@gmail.com> wrote:
>
> The KUnit documentation [1] suggests allowing build tests as a module.
>
> In addition, it is recommended [2] to use snake case names for
> kunit_suite and test cases.

Test parameters don't fall under "test cases", though I see how that
can be construed as such.
I don't think anyone has stated any preference to standardize the naming there.

We currently have parameterized tests using spaces and punctuation, e.g.
ok 7 - binfmt_elf
# Subtest: ext4_inode_test
1..1
# Subtest: inode_test_xtimestamp_decoding
ok 1 - 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits
ok 2 - 1969-12-31 Upper bound of 32bit < 0 timestamp, no extra bits
...
ok 1 - mctp_test_fragment
# Subtest: mctp_test_rx_input
ok 1 - {1,a,8,0}
ok 2 - {1,a,9,0}
ok 3 - {2,a,8,0}

So I think the old names were more conventional.

>
> Change the Kconfig entry from bool to tristate and stick to the naming
> conventions to avoid style issues with future tests.
>
> Link: https://docs.kernel.org/dev-tools/kunit/style.html#test-kconfig-entries [1]
> Link: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html [2]
> Signed-off-by: José Expósito <jose.ex...@gmail.com>

Everything else (renaming the suite and switching to tristate) sounds
good to me though.

Acked-by: Daniel Latypov <dlat...@google.com>

José Expósito

unread,

Jul 11, 2022, 12:16:56 PM7/11/22

to Daniel Latypov, ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com

Hi Daniel,

On Mon, Jul 11, 2022 at 07:41:53AM -0700, Daniel Latypov wrote:
> On Sun, Jul 10, 2022 at 10:51 AM José Expósito
> <jose.ex...@gmail.com> wrote:
> >
> > The KUnit documentation [1] suggests allowing build tests as a module.
> >
> > In addition, it is recommended [2] to use snake case names for
> > kunit_suite and test cases.
>
> Test parameters don't fall under "test cases", though I see how that
> can be construed as such.
> I don't think anyone has stated any preference to standardize the naming there.
>
> We currently have parameterized tests using spaces and punctuation, e.g.
> ok 7 - binfmt_elf
> # Subtest: ext4_inode_test
> 1..1
> # Subtest: inode_test_xtimestamp_decoding
> ok 1 - 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits
> ok 2 - 1969-12-31 Upper bound of 32bit < 0 timestamp, no extra bits
> ...
> ok 1 - mctp_test_fragment
> # Subtest: mctp_test_rx_input
> ok 1 - {1,a,8,0}
> ok 2 - {1,a,9,0}
> ok 3 - {2,a,8,0}
>
> So I think the old names were more conventional.

I changed the names to be consistent with other tests I'm working on
present in "gpu/drm/tests/drm_format_helper_test.c".

My first version there used full sentences for the test cases, but, if
I remember correctly, it was suggested to use snake case.

I don't have a strong preference about using one approach or the other.
If there is not a rule, I'd prefer to be consistent with the work I'm
doing in the DRM subsystem to avoid mixing notation or refactoring
there, but I'm open to change it.

> > Change the Kconfig entry from bool to tristate and stick to the naming
> > conventions to avoid style issues with future tests.
> >
> > Link: https://docs.kernel.org/dev-tools/kunit/style.html#test-kconfig-entries [1]
> > Link: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html [2]
> > Signed-off-by: José Expósito <jose.ex...@gmail.com>
>
> Everything else (renaming the suite and switching to tristate) sounds
> good to me though.
>
> Acked-by: Daniel Latypov <dlat...@google.com>

Thanks a lot for your review! I'll wait a couple of days before
sending v2 with the Acked-by tag just in case you or somebody else
wants to add more comments.

Best wishes,
Jose

José Expósito

unread,

Jul 17, 2022, 10:43:39 AM7/17/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Hello everyone,

This patchset adds support for the XP-PEN Deco Pro S.

Like the Deco L (for-5.20/uclogic) this tablet is a UGEE v2 tablet.

The first patch is not related to this tablet. It fixes some naming
conventions on the KUnit tests.

Patches 2 and 3 just move code to functions and add more unit tests.

Patch 4 parses the tablet string descriptor to get its frame type. The
values discovered so far are: only buttons, buttons + dial or
buttons + dial + mouse.

Patches 5 and 6 add the descriptors for the new frame types.

The last patch uses all of this to add support for the tablet.

I don't have access to the tablet. Credit for testing the patches and
providing the required recordings goes to Jouke Witteveen.
This fixes an issue reported in libinput:
https://gitlab.freedesktop.org/libinput/libinput/-/issues/738

Best wishes,
José Expósito

v1 -> v2:

- First patch acked-by Daniel Latypov

José Expósito (7):
HID: uclogic: KUnit best practices and naming conventions
HID: uclogic: Refactor UGEE v2 string descriptor parsing
HID: uclogic: Refactor UGEE v2 frame initialization
HID: uclogic: Parse the UGEE v2 frame type
HID: uclogic: Add support for UGEE v2 dial frames
HID: uclogic: Add support for UGEE v2 mouse frames
HID: uclogic: Add support for XP-PEN Deco Pro S

drivers/hid/Kconfig | 2 +-
drivers/hid/Makefile | 4 +-

drivers/hid/hid-ids.h | 1 +

10 files changed, 494 insertions(+), 46 deletions(-)
create mode 100644 drivers/hid/hid-uclogic-params-test.c

base-commit: 0cb1fc0988e32bda84c2b7218e0c761af1430baf
--
2.25.1

José Expósito

unread,

Jul 17, 2022, 10:43:40 AM7/17/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito, Daniel Latypov

The KUnit documentation [1] suggests allowing build tests as a module.

In addition, it is recommended [2] to use snake case names for
kunit_suite and test cases.

Change the Kconfig entry from bool to tristate and stick to the naming
conventions to avoid style issues with future tests.

Link: https://docs.kernel.org/dev-tools/kunit/style.html#test-kconfig-entries [1]
Link: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html [2]

Acked-by: Daniel Latypov <dlat...@google.com>

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/Kconfig | 2 +-
drivers/hid/Makefile | 3 ++-
drivers/hid/hid-uclogic-rdesc-test.c | 22 +++++++++++-----------
3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 6ce92830b5d1..36a17958493f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1307,7 +1307,7 @@ config HID_MCP2221
will be called hid-mcp2221.ko.

config HID_KUNIT_TEST
- bool "KUnit tests for HID" if !KUNIT_ALL_TESTS
+ tristate "KUnit tests for HID" if !KUNIT_ALL_TESTS
depends on KUNIT=y
depends on HID_UCLOGIC
default KUNIT_ALL_TESTS

diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index b0bef8098139..82d8fd97d96c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile

@@ -144,8 +144,9 @@ obj-$(CONFIG_HID_WIIMOTE) += hid-wiimote.o

obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o

José Expósito

unread,

Jul 17, 2022, 10:43:40 AM7/17/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The UGEE v2 tablets expose a string descriptor with their capabilities.

Move the code used to parse the descriptors and generate a parameter
list from it to its own function and add KUnit tests to validate the
parser.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/Makefile | 1 +
drivers/hid/hid-uclogic-params-test.c | 159 ++++++++++++++++++++++++++
drivers/hid/hid-uclogic-params.c | 86 ++++++++++----

3 files changed, 226 insertions(+), 20 deletions(-)
create mode 100644 drivers/hid/hid-uclogic-params-test.c

diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 82d8fd97d96c..fe69dece2a46 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -145,6 +145,7 @@ obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o

+ .name = "resolution_with_value_0",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0x70, 0xB2,
+ 0x10, 0x77,
+ 0x08,
+ 0x00,
+ 0xFF, 0x1F,
+ 0x00, 0x00,

+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB270,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7710,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },

+ },

+ /* XP-PEN Deco L str_desc: Frame with 8 buttons */
+ {

+ .name = "frame_type_buttons",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0x70, 0xB2,
+ 0x10, 0x77,
+ 0x08,
+ 0x00,

+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB270,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2320,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7710,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x1770,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },

+ },
+ /* PARBLO A610 PRO str_desc: Frame with 9 buttons and dial */
+ {

+ .name = "frame_type_dial",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0x96, 0xC7,
+ 0xF9, 0x7C,
+ 0x09,
+ 0x01,

+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xC796,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2749,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7CF9,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x1899,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index f24a4aca7920..ab54f0125e05 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c

@@ -1052,6 +1052,62 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,
return rc;
}

+/**
+ * uclogic_params_parse_ugee_v2_desc - parse the string descriptor containing
+ * pen and frame parameters returned by UGEE v2 devices.
+ *

+ * @str_desc: String descriptor, cannot be NULL.
+ * @str_desc_size: Size of the string descriptor.
+ * @desc_params: Output description params list.
+ * @desc_params_size: Size of the output description params list.

+ *
+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,
+ size_t str_desc_size,

+ s32 *desc_params,
+ size_t desc_params_size)
+{

+ s32 pen_x_lm, pen_y_lm;
+ s32 pen_x_pm, pen_y_pm;
+ s32 pen_pressure_lm;
+ s32 frame_num_buttons;
+ s32 resolution;
+
+ /* Minimum descriptor length required, maximum seen so far is 14 */
+ const int min_str_desc_size = 12;
+
+ if (!str_desc || str_desc_size < min_str_desc_size)

+ return -EINVAL;
+
+ if (desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+

+ pen_x_lm = get_unaligned_le16(str_desc + 2);
+ pen_y_lm = get_unaligned_le16(str_desc + 4);
+ frame_num_buttons = str_desc[6];
+ pen_pressure_lm = get_unaligned_le16(str_desc + 8);
+
+ resolution = get_unaligned_le16(str_desc + 10);
+ if (resolution == 0) {
+ pen_x_pm = 0;
+ pen_y_pm = 0;
+ } else {
+ pen_x_pm = pen_x_lm * 1000 / resolution;
+ pen_y_pm = pen_y_lm * 1000 / resolution;
+ }
+
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_LM] = pen_x_lm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_PM] = pen_x_pm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = pen_y_lm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = pen_y_pm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = pen_pressure_lm;
+ desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM] = frame_num_buttons;

+
+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1082,7 +1138,6 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
__u8 *rdesc_pen = NULL;

__u8 *rdesc_frame = NULL;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
- s32 resolution;

__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

José Expósito

unread,

Jul 17, 2022, 10:43:41 AM7/17/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

At the moment, the driver only supports UGEE v2 devices that have
buttons in their frames.

In order to support other types of frames in the future, move the code
used to initialize this kind of frames to its own function.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 55 +++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index ab54f0125e05..7b9e1892aa91 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1108,6 +1108,44 @@ static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,
return 0;
}

+/**
+ * uclogic_params_ugee_v2_init_frame_buttons() - initialize a UGEE v2 frame with
+ * buttons.
+ * @p: Parameters to fill in, cannot be NULL.
+ * @desc_params: Device description params list.
+ * @desc_params_size: Size of the description params list.

+ *
+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_ugee_v2_init_frame_buttons(struct uclogic_params *p,

+ const s32 *desc_params,
+ size_t desc_params_size)
+{
+ __u8 *rdesc_frame = NULL;

+ int rc = 0;
+

+ if (!p || desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+

+ rdesc_frame = uclogic_rdesc_template_apply(
+ uclogic_rdesc_ugee_v2_frame_btn_template_arr,
+ uclogic_rdesc_ugee_v2_frame_btn_template_size,

+ desc_params, UCLOGIC_RDESC_PH_ID_NUM);
+ if (!rdesc_frame)
+ return -ENOMEM;

+

+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[0],
+ rdesc_frame,

+ uclogic_rdesc_ugee_v2_frame_btn_template_size,
+ UCLOGIC_RDESC_V1_FRAME_ID);
+ kfree(rdesc_frame);

+ if (rc)
+ return rc;

+
+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1136,7 +1174,6 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
const int str_desc_len = 12;

__u8 *str_desc = NULL;
__u8 *rdesc_pen = NULL;

- __u8 *rdesc_frame = NULL;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];

__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00

@@ -1205,20 +1242,8 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

p.pen.subreport_list[0].id = UCLOGIC_RDESC_V1_FRAME_ID;

/* Initialize the frame interface */

- rdesc_frame = uclogic_rdesc_template_apply(
- uclogic_rdesc_ugee_v2_frame_btn_template_arr,
- uclogic_rdesc_ugee_v2_frame_btn_template_size,
- desc_params, ARRAY_SIZE(desc_params));
- if (!rdesc_frame) {
- rc = -ENOMEM;
- goto cleanup;
- }
-
- rc = uclogic_params_frame_init_with_desc(&p.frame_list[0],
- rdesc_frame,
- uclogic_rdesc_ugee_v2_frame_btn_template_size,
- UCLOGIC_RDESC_V1_FRAME_ID);
- kfree(rdesc_frame);

+ rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
+ ARRAY_SIZE(desc_params));

if (rc) {
uclogic_params_init_invalid(&p);
goto output;

--
2.25.1

José Expósito

unread,

Jul 17, 2022, 10:43:42 AM7/17/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The string descriptor returned by UGEE v2 devices contains a byte

indicating the device frame type.

The values discovered so far are:

- 0: Frame with buttons, present in the XP-PEN Deco L.
- 1: Frame with buttons and dial, present in the PARBLO A610 PRO.
- 2: Frame with buttons and a mouse, shaped as a dial + touchpad.
Present in the XP-PEN Deco Pro S.

Parse the frame type and add KUnit tests.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params-test.c | 35 ++++++++++++++++++++++++++-
drivers/hid/hid-uclogic-params.c | 19 ++++++++++++---
drivers/hid/hid-uclogic-params.h | 10 ++++++++
3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-uclogic-params-test.c b/drivers/hid/hid-uclogic-params-test.c
index 9f043f2ab387..57ef5d3e4b74 100644
--- a/drivers/hid/hid-uclogic-params-test.c
+++ b/drivers/hid/hid-uclogic-params-test.c
@@ -7,6 +7,7 @@
*/

#include <kunit/test.h>
+#include "./hid-uclogic-params.h"
#include "./hid-uclogic-rdesc.h"

#define MAX_STR_DESC_SIZE 14
@@ -17,6 +18,7 @@ struct uclogic_parse_ugee_v2_desc_case {
const __u8 str_desc[MAX_STR_DESC_SIZE];
size_t str_desc_size;

const s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];

+ enum uclogic_params_frame_type frame_type;
};

static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[] = {
@@ -26,6 +28,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
.str_desc = {},
.str_desc_size = 0,
.desc_params = {},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},
{
.name = "resolution_with_value_0",
@@ -48,6 +51,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},

/* XP-PEN Deco L str_desc: Frame with 8 buttons */

{
@@ -71,6 +75,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},

/* PARBLO A610 PRO str_desc: Frame with 9 buttons and dial */

{
@@ -94,6 +99,31 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x09,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_DIAL,

+ },
+ /* XP-PEN Deco Pro S str_desc: Frame with 8 buttons and mouse */
+ {
+ .name = "frame_type_mouse",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0xC8, 0xB3,
+ 0x34, 0x65,
+ 0x08,
+ 0x02,

+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB3C8,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2363,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x6534,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x13EC,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },

+ .frame_type = UCLOGIC_PARAMS_FRAME_MOUSE,
},
};

@@ -110,12 +140,14 @@ static void uclogic_parse_ugee_v2_desc_test(struct kunit *test)
{
int res;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+ enum uclogic_params_frame_type frame_type;

const struct uclogic_parse_ugee_v2_desc_case *params = test->param_value;

res = uclogic_params_parse_ugee_v2_desc(params->str_desc,
params->str_desc_size,
desc_params,
- ARRAY_SIZE(desc_params));
+ ARRAY_SIZE(desc_params),
+ &frame_type);
KUNIT_ASSERT_EQ(test, res, params->res);

if (res)
@@ -139,6 +171,7 @@ static void uclogic_parse_ugee_v2_desc_test(struct kunit *test)
KUNIT_EXPECT_EQ(test,
params->desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM],
desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM]);
+ KUNIT_EXPECT_EQ(test, params->frame_type, frame_type);
}

static struct kunit_case hid_uclogic_params_test_cases[] = {

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 7b9e1892aa91..b232125f6972 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1060,6 +1060,7 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,

* @str_desc_size: Size of the string descriptor.

* @desc_params: Output description params list.

* @desc_params_size: Size of the output description params list.
+ * @frame_type: Output frame type.
*
* Returns:

* Zero, if successful. A negative errno code on error.

@@ -1067,7 +1068,8 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,

static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,

size_t str_desc_size,
s32 *desc_params,
- size_t desc_params_size)
+ size_t desc_params_size,
+ enum uclogic_params_frame_type *frame_type)
{
s32 pen_x_lm, pen_y_lm;
s32 pen_x_pm, pen_y_pm;

@@ -1087,6 +1089,7 @@ static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,

pen_x_lm = get_unaligned_le16(str_desc + 2);

pen_y_lm = get_unaligned_le16(str_desc + 4);

frame_num_buttons = str_desc[6];
+ *frame_type = str_desc[7];

pen_pressure_lm = get_unaligned_le16(str_desc + 8);

resolution = get_unaligned_le16(str_desc + 10);

@@ -1175,6 +1178,7 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

__u8 *str_desc = NULL;
__u8 *rdesc_pen = NULL;

s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+ enum uclogic_params_frame_type frame_type;

__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

@@ -1218,7 +1222,8 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

rc = uclogic_params_parse_ugee_v2_desc(str_desc, str_desc_len,
desc_params,
- ARRAY_SIZE(desc_params));
+ ARRAY_SIZE(desc_params),
+ &frame_type);
if (rc)
goto cleanup;

@@ -1242,8 +1247,14 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

p.pen.subreport_list[0].id = UCLOGIC_RDESC_V1_FRAME_ID;

/* Initialize the frame interface */

- rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
- ARRAY_SIZE(desc_params));
+ switch (frame_type) {
+ case UCLOGIC_PARAMS_FRAME_BUTTONS:
+ default:

+ rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
+ ARRAY_SIZE(desc_params));

+ break;
+ }
+

if (rc) {
uclogic_params_init_invalid(&p);
goto output;

José Expósito

unread,

Jul 17, 2022, 10:43:42 AM7/17/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Add the required HID descriptors and the initialization function for

UGEE v2 frames with a bitmap dial.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 44 ++++++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.c | 40 +++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.h | 4 +++
3 files changed, 88 insertions(+)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index b232125f6972..8ac7caeda535 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c

@@ -1149,6 +1149,45 @@ static int uclogic_params_ugee_v2_init_frame_buttons(struct uclogic_params *p,

return 0;
}

+/**

+ * uclogic_params_ugee_v2_init_frame_dial() - initialize a UGEE v2 frame with a
+ * bitmap dial.

+ * @p: Parameters to fill in, cannot be NULL.

+ * @desc_params: Device description params list.
+ * @desc_params_size: Size of the description params list.
+ *

+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_ugee_v2_init_frame_dial(struct uclogic_params *p,

+ const s32 *desc_params,
+ size_t desc_params_size)
+{
+ __u8 *rdesc_frame = NULL;
+ int rc = 0;
+
+ if (!p || desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+
+ rdesc_frame = uclogic_rdesc_template_apply(

+ uclogic_rdesc_ugee_v2_frame_dial_template_arr,
+ uclogic_rdesc_ugee_v2_frame_dial_template_size,

+ desc_params, UCLOGIC_RDESC_PH_ID_NUM);
+ if (!rdesc_frame)
+ return -ENOMEM;
+
+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[0],
+ rdesc_frame,

+ uclogic_rdesc_ugee_v2_frame_dial_template_size,

+ UCLOGIC_RDESC_V1_FRAME_ID);
+ kfree(rdesc_frame);
+ if (rc)
+ return rc;
+

+ p->frame_list[0].bitmap_dial_byte = 7;

+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1248,6 +1287,11 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

/* Initialize the frame interface */

switch (frame_type) {
+ case UCLOGIC_PARAMS_FRAME_DIAL:
+ case UCLOGIC_PARAMS_FRAME_MOUSE:

+ rc = uclogic_params_ugee_v2_init_frame_dial(&p, desc_params,
+ ARRAY_SIZE(desc_params));
+ break;

case UCLOGIC_PARAMS_FRAME_BUTTONS:
default:
rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,

diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index 3d68e8b0784d..22429df693fc 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c

@@ -961,6 +961,46 @@ const __u8 uclogic_rdesc_ugee_v2_frame_btn_template_arr[] = {
const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size =
sizeof(uclogic_rdesc_ugee_v2_frame_btn_template_arr);

+/* Fixed report descriptor template for UGEE v2 frame reports (dial) */

+const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[] = {

+ 0x05, 0x01, /* Usage Page (Desktop), */

+ 0x09, 0x07, /* Usage (Keypad), */

+ 0xA1, 0x01, /* Collection (Application), */

+ 0x85, UCLOGIC_RDESC_V1_FRAME_ID,
+ /* Report ID, */
+ 0x05, 0x0D, /* Usage Page (Digitizer), */

+ 0x09, 0x39, /* Usage (Tablet Function Keys), */

+ 0xA0, /* Collection (Physical), */
+ 0x75, 0x01, /* Report Size (1), */

+ 0x95, 0x08, /* Report Count (8), */

+ 0x81, 0x01, /* Input (Constant), */

+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x19, 0x01, /* Usage Minimum (01h), */

+ UCLOGIC_RDESC_FRAME_PH_BTN,
+ /* Usage Maximum (PLACEHOLDER), */

+ 0x95, 0x0A, /* Report Count (10), */

+ 0x14, /* Logical Minimum (0), */
+ 0x25, 0x01, /* Logical Maximum (1), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x95, 0x06, /* Report Count (6), */
+ 0x81, 0x01, /* Input (Constant), */

+ 0x75, 0x08, /* Report Size (8), */

+ 0x95, 0x03, /* Report Count (3), */

+ 0x81, 0x01, /* Input (Constant), */

+ 0x05, 0x01, /* Usage Page (Desktop), */

+ 0x09, 0x38, /* Usage (Wheel), */

+ 0x95, 0x01, /* Report Count (1), */

+ 0x15, 0xFF, /* Logical Minimum (-1), */

+ 0x25, 0x01, /* Logical Maximum (1), */

+ 0x81, 0x06, /* Input (Variable, Relative), */

+ 0x95, 0x02, /* Report Count (2), */

+ 0x81, 0x01, /* Input (Constant), */
+ 0xC0, /* End Collection, */
+ 0xC0 /* End Collection */
+};

+const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size =
+ sizeof(uclogic_rdesc_ugee_v2_frame_dial_template_arr);

+
/* Fixed report descriptor for Ugee EX07 frame */
const __u8 uclogic_rdesc_ugee_ex07_frame_arr[] = {
0x05, 0x01, /* Usage Page (Desktop), */
diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h

index 86e64a9ee6bd..1a2d658bad3a 100644
--- a/drivers/hid/hid-uclogic-rdesc.h
+++ b/drivers/hid/hid-uclogic-rdesc.h

@@ -169,6 +169,10 @@ extern const size_t uclogic_rdesc_ugee_v2_pen_template_size;
extern const __u8 uclogic_rdesc_ugee_v2_frame_btn_template_arr[];
extern const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size;

+/* Fixed report descriptor template for UGEE v2 frame reports (dial) */
+extern const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[];
+extern const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size;

José Expósito

unread,

Jul 17, 2022, 10:43:43 AM7/17/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Add the required HID descriptors and the initialization function for

UGEE v2 frames with a mouse in the frame.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 31 +++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.c | 34 ++++++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.h | 4 ++++

3 files changed, 69 insertions(+)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 8ac7caeda535..f25c483b794b 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1188,6 +1188,31 @@ static int uclogic_params_ugee_v2_init_frame_dial(struct uclogic_params *p,
return 0;
}

+/**
+ * uclogic_params_ugee_v2_init_frame_mouse() - initialize a UGEE v2 frame with a
+ * mouse.

+ * @p: Parameters to fill in, cannot be NULL.
+ *

+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_ugee_v2_init_frame_mouse(struct uclogic_params *p)
+{

+ int rc = 0;
+

+ if (!p)
+ return -EINVAL;
+

+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[1],
+ uclogic_rdesc_ugee_v2_frame_mouse_template_arr,

+ uclogic_rdesc_ugee_v2_frame_dial_template_size,
+ UCLOGIC_RDESC_V1_FRAME_ID);

+ if (rc)
+ return rc;
+

+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1231,6 +1256,12 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

iface = to_usb_interface(hdev->dev.parent);
bInterfaceNumber = iface->cur_altsetting->desc.bInterfaceNumber;
+
+ if (bInterfaceNumber == 0) {
+ uclogic_params_ugee_v2_init_frame_mouse(&p);
+ goto output;
+ }
+
if (bInterfaceNumber != 2) {
uclogic_params_init_invalid(&p);
goto output;

diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index 22429df693fc..fb9ca69f72c2 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c

@@ -1001,6 +1001,40 @@ const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[] = {
const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size =
sizeof(uclogic_rdesc_ugee_v2_frame_dial_template_arr);

+/* Fixed report descriptor template for UGEE v2 frame reports (mouse) */

+const __u8 uclogic_rdesc_ugee_v2_frame_mouse_template_arr[] = {

+ 0x05, 0x01, /* Usage Page (Desktop), */

+ 0x09, 0x02, /* Usage (Mouse), */

+ 0xA1, 0x01, /* Collection (Application), */

+ 0x85, 0x01, /* Report ID (1), */
+ 0x05, 0x01, /* Usage Page (Pointer), */

+ 0xA0, /* Collection (Physical), */
+ 0x75, 0x01, /* Report Size (1), */

+ 0x95, 0x02, /* Report Count (2), */

+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x19, 0x01, /* Usage Minimum (01h), */

+ 0x29, 0x02, /* Usage Maximum (02h), */

+ 0x14, /* Logical Minimum (0), */
+ 0x25, 0x01, /* Logical Maximum (1), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x95, 0x06, /* Report Count (6), */
+ 0x81, 0x01, /* Input (Constant), */

+ 0x05, 0x01, /* Usage Page (Generic Desktop), */
+ 0x09, 0x30, /* Usage (X), */
+ 0x09, 0x31, /* Usage (Y), */

+ 0x75, 0x10, /* Report Size (16), */

+ 0x95, 0x02, /* Report Count (2), */

+ 0x16, 0x00, 0x80, /* Logical Minimum (-32768), */

+ 0x26, 0xFF, 0x7F, /* Logical Maximum (32767), */

+ 0x81, 0x06, /* Input (Variable, Relative), */

+ 0x95, 0x01, /* Report Count (1), */

+ 0x81, 0x01, /* Input (Constant), */
+ 0xC0, /* End Collection, */
+ 0xC0 /* End Collection */
+};

+const size_t uclogic_rdesc_ugee_v2_frame_mouse_template_size =
+ sizeof(uclogic_rdesc_ugee_v2_frame_mouse_template_arr);

+
/* Fixed report descriptor for Ugee EX07 frame */
const __u8 uclogic_rdesc_ugee_ex07_frame_arr[] = {
0x05, 0x01, /* Usage Page (Desktop), */
diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h

index 1a2d658bad3a..0502a0656496 100644
--- a/drivers/hid/hid-uclogic-rdesc.h
+++ b/drivers/hid/hid-uclogic-rdesc.h

@@ -173,6 +173,10 @@ extern const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size;
extern const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[];
extern const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size;

+/* Fixed report descriptor template for UGEE v2 frame reports (mouse) */
+extern const __u8 uclogic_rdesc_ugee_v2_frame_mouse_template_arr[];
+extern const size_t uclogic_rdesc_ugee_v2_frame_mouse_template_size;

José Expósito

unread,

Jul 17, 2022, 10:43:44 AM7/17/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The XP-PEN Deco Pro S is a UGEE v2 device with a frame with 8 buttons,
a bitmap dial and a mouse. Its pen has 2 buttons, supports tilt and
pressure.

All the pieces to support it are already in place. Add its ID in order
to support the device.

The required Wireshark traces were captured by Jouke Witteveen.
For more information check [1].

Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/738 [1]

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-ids.h | 1 +

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index f25c483b794b..2407e927d1bf 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c

kernel test robot

unread,

Jul 25, 2022, 10:34:33 PM7/25/22

to José Expósito, ji...@kernel.org, ll...@lists.linux.dev, kbuil...@lists.01.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Hi "José,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 0cb1fc0988e32bda84c2b7218e0c761af1430baf]

url: https://github.com/intel-lab-lkp/linux/commits/Jos-Exp-sito/XP-PEN-Deco-Pro-S-support-for-5-20-uclogic/20220717-224559
base: 0cb1fc0988e32bda84c2b7218e0c761af1430baf
config: x86_64-buildonly-randconfig-r002-20220718 (https://download.01.org/0day-ci/archive/20220726/202207261047...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project fa0c7639e91fa1cd0cf2ff0445a1634a90fe850a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/10fcf5d5cca4657c53477c392b1fb675d72cfda3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jos-Exp-sito/XP-PEN-Deco-Pro-S-support-for-5-20-uclogic/20220717-224559
git checkout 10fcf5d5cca4657c53477c392b1fb675d72cfda3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <l...@intel.com>

All errors (new ones prefixed by >>):

>> ld.lld: error: call to __read_overflow marked "dontcall-error": detected read beyond size of object (1st parameter)

--
0-DAY CI Kernel Test Service
https://01.org/lkp

Nathan Chancellor

unread,

Jul 26, 2022, 11:48:34 AM7/26/22

to kernel test robot, José Expósito, ji...@kernel.org, ll...@lists.linux.dev, kbuil...@lists.01.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com

Unfortunately, LTO makes this warning kind of cryptic but it seems like
the wrong template size is being used? This appears to resolve the
warning for me.

Cheers,
Nathan

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 2407e927d1bf..dd4b1ed6fd1e 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1205,7 +1205,7 @@ static int uclogic_params_ugee_v2_init_frame_mouse(struct uclogic_params *p)

rc = uclogic_params_frame_init_with_desc(&p->frame_list[1],

uclogic_rdesc_ugee_v2_frame_mouse_template_arr,
- uclogic_rdesc_ugee_v2_frame_dial_template_size,
+ uclogic_rdesc_ugee_v2_frame_mouse_template_size,
UCLOGIC_RDESC_V1_FRAME_ID);
if (rc)
return rc;

José Expósito

unread,

Jul 26, 2022, 12:15:01 PM7/26/22

to Nathan Chancellor, kernel test robot, ji...@kernel.org, ll...@lists.linux.dev, kbuil...@lists.01.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com

Hi Nathan,

Yes, I was about to send v3 fixing it. As you mentioned, I copy-pasted
the wrong template size.

I really need to learn how to enable those warnings in my dev
environment. My toolchain didn't complain about this.
Thanks for pointing me to LTO. I didn't know about the CONFIG_LTO_*
options, I'll play a bit with them.

Thanks a lot for looking into this,
Jose

Nathan Chancellor

unread,

Jul 26, 2022, 12:24:24 PM7/26/22

to José Expósito, kernel test robot, ji...@kernel.org, ll...@lists.linux.dev, kbuil...@lists.01.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com

Good to know! At least we caught it early :)

> I really need to learn how to enable those warnings in my dev
> environment. My toolchain didn't complain about this.

CONFIG_FORTIFY_SOURCE will be the configuration that can catch these
issues but I did not see a warning from clang without LTO, so it is
likely the case that LTO helped the compiler see through translation
units that the size was not correct.

> Thanks for pointing me to LTO. I didn't know about the CONFIG_LTO_*
> options, I'll play a bit with them.

If you have any questions about it, feel free to reach out to our
mailing list (ll...@lists.linux.dev), we're always happy to help!

> Thanks a lot for looking into this,
> Jose

Any time!

Cheers,
Nathan

José Expósito

unread,

Jul 26, 2022, 12:39:32 PM7/26/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, nat...@kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito, kernel test robot

Hello everyone,

This is v3 of [1]. Check the description in the link for more
information, please.

v1 -> v2:

First patch acked-by Daniel Latypov

v2 -> v3:

Fix bug in "HID: uclogic: Add support for UGEE v2 mouse frames".

Reported-by: kernel test robot <l...@intel.com>

I copy-pasted the wrong template size.

Thanks to Nathan Chancellor for looking into it. As he mentioned [2],
CONFIG_FORTIFY_SOURCE doesn't catch this error without LTO enabled.

[1] https://lore.kernel.org/linux-input/20220717144333.2511...@gmail.com/T/
[2] https://lore.kernel.org/linux-input/20220717144333.2511...@gmail.com/T/#m796ac6c8f7484b0bafc1f1ed80d3ead90ae5dbb7

José Expósito (7):
HID: uclogic: KUnit best practices and naming conventions
HID: uclogic: Refactor UGEE v2 string descriptor parsing
HID: uclogic: Refactor UGEE v2 frame initialization
HID: uclogic: Parse the UGEE v2 frame type
HID: uclogic: Add support for UGEE v2 dial frames
HID: uclogic: Add support for UGEE v2 mouse frames
HID: uclogic: Add support for XP-PEN Deco Pro S

drivers/hid/Kconfig | 2 +-
drivers/hid/Makefile | 4 +-

drivers/hid/hid-ids.h | 1 +

10 files changed, 494 insertions(+), 46 deletions(-)
create mode 100644 drivers/hid/hid-uclogic-params-test.c

base-commit: 0cb1fc0988e32bda84c2b7218e0c761af1430baf
--
2.25.1

José Expósito

unread,

Jul 26, 2022, 12:39:33 PM7/26/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, nat...@kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito, Daniel Latypov

The KUnit documentation [1] suggests allowing build tests as a module.

In addition, it is recommended [2] to use snake case names for
kunit_suite and test cases.

Change the Kconfig entry from bool to tristate and stick to the naming
conventions to avoid style issues with future tests.

Link: https://docs.kernel.org/dev-tools/kunit/style.html#test-kconfig-entries [1]
Link: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html [2]
Acked-by: Daniel Latypov <dlat...@google.com>

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/Kconfig | 2 +-
drivers/hid/Makefile | 3 ++-
drivers/hid/hid-uclogic-rdesc-test.c | 22 +++++++++++-----------
3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 6ce92830b5d1..36a17958493f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1307,7 +1307,7 @@ config HID_MCP2221
will be called hid-mcp2221.ko.

config HID_KUNIT_TEST
- bool "KUnit tests for HID" if !KUNIT_ALL_TESTS
+ tristate "KUnit tests for HID" if !KUNIT_ALL_TESTS
depends on KUNIT=y
depends on HID_UCLOGIC
default KUNIT_ALL_TESTS

diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index b0bef8098139..82d8fd97d96c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile

@@ -144,8 +144,9 @@ obj-$(CONFIG_HID_WIIMOTE) += hid-wiimote.o

obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o

José Expósito

unread,

Jul 26, 2022, 12:39:34 PM7/26/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, nat...@kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The UGEE v2 tablets expose a string descriptor with their capabilities.

Move the code used to parse the descriptors and generate a parameter
list from it to its own function and add KUnit tests to validate the
parser.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>

Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/Makefile | 1 +
drivers/hid/hid-uclogic-params-test.c | 159 ++++++++++++++++++++++++++
drivers/hid/hid-uclogic-params.c | 86 ++++++++++----

3 files changed, 226 insertions(+), 20 deletions(-)
create mode 100644 drivers/hid/hid-uclogic-params-test.c

diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 82d8fd97d96c..fe69dece2a46 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -145,6 +145,7 @@ obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o

+ .name = "resolution_with_value_0",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0x70, 0xB2,
+ 0x10, 0x77,
+ 0x08,
+ 0x00,
+ 0xFF, 0x1F,
+ 0x00, 0x00,

+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB270,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7710,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },

+ },

+ /* XP-PEN Deco L str_desc: Frame with 8 buttons */
+ {

+ .name = "frame_type_buttons",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0x70, 0xB2,
+ 0x10, 0x77,
+ 0x08,
+ 0x00,

+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB270,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2320,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7710,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x1770,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },

+ },
+ /* PARBLO A610 PRO str_desc: Frame with 9 buttons and dial */
+ {

+ .name = "frame_type_dial",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0x96, 0xC7,
+ 0xF9, 0x7C,
+ 0x09,
+ 0x01,

+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xC796,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2749,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x7CF9,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x1899,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index f24a4aca7920..ab54f0125e05 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c

@@ -1052,6 +1052,62 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,
return rc;
}

+/**
+ * uclogic_params_parse_ugee_v2_desc - parse the string descriptor containing
+ * pen and frame parameters returned by UGEE v2 devices.
+ *

+ * @str_desc: String descriptor, cannot be NULL.
+ * @str_desc_size: Size of the string descriptor.
+ * @desc_params: Output description params list.
+ * @desc_params_size: Size of the output description params list.

+ *
+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,
+ size_t str_desc_size,

+ s32 *desc_params,
+ size_t desc_params_size)
+{

+ s32 pen_x_lm, pen_y_lm;
+ s32 pen_x_pm, pen_y_pm;
+ s32 pen_pressure_lm;
+ s32 frame_num_buttons;
+ s32 resolution;
+
+ /* Minimum descriptor length required, maximum seen so far is 14 */
+ const int min_str_desc_size = 12;
+
+ if (!str_desc || str_desc_size < min_str_desc_size)

+ return -EINVAL;
+
+ if (desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+

+ pen_x_lm = get_unaligned_le16(str_desc + 2);
+ pen_y_lm = get_unaligned_le16(str_desc + 4);
+ frame_num_buttons = str_desc[6];
+ pen_pressure_lm = get_unaligned_le16(str_desc + 8);
+
+ resolution = get_unaligned_le16(str_desc + 10);
+ if (resolution == 0) {
+ pen_x_pm = 0;
+ pen_y_pm = 0;
+ } else {
+ pen_x_pm = pen_x_lm * 1000 / resolution;
+ pen_y_pm = pen_y_lm * 1000 / resolution;
+ }
+
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_LM] = pen_x_lm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_PM] = pen_x_pm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = pen_y_lm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = pen_y_pm;
+ desc_params[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = pen_pressure_lm;
+ desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM] = frame_num_buttons;

+
+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1082,7 +1138,6 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
__u8 *rdesc_pen = NULL;

__u8 *rdesc_frame = NULL;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
- s32 resolution;

__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

José Expósito

unread,

Jul 26, 2022, 12:39:34 PM7/26/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, nat...@kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

At the moment, the driver only supports UGEE v2 devices that have
buttons in their frames.

In order to support other types of frames in the future, move the code
used to initialize this kind of frames to its own function.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 55 +++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index ab54f0125e05..7b9e1892aa91 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1108,6 +1108,44 @@ static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,
return 0;
}

+/**
+ * uclogic_params_ugee_v2_init_frame_buttons() - initialize a UGEE v2 frame with
+ * buttons.
+ * @p: Parameters to fill in, cannot be NULL.
+ * @desc_params: Device description params list.
+ * @desc_params_size: Size of the description params list.

+ *
+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_ugee_v2_init_frame_buttons(struct uclogic_params *p,

+ const s32 *desc_params,
+ size_t desc_params_size)
+{
+ __u8 *rdesc_frame = NULL;

+ int rc = 0;
+

+ if (!p || desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+

+ rdesc_frame = uclogic_rdesc_template_apply(
+ uclogic_rdesc_ugee_v2_frame_btn_template_arr,
+ uclogic_rdesc_ugee_v2_frame_btn_template_size,

+ desc_params, UCLOGIC_RDESC_PH_ID_NUM);
+ if (!rdesc_frame)
+ return -ENOMEM;

+

+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[0],
+ rdesc_frame,

+ uclogic_rdesc_ugee_v2_frame_btn_template_size,
+ UCLOGIC_RDESC_V1_FRAME_ID);
+ kfree(rdesc_frame);

+ if (rc)
+ return rc;

+
+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1136,7 +1174,6 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
const int str_desc_len = 12;

__u8 *str_desc = NULL;
__u8 *rdesc_pen = NULL;

- __u8 *rdesc_frame = NULL;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];

__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00

@@ -1205,20 +1242,8 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

p.pen.subreport_list[0].id = UCLOGIC_RDESC_V1_FRAME_ID;

/* Initialize the frame interface */

- rdesc_frame = uclogic_rdesc_template_apply(
- uclogic_rdesc_ugee_v2_frame_btn_template_arr,
- uclogic_rdesc_ugee_v2_frame_btn_template_size,
- desc_params, ARRAY_SIZE(desc_params));
- if (!rdesc_frame) {
- rc = -ENOMEM;
- goto cleanup;
- }
-
- rc = uclogic_params_frame_init_with_desc(&p.frame_list[0],
- rdesc_frame,
- uclogic_rdesc_ugee_v2_frame_btn_template_size,
- UCLOGIC_RDESC_V1_FRAME_ID);
- kfree(rdesc_frame);

+ rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
+ ARRAY_SIZE(desc_params));

if (rc) {
uclogic_params_init_invalid(&p);
goto output;

--
2.25.1

José Expósito

unread,

Jul 26, 2022, 12:39:35 PM7/26/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, nat...@kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The string descriptor returned by UGEE v2 devices contains a byte
indicating the device frame type.

The values discovered so far are:

- 0: Frame with buttons, present in the XP-PEN Deco L.
- 1: Frame with buttons and dial, present in the PARBLO A610 PRO.
- 2: Frame with buttons and a mouse, shaped as a dial + touchpad.
Present in the XP-PEN Deco Pro S.

Parse the frame type and add KUnit tests.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params-test.c | 35 ++++++++++++++++++++++++++-
drivers/hid/hid-uclogic-params.c | 19 ++++++++++++---
drivers/hid/hid-uclogic-params.h | 10 ++++++++
3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-uclogic-params-test.c b/drivers/hid/hid-uclogic-params-test.c
index 9f043f2ab387..57ef5d3e4b74 100644
--- a/drivers/hid/hid-uclogic-params-test.c
+++ b/drivers/hid/hid-uclogic-params-test.c
@@ -7,6 +7,7 @@
*/

#include <kunit/test.h>
+#include "./hid-uclogic-params.h"
#include "./hid-uclogic-rdesc.h"

#define MAX_STR_DESC_SIZE 14
@@ -17,6 +18,7 @@ struct uclogic_parse_ugee_v2_desc_case {
const __u8 str_desc[MAX_STR_DESC_SIZE];
size_t str_desc_size;

const s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];

+ enum uclogic_params_frame_type frame_type;
};

static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[] = {
@@ -26,6 +28,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
.str_desc = {},
.str_desc_size = 0,
.desc_params = {},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},
{
.name = "resolution_with_value_0",
@@ -48,6 +51,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},

/* XP-PEN Deco L str_desc: Frame with 8 buttons */

{
@@ -71,6 +75,7 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_BUTTONS,
},

/* PARBLO A610 PRO str_desc: Frame with 9 buttons and dial */

{
@@ -94,6 +99,31 @@ static struct uclogic_parse_ugee_v2_desc_case uclogic_parse_ugee_v2_desc_cases[]
[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x09,
},
+ .frame_type = UCLOGIC_PARAMS_FRAME_DIAL,

+ },
+ /* XP-PEN Deco Pro S str_desc: Frame with 8 buttons and mouse */
+ {
+ .name = "frame_type_mouse",

+ .res = 0,
+ .str_desc = {
+ 0x0E, 0x03,

+ 0xC8, 0xB3,
+ 0x34, 0x65,
+ 0x08,
+ 0x02,

+ 0xFF, 0x1F,
+ 0xD8, 0x13,
+ },
+ .str_desc_size = 12,
+ .desc_params = {

+ [UCLOGIC_RDESC_PEN_PH_ID_X_LM] = 0xB3C8,
+ [UCLOGIC_RDESC_PEN_PH_ID_X_PM] = 0x2363,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = 0x6534,
+ [UCLOGIC_RDESC_PEN_PH_ID_Y_PM] = 0x13EC,

+ [UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] = 0x1FFF,
+ [UCLOGIC_RDESC_FRAME_PH_ID_UM] = 0x08,
+ },

+ .frame_type = UCLOGIC_PARAMS_FRAME_MOUSE,
},
};

@@ -110,12 +140,14 @@ static void uclogic_parse_ugee_v2_desc_test(struct kunit *test)
{
int res;
s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+ enum uclogic_params_frame_type frame_type;

const struct uclogic_parse_ugee_v2_desc_case *params = test->param_value;

res = uclogic_params_parse_ugee_v2_desc(params->str_desc,
params->str_desc_size,
desc_params,
- ARRAY_SIZE(desc_params));
+ ARRAY_SIZE(desc_params),
+ &frame_type);
KUNIT_ASSERT_EQ(test, res, params->res);

if (res)
@@ -139,6 +171,7 @@ static void uclogic_parse_ugee_v2_desc_test(struct kunit *test)
KUNIT_EXPECT_EQ(test,
params->desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM],
desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM]);
+ KUNIT_EXPECT_EQ(test, params->frame_type, frame_type);
}

static struct kunit_case hid_uclogic_params_test_cases[] = {

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 7b9e1892aa91..b232125f6972 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1060,6 +1060,7 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,

* @str_desc_size: Size of the string descriptor.

* @desc_params: Output description params list.

* @desc_params_size: Size of the output description params list.
+ * @frame_type: Output frame type.
*
* Returns:

* Zero, if successful. A negative errno code on error.

@@ -1067,7 +1068,8 @@ static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,

static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,

size_t str_desc_size,
s32 *desc_params,
- size_t desc_params_size)
+ size_t desc_params_size,
+ enum uclogic_params_frame_type *frame_type)
{
s32 pen_x_lm, pen_y_lm;
s32 pen_x_pm, pen_y_pm;

@@ -1087,6 +1089,7 @@ static int uclogic_params_parse_ugee_v2_desc(const __u8 *str_desc,

pen_x_lm = get_unaligned_le16(str_desc + 2);

pen_y_lm = get_unaligned_le16(str_desc + 4);

frame_num_buttons = str_desc[6];
+ *frame_type = str_desc[7];

pen_pressure_lm = get_unaligned_le16(str_desc + 8);

resolution = get_unaligned_le16(str_desc + 10);

@@ -1175,6 +1178,7 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

__u8 *str_desc = NULL;
__u8 *rdesc_pen = NULL;

s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+ enum uclogic_params_frame_type frame_type;

__u8 magic_arr[] = {
0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

@@ -1218,7 +1222,8 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

rc = uclogic_params_parse_ugee_v2_desc(str_desc, str_desc_len,
desc_params,
- ARRAY_SIZE(desc_params));
+ ARRAY_SIZE(desc_params),
+ &frame_type);
if (rc)
goto cleanup;

@@ -1242,8 +1247,14 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

p.pen.subreport_list[0].id = UCLOGIC_RDESC_V1_FRAME_ID;

/* Initialize the frame interface */

- rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
- ARRAY_SIZE(desc_params));
+ switch (frame_type) {
+ case UCLOGIC_PARAMS_FRAME_BUTTONS:
+ default:

+ rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,
+ ARRAY_SIZE(desc_params));

+ break;
+ }
+

if (rc) {
uclogic_params_init_invalid(&p);
goto output;

José Expósito

unread,

Jul 26, 2022, 12:39:37 PM7/26/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, nat...@kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Add the required HID descriptors and the initialization function for

UGEE v2 frames with a bitmap dial.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 44 ++++++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.c | 40 +++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.h | 4 +++
3 files changed, 88 insertions(+)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index b232125f6972..8ac7caeda535 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c

@@ -1149,6 +1149,45 @@ static int uclogic_params_ugee_v2_init_frame_buttons(struct uclogic_params *p,

return 0;
}

+/**

+ * uclogic_params_ugee_v2_init_frame_dial() - initialize a UGEE v2 frame with a
+ * bitmap dial.

+ * @p: Parameters to fill in, cannot be NULL.

+ * @desc_params: Device description params list.
+ * @desc_params_size: Size of the description params list.
+ *

+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_ugee_v2_init_frame_dial(struct uclogic_params *p,

+ const s32 *desc_params,
+ size_t desc_params_size)
+{
+ __u8 *rdesc_frame = NULL;
+ int rc = 0;
+
+ if (!p || desc_params_size != UCLOGIC_RDESC_PH_ID_NUM)
+ return -EINVAL;
+
+ rdesc_frame = uclogic_rdesc_template_apply(

+ uclogic_rdesc_ugee_v2_frame_dial_template_arr,
+ uclogic_rdesc_ugee_v2_frame_dial_template_size,

+ desc_params, UCLOGIC_RDESC_PH_ID_NUM);
+ if (!rdesc_frame)
+ return -ENOMEM;
+
+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[0],
+ rdesc_frame,

+ uclogic_rdesc_ugee_v2_frame_dial_template_size,

+ UCLOGIC_RDESC_V1_FRAME_ID);
+ kfree(rdesc_frame);
+ if (rc)
+ return rc;
+

+ p->frame_list[0].bitmap_dial_byte = 7;

+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1248,6 +1287,11 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

/* Initialize the frame interface */

switch (frame_type) {
+ case UCLOGIC_PARAMS_FRAME_DIAL:
+ case UCLOGIC_PARAMS_FRAME_MOUSE:

+ rc = uclogic_params_ugee_v2_init_frame_dial(&p, desc_params,
+ ARRAY_SIZE(desc_params));
+ break;

case UCLOGIC_PARAMS_FRAME_BUTTONS:
default:
rc = uclogic_params_ugee_v2_init_frame_buttons(&p, desc_params,

diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index 3d68e8b0784d..22429df693fc 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c

@@ -961,6 +961,46 @@ const __u8 uclogic_rdesc_ugee_v2_frame_btn_template_arr[] = {
const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size =
sizeof(uclogic_rdesc_ugee_v2_frame_btn_template_arr);

+/* Fixed report descriptor template for UGEE v2 frame reports (dial) */

+const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[] = {

+ 0x05, 0x01, /* Usage Page (Desktop), */

+ 0x09, 0x07, /* Usage (Keypad), */

+ 0xA1, 0x01, /* Collection (Application), */

+ 0x85, UCLOGIC_RDESC_V1_FRAME_ID,
+ /* Report ID, */
+ 0x05, 0x0D, /* Usage Page (Digitizer), */

+ 0x09, 0x39, /* Usage (Tablet Function Keys), */

+ 0xA0, /* Collection (Physical), */
+ 0x75, 0x01, /* Report Size (1), */

+ 0x95, 0x08, /* Report Count (8), */

+ 0x81, 0x01, /* Input (Constant), */

+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x19, 0x01, /* Usage Minimum (01h), */

+ UCLOGIC_RDESC_FRAME_PH_BTN,
+ /* Usage Maximum (PLACEHOLDER), */

+ 0x95, 0x0A, /* Report Count (10), */

+ 0x14, /* Logical Minimum (0), */
+ 0x25, 0x01, /* Logical Maximum (1), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x95, 0x06, /* Report Count (6), */
+ 0x81, 0x01, /* Input (Constant), */

+ 0x75, 0x08, /* Report Size (8), */

+ 0x95, 0x03, /* Report Count (3), */

+ 0x81, 0x01, /* Input (Constant), */

+ 0x05, 0x01, /* Usage Page (Desktop), */

+ 0x09, 0x38, /* Usage (Wheel), */

+ 0x95, 0x01, /* Report Count (1), */

+ 0x15, 0xFF, /* Logical Minimum (-1), */

+ 0x25, 0x01, /* Logical Maximum (1), */

+ 0x81, 0x06, /* Input (Variable, Relative), */

+ 0x95, 0x02, /* Report Count (2), */

+ 0x81, 0x01, /* Input (Constant), */
+ 0xC0, /* End Collection, */
+ 0xC0 /* End Collection */
+};

+const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size =
+ sizeof(uclogic_rdesc_ugee_v2_frame_dial_template_arr);

+
/* Fixed report descriptor for Ugee EX07 frame */
const __u8 uclogic_rdesc_ugee_ex07_frame_arr[] = {
0x05, 0x01, /* Usage Page (Desktop), */
diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h

index 86e64a9ee6bd..1a2d658bad3a 100644
--- a/drivers/hid/hid-uclogic-rdesc.h
+++ b/drivers/hid/hid-uclogic-rdesc.h

@@ -169,6 +169,10 @@ extern const size_t uclogic_rdesc_ugee_v2_pen_template_size;
extern const __u8 uclogic_rdesc_ugee_v2_frame_btn_template_arr[];
extern const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size;

+/* Fixed report descriptor template for UGEE v2 frame reports (dial) */
+extern const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[];
+extern const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size;

José Expósito

unread,

Jul 26, 2022, 12:39:37 PM7/26/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, nat...@kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

Add the required HID descriptors and the initialization function for

UGEE v2 frames with a mouse in the frame.

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-uclogic-params.c | 31 +++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.c | 34 ++++++++++++++++++++++++++++++++
drivers/hid/hid-uclogic-rdesc.h | 4 ++++

3 files changed, 69 insertions(+)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 8ac7caeda535..797eeb443c18 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1188,6 +1188,31 @@ static int uclogic_params_ugee_v2_init_frame_dial(struct uclogic_params *p,
return 0;
}

+/**
+ * uclogic_params_ugee_v2_init_frame_mouse() - initialize a UGEE v2 frame with a
+ * mouse.

+ * @p: Parameters to fill in, cannot be NULL.
+ *

+ * Returns:
+ * Zero, if successful. A negative errno code on error.
+ */

+static int uclogic_params_ugee_v2_init_frame_mouse(struct uclogic_params *p)
+{

+ int rc = 0;
+

+ if (!p)
+ return -EINVAL;
+
+ rc = uclogic_params_frame_init_with_desc(&p->frame_list[1],
+ uclogic_rdesc_ugee_v2_frame_mouse_template_arr,
+ uclogic_rdesc_ugee_v2_frame_mouse_template_size,
+ UCLOGIC_RDESC_V1_FRAME_ID);

+ if (rc)
+ return rc;
+

+ return 0;
+}
+
/**
* uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by
* discovering their parameters.

@@ -1231,6 +1256,12 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,

iface = to_usb_interface(hdev->dev.parent);
bInterfaceNumber = iface->cur_altsetting->desc.bInterfaceNumber;
+
+ if (bInterfaceNumber == 0) {
+ uclogic_params_ugee_v2_init_frame_mouse(&p);
+ goto output;
+ }
+
if (bInterfaceNumber != 2) {
uclogic_params_init_invalid(&p);
goto output;

diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index 22429df693fc..fb9ca69f72c2 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c

@@ -1001,6 +1001,40 @@ const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[] = {
const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size =
sizeof(uclogic_rdesc_ugee_v2_frame_dial_template_arr);

+/* Fixed report descriptor template for UGEE v2 frame reports (mouse) */

+const __u8 uclogic_rdesc_ugee_v2_frame_mouse_template_arr[] = {

+ 0x05, 0x01, /* Usage Page (Desktop), */

+ 0x09, 0x02, /* Usage (Mouse), */

+ 0xA1, 0x01, /* Collection (Application), */

+ 0x85, 0x01, /* Report ID (1), */
+ 0x05, 0x01, /* Usage Page (Pointer), */

+ 0xA0, /* Collection (Physical), */
+ 0x75, 0x01, /* Report Size (1), */

+ 0x95, 0x02, /* Report Count (2), */

+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x19, 0x01, /* Usage Minimum (01h), */

+ 0x29, 0x02, /* Usage Maximum (02h), */

+ 0x14, /* Logical Minimum (0), */
+ 0x25, 0x01, /* Logical Maximum (1), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x95, 0x06, /* Report Count (6), */
+ 0x81, 0x01, /* Input (Constant), */

+ 0x05, 0x01, /* Usage Page (Generic Desktop), */
+ 0x09, 0x30, /* Usage (X), */
+ 0x09, 0x31, /* Usage (Y), */

+ 0x75, 0x10, /* Report Size (16), */

+ 0x95, 0x02, /* Report Count (2), */

+ 0x16, 0x00, 0x80, /* Logical Minimum (-32768), */

+ 0x26, 0xFF, 0x7F, /* Logical Maximum (32767), */

+ 0x81, 0x06, /* Input (Variable, Relative), */

+ 0x95, 0x01, /* Report Count (1), */

+ 0x81, 0x01, /* Input (Constant), */
+ 0xC0, /* End Collection, */
+ 0xC0 /* End Collection */
+};

+const size_t uclogic_rdesc_ugee_v2_frame_mouse_template_size =
+ sizeof(uclogic_rdesc_ugee_v2_frame_mouse_template_arr);

+
/* Fixed report descriptor for Ugee EX07 frame */
const __u8 uclogic_rdesc_ugee_ex07_frame_arr[] = {
0x05, 0x01, /* Usage Page (Desktop), */
diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h

index 1a2d658bad3a..0502a0656496 100644
--- a/drivers/hid/hid-uclogic-rdesc.h
+++ b/drivers/hid/hid-uclogic-rdesc.h

@@ -173,6 +173,10 @@ extern const size_t uclogic_rdesc_ugee_v2_frame_btn_template_size;
extern const __u8 uclogic_rdesc_ugee_v2_frame_dial_template_arr[];
extern const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size;

+/* Fixed report descriptor template for UGEE v2 frame reports (mouse) */
+extern const __u8 uclogic_rdesc_ugee_v2_frame_mouse_template_arr[];
+extern const size_t uclogic_rdesc_ugee_v2_frame_mouse_template_size;

José Expósito

unread,

Jul 26, 2022, 12:39:38 PM7/26/22

to ji...@kernel.org, benjamin....@redhat.com, spb...@gmail.com, j.wit...@gmail.com, stefa...@gmail.com, nat...@kernel.org, linux...@vger.kernel.org, linux-...@vger.kernel.org, kuni...@googlegroups.com, José Expósito

The XP-PEN Deco Pro S is a UGEE v2 device with a frame with 8 buttons,
a bitmap dial and a mouse. Its pen has 2 buttons, supports tilt and
pressure.

All the pieces to support it are already in place. Add its ID in order
to support the device.

The required Wireshark traces were captured by Jouke Witteveen.
For more information check [1].

Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/738 [1]

Tested-by: Jouke Witteveen <j.wit...@gmail.com>
Signed-off-by: José Expósito <jose.ex...@gmail.com>
---

drivers/hid/hid-ids.h | 1 +

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 797eeb443c18..dd4b1ed6fd1e 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c

Reply all

Reply to author

Forward