From c3df5806cdae6fac678c662b527cb974bef4b60c Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Wed, 26 Oct 2005 21:39:40 +0200 Subject: [PATCH] [PATCH] hwmon: Add PEC support to the lm90 driver 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 sysfs file (unsurprisingly named "pec"). Signed-off-by: Jean Delvare Signed-off-by: Greg Kroah-Hartman --- Documentation/hwmon/lm90 | 39 +++++++++++++++++++- Documentation/hwmon/sysfs-interface | 3 ++ drivers/hwmon/lm90.c | 57 ++++++++++++++++++++++++++++- 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/Documentation/hwmon/lm90 b/Documentation/hwmon/lm90 index 2c4cf39471f..70abf93ea37 100644 --- a/Documentation/hwmon/lm90 +++ b/Documentation/hwmon/lm90 @@ -71,8 +71,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. @@ -91,6 +91,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) @@ -119,3 +120,37 @@ The lm90 driver will not update its values more frequently than every 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 +sysfs. Just write 0 to the "pec" file and PEC will be disabled. Write 1 +to that file to enable PEC again. diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface index 346400519d0..764cdc5480e 100644 --- a/Documentation/hwmon/sysfs-interface +++ b/Documentation/hwmon/sysfs-interface @@ -272,3 +272,6 @@ beep_mask Bitmask for beep. eeprom Raw EEPROM data in binary form. Read only. + +pec Enable or disable PEC (SMBus only) + Read/Write diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index 5acc12ae4e3..fa0793e684f 100644 --- a/drivers/hwmon/lm90.c +++ b/drivers/hwmon/lm90.c @@ -345,15 +345,63 @@ static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_temphyst, static SENSOR_DEVICE_ATTR(temp2_crit_hyst, S_IRUGO, show_temphyst, NULL, 4); static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL); +/* pec used for ADM1032 only */ +static ssize_t show_pec(struct device *dev, struct device_attribute *dummy, + char *buf) +{ + struct i2c_client *client = to_i2c_client(dev); + return sprintf(buf, "%d\n", !!(client->flags & I2C_CLIENT_PEC)); +} + +static ssize_t set_pec(struct device *dev, struct device_attribute *dummy, + const char *buf, size_t count) +{ + struct i2c_client *client = to_i2c_client(dev); + long val = simple_strtol(buf, NULL, 10); + + switch (val) { + case 0: + client->flags &= ~I2C_CLIENT_PEC; + break; + case 1: + client->flags |= I2C_CLIENT_PEC; + break; + default: + return -EINVAL; + } + + return count; +} + +static DEVICE_ATTR(pec, S_IWUSR | S_IRUGO, show_pec, set_pec); + /* * 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) { dev_warn(&client->dev, "Register %#02x read failed (%d)\n", @@ -494,6 +542,10 @@ static int lm90_detect(struct i2c_adapter *adapter, int address, int kind) name = "lm90"; } else if (kind == adm1032) { name = "adm1032"; + /* 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; } else if (kind == lm99) { name = "lm99"; } else if (kind == lm86) { @@ -546,6 +598,9 @@ static int lm90_detect(struct i2c_adapter *adapter, int address, int kind) &sensor_dev_attr_temp2_crit_hyst.dev_attr); device_create_file(&new_client->dev, &dev_attr_alarms); + if (new_client->flags & I2C_CLIENT_PEC) + device_create_file(&new_client->dev, &dev_attr_pec); + return 0; exit_detach: