* [PATCH] powerpc/windfarm: fix overtemperature clearing
@ 2013-06-30 19:00 Aaro Koskinen
2013-07-01 1:52 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 2+ messages in thread
From: Aaro Koskinen @ 2013-06-30 19:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev; +Cc: Aaro Koskinen
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:
[ 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.koskinen@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;
+ }
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] powerpc/windfarm: fix overtemperature clearing
2013-06-30 19:00 [PATCH] powerpc/windfarm: fix overtemperature clearing Aaro Koskinen
@ 2013-07-01 1:52 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 2+ messages in thread
From: Benjamin Herrenschmidt @ 2013-07-01 1:52 UTC (permalink / raw)
To: Aaro Koskinen; +Cc: linuxppc-dev
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.koskinen@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;
> + }
> }
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-07-01 1:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-30 19:00 [PATCH] powerpc/windfarm: fix overtemperature clearing Aaro Koskinen
2013-07-01 1:52 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).