1. Patchew
  2. linux
  3. [PATCH v3 0/8] Enhanced autonomous selection and improvements
  4. View patch

[PATCH v3 1/8] cpufreq: CPPC: Add generic helpers for sysfs show/store

Sumit Gupta posted 8 patches 4 months, 1 week ago
There is a newer version of this series
[PATCH v3 1/8] cpufreq: CPPC: Add generic helpers for sysfs show/store
Posted by Sumit Gupta 4 months, 1 week ago 
Add generic show/store helper functions for u64 sysfs attributes:
- cppc_cpufreq_sysfs_show_u64()
- cppc_cpufreq_sysfs_store_u64()

Refactor auto_act_window and energy_performance_preference_val
attributes to use these helpers, eliminating code duplication.

No functional changes.

Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/cpufreq/cppc_cpufreq.c | 87 ++++++++++++++--------------------
 1 file changed, 35 insertions(+), 52 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 12de0ac7bbaf..732f35096991 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -781,6 +781,36 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
 	return 0;
 }
 
+static ssize_t cppc_cpufreq_sysfs_show_u64(unsigned int cpu, int (*get_func)(int, u64 *), char *buf)
+{
+	u64 val;
+	int ret = get_func(cpu, &val);
+
+	if (ret == -EOPNOTSUPP)
+		return sysfs_emit(buf, "<unsupported>\n");
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%llu\n", val);
+}
+
+static ssize_t cppc_cpufreq_sysfs_store_u64(const char *buf, size_t count,
+					    int (*set_func)(int, u64), unsigned int cpu)
+{
+	u64 val;
+	int ret;
+
+	if (!buf || !set_func)
+		return -EINVAL;
+
+	ret = kstrtou64(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = set_func((int)cpu, val);
+	return ret ? ret : count;
+}
+
 static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
 {
 	struct cppc_cpudata *cpu_data = policy->driver_data;
@@ -824,70 +854,23 @@ static ssize_t store_auto_select(struct cpufreq_policy *policy,
 
 static ssize_t show_auto_act_window(struct cpufreq_policy *policy, char *buf)
 {
-	u64 val;
-	int ret;
-
-	ret = cppc_get_auto_act_window(policy->cpu, &val);
-
-	/* show "<unsupported>" when this register is not supported by cpc */
-	if (ret == -EOPNOTSUPP)
-		return sysfs_emit(buf, "<unsupported>\n");
-
-	if (ret)
-		return ret;
-
-	return sysfs_emit(buf, "%llu\n", val);
+	return cppc_cpufreq_sysfs_show_u64(policy->cpu, cppc_get_auto_act_window, buf);
 }
 
-static ssize_t store_auto_act_window(struct cpufreq_policy *policy,
-				     const char *buf, size_t count)
+static ssize_t store_auto_act_window(struct cpufreq_policy *policy, const char *buf, size_t count)
 {
-	u64 usec;
-	int ret;
-
-	ret = kstrtou64(buf, 0, &usec);
-	if (ret)
-		return ret;
-
-	ret = cppc_set_auto_act_window(policy->cpu, usec);
-	if (ret)
-		return ret;
-
-	return count;
+	return cppc_cpufreq_sysfs_store_u64(buf, count, cppc_set_auto_act_window, policy->cpu);
 }
 
 static ssize_t show_energy_performance_preference_val(struct cpufreq_policy *policy, char *buf)
 {
-	u64 val;
-	int ret;
-
-	ret = cppc_get_epp_perf(policy->cpu, &val);
-
-	/* show "<unsupported>" when this register is not supported by cpc */
-	if (ret == -EOPNOTSUPP)
-		return sysfs_emit(buf, "<unsupported>\n");
-
-	if (ret)
-		return ret;
-
-	return sysfs_emit(buf, "%llu\n", val);
+	return cppc_cpufreq_sysfs_show_u64(policy->cpu, cppc_get_epp_perf, buf);
 }
 
 static ssize_t store_energy_performance_preference_val(struct cpufreq_policy *policy,
 						       const char *buf, size_t count)
 {
-	u64 val;
-	int ret;
-
-	ret = kstrtou64(buf, 0, &val);
-	if (ret)
-		return ret;
-
-	ret = cppc_set_epp(policy->cpu, val);
-	if (ret)
-		return ret;
-
-	return count;
+	return cppc_cpufreq_sysfs_store_u64(buf, count, cppc_set_epp, policy->cpu);
 }
 
 cpufreq_freq_attr_ro(freqdomain_cpus);
-- 
2.34.1
Re: [PATCH v3 1/8] cpufreq: CPPC: Add generic helpers for sysfs show/store
Posted by Jie Zhan 4 months ago 
Hi Sumit,

On 10/1/2025 11:00 PM, Sumit Gupta wrote:
> Add generic show/store helper functions for u64 sysfs attributes:
> - cppc_cpufreq_sysfs_show_u64()
> - cppc_cpufreq_sysfs_store_u64()
> 
> Refactor auto_act_window and energy_performance_preference_val
> attributes to use these helpers, eliminating code duplication.
> 
> No functional changes.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
Nice cleanup in general.  Some minor bits inline.
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 87 ++++++++++++++--------------------
>  1 file changed, 35 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 12de0ac7bbaf..732f35096991 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -781,6 +781,36 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
>  	return 0;
>  }
>  
> +static ssize_t cppc_cpufreq_sysfs_show_u64(unsigned int cpu, int (*get_func)(int, u64 *), char *buf)
Wrap a bit into 80 chars?

BTW, trivial but I would prefer a symmetric param order, like:
show(buf, get_func, cpu)
store(buf, count, set_func, cpu)
> +{
> +	u64 val;
> +	int ret = get_func(cpu, &val);
> +
> +	if (ret == -EOPNOTSUPP)
> +		return sysfs_emit(buf, "<unsupported>\n");
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%llu\n", val);
> +}
> +
> +static ssize_t cppc_cpufreq_sysfs_store_u64(const char *buf, size_t count,
> +					    int (*set_func)(int, u64), unsigned int cpu)
> +{
> +	u64 val;
> +	int ret;
> +
> +	if (!buf || !set_func)
> +		return -EINVAL;
No need.
> +
> +	ret = kstrtou64(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = set_func((int)cpu, val);
> +	return ret ? ret : count;
I suppose it's preferred to avoid using ternary operators like this.
> +}
> +
Would be nicer to move cppc_cpufreq_sysfs_show/store_u64() to just above
where they are used, i.e. just before show_auto_act_window().
>  static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>  {
>  	struct cppc_cpudata *cpu_data = policy->driver_data;
> @@ -824,70 +854,23 @@ static ssize_t store_auto_select(struct cpufreq_policy *policy,
>  
>  static ssize_t show_auto_act_window(struct cpufreq_policy *policy, char *buf)
>  {
> -	u64 val;
> -	int ret;
> -
> -	ret = cppc_get_auto_act_window(policy->cpu, &val);
> -
> -	/* show "<unsupported>" when this register is not supported by cpc */
> -	if (ret == -EOPNOTSUPP)
> -		return sysfs_emit(buf, "<unsupported>\n");
> -
> -	if (ret)
> -		return ret;
> -
> -	return sysfs_emit(buf, "%llu\n", val);
> +	return cppc_cpufreq_sysfs_show_u64(policy->cpu, cppc_get_auto_act_window, buf);
>  }
>  
> -static ssize_t store_auto_act_window(struct cpufreq_policy *policy,
> -				     const char *buf, size_t count)
> +static ssize_t store_auto_act_window(struct cpufreq_policy *policy, const char *buf, size_t count)
>  {
> -	u64 usec;
> -	int ret;
> -
> -	ret = kstrtou64(buf, 0, &usec);
> -	if (ret)
> -		return ret;
> -
> -	ret = cppc_set_auto_act_window(policy->cpu, usec);
> -	if (ret)
> -		return ret;
> -
> -	return count;
> +	return cppc_cpufreq_sysfs_store_u64(buf, count, cppc_set_auto_act_window, policy->cpu);
>  }
>  
>  static ssize_t show_energy_performance_preference_val(struct cpufreq_policy *policy, char *buf)
>  {
> -	u64 val;
> -	int ret;
> -
> -	ret = cppc_get_epp_perf(policy->cpu, &val);
> -
> -	/* show "<unsupported>" when this register is not supported by cpc */
> -	if (ret == -EOPNOTSUPP)
> -		return sysfs_emit(buf, "<unsupported>\n");
> -
> -	if (ret)
> -		return ret;
> -
> -	return sysfs_emit(buf, "%llu\n", val);
> +	return cppc_cpufreq_sysfs_show_u64(policy->cpu, cppc_get_epp_perf, buf);
>  }
>  
>  static ssize_t store_energy_performance_preference_val(struct cpufreq_policy *policy,
>  						       const char *buf, size_t count)
>  {
> -	u64 val;
> -	int ret;
> -
> -	ret = kstrtou64(buf, 0, &val);
> -	if (ret)
> -		return ret;
> -
> -	ret = cppc_set_epp(policy->cpu, val);
> -	if (ret)
> -		return ret;
> -
> -	return count;
> +	return cppc_cpufreq_sysfs_store_u64(buf, count, cppc_set_epp, policy->cpu);
>  }
>  
>  cpufreq_freq_attr_ro(freqdomain_cpus);
Re: [PATCH v3 1/8] cpufreq: CPPC: Add generic helpers for sysfs show/store
Posted by Sumit Gupta 3 months, 3 weeks ago 
On 10/10/25 08:54, Jie Zhan wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Sumit,
>
> On 10/1/2025 11:00 PM, Sumit Gupta wrote:
>> Add generic show/store helper functions for u64 sysfs attributes:
>> - cppc_cpufreq_sysfs_show_u64()
>> - cppc_cpufreq_sysfs_store_u64()
>>
>> Refactor auto_act_window and energy_performance_preference_val
>> attributes to use these helpers, eliminating code duplication.
>>
>> No functional changes.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> Nice cleanup in general.  Some minor bits inline.
>> ---
>>   drivers/cpufreq/cppc_cpufreq.c | 87 ++++++++++++++--------------------
>>   1 file changed, 35 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 12de0ac7bbaf..732f35096991 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -781,6 +781,36 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
>>        return 0;
>>   }
>>
>> +static ssize_t cppc_cpufreq_sysfs_show_u64(unsigned int cpu, int (*get_func)(int, u64 *), char *buf)
> Wrap a bit into 80 chars?

Wrapped line length to 100 as per the max limit.

  $ grep "max_line_length =" scripts/checkpatch.pl
  my $max_line_length = 100;


>
> BTW, trivial but I would prefer a symmetric param order, like:
> show(buf, get_func, cpu)
> store(buf, count, set_func, cpu)

Sure, will change the param order.


>> +{
>> +     u64 val;
>> +     int ret = get_func(cpu, &val);
>> +
>> +     if (ret == -EOPNOTSUPP)
>> +             return sysfs_emit(buf, "<unsupported>\n");
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%llu\n", val);
>> +}
>> +
>> +static ssize_t cppc_cpufreq_sysfs_store_u64(const char *buf, size_t count,
>> +                                         int (*set_func)(int, u64), unsigned int cpu)
>> +{
>> +     u64 val;
>> +     int ret;
>> +
>> +     if (!buf || !set_func)
>> +             return -EINVAL;
> No need.

Ok.


>> +
>> +     ret = kstrtou64(buf, 0, &val);
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = set_func((int)cpu, val);
>> +     return ret ? ret : count;
> I suppose it's preferred to avoid using ternary operators like this.

These are commonly use. e.g. within cpufreq:

  $ grep -inr "return ret ?" drivers/cpufreq/*
  drivers/cpufreq/amd-pstate.c:1184:      return ret ? ret : count;
  drivers/cpufreq/cpufreq.c:839:  return ret ? ret : count;
  drivers/cpufreq/intel_pstate.c:897:     return ret ?: count;

>> +}
>> +
> Would be nicer to move cppc_cpufreq_sysfs_show/store_u64() to just above
> where they are used, i.e. just before show_auto_act_window().

Sure.

Thank you,
Sumit

>>   static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>>   {
>>        struct cppc_cpudata *cpu_data = policy->driver_data;
>> @@ -824,70 +854,23 @@ static ssize_t store_auto_select(struct cpufreq_policy *policy,
>>
>>   static ssize_t show_auto_act_window(struct cpufreq_policy *policy, char *buf)
>>   {
>> -     u64 val;
>> -     int ret;
>> -
>> -     ret = cppc_get_auto_act_window(policy->cpu, &val);
>> -
>> -     /* show "<unsupported>" when this register is not supported by cpc */
>> -     if (ret == -EOPNOTSUPP)
>> -             return sysfs_emit(buf, "<unsupported>\n");
>> -
>> -     if (ret)
>> -             return ret;
>> -
>> -     return sysfs_emit(buf, "%llu\n", val);
>> +     return cppc_cpufreq_sysfs_show_u64(policy->cpu, cppc_get_auto_act_window, buf);
>>   }
>>
>> -static ssize_t store_auto_act_window(struct cpufreq_policy *policy,
>> -                                  const char *buf, size_t count)
>> +static ssize_t store_auto_act_window(struct cpufreq_policy *policy, const char *buf, size_t count)
>>   {
>> -     u64 usec;
>> -     int ret;
>> -
>> -     ret = kstrtou64(buf, 0, &usec);
>> -     if (ret)
>> -             return ret;
>> -
>> -     ret = cppc_set_auto_act_window(policy->cpu, usec);
>> -     if (ret)
>> -             return ret;
>> -
>> -     return count;
>> +     return cppc_cpufreq_sysfs_store_u64(buf, count, cppc_set_auto_act_window, policy->cpu);
>>   }
>>
>>   static ssize_t show_energy_performance_preference_val(struct cpufreq_policy *policy, char *buf)
>>   {
>> -     u64 val;
>> -     int ret;
>> -
>> -     ret = cppc_get_epp_perf(policy->cpu, &val);
>> -
>> -     /* show "<unsupported>" when this register is not supported by cpc */
>> -     if (ret == -EOPNOTSUPP)
>> -             return sysfs_emit(buf, "<unsupported>\n");
>> -
>> -     if (ret)
>> -             return ret;
>> -
>> -     return sysfs_emit(buf, "%llu\n", val);
>> +     return cppc_cpufreq_sysfs_show_u64(policy->cpu, cppc_get_epp_perf, buf);
>>   }
>>
>>   static ssize_t store_energy_performance_preference_val(struct cpufreq_policy *policy,
>>                                                       const char *buf, size_t count)
>>   {
>> -     u64 val;
>> -     int ret;
>> -
>> -     ret = kstrtou64(buf, 0, &val);
>> -     if (ret)
>> -             return ret;
>> -
>> -     ret = cppc_set_epp(policy->cpu, val);
>> -     if (ret)
>> -             return ret;
>> -
>> -     return count;
>> +     return cppc_cpufreq_sysfs_store_u64(buf, count, cppc_set_epp, policy->cpu);
>>   }
>>
>>   cpufreq_freq_attr_ro(freqdomain_cpus);

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.