From 245b7171b3adf492ee7b8c7b652976fb77a6df30 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Sat, 5 Nov 2005 20:47:45 +0000 Subject: [PATCH] Backport from Linux 2.6: Add PEC support to the lm90 driver. Only the ADM1032 chip supports it, and in a rather tricky way, which is why this patch comes with documentation reinforcements. At least, this demonstrates that the new PEC support logic in i2c-core can properly deal with chips with partial PEC support. As enabling PEC causes a significant performance drop, it can be disabled through a procfs file (unsurprisingly named "pec"). git-svn-id: http://lm-sensors.org/svn/lm-sensors/trunk@3181 7894878c-1315-0410-8ee3-d5d059ff63e0 --- doc/chips/lm90 | 41 +++++++++++++++++++++++-- kernel/chips/lm90.c | 75 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 113 insertions(+), 3 deletions(-) diff --git a/doc/chips/lm90 b/doc/chips/lm90 index 80d00c61..ffa58cc5 100644 --- a/doc/chips/lm90 +++ b/doc/chips/lm90 @@ -106,8 +106,8 @@ increased resolution of the remote temperature measurement. The different chipsets of the family are not strictly identical, although very similar. This driver doesn't handle any specific feature for now, -but could if there ever was a need for it. For reference, here comes a -non-exhaustive list of specific features: +with the exception of SMBus PEC. For reference, here comes a non-exhaustive +list of specific features: LM90: * Filter and alert configuration register at 0xBF. @@ -126,6 +126,7 @@ ADM1032: * Conversion averaging. * Up to 64 conversions/s. * ALERT is triggered by open remote sensor. + * SMBus PEC support for Write Byte and Receive Byte transactions. ADT7461 * Extended temperature range (breaks compatibility) @@ -154,6 +155,42 @@ other second; reading them more often will do no harm, but will return 'old' values. +PEC Support +----------- + +The ADM1032 is the only chip of the family which supports PEC. It does +not support PEC on all transactions though, so some care must be taken. + +When reading a register value, the PEC byte is computed and sent by the +ADM1032 chip. However, in the case of a combined transaction (SMBus Read +Byte), the ADM1032 computes the CRC value over only the second half of +the message rather than its entirety, because it thinks the first half +of the message belongs to a different transaction. As a result, the CRC +value differs from what the SMBus master expects, and all reads fail. + +For this reason, the lm90 driver will enable PEC for the ADM1032 only if +the bus supports the SMBus Send Byte and Receive Byte transaction types. +These transactions will be used to read register values, instead of +SMBus Read Byte, and PEC will work properly. + +Additionally, the ADM1032 doesn't support SMBus Send Byte with PEC. +Instead, it will try to write the PEC value to the register (because the +SMBus Send Byte transaction with PEC is similar to a Write Byte transaction +without PEC), which is not what we want. Thus, PEC is explicitely disabled +on SMBus Send Byte transactions in the lm90 driver. + +PEC on byte data transactions represents a significant increase in bandwidth +usage (+33% for writes, +25% for reads) in normal conditions. With the need +to use two SMBus transaction for reads, this overhead jumps to +50%. Worse, +two transactions will typically mean twice as much delay waiting for +transaction completion, effectively doubling the register cache refresh time. +I guess reliability comes at a price, but it's quite expensive this time. + +So, as not everyone might enjoy the slowdown, PEC can be disabled through +procfs. Just write 0 to the "pec" file and PEC will be disabled. Write 1 +to that file to enable PEC again. + + Chip Features ------------- diff --git a/kernel/chips/lm90.c b/kernel/chips/lm90.c index 21d4b525..c6e4f3c0 100644 --- a/kernel/chips/lm90.c +++ b/kernel/chips/lm90.c @@ -194,6 +194,8 @@ static void lm90_remote_hyst(struct i2c_client *client, int operation, int ctl_name, int *nrels_mag, long *results); static void lm90_alarms(struct i2c_client *client, int operation, int ctl_name, int *nrels_mag, long *results); +static void adm1032_pec(struct i2c_client *client, int operation, + int ctl_name, int *nrels_mag, long *results); /* * Driver data (common to all clients) @@ -242,6 +244,7 @@ struct lm90_data { #define LM90_SYSCTL_LOCAL_HYST 1207 #define LM90_SYSCTL_REMOTE_HYST 1208 #define LM90_SYSCTL_ALARMS 1210 +#define LM90_SYSCTL_PEC 1214 #define LM90_ALARM_LOCAL_HIGH 0x40 #define LM90_ALARM_LOCAL_LOW 0x20 @@ -273,15 +276,54 @@ static ctl_table lm90_dir_table_template[] = {0} }; +static ctl_table adm1032_dir_table_template[] = +{ + {LM90_SYSCTL_LOCAL_TEMP, "temp1", NULL, 0, 0644, NULL, + &i2c_proc_real, &i2c_sysctl_real, NULL, &lm90_local_temp}, + {LM90_SYSCTL_REMOTE_TEMP, "temp2", NULL, 0, 0644, NULL, + &i2c_proc_real, &i2c_sysctl_real, NULL, &lm90_remote_temp}, + {LM90_SYSCTL_LOCAL_TCRIT, "tcrit1", NULL, 0, 0644, NULL, + &i2c_proc_real, &i2c_sysctl_real, NULL, &lm90_local_tcrit}, + {LM90_SYSCTL_REMOTE_TCRIT, "tcrit2", NULL, 0, 0644, NULL, + &i2c_proc_real, &i2c_sysctl_real, NULL, &lm90_remote_tcrit}, + {LM90_SYSCTL_LOCAL_HYST, "hyst1", NULL, 0, 0644, NULL, + &i2c_proc_real, &i2c_sysctl_real, NULL, &lm90_local_hyst}, + {LM90_SYSCTL_REMOTE_HYST, "hyst2", NULL, 0, 0444, NULL, + &i2c_proc_real, &i2c_sysctl_real, NULL, &lm90_remote_hyst}, + {LM90_SYSCTL_ALARMS, "alarms", NULL, 0, 0444, NULL, + &i2c_proc_real, &i2c_sysctl_real, NULL, &lm90_alarms}, + {LM90_SYSCTL_PEC, "pec", NULL, 0, 0644, NULL, + &i2c_proc_real, &i2c_sysctl_real, NULL, &adm1032_pec}, + {0} +}; + /* * Real code */ +/* The ADM1032 supports PEC but not on write byte transactions, so we need + to explicitely ask for a transaction without PEC. */ +static inline s32 adm1032_write_byte(struct i2c_client *client, u8 value) +{ + return i2c_smbus_xfer(client->adapter, client->addr, + client->flags & ~I2C_CLIENT_PEC, + I2C_SMBUS_WRITE, value, I2C_SMBUS_BYTE, NULL); +} + +/* It is assumed that client->update_lock is held (unless we are in + detection or initialization steps). This matters when PEC is enabled, + because we don't want the address pointer to change between the write + byte and the read byte transactions. */ static int lm90_read_reg(struct i2c_client* client, u8 reg, u8 *value) { int err; - err = i2c_smbus_read_byte_data(client, reg); + if (client->flags & I2C_CLIENT_PEC) { + err = adm1032_write_byte(client, reg); + if (err >= 0) + err = i2c_smbus_read_byte(client); + } else + err = i2c_smbus_read_byte_data(client, reg); if (err < 0) { printk(KERN_WARNING "lm90: Register 0x%02x read failed (%d)\n", @@ -424,6 +466,12 @@ static int lm90_detect(struct i2c_adapter *adapter, int address, } else if (kind == adm1032) { type_name = "adm1032"; client_name = "ADM1032 chip"; + /* The ADM1032 supports PEC, but only if combined + transactions are not used. */ + if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE)) { + new_client->flags |= I2C_CLIENT_PEC; + printk(KERN_DEBUG "lm90: Enabling PEC for ADM1032\n"); + } } else if (kind == lm99) { type_name = "lm99"; client_name = "LM99 chip"; @@ -465,6 +513,8 @@ static int lm90_detect(struct i2c_adapter *adapter, int address, */ if ((err = i2c_register_entry(new_client, type_name, + (new_client->flags & I2C_CLIENT_PEC) ? + adm1032_dir_table_template : lm90_dir_table_template, THIS_MODULE)) < 0) { printk(KERN_ERR "lm90: Failed to register directory entry " "(%d)\n", err); @@ -765,6 +815,29 @@ static void lm90_alarms(struct i2c_client *client, int operation, } } +/* pec used for ADM1032 only */ +static void adm1032_pec(struct i2c_client *client, int operation, + int ctl_name, int *nrels_mag, long *results) +{ + if (operation == SENSORS_PROC_REAL_INFO) + *nrels_mag = 0; /* magnitude */ + else if (operation == SENSORS_PROC_REAL_READ) { + results[0] = !!(client->flags & I2C_CLIENT_PEC); + *nrels_mag = 1; + } else if (operation == SENSORS_PROC_REAL_WRITE) { + if (*nrels_mag >= 1) { + switch (results[0]) { + case 0: + client->flags &= ~I2C_CLIENT_PEC; + break; + case 1: + client->flags |= I2C_CLIENT_PEC; + break; + } + } + } +} + static int __init sm_lm90_init(void) { printk(KERN_INFO "lm90 driver version %s (%s)\n", LM_VERSION,