hwmon: (tmp401) Simplification and cleanup

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@roeck-us.net>
Acked-by: Jean Delvare <khali@linux-fr.org>

1 files changed, 137 insertions, 234 deletions

diff --git a/drivers/hwmon/tmp401.c b/drivers/hwmon/tmp401.c
index f9c492e35fe4..4d306b29ba71 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);