Commit c3df5806 authored by Jean Delvare's avatar Jean Delvare Committed by Greg Kroah-Hartman

[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: default avatarJean Delvare <khali@linux-fr.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent 8256fe0f
......@@ -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.
......@@ -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
......@@ -345,14 +345,62 @@ 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;
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) {
......@@ -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:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment