From 50fe870273543a821f25e7f78ea7389183d819e7 Mon Sep 17 00:00:00 2001
From: Phil Elwell <phil@raspberrypi.com>
Date: Thu, 5 May 2022 15:46:07 +0100
Subject: [PATCH] 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>
---
 drivers/hwmon/emc2305.c | 86 ++++++++++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 36 deletions(-)

--- 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, 0x2
 #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 emc230
 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
 	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
 	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 devi
 	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 devi
 			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
 	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
 								&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
 	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_clie
 	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_clie
 	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_clie
 			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;
 }
