hwmon: emc2305: fixups for driver submitted to mailing lists

The driver had a number of issues, checkpatch warnings/errors,
and other limitations, so fix these up to make it usable.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index 5c896fd..c78d672 100644 (file)
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Hardware monitoring driver for EMC2305 fan controller
+ * Hardware monitoring driver for Microchip EMC2305 fan controller
  *
  * Copyright (C) 2022 Nvidia Technologies Ltd and Delta Networks, Inc.
  */
@@ -17,17 +17,17 @@
 static const unsigned short
 emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d, I2C_CLIENT_END };
 
+#define EMC2305_REG_FAN_STATUS         0x24
+#define EMC2305_REG_FAN_STALL_STATUS   0x25
 #define EMC2305_REG_DRIVE_FAIL_STATUS  0x27
-#define EMC2305_REG_DEVICE             0xfd
 #define EMC2305_REG_VENDOR             0xfe
 #define EMC2305_FAN_MAX                        0xff
 #define EMC2305_FAN_MIN                        0x00
 #define EMC2305_FAN_MAX_STATE          10
-#define EMC2305_DEVICE                 0x34
 #define EMC2305_VENDOR                 0x5d
 #define EMC2305_REG_PRODUCT_ID         0xfd
 #define EMC2305_TACH_REGS_UNUSE_BITS   3
-#define EMC2305_TACH_CNT_MULTIPLIER    0x02
+#define EMC2305_TACH_CNT_MULTIPLIER    1       /* Set by FAN_CFG RNGx bits */
 #define EMC2305_PWM_MAX                        5
 #define EMC2305_PWM_CHNL_CMN           0
 
@@ -42,6 +42,7 @@ emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d, I2C_CLIENT_EN
 #define EMC2305_RPM_FACTOR             3932160
 
 #define EMC2305_REG_FAN_DRIVE(n) (0x30 + 0x10 * (n))
+#define EMC2305_REG_FAN_CFG(n) (0x32 + 0x10 * (n))
 #define EMC2305_REG_FAN_MIN_DRIVE(n) (0x38 + 0x10 * (n))
 #define EMC2305_REG_FAN_TACH(n) (0x3e + 0x10 * (n))
 
@@ -62,7 +63,8 @@ static const struct i2c_device_id emc2305_ids[] = {
 MODULE_DEVICE_TABLE(i2c, emc2305_ids);
 
 static const struct of_device_id emc2305_dt_ids[] = {
-       { .compatible = "smsc,emc2305" },
+       { .compatible = "microchip,emc2305" },
+       { .compatible = "microchip,emc2301" },
        { }
 };
 MODULE_DEVICE_TABLE(of, emc2305_dt_ids);
@@ -187,7 +189,7 @@ static int emc2305_set_cur_state(struct thermal_cooling_device *cdev, unsigned l
        struct emc2305_data *data = cdev->devdata;
        struct i2c_client *client = data->client;
        u8 val, i;
-       
+
        if (state > data->max_state)
                return -EINVAL;
 
@@ -200,19 +202,20 @@ static int emc2305_set_cur_state(struct thermal_cooling_device *cdev, unsigned l
        state = max_t(unsigned long, state, data->cdev_data[cdev_idx].last_hwmon_state);
 
        val = EMC2305_PWM_STATE2DUTY(state, data->max_state, data->pwm_max);
+
        if (val > EMC2305_FAN_MAX)
                return -EINVAL;
 
        data->cdev_data[cdev_idx].cur_state = state;
        if (data->pwm_channel == EMC2305_PWM_CHNL_CMN)
-       /* Set the same PWM value in all channels 
+       /* Set the same PWM value in all channels
         * if common PWM channel is used.
         */
                for (i = 0; i < data->pwm_num; i++)
                        i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_DRIVE(i), val);
        else
                i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_DRIVE(cdev_idx), val);
-       
+
        return 0;
 }
 
@@ -274,7 +277,7 @@ static int emc2305_get_tz_of(struct device *dev)
        struct emc2305_data *data = dev_get_drvdata(dev);
        int ret = 0;
 
-       /* OF parameters are optional - overwrite default setting 
+       /* OF parameters are optional - overwrite default setting
         * if some of them are provided.
         */
 
@@ -296,8 +299,8 @@ static int emc2305_get_tz_of(struct device *dev)
                        return ret;
        }
 
