Use two-dimensional array pointing to registers
Merge temperature and limit access functions into a single function
Return error codes from I2C reads
Use DIV_ROUND_CLOSEST for rounding operations and improve rounding
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
v3: Improve rounding
Fix spelling error in commit log
Fix rounding error when writing critical limit
v2: Fix spelling error in commit log, and clarify that only i2c reads
return errors
Mark unused array entries by using 0 instead of 0x00 for improved
readability
Fix rounding error when writing critical limit
Don't rename reg to regval in store_temp
Use local client variable in store_temp
drivers/hwmon/tmp401.c | 371 ++++++++++++++++++------------------------------
1 file changed, 137 insertions(+), 234 deletions(-)
diff --git a/drivers/hwmon/tmp401.c b/drivers/hwmon/tmp401.c
index f9c492e..4d306b2 100644
--- a/drivers/hwmon/tmp401.c
+++ b/drivers/hwmon/tmp401.c
@@ -58,21 +58,32 @@ enum chips { tmp401, tmp411, tmp431 };
#define TMP401_MANUFACTURER_ID_REG 0xFE
#define TMP401_DEVICE_ID_REG 0xFF
-static const u8 TMP401_TEMP_MSB[2] = { 0x00, 0x01 };
-static const u8 TMP401_TEMP_LSB[2] = { 0x15, 0x10 };
-static const u8 TMP401_TEMP_LOW_LIMIT_MSB_READ[2] = { 0x06, 0x08 };
-static const u8 TMP401_TEMP_LOW_LIMIT_MSB_WRITE[2] = { 0x0C, 0x0E };
-static const u8 TMP401_TEMP_LOW_LIMIT_LSB[2] = { 0x17, 0x14 };
-static const u8 TMP401_TEMP_HIGH_LIMIT_MSB_READ[2] = { 0x05, 0x07 };
-static const u8 TMP401_TEMP_HIGH_LIMIT_MSB_WRITE[2] = { 0x0B, 0x0D };
-static const u8 TMP401_TEMP_HIGH_LIMIT_LSB[2] = { 0x16, 0x13 };
-/* These are called the THERM limit / hysteresis / mask in the datasheet */
-static const u8 TMP401_TEMP_CRIT_LIMIT[2] = { 0x20, 0x19 };
-
-static const u8 TMP411_TEMP_LOWEST_MSB[2] = { 0x30, 0x34 };
-static const u8 TMP411_TEMP_LOWEST_LSB[2] = { 0x31, 0x35 };
-static const u8 TMP411_TEMP_HIGHEST_MSB[2] = { 0x32, 0x36 };
-static const u8 TMP411_TEMP_HIGHEST_LSB[2] = { 0x33, 0x37 };
+static const u8 TMP401_TEMP_MSB_READ[6][2] = {
+ { 0x00, 0x01 }, /* temp */
+ { 0x06, 0x08 }, /* low limit */
+ { 0x05, 0x07 }, /* high limit */
+ { 0x20, 0x19 }, /* therm (crit) limit */
+ { 0x30, 0x34 }, /* lowest */
+ { 0x32, 0x36 }, /* highest */
+};
+
+static const u8 TMP401_TEMP_MSB_WRITE[6][2] = {
+ { 0, 0 }, /* temp (unused) */
+ { 0x0C, 0x0E }, /* low limit */
+ { 0x0B, 0x0D }, /* high limit */
+ { 0x20, 0x19 }, /* therm (crit) limit */
+ { 0x30, 0x34 }, /* lowest */
+ { 0x32, 0x36 }, /* highest */
+};
+
+static const u8 TMP401_TEMP_LSB[6][2] = {
+ { 0x15, 0x10 }, /* temp */
+ { 0x17, 0x14 }, /* low limit */
+ { 0x16, 0x13 }, /* high limit */
+ { 0, 0 }, /* therm (crit) limit (unused) */
+ { 0x31, 0x35 }, /* lowest */
+ { 0x33, 0x37 }, /* highest */
+};
/* Flags */
#define TMP401_CONFIG_RANGE BIT(2)
@@ -119,13 +130,8 @@ struct tmp401_data {
/* register values */
u8 status;
u8 config;
- u16 temp[2];
- u16 temp_low[2];
- u16 temp_high[2];
- u8 temp_crit[2];
+ u16 temp[6][2];
u8 temp_crit_hyst;
- u16 temp_lowest[2];
- u16 temp_highest[2];
};
/*
@@ -139,31 +145,10 @@ static int tmp401_register_to_temp(u16 reg, u8 config)
if (config & TMP401_CONFIG_RANGE)
temp -= 64 * 256;
- return (temp * 625 + 80) / 160;
-}
-
-static u16 tmp401_temp_to_register(long temp, u8 config)
-{
- if (config & TMP401_CONFIG_RANGE) {
- temp = clamp_val(temp, -64000, 191000);
- temp += 64000;
- } else
- temp = clamp_val(temp, 0, 127000);
-
- return (temp * 160 + 312) / 625;
+ return DIV_ROUND_CLOSEST(temp * 125, 32);
}
-static int tmp401_crit_register_to_temp(u8 reg, u8 config)
-{
- int temp = reg;
-
- if (config & TMP401_CONFIG_RANGE)
- temp -= 64;
-
- return temp * 1000;
-}
-
-static u8 tmp401_crit_temp_to_register(long temp, u8 config)
+static u16 tmp401_temp_to_register(long temp, u8 config, int zbits)
{
if (config & TMP401_CONFIG_RANGE) {
temp = clamp_val(temp, -64000, 191000);
@@ -171,113 +156,93 @@ static u8 tmp401_crit_temp_to_register(long temp, u8 config)
} else
temp = clamp_val(temp, 0, 127000);
- return (temp + 500) / 1000;
+ return DIV_ROUND_CLOSEST(temp * (1 << (8 - zbits)), 1000) << zbits;
}
-static struct tmp401_data *tmp401_update_device_reg16(
- struct i2c_client *client, struct tmp401_data *data)
+static int tmp401_update_device_reg16(struct i2c_client *client,
+ struct tmp401_data *data)
{
- int i;
-
- for (i = 0; i < 2; i++) {
- /*
- * High byte must be read first immediately followed
- * by the low byte
- */
- data->temp[i] = i2c_smbus_read_byte_data(client,
- TMP401_TEMP_MSB[i]) << 8;
- data->temp[i] |= i2c_smbus_read_byte_data(client,
- TMP401_TEMP_LSB[i]);
- data->temp_low[i] = i2c_smbus_read_byte_data(client,
- TMP401_TEMP_LOW_LIMIT_MSB_READ[i]) << 8;
- data->temp_low[i] |= i2c_smbus_read_byte_data(client,
- TMP401_TEMP_LOW_LIMIT_LSB[i]);
- data->temp_high[i] = i2c_smbus_read_byte_data(client,
- TMP401_TEMP_HIGH_LIMIT_MSB_READ[i]) << 8;
- data->temp_high[i] |= i2c_smbus_read_byte_data(client,
- TMP401_TEMP_HIGH_LIMIT_LSB[i]);
- data->temp_crit[i] = i2c_smbus_read_byte_data(client,
- TMP401_TEMP_CRIT_LIMIT[i]);
-
- if (data->kind == tmp411) {
- data->temp_lowest[i] = i2c_smbus_read_byte_data(client,
- TMP411_TEMP_LOWEST_MSB[i]) << 8;
- data->temp_lowest[i] |= i2c_smbus_read_byte_data(
- client, TMP411_TEMP_LOWEST_LSB[i]);
-
- data->temp_highest[i] = i2c_smbus_read_byte_data(
- client, TMP411_TEMP_HIGHEST_MSB[i]) << 8;
- data->temp_highest[i] |= i2c_smbus_read_byte_data(
- client, TMP411_TEMP_HIGHEST_LSB[i]);
+ int i, j, val;
+ int num_regs = data->kind == tmp411 ? 6 : 4;
+
+ for (i = 0; i < 2; i++) { /* local / rem1 */
+ for (j = 0; j < num_regs; j++) { /* temp / low / ... */
+ /*
+ * High byte must be read first immediately followed
+ * by the low byte
+ */
+ val = i2c_smbus_read_byte_data(client,
+ TMP401_TEMP_MSB_READ[j][i]);
+ if (val < 0)
+ return val;
+ data->temp[j][i] = val << 8;
+ if (j == 3) /* crit is msb only */
+ continue;
+ val = i2c_smbus_read_byte_data(client,
+ TMP401_TEMP_LSB[j][i]);
+ if (val < 0)
+ return val;
+ data->temp[j][i] |= val;
}
}
- return data;
+ return 0;
}
static struct tmp401_data *tmp401_update_device(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct tmp401_data *data = i2c_get_clientdata(client);
+ struct tmp401_data *ret = data;
+ int val;
mutex_lock(&data->update_lock);
if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
- data->status = i2c_smbus_read_byte_data(client, TMP401_STATUS);
- data->config = i2c_smbus_read_byte_data(client,
- TMP401_CONFIG_READ);
- tmp401_update_device_reg16(client, data);
-
- data->temp_crit_hyst = i2c_smbus_read_byte_data(client,
- TMP401_TEMP_CRIT_HYST);
+ val = i2c_smbus_read_byte_data(client, TMP401_STATUS);
+ if (val < 0) {
+ ret = ERR_PTR(val);
+ goto abort;
+ }
+ data->status = val;
+ val = i2c_smbus_read_byte_data(client, TMP401_CONFIG_READ);
+ if (val < 0) {
+ ret = ERR_PTR(val);
+ goto abort;
+ }
+ data->config = val;
+ val = tmp401_update_device_reg16(client, data);
+ if (val < 0) {
+ ret = ERR_PTR(val);
+ goto abort;
+ }
+ val = i2c_smbus_read_byte_data(client, TMP401_TEMP_CRIT_HYST);
+ if (val < 0) {
+ ret = ERR_PTR(val);
+ goto abort;
+ }
+ data->temp_crit_hyst = val;
data->last_updated = jiffies;
data->valid = 1;
}
+abort:
mutex_unlock(&data->update_lock);
-
- return data;
+ return ret;
}
-static ssize_t show_temp_value(struct device *dev,
- struct device_attribute *devattr, char *buf)
-{
- int index = to_sensor_dev_attr(devattr)->index;
- struct tmp401_data *data = tmp401_update_device(dev);
-
- return sprintf(buf, "%d\n",
- tmp401_register_to_temp(data->temp[index], data->config));
-}
-
-static ssize_t show_temp_min(struct device *dev,
- struct device_attribute *devattr, char *buf)
-{
- int index = to_sensor_dev_attr(devattr)->index;
- struct tmp401_data *data = tmp401_update_device(dev);
-
- return sprintf(buf, "%d\n",
- tmp401_register_to_temp(data->temp_low[index], data->config));
-}
-
-static ssize_t show_temp_max(struct device *dev,
- struct device_attribute *devattr, char *buf)
+static ssize_t show_temp(struct device *dev,
+ struct device_attribute *devattr, char *buf)
{
- int index = to_sensor_dev_attr(devattr)->index;
+ int nr = to_sensor_dev_attr_2(devattr)->nr;
+ int index = to_sensor_dev_attr_2(devattr)->index;
struct tmp401_data *data = tmp401_update_device(dev);
- return sprintf(buf, "%d\n",
- tmp401_register_to_temp(data->temp_high[index], data->config));
-}
-
-static ssize_t show_temp_crit(struct device *dev,
- struct device_attribute *devattr, char *buf)
-{
- int index = to_sensor_dev_attr(devattr)->index;
- struct tmp401_data *data = tmp401_update_device(dev);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
return sprintf(buf, "%d\n",
- tmp401_crit_register_to_temp(data->temp_crit[index],
- data->config));
+ tmp401_register_to_temp(data->temp[nr][index], data->config));
}
static ssize_t show_temp_crit_hyst(struct device *dev,
@@ -286,122 +251,58 @@ static ssize_t show_temp_crit_hyst(struct device *dev,
int temp, index = to_sensor_dev_attr(devattr)->index;
struct tmp401_data *data = tmp401_update_device(dev);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
mutex_lock(&data->update_lock);
- temp = tmp401_crit_register_to_temp(data->temp_crit[index],
- data->config);
+ temp = tmp401_register_to_temp(data->temp[3][index], data->config);
temp -= data->temp_crit_hyst * 1000;
mutex_unlock(&data->update_lock);
return sprintf(buf, "%d\n", temp);
}
-static ssize_t show_temp_lowest(struct device *dev,
- struct device_attribute *devattr, char *buf)
-{
- int index = to_sensor_dev_attr(devattr)->index;
- struct tmp401_data *data = tmp401_update_device(dev);
-
- return sprintf(buf, "%d\n",
- tmp401_register_to_temp(data->temp_lowest[index],
- data->config));
-}
-
-static ssize_t show_temp_highest(struct device *dev,
- struct device_attribute *devattr, char *buf)
-{
- int index = to_sensor_dev_attr(devattr)->index;
- struct tmp401_data *data = tmp401_update_device(dev);
-
- return sprintf(buf, "%d\n",
- tmp401_register_to_temp(data->temp_highest[index],
- data->config));
-}
-
static ssize_t show_status(struct device *dev,
struct device_attribute *devattr, char *buf)
{
int mask = to_sensor_dev_attr(devattr)->index;
struct tmp401_data *data = tmp401_update_device(dev);
- if (data->status & mask)
- return sprintf(buf, "1\n");
- else
- return sprintf(buf, "0\n");
-}
-
-static ssize_t store_temp_min(struct device *dev, struct device_attribute
- *devattr, const char *buf, size_t count)
-{
- int index = to_sensor_dev_attr(devattr)->index;
- struct tmp401_data *data = tmp401_update_device(dev);
- long val;
- u16 reg;
-
- if (kstrtol(buf, 10, &val))
- return -EINVAL;
-
- reg = tmp401_temp_to_register(val, data->config);
-
- mutex_lock(&data->update_lock);
-
- i2c_smbus_write_byte_data(to_i2c_client(dev),
- TMP401_TEMP_LOW_LIMIT_MSB_WRITE[index], reg >> 8);
- i2c_smbus_write_byte_data(to_i2c_client(dev),
- TMP401_TEMP_LOW_LIMIT_LSB[index], reg & 0xFF);
-
- data->temp_low[index] = reg;
-
- mutex_unlock(&data->update_lock);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
- return count;
+ return sprintf(buf, "%d\n", !!(data->status & mask));
}
-static ssize_t store_temp_max(struct device *dev, struct device_attribute
- *devattr, const char *buf, size_t count)
+static ssize_t store_temp(struct device *dev, struct device_attribute *devattr,
+ const char *buf, size_t count)
{
- int index = to_sensor_dev_attr(devattr)->index;
+ int nr = to_sensor_dev_attr_2(devattr)->nr;
+ int index = to_sensor_dev_attr_2(devattr)->index;
+ struct i2c_client *client = to_i2c_client(dev);
struct tmp401_data *data = tmp401_update_device(dev);
long val;
u16 reg;
- if (kstrtol(buf, 10, &val))
- return -EINVAL;
-
- reg = tmp401_temp_to_register(val, data->config);
-
- mutex_lock(&data->update_lock);
-
- i2c_smbus_write_byte_data(to_i2c_client(dev),
- TMP401_TEMP_HIGH_LIMIT_MSB_WRITE[index], reg >> 8);
- i2c_smbus_write_byte_data(to_i2c_client(dev),
- TMP401_TEMP_HIGH_LIMIT_LSB[index], reg & 0xFF);
-
- data->temp_high[index] = reg;
-
- mutex_unlock(&data->update_lock);
-
- return count;
-}
-
-static ssize_t store_temp_crit(struct device *dev, struct device_attribute
- *devattr, const char *buf, size_t count)
-{
- int index = to_sensor_dev_attr(devattr)->index;
- struct tmp401_data *data = tmp401_update_device(dev);
- long val;
- u8 reg;
+ if (IS_ERR(data))
+ return PTR_ERR(data);
if (kstrtol(buf, 10, &val))
return -EINVAL;
- reg = tmp401_crit_temp_to_register(val, data->config);
+ reg = tmp401_temp_to_register(val, data->config, nr == 3 ? 8 : 4);
mutex_lock(&data->update_lock);
- i2c_smbus_write_byte_data(to_i2c_client(dev),
- TMP401_TEMP_CRIT_LIMIT[index], reg);
-
- data->temp_crit[index] = reg;
+ i2c_smbus_write_byte_data(client,
+ TMP401_TEMP_MSB_WRITE[nr][index],
+ reg >> 8);
+ if (nr != 3) {
+ i2c_smbus_write_byte_data(client,
+ TMP401_TEMP_LSB[nr][index],
+ reg & 0xFF);
+ }
+ data->temp[nr][index] = reg;
mutex_unlock(&data->update_lock);
@@ -416,6 +317,9 @@ static ssize_t store_temp_crit_hyst(struct device *dev, struct device_attribute
long val;
u8 reg;
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
if (kstrtol(buf, 10, &val))
return -EINVAL;
@@ -425,13 +329,12 @@ static ssize_t store_temp_crit_hyst(struct device *dev, struct device_attribute
val = clamp_val(val, 0, 127000);
mutex_lock(&data->update_lock);
- temp = tmp401_crit_register_to_temp(data->temp_crit[index],
- data->config);
+ temp = tmp401_register_to_temp(data->temp[3][index], data->config);
val = clamp_val(val, temp - 255000, temp);
reg = ((temp - val) + 500) / 1000;
- i2c_smbus_write_byte_data(to_i2c_client(dev),
- TMP401_TEMP_CRIT_HYST, reg);
+ i2c_smbus_write_byte_data(to_i2c_client(dev), TMP401_TEMP_CRIT_HYST,
+ reg);
data->temp_crit_hyst = reg;
@@ -460,18 +363,18 @@ static ssize_t reset_temp_history(struct device *dev,
return -EINVAL;
}
i2c_smbus_write_byte_data(to_i2c_client(dev),
- TMP411_TEMP_LOWEST_MSB[0], val);
+ TMP401_TEMP_MSB_WRITE[5][0], val);
return count;
}
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp_min,
- store_temp_min, 0);
-static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
- store_temp_max, 0);
-static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp_crit,
- store_temp_crit, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_input, S_IRUGO, show_temp, NULL, 0, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_min, S_IWUSR | S_IRUGO, show_temp,
+ store_temp, 1, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_max, S_IWUSR | S_IRUGO, show_temp,
+ store_temp, 2, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IWUSR | S_IRUGO, show_temp,
+ store_temp, 3, 0);
static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO,
show_temp_crit_hyst, store_temp_crit_hyst, 0);
static SENSOR_DEVICE_ATTR(temp1_min_alarm, S_IRUGO, show_status, NULL,
@@ -480,13 +383,13 @@ static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, show_status, NULL,
TMP401_STATUS_LOCAL_HIGH);
static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_status, NULL,
TMP401_STATUS_LOCAL_CRIT);
-static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp_min,
- store_temp_min, 1);
-static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp_max,
- store_temp_max, 1);
-static SENSOR_DEVICE_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_temp_crit,
- store_temp_crit, 1);
+static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, 0, 1);
+static SENSOR_DEVICE_ATTR_2(temp2_min, S_IWUSR | S_IRUGO, show_temp,
+ store_temp, 1, 1);
+static SENSOR_DEVICE_ATTR_2(temp2_max, S_IWUSR | S_IRUGO, show_temp,
+ store_temp, 2, 1);
+static SENSOR_DEVICE_ATTR_2(temp2_crit, S_IWUSR | S_IRUGO, show_temp,
+ store_temp, 3, 1);
static SENSOR_DEVICE_ATTR(temp2_crit_hyst, S_IRUGO, show_temp_crit_hyst,
NULL, 1);
static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_status, NULL,
@@ -532,10 +435,10 @@ static const struct attribute_group tmp401_group = {
* minimum and maximum register reset for both the local
* and remote channels.
*/
-static SENSOR_DEVICE_ATTR(temp1_highest, S_IRUGO, show_temp_highest, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp1_lowest, S_IRUGO, show_temp_lowest, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp2_highest, S_IRUGO, show_temp_highest, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp2_lowest, S_IRUGO, show_temp_lowest, NULL, 1);
+static SENSOR_DEVICE_ATTR_2(temp1_lowest, S_IRUGO, show_temp, NULL, 4, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_highest, S_IRUGO, show_temp, NULL, 5, 0);
+static SENSOR_DEVICE_ATTR_2(temp2_lowest, S_IRUGO, show_temp, NULL, 4, 1);
+static SENSOR_DEVICE_ATTR_2(temp2_highest, S_IRUGO, show_temp, NULL, 5, 1);
static SENSOR_DEVICE_ATTR(temp_reset_history, S_IWUSR, NULL, reset_temp_history,
0);
--
1.7.9.7
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
