| Message ID | 20220930094821.31665-1-lukasz.luba@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] cpufreq: Change macro for store scaling min/max frequency | expand |
On 30-09-22, 10:48, Lukasz Luba wrote: > In order to prepare extension to the store_scaling_max_freq() remove > the macro and use two normal functions. The set value for max frequency > is important for the task scheduler. > > Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> > --- > drivers/cpufreq/cpufreq.c | 47 ++++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 20 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 69b3d61852ac..1f8b93f42c76 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -715,26 +715,33 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf) > return ret; > } > > -/* > - * cpufreq_per_cpu_attr_write() / store_##file_name() - sysfs write access > - */ > -#define store_one(file_name, object) \ > -static ssize_t store_##file_name \ > -(struct cpufreq_policy *policy, const char *buf, size_t count) \ > -{ \ > - unsigned long val; \ > - int ret; \ > - \ > - ret = sscanf(buf, "%lu", &val); \ > - if (ret != 1) \ > - return -EINVAL; \ > - \ > - ret = freq_qos_update_request(policy->object##_freq_req, val);\ > - return ret >= 0 ? count : ret; \ > -} > - > -store_one(scaling_min_freq, min); > -store_one(scaling_max_freq, max); > +static ssize_t store_scaling_max_freq > +(struct cpufreq_policy *policy, const char *buf, size_t count) I thought we can't break right after function's name according to kernel's coding guidelines. Did you run checkpatch on this ? > +{ > + unsigned long val; > + int ret; > + > + ret = sscanf(buf, "%lu", &val); > + if (ret != 1) > + return -EINVAL; > + > + ret = freq_qos_update_request(policy->max_freq_req, val); > + return ret >= 0 ? count : ret; > +} > + > +static ssize_t store_scaling_min_freq > +(struct cpufreq_policy *policy, const char *buf, size_t count) > +{ > + unsigned long val; > + int ret; > + > + ret = sscanf(buf, "%lu", &val); > + if (ret != 1) > + return -EINVAL; > + > + ret = freq_qos_update_request(policy->min_freq_req, val); > + return ret >= 0 ? count : ret; > +} > > /* > * show_cpuinfo_cur_freq - current CPU frequency as detected by hardware > -- > 2.17.1
Hi Viresh, On 10/10/22 06:36, Viresh Kumar wrote: > On 30-09-22, 10:48, Lukasz Luba wrote: >> In order to prepare extension to the store_scaling_max_freq() remove >> the macro and use two normal functions. The set value for max frequency >> is important for the task scheduler. >> >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> >> --- >> drivers/cpufreq/cpufreq.c | 47 ++++++++++++++++++++++----------------- >> 1 file changed, 27 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >> index 69b3d61852ac..1f8b93f42c76 100644 >> --- a/drivers/cpufreq/cpufreq.c >> +++ b/drivers/cpufreq/cpufreq.c >> @@ -715,26 +715,33 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf) >> return ret; >> } >> >> -/* >> - * cpufreq_per_cpu_attr_write() / store_##file_name() - sysfs write access >> - */ >> -#define store_one(file_name, object) \ >> -static ssize_t store_##file_name \ >> -(struct cpufreq_policy *policy, const char *buf, size_t count) \ >> -{ \ >> - unsigned long val; \ >> - int ret; \ >> - \ >> - ret = sscanf(buf, "%lu", &val); \ >> - if (ret != 1) \ >> - return -EINVAL; \ >> - \ >> - ret = freq_qos_update_request(policy->object##_freq_req, val);\ >> - return ret >= 0 ? count : ret; \ >> -} >> - >> -store_one(scaling_min_freq, min); >> -store_one(scaling_max_freq, max); >> +static ssize_t store_scaling_max_freq >> +(struct cpufreq_policy *policy, const char *buf, size_t count) > > I thought we can't break right after function's name according to > kernel's coding guidelines. Did you run checkpatch on this ? The checkpatch didn't capture this, but you are right. I'll change this. ----------------------------------------------- /tmp/cpufreq//0001-cpufreq-Change-macro-for-store-scaling-min-max-frequ.patch WARNING: Prefer kstrto<type> to single variable sscanf #49: FILE: drivers/cpufreq/cpufreq.c:724: + ret = sscanf(buf, "%lu", &val); WARNING: Prefer kstrto<type> to single variable sscanf #63: FILE: drivers/cpufreq/cpufreq.c:738: + ret = sscanf(buf, "%lu", &val); total: 0 errors, 2 warnings, 53 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /tmp/cpufreq//0001-cpufreq-Change-macro-for-store-scaling-min-max-frequ.patch has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. ============================================================== /tmp/cpufreq//0002-cpufreq-Update-CPU-capacity-reduction-in-store_scali.patch total: 0 errors, 0 warnings, 36 lines checked /tmp/cpufreq//0002-cpufreq-Update-CPU-capacity-reduction-in-store_scali.patch has no obvious style problems and is ready for submission. ============================================================== -----------------------------------------------
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 69b3d61852ac..1f8b93f42c76 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -715,26 +715,33 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf) return ret; } -/* - * cpufreq_per_cpu_attr_write() / store_##file_name() - sysfs write access - */ -#define store_one(file_name, object) \ -static ssize_t store_##file_name \ -(struct cpufreq_policy *policy, const char *buf, size_t count) \ -{ \ - unsigned long val; \ - int ret; \ - \ - ret = sscanf(buf, "%lu", &val); \ - if (ret != 1) \ - return -EINVAL; \ - \ - ret = freq_qos_update_request(policy->object##_freq_req, val);\ - return ret >= 0 ? count : ret; \ -} - -store_one(scaling_min_freq, min); -store_one(scaling_max_freq, max); +static ssize_t store_scaling_max_freq +(struct cpufreq_policy *policy, const char *buf, size_t count) +{ + unsigned long val; + int ret; + + ret = sscanf(buf, "%lu", &val); + if (ret != 1) + return -EINVAL; + + ret = freq_qos_update_request(policy->max_freq_req, val); + return ret >= 0 ? count : ret; +} + +static ssize_t store_scaling_min_freq +(struct cpufreq_policy *policy, const char *buf, size_t count) +{ + unsigned long val; + int ret; + + ret = sscanf(buf, "%lu", &val); + if (ret != 1) + return -EINVAL; + + ret = freq_qos_update_request(policy->min_freq_req, val); + return ret >= 0 ? count : ret; +} /* * show_cpuinfo_cur_freq - current CPU frequency as detected by hardware
In order to prepare extension to the store_scaling_max_freq() remove the macro and use two normal functions. The set value for max frequency is important for the task scheduler. Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> --- drivers/cpufreq/cpufreq.c | 47 ++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 20 deletions(-)