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

[Ram Kumar Dwivedi <ram.dwivedi@oss.qualcomm.com>]

On 12-Dec-25 6:15 AM, Manivannan Sadhasivam wrote:
> 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@quicinc.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@quicinc.com>
>>>> Signed-off-by: Anjana Hari <quic_ahari@quicinc.com>
>>>> Co-developed-by: Shazad Hussain <quic_shazhuss@quicinc.com>
>>>> Signed-off-by: Shazad Hussain <quic_shazhuss@quicinc.com>
>>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.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.
>>
>> 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.
Hi Mani,

I have addressed the issue in latest patchset.

Thanks,
Ram.


> 
> - Mani
>