Skip to contentSkip to conversation

Add support for EMC2305 Fan Speed Controller.

PATCH Superseded [email protected] 3 participants 14 messages Apr 30, 2022 – May 24, 2022

Subsystems (3)

v1 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

cover Add support for EMC2305 Fan Speed Controller.

From: Michael Shych <[email protected]>

Introduce EMC2305 RPM-based PWM Fan Speed Controller
The EMC2305 is an SMBus compliant fan controller with up to five controlled PWM fan drivers. All fan drivers are controlled by a programmable frequency PWM driver and Fan Speed Control algorithm that operates as a directly PWM-controlled device.

The closed-loop Fan Speed Control algorithm (FSC) has the capability to detect aging fans and alert the system. It will likewise detect stalled or locked fans and trigger an interrupt.

EMC2305 offers a clock output so that multiple devices may be chained and slaved to the same clock source for optimal performance in large distributed systems.

Patch set includes:
Patch#1 - add support for EMC2301/2/3/5 RPM-based PWM Fan Speed Controller. Patch#2 - add microchip,emc2306.yaml
Patch#3 - add emc2305.rst into docs.

Michael Shych (3):
hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based PWM Fan Speed
Controller.
dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.
docs: hwmon: add emc2305.rst to docs

 .../bindings/hwmon/microchip,emc2305.yaml          |  55 ++
 Documentation/hwmon/emc2305.rst                    |  40 ++
 drivers/hwmon/Kconfig                              |  13 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/emc2305.c                            | 629 +++++++++++++++++++++
 5 files changed, 738 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
 create mode 100644 Documentation/hwmon/emc2305.rst
 create mode 100644 drivers/hwmon/emc2305.c
Signature · 1 line
2.14.1
#1 lore raw

PATCH 1/3 hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based PWM Fan Speed Controller.

From: Michael Shych <[email protected]>

Add driver for Microchip EMC2301/2/3/5 RPM-based PWM Fan Speed Controller. Modify Makefile and Kconfig to support Microchip EMC2305 RPM-based PWM Fan Speed Controller.

---
 drivers/hwmon/Kconfig   |  13 +
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/emc2305.c | 629 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 643 insertions(+)
 create mode 100644 drivers/hwmon/emc2305.c
drivers/hwmon/Kconfig +13 -0
index 85c22bba439b..3c25ba0e6ef7 100644
@@ -1750,6 +1750,19 @@ config SENSORS_EMC2103
 	  This driver can also be built as a module. If so, the module
 	  will be called emc2103.
 
+config SENSORS_EMC2305
+	tristate "Microchip EMC2305 and compatible EMC2301/2/3"
+	depends on I2C
+	imply THERMAL
+	help
+	  If you say yes here you get support for the Microchip EMC2305
+	  fan controller chips.
+	  The Microchip EMC2305 is a fan controller for up to 5 fans.
+	  Fan rotation speeds are reported in RPM.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called emc2305.
+
 config SENSORS_EMC6W201
 	tristate "SMSC EMC6W201"
 	depends on I2C
drivers/hwmon/Makefile +1 -0
index 93f2b774cc5e..e2238c207ef4 100644
@@ -70,6 +70,7 @@ obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EMC1403)	+= emc1403.o
 obj-$(CONFIG_SENSORS_EMC2103)	+= emc2103.o
+obj-$(CONFIG_SENSORS_EMC2305)	+= emc2305.o
 obj-$(CONFIG_SENSORS_EMC6W201)	+= emc6w201.o
 obj-$(CONFIG_SENSORS_F71805F)	+= f71805f.o
 obj-$(CONFIG_SENSORS_F71882FG)	+= f71882fg.o
drivers/hwmon/emc2305.c new file +629 -0

629 lines not on this page · raw message

Signature · 1 line
2.14.1
maintainer
#2 lore raw

Re: [PATCH v1 1/3] hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based PWM Fan Speed Controller.

Quoted 57 lines · On Sat, Apr 30, 2022 at 02:49:03PM +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Add driver for Microchip EMC2301/2/3/5 RPM-based PWM Fan Speed Controller. Modify Makefile and Kconfig to support Microchip EMC2305 RPM-based PWM Fan Speed Controller.

---
 drivers/hwmon/Kconfig   |  13 +
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/emc2305.c | 629 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 643 insertions(+)
 create mode 100644 drivers/hwmon/emc2305.c
drivers/hwmon/Kconfig +13 -0
index 85c22bba439b..3c25ba0e6ef7 100644
@@ -1750,6 +1750,19 @@ config SENSORS_EMC2103
 	  This driver can also be built as a module. If so, the module
 	  will be called emc2103.
 
+config SENSORS_EMC2305
+	tristate "Microchip EMC2305 and compatible EMC2301/2/3"
+	depends on I2C
+	imply THERMAL
+	help
+	  If you say yes here you get support for the Microchip EMC2305
+	  fan controller chips.
+	  The Microchip EMC2305 is a fan controller for up to 5 fans.
+	  Fan rotation speeds are reported in RPM.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called emc2305.
+
 config SENSORS_EMC6W201
 	tristate "SMSC EMC6W201"
 	depends on I2C
drivers/hwmon/Makefile +1 -0
index 93f2b774cc5e..e2238c207ef4 100644
@@ -70,6 +70,7 @@ obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EMC1403)	+= emc1403.o
 obj-$(CONFIG_SENSORS_EMC2103)	+= emc2103.o
+obj-$(CONFIG_SENSORS_EMC2305)	+= emc2305.o
 obj-$(CONFIG_SENSORS_EMC6W201)	+= emc6w201.o
 obj-$(CONFIG_SENSORS_F71805F)	+= f71805f.o
 obj-$(CONFIG_SENSORS_F71882FG)	+= f71882fg.o
drivers/hwmon/emc2305.c new file +0 -0
new file mode 100644
index 000000000000..5c896fdfc525

drivers/hwmon/emc2305.c line 10

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
@@ -0,0 +1,629 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for EMC2305 fan controller
+ *
+ * Copyright (C) 2022 Nvidia Technologies Ltd and Delta Networks, Inc.
+ */
+
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>

Not necessary

Quoted 21 lines
drivers/hwmon/emc2305.c new file +21 -0
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/thermal.h>
+#include <linux/version.h>
+
+static const unsigned short
+emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d, I2C_CLIENT_END };
+
+#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_PWM_MAX			5

drivers/hwmon/emc2305.c line 41

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+#define EMC2305_PWM_CHNL_CMN		0
+
+#define EMC2305_PWM_DUTY2STATE(duty, max_state, pwm_max) \
+	(DIV_ROUND_CLOSEST((duty) * (max_state), (pwm_max)))
+#define EMC2305_PWM_STATE2DUTY(state, max_state, pwm_max) \
+	(DIV_ROUND_CLOSEST((state) * (pwm_max), (max_state)))
+
+/* Factor by equations [2] and [3] from data sheet; valid for fans where the number of edges
+ * equal (poles * 2 + 1).
+ */

/*
* Please use standard multi-line comments.
*/

drivers/hwmon/emc2305.c line 46

Quoted 5 lines
drivers/hwmon/emc2305.c new file +5 -0
+#define EMC2305_RPM_FACTOR		3932160
+
+#define EMC2305_REG_FAN_DRIVE(n) (0x30 + 0x10 * (n))
+#define EMC2305_REG_FAN_MIN_DRIVE(n) (0x38 + 0x10 * (n))
+#define EMC2305_REG_FAN_TACH(n) (0x3e + 0x10 * (n))

Please use

#define<space>NAME<tab>value