-       /* Not defined or 0 means one thermal zone over all colling devices. 
-        * Otherwise - separted thermal zones for each PWM channel.
+       /* Not defined or 0 means one thermal zone over all cooling devices.
+        * Otherwise - separated thermal zones for each PWM channel.
         */
        if (of_find_property(np, "emc2305,pwm-channel", NULL)) {
                ret = of_property_read_u8(np, "emc2305,pwm-channel", &data->pwm_channel);
@@ -313,7 +316,7 @@ static int emc2305_set_single_tz(struct device *dev, int idx)
        struct emc2305_data *data = dev_get_drvdata(dev);
        long pwm = data->pwm_max;
        int cdev_idx;
-       
+
        cdev_idx = (idx) ? idx - 1 : 0;
 
        if (dev->of_node)
@@ -323,7 +326,7 @@ static int emc2305_set_single_tz(struct device *dev, int idx)
                                                                &emc2305_cooling_ops);
        else
                data->cdev_data[cdev_idx].cdev =
-                       thermal_cooling_device_register(emc2305_fan_name[idx], data, 
+                       thermal_cooling_device_register(emc2305_fan_name[idx], data,
                                                        &emc2305_cooling_ops);
 
        if (IS_ERR(data->cdev_data[cdev_idx].cdev)) {
@@ -339,6 +342,22 @@ static int emc2305_set_single_tz(struct device *dev, int idx)
        return 0;
 }
 
+static void emc2305_unset_tz(struct device *dev)
+{
+       struct emc2305_data *data = dev_get_drvdata(dev);
+       int i;
+
+       /* Unregister cooling device in case they have been registered by
+        * thermal_cooling_device_unregister(). No need for clean-up flow in case they
+        * have been registered by devm_thermal_of_cooling_device_register()
+        */
+       if (!dev->of_node) {
+               for (i = 0; i < EMC2305_PWM_MAX; i++)
+                       if (data->cdev_data[i].cdev)
+                               thermal_cooling_device_unregister(data->cdev_data[i].cdev);
+       }
+}
+
 static int emc2305_set_tz(struct device *dev)
 {
        struct emc2305_data *data = dev_get_drvdata(dev);
@@ -359,22 +378,6 @@ thermal_cooling_device_register_fail:
        return ret;
 }
 
-static void emc2305_unset_tz(struct device *dev)
-{
-       struct emc2305_data *data = dev_get_drvdata(dev);
-       int i;
-
-       /* Unregister cooling device in case they have been registered by
-        * thermal_cooling_device_unregister(). No need for clean-up flow in case they 
-        * have been registered by devm_thermal_of_cooling_device_register()
-        */
-       if (!dev->of_node) {
-               for (i = 0; i < EMC2305_PWM_MAX; i++)
-                       if (data->cdev_data[i].cdev)
-                               thermal_cooling_device_unregister(data->cdev_data[i].cdev);
-       }
-}
-
 static umode_t
 emc2305_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel)
 {
@@ -548,7 +551,7 @@ static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
        struct i2c_adapter *adapter = client->adapter;
        struct device *dev = &client->dev;
        struct emc2305_data *data;
-       int vendor, device;
+       int vendor;
        int ret;
        int i;
 
@@ -559,10 +562,6 @@ static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
        if (vendor != EMC2305_VENDOR)
                return -ENODEV;
 
-       device = i2c_smbus_read_byte_data(client, EMC2305_REG_DEVICE);
-       if (device != EMC2305_DEVICE)
-               return -ENODEV;
-
        data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
        if (!data)
                return -ENOMEM;
@@ -595,8 +594,23 @@ static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
                        return ret;
        }
 
-       for (i = 0; i < data->pwm_num; i++)
-               i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_MIN_DRIVE(i), data->pwm_min);
+       for (i = 0; i < data->pwm_num; i++) {
+               u8 val;
+
+               i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_MIN_DRIVE(i),
+                                         data->pwm_min);
+
+               val = i2c_smbus_read_byte_data(client, EMC2305_REG_FAN_CFG(i));
+               /* Set RNGx so minRPM=500 */
+               val &= ~0x60;
+               i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_CFG(i), val);
+       }
+
+       /* Acknowledge any existing faults. Stops the device responding on the
+        * SMBus alert address.
+        */
+       i2c_smbus_read_byte_data(client, EMC2305_REG_FAN_STALL_STATUS);
+       i2c_smbus_read_byte_data(client, EMC2305_REG_FAN_STATUS);
 
        return 0;
 }