HID: uclogic: Refactor UGEE v2 string descriptor parsing

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.witteveen@gmail.com>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 82d8fd97d96cdcff7060c11a74c31371c581ea67..fe69dece2a460c63eca94badd7b1fdf8ae786f9f 100644 (file)
--- 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 (file)
index 0000000..9f043f2
--- /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.exposito89@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.exposito89@gmail.com>");

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index c11fa239e6a29281c19fe5237583fa6d35cf8bd4..07c5a21112cee8b803c4ea71946922dfa3732938 100644 (file)
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1056,6 +1056,62 @@ cleanup:
        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.
@@ -1086,7 +1142,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
        };
@@ -1128,25 +1183,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;
 
@@ -1517,3 +1559,7 @@ cleanup:
        uclogic_params_cleanup(&p);
        return rc;
 }
+
+#ifdef CONFIG_HID_KUNIT_TEST
+#include "hid-uclogic-params-test.c"
+#endif