To configure a device to a specific performance level, consumer drivers
currently need to determine the OPP based on the exact level and then
set it, resulting in code duplication across drivers.

The new helper API, dev_pm_opp_set_level(), addresses this issue by
providing a streamlined method for consumer drivers to find and set the
OPP based on the desired performance level, thereby eliminating
redundancy.

Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>

v2 -> v3
- moved function defination to pm_opp.h from core.c with inline
- updated return value with IS_ERR(opp)

v1 -> v2
- reorder sequence of tags in commit text
---
 include/linux/pm_opp.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index e7b5c602c92f..31ed8a7b554e 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -197,6 +197,28 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask)
 void dev_pm_opp_remove_table(struct device *dev);
 void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask);
 int dev_pm_opp_sync_regulators(struct device *dev);
+
+/*
+ * dev_pm_opp_set_level() - Configure device for a level
+ * @dev: device for which we do this operation
+ * @level: level to set to
+ *
+ * Return: 0 on success, a non-zero value if there is an error otherwise.
+ */
+static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
+{
+	struct dev_pm_opp *opp = dev_pm_opp_find_level_exact(dev, level);
+	int ret;
+
+	if (IS_ERR(opp))
+		return IS_ERR(opp);
+
+	ret = dev_pm_opp_set_opp(dev, opp);
+	dev_pm_opp_put(opp);
+
+	return ret;
+}
+
 #else
 static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
 {
@@ -461,6 +483,11 @@ static inline int dev_pm_opp_sync_regulators(struct device *dev)
 	return -EOPNOTSUPP;
 }
 
+static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
+{
+	return -EOPNOTSUPP;
+}
+
 #endif		/* CONFIG_PM_OPP */
 
 #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
-- 
2.17.1

On 02-05-25, 08:40, Praveen Talari wrote:
> To configure a device to a specific performance level, consumer drivers
> currently need to determine the OPP based on the exact level and then
> set it, resulting in code duplication across drivers.
> 
> The new helper API, dev_pm_opp_set_level(), addresses this issue by
> providing a streamlined method for consumer drivers to find and set the
> OPP based on the desired performance level, thereby eliminating
> redundancy.
> 
> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
> 
> v2 -> v3
> - moved function defination to pm_opp.h from core.c with inline
> - updated return value with IS_ERR(opp)
> 
> v1 -> v2
> - reorder sequence of tags in commit text

As Trilok mentioned, this is not the right place for this.

> ---
>  include/linux/pm_opp.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index e7b5c602c92f..31ed8a7b554e 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -197,6 +197,28 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask)
>  void dev_pm_opp_remove_table(struct device *dev);
>  void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask);
>  int dev_pm_opp_sync_regulators(struct device *dev);
> +
> +/*
> + * dev_pm_opp_set_level() - Configure device for a level
> + * @dev: device for which we do this operation
> + * @level: level to set to
> + *
> + * Return: 0 on success, a non-zero value if there is an error otherwise.
> + */

No need of these for simple wrappers.

> +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
> +{
> +	struct dev_pm_opp *opp = dev_pm_opp_find_level_exact(dev, level);
> +	int ret;
> +
> +	if (IS_ERR(opp))
> +		return IS_ERR(opp);

IS_ERR is wrong here, should be PTR_ERR.

> +
> +	ret = dev_pm_opp_set_opp(dev, opp);
> +	dev_pm_opp_put(opp);
> +
> +	return ret;
> +}
> +
>  #else
>  static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
>  {
> @@ -461,6 +483,11 @@ static inline int dev_pm_opp_sync_regulators(struct device *dev)
>  	return -EOPNOTSUPP;
>  }
>  
> +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
> +{
> +	return -EOPNOTSUPP;
> +}
> +

No need of these too for such wrappers. And then this isn't rebased
over latest changes in the OPP core.

I modified it and applied the below version to my tree now.

-- 
viresh

Author: Praveen Talari <quic_ptalari@quicinc.com>
Date:   Fri May 2 10:58:22 2025 +0530

    OPP: Add dev_pm_opp_set_level()

    To configure a device to a specific performance level, consumer drivers
    currently need to determine the OPP based on the exact level and then
    set it, resulting in code duplication across drivers.

    The new helper API, dev_pm_opp_set_level(), addresses this issue by
    providing a streamlined method for consumer drivers to find and set the
    OPP based on the desired performance level, thereby eliminating
    redundancy.

    Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
    [ Viresh: Lot of fixes in the code, and rebased over latest changes.
               Fixed commit log too. ]
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/pm_opp.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 8313ed981535..cf477beae4bb 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -197,6 +197,7 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask)
 void dev_pm_opp_remove_table(struct device *dev);
 void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask);
 int dev_pm_opp_sync_regulators(struct device *dev);
+
 #else
 static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
 {
@@ -717,4 +718,14 @@ static inline unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp)
        return dev_pm_opp_get_freq_indexed(opp, 0);
 }

