[PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4

[PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4

[Goffredo Baroncelli]

Hi All,

I am writing to you Jean and Benjamin because it seem that both 
worked on these items.

On a PowerMac G4 I noticed that between the kernel v3.2 and v3.14 I lost
the fan management.

I found on internet other references to this kind of problem [2]


**How reproduce:
- booting with the kernel 3.2, the fan is "quite" silent.
The module therm_windtunnel is loaded and in the log there are  
lines like:

	[ 1342.614956] CPU-temp: 58.7 C, Case: 33.7 C,  Fan: 5 (tuned +0)
	[ 1390.637793] CPU-temp: 58.6 C, Case: 33.6 C,  Fan: 5

I had also access to the temperature via the sysfs files:
/sys/devices/temperature/case_temperature  
/sys/devices/temperature/cpu_temperature


- booting with the kernel 3.14, the fan is very loud. The module 
therm_windtunnel is not loaded. In the log there aren't any message
related to the temperature. The sysfs entries don't exist.


** Analysis 
In these Apple machines the module i2c-powermac requires the
i2c drivers provided by the module therm_windtunnel.

Between the kernel v3.2 and v3.14 [1] some patches changed the 
driver name requested by the i2c-powermac module, 
so the therm_windtunnel modules is not instantiated anymore.


** Proposed solution
In the following emails I sent you three patches to solve this 
problem (tested on my PowerMac G4)

1) change the driver name
	therm_ds1775 -> MAC,ds1775
        therm_adm1030 -> MAC,adm1030
so the i2c driver are instantiated by i2c-powermac

2) remove the (unused) method do_attach from the i2c-driver
3) add two parameters to the therm_windtunnel module 
to control the kernel log message 

The patch 1) solve the problem. The patch 2) is a small cleanup.
The patch 3) allow a better control of the log in dmesg.

Could you be so kindly to apply these patches ?

BR
G.Baroncelli



[1] I think that the guilty commit is 

commit 81e5d8646ff6bf323dddcf172aa3cef84468fa12
Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date:   Wed Apr 18 22:16:42 2012 +0000

    i2c/powermac: Register i2c devices from device-tree
    
    This causes i2c-powermac to register i2c devices exposed in the
    device-tree, enabling new-style probing of devices.
    
    Note that we prefix the IDs with "MAC," in order to prevent the
    generic drivers from matching. This is done on purpose as we only
    want drivers specifically tested/designed to operate on powermacs
    to match.
    
    This removes the special case we had for the AMS driver, and updates
    the driver's match table instead.
    
    Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

[2] There is the debian bug #741663 which highlight the same problem. In
the bug discussion there is a patch like the my ones.

See also
https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-July/099561.html




-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4

[Benjamin Herrenschmidt]

On Wed, 2014-07-30 at 22:50 +0200, Goffredo Baroncelli wrote:
> Hi All,
> 
> I am writing to you Jean and Benjamin because it seem that both 
> worked on these items.
> 
> On a PowerMac G4 I noticed that between the kernel v3.2 and v3.14 I lost
> the fan management.
> 
> I found on internet other references to this kind of problem [2]

Patches look good, thanks. Jean do you want to apply them or should I ?

Cheers,
Ben.

> 
> **How reproduce:
> - booting with the kernel 3.2, the fan is "quite" silent.
> The module therm_windtunnel is loaded and in the log there are  
> lines like:
> 
> 	[ 1342.614956] CPU-temp: 58.7 C, Case: 33.7 C,  Fan: 5 (tuned +0)
> 	[ 1390.637793] CPU-temp: 58.6 C, Case: 33.6 C,  Fan: 5
> 
> I had also access to the temperature via the sysfs files:
> /sys/devices/temperature/case_temperature  
> /sys/devices/temperature/cpu_temperature
> 
> 
> - booting with the kernel 3.14, the fan is very loud. The module 
> therm_windtunnel is not loaded. In the log there aren't any message
> related to the temperature. The sysfs entries don't exist.
> 
> 
> ** Analysis 
> In these Apple machines the module i2c-powermac requires the
> i2c drivers provided by the module therm_windtunnel.
> 
> Between the kernel v3.2 and v3.14 [1] some patches changed the 
> driver name requested by the i2c-powermac module, 
> so the therm_windtunnel modules is not instantiated anymore.
> 
> 
> ** Proposed solution
> In the following emails I sent you three patches to solve this 
> problem (tested on my PowerMac G4)
> 
> 1) change the driver name
> 	therm_ds1775 -> MAC,ds1775
>         therm_adm1030 -> MAC,adm1030
> so the i2c driver are instantiated by i2c-powermac
> 
> 2) remove the (unused) method do_attach from the i2c-driver
> 3) add two parameters to the therm_windtunnel module 
> to control the kernel log message 
> 
> The patch 1) solve the problem. The patch 2) is a small cleanup.
> The patch 3) allow a better control of the log in dmesg.
> 
> Could you be so kindly to apply these patches ?
> 
> BR
> G.Baroncelli
> 
> 
> 
> [1] I think that the guilty commit is 
> 
> commit 81e5d8646ff6bf323dddcf172aa3cef84468fa12
> Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date:   Wed Apr 18 22:16:42 2012 +0000
> 
>     i2c/powermac: Register i2c devices from device-tree
>     
>     This causes i2c-powermac to register i2c devices exposed in the
>     device-tree, enabling new-style probing of devices.
>     
>     Note that we prefix the IDs with "MAC," in order to prevent the
>     generic drivers from matching. This is done on purpose as we only
>     want drivers specifically tested/designed to operate on powermacs
>     to match.
>     
>     This removes the special case we had for the AMS driver, and updates
>     the driver's match table instead.
>     
>     Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> [2] There is the debian bug #741663 which highlight the same problem. In
> the bug discussion there is a patch like the my ones.
> 
> See also
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-July/099561.html
> 
> 
> 
> 

