Bug 2588 - Can set all but active[2] temperature via trip_points
Can set all but active[2] temperature via trip_points
Status: CLOSED CODE_FIX
Product: ACPI
Classification: Unclassified
Component: Config-Other
i386 Linux
: P2 normal
Assigned To: acpi_config-other
:
: 2074 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-04-24 23:21 UTC by Hugo Haas
Modified: 2004-11-15 20:59 UTC (History)
3 users (show)

See Also:
Kernel Version: 2.6.5 + swsusp2
Tree: Mainline
Regression: ---


Attachments
dmesg (7.19 KB, text/plain)
2004-04-24 23:22 UTC, Hugo Haas
Details
dmidecode output (10.00 KB, text/plain)
2004-04-24 23:23 UTC, Hugo Haas
Details
acpidmp output (115.36 KB, text/plain)
2004-04-24 23:25 UTC, Hugo Haas
Details
Patch making acpi_thermal_write_trip_points() consider all trip points (2.74 KB, patch)
2004-04-25 03:36 UTC, Hugo Haas
Details | Diff
Patch making acpi_thermal_write_trip_points() consider all trip points v2 (2.62 KB, patch)
2004-04-25 07:36 UTC, Hugo Haas
Details | Diff
another (shorter) patch for the same problem (1.72 KB, patch)
2004-04-27 02:05 UTC, Stefan Seyfried
Details | Diff
corrected patch #2730, against vanilla 2.6.7 (1.64 KB, patch)
2004-06-29 14:48 UTC, Stefan Seyfried
Details | Diff

Description Hugo Haas 2004-04-24 23:21:02 UTC
Distribution: Debian
Hardware Environment: Compaq Evo N410c
Problem Description:

I have found a weird problem: using
/proc/acpi/thermal_zone/TZ1/trip_points, I can set all but the
active[2] temperature.

Steps to reproduce:

root@buena:/proc/acpi/thermal_zone/TZ1# cat *
<not supported>
<polling disabled>
state:                   active[2]
temperature:             44 C
critical (S5):           94 C
passive:                 90 C: tc1=1 tc2=2 tsp=100 devices=0xc1522c00
active[0]:               80 C: devices=0xc1524d20
active[1]:               65 C: devices=0xc1524c80
active[2]:               40 C: devices=0xc1524c00

root@buena:/proc/acpi/thermal_zone/TZ1# echo -n "95:0:91:81:66:41" > trip_points

root@buena:/proc/acpi/thermal_zone/TZ1# cat *
<not supported>
<polling disabled>
state:                   active[2]
temperature:             44 C
critical (S5):           95 C
passive:                 91 C: tc1=1 tc2=2 tsp=100 devices=0xc1522c00
active[0]:               81 C: devices=0xc1524d20
active[1]:               66 C: devices=0xc1524c80
active[2]:               40 C: devices=0xc1524c00

Regards,

Hugo
Comment 1 Hugo Haas 2004-04-24 23:22:43 UTC
Created attachment 2693 [details]
dmesg
Comment 2 Hugo Haas 2004-04-24 23:23:56 UTC
Created attachment 2694 [details]
dmidecode output
Comment 3 Hugo Haas 2004-04-24 23:25:23 UTC
Created attachment 2695 [details]
acpidmp output
Comment 4 Hugo Haas 2004-04-25 00:15:16 UTC
acpi_thermal_write_trip_points() in thermal.c only reads up to active1. It
doesn't take into account other values.
Comment 5 Hugo Haas 2004-04-25 03:36:37 UTC
Created attachment 2697 [details]
Patch making acpi_thermal_write_trip_points() consider all trip points

The current acpi_thermal_write_trip_points() only considers 5 values: critical,
hot, passive, active0 and active1.

This patch makes it consider all the relevant values.

One can now do, with 1 active point:

  echo -n "100:90:80:70" > trip_points

With 3, you can do:

  echo -n "100:90:80:70:60:50" > trip_points

Etc, trip_points being under /proc/acpi/thermal_zone/*.

With this code, the limitation on the number of trip points is set by
ACPI_THERMAL_MAX_ACTIVE, which is AFAICT a global limitation.

I have tested it and it works for me.

Regards,

Hugo
Comment 6 Hugo Haas 2004-04-25 07:36:42 UTC
Created attachment 2698 [details]
Patch making acpi_thermal_write_trip_points() consider all trip points v2

The patch I previously sent had some memory management issues. This version
should be OK. For a description, please refer to #2697.
Comment 7 Stefan Seyfried 2004-04-27 02:05:59 UTC
Created attachment 2730 [details]
another (shorter) patch for the same problem

Note that with my patch, you need to supply at least 5 values, to be compatible
to the old behaviour, but lowering the limit is trivial.
Comment 8 Hugo Haas 2004-04-27 02:24:21 UTC
Yes, my patch is slightly more complex but doesn't have any hard-coded limits. 

The drawback with the simpler patch is that, would somebody have 50 trip points
(would that ever happen?), we would need to change the code again.

Another one is, as you mention, that one needs to echo a lot of trailing :0's.

But it does fix my problem (well, at least for a while as mentioned in #2592),
so either is fine by me.
Comment 9 Stefan Seyfried 2004-04-27 09:29:10 UTC
the limit of 10 trip points is in the ACPI spec,
you don't need to echo additional ":0", since sscanf just returns the number of
matches found. Thus the for "(i = 0; i < num - 3; i++)" loop. I explicitly tested
this case (my machine has four trip points and i set only 2 or three). The lower
limit of 5 arguments (2 trip points) is just for backwards compatibility with
the old behaviour where you always needed to provide exactly 5 values and can
easily be changed to allow setting of a single trip point).

So let the gurus decide on what they like better, i believe both are working well. 
;-)
Comment 10 Wang, Zhenyu Z 2004-06-28 22:34:00 UTC
Stefan,
Could you update and clear your patch? So, Len will apply it.

thanks,
Comment 11 Stefan Seyfried 2004-06-29 14:48:47 UTC
Created attachment 3284 [details]
corrected patch #2730, against vanilla 2.6.7

the change to patch #2730 is

-	int    active[9];
+	int    active[ACPI_THERMAL_MAX_ACTIVE];

it is obviously correct since i use active[0]...active[9],
ACPI_THERMAL_MAX_ACTIVE is defined as 10.
Comment 12 Len Brown 2004-11-04 13:03:57 UTC
shipped in 2.6.9
closing
Comment 13 Len Brown 2004-11-15 20:59:08 UTC
*** Bug 2074 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.