The conservative governor documentation incorrectly states that setting
freq_step to 0 will use the default 5% frequency step. In reality, since
the governor's initial implementation
commit b9170836d1aa ("[CPUFREQ] Conservative cpufreq governer"),
freq_step=0 has always caused the governor to skip frequency updates
entirely.

Correct the documentation to reflect the actual behavior: freq_step=0
disables frequency changes by the governor entirely.

Fixes: 2a0e49279850 ("cpufreq: User/admin documentation update and consolidation")
Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
---
 Documentation/admin-guide/pm/cpufreq.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/pm/cpufreq.rst b/Documentation/admin-guide/pm/cpufreq.rst
index dbe6d23a5d67..98c724d49047 100644
--- a/Documentation/admin-guide/pm/cpufreq.rst
+++ b/Documentation/admin-guide/pm/cpufreq.rst
@@ -586,8 +586,8 @@ This governor exposes the following tunables:
 	100 (5 by default).
 
 	This is how much the frequency is allowed to change in one go.  Setting
-	it to 0 will cause the default frequency step (5 percent) to be used
-	and setting it to 100 effectively causes the governor to periodically
+	it to 0 disables frequency changes by the governor entirely and setting
+	it to 100 effectively causes the governor to periodically
 	switch the frequency between the ``scaling_min_freq`` and
 	``scaling_max_freq`` policy limits.
 
-- 
2.33.0

On 5/29/2026 7:11 PM, Pengjie Zhang wrote:
> The conservative governor documentation incorrectly states that setting
> freq_step to 0 will use the default 5% frequency step. In reality, since
> the governor's initial implementation
> commit b9170836d1aa ("[CPUFREQ] Conservative cpufreq governer"),
> freq_step=0 has always caused the governor to skip frequency updates
> entirely.

Hi Pengjie,

Thanks for the patch.

The documentation fix looks correct: in the current code,
cs_dbs_update() has an early goto out when freq_step == 0, which skips
the call to get_freq_step() and all subsequent frequency change logic.

However, the commit message's historical claim appears to be inaccurate.
In the original implementation (b9170836d1aa), freq_step=0 had
asymmetric behavior: frequency decreases were skipped (early return),
but frequency increases still used the hardcoded 5% fallback (freq_step
= 5 after the unlikely(freq_step == 0) check).

If so, would it make sense to remove/update the historical claim to
avoid the incorrect historical claim?

> 
> Correct the documentation to reflect the actual behavior: freq_step=0
> disables frequency changes by the governor entirely.
> 
> Fixes: 2a0e49279850 ("cpufreq: User/admin documentation update and consolidation")
> Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
> ---
>   Documentation/admin-guide/pm/cpufreq.rst | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/pm/cpufreq.rst b/Documentation/admin-guide/pm/cpufreq.rst
> index dbe6d23a5d67..98c724d49047 100644
> --- a/Documentation/admin-guide/pm/cpufreq.rst
> +++ b/Documentation/admin-guide/pm/cpufreq.rst
> @@ -586,8 +586,8 @@ This governor exposes the following tunables:
>   	100 (5 by default).
>   
>   	This is how much the frequency is allowed to change in one go.  Setting
> -	it to 0 will cause the default frequency step (5 percent) to be used
> -	and setting it to 100 effectively causes the governor to periodically
> +	it to 0 disables frequency changes by the governor entirely and setting
> +	it to 100 effectively causes the governor to periodically
>   	switch the frequency between the ``scaling_min_freq`` and
>   	``scaling_max_freq`` policy limits.
>   


-- 
Thx and BRs,
Zhongqiu Han

On 5/30/2026 10:36 PM, Zhongqiu Han wrote:
> On 5/29/2026 7:11 PM, Pengjie Zhang wrote:
>> The conservative governor documentation incorrectly states that setting
>> freq_step to 0 will use the default 5% frequency step. In reality, since
>> the governor's initial implementation
>> commit b9170836d1aa ("[CPUFREQ] Conservative cpufreq governer"),
>> freq_step=0 has always caused the governor to skip frequency updates
>> entirely.
>
> Hi Pengjie,
>
> Thanks for the patch.
>
> The documentation fix looks correct: in the current code,
> cs_dbs_update() has an early goto out when freq_step == 0, which skips
> the call to get_freq_step() and all subsequent frequency change logic.
>
> However, the commit message's historical claim appears to be inaccurate.
> In the original implementation (b9170836d1aa), freq_step=0 had
> asymmetric behavior: frequency decreases were skipped (early return),
> but frequency increases still used the hardcoded 5% fallback (freq_step
> = 5 after the unlikely(freq_step == 0) check).
>
> If so, would it make sense to remove/update the historical claim to
> avoid the incorrect historical claim?
>
Thanks for the careful review.

