term_adt746x: Invert bit required on this Powerbook G4

term_adt746x: Invert bit required on this Powerbook G4

[Max Vozeler]

I installed Ubuntu 8.10 on this Powerbook G4 ("alu" I think) 
for a friend of mine. As soon as therm_adt746x got loaded, the 
fan turned into a noise steam engine. 

This was on Ubuntu's 2.6.31 kernel (2.6.31-14-powerpc) which 
includes 0512a9a8e277a9de2. I could reproduce it with latest
mainline as well.

The effect was just as Michel described in the changelog; The
fan was running while temps were well below any of the limits,
and it stopped only when I set ridiculously low limits.

[ 1087.141482] adt746x: version 1 (supported)
[ 1087.141495] adt746x: Thermostat bus: 1, address: 0x2e, limit_adjust: 0, fan_speed: -1
[ 1087.141503] sensor 0: HDD BOTTOMSIDE
[ 1087.141507] sensor 1: CPU TOPSIDE
[ 1087.141512] sensor 2: GPU ON DIE
[ 1087.161365] adt746x: ADT7460 initializing
[ 1087.165245] adt746x: Lowering max temperatures from 73, 80, 109 to 70, 50, 70
[ 1087.165261] adt746x: Setting speed to 0 for CPU TOPSIDE fan.
[ 1087.166302] adt746x: Setting speed to 0 for GPU ON DIE fan.

(What "worked" was limit_adjust=-30, fan did turn off, but so did
the Powerbook shortly after, despite being cold.)

After some poking around, in which everything seemed to be
according to plan including write of 0 to both FAN_SPD_SET regs,
I noticed that explicitly *setting* the invert bit as in 

 -		write_reg(th, MANUAL_MODE[fan],
 -			(manual|MANUAL_MASK) & (~INVERT_MASK));
 +		write_reg(th, MANUAL_MODE[fan],
 +			(manual|MANUAL_MASK|INVERT_MASK));

seems to cure it. The fan appears to behave normally now, it
turns on slowly when the temp limits are reached, otherwise it
stays off. The temperature is reasonable (ie, no too hot).

So, puzzeled, I checked the spec, and it appears very clear on
the question of invert: It should be off by default.

I'm about to give the laptop back to my friend, but I suppose I 
could borrow it for further tests if anyone could guide me how 
to dig further into this.

Any hw/openfirmware details interesting?

	Max

Re: term_adt746x: Invert bit required on this Powerbook G4

[Max Vozeler]

On Wed, Nov 04, 2009 at 01:23:55PM +0100, Max Vozeler wrote:
> After some poking around, in which everything seemed to be
> according to plan including write of 0 to both FAN_SPD_SET regs,
> I noticed that explicitly *setting* the invert bit as in 
> 
>  -		write_reg(th, MANUAL_MODE[fan],
>  -			(manual|MANUAL_MASK) & (~INVERT_MASK));
>  +		write_reg(th, MANUAL_MODE[fan],
>  +			(manual|MANUAL_MASK|INVERT_MASK));
> 
> seems to cure it. The fan appears to behave normally now, it
> turns on slowly when the temp limits are reached, otherwise it
> stays off. The temperature is reasonable (ie, no too hot).
> 
> So, puzzeled, I checked the spec, and it appears very clear on
> the question of invert: It should be off by default.

Another datapoint: 

The PRAM battery in this Powerbook is old, so it forgets e.g.
clock state completely when not connected to the power supply.

These are the PWM1/PWM2 registers during probe_thermostat() 
after such a complete poweroff:

[   28.332700] adt746x: ADT7460 initializing
[   28.385266] ZZZ MANUAL_MODE[0] 0xd2
[   28.385832] ZZZ MANUAL_MODE[1] 0x82

So if I'm reading this correctly, PWM1 starts off with the 
invert bit (0x10) set, for some reason.

	Max

Re: term_adt746x: Invert bit required on this Powerbook G4

[Michel Dänzer]

[ Resending with linuxppc-dev list domain fixed ]

On Wed, 2009-11-04 at 13:23 +0100, Max Vozeler wrote:=20
> I installed Ubuntu 8.10 on this Powerbook G4 ("alu" I think)=20
> for a friend of mine. As soon as therm_adt746x got loaded, the=20
> fan turned into a noise steam engine.=20
>=20
> This was on Ubuntu's 2.6.31 kernel (2.6.31-14-powerpc) which=20
> includes 0512a9a8e277a9de2. I could reproduce it with latest
> mainline as well.
>=20
> The effect was just as Michel described in the changelog; The
> fan was running while temps were well below any of the limits,
> and it stopped only when I set ridiculously low limits.
>=20
> [ 1087.141482] adt746x: version 1 (supported)
> [ 1087.141495] adt746x: Thermostat bus: 1, address: 0x2e, limit_adjust: 0=
, fan_speed: -1
> [ 1087.141503] sensor 0: HDD BOTTOMSIDE
> [ 1087.141507] sensor 1: CPU TOPSIDE
> [ 1087.141512] sensor 2: GPU ON DIE
> [ 1087.161365] adt746x: ADT7460 initializing
> [ 1087.165245] adt746x: Lowering max temperatures from 73, 80, 109 to 70,=
 50, 70
> [ 1087.165261] adt746x: Setting speed to 0 for CPU TOPSIDE fan.
> [ 1087.166302] adt746x: Setting speed to 0 for GPU ON DIE fan.
>=20
> (What "worked" was limit_adjust=3D-30, fan did turn off, but so did
> the Powerbook shortly after, despite being cold.)
>=20
> After some poking around, in which everything seemed to be
> according to plan including write of 0 to both FAN_SPD_SET regs,
> I noticed that explicitly *setting* the invert bit as in=20
>=20
>  -		write_reg(th, MANUAL_MODE[fan],
>  -			(manual|MANUAL_MASK) & (~INVERT_MASK));
>  +		write_reg(th, MANUAL_MODE[fan],
>  +			(manual|MANUAL_MASK|INVERT_MASK));
>=20
> seems to cure it. The fan appears to behave normally now, it
> turns on slowly when the temp limits are reached, otherwise it
> stays off. The temperature is reasonable (ie, no too hot).
>=20
> So, puzzeled, I checked the spec, and it appears very clear on
> the question of invert: It should be off by default.

Right, that's what I based my patch on.

However, it sounds like your PowerBook model (mine is a PowerBook5,8) is
wired up such that the invert bit needs to be enabled. I can think of
two basic approaches for dealing with this offhand:

      * Set or clear the invert bit depending on the machine model or
        whatever is relevant.=20
      * Save the bit value during initialization and preserve it
        whenever writing to the register. Or maybe even add proper
        suspend/resume hooks which save/restore all hardware state, it
        seems like it may be luck that the current code works more or
        less for suspend/resume.

I'm not too interested in working on this anymore and I definitely won't
have time this or next week, anyone feel free to take it on.


--=20
Earthling Michel D=C3=A4nzer           |                http://www.vmware.c=
om
Libre software enthusiast         |          Debian, X and DRI developer