Message-ID: <avpwp57yqkljxkld7dsqdsc7m26wvmwwhvph6ljv43yjjdyqof@szlfmik6betd>
Date: Thu, 20 Nov 2025 11:23:24 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Ram Kumar Dwivedi <ram.dwivedi@....qualcomm.com>
Cc: alim.akhtar@...sung.com, avri.altman@....com, bvanassche@....org, 
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	James.Bottomley@...senpartnership.com, martin.petersen@...cle.com, quic_ahari@...cinc.com, 
	linux-arm-msm@...r.kernel.org, linux-scsi@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Shazad Hussain <quic_shazhuss@...cinc.com>
Subject: Re: [PATCH V1 3/3] ufs: ufs-qcom: Add support for firmware-managed
 resource abstraction

On Fri, Nov 14, 2025 at 08:26:46PM +0530, Ram Kumar Dwivedi wrote:
> From: Ram Kumar Dwivedi <quic_rdwivedi@...cinc.com>
> 
> Add a compatible string for SA8255p platforms where resources such as
> PHY, clocks, regulators, and resets are managed by firmware through an
> SCMI server. Use the SCMI power protocol to abstract these resources and
> invoke power operations via runtime PM APIs (pm_runtime_get/put_sync).
> 
> Introduce vendor operations (vops) for SA8255p targets to enable SCMI-
> based resource control. In this model, capabilities like clock scaling
> and gating are not yet supported; these will be added incrementally.
> 
> Co-developed-by: Anjana Hari <quic_ahari@...cinc.com>
> Signed-off-by: Anjana Hari <quic_ahari@...cinc.com>
> Co-developed-by: Shazad Hussain <quic_shazhuss@...cinc.com>
> Signed-off-by: Shazad Hussain <quic_shazhuss@...cinc.com>
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@...cinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 161 +++++++++++++++++++++++++++++++++++-
>  drivers/ufs/host/ufs-qcom.h |   1 +
>  2 files changed, 161 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 8d119b3223cb..13ccf1fb2ebf 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -14,6 +14,7 @@
>  #include <linux/of.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/reset-controller.h>
>  #include <linux/time.h>
>  #include <linux/unaligned.h>
> @@ -619,6 +620,27 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba,
>  	return err;
>  }
>  
> +static int ufs_qcom_fw_managed_hce_enable_notify(struct ufs_hba *hba,
> +						 enum ufs_notify_change_status status)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +
> +	switch (status) {
> +	case PRE_CHANGE:
> +		ufs_qcom_select_unipro_mode(host);
> +		break;
> +	case POST_CHANGE:
> +		ufs_qcom_enable_hw_clk_gating(hba);
> +		ufs_qcom_ice_enable(host);
> +		break;
> +	default:
> +		dev_err(hba->dev, "Invalid status %d\n", status);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * ufs_qcom_cfg_timers - Configure ufs qcom cfg timers
>   *
> @@ -789,6 +811,38 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  	return ufs_qcom_ice_resume(host);
>  }
>  
> +static int ufs_qcom_fw_managed_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
> +				       enum ufs_notify_change_status status)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +
> +	if (status == PRE_CHANGE)
> +		return 0;
> +
> +	if (hba->spm_lvl != UFS_PM_LVL_5) {
> +		dev_err(hba->dev, "Unsupported spm level %d\n", hba->spm_lvl);
> +		return -EINVAL;
> +	}

You should consider moving this check to ufs-sysfs.c where the sysfs write is
handled. Failing due to unsupported suspend level at the last moment could be
avoided.

> +
> +	pm_runtime_put_sync(hba->dev);
> +
> +	return ufs_qcom_ice_suspend(host);
> +}
> +
> +static int ufs_qcom_fw_managed_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +	int err;
> +
> +	err = pm_runtime_resume_and_get(hba->dev);
> +	if (err) {
> +		dev_err(hba->dev, "PM runtime resume failed: %d\n", err);
> +		return err;
> +	}
> +
> +	return ufs_qcom_ice_resume(host);
> +}
> +
>  static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)
>  {
>  	if (host->dev_ref_clk_ctrl_mmio &&
> @@ -1421,6 +1475,52 @@ static void ufs_qcom_exit(struct ufs_hba *hba)
>  	phy_exit(host->generic_phy);
>  }
>  
> +static int ufs_qcom_fw_managed_init(struct ufs_hba *hba)
> +{
> +	struct device *dev = hba->dev;
> +	struct ufs_qcom_host *host;
> +	int err;
> +
> +	host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	host->hba = hba;
> +	ufshcd_set_variant(hba, host);
> +
> +	ufs_qcom_get_controller_revision(hba, &host->hw_ver.major,
> +					 &host->hw_ver.minor, &host->hw_ver.step);
> +
> +	err = ufs_qcom_ice_init(host);
> +	if (err)
> +		goto out_variant_clear;
> +
> +	ufs_qcom_get_default_testbus_cfg(host);
> +	err = ufs_qcom_testbus_config(host);
> +	if (err)
> +		/* Failure is non-fatal */
> +		dev_warn(dev, "Failed to configure the testbus %d\n", err);
> +
> +	hba->caps |= UFSHCD_CAP_WB_EN;
> +
> +	ufs_qcom_advertise_quirks(hba);
> +	host->hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
> +
> +	ufs_qcom_set_host_params(hba);
> +	ufs_qcom_parse_gear_limits(hba);
> +
> +	return 0;
> +
> +out_variant_clear:
> +	ufshcd_set_variant(hba, NULL);
> +	return err;
> +}
> +
> +static void ufs_qcom_fw_managed_exit(struct ufs_hba *hba)
> +{
> +	pm_runtime_put_sync(hba->dev);
> +}
> +
>  /**
>   * ufs_qcom_set_clk_40ns_cycles - Configure 40ns clk cycles
>   *
> @@ -1952,6 +2052,39 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)
>  	return 0;
>  }
>  
> +/**
> + * ufs_qcom_fw_managed_device_reset - Reset UFS device under FW-managed design

I believe this is not just device reset but both controller + device reset. So
not pretty sure that this is the right place to reset both.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists