From: Zhang Lixu lixu.zhang@intel.com
[ Upstream commit 56e230723e3a818373bd62331bccb1c6d2b3881b ]
Some systems have enabled ISH without any sensors. In this case sending HOSTIF_DM_ENUM_DEVICES results in 0 sensors. This triggers ISH hardware reset on subsequent enumeration after S3/S4 resume.
The enum_devices_done flag was not reset before sending the HOSTIF_DM_ENUM_DEVICES command. On subsequent enumeration calls (such as after S3/S4 resume), this flag retains its previous true value, causing the wait loop to be skipped and returning prematurely to hid_ishtp_cl_init(). If 0 HID devices are found, hid_ishtp_cl_init() skips getting HID device descriptors and sets init_done to true. When the delayed enumeration response arrives with init_done already true, the driver treats it as a bad packet and triggers an ISH hardware reset.
Set enum_devices_done to false before sending the enumeration command, consistent with similar functions like ishtp_get_hid_descriptor() and ishtp_get_report_descriptor() which reset their respective flags.
Signed-off-by: Zhang Lixu lixu.zhang@intel.com Acked-by: Srinivas Pandruvada srinivas.pandruvada@linux.intel.com Signed-off-by: Benjamin Tissoires bentiss@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis of Commit: HID: intel-ish-hid: Reset enum_devices_done before enumeration
### 1. COMMIT MESSAGE ANALYSIS
The commit message clearly describes a **bug in suspend/resume (S3/S4) handling**: - Systems with ISH enabled but no sensors return 0 devices from HOSTIF_DM_ENUM_DEVICES - The `enum_devices_done` flag wasn't reset before sending enumeration commands - On subsequent enumerations (after resume), the stale `true` value causes the wait loop to be skipped - This leads to a race where delayed responses are treated as bad packets - Result: **ISH hardware reset** during resume
Key indicators: - "S3/S4 resume" - power management bug affecting real user workflows - Clear explanation of the timing/race condition - Acked-by Srinivas Pandruvada (Intel ISH maintainer)
### 2. CODE CHANGE ANALYSIS
The diff shows a **single line addition**: ```c + client_data->enum_devices_done = false; ```
Added immediately before the `HOSTIF_DM_ENUM_DEVICES` command is sent. The logic: - The function sends an enumeration request and waits for `enum_devices_done` to become true - Without resetting it first, a stale `true` value from a previous call causes immediate exit from the wait loop - This creates a race: the response arrives after `init_done` is already set, triggering a hardware reset
The fix follows the **established pattern** in the same driver - the commit message notes that `ishtp_get_hid_descriptor()` and `ishtp_get_report_descriptor()` already reset their respective flags similarly.
### 3. CLASSIFICATION
- **Bug fix**: Yes - fixes incorrect state handling causing hardware reset - **Feature addition**: No - **API change**: No - **Hardware quirk**: No - this is a driver logic bug, not a hardware workaround
### 4. SCOPE AND RISK ASSESSMENT
| Metric | Value | |--------|-------| | Lines changed | 1 | | Files touched | 1 | | Complexity | Very low | | Risk | Minimal |
The change: - Sets a boolean flag to false before an operation that expects it to transition to true - Follows identical patterns elsewhere in the same file - Cannot break any existing functionality that works correctly - Isolated to Intel ISH subsystem
### 5. USER IMPACT
**Affected users:** Laptop users with Intel Sensor Hub hardware (common in modern Intel laptops)
**Symptom:** ISH hardware reset during S3/S4 resume, which can cause: - Delays in resume - Sensor hub functionality issues after resume - System instability during power transitions
**Severity:** Moderate-High - affects common laptop power management workflows
### 6. STABILITY INDICATORS
- **Acked-by:** Srinivas Pandruvada (Intel ISH maintainer) - domain expert approval - **Signed-off-by:** Benjamin Tissoires (HID maintainer) - Pattern-following fix consistent with other similar functions in the driver
### 7. DEPENDENCY CHECK
This commit: - Is self-contained with no dependencies - Modifies code that has existed in stable trees for years - The `enum_devices_done` flag, `client_data` structure, and `ishtp_enum_enum_devices()` function are all well-established
---
## Summary
**What it fixes:** Race condition during S3/S4 resume where stale boolean flag causes ISH hardware reset
**Why it matters:** Users with Intel Sensor Hub hardware experience hardware reset/instability on resume from suspend/hibernate - a common laptop workflow
**Stable criteria met:** - ✅ Obviously correct (follows existing pattern in same driver) - ✅ Fixes real bug (hardware reset during resume) - ✅ Small and contained (1 line, 1 file) - ✅ No new features - ✅ No API changes - ✅ Has maintainer acknowledgment
**Risk assessment:** Very low - single boolean assignment following established pattern
**Benefit:** Fixes real user-facing bug affecting laptop power management
This is an ideal stable candidate: a minimal, surgical fix for a real bug affecting common hardware in a common usage scenario, with maintainer review and following established driver patterns.
**YES**
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c index d8c3c54a8c0f2..537145ec50586 100644 --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c @@ -495,6 +495,7 @@ static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl) int rv;
/* Send HOSTIF_DM_ENUM_DEVICES */ + client_data->enum_devices_done = false; memset(&msg, 0, sizeof(struct hostif_msg)); msg.hdr.command = HOSTIF_DM_ENUM_DEVICES; rv = ishtp_cl_send(hid_ishtp_cl, (unsigned char *)&msg,