Re: [PATCH] i2c: designware: Enable PSP semaphore for AMDI0010 and fix probe deferral

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Thu, Feb 05, 2026 at 06:30:47PM +0800, WangYuli wrote:
> From: WangYuli <wangyl5933@chinaunicom.cn>
> 
> AMD Strix Point platforms use the AMDI0010 ACPI HID for their I2C
> controllers, but this entry was missing the ARBITRATION_SEMAPHORE flag
> that enables PSP-based bus arbitration.
> 
> Without proper arbitration, when both the x86 host and AMD PSP
> (Platform Security Processor) attempt to access the shared I2C bus
> simultaneously, the DesignWare controller loses arbitration and reports:
> 
>   i2c_designware AMDI0010:01: i2c_dw_handle_tx_abort: lost arbitration
> 
> This causes communication failures with I2C devices such as touchpads
> (e.g., BLTP7853 HID-over-I2C).
> 
> Add the ARBITRATION_SEMAPHORE flag to the AMDI0010 entry to enable PSP
> mailbox-based I2C bus arbitration, consistent with how AMDI0019 was
> handled for AMD Cezanne platforms.
> 
> However, simply enabling this flag exposes a latent bug introduced by
> commit 440da737cf8d ("i2c: designware: Use PCI PSP driver for
> communication"): the driver unconditionally returns -EPROBE_DEFER when
> psp_check_platform_access_status() fails, causing an infinite probe
> deferral loop on platforms that lack PSP platform access support.
> 
> The problem is that psp_check_platform_access_status() returned -ENODEV
> for all failure cases, but there are two distinct scenarios:
> 
>   1. PSP is still initializing (psp pointer exists but platform_access_data
>      is not yet ready, while vdata->platform_access indicates support) -
>      this is a transient condition that warrants probe deferral.
> 
>   2. The platform genuinely lacks PSP platform access support (either no
>      psp pointer, or vdata->platform_access is not set) - this is a
>      permanent condition where probe deferral would loop indefinitely.
> 
> Fix this by updating psp_check_platform_access_status() to return:
> 
>   - -EPROBE_DEFER: when PSP exists with platform_access capability but
>     platform_access_data is not yet initialized (transient)
>   - -ENODEV: when the platform lacks PSP platform access support (permanent)
> 
> Then update the I2C driver to pass through the actual return code from
> psp_check_platform_access_status() instead of forcing -EPROBE_DEFER,
> allowing the driver to fail gracefully on unsupported platforms.
> 
> Tested on MECHREVO XINGYAO 14 with AMD Ryzen AI 9 H 365.
> 
> Fixes: 440da737cf8d ("i2c: designware: Use PCI PSP driver for communication")
> Cc: stable@vger.kernel.org
> Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
> ---
>  drivers/crypto/ccp/platform-access.c        |  7 ++++++-
>  drivers/i2c/busses/i2c-designware-amdpsp.c  | 11 +++++++++--
>  drivers/i2c/busses/i2c-designware-platdrv.c |  2 +-
>  include/linux/psp-platform-access.h         |  5 +++--
>  4 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/platform-access.c b/drivers/crypto/ccp/platform-access.c
> index 1b8ed3389733..3f20cf194cb6 100644
> --- a/drivers/crypto/ccp/platform-access.c
> +++ b/drivers/crypto/ccp/platform-access.c
> @@ -46,7 +46,12 @@ int psp_check_platform_access_status(void)
>  {
>  	struct psp_device *psp = psp_get_master_device();
>  
> -	if (!psp || !psp->platform_access_data)
> +	/* PSP driver not loaded yet, caller should defer */
> +	if ((!psp) || (!psp->platform_access_data && psp->vdata->platform_access))
> +		return -EPROBE_DEFER;
> +
> +	/* PSP loaded but platform_access not supported by hardware */
> +	if (!psp->platform_access_data && !psp->vdata->platform_access)
>  		return -ENODEV;
>  
>  	return 0;
> diff --git a/drivers/i2c/busses/i2c-designware-amdpsp.c b/drivers/i2c/busses/i2c-designware-amdpsp.c
> index 404571ad61a8..341232767177 100644
> --- a/drivers/i2c/busses/i2c-designware-amdpsp.c
> +++ b/drivers/i2c/busses/i2c-designware-amdpsp.c
> @@ -269,6 +269,7 @@ static const struct i2c_lock_operations i2c_dw_psp_lock_ops = {
>  int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev)
>  {
>  	struct pci_dev *rdev;
> +	int ret;
>  
>  	if (!IS_REACHABLE(CONFIG_CRYPTO_DEV_CCP_DD))
>  		return -ENODEV;
> @@ -291,8 +292,14 @@ int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev)
>  		_psp_send_i2c_req = psp_send_i2c_req_doorbell;
>  	pci_dev_put(rdev);
>  
> -	if (psp_check_platform_access_status())
> -		return -EPROBE_DEFER;
> +	/*
> +	 * Check if PSP platform access is available.
> +	 * Returns 0 on success, -EPROBE_DEFER if PSP driver not loaded,
> +	 * -ENODEV if platform_access is not supported by hardware.
> +	 */

This is useless comment.

> +	ret = psp_check_platform_access_status();
> +	if (ret)
> +		return ret;
>  
>  	psp_i2c_dev = dev->dev;
>  
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 7be99656a67d..63b1c06ee111 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -345,7 +345,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
>  	{ "80860F41", ACCESS_NO_IRQ_SUSPEND },
>  	{ "808622C1", ACCESS_NO_IRQ_SUSPEND },
>  	{ "AMD0010", ACCESS_INTR_MASK },
> -	{ "AMDI0010", ACCESS_INTR_MASK },
> +	{ "AMDI0010", ACCESS_INTR_MASK | ARBITRATION_SEMAPHORE },
>  	{ "AMDI0019", ACCESS_INTR_MASK | ARBITRATION_SEMAPHORE },
>  	{ "AMDI0510", 0 },
>  	{ "APMC0D0F", 0 },
> diff --git a/include/linux/psp-platform-access.h b/include/linux/psp-platform-access.h
> index 540abf7de048..84dbdbeb61d6 100644
> --- a/include/linux/psp-platform-access.h
> +++ b/include/linux/psp-platform-access.h
> @@ -64,8 +64,9 @@ int psp_ring_platform_doorbell(int msg, u32 *result);
>   * if platform features has initialized.
>   *
>   * Returns:
> - * 0          platform features is ready
> - * -%ENODEV   platform features is not ready or present
> + *  0:            platform features is ready
> + *  -%ENODEV:     platform_access is not supported by hardware
> + *  -%EPROBE_DEFER: PSP driver not ready or platform features not yet initialized
>   */
>  int psp_check_platform_access_status(void);
>  
> -- 
> 2.51.0