[PATCH 0/3] hwmon: (ctf2301) Add support for CTF2301

[PATCH 0/3] hwmon: (ctf2301) Add support for CTF2301

[Troy Mitchell]

Sensylink CTF2301 is a system-level thermal management solution chip.

The CTF2301 is an I2C/SMBus compatible device featuring:
  - One local temperature sensor with ±0.5°C accuracy and 0.0625°C resolution.
  - One remote temperature sensor for external diode-connected transistors,
    offering ±1°C accuracy and 0.125°C resolution (temperature range: -40°C to +125°C).
  - An integrated PWM fan controller capable of operating in two modes:
      1. Direct-DCY: Open-loop direct duty cycle control.
      2. Auto-Temp: Closed-loop automatic fan speed control based on measured temperature.
  - A 1-channel fan speed monitor (TACH input) for RPM measurement.

Check CTF2301 datasheet for more details[1]

Link:
https://www.sensylink.com/upload/1/net.sensylink.portal/1689557281035.pdf[1]

Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
---
Troy Mitchell (3):
      dt-bindings: vendor-prefixes: Add Sensylink
      dt-bindings: Add CTF2301 devicetree bindings
      hwmon: (ctf2301) Add support for CTF2301

 .../bindings/hwmon/sensylink,ctf2301.yaml          |  49 ++++
 .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
 drivers/hwmon/Kconfig                              |  11 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/ctf2301.c                            | 326 +++++++++++++++++++++
 5 files changed, 389 insertions(+)
---
base-commit: 250a683466384b4d36f98b64f20412f3c26ca69e
change-id: 20250916-ctl2301-0416b073c280

Best regards,
-- 
Troy Mitchell <troy.mitchell@linux.dev>

[PATCH 1/3] dt-bindings: vendor-prefixes: Add Sensylink

[Troy Mitchell]

Link: https://www.sensylink.com/

Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 77160cd47f54079a39f35b570d69f7c4c2274724..ea4011d64ab9081d212a738839849d5814cf6c98 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1353,6 +1353,8 @@ patternProperties:
     description: Sensirion AG
   "^sensortek,.*":
     description: Sensortek Technology Corporation
+  "^sensylink,.*":
+    description: Sensylink Microelectronics Technology Co., Ltd.
   "^sercomm,.*":
     description: Sercomm (Suzhou) Corporation
   "^sff,.*":

-- 
2.51.0

Re: [PATCH 1/3] dt-bindings: vendor-prefixes: Add Sensylink

[Guenter Roeck]

On Tue, Sep 16, 2025 at 12:46:44PM +0800, Troy Mitchell wrote:
> Link: https://www.sensylink.com/
> 
This is not an appropriate patch description.

> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 77160cd47f54079a39f35b570d69f7c4c2274724..ea4011d64ab9081d212a738839849d5814cf6c98 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1353,6 +1353,8 @@ patternProperties:
>      description: Sensirion AG
>    "^sensortek,.*":
>      description: Sensortek Technology Corporation
> +  "^sensylink,.*":
> +    description: Sensylink Microelectronics Technology Co., Ltd.
>    "^sercomm,.*":
>      description: Sercomm (Suzhou) Corporation
>    "^sff,.*":

Re: [PATCH 1/3] dt-bindings: vendor-prefixes: Add Sensylink

[Troy Mitchell]

On Wed, Sep 24, 2025 at 07:41:44AM -0700, Guenter Roeck wrote:
> On Tue, Sep 16, 2025 at 12:46:44PM +0800, Troy Mitchell wrote:
> > Link: https://www.sensylink.com/
> > 
> This is not an appropriate patch description.
Okay, I'll add a company description.
Btw, I've seen other commits with the same info merged successfully. So
that's why I did that.

                - Troy
> 
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> > ---
> >  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index 77160cd47f54079a39f35b570d69f7c4c2274724..ea4011d64ab9081d212a738839849d5814cf6c98 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -1353,6 +1353,8 @@ patternProperties:
> >      description: Sensirion AG
> >    "^sensortek,.*":
> >      description: Sensortek Technology Corporation
> > +  "^sensylink,.*":
> > +    description: Sensylink Microelectronics Technology Co., Ltd.
> >    "^sercomm,.*":
> >      description: Sercomm (Suzhou) Corporation
> >    "^sff,.*":

[PATCH 2/3] dt-bindings: Add CTF2301 devicetree bindings

[Troy Mitchell]

Add dt-binding for the hwmon driver of Sensylink's CTF2301 chip.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
---
 .../bindings/hwmon/sensylink,ctf2301.yaml          | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml b/Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..fe98f5b578320bc1b43ff88f76667990821a88f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/sensylink,ctf2301.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sensylink CTF2301 system-level thermal management solution chip
+
+maintainers:
+  - Troy Mitchell <troy.mitchell@linux.dev>
+
+allOf:
+  - $ref: hwmon-common.yaml#
+
+description: |
+  The CTF2301B is an I2C/SMBus compatible device featuring:
+    - One local temperature sensor with ±0.5°C accuracy and 0.0625°C resolution.
+    - One remote temperature sensor for external diode-connected transistors, offering ±1°C accuracy and 0.125°C resolution (temperature range: -40°C to +125°C).
+    - An integrated PWM fan controller.
+    - A 1-channel fan speed monitor (TACH input) for RPM measurement.
+
+  Datasheets:
+    https://www.sensylink.com/upload/1/net.sensylink.portal/1689557281035.pdf
+
+properties:
+  compatible:
+    const:
+      - sensylink,ctf2301
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ctf2301@4c {
+            compatible = "sensylink,ctf2301";
+            reg = <0x4c>;
+        };
+    };

-- 
2.51.0

Re: [PATCH 2/3] dt-bindings: Add CTF2301 devicetree bindings

[Rob Herring (Arm)]

On Tue, 16 Sep 2025 12:46:45 +0800, Troy Mitchell wrote:
> Add dt-binding for the hwmon driver of Sensylink's CTF2301 chip.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> ---
>  .../bindings/hwmon/sensylink,ctf2301.yaml          | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml:18:111: [warning] line too long (161 > 110 characters) (line-length)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml: properties:compatible:const: ['sensylink,ctf2301'] is not of type 'integer', 'string'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml: properties:compatible:const: ['sensylink,ctf2301'] is not of type 'string'
	from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.example.dtb: /example-0/i2c/ctf2301@4c: failed to match any schema with compatible: ['sensylink,ctf2301']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250916-ctl2301-v1-2-97e7c84f2c47@linux.dev

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 2/3] dt-bindings: Add CTF2301 devicetree bindings

[Rob Herring]

On Tue, Sep 16, 2025 at 12:46:45PM +0800, Troy Mitchell wrote:
> Add dt-binding for the hwmon driver of Sensylink's CTF2301 chip.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> ---
>  .../bindings/hwmon/sensylink,ctf2301.yaml          | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml b/Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..fe98f5b578320bc1b43ff88f76667990821a88f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/sensylink,ctf2301.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sensylink CTF2301 system-level thermal management solution chip
> +
> +maintainers:
> +  - Troy Mitchell <troy.mitchell@linux.dev>
> +
> +allOf:
> +  - $ref: hwmon-common.yaml#
> +
> +description: |
> +  The CTF2301B is an I2C/SMBus compatible device featuring:
> +    - One local temperature sensor with ±0.5°C accuracy and 0.0625°C resolution.
> +    - One remote temperature sensor for external diode-connected transistors, offering ±1°C accuracy and 0.125°C resolution (temperature range: -40°C to +125°C).

Wrap at 80 chars.

> +    - An integrated PWM fan controller.
> +    - A 1-channel fan speed monitor (TACH input) for RPM measurement.
> +
> +  Datasheets:
> +    https://www.sensylink.com/upload/1/net.sensylink.portal/1689557281035.pdf
> +
> +properties:
> +  compatible:
> +    const:
> +      - sensylink,ctf2301
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ctf2301@4c {
> +            compatible = "sensylink,ctf2301";
> +            reg = <0x4c>;
> +        };
> +    };
> 
> -- 
> 2.51.0
> 

Re: [PATCH 2/3] dt-bindings: Add CTF2301 devicetree bindings

[Troy Mitchell]

Hi Rob, Thanks for your review.

On Tue, Sep 16, 2025 at 08:52:16AM -0500, Rob Herring wrote:
> On Tue, Sep 16, 2025 at 12:46:45PM +0800, Troy Mitchell wrote:
> > Add dt-binding for the hwmon driver of Sensylink's CTF2301 chip.
> > 
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> > ---
> >  .../bindings/hwmon/sensylink,ctf2301.yaml          | 49 ++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> > +description: |
> > +  The CTF2301B is an I2C/SMBus compatible device featuring:
> > +    - One local temperature sensor with ±0.5°C accuracy and 0.0625°C resolution.
> > +    - One remote temperature sensor for external diode-connected transistors, offering ±1°C accuracy and 0.125°C resolution (temperature range: -40°C to +125°C).
> 
> Wrap at 80 chars.
I'll fix it in the next version.
> 
> > +    - An integrated PWM fan controller.
> > +    - A 1-channel fan speed monitor (TACH input) for RPM measurement.
> > +
> > +  Datasheets:
> > +    https://www.sensylink.com/upload/1/net.sensylink.portal/1689557281035.pdf
> > +
> > +properties:
> > +  compatible:
> > +    const:
> > +      - sensylink,ctf2301
I will fix this warning in the next version (your robot reported)

                - Troy
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        ctf2301@4c {
> > +            compatible = "sensylink,ctf2301";
> > +            reg = <0x4c>;
> > +        };
> > +    };
> > 
> > -- 
> > 2.51.0
> > 

[PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301

[Troy Mitchell]

This commit introduces driver for the Sensylink CTF2301
system-level thermal management solution chip.

Currently, the driver does NOT support the Auto-Temp mode of the PWM
fan controller, which provides closed-loop automatic fan speed control
based on temperature.

Now this driver supports:
  - Reading local temperature.
  - Reading remote temperature.
  - Controlling the PWM fan output in Direct-DCY mode (direct duty cycle control).
  - Monitoring fan speed via the TACH input (RPM measurement).

Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
---
 drivers/hwmon/Kconfig   |  11 ++
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/ctf2301.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 338 insertions(+)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46..2120d891e549795c3f3416d08f71916af714f6b6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -537,6 +537,17 @@ config SENSORS_CROS_EC
 	  This driver can also be built as a module. If so, the module
 	  will be called cros_ec_hwmon.
 
+config SENSORS_CTF2301
+	tristate "Sensylink CTF2301"
+	depends on I2C
+	select REGMAP
+	help
+	  If you say yes here you get support for Sensylink CTF2301
+	  sensor chip.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called ctf2301.
+
 config SENSORS_DRIVETEMP
 	tristate "Hard disk drives with temperature sensors"
 	depends on SCSI && ATA
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index cd8bc4752b4dbf015c6eb46157626f4e8f87dfae..12f2894ce8d5fbfd942409f6c43d78fbdece57b4 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
 obj-$(CONFIG_SENSORS_CORSAIR_CPRO) += corsair-cpro.o
 obj-$(CONFIG_SENSORS_CORSAIR_PSU) += corsair-psu.o
 obj-$(CONFIG_SENSORS_CROS_EC)	+= cros_ec_hwmon.o
+obj-$(CONFIG_SENSORS_CTF2301)	+= ctf2301.o
 obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
 obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
 obj-$(CONFIG_SENSORS_DELL_SMM)	+= dell-smm-hwmon.o
diff --git a/drivers/hwmon/ctf2301.c b/drivers/hwmon/ctf2301.c
new file mode 100644
index 0000000000000000000000000000000000000000..2fea4d195519ea34c1d4bf67456098b225d4d13c
--- /dev/null
+++ b/drivers/hwmon/ctf2301.c
@@ -0,0 +1,326 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for CTF2301 system-level thermal management solution chip
+ * Datasheet: https://www.sensylink.com/upload/1/net.sensylink.portal/1689557281035.pdf
+ *
+ * Copyright (C) 2025 Troy Mitchell <troy.mitchell@linux.dev>
+ */
+
+#include <linux/hwmon.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+#define PWM_PARENT_CLOCK			360000
+
+#define CTF2301_LOCAL_TEMP_MSB			0x00
+#define CTF2301_RMT_TEMP_MSB			0x01
+#define CTF2301_ALERT_STATUS			0x02
+#define CTF2301_GLOBAL_CFG			0x03
+#define CTF2301_RMT_TEMP_LSB			0x10
+#define CTF2301_LOCAL_TEMP_LSB			0x15
+#define CTF2301_ALERT_MASK			0x16
+#define	CTF2301_ENHANCED_CFG			0x45
+#define CTF2301_TACH_COUNT_LSB			0x46
+#define CTF2301_TACH_COUNT_MSB			0x47
+#define CTF2301_PWM_AND_TACH_CFG		0x4a
+#define CTF2301_PWM_VALUE			0x4c
+#define CTF2301_PWM_FREQ			0x4d
+#define CTF2301_RMT_DIODE_TEMP_FILTER		0xbf
+
+/* remote diode fault alarm */
+#define ALERT_STATUS_RDFA			BIT(2)
+
+/* alert interrupts enable  */
+#define GLOBAL_CFG_ALERT_MASK			BIT(7)
+/* tach input enable  */
+#define GLOBAL_CFG_TACH_SEL			BIT(2)
+
+/* local high temperature alarm mask */
+#define ALERT_MASK_LHAM				BIT(6)
+/* remote high temperature alarm mask */
+#define ALERT_MASK_RHAM				BIT(4)
+/* remote low temperature alarm mask */
+#define ALERT_MASK_RLAM				BIT(3)
+/* remote t_crit alarm mask */
+#define ALERT_MASK_RCAM				BIT(1)
+/* tachometer alarm mask */
+#define ALERT_MASK_TCHAM			BIT(0)
+
+#define ALERT_MASK_ALL				(ALERT_MASK_LHAM | ALERT_MASK_RHAM | \
+						ALERT_MASK_RLAM | ALERT_MASK_RCAM | \
+						ALERT_MASK_TCHAM)
+
+/* enables signed format for high and t_crit setpoints */
+#define ENHANGCED_CFG_USF			BIT(3)
+
+/* PWM Programming enable */
+#define PWM_AND_TACH_CFG_PWPGM			BIT(5)
+
+#define PWM_DEFAULT_FREQ_CODE			0x17
+
+
+struct ctf2301 {
+	struct i2c_client *client;
+
+	struct regmap *regmap;
+
+	unsigned int pwm_freq_code;
+};
+
+static int ctf2301_read_temp(struct device *dev, u32 attr, int channel, long *val)
+{
+	int regval[2], raw, err, flag = 1, shift = 4, scale = 625;
+	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
+	unsigned int reg_msb = CTF2301_LOCAL_TEMP_MSB,
+		     reg_lsb = CTF2301_LOCAL_TEMP_LSB;
+
+	switch (attr) {
+	case hwmon_temp_input:
+		if (channel != 0 && channel != 1)
+			return -EOPNOTSUPP;
+
+		if (channel == 1) {
+			err = regmap_read(ctf2301->regmap, CTF2301_ALERT_STATUS, regval);
+			if (err)
+				return err;
+
+			if (regval[0] & ALERT_STATUS_RDFA)
+				return -ENODEV;
+
+			shift = 5;
+			scale = 1250;
+			reg_msb = CTF2301_RMT_TEMP_MSB;
+			reg_lsb = CTF2301_RMT_TEMP_LSB;
+		}
+
+		err = regmap_read(ctf2301->regmap, reg_msb, regval);
+		if (err)
+			return err;
+
+		err = regmap_read(ctf2301->regmap, reg_lsb, regval + 1);
+		if (err)
+			return err;
+
+		dev_err(dev, "local temp: lsb->0x%x, msb->0x%x", regval[1], regval[0]);
+
+		raw = (s16)((regval[0] << 8) | regval[1]);
+
+		raw >>= shift;
+
+		*val = raw * scale * flag;
+
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int ctf2301_read_fan(struct device *dev, u32 attr, long *val)
+{
+	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
+	int regval[2], err, speed;
+
+	switch (attr) {
+	case hwmon_fan_input:
+		err = regmap_read(ctf2301->regmap, CTF2301_TACH_COUNT_MSB, regval);
+		if (err)
+			return err;
+
+		err = regmap_read(ctf2301->regmap, CTF2301_TACH_COUNT_LSB, regval + 1);
+		if (err)
+			return err;
+
+		speed = (regval[0] << 8) | regval[1];
+
+		*val = (unsigned int)(1 * (5400000 / speed));
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int ctf2301_write_pwm(struct device *dev, u32 attr, long val)
+{
+	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
+	int err, map_val;
+
+	dev_err(dev, "write pwm: %d", attr);
+
+	switch (attr) {
+	case hwmon_pwm_input:
+		map_val = (val * ctf2301->pwm_freq_code * 2) / 255;
+		dev_err(dev, "val:%ld, map_val: %d", val, map_val);
+		err = regmap_write(ctf2301->regmap, CTF2301_PWM_VALUE, map_val);
+		if (err)
+			return err;
+		break;
+	case hwmon_pwm_freq:
+		ctf2301->pwm_freq_code = DIV_ROUND_UP(PWM_PARENT_CLOCK, val) / 2;
+		dev_err(dev, "pwm_freq_code: %d", ctf2301->pwm_freq_code);
+		err = regmap_write(ctf2301->regmap, CTF2301_PWM_FREQ, ctf2301->pwm_freq_code);
+		if (err)
+			return err;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static umode_t ctf2301_is_visible(const void *drvdata,
+				 enum hwmon_sensor_types type,
+				 u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_temp:
+		switch (attr) {
+		case hwmon_temp_input:
+			return 0444;
+		default:
+			return 0;
+		}
+	case hwmon_fan:
+		switch (attr) {
+		case hwmon_fan_input:
+			return 0444;
+		default:
+			return 0;
+		}
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+		case hwmon_pwm_freq:
+			return 0644;
+		default:
+			return 0;
+		}
+	default:
+		return 0;
+	}
+}
+
+static int ctf2301_read(struct device *dev, enum hwmon_sensor_types type,
+		       u32 attr, int channel, long *val)
+{
+	switch (type) {
+	case hwmon_temp:
+		return ctf2301_read_temp(dev, attr, channel, val);
+	case hwmon_fan:
+		return ctf2301_read_fan(dev, attr, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
+static int ctf2301_write(struct device *dev, enum hwmon_sensor_types type,
+			 u32 attr, int channel, long val)
+{
+	switch (type) {
+	case hwmon_pwm:
+		return ctf2301_write_pwm(dev, attr, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
+static const struct hwmon_channel_info * const ctf2301_info[] = {
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT),
+	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT | HWMON_PWM_FREQ),
+	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops ctf2301_hwmon_ops = {
+	.is_visible = ctf2301_is_visible,
+	.read = ctf2301_read,
+	.write = ctf2301_write
+};
+
+static const struct hwmon_chip_info ctf2301_chip_info = {
+	.ops = &ctf2301_hwmon_ops,
+	.info = ctf2301_info,
+};
+
+static const struct regmap_config ctf2301_regmap_config = {
+	.max_register = CTF2301_RMT_DIODE_TEMP_FILTER,
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int ctf2301_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct device *hwmon_dev;
+	struct ctf2301 *ctf2301;
+	int err;
+
+	ctf2301 = devm_kzalloc(dev, sizeof(*ctf2301), GFP_KERNEL);
+	if (!ctf2301)
+		return -ENOMEM;
+	ctf2301->client = client;
+
+	ctf2301->regmap = devm_regmap_init_i2c(client, &ctf2301_regmap_config);
+	if (IS_ERR(ctf2301->regmap))
+		return dev_err_probe(dev, PTR_ERR(ctf2301->regmap),
+				     "failed to allocate register map");
+
+	err = regmap_write(ctf2301->regmap, CTF2301_GLOBAL_CFG,
+			   GLOBAL_CFG_ALERT_MASK | GLOBAL_CFG_TACH_SEL);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "failed to write CTF2301_GLOBAL_CFG register");
+
+	/*err = regmap_write(ctf2301->regmap, CTF2301_ALERT_MASK, ALERT_MASK_ALL);*/
+	/*if (err)*/
+		/*return dev_err_probe(dev, err,*/
+				     /*"failed to write CTF2301_ALERT_MASK");*/
+
+	err = regmap_write(ctf2301->regmap, CTF2301_ENHANCED_CFG, ENHANGCED_CFG_USF);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "failed to write CTF2301_ENHANCED_CFG");
+
+	err = regmap_write(ctf2301->regmap, CTF2301_PWM_AND_TACH_CFG, PWM_AND_TACH_CFG_PWPGM);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "failed to write CTF2301_PWM_AND_TACH_CFG");
+
+	ctf2301->pwm_freq_code = PWM_DEFAULT_FREQ_CODE;
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, ctf2301,
+							 &ctf2301_chip_info,
+							 NULL);
+	if (IS_ERR(hwmon_dev))
+		return dev_err_probe(dev, PTR_ERR(hwmon_dev),
+				     "failed to register hwmon device");
+
+	return 0;
+}
+
+static const struct of_device_id ctf2301_of_match[] = {
+	{ .compatible = "sensylink,ctf2301", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ctf2301_of_match);
+
+static struct i2c_driver ctf2301_driver = {
+	.driver = {
+		.name	= "ctf2301",
+		.of_match_table = of_match_ptr(ctf2301_of_match),
+	},
+	.probe		= ctf2301_probe,
+};
+module_i2c_driver(ctf2301_driver);
+
+MODULE_AUTHOR("Troy Mitchell <troy.mitchell@linux.dev>");
+MODULE_DESCRIPTION("ctf2301 driver");
+MODULE_LICENSE("GPL");

-- 
2.51.0

Re: [PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301

[Troy Mitchell]

On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
> This commit introduces driver for the Sensylink CTF2301
> system-level thermal management solution chip.
> 
> Currently, the driver does NOT support the Auto-Temp mode of the PWM
> fan controller, which provides closed-loop automatic fan speed control
> based on temperature.
> 
> Now this driver supports:
>   - Reading local temperature.
>   - Reading remote temperature.
>   - Controlling the PWM fan output in Direct-DCY mode (direct duty cycle control).
>   - Monitoring fan speed via the TACH input (RPM measurement).
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> ---
>  drivers/hwmon/Kconfig   |  11 ++
>  drivers/hwmon/Makefile  |   1 +
>  drivers/hwmon/ctf2301.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 338 insertions(+)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46..2120d891e549795c3f3416d08f71916af714f6b6 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -537,6 +537,17 @@ config SENSORS_CROS_EC
>  	  This driver can also be built as a module. If so, the module
>  	  will be called cros_ec_hwmon.
>  
> +config SENSORS_CTF2301
> +	tristate "Sensylink CTF2301"
> +	depends on I2C
> +	select REGMAP
> +	help
> +	  If you say yes here you get support for Sensylink CTF2301
> +	  sensor chip.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called ctf2301.
> +
>  config SENSORS_DRIVETEMP
>  	tristate "Hard disk drives with temperature sensors"
>  	depends on SCSI && ATA
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index cd8bc4752b4dbf015c6eb46157626f4e8f87dfae..12f2894ce8d5fbfd942409f6c43d78fbdece57b4 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
>  obj-$(CONFIG_SENSORS_CORSAIR_CPRO) += corsair-cpro.o
>  obj-$(CONFIG_SENSORS_CORSAIR_PSU) += corsair-psu.o
>  obj-$(CONFIG_SENSORS_CROS_EC)	+= cros_ec_hwmon.o
> +obj-$(CONFIG_SENSORS_CTF2301)	+= ctf2301.o
>  obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
>  obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
>  obj-$(CONFIG_SENSORS_DELL_SMM)	+= dell-smm-hwmon.o
> diff --git a/drivers/hwmon/ctf2301.c b/drivers/hwmon/ctf2301.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2fea4d195519ea34c1d4bf67456098b225d4d13c
> --- /dev/null
> +++ b/drivers/hwmon/ctf2301.c
> @@ -0,0 +1,326 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for CTF2301 system-level thermal management solution chip
> + * Datasheet: https://www.sensylink.com/upload/1/net.sensylink.portal/1689557281035.pdf
> + *
> + * Copyright (C) 2025 Troy Mitchell <troy.mitchell@linux.dev>
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +#define PWM_PARENT_CLOCK			360000
> +
> +#define CTF2301_LOCAL_TEMP_MSB			0x00
> +#define CTF2301_RMT_TEMP_MSB			0x01
> +#define CTF2301_ALERT_STATUS			0x02
> +#define CTF2301_GLOBAL_CFG			0x03
> +#define CTF2301_RMT_TEMP_LSB			0x10
> +#define CTF2301_LOCAL_TEMP_LSB			0x15
> +#define CTF2301_ALERT_MASK			0x16
> +#define	CTF2301_ENHANCED_CFG			0x45
> +#define CTF2301_TACH_COUNT_LSB			0x46
> +#define CTF2301_TACH_COUNT_MSB			0x47
> +#define CTF2301_PWM_AND_TACH_CFG		0x4a
> +#define CTF2301_PWM_VALUE			0x4c
> +#define CTF2301_PWM_FREQ			0x4d
> +#define CTF2301_RMT_DIODE_TEMP_FILTER		0xbf
> +
> +/* remote diode fault alarm */
> +#define ALERT_STATUS_RDFA			BIT(2)
> +
> +/* alert interrupts enable  */
> +#define GLOBAL_CFG_ALERT_MASK			BIT(7)
> +/* tach input enable  */
> +#define GLOBAL_CFG_TACH_SEL			BIT(2)
> +
> +/* local high temperature alarm mask */
> +#define ALERT_MASK_LHAM				BIT(6)
> +/* remote high temperature alarm mask */
> +#define ALERT_MASK_RHAM				BIT(4)
> +/* remote low temperature alarm mask */
> +#define ALERT_MASK_RLAM				BIT(3)
> +/* remote t_crit alarm mask */
> +#define ALERT_MASK_RCAM				BIT(1)
> +/* tachometer alarm mask */
> +#define ALERT_MASK_TCHAM			BIT(0)
> +
> +#define ALERT_MASK_ALL				(ALERT_MASK_LHAM | ALERT_MASK_RHAM | \
> +						ALERT_MASK_RLAM | ALERT_MASK_RCAM | \
> +						ALERT_MASK_TCHAM)
> +
> +/* enables signed format for high and t_crit setpoints */
> +#define ENHANGCED_CFG_USF			BIT(3)
> +
> +/* PWM Programming enable */
> +#define PWM_AND_TACH_CFG_PWPGM			BIT(5)
> +
> +#define PWM_DEFAULT_FREQ_CODE			0x17
> +
> +
> +struct ctf2301 {
> +	struct i2c_client *client;
> +
> +	struct regmap *regmap;
> +
> +	unsigned int pwm_freq_code;
> +};
> +
> +static int ctf2301_read_temp(struct device *dev, u32 attr, int channel, long *val)
> +{
> +	int regval[2], raw, err, flag = 1, shift = 4, scale = 625;
> +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> +	unsigned int reg_msb = CTF2301_LOCAL_TEMP_MSB,
> +		     reg_lsb = CTF2301_LOCAL_TEMP_LSB;
> +
> +	switch (attr) {
> +	case hwmon_temp_input:
> +		if (channel != 0 && channel != 1)
> +			return -EOPNOTSUPP;
> +
> +		if (channel == 1) {
> +			err = regmap_read(ctf2301->regmap, CTF2301_ALERT_STATUS, regval);
> +			if (err)
> +				return err;
> +
> +			if (regval[0] & ALERT_STATUS_RDFA)
> +				return -ENODEV;
> +
> +			shift = 5;
> +			scale = 1250;
> +			reg_msb = CTF2301_RMT_TEMP_MSB;
> +			reg_lsb = CTF2301_RMT_TEMP_LSB;
> +		}
> +
> +		err = regmap_read(ctf2301->regmap, reg_msb, regval);
> +		if (err)
> +			return err;
> +
> +		err = regmap_read(ctf2301->regmap, reg_lsb, regval + 1);
> +		if (err)
> +			return err;
> +
> +		dev_err(dev, "local temp: lsb->0x%x, msb->0x%x", regval[1], regval[0]);
Sry I forget to remove debug info.
I'll remove them in the next version.
There are some more below, please ignore.

                    - Troy
> +
> +		raw = (s16)((regval[0] << 8) | regval[1]);
> +
> +		raw >>= shift;
> +
> +		*val = raw * scale * flag;
> +
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ctf2301_read_fan(struct device *dev, u32 attr, long *val)
> +{
> +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> +	int regval[2], err, speed;
> +
> +	switch (attr) {
> +	case hwmon_fan_input:
> +		err = regmap_read(ctf2301->regmap, CTF2301_TACH_COUNT_MSB, regval);
> +		if (err)
> +			return err;
> +
> +		err = regmap_read(ctf2301->regmap, CTF2301_TACH_COUNT_LSB, regval + 1);
> +		if (err)
> +			return err;
> +
> +		speed = (regval[0] << 8) | regval[1];
> +
> +		*val = (unsigned int)(1 * (5400000 / speed));
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ctf2301_write_pwm(struct device *dev, u32 attr, long val)
> +{
> +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> +	int err, map_val;
> +
> +	dev_err(dev, "write pwm: %d", attr);
> +
> +	switch (attr) {
> +	case hwmon_pwm_input:
> +		map_val = (val * ctf2301->pwm_freq_code * 2) / 255;
> +		dev_err(dev, "val:%ld, map_val: %d", val, map_val);
> +		err = regmap_write(ctf2301->regmap, CTF2301_PWM_VALUE, map_val);
> +		if (err)
> +			return err;
> +		break;
> +	case hwmon_pwm_freq:
> +		ctf2301->pwm_freq_code = DIV_ROUND_UP(PWM_PARENT_CLOCK, val) / 2;
> +		dev_err(dev, "pwm_freq_code: %d", ctf2301->pwm_freq_code);
> +		err = regmap_write(ctf2301->regmap, CTF2301_PWM_FREQ, ctf2301->pwm_freq_code);
> +		if (err)
> +			return err;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static umode_t ctf2301_is_visible(const void *drvdata,
> +				 enum hwmon_sensor_types type,
> +				 u32 attr, int channel)
> +{
> +	switch (type) {
> +	case hwmon_temp:
> +		switch (attr) {
> +		case hwmon_temp_input:
> +			return 0444;
> +		default:
> +			return 0;
> +		}
> +	case hwmon_fan:
> +		switch (attr) {
> +		case hwmon_fan_input:
> +			return 0444;
> +		default:
> +			return 0;
> +		}
> +	case hwmon_pwm:
> +		switch (attr) {
> +		case hwmon_pwm_input:
> +		case hwmon_pwm_freq:
> +			return 0644;
> +		default:
> +			return 0;
> +		}
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static int ctf2301_read(struct device *dev, enum hwmon_sensor_types type,
> +		       u32 attr, int channel, long *val)
> +{
> +	switch (type) {
> +	case hwmon_temp:
> +		return ctf2301_read_temp(dev, attr, channel, val);
> +	case hwmon_fan:
> +		return ctf2301_read_fan(dev, attr, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +	return 0;
> +}
> +
> +static int ctf2301_write(struct device *dev, enum hwmon_sensor_types type,
> +			 u32 attr, int channel, long val)
> +{
> +	switch (type) {
> +	case hwmon_pwm:
> +		return ctf2301_write_pwm(dev, attr, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +	return 0;
> +}
> +
> +static const struct hwmon_channel_info * const ctf2301_info[] = {
> +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT),
> +	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT | HWMON_PWM_FREQ),
> +	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> +	NULL
> +};
> +
> +static const struct hwmon_ops ctf2301_hwmon_ops = {
> +	.is_visible = ctf2301_is_visible,
> +	.read = ctf2301_read,
> +	.write = ctf2301_write
> +};
> +
> +static const struct hwmon_chip_info ctf2301_chip_info = {
> +	.ops = &ctf2301_hwmon_ops,
> +	.info = ctf2301_info,
> +};
> +
> +static const struct regmap_config ctf2301_regmap_config = {
> +	.max_register = CTF2301_RMT_DIODE_TEMP_FILTER,
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +
> +static int ctf2301_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct device *hwmon_dev;
> +	struct ctf2301 *ctf2301;
> +	int err;
> +
> +	ctf2301 = devm_kzalloc(dev, sizeof(*ctf2301), GFP_KERNEL);
> +	if (!ctf2301)
> +		return -ENOMEM;
> +	ctf2301->client = client;
> +
> +	ctf2301->regmap = devm_regmap_init_i2c(client, &ctf2301_regmap_config);
> +	if (IS_ERR(ctf2301->regmap))
> +		return dev_err_probe(dev, PTR_ERR(ctf2301->regmap),
> +				     "failed to allocate register map");
> +
> +	err = regmap_write(ctf2301->regmap, CTF2301_GLOBAL_CFG,
> +			   GLOBAL_CFG_ALERT_MASK | GLOBAL_CFG_TACH_SEL);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "failed to write CTF2301_GLOBAL_CFG register");
> +
> +	/*err = regmap_write(ctf2301->regmap, CTF2301_ALERT_MASK, ALERT_MASK_ALL);*/
> +	/*if (err)*/
> +		/*return dev_err_probe(dev, err,*/
> +				     /*"failed to write CTF2301_ALERT_MASK");*/
> +
> +	err = regmap_write(ctf2301->regmap, CTF2301_ENHANCED_CFG, ENHANGCED_CFG_USF);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "failed to write CTF2301_ENHANCED_CFG");
> +
> +	err = regmap_write(ctf2301->regmap, CTF2301_PWM_AND_TACH_CFG, PWM_AND_TACH_CFG_PWPGM);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "failed to write CTF2301_PWM_AND_TACH_CFG");
> +
> +	ctf2301->pwm_freq_code = PWM_DEFAULT_FREQ_CODE;
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, ctf2301,
> +							 &ctf2301_chip_info,
> +							 NULL);
> +	if (IS_ERR(hwmon_dev))
> +		return dev_err_probe(dev, PTR_ERR(hwmon_dev),
> +				     "failed to register hwmon device");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ctf2301_of_match[] = {
> +	{ .compatible = "sensylink,ctf2301", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ctf2301_of_match);
> +
> +static struct i2c_driver ctf2301_driver = {
> +	.driver = {
> +		.name	= "ctf2301",
> +		.of_match_table = of_match_ptr(ctf2301_of_match),
> +	},
> +	.probe		= ctf2301_probe,
> +};
> +module_i2c_driver(ctf2301_driver);
> +
> +MODULE_AUTHOR("Troy Mitchell <troy.mitchell@linux.dev>");
> +MODULE_DESCRIPTION("ctf2301 driver");
> +MODULE_LICENSE("GPL");
> 
> -- 
> 2.51.0
> 

Re: [PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301

[Guenter Roeck]

On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
> This commit introduces driver for the Sensylink CTF2301
> system-level thermal management solution chip.
> 
Documentation/process/submitting-patches.rst:

"Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
 instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
 to do frotz", as if you are giving orders to the codebase to change
 its behaviour."

-->

"Add support for ..."

> Currently, the driver does NOT support the Auto-Temp mode of the PWM
> fan controller, which provides closed-loop automatic fan speed control
> based on temperature.
> 
> Now this driver supports:

Now -> Currently

>   - Reading local temperature.
>   - Reading remote temperature.
>   - Controlling the PWM fan output in Direct-DCY mode (direct duty cycle control).
>   - Monitoring fan speed via the TACH input (RPM measurement).
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
> ---
>  drivers/hwmon/Kconfig   |  11 ++
>  drivers/hwmon/Makefile  |   1 +
>  drivers/hwmon/ctf2301.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 338 insertions(+)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46..2120d891e549795c3f3416d08f71916af714f6b6 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -537,6 +537,17 @@ config SENSORS_CROS_EC
>  	  This driver can also be built as a module. If so, the module
>  	  will be called cros_ec_hwmon.
>  
> +config SENSORS_CTF2301
> +	tristate "Sensylink CTF2301"
> +	depends on I2C
> +	select REGMAP
> +	help
> +	  If you say yes here you get support for Sensylink CTF2301
> +	  sensor chip.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called ctf2301.
> +
>  config SENSORS_DRIVETEMP
>  	tristate "Hard disk drives with temperature sensors"
>  	depends on SCSI && ATA
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index cd8bc4752b4dbf015c6eb46157626f4e8f87dfae..12f2894ce8d5fbfd942409f6c43d78fbdece57b4 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
>  obj-$(CONFIG_SENSORS_CORSAIR_CPRO) += corsair-cpro.o
>  obj-$(CONFIG_SENSORS_CORSAIR_PSU) += corsair-psu.o
>  obj-$(CONFIG_SENSORS_CROS_EC)	+= cros_ec_hwmon.o
> +obj-$(CONFIG_SENSORS_CTF2301)	+= ctf2301.o
>  obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
>  obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
>  obj-$(CONFIG_SENSORS_DELL_SMM)	+= dell-smm-hwmon.o
> diff --git a/drivers/hwmon/ctf2301.c b/drivers/hwmon/ctf2301.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2fea4d195519ea34c1d4bf67456098b225d4d13c
> --- /dev/null
> +++ b/drivers/hwmon/ctf2301.c
> @@ -0,0 +1,326 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for CTF2301 system-level thermal management solution chip
> + * Datasheet: https://www.sensylink.com/upload/1/net.sensylink.portal/1689557281035.pdf
> + *
> + * Copyright (C) 2025 Troy Mitchell <troy.mitchell@linux.dev>
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +#define PWM_PARENT_CLOCK			360000
> +
> +#define CTF2301_LOCAL_TEMP_MSB			0x00
	LM90_REG_LOCAL_TEMP
> +#define CTF2301_RMT_TEMP_MSB			0x01
	LM90_REG_REMOTE_TEMPH
> +#define CTF2301_ALERT_STATUS			0x02
	LM90_REG_STATUS
> +#define CTF2301_GLOBAL_CFG			0x03
	LM90_REG_CONFIG1
> +#define CTF2301_RMT_TEMP_LSB			0x10
	LM90_REG_REMOTE_TEMPL
> +#define CTF2301_LOCAL_TEMP_LSB			0x15
	TMP451_REG_LOCAL_TEMPL
> +#define CTF2301_ALERT_MASK			0x16
	TMP461_REG_CHEN

So far this looks like a chip based on LM90 or TMP451/TMP461
with an added fan controller. I can not immediatey determine
if it would be better to add the pwm/tach support to the lm90
driver. Given that the chip (based on registers) does support
limits, which is not implemented here but essential for a chip
like this, I would very much prefer adding support for it to the
lm90 driver if possible.

The public datasheet does not provide register details, making it
all but impossible to do a real evaluation. Any idea how to get
a complete datasheet ?

> +#define	CTF2301_ENHANCED_CFG			0x45
> +#define CTF2301_TACH_COUNT_LSB			0x46
> +#define CTF2301_TACH_COUNT_MSB			0x47
> +#define CTF2301_PWM_AND_TACH_CFG		0x4a
> +#define CTF2301_PWM_VALUE			0x4c
> +#define CTF2301_PWM_FREQ			0x4d
> +#define CTF2301_RMT_DIODE_TEMP_FILTER		0xbf
> +
> +/* remote diode fault alarm */
> +#define ALERT_STATUS_RDFA			BIT(2)
> +
> +/* alert interrupts enable  */
> +#define GLOBAL_CFG_ALERT_MASK			BIT(7)
> +/* tach input enable  */
> +#define GLOBAL_CFG_TACH_SEL			BIT(2)
> +
> +/* local high temperature alarm mask */
> +#define ALERT_MASK_LHAM				BIT(6)
> +/* remote high temperature alarm mask */
> +#define ALERT_MASK_RHAM				BIT(4)
> +/* remote low temperature alarm mask */
> +#define ALERT_MASK_RLAM				BIT(3)
> +/* remote t_crit alarm mask */
> +#define ALERT_MASK_RCAM				BIT(1)
> +/* tachometer alarm mask */
> +#define ALERT_MASK_TCHAM			BIT(0)
> +
> +#define ALERT_MASK_ALL				(ALERT_MASK_LHAM | ALERT_MASK_RHAM | \
> +						ALERT_MASK_RLAM | ALERT_MASK_RCAM | \
> +						ALERT_MASK_TCHAM)
> +
> +/* enables signed format for high and t_crit setpoints */
> +#define ENHANGCED_CFG_USF			BIT(3)
> +
> +/* PWM Programming enable */
> +#define PWM_AND_TACH_CFG_PWPGM			BIT(5)
> +
> +#define PWM_DEFAULT_FREQ_CODE			0x17
> +
> +

No mode than one empty line. checkpatch --strict would tell.
And, indeed, it reports:

total: 0 errors, 4 warnings, 3 checks, 350 lines checked

where

WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
CHECK: Please don't use multiple blank lines
CHECK: Alignment should match open parenthesis

are relevant and need to be fixed.

> +struct ctf2301 {
> +	struct i2c_client *client;

Not used anywhere.

> +
> +	struct regmap *regmap;
> +
> +	unsigned int pwm_freq_code;

Unnecessary empty lines.

> +};
> +
> +static int ctf2301_read_temp(struct device *dev, u32 attr, int channel, long *val)
> +{
> +	int regval[2], raw, err, flag = 1, shift = 4, scale = 625;
> +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> +	unsigned int reg_msb = CTF2301_LOCAL_TEMP_MSB,
> +		     reg_lsb = CTF2301_LOCAL_TEMP_LSB;
> +
> +	switch (attr) {
> +	case hwmon_temp_input:
> +		if (channel != 0 && channel != 1)
> +			return -EOPNOTSUPP;

Should have been handled in is_visible function. It is, therefore
this check is unnecessary.

> +
> +		if (channel == 1) {
> +			err = regmap_read(ctf2301->regmap, CTF2301_ALERT_STATUS, regval);
> +			if (err)
> +				return err;
> +
> +			if (regval[0] & ALERT_STATUS_RDFA)
> +				return -ENODEV;

Wrong return value. The device does obviously exist. This should return
-ENODATA.

> +
> +			shift = 5;
> +			scale = 1250;
> +			reg_msb = CTF2301_RMT_TEMP_MSB;
> +			reg_lsb = CTF2301_RMT_TEMP_LSB;
> +		}
> +
> +		err = regmap_read(ctf2301->regmap, reg_msb, regval);
> +		if (err)
> +			return err;
> +
> +		err = regmap_read(ctf2301->regmap, reg_lsb, regval + 1);
> +		if (err)
> +			return err;

Consider using regmap_multi_reg_read() instead.

> +
> +		dev_err(dev, "local temp: lsb->0x%x, msb->0x%x", regval[1], regval[0]);

Really ?

Stopping complete review here. The driver is obviously not ready
for submission. Some more obvious comments below.


> +
> +		raw = (s16)((regval[0] << 8) | regval[1]);
> +
> +		raw >>= shift;
> +
> +		*val = raw * scale * flag;
> +
> +		break;

Drop empty lines.

> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ctf2301_read_fan(struct device *dev, u32 attr, long *val)

There is only a single supported attribute, so passing its vbalue
to this function and checking it adds unnecessary complexity.
Just drop the parameter and all its complexity.

> +{
> +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> +	int regval[2], err, speed;
> +
> +	switch (attr) {
> +	case hwmon_fan_input:
> +		err = regmap_read(ctf2301->regmap, CTF2301_TACH_COUNT_MSB, regval);
> +		if (err)
> +			return err;
> +
> +		err = regmap_read(ctf2301->regmap, CTF2301_TACH_COUNT_LSB, regval + 1);
> +		if (err)
> +			return err;

CTF2301_TACH_COUNT_LSB and CTF2301_TACH_COUNT_MSB are consecutive registers,
so it should be possible to use regmap_bulk_read(). If not, consider using
regmap_multi_reg_read() and explain why regmap_bulk_read() does not work.

> +
> +		speed = (regval[0] << 8) | regval[1];
> +

speed can be 0.

> +		*val = (unsigned int)(1 * (5400000 / speed));
				      ^^^^ what is this for ?

The typecast is unnecessary, and speed needs to be checked to
ensure that there is no divide by zero error.

> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ctf2301_write_pwm(struct device *dev, u32 attr, long val)
> +{
> +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> +	int err, map_val;
> +
> +	dev_err(dev, "write pwm: %d", attr);

Not commmenting on those any further.

> +
> +	switch (attr) {
> +	case hwmon_pwm_input:
> +		map_val = (val * ctf2301->pwm_freq_code * 2) / 255;

val needs to be range checked, and the function needs to return
-EINVAL if it is out of range. Also consider using DIV_ROUND_CLOSEST().

> +		dev_err(dev, "val:%ld, map_val: %d", val, map_val);
> +		err = regmap_write(ctf2301->regmap, CTF2301_PWM_VALUE, map_val);
> +		if (err)
> +			return err;
> +		break;
> +	case hwmon_pwm_freq:
> +		ctf2301->pwm_freq_code = DIV_ROUND_UP(PWM_PARENT_CLOCK, val) / 2;

val needs to be clamped to its valid range.

> +		dev_err(dev, "pwm_freq_code: %d", ctf2301->pwm_freq_code);
> +		err = regmap_write(ctf2301->regmap, CTF2301_PWM_FREQ, ctf2301->pwm_freq_code);
> +		if (err)
> +			return err;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static umode_t ctf2301_is_visible(const void *drvdata,
> +				 enum hwmon_sensor_types type,
> +				 u32 attr, int channel)
> +{
> +	switch (type) {
> +	case hwmon_temp:
> +		switch (attr) {
> +		case hwmon_temp_input:
> +			return 0444;
> +		default:
> +			return 0;
> +		}
> +	case hwmon_fan:
> +		switch (attr) {
> +		case hwmon_fan_input:
> +			return 0444;
> +		default:
> +			return 0;
> +		}
> +	case hwmon_pwm:
> +		switch (attr) {
> +		case hwmon_pwm_input:
> +		case hwmon_pwm_freq:
> +			return 0644;
> +		default:
> +			return 0;
> +		}
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static int ctf2301_read(struct device *dev, enum hwmon_sensor_types type,
> +		       u32 attr, int channel, long *val)
> +{
> +	switch (type) {
> +	case hwmon_temp:
> +		return ctf2301_read_temp(dev, attr, channel, val);
> +	case hwmon_fan:
> +		return ctf2301_read_fan(dev, attr, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +	return 0;
> +}
> +
> +static int ctf2301_write(struct device *dev, enum hwmon_sensor_types type,
> +			 u32 attr, int channel, long val)
> +{
> +	switch (type) {
> +	case hwmon_pwm:
> +		return ctf2301_write_pwm(dev, attr, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +	return 0;
> +}
> +
> +static const struct hwmon_channel_info * const ctf2301_info[] = {
> +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT),
> +	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT | HWMON_PWM_FREQ),
> +	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> +	NULL
> +};
> +
> +static const struct hwmon_ops ctf2301_hwmon_ops = {
> +	.is_visible = ctf2301_is_visible,
> +	.read = ctf2301_read,
> +	.write = ctf2301_write
> +};
> +
> +static const struct hwmon_chip_info ctf2301_chip_info = {
> +	.ops = &ctf2301_hwmon_ops,
> +	.info = ctf2301_info,
> +};
> +
> +static const struct regmap_config ctf2301_regmap_config = {
> +	.max_register = CTF2301_RMT_DIODE_TEMP_FILTER,
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +
> +static int ctf2301_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct device *hwmon_dev;
> +	struct ctf2301 *ctf2301;
> +	int err;
> +
> +	ctf2301 = devm_kzalloc(dev, sizeof(*ctf2301), GFP_KERNEL);
> +	if (!ctf2301)
> +		return -ENOMEM;
> +	ctf2301->client = client;
> +
> +	ctf2301->regmap = devm_regmap_init_i2c(client, &ctf2301_regmap_config);
> +	if (IS_ERR(ctf2301->regmap))
> +		return dev_err_probe(dev, PTR_ERR(ctf2301->regmap),
> +				     "failed to allocate register map");
> +
> +	err = regmap_write(ctf2301->regmap, CTF2301_GLOBAL_CFG,
> +			   GLOBAL_CFG_ALERT_MASK | GLOBAL_CFG_TACH_SEL);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "failed to write CTF2301_GLOBAL_CFG register");
> +
> +	/*err = regmap_write(ctf2301->regmap, CTF2301_ALERT_MASK, ALERT_MASK_ALL);*/
> +	/*if (err)*/
> +		/*return dev_err_probe(dev, err,*/
> +				     /*"failed to write CTF2301_ALERT_MASK");*/
> +
> +	err = regmap_write(ctf2301->regmap, CTF2301_ENHANCED_CFG, ENHANGCED_CFG_USF);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "failed to write CTF2301_ENHANCED_CFG");
> +
> +	err = regmap_write(ctf2301->regmap, CTF2301_PWM_AND_TACH_CFG, PWM_AND_TACH_CFG_PWPGM);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "failed to write CTF2301_PWM_AND_TACH_CFG");
> +
> +	ctf2301->pwm_freq_code = PWM_DEFAULT_FREQ_CODE;
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, ctf2301,
> +							 &ctf2301_chip_info,
> +							 NULL);
> +	if (IS_ERR(hwmon_dev))
> +		return dev_err_probe(dev, PTR_ERR(hwmon_dev),
> +				     "failed to register hwmon device");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ctf2301_of_match[] = {
> +	{ .compatible = "sensylink,ctf2301", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ctf2301_of_match);
> +
> +static struct i2c_driver ctf2301_driver = {
> +	.driver = {
> +		.name	= "ctf2301",
> +		.of_match_table = of_match_ptr(ctf2301_of_match),
> +	},
> +	.probe		= ctf2301_probe,
> +};
> +module_i2c_driver(ctf2301_driver);
> +
> +MODULE_AUTHOR("Troy Mitchell <troy.mitchell@linux.dev>");
> +MODULE_DESCRIPTION("ctf2301 driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301

[Troy Mitchell]

Hi Guenter, Thanks for your review.
There are many things to improve in this driver.

On Wed, Sep 24, 2025 at 08:43:35AM -0700, Guenter Roeck wrote:
> On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
[...]
> > diff --git a/drivers/hwmon/ctf2301.c b/drivers/hwmon/ctf2301.c
[...]
> > +
> > +#define CTF2301_LOCAL_TEMP_MSB			0x00
> 	LM90_REG_LOCAL_TEMP
> > +#define CTF2301_RMT_TEMP_MSB			0x01
> 	LM90_REG_REMOTE_TEMPH
> > +#define CTF2301_ALERT_STATUS			0x02
> 	LM90_REG_STATUS
> > +#define CTF2301_GLOBAL_CFG			0x03
> 	LM90_REG_CONFIG1
> > +#define CTF2301_RMT_TEMP_LSB			0x10
> 	LM90_REG_REMOTE_TEMPL
> > +#define CTF2301_LOCAL_TEMP_LSB			0x15
> 	TMP451_REG_LOCAL_TEMPL
> > +#define CTF2301_ALERT_MASK			0x16
> 	TMP461_REG_CHEN
> 
> So far this looks like a chip based on LM90 or TMP451/TMP461
> with an added fan controller. I can not immediatey determine
> if it would be better to add the pwm/tach support to the lm90
> driver. Given that the chip (based on registers) does support
> limits, which is not implemented here but essential for a chip
> like this, I would very much prefer adding support for it to the
> lm90 driver if possible.
> 
> The public datasheet does not provide register details, making it
> all but impossible to do a real evaluation. Any idea how to get
> a complete datasheet ?
Yeah, more register info at [1].
I've checked the detailed review below,
but I'll hold off on sending v2 until you decide if we really need a new driver.

Is this chip more like the LM63, by the way?

                - Troy

Link:
https://github.com/TroyMitchell911/ctf2301-datasheet
> 
> > +#define	CTF2301_ENHANCED_CFG			0x45
> > +#define CTF2301_TACH_COUNT_LSB			0x46
> > +#define CTF2301_TACH_COUNT_MSB			0x47
> > +#define CTF2301_PWM_AND_TACH_CFG		0x4a
> > +#define CTF2301_PWM_VALUE			0x4c
> > +#define CTF2301_PWM_FREQ			0x4d
> > +#define CTF2301_RMT_DIODE_TEMP_FILTER		0xbf
> > +
> > +/* remote diode fault alarm */
> > +#define ALERT_STATUS_RDFA			BIT(2)
> > +
> > +/* alert interrupts enable  */
> > +#define GLOBAL_CFG_ALERT_MASK			BIT(7)
> > +/* tach input enable  */
> > +#define GLOBAL_CFG_TACH_SEL			BIT(2)
> > +
> > +/* local high temperature alarm mask */
> > +#define ALERT_MASK_LHAM				BIT(6)
> > +/* remote high temperature alarm mask */
> > +#define ALERT_MASK_RHAM				BIT(4)
> > +/* remote low temperature alarm mask */
> > +#define ALERT_MASK_RLAM				BIT(3)
> > +/* remote t_crit alarm mask */
> > +#define ALERT_MASK_RCAM				BIT(1)
> > +/* tachometer alarm mask */
> > +#define ALERT_MASK_TCHAM			BIT(0)
> > +
> > +#define ALERT_MASK_ALL				(ALERT_MASK_LHAM | ALERT_MASK_RHAM | \
> > +						ALERT_MASK_RLAM | ALERT_MASK_RCAM | \
> > +						ALERT_MASK_TCHAM)
> > +
> > +/* enables signed format for high and t_crit setpoints */
> > +#define ENHANGCED_CFG_USF			BIT(3)
> > +
> > +/* PWM Programming enable */
> > +#define PWM_AND_TACH_CFG_PWPGM			BIT(5)
> > +
> > +#define PWM_DEFAULT_FREQ_CODE			0x17
> > +
> > +
> 
> No mode than one empty line. checkpatch --strict would tell.
> And, indeed, it reports:
> 
> total: 0 errors, 4 warnings, 3 checks, 350 lines checked
> 
> where
> 
> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
> CHECK: Please don't use multiple blank lines
> CHECK: Alignment should match open parenthesis
> 
> are relevant and need to be fixed.
> 
> > +struct ctf2301 {
> > +	struct i2c_client *client;
> 
> Not used anywhere.
> 
> > +
> > +	struct regmap *regmap;
> > +
> > +	unsigned int pwm_freq_code;
> 
> Unnecessary empty lines.
> 
> > +};
> > +
> > +static int ctf2301_read_temp(struct device *dev, u32 attr, int channel, long *val)
> > +{
> > +	int regval[2], raw, err, flag = 1, shift = 4, scale = 625;
> > +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> > +	unsigned int reg_msb = CTF2301_LOCAL_TEMP_MSB,
> > +		     reg_lsb = CTF2301_LOCAL_TEMP_LSB;
> > +
> > +	switch (attr) {
> > +	case hwmon_temp_input:
> > +		if (channel != 0 && channel != 1)
> > +			return -EOPNOTSUPP;
> 
> Should have been handled in is_visible function. It is, therefore
> this check is unnecessary.
> 
> > +
> > +		if (channel == 1) {
> > +			err = regmap_read(ctf2301->regmap, CTF2301_ALERT_STATUS, regval);
> > +			if (err)
> > +				return err;
> > +
> > +			if (regval[0] & ALERT_STATUS_RDFA)
> > +				return -ENODEV;
> 
> Wrong return value. The device does obviously exist. This should return
> -ENODATA.
> 
> > +
> > +			shift = 5;
> > +			scale = 1250;
> > +			reg_msb = CTF2301_RMT_TEMP_MSB;
> > +			reg_lsb = CTF2301_RMT_TEMP_LSB;
> > +		}
> > +
> > +		err = regmap_read(ctf2301->regmap, reg_msb, regval);
> > +		if (err)
> > +			return err;
> > +
> > +		err = regmap_read(ctf2301->regmap, reg_lsb, regval + 1);
> > +		if (err)
> > +			return err;
> 
> Consider using regmap_multi_reg_read() instead.
> 
> > +
> > +		dev_err(dev, "local temp: lsb->0x%x, msb->0x%x", regval[1], regval[0]);
> 
> Really ?
> 
> Stopping complete review here. The driver is obviously not ready
> for submission. Some more obvious comments below.
> 
> 
> > +
> > +		raw = (s16)((regval[0] << 8) | regval[1]);
> > +
> > +		raw >>= shift;
> > +
> > +		*val = raw * scale * flag;
> > +
> > +		break;
> 
> Drop empty lines.
> 
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ctf2301_read_fan(struct device *dev, u32 attr, long *val)
> 
> There is only a single supported attribute, so passing its vbalue
> to this function and checking it adds unnecessary complexity.
> Just drop the parameter and all its complexity.
> 
> > +{
> > +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> > +	int regval[2], err, speed;
> > +
> > +	switch (attr) {
> > +	case hwmon_fan_input:
> > +		err = regmap_read(ctf2301->regmap, CTF2301_TACH_COUNT_MSB, regval);
> > +		if (err)
> > +			return err;
> > +
> > +		err = regmap_read(ctf2301->regmap, CTF2301_TACH_COUNT_LSB, regval + 1);
> > +		if (err)
> > +			return err;
> 
> CTF2301_TACH_COUNT_LSB and CTF2301_TACH_COUNT_MSB are consecutive registers,
> so it should be possible to use regmap_bulk_read(). If not, consider using
> regmap_multi_reg_read() and explain why regmap_bulk_read() does not work.
> 
> > +
> > +		speed = (regval[0] << 8) | regval[1];
> > +
> 
> speed can be 0.
> 
> > +		*val = (unsigned int)(1 * (5400000 / speed));
> 				      ^^^^ what is this for ?
> 
> The typecast is unnecessary, and speed needs to be checked to
> ensure that there is no divide by zero error.
> 
> > +		break;
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ctf2301_write_pwm(struct device *dev, u32 attr, long val)
> > +{
> > +	struct ctf2301 *ctf2301 = dev_get_drvdata(dev);
> > +	int err, map_val;
> > +
> > +	dev_err(dev, "write pwm: %d", attr);
> 
> Not commmenting on those any further.
> 
> > +
> > +	switch (attr) {
> > +	case hwmon_pwm_input:
> > +		map_val = (val * ctf2301->pwm_freq_code * 2) / 255;
> 
> val needs to be range checked, and the function needs to return
> -EINVAL if it is out of range. Also consider using DIV_ROUND_CLOSEST().
> 
> > +		dev_err(dev, "val:%ld, map_val: %d", val, map_val);
> > +		err = regmap_write(ctf2301->regmap, CTF2301_PWM_VALUE, map_val);
> > +		if (err)
> > +			return err;
> > +		break;
> > +	case hwmon_pwm_freq:
> > +		ctf2301->pwm_freq_code = DIV_ROUND_UP(PWM_PARENT_CLOCK, val) / 2;
> 
> val needs to be clamped to its valid range.
> 
> > +		dev_err(dev, "pwm_freq_code: %d", ctf2301->pwm_freq_code);
> > +		err = regmap_write(ctf2301->regmap, CTF2301_PWM_FREQ, ctf2301->pwm_freq_code);
> > +		if (err)
> > +			return err;
> > +		break;
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static umode_t ctf2301_is_visible(const void *drvdata,
> > +				 enum hwmon_sensor_types type,
> > +				 u32 attr, int channel)
> > +{
> > +	switch (type) {
> > +	case hwmon_temp:
> > +		switch (attr) {
> > +		case hwmon_temp_input:
> > +			return 0444;
> > +		default:
> > +			return 0;
> > +		}
> > +	case hwmon_fan:
> > +		switch (attr) {
> > +		case hwmon_fan_input:
> > +			return 0444;
> > +		default:
> > +			return 0;
> > +		}
> > +	case hwmon_pwm:
> > +		switch (attr) {
> > +		case hwmon_pwm_input:
> > +		case hwmon_pwm_freq:
> > +			return 0644;
> > +		default:
> > +			return 0;
> > +		}
> > +	default:
> > +		return 0;
> > +	}
> > +}
> > +
> > +static int ctf2301_read(struct device *dev, enum hwmon_sensor_types type,
> > +		       u32 attr, int channel, long *val)
> > +{
> > +	switch (type) {
> > +	case hwmon_temp:
> > +		return ctf2301_read_temp(dev, attr, channel, val);
> > +	case hwmon_fan:
> > +		return ctf2301_read_fan(dev, attr, val);
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int ctf2301_write(struct device *dev, enum hwmon_sensor_types type,
> > +			 u32 attr, int channel, long val)
> > +{
> > +	switch (type) {
> > +	case hwmon_pwm:
> > +		return ctf2301_write_pwm(dev, attr, val);
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static const struct hwmon_channel_info * const ctf2301_info[] = {
> > +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT),
> > +	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT | HWMON_PWM_FREQ),
> > +	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> > +	NULL
> > +};
> > +
> > +static const struct hwmon_ops ctf2301_hwmon_ops = {
> > +	.is_visible = ctf2301_is_visible,
> > +	.read = ctf2301_read,
> > +	.write = ctf2301_write
> > +};
> > +
> > +static const struct hwmon_chip_info ctf2301_chip_info = {
> > +	.ops = &ctf2301_hwmon_ops,
> > +	.info = ctf2301_info,
> > +};
> > +
> > +static const struct regmap_config ctf2301_regmap_config = {
> > +	.max_register = CTF2301_RMT_DIODE_TEMP_FILTER,
> > +	.reg_bits = 8,
> > +	.val_bits = 8,
> > +};
> > +
> > +static int ctf2301_probe(struct i2c_client *client)
> > +{
> > +	struct device *dev = &client->dev;
> > +	struct device *hwmon_dev;
> > +	struct ctf2301 *ctf2301;
> > +	int err;
> > +
> > +	ctf2301 = devm_kzalloc(dev, sizeof(*ctf2301), GFP_KERNEL);
> > +	if (!ctf2301)
> > +		return -ENOMEM;
> > +	ctf2301->client = client;
> > +
> > +	ctf2301->regmap = devm_regmap_init_i2c(client, &ctf2301_regmap_config);
> > +	if (IS_ERR(ctf2301->regmap))
> > +		return dev_err_probe(dev, PTR_ERR(ctf2301->regmap),
> > +				     "failed to allocate register map");
> > +
> > +	err = regmap_write(ctf2301->regmap, CTF2301_GLOBAL_CFG,
> > +			   GLOBAL_CFG_ALERT_MASK | GLOBAL_CFG_TACH_SEL);
> > +	if (err)
> > +		return dev_err_probe(dev, err,
> > +				     "failed to write CTF2301_GLOBAL_CFG register");
> > +
> > +	/*err = regmap_write(ctf2301->regmap, CTF2301_ALERT_MASK, ALERT_MASK_ALL);*/
> > +	/*if (err)*/
> > +		/*return dev_err_probe(dev, err,*/
> > +				     /*"failed to write CTF2301_ALERT_MASK");*/
> > +
> > +	err = regmap_write(ctf2301->regmap, CTF2301_ENHANCED_CFG, ENHANGCED_CFG_USF);
> > +	if (err)
> > +		return dev_err_probe(dev, err,
> > +				     "failed to write CTF2301_ENHANCED_CFG");
> > +
> > +	err = regmap_write(ctf2301->regmap, CTF2301_PWM_AND_TACH_CFG, PWM_AND_TACH_CFG_PWPGM);
> > +	if (err)
> > +		return dev_err_probe(dev, err,
> > +				     "failed to write CTF2301_PWM_AND_TACH_CFG");
> > +
> > +	ctf2301->pwm_freq_code = PWM_DEFAULT_FREQ_CODE;
> > +
> > +	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, ctf2301,
> > +							 &ctf2301_chip_info,
> > +							 NULL);
> > +	if (IS_ERR(hwmon_dev))
> > +		return dev_err_probe(dev, PTR_ERR(hwmon_dev),
> > +				     "failed to register hwmon device");
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id ctf2301_of_match[] = {
> > +	{ .compatible = "sensylink,ctf2301", },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ctf2301_of_match);
> > +
> > +static struct i2c_driver ctf2301_driver = {
> > +	.driver = {
> > +		.name	= "ctf2301",
> > +		.of_match_table = of_match_ptr(ctf2301_of_match),
> > +	},
> > +	.probe		= ctf2301_probe,
> > +};
> > +module_i2c_driver(ctf2301_driver);
> > +
> > +MODULE_AUTHOR("Troy Mitchell <troy.mitchell@linux.dev>");
> > +MODULE_DESCRIPTION("ctf2301 driver");
> > +MODULE_LICENSE("GPL");

Re: [PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301

[Guenter Roeck]

On 9/25/25 18:32, Troy Mitchell wrote:
> Hi Guenter, Thanks for your review.
> There are many things to improve in this driver.
> 
> On Wed, Sep 24, 2025 at 08:43:35AM -0700, Guenter Roeck wrote:
>> On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
> [...]
>>> diff --git a/drivers/hwmon/ctf2301.c b/drivers/hwmon/ctf2301.c
> [...]
>>> +
>>> +#define CTF2301_LOCAL_TEMP_MSB			0x00
>> 	LM90_REG_LOCAL_TEMP
>>> +#define CTF2301_RMT_TEMP_MSB			0x01
>> 	LM90_REG_REMOTE_TEMPH
>>> +#define CTF2301_ALERT_STATUS			0x02
>> 	LM90_REG_STATUS
>>> +#define CTF2301_GLOBAL_CFG			0x03
>> 	LM90_REG_CONFIG1
>>> +#define CTF2301_RMT_TEMP_LSB			0x10
>> 	LM90_REG_REMOTE_TEMPL
>>> +#define CTF2301_LOCAL_TEMP_LSB			0x15
>> 	TMP451_REG_LOCAL_TEMPL
>>> +#define CTF2301_ALERT_MASK			0x16
>> 	TMP461_REG_CHEN
>>
>> So far this looks like a chip based on LM90 or TMP451/TMP461
>> with an added fan controller. I can not immediatey determine
>> if it would be better to add the pwm/tach support to the lm90
>> driver. Given that the chip (based on registers) does support
>> limits, which is not implemented here but essential for a chip
>> like this, I would very much prefer adding support for it to the
>> lm90 driver if possible.
>>
>> The public datasheet does not provide register details, making it
>> all but impossible to do a real evaluation. Any idea how to get
>> a complete datasheet ?
> Yeah, more register info at [1].
> I've checked the detailed review below,
> but I'll hold off on sending v2 until you decide if we really need a new driver.
> 
> Is this chip more like the LM63, by the way?
> 

Good catch. Yes, looks like you are correct. LM63 is an almost perfect match.
CTF2301 has a couple of extra registers, mostly local setpoint and temp LSB
plus the registers in the 0x3x range. Actually, those registers _are_ defined
for LM96163, so that chip is an even closer match.

What happens if you just instantiate the lm63 driver, possibly after updating
the detect function ?

Guenter

Re: [PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301

[Troy Mitchell]

On Thu, Sep 25, 2025 at 08:57:13PM -0700, Guenter Roeck wrote:
> On 9/25/25 18:32, Troy Mitchell wrote:
> > Hi Guenter, Thanks for your review.
> > There are many things to improve in this driver.
> > 
> > On Wed, Sep 24, 2025 at 08:43:35AM -0700, Guenter Roeck wrote:
> > > On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
> > [...]
> > > > diff --git a/drivers/hwmon/ctf2301.c b/drivers/hwmon/ctf2301.c
> > [...]
> > > > +
> > > > +#define CTF2301_LOCAL_TEMP_MSB			0x00
> > > 	LM90_REG_LOCAL_TEMP
> > > > +#define CTF2301_RMT_TEMP_MSB			0x01
> > > 	LM90_REG_REMOTE_TEMPH
> > > > +#define CTF2301_ALERT_STATUS			0x02
> > > 	LM90_REG_STATUS
> > > > +#define CTF2301_GLOBAL_CFG			0x03
> > > 	LM90_REG_CONFIG1
> > > > +#define CTF2301_RMT_TEMP_LSB			0x10
> > > 	LM90_REG_REMOTE_TEMPL
> > > > +#define CTF2301_LOCAL_TEMP_LSB			0x15
> > > 	TMP451_REG_LOCAL_TEMPL
> > > > +#define CTF2301_ALERT_MASK			0x16
> > > 	TMP461_REG_CHEN
> > > 
> > > So far this looks like a chip based on LM90 or TMP451/TMP461
> > > with an added fan controller. I can not immediatey determine
> > > if it would be better to add the pwm/tach support to the lm90
> > > driver. Given that the chip (based on registers) does support
> > > limits, which is not implemented here but essential for a chip
> > > like this, I would very much prefer adding support for it to the
> > > lm90 driver if possible.
> > > 
> > > The public datasheet does not provide register details, making it
> > > all but impossible to do a real evaluation. Any idea how to get
> > > a complete datasheet ?
> > Yeah, more register info at [1].
> > I've checked the detailed review below,
> > but I'll hold off on sending v2 until you decide if we really need a new driver.
> > 
> > Is this chip more like the LM63, by the way?
> > 
> 
> Good catch. Yes, looks like you are correct. LM63 is an almost perfect match.
> CTF2301 has a couple of extra registers, mostly local setpoint and temp LSB
> plus the registers in the 0x3x range. Actually, those registers _are_ defined
> for LM96163, so that chip is an even closer match.
Yes, so just to confirm,
you agree that the development should be done on top of the lm63 driver, right?

> 
> What happens if you just instantiate the lm63 driver, possibly after updating
> the detect function ?
I will run the tests the day after tomorrow and provide a log.

                - Troy

> 
> Guenter
> 

Re: [PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301

[Guenter Roeck]

On 9/26/25 06:21, Troy Mitchell wrote:
> On Thu, Sep 25, 2025 at 08:57:13PM -0700, Guenter Roeck wrote:
>> On 9/25/25 18:32, Troy Mitchell wrote:
>>> Hi Guenter, Thanks for your review.
>>> There are many things to improve in this driver.
>>>
>>> On Wed, Sep 24, 2025 at 08:43:35AM -0700, Guenter Roeck wrote:
>>>> On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
>>> [...]
>>>>> diff --git a/drivers/hwmon/ctf2301.c b/drivers/hwmon/ctf2301.c
>>> [...]
>>>>> +
>>>>> +#define CTF2301_LOCAL_TEMP_MSB			0x00
>>>> 	LM90_REG_LOCAL_TEMP
>>>>> +#define CTF2301_RMT_TEMP_MSB			0x01
>>>> 	LM90_REG_REMOTE_TEMPH
>>>>> +#define CTF2301_ALERT_STATUS			0x02
>>>> 	LM90_REG_STATUS
>>>>> +#define CTF2301_GLOBAL_CFG			0x03
>>>> 	LM90_REG_CONFIG1
>>>>> +#define CTF2301_RMT_TEMP_LSB			0x10
>>>> 	LM90_REG_REMOTE_TEMPL
>>>>> +#define CTF2301_LOCAL_TEMP_LSB			0x15
>>>> 	TMP451_REG_LOCAL_TEMPL
>>>>> +#define CTF2301_ALERT_MASK			0x16
>>>> 	TMP461_REG_CHEN
>>>>
>>>> So far this looks like a chip based on LM90 or TMP451/TMP461
>>>> with an added fan controller. I can not immediatey determine
>>>> if it would be better to add the pwm/tach support to the lm90
>>>> driver. Given that the chip (based on registers) does support
>>>> limits, which is not implemented here but essential for a chip
>>>> like this, I would very much prefer adding support for it to the
>>>> lm90 driver if possible.
>>>>
>>>> The public datasheet does not provide register details, making it
>>>> all but impossible to do a real evaluation. Any idea how to get
>>>> a complete datasheet ?
>>> Yeah, more register info at [1].
>>> I've checked the detailed review below,
>>> but I'll hold off on sending v2 until you decide if we really need a new driver.
>>>
>>> Is this chip more like the LM63, by the way?
>>>
>>
>> Good catch. Yes, looks like you are correct. LM63 is an almost perfect match.
>> CTF2301 has a couple of extra registers, mostly local setpoint and temp LSB
>> plus the registers in the 0x3x range. Actually, those registers _are_ defined
>> for LM96163, so that chip is an even closer match.
> Yes, so just to confirm,
> you agree that the development should be done on top of the lm63 driver, right?
> 
Yes.

Thanks,
Guenter