From: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
To: Michal Simek <michal.simek@amd.com>,
linux-kernel@vger.kernel.org, monstr@monstr.eu, git@amd.com
Cc: Ajay Neeli <ajay.neeli@amd.com>,
"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
"moderated list:ARM/ZYNQ ARCHITECTURE"
<linux-arm-kernel@lists.infradead.org>,
"open list:SCSI SUBSYSTEM" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH] firmware: xilinx: ufs: move PHY/SRAM ready polling into the firmware backend
Date: Tue, 11 Aug 2026 19:55:48 +0530 [thread overview]
Message-ID: <6cf0791d-902d-4a89-bb57-d4c607991579@amd.com> (raw)
In-Reply-To: <eaeaaea8ed76069943e0706a0917139ff6569929.1785855749.git.michal.simek@amd.com>
On 8/4/2026 8:32 PM, Michal Simek wrote:
> The Versal Gen 2 UFS driver polls the firmware for M-PHY TX/RX
> configuration readiness and SRAM initialisation completion with two
> open-coded do/while loops. Each iteration is a full firmware round-trip
> (PM_IOCTL/IOCTL_READ_REG of a protected PMC_IOU_SLCR register), so the
> loop can issue up to a million EEMI calls, and it hard-codes the wait
> policy inside the controller driver.
>
> Introduce coarse blocking helpers, zynqmp_pm_wait_mphy_tx_rx_config_ready()
> and zynqmp_pm_wait_sram_init_done(), that take a caller-supplied timeout
> budget and contain the poll loop. The loop is EEMI-specific (legacy
> firmware only exposes the per-read status primitive) so it lives in the
> firmware driver, keeping the UFS driver backend-agnostic: a future
> backend can offload the wait to the platform in a single call without
> touching the controller driver again. The existing per-read primitives stay
> exported, so the current EEMI interface is unchanged.
>
> The timeout budget remains owned by the UFS driver (the consumer that
> knows the hardware) and is passed down, so EEMI and any future backend
> stay consistent.
>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
Reviewed-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
Regards
Sai Krishna
> ---
>
> drivers/firmware/xilinx/zynqmp-ufs.c | 70 ++++++++++++++++++++++--
> drivers/ufs/host/ufs-amd-versal2.c | 46 ++++------------
> include/linux/firmware/xlnx-zynqmp-ufs.h | 8 +--
> 3 files changed, 82 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/firmware/xilinx/zynqmp-ufs.c b/drivers/firmware/xilinx/zynqmp-ufs.c
> index 85da8a822f3a..81ccf61a037c 100644
> --- a/drivers/firmware/xilinx/zynqmp-ufs.c
> +++ b/drivers/firmware/xilinx/zynqmp-ufs.c
> @@ -5,6 +5,7 @@
> * Copyright (C) 2025 Advanced Micro Devices, Inc.
> */
>
> +#include <linux/delay.h>
> #include <linux/firmware/xlnx-zynqmp.h>
> #include <linux/module.h>
>
> @@ -33,7 +34,7 @@
> *
> * Return: Returns 0 on success or error value on failure.
> */
> -int zynqmp_pm_is_mphy_tx_rx_config_ready(bool *is_ready)
> +static int zynqmp_pm_is_mphy_tx_rx_config_ready(bool *is_ready)
> {
> u32 regval;
> int ret;
> @@ -53,7 +54,6 @@ int zynqmp_pm_is_mphy_tx_rx_config_ready(bool *is_ready)
>
> return ret;
> }
> -EXPORT_SYMBOL_GPL(zynqmp_pm_is_mphy_tx_rx_config_ready);
>
> /**
> * zynqmp_pm_is_sram_init_done - check SRAM initialization
> @@ -61,7 +61,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_is_mphy_tx_rx_config_ready);
> *
> * Return: Returns 0 on success or error value on failure.
> */
> -int zynqmp_pm_is_sram_init_done(bool *is_done)
> +static int zynqmp_pm_is_sram_init_done(bool *is_done)
> {
> u32 regval;
> int ret;
> @@ -81,7 +81,69 @@ int zynqmp_pm_is_sram_init_done(bool *is_done)
>
> return ret;
> }
> -EXPORT_SYMBOL_GPL(zynqmp_pm_is_sram_init_done);
> +
> +/**
> + * zynqmp_pm_wait_mphy_tx_rx_config_ready - wait for M-PHY TX-RX config ready
> + * @timeout_us: Caller-supplied timeout budget in microseconds
> + *
> + * Poll the M-PHY TX-RX configuration-ready status until it settles or the
> + * timeout elapses. The poll loop is EEMI-specific (legacy firmware only offers
> + * the per-read status primitive), so it lives here in the firmware driver
> + * rather than in the UFS controller driver; an SCMI-based backend can instead
> + * offload the wait to the platform in a single call. The timeout budget is
> + * owned by the caller (UFS driver), keeping the policy with the consumer.
> + *
> + * Return: Returns 0 once ready, -ETIMEDOUT on timeout, or error value.
> + */
> +int zynqmp_pm_wait_mphy_tx_rx_config_ready(u32 timeout_us)
> +{
> + bool is_ready;
> + int ret;
> +
> + while (timeout_us--) {
> + ret = zynqmp_pm_is_mphy_tx_rx_config_ready(&is_ready);
> + if (ret)
> + return ret;
> +
> + if (!is_ready)
> + return 0;
> +
> + usleep_range(1, 5);
> + }
> +
> + return -ETIMEDOUT;
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_wait_mphy_tx_rx_config_ready);
> +
> +/**
> + * zynqmp_pm_wait_sram_init_done - wait for SRAM initialization to complete
> + * @timeout_us: Caller-supplied timeout budget in microseconds
> + *
> + * Poll the SRAM initialization-done status until it is set or the timeout
> + * elapses. As with the M-PHY wait, the poll loop is EEMI-specific and kept in
> + * the firmware driver so the UFS controller driver stays backend-agnostic.
> + *
> + * Return: Returns 0 once done, -ETIMEDOUT on timeout, or error value.
> + */
> +int zynqmp_pm_wait_sram_init_done(u32 timeout_us)
> +{
> + bool is_done;
> + int ret;
> +
> + while (timeout_us--) {
> + ret = zynqmp_pm_is_sram_init_done(&is_done);
> + if (ret)
> + return ret;
> +
> + if (is_done)
> + return 0;
> +
> + usleep_range(1, 5);
> + }
> +
> + return -ETIMEDOUT;
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_wait_sram_init_done);
>
> /**
> * zynqmp_pm_set_sram_bypass - Set SRAM bypass Control
> diff --git a/drivers/ufs/host/ufs-amd-versal2.c b/drivers/ufs/host/ufs-amd-versal2.c
> index 2154d6286817..dff0c2c95486 100644
> --- a/drivers/ufs/host/ufs-amd-versal2.c
> +++ b/drivers/ufs/host/ufs-amd-versal2.c
> @@ -225,8 +225,6 @@ static int ufs_versal2_setup_phy(struct ufs_hba *hba)
> static int ufs_versal2_phy_init(struct ufs_hba *hba)
> {
> struct ufs_versal2_host *host = ufshcd_get_variant(hba);
> - u32 time_left;
> - bool is_ready;
> int ret;
> static const struct ufshcd_dme_attr_val rmmi_attrs[] = {
> { UIC_ARG_MIB(CBREFCLKCTRL2), CBREFREFCLK_GATE_OVR_EN, DME_LOCAL },
> @@ -235,23 +233,15 @@ static int ufs_versal2_phy_init(struct ufs_hba *hba)
> { UIC_ARG_MIB(VS_MPHYCFGUPDT), 1, DME_LOCAL }
> };
>
> - /* Wait for Tx/Rx config_rdy */
> - time_left = TIMEOUT_MICROSEC;
> - do {
> - time_left--;
> - ret = zynqmp_pm_is_mphy_tx_rx_config_ready(&is_ready);
> - if (ret)
> - return ret;
> -
> - if (!is_ready)
> - break;
> -
> - usleep_range(1, 5);
> - } while (time_left);
> -
> - if (!time_left) {
> + /*
> + * Wait for Tx/Rx config_rdy. The poll loop lives in the firmware
> + * backend (EEMI today, SCMI in future) so this driver stays
> + * backend-agnostic; the timeout budget stays here with the consumer.
> + */
> + ret = zynqmp_pm_wait_mphy_tx_rx_config_ready(TIMEOUT_MICROSEC);
> + if (ret) {
> dev_err(hba->dev, "Tx/Rx configuration signal busy.\n");
> - return -ETIMEDOUT;
> + return ret;
> }
>
> ret = ufshcd_dwc_dme_set_attrs(hba, rmmi_attrs, ARRAY_SIZE(rmmi_attrs));
> @@ -264,23 +254,11 @@ static int ufs_versal2_phy_init(struct ufs_hba *hba)
> return ret;
> }
>
> - /* Wait for SRAM init done */
> - time_left = TIMEOUT_MICROSEC;
> - do {
> - time_left--;
> - ret = zynqmp_pm_is_sram_init_done(&is_ready);
> - if (ret)
> - return ret;
> -
> - if (is_ready)
> - break;
> -
> - usleep_range(1, 5);
> - } while (time_left);
> -
> - if (!time_left) {
> + /* Wait for SRAM init done (poll handled by the firmware backend). */
> + ret = zynqmp_pm_wait_sram_init_done(TIMEOUT_MICROSEC);
> + if (ret) {
> dev_err(hba->dev, "SRAM initialization failed.\n");
> - return -ETIMEDOUT;
> + return ret;
> }
>
> ret = ufs_versal2_setup_phy(hba);
> diff --git a/include/linux/firmware/xlnx-zynqmp-ufs.h b/include/linux/firmware/xlnx-zynqmp-ufs.h
> index d3538dd5822a..00383dd835f2 100644
> --- a/include/linux/firmware/xlnx-zynqmp-ufs.h
> +++ b/include/linux/firmware/xlnx-zynqmp-ufs.h
> @@ -9,17 +9,17 @@
> #define __FIRMWARE_XLNX_ZYNQMP_UFS_H__
>
> #if IS_REACHABLE(CONFIG_ZYNQMP_FIRMWARE)
> -int zynqmp_pm_is_mphy_tx_rx_config_ready(bool *is_ready);
> -int zynqmp_pm_is_sram_init_done(bool *is_done);
> +int zynqmp_pm_wait_mphy_tx_rx_config_ready(u32 timeout_us);
> +int zynqmp_pm_wait_sram_init_done(u32 timeout_us);
> int zynqmp_pm_set_sram_bypass(void);
> int zynqmp_pm_get_ufs_calibration_values(u32 *val);
> #else
> -static inline int zynqmp_pm_is_mphy_tx_rx_config_ready(bool *is_ready)
> +static inline int zynqmp_pm_wait_mphy_tx_rx_config_ready(u32 timeout_us)
> {
> return -ENODEV;
> }
>
> -static inline int zynqmp_pm_is_sram_init_done(bool *is_done)
> +static inline int zynqmp_pm_wait_sram_init_done(u32 timeout_us)
> {
> return -ENODEV;
> }
> ---
> base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
> branch: xnext/ufs
>
prev parent reply other threads:[~2026-08-11 14:26 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-04 15:02 [PATCH] firmware: xilinx: ufs: move PHY/SRAM ready polling into the firmware backend Michal Simek
2026-08-11 14:25 ` Sai Krishna Potthuri [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6cf0791d-902d-4a89-bb57-d4c607991579@amd.com \
--to=sai.krishna.potthuri@amd.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=ajay.neeli@amd.com \
--cc=git@amd.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=michal.simek@amd.com \
--cc=monstr@monstr.eu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox