2022-09-30 10:13:16

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH 1/2] cpufreq: Change macro for store scaling min/max frequency

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 <[email protected]>
---
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)
+{
+ 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


2022-10-10 06:14:21

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Change macro for store scaling min/max frequency

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 <[email protected]>
> ---
> 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

--
viresh

2022-10-10 09:10:57

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH 1/2] cpufreq: Change macro for store scaling min/max frequency

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 <[email protected]>
>> ---
>> 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.
==============================================================

-----------------------------------------------