On Mon, Jan 02, 2023 at 02:49:10PM -0500, Joshua Goins wrote:
> @@ -91,9 +115,27 @@ static int uclogic_input_mapping(struct hid_device *hdev,
> struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
> struct uclogic_params *params = &drvdata->params;
>
> - /* Discard invalid pen usages */
> - if (params->pen.usage_invalid && (field->application == HID_DG_PEN))
> - return -1;
> + if (field->application == HID_GD_KEYPAD) {
> + /*
> + * Remap input buttons to sensible ones that are not invalid.
> + * This only affects previous behavior for devices with more than ten or so buttons.
> + */
> + const int key = (usage->hid & HID_USAGE) - 1;
> +
> + if (key > 0 && key < ARRAY_SIZE(uclogic_extra_input_mapping)) {
It's an unusual that zero is not valid but I don't know the code at all.
> + hid_map_usage(hi,
> + usage,
> + bit,
> + max,
> + EV_KEY,
> + uclogic_extra_input_mapping[key]);
> + return 1;
> + }
> + } else if (field->application == HID_DG_PEN) {
> + /* Discard invalid pen usages */
> + if (params->pen.usage_invalid)
> + return -1;
> + }
>
> /* Let hid-core decide what to do */
> return 0;
[ snip ]
> +/*
> + * uclogic_params_init_ugee_xppen_pro() - Initializes a UGEE XP-Pen Pro tablet device.
> + *
> + * @hdev: The HID device of the tablet interface to initialize and get
> + * parameters from. Cannot be NULL.
> + * @params: Parameters to fill in (to be cleaned with
> + * uclogic_params_cleanup()). Not modified in case of error.
> + * Cannot be NULL.
> + *
> + * Returns:
> + * Zero, if successful. A negative errno code on error.
> + */
> +static int uclogic_params_init_ugee_xppen_pro(struct uclogic_params *params,
> + struct hid_device *hdev,
> + const u8 rdesc_frame_arr[],
> + const size_t rdesc_frame_size)
> +{
> + int rc = 0;
> + struct usb_interface *iface;
> + __u8 bInterfaceNumber;
> + const int str_desc_len = 12;
> + u8 *str_desc = NULL;
> + __u8 *rdesc_pen = NULL;
> + s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
> + /* The resulting parameters (noop) */
> + struct uclogic_params p = {0, };
> +
> + if (!hdev || !params) {
> + rc = -EINVAL;
> + goto cleanup;
> + }
> +
> + iface = to_usb_interface(hdev->dev.parent);
> + bInterfaceNumber = iface->cur_altsetting->desc.bInterfaceNumber;
> +
> + /* Ignore non-pen interfaces */
> + if (bInterfaceNumber != 2) {
> + uclogic_params_init_invalid(&p);
> + goto output;
So this is a success path? "ret = 0?" The comments kind of suggest
that but I want to be sure.
> + }
> +
> + /*
> + * Initialize the interface by sending magic data.
> + * This magic data is the same as other UGEE v2 tablets.
> + */
> + rc = uclogic_probe_interface(hdev,
> + uclogic_ugee_v2_probe_arr,
> + uclogic_ugee_v2_probe_size,
> + 0x03);
> + if (rc) {
> + uclogic_params_init_invalid(&p);
> + goto output;
> + }
> +
> + /**
> + * Read the string descriptor containing pen and frame parameters.
> + * These are slightly different than typical UGEE v2 devices.
> + */
> + rc = uclogic_params_get_str_desc(&str_desc, hdev, 100, str_desc_len);
> + if (rc != str_desc_len) {
> + hid_err(hdev, "failed retrieving pen and frame parameters: %d\n", rc);
> + uclogic_params_init_invalid(&p);
> + goto output;
This isn't correct. You need to do something like:
rc = (rc < 0) ? rc : -EINVAL;
regards,
dan carpenter
> + }
> +
> + rc = uclogic_params_parse_ugee_xppen_pro_desc(str_desc, str_desc_len,
> + desc_params,
> + ARRAY_SIZE(desc_params));
> + if (rc)
> + goto cleanup;
> +
> + kfree(str_desc);
> + str_desc = NULL;
> +
> + /* Initialize the pen interface */
> + rdesc_pen = uclogic_rdesc_template_apply(
> + uclogic_rdesc_ugee_v2_pen_template_arr,
> + uclogic_rdesc_ugee_v2_pen_template_size,
> + desc_params, ARRAY_SIZE(desc_params));
> + if (!rdesc_pen) {
> + rc = -ENOMEM;
> + goto cleanup;
> + }
> +
> + p.pen.desc_ptr = rdesc_pen;
> + p.pen.desc_size = uclogic_rdesc_ugee_v2_pen_template_size;
> + p.pen.id = 0x02;
> + p.pen.subreport_list[0].value = 0xf0;
> + p.pen.subreport_list[0].id = UCLOGIC_RDESC_V1_FRAME_ID;
> +
> + /* Initialize the frame interface */
> + rc = uclogic_params_frame_init_with_desc(
> + &p.frame_list[0],
> + rdesc_frame_arr,
> + rdesc_frame_size,
> + UCLOGIC_RDESC_V1_FRAME_ID);
> + if (rc < 0) {
> + hid_err(hdev, "initializing frame params failed: %d\n", rc);
> + goto output;
> + }
> +
> + p.frame_list[0].bitmap_dial_byte = 7;
> + p.frame_list[0].bitmap_second_dial_destination_byte = 8;
> +
> +output:
> + /* Output parameters */
> + memcpy(params, &p, sizeof(*params));
> + memset(&p, 0, sizeof(p));
> + rc = 0;
> +cleanup:
> + kfree(str_desc);
> + uclogic_params_cleanup(&p);
> + return rc;
> +}
