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

[Xu, Even]

> -----Original Message-----
> From: d3z <d3z.the.dev@xxxxxxxxx>
> Sent: Tuesday, June 2, 2026 5:18 AM
> To: Xu, Even <even.xu@xxxxxxxxx>; jikos@xxxxxxxxxx; bentiss@xxxxxxxxxx
> Cc: Sun, Xinpeng <xinpeng.sun@xxxxxxxxx>; linux-input@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; sakari.ailus@xxxxxxxxxxxxxxx;
> abhishektamboli9@xxxxxxxxx; Danny D . <d3z.the.dev@xxxxxxxxx>
> Subject: [PATCH v2] HID: intel-thc-hid: intel-quickspi: reset touch IC on system
> resume
> 
> From: Danny D. <d3z.the.dev@xxxxxxxxx>
> 
> 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@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>

> --
> 2.43.0