Re: [PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4

[Jean Delvare]

Hi Ben, Goffredo,

On Thu, 31 Jul 2014 11:27:00 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2014-07-30 at 22:50 +0200, Goffredo Baroncelli wrote:
> > I am writing to you Jean and Benjamin because it seem that both 
> > worked on these items.
> > 
> > On a PowerMac G4 I noticed that between the kernel v3.2 and v3.14 I lost
> > the fan management.
> > 
> > I found on internet other references to this kind of problem [2]

Yay! Thanks a lot Goffredo! I've been waiting for this driver to be
finally converter to the standard device driver model for years,
literally. I'm very happy to finally see this happen :-)

The i2c_driver.attach_adapter method was marked for removal in
September 2011 so we are almost 3 year late on the schedule :-(

> Patches look good, thanks. Jean do you want to apply them or should I ?

Please pick them, they are totally macintosh-specific so they won't
interact with anything I have in my tree. Plus I can't even built-test
them.

This leaves only two drivers still using the old binding model:
macintosh/therm_pm72 and sound/ppc/keywest. Could any of you please
convert these to the standard binding model so that I can finally get
rid of i2c_driver.attach_adapter? That would be wonderful.

Thanks again,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4

[Benjamin Herrenschmidt]

On Thu, 2014-07-31 at 08:52 +0200, Jean Delvare wrote:
> This leaves only two drivers still using the old binding model:
> macintosh/therm_pm72 and sound/ppc/keywest. Could any of you please
> convert these to the standard binding model so that I can finally get
> rid of i2c_driver.attach_adapter? That would be wonderful.

therm_pm72 is deprecated, I wrote windfarm_pm72 to replace it so that I
wouldn't have to convert the old one :-)

We can probably kill it now, it's been long enough.

As for the old audio one, that's the trickiest bit. I'm so completely
swamped at the moment, I really haven't had a chance, on the other hand
I'm not sure anybody will notice anymore if we just kill the bloody
thing ...

Cheers,
Ben.

Re: [PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4

[Jean Delvare]

Hi Ben,

Le Thursday 31 July 2014 à 19:05 +1000, Benjamin Herrenschmidt a écrit :
> On Thu, 2014-07-31 at 08:52 +0200, Jean Delvare wrote:
> > This leaves only two drivers still using the old binding model:
> > macintosh/therm_pm72 and sound/ppc/keywest. Could any of you please
> > convert these to the standard binding model so that I can finally get
> > rid of i2c_driver.attach_adapter? That would be wonderful.
> 
> therm_pm72 is deprecated, I wrote windfarm_pm72 to replace it so that I
> wouldn't have to convert the old one :-)
> 
> We can probably kill it now, it's been long enough.

Please do!

> As for the old audio one, that's the trickiest bit. I'm so completely
> swamped at the moment, I really haven't had a chance, on the other hand
> I'm not sure anybody will notice anymore if we just kill the bloody
> thing ...

Maybe we can make it depend on BROKEN, and remove the legacy API it was
using. If anyone notices and complains, they get to convert the driver.
Otherwise we just kill the driver after some time. Would that work for
you?

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4

[Goffredo Baroncelli]

On 07/31/2014 08:52 AM, Jean Delvare wrote:
> Hi Ben, Goffredo,
> 
[...]
> 
> This leaves only two drivers still using the old binding model:
> macintosh/therm_pm72 and sound/ppc/keywest. Could any of you please
> convert these to the standard binding model so that I can finally get
> rid of i2c_driver.attach_adapter? That would be wonderful.

I will try to give a look of keywest, because I can (?) test the change.

I can't update therm_pm72 because I am not in position to test it. Anyway
Ben in the other email stated that it is obsolete by the windfarm_pm72.

> 
> Thanks again,
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5