Re: [PATCH V1 3/3] ufs: ufs-qcom: Add support for firmware-managed resource abstraction

[Manivannan Sadhasivam]

On Wed, Dec 10, 2025 at 09:33:08PM +0530, Ram Kumar Dwivedi wrote:
> 
> 
> On 20-Nov-25 11:23 AM, Manivannan Sadhasivam wrote:
> > On Fri, Nov 14, 2025 at 08:26:46PM +0530, Ram Kumar Dwivedi wrote:
> >> From: Ram Kumar Dwivedi <quic_rdwivedi@xxxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxxx>
> >> Signed-off-by: Anjana Hari <quic_ahari@xxxxxxxxxxx>
> >> Co-developed-by: Shazad Hussain <quic_shazhuss@xxxxxxxxxxx>
> >> Signed-off-by: Shazad Hussain <quic_shazhuss@xxxxxxxxxxx>
> >> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@xxxxxxxxxxx>
> >> ---
> >>  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.
> 
> Hi Mani,
> 
> We have planned to support other spm levels also in follow up series
> once the basic UFS SCMI functionality is upstreamed.  This spm_lvl check
> is intended as a temporary safeguard while we only support SPM level 5. 
> If you'd still prefer a change, I caupdate this in the next patchset.
> 

Please do it now as I don't see it logical to error out in suspend callback.

- Mani

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