On Sun, 2013-06-30 at 22:00 +0300, Aaro Koskinen wrote:
> With pm81/pm91/pm121, when the overtemperature state is entered, and
> when it remains on after skipped ticks, the driver will try to leave
> it too soon (immediately on the next tick). This is because the active
> FAILURE_OVERTEMP state is not visible in "new_failure" variable of the
> current tick. Furthermore, the driver will keep trying to clear condition
> in subsequent ticks as FAILURE_OVERTEMP remains set in the "last_failure"
> variable. These will start to trigger WARNINGS from windfarm core:
Thanks, looks good. Applied.
Cheers,
Ben.
> [ 100.082735] windfarm: Clamping CPU frequency to minimum !
> [ 100.108132] windfarm: Overtemp condition detected !
> [ 101.952908] windfarm: Overtemp condition cleared !
> [...]
> [ 102.980388] WARNING: at drivers/macintosh/windfarm_core.c:463
> [...]
> [ 103.982227] WARNING: at drivers/macintosh/windfarm_core.c:463
> [...]
> [ 105.030494] WARNING: at drivers/macintosh/windfarm_core.c:463
> [...]
> [ 105.973666] WARNING: at drivers/macintosh/windfarm_core.c:463
> [...]
> [ 106.977913] WARNING: at drivers/macintosh/windfarm_core.c:463
>
> Fix by adding a helper global variable. We leave the overtemp state only
> after all failure bits have been cleared.
>
> I saw this error on iMac G5 iSight (pm121). Also pm81/pm91 are fixed
> based on the observation that these are almost identical/copy-pasted code.
>
> Signed-off-by: Aaro Koskinen <aaro.koski...@iki.fi>
> ---
> drivers/macintosh/windfarm_pm121.c | 6 +++++-
> drivers/macintosh/windfarm_pm81.c | 6 +++++-
> drivers/macintosh/windfarm_pm91.c | 6 +++++-
> 3 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/macintosh/windfarm_pm121.c
> b/drivers/macintosh/windfarm_pm121.c
> index af605e9..7fe58b0 100644
> --- a/drivers/macintosh/windfarm_pm121.c
> +++ b/drivers/macintosh/windfarm_pm121.c
> @@ -276,6 +276,7 @@ static const char *loop_names[N_LOOPS] = {
>
> static unsigned int pm121_failure_state;
> static int pm121_readjust, pm121_skipping;
> +static bool pm121_overtemp;
> static s32 average_power;
>
> struct pm121_correction {
> @@ -847,6 +848,7 @@ static void pm121_tick(void)
> if (new_failure & FAILURE_OVERTEMP) {
> wf_set_overtemp();
> pm121_skipping = 2;
> + pm121_overtemp = true;
> }
>
> /* We only clear the overtemp condition if overtemp is cleared
> @@ -855,8 +857,10 @@ static void pm121_tick(void)
> * the control loop levels, but we don't want to keep it clear
> * here in this case
> */
> - if (new_failure == 0 && last_failure & FAILURE_OVERTEMP)
> + if (!pm121_failure_state && pm121_overtemp) {
> wf_clear_overtemp();
> + pm121_overtemp = false;
> + }
> }
>
>
> diff --git a/drivers/macintosh/windfarm_pm81.c
> b/drivers/macintosh/windfarm_pm81.c
> index f84933f..2a5e1b1 100644
> --- a/drivers/macintosh/windfarm_pm81.c
> +++ b/drivers/macintosh/windfarm_pm81.c
> @@ -149,6 +149,7 @@ static int wf_smu_all_controls_ok, wf_smu_all_sensors_ok,
> wf_smu_started;
>
> static unsigned int wf_smu_failure_state;
> static int wf_smu_readjust, wf_smu_skipping;
> +static bool wf_smu_overtemp;
>
> /*
> * ****** System Fans Control Loop ******
> @@ -593,6 +594,7 @@ static void wf_smu_tick(void)
> if (new_failure & FAILURE_OVERTEMP) {
> wf_set_overtemp();
> wf_smu_skipping = 2;
> + wf_smu_overtemp = true;
> }
>
> /* We only clear the overtemp condition if overtemp is cleared
> @@ -601,8 +603,10 @@ static void wf_smu_tick(void)
> * the control loop levels, but we don't want to keep it clear
> * here in this case
> */
> - if (new_failure == 0 && last_failure & FAILURE_OVERTEMP)
> + if (!wf_smu_failure_state && wf_smu_overtemp) {
> wf_clear_overtemp();
> + wf_smu_overtemp = false;
> + }
> }
>
> static void wf_smu_new_control(struct wf_control *ct)
> diff --git a/drivers/macintosh/windfarm_pm91.c
> b/drivers/macintosh/windfarm_pm91.c
> index 2eb484f..a8ac66c 100644
> --- a/drivers/macintosh/windfarm_pm91.c
> +++ b/drivers/macintosh/windfarm_pm91.c
> @@ -76,6 +76,7 @@ static struct wf_control *cpufreq_clamp;
>
> /* Set to kick the control loop into life */
> static int wf_smu_all_controls_ok, wf_smu_all_sensors_ok, wf_smu_started;
> +static bool wf_smu_overtemp;
>
> /* Failure handling.. could be nicer */
> #define FAILURE_FAN 0x01
> @@ -517,6 +518,7 @@ static void wf_smu_tick(void)
> if (new_failure & FAILURE_OVERTEMP) {
> wf_set_overtemp();
> wf_smu_skipping = 2;
> + wf_smu_overtemp = true;
> }
>
> /* We only clear the overtemp condition if overtemp is cleared
> @@ -525,8 +527,10 @@ static void wf_smu_tick(void)
> * the control loop levels, but we don't want to keep it clear
> * here in this case
> */
> - if (new_failure == 0 && last_failure & FAILURE_OVERTEMP)
> + if (!wf_smu_failure_state && wf_smu_overtemp) {
> wf_clear_overtemp();
> + wf_smu_overtemp = false;
> + }
> }
>
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
