[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260205112451.GX2275908@black.igk.intel.com>
Date: Thu, 5 Feb 2026 12:24:51 +0100
From: Mika Westerberg <mika.westerberg@...ux.intel.com>
To: WangYuli <wangyuli@...c.io>
Cc: mario.limonciello@....com, thomas.lendacky@....com, john.allen@....com,
herbert@...dor.apana.org.au, davem@...emloft.net,
andriy.shevchenko@...ux.intel.com, jsd@...ihalf.com,
andi.shyti@...nel.org, linux-crypto@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
bp@...en8.de, ashish.kalra@....com, markhas@...omium.org,
jarkko.nikula@...ux.intel.com, wsa@...nel.org,
WangYuli <wangyl5933@...naunicom.cn>, stable@...r.kernel.org
Subject: Re: [PATCH] i2c: designware: Enable PSP semaphore for AMDI0010 and
fix probe deferral
On Thu, Feb 05, 2026 at 06:30:47PM +0800, WangYuli wrote:
> From: WangYuli <wangyl5933@...naunicom.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@...r.kernel.org
> Signed-off-by: WangYuli <wangyl5933@...naunicom.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
Powered by blists - more mailing lists