Re: [PATCH] Latest patch to therm_adt7467
I have a question, in the following part of the patch code, you set the limits, and you take care if you have 7460 or 7467. My question is, is correct the last asignation (below write)? I think you have to make an if (like a couples of lines before) to set the correct values for 7467 or 7460, if not, you will set the values from 7467. I commentd out this line (I have an 7460 chipset) and everything works great (fan starts at 60 and stops at 57). I am missing something? + for (i = 0; i 3; i++) { + th-initial_limits[i] = read_reg(th, LIMIT_REG[i]); + + if (therm_type == ADT7467) { + th-limits[i] = default_limits7467[i] - limit_decrease; + } else { + /* Set CPU limit higher on 7460 to avoid powerdowns */ + th-limits[i] = default_limits7460[i] - limit_decrease; + } + write_reg(th, LIMIT_REG[i], th-limits[i]); + + /* set our limits to normal */ + th-limits[i] = default_limits7467[i] - limit_decrease; ^^^ + } + El sáb, 24-01-2004 a las 06:56, Colin Leroy escribió: Hi, Ok, here's the latest patch for therm_adt7467, based on the feedback provided by our generous testers :-) What is done now: . fan_speed defaults to 128 (for both chips), allowing the fan to start more quietly. The chip is still put back to automatic mode when we rmmod the module. . limits are set quite high on the ADT7460 chip, but the fan starts much below these limits (in order to avoid poweroffs). I think it's now quite OK. HTH, -- My software never has bugs. | ASCII Ribbon Campaign /\ It just develops random features. | For Standards-Complaint Email \ / Public GnuPG key avaible at: http://www.keyserver.net X 1024D/203E154B 2003-10-03 Federico Gamio [EMAIL PROTECTED] / \ Key fingerprint = 0B9E 3A19 C88B EBAC 5422 C05F 76B5 B922 203E 154B sub 2048g/32D2F465 2003-10-03 signature.asc Description: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada digitalmente
Re: [PATCH] Latest patch to therm_adt7467
I commentd out this line (I have an 7460 chipset) and everything works great (fan starts at 60 and stops at 57). I am missing something? + th-limits[i] = default_limits7467[i] - limit_decrease; You'll probably have the machine poweroff at about 62/63. Anyway, the latest patch is no more this one but rather this one: http://lists.debian.org/debian-powerpc/2004/debian-powerpc-200401/msg00918.html -- Colin Ne disez pas disez, mais disez dites
[PATCH] Latest patch to therm_adt7467
Hi, Ok, here's the latest patch for therm_adt7467, based on the feedback provided by our generous testers :-) What is done now: . fan_speed defaults to 128 (for both chips), allowing the fan to start more quietly. The chip is still put back to automatic mode when we rmmod the module. . limits are set quite high on the ADT7460 chip, but the fan starts much below these limits (in order to avoid poweroffs). I think it's now quite OK. HTH, -- Colin Index: drivers/macintosh/therm_adt7467.c === RCS file: /home/cvsroot/linuxppc/drivers/macintosh/therm_adt7467.c,v retrieving revision 1.2 diff -u -u -r1.2 therm_adt7467.c --- drivers/macintosh/therm_adt7467.c 21 Jan 2004 11:50:29 - 1.2 +++ drivers/macintosh/therm_adt7467.c 24 Jan 2004 09:47:25 - @@ -1,10 +1,11 @@ /* - * Device driver for the i2c thermostat found on the iBook G4 + * Device driver for the i2c thermostat found on the iBook G4, Albook G4 * - * Copyright (C) 2003 Colin Leroy, Benjamin Herrenschmidt + * Copyright (C) 2003, 2004 Colin Leroy, Rasmus Rohde, Benjamin Herrenschmidt * * Documentation from * http://www.analog.com/UploadedFiles/Data_Sheets/115254175ADT7467_pra.pdf + * http://www.analog.com/UploadedFiles/Data_Sheets/3686221171167ADT7460_b.pdf * */ @@ -30,18 +31,19 @@ #undef DEBUG -#define TEMP_LOCAL 0x26 -#define TEMP_REMOTE1 0x25 -#define TEMP_REMOTE2 0x27 -#define LIM_LOCAL0x6a -#define LIM_REMOTE1 0x6b -#define LIM_REMOTE2 0x6c -#define FAN0_SPEED 0x28 - -#define MANUAL_MODE 0x5c +#define CONFIG_REG 0x40 #define MANUAL_MASK 0xe0 #define AUTO_MASK0x20 -#define FAN_SPD_SET 0x30 + +static u8 TEMP_REG[3]= {0x26, 0x25, 0x27}; /* local, cpu, gpu */ +static u8 LIMIT_REG[3] = {0x6b, 0x6a, 0x6c}; /* local, cpu, gpu */ +static u8 MANUAL_MODE[2] = {0x5c, 0x5d}; +static u8 REM_CONTROL[2] = {0x00, 0x40}; +static u8 FAN_SPEED[2] = {0x28, 0x2a}; +static u8 FAN_SPD_SET[2] = {0x30, 0x31}; + +static u8 default_limits7467[3] = {70, 50, 70};/* local, cpu, gpu */ +static u8 default_limits7460[3] = {80, 65, 80};/* local, cpu, gpu */ static int limit_decrease = 0; static int fan_speed = -1; @@ -53,17 +55,18 @@ MODULE_PARM(limit_decrease,i); MODULE_PARM_DESC(limit_decrease,Decrease maximum temperatures (50°C cpu, 70°C gpu) by N °C.); MODULE_PARM(fan_speed,i); -MODULE_PARM_DESC(fan_speed,Specify fan speed (0-255) when lim temp lim+8 (dangerous !), default is automatic); +MODULE_PARM_DESC(fan_speed,Specify fan speed (0-255) when lim temp lim+8 (default 128)); struct thermostat { struct i2c_client clt; u8 cached_temp[3]; u8 initial_limits[3]; u8 limits[3]; - int last_speed; - int overriding; + int last_speed[2]; + int overriding[2]; }; +static enum {ADT7460, ADT7467} therm_type; static int therm_bus, therm_address; static struct of_device * of_dev; static struct thermostat* thermostat; @@ -72,7 +75,8 @@ static struct completion monitor_task_compl; static int attach_one_thermostat(struct i2c_adapter *adapter, int addr, int busno); -static void write_fan_speed(struct thermostat *th, int speed); +static void write_both_fan_speed(struct thermostat *th, int speed); +static void write_fan_speed(struct thermostat *th, int speed, int fan); static int write_reg(struct thermostat* th, int reg, u8 data) @@ -125,6 +129,7 @@ detach_thermostat(struct i2c_adapter *adapter) { struct thermostat* th; + int i; if (thermostat == NULL) return 0; @@ -136,14 +141,15 @@ wait_for_completion(monitor_task_compl); } - printk(KERN_INFO adt7467: Putting max temperatures back from %d, %d, %d, + printk(KERN_INFO adt746x: Putting max temperatures back from %d, %d, %d, to %d, %d, %d, (°C)\n, th-limits[0], th-limits[1], th-limits[2], th-initial_limits[0], th-initial_limits[1], th-initial_limits[2]); - write_reg(th, LIM_LOCAL, th-initial_limits[0]); - write_reg(th, LIM_REMOTE1, th-initial_limits[1]); - write_reg(th, LIM_REMOTE2, th-initial_limits[2]); - write_fan_speed(th, -1); + + for (i = 0; i 3; i++) + write_reg(th, LIMIT_REG[i], th-initial_limits[i]); + + write_both_fan_speed(th, -1); i2c_detach_client(th-clt); @@ -175,7 +181,14 @@ return (9*60)/res; } -static void write_fan_speed(struct thermostat *th, int speed) +static void write_both_fan_speed(struct thermostat *th, int speed) +{ + write_fan_speed(th, speed, 0); + if (therm_type == ADT7460) + write_fan_speed(th, speed, 1); +} + +static void write_fan_speed(struct thermostat *th, int speed, int
Re: [PATCH] Latest patch to therm_adt7467
On 24 Jan 2004 at 10h01, Colin Leroy wrote: Hi, . limits are set quite high on the ADT7460 chip, but the fan starts much below these limits (in order to avoid poweroffs). Yuck. My iBook just powered off. Probably did not happen before because I was in colder environments... This new patch sets limits higher on iBook too, and obsolotes the previous one. Phew. -- Colin Index: drivers/macintosh/therm_adt7467.c === RCS file: /home/cvsroot/linuxppc/drivers/macintosh/therm_adt7467.c,v retrieving revision 1.4 diff -u -u -r1.4 therm_adt7467.c --- drivers/macintosh/therm_adt7467.c 24 Jan 2004 10:04:30 - 1.4 +++ drivers/macintosh/therm_adt7467.c 24 Jan 2004 10:36:46 - @@ -1,10 +1,11 @@ /* - * Device driver for the i2c thermostat found on the iBook G4 + * Device driver for the i2c thermostat found on the iBook G4, Albook G4 * - * Copyright (C) 2003 Colin Leroy, Benjamin Herrenschmidt + * Copyright (C) 2003, 2004 Colin Leroy, Rasmus Rohde, Benjamin Herrenschmidt * * Documentation from * http://www.analog.com/UploadedFiles/Data_Sheets/115254175ADT7467_pra.pdf + * http://www.analog.com/UploadedFiles/Data_Sheets/3686221171167ADT7460_b.pdf * */ @@ -30,18 +31,19 @@ #undef DEBUG -#define TEMP_LOCAL 0x26 -#define TEMP_REMOTE1 0x25 -#define TEMP_REMOTE2 0x27 -#define LIM_LOCAL0x6a -#define LIM_REMOTE1 0x6b -#define LIM_REMOTE2 0x6c -#define FAN0_SPEED 0x28 - -#define MANUAL_MODE 0x5c +#define CONFIG_REG 0x40 #define MANUAL_MASK 0xe0 #define AUTO_MASK0x20 -#define FAN_SPD_SET 0x30 + +static u8 TEMP_REG[3]= {0x26, 0x25, 0x27}; /* local, cpu, gpu */ +static u8 LIMIT_REG[3] = {0x6b, 0x6a, 0x6c}; /* local, cpu, gpu */ +static u8 MANUAL_MODE[2] = {0x5c, 0x5d}; +static u8 REM_CONTROL[2] = {0x00, 0x40}; +static u8 FAN_SPEED[2] = {0x28, 0x2a}; +static u8 FAN_SPD_SET[2] = {0x30, 0x31}; + +static u8 default_limits_local[3] = {70, 50, 70};/* local, cpu, gpu */ +static u8 default_limits_chip[3] = {80, 65, 80};/* local, cpu, gpu */ static int limit_decrease = 0; static int fan_speed = -1; @@ -53,17 +55,18 @@ MODULE_PARM(limit_decrease,i); MODULE_PARM_DESC(limit_decrease,Decrease maximum temperatures (50°C cpu, 70°C gpu) by N °C.); MODULE_PARM(fan_speed,i); -MODULE_PARM_DESC(fan_speed,Specify fan speed (0-255) when lim temp lim+8 (dangerous !), default is automatic); +MODULE_PARM_DESC(fan_speed,Specify fan speed (0-255) when lim temp lim+8 (default 128)); struct thermostat { struct i2c_client clt; u8 cached_temp[3]; u8 initial_limits[3]; u8 limits[3]; - int last_speed; - int overriding; + int last_speed[2]; + int overriding[2]; }; +static enum {ADT7460, ADT7467} therm_type; static int therm_bus, therm_address; static struct of_device * of_dev; static struct thermostat* thermostat; @@ -72,7 +75,8 @@ static struct completion monitor_task_compl; static int attach_one_thermostat(struct i2c_adapter *adapter, int addr, int busno); -static void write_fan_speed(struct thermostat *th, int speed); +static void write_both_fan_speed(struct thermostat *th, int speed); +static void write_fan_speed(struct thermostat *th, int speed, int fan); static int write_reg(struct thermostat* th, int reg, u8 data) @@ -125,6 +129,7 @@ detach_thermostat(struct i2c_adapter *adapter) { struct thermostat* th; + int i; if (thermostat == NULL) return 0; @@ -136,14 +141,15 @@ wait_for_completion(monitor_task_compl); } - printk(KERN_INFO adt7467: Putting max temperatures back from %d, %d, %d, + printk(KERN_INFO adt746x: Putting max temperatures back from %d, %d, %d, to %d, %d, %d, (°C)\n, th-limits[0], th-limits[1], th-limits[2], th-initial_limits[0], th-initial_limits[1], th-initial_limits[2]); - write_reg(th, LIM_LOCAL, th-initial_limits[0]); - write_reg(th, LIM_REMOTE1, th-initial_limits[1]); - write_reg(th, LIM_REMOTE2, th-initial_limits[2]); - write_fan_speed(th, -1); + + for (i = 0; i 3; i++) + write_reg(th, LIMIT_REG[i], th-initial_limits[i]); + + write_both_fan_speed(th, -1); i2c_detach_client(th-clt); @@ -175,7 +181,14 @@ return (9*60)/res; } -static void write_fan_speed(struct thermostat *th, int speed) +static void write_both_fan_speed(struct thermostat *th, int speed) +{ + write_fan_speed(th, speed, 0); + if (therm_type == ADT7460) + write_fan_speed(th, speed, 1); +} + +static void write_fan_speed(struct thermostat *th, int speed, int fan) { u8 manual; @@ -184,21 +197,35 @@ else if
Re: [PATCH] Latest patch to therm_adt7467
Yuck. My iBook just powered off. Probably did not happen before because I was in colder environments... This new patch sets limits higher on iBook too, and obsolotes the previous one. Can you figure out precisely what Darwin is doing and reproduce the behaviour instead ? Ben.
Re: [PATCH] Latest patch to therm_adt7467
On 24 Jan 2004 at 22h01, Benjamin Herrenschmidt wrote: Hi, Yuck. My iBook just powered off. Probably did not happen before because I was in colder environments... This new patch sets limits higher on iBook too, and obsolotes the previous one. Can you figure out precisely what Darwin is doing and reproduce the behaviour instead ? Not really - i still don't have it installed. I tried to make a clean build of 2.6.2-rc1 with this latest patch, temperature went up to 62°C for the whole compilation, and no problems appeared. Played bzflag a bit after that and no problem either. I think it should do the job quite correctly... -- Colin
Re: [PATCH] Latest patch to therm_adt7467
On Sat, 2004-01-24 at 22:39, Colin Leroy wrote: On 24 Jan 2004 at 22h01, Benjamin Herrenschmidt wrote: Hi, Yuck. My iBook just powered off. Probably did not happen before because I was in colder environments... This new patch sets limits higher on iBook too, and obsolotes the previous one. Can you figure out precisely what Darwin is doing and reproduce the behaviour instead ? Not really - i still don't have it installed. I mean, deduce it from the source code ... Ben.
Re: [PATCH] Latest patch to therm_adt7467
On 24 Jan 2004 at 23h01, Benjamin Herrenschmidt wrote: Hi, I mean, deduce it from the source code ... oh, ok. Do you have an idea about which source package I should look at ? -- Colin
Re: [PATCH] Latest patch to therm_adt7467
On 24 Jan 2004 at 13h01, Colin Leroy wrote: Hi, I mean, deduce it from the source code ... oh, ok. Do you have an idea about which source package I should look at ? I already had found them, in AppleMacRISC2PE-142.2.3/Portable2003_PlatformMonitor.cpp. PowerBook5,1 : not in file PowerBook5,2 : starts at 45°C PowerBook5,3 : starts at 45°C PowerBook6,1 : not in file PowerBook6,2 : starts at 51°C PowerBook6,3 : not in file I took the lowest values, some are higher for different sensors, but I don't know which is which... -- Colin
Re: [PATCH] Latest patch to therm_adt7467
On Sat, 2004-01-24 at 23:04, Colin Leroy wrote: On 24 Jan 2004 at 23h01, Benjamin Herrenschmidt wrote: Hi, I mean, deduce it from the source code ... oh, ok. Do you have an idea about which source package I should look at ? The limits for the various thermal states are in AppleMacRISC2 afaik, in the platform monitors. Not sure how things are done then. Or just make sure you don't get any more reports of shutdown for a few days with your latest version, that should be fine too. Ben.
Re: [PATCH] Latest patch to therm_adt7467
PowerBook5,1 : not in file PowerBook5,2 : starts at 45°C PowerBook5,3 : starts at 45°C PowerBook6,1 : not in file PowerBook6,2 : starts at 51°C PowerBook6,3 : not in file I took the lowest values, some are higher for different sensors, but I don't know which is which... Note sure how it works. On the G5, the Info.plist file contained bindings between the sensor IDs and the entries, maybe this is different... ben.