Quoted 74 lines
drivers/hwmon/emc2305.c new file +74 -0
+
+enum emc230x_product_id {
+	EMC2305 = 0x34,
+	EMC2303 = 0x35,
+	EMC2302 = 0x36,
+	EMC2301 = 0x37,
+};
+
+static const struct i2c_device_id emc2305_ids[] = {
+	{ "emc2305", 0 },
+	{ "emc2303", 0 },
+	{ "emc2302", 0 },
+	{ "emc2301", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, emc2305_ids);
+
+static const struct of_device_id emc2305_dt_ids[] = {
+	{ .compatible = "smsc,emc2305" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, emc2305_dt_ids);
+
+/**
+ * @cdev: cooling device;
+ * @curr_state: cooling current state;
+ * @last_hwmon_state: last cooling state updated by hwmon subsystem;
+ * @last_thermal_state: last cooling state updated by thermal subsystem;
+ *
+ * The 'last_hwmon_state' and 'last_thermal_state' fields are provided to support fan low limit
+ * speed feature. The purpose of this feature is to provides ability to limit fan speed
+ * according to some system wise considerations, like absence of some replaceable units (PSU or
+ * line cards), high system ambient temperature, unreliable transceivers temperature sensing or
+ * some other factors which indirectly impacts system's airflow
+ * Fan low limit feature is supported through 'hwmon' interface: 'hwmon' 'pwm' attribute is
+ * used for setting low limit for fan speed in case 'thermal' subsystem is configured in
+ * kernel. In this case setting fan speed through 'hwmon' will never let the 'thermal'
+ * subsystem to select a lower duty cycle than the duty cycle selected with the 'pwm'
+ * attribute.
+ * From other side, fan speed is to be updated in hardware through 'pwm' only in case the
+ * requested fan speed is above last speed set by 'thermal' subsystem, otherwise requested fan
+ * speed will be just stored with no PWM update.
+ */
+struct emc2305_cdev_data {
+	struct thermal_cooling_device *cdev;
+	unsigned int cur_state;
+	unsigned long last_hwmon_state;
+	unsigned long last_thermal_state;
+};
+
+/**
+ * @client: i2c client;
+ * @hwmon_dev: hwmon device;
+ * @max_state: maximum cooling state of the cooling device;
+ * @pwm_max: maximum PWM;
+ * @pwm_min: minimum PWM;
+ * @pwm_channel: maximum number of PWM channels;
+ * @cdev_data: array of cooling devices data;
+ */
+struct emc2305_data {
+	struct i2c_client *client;
+	struct device *hwmon_dev;
+	u8 max_state;
+	u8 pwm_max;
+	u8 pwm_min;
+	u8 pwm_num;
+	u8 pwm_channel;
+	struct emc2305_cdev_data cdev_data[EMC2305_PWM_MAX];
+};
+
+static char *emc2305_fan_name[] = {
+	"emc2305_fan",
+	"emc2305_fan1",
+	"emc2305_fan2",

drivers/hwmon/emc2305.c line 130

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+	"emc2305_fan3",
+	"emc2305_fan4",
+	"emc2305_fan5",
+};
+
+static int emc2305_get_max_channel(struct emc2305_data *data)
+{
+	if (data->pwm_channel == EMC2305_PWM_CHNL_CMN)
+		return data->pwm_num;
+	else

else after return is unnecessary

Quoted 159 lines
drivers/hwmon/emc2305.c new file +159 -0
+		return data->pwm_channel;
+}
+
+static int emc2305_get_cdev_idx(struct thermal_cooling_device *cdev)
+{
+	struct emc2305_data *data = cdev->devdata;
+	size_t len = strlen(cdev->type);
+	int ret;
+
+	if (len <= 0)
+		return -EINVAL;
+
+	/* Retuns index of cooling device 0..4 in case of separate PWM setting.
+	 * Zero index is used in case of one common PWM setting.
+	 * If the mode is set as EMC2305_PWM_CHNL_CMN, all PWMs are to be bound
+	 * to the common thermal zone and should work at the same speed
+	 * to perform cooling for the same thermal junction.
+	 * Otherwise, return specific channel that will be used in bound
+	 * related PWM to the thermal zone.
+	 */
+	if (data->pwm_channel == EMC2305_PWM_CHNL_CMN)
+		return 0;
+
+	ret = cdev->type[len - 1];
+	switch (ret) {
+	case '1' ... '5':
+		return ret - '1';
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static int emc2305_get_cur_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+	int cdev_idx;
+	struct emc2305_data *data = cdev->devdata;
+
+	cdev_idx = emc2305_get_cdev_idx(cdev);
+	if (cdev_idx < 0)
+		return cdev_idx;
+
+	*state = data->cdev_data[cdev_idx].cur_state;
+	return 0;
+}
+
+static int emc2305_get_max_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+	struct emc2305_data *data = cdev->devdata;
+	*state = data->max_state;
+	return 0;
+}
+
+static int emc2305_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
+{
+	int cdev_idx;
+	struct emc2305_data *data = cdev->devdata;
+	struct i2c_client *client = data->client;
+	u8 val, i;
+	
+	if (state > data->max_state)
+		return -EINVAL;
+
+	cdev_idx =  emc2305_get_cdev_idx(cdev);
+	if (cdev_idx < 0)
+		return cdev_idx;
+
+	/* Save thermal state. */
+	data->cdev_data[cdev_idx].last_thermal_state = state;
+	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 
+	 * 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;
+}
+
+static const struct thermal_cooling_device_ops emc2305_cooling_ops = {
+	.get_max_state = emc2305_get_max_state,
+	.get_cur_state = emc2305_get_cur_state,
+	.set_cur_state = emc2305_set_cur_state,
+};
+
+static int emc2305_show_fault(struct device *dev, int channel)
+{
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	int status_reg;
+
+	status_reg = i2c_smbus_read_byte_data(client, EMC2305_REG_DRIVE_FAIL_STATUS);
+	return status_reg & (1 << channel) ? 1 : 0;
+}
+
+static int emc2305_show_fan(struct device *dev, int channel)
+{
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	int ret;
+
+	ret = i2c_smbus_read_word_swapped(client, EMC2305_REG_FAN_TACH(channel));
+	if (ret < 0)
+		return ret;
+
+	ret = ret >> EMC2305_TACH_REGS_UNUSE_BITS;
+	return EMC2305_RPM_FACTOR * EMC2305_TACH_CNT_MULTIPLIER / (ret > 0 ? ret : 1);
+}
+
+static int emc2305_show_pwm(struct device *dev, int channel)
+{
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+
+	return i2c_smbus_read_byte_data(client, EMC2305_REG_FAN_DRIVE(channel));
+}
+
+static int emc2305_set_pwm(struct device *dev, long val, int channel)
+{
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+
+	if (val < data->pwm_min || val > data->pwm_max)
+		return -EINVAL;
+
+	i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_DRIVE(channel), val);
+	data->cdev_data[channel].cur_state = EMC2305_PWM_DUTY2STATE(val, data->max_state,
+								    data->pwm_max);
+	return 0;
+}
+
+static int emc2305_get_tz_of(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	int ret = 0;
+
+	/* OF parameters are optional - overwrite default setting 
+	 * if some of them are provided.
+	 */
+
+	if (of_find_property(np, "emc2305,cooling-levels", NULL)) {
+		ret = of_property_read_u8(np, "emc2305,cooling-levels", &data->max_state);
+		if (ret)
+			return ret;
+	}
+
+	if (of_find_property(np, "emc2305,pwm-max", NULL)) {
+		ret = of_property_read_u8(np, "emc2305,pwm-max", &data->pwm_max);
+		if (ret)

drivers/hwmon/emc2305.c line 299

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+			return ret;
+	}
+
+	if (of_find_property(np, "emc2305,pwm-min", NULL)) {
+		ret = of_property_read_u8(np, "emc2305,pwm-min", &data->pwm_min);
+		if (ret)
+			return ret;
+	}
+
+	/* Not defined or 0 means one thermal zone over all colling devices. 

cooling

drivers/hwmon/emc2305.c line 300

Quoted 1 line
drivers/hwmon/emc2305.c new file +1 -0
+	 * Otherwise - separted thermal zones for each PWM channel.

separate ?

drivers/hwmon/emc2305.c line 306

Quoted 6 lines
drivers/hwmon/emc2305.c new file +6 -0
+	 */
+	if (of_find_property(np, "emc2305,pwm-channel", NULL)) {
+		ret = of_property_read_u8(np, "emc2305,pwm-channel", &data->pwm_channel);
+		if (ret)
+			return ret;
+	}

I think those values should be validated.

Quoted 65 lines
drivers/hwmon/emc2305.c new file +65 -0
+
+	return ret;
+}
+
+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)
+		data->cdev_data[cdev_idx].cdev =
+			devm_thermal_of_cooling_device_register(dev, dev->of_node,
+								emc2305_fan_name[idx], data,
+								&emc2305_cooling_ops);
+	else
+		data->cdev_data[cdev_idx].cdev =
+			thermal_cooling_device_register(emc2305_fan_name[idx], data, 
+							&emc2305_cooling_ops);
+
+	if (IS_ERR(data->cdev_data[cdev_idx].cdev)) {
+		dev_err(dev, "Failed to register cooling device %s\n", emc2305_fan_name[idx]);
+		return PTR_ERR(data->cdev_data[cdev_idx].cdev);
+	}
+	emc2305_set_pwm(dev, pwm, cdev_idx);
+	data->cdev_data[cdev_idx].cur_state = data->max_state;
+	/* Set minimal PWM speed. */
+	data->cdev_data[cdev_idx].last_hwmon_state = EMC2305_PWM_DUTY2STATE(data->pwm_min,
+									    data->max_state,
+									    data->pwm_max);
+	return 0;
+}
+
+static int emc2305_set_tz(struct device *dev)
+{
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	int i, ret;
+
+	if (data->pwm_channel == EMC2305_PWM_CHNL_CMN)
+		return emc2305_set_single_tz(dev, 0);
+
+	for (i = 0; i < data->pwm_channel; i++) {
+		ret = emc2305_set_single_tz(dev, i + 1);
+		if (ret)
+			goto thermal_cooling_device_register_fail;
+	}
+	return 0;
+
+thermal_cooling_device_register_fail:
+	emc2305_unset_tz(dev);
+	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) {

drivers/hwmon/emc2305.c line 381

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+		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)
+{
+	int max_channel = emc2305_get_max_channel((struct emc2305_data *)data);

Unnecessary typecast.

drivers/hwmon/emc2305.c line 384

Quoted 3 lines
drivers/hwmon/emc2305.c new file +3 -0
+
+	/* Don't show channels which are not physically connected. */
+	if ((channel + 1) > max_channel)

Unnecessary ()

Quoted 215 lines
drivers/hwmon/emc2305.c new file +215 -0
+		return 0;
+	switch (type) {
+	case hwmon_fan:
+		switch (attr) {
+		case hwmon_fan_input:
+			return 0444;
+		case hwmon_fan_fault:
+			return 0444;
+		default:
+			break;
+		}
+		break;
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			return 0644;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+};
+
+static int
+emc2305_write(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel, long val)
+{
+	struct emc2305_data *data = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			/* If thermal is configured - handle PWM limit setting. */
+			if (IS_REACHABLE(CONFIG_THERMAL)) {
+				data->cdev_data[channel].last_hwmon_state =
+					EMC2305_PWM_DUTY2STATE(val, data->max_state, data->pwm_max);
+				/* Update PWM only in case requested state is not less than the
+				 * last thermal state.
+				 */
+				if (data->cdev_data[channel].last_hwmon_state >=
+				    data->cdev_data[channel].last_thermal_state)
+					return emc2305_set_cur_state(data->cdev_data[channel].cdev,
+							data->cdev_data[channel].last_hwmon_state);
+				return 0;
+			}
+			return emc2305_set_pwm(dev, val, channel);
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+};
+
+static int
+emc2305_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel, long *val)
+{
+	int ret;
+
+	switch (type) {
+	case hwmon_fan:
+		switch (attr) {
+		case hwmon_fan_input:
+			ret = emc2305_show_fan(dev, channel);
+			if (ret < 0)
+				return ret;
+			*val = ret;
+			return 0;
+		case hwmon_fan_fault:
+			ret = emc2305_show_fault(dev, channel);
+			if (ret < 0)
+				return ret;
+			*val = ret;
+			return 0;
+		default:
+			break;
+		}
+		break;
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			ret = emc2305_show_pwm(dev, channel);
+			if (ret < 0)
+				return ret;
+			*val = ret;
+			return 0;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+};
+
+static const struct hwmon_ops emc2305_ops = {
+	.is_visible = emc2305_is_visible,
+	.read = emc2305_read,
+	.write = emc2305_write,
+};
+
+static const struct hwmon_channel_info *emc2305_info[] = {
+	HWMON_CHANNEL_INFO(fan,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT),
+	HWMON_CHANNEL_INFO(pwm,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT),
+	NULL
+};
+
+static const struct hwmon_chip_info emc2305_chip_info = {
+	.ops = &emc2305_ops,
+	.info = emc2305_info,
+};
+
+static int emc2305_identify(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct emc2305_data *data = i2c_get_clientdata(client);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client, EMC2305_REG_PRODUCT_ID);
+	if (ret < 0)
+		return ret;
+
+	switch (ret) {
+	case EMC2305:
+		data->pwm_num = 5;
+		break;
+	case EMC2303:
+		data->pwm_num = 3;
+		break;
+	case EMC2302:
+		data->pwm_num = 2;
+		break;
+	case EMC2301:
+		data->pwm_num = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adapter = client->adapter;
+	struct device *dev = &client->dev;
+	struct emc2305_data *data;
+	int vendor, device;
+	int ret;
+	int i;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
+		return -ENODEV;
+
+	vendor = i2c_smbus_read_byte_data(client, EMC2305_REG_VENDOR);
+	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;
+
+	i2c_set_clientdata(client, data);
+	data->client = client;
+
+	ret = emc2305_identify(dev);
+	if (ret)
+		return ret;
+
+	data->max_state = EMC2305_FAN_MAX_STATE;
+	data->pwm_max = EMC2305_FAN_MAX;
+	data->pwm_min = EMC2305_FAN_MIN;
+	data->pwm_channel = EMC2305_PWM_CHNL_CMN;
+	if (dev->of_node) {
+		ret = emc2305_get_tz_of(dev);
+		if (ret < 0)
+			return ret;
+	}
+
+	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "emc2305", data,
+							       &emc2305_chip_info, NULL);
+	if (IS_ERR(data->hwmon_dev))
+		return PTR_ERR(data->hwmon_dev);
+
+	if (IS_REACHABLE(CONFIG_THERMAL)) {
+		ret = emc2305_set_tz(dev);
+		if (ret != 0)
+			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);

drivers/hwmon/emc2305.c line 609

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+
+	return 0;
+}
+
+static int emc2305_remove(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+
+	if (IS_REACHABLE(CONFIG_THERMAL))
+		emc2305_unset_tz(dev);

It would be desirable to handle this with devm_add_action_or_reset() from emc2305_set_tz().

Quoted 23 lines
drivers/hwmon/emc2305.c new file +20 -0
+	return 0;
+}
+
+static struct i2c_driver emc2305_driver = {
+	.class  = I2C_CLASS_HWMON,
+	.driver = {
+		.name = "emc2305",
+		.of_match_table = emc2305_dt_ids,
+	},
+	.probe    = emc2305_probe,
+	.remove	  = emc2305_remove,
+	.id_table = emc2305_ids,
+	.address_list = emc2305_normal_i2c,
+};
+
+module_i2c_driver(emc2305_driver);
+
+MODULE_AUTHOR("Nvidia");
+MODULE_DESCRIPTION("Microchip EMC2305 fan controller driver");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.14.1
#3 lore raw

