| Message ID | 20250418151235.27787-2-quic_ptalari@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/9] opp: add new helper API dev_pm_opp_set_level() | expand |
Thank viresh for review. On 4/21/2025 1:10 PM, Viresh Kumar wrote: > On 18-04-25, 20:42, Praveen Talari wrote: >> diff --git a/drivers/opp/core.c b/drivers/opp/core.c >> index 73e9a3b2f29b..a9bca9502f71 100644 >> --- a/drivers/opp/core.c >> +++ b/drivers/opp/core.c >> @@ -3151,3 +3151,25 @@ void dev_pm_opp_remove_table(struct device *dev) >> dev_pm_opp_put_opp_table(opp_table); >> } >> EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table); >> + >> +/* >> + * 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 negative error number otherwise. >> + */ >> +int dev_pm_opp_set_level(struct device *dev, unsigned int level) > I would rather move this to pm_opp.h as an inline helper. most of helper APIs in core.c and even i don't see any helper API in pm_opp.c. please let me know if you still need to add in pm_opp.h. > >> +{ >> + struct dev_pm_opp *opp = dev_pm_opp_find_level_exact(dev, level); >> + int ret; >> + >> + if (IS_ERR(opp)) >> + return -EINVAL; > Why not reuse the same error value ? as reference of APIs in core.c, i have used -EINVAl instead of IS_ERR(opp). Let me know your thoughts on return value. > >> + >> + ret = dev_pm_opp_set_opp(dev, opp); >> + dev_pm_opp_put(opp); >> + >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(dev_pm_opp_set_level); > Make the changes and send it separately (or with the series, your > choice), I will apply it to the OPP tree. Thanks. >
Hi Viresh Thank you for reviewing. On 4/23/2025 11:06 AM, Viresh Kumar wrote: > On 22-04-25, 22:37, Praveen Talari wrote: >> most of helper APIs in core.c and even i don't see any helper API in >> pm_opp.c. > This is more of a wrapper over the existing C routines which is being > added to reduce some boilerplate code from drivers. And so it makes > sense to add this as an inline helper. May be there are others which > can be moved too. i agree and move to pm_opp.h. Will update in next > >> as reference of APIs in core.c, i have used -EINVAl instead of IS_ERR(opp). > That would likely be wrong, maybe we should fix those too. Ok i will update as per if statement check for return as well. >
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 73e9a3b2f29b..a9bca9502f71 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -3151,3 +3151,25 @@ void dev_pm_opp_remove_table(struct device *dev) dev_pm_opp_put_opp_table(opp_table); } EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table); + +/* + * 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 negative error number otherwise. + */ +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 -EINVAL; + + ret = dev_pm_opp_set_opp(dev, opp); + dev_pm_opp_put(opp); + + return ret; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_set_level); diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index c247317aae38..c17271947f83 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -196,6 +196,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); +int dev_pm_opp_set_level(struct device *dev, unsigned int level); #else static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev) { @@ -454,6 +455,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)