Skip to content

Commit

Permalink
HID: wacom: Remove static WACOM_PKGLEN_MAX limit
Browse files Browse the repository at this point in the history
Rather than memcpy every packet that we receive from HID into our own
local fixed-size array, we can just access the data directly through
the original pointer. While we're at it, remove the few other places
that assume a fixed maximum packet size and make them dynamic (in
particular: temporary buffers created by the wacom_wac_queue_flush and
wacom_intuos_bt_process_data functions; and the pen_fifo FIFO).

To ensure pen_fifo allocation has access to the maximum used packet
length, this commit also moves the function call to occur a little
later in the probe process.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
  • Loading branch information
Jason Gerecke authored and Jiri Kosina committed Mar 4, 2025
1 parent 58c9bf3 commit 5e013ad
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 23 deletions.
35 changes: 21 additions & 14 deletions drivers/hid/wacom_sys.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,27 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
struct kfifo_rec_ptr_2 *fifo)
{
while (!kfifo_is_empty(fifo)) {
u8 buf[WACOM_PKGLEN_MAX];
int size;
int size = kfifo_peek_len(fifo);
u8 *buf = kzalloc(size, GFP_KERNEL);
unsigned int count;
int err;

size = kfifo_out(fifo, buf, sizeof(buf));
count = kfifo_out(fifo, buf, size);
if (count != size) {
// Hard to say what is the "right" action in this
// circumstance. Skipping the entry and continuing
// to flush seems reasonable enough, however.
hid_warn(hdev, "%s: removed fifo entry with unexpected size\n",
__func__);
continue;
}
err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false);
if (err) {
hid_warn(hdev, "%s: unable to flush event due to error %d\n",
__func__, err);
}

kfree(buf);
}
}

Expand Down Expand Up @@ -158,13 +169,10 @@ static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report,
if (wacom->wacom_wac.features.type == BOOTLOADER)
return 0;

if (size > WACOM_PKGLEN_MAX)
return 1;

if (wacom_wac_pen_serial_enforce(hdev, report, raw_data, size))
return -1;

memcpy(wacom->wacom_wac.data, raw_data, size);
wacom->wacom_wac.data = raw_data;

wacom_wac_irq(&wacom->wacom_wac, size);

Expand Down Expand Up @@ -1286,6 +1294,7 @@ static void wacom_devm_kfifo_release(struct device *dev, void *res)
static int wacom_devm_kfifo_alloc(struct wacom *wacom)
{
struct wacom_wac *wacom_wac = &wacom->wacom_wac;
int fifo_size = min(PAGE_SIZE, 10 * wacom_wac->features.pktlen);
struct kfifo_rec_ptr_2 *pen_fifo;
int error;

Expand All @@ -1296,7 +1305,7 @@ static int wacom_devm_kfifo_alloc(struct wacom *wacom)
if (!pen_fifo)
return -ENOMEM;

error = kfifo_alloc(pen_fifo, WACOM_PKGLEN_MAX, GFP_KERNEL);
error = kfifo_alloc(pen_fifo, fifo_size, GFP_KERNEL);
if (error) {
devres_free(pen_fifo);
return error;
Expand Down Expand Up @@ -2352,12 +2361,14 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
unsigned int connect_mask = HID_CONNECT_HIDRAW;

features->pktlen = wacom_compute_pktlen(hdev);
if (features->pktlen > WACOM_PKGLEN_MAX)
return -EINVAL;

if (!devres_open_group(&hdev->dev, wacom, GFP_KERNEL))
return -ENOMEM;

error = wacom_devm_kfifo_alloc(wacom);
if (error)
goto fail;

wacom->resources = true;

error = wacom_allocate_inputs(wacom);
Expand Down Expand Up @@ -2821,10 +2832,6 @@ static int wacom_probe(struct hid_device *hdev,
if (features->check_for_hid_type && features->hid_type != hdev->type)
return -ENODEV;

error = wacom_devm_kfifo_alloc(wacom);
if (error)
return error;

wacom_wac->hid_data.inputmode = -1;
wacom_wac->mode_report = -1;

Expand Down
8 changes: 4 additions & 4 deletions drivers/hid/wacom_wac.c
Original file line number Diff line number Diff line change
Expand Up @@ -1201,12 +1201,10 @@ static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,

static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
{
unsigned char data[WACOM_PKGLEN_MAX];
u8 *data = kmemdup(wacom->data, len, GFP_KERNEL);
int i = 1;
unsigned power_raw, battery_capacity, bat_charging, ps_connected;

memcpy(data, wacom->data, len);

switch (data[0]) {
case 0x04:
wacom_intuos_bt_process_data(wacom, data + i);
Expand All @@ -1230,8 +1228,10 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
dev_dbg(wacom->pen_input->dev.parent,
"Unknown report: %d,%d size:%zu\n",
data[0], data[1], len);
return 0;
break;
}

kfree(data);
return 0;
}

Expand Down
7 changes: 2 additions & 5 deletions drivers/hid/wacom_wac.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
#include <linux/hid.h>
#include <linux/kfifo.h>

/* maximum packet length for USB/BT devices */
#define WACOM_PKGLEN_MAX 361

#define WACOM_NAME_MAX 64
#define WACOM_MAX_REMOTES 5
#define WACOM_STATUS_UNKNOWN 255
Expand Down Expand Up @@ -277,7 +274,7 @@ struct wacom_features {
unsigned touch_max;
int oVid;
int oPid;
int pktlen;
unsigned int pktlen;
bool check_for_hid_type;
int hid_type;
};
Expand Down Expand Up @@ -341,7 +338,7 @@ struct wacom_wac {
char pen_name[WACOM_NAME_MAX];
char touch_name[WACOM_NAME_MAX];
char pad_name[WACOM_NAME_MAX];
unsigned char data[WACOM_PKGLEN_MAX];
u8 *data;
int tool[2];
int id[2];
__u64 serial[2];
Expand Down

0 comments on commit 5e013ad

Please sign in to comment.