[v4] ufs: ufs-qcom: Add support firmware managed platforms

[PATCH V4 3/4] scsi: ufs: core Enforce minimum pm level for sysfs configuration

Posted by Ram Kumar Dwivedi 1 month ago

Some UFS platforms only support a limited subset of power levels.
Currently, the sysfs interface allows users to set any pm level
without validating the minimum supported value. If an unsupported
level is selected, suspend may fail.

Introduce an pm_lvl_min field in the ufs_hba structure and use it
to clamp the pm level requested via sysfs so that only supported
levels are accepted. Platforms that require a minimum pm level
can set this field during probe.

Signed-off-by: Ram Kumar Dwivedi <ram.dwivedi@oss.qualcomm.com>
---
 drivers/ufs/core/ufs-sysfs.c | 2 +-
 include/ufs/ufshcd.h         | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index b33f8656edb5..02e5468ad49d 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -141,7 +141,7 @@ static inline ssize_t ufs_sysfs_pm_lvl_store(struct device *dev,
 	if (kstrtoul(buf, 0, &value))
 		return -EINVAL;
 
-	if (value >= UFS_PM_LVL_MAX)
+	if (value >= UFS_PM_LVL_MAX || value < hba->pm_lvl_min)
 		return -EINVAL;
 
 	if (ufs_pm_lvl_states[value].dev_state == UFS_DEEPSLEEP_PWR_MODE &&
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 19154228780b..ac8697a7355b 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -972,6 +972,7 @@ struct ufs_hba {
 	enum ufs_pm_level rpm_lvl;
 	/* Desired UFS power management level during system PM */
 	enum ufs_pm_level spm_lvl;
+	enum ufs_pm_level pm_lvl_min;
 	int pm_op_in_progress;
 
 	/* Auto-Hibernate Idle Timer register value */
-- 
2.34.1

Re: [PATCH V4 3/4] scsi: ufs: core Enforce minimum pm level for sysfs configuration

Posted by Shawn Lin 1 month ago

在 2026/01/06 星期二 21:40, Ram Kumar Dwivedi 写道:
> Some UFS platforms only support a limited subset of power levels.
> Currently, the sysfs interface allows users to set any pm level
> without validating the minimum supported value. If an unsupported
> level is selected, suspend may fail.
> 
> Introduce an pm_lvl_min field in the ufs_hba structure and use it
> to clamp the pm level requested via sysfs so that only supported
> levels are accepted. Platforms that require a minimum pm level
> can set this field during probe.
> 
> Signed-off-by: Ram Kumar Dwivedi <ram.dwivedi@oss.qualcomm.com>
> ---
>   drivers/ufs/core/ufs-sysfs.c | 2 +-
>   include/ufs/ufshcd.h         | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
> index b33f8656edb5..02e5468ad49d 100644
> --- a/drivers/ufs/core/ufs-sysfs.c
> +++ b/drivers/ufs/core/ufs-sysfs.c
> @@ -141,7 +141,7 @@ static inline ssize_t ufs_sysfs_pm_lvl_store(struct device *dev,
>   	if (kstrtoul(buf, 0, &value))
>   		return -EINVAL;
>   
> -	if (value >= UFS_PM_LVL_MAX)
> +	if (value >= UFS_PM_LVL_MAX || value < hba->pm_lvl_min)

It makes sense that some platform support a limited subset of power
levels. But each level is in increasing order of power savings, and you
set it to UFS_PM_LVL_5. Don't you support UFS_PM_LVL_0 the full active
mode?

>   		return -EINVAL;
>   
>   	if (ufs_pm_lvl_states[value].dev_state == UFS_DEEPSLEEP_PWR_MODE &&
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 19154228780b..ac8697a7355b 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -972,6 +972,7 @@ struct ufs_hba {
>   	enum ufs_pm_level rpm_lvl;
>   	/* Desired UFS power management level during system PM */
>   	enum ufs_pm_level spm_lvl;
> +	enum ufs_pm_level pm_lvl_min;
>   	int pm_op_in_progress;
>   
>   	/* Auto-Hibernate Idle Timer register value */

Re: [PATCH V4 3/4] scsi: ufs: core Enforce minimum pm level for sysfs configuration

Posted by Manivannan Sadhasivam 1 month ago

On Tue, Jan 06, 2026 at 10:27:29PM +0800, Shawn Lin wrote:
> 在 2026/01/06 星期二 21:40, Ram Kumar Dwivedi 写道:
> > Some UFS platforms only support a limited subset of power levels.
> > Currently, the sysfs interface allows users to set any pm level
> > without validating the minimum supported value. If an unsupported
> > level is selected, suspend may fail.
> > 
> > Introduce an pm_lvl_min field in the ufs_hba structure and use it
> > to clamp the pm level requested via sysfs so that only supported
> > levels are accepted. Platforms that require a minimum pm level
> > can set this field during probe.
> > 
> > Signed-off-by: Ram Kumar Dwivedi <ram.dwivedi@oss.qualcomm.com>
> > ---
> >   drivers/ufs/core/ufs-sysfs.c | 2 +-
> >   include/ufs/ufshcd.h         | 1 +
> >   2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
> > index b33f8656edb5..02e5468ad49d 100644
> > --- a/drivers/ufs/core/ufs-sysfs.c
> > +++ b/drivers/ufs/core/ufs-sysfs.c
> > @@ -141,7 +141,7 @@ static inline ssize_t ufs_sysfs_pm_lvl_store(struct device *dev,
> >   	if (kstrtoul(buf, 0, &value))
> >   		return -EINVAL;
> > -	if (value >= UFS_PM_LVL_MAX)
> > +	if (value >= UFS_PM_LVL_MAX || value < hba->pm_lvl_min)
> 
> It makes sense that some platform support a limited subset of power
> levels. But each level is in increasing order of power savings, and you
> set it to UFS_PM_LVL_5. Don't you support UFS_PM_LVL_0 the full active
> mode?
> 

These are the suspend levels, not runtime levels. So yes, our platform doesn't
support full power mode when it is in suspend state.

- Mani

> >   		return -EINVAL;
> >   	if (ufs_pm_lvl_states[value].dev_state == UFS_DEEPSLEEP_PWR_MODE &&
> > diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> > index 19154228780b..ac8697a7355b 100644
> > --- a/include/ufs/ufshcd.h
> > +++ b/include/ufs/ufshcd.h
> > @@ -972,6 +972,7 @@ struct ufs_hba {
> >   	enum ufs_pm_level rpm_lvl;
> >   	/* Desired UFS power management level during system PM */
> >   	enum ufs_pm_level spm_lvl;
> > +	enum ufs_pm_level pm_lvl_min;
> >   	int pm_op_in_progress;
> >   	/* Auto-Hibernate Idle Timer register value */
> 

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

Re: [PATCH V4 3/4] scsi: ufs: core Enforce minimum pm level for sysfs configuration

Posted by Bart Van Assche 1 month ago

On 1/6/26 5:40 AM, Ram Kumar Dwivedi wrote:
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 19154228780b..ac8697a7355b 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -972,6 +972,7 @@ struct ufs_hba {
>   	enum ufs_pm_level rpm_lvl;
>   	/* Desired UFS power management level during system PM */
>   	enum ufs_pm_level spm_lvl;
> +	enum ufs_pm_level pm_lvl_min;
>   	int pm_op_in_progress;
>   
>   	/* Auto-Hibernate Idle Timer register value */

Please do not introduce new kernel-doc warnings and update the 
documentation block above this data structure when adding new members.

Thanks,

Bart.

Re: [PATCH V4 3/4] scsi: ufs: core Enforce minimum pm level for sysfs configuration

Posted by Ram Kumar Dwivedi 3 weeks, 5 days ago


On 06-Jan-26 7:26 PM, Bart Van Assche wrote:
> On 1/6/26 5:40 AM, Ram Kumar Dwivedi wrote:
>> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
>> index 19154228780b..ac8697a7355b 100644
>> --- a/include/ufs/ufshcd.h
>> +++ b/include/ufs/ufshcd.h
>> @@ -972,6 +972,7 @@ struct ufs_hba {
>>       enum ufs_pm_level rpm_lvl;
>>       /* Desired UFS power management level during system PM */
>>       enum ufs_pm_level spm_lvl;
>> +    enum ufs_pm_level pm_lvl_min;
>>       int pm_op_in_progress;
>>         /* Auto-Hibernate Idle Timer register value */
> 
> Please do not introduce new kernel-doc warnings and update the documentation block above this data structure when adding new members.

Hi Bart,

I will add the kernel-doc description for pm_lvl_min in the next version.

Thanks, 
Ram


> 
> Thanks,
> 
> Bart.