RE: [PATCH v1 1/3] hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based PWM Fan Speed Controller.

Quoted 75 lines

-----Original Message-----
From: Guenter Roeck <[email protected]> On Behalf Of Guenter Roeck
Sent: Saturday, April 30, 2022 3:57 PM
To: Michael Shych <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Vadim Pasternak <[email protected]>
Subject: Re: [PATCH v1 1/3] hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based PWM Fan Speed Controller.

Quoted 3 lines · On Sat, Apr 30, 2022 at 02:49:03PM +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Add driver for Microchip EMC2301/2/3/5 RPM-based PWM Fan Speed

Controller.

Quoted 1 line

Modify Makefile and Kconfig to support Microchip EMC2305 RPM-based

PWM

Quoted 57 lines

Fan Speed Controller.

---
 drivers/hwmon/Kconfig   |  13 +
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/emc2305.c | 629
++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 643 insertions(+)
 create mode 100644 drivers/hwmon/emc2305.c
drivers/hwmon/Kconfig index renamed from drivers/hwmon/Kconfig +0 -0

85c22bba439b..3c25ba0e6ef7 100644

drivers/hwmon/Kconfig +13 -0
@@ -1750,6 +1750,19 @@ config SENSORS_EMC2103
 	  This driver can also be built as a module. If so, the module
 	  will be called emc2103.
 
+config SENSORS_EMC2305
+	tristate "Microchip EMC2305 and compatible EMC2301/2/3"
+	depends on I2C
+	imply THERMAL
+	help
+	  If you say yes here you get support for the Microchip EMC2305
+	  fan controller chips.
+	  The Microchip EMC2305 is a fan controller for up to 5 fans.
+	  Fan rotation speeds are reported in RPM.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called emc2305.
+
 config SENSORS_EMC6W201
 	tristate "SMSC EMC6W201"
 	depends on I2C
drivers/hwmon/Makefile index renamed from drivers/hwmon/Makefile +0 -0

93f2b774cc5e..e2238c207ef4 100644

drivers/hwmon/Makefile +1 -0
@@ -70,6 +70,7 @@ obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EMC1403)	+= emc1403.o
 obj-$(CONFIG_SENSORS_EMC2103)	+= emc2103.o
