Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
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
> 



      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