[PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume

d3z posted 1 patch 6 days, 7 hours ago
.../hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 60 +++++++++++++++++--
1 file changed, 56 insertions(+), 4 deletions(-)
[PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
Posted by d3z 6 days, 7 hours ago
From: Danny D. <d3z.the.dev@gmail.com>

On the Surface Pro 10 (Meteor Lake) the touchscreen stops working after a
suspend/resume cycle and only recovers after a reboot. The driver logs
"GET_DEVICE_INFO: recv failed: -11" on resume.

This platform suspends through s2idle: /sys/power/mem_sleep exposes
"[s2idle]" as the only state, there is no "deep"/S3 entry at all. The
touch IC nonetheless loses power across that s2idle suspend, the same
way it does across hibernation. quickspi_resume() only re-selects the
THC port, restores interrupts and DMA and sends a HIDSPI_ON command,
assuming the touch IC kept its power and state. When it has actually
lost power the HIDSPI_ON command is never acknowledged and the
descriptor read fails, leaving the touchscreen dead until the module is
reloaded.

quickspi_restore() already handles this for hibernation by
reconfiguring the THC SPI/LTR settings and running reset_tic() to
re-enumerate the device. Make quickspi_resume() do the same when the
device is not a wake source. A wake-enabled device keeps its power and
state across suspend, so it stays on the light restore path: resetting
it would discard a pending wake touch event and break wake-on-touch.

The non-wake path mirrors the existing quickspi_restore() sequence,
including enabling interrupts before reset_tic(), so it introduces no
new ordering relative to code already in the driver.

This change has been validated on a Surface Pro 10 running the
linux-surface kernel across multiple s2idle suspend/resume cycles; it
has not been tested on a mainline build.

Closes: https://github.com/linux-surface/linux-surface/issues/1799
Signed-off-by: Danny D. <d3z.the.dev@gmail.com>
---
v1 -> v2:
 - Only run the full reset when the device is not a wake source
   (device_may_wakeup()), so wake-on-touch is no longer disturbed.
 - Reword the changelog around s2idle: the SP10 has no "deep"/S3 state, the
   touch IC loses power across s2idle.

 .../hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 60 +++++++++++++++++--
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -784,20 +784,72 @@ static int quickspi_resume(struct device
 	if (ret)
 		return ret;
 
+	/*
+	 * A wake-enabled device keeps its power and state across suspend, so
+	 * only restore the THC context. Resetting it here would discard a
+	 * pending wake touch event and break wake-on-touch.
+	 */
+	if (device_may_wakeup(qsdev->dev)) {
+		thc_interrupt_config(qsdev->thc_hw);
+
+		thc_interrupt_enable(qsdev->thc_hw, true);
+
+		ret = thc_dma_configure(qsdev->thc_hw);
+		if (ret)
+			return ret;
+
+		return thc_interrupt_quiesce(qsdev->thc_hw, false);
+	}
+
+	/*
+	 * Otherwise the touch IC may have lost power across suspend. On
+	 * platforms that suspend through s2idle (for example the Surface Pro
+	 * 10, whose firmware exposes s2idle as the only mem_sleep state) the
+	 * IC loses power the same way it does across hibernation. A plain
+	 * HIDSPI_ON is then not acknowledged and the descriptor read fails, so
+	 * re-enumerate the device through the full reset flow already used by
+	 * quickspi_restore().
+	 */
+	thc_spi_input_output_address_config(qsdev->thc_hw,
+					    qsdev->input_report_hdr_addr,
+					    qsdev->input_report_bdy_addr,
+					    qsdev->output_report_addr);
+
+	ret = thc_spi_read_config(qsdev->thc_hw, qsdev->spi_freq_val,
+				  qsdev->spi_read_io_mode,
+				  qsdev->spi_read_opcode,
+				  qsdev->spi_packet_size);
+	if (ret)
+		return ret;
+
+	ret = thc_spi_write_config(qsdev->thc_hw, qsdev->spi_freq_val,
+				   qsdev->spi_write_io_mode,
+				   qsdev->spi_write_opcode,
+				   qsdev->spi_packet_size,
+				   qsdev->performance_limit);
+	if (ret)
+		return ret;
+
 	thc_interrupt_config(qsdev->thc_hw);
 
 	thc_interrupt_enable(qsdev->thc_hw, true);
 
-	ret = thc_dma_configure(qsdev->thc_hw);
+	/* The touch IC may have lost power, reset it to recover */
+	ret = reset_tic(qsdev);
 	if (ret)
 		return ret;
 
-	ret = thc_interrupt_quiesce(qsdev->thc_hw, false);
+	ret = thc_dma_configure(qsdev->thc_hw);
 	if (ret)
 		return ret;
 
-	if (!device_may_wakeup(qsdev->dev))
-		return quickspi_set_power(qsdev, HIDSPI_ON);
+	thc_ltr_config(qsdev->thc_hw,
+		       qsdev->active_ltr_val,
+		       qsdev->low_power_ltr_val);
+
+	thc_change_ltr_mode(qsdev->thc_hw, THC_LTR_MODE_ACTIVE);
+
+	qsdev->state = QUICKSPI_ENABLED;
 
 	return 0;
 }
-- 
2.43.0
RE: [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
Posted by Xu, Even an hour ago

> -----Original Message-----
> From: d3z <d3z.the.dev@gmail.com>
> Sent: Tuesday, June 2, 2026 5:18 AM
> To: Xu, Even <even.xu@intel.com>; jikos@kernel.org; bentiss@kernel.org
> Cc: Sun, Xinpeng <xinpeng.sun@intel.com>; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; sakari.ailus@linux.intel.com;
> abhishektamboli9@gmail.com; Danny D . <d3z.the.dev@gmail.com>
> Subject: [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system
> resume
> 
> From: Danny D. <d3z.the.dev@gmail.com>
> 
> On the Surface Pro 10 (Meteor Lake) the touchscreen stops working after a
> suspend/resume cycle and only recovers after a reboot. The driver logs
> "GET_DEVICE_INFO: recv failed: -11" on resume.
> 
> This platform suspends through s2idle: /sys/power/mem_sleep exposes "[s2idle]"
> as the only state, there is no "deep"/S3 entry at all. The touch IC nonetheless
> loses power across that s2idle suspend, the same way it does across hibernation.
> quickspi_resume() only re-selects the THC port, restores interrupts and DMA and
> sends a HIDSPI_ON command, assuming the touch IC kept its power and state.
> When it has actually lost power the HIDSPI_ON command is never acknowledged
> and the descriptor read fails, leaving the touchscreen dead until the module is
> reloaded.
> 
> quickspi_restore() already handles this for hibernation by reconfiguring the THC
> SPI/LTR settings and running reset_tic() to re-enumerate the device. Make
> quickspi_resume() do the same when the device is not a wake source. A wake-
> enabled device keeps its power and state across suspend, so it stays on the light
> restore path: resetting it would discard a pending wake touch event and break
> wake-on-touch.
> 
> The non-wake path mirrors the existing quickspi_restore() sequence, including
> enabling interrupts before reset_tic(), so it introduces no new ordering relative to
> code already in the driver.
> 
> This change has been validated on a Surface Pro 10 running the linux-surface
> kernel across multiple s2idle suspend/resume cycles; it has not been tested on a
> mainline build.
> 
> Closes: https://github.com/linux-surface/linux-surface/issues/1799
> Signed-off-by: Danny D. <d3z.the.dev@gmail.com>
> ---
> v1 -> v2:
>  - Only run the full reset when the device is not a wake source
>    (device_may_wakeup()), so wake-on-touch is no longer disturbed.
>  - Reword the changelog around s2idle: the SP10 has no "deep"/S3 state, the
>    touch IC loses power across s2idle.
> 
>  .../hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 60 +++++++++++++++++--
>  1 file changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -784,20 +784,72 @@ static int quickspi_resume(struct device
>  	if (ret)
>  		return ret;
> 
> +	/*
> +	 * A wake-enabled device keeps its power and state across suspend, so
> +	 * only restore the THC context. Resetting it here would discard a
> +	 * pending wake touch event and break wake-on-touch.
> +	 */
> +	if (device_may_wakeup(qsdev->dev)) {
> +		thc_interrupt_config(qsdev->thc_hw);
> +
> +		thc_interrupt_enable(qsdev->thc_hw, true);
> +
> +		ret = thc_dma_configure(qsdev->thc_hw);
> +		if (ret)
> +			return ret;
> +
> +		return thc_interrupt_quiesce(qsdev->thc_hw, false);
> +	}
> +
> +	/*
> +	 * Otherwise the touch IC may have lost power across suspend. On
> +	 * platforms that suspend through s2idle (for example the Surface Pro
> +	 * 10, whose firmware exposes s2idle as the only mem_sleep state) the
> +	 * IC loses power the same way it does across hibernation. A plain
> +	 * HIDSPI_ON is then not acknowledged and the descriptor read fails, so
> +	 * re-enumerate the device through the full reset flow already used by
> +	 * quickspi_restore().
> +	 */
> +	thc_spi_input_output_address_config(qsdev->thc_hw,
> +					    qsdev->input_report_hdr_addr,
> +					    qsdev->input_report_bdy_addr,
> +					    qsdev->output_report_addr);
> +
> +	ret = thc_spi_read_config(qsdev->thc_hw, qsdev->spi_freq_val,
> +				  qsdev->spi_read_io_mode,
> +				  qsdev->spi_read_opcode,
> +				  qsdev->spi_packet_size);
> +	if (ret)
> +		return ret;
> +
> +	ret = thc_spi_write_config(qsdev->thc_hw, qsdev->spi_freq_val,
> +				   qsdev->spi_write_io_mode,
> +				   qsdev->spi_write_opcode,
> +				   qsdev->spi_packet_size,
> +				   qsdev->performance_limit);
> +	if (ret)
> +		return ret;
> +
>  	thc_interrupt_config(qsdev->thc_hw);
> 
>  	thc_interrupt_enable(qsdev->thc_hw, true);
> 
> -	ret = thc_dma_configure(qsdev->thc_hw);
> +	/* The touch IC may have lost power, reset it to recover */
> +	ret = reset_tic(qsdev);
>  	if (ret)
>  		return ret;
> 
> -	ret = thc_interrupt_quiesce(qsdev->thc_hw, false);
> +	ret = thc_dma_configure(qsdev->thc_hw);
>  	if (ret)
>  		return ret;
> 
> -	if (!device_may_wakeup(qsdev->dev))
> -		return quickspi_set_power(qsdev, HIDSPI_ON);
> +	thc_ltr_config(qsdev->thc_hw,
> +		       qsdev->active_ltr_val,
> +		       qsdev->low_power_ltr_val);
> +
> +	thc_change_ltr_mode(qsdev->thc_hw, THC_LTR_MODE_ACTIVE);
> +
> +	qsdev->state = QUICKSPI_ENABLED;
> 
>  	return 0;
>  }

Thanks for your patch!

Reviewed-by: Even Xu <even.xu@intel.com>

> --
> 2.43.0