+static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
+{
+       struct dev_pm_opp *opp __free(put_opp) = dev_pm_opp_find_level_exact(dev, level);
+
+       if (IS_ERR(opp))
+               return PTR_ERR(opp);
+
+       return dev_pm_opp_set_opp(dev, opp);
+}
+
 #endif         /* __LINUX_OPP_H__ */

HI Viresh

On 5/2/2025 11:07 AM, Viresh Kumar wrote:
> On 02-05-25, 08:40, Praveen Talari wrote:
>> To configure a device to a specific performance level, consumer drivers
>> currently need to determine the OPP based on the exact level and then
>> set it, resulting in code duplication across drivers.
>>
>> The new helper API, dev_pm_opp_set_level(), addresses this issue by
>> providing a streamlined method for consumer drivers to find and set the
>> OPP based on the desired performance level, thereby eliminating
>> redundancy.
>>
>> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
>>
>> v2 -> v3
>> - moved function defination to pm_opp.h from core.c with inline
>> - updated return value with IS_ERR(opp)
>>
>> v1 -> v2
>> - reorder sequence of tags in commit text
> As Trilok mentioned, this is not the right place for this.
>
>> ---
>>   include/linux/pm_opp.h | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
>> index e7b5c602c92f..31ed8a7b554e 100644
>> --- a/include/linux/pm_opp.h
>> +++ b/include/linux/pm_opp.h
>> @@ -197,6 +197,28 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask)
>>   void dev_pm_opp_remove_table(struct device *dev);
>>   void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask);
>>   int dev_pm_opp_sync_regulators(struct device *dev);
>> +
>> +/*
>> + * dev_pm_opp_set_level() - Configure device for a level
>> + * @dev: device for which we do this operation
>> + * @level: level to set to
>> + *
>> + * Return: 0 on success, a non-zero value if there is an error otherwise.
>> + */
> No need of these for simple wrappers.
>
>> +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
>> +{
>> +	struct dev_pm_opp *opp = dev_pm_opp_find_level_exact(dev, level);
>> +	int ret;
>> +
>> +	if (IS_ERR(opp))
>> +		return IS_ERR(opp);
> IS_ERR is wrong here, should be PTR_ERR.
>
>> +
>> +	ret = dev_pm_opp_set_opp(dev, opp);
>> +	dev_pm_opp_put(opp);
>> +
>> +	return ret;
>> +}
>> +
>>   #else
>>   static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
>>   {
>> @@ -461,6 +483,11 @@ static inline int dev_pm_opp_sync_regulators(struct device *dev)
>>   	return -EOPNOTSUPP;
>>   }
>>   
>> +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>> +
> No need of these too for such wrappers. And then this isn't rebased
> over latest changes in the OPP core.
>
> I modified it and applied the below version to my tree now.

Shall i keep commit as you suggested with your SIB.

Thanks,

Praveen

>

On 02-05-25, 13:31, Praveen Talari wrote:
> Shall i keep commit as you suggested with your SIB.

I already applied it to my tree. You can drop it from your series now.

-- 
viresh

Hi Viresh

On 5/2/2025 1:44 PM, Viresh Kumar wrote:
> On 02-05-25, 13:31, Praveen Talari wrote:
>> Shall i keep commit as you suggested with your SIB.
> I already applied it to my tree. You can drop it from your series now.

now i can push V4 right and will not face errors on my series w.r.t this 
API.


Thanks,

Praveen

>

On Fri, 2 May 2025 at 19:32, Praveen Talari <quic_ptalari@quicinc.com> wrote:
> now i can push V4 right and will not face errors on my series w.r.t this
> API.

Not fully sure what you meant, but you can send a V4 of the series,
without the first patch. Please mention it as an dependency in the
cover letter and that it is applied in the OPP tree's linux-next branch.

The one who applies your series needs to apply the series over the commit
in my branch to avoid breakage (if your series is going in 6.16-rc1).

--
Viresh

Hi

On 5/2/2025 7:41 PM, Viresh Kumar wrote:
> On Fri, 2 May 2025 at 19:32, Praveen Talari <quic_ptalari@quicinc.com> wrote:
>> now i can push V4 right and will not face errors on my series w.r.t this
>> API.
> Not fully sure what you meant, but you can send a V4 of the series,

i mean one of the patch from series is depended on patch-1, that i have 
removed from series now

so will i face any issue like kernel bot

Thanks,

Praveen Talari

> without the first patch. Please mention it as an dependency in the
> cover letter and that it is applied in the OPP tree's linux-next branch.
>
> The one who applies your series needs to apply the series over the commit
> in my branch to avoid breakage (if your series is going in 6.16-rc1).
>
> --
> Viresh

Hi Viresh

Thank you for review.