+obj-$(CONFIG_SENSORS_EMC2305)	+= emc2305.o
 obj-$(CONFIG_SENSORS_EMC6W201)	+= emc6w201.o
 obj-$(CONFIG_SENSORS_F71805F)	+= f71805f.o
 obj-$(CONFIG_SENSORS_F71882FG)	+= f71882fg.o
drivers/hwmon/emc2305.c new renamed from drivers/hwmon/emc2305.c +0 -0

file mode 100644 index 000000000000..5c896fdfc525

drivers/hwmon/emc2305.c new file +4 -0
@@ -0,0 +1,629 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for EMC2305 fan controller
+ *
Quoted 8 lines

drivers/hwmon/emc2305.c line 10

Quoted 6 lines
drivers/hwmon/emc2305.c new file +6 -0
+ * Copyright (C) 2022 Nvidia Technologies Ltd and Delta Networks, Inc.
+ */
+
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>

Not necessary

Removed.

Quoted 30 lines
Quoted 30 lines
drivers/hwmon/emc2305.c new file +29 -0
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/thermal.h>
+#include <linux/version.h>
+
+static const unsigned short
+emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d,
+I2C_CLIENT_END };
+
+#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_PWM_MAX			5
+#define EMC2305_PWM_CHNL_CMN		0
+
+#define EMC2305_PWM_DUTY2STATE(duty, max_state, pwm_max) \
+	(DIV_ROUND_CLOSEST((duty) * (max_state), (pwm_max))) #define
+EMC2305_PWM_STATE2DUTY(state, max_state, pwm_max) \
+	(DIV_ROUND_CLOSEST((state) * (pwm_max), (max_state)))
+
Quoted 8 lines

drivers/hwmon/emc2305.c line 41

Quoted 4 lines
drivers/hwmon/emc2305.c new file +4 -0
+/* Factor by equations [2] and [3] from data sheet; valid for fans
+where the number of edges
+ * equal (poles * 2 + 1).
+ */

/*
* Please use standard multi-line comments.
*/

Ack.

Quoted 11 lines

drivers/hwmon/emc2305.c line 48

Quoted 6 lines
drivers/hwmon/emc2305.c new file +5 -0
+#define EMC2305_RPM_FACTOR		3932160
+
+#define EMC2305_REG_FAN_DRIVE(n) (0x30 + 0x10 * (n)) #define
+EMC2305_REG_FAN_MIN_DRIVE(n) (0x38 + 0x10 * (n)) #define
+EMC2305_REG_FAN_TACH(n) (0x3e + 0x10 * (n))

Please use

#define<space>NAME<tab>value

Ack.

