From 0d30dae38fe01cd1de358c6039a0b1184689fe51 Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 10 Oct 2025 13:52:54 +0800 Subject: [PATCH 1/8] 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 --- drivers/hid/intel-ish-hid/ipc/ipc.c | 21 +++++++++++++++++++- drivers/hid/intel-ish-hid/ipc/pci-ish.c | 2 +- drivers/hid/intel-ish-hid/ishtp-hid-client.c | 4 ++-- drivers/hid/intel-ish-hid/ishtp/bus.c | 18 ++++++++++++++++- drivers/hid/intel-ish-hid/ishtp/hbm.c | 4 ++-- drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 3 +++ include/linux/intel-ish-client-if.h | 2 ++ 7 files changed, 47 insertions(+), 7 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c index 3ddaa2cd39d5..9958f2968c4f 100644 --- a/drivers/hid/intel-ish-hid/ipc/ipc.c +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c @@ -628,7 +628,7 @@ static void recv_ipc(struct ishtp_device *dev, uint32_t doorbell_val) if (!ishtp_dev) { ishtp_dev = dev; } - schedule_work(&fw_reset_work); + queue_work(dev->unbound_wq, &fw_reset_work); break; case MNG_RESET_NOTIFY_ACK: @@ -933,6 +933,21 @@ static const struct ishtp_hw_ops ish_hw_ops = { .dma_no_cache_snooping = _dma_no_cache_snooping }; +static struct workqueue_struct *devm_ishtp_alloc_workqueue(struct device *dev) +{ + struct workqueue_struct *wq; + + wq = alloc_workqueue("ishtp_unbound_%d", WQ_UNBOUND, 0, dev->id); + if (!wq) + return NULL; + + if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue, + wq)) + return NULL; + + return wq; +} + /** * ish_dev_init() -Initialize ISH devoce * @pdev: PCI device @@ -953,6 +968,10 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev) if (!dev) return NULL; + dev->unbound_wq = devm_ishtp_alloc_workqueue(&pdev->dev); + if (!dev->unbound_wq) + return NULL; + dev->devc = &pdev->dev; ishtp_device_init(dev); diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c index 9d150ce234f2..b748ac6fbfdc 100644 --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c @@ -384,7 +384,7 @@ static int __maybe_unused ish_resume(struct device *device) ish_resume_device = device; dev->resume_flag = 1; - schedule_work(&resume_work); + queue_work(dev->unbound_wq, &resume_work); return 0; } diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c index d8c3c54a8c0f..f61add862b6b 100644 --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c @@ -860,7 +860,7 @@ static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device) hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__, hid_ishtp_cl); - schedule_work(&client_data->work); + queue_work(ishtp_get_workqueue(cl_device), &client_data->work); return 0; } @@ -902,7 +902,7 @@ static int hid_ishtp_cl_resume(struct device *device) hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__, hid_ishtp_cl); - schedule_work(&client_data->resume_work); + queue_work(ishtp_get_workqueue(cl_device), &client_data->resume_work); return 0; } diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c index 93a0432e7058..c6ce37244e49 100644 --- a/drivers/hid/intel-ish-hid/ishtp/bus.c +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c @@ -541,7 +541,7 @@ void ishtp_cl_bus_rx_event(struct ishtp_cl_device *device) return; if (device->event_cb) - schedule_work(&device->event_work); + queue_work(device->ishtp_dev->unbound_wq, &device->event_work); } /** @@ -876,6 +876,22 @@ struct device *ishtp_get_pci_device(struct ishtp_cl_device *device) } EXPORT_SYMBOL(ishtp_get_pci_device); +/** + * ishtp_get_workqueue - Retrieve the workqueue associated with an ISHTP device + * @cl_device: Pointer to the ISHTP client device structure + * + * Returns the workqueue_struct pointer (unbound_wq) associated with the given + * ISHTP client device. This workqueue is typically used for scheduling work + * related to the device. + * + * Return: Pointer to struct workqueue_struct. + */ +struct workqueue_struct *ishtp_get_workqueue(struct ishtp_cl_device *cl_device) +{ + return cl_device->ishtp_dev->unbound_wq; +} +EXPORT_SYMBOL(ishtp_get_workqueue); + /** * ishtp_trace_callback() - Return trace callback * @cl_device: ISH-TP client device instance diff --git a/drivers/hid/intel-ish-hid/ishtp/hbm.c b/drivers/hid/intel-ish-hid/ishtp/hbm.c index 8ee5467127d8..97c4fcd9e3c6 100644 --- a/drivers/hid/intel-ish-hid/ishtp/hbm.c +++ b/drivers/hid/intel-ish-hid/ishtp/hbm.c @@ -573,7 +573,7 @@ void ishtp_hbm_dispatch(struct ishtp_device *dev, /* Start firmware loading process if it has loader capability */ if (version_res->host_version_supported & ISHTP_SUPPORT_CAP_LOADER) - schedule_work(&dev->work_fw_loader); + queue_work(dev->unbound_wq, &dev->work_fw_loader); dev->version.major_version = HBM_MAJOR_VERSION; dev->version.minor_version = HBM_MINOR_VERSION; @@ -864,7 +864,7 @@ void recv_hbm(struct ishtp_device *dev, struct ishtp_msg_hdr *ishtp_hdr) dev->rd_msg_fifo_tail = (dev->rd_msg_fifo_tail + IPC_PAYLOAD_SIZE) % (RD_INT_FIFO_SIZE * IPC_PAYLOAD_SIZE); spin_unlock_irqrestore(&dev->rd_msg_spinlock, flags); - schedule_work(&dev->bh_hbm_work); + queue_work(dev->unbound_wq, &dev->bh_hbm_work); eoi: return; } diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h index 23db97ecf21c..4b0596eadf1c 100644 --- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h +++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h @@ -175,6 +175,9 @@ struct ishtp_device { struct hbm_version version; int transfer_path; /* Choice of transfer path: IPC or DMA */ + /* Alloc a dedicated unbound workqueue for ishtp device */ + struct workqueue_struct *unbound_wq; + /* work structure for scheduling firmware loading tasks */ struct work_struct work_fw_loader; /* waitq for waiting for command response from the firmware loader */ 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); From 011aa2aa2c4c2b3356c32f195f306df6e177ac38 Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 17 Oct 2025 10:22:13 +0800 Subject: [PATCH 2/8] 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(+) 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); From 3cbf6544b0af61e8f9201f2c4c82fdaf2b5f3dd3 Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 17 Oct 2025 10:22:14 +0800 Subject: [PATCH 3/8] HID: intel-ishtp-hid: Clear suspended flag only after connected on resume When resuming from suspend-to-RAM or hibernate, the ISH firmware is powered on from D3, causing all previous client connections between the firmware and driver to be lost. Although the underlying ishtp bus driver initiates a client reconnection flow, this process is asynchronous. As a result, when hid_ishtp_cl_resume_handler() is executed, the connection may not have been re-established yet. Clearing the suspended flag prematurely in this scenario can lead to a timeout when the upper-layer HID sensor driver set feature during resume. To prevent such timeouts, only clear the suspended flag after confirming that the connection state is ISHTP_CL_CONNECTED. Signed-off-by: Zhang Lixu Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ishtp-hid-client.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c index f61add862b6b..f37b3bc2bb7d 100644 --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c @@ -757,8 +757,15 @@ static void hid_ishtp_cl_resume_handler(struct work_struct *work) struct ishtp_cl *hid_ishtp_cl = client_data->hid_ishtp_cl; if (ishtp_wait_resume(ishtp_get_ishtp_device(hid_ishtp_cl))) { - client_data->suspended = false; - wake_up_interruptible(&client_data->ishtp_resume_wait); + /* + * Clear the suspended flag only when the connection is established. + * If the connection is not established, the suspended flag will be cleared after + * the connection is made. + */ + if (ishtp_get_connection_state(hid_ishtp_cl) == ISHTP_CL_CONNECTED) { + client_data->suspended = false; + wake_up_interruptible(&client_data->ishtp_resume_wait); + } } else { hid_ishtp_trace(client_data, "hid client: wait for resume timed out"); dev_err(cl_data_to_dev(client_data), "wait for resume timed out"); From bd1b9a8df598882c69403ee83ba2903b45f9d607 Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 17 Oct 2025 10:22:15 +0800 Subject: [PATCH 4/8] HID: intel-ish-ipc: Reset clients state on resume from D3 When ISH resumes from D3, the connection between ishtp clients and firmware is lost. The ish_resume() function schedules resume_work asynchronously to re-initiate the connection and then returns immediately. This can cause a race where the upper-layer ishtp client driver's .resume() may execute before the connection is fully restored, leaving the client in a stale connected state. If the client sends messages during this window, the firmware cannot respond. To avoid this, reset the ishtp clients' state before returning from ish_resume() if ISH is resuming from D3. Signed-off-by: Zhang Lixu Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ipc/pci-ish.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c index b748ac6fbfdc..e4499c83c62e 100644 --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c @@ -147,6 +147,12 @@ static inline bool ish_should_enter_d0i3(struct pci_dev *pdev) static inline bool ish_should_leave_d0i3(struct pci_dev *pdev) { + struct ishtp_device *dev = pci_get_drvdata(pdev); + u32 fwsts = dev->ops->get_fw_status(dev); + + if (dev->suspend_flag || !IPC_IS_ISH_ILUP(fwsts)) + return false; + return !pm_resume_via_firmware() || pdev->device == PCI_DEVICE_ID_INTEL_ISH_CHV; } @@ -277,10 +283,8 @@ static void __maybe_unused ish_resume_handler(struct work_struct *work) { struct pci_dev *pdev = to_pci_dev(ish_resume_device); struct ishtp_device *dev = pci_get_drvdata(pdev); - uint32_t fwsts = dev->ops->get_fw_status(dev); - if (ish_should_leave_d0i3(pdev) && !dev->suspend_flag - && IPC_IS_ISH_ILUP(fwsts)) { + if (ish_should_leave_d0i3(pdev)) { if (device_may_wakeup(&pdev->dev)) disable_irq_wake(pdev->irq); @@ -384,6 +388,10 @@ static int __maybe_unused ish_resume(struct device *device) ish_resume_device = device; dev->resume_flag = 1; + /* If ISH resume from D3, reset ishtp clients before return */ + if (!ish_should_leave_d0i3(pdev)) + ishtp_reset_handler(dev); + queue_work(dev->unbound_wq, &resume_work); return 0; From 9e097dc9df8027a590dbca503d8b52e1a86024d7 Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 17 Oct 2025 10:22:16 +0800 Subject: [PATCH 5/8] HID: intel-ish-hid: ipc: Always schedule FW reset work on RESET_NOTIFY/ACK Both ISH firmware and driver can actively send MNG_RESET_NOTIFY to initiate an FW reset handshake. Upon receiving this, the peer should reply with MNG_RESET_NOTIFY_ACK. Therefore, the driver should schedule the FW reset handshake work function when receiving either MNG_RESET_NOTIFY or MNG_RESET_NOTIFY_ACK. Previously, driver only scheduled the work function on MNG_RESET_NOTIFY. This patch ensures the work function is scheduled on both messages, but only replies with MNG_RESET_NOTIFY_ACK when receiving MNG_RESET_NOTIFY. Signed-off-by: Zhang Lixu Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ipc/ipc.c | 39 ++++++++++++++--------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c index 9958f2968c4f..01a139e17cb5 100644 --- a/drivers/hid/intel-ish-hid/ipc/ipc.c +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c @@ -481,6 +481,20 @@ static int timed_wait_for_timeout(struct ishtp_device *dev, int condition, return ret; } +static void ish_send_reset_notify_ack(struct ishtp_device *dev) +{ + /* Read reset ID */ + u32 reset_id = ish_reg_read(dev, IPC_REG_ISH2HOST_MSG) & 0xFFFF; + + /* + * Set HOST2ISH.ILUP. Apparently we need this BEFORE sending + * RESET_NOTIFY_ACK - FW will be checking for it + */ + ish_set_host_rdy(dev); + /* Send RESET_NOTIFY_ACK (with reset_id) */ + ipc_send_mng_msg(dev, MNG_RESET_NOTIFY_ACK, &reset_id, sizeof(u32)); +} + #define TIME_SLICE_FOR_FW_RDY_MS 100 #define TIME_SLICE_FOR_INPUT_RDY_MS 100 #define TIMEOUT_FOR_FW_RDY_MS 2000 @@ -496,13 +510,9 @@ static int timed_wait_for_timeout(struct ishtp_device *dev, int condition, */ static int ish_fw_reset_handler(struct ishtp_device *dev) { - uint32_t reset_id; unsigned long flags; int ret; - /* Read reset ID */ - reset_id = ish_reg_read(dev, IPC_REG_ISH2HOST_MSG) & 0xFFFF; - /* Clear IPC output queue */ spin_lock_irqsave(&dev->wr_processing_spinlock, flags); list_splice_init(&dev->wr_processing_list, &dev->wr_free_list); @@ -521,15 +531,6 @@ static int ish_fw_reset_handler(struct ishtp_device *dev) /* Send clock sync at once after reset */ ishtp_dev->prev_sync = 0; - /* - * Set HOST2ISH.ILUP. Apparently we need this BEFORE sending - * RESET_NOTIFY_ACK - FW will be checking for it - */ - ish_set_host_rdy(dev); - /* Send RESET_NOTIFY_ACK (with reset_id) */ - ipc_send_mng_msg(dev, MNG_RESET_NOTIFY_ACK, &reset_id, - sizeof(uint32_t)); - /* Wait for ISH FW'es ILUP and ISHTP_READY */ ret = timed_wait_for_timeout(dev, WAIT_FOR_FW_RDY, TIME_SLICE_FOR_FW_RDY_MS, @@ -563,8 +564,6 @@ static void fw_reset_work_fn(struct work_struct *work) if (!rv) { /* ISH is ILUP & ISHTP-ready. Restart ISHTP */ msleep_interruptible(TIMEOUT_FOR_HW_RDY_MS); - ishtp_dev->recvd_hw_ready = 1; - wake_up_interruptible(&ishtp_dev->wait_hw_ready); /* ISHTP notification in IPC_RESET sequence completion */ if (!work_pending(work)) @@ -625,15 +624,14 @@ static void recv_ipc(struct ishtp_device *dev, uint32_t doorbell_val) break; case MNG_RESET_NOTIFY: - if (!ishtp_dev) { - ishtp_dev = dev; - } - queue_work(dev->unbound_wq, &fw_reset_work); - break; + ish_send_reset_notify_ack(ishtp_dev); + fallthrough; case MNG_RESET_NOTIFY_ACK: dev->recvd_hw_ready = 1; wake_up_interruptible(&dev->wait_hw_ready); + if (!work_pending(&fw_reset_work)) + queue_work(dev->unbound_wq, &fw_reset_work); break; } } @@ -1001,6 +999,7 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev) list_add_tail(&tx_buf->link, &dev->wr_free_list); } + ishtp_dev = dev; ret = devm_work_autocancel(&pdev->dev, &fw_reset_work, fw_reset_work_fn); if (ret) { dev_err(dev->devc, "Failed to initialise FW reset work\n"); From 507561b00ac2481eaf5fd790801f2ca135f23ff0 Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 17 Oct 2025 10:22:17 +0800 Subject: [PATCH 6/8] HID: intel-ish-hid: Use IPC RESET instead of void message in ish_wakeup() On ISH power-up, the bootloader enters sleep after preparing to load the main firmware, waiting for the driver to be ready. When the driver is ready, it sends a void message to wake up the bootloader and load the main firmware. The main firmware then sends MNG_RESET_NOTIFY to the driver for handshake. This void message-based IPC handshake only works if the main firmware has not been loaded. During hibernation resume, if the restore kernel has the ISH driver, the driver wakes up the bootloader to load the main firmware and perform IPC handshake. However, when switching to the image kernel, since the main firmware is already loaded, sending a void message in the .restore() callback does not trigger IPC handshake. By sending MNG_RESET_NOTIFY (IPC RESET message) in ish_wakeup() instead of a void message, we can explicitly wake up the bootloader and perform IPC handshake, regardless of the firmware state. Additionally, since ish_ipc_reset() already waits for recvd_hw_ready, the redundant wait for recvd_hw_ready in ish_hw_start() is removed. The timeout for waiting for HW ready is set to 10 seconds, matching the original timeout value used in ish_wakeup(), to ensure reliable wakeup on hardware that requires more time, such as the Lenovo ThinkPad X1 Titanium Gen 1. Signed-off-by: Zhang Lixu Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ipc/ipc.c | 39 ++++++++++++----------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c index 01a139e17cb5..59355e4a61f8 100644 --- a/drivers/hid/intel-ish-hid/ipc/ipc.c +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c @@ -728,22 +728,28 @@ int ish_disable_dma(struct ishtp_device *dev) * ish_wakeup() - wakeup ishfw from waiting-for-host state * @dev: ishtp device pointer * - * Set the dma enable bit and send a void message to FW, + * Set the dma enable bit and send a IPC RESET message to FW, * it wil wakeup FW from waiting-for-host state. + * + * Return: 0 for success else error code. */ -static void ish_wakeup(struct ishtp_device *dev) +static int ish_wakeup(struct ishtp_device *dev) { + int ret; + /* Set dma enable bit */ ish_reg_write(dev, IPC_REG_ISH_RMP2, IPC_RMP2_DMA_ENABLED); /* - * Send 0 IPC message so that ISH FW wakes up if it was already + * Send IPC RESET message so that ISH FW wakes up if it was already * asleep. */ - ish_reg_write(dev, IPC_REG_HOST2ISH_DRBL, IPC_DRBL_BUSY_BIT); + ret = ish_ipc_reset(dev); /* Flush writes to doorbell and REMAP2 */ ish_reg_read(dev, IPC_REG_ISH_HOST_FWSTS); + + return ret; } /** @@ -792,11 +798,11 @@ static int _ish_hw_reset(struct ishtp_device *dev) pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, csr); /* Now we can enable ISH DMA operation and wakeup ISHFW */ - ish_wakeup(dev); - - return 0; + return ish_wakeup(dev); } +#define RECVD_HW_READY_TIMEOUT (10 * HZ) + /** * _ish_ipc_reset() - IPC reset * @dev: ishtp device pointer @@ -831,7 +837,8 @@ static int _ish_ipc_reset(struct ishtp_device *dev) } wait_event_interruptible_timeout(dev->wait_hw_ready, - dev->recvd_hw_ready, 2 * HZ); + dev->recvd_hw_ready, + RECVD_HW_READY_TIMEOUT); if (!dev->recvd_hw_ready) { dev_err(dev->devc, "Timed out waiting for HW ready\n"); rv = -ENODEV; @@ -855,21 +862,7 @@ int ish_hw_start(struct ishtp_device *dev) set_host_ready(dev); /* After that we can enable ISH DMA operation and wakeup ISHFW */ - ish_wakeup(dev); - - /* wait for FW-initiated reset flow */ - if (!dev->recvd_hw_ready) - wait_event_interruptible_timeout(dev->wait_hw_ready, - dev->recvd_hw_ready, - 10 * HZ); - - if (!dev->recvd_hw_ready) { - dev_err(dev->devc, - "[ishtp-ish]: Timed out waiting for FW-initiated reset\n"); - return -ENODEV; - } - - return 0; + return ish_wakeup(dev); } /** From 5677aa6a08c1df8bc1ec71516fe1ced9b7cb545f Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Fri, 17 Oct 2025 10:22:18 +0800 Subject: [PATCH 7/8] HID: intel-ish-hid: ipc: Separate hibernate callbacks in dev_pm_ops The same suspend and resume callbacks are used for both suspend-to-RAM/idle and hibernation. These callbacks invoke pm_suspend_via_firmware() and pm_resume_via_firmware(), respectively. In the .freeze() of hibernation, pm_suspend_via_firmware() returns false, causing the driver to put ISH into D0i3. However, during the .thaw(), pm_resume_via_firmware() returns true, leading the driver to treat ISH as resuming from D3 instead of D0i3. The asymmetric behavior between .freeze() and .thaw() during hibernation can cause the client connection states on the firmware side and the driver side to become inconsistent. To address the inconsistent client connection states issue, separate hibernate-related callbacks (freeze, thaw) in dev_pm_ops. Since ISH does not need to save any firmware-related state when entering hibernation, it is sufficient to call pci_save_state() in .freeze() to prevent the PCI bus from changing the ISH power state. No actions are required in .thaw(). Signed-off-by: Zhang Lixu Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ipc/pci-ish.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c index e4499c83c62e..1612e8cb23f0 100644 --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c @@ -397,7 +397,20 @@ static int __maybe_unused ish_resume(struct device *device) return 0; } -static SIMPLE_DEV_PM_OPS(ish_pm_ops, ish_suspend, ish_resume); +static int __maybe_unused ish_freeze(struct device *device) +{ + struct pci_dev *pdev = to_pci_dev(device); + + return pci_save_state(pdev); +} + +static const struct dev_pm_ops __maybe_unused ish_pm_ops = { + .suspend = pm_sleep_ptr(ish_suspend), + .resume = pm_sleep_ptr(ish_resume), + .freeze = pm_sleep_ptr(ish_freeze), + .restore = pm_sleep_ptr(ish_resume), + .poweroff = pm_sleep_ptr(ish_suspend), +}; static ssize_t base_version_show(struct device *cdev, struct device_attribute *attr, char *buf) From 3644f4411713f52bf231574aa8759e3d8e20b341 Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Wed, 22 Oct 2025 00:49:08 +0200 Subject: [PATCH 8/8] HID: intel-ish-hid: Fix -Wcast-function-type-strict in devm_ishtp_alloc_workqueue() Clang warns (or errors with CONFIG_WERROR=y / W=e): drivers/hid/intel-ish-hid/ipc/ipc.c:935:36: error: cast from 'void (*)(struct workqueue_struct *)' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict] 935 | if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/device/devres.h:168:34: note: expanded from macro 'devm_add_action_or_reset' 168 | __devm_add_action_or_ireset(dev, action, data, #action) | ^~~~~~ This warning is pointing out a kernel control flow integrity (kCFI / CONFIG_CFI=y) violation will occur due to this function cast when the destroy_workqueue() is indirectly called via devm_action_release() because the prototype of destroy_workqueue() does not match the prototype of (*action)(). Use a local function with the correct prototype to wrap destroy_workqueue() to resolve the warning and CFI violation. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202510190103.qTZvfdjj-lkp@intel.com/ Closes: https://github.com/ClangBuiltLinux/linux/issues/2139 Fixes: 0d30dae38fe0 ("HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking") Signed-off-by: Nathan Chancellor Acked-by: Srinivas Pandruvada Reviewed-by: Zhang Lixu Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ipc/ipc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c index 59355e4a61f8..abf9c9a31c39 100644 --- a/drivers/hid/intel-ish-hid/ipc/ipc.c +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c @@ -924,6 +924,11 @@ static const struct ishtp_hw_ops ish_hw_ops = { .dma_no_cache_snooping = _dma_no_cache_snooping }; +static void ishtp_free_workqueue(void *wq) +{ + destroy_workqueue(wq); +} + static struct workqueue_struct *devm_ishtp_alloc_workqueue(struct device *dev) { struct workqueue_struct *wq; @@ -932,8 +937,7 @@ static struct workqueue_struct *devm_ishtp_alloc_workqueue(struct device *dev) if (!wq) return NULL; - if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue, - wq)) + if (devm_add_action_or_reset(dev, ishtp_free_workqueue, wq)) return NULL; return wq;