Re: [PATCH] Rename inode_cmp_iversion{+raw} to inode_eq_iversion{+raw}
On 01/31/2018 10:55 PM, Matthew Wilcox wrote:
> On Wed, Jan 31, 2018 at 09:43:09PM +0100, Goffredo Baroncelli wrote:
>> The function inode_cmp_iversion{+raw} is counter-intuitive, because it
>> returns true when the counters are different and false when these are equal.
>>
>> Rename it to inode_eq_iversion{+raw}, which will returns true when
>> the counters are equal and false otherwise.
>
> A lot of places use !inode_eq_iversion(). I think we should have both
> inode_eq_iversion() and inode_ne_iversion().
A function is needed because before doing the comparing, a "conversion"
is needed.
My feeling is that the positive "form" is the more natural. And the notion
"!*eq*" is intuitive as the "*ne*".
>
> Also, we have 'inode' in the name, why keep the 'i'? inode_eq_version()
> and inode_ne_version() are shorter. We could even go so far as
> iversion_eq() and iversion_ne() if keeping 'iversion' in the string
> is important.
All the functions introduced by Jeff are in the form inode__iversion.
So for consistency, inode_eq_iversion() makes sense.
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
[PATCH] Rename make inode_cmp_iversion{+raw} to inode_eq_iversion{+raw}
From: Goffredo Baroncelli
The function inode_cmp_iversion{+raw} is counter-intuitive, because it
returns true when the counters are different and false when these are equal.
Rename it to inode_eq_iversion{+raw}, which will returns true when
the counters are equal and false otherwise.
Signed-off-by: Goffredo Baroncelli
---
fs/affs/dir.c | 2 +-
fs/exofs/dir.c| 2 +-
fs/ext2/dir.c | 2 +-
fs/ext4/dir.c | 4 ++--
fs/ext4/inline.c | 2 +-
fs/fat/namei_vfat.c | 2 +-
fs/nfs/inode.c| 6 +++---
fs/ocfs2/dir.c| 4 ++--
fs/ufs/dir.c | 2 +-
include/linux/iversion.h | 22 +++---
security/integrity/ima/ima_main.c | 2 +-
11 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/fs/affs/dir.c b/fs/affs/dir.c
index d180b46453cf..b2bf7016e1b3 100644
--- a/fs/affs/dir.c
+++ b/fs/affs/dir.c
@@ -81,7 +81,7 @@ affs_readdir(struct file *file, struct dir_context *ctx)
* we can jump directly to where we left off.
*/
ino = (u32)(long)file->private_data;
- if (ino && inode_cmp_iversion(inode, file->f_version) == 0) {
+ if (ino && inode_eq_iversion(inode, file->f_version)) {
pr_debug("readdir() left off=%d\n", ino);
goto inside;
}
diff --git a/fs/exofs/dir.c b/fs/exofs/dir.c
index c5a53fcc43ea..f0138674c1ed 100644
--- a/fs/exofs/dir.c
+++ b/fs/exofs/dir.c
@@ -242,7 +242,7 @@ exofs_readdir(struct file *file, struct dir_context *ctx)
unsigned long n = pos >> PAGE_SHIFT;
unsigned long npages = dir_pages(inode);
unsigned chunk_mask = ~(exofs_chunk_size(inode)-1);
- bool need_revalidate = inode_cmp_iversion(inode, file->f_version);
+ bool need_revalidate = !inode_eq_iversion(inode, file->f_version);
if (pos > inode->i_size - EXOFS_DIR_REC_LEN(1))
return 0;
diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 4111085a129f..3b8114def693 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -294,7 +294,7 @@ ext2_readdir(struct file *file, struct dir_context *ctx)
unsigned long npages = dir_pages(inode);
unsigned chunk_mask = ~(ext2_chunk_size(inode)-1);
unsigned char *types = NULL;
- bool need_revalidate = inode_cmp_iversion(inode, file->f_version);
+ bool need_revalidate = !inode_eq_iversion(inode, file->f_version);
if (pos > inode->i_size - EXT2_DIR_REC_LEN(1))
return 0;
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index afda0a0499ce..da87cf757f7d 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -209,7 +209,7 @@ static int ext4_readdir(struct file *file, struct
dir_context *ctx)
* readdir(2), then we might be pointing to an invalid
* dirent right now. Scan from the start of the block
* to make sure. */
- if (inode_cmp_iversion(inode, file->f_version)) {
+ if (!inode_eq_iversion(inode, file->f_version)) {
for (i = 0; i < sb->s_blocksize && i < offset; ) {
de = (struct ext4_dir_entry_2 *)
(bh->b_data + i);
@@ -569,7 +569,7 @@ static int ext4_dx_readdir(struct file *file, struct
dir_context *ctx)
* cached entries.
*/
if ((!info->curr_node) ||
- inode_cmp_iversion(inode, file->f_version)) {
+ !inode_eq_iversion(inode, file->f_version)) {
info->curr_node = NULL;
free_rb_tree_fname(&info->root);
file->f_version = inode_query_iversion(inode);
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index a8b987b71173..adfc1f360dae 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1495,7 +1495,7 @@ int ext4_read_inline_dir(struct file *file,
* dirent right now. Scan from the start of the inline
* dir to make sure.
*/
- if (inode_cmp_iversion(inode, file->f_version)) {
+ if (!inode_eq_iversion(inode, file->f_version)) {
for (i = 0; i < extra_size && i < offset;) {
/*
* "." is with offset 0 and
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index cefea792cde8..2649759c478a 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -46,7 +46,7 @@ static int vfat_revalidate_shortname(struct dentry *dentry)
{
int ret = 1;
spin_lock(&dentry->d_lock);
- if (inode_cmp_iversion(d_inode(dentry->d_parent),
vfat_d_version(dentry)))
+ if (!inode_eq_iversion(d_inode(dentry->d_parent),
vfat_d_version(dentry)))
[RESEND][PATCH 5/5] Export the temperatures via hwmon
Export the temperature via the hwmon subsystem.
See the list below for the sensors exported:
$ cd /sys/devices/temperature/hwmon/hwmon0
$ echo "name: $(cat name)"; for i in temp*; do echo "$i: $(cat $i)"; done
name: therm_windtunnel
temp1_input: 59312
temp1_label: CPU
temp2_input: 36750
temp2_label: Case
temp3_input: 37750
temp3_label: Case2
The Case2 temperature is the sensor temperature inside the adm1030.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 96 ++--
1 file changed, 92 insertions(+), 4 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 30a6588..fd9061d 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -37,6 +37,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -58,9 +60,12 @@ static struct {
struct i2c_client *thermostat;
struct i2c_client *fan;
+ struct device *hwmon;
+
int overheat_temp; /* 100% fan at this
temp */
int overheat_hyst;
int temp;
+ int casetemp2;
int casetemp;
int fan_level; /* active fan_table
setting */
@@ -120,6 +125,66 @@ static DEVICE_ATTR(case_temperature, S_IRUGO,
show_case_temperature, NULL );
static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
+static ssize_t
+show_temp1(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.temp>>8,
+ (x.temp & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp1_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "CPU\n");
+}
+
+static ssize_t
+show_temp2(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.casetemp>>8,
+ (x.casetemp & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp2_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "Case\n");
+}
+
+
+static ssize_t
+show_temp3(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.casetemp2>>8,
+ (x.casetemp2 & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp3_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "Case2\n");
+}
+
+static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp1, NULL);
+static DEVICE_ATTR(temp1_label, S_IRUGO, show_temp1_label, NULL);
+static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp2, NULL);
+static DEVICE_ATTR(temp2_label, S_IRUGO, show_temp2_label, NULL);
+static DEVICE_ATTR(temp3_input, S_IRUGO, show_temp3, NULL);
+static DEVICE_ATTR(temp3_label, S_IRUGO, show_temp3_label, NULL);
+
+static struct attribute *therm_windtunnel_attrs[] = {
+ &dev_attr_temp1_input.attr,
+ &dev_attr_temp1_label.attr,
+ &dev_attr_temp2_input.attr,
+ &dev_attr_temp2_label.attr,
+ &dev_attr_temp3_input.attr,
+ &dev_attr_temp3_label.attr,
+
+ NULL,
+};
+
+ATTRIBUTE_GROUPS(therm_windtunnel);
+
//
/* controller thread */
//
@@ -172,16 +237,23 @@ tune_fan( int fan_setting )
static void
poll_temp( void )
{
- int temp, i, level, casetemp, tempchanged;
+ int temp, i, level, casetemp, tempchanged, casetemp2, reg06;
+ /* temperature read from ds1775 */
temp = read_reg( x.thermostat, 0, 2 );
/* this actually occurs when the computer is loaded */
if( temp < 0 )
return;
- casetemp = read_reg(x.fan, 0x0b, 1) << 8;
- casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
+ /*
+* temperatures read from the adm1030
+* casetemp is the external temperature sensor
+* casetemp2 is the internal temperature sensor
+*/
+ reg06 = read_reg(x.fan, 0x06, 1);
+ casetemp = (read_reg(x.fan, 0x0b, 1) << 8) | (reg06 & 0x07 << 5);
+ casetemp2 = (read_reg(x.fan, 0x0a, 1) << 8) | (reg06 & 0xc0);
level = -1;
for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
@@ -200,12 +272,15 @@ poll_temp( void )
* if verbose >=1 log each fan tuning
* if verbose >=2 log each cpu temperature change
*/
- tempchanged = x.temp != temp || x.casetemp != casetemp
[RESEND][PATCH][v5] therm_windtunnel does not work properly on PowerMac G4
I am resending these emails for submission. The patch is still applicable to the kernel v3.18 The previous submission was in Agoust... Hi All, 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 5 patches to solve this problem (tested on my PowerMac G4). Only the first two are strictly related to the problem, the others three may be skipped. 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 a parameter to the therm_windtunnel module to control the kernel log message 4) export the fan speed via sysfs 5) export the temperature via the hwmon subsistem 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. The patch 4) is copyied from the Bryan Christianson's patch (see debian bug #741663) The patch 5) export the temperatures via hwmon, a more standard interface. I also added the internal sensor of the adm1030, which I called "Case2", because it measure the same temperature /+/- 1C) of the "Case" sensor. I have to highlight that I tried to export also the fan speed, but I was unable to get it, without affecting the fan speed: when I set the bit 2 (TACH 1 En) of the register 0x1 (Configuration 2), it seemed that the speed every 4/5s dropped, then it raised quickly I didn't perform other test to avoid damages. Could you be so kindly to apply these patches ? PS: I am not LKML subscriber, so please put me in CC in case of reply. BR G.Baroncelli Changelog: v1: 2014/07/30 - first issue v2: 2014/08/01 - protect with a mutex the check before starting the fan daemon (to protect from parallel drivers instantation) - reduce the number of module parameters to 1 as suggested by Jean Delvare - export the fan speed via sysfs v3: 2014/08/06 - export the temperatures via hwmon - export the internal temperature sensor of the adm1030 - little cleanup due to the suggestion of checkpatch.pl ( and Jean Delvare) - removed the "(tune +0)" in the log. v4: 2014/08/07 - accepted some small cleanup suggestions from Jean Delvare - replaced SENSOR_DEVICE_ATTR with DEVICE_ATTR, and added ATTRIBUTE_GROUPS() use v5: 2014/08/09 - hanlde the return error of hwmon_device_register_with_groups() with IS_ERR() "macro" - better explanation about the source of patch #4 [1] I think that the guilty commit is commit 81e5d8646ff6bf323dddcf172aa3cef84468fa12 Author: Benjamin Herrenschmidt Date: Wed Apr 18 22:16:42 2012 + 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 [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 Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RESEND][PATCH 4/5] Return the fan speed via sysfs
Return the fan speed via sysfs:
/sys/devices/temperature/fan_level
This patch is extracted from a Bryan Christianson's bigger one (see
debian bug #741663)
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 68f1067..30a6588 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -109,9 +109,15 @@ show_case_temperature( struct device *dev, struct
device_attribute *attr, char *
return sprintf(buf, "%d.%d\n", x.casetemp>>8, (x.casetemp & 255)*10/256
);
}
+static ssize_t
+show_fan_level(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", 11 - x.fan_level);
+}
+
static DEVICE_ATTR(cpu_temperature, S_IRUGO, show_cpu_temperature, NULL );
static DEVICE_ATTR(case_temperature, S_IRUGO, show_case_temperature, NULL );
-
+static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
//
@@ -264,6 +270,7 @@ setup_hardware( void )
err = device_create_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
err |= device_create_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ err |= device_create_file(&x.of_dev->dev, &dev_attr_fan_level);
if (err)
printk(KERN_WARNING
"Failed to create temperature attribute file(s).\n");
@@ -274,6 +281,7 @@ restore_regs( void )
{
device_remove_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
device_remove_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ device_remove_file(&x.of_dev->dev, &dev_attr_fan_level);
write_reg( x.fan, 0x01, x.r1, 1 );
write_reg( x.fan, 0x20, x.r20, 1 );
--
2.1.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RESEND][PATCH 1/5] Update drivers names to the ones invoked by i2c-powermac
Update drivers names to the ones invoked by i2c-powermac:
- therm_ds1775 -> MAC,ds1775
- therm_adm1030 -> MAC,adm1030
The background fan control loop is started from
the devices probing methods.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 3b4a157..a64a06f 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -334,6 +334,23 @@ do_attach( struct i2c_adapter *adapter )
return 0;
}
+static void
+try_start_control_loop(void)
+{
+
+ mutex_lock(&x.lock);
+ if (!x.thermostat || !x.fan || x.running) {
+ mutex_unlock(&x.lock);
+ return;
+ }
+
+ x.running = 1;
+ mutex_unlock(&x.lock);
+
+ x.poll_task = kthread_run(control_loop, NULL, "g4fand");
+
+}
+
static int
do_remove(struct i2c_client *client)
{
@@ -364,6 +381,7 @@ attach_fan( struct i2c_client *cl )
printk("ADM1030 fan controller [@%02x]\n", cl->addr );
x.fan = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -397,6 +415,7 @@ attach_thermostat( struct i2c_client *cl )
x.overheat_temp = os_temp;
x.overheat_hyst = hyst_temp;
x.thermostat = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -404,8 +423,8 @@ out:
enum chip { ds1775, adm1030 };
static const struct i2c_device_id therm_windtunnel_id[] = {
- { "therm_ds1775", ds1775 },
- { "therm_adm1030", adm1030 },
+ { "MAC,ds1775", ds1775 },
+ { "MAC,adm1030", adm1030 },
{ }
};
--
2.1.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RESEND][PATCH 2/5] Remove attach_method because un-used
Remove attach_method because i2c-powermac is
in charge to instantiate the driver directly.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 35 ---
1 file changed, 35 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index a64a06f..1e50455 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -300,40 +300,6 @@ static int control_loop(void *dummy)
/* i2c probing and setup */
//
-static int
-do_attach( struct i2c_adapter *adapter )
-{
- /* scan 0x48-0x4f (DS1775) and 0x2c-2x2f (ADM1030) */
- static const unsigned short scan_ds1775[] = {
- 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f,
- I2C_CLIENT_END
- };
- static const unsigned short scan_adm1030[] = {
- 0x2c, 0x2d, 0x2e, 0x2f,
- I2C_CLIENT_END
- };
-
- if( strncmp(adapter->name, "uni-n", 5) )
- return 0;
-
- if( !x.running ) {
- struct i2c_board_info info;
-
- memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "therm_ds1775", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_ds1775, NULL);
-
- strlcpy(info.type, "therm_adm1030", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_adm1030, NULL);
-
- if( x.thermostat && x.fan ) {
- x.running = 1;
- x.poll_task = kthread_run(control_loop, NULL, "g4fand");
- }
- }
- return 0;
-}
-
static void
try_start_control_loop(void)
{
@@ -450,7 +416,6 @@ static struct i2c_driver g4fan_driver = {
.driver = {
.name = "therm_windtunnel",
},
- .attach_adapter = do_attach,
.probe = do_probe,
.remove = do_remove,
.id_table = therm_windtunnel_id,
--
2.1.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RESEND][PATCH 3/5] Add the "verbose" module option.
Add a "verbose" option to control the message in the kernel log
verbose = 0 no message
verbose = 1 log only the fan speed changes
verbose = 2 log the fan speed changes and the temperature changes
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 38 +++-
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 1e50455..68f1067 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -44,7 +44,9 @@
#include
#include
-#define LOG_TEMP 0 /* continuously log
temperature */
+static int verbose = 1;
+module_param(verbose, int, 0644);
+MODULE_PARM_DESC(verbose, "Verbosity level: 0=silent, 1=log the fan tuning,
2=log the temperature");
static struct {
volatile intrunning;
@@ -157,10 +159,6 @@ tune_fan( int fan_setting )
/* write_reg( x.fan, 0x24, val, 1 ); */
write_reg( x.fan, 0x25, val, 1 );
write_reg( x.fan, 0x20, 0, 1 );
- print_temp("CPU-temp: ", x.temp );
- if( x.casetemp )
- print_temp(", Case: ", x.casetemp );
- printk(", Fan: %d (tuned %+d)\n", 11-fan_setting,
x.fan_level-fan_setting );
x.fan_level = fan_setting;
}
@@ -168,7 +166,7 @@ tune_fan( int fan_setting )
static void
poll_temp( void )
{
- int temp, i, level, casetemp;
+ int temp, i, level, casetemp, tempchanged;
temp = read_reg( x.thermostat, 0, 2 );
@@ -179,14 +177,6 @@ poll_temp( void )
casetemp = read_reg(x.fan, 0x0b, 1) << 8;
casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
- if( LOG_TEMP && x.temp != temp ) {
- print_temp("CPU-temp: ", temp );
- print_temp(", Case: ", casetemp );
- printk(", Fan: %d\n", 11-x.fan_level );
- }
- x.temp = temp;
- x.casetemp = casetemp;
-
level = -1;
for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
;
@@ -200,6 +190,26 @@ poll_temp( void )
level = fan_table[i].fan_up_setting;
x.upind = i;
+ /*
+* if verbose >=1 log each fan tuning
+* if verbose >=2 log each cpu temperature change
+*/
+ tempchanged = x.temp != temp || x.casetemp != casetemp;
+ if ((verbose > 1 && tempchanged) ||
+ (verbose > 0 && level >= 0)) {
+ print_temp(KERN_INFO "CPU-temp: ", temp);
+ if (casetemp)
+ print_temp(", Case: ", casetemp);
+ if (level >= 0)
+ printk(", Fan: %d (tuned %+d)\n", 11-level,
+ x.fan_level-level);
+ else
+ printk(", Fan: %d\n", 11-x.fan_level);
+ }
+
+ x.temp = temp;
+ x.casetemp = casetemp;
+
if( level >= 0 )
tune_fan( level );
}
--
2.1.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][v5] therm_windtunnel does not work properly on PowerMac G4
Hi Benjamin, do you have any feedback about this ? Do you think that it would be possible to include these patches in a next pull-for-linus ? Let me know, if you want other changes. BR G.Baroncelli On 08/09/2014 08:49 AM, Goffredo Baroncelli wrote: > Hi All, > > 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 5 patches to solve this > problem (tested on my PowerMac G4). Only the first two are strictly > related to the problem, the others three may be skipped. > > 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 a parameter to the therm_windtunnel module > to control the kernel log message > > 4) export the fan speed via sysfs > > 5) export the temperature via the hwmon subsistem > > 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. > The patch 4) is copyied from the Bryan Christianson's patch (see > debian bug #741663) > The patch 5) export the temperatures via hwmon, a more standard > interface. I also added the internal sensor of the adm1030, which > I called "Case2", because it measure the same temperature /+/- 1C) > of the "Case" sensor. > > I have to highlight that I tried to export also the fan speed, > but I was unable to get it, without affecting the fan speed: > when I set the bit 2 (TACH 1 En) of the register 0x1 > (Configuration 2), it seemed that the speed every 4/5s dropped, > then it raised quickly > I didn't perform other test to avoid damages. > > Could you be so kindly to apply these patches ? > > PS: > I am not LKML subscriber, so please put me in CC in case of reply. > > BR > G.Baroncelli > > Changelog: > v1: 2014/07/30 > - first issue > v2: 2014/08/01 > - protect with a mutex the check before starting the fan > daemon (to protect from parallel drivers instantation) > - reduce the number of module parameters to 1 as suggested by > Jean Delvare > - export the fan speed via sysfs > v3: 2014/08/06 > - export the temperatures via hwmon > - export the internal temperature sensor of the adm1030 > - little cleanup due to the suggestion of checkpatch.pl ( > and Jean Delvare) > - removed the "(tune +0)" in the log. > v4: 2014/08/07 > - accepted some small cleanup suggestions from Jean Delvare > - replaced SENSOR_DEVICE_ATTR with DEVICE_ATTR, and > added ATTRIBUTE_GROUPS() use > v5: 2014/08/09 > - hanlde the return error of > hwmon_device_register_with_groups() with IS_ERR() "macro" > - better explanation about the source of patch #4 > > [1] I think that the guilty commit is > commit 81e5d8646ff6bf323dddcf172aa3cef84468fa12 > Author: Benjamin Herrenschmidt > Date: Wed Apr 18 22:16:42 2012 + > 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. > &g
[PATCH 5/5] Export the temperatures via hwmon
Export the temperature via the hwmon subsystem.
See the list below for the sensors exported:
$ cd /sys/devices/temperature/hwmon/hwmon0
$ echo "name: $(cat name)"; for i in temp*; do echo "$i: $(cat $i)"; done
name: therm_windtunnel
temp1_input: 59312
temp1_label: CPU
temp2_input: 36750
temp2_label: Case
temp3_input: 37750
temp3_label: Case2
The Case2 temperature is the sensor temperature inside the adm1030.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 96 ++--
1 file changed, 92 insertions(+), 4 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 30a6588..fd9061d 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -37,6 +37,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -58,9 +60,12 @@ static struct {
struct i2c_client *thermostat;
struct i2c_client *fan;
+ struct device *hwmon;
+
int overheat_temp; /* 100% fan at this
temp */
int overheat_hyst;
int temp;
+ int casetemp2;
int casetemp;
int fan_level; /* active fan_table
setting */
@@ -120,6 +125,66 @@ static DEVICE_ATTR(case_temperature, S_IRUGO,
show_case_temperature, NULL );
static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
+static ssize_t
+show_temp1(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.temp>>8,
+ (x.temp & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp1_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "CPU\n");
+}
+
+static ssize_t
+show_temp2(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.casetemp>>8,
+ (x.casetemp & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp2_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "Case\n");
+}
+
+
+static ssize_t
+show_temp3(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.casetemp2>>8,
+ (x.casetemp2 & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp3_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "Case2\n");
+}
+
+static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp1, NULL);
+static DEVICE_ATTR(temp1_label, S_IRUGO, show_temp1_label, NULL);
+static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp2, NULL);
+static DEVICE_ATTR(temp2_label, S_IRUGO, show_temp2_label, NULL);
+static DEVICE_ATTR(temp3_input, S_IRUGO, show_temp3, NULL);
+static DEVICE_ATTR(temp3_label, S_IRUGO, show_temp3_label, NULL);
+
+static struct attribute *therm_windtunnel_attrs[] = {
+ &dev_attr_temp1_input.attr,
+ &dev_attr_temp1_label.attr,
+ &dev_attr_temp2_input.attr,
+ &dev_attr_temp2_label.attr,
+ &dev_attr_temp3_input.attr,
+ &dev_attr_temp3_label.attr,
+
+ NULL,
+};
+
+ATTRIBUTE_GROUPS(therm_windtunnel);
+
//
/* controller thread */
//
@@ -172,16 +237,23 @@ tune_fan( int fan_setting )
static void
poll_temp( void )
{
- int temp, i, level, casetemp, tempchanged;
+ int temp, i, level, casetemp, tempchanged, casetemp2, reg06;
+ /* temperature read from ds1775 */
temp = read_reg( x.thermostat, 0, 2 );
/* this actually occurs when the computer is loaded */
if( temp < 0 )
return;
- casetemp = read_reg(x.fan, 0x0b, 1) << 8;
- casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
+ /*
+* temperatures read from the adm1030
+* casetemp is the external temperature sensor
+* casetemp2 is the internal temperature sensor
+*/
+ reg06 = read_reg(x.fan, 0x06, 1);
+ casetemp = (read_reg(x.fan, 0x0b, 1) << 8) | (reg06 & 0x07 << 5);
+ casetemp2 = (read_reg(x.fan, 0x0a, 1) << 8) | (reg06 & 0xc0);
level = -1;
for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
@@ -200,12 +272,15 @@ poll_temp( void )
* if verbose >=1 log each fan tuning
* if verbose >=2 log each cpu temperature change
*/
- tempchanged = x.temp != temp || x.casetemp != casetemp
[PATCH 2/5] Remove attach_method because un-used
Remove attach_method because i2c-powermac is
in charge to instantiate the driver directly.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 35 ---
1 file changed, 35 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index a64a06f..1e50455 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -300,40 +300,6 @@ static int control_loop(void *dummy)
/* i2c probing and setup */
//
-static int
-do_attach( struct i2c_adapter *adapter )
-{
- /* scan 0x48-0x4f (DS1775) and 0x2c-2x2f (ADM1030) */
- static const unsigned short scan_ds1775[] = {
- 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f,
- I2C_CLIENT_END
- };
- static const unsigned short scan_adm1030[] = {
- 0x2c, 0x2d, 0x2e, 0x2f,
- I2C_CLIENT_END
- };
-
- if( strncmp(adapter->name, "uni-n", 5) )
- return 0;
-
- if( !x.running ) {
- struct i2c_board_info info;
-
- memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "therm_ds1775", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_ds1775, NULL);
-
- strlcpy(info.type, "therm_adm1030", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_adm1030, NULL);
-
- if( x.thermostat && x.fan ) {
- x.running = 1;
- x.poll_task = kthread_run(control_loop, NULL, "g4fand");
- }
- }
- return 0;
-}
-
static void
try_start_control_loop(void)
{
@@ -450,7 +416,6 @@ static struct i2c_driver g4fan_driver = {
.driver = {
.name = "therm_windtunnel",
},
- .attach_adapter = do_attach,
.probe = do_probe,
.remove = do_remove,
.id_table = therm_windtunnel_id,
--
2.1.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] Update drivers names to the ones invoked by i2c-powermac
Update drivers names to the ones invoked by i2c-powermac:
- therm_ds1775 -> MAC,ds1775
- therm_adm1030 -> MAC,adm1030
The background fan control loop is started from
the devices probing methods.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 3b4a157..a64a06f 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -334,6 +334,23 @@ do_attach( struct i2c_adapter *adapter )
return 0;
}
+static void
+try_start_control_loop(void)
+{
+
+ mutex_lock(&x.lock);
+ if (!x.thermostat || !x.fan || x.running) {
+ mutex_unlock(&x.lock);
+ return;
+ }
+
+ x.running = 1;
+ mutex_unlock(&x.lock);
+
+ x.poll_task = kthread_run(control_loop, NULL, "g4fand");
+
+}
+
static int
do_remove(struct i2c_client *client)
{
@@ -364,6 +381,7 @@ attach_fan( struct i2c_client *cl )
printk("ADM1030 fan controller [@%02x]\n", cl->addr );
x.fan = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -397,6 +415,7 @@ attach_thermostat( struct i2c_client *cl )
x.overheat_temp = os_temp;
x.overheat_hyst = hyst_temp;
x.thermostat = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -404,8 +423,8 @@ out:
enum chip { ds1775, adm1030 };
static const struct i2c_device_id therm_windtunnel_id[] = {
- { "therm_ds1775", ds1775 },
- { "therm_adm1030", adm1030 },
+ { "MAC,ds1775", ds1775 },
+ { "MAC,adm1030", adm1030 },
{ }
};
--
2.1.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] Return the fan speed via sysfs
Return the fan speed via sysfs:
/sys/devices/temperature/fan_level
This patch is extracted from a Bryan Christianson's bigger one (see
debian bug #741663)
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 68f1067..30a6588 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -109,9 +109,15 @@ show_case_temperature( struct device *dev, struct
device_attribute *attr, char *
return sprintf(buf, "%d.%d\n", x.casetemp>>8, (x.casetemp & 255)*10/256
);
}
+static ssize_t
+show_fan_level(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", 11 - x.fan_level);
+}
+
static DEVICE_ATTR(cpu_temperature, S_IRUGO, show_cpu_temperature, NULL );
static DEVICE_ATTR(case_temperature, S_IRUGO, show_case_temperature, NULL );
-
+static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
//
@@ -264,6 +270,7 @@ setup_hardware( void )
err = device_create_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
err |= device_create_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ err |= device_create_file(&x.of_dev->dev, &dev_attr_fan_level);
if (err)
printk(KERN_WARNING
"Failed to create temperature attribute file(s).\n");
@@ -274,6 +281,7 @@ restore_regs( void )
{
device_remove_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
device_remove_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ device_remove_file(&x.of_dev->dev, &dev_attr_fan_level);
write_reg( x.fan, 0x01, x.r1, 1 );
write_reg( x.fan, 0x20, x.r20, 1 );
--
2.1.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/5] Add the "verbose" module option.
Add a "verbose" option to control the message in the kernel log
verbose = 0 no message
verbose = 1 log only the fan speed changes
verbose = 2 log the fan speed changes and the temperature changes
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 38 +++-
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 1e50455..68f1067 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -44,7 +44,9 @@
#include
#include
-#define LOG_TEMP 0 /* continuously log
temperature */
+static int verbose = 1;
+module_param(verbose, int, 0644);
+MODULE_PARM_DESC(verbose, "Verbosity level: 0=silent, 1=log the fan tuning,
2=log the temperature");
static struct {
volatile intrunning;
@@ -157,10 +159,6 @@ tune_fan( int fan_setting )
/* write_reg( x.fan, 0x24, val, 1 ); */
write_reg( x.fan, 0x25, val, 1 );
write_reg( x.fan, 0x20, 0, 1 );
- print_temp("CPU-temp: ", x.temp );
- if( x.casetemp )
- print_temp(", Case: ", x.casetemp );
- printk(", Fan: %d (tuned %+d)\n", 11-fan_setting,
x.fan_level-fan_setting );
x.fan_level = fan_setting;
}
@@ -168,7 +166,7 @@ tune_fan( int fan_setting )
static void
poll_temp( void )
{
- int temp, i, level, casetemp;
+ int temp, i, level, casetemp, tempchanged;
temp = read_reg( x.thermostat, 0, 2 );
@@ -179,14 +177,6 @@ poll_temp( void )
casetemp = read_reg(x.fan, 0x0b, 1) << 8;
casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
- if( LOG_TEMP && x.temp != temp ) {
- print_temp("CPU-temp: ", temp );
- print_temp(", Case: ", casetemp );
- printk(", Fan: %d\n", 11-x.fan_level );
- }
- x.temp = temp;
- x.casetemp = casetemp;
-
level = -1;
for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
;
@@ -200,6 +190,26 @@ poll_temp( void )
level = fan_table[i].fan_up_setting;
x.upind = i;
+ /*
+* if verbose >=1 log each fan tuning
+* if verbose >=2 log each cpu temperature change
+*/
+ tempchanged = x.temp != temp || x.casetemp != casetemp;
+ if ((verbose > 1 && tempchanged) ||
+ (verbose > 0 && level >= 0)) {
+ print_temp(KERN_INFO "CPU-temp: ", temp);
+ if (casetemp)
+ print_temp(", Case: ", casetemp);
+ if (level >= 0)
+ printk(", Fan: %d (tuned %+d)\n", 11-level,
+ x.fan_level-level);
+ else
+ printk(", Fan: %d\n", 11-x.fan_level);
+ }
+
+ x.temp = temp;
+ x.casetemp = casetemp;
+
if( level >= 0 )
tune_fan( level );
}
--
2.1.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH][v5] therm_windtunnel does not work properly on PowerMac G4
Hi All, 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 5 patches to solve this problem (tested on my PowerMac G4). Only the first two are strictly related to the problem, the others three may be skipped. 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 a parameter to the therm_windtunnel module to control the kernel log message 4) export the fan speed via sysfs 5) export the temperature via the hwmon subsistem 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. The patch 4) is copyied from the Bryan Christianson's patch (see debian bug #741663) The patch 5) export the temperatures via hwmon, a more standard interface. I also added the internal sensor of the adm1030, which I called "Case2", because it measure the same temperature /+/- 1C) of the "Case" sensor. I have to highlight that I tried to export also the fan speed, but I was unable to get it, without affecting the fan speed: when I set the bit 2 (TACH 1 En) of the register 0x1 (Configuration 2), it seemed that the speed every 4/5s dropped, then it raised quickly I didn't perform other test to avoid damages. Could you be so kindly to apply these patches ? PS: I am not LKML subscriber, so please put me in CC in case of reply. BR G.Baroncelli Changelog: v1: 2014/07/30 - first issue v2: 2014/08/01 - protect with a mutex the check before starting the fan daemon (to protect from parallel drivers instantation) - reduce the number of module parameters to 1 as suggested by Jean Delvare - export the fan speed via sysfs v3: 2014/08/06 - export the temperatures via hwmon - export the internal temperature sensor of the adm1030 - little cleanup due to the suggestion of checkpatch.pl ( and Jean Delvare) - removed the "(tune +0)" in the log. v4: 2014/08/07 - accepted some small cleanup suggestions from Jean Delvare - replaced SENSOR_DEVICE_ATTR with DEVICE_ATTR, and added ATTRIBUTE_GROUPS() use v5: 2014/08/09 - hanlde the return error of hwmon_device_register_with_groups() with IS_ERR() "macro" - better explanation about the source of patch #4 [1] I think that the guilty commit is commit 81e5d8646ff6bf323dddcf172aa3cef84468fa12 Author: Benjamin Herrenschmidt Date: Wed Apr 18 22:16:42 2012 + 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 [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 Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] Export the temperatures via hwmon
On 08/08/2014 06:30 PM, Guenter Roeck wrote: > On 08/08/2014 07:54 AM, Goffredo Baroncelli wrote: [...] > I just noticed - hwmon_device_register_with_groups() returns ERR_PTR > on errors, not NULL, so checking for a NULL return value won't help > much if there is an error. Thanks, due to this, in the next day I will issue a new revision. > Guenter > > -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] Export the temperatures via hwmon
On 08/07/2014 11:19 PM, Matt Helsley wrote: > On Thu, Aug 07, 2014 at 07:50:00PM +0200, Goffredo Baroncelli wrote: >> On 08/07/2014 09:36 AM, Guenter Roeck wrote: >>> On 08/06/2014 11:52 PM, Jean Delvare wrote: >>>> Hi Guenter, >>>> >>>> On Wed, 06 Aug 2014 23:20:32 -0700, Guenter Roeck wrote: >>>>> Patch 4/5 is "Return the fan speed via sysfs: >>>>> /sys/devices/temperature/fan_level". >>>>> >>>>> So you are saying that returning the fan speed with a non-hwmon attribute >>>>> works, >>>>> but returning it with a hwmon attribute doesn't ? Not really sure if I >>>>> understand >>>>> your logic. Either fan_level doesn't return the fan speed (or an >>>>> abstraction of it), >>>>> or something in your line of argument is inconsistent. >>>> >>>> fan_level is a fan speed _control_ value, like pwm1. It is not a fan >>>> speed monitoring value. >>>> >>> Ah, ok. The patch description doesn't seem to match, though. >>> And why not export it as pwm1, if that is what it is ? >>> >>> Guenter >>> >>> >> >> the exported fan_level value is a coefficient near proportional to the speed >> [*]; >> so it is not the speed nor the pwm. > >> I tried to read the pwm/speed value, but when I did it, every 5/6 seconds the >> fan seemed to stop for 1s, then the speed raised So I stopped the test. >> >> These patches (the first two) solved a real issue: with the last kernels this >> driver doesn't work at all, and the fan go to maximum speed (very loud !) >> The other three are an improvement. >> >> When (if) these patches will be accepted I want to write another solution, >> but definitely not now. And even if it would work for me, it is very likely >> that will be accepted because nobody is able to test it on all hardware. >> >> BR >> G.Baroncelli >> >> [*] It is a bit more complicated: basically the adm1030 controls the fan >> speed >> on the basis of an external temperature sensor (the "case" sensor). >> Higher is this temperature higher is the fan speed. >> The pwm of the fan is computed on the basis of the following value: >> - min_temp >> - range_temp -> max_temp=min_temp+range_temp >> - min_pwm_value >> and of course >> - "case" sensor >> >> >> The fan_level is an index computed on the basis of the "cpu" temperature ( >> which is different from the "case" temperature referred above). This >> temperature is used to update the min_temp above. >> >> So the fan speed id computed on the basis of two external sensor: >> - cpu sensor >> - case sensor. >> >> To complicate further the things, the range_temp is set to an undocumented >> value: this parameter is set via a register (0x25, bit 2:0), which accepted >> values between 0x00 and 0x04. However it is set to 0x07 >> >> See >> http://lxr.free-electrons.com/source/drivers/macintosh/therm_windtunnel.c#L153 >> and the adm1030 datasheet. >> >> I am reluctant to change this thing because this driver is for an obsolete >> platform (apple stopped the powermac in 2004/2005), and bad or good I have to >> suppose that it works. Moreover I am not in the position to test the changes >> outside my hardware (1 powermac DP@1Ghz mdd) > > This seems like a valuable explanation of the meaning of the fan_level > values. When I read the code I was wondering what the magic number "11" was > doing for instance and this helps clear it up a little. It's a nit perhaps > but some of this information might be useful in the commit message for that > patch, the code, or both. I will take in account your suggestion. In case of another revision, I will improve the comment. > Also, I think the patches from "Bryan" should have a "From:" attribution line > at the start of the commit message (See Documentation/SubmittingPatches). Thanks for the suggestion. In case of another revision I will put the "From:" tag. > Cheers, > -Matt Helsley > -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] Export the temperatures via hwmon
On 08/07/2014 08:16 PM, Guenter Roeck wrote: > On Thu, Aug 07, 2014 at 07:50:00PM +0200, Goffredo Baroncelli wrote: >> When (if) these patches will be accepted I want to write another solution, >> but definitely not now. And even if it would work for me, it is very likely >> that will be accepted because nobody is able to test it on all hardware. >> > Might have been easier to just drop all this non-standard code and instantiate > the adm1030 using the adm1031 driver instead. I really like the idea, and this is a possible future work; but now we can't. We have to suppose that the current driver work (they stopped to work because it was changed the i2c api) and because I can't test a change on all the apple hardware (powermac, apple laptop...), I am allowed to make only very small changes. My (our) first goal is to bring to life *this* driver. I repeat I am fully convinced that re-using the "official" i2c drive/client is the "right thing to do" (tm). But due to the lacking of the hardware where it is possible to test this module, we need to be very conservative. > > Guenter > Goffredo -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] Update drivers names to the ones invoked by i2c-powermac
Update drivers names to the ones invoked by i2c-powermac:
- therm_ds1775 -> MAC,ds1775
- therm_adm1030 -> MAC,adm1030
The background fan control loop is started from
the devices probing methods.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 3b4a157..a64a06f 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -334,6 +334,23 @@ do_attach( struct i2c_adapter *adapter )
return 0;
}
+static void
+try_start_control_loop(void)
+{
+
+ mutex_lock(&x.lock);
+ if (!x.thermostat || !x.fan || x.running) {
+ mutex_unlock(&x.lock);
+ return;
+ }
+
+ x.running = 1;
+ mutex_unlock(&x.lock);
+
+ x.poll_task = kthread_run(control_loop, NULL, "g4fand");
+
+}
+
static int
do_remove(struct i2c_client *client)
{
@@ -364,6 +381,7 @@ attach_fan( struct i2c_client *cl )
printk("ADM1030 fan controller [@%02x]\n", cl->addr );
x.fan = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -397,6 +415,7 @@ attach_thermostat( struct i2c_client *cl )
x.overheat_temp = os_temp;
x.overheat_hyst = hyst_temp;
x.thermostat = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -404,8 +423,8 @@ out:
enum chip { ds1775, adm1030 };
static const struct i2c_device_id therm_windtunnel_id[] = {
- { "therm_ds1775", ds1775 },
- { "therm_adm1030", adm1030 },
+ { "MAC,ds1775", ds1775 },
+ { "MAC,adm1030", adm1030 },
{ }
};
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH][v4] therm_windtunnel does not work properly on PowerMac G4
Hi All, 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 5 patches to solve this problem (tested on my PowerMac G4). Only the first two are strictly related to the problem, the others three may be skipped. 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 a parameter to the therm_windtunnel module to control the kernel log message 4) export the fan speed via sysfs 5) export the temperature via the hwmon subsistem 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. The patch 4) is copyied from the Bryan Christianson's patch (see debian bug #741663) The patch 5) export the temperatures via hwmon, a more standard interface. I also added the internal sensor of the adm1030, which I called "Case2", because it measure the same temperature /+/- 1C) of the "Case" sensor. I have to highlight that I tried to export also the fan speed, but I was unable to get it, without affecting the fan speed: when I set the bit 2 (TACH 1 En) of the register 0x1 (Configuration 2), it seemed that the speed every 4/5s dropped, then it raised quickly I didn't perform other test to avoid damages. Could you be so kindly to apply these patches ? PS: I am not LKML subscriber, so please put me in CC in case of reply. BR G.Baroncelli Changelog: v1: 2014/07/30 - first issue v2: 2014/08/01 - protect with a mutex the check before starting the fan daemon (to protect from parallel drivers instantation) - reduce the number of module parameters to 1 as suggested by Jean Delvare - export the fan speed via sysfs v3: 2014/08/06 - export the temperatures via hwmon - export the internal temperature sensor of the adm1030 - little cleanup due to the suggestion of checkpatch.pl ( and Jean Delvare) - removed the "(tune +0)" in the log. v4: 2014/08/07 - accepted some small cleanup suggestions from Jean Delvare - replaced SENSOR_DEVICE_ATTR with DEVICE_ATTR, and added ATTRIBUTE_GROUPS() use [1] I think that the guilty commit is commit 81e5d8646ff6bf323dddcf172aa3cef84468fa12 Author: Benjamin Herrenschmidt Date: Wed Apr 18 22:16:42 2012 + 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 [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 Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/5] Add the "verbose" module option.
Add a "verbose" option to control the message in the kernel log
verbose = 0 no message
verbose = 1 log only the fan speed changes
verbose = 2 log the fan speed changes and the temperature changes
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 38 +++-
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 1e50455..68f1067 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -44,7 +44,9 @@
#include
#include
-#define LOG_TEMP 0 /* continuously log
temperature */
+static int verbose = 1;
+module_param(verbose, int, 0644);
+MODULE_PARM_DESC(verbose, "Verbosity level: 0=silent, 1=log the fan tuning,
2=log the temperature");
static struct {
volatile intrunning;
@@ -157,10 +159,6 @@ tune_fan( int fan_setting )
/* write_reg( x.fan, 0x24, val, 1 ); */
write_reg( x.fan, 0x25, val, 1 );
write_reg( x.fan, 0x20, 0, 1 );
- print_temp("CPU-temp: ", x.temp );
- if( x.casetemp )
- print_temp(", Case: ", x.casetemp );
- printk(", Fan: %d (tuned %+d)\n", 11-fan_setting,
x.fan_level-fan_setting );
x.fan_level = fan_setting;
}
@@ -168,7 +166,7 @@ tune_fan( int fan_setting )
static void
poll_temp( void )
{
- int temp, i, level, casetemp;
+ int temp, i, level, casetemp, tempchanged;
temp = read_reg( x.thermostat, 0, 2 );
@@ -179,14 +177,6 @@ poll_temp( void )
casetemp = read_reg(x.fan, 0x0b, 1) << 8;
casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
- if( LOG_TEMP && x.temp != temp ) {
- print_temp("CPU-temp: ", temp );
- print_temp(", Case: ", casetemp );
- printk(", Fan: %d\n", 11-x.fan_level );
- }
- x.temp = temp;
- x.casetemp = casetemp;
-
level = -1;
for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
;
@@ -200,6 +190,26 @@ poll_temp( void )
level = fan_table[i].fan_up_setting;
x.upind = i;
+ /*
+* if verbose >=1 log each fan tuning
+* if verbose >=2 log each cpu temperature change
+*/
+ tempchanged = x.temp != temp || x.casetemp != casetemp;
+ if ((verbose > 1 && tempchanged) ||
+ (verbose > 0 && level >= 0)) {
+ print_temp(KERN_INFO "CPU-temp: ", temp);
+ if (casetemp)
+ print_temp(", Case: ", casetemp);
+ if (level >= 0)
+ printk(", Fan: %d (tuned %+d)\n", 11-level,
+ x.fan_level-level);
+ else
+ printk(", Fan: %d\n", 11-x.fan_level);
+ }
+
+ x.temp = temp;
+ x.casetemp = casetemp;
+
if( level >= 0 )
tune_fan( level );
}
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] Return the fan speed via sysfs
Return the fan speed via sysfs:
/sys/devices/temperature/fan_level
This patch is copied from the Bryan Christianson's patch (see
debian bug #741663)
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 68f1067..30a6588 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -109,9 +109,15 @@ show_case_temperature( struct device *dev, struct
device_attribute *attr, char *
return sprintf(buf, "%d.%d\n", x.casetemp>>8, (x.casetemp & 255)*10/256
);
}
+static ssize_t
+show_fan_level(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", 11 - x.fan_level);
+}
+
static DEVICE_ATTR(cpu_temperature, S_IRUGO, show_cpu_temperature, NULL );
static DEVICE_ATTR(case_temperature, S_IRUGO, show_case_temperature, NULL );
-
+static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
//
@@ -264,6 +270,7 @@ setup_hardware( void )
err = device_create_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
err |= device_create_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ err |= device_create_file(&x.of_dev->dev, &dev_attr_fan_level);
if (err)
printk(KERN_WARNING
"Failed to create temperature attribute file(s).\n");
@@ -274,6 +281,7 @@ restore_regs( void )
{
device_remove_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
device_remove_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ device_remove_file(&x.of_dev->dev, &dev_attr_fan_level);
write_reg( x.fan, 0x01, x.r1, 1 );
write_reg( x.fan, 0x20, x.r20, 1 );
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] Export the temperatures via hwmon
Export the temperature via the hwmon subsystem.
See the list below for the sensors exported:
$ cd /sys/devices/temperature/hwmon/hwmon0
$ echo "name: $(cat name)"; for i in temp*; do echo "$i: $(cat $i)"; done
name: therm_windtunnel
temp1_input: 59312
temp1_label: CPU
temp2_input: 36750
temp2_label: Case
temp3_input: 37750
temp3_label: Case2
The Case2 temperature is the sensor temperature inside the adm1030.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 94 ++--
1 file changed, 90 insertions(+), 4 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 30a6588..f41b0d3 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -37,6 +37,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -58,9 +60,12 @@ static struct {
struct i2c_client *thermostat;
struct i2c_client *fan;
+ struct device *hwmon;
+
int overheat_temp; /* 100% fan at this
temp */
int overheat_hyst;
int temp;
+ int casetemp2;
int casetemp;
int fan_level; /* active fan_table
setting */
@@ -120,6 +125,66 @@ static DEVICE_ATTR(case_temperature, S_IRUGO,
show_case_temperature, NULL );
static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
+static ssize_t
+show_temp1(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.temp>>8,
+ (x.temp & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp1_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "CPU\n");
+}
+
+static ssize_t
+show_temp2(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.casetemp>>8,
+ (x.casetemp & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp2_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "Case\n");
+}
+
+
+static ssize_t
+show_temp3(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.casetemp2>>8,
+ (x.casetemp2 & 0xff) * 1000 / 256);
+}
+
+static ssize_t
+show_temp3_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "Case2\n");
+}
+
+static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp1, NULL);
+static DEVICE_ATTR(temp1_label, S_IRUGO, show_temp1_label, NULL);
+static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp2, NULL);
+static DEVICE_ATTR(temp2_label, S_IRUGO, show_temp2_label, NULL);
+static DEVICE_ATTR(temp3_input, S_IRUGO, show_temp3, NULL);
+static DEVICE_ATTR(temp3_label, S_IRUGO, show_temp3_label, NULL);
+
+static struct attribute *therm_windtunnel_attrs[] = {
+ &dev_attr_temp1_input.attr,
+ &dev_attr_temp1_label.attr,
+ &dev_attr_temp2_input.attr,
+ &dev_attr_temp2_label.attr,
+ &dev_attr_temp3_input.attr,
+ &dev_attr_temp3_label.attr,
+
+ NULL,
+};
+
+ATTRIBUTE_GROUPS(therm_windtunnel);
+
//
/* controller thread */
//
@@ -172,16 +237,23 @@ tune_fan( int fan_setting )
static void
poll_temp( void )
{
- int temp, i, level, casetemp, tempchanged;
+ int temp, i, level, casetemp, tempchanged, casetemp2, reg06;
+ /* temperature read from ds1775 */
temp = read_reg( x.thermostat, 0, 2 );
/* this actually occurs when the computer is loaded */
if( temp < 0 )
return;
- casetemp = read_reg(x.fan, 0x0b, 1) << 8;
- casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
+ /*
+* temperatures read from the adm1030
+* casetemp is the external temperature sensor
+* casetemp2 is the internal temperature sensor
+*/
+ reg06 = read_reg(x.fan, 0x06, 1);
+ casetemp = (read_reg(x.fan, 0x0b, 1) << 8) | (reg06 & 0x07 << 5);
+ casetemp2 = (read_reg(x.fan, 0x0a, 1) << 8) | (reg06 & 0xc0);
level = -1;
for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
@@ -200,12 +272,15 @@ poll_temp( void )
* if verbose >=1 log each fan tuning
* if verbose >=2 log each cpu temperature change
*/
- tempchanged = x.temp != temp || x.casetemp != casetemp
[PATCH 2/5] Remove attach_method because un-used
Remove attach_method because i2c-powermac is
in charge to instantiate the driver directly.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 35 ---
1 file changed, 35 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index a64a06f..1e50455 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -300,40 +300,6 @@ static int control_loop(void *dummy)
/* i2c probing and setup */
//
-static int
-do_attach( struct i2c_adapter *adapter )
-{
- /* scan 0x48-0x4f (DS1775) and 0x2c-2x2f (ADM1030) */
- static const unsigned short scan_ds1775[] = {
- 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f,
- I2C_CLIENT_END
- };
- static const unsigned short scan_adm1030[] = {
- 0x2c, 0x2d, 0x2e, 0x2f,
- I2C_CLIENT_END
- };
-
- if( strncmp(adapter->name, "uni-n", 5) )
- return 0;
-
- if( !x.running ) {
- struct i2c_board_info info;
-
- memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "therm_ds1775", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_ds1775, NULL);
-
- strlcpy(info.type, "therm_adm1030", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_adm1030, NULL);
-
- if( x.thermostat && x.fan ) {
- x.running = 1;
- x.poll_task = kthread_run(control_loop, NULL, "g4fand");
- }
- }
- return 0;
-}
-
static void
try_start_control_loop(void)
{
@@ -450,7 +416,6 @@ static struct i2c_driver g4fan_driver = {
.driver = {
.name = "therm_windtunnel",
},
- .attach_adapter = do_attach,
.probe = do_probe,
.remove = do_remove,
.id_table = therm_windtunnel_id,
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] Export the temperatures via hwmon
On 08/07/2014 09:36 AM, Guenter Roeck wrote: > On 08/06/2014 11:52 PM, Jean Delvare wrote: >> Hi Guenter, >> >> On Wed, 06 Aug 2014 23:20:32 -0700, Guenter Roeck wrote: >>> Patch 4/5 is "Return the fan speed via sysfs: >>> /sys/devices/temperature/fan_level". >>> >>> So you are saying that returning the fan speed with a non-hwmon attribute >>> works, >>> but returning it with a hwmon attribute doesn't ? Not really sure if I >>> understand >>> your logic. Either fan_level doesn't return the fan speed (or an >>> abstraction of it), >>> or something in your line of argument is inconsistent. >> >> fan_level is a fan speed _control_ value, like pwm1. It is not a fan >> speed monitoring value. >> > Ah, ok. The patch description doesn't seem to match, though. > And why not export it as pwm1, if that is what it is ? > > Guenter > > the exported fan_level value is a coefficient near proportional to the speed [*]; so it is not the speed nor the pwm. I tried to read the pwm/speed value, but when I did it, every 5/6 seconds the fan seemed to stop for 1s, then the speed raised So I stopped the test. These patches (the first two) solved a real issue: with the last kernels this driver doesn't work at all, and the fan go to maximum speed (very loud !) The other three are an improvement. When (if) these patches will be accepted I want to write another solution, but definitely not now. And even if it would work for me, it is very likely that will be accepted because nobody is able to test it on all hardware. BR G.Baroncelli [*] It is a bit more complicated: basically the adm1030 controls the fan speed on the basis of an external temperature sensor (the "case" sensor). Higher is this temperature higher is the fan speed. The pwm of the fan is computed on the basis of the following value: - min_temp - range_temp -> max_temp=min_temp+range_temp - min_pwm_value and of course - "case" sensor The fan_level is an index computed on the basis of the "cpu" temperature ( which is different from the "case" temperature referred above). This temperature is used to update the min_temp above. So the fan speed id computed on the basis of two external sensor: - cpu sensor - case sensor. To complicate further the things, the range_temp is set to an undocumented value: this parameter is set via a register (0x25, bit 2:0), which accepted values between 0x00 and 0x04. However it is set to 0x07 See http://lxr.free-electrons.com/source/drivers/macintosh/therm_windtunnel.c#L153 and the adm1030 datasheet. I am reluctant to change this thing because this driver is for an obsolete platform (apple stopped the powermac in 2004/2005), and bad or good I have to suppose that it works. Moreover I am not in the position to test the changes outside my hardware (1 powermac DP@1Ghz mdd) -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/5] Add the "verbose" module option.
On 08/07/2014 06:43 PM, Jean Delvare wrote:
> On Thu, 07 Aug 2014 18:29:23 +0200, Goffredo Baroncelli wrote:
>> On 08/07/2014 10:52 AM, Jean Delvare wrote:
>>> Le Wednesday 06 August 2014 à 21:05 +, Goffredo Baroncelli a écrit :
>>>> + */
>>>> + tempchanged = x.temp != temp || x.casetemp != casetemp;
>>>> + if ((verbose > 1 && tempchanged) ||
>>>> + (verbose > 0 && level >= 0)) {
>>>> + printk(KERN_INFO);
>>>> + print_temp("CPU-temp: ", temp);
>>>
>>> This can be written more efficiently as a single statement:
>>>
>>> print_temp(KERN_INFO "CPU-temp: ", temp);
>>
>> I suppose that KERN_* has to be in the beginning of the line.
>
> Correct.
>
>> Because a single line is composed by several prink,
>
> In this case, it is, but FYI, this is generally discouraged. The reason
> is that another piece of the kernel may be calling printk at the same
> time, and then that other message may split your own message into
> pieces. If you run checkpatch.pl on this file, you'll see it complains
> about this.
>
>> KERN_INFO has
>> to be only in the first printk. To me it seems more polite to have
>> one printk for the level, and the others (there are more than one)
>> for the message parts.
>
> The fewer printks is better. Ideally there would be only one to avoid
> the risk of line splitting altogether. I understand this isn't easy to
> achieve in this case, but I still believe that you shouldn't have more
> calls to printk than necessary, to reduce the risk.
>
Ok, now I understand the reason. I will remove the first printk.
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/5] Add the "verbose" module option.
On 08/07/2014 10:52 AM, Jean Delvare wrote:
> Le Wednesday 06 August 2014 à 21:05 +0000, Goffredo Baroncelli a écrit :
>> Add a "verbose" option to control the message in the kernel log
>> verbose = 0 no message
>> verbose = 1 log only the fan speed changes
>> verbose = 2 log the fan speed changes and the temperature changes
>>
>> Signed-off-by: Goffredo Baroncelli
>> ---
>> drivers/macintosh/therm_windtunnel.c | 39
>> +++-
>> 1 file changed, 25 insertions(+), 14 deletions(-)
>
> Looks overall good, minor comments inline below.
>
>> diff --git a/drivers/macintosh/therm_windtunnel.c
>> b/drivers/macintosh/therm_windtunnel.c
>> index 1e50455..7c512db 100644
>> --- a/drivers/macintosh/therm_windtunnel.c
>> +++ b/drivers/macintosh/therm_windtunnel.c
>> @@ -44,7 +44,9 @@
>> #include
>> #include
>>
>> -#define LOG_TEMP0 /* continuously log
>> temperature */
>> +static int verbose = 1;
>> +module_param(verbose, int, 0644);
>> +MODULE_PARM_DESC(verbose, "Verbosity level: 0=silent, 1=log the fan tuning,
>> 2=log the temperature");
>>
>> static struct {
>> volatile intrunning;
>> @@ -157,10 +159,6 @@ tune_fan( int fan_setting )
>> /* write_reg( x.fan, 0x24, val, 1 ); */
>> write_reg( x.fan, 0x25, val, 1 );
>> write_reg( x.fan, 0x20, 0, 1 );
>> -print_temp("CPU-temp: ", x.temp );
>> -if( x.casetemp )
>> -print_temp(", Case: ", x.casetemp );
>> -printk(", Fan: %d (tuned %+d)\n", 11-fan_setting,
>> x.fan_level-fan_setting );
>>
>> x.fan_level = fan_setting;
>> }
>> @@ -168,7 +166,7 @@ tune_fan( int fan_setting )
>> static void
>> poll_temp( void )
>> {
>> -int temp, i, level, casetemp;
>> +int temp, i, level, casetemp, tempchanged;
>>
>> temp = read_reg( x.thermostat, 0, 2 );
>>
>> @@ -179,14 +177,6 @@ poll_temp( void )
>> casetemp = read_reg(x.fan, 0x0b, 1) << 8;
>> casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
>>
>> -if( LOG_TEMP && x.temp != temp ) {
>> -print_temp("CPU-temp: ", temp );
>> -print_temp(", Case: ", casetemp );
>> -printk(", Fan: %d\n", 11-x.fan_level );
>> -}
>> -x.temp = temp;
>> -x.casetemp = casetemp;
>> -
>> level = -1;
>> for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
>> ;
>> @@ -200,6 +190,27 @@ poll_temp( void )
>> level = fan_table[i].fan_up_setting;
>> x.upind = i;
>>
>> +/*
>> + * if verbose >0 log each fan tuning
>> + * if verbose >1 log each cpu temperature change
>
> Maybe this is just me but I think "verbose >= 1" and "verbose >= 2"
> would be easier to understand. Same in the code below.
correct, I am planning another revision due to Guenter concern. So
I will correct it.
>
>> + */
>> +tempchanged = x.temp != temp || x.casetemp != casetemp;
>> +if ((verbose > 1 && tempchanged) ||
>> +(verbose > 0 && level >= 0)) {
>> +printk(KERN_INFO);
>> +print_temp("CPU-temp: ", temp);
>
> This can be written more efficiently as a single statement:
>
> print_temp(KERN_INFO "CPU-temp: ", temp);
I suppose that KERN_* has to be in the beginning of the line.
Because a single line is composed by several prink, KERN_INFO has
to be only in the first printk. To me it seems more polite to have
one printk for the level, and the others (there are more than one)
for the message parts.
>
>> +if (casetemp)
>> +print_temp(", Case: ", casetemp);
>> +if (level >= 0)
>> +printk(", Fan: %d (tuned %+d)\n", 11-level,
>> +x.fan_level-level);
>> +else
>> +printk(", Fan: %d\n", 11-x.fan_level);
>> +}
>> +
>> +x.temp = temp;
>> +x.casetemp = casetemp;
>> +
>> if( level >= 0 )
>> tune_fan( level );
>> }
>
> Reviewed-by: Jean Delvare
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] Export the temperatures via hwmon
On 08/07/2014 01:18 AM, Guenter Roeck wrote:
> On Wed, Aug 06, 2014 at 09:05:03PM +0000, Goffredo Baroncelli wrote:
>> From: Goffredo Baroncelli
>>
>> Export the temperature via the hwmon subsystem.
>> See the list below for the sensors exported:
>>
>> $ cd /sys/devices/temperature/hwmon/hwmon0
>> $ echo "name: $(cat name)"; for i in temp*; do echo "$i: $(cat $i)"; done
>> name: therm_windtunnel
>> temp1_input: 59312
>> temp1_label: CPU
>> temp2_input: 36750
>> temp2_label: Case
>> temp3_input: 37750
>> temp3_label: Case2
>>
> Makes me wonder why you don't report the fan speed through hwmon.
See the first email. Basically when I start to read the fan speed,
it became irregular, so I stopped the test. I don't know
if it is a my faulted hardware, or an hw design problem.
The fan is a 2 wire fan.
>
>> The Case2 temperature is the sensor temperature inside the adm1030.
>>
> There are standard hwmon drivers for lm75, ds1775, and adm1030.
> Personally I think it would make more sense to use those directly.
> But I guess that would be for another day.
>
>> Signed-off-by: Goffredo Baroncelli
>> ---
>> drivers/macintosh/therm_windtunnel.c | 103
>> +--
>> 1 file changed, 99 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/macintosh/therm_windtunnel.c
>> b/drivers/macintosh/therm_windtunnel.c
>> index b6cba98..a6757d7 100644
>> --- a/drivers/macintosh/therm_windtunnel.c
>> +++ b/drivers/macintosh/therm_windtunnel.c
>> @@ -37,6 +37,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>>
>> #include
>> #include
>> @@ -58,9 +60,12 @@ static struct {
>> struct i2c_client *thermostat;
>> struct i2c_client *fan;
>>
>> +struct device *hwmon;
>> +
>> int overheat_temp; /* 100% fan at this
>> temp */
>> int overheat_hyst;
>> int temp;
>> +int casetemp2;
>> int casetemp;
>> int fan_level; /* active fan_table
>> setting */
>>
>> @@ -120,6 +125,75 @@ static DEVICE_ATTR(case_temperature, S_IRUGO,
>> show_case_temperature, NULL );
>> static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
>>
>>
>> +static ssize_t
>> +show_temp1(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +return sprintf(buf, "%d%03d\n", x.temp>>8, (x.temp & 0xff)*1000>>8);
>
> I personally prefer if code follows coding style, with spaces around operands,
> and if calculations are less cryptic.
>
>> +}
>> +
>> +static ssize_t
>> +show_temp1_label(struct device *dev, struct device_attribute *attr, char
>> *buf)
>> +{
>> +return sprintf(buf, "CPU\n");
>> +}
>> +
>> +static ssize_t
>> +show_temp2(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +return sprintf(buf, "%d%03d\n", x.casetemp>>8,
>> +(x.casetemp & 0xff)*1000>>8);
>> +}
>> +
>> +static ssize_t
>> +show_temp2_label(struct device *dev, struct device_attribute *attr, char
>> *buf)
>> +{
>> +return sprintf(buf, "Case\n");
>> +}
>> +
>> +
>> +static ssize_t
>> +show_temp3(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +return sprintf(buf, "%d%03d\n", x.casetemp2>>8,
>> +(x.casetemp2 & 0xff)*1000>>8);
>
> ... and aligned second lines.
>
>> +}
>> +
>> +static ssize_t
>> +show_temp3_label(struct device *dev, struct device_attribute *attr, char
>> *buf)
>> +{
>> +return sprintf(buf, "Case2\n");
>> +}
>> +
>> +#define TWT_ATTRIB(name, func) \
>> +static SENSOR_DEVICE_ATTR(name##_input, S_IRUGO, func, NULL, 0); \
>> +static SENSOR_DEVICE_ATTR(name##_label, S_IRUGO, func##_label, \
>> +NULL, 0)
>> +
>> +TWT_ATTRIB(temp1, show_temp1);
>> +TWT_ATTRIB(temp2, show_temp2);
>> +TWT_ATTRIB(temp3, show_temp3);
>> +
>> +
> A single empty line should be sufficient. Personally I am also not
> a fan of such macros (and neither is checkpatch), and prefer the use
> of direct macros as less confusing.
True
[PATCH 2/5] Remove attach_method because un-used
Remove attach_method because i2c-powermac is
in charge to instantiate the driver directly.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 35 ---
1 file changed, 35 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index a64a06f..1e50455 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -300,40 +300,6 @@ static int control_loop(void *dummy)
/* i2c probing and setup */
//
-static int
-do_attach( struct i2c_adapter *adapter )
-{
- /* scan 0x48-0x4f (DS1775) and 0x2c-2x2f (ADM1030) */
- static const unsigned short scan_ds1775[] = {
- 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f,
- I2C_CLIENT_END
- };
- static const unsigned short scan_adm1030[] = {
- 0x2c, 0x2d, 0x2e, 0x2f,
- I2C_CLIENT_END
- };
-
- if( strncmp(adapter->name, "uni-n", 5) )
- return 0;
-
- if( !x.running ) {
- struct i2c_board_info info;
-
- memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "therm_ds1775", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_ds1775, NULL);
-
- strlcpy(info.type, "therm_adm1030", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_adm1030, NULL);
-
- if( x.thermostat && x.fan ) {
- x.running = 1;
- x.poll_task = kthread_run(control_loop, NULL, "g4fand");
- }
- }
- return 0;
-}
-
static void
try_start_control_loop(void)
{
@@ -450,7 +416,6 @@ static struct i2c_driver g4fan_driver = {
.driver = {
.name = "therm_windtunnel",
},
- .attach_adapter = do_attach,
.probe = do_probe,
.remove = do_remove,
.id_table = therm_windtunnel_id,
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH][v3] therm_windtunnel doesn't work properly on PowerMac G4
Hi All, 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 5 patches to solve this problem (tested on my PowerMac G4). Only the first two are strictly related to the problem, the others three may be skipped. 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 a parameter to the therm_windtunnel module to control the kernel log message 4) export the fan speed via sysfs 5) export the temperature via the hwmon subsistem 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. The patch 4) is copyied from the Bryan Christianson's patch (see debian bug #741663) The patch 5) export the temperatures via hwmon, a more standard interface. I also added the internal sensor of the adm1030, which I called "Case2", because it measure the same temperature /+/- 1C) of the "Case" sensor. I have to highlight that I tried to export also the fan speed, but I was unable to get it, without affecting the fan speed: when I set the bit 2 (TACH 1 En) of the register 0x1 (Configuration 2), it seemed that the speed every 4/5s dropped, then it raised quickly I didn't perform other test to avoid damages. Could you be so kindly to apply these patches ? PS: I am not LKML subscriber, so please put me in CC in case of reply. BR G.Baroncelli Changelog: v0: 2014/07/30 - first issue v1: 2014/08/01 - protect with a mutex the check before starting the fan daemon (to protect from parallel drivers instantation) - reduce the number of module parameters to 1 as suggested by Jean Delvare - export the fan speed via sysfs v2: 2014/08/06 - export the temperatures via hwmon - export the internal temperature sensor of the adm1030 - little cleanup due to the suggestion of checkpatch.pl ( and Jean Delvare) - removed the "(tune +0)" in the log. [1] I think that the guilty commit is commit 81e5d8646ff6bf323dddcf172aa3cef84468fa12 Author: Benjamin Herrenschmidt Date: Wed Apr 18 22:16:42 2012 + 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 [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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/5] Add the "verbose" module option.
Add a "verbose" option to control the message in the kernel log
verbose = 0 no message
verbose = 1 log only the fan speed changes
verbose = 2 log the fan speed changes and the temperature changes
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 39 +++-
1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 1e50455..7c512db 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -44,7 +44,9 @@
#include
#include
-#define LOG_TEMP 0 /* continuously log
temperature */
+static int verbose = 1;
+module_param(verbose, int, 0644);
+MODULE_PARM_DESC(verbose, "Verbosity level: 0=silent, 1=log the fan tuning,
2=log the temperature");
static struct {
volatile intrunning;
@@ -157,10 +159,6 @@ tune_fan( int fan_setting )
/* write_reg( x.fan, 0x24, val, 1 ); */
write_reg( x.fan, 0x25, val, 1 );
write_reg( x.fan, 0x20, 0, 1 );
- print_temp("CPU-temp: ", x.temp );
- if( x.casetemp )
- print_temp(", Case: ", x.casetemp );
- printk(", Fan: %d (tuned %+d)\n", 11-fan_setting,
x.fan_level-fan_setting );
x.fan_level = fan_setting;
}
@@ -168,7 +166,7 @@ tune_fan( int fan_setting )
static void
poll_temp( void )
{
- int temp, i, level, casetemp;
+ int temp, i, level, casetemp, tempchanged;
temp = read_reg( x.thermostat, 0, 2 );
@@ -179,14 +177,6 @@ poll_temp( void )
casetemp = read_reg(x.fan, 0x0b, 1) << 8;
casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
- if( LOG_TEMP && x.temp != temp ) {
- print_temp("CPU-temp: ", temp );
- print_temp(", Case: ", casetemp );
- printk(", Fan: %d\n", 11-x.fan_level );
- }
- x.temp = temp;
- x.casetemp = casetemp;
-
level = -1;
for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
;
@@ -200,6 +190,27 @@ poll_temp( void )
level = fan_table[i].fan_up_setting;
x.upind = i;
+ /*
+* if verbose >0 log each fan tuning
+* if verbose >1 log each cpu temperature change
+*/
+ tempchanged = x.temp != temp || x.casetemp != casetemp;
+ if ((verbose > 1 && tempchanged) ||
+ (verbose > 0 && level >= 0)) {
+ printk(KERN_INFO);
+ print_temp("CPU-temp: ", temp);
+ if (casetemp)
+ print_temp(", Case: ", casetemp);
+ if (level >= 0)
+ printk(", Fan: %d (tuned %+d)\n", 11-level,
+ x.fan_level-level);
+ else
+ printk(", Fan: %d\n", 11-x.fan_level);
+ }
+
+ x.temp = temp;
+ x.casetemp = casetemp;
+
if( level >= 0 )
tune_fan( level );
}
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] Update drivers names to the ones invoked by i2c-powermac
Update drivers names to the ones invoked by i2c-powermac:
- therm_ds1775 -> MAC,ds1775
- therm_adm1030 -> MAC,adm1030
The background fan control loop is started from
the devices probing methods.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 3b4a157..a64a06f 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -334,6 +334,23 @@ do_attach( struct i2c_adapter *adapter )
return 0;
}
+static void
+try_start_control_loop(void)
+{
+
+ mutex_lock(&x.lock);
+ if (!x.thermostat || !x.fan || x.running) {
+ mutex_unlock(&x.lock);
+ return;
+ }
+
+ x.running = 1;
+ mutex_unlock(&x.lock);
+
+ x.poll_task = kthread_run(control_loop, NULL, "g4fand");
+
+}
+
static int
do_remove(struct i2c_client *client)
{
@@ -364,6 +381,7 @@ attach_fan( struct i2c_client *cl )
printk("ADM1030 fan controller [@%02x]\n", cl->addr );
x.fan = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -397,6 +415,7 @@ attach_thermostat( struct i2c_client *cl )
x.overheat_temp = os_temp;
x.overheat_hyst = hyst_temp;
x.thermostat = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -404,8 +423,8 @@ out:
enum chip { ds1775, adm1030 };
static const struct i2c_device_id therm_windtunnel_id[] = {
- { "therm_ds1775", ds1775 },
- { "therm_adm1030", adm1030 },
+ { "MAC,ds1775", ds1775 },
+ { "MAC,adm1030", adm1030 },
{ }
};
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] Export the temperatures via hwmon
From: Goffredo Baroncelli
Export the temperature via the hwmon subsystem.
See the list below for the sensors exported:
$ cd /sys/devices/temperature/hwmon/hwmon0
$ echo "name: $(cat name)"; for i in temp*; do echo "$i: $(cat $i)"; done
name: therm_windtunnel
temp1_input: 59312
temp1_label: CPU
temp2_input: 36750
temp2_label: Case
temp3_input: 37750
temp3_label: Case2
The Case2 temperature is the sensor temperature inside the adm1030.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 103 +--
1 file changed, 99 insertions(+), 4 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index b6cba98..a6757d7 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -37,6 +37,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -58,9 +60,12 @@ static struct {
struct i2c_client *thermostat;
struct i2c_client *fan;
+ struct device *hwmon;
+
int overheat_temp; /* 100% fan at this
temp */
int overheat_hyst;
int temp;
+ int casetemp2;
int casetemp;
int fan_level; /* active fan_table
setting */
@@ -120,6 +125,75 @@ static DEVICE_ATTR(case_temperature, S_IRUGO,
show_case_temperature, NULL );
static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
+static ssize_t
+show_temp1(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.temp>>8, (x.temp & 0xff)*1000>>8);
+}
+
+static ssize_t
+show_temp1_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "CPU\n");
+}
+
+static ssize_t
+show_temp2(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.casetemp>>8,
+ (x.casetemp & 0xff)*1000>>8);
+}
+
+static ssize_t
+show_temp2_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "Case\n");
+}
+
+
+static ssize_t
+show_temp3(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d%03d\n", x.casetemp2>>8,
+ (x.casetemp2 & 0xff)*1000>>8);
+}
+
+static ssize_t
+show_temp3_label(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "Case2\n");
+}
+
+#define TWT_ATTRIB(name, func) \
+ static SENSOR_DEVICE_ATTR(name##_input, S_IRUGO, func, NULL, 0); \
+ static SENSOR_DEVICE_ATTR(name##_label, S_IRUGO, func##_label, \
+ NULL, 0)
+
+TWT_ATTRIB(temp1, show_temp1);
+TWT_ATTRIB(temp2, show_temp2);
+TWT_ATTRIB(temp3, show_temp3);
+
+
+static struct attribute *therm_windtunnel_attributes[] = {
+ &sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_label.dev_attr.attr,
+ &sensor_dev_attr_temp2_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_label.dev_attr.attr,
+ &sensor_dev_attr_temp3_input.dev_attr.attr,
+ &sensor_dev_attr_temp3_label.dev_attr.attr,
+
+ NULL,
+};
+
+static const struct attribute_group therm_windtunnel_attr_group = {
+ .attrs = therm_windtunnel_attributes,
+};
+
+static const struct attribute_group *therm_windtunnel_attr_groups[] = {
+ &therm_windtunnel_attr_group,
+ NULL,
+};
+
//
/* controller thread */
//
@@ -172,16 +246,23 @@ tune_fan( int fan_setting )
static void
poll_temp( void )
{
- int temp, i, level, casetemp, tempchanged;
+ int temp, i, level, casetemp, tempchanged, casetemp2, reg06;
+ /* temperature read from ds1775 */
temp = read_reg( x.thermostat, 0, 2 );
/* this actually occurs when the computer is loaded */
if( temp < 0 )
return;
- casetemp = read_reg(x.fan, 0x0b, 1) << 8;
- casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
+ /*
+* temperatures read from the adm1030
+* casetemp is the external temperature sensor
+* casetemp2 is the internal temperature sensor
+*/
+ reg06 = read_reg(x.fan, 0x06, 1);
+ casetemp = (read_reg(x.fan, 0x0b, 1) << 8) | (reg06 & 0x07 << 5);
+ casetemp2 = (read_reg(x.fan, 0x0a, 1) << 8) | (reg06 & 0xc0);
level = -1;
for( i=0; (temp & 0x) > fan_tabl
[PATCH 4/5] Return the fan speed via sysfs
Return the fan speed via sysfs:
/sys/devices/temperature/fan_level
This patch is copied from the Bryan Christianson's patch (see
debian bug #741663)
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 7c512db..b6cba98 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -109,9 +109,15 @@ show_case_temperature( struct device *dev, struct
device_attribute *attr, char *
return sprintf(buf, "%d.%d\n", x.casetemp>>8, (x.casetemp & 255)*10/256
);
}
+static ssize_t
+show_fan_level(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", 11 - x.fan_level);
+}
+
static DEVICE_ATTR(cpu_temperature, S_IRUGO, show_cpu_temperature, NULL );
static DEVICE_ATTR(case_temperature, S_IRUGO, show_case_temperature, NULL );
-
+static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL);
//
@@ -265,6 +271,7 @@ setup_hardware( void )
err = device_create_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
err |= device_create_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ err |= device_create_file(&x.of_dev->dev, &dev_attr_fan_level);
if (err)
printk(KERN_WARNING
"Failed to create temperature attribute file(s).\n");
@@ -275,6 +282,7 @@ restore_regs( void )
{
device_remove_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
device_remove_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ device_remove_file(&x.of_dev->dev, &dev_attr_fan_level);
write_reg( x.fan, 0x01, x.r1, 1 );
write_reg( x.fan, 0x20, x.r20, 1 );
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] Add the "verbose" module option.
On 08/04/2014 10:46 AM, Jean Delvare wrote:
> Le Sunday 03 August 2014 à 18:36 +0200, Goffredo Baroncelli a écrit :
>> On 08/03/2014 05:52 PM, Jean Delvare wrote:
>>> On Sun, 03 Aug 2014 17:12:57 +0200, Goffredo Baroncelli wrote:
>>>> On 08/03/2014 04:12 PM, Jean Delvare wrote:
>>>>>> +(verbose > 0 && level >= 0)) {
>>>>>> +print_temp("CPU-temp: ", temp );
>>>>>> +if (casetemp)
>>>>>> +print_temp(", Case: ", casetemp );
>>>>>> +if (level >= 0)
>>>>>> +printk(", Fan: %d (tuned %+d)\n", 11-level,
>>>>>> +x.fan_level-level );
>>>>>> +else
>>>>>> +printk(", Fan: %d (tuned +0)\n",x.fan_level);
>>>>>
>>>>> I think you can do without the "tuned +0" which doesn't add much value.
>>>>
>>>> Me too. But the old driver does the same, so I preferred to
>>>> leave it as is.
>>>
>>> I looked at the code again and no, I can't see the old code doing that.
>>> It has "tuned %+d" only in tune_fan() which is only called if
>>> level >= 0. The other printk (when tune_fan isn't called) doesn't have
>>> a "tuned" part.
>>
>> This is taken from an old log of a v3.2 kernel (no changes here):
>>
>> [ 886.510879] CPU-temp: 55.4 C, Case: 33.1 C, Fan: 0 (tuned -11)
>> [ 910.522869] CPU-temp: 56.0 C, Case: 33.5 C, Fan: 0 (tuned +0)
>> [ 958.546880] CPU-temp: 57.0 C, Case: 34.1 C, Fan: 3 (tuned +3)
>>
>> in the code if level <0, then there is no update in the log. But if
>> level >0 and level is equal to the previous one, this leads to
>> have "tuned +0"...
>
> I agree with that.
>
>> But I have to be honest: I have not fully understand how
>> "level" is computed.
>
> I agree with that too :/
>
>> The printk without "(tuned %+d)" is never called because
>> LOG_TEMP was #define(d) equal to 0.
>
> And this is what your second printk is replacing. So it should not have
> the "(tuned *)" either.
>
I removed the printk(s) from tune_fan(); the ones leaved replaced
both the ones inside tune_fan() and the ones outside.
Anyway, Benjamin which is your opinion ?
For me is equal to remove or to leave "(tune +0)" (when the tuning is equal to
0).
Jean think it is better to remove "(tune +0)" (when the tuning is equal to 0).
So if you haven't any objection I will remove it.
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] Return the fan speed via sysfs
On 08/03/2014 05:59 PM, Jean Delvare wrote: > On Sun, 03 Aug 2014 17:27:17 +0200, Goffredo Baroncelli wrote: >> On 08/03/2014 04:17 PM, Jean Delvare wrote: >>> On Fri, 1 Aug 2014 14:00:50 +, Goffredo Baroncelli wrote: >>>> Return the fan speed via sysfs: >>>> /sys/devices/temperature/fan_level >>> >>> Good idea. Even better would be if the driver would expose a standard >>> hwmon interface for the temperature values. Fan level could go in >>> attribute "pwm1" after proper scaling. >> >> I tought the same. But until now the value logged is an integer value >> between 1 and 11. So I preferred to leave it as is. >> >> The thing that I can do is to *add* a further attribute called pwm1. >> ( I have to check how adm1031.c computes its pwm), because is a >> more standard interface. > > The temperature attributes in hwmon would need different names and > units too (temp1_input and temp2_input, in millidegree C.) The > advantage is that all monitoring applications out there would pick up > these values automatically. Are you suggesting to add also a "temp1_input" attribute ? > >> I even thought to allow to change the fan speed from user space > > Ben will never let you do that ;-) What would be the risk ?. When the CPU temperature goes behind the limit, then the computer is switched off by an hardware protection (I am sure because I had to changed a cpu board because a drift of the temperature sensor). I am not suggesting to allow to change the fan speed, I am only asking which would be the risk. > >>>> @@ -265,6 +271,7 @@ setup_hardware( void ) >>>> >>>>err = device_create_file( &x.of_dev->dev, &dev_attr_cpu_temperature ); >>>>err |= device_create_file( &x.of_dev->dev, &dev_attr_case_temperature ); >>>> + err |= device_create_file( &x.of_dev->dev, &dev_attr_fan_level ); >>>>if (err) >>>>printk(KERN_WARNING >>>>"Failed to create temperature attribute file(s).\n"); >>> >>> That's not your fault but please note that this construct is broken. >>> You can't "or" error codes together, the result if two or more calls >>> fail with different error codes is pretty random. Instead, each error >>> must be tested individually. I know checkpatch.pl will complain if you >>> do that but you can ignore it as is it the right thing to do in that >>> case. >> >> The really question is what we should do if the 2nd device_create_file() >> would fail: we should fails the driver initialization ? or we should >> continue, because even if there aren't some sysfs attributes the driver >> work enough ? > > I would log a warning and continue because it's a secondary feature of > the driver. Exactly as the driver already does - no change here. > > In practice it's never going to happen so it doesn't really matter. I > just wanted to point out that the construct used in the driver was > dangerous. In this specific case it's harmless because the value of > "err" is never used (other than the fact that it's non-zero.) But if > the error code was included in the log message for example (which is > recommended), it would possibly make no sense. > > Feel free to ignore this problem in this patch, it's not so important. > -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] Add the "verbose" module option.
On 08/03/2014 05:52 PM, Jean Delvare wrote:
> On Sun, 03 Aug 2014 17:12:57 +0200, Goffredo Baroncelli wrote:
>> On 08/03/2014 04:12 PM, Jean Delvare wrote:
>>>> + (verbose > 0 && level >= 0)) {
>>>> + print_temp("CPU-temp: ", temp );
>>>> + if (casetemp)
>>>> + print_temp(", Case: ", casetemp );
>>>> + if (level >= 0)
>>>> + printk(", Fan: %d (tuned %+d)\n", 11-level,
>>>> + x.fan_level-level );
>>>> + else
>>>> + printk(", Fan: %d (tuned +0)\n",x.fan_level);
>>>
>>> I think you can do without the "tuned +0" which doesn't add much value.
>>
>> Me too. But the old driver does the same, so I preferred to
>> leave it as is.
>
> I looked at the code again and no, I can't see the old code doing that.
> It has "tuned %+d" only in tune_fan() which is only called if
> level >= 0. The other printk (when tune_fan isn't called) doesn't have
> a "tuned" part.
>
This is taken from an old log of a v3.2 kernel (no changes here):
[ 886.510879] CPU-temp: 55.4 C, Case: 33.1 C, Fan: 0 (tuned -11)
[ 910.522869] CPU-temp: 56.0 C, Case: 33.5 C, Fan: 0 (tuned +0)
[ 958.546880] CPU-temp: 57.0 C, Case: 34.1 C, Fan: 3 (tuned +3)
in the code if level <0, then there is no update in the log. But if
level >0 and level is equal to the previous one, this leads to
have "tuned +0"...
But I have to be honest: I have not fully understand how
"level" is computed.
The printk without "(tuned %+d)" is never called because
LOG_TEMP was #define(d) equal to 0.
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] Return the fan speed via sysfs
On 08/03/2014 04:17 PM, Jean Delvare wrote:
> On Fri, 1 Aug 2014 14:00:50 +0000, Goffredo Baroncelli wrote:
>> Return the fan speed via sysfs:
>> /sys/devices/temperature/fan_level
>
> Good idea. Even better would be if the driver would expose a standard
> hwmon interface for the temperature values. Fan level could go in
> attribute "pwm1" after proper scaling.
I tought the same. But until now the value logged is an integer value
between 1 and 11. So I preferred to leave it as is.
The thing that I can do is to *add* a further attribute called pwm1.
( I have to check how adm1031.c computes its pwm), because is a
more standard interface.
I even thought to allow to change the fan speed from user space
>
> Please follow scripts/checkpatch.pl's advice to fix the coding style.
>
>> This patch is copied from the Bryan Christianson's patch (see
>> debian bug #741663)
>>
>> Signed-off-by: Goffredo Baroncelli
>> ---
>> drivers/macintosh/therm_windtunnel.c | 10 +-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/macintosh/therm_windtunnel.c
>> b/drivers/macintosh/therm_windtunnel.c
>> index 0c4eb85..a2af7db 100644
>> --- a/drivers/macintosh/therm_windtunnel.c
>> +++ b/drivers/macintosh/therm_windtunnel.c
>> @@ -111,9 +111,15 @@ show_case_temperature( struct device *dev, struct
>> device_attribute *attr, char *
>> return sprintf(buf, "%d.%d\n", x.casetemp>>8, (x.casetemp & 255)*10/256
>> );
>> }
>>
>> +static ssize_t
>> +show_fan_level( struct device *dev, struct device_attribute *attr, char
>> *buf )
>> +{
>> +return sprintf(buf, "%d\n", 11 - x.fan_level );
>> +}
>> +
>> static DEVICE_ATTR(cpu_temperature, S_IRUGO, show_cpu_temperature, NULL );
>> static DEVICE_ATTR(case_temperature, S_IRUGO, show_case_temperature, NULL );
>> -
>> +static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL );
>>
>>
>> //
>> @@ -265,6 +271,7 @@ setup_hardware( void )
>>
>> err = device_create_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
>> err |= device_create_file( &x.of_dev->dev, &dev_attr_case_temperature );
>> +err |= device_create_file( &x.of_dev->dev, &dev_attr_fan_level );
>> if (err)
>> printk(KERN_WARNING
>> "Failed to create temperature attribute file(s).\n");
>
> That's not your fault but please note that this construct is broken.
> You can't "or" error codes together, the result if two or more calls
> fail with different error codes is pretty random. Instead, each error
> must be tested individually. I know checkpatch.pl will complain if you
> do that but you can ignore it as is it the right thing to do in that
> case.
The really question is what we should do if the 2nd device_create_file()
would fail: we should fails the driver initialization ? or we should
continue, because even if there aren't some sysfs attributes the driver
work enough ?
>
>> @@ -275,6 +282,7 @@ restore_regs( void )
>> {
>> device_remove_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
>> device_remove_file( &x.of_dev->dev, &dev_attr_case_temperature );
>> +device_remove_file( &x.of_dev->dev, &dev_attr_fan_level );
>>
>> write_reg( x.fan, 0x01, x.r1, 1 );
>> write_reg( x.fan, 0x20, x.r20, 1 );
>
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] Add the "verbose" module option.
On 08/03/2014 04:12 PM, Jean Delvare wrote:
> Hi Goffredo,
>
> You messed up your Cc's ;-)
I fight hard with git-send-email In the next trip I will check two times !
>
> On Fri, 1 Aug 2014 14:00:49 +, Goffredo Baroncelli wrote:
>> The "verbose" option controls the message in the kernel log
>> verbose = 0 no message
>> verbose = 1 log only the fan speed changes
>> verbose = 2 log the fan speed changes and the temperature changes
>>
>> Signed-off-by: Goffredo Baroncelli
>> ---
>> drivers/macintosh/therm_windtunnel.c | 37
>> +++-
>> 1 file changed, 24 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/macintosh/therm_windtunnel.c
>> b/drivers/macintosh/therm_windtunnel.c
>> index 1e50455..0c4eb85 100644
>> --- a/drivers/macintosh/therm_windtunnel.c
>> +++ b/drivers/macintosh/therm_windtunnel.c
>> @@ -44,7 +44,11 @@
>> #include
>> #include
>>
>> -#define LOG_TEMP0 /* continuously log
>> temperature */
>> +static int verbose = 1; /* see description below */
>
> This comment seems useless.
ok
>
>> +module_param(verbose, int, 0644);
>> +MODULE_PARM_DESC(verbose, "Vebosity level: 0=silent, "
>
> Typo: Verbosity
Ok
>
>> +"1=log the fan tuning, "
>> +"2=log the temperature.");
>
> Trailing dot is not needed.
OK
>>
>> static struct {
[... cut cut cut ...]
>> +/*
>> + * if verbose >0 log each fan tuning
>> + * if verbose >1 log each cpu temperature change
>> + */
>
> I think it is a good idea to have all the printing in a single place.
>
>> +if ((verbose > 1 && x.temp != temp ) ||
>
> No space before closing parenthesis. I know the original code did not
> follow the standard coding style but you have the opportunity to make
> it better. Same many times below. scripts/checkpatch.pl tells you about
> that and many other style issues, most of which are easily fixable.
>
> Also don't you want to log changes of case temperature too?
yes it make sense.
>
>> +(verbose > 0 && level >= 0)) {
>> +print_temp("CPU-temp: ", temp );
>> +if (casetemp)
>> +print_temp(", Case: ", casetemp );
>> +if (level >= 0)
>> +printk(", Fan: %d (tuned %+d)\n", 11-level,
>> +x.fan_level-level );
>> +else
>> +printk(", Fan: %d (tuned +0)\n",x.fan_level);
>
> I think you can do without the "tuned +0" which doesn't add much value.
Me too. But the old driver does the same, so I preferred to
leave it as is.
>
>> +}
>> +
>> +x.temp = temp;
>> +x.casetemp = casetemp;
>> +
>> if( level >= 0 )
>> tune_fan( level );
>> }
>
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] Add the "verbose" module option.
The "verbose" option controls the message in the kernel log
verbose = 0 no message
verbose = 1 log only the fan speed changes
verbose = 2 log the fan speed changes and the temperature changes
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 37 +++-
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 1e50455..0c4eb85 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -44,7 +44,11 @@
#include
#include
-#define LOG_TEMP 0 /* continuously log
temperature */
+static int verbose = 1; /* see description below */
+module_param(verbose, int, 0644);
+MODULE_PARM_DESC(verbose, "Vebosity level: 0=silent, "
+ "1=log the fan tuning, "
+ "2=log the temperature.");
static struct {
volatile intrunning;
@@ -157,10 +161,6 @@ tune_fan( int fan_setting )
/* write_reg( x.fan, 0x24, val, 1 ); */
write_reg( x.fan, 0x25, val, 1 );
write_reg( x.fan, 0x20, 0, 1 );
- print_temp("CPU-temp: ", x.temp );
- if( x.casetemp )
- print_temp(", Case: ", x.casetemp );
- printk(", Fan: %d (tuned %+d)\n", 11-fan_setting,
x.fan_level-fan_setting );
x.fan_level = fan_setting;
}
@@ -179,14 +179,6 @@ poll_temp( void )
casetemp = read_reg(x.fan, 0x0b, 1) << 8;
casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
- if( LOG_TEMP && x.temp != temp ) {
- print_temp("CPU-temp: ", temp );
- print_temp(", Case: ", casetemp );
- printk(", Fan: %d\n", 11-x.fan_level );
- }
- x.temp = temp;
- x.casetemp = casetemp;
-
level = -1;
for( i=0; (temp & 0x) > fan_table[i].temp ; i++ )
;
@@ -200,6 +192,25 @@ poll_temp( void )
level = fan_table[i].fan_up_setting;
x.upind = i;
+ /*
+* if verbose >0 log each fan tuning
+* if verbose >1 log each cpu temperature change
+*/
+ if ((verbose > 1 && x.temp != temp ) ||
+ (verbose > 0 && level >= 0)) {
+ print_temp("CPU-temp: ", temp );
+ if (casetemp)
+ print_temp(", Case: ", casetemp );
+ if (level >= 0)
+ printk(", Fan: %d (tuned %+d)\n", 11-level,
+ x.fan_level-level );
+ else
+ printk(", Fan: %d (tuned +0)\n",x.fan_level);
+ }
+
+ x.temp = temp;
+ x.casetemp = casetemp;
+
if( level >= 0 )
tune_fan( level );
}
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH][v2] therm_windtunnel doesn't work properly on PowerMac G4
Hi All, 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 4 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 a parameter to the therm_windtunnel module to control the kernel log message 4) export the fan speed via sysfs 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. The patch 4) is copyied from the Bryan Christianson's patch (see debian bug #741663) Could you be so kindly to apply these patches ? PS: I am not LKML subscriber, so please put me in CC in case of reply. BR G.Baroncelli Changelog: v0: 2014/07/30 - first issue v1: 2014/08/01 - protect with a mutex the check before starting the fan daemon (to protect from parallel drivers instantation) - reduce the number of module parameters to 1 as suggested by Jean Delvare - export the fan speed via sysfs [1] I think that the guilty commit is commit 81e5d8646ff6bf323dddcf172aa3cef84468fa12 Author: Benjamin Herrenschmidt Date: Wed Apr 18 22:16:42 2012 + 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 [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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4] Remove attach_method because un-used
Remove attach_method because i2c-powermac is
in charge to instantiate the driver directly.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 35 ---
1 file changed, 35 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index a64a06f..1e50455 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -300,40 +300,6 @@ static int control_loop(void *dummy)
/* i2c probing and setup */
//
-static int
-do_attach( struct i2c_adapter *adapter )
-{
- /* scan 0x48-0x4f (DS1775) and 0x2c-2x2f (ADM1030) */
- static const unsigned short scan_ds1775[] = {
- 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f,
- I2C_CLIENT_END
- };
- static const unsigned short scan_adm1030[] = {
- 0x2c, 0x2d, 0x2e, 0x2f,
- I2C_CLIENT_END
- };
-
- if( strncmp(adapter->name, "uni-n", 5) )
- return 0;
-
- if( !x.running ) {
- struct i2c_board_info info;
-
- memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "therm_ds1775", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_ds1775, NULL);
-
- strlcpy(info.type, "therm_adm1030", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_adm1030, NULL);
-
- if( x.thermostat && x.fan ) {
- x.running = 1;
- x.poll_task = kthread_run(control_loop, NULL, "g4fand");
- }
- }
- return 0;
-}
-
static void
try_start_control_loop(void)
{
@@ -450,7 +416,6 @@ static struct i2c_driver g4fan_driver = {
.driver = {
.name = "therm_windtunnel",
},
- .attach_adapter = do_attach,
.probe = do_probe,
.remove = do_remove,
.id_table = therm_windtunnel_id,
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/4] Return the fan speed via sysfs
Return the fan speed via sysfs:
/sys/devices/temperature/fan_level
This patch is copied from the Bryan Christianson's patch (see
debian bug #741663)
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 0c4eb85..a2af7db 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -111,9 +111,15 @@ show_case_temperature( struct device *dev, struct
device_attribute *attr, char *
return sprintf(buf, "%d.%d\n", x.casetemp>>8, (x.casetemp & 255)*10/256
);
}
+static ssize_t
+show_fan_level( struct device *dev, struct device_attribute *attr, char *buf )
+{
+ return sprintf(buf, "%d\n", 11 - x.fan_level );
+}
+
static DEVICE_ATTR(cpu_temperature, S_IRUGO, show_cpu_temperature, NULL );
static DEVICE_ATTR(case_temperature, S_IRUGO, show_case_temperature, NULL );
-
+static DEVICE_ATTR(fan_level, S_IRUGO, show_fan_level, NULL );
//
@@ -265,6 +271,7 @@ setup_hardware( void )
err = device_create_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
err |= device_create_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ err |= device_create_file( &x.of_dev->dev, &dev_attr_fan_level );
if (err)
printk(KERN_WARNING
"Failed to create temperature attribute file(s).\n");
@@ -275,6 +282,7 @@ restore_regs( void )
{
device_remove_file( &x.of_dev->dev, &dev_attr_cpu_temperature );
device_remove_file( &x.of_dev->dev, &dev_attr_case_temperature );
+ device_remove_file( &x.of_dev->dev, &dev_attr_fan_level );
write_reg( x.fan, 0x01, x.r1, 1 );
write_reg( x.fan, 0x20, x.r20, 1 );
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] Update drivers names to the ones invoked by i2c-powermac
Update drivers names to the ones invoked by i2c-powermac:
- therm_ds1775 -> MAC,ds1775
- therm_adm1030 -> MAC,adm1030
The background fan control loop is started from
the devices probing methods.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 3b4a157..a64a06f 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -334,6 +334,23 @@ do_attach( struct i2c_adapter *adapter )
return 0;
}
+static void
+try_start_control_loop(void)
+{
+
+ mutex_lock(&x.lock);
+ if (!x.thermostat || !x.fan || x.running) {
+ mutex_unlock(&x.lock);
+ return;
+ }
+
+ x.running = 1;
+ mutex_unlock(&x.lock);
+
+ x.poll_task = kthread_run(control_loop, NULL, "g4fand");
+
+}
+
static int
do_remove(struct i2c_client *client)
{
@@ -364,6 +381,7 @@ attach_fan( struct i2c_client *cl )
printk("ADM1030 fan controller [@%02x]\n", cl->addr );
x.fan = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -397,6 +415,7 @@ attach_thermostat( struct i2c_client *cl )
x.overheat_temp = os_temp;
x.overheat_hyst = hyst_temp;
x.thermostat = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -404,8 +423,8 @@ out:
enum chip { ds1775, adm1030 };
static const struct i2c_device_id therm_windtunnel_id[] = {
- { "therm_ds1775", ds1775 },
- { "therm_adm1030", adm1030 },
+ { "MAC,ds1775", ds1775 },
+ { "MAC,adm1030", adm1030 },
{ }
};
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] therm_windtunnel doesn't work properly on PowerMac G4
On 07/31/2014 09:07 AM, Jean Delvare wrote:
> Hi Goffredo,
>
> For next time: please give each individual patch an appropriate
> subject. Otherwise it is difficult to keep track of what each patch
> does exactly.
I had to use the same subject because the email weren't threaded.
The subject was the only way to group the patches.
The next time I will use git send-email (which I hate !), so this
problem will disappear.
>
> On Wed, 30 Jul 2014 22:50:57 +0200, Goffredo Baroncelli wrote:
>> Add the "log_temp" and "verbose" module parameters.
>
> I think this is a good idea, much better than build-time settings.
>> log_tempenable/disable the temperature logging
>> verbose enable/disable the fan tune logging
>
> These names are not very consistent, both printks are logging both the
> temperatures and the fan speed.
>
> Also I'm unsure if you really need 2 parameters for this. I see these
> more as two degrees of verbosity of the same logging feature. I would
> like to suggest having a single module parameter, with 3 different
> values:
> 0: log nothing
> 1: log fan tuning (default)
> 2: log fan tuning + temperature continuously
>
> What do you think?
I definitely agree. I will work on that in the next few days
>
>>
>> Signed-off-by: Goffredo Baroncelli
>>
>> ---
>> drivers/macintosh/therm_windtunnel.c | 19 +--
>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/macintosh/therm_windtunnel.c
>> b/drivers/macintosh/therm_windtunnel.c
>> index fbe4516..7efba5d 100644
>> --- a/drivers/macintosh/therm_windtunnel.c
>> +++ b/drivers/macintosh/therm_windtunnel.c
>> @@ -44,7 +44,13 @@
>> #include
>> #include
>>
>> -#define LOG_TEMP0 /* continuously log
>> temperature */
>> +static bool log_temp = 0;
>> +module_param(log_temp, bool, 0644);
>> +MODULE_PARM_DESC(log_temp, "Enable the temperature logging");
>> +
>> +static bool verbose = 1;
>> +module_param(verbose, bool, 0644);
>> +MODULE_PARM_DESC(verbose, "Enable the fan speed logging");
>>
>> static struct {
>> volatile intrunning;
>> @@ -157,11 +163,12 @@ tune_fan( int fan_setting )
>> /* write_reg( x.fan, 0x24, val, 1 ); */
>> write_reg( x.fan, 0x25, val, 1 );
>> write_reg( x.fan, 0x20, 0, 1 );
>> -print_temp("CPU-temp: ", x.temp );
>> -if( x.casetemp )
>> +if (verbose) {
>> +print_temp("CPU-temp: ", x.temp );
>> print_temp(", Case: ", x.casetemp );
>
> In the original code, there was a condition for printing the case
> temperature, which you dropped. Is this on purpose? If so it should be
> explained in the patch description.
>
> BTW I see that the continuous temperature logging below does not have
> such a condition. For consistency I think it should be either always or
> never included.
I removed the "if" because the same printk() happens before (the tune_fan()
function is called after the 2nd printk() ).
So remove the "if" is safe.
I agree that a better explanation in the comments helps. Because I have to
update the patch for the log_temp/verbose module parameters, I will enhance
the patch description.
>
>> -printk(", Fan: %d (tuned %+d)\n", 11-fan_setting,
>> x.fan_level-fan_setting );
>> -
>> +printk(", Fan: %d (tuned %+d)\n",
>> +11-fan_setting, x.fan_level-fan_setting );
>> +}
>> x.fan_level = fan_setting;
>> }
>>
>> @@ -179,7 +186,7 @@ poll_temp( void )
>> casetemp = read_reg(x.fan, 0x0b, 1) << 8;
>> casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
>>
>> -if( LOG_TEMP && x.temp != temp ) {
>> +if( log_temp && x.temp != temp ) {
>> print_temp("CPU-temp: ", temp );
>> print_temp(", Case: ", casetemp );
>> printk(", Fan: %d\n", 11-x.fan_level );
>
> Thanks,
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] therm_windtunnel doesn't work properly on PowerMac G4
Remove attach_method because un-used: now i2c-powermac is in charge to
instantiate the driver.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 35 --
1 file changed, 35 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 9583413..fbe4516 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -300,40 +300,6 @@ static int control_loop(void *dummy)
/* i2c probing and setup */
//
-static int
-do_attach( struct i2c_adapter *adapter )
-{
- /* scan 0x48-0x4f (DS1775) and 0x2c-2x2f (ADM1030) */
- static const unsigned short scan_ds1775[] = {
- 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f,
- I2C_CLIENT_END
- };
- static const unsigned short scan_adm1030[] = {
- 0x2c, 0x2d, 0x2e, 0x2f,
- I2C_CLIENT_END
- };
-
- if( strncmp(adapter->name, "uni-n", 5) )
- return 0;
-
- if( !x.running ) {
- struct i2c_board_info info;
-
- memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "therm_ds1775", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_ds1775, NULL);
-
- strlcpy(info.type, "therm_adm1030", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_adm1030, NULL);
-
- if( x.thermostat && x.fan ) {
- x.running = 1;
- x.poll_task = kthread_run(control_loop, NULL, "g4fand");
- }
- }
- return 0;
-}
-
static void
try_start_control_loop(void)
{
@@ -442,7 +408,6 @@ static struct i2c_driver g4fan_driver = {
.driver = {
.name = "therm_windtunnel",
},
- .attach_adapter = do_attach,
.probe = do_probe,
.remove = do_remove,
.id_table = therm_windtunnel_id,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] therm_windtunnel doesn't work properly on PowerMac G4
Rename the driver name
therm_ds1775 -> MAC,ds1775
therm_adm1030 -> MAC,adm1030
Start the background fan control loop from
the devices probing methods.
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 3b4a157..9583413 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -334,6 +334,15 @@ do_attach( struct i2c_adapter *adapter )
return 0;
}
+static void
+try_start_control_loop(void)
+{
+ if (x.thermostat && x.fan && !x.running) {
+ x.running = 1;
+ x.poll_task = kthread_run(control_loop, NULL, "g4fand");
+ }
+}
+
static int
do_remove(struct i2c_client *client)
{
@@ -364,6 +373,7 @@ attach_fan( struct i2c_client *cl )
printk("ADM1030 fan controller [@%02x]\n", cl->addr );
x.fan = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -397,6 +407,7 @@ attach_thermostat( struct i2c_client *cl )
x.overheat_temp = os_temp;
x.overheat_hyst = hyst_temp;
x.thermostat = cl;
+ try_start_control_loop();
out:
return 0;
}
@@ -404,8 +415,8 @@ out:
enum chip { ds1775, adm1030 };
static const struct i2c_device_id therm_windtunnel_id[] = {
- { "therm_ds1775", ds1775 },
- { "therm_adm1030", adm1030 },
+ { "MAC,ds1775", ds1775 },
+ { "MAC,adm1030", adm1030 },
{ }
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] therm_windtunnel doesn't work properly on PowerMac G4
Add the "log_temp" and "verbose" module parameters.
log_tempenable/disable the temperature logging
verbose enable/disable the fan tune logging
Signed-off-by: Goffredo Baroncelli
---
drivers/macintosh/therm_windtunnel.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index fbe4516..7efba5d 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -44,7 +44,13 @@
#include
#include
-#define LOG_TEMP 0 /* continuously log
temperature */
+static bool log_temp = 0;
+module_param(log_temp, bool, 0644);
+MODULE_PARM_DESC(log_temp, "Enable the temperature logging");
+
+static bool verbose = 1;
+module_param(verbose, bool, 0644);
+MODULE_PARM_DESC(verbose, "Enable the fan speed logging");
static struct {
volatile intrunning;
@@ -157,11 +163,12 @@ tune_fan( int fan_setting )
/* write_reg( x.fan, 0x24, val, 1 ); */
write_reg( x.fan, 0x25, val, 1 );
write_reg( x.fan, 0x20, 0, 1 );
- print_temp("CPU-temp: ", x.temp );
- if( x.casetemp )
+ if (verbose) {
+ print_temp("CPU-temp: ", x.temp );
print_temp(", Case: ", x.casetemp );
- printk(", Fan: %d (tuned %+d)\n", 11-fan_setting,
x.fan_level-fan_setting );
-
+ printk(", Fan: %d (tuned %+d)\n",
+ 11-fan_setting, x.fan_level-fan_setting );
+ }
x.fan_level = fan_setting;
}
@@ -179,7 +186,7 @@ poll_temp( void )
casetemp = read_reg(x.fan, 0x0b, 1) << 8;
casetemp |= (read_reg(x.fan, 0x06, 1) & 0x7) << 5;
- if( LOG_TEMP && x.temp != temp ) {
+ if( log_temp && x.temp != temp ) {
print_temp("CPU-temp: ", temp );
print_temp(", Case: ", casetemp );
printk(", Fan: %d\n", 11-x.fan_level );
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] therm_windtunnel doesn't work properly on PowerMac G4
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 Date: Wed Apr 18 22:16:42 2012 + 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 [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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] btrfs-progs: Fix up memory leakage
On 09/25/2012 07:14 PM, Goffredo Baroncelli wrote: I strongly disagree with this approach. The callee often don't know what happen after and before the call. The same is true for the programmer, because the code is quite often updated by several people. A clean exit() is the right thing to do as general rule. My fingers were faster than my brain :-) s/clean exit()/clean-up/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] btrfs-progs: Fix up memory leakage
On 09/25/2012 12:14 PM, David Sterba wrote: On Tue, Sep 25, 2012 at 10:02:16AM +0800, zwu.ker...@gmail.com wrote: From: Zhi Yong Wu Some code pathes forget to free memory on exit. Same as with the fd's, kernel will free all memory for us at exit(). I strongly disagree with this approach. The callee often don't know what happen after and before the call. The same is true for the programmer, because the code is quite often updated by several people. A clean exit() is the right thing to do as general rule. I don't see any valid reason (in the btrfs context) to do otherwise. Relying on the exit() for a proper clean-up increase the likelihood of bug when the code evolves (see my patch [RESPOST][BTRFS-PROGS][PATCH] btrfs_read_dev_super(): uninitialized variable for an example of what means an incorrect deallocation of resource). If there's lots of memory allocated, it may be even faster to leave the unallocation process to kernel as it will do it in one go, while the application would unnecessarily free it chunk by chunk. May be I am wrong, but I don't think that the increase of speed of the btrfs "command" is even measurable relying on exit instead of free()-ing each chunk of memory one at time The same should be true for the open()/close() My 2¢ BR G.Baroncelli david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html . -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] btrfs: fix disk-io.c/btrfs_read_dev_super with BTRFS_SUPER_MIRROR_MAX to control the loops
Hi,
On 09/10/2012 08:38 AM, Wang Sheng-Hui wrote:
To check the duplicated super blocks, use BTRFS_SUPER_MIRROR_MAX
as the loops limit.
Signed-off-by: Wang Sheng-Hui
---
fs/btrfs/disk-io.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 22e98e0..a431144 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2723,7 +2723,7 @@ struct buffer_head *btrfs_read_dev_super(struct
block_device *bdev)
* So, we need to add a special mount option to scan for
* later supers, using BTRFS_SUPER_MIRROR_MAX instead
*/
- for (i = 0; i< 1; i++) {
+ for (i = 0; i< BTRFS_SUPER_MIRROR_MAX; i++) {
bytenr = btrfs_sb_offset(i);
if (bytenr + 4096>= i_size_read(bdev->bd_inode))
break;
Pay attention that when a device is removed from a filesystem by the
command "btrfs device delete", *only* the 1st superblock btrfs signature
is zeroed [1]. This means that the other (backup) superblocks will be
considered valid with your change.
[1] See the function btrfs_rm_device() in volume.c
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
