From 0d30dae38fe01cd1de358c6039a0b1184689fe51 Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 10 Oct 2025 13:52:54 +0800 Subject: HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking During suspend/resume tests with S2IDLE, some ISH functional failures were observed because of delay in executing ISH resume handler. Here schedule_work() is used from resume handler to do actual work. schedule_work() uses system_wq, which is a per CPU work queue. Although the queuing is not bound to a CPU, but it prefers local CPU of the caller, unless prohibited. Users of this work queue are not supposed to queue long running work. But in practice, there are scenarios where long running work items are queued on other unbound workqueues, occupying the CPU. As a result, the ISH resume handler may not get a chance to execute in a timely manner. In one scenario, one of the ish_resume_handler() executions was delayed nearly 1 second because another work item on an unbound workqueue occupied the same CPU. This delay causes ISH functionality failures. A similar issue was previously observed where the ISH HID driver timed out while getting the HID descriptor during S4 resume in the recovery kernel, likely caused by the same workqueue contention problem. Create dedicated unbound workqueues for all ISH operations to allow work items to execute on any available CPU, eliminating CPU-specific bottlenecks and improving resume reliability under varying system loads. Also ISH has three different components, a bus driver which implements ISH protocols, a PCI interface layer and HID interface. Use one dedicated work queue for all of them. Signed-off-by: Zhang Lixu Signed-off-by: Jiri Kosina --- include/linux/intel-ish-client-if.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include/linux') diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h index dfbf7d9d7bb5..b235fd84f478 100644 --- a/include/linux/intel-ish-client-if.h +++ b/include/linux/intel-ish-client-if.h @@ -87,6 +87,8 @@ bool ishtp_wait_resume(struct ishtp_device *dev); ishtp_print_log ishtp_trace_callback(struct ishtp_cl_device *cl_device); /* Get device pointer of PCI device for DMA acces */ struct device *ishtp_get_pci_device(struct ishtp_cl_device *cl_device); +/* Get the ISHTP workqueue */ +struct workqueue_struct *ishtp_get_workqueue(struct ishtp_cl_device *cl_device); struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device); void ishtp_cl_free(struct ishtp_cl *cl); -- cgit v1.2.3 From 011aa2aa2c4c2b3356c32f195f306df6e177ac38 Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 17 Oct 2025 10:22:13 +0800 Subject: HID: intel-ish-hid: Add ishtp_get_connection_state() interface Add the ishtp_get_connection_state() function for struct ishtp_cl, allowing ishtp client drivers to retrieve the current connection state. Signed-off-by: Zhang Lixu Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ishtp/client.c | 6 ++++++ include/linux/intel-ish-client-if.h | 1 + 2 files changed, 7 insertions(+) (limited to 'include/linux') diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c index 21a2c0773cc2..40f510b1c072 100644 --- a/drivers/hid/intel-ish-hid/ishtp/client.c +++ b/drivers/hid/intel-ish-hid/ishtp/client.c @@ -1261,6 +1261,12 @@ void ishtp_set_connection_state(struct ishtp_cl *cl, int state) } EXPORT_SYMBOL(ishtp_set_connection_state); +int ishtp_get_connection_state(struct ishtp_cl *cl) +{ + return cl->state; +} +EXPORT_SYMBOL(ishtp_get_connection_state); + void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id) { cl->fw_client_id = fw_client_id; diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h index b235fd84f478..2cd4f65aaa37 100644 --- a/include/linux/intel-ish-client-if.h +++ b/include/linux/intel-ish-client-if.h @@ -109,6 +109,7 @@ struct ishtp_device *ishtp_get_ishtp_device(struct ishtp_cl *cl); void ishtp_set_tx_ring_size(struct ishtp_cl *cl, int size); void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size); void ishtp_set_connection_state(struct ishtp_cl *cl, int state); +int ishtp_get_connection_state(struct ishtp_cl *cl); void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id); void ishtp_put_device(struct ishtp_cl_device *cl_dev); -- cgit v1.2.3 From d6f4941f1b4f3e701e422dfbfee024264294f91f Mon Sep 17 00:00:00 2001 From: Benedek Kupper Date: Tue, 7 Oct 2025 22:35:44 +0200 Subject: drivers: hid: renegotiate resolution multipliers with device after reset The scroll resolution multipliers are set in the context of hidinput_connect(), which is only called at probe time: when the host changes the value on the device with a SET_REPORT(FEATURE), and the device accepts it, these multipliers are stored on the host side, and used to calculate the final scroll event values sent to userspace. After a USB suspend, the resume operation on many hubs and chipsets involve a USB reset signal as well. A reset on the device side clears all previous state information, including the value of the multiplier report. This reset is not handled by the multiplier handling logic, so what ends up happening is the host is still expecting high-resolution scroll events, but the device is reset to default resolution, making the effective, user-perceived scroll speed incredibly slow. The solution is to renegotiate the multiplier selection after each reset. This is not the only bug related to the high-resolution scrolling implementation in the kernel (the other one is https://bugzilla.kernel.org/show_bug.cgi?id=220144), but for this one, there is no device side workaround for, leading to poor user experience with our product: https://github.com/UltimateHackingKeyboard/firmware/issues/1155 https://github.com/UltimateHackingKeyboard/firmware/issues/1261 https://github.com/UltimateHackingKeyboard/firmware/pull/1355 This patch was tested by an affected user and has been reported to fix the issue (see discussion in 1355). Signed-off-by: Benedek Kupper Signed-off-by: Jiri Kosina --- drivers/hid/hid-generic.c | 9 +++++++++ drivers/hid/hid-input.c | 7 +++++++ include/linux/hid.h | 1 + 3 files changed, 17 insertions(+) (limited to 'include/linux') diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c index 9e04c6d0fcc8..c2de916747de 100644 --- a/drivers/hid/hid-generic.c +++ b/drivers/hid/hid-generic.c @@ -70,6 +70,14 @@ static int hid_generic_probe(struct hid_device *hdev, return hid_hw_start(hdev, HID_CONNECT_DEFAULT); } +static int hid_generic_reset_resume(struct hid_device *hdev) +{ + if (hdev->claimed & HID_CLAIMED_INPUT) + hidinput_reset_resume(hdev); + + return 0; +} + static const struct hid_device_id hid_table[] = { { HID_DEVICE(HID_BUS_ANY, HID_GROUP_ANY, HID_ANY_ID, HID_ANY_ID) }, { } @@ -81,6 +89,7 @@ static struct hid_driver hid_generic = { .id_table = hid_table, .match = hid_generic_match, .probe = hid_generic_probe, + .reset_resume = hid_generic_reset_resume, }; module_hid_driver(hid_generic); diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 2bbb645c2ff4..9f899ee83f0b 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -2400,6 +2400,13 @@ void hidinput_disconnect(struct hid_device *hid) } EXPORT_SYMBOL_GPL(hidinput_disconnect); +void hidinput_reset_resume(struct hid_device *hid) +{ + /* renegotiate host-device shared state after reset */ + hidinput_change_resolution_multipliers(hid); +} +EXPORT_SYMBOL_GPL(hidinput_reset_resume); + #ifdef CONFIG_HID_KUNIT_TEST #include "hid-input-test.c" #endif diff --git a/include/linux/hid.h b/include/linux/hid.h index a4ddb94e3ee5..dce862cafbbd 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -984,6 +984,7 @@ extern void hidinput_hid_event(struct hid_device *, struct hid_field *, struct h extern void hidinput_report_event(struct hid_device *hid, struct hid_report *report); extern int hidinput_connect(struct hid_device *hid, unsigned int force); extern void hidinput_disconnect(struct hid_device *); +void hidinput_reset_resume(struct hid_device *hid); struct hid_field *hid_find_field(struct hid_device *hdev, unsigned int report_type, unsigned int application, unsigned int usage); -- cgit v1.2.3