Agreed. The correct commit for the symmetric freq_step=0 behavior
should be 8e677ce83bf4 ("[CPUFREQ] conservative: fixup governor to
function more like ondemand logic"), not b9170836d1aa.

I'll fix the commit message in v2.

On a related note, I have a quick question regarding code readability in
this area. Currently, the code uses the name "freq_step" for two different
concepts:

1. `cs_tuners->freq_step`: The tunable exposed via sysfs/documentation,
    which represents a percentage.
2. `freq_step = get_freq_step(...)`: The local variable representing the
    actual calculated frequency step (in kHz). The `if 
(unlikely(freq_step == 0))`
    check also applies to this absolute value.

Since mixing a percentage and an absolute kHz value under the same name
might be slightly confusing for readers, would it make sense to rename the
local variable (e.g., to `freq_step_khz`) to clearly distinguish the two?

Cheers,
     Pengjie

>>
>> Correct the documentation to reflect the actual behavior: freq_step=0
>> disables frequency changes by the governor entirely.
>>
>> Fixes: 2a0e49279850 ("cpufreq: User/admin documentation update and 
>> consolidation")
>> Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
>> ---
>>   Documentation/admin-guide/pm/cpufreq.rst | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/pm/cpufreq.rst 
>> b/Documentation/admin-guide/pm/cpufreq.rst
>> index dbe6d23a5d67..98c724d49047 100644
>> --- a/Documentation/admin-guide/pm/cpufreq.rst
>> +++ b/Documentation/admin-guide/pm/cpufreq.rst
>> @@ -586,8 +586,8 @@ This governor exposes the following tunables:
>>       100 (5 by default).
>>         This is how much the frequency is allowed to change in one 
>> go.  Setting
>> -    it to 0 will cause the default frequency step (5 percent) to be 
>> used
>> -    and setting it to 100 effectively causes the governor to 
>> periodically
>> +    it to 0 disables frequency changes by the governor entirely and 
>> setting
>> +    it to 100 effectively causes the governor to periodically
>>       switch the frequency between the ``scaling_min_freq`` and
>>       ``scaling_max_freq`` policy limits.
>
>

On 6/1/2026 10:04 AM, Pengjie Zhang wrote:
> 
> On 5/30/2026 10:36 PM, Zhongqiu Han wrote:
>> On 5/29/2026 7:11 PM, Pengjie Zhang wrote:
>>> The conservative governor documentation incorrectly states that setting
>>> freq_step to 0 will use the default 5% frequency step. In reality, since
>>> the governor's initial implementation
>>> commit b9170836d1aa ("[CPUFREQ] Conservative cpufreq governer"),
>>> freq_step=0 has always caused the governor to skip frequency updates
>>> entirely.
>>
>> Hi Pengjie,
>>
>> Thanks for the patch.
>>
>> The documentation fix looks correct: in the current code,
>> cs_dbs_update() has an early goto out when freq_step == 0, which skips
>> the call to get_freq_step() and all subsequent frequency change logic.
>>
>> However, the commit message's historical claim appears to be inaccurate.
>> In the original implementation (b9170836d1aa), freq_step=0 had
>> asymmetric behavior: frequency decreases were skipped (early return),
>> but frequency increases still used the hardcoded 5% fallback (freq_step
>> = 5 after the unlikely(freq_step == 0) check).
>>
>> If so, would it make sense to remove/update the historical claim to
>> avoid the incorrect historical claim?
>>
> Thanks for the careful review.
> 
> Agreed. The correct commit for the symmetric freq_step=0 behavior
> should be 8e677ce83bf4 ("[CPUFREQ] conservative: fixup governor to
> function more like ondemand logic"), not b9170836d1aa.
> 
> I'll fix the commit message in v2.
> 
> On a related note, I have a quick question regarding code readability in
> this area. Currently, the code uses the name "freq_step" for two different
> concepts:
> 
> 1. `cs_tuners->freq_step`: The tunable exposed via sysfs/documentation,
>     which represents a percentage.
> 2. `freq_step = get_freq_step(...)`: The local variable representing the
>     actual calculated frequency step (in kHz). The `if 
> (unlikely(freq_step == 0))`
>     check also applies to this absolute value.
> 
> Since mixing a percentage and an absolute kHz value under the same name
> might be slightly confusing for readers, would it make sense to rename the
> local variable (e.g., to `freq_step_khz`) to clearly distinguish the two?

Hi Pengjie,

Agreed, the shadowing is confusing. That said, renaming just the local
variable alone might be a bit too small to warrant a separate patch
-- but while you're looking at this area, you might want to consider
DEF_FREQUENCY_STEP itself as well, which seems to be overloaded:

   - In cs_init() it is used as a percentage (the default 5% tunable).
   - In get_freq_step() it is assigned to a kHz-valued variable as a
     fallback, where "5" means "5 kHz".

As a suggestion, you could fix both while you're at it.

> 
> Cheers,
>      Pengjie
> 
>>>
>>> Correct the documentation to reflect the actual behavior: freq_step=0
>>> disables frequency changes by the governor entirely.
>>>
>>> Fixes: 2a0e49279850 ("cpufreq: User/admin documentation update and 
>>> consolidation")
>>> Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
>>> ---
>>>   Documentation/admin-guide/pm/cpufreq.rst | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/admin-guide/pm/cpufreq.rst b/ 
>>> Documentation/admin-guide/pm/cpufreq.rst
>>> index dbe6d23a5d67..98c724d49047 100644
>>> --- a/Documentation/admin-guide/pm/cpufreq.rst
>>> +++ b/Documentation/admin-guide/pm/cpufreq.rst
>>> @@ -586,8 +586,8 @@ This governor exposes the following tunables:
>>>       100 (5 by default).
>>>         This is how much the frequency is allowed to change in one 
>>> go.  Setting
>>> -    it to 0 will cause the default frequency step (5 percent) to be 
>>> used
>>> -    and setting it to 100 effectively causes the governor to 
>>> periodically
>>> +    it to 0 disables frequency changes by the governor entirely and 
>>> setting
>>> +    it to 100 effectively causes the governor to periodically
>>>       switch the frequency between the ``scaling_min_freq`` and
>>>       ``scaling_max_freq`` policy limits.
>>
>>


-- 
Thx and BRs,
Zhongqiu Han