On 5/2/2025 11:07 AM, Viresh Kumar wrote:
> On 02-05-25, 08:40, Praveen Talari wrote:
>> To configure a device to a specific performance level, consumer drivers
>> currently need to determine the OPP based on the exact level and then
>> set it, resulting in code duplication across drivers.
>>
>> The new helper API, dev_pm_opp_set_level(), addresses this issue by
>> providing a streamlined method for consumer drivers to find and set the
>> OPP based on the desired performance level, thereby eliminating
>> redundancy.
>>
>> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
>>
>> v2 -> v3
>> - moved function defination to pm_opp.h from core.c with inline
>> - updated return value with IS_ERR(opp)
>>
>> v1 -> v2
>> - reorder sequence of tags in commit text
> As Trilok mentioned, this is not the right place for this.
>
>> ---
>>   include/linux/pm_opp.h | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
>> index e7b5c602c92f..31ed8a7b554e 100644
>> --- a/include/linux/pm_opp.h
>> +++ b/include/linux/pm_opp.h
>> @@ -197,6 +197,28 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask)
>>   void dev_pm_opp_remove_table(struct device *dev);
>>   void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask);
>>   int dev_pm_opp_sync_regulators(struct device *dev);
>> +
>> +/*
>> + * dev_pm_opp_set_level() - Configure device for a level
>> + * @dev: device for which we do this operation
>> + * @level: level to set to
>> + *
>> + * Return: 0 on success, a non-zero value if there is an error otherwise.
>> + */
> No need of these for simple wrappers.
>
>> +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
>> +{
>> +	struct dev_pm_opp *opp = dev_pm_opp_find_level_exact(dev, level);
>> +	int ret;
>> +
>> +	if (IS_ERR(opp))
>> +		return IS_ERR(opp);
> IS_ERR is wrong here, should be PTR_ERR.
>
>> +
>> +	ret = dev_pm_opp_set_opp(dev, opp);
>> +	dev_pm_opp_put(opp);
>> +
>> +	return ret;
>> +}
>> +
>>   #else
>>   static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
>>   {
>> @@ -461,6 +483,11 @@ static inline int dev_pm_opp_sync_regulators(struct device *dev)
>>   	return -EOPNOTSUPP;
>>   }
>>   
>> +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>> +
> No need of these too for such wrappers. And then this isn't rebased
How come? i have synced  linux-next today itself and pushed from it, 
even i didn't face any issue.
Let me know how/where rebased i.e linux-next or linux?
> over latest changes in the OPP core.
>
> I modified it and applied the below version to my tree now.
>

On 02-05-25, 13:07, Praveen Talari wrote:
> How come? i have synced  linux-next today itself and pushed from it, even i
> didn't face any issue.

Yeah, you won't face any issues, but scope-based cleanup helpers were
added recently and should have been used here, though it wasn't
obvious, so its fine.

> Let me know how/where rebased i.e linux-next or linux?

linux-next should be fine normally.

-- 
viresh

On 5/1/2025 8:10 PM, Praveen Talari wrote:
> To configure a device to a specific performance level, consumer drivers
> currently need to determine the OPP based on the exact level and then
> set it, resulting in code duplication across drivers.
> 
> The new helper API, dev_pm_opp_set_level(), addresses this issue by
> providing a streamlined method for consumer drivers to find and set the
> OPP based on the desired performance level, thereby eliminating
> redundancy.
> 
> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
> 
> v2 -> v3
> - moved function defination to pm_opp.h from core.c with inline
> - updated return value with IS_ERR(opp)
> 
> v1 -> v2
> - reorder sequence of tags in commit text

The version log above will go into the commit text. Please keep it after
---. 

> ---
>  include/linux/pm_opp.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index e7b5c602c92f..31ed8a7b554e 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -197,6 +197,28 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask)
>  void dev_pm_opp_remove_table(struct device *dev);
>  void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask);
>  int dev_pm_opp_sync_regulators(struct device *dev);
> +
> +/*
> + * dev_pm_opp_set_level() - Configure device for a level
> + * @dev: device for which we do this operation
> + * @level: level to set to
> + *
> + * Return: 0 on success, a non-zero value if there is an error otherwise.
> + */
> +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
> +{
> +	struct dev_pm_opp *opp = dev_pm_opp_find_level_exact(dev, level);
> +	int ret;
> +
> +	if (IS_ERR(opp))
> +		return IS_ERR(opp);
> +
> +	ret = dev_pm_opp_set_opp(dev, opp);
> +	dev_pm_opp_put(opp);
> +
> +	return ret;
> +}
> +
>  #else
>  static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
>  {
> @@ -461,6 +483,11 @@ static inline int dev_pm_opp_sync_regulators(struct device *dev)
>  	return -EOPNOTSUPP;
>  }
>  
> +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  #endif		/* CONFIG_PM_OPP */
>  
>  #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)


-- 
---Trilok Soni