From f4abaa9eebde334045ed6ac4e564d050f1df3013 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Tue, 29 Jun 2021 11:25:50 -0700 Subject: [PATCH 01/31] HID: input: do not report stylus battery state as "full" The power supply states of discharging, charging, full, etc, represent state of charging, not the capacity level of the battery (for which we have a separate property). Current HID usage tables to not allow for expressing charging state of the batteries found in generic styli, so we should simply assume that the battery is discharging even if current capacity is at 100% when battery strength reporting is done via HID interface. In fact, we were doing just that before commit 581c4484769e. This change helps UIs to not mis-represent fully charged batteries in styli as being charging/topping-off. Fixes: 581c4484769e ("HID: input: map digitizer battery usage") Reported-by: Kenneth Albanowski Signed-off-by: Dmitry Torokhov Signed-off-by: Jiri Kosina --- drivers/hid/hid-input.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 4286a51f7f16..4b5ebeacd283 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -419,8 +419,6 @@ static int hidinput_get_battery_property(struct power_supply *psy, if (dev->battery_status == HID_BATTERY_UNKNOWN) val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - else if (dev->battery_capacity == 100) - val->intval = POWER_SUPPLY_STATUS_FULL; else val->intval = POWER_SUPPLY_STATUS_DISCHARGING; break; From d4b9f10a0eb64c623217eb5938d913974afbe9d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= Date: Wed, 7 Jul 2021 17:58:21 +0200 Subject: [PATCH 02/31] HID: magicmouse: enable high-resolution scroll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Magic Mouse, generations 1 and 2, doesn't have a physical scroll wheel, instead, the REL_WHEEL and REL_HWHEEL events are emulated when sliding a finger on the surface of the mouse. However, the smooth movement of the finger is transformed into a step based scroll on the screen, leading to a suboptimal user experience. Emulate high-resolution scroll by sending REL_WHEEL_HI_RES and REL_HWHEEL_HI_RES events so the scroll on the screen is closer to the finger movement. Signed-off-by: José Expósito Signed-off-by: Jiri Kosina --- drivers/hid/hid-magicmouse.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index 8bcaee4ccae0..e95f46dab4ad 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -68,6 +68,9 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie #define TOUCH_STATE_START 0x30 #define TOUCH_STATE_DRAG 0x40 +/* Number of high-resolution events for each low-resolution detent. */ +#define SCROLL_HR_STEPS 10 +#define SCROLL_HR_MULT (120 / SCROLL_HR_STEPS) #define SCROLL_ACCEL_DEFAULT 7 /* Touch surface information. Dimension is in hundredths of a mm, min and max @@ -126,6 +129,8 @@ struct magicmouse_sc { short y; short scroll_x; short scroll_y; + short scroll_x_hr; + short scroll_y_hr; u8 size; } touches[16]; int tracking_ids[16]; @@ -248,12 +253,18 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda unsigned long now = jiffies; int step_x = msc->touches[id].scroll_x - x; int step_y = msc->touches[id].scroll_y - y; + int step_hr = ((64 - (int)scroll_speed) * msc->scroll_accel) / + SCROLL_HR_STEPS; + int step_x_hr = msc->touches[id].scroll_x_hr - x; + int step_y_hr = msc->touches[id].scroll_y_hr - y; /* Calculate and apply the scroll motion. */ switch (state) { case TOUCH_STATE_START: msc->touches[id].scroll_x = x; msc->touches[id].scroll_y = y; + msc->touches[id].scroll_x_hr = x; + msc->touches[id].scroll_y_hr = y; /* Reset acceleration after half a second. */ if (scroll_acceleration && time_before(now, @@ -280,6 +291,24 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda msc->scroll_jiffies = now; input_report_rel(input, REL_WHEEL, step_y); } + + step_x_hr /= step_hr; + if (step_x_hr != 0) { + msc->touches[id].scroll_x_hr -= step_x_hr * + step_hr; + input_report_rel(input, + REL_HWHEEL_HI_RES, + -step_x_hr * SCROLL_HR_MULT); + } + + step_y_hr /= step_hr; + if (step_y_hr != 0) { + msc->touches[id].scroll_y_hr -= step_y_hr * + step_hr; + input_report_rel(input, + REL_WHEEL_HI_RES, + step_y_hr * SCROLL_HR_MULT); + } break; } } @@ -481,6 +510,8 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd if (emulate_scroll_wheel) { __set_bit(REL_WHEEL, input->relbit); __set_bit(REL_HWHEEL, input->relbit); + __set_bit(REL_WHEEL_HI_RES, input->relbit); + __set_bit(REL_HWHEEL_HI_RES, input->relbit); } } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) { /* setting the device name to ensure the same driver settings From 9d60648c607a2bfeef5e563e49de5d2e1b7a639a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= Date: Wed, 7 Jul 2021 17:58:22 +0200 Subject: [PATCH 03/31] HID: magicmouse: high-resolution scroll threshold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to avoid triggering involuntary high-resolution scroll events due to tiny touch movement deltas, add a movement threshold. The value chosen for the threshold, about 1.5 ~ 2 mm, is similar to the threshold used on touchpads by libinput (see libinput evdev-mt-touchpad-gestures.c) to try to keep the scroll experience consistent. Signed-off-by: José Expósito Signed-off-by: Jiri Kosina --- drivers/hid/hid-magicmouse.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index e95f46dab4ad..686788ebf3e1 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -71,6 +71,7 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie /* Number of high-resolution events for each low-resolution detent. */ #define SCROLL_HR_STEPS 10 #define SCROLL_HR_MULT (120 / SCROLL_HR_STEPS) +#define SCROLL_HR_THRESHOLD 90 /* units */ #define SCROLL_ACCEL_DEFAULT 7 /* Touch surface information. Dimension is in hundredths of a mm, min and max @@ -132,6 +133,8 @@ struct magicmouse_sc { short scroll_x_hr; short scroll_y_hr; u8 size; + bool scroll_x_active; + bool scroll_y_active; } touches[16]; int tracking_ids[16]; @@ -265,6 +268,8 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda msc->touches[id].scroll_y = y; msc->touches[id].scroll_x_hr = x; msc->touches[id].scroll_y_hr = y; + msc->touches[id].scroll_x_active = false; + msc->touches[id].scroll_y_active = false; /* Reset acceleration after half a second. */ if (scroll_acceleration && time_before(now, @@ -292,8 +297,16 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda input_report_rel(input, REL_WHEEL, step_y); } + if (!msc->touches[id].scroll_x_active && + abs(step_x_hr) > SCROLL_HR_THRESHOLD) { + msc->touches[id].scroll_x_active = true; + msc->touches[id].scroll_x_hr = x; + step_x_hr = 0; + } + step_x_hr /= step_hr; - if (step_x_hr != 0) { + if (step_x_hr != 0 && + msc->touches[id].scroll_x_active) { msc->touches[id].scroll_x_hr -= step_x_hr * step_hr; input_report_rel(input, @@ -301,8 +314,16 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda -step_x_hr * SCROLL_HR_MULT); } + if (!msc->touches[id].scroll_y_active && + abs(step_y_hr) > SCROLL_HR_THRESHOLD) { + msc->touches[id].scroll_y_active = true; + msc->touches[id].scroll_y_hr = y; + step_y_hr = 0; + } + step_y_hr /= step_hr; - if (step_y_hr != 0) { + if (step_y_hr != 0 && + msc->touches[id].scroll_y_active) { msc->touches[id].scroll_y_hr -= step_y_hr * step_hr; input_report_rel(input, From 18eeef46d3593e3bc3d6a8f7a6f94ace356578ff Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Fri, 25 Jun 2021 08:18:36 -0700 Subject: [PATCH 04/31] HID: i2c-hid: goodix: Tie the reset line to true state of the regulator The regulator for the touchscreen could be: * A dedicated regulator just for the touchscreen. * A regulator shared with something else in the system. * An always-on regulator. How we want the "reset" line to behave depends a bit on which of those three cases we're in. Currently the code is written with the assumption that it has a dedicated regulator, but that's not really guaranteed to be the case. The problem we run into is that if we leave the touchscreen powered on (because someone else is requesting the regulator or it's an always-on regulator) and we assert reset then we apparently burn an extra 67 mW of power. That's not great. Let's instead tie the control of the reset line to the true state of the regulator as reported by regulator notifiers. If we have an always-on regulator our notifier will never be called. If we have a shared regulator then our notifier will be called when the touchscreen is truly turned on or truly turned off. Using notifiers like this nicely handles all the cases without resorting to hacks like pretending that there is no "reset" GPIO if we have an always-on regulator. NOTE: if the regulator is on a shared line it's still possible that things could be a little off. Specifically, this case is not handled even after this patch: 1. Suspend goodix (send "sleep", goodix stops requesting regulator on) 2. Other regulator user turns off (regulator fully turns off). 3. Goodix driver gets notified and asserts reset. 4. Other regulator user turns on. 5. Goodix driver gets notified and deasserts reset. 6. Nobody resumes goodix. With that set of steps we'll have reset deasserted but we will have lost the results of the I2C_HID_PWR_SLEEP from the suspend path. That means we might be in higher power than we could be even if the goodix driver thinks things are suspended. Presumably, however, we're still in better shape than if we were asserting "reset" the whole time. If somehow the above situation is actually affecting someone and we want to do better we can deal with it when we have a real use case. Signed-off-by: Douglas Anderson Signed-off-by: Jiri Kosina --- drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 92 +++++++++++++++++++++---- 1 file changed, 79 insertions(+), 13 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c index ee0225982a82..31a4c229fdb7 100644 --- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c +++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c @@ -26,28 +26,29 @@ struct i2c_hid_of_goodix { struct i2chid_ops ops; struct regulator *vdd; + struct notifier_block nb; + struct mutex regulator_mutex; struct gpio_desc *reset_gpio; const struct goodix_i2c_hid_timing_data *timings; }; -static int goodix_i2c_hid_power_up(struct i2chid_ops *ops) +static void goodix_i2c_hid_deassert_reset(struct i2c_hid_of_goodix *ihid_goodix, + bool regulator_just_turned_on) { - struct i2c_hid_of_goodix *ihid_goodix = - container_of(ops, struct i2c_hid_of_goodix, ops); - int ret; - - ret = regulator_enable(ihid_goodix->vdd); - if (ret) - return ret; - - if (ihid_goodix->timings->post_power_delay_ms) + if (regulator_just_turned_on && ihid_goodix->timings->post_power_delay_ms) msleep(ihid_goodix->timings->post_power_delay_ms); gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 0); if (ihid_goodix->timings->post_gpio_reset_delay_ms) msleep(ihid_goodix->timings->post_gpio_reset_delay_ms); +} - return 0; +static int goodix_i2c_hid_power_up(struct i2chid_ops *ops) +{ + struct i2c_hid_of_goodix *ihid_goodix = + container_of(ops, struct i2c_hid_of_goodix, ops); + + return regulator_enable(ihid_goodix->vdd); } static void goodix_i2c_hid_power_down(struct i2chid_ops *ops) @@ -55,20 +56,54 @@ static void goodix_i2c_hid_power_down(struct i2chid_ops *ops) struct i2c_hid_of_goodix *ihid_goodix = container_of(ops, struct i2c_hid_of_goodix, ops); - gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1); regulator_disable(ihid_goodix->vdd); } +static int ihid_goodix_vdd_notify(struct notifier_block *nb, + unsigned long event, + void *ignored) +{ + struct i2c_hid_of_goodix *ihid_goodix = + container_of(nb, struct i2c_hid_of_goodix, nb); + int ret = NOTIFY_OK; + + mutex_lock(&ihid_goodix->regulator_mutex); + + switch (event) { + case REGULATOR_EVENT_PRE_DISABLE: + gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1); + break; + + case REGULATOR_EVENT_ENABLE: + goodix_i2c_hid_deassert_reset(ihid_goodix, true); + break; + + case REGULATOR_EVENT_ABORT_DISABLE: + goodix_i2c_hid_deassert_reset(ihid_goodix, false); + break; + + default: + ret = NOTIFY_DONE; + break; + } + + mutex_unlock(&ihid_goodix->regulator_mutex); + + return ret; +} + static int i2c_hid_of_goodix_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct i2c_hid_of_goodix *ihid_goodix; - + int ret; ihid_goodix = devm_kzalloc(&client->dev, sizeof(*ihid_goodix), GFP_KERNEL); if (!ihid_goodix) return -ENOMEM; + mutex_init(&ihid_goodix->regulator_mutex); + ihid_goodix->ops.power_up = goodix_i2c_hid_power_up; ihid_goodix->ops.power_down = goodix_i2c_hid_power_down; @@ -84,6 +119,37 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client, ihid_goodix->timings = device_get_match_data(&client->dev); + /* + * We need to control the "reset" line in lockstep with the regulator + * actually turning on an off instead of just when we make the request. + * This matters if the regulator is shared with another consumer. + * - If the regulator is off then we must assert reset. The reset + * line is active low and on some boards it could cause a current + * leak if left high. + * - If the regulator is on then we don't want reset asserted for very + * long. Holding the controller in reset apparently draws extra + * power. + */ + mutex_lock(&ihid_goodix->regulator_mutex); + ihid_goodix->nb.notifier_call = ihid_goodix_vdd_notify; + ret = regulator_register_notifier(ihid_goodix->vdd, &ihid_goodix->nb); + if (ret) { + mutex_unlock(&ihid_goodix->regulator_mutex); + return dev_err_probe(&client->dev, ret, + "regulator notifier request failed\n"); + } + + /* + * If someone else is holding the regulator on (or the regulator is + * an always-on one) we might never be told to deassert reset. Do it + * now. Here we'll assume that someone else might have _just + * barely_ turned the regulator on so we'll do the full + * "post_power_delay" just in case. + */ + if (ihid_goodix->reset_gpio && regulator_is_enabled(ihid_goodix->vdd)) + goodix_i2c_hid_deassert_reset(ihid_goodix, true); + mutex_unlock(&ihid_goodix->regulator_mutex); + return i2c_hid_core_probe(client, &ihid_goodix->ops, 0x0001); } From f7744fa16b96da57187dc8e5634152d3b63d72de Mon Sep 17 00:00:00 2001 From: Anirudh Rayabharam Date: Thu, 24 Jun 2021 00:10:29 +0530 Subject: [PATCH 05/31] HID: usbhid: free raw_report buffers in usbhid_stop Free the unsent raw_report buffers when the device is removed. Fixes a memory leak reported by syzbot at: https://syzkaller.appspot.com/bug?id=7b4fa7cb1a7c2d3342a2a8a6c53371c8c418ab47 Reported-by: syzbot+47b26cd837ececfc666d@syzkaller.appspotmail.com Tested-by: syzbot+47b26cd837ececfc666d@syzkaller.appspotmail.com Signed-off-by: Anirudh Rayabharam Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 06130dc431a0..0bf123bf2ef8 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -505,7 +505,7 @@ static void hid_ctrl(struct urb *urb) if (unplug) { usbhid->ctrltail = usbhid->ctrlhead; - } else { + } else if (usbhid->ctrlhead != usbhid->ctrltail) { usbhid->ctrltail = (usbhid->ctrltail + 1) & (HID_CONTROL_FIFO_SIZE - 1); if (usbhid->ctrlhead != usbhid->ctrltail && @@ -1223,9 +1223,20 @@ static void usbhid_stop(struct hid_device *hid) mutex_lock(&usbhid->mutex); clear_bit(HID_STARTED, &usbhid->iofl); + spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */ set_bit(HID_DISCONNECTED, &usbhid->iofl); + while (usbhid->ctrltail != usbhid->ctrlhead) { + if (usbhid->ctrl[usbhid->ctrltail].dir == USB_DIR_OUT) { + kfree(usbhid->ctrl[usbhid->ctrltail].raw_report); + usbhid->ctrl[usbhid->ctrltail].raw_report = NULL; + } + + usbhid->ctrltail = (usbhid->ctrltail + 1) & + (HID_CONTROL_FIFO_SIZE - 1); + } spin_unlock_irq(&usbhid->lock); + usb_kill_urb(usbhid->urbin); usb_kill_urb(usbhid->urbout); usb_kill_urb(usbhid->urbctrl); From dc9dc864f35dd3b87d52fbe9809773576ac77d32 Mon Sep 17 00:00:00 2001 From: Ping Cheng Date: Mon, 19 Jul 2021 13:55:29 -0700 Subject: [PATCH 06/31] HID: wacom: set initial hardware touch switch state to 'off' Wacom touch devices have two types of touch switches: softkey touch toggle and hardware touch switch. For softkey toggle, we assume touch is on by default in the driver. However the hardware touch switch is controlled by end users. We don't know if it's on or off before getting the status event. This patch sets touch off for devices with a hardware switch until we get the status. This is a bit safer for users who leave the switch "off" and don't want any accidental touches. The tradeoff is a slight delay between device connection and touch becoming enabled for users who leave the switch "on". Signed-off-by: Ping Cheng Reviewed-by: Jason Gerecke Tested-by: Jason Gerecke Signed-off-by: Jiri Kosina --- drivers/hid/wacom_sys.c | 8 +++++++- drivers/hid/wacom_wac.c | 12 ++++++++---- drivers/hid/wacom_wac.h | 1 + 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index 57bfa0ae9836..713a2504092f 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -2287,7 +2287,13 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac) if (wacom_wac->has_mute_touch_switch) { wacom_wac->shared->has_mute_touch_switch = true; - wacom_wac->shared->is_touch_on = true; + /* Hardware touch switch may be off. Wait until + * we know the switch state to decide is_touch_on. + * Softkey state should be initialized to "on" to + * match historic default. + */ + if (wacom_wac->is_soft_touch_switch) + wacom_wac->shared->is_touch_on = true; } if (wacom_wac->shared->has_mute_touch_switch && diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 81ba642adcb7..fa2b4222bcad 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -1987,14 +1987,17 @@ static void wacom_wac_pad_usage_mapping(struct hid_device *hdev, features->numbered_buttons++; features->device_type |= WACOM_DEVICETYPE_PAD; break; - case WACOM_HID_WD_TOUCHONOFF: case WACOM_HID_WD_MUTE_DEVICE: + /* softkey touch switch */ + wacom_wac->is_soft_touch_switch = true; + fallthrough; + case WACOM_HID_WD_TOUCHONOFF: /* - * This usage, which is used to mute touch events, comes - * from the pad packet, but is reported on the touch + * These two usages, which are used to mute touch events, come + * from the pad packet, but are reported on the touch * interface. Because the touch interface may not have * been created yet, we cannot call wacom_map_usage(). In - * order to process this usage when we receive it, we set + * order to process the usages when we receive them, we set * the usage type and code directly. */ wacom_wac->has_mute_touch_switch = true; @@ -3835,6 +3838,7 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev, input_dev->evbit[0] |= BIT_MASK(EV_SW); __set_bit(SW_MUTE_DEVICE, input_dev->swbit); wacom_wac->has_mute_touch_switch = true; + wacom_wac->is_soft_touch_switch = true; } fallthrough; diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h index 8f16654eca09..4e9eb0c1eff6 100644 --- a/drivers/hid/wacom_wac.h +++ b/drivers/hid/wacom_wac.h @@ -352,6 +352,7 @@ struct wacom_wac { int mode_value; struct hid_data hid_data; bool has_mute_touch_switch; + bool is_soft_touch_switch; bool has_mode_change; bool is_direct_mode; bool is_invalid_bt_frame; From 5bed0128868ce0e71f243973b3fc7b3bfca902b5 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Mon, 19 Jul 2021 13:55:30 -0700 Subject: [PATCH 07/31] HID: wacom: Short-circuit processing of touch when it is disabled Avoid doing unnecessary work when touch is disabled by detecting this condition and returning early. Note that the probe process sends GET FEATURE requests to discover e.g. HID_DG_CONTACTMAX, so we can't start ignoring touch reports until probe finishes. Signed-off-by: Ping Cheng Signed-off-by: Jason Gerecke Signed-off-by: Jiri Kosina --- drivers/hid/wacom_sys.c | 1 + drivers/hid/wacom_wac.c | 12 ++++++++++++ drivers/hid/wacom_wac.h | 1 + 3 files changed, 14 insertions(+) diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index 713a2504092f..93f49b766376 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -2797,6 +2797,7 @@ static int wacom_probe(struct hid_device *hdev, error); } + wacom_wac->probe_complete = true; return 0; } diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index fa2b4222bcad..ce9e8e9b48b6 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -2584,6 +2584,12 @@ static void wacom_wac_finger_event(struct hid_device *hdev, unsigned equivalent_usage = wacom_equivalent_usage(usage->hid); struct wacom_features *features = &wacom->wacom_wac.features; + /* don't process touch events when touch is off */ + if (wacom_wac->probe_complete && + !wacom_wac->shared->is_touch_on && + !wacom_wac->shared->touch_down) + return; + if (wacom_wac->is_invalid_bt_frame) return; @@ -2633,6 +2639,12 @@ static void wacom_wac_finger_pre_report(struct hid_device *hdev, struct hid_data* hid_data = &wacom_wac->hid_data; int i; + /* don't process touch events when touch is off */ + if (wacom_wac->probe_complete && + !wacom_wac->shared->is_touch_on && + !wacom_wac->shared->touch_down) + return; + wacom_wac->is_invalid_bt_frame = false; for (i = 0; i < report->maxfield; i++) { diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h index 4e9eb0c1eff6..8b2d4e5b2303 100644 --- a/drivers/hid/wacom_wac.h +++ b/drivers/hid/wacom_wac.h @@ -337,6 +337,7 @@ struct wacom_wac { int tool[2]; int id[2]; __u64 serial[2]; + bool probe_complete; bool reporting_data; struct wacom_features features; struct wacom_shared *shared; From ccb51c2e3f0598934c3698f3645a3cb99add201c Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Mon, 19 Jul 2021 13:55:32 -0700 Subject: [PATCH 08/31] HID: wacom: Avoid sending empty sync events Empty sync events clutter up logs and are a waste of CPU cycles. We can avoid sending mt_sync events if touch is disabled or a specific slot is unused. We can avoid sending full sync events if no events were generated. Signed-off-by: Ping Cheng Signed-off-by: Jason Gerecke Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index ce9e8e9b48b6..3f992c9dca4d 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -2551,8 +2551,17 @@ static void wacom_wac_finger_slot(struct wacom_wac *wacom_wac, int slot; slot = input_mt_get_slot_by_key(input, hid_data->id); - if (slot < 0) + if (slot < 0) { return; + } else { + struct input_mt_slot *ps = &input->mt->slots[slot]; + int mt_id = input_mt_get_value(ps, ABS_MT_TRACKING_ID); + + if (!prox && mt_id < 0) { + // No data to send for this slot; short-circuit + return; + } + } input_mt_slot(input, slot); input_mt_report_slot_state(input, MT_TOOL_FINGER, prox); @@ -2696,6 +2705,10 @@ static void wacom_wac_finger_report(struct hid_device *hdev, struct input_dev *input = wacom_wac->touch_input; unsigned touch_max = wacom_wac->features.touch_max; + /* if there was nothing to process, don't send an empty sync */ + if (wacom_wac->hid_data.num_expected == 0) + return; + /* If more packets of data are expected, give us a chance to * process them rather than immediately syncing a partial * update. From 9d339fe4cbd5de150f2d9fd554cc7e7213d09f08 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Mon, 19 Jul 2021 13:55:33 -0700 Subject: [PATCH 09/31] HID: wacom: Refactor touch input mute checks into a common function We perform this same set of tests to see if touch input is muted in several places. We might as well replace these independent copies with an inline function. Signed-off-by: Jason Gerecke Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 3f992c9dca4d..fd51769d0994 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -824,6 +824,13 @@ static int wacom_intuos_inout(struct wacom_wac *wacom) return 0; } +static inline bool touch_is_muted(struct wacom_wac *wacom_wac) +{ + return wacom_wac->probe_complete && + wacom_wac->shared->has_mute_touch_switch && + !wacom_wac->shared->is_touch_on; +} + static inline bool report_touch_events(struct wacom_wac *wacom) { return (touch_arbitration ? !wacom->shared->stylus_in_proximity : 1); @@ -1525,11 +1532,8 @@ static int wacom_24hdt_irq(struct wacom_wac *wacom) int byte_per_packet = WACOM_BYTES_PER_24HDT_PACKET; int y_offset = 2; - if (wacom->shared->has_mute_touch_switch && - !wacom->shared->is_touch_on) { - if (!wacom->shared->touch_down) - return 0; - } + if (touch_is_muted(wacom) && !wacom->shared->touch_down) + return 0; if (wacom->features.type == WACOM_27QHDT) { current_num_contacts = data[63]; @@ -2536,8 +2540,7 @@ static void wacom_wac_finger_slot(struct wacom_wac *wacom_wac, bool prox = hid_data->tipswitch && report_touch_events(wacom_wac); - if (wacom_wac->shared->has_mute_touch_switch && - !wacom_wac->shared->is_touch_on) { + if (touch_is_muted(wacom_wac)) { if (!wacom_wac->shared->touch_down) return; prox = false; @@ -2593,10 +2596,7 @@ static void wacom_wac_finger_event(struct hid_device *hdev, unsigned equivalent_usage = wacom_equivalent_usage(usage->hid); struct wacom_features *features = &wacom->wacom_wac.features; - /* don't process touch events when touch is off */ - if (wacom_wac->probe_complete && - !wacom_wac->shared->is_touch_on && - !wacom_wac->shared->touch_down) + if (touch_is_muted(wacom_wac) && !wacom_wac->shared->touch_down) return; if (wacom_wac->is_invalid_bt_frame) @@ -2648,10 +2648,7 @@ static void wacom_wac_finger_pre_report(struct hid_device *hdev, struct hid_data* hid_data = &wacom_wac->hid_data; int i; - /* don't process touch events when touch is off */ - if (wacom_wac->probe_complete && - !wacom_wac->shared->is_touch_on && - !wacom_wac->shared->touch_down) + if (touch_is_muted(wacom_wac) && !wacom_wac->shared->touch_down) return; wacom_wac->is_invalid_bt_frame = false; From 25ddd7cfc582d6001c3b9e18e96c9b3a58523d66 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Tue, 20 Jul 2021 09:07:49 -0700 Subject: [PATCH 10/31] HID: i2c-hid: goodix: Use the devm variant of regulator_register_notifier() In commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to true state of the regulator") I added a call to regulator_register_notifier() but no call to unregister. That's a bug. Let's use the devm variant to handle the unregistering. Fixes: 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to true state of the regulator") Signed-off-by: Douglas Anderson Signed-off-by: Jiri Kosina --- drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c index 31a4c229fdb7..52674149a275 100644 --- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c +++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c @@ -132,7 +132,7 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client, */ mutex_lock(&ihid_goodix->regulator_mutex); ihid_goodix->nb.notifier_call = ihid_goodix_vdd_notify; - ret = regulator_register_notifier(ihid_goodix->vdd, &ihid_goodix->nb); + ret = devm_regulator_register_notifier(ihid_goodix->vdd, &ihid_goodix->nb); if (ret) { mutex_unlock(&ihid_goodix->regulator_mutex); return dev_err_probe(&client->dev, ret, From bebf8820b355e6ac00487f3f36440d502eb4a44c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Tue, 20 Jul 2021 22:27:08 +0200 Subject: [PATCH 11/31] HID: cmedia: add support for HS-100B mute button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These chips report mute button events in bit 4 of their report without it being part of the report descriptor. Use a custom descriptor that maps this bit. Signed-off-by: Thomas Weißschuh Signed-off-by: Jiri Kosina --- drivers/hid/Kconfig | 5 ++- drivers/hid/hid-cmedia.c | 90 +++++++++++++++++++++++++++++++++++++++- drivers/hid/hid-ids.h | 1 + 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 160554903ef9..6f72ecd79db0 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -259,10 +259,11 @@ config HID_PRODIKEYS and some additional multimedia keys. config HID_CMEDIA - tristate "CMedia CM6533 HID audio jack controls" + tristate "CMedia audio chips" depends on HID help - Support for CMedia CM6533 HID audio jack controls. + Support for CMedia CM6533 HID audio jack controls + and HS100B mute buttons. config HID_CP2112 tristate "Silicon Labs CP2112 HID USB-to-SMBus Bridge support" diff --git a/drivers/hid/hid-cmedia.c b/drivers/hid/hid-cmedia.c index 3296c5050264..cab42047bc99 100644 --- a/drivers/hid/hid-cmedia.c +++ b/drivers/hid/hid-cmedia.c @@ -1,8 +1,10 @@ // SPDX-License-Identifier: GPL-2.0-only /* * HID driver for CMedia CM6533 audio jack controls + * and HS100B mute buttons * * Copyright (C) 2015 Ben Chen + * Copyright (C) 2021 Thomas Weißschuh */ #include @@ -11,13 +13,53 @@ #include "hid-ids.h" MODULE_AUTHOR("Ben Chen"); -MODULE_DESCRIPTION("CM6533 HID jack controls"); +MODULE_AUTHOR("Thomas Weißschuh"); +MODULE_DESCRIPTION("CM6533 HID jack controls and HS100B mute button"); MODULE_LICENSE("GPL"); #define CM6533_JD_TYPE_COUNT 1 #define CM6533_JD_RAWEV_LEN 16 #define CM6533_JD_SFX_OFFSET 8 +#define HS100B_RDESC_ORIG_SIZE 60 + +/* Fixed report descriptor of HS-100B audio chip + * Bit 4 is an abolute Microphone mute usage instead of being unassigned. + */ +static __u8 hs100b_rdesc_fixed[] = { + 0x05, 0x0C, /* Usage Page (Consumer), */ + 0x09, 0x01, /* Usage (Consumer Control), */ + 0xA1, 0x01, /* Collection (Application), */ + 0x15, 0x00, /* Logical Minimum (0), */ + 0x25, 0x01, /* Logical Maximum (1), */ + 0x09, 0xE9, /* Usage (Volume Inc), */ + 0x09, 0xEA, /* Usage (Volume Dec), */ + 0x75, 0x01, /* Report Size (1), */ + 0x95, 0x02, /* Report Count (2), */ + 0x81, 0x02, /* Input (Variable), */ + 0x09, 0xE2, /* Usage (Mute), */ + 0x95, 0x01, /* Report Count (1), */ + 0x81, 0x06, /* Input (Variable, Relative), */ + 0x05, 0x0B, /* Usage Page (Telephony), */ + 0x09, 0x2F, /* Usage (2Fh), */ + 0x81, 0x02, /* Input (Variable), */ + 0x09, 0x20, /* Usage (20h), */ + 0x81, 0x06, /* Input (Variable, Relative), */ + 0x05, 0x0C, /* Usage Page (Consumer), */ + 0x09, 0x00, /* Usage (00h), */ + 0x95, 0x03, /* Report Count (3), */ + 0x81, 0x02, /* Input (Variable), */ + 0x26, 0xFF, 0x00, /* Logical Maximum (255), */ + 0x09, 0x00, /* Usage (00h), */ + 0x75, 0x08, /* Report Size (8), */ + 0x95, 0x03, /* Report Count (3), */ + 0x81, 0x02, /* Input (Variable), */ + 0x09, 0x00, /* Usage (00h), */ + 0x95, 0x04, /* Report Count (4), */ + 0x91, 0x02, /* Output (Variable), */ + 0xC0 /* End Collection */ +}; + /* * *CM6533 audio jack HID raw events: @@ -156,5 +198,49 @@ static struct hid_driver cmhid_driver = { .remove = cmhid_remove, .input_mapping = cmhid_input_mapping, }; -module_hid_driver(cmhid_driver); +static __u8 *cmhid_hs100b_report_fixup(struct hid_device *hid, __u8 *rdesc, + unsigned int *rsize) +{ + if (*rsize == HS100B_RDESC_ORIG_SIZE) { + hid_info(hid, "Fixing CMedia HS-100B report descriptor\n"); + rdesc = hs100b_rdesc_fixed; + *rsize = sizeof(hs100b_rdesc_fixed); + } + return rdesc; +} + +static const struct hid_device_id cmhid_hs100b_devices[] = { + { HID_USB_DEVICE(USB_VENDOR_ID_CMEDIA, USB_DEVICE_ID_CMEDIA_HS100B) }, + { } +}; +MODULE_DEVICE_TABLE(hid, cmhid_hs100b_devices); + +static struct hid_driver cmhid_hs100b_driver = { + .name = "cmedia_hs100b", + .id_table = cmhid_hs100b_devices, + .report_fixup = cmhid_hs100b_report_fixup, +}; + +static int cmedia_init(void) +{ + int ret; + + ret = hid_register_driver(&cmhid_driver); + if (ret) + return ret; + + ret = hid_register_driver(&cmhid_hs100b_driver); + if (ret) + hid_unregister_driver(&cmhid_driver); + + return ret; +} +module_init(cmedia_init); + +static void cmedia_exit(void) +{ + hid_unregister_driver(&cmhid_driver); + hid_unregister_driver(&cmhid_hs100b_driver); +} +module_exit(cmedia_exit); diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 8f1893e68112..6864e4e6ac8b 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -292,6 +292,7 @@ #define USB_VENDOR_ID_CMEDIA 0x0d8c #define USB_DEVICE_ID_CM109 0x000e +#define USB_DEVICE_ID_CMEDIA_HS100B 0x0014 #define USB_DEVICE_ID_CM6533 0x0022 #define USB_VENDOR_ID_CODEMERCS 0x07c0 From 3b41fb4094914903fd8e50a13def9e47763dc101 Mon Sep 17 00:00:00 2001 From: Vincent Lefevre Date: Thu, 22 Jul 2021 03:25:44 +0200 Subject: [PATCH 12/31] HID: apple: Add missing scan code event for keys handled by hid-apple When an EV_KEY event is generated by hid-apple due to special key mapping, the usual associated scan code event (EV_MSC) is missing. This issue can be seen with the evtest utility. Add the scan code event for these special keys. BugLink: https://bugs.debian.org/757356 Co-developed-by: Daniel Lin Signed-off-by: Daniel Lin Signed-off-by: Vincent Lefevre Signed-off-by: Jiri Kosina --- drivers/hid/hid-apple.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index 6b8f0d004d34..cde92de7fca7 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -187,6 +187,15 @@ static const struct apple_key_translation *apple_find_translation( return NULL; } +static void input_event_with_scancode(struct input_dev *input, + __u8 type, __u16 code, unsigned int hid, __s32 value) +{ + if (type == EV_KEY && + (!test_bit(code, input->key)) == value) + input_event(input, EV_MSC, MSC_SCAN, hid); + input_event(input, type, code, value); +} + static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, struct hid_usage *usage, __s32 value) { @@ -199,7 +208,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, if (usage->code == fn_keycode) { asc->fn_on = !!value; - input_event(input, usage->type, KEY_FN, value); + input_event_with_scancode(input, usage->type, KEY_FN, + usage->hid, value); return 1; } @@ -240,7 +250,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, code = do_translate ? trans->to : trans->from; } - input_event(input, usage->type, code, value); + input_event_with_scancode(input, usage->type, code, + usage->hid, value); return 1; } @@ -258,8 +269,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, clear_bit(usage->code, asc->pressed_numlock); - input_event(input, usage->type, trans->to, - value); + input_event_with_scancode(input, usage->type, + trans->to, usage->hid, value); } return 1; @@ -270,7 +281,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, if (hid->country == HID_COUNTRY_INTERNATIONAL_ISO) { trans = apple_find_translation(apple_iso_keyboard, usage->code); if (trans) { - input_event(input, usage->type, trans->to, value); + input_event_with_scancode(input, usage->type, + trans->to, usage->hid, value); return 1; } } @@ -279,7 +291,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, if (swap_opt_cmd) { trans = apple_find_translation(swapped_option_cmd_keys, usage->code); if (trans) { - input_event(input, usage->type, trans->to, value); + input_event_with_scancode(input, usage->type, + trans->to, usage->hid, value); return 1; } } @@ -287,7 +300,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, if (swap_fn_leftctrl) { trans = apple_find_translation(swapped_fn_leftctrl_keys, usage->code); if (trans) { - input_event(input, usage->type, trans->to, value); + input_event_with_scancode(input, usage->type, + trans->to, usage->hid, value); return 1; } } @@ -306,8 +320,8 @@ static int apple_event(struct hid_device *hdev, struct hid_field *field, if ((asc->quirks & APPLE_INVERT_HWHEEL) && usage->code == REL_HWHEEL) { - input_event(field->hidinput->input, usage->type, usage->code, - -value); + input_event_with_scancode(field->hidinput->input, usage->type, + usage->code, usage->hid, -value); return 1; } From 46dcd1cc2b2fa43bd99f39c7c236d5cdb80522f0 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Fri, 23 Jul 2021 17:41:52 +0200 Subject: [PATCH 13/31] HID: logitech-hidpp: Use 'atomic_inc_return' instead of hand-writing it This function logs a warning if the workqueue gets too big. In order to save a few cycles, use 'atomic_inc_return()' instead of an 'atomic_inc()/atomic_read()' sequence. This axes a line of code and saves a 'atomic_read()' call. Signed-off-by: Christophe JAILLET Signed-off-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 61635e629469..a7fa35245c2e 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -2240,11 +2240,10 @@ static int hidpp_ff_queue_work(struct hidpp_ff_private_data *data, int effect_id wd->size = size; memcpy(wd->params, params, size); - atomic_inc(&data->workqueue_size); + s = atomic_inc_return(&data->workqueue_size); queue_work(data->wq, &wd->work); /* warn about excessive queue size */ - s = atomic_read(&data->workqueue_size); if (s >= 20 && s % 20 == 0) hid_warn(data->hidpp->hid_dev, "Force feedback command queue contains %d commands, causing substantial delays!", s); From fbf42729d0e91332e8ce75a1ecce08b8a2dab9c1 Mon Sep 17 00:00:00 2001 From: Salah Triki Date: Thu, 29 Jul 2021 01:45:12 +0100 Subject: [PATCH 14/31] HID: elo: update the reference count of the usb device structure Use usb_get_dev() to increment the reference count of the usb device structure in order to avoid releasing the structure while it is still in use. And use usb_put_dev() to decrement the reference count and thus, when it will be equal to 0 the structure will be released. Signed-off-by: Salah Triki Signed-off-by: Jiri Kosina --- drivers/hid/hid-elo.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-elo.c b/drivers/hid/hid-elo.c index 0d22713a3874..383dfda8c12f 100644 --- a/drivers/hid/hid-elo.c +++ b/drivers/hid/hid-elo.c @@ -228,13 +228,15 @@ static int elo_probe(struct hid_device *hdev, const struct hid_device_id *id) { struct elo_priv *priv; int ret; + struct usb_device *udev; priv = kzalloc(sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; INIT_DELAYED_WORK(&priv->work, elo_work); - priv->usbdev = interface_to_usbdev(to_usb_interface(hdev->dev.parent)); + udev = interface_to_usbdev(to_usb_interface(hdev->dev.parent)); + priv->usbdev = usb_get_dev(udev); hid_set_drvdata(hdev, priv); @@ -265,6 +267,8 @@ static void elo_remove(struct hid_device *hdev) { struct elo_priv *priv = hid_get_drvdata(hdev); + usb_put_dev(priv->usbdev); + hid_hw_stop(hdev); cancel_delayed_work_sync(&priv->work); kfree(priv); From d0f1d5ae23803bd82647a337fa508fa8615defc5 Mon Sep 17 00:00:00 2001 From: Evgeny Novikov Date: Fri, 30 Jul 2021 19:51:08 +0300 Subject: [PATCH 15/31] HID: thrustmaster: Fix memory leaks in probe When thrustmaster_probe() handles errors of usb_submit_urb() it does not free allocated resources and fails. The patch fixes that. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Evgeny Novikov Signed-off-by: Jiri Kosina --- drivers/hid/hid-thrustmaster.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-thrustmaster.c b/drivers/hid/hid-thrustmaster.c index cdc7d82ae9ed..e94d3409fd10 100644 --- a/drivers/hid/hid-thrustmaster.c +++ b/drivers/hid/hid-thrustmaster.c @@ -336,11 +336,14 @@ static int thrustmaster_probe(struct hid_device *hdev, const struct hid_device_i ); ret = usb_submit_urb(tm_wheel->urb, GFP_ATOMIC); - if (ret) + if (ret) { hid_err(hdev, "Error %d while submitting the URB. I am unable to initialize this wheel...\n", ret); + goto error6; + } return ret; +error6: kfree(tm_wheel->change_request); error5: kfree(tm_wheel->response); error4: kfree(tm_wheel->model_request); error3: usb_free_urb(tm_wheel->urb); From df3a97bdbc252d3421f1c5d6d713ad6e4f36a469 Mon Sep 17 00:00:00 2001 From: Evgeny Novikov Date: Fri, 30 Jul 2021 19:51:09 +0300 Subject: [PATCH 16/31] HID: thrustmaster: Fix memory leak in remove thrustmaster_remove() does not release memory for tm_wheel->change_request. This is fixed by the patch. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Evgeny Novikov Signed-off-by: Jiri Kosina --- drivers/hid/hid-thrustmaster.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hid/hid-thrustmaster.c b/drivers/hid/hid-thrustmaster.c index e94d3409fd10..9cb4248f95af 100644 --- a/drivers/hid/hid-thrustmaster.c +++ b/drivers/hid/hid-thrustmaster.c @@ -253,6 +253,7 @@ static void thrustmaster_remove(struct hid_device *hdev) usb_kill_urb(tm_wheel->urb); + kfree(tm_wheel->change_request); kfree(tm_wheel->response); kfree(tm_wheel->model_request); usb_free_urb(tm_wheel->urb); From c3800eed22d21c66912b4461a403b4448ed88d95 Mon Sep 17 00:00:00 2001 From: Evgeny Novikov Date: Fri, 30 Jul 2021 19:53:33 +0300 Subject: [PATCH 17/31] HID: thrustmaster: Fix memory leak in thrustmaster_interrupts() thrustmaster_interrupts() does not free memory for send_buf when usb_interrupt_msg() fails. This is fixed by the given patch. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Evgeny Novikov Signed-off-by: Jiri Kosina --- drivers/hid/hid-thrustmaster.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hid/hid-thrustmaster.c b/drivers/hid/hid-thrustmaster.c index 9cb4248f95af..d44550aa8805 100644 --- a/drivers/hid/hid-thrustmaster.c +++ b/drivers/hid/hid-thrustmaster.c @@ -173,6 +173,7 @@ static void thrustmaster_interrupts(struct hid_device *hdev) if (ret) { hid_err(hdev, "setup data couldn't be sent\n"); + kfree(send_buf); return; } } From b23cdfbddb73bad9d835baabd076ee4bfe1ab2bb Mon Sep 17 00:00:00 2001 From: Hamza Mahfooz Date: Mon, 2 Aug 2021 08:52:32 -0400 Subject: [PATCH 18/31] HID: logitech-hidpp: battery: provide CAPACITY property for newer devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For devices that only support the BATTERY_VOLTAGE (0x1001) feature, UPower requires the additional information provided by this patch, to set them up. Signed-off-by: Hamza Mahfooz Reviewed-by: Filipe Laíns Signed-off-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 44 +++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index a7fa35245c2e..81de88ab2ecc 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -1331,6 +1331,43 @@ static int hidpp20_battery_get_battery_voltage(struct hidpp_device *hidpp, return 0; } +static int hidpp20_map_battery_capacity(struct hid_device *hid_dev, int voltage) +{ + /* NB: This voltage curve doesn't necessarily map perfectly to all + * devices that implement the BATTERY_VOLTAGE feature. This is because + * there are a few devices that use different battery technology. + */ + + static const int voltages[] = { + 4186, 4156, 4143, 4133, 4122, 4113, 4103, 4094, 4086, 4075, + 4067, 4059, 4051, 4043, 4035, 4027, 4019, 4011, 4003, 3997, + 3989, 3983, 3976, 3969, 3961, 3955, 3949, 3942, 3935, 3929, + 3922, 3916, 3909, 3902, 3896, 3890, 3883, 3877, 3870, 3865, + 3859, 3853, 3848, 3842, 3837, 3833, 3828, 3824, 3819, 3815, + 3811, 3808, 3804, 3800, 3797, 3793, 3790, 3787, 3784, 3781, + 3778, 3775, 3772, 3770, 3767, 3764, 3762, 3759, 3757, 3754, + 3751, 3748, 3744, 3741, 3737, 3734, 3730, 3726, 3724, 3720, + 3717, 3714, 3710, 3706, 3702, 3697, 3693, 3688, 3683, 3677, + 3671, 3666, 3662, 3658, 3654, 3646, 3633, 3612, 3579, 3537 + }; + + int i; + + BUILD_BUG_ON(ARRAY_SIZE(voltages) != 100); + + if (unlikely(voltage < 3500 || voltage >= 5000)) + hid_warn_once(hid_dev, + "%s: possibly using the wrong voltage curve\n", + __func__); + + for (i = 0; i < ARRAY_SIZE(voltages); i++) { + if (voltage >= voltages[i]) + return ARRAY_SIZE(voltages) - i; + } + + return 0; +} + static int hidpp20_query_battery_voltage_info(struct hidpp_device *hidpp) { u8 feature_type; @@ -1354,6 +1391,8 @@ static int hidpp20_query_battery_voltage_info(struct hidpp_device *hidpp) hidpp->battery.status = status; hidpp->battery.voltage = voltage; + hidpp->battery.capacity = hidpp20_map_battery_capacity(hidpp->hid_dev, + voltage); hidpp->battery.level = level; hidpp->battery.charge_type = charge_type; hidpp->battery.online = status != POWER_SUPPLY_STATUS_NOT_CHARGING; @@ -1378,6 +1417,8 @@ static int hidpp20_battery_voltage_event(struct hidpp_device *hidpp, if (voltage != hidpp->battery.voltage || status != hidpp->battery.status) { hidpp->battery.voltage = voltage; + hidpp->battery.capacity = hidpp20_map_battery_capacity(hidpp->hid_dev, + voltage); hidpp->battery.status = status; hidpp->battery.level = level; hidpp->battery.charge_type = charge_type; @@ -3716,7 +3757,8 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp) num_battery_props = ARRAY_SIZE(hidpp_battery_props) - 3; if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_MILEAGE || - hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_PERCENTAGE) + hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_PERCENTAGE || + hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE) battery_props[num_battery_props++] = POWER_SUPPLY_PROP_CAPACITY; From 3978f54817559b28535c58a00d3d31bbd5d0b65a Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Mon, 2 Aug 2021 19:33:37 +0530 Subject: [PATCH 19/31] HID: amd_sfh: Fix period data field to enable sensor Existing amd-sfh driver is programming the MP2 firmware period field in units of jiffies, but the MP2 firmware expects in milliseconds unit. Changing it to milliseconds. Fixes: 4b2c53d93a4b ("SFH:Transport Driver to add support of AMD Sensor Fusion Hub (SFH)") Reviewed-by: Shyam Sundar S K Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c index efb849411d25..4710b9aa24a5 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c @@ -184,7 +184,7 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata) rc = -ENOMEM; goto cleanup; } - info.period = msecs_to_jiffies(AMD_SFH_IDLE_LOOP); + info.period = AMD_SFH_IDLE_LOOP; info.sensor_idx = cl_idx; info.dma_address = cl_data->sensor_dma_addr[i]; From 173709f50e98df4c49c2776834605a2f7ed3e681 Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Mon, 2 Aug 2021 19:33:38 +0530 Subject: [PATCH 20/31] HID: amd_sfh: Add command response to check command status Sometimes sensor enable/disable may take time, without checking the actual status bits from MP2 FW can lead the amd-sfh to misbehave. Hence add a status check of enable/disable command by waiting on the command response before sending the next command to FW. Reviewed-by: Shyam Sundar S K Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_client.c | 40 ++++++++++++++++++------ drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 16 ++++++++++ drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 18 +++++++++++ 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c index 4710b9aa24a5..b7b66a1eb971 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c @@ -123,14 +123,24 @@ static void amd_sfh_work_buffer(struct work_struct *work) int i; for (i = 0; i < cli_data->num_hid_devices; i++) { - report_size = get_input_report(i, cli_data->sensor_idx[i], cli_data->report_id[i], - in_data); - hid_input_report(cli_data->hid_sensor_hubs[i], HID_INPUT_REPORT, - in_data->input_report[i], report_size, 0); + if (cli_data->sensor_sts[i] == SENSOR_ENABLED) { + report_size = get_input_report + (i, cli_data->sensor_idx[i], cli_data->report_id[i], in_data); + hid_input_report(cli_data->hid_sensor_hubs[i], HID_INPUT_REPORT, + in_data->input_report[i], report_size, 0); + } } schedule_delayed_work(&cli_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP)); } +u32 amd_sfh_wait_for_response(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts) +{ + if (mp2->mp2_ops->response) + sensor_sts = mp2->mp2_ops->response(mp2, sid, sensor_sts); + + return sensor_sts; +} + int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata) { struct amd_input_data *in_data = &privdata->in_data; @@ -139,8 +149,8 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata) struct device *dev; u32 feature_report_size; u32 input_report_size; + int rc, i, status; u8 cl_idx; - int rc, i; dev = &privdata->pdev->dev; @@ -155,7 +165,7 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata) in_data->sensor_virt_addr[i] = dma_alloc_coherent(dev, sizeof(int) * 8, &cl_data->sensor_dma_addr[i], GFP_KERNEL); - cl_data->sensor_sts[i] = 0; + cl_data->sensor_sts[i] = SENSOR_DISABLED; cl_data->sensor_requested_cnt[i] = 0; cl_data->cur_hid_dev = i; cl_idx = cl_data->sensor_idx[i]; @@ -201,7 +211,10 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata) if (rc) return rc; privdata->mp2_ops->start(privdata, info); - cl_data->sensor_sts[i] = 1; + status = amd_sfh_wait_for_response + (privdata, cl_data->sensor_idx[i], SENSOR_ENABLED); + if (status == SENSOR_ENABLED) + cl_data->sensor_sts[i] = SENSOR_ENABLED; } schedule_delayed_work(&cl_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP)); return 0; @@ -224,10 +237,17 @@ int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata) { struct amdtp_cl_data *cl_data = privdata->cl_data; struct amd_input_data *in_data = cl_data->in_data; - int i; + int i, status; - for (i = 0; i < cl_data->num_hid_devices; i++) - privdata->mp2_ops->stop(privdata, i); + for (i = 0; i < cl_data->num_hid_devices; i++) { + if (cl_data->sensor_sts[i] == SENSOR_ENABLED) { + privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]); + status = amd_sfh_wait_for_response + (privdata, cl_data->sensor_idx[i], SENSOR_DISABLED); + if (status != SENSOR_ENABLED) + cl_data->sensor_sts[i] = SENSOR_DISABLED; + } + } cancel_delayed_work_sync(&cl_data->work); cancel_delayed_work_sync(&cl_data->work_buffer); diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c index 96e2577fa37e..eff61e739ec2 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -31,6 +32,20 @@ static int sensor_mask_override = -1; module_param_named(sensor_mask, sensor_mask_override, int, 0444); MODULE_PARM_DESC(sensor_mask, "override the detected sensors mask"); +static int amd_sfh_wait_response_v2(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts) +{ + union cmd_response cmd_resp; + + /* Get response with status within a max of 800 ms timeout */ + if (!readl_poll_timeout(mp2->mmio + AMD_P2C_MSG(0), cmd_resp.resp, + (cmd_resp.response_v2.response == sensor_sts && + cmd_resp.response_v2.status == 0 && (sid == 0xff || + cmd_resp.response_v2.sensor_id == sid)), 500, 800000)) + return cmd_resp.response_v2.response; + + return SENSOR_DISABLED; +} + static void amd_start_sensor_v2(struct amd_mp2_dev *privdata, struct amd_mp2_sensor_info info) { union sfh_cmd_base cmd_base; @@ -183,6 +198,7 @@ static const struct amd_mp2_ops amd_sfh_ops_v2 = { .start = amd_start_sensor_v2, .stop = amd_stop_sensor_v2, .stop_all = amd_stop_all_sensor_v2, + .response = amd_sfh_wait_response_v2, }; static const struct amd_mp2_ops amd_sfh_ops = { diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h index 2d5c57e3782d..21ef55da712a 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h @@ -24,12 +24,16 @@ #define AMD_C2P_MSG2 0x10508 #define AMD_C2P_MSG(regno) (0x10500 + ((regno) * 4)) +#define AMD_P2C_MSG(regno) (0x10680 + ((regno) * 4)) /* MP2 P2C Message Registers */ #define AMD_P2C_MSG3 0x1068C /* Supported Sensors info */ #define V2_STATUS 0x2 +#define SENSOR_ENABLED 4 +#define SENSOR_DISABLED 5 + #define HPD_IDX 16 /* SFH Command register */ @@ -51,6 +55,19 @@ union sfh_cmd_base { } cmd_v2; }; +union cmd_response { + u32 resp; + struct { + u32 status : 2; + u32 out_in_c2p : 1; + u32 rsvd1 : 1; + u32 response : 4; + u32 sub_cmd : 8; + u32 sensor_id : 6; + u32 rsvd2 : 10; + } response_v2; +}; + union sfh_cmd_param { u32 ul; struct { @@ -117,5 +134,6 @@ struct amd_mp2_ops { void (*start)(struct amd_mp2_dev *privdata, struct amd_mp2_sensor_info info); void (*stop)(struct amd_mp2_dev *privdata, u16 sensor_idx); void (*stop_all)(struct amd_mp2_dev *privdata); + int (*response)(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts); }; #endif From ac15e9196f35d88b4d94bbcf3a5294ebb5622eb0 Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Mon, 2 Aug 2021 19:33:39 +0530 Subject: [PATCH 21/31] HID: amd_sfh: Move hid probe after sensor is enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Earlier platforms don’t have sensor status checking mechanism. Sensors are always enabled without checking sensor status. Hence invoke hid probe only after the sensor is enabled by checking sensor status. Reviewed-by: Shyam Sundar S K Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_client.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c index b7b66a1eb971..4982ccf9dc25 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c @@ -205,16 +205,23 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata) goto cleanup; } rc = get_report_descriptor(cl_idx, cl_data->report_descr[i]); - if (rc) - return rc; - rc = amdtp_hid_probe(cl_data->cur_hid_dev, cl_data); if (rc) return rc; privdata->mp2_ops->start(privdata, info); status = amd_sfh_wait_for_response (privdata, cl_data->sensor_idx[i], SENSOR_ENABLED); - if (status == SENSOR_ENABLED) + if (status == SENSOR_ENABLED) { cl_data->sensor_sts[i] = SENSOR_ENABLED; + rc = amdtp_hid_probe(cl_data->cur_hid_dev, cl_data); + if (rc) { + privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]); + status = amd_sfh_wait_for_response + (privdata, cl_data->sensor_idx[i], SENSOR_DISABLED); + if (status != SENSOR_ENABLED) + cl_data->sensor_sts[i] = SENSOR_DISABLED; + goto cleanup; + } + } } schedule_delayed_work(&cl_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP)); return 0; From 0873d1afacd2167e717ea751fe7274011cb4c26a Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Mon, 2 Aug 2021 19:33:40 +0530 Subject: [PATCH 22/31] HID: amd_sfh: Add support for PM suspend and resume Add support for power management routines. Reviewed-by: Shyam Sundar S K Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_client.c | 1 - drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 49 ++++++++++++++++++++++++ drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 5 +++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c index 4982ccf9dc25..050df796aa2e 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c @@ -17,7 +17,6 @@ #include "amd_sfh_pcie.h" #include "amd_sfh_hid.h" -#define AMD_SFH_IDLE_LOOP 200 struct request_list { struct hid_device *hid; diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c index eff61e739ec2..7eb5971db03e 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c @@ -264,6 +264,54 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i return amd_sfh_hid_client_init(privdata); } +static int __maybe_unused amd_mp2_pci_resume(struct device *dev) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct amd_mp2_dev *mp2 = pci_get_drvdata(pdev); + struct amdtp_cl_data *cl_data = mp2->cl_data; + struct amd_mp2_sensor_info info; + int i, status; + + for (i = 0; i < cl_data->num_hid_devices; i++) { + if (cl_data->sensor_sts[i] == SENSOR_DISABLED) { + info.period = AMD_SFH_IDLE_LOOP; + info.sensor_idx = cl_data->sensor_idx[i]; + info.dma_address = cl_data->sensor_dma_addr[i]; + mp2->mp2_ops->start(mp2, info); + status = amd_sfh_wait_for_response + (mp2, cl_data->sensor_idx[i], SENSOR_ENABLED); + if (status == SENSOR_ENABLED) + cl_data->sensor_sts[i] = SENSOR_ENABLED; + } + } + + return 0; +} + +static int __maybe_unused amd_mp2_pci_suspend(struct device *dev) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct amd_mp2_dev *mp2 = pci_get_drvdata(pdev); + struct amdtp_cl_data *cl_data = mp2->cl_data; + int i, status; + + for (i = 0; i < cl_data->num_hid_devices; i++) { + if (cl_data->sensor_idx[i] != HPD_IDX && + cl_data->sensor_sts[i] == SENSOR_ENABLED) { + mp2->mp2_ops->stop(mp2, cl_data->sensor_idx[i]); + status = amd_sfh_wait_for_response + (mp2, cl_data->sensor_idx[i], SENSOR_DISABLED); + if (status != SENSOR_ENABLED) + cl_data->sensor_sts[i] = SENSOR_DISABLED; + } + } + + return 0; +} + +static SIMPLE_DEV_PM_OPS(amd_mp2_pm_ops, amd_mp2_pci_suspend, + amd_mp2_pci_resume); + static const struct pci_device_id amd_mp2_pci_tbl[] = { { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_MP2) }, { } @@ -274,6 +322,7 @@ static struct pci_driver amd_mp2_pci_driver = { .name = DRIVER_NAME, .id_table = amd_mp2_pci_tbl, .probe = amd_mp2_pci_probe, + .driver.pm = &amd_mp2_pm_ops, }; module_pci_driver(amd_mp2_pci_driver); diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h index 21ef55da712a..1ff6f83cb6fd 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h @@ -36,6 +36,8 @@ #define HPD_IDX 16 +#define AMD_SFH_IDLE_LOOP 200 + /* SFH Command register */ union sfh_cmd_base { u32 ul; @@ -129,6 +131,9 @@ void amd_stop_all_sensors(struct amd_mp2_dev *privdata); int amd_mp2_get_sensor_num(struct amd_mp2_dev *privdata, u8 *sensor_id); int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata); int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata); +u32 amd_sfh_wait_for_response(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts); +void amd_mp2_suspend(struct amd_mp2_dev *mp2); +void amd_mp2_resume(struct amd_mp2_dev *mp2); struct amd_mp2_ops { void (*start)(struct amd_mp2_dev *privdata, struct amd_mp2_sensor_info info); From e665775591865f9f5c3ed6ff35d625a99795e4e0 Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Mon, 2 Aug 2021 19:33:41 +0530 Subject: [PATCH 23/31] HID: amd_sfh: Add dyndbg prints for debugging Add dynamic debug for debugging sensors states during initialization, stop, suspend and resume. Reviewed-by: Shyam Sundar S K Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_client.c | 6 ++++++ drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c index 050df796aa2e..840fd075c56f 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c @@ -218,9 +218,13 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata) (privdata, cl_data->sensor_idx[i], SENSOR_DISABLED); if (status != SENSOR_ENABLED) cl_data->sensor_sts[i] = SENSOR_DISABLED; + dev_dbg(dev, "sid 0x%x status 0x%x\n", + cl_data->sensor_idx[i], cl_data->sensor_sts[i]); goto cleanup; } } + dev_dbg(dev, "sid 0x%x status 0x%x\n", + cl_data->sensor_idx[i], cl_data->sensor_sts[i]); } schedule_delayed_work(&cl_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP)); return 0; @@ -252,6 +256,8 @@ int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata) (privdata, cl_data->sensor_idx[i], SENSOR_DISABLED); if (status != SENSOR_ENABLED) cl_data->sensor_sts[i] = SENSOR_DISABLED; + dev_dbg(&privdata->pdev->dev, "stopping sid 0x%x status 0x%x\n", + cl_data->sensor_idx[i], cl_data->sensor_sts[i]); } } diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c index 7eb5971db03e..8396a9a579f2 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c @@ -282,6 +282,8 @@ static int __maybe_unused amd_mp2_pci_resume(struct device *dev) (mp2, cl_data->sensor_idx[i], SENSOR_ENABLED); if (status == SENSOR_ENABLED) cl_data->sensor_sts[i] = SENSOR_ENABLED; + dev_dbg(dev, "resume sid 0x%x status 0x%x\n", + cl_data->sensor_idx[i], cl_data->sensor_sts[i]); } } @@ -303,6 +305,8 @@ static int __maybe_unused amd_mp2_pci_suspend(struct device *dev) (mp2, cl_data->sensor_idx[i], SENSOR_DISABLED); if (status != SENSOR_ENABLED) cl_data->sensor_sts[i] = SENSOR_DISABLED; + dev_dbg(dev, "suspend sid 0x%x status 0x%x\n", + cl_data->sensor_idx[i], cl_data->sensor_sts[i]); } } From 87c7ee7ad85afafc00f3ee70a61b0b5421584626 Mon Sep 17 00:00:00 2001 From: "Luke D. Jones" Date: Sat, 7 Aug 2021 14:49:21 +1200 Subject: [PATCH 24/31] HID: asus: Prevent Claymore sending suspend event Prevent the ASUS Claymore keyboard from sending a suspend event when the device sleeps itself. The suspend event causes a system suspend if uncaught. Signed off by: Luke D Jones Signed-off-by: Jiri Kosina --- drivers/hid/hid-asus.c | 15 +++++++++++++++ drivers/hid/hid-ids.h | 1 + 2 files changed, 16 insertions(+) diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index fb807c8e989b..f3ecddc519ee 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c @@ -82,6 +82,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad"); #define QUIRK_T90CHI BIT(9) #define QUIRK_MEDION_E1239T BIT(10) #define QUIRK_ROG_NKEY_KEYBOARD BIT(11) +#define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12) #define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \ QUIRK_NO_INIT_REPORTS | \ @@ -366,6 +367,17 @@ static int asus_raw_event(struct hid_device *hdev, } + if (drvdata->quirks & QUIRK_ROG_CLAYMORE_II_KEYBOARD) { + /* + * CLAYMORE II keyboard sends this packet when it goes to sleep + * this causes the whole system to go into suspend. + */ + + if(size == 2 && data[0] == 0x02 && data[1] == 0x00) { + return -1; + } + } + return 0; } @@ -1225,6 +1237,9 @@ static const struct hid_device_id asus_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2), QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD }, + { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, + USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD), + QUIRK_ROG_CLAYMORE_II_KEYBOARD }, { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T100TA_KEYBOARD), QUIRK_T100_KEYBOARD | QUIRK_NO_CONSUMER_USAGES }, diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 8f1893e68112..49efdd4eaac0 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -197,6 +197,7 @@ #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD3 0x1822 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD 0x1866 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2 0x19b6 +#define USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD 0x196b #define USB_DEVICE_ID_ASUSTEK_FX503VD_KEYBOARD 0x1869 #define USB_VENDOR_ID_ATEN 0x0557 From 786537063bbfb3a7ebc6fc21b2baf37fb91df401 Mon Sep 17 00:00:00 2001 From: Jim Broadus Date: Sun, 8 Aug 2021 23:55:05 -0700 Subject: [PATCH 25/31] HID: i2c-hid: Fix Elan touchpad regression A quirk was recently added for Elan devices that has same device match as an entry earlier in the list. The i2c_hid_lookup_quirk function will always return the last match in the list, so the new entry shadows the old entry. The quirk in the previous entry, I2C_HID_QUIRK_BOGUS_IRQ, silenced a flood of messages which have reappeared in the 5.13 kernel. This change moves the two quirk flags into the same entry. Fixes: ca66a6770bd9 (HID: i2c-hid: Skip ELAN power-on command after reset) Signed-off-by: Jim Broadus Signed-off-by: Jiri Kosina --- drivers/hid/i2c-hid/i2c-hid-core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 46474612e73c..517141138b00 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -171,8 +171,6 @@ static const struct i2c_hid_quirks { I2C_HID_QUIRK_NO_IRQ_AFTER_RESET }, { I2C_VENDOR_ID_RAYDIUM, I2C_PRODUCT_ID_RAYDIUM_3118, I2C_HID_QUIRK_NO_IRQ_AFTER_RESET }, - { USB_VENDOR_ID_ELAN, HID_ANY_ID, - I2C_HID_QUIRK_BOGUS_IRQ }, { USB_VENDOR_ID_ALPS_JP, HID_ANY_ID, I2C_HID_QUIRK_RESET_ON_RESUME }, { I2C_VENDOR_ID_SYNAPTICS, I2C_PRODUCT_ID_SYNAPTICS_SYNA2393, @@ -183,7 +181,8 @@ static const struct i2c_hid_quirks { * Sending the wakeup after reset actually break ELAN touchscreen controller */ { USB_VENDOR_ID_ELAN, HID_ANY_ID, - I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET }, + I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET | + I2C_HID_QUIRK_BOGUS_IRQ }, { 0, 0 } }; From 462ba66198a4a8ea996028915af10a698086e302 Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Tue, 10 Aug 2021 12:21:48 +0200 Subject: [PATCH 26/31] HID: thrustmaster: clean up Makefile and adapt quirks Commit c49c33637802 ("HID: support for initialization of some Thrustmaster wheels") messed up the Makefile and quirks during the refactoring of this commit. Luckily, ./scripts/checkkconfigsymbols.py warns on non-existing configs: HID_TMINIT Referencing files: drivers/hid/Makefile, drivers/hid/hid-quirks.c Following the discussion (see Link), CONFIG_HID_THRUSTMASTER is the intended config for CONFIG_HID_TMINIT and the file hid-tminit.c was actually added as hid-thrustmaster.c. So, clean up Makefile and adapt quirks to that refactoring. Fixes: c49c33637802 ("HID: support for initialization of some Thrustmaster wheels") Link: https://lore.kernel.org/linux-input/CAKXUXMx6dByO03f3dX0X5zjvQp0j2AhJBg0vQFDmhZUhtKxRxw@mail.gmail.com/ Signed-off-by: Lukas Bulwahn Signed-off-by: Jiri Kosina --- drivers/hid/Makefile | 1 - drivers/hid/hid-quirks.c | 2 -- 2 files changed, 3 deletions(-) diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index 1ea1a7c0b20f..e29efcb1c040 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -115,7 +115,6 @@ obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o hid-thrustmaster.o -obj-$(CONFIG_HID_TMINIT) += hid-tminit.o obj-$(CONFIG_HID_TIVO) += hid-tivo.o obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 51b39bda9a9d..2e104682c22b 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -662,8 +662,6 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb653) }, { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb654) }, { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb65a) }, -#endif -#if IS_ENABLED(CONFIG_HID_TMINIT) { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb65d) }, #endif #if IS_ENABLED(CONFIG_HID_TIVO) From a4bfe13f96bf8b1b2f233d16e960a3e0680d67fb Mon Sep 17 00:00:00 2001 From: Daniel Nguyen Date: Tue, 10 Aug 2021 10:09:32 -0400 Subject: [PATCH 27/31] HID: sony: support for the ghlive ps4 dongles This commit adds support for the Guitar Hero Live PS4 dongles. These dongles require a "magic" USB control message to be sent every 8 seconds otherwise the dongle will not report events where the strumbar is hit while a fret is being held. Note that the GHL_GUITAR_POKE_INTERVAL is reduced to 8 seconds in order to support PS3, Wii U, and PS4 GHL dongles. Also note that the constant for vendor id 0x1430 has been renamed from Activision to RedOctane as self-declared by the device. Co-developed-by: Pascal Giard Signed-off-by: Pascal Giard Signed-off-by: Daniel Nguyen Signed-off-by: Jiri Kosina --- drivers/hid/Kconfig | 2 +- drivers/hid/hid-ids.h | 7 ++++--- drivers/hid/hid-sony.c | 46 ++++++++++++++++++++++++++++++------------ 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 160554903ef9..6b17bebcd4a9 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -952,7 +952,7 @@ config HID_SONY * Buzz controllers * Sony PS3 Blue-ray Disk Remote Control (Bluetooth) * Logitech Harmony adapter for Sony Playstation 3 (Bluetooth) - * Guitar Hero Live PS3 and Wii U guitar dongles + * Guitar Hero Live PS3, Wii U and PS4 guitar dongles * Guitar Hero PS3 and PC guitar dongles config SONY_FF diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 8f1893e68112..55990018836a 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -41,9 +41,6 @@ #define USB_VENDOR_ID_ACTIONSTAR 0x2101 #define USB_DEVICE_ID_ACTIONSTAR_1011 0x1011 -#define USB_VENDOR_ID_ACTIVISION 0x1430 -#define USB_DEVICE_ID_ACTIVISION_GUITAR_DONGLE 0x474c - #define USB_VENDOR_ID_ADS_TECH 0x06e1 #define USB_DEVICE_ID_ADS_TECH_RADIO_SI470X 0xa155 @@ -1018,6 +1015,10 @@ #define USB_VENDOR_ID_REALTEK 0x0bda #define USB_DEVICE_ID_REALTEK_READER 0x0152 +#define USB_VENDOR_ID_REDOCTANE 0x1430 +#define USB_DEVICE_ID_REDOCTANE_GUITAR_DONGLE 0x474c +#define USB_DEVICE_ID_REDOCTANE_PS4_GHLIVE_DONGLE 0x07bb + #define USB_VENDOR_ID_RETROUSB 0xf000 #define USB_DEVICE_ID_RETROUSB_SNES_RETROPAD 0x0003 #define USB_DEVICE_ID_RETROUSB_SNES_RETROPORT 0x00f1 diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index b3722c51ec78..e48f7720a47c 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -11,8 +11,9 @@ * Copyright (c) 2013 Colin Leitner * Copyright (c) 2014-2016 Frank Praznik * Copyright (c) 2018 Todd Kelner - * Copyright (c) 2020 Pascal Giard + * Copyright (c) 2020-2021 Pascal Giard * Copyright (c) 2020 Sanjay Govind + * Copyright (c) 2021 Daniel Nguyen */ /* @@ -62,6 +63,7 @@ #define SHANWAN_GAMEPAD BIT(16) #define GH_GUITAR_CONTROLLER BIT(17) #define GHL_GUITAR_PS3WIIU BIT(18) +#define GHL_GUITAR_PS4 BIT(19) #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT) #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT) @@ -85,18 +87,27 @@ #define NSG_MRXU_MAX_X 1667 #define NSG_MRXU_MAX_Y 1868 -#define GHL_GUITAR_POKE_INTERVAL 10 /* In seconds */ +/* The PS3/Wii U dongles require a poke every 10 seconds, but the PS4 + * requires one every 8 seconds. Using 8 seconds for all for simplicity. + */ +#define GHL_GUITAR_POKE_INTERVAL 8 /* In seconds */ #define GUITAR_TILT_USAGE 44 -/* Magic value and data taken from GHLtarUtility: +/* Magic data taken from GHLtarUtility: * https://github.com/ghlre/GHLtarUtility/blob/master/PS3Guitar.cs * Note: The Wii U and PS3 dongles happen to share the same! */ -static const u16 ghl_ps3wiiu_magic_value = 0x201; static const char ghl_ps3wiiu_magic_data[] = { 0x02, 0x08, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00 }; +/* Magic data for the PS4 dongles sniffed with a USB protocol + * analyzer. + */ +static const char ghl_ps4_magic_data[] = { + 0x30, 0x02, 0x08, 0x0A, 0x00, 0x00, 0x00, 0x00, 0x00 +}; + /* PS/3 Motion controller */ static u8 motion_rdesc[] = { 0x05, 0x01, /* Usage Page (Desktop), */ @@ -642,14 +653,14 @@ static void ghl_magic_poke(struct timer_list *t) hid_err(sc->hdev, "usb_submit_urb failed: %d", ret); } -static int ghl_init_urb(struct sony_sc *sc, struct usb_device *usbdev) +static int ghl_init_urb(struct sony_sc *sc, struct usb_device *usbdev, + const char ghl_magic_data[], u16 poke_size) { struct usb_ctrlrequest *cr; - u16 poke_size; u8 *databuf; unsigned int pipe; + u16 ghl_magic_value = (((HID_OUTPUT_REPORT + 1) << 8) | ghl_magic_data[0]); - poke_size = ARRAY_SIZE(ghl_ps3wiiu_magic_data); pipe = usb_sndctrlpipe(usbdev, 0); cr = devm_kzalloc(&sc->hdev->dev, sizeof(*cr), GFP_ATOMIC); @@ -663,10 +674,10 @@ static int ghl_init_urb(struct sony_sc *sc, struct usb_device *usbdev) cr->bRequestType = USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT; cr->bRequest = USB_REQ_SET_CONFIGURATION; - cr->wValue = cpu_to_le16(ghl_ps3wiiu_magic_value); + cr->wValue = cpu_to_le16(ghl_magic_value); cr->wIndex = 0; cr->wLength = cpu_to_le16(poke_size); - memcpy(databuf, ghl_ps3wiiu_magic_data, poke_size); + memcpy(databuf, ghl_magic_data, poke_size); usb_fill_control_urb( sc->ghl_urb, usbdev, pipe, (unsigned char *) cr, databuf, poke_size, @@ -3030,11 +3041,17 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id) return -ENODEV; } - if (sc->quirks & GHL_GUITAR_PS3WIIU) { + if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) { sc->ghl_urb = usb_alloc_urb(0, GFP_ATOMIC); if (!sc->ghl_urb) return -ENOMEM; - ret = ghl_init_urb(sc, usbdev); + + if (sc->quirks & GHL_GUITAR_PS3WIIU) + ret = ghl_init_urb(sc, usbdev, ghl_ps3wiiu_magic_data, + ARRAY_SIZE(ghl_ps3wiiu_magic_data)); + else if (sc->quirks & GHL_GUITAR_PS4) + ret = ghl_init_urb(sc, usbdev, ghl_ps4_magic_data, + ARRAY_SIZE(ghl_ps4_magic_data)); if (ret) { hid_err(hdev, "error preparing URB\n"); return ret; @@ -3052,7 +3069,7 @@ static void sony_remove(struct hid_device *hdev) { struct sony_sc *sc = hid_get_drvdata(hdev); - if (sc->quirks & GHL_GUITAR_PS3WIIU) { + if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) { del_timer_sync(&sc->ghl_poke_timer); usb_free_urb(sc->ghl_urb); } @@ -3172,11 +3189,14 @@ static const struct hid_device_id sony_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3WIIU_GHLIVE_DONGLE), .driver_data = GHL_GUITAR_PS3WIIU | GH_GUITAR_CONTROLLER }, /* Guitar Hero PC Guitar Dongle */ - { HID_USB_DEVICE(USB_VENDOR_ID_ACTIVISION, USB_DEVICE_ID_ACTIVISION_GUITAR_DONGLE), + { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_GUITAR_DONGLE), .driver_data = GH_GUITAR_CONTROLLER }, /* Guitar Hero PS3 World Tour Guitar Dongle */ { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE), .driver_data = GH_GUITAR_CONTROLLER }, + /* Guitar Hero Live PS4 guitar dongles */ + { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_PS4_GHLIVE_DONGLE), + .driver_data = GHL_GUITAR_PS4 | GH_GUITAR_CONTROLLER }, { } }; MODULE_DEVICE_TABLE(hid, sony_devices); From bab94e97323baefe0afccad66e776f9c78b4f521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulrich=20Sp=C3=B6rlein?= Date: Sat, 14 Aug 2021 17:52:14 +0200 Subject: [PATCH 28/31] HID: sony: Fix more ShanWan clone gamepads to not rumble when plugged in. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The device string on these can differ, apparently, including typos. I've bought 2 of these in 2012 and googling shows many folks out there with that broken spelling in their dmesg. Signed-off-by: Ulrich Spörlein Signed-off-by: Jiri Kosina --- drivers/hid/hid-sony.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index b3722c51ec78..a2fef59063a6 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -2974,7 +2974,8 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id) if (!strcmp(hdev->name, "FutureMax Dance Mat")) quirks |= FUTUREMAX_DANCE_MAT; - if (!strcmp(hdev->name, "SHANWAN PS3 GamePad")) + if (!strcmp(hdev->name, "SHANWAN PS3 GamePad") || + !strcmp(hdev->name, "ShanWan PS(R) Ga`epad")) quirks |= SHANWAN_GAMEPAD; sc = devm_kzalloc(&hdev->dev, sizeof(*sc), GFP_KERNEL); From 5049307d37a760e304ad191c5dc7c6851266d2f8 Mon Sep 17 00:00:00 2001 From: Michal Kubecek Date: Wed, 1 Sep 2021 12:35:49 -0400 Subject: [PATCH 29/31] HID: usbhid: Fix flood of "control queue full" messages [patch description by Alan Stern] Commit 7652dd2c5cb7 ("USB: core: Check buffer length matches wLength for control transfers") causes control URB submissions to fail if the transfer_buffer_length value disagrees with the setup packet's wLength valuel. Unfortunately, it turns out that the usbhid can trigger this failure mode when it submits a control request for an input report: It pads the transfer buffer size to a multiple of the maxpacket value but does not increase wLength correspondingly. These failures have caused problems for people using an APS UPC, in the form of a flood of log messages resembling: hid-generic 0003:051D:0002.0002: control queue full This patch fixes the problem by setting the wLength value equal to the padded transfer_buffer_length value in hid_submit_ctrl(). As a nice bonus, the code which stores the transfer_buffer_length value is now shared between the two branches of an "if" statement, so it can be de-duplicated. Signed-off-by: Michal Kubecek Signed-off-by: Alan Stern Fixes: 7652dd2c5cb7 ("USB: core: Check buffer length matches wLength for control transfers") Tested-by: Oleksandr Natalenko Tested-by: Benjamin Tissoires Acked-by: Benjamin Tissoires Cc: stable@vger.kernel.org Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 0bf123bf2ef8..6b8690878435 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -377,27 +377,26 @@ static int hid_submit_ctrl(struct hid_device *hid) len = hid_report_len(report); if (dir == USB_DIR_OUT) { usbhid->urbctrl->pipe = usb_sndctrlpipe(hid_to_usb_dev(hid), 0); - usbhid->urbctrl->transfer_buffer_length = len; if (raw_report) { memcpy(usbhid->ctrlbuf, raw_report, len); kfree(raw_report); usbhid->ctrl[usbhid->ctrltail].raw_report = NULL; } } else { - int maxpacket, padlen; + int maxpacket; usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid), 0); maxpacket = usb_maxpacket(hid_to_usb_dev(hid), usbhid->urbctrl->pipe, 0); if (maxpacket > 0) { - padlen = DIV_ROUND_UP(len, maxpacket); - padlen *= maxpacket; - if (padlen > usbhid->bufsize) - padlen = usbhid->bufsize; + len = DIV_ROUND_UP(len, maxpacket); + len *= maxpacket; + if (len > usbhid->bufsize) + len = usbhid->bufsize; } else - padlen = 0; - usbhid->urbctrl->transfer_buffer_length = padlen; + len = 0; } + usbhid->urbctrl->transfer_buffer_length = len; usbhid->urbctrl->dev = hid_to_usb_dev(hid); usbhid->cr->bRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | dir; From 0a824efdb724e07574bafcd2c2486b2a3de35ff6 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 1 Sep 2021 12:36:00 -0400 Subject: [PATCH 30/31] HID: usbhid: Fix warning caused by 0-length input reports Syzbot found a warning caused by hid_submit_ctrl() submitting a control request to transfer a 0-length input report: usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1 (The warning message is a little difficult to understand. It means that the control request claims to be for an IN transfer but this contradicts the USB spec, which requires 0-length control transfers always to be in the OUT direction.) Now, a zero-length report isn't good for anything and there's no reason for a device to have one, but the fuzzer likes to pick out these weird edge cases. In the future, perhaps we will decide to reject 0-length reports at probe time. For now, the simplest approach for avoiding these warnings is to pretend that the report actually has length 1. Signed-off-by: Alan Stern Reported-and-tested-by: syzbot+9b57a46bf1801ce2a2ca@syzkaller.appspotmail.com Tested-by: Oleksandr Natalenko Tested-by: Benjamin Tissoires Acked-by: Benjamin Tissoires Cc: stable@vger.kernel.org Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 6b8690878435..c56cb03c1551 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -389,6 +389,7 @@ static int hid_submit_ctrl(struct hid_device *hid) maxpacket = usb_maxpacket(hid_to_usb_dev(hid), usbhid->urbctrl->pipe, 0); if (maxpacket > 0) { + len += (len == 0); /* Don't allow 0-length reports */ len = DIV_ROUND_UP(len, maxpacket); len *= maxpacket; if (len > usbhid->bufsize) From d2f311ec91984adb219ac6985d4dd72c37ae734d Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 1 Sep 2021 12:36:06 -0400 Subject: [PATCH 31/31] HID: usbhid: Simplify code in hid_submit_ctrl() This patch makes a small simplification to the code in hid_submit_ctrl(). The test for maxpacket being > 0 is unnecessary, because endpoint 0 always has a maxpacket value which is >= 8. Furthermore, endpoint 0's maxpacket value is always a power of 2, so instead of open-coding the round-to-next-multiple computation we can call the optimized round_up() routine. Signed-off-by: Alan Stern Tested-by: Benjamin Tissoires Acked-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index c56cb03c1551..2dcaf31eb9cd 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -388,14 +388,10 @@ static int hid_submit_ctrl(struct hid_device *hid) usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid), 0); maxpacket = usb_maxpacket(hid_to_usb_dev(hid), usbhid->urbctrl->pipe, 0); - if (maxpacket > 0) { - len += (len == 0); /* Don't allow 0-length reports */ - len = DIV_ROUND_UP(len, maxpacket); - len *= maxpacket; - if (len > usbhid->bufsize) - len = usbhid->bufsize; - } else - len = 0; + len += (len == 0); /* Don't allow 0-length reports */ + len = round_up(len, maxpacket); + if (len > usbhid->bufsize) + len = usbhid->bufsize; } usbhid->urbctrl->transfer_buffer_length = len; usbhid->urbctrl->dev = hid_to_usb_dev(hid);