Hi,

ok it's a bit late now..., just one comment:

On Thu, 2008-02-07 at 04:34 -0500, Len Brown wrote:
> From: Zhang Rui <[EMAIL PROTECTED]>
> 
> Register ACPI processor as thermal cooling devices.
> A combination of processor T-state and P-state are used for thermal 
> throttling.
> the processor will reduce the frequency first and then set the T-state.
> 
> we use cpufreq_thermal_reduction_pctg to calculate the cpufreq limit,
> and call cpufreq_verify_with_limit to set the cpufreq limit.
> if cpufreq driver is loaded, then we have four cooling state for cpufreq 
> control.
> cooling state 0: cpufreq limit == max_freq
> cooling state 1: cpufreq limit == max_freq * 80%
> cooling state 2: cpufreq limit == max_freq * 60%
> cooling state 3: cpufreq limit == max_freq * 40%

> @@ -93,6 +94,9 @@ static int acpi_processor_apply_limit(struct acpi_processor 
> *pr)
>   * _any_ cpufreq driver and not only the acpi-cpufreq driver.
>   */
>  
> +#define CPUFREQ_THERMAL_MIN_STEP 0
> +#define CPUFREQ_THERMAL_MAX_STEP 3
> +
>  static unsigned int cpufreq_thermal_reduction_pctg[NR_CPUS];
>  static unsigned int acpi_thermal_cpufreq_is_init = 0;
>  
> @@ -109,8 +113,9 @@ static int acpi_thermal_cpufreq_increase(unsigned int cpu)
>       if (!cpu_has_cpufreq(cpu))
>               return -ENODEV;
>  
> -     if (cpufreq_thermal_reduction_pctg[cpu] < 60) {
> -             cpufreq_thermal_reduction_pctg[cpu] += 20;
> +     if (cpufreq_thermal_reduction_pctg[cpu] <
> +             CPUFREQ_THERMAL_MAX_STEP) {
> +             cpufreq_thermal_reduction_pctg[cpu]++;
>               cpufreq_update_policy(cpu);
>               return 0;
>       }
> @@ -123,8 +128,9 @@ static int acpi_thermal_cpufreq_decrease(unsigned int cpu)
>       if (!cpu_has_cpufreq(cpu))
>               return -ENODEV;
>  
> -     if (cpufreq_thermal_reduction_pctg[cpu] > 20)
> -             cpufreq_thermal_reduction_pctg[cpu] -= 20;
> +     if (cpufreq_thermal_reduction_pctg[cpu] >
> +             (CPUFREQ_THERMAL_MIN_STEP + 1))
cpufreq_thermal_reduction_pctg[cpu] must never be:
CPUFREQ_THERMAL_MIN_STEP 0
or the frequency is set to zero?

This looks a bit confusing, why not define:
#define CPUFREQ_THERMAL_MIN_STEP 1

and then remove the:
CPUFREQ_THERMAL_MIN_STEP + 1
to:
+       if (cpufreq_thermal_reduction_pctg[cpu] >
+               (CPUFREQ_THERMAL_MIN_STEP))
+               cpufreq_thermal_reduction_pctg[cpu]--;

> +             cpufreq_thermal_reduction_pctg[cpu]--;
>       else
>               cpufreq_thermal_reduction_pctg[cpu] = 0;
>       cpufreq_update_policy(cpu);

   Thomas

-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to