Quoted 86 lines
Quoted 34 lines
drivers/hwmon/emc2305.c new file +34 -0
+
+enum emc230x_product_id {
+	EMC2305 = 0x34,
+	EMC2303 = 0x35,
+	EMC2302 = 0x36,
+	EMC2301 = 0x37,
+};
+
+static const struct i2c_device_id emc2305_ids[] = {
+	{ "emc2305", 0 },
+	{ "emc2303", 0 },
+	{ "emc2302", 0 },
+	{ "emc2301", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, emc2305_ids);
+
+static const struct of_device_id emc2305_dt_ids[] = {
+	{ .compatible = "smsc,emc2305" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, emc2305_dt_ids);
+
+/**
+ * @cdev: cooling device;
+ * @curr_state: cooling current state;
+ * @last_hwmon_state: last cooling state updated by hwmon subsystem;
+ * @last_thermal_state: last cooling state updated by thermal
+subsystem;
+ *
+ * The 'last_hwmon_state' and 'last_thermal_state' fields are
+provided to support fan low limit
+ * speed feature. The purpose of this feature is to provides ability
+to limit fan speed

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+ * according to some system wise considerations, like absence of some
+replaceable units (PSU or
+ * line cards), high system ambient temperature, unreliable
+transceivers temperature sensing or
+ * some other factors which indirectly impacts system's airflow
+ * Fan low limit feature is supported through 'hwmon' interface:
+'hwmon' 'pwm' attribute is
+ * used for setting low limit for fan speed in case 'thermal'
+subsystem is configured in
+ * kernel. In this case setting fan speed through 'hwmon' will never let the

'thermal'

drivers/hwmon/emc2305.c line 93

Quoted 1 line
drivers/hwmon/emc2305.c new file +1 -0
+ * subsystem to select a lower duty cycle than the duty cycle selected with

the 'pwm'

Quoted 39 lines
drivers/hwmon/emc2305.c new file +39 -0
+ * attribute.
+ * From other side, fan speed is to be updated in hardware through
+'pwm' only in case the
+ * requested fan speed is above last speed set by 'thermal'
+subsystem, otherwise requested fan
+ * speed will be just stored with no PWM update.
+ */
+struct emc2305_cdev_data {
+	struct thermal_cooling_device *cdev;
+	unsigned int cur_state;
+	unsigned long last_hwmon_state;
+	unsigned long last_thermal_state;
+};
+
+/**
+ * @client: i2c client;
+ * @hwmon_dev: hwmon device;
+ * @max_state: maximum cooling state of the cooling device;
+ * @pwm_max: maximum PWM;
+ * @pwm_min: minimum PWM;
+ * @pwm_channel: maximum number of PWM channels;
+ * @cdev_data: array of cooling devices data;  */ struct emc2305_data
+{
+	struct i2c_client *client;
+	struct device *hwmon_dev;
+	u8 max_state;
+	u8 pwm_max;
+	u8 pwm_min;
+	u8 pwm_num;
+	u8 pwm_channel;
+	struct emc2305_cdev_data cdev_data[EMC2305_PWM_MAX]; };
+
+static char *emc2305_fan_name[] = {
+	"emc2305_fan",
+	"emc2305_fan1",
+	"emc2305_fan2",
+	"emc2305_fan3",
+	"emc2305_fan4",
+	"emc2305_fan5",
Quoted 8 lines

drivers/hwmon/emc2305.c line 130

Quoted 6 lines
drivers/hwmon/emc2305.c new file +6 -0
+};
+
+static int emc2305_get_max_channel(struct emc2305_data *data) {
+	if (data->pwm_channel == EMC2305_PWM_CHNL_CMN)
+		return data->pwm_num;
+	else

else after return is unnecessary

Changed.

Quoted 174 lines
Quoted 4 lines
drivers/hwmon/emc2305.c new file +3 -0
+		return data->pwm_channel;
+}
+

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+static int emc2305_get_cdev_idx(struct thermal_cooling_device *cdev)
+{
+	struct emc2305_data *data = cdev->devdata;
+	size_t len = strlen(cdev->type);
+	int ret;
+
+	if (len <= 0)
+		return -EINVAL;
+
+	/* Retuns index of cooling device 0..4 in case of separate PWM

setting.

drivers/hwmon/emc2305.c line 153

Quoted 2 lines
drivers/hwmon/emc2305.c new file +2 -0
+	 * Zero index is used in case of one common PWM setting.
+	 * If the mode is set as EMC2305_PWM_CHNL_CMN, all PWMs are to

be bound

Quoted 59 lines
drivers/hwmon/emc2305.c new file +58 -0
+	 * to the common thermal zone and should work at the same speed
+	 * to perform cooling for the same thermal junction.
+	 * Otherwise, return specific channel that will be used in bound
+	 * related PWM to the thermal zone.
+	 */
+	if (data->pwm_channel == EMC2305_PWM_CHNL_CMN)
+		return 0;
+
+	ret = cdev->type[len - 1];
+	switch (ret) {
+	case '1' ... '5':
+		return ret - '1';
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static int emc2305_get_cur_state(struct thermal_cooling_device *cdev,
+unsigned long *state) {
+	int cdev_idx;
+	struct emc2305_data *data = cdev->devdata;
+
+	cdev_idx = emc2305_get_cdev_idx(cdev);
+	if (cdev_idx < 0)
+		return cdev_idx;
+
+	*state = data->cdev_data[cdev_idx].cur_state;
+	return 0;
+}
+
+static int emc2305_get_max_state(struct thermal_cooling_device *cdev,
+unsigned long *state) {
+	struct emc2305_data *data = cdev->devdata;
+	*state = data->max_state;
+	return 0;
+}
+
+static int emc2305_set_cur_state(struct thermal_cooling_device *cdev,
+unsigned long state) {
+	int cdev_idx;
+	struct emc2305_data *data = cdev->devdata;
+	struct i2c_client *client = data->client;
+	u8 val, i;
+
+	if (state > data->max_state)
+		return -EINVAL;
+
+	cdev_idx =  emc2305_get_cdev_idx(cdev);
+	if (cdev_idx < 0)
+		return cdev_idx;
+
+	/* Save thermal state. */
+	data->cdev_data[cdev_idx].last_thermal_state = state;
+	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);

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+	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
+	 * 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);

drivers/hwmon/emc2305.c

Quoted 2 lines
drivers/hwmon/emc2305.c new file +2 -0
+	else
+		i2c_smbus_write_byte_data(client,

EMC2305_REG_FAN_DRIVE(cdev_idx),

Quoted 6 lines
drivers/hwmon/emc2305.c new file +6 -0
+val);
+
+	return 0;
+}
+
+static const struct thermal_cooling_device_ops emc2305_cooling_ops = {

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+	.get_max_state = emc2305_get_max_state,
+	.get_cur_state = emc2305_get_cur_state,
+	.set_cur_state = emc2305_set_cur_state, };
+
+static int emc2305_show_fault(struct device *dev, int channel) {
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	int status_reg;
+
+	status_reg = i2c_smbus_read_byte_data(client,

EMC2305_REG_DRIVE_FAIL_STATUS);

drivers/hwmon/emc2305.c

Quoted 8 lines
drivers/hwmon/emc2305.c new file +8 -0
+	return status_reg & (1 << channel) ? 1 : 0; }
+
+static int emc2305_show_fan(struct device *dev, int channel) {
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	int ret;
+
+	ret = i2c_smbus_read_word_swapped(client,

EMC2305_REG_FAN_TACH(channel));

drivers/hwmon/emc2305.c

Quoted 5 lines
drivers/hwmon/emc2305.c new file +5 -0
+	if (ret < 0)
+		return ret;
+
+	ret = ret >> EMC2305_TACH_REGS_UNUSE_BITS;
+	return EMC2305_RPM_FACTOR * EMC2305_TACH_CNT_MULTIPLIER

/ (ret > 0 ?

Quoted 8 lines
drivers/hwmon/emc2305.c new file +8 -0
+ret : 1); }
+
+static int emc2305_show_pwm(struct device *dev, int channel) {
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+
+	return i2c_smbus_read_byte_data(client,
+EMC2305_REG_FAN_DRIVE(channel)); }

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+
+static int emc2305_set_pwm(struct device *dev, long val, int channel)
+{
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+
+	if (val < data->pwm_min || val > data->pwm_max)
+		return -EINVAL;
+
+	i2c_smbus_write_byte_data(client,

EMC2305_REG_FAN_DRIVE(channel), val);

drivers/hwmon/emc2305.c

Quoted 1 line
drivers/hwmon/emc2305.c new file +1 -0
+	data->cdev_data[channel].cur_state =

EMC2305_PWM_DUTY2STATE(val, data->max_state,

Quoted 6 lines
drivers/hwmon/emc2305.c new file +1 -0
+								    data-

pwm_max);

drivers/hwmon/emc2305.c new file +4 -0
+	return 0;
+}
+
+static int emc2305_get_tz_of(struct device *dev) {

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+	struct device_node *np = dev->of_node;
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	int ret = 0;
+
+	/* OF parameters are optional - overwrite default setting
+	 * if some of them are provided.
+	 */
+
+	if (of_find_property(np, "emc2305,cooling-levels", NULL)) {
+		ret = of_property_read_u8(np, "emc2305,cooling-levels",

&data->max_state);

Quoted 13 lines
drivers/hwmon/emc2305.c new file +6 -0
+		if (ret)
+			return ret;
+	}
+
+	if (of_find_property(np, "emc2305,pwm-max", NULL)) {
+		ret = of_property_read_u8(np, "emc2305,pwm-max", &data-

pwm_max);

drivers/hwmon/emc2305.c new file +6 -0
+		if (ret)
+			return ret;
+	}
+
+	if (of_find_property(np, "emc2305,pwm-min", NULL)) {
+		ret = of_property_read_u8(np, "emc2305,pwm-min", &data-
Quoted 8 lines

drivers/hwmon/emc2305.c line 299

Quoted 6 lines

pwm_min);

drivers/hwmon/emc2305.c new file +5 -0
+		if (ret)
+			return ret;
+	}
+
+	/* Not defined or 0 means one thermal zone over all colling devices.

cooling

Changed.

Quoted 9 lines

drivers/hwmon/emc2305.c line 300

Quoted 2 lines
drivers/hwmon/emc2305.c new file +1 -0
+	 * Otherwise - separted thermal zones for each PWM channel.

separate ?

drivers/hwmon/emc2305.c

Quoted 3 lines
drivers/hwmon/emc2305.c new file +3 -0
+	 */
+	if (of_find_property(np, "emc2305,pwm-channel", NULL)) {
+		ret = of_property_read_u8(np, "emc2305,pwm-channel",

&data->pwm_channel);

Quoted 5 lines

drivers/hwmon/emc2305.c line 285

Quoted 3 lines
drivers/hwmon/emc2305.c new file +3 -0
+		if (ret)
+			return ret;
+	}

I think those values should be validated.

Checks are added.

Quoted 72 lines
Quoted 15 lines
drivers/hwmon/emc2305.c new file +14 -0
+
+	return ret;
+}
+
+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)
+		data->cdev_data[cdev_idx].cdev =
+			devm_thermal_of_cooling_device_register(dev, dev-
Quoted 2 lines

of_node,
+

	emc2305_fan_name[idx], data,
Quoted 1 line

+

	&emc2305_cooling_ops);

drivers/hwmon/emc2305.c line 328

Quoted 3 lines
drivers/hwmon/emc2305.c new file +3 -0
+	else
+		data->cdev_data[cdev_idx].cdev =
+

thermal_cooling_device_register(emc2305_fan_name[idx], data,

Quoted 1 line

+

	&emc2305_cooling_ops);

drivers/hwmon/emc2305.c

Quoted 3 lines
drivers/hwmon/emc2305.c new file +3 -0
+
+	if (IS_ERR(data->cdev_data[cdev_idx].cdev)) {
+		dev_err(dev, "Failed to register cooling device %s\n",

emc2305_fan_name[idx]);

drivers/hwmon/emc2305.c

Quoted 6 lines
drivers/hwmon/emc2305.c new file +6 -0
+		return PTR_ERR(data->cdev_data[cdev_idx].cdev);
+	}
+	emc2305_set_pwm(dev, pwm, cdev_idx);
+	data->cdev_data[cdev_idx].cur_state = data->max_state;
+	/* Set minimal PWM speed. */
+	data->cdev_data[cdev_idx].last_hwmon_state =

EMC2305_PWM_DUTY2STATE(data->pwm_min,

Quoted 1 line

+

data->max_state,

Quoted 1 line

+

data->pwm_max);

Quoted 18 lines
drivers/hwmon/emc2305.c new file +18 -0
+	return 0;
+}
+
+static int emc2305_set_tz(struct device *dev) {
+	struct emc2305_data *data = dev_get_drvdata(dev);
+	int i, ret;
+
+	if (data->pwm_channel == EMC2305_PWM_CHNL_CMN)
+		return emc2305_set_single_tz(dev, 0);
+
+	for (i = 0; i < data->pwm_channel; i++) {
+		ret = emc2305_set_single_tz(dev, i + 1);
+		if (ret)
+			goto thermal_cooling_device_register_fail;
+	}
+	return 0;
+
+thermal_cooling_device_register_fail:

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+	emc2305_unset_tz(dev);
+	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

drivers/hwmon/emc2305.c

Quoted 1 line
drivers/hwmon/emc2305.c new file +1 -0
+	 * have been registered by

devm_thermal_of_cooling_device_register()

Quoted 16 lines
Quoted 6 lines
drivers/hwmon/emc2305.c new file +5 -0
+	 */
+	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);

drivers/hwmon/emc2305.c

Quoted 8 lines
drivers/hwmon/emc2305.c new file +8 -0
+	}
+}
+
+static umode_t
+emc2305_is_visible(const void *data, enum hwmon_sensor_types type,
+u32 attr, int channel) {
+	int max_channel = emc2305_get_max_channel((struct emc2305_data
+*)data);

Unnecessary typecast.

It's required as otherwise there is compilation warning.

Quoted 7 lines

drivers/hwmon/emc2305.c line 384

Quoted 4 lines
drivers/hwmon/emc2305.c new file +3 -0
+
+	/* Don't show channels which are not physically connected. */
+	if ((channel + 1) > max_channel)

Unnecessary ()

Removed

Quoted 228 lines
Quoted 27 lines
drivers/hwmon/emc2305.c new file +27 -0
+		return 0;
+	switch (type) {
+	case hwmon_fan:
+		switch (attr) {
+		case hwmon_fan_input:
+			return 0444;
+		case hwmon_fan_fault:
+			return 0444;
+		default:
+			break;
+		}
+		break;
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			return 0644;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+};
+

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+static int
+emc2305_write(struct device *dev, enum hwmon_sensor_types type, u32
+attr, int channel, long val) {
+	struct emc2305_data *data = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			/* If thermal is configured - handle PWM limit setting.

*/

drivers/hwmon/emc2305.c

Quoted 2 lines
drivers/hwmon/emc2305.c new file +2 -0
+			if (IS_REACHABLE(CONFIG_THERMAL)) {
+				data->cdev_data[channel].last_hwmon_state

=

drivers/hwmon/emc2305.c

Quoted 1 line
drivers/hwmon/emc2305.c new file +1 -0
+					EMC2305_PWM_DUTY2STATE(val,

data->max_state, data->pwm_max);

drivers/hwmon/emc2305.c

Quoted 1 line
drivers/hwmon/emc2305.c new file +1 -0
+				/* Update PWM only in case requested state

is not less than the

Quoted 90 lines
drivers/hwmon/emc2305.c new file +3 -0
+				 * last thermal state.
+				 */
+				if (data-

cdev_data[channel].last_hwmon_state >=

drivers/hwmon/emc2305.c new file +1 -0
+				    data-

cdev_data[channel].last_thermal_state)

drivers/hwmon/emc2305.c new file +1 -0
+					return emc2305_set_cur_state(data-

cdev_data[channel].cdev,

drivers/hwmon/emc2305.c new file +1 -0
+							data-

cdev_data[channel].last_hwmon_state);

drivers/hwmon/emc2305.c new file +80 -0
+				return 0;
+			}
+			return emc2305_set_pwm(dev, val, channel);
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+};
+
+static int
+emc2305_read(struct device *dev, enum hwmon_sensor_types type, u32
+attr, int channel, long *val) {
+	int ret;
+
+	switch (type) {
+	case hwmon_fan:
+		switch (attr) {
+		case hwmon_fan_input:
+			ret = emc2305_show_fan(dev, channel);
+			if (ret < 0)
+				return ret;
+			*val = ret;
+			return 0;
+		case hwmon_fan_fault:
+			ret = emc2305_show_fault(dev, channel);
+			if (ret < 0)
+				return ret;
+			*val = ret;
+			return 0;
+		default:
+			break;
+		}
+		break;
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			ret = emc2305_show_pwm(dev, channel);
+			if (ret < 0)
+				return ret;
+			*val = ret;
+			return 0;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+};
+
+static const struct hwmon_ops emc2305_ops = {
+	.is_visible = emc2305_is_visible,
+	.read = emc2305_read,
+	.write = emc2305_write,
+};
+
+static const struct hwmon_channel_info *emc2305_info[] = {
+	HWMON_CHANNEL_INFO(fan,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT),
+	HWMON_CHANNEL_INFO(pwm,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT),
+	NULL
+};
+
+static const struct hwmon_chip_info emc2305_chip_info = {

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+	.ops = &emc2305_ops,
+	.info = emc2305_info,
+};
+
+static int emc2305_identify(struct device *dev) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct emc2305_data *data = i2c_get_clientdata(client);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client,

EMC2305_REG_PRODUCT_ID);

Quoted 23 lines
drivers/hwmon/emc2305.c new file +23 -0
+	if (ret < 0)
+		return ret;
+
+	switch (ret) {
+	case EMC2305:
+		data->pwm_num = 5;
+		break;
+	case EMC2303:
+		data->pwm_num = 3;
+		break;
+	case EMC2302:
+		data->pwm_num = 2;
+		break;
+	case EMC2301:
+		data->pwm_num = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+static int emc2305_probe(struct i2c_client *client, const struct
+i2c_device_id *id) {
+	struct i2c_adapter *adapter = client->adapter;
+	struct device *dev = &client->dev;
+	struct emc2305_data *data;
+	int vendor, device;
+	int ret;
+	int i;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |

I2C_FUNC_SMBUS_WORD_DATA))

drivers/hwmon/emc2305.c

Quoted 3 lines
drivers/hwmon/emc2305.c new file +3 -0
+		return -ENODEV;
+
+	vendor = i2c_smbus_read_byte_data(client,

EMC2305_REG_VENDOR);

Quoted 19 lines
drivers/hwmon/emc2305.c new file +19 -0
+	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;
+
+	i2c_set_clientdata(client, data);
+	data->client = client;
+
+	ret = emc2305_identify(dev);
+	if (ret)
+		return ret;
+
+	data->max_state = EMC2305_FAN_MAX_STATE;

drivers/hwmon/emc2305.c

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+	data->pwm_max = EMC2305_FAN_MAX;
+	data->pwm_min = EMC2305_FAN_MIN;
+	data->pwm_channel = EMC2305_PWM_CHNL_CMN;
+	if (dev->of_node) {
+		ret = emc2305_get_tz_of(dev);
+		if (ret < 0)
+			return ret;
+	}
+
+	data->hwmon_dev = devm_hwmon_device_register_with_info(dev,

"emc2305", data,

Quoted 1 line

+

&emc2305_chip_info, NULL);

drivers/hwmon/emc2305.c

Quoted 11 lines
drivers/hwmon/emc2305.c new file +11 -0
+	if (IS_ERR(data->hwmon_dev))
+		return PTR_ERR(data->hwmon_dev);
+
+	if (IS_REACHABLE(CONFIG_THERMAL)) {
+		ret = emc2305_set_tz(dev);
+		if (ret != 0)
+			return ret;
+	}
+
+	for (i = 0; i < data->pwm_num; i++)
+		i2c_smbus_write_byte_data(client,

EMC2305_REG_FAN_MIN_DRIVE(i),

Quoted 14 lines

drivers/hwmon/emc2305.c line 609

Quoted 10 lines
drivers/hwmon/emc2305.c new file +10 -0
+data->pwm_min);
+
+	return 0;
+}
+
+static int emc2305_remove(struct i2c_client *client) {
+	struct device *dev = &client->dev;
+
+	if (IS_REACHABLE(CONFIG_THERMAL))
+		emc2305_unset_tz(dev);

It would be desirable to handle this with devm_add_action_or_reset() from emc2305_set_tz().

It's not required in case of devm_thermal_of_cooling_device_register Added comment.

Quoted 15 lines
Quoted 15 lines
drivers/hwmon/emc2305.c new file +15 -0
+	return 0;
+}
+
+static struct i2c_driver emc2305_driver = {
+	.class  = I2C_CLASS_HWMON,
+	.driver = {
+		.name = "emc2305",
+		.of_match_table = emc2305_dt_ids,
+	},
+	.probe    = emc2305_probe,
+	.remove	  = emc2305_remove,
+	.id_table = emc2305_ids,
+	.address_list = emc2305_normal_i2c,
+};
+
Quoted 8 lines
Quoted 8 lines
drivers/hwmon/emc2305.c new file +5 -1
+module_i2c_driver(emc2305_driver);
+
+MODULE_AUTHOR("Nvidia");
+MODULE_DESCRIPTION("Microchip EMC2305 fan controller driver");
+MODULE_LICENSE("GPL");
--

2.14.1

Thank you,
Michael.

maintainer
#4 lore raw

Re: [PATCH v1 1/3] hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based PWM Fan Speed Controller.

Quoted 10 lines · Michael Shych
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +5 -0
+static umode_t
+emc2305_is_visible(const void *data, enum hwmon_sensor_types type,
+u32 attr, int channel) {
+	int max_channel = emc2305_get_max_channel((struct emc2305_data
+*)data);

Unnecessary typecast.

It's required as otherwise there is compilation warning.

Arguable. The critical part is the change from a const pointer to a non-const pointer, which is in general not a good idea. It doesn't matter here, but it would be better to declare the parameter of emc2305_get_max_channel to be a const *.

Guenter

#5 lore raw

PATCH 2/3 dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

In reply to [email protected]

From: Michael Shych <[email protected]>

Add basic description of emc2305 driver device tree binding.

---
 .../bindings/hwmon/microchip,emc2305.yaml          | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +55 -0
new file mode 100644
index 000000000000..c873172b7268
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/emc2305.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip EMC2305 RPM-based PWM Fan Speed Controller
+
+properties:
+  compatible:
+    enum:
+      - microcip,emc2305
+
+  emc2305,pwm-min:
+    description:
+      Min pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-max:
+    description:
+      Max pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-channel:
+    description:
+      Max number of pwm channels
+    maxItems: 1
+  emcs205,max-state:
+    description:
+    maxItems: 1
+  emc2305,cooling-levels:
+    description:
+      Quantity of cooling level state.
+    maxItems: 1
+
+required:
+  - compatible
+
+optional:
+  - emc2305,min-pwm
+  - emc2305,max-pwm
+  - emc2305,pwm-channels
+  - emc2305,cooling-levels
+
+additionalProperties: false
+
+examples:
+  - |
+    fan {
+        emc2305,compatible = "microchip,emc2305";
+        emc2305,pwm-min = <0>;
+        emc2305,pwm-max = <255>;
+        emc2305,pwm-channel = <5>
+        emc2305,cooling-levels = <10>;
+    };
+
Signature · 1 line
2.14.1
maintainer
#6 lore raw

Re: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

Quoted 33 lines · On Sat, Apr 30, 2022 at 02:49:04PM +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Add basic description of emc2305 driver device tree binding.

---
 .../bindings/hwmon/microchip,emc2305.yaml          | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +16 -0
new file mode 100644
index 000000000000..c873172b7268
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/emc2305.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip EMC2305 RPM-based PWM Fan Speed Controller
+
+properties:
+  compatible:
+    enum:
+      - microcip,emc2305
+
+  emc2305,pwm-min:
+    description:

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml line 26

Quoted 10 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +10 -0
+      Min pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-max:
+    description:
+      Max pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-channel:
+    description:
+      Max number of pwm channels
+    maxItems: 1

I think this needs to be explained further.

Quoted 32 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +29 -0
+  emcs205,max-state:
+    description:
+    maxItems: 1
+  emc2305,cooling-levels:
+    description:
+      Quantity of cooling level state.
+    maxItems: 1
+
+required:
+  - compatible
+
+optional:
+  - emc2305,min-pwm
+  - emc2305,max-pwm
+  - emc2305,pwm-channels
+  - emc2305,cooling-levels
+
+additionalProperties: false
+
+examples:
+  - |
+    fan {
+        emc2305,compatible = "microchip,emc2305";
+        emc2305,pwm-min = <0>;
+        emc2305,pwm-max = <255>;
+        emc2305,pwm-channel = <5>
+        emc2305,cooling-levels = <10>;
+    };
+
Signature · 1 line
2.14.1
maintainer
#7 lore raw

Re: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

In reply to [email protected]
Quoted 11 lines · On Sat, 30 Apr 2022 14:49:04 +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Add basic description of emc2305 driver device tree binding.

---
 .../bindings/hwmon/microchip,emc2305.yaml          | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml:28:17: [error] empty value in block mapping (empty-values)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emcs205,max-state:description: None is not of type 'string'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: 'optional' is not one of ['$id', '$schema', 'title', 'description', 'examples', 'required', 'allOf', 'anyOf', 'oneOf', 'definitions', '$defs', 'additionalProperties', 'dependencies', 'dependentRequired', 'dependentSchemas', 'patternProperties', 'properties', 'if', 'then', 'else', 'unevaluatedProperties', 'deprecated', 'maintainers', 'select', '$ref']
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: 'maintainers' is a required property
hint: Metaschema for devicetree binding documentation
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-channel: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('maxItems' was unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-channel: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-channel: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emcs205,max-state: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('maxItems' was unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emcs205,max-state: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emcs205,max-state: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,cooling-levels: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('maxItems' was unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,cooling-levels: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,cooling-levels: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-max: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('maxItems' was unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-max: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-max: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-min: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('maxItems' was unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-min: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: properties:emc2305,pwm-min: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
./Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml: ignoring, error in schema: properties: emcs205,max-state: description
Error: Documentation/devicetree/bindings/hwmon/microchip,emc2305.example.dts:25.13-14 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:364: Documentation/devicetree/bindings/hwmon/microchip,emc2305.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1401: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

maintainer
#8 lore raw

Re: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

In reply to [email protected]
Quoted 56 lines · On Sat, Apr 30, 2022 at 02:49:04PM +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Add basic description of emc2305 driver device tree binding.

---
 .../bindings/hwmon/microchip,emc2305.yaml          | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +39 -0
new file mode 100644
index 000000000000..c873172b7268
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/emc2305.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip EMC2305 RPM-based PWM Fan Speed Controller
+
+properties:
+  compatible:
+    enum:
+      - microcip,emc2305
+
+  emc2305,pwm-min:
+    description:
+      Min pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-max:
+    description:
+      Max pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-channel:
+    description:
+      Max number of pwm channels
+    maxItems: 1
+  emcs205,max-state:
+    description:
+    maxItems: 1
+  emc2305,cooling-levels:
+    description:
+      Quantity of cooling level state.
+    maxItems: 1
+
+required:
+  - compatible
+
+optional:
+  - emc2305,min-pwm

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml line 49

Quoted 10 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +10 -0
+  - emc2305,max-pwm
+  - emc2305,pwm-channels
+  - emc2305,cooling-levels
+
+additionalProperties: false
+
+examples:
+  - |
+    fan {
+        emc2305,compatible = "microchip,emc2305";

Err, what?

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml line 53

Quoted 4 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +4 -0
+        emc2305,pwm-min = <0>;
+        emc2305,pwm-max = <255>;
+        emc2305,pwm-channel = <5>
+        emc2305,cooling-levels = <10>;

All possible fans attached to this controller are the same and don't have anything that needs to be described? Based on other fan controllers, I don't think so. As I've said multiple times, there's a need for a common fan and fan-controller binding. Until that happens, I'm not inclined to accept fan controller bindings with custom properties.

Rob

#9 lore raw

RE: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

Hi Rob,

I fixed all dt binding check errors and changed the patch subject name
that you pointed out.
However, it's not clear completely your comment in this reply.
emc2305 device support 5 pwms.
The intention was to specify a simple configuration per pwm{n} - to allow setting per pwm{n} minimum and maximum duty cycle, cooling level stepping (10%, 5%, 1%).

Is there some way to provide such a configuration? Do you think that the following example is OK?

fan {
    microchip,compatible = "microchip,emc2305";
    microchip,pwm-channel = <5>;
    microchip,cooling-levels = <10>;
	pwm1 {
		microchip,pwm-min = <0>;
		microchip,pwm-max = <255>;
	};
	pwm2 {
		microchip,pwm-min = <0>;
		microchip,pwm-max = <255>;
	};
	...
};

Regards,
Michael.

Quoted 95 lines

-----Original Message-----
From: Rob Herring <[email protected]>
Sent: Monday, May 2, 2022 9:17 PM
To: Michael Shych <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Vadim Pasternak <[email protected]>
Subject: Re: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

Quoted 8 lines · On Sat, Apr 30, 2022 at 02:49:04PM +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Add basic description of emc2305 driver device tree binding.

---
 .../bindings/hwmon/microchip,emc2305.yaml          | 55

++++++++++++++++++++++

Quoted 11 lines
 1 file changed, 55 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file mode 100644
index 000000000000..c873172b7268
--- /dev/null
+++

b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml

Quoted 39 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +38 -0
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/emc2305.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip EMC2305 RPM-based PWM Fan Speed Controller
+
+properties:
+  compatible:
+    enum:
+      - microcip,emc2305
+
+  emc2305,pwm-min:
+    description:
+      Min pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-max:
+    description:
+      Max pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-channel:
+    description:
+      Max number of pwm channels
+    maxItems: 1
+  emcs205,max-state:
+    description:
+    maxItems: 1
+  emc2305,cooling-levels:
+    description:
+      Quantity of cooling level state.
+    maxItems: 1
+
+required:
+  - compatible
+
+optional:
+  - emc2305,min-pwm

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml line 48

Quoted 10 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +10 -0
+  - emc2305,max-pwm
+  - emc2305,pwm-channels
+  - emc2305,cooling-levels
+
+additionalProperties: false
+
+examples:
+  - |
+    fan {
+        emc2305,compatible = "microchip,emc2305";

Err, what?

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml line 53

Quoted 4 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +4 -0
+        emc2305,pwm-min = <0>;
+        emc2305,pwm-max = <255>;
+        emc2305,pwm-channel = <5>
+        emc2305,cooling-levels = <10>;

All possible fans attached to this controller are the same and don't have anything that needs to be described? Based on other fan controllers, I don't think so. As I've said multiple times, there's a need for a common fan and fan-controller binding. Until that happens, I'm not inclined to accept fan controller bindings with custom properties.

Rob

#10 lore raw

RE: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

Hi Rob,

I'm resending v2 patch set with mentioned below changes.

Thank you,
Michael.

Quoted 137 lines

-----Original Message-----
From: Michael Shych
Sent: Sunday, May 15, 2022 8:15 PM
To: Rob Herring <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Vadim Pasternak <[email protected]>
Subject: RE: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

Hi Rob,

I fixed all dt binding check errors and changed the patch subject name that you pointed out.
However, it's not clear completely your comment in this reply. emc2305 device support 5 pwms.
The intention was to specify a simple configuration per pwm{n} - to allow setting per pwm{n} minimum and maximum duty cycle, cooling level stepping (10%, 5%, 1%).

Is there some way to provide such a configuration? Do you think that the following example is OK?

fan {
    microchip,compatible = "microchip,emc2305";
    microchip,pwm-channel = <5>;
    microchip,cooling-levels = <10>;
	pwm1 {
		microchip,pwm-min = <0>;
		microchip,pwm-max = <255>;
	};
	pwm2 {
		microchip,pwm-min = <0>;
		microchip,pwm-max = <255>;
	};
	...
};

Regards,
Michael.

Quoted 95 lines

-----Original Message-----
From: Rob Herring <[email protected]>
Sent: Monday, May 2, 2022 9:17 PM
To: Michael Shych <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Vadim Pasternak <[email protected]>
Subject: Re: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

Quoted 8 lines · On Sat, Apr 30, 2022 at 02:49:04PM +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Add basic description of emc2305 driver device tree binding.

---
 .../bindings/hwmon/microchip,emc2305.yaml          | 55

++++++++++++++++++++++

Quoted 11 lines
 1 file changed, 55 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file mode 100644
index 000000000000..c873172b7268
--- /dev/null
+++

b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml

Quoted 39 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +38 -0
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/emc2305.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip EMC2305 RPM-based PWM Fan Speed Controller
+
+properties:
+  compatible:
+    enum:
+      - microcip,emc2305
+
+  emc2305,pwm-min:
+    description:
+      Min pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-max:
+    description:
+      Max pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-channel:
+    description:
+      Max number of pwm channels
+    maxItems: 1
+  emcs205,max-state:
+    description:
+    maxItems: 1
+  emc2305,cooling-levels:
+    description:
+      Quantity of cooling level state.
+    maxItems: 1
+
+required:
+  - compatible
+
+optional:
+  - emc2305,min-pwm

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml line 48

Quoted 10 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +10 -0
+  - emc2305,max-pwm
+  - emc2305,pwm-channels
+  - emc2305,cooling-levels
+
+additionalProperties: false
+
+examples:
+  - |
+    fan {
+        emc2305,compatible = "microchip,emc2305";

Err, what?

Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml line 53

Quoted 4 lines
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +4 -0
+        emc2305,pwm-min = <0>;
+        emc2305,pwm-max = <255>;
+        emc2305,pwm-channel = <5>
+        emc2305,cooling-levels = <10>;

All possible fans attached to this controller are the same and don't have anything that needs to be described? Based on other fan controllers, I don't think so. As I've said multiple times, there's a need for a common fan and fan-controller binding. Until that happens, I'm not inclined to accept fan controller bindings with custom properties.

Rob

maintainer
#11 lore raw

Re: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

In reply to [email protected]
Quoted 1 line · On Sat, Apr 30, 2022 at 02:49:04PM +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Also, for the subject no need for 'dt binding description.', you already said that with 'dt-bindings'. So:

dt-bindings: hwmon: Add Microchip EMC2305 fan controller

Quoted 75 lines

Add basic description of emc2305 driver device tree binding.

---
 .../bindings/hwmon/microchip,emc2305.yaml          | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file +55 -0
new file mode 100644
index 000000000000..c873172b7268
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/emc2305.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip EMC2305 RPM-based PWM Fan Speed Controller
+
+properties:
+  compatible:
+    enum:
+      - microcip,emc2305
+
+  emc2305,pwm-min:
+    description:
+      Min pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-max:
+    description:
+      Max pwm of emc2305
+    maxItems: 1
+  emc2305,pwm-channel:
+    description:
+      Max number of pwm channels
+    maxItems: 1
+  emcs205,max-state:
+    description:
+    maxItems: 1
+  emc2305,cooling-levels:
+    description:
+      Quantity of cooling level state.
+    maxItems: 1
+
+required:
+  - compatible
+
+optional:
+  - emc2305,min-pwm
+  - emc2305,max-pwm
+  - emc2305,pwm-channels
+  - emc2305,cooling-levels
+
+additionalProperties: false
+
+examples:
+  - |
+    fan {
+        emc2305,compatible = "microchip,emc2305";
+        emc2305,pwm-min = <0>;
+        emc2305,pwm-max = <255>;
+        emc2305,pwm-channel = <5>
+        emc2305,cooling-levels = <10>;
+    };
+
Signature · 1 line
2.14.1
#12 lore raw

PATCH 3/3 docs: hwmon: add emc2305.rst to docs

In reply to [email protected]

From: Michael Shych <[email protected]>

Add description of emc2305 driver.

---
 Documentation/hwmon/emc2305.rst | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/hwmon/emc2305.rst
Documentation/hwmon/emc2305.rst new file +40 -0
new file mode 100644
index 000000000000..258da49d18f9
@@ -0,0 +1,40 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver emc2305
+=====================
+
+Supported chips:
+   Microchip EMC2305, EMC2303, EMC2302, EMC2301
+
+   Addresses scanned: I2C 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d
+   Prefixes: 'emc2305'
+
+   Datasheet: Publicly available at the Microchip website :
+      https://www.microchip.com/en-us/product/EMC2305
+
+Description:
+-----------
+This driver implements support for Microchip EMC2301/2/3/5 RPM-based PWM Fan Controller.
+The EMC2305 Fan Controller supports up to 5 independently controlled PWM fan drives.
+Fan rotation speeds are reported in RPM.
+The driver supports the RPM-based PWM control to keep a fan at the desired speed.
+The driver provides the possibility to have one common PWM interface for all FANs
+or up to the maximum available or configured independent PWMs.
+
+The driver provides the following sysfs interfaces in hwmon subsystem:
+
+================= == ===================================================
+fan[1-5]_fault    RO files for tachometers TACH1-TACH5 fault indication
+fan[1-5]_input    RO files for tachometers TACH1-TACH5 input (in RPM)
+pwm[1-5]          RW file for fan[1-5] target duty cycle (0..255)
+================= == ===================================================
+
+sysfs interfaces in thermal subsystem:
+
+================= == ========================================================================
+cur_state         RW file for the current cooling state of the cooling device (0..max_state)
+max_state         RO file for the maximum cooling state of the cooling device
+================= == ========================================================================
+
+Configuration is possible via device tree:
+Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
Signature · 1 line
2.14.1
maintainer
#13 lore raw

Re: [PATCH v1 3/3] docs: hwmon: add emc2305.rst to docs

Quoted 8 lines · On Sat, Apr 30, 2022 at 02:49:05PM +0300, [email protected] wrote:

From: Michael Shych <[email protected]>

Add description of emc2305 driver.

---
 Documentation/hwmon/emc2305.rst | 40 ++++++++++++++++++++++++++++++++++++++++

Also add to index.rst.

Quoted 52 lines
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/hwmon/emc2305.rst
Documentation/hwmon/emc2305.rst new file +40 -0
new file mode 100644
index 000000000000..258da49d18f9
@@ -0,0 +1,40 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver emc2305
+=====================
+
+Supported chips:
+   Microchip EMC2305, EMC2303, EMC2302, EMC2301
+
+   Addresses scanned: I2C 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d
+   Prefixes: 'emc2305'
+
+   Datasheet: Publicly available at the Microchip website :
+      https://www.microchip.com/en-us/product/EMC2305
+
+Description:
+-----------
+This driver implements support for Microchip EMC2301/2/3/5 RPM-based PWM Fan Controller.
+The EMC2305 Fan Controller supports up to 5 independently controlled PWM fan drives.
+Fan rotation speeds are reported in RPM.
+The driver supports the RPM-based PWM control to keep a fan at the desired speed.
+The driver provides the possibility to have one common PWM interface for all FANs
+or up to the maximum available or configured independent PWMs.
+
+The driver provides the following sysfs interfaces in hwmon subsystem:
+
+================= == ===================================================
+fan[1-5]_fault    RO files for tachometers TACH1-TACH5 fault indication
+fan[1-5]_input    RO files for tachometers TACH1-TACH5 input (in RPM)
+pwm[1-5]          RW file for fan[1-5] target duty cycle (0..255)
+================= == ===================================================
+
+sysfs interfaces in thermal subsystem:
+
+================= == ========================================================================
+cur_state         RW file for the current cooling state of the cooling device (0..max_state)
+max_state         RO file for the maximum cooling state of the cooling device
+================= == ========================================================================
+
+Configuration is possible via device tree:
+Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
Signature · 1 line
2.14.1