hwmon: (pmbus/adm1275) Fix problems with temperature monitoring on ADM1272

[ Upstream commit b153a0bb4199566abd337119207f82b59a8cd1ca ]

The PMON_CONFIG register on ADM1272 is a 16 bit register. Writing a 8 bit
value into it clears the upper 8 bits of the register, resulting in
unexpected side effects. Fix by writing the 16 bit register value.

Also, it has been reported that temperature readings are sometimes widely
inaccurate, to the point where readings may result in device shutdown due
to errant overtemperature faults. Improve by enabling temperature sampling.

While at it, move the common code for ADM1272 and ADM1278 into a separate
function, and clarify in the error message that an attempt was made to
enable both VOUT and temperature monitoring.

Last but not least, return the error code reported by the underlying I2C
controller and not -ENODEV if updating the PMON_CONFIG register fails.
After all, this does not indicate that the chip is not present, but an
error in the communication with the chip.

Fixes: 4ff0ce227a1e ("hwmon: (pmbus/adm1275) Add support for ADM1272")
Fixes: 9da9c2dc57b2 ("hwmon: (adm1275) enable adm1272 temperature reporting")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20230602213447.3557346-1-linux@roeck-us.net
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index 3b07bfb..b8543c0 100644 (file)
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -37,10 +37,13 @@ enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1293, adm1294 };
 
 #define ADM1272_IRANGE                 BIT(0)
 
+#define ADM1278_TSFILT                 BIT(15)
 #define ADM1278_TEMP1_EN               BIT(3)
 #define ADM1278_VIN_EN                 BIT(2)
 #define ADM1278_VOUT_EN                        BIT(1)
 
+#define ADM1278_PMON_DEFCONFIG         (ADM1278_VOUT_EN | ADM1278_TEMP1_EN | ADM1278_TSFILT)
+
 #define ADM1293_IRANGE_25              0
 #define ADM1293_IRANGE_50              BIT(6)
 #define ADM1293_IRANGE_100             BIT(7)
@@ -462,6 +465,22 @@ static const struct i2c_device_id adm1275_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, adm1275_id);
 
+/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
+static int adm1275_enable_vout_temp(struct i2c_client *client, int config)
+{
+       int ret;
+
+       if ((config & ADM1278_PMON_DEFCONFIG) != ADM1278_PMON_DEFCONFIG) {
+               config |= ADM1278_PMON_DEFCONFIG;
+               ret = i2c_smbus_write_word_data(client, ADM1275_PMON_CONFIG, config);
+               if (ret < 0) {
+                       dev_err(&client->dev, "Failed to enable VOUT/TEMP1 monitoring\n");
+                       return ret;
+               }
+       }
+       return 0;
+}
+
 static int adm1275_probe(struct i2c_client *client)
 {
        s32 (*config_read_fn)(const struct i2c_client *client, u8 reg);
@@ -615,19 +634,10 @@ static int adm1275_probe(struct i2c_client *client)
                        PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
                        PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 
-               /* Enable VOUT & TEMP1 if not enabled (disabled by default) */
-               if ((config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) !=
-                   (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) {
-                       config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
-                       ret = i2c_smbus_write_byte_data(client,
-                                                       ADM1275_PMON_CONFIG,
-                                                       config);
-                       if (ret < 0) {
-                               dev_err(&client->dev,
-                                       "Failed to enable VOUT monitoring\n");
-                               return -ENODEV;
-                       }
-               }
+               ret = adm1275_enable_vout_temp(client, config);
+               if (ret)
+                       return ret;
+
                if (config & ADM1278_VIN_EN)
                        info->func[0] |= PMBUS_HAVE_VIN;
                break;
@@ -684,19 +694,9 @@ static int adm1275_probe(struct i2c_client *client)
                        PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
                        PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 
-               /* Enable VOUT & TEMP1 if not enabled (disabled by default) */
-               if ((config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) !=
-                   (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) {
-                       config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
-                       ret = i2c_smbus_write_word_data(client,
-                                                       ADM1275_PMON_CONFIG,
-                                                       config);
-                       if (ret < 0) {
-                               dev_err(&client->dev,
-                                       "Failed to enable VOUT monitoring\n");
-                               return -ENODEV;
-                       }
-               }
+               ret = adm1275_enable_vout_temp(client, config);
+               if (ret)
+                       return ret;
 
                if (config & ADM1278_VIN_EN)
                        info->func[0] |= PMBUS_HAVE_VIN;