hwmon: (ctf2301) Add support for CTF2301
Subsystems (2)
cover hwmon: (ctf2301) Add support for CTF2301
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]
---
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,
Signature · 1 line
Troy Mitchell <[email protected]>
PATCH 1/3 dt-bindings: vendor-prefixes: Add Sensylink
--- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0
@@ -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,.*":
Signature · 1 line
2.51.0
Guenter Roeck
maintainerRe: [PATCH 1/3] dt-bindings: vendor-prefixes: Add Sensylink
Quoted 2 lines · Troy Mitchell
This is not an appropriate patch description.
Quoted 18 lines
Signed-off-by Troy Mitchell <[email protected]>--- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+)Documentation/devicetree/bindings/vendor-prefixes.yaml +2
-0@@ -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
Quoted 4 lines · Guenter Roeck
Quoted 2 lines · Troy Mitchell
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
Quoted 19 lines
Quoted 19 lines
Signed-off-by Troy Mitchell <[email protected]>--- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+)Documentation/devicetree/bindings/vendor-prefixes.yaml +2
-0@@ -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
In reply to Troy MitchellAdd dt-binding for the hwmon driver of Sensylink's CTF2301 chip.
--- .../bindings/hwmon/sensylink,ctf2301.yaml | 49 ++++++++++++++++++++++ 1 file changed, 49 insertions(+)
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml new file +49 -0
@@ -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 <[email protected]> + +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>; + }; + };
Signature · 1 line
2.51.0
Rob Herring (Arm)
maintainerRe: [PATCH 2/3] dt-bindings: Add CTF2301 devicetree bindings
Quoted 7 lines · Troy Mitchell
Add dt-binding for the hwmon driver of Sensylink's CTF2301 chip.
Signed-off-by Troy Mitchell <[email protected]>--- .../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/[email protected]
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.
Rob Herring
maintainerRe: [PATCH 2/3] dt-bindings: Add CTF2301 devicetree bindings
In reply to Troy MitchellQuoted 21 lines · Troy Mitchell
Add dt-binding for the hwmon driver of Sensylink's CTF2301 chip.
Signed-off-by Troy Mitchell <[email protected]>--- .../bindings/hwmon/sensylink,ctf2301.yaml | 49 ++++++++++++++++++++++ 1 file changed, 49 insertions(+)Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml new file +8
-0@@ -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 +
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml line 18
Quoted 10 lines
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml new file +10
-0+maintainers: + - Troy Mitchell <[email protected]> + +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.
Quoted 35 lines
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml new file +31
-0+ - 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>; + }; + };Signature · 2 lines
-- 2.51.0
Re: [PATCH 2/3] dt-bindings: Add CTF2301 devicetree bindings
Hi Rob, Thanks for your review.
Quoted 12 lines · Rob Herring
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml line 18
Quoted 10 lines · Troy Mitchell
Add dt-binding for the hwmon driver of Sensylink's CTF2301 chip.
Signed-off-by Troy Mitchell <[email protected]>--- .../bindings/hwmon/sensylink,ctf2301.yaml | 49 ++++++++++++++++++++++ 1 file changed, 49 insertions(+)Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml new file +4
-0+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).
Quoted 1 line
Wrap at 80 chars.
I'll fix it in the next version.
Quoted 11 lines
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml line 28
Quoted 11 lines
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml new file +10
-0+ - 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
Quoted 25 lines
Quoted 25 lines
Documentation/devicetree/bindings/hwmon/sensylink,ctf2301.yaml new file +21
-0+ + reg: + maxItems: 1 + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ctf2301@4c { + compatible = "sensylink,ctf2301"; + reg = <0x4c>; + }; + };Signature · 2 lines
-- 2.51.0
PATCH 3/3 hwmon: (ctf2301) Add support for CTF2301
In reply to Troy MitchellThis 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).
--- drivers/hwmon/Kconfig | 11 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/ctf2301.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 338 insertions(+)
drivers/hwmon/Kconfig +11 -0
@@ -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
drivers/hwmon/Makefile +1 -0
@@ -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
drivers/hwmon/ctf2301.c new file +326 -0
326 lines not on this page · raw message
Signature · 1 line
2.51.0
Quoted 156 lines · 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 <[email protected]>--- drivers/hwmon/Kconfig | 11 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/ctf2301.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 338 insertions(+)drivers/hwmon/Kconfig +11
-0@@ -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 && ATAdrivers/hwmon/Makefile +1
-0@@ -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.odrivers/hwmon/ctf2301.c new file +96
-0@@ -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 <[email protected]> +*/ + +#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 ; + }
drivers/hwmon/ctf2301.c line 106
Quoted 10 lines
drivers/hwmon/ctf2301.c new file +10
-0+ + 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
Quoted 224 lines
drivers/hwmon/ctf2301.c new file +220
-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 <[email protected]>" ); +MODULE_DESCRIPTION ("ctf2301 driver" ); +MODULE_LICENSE ("GPL" );Signature · 2 lines
-- 2.51.0
Guenter Roeck
maintainerQuoted 3 lines · Troy Mitchell
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 ..."
Quoted 5 lines
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
Quoted 59 lines
- 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 <[email protected]>--- drivers/hwmon/Kconfig | 11 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/ctf2301.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 338 insertions(+)drivers/hwmon/Kconfig +11
-0@@ -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 && ATAdrivers/hwmon/Makefile +1
-0@@ -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.odrivers/hwmon/ctf2301.c new file +7
-0@@ -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 <[email protected]> +*/
drivers/hwmon/ctf2301.c line 17
Quoted 10 lines
drivers/hwmon/ctf2301.c new file +10
-0+ +#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
drivers/hwmon/ctf2301.c line 18
Quoted 1 line
drivers/hwmon/ctf2301.c new file +1
-0+#define CTF2301_RMT_TEMP_MSB 0x01
LM90_REG_REMOTE_TEMPH
drivers/hwmon/ctf2301.c line 19
Quoted 1 line
drivers/hwmon/ctf2301.c new file +1
-0+#define CTF2301_ALERT_STATUS 0x02
LM90_REG_STATUS
drivers/hwmon/ctf2301.c line 20
Quoted 1 line
drivers/hwmon/ctf2301.c new file +1
-0+#define CTF2301_GLOBAL_CFG 0x03
LM90_REG_CONFIG1
drivers/hwmon/ctf2301.c line 21
Quoted 1 line
drivers/hwmon/ctf2301.c new file +1
-0+#define CTF2301_RMT_TEMP_LSB 0x10
LM90_REG_REMOTE_TEMPL
drivers/hwmon/ctf2301.c line 22
Quoted 1 line
drivers/hwmon/ctf2301.c new file +1
-0+#define CTF2301_LOCAL_TEMP_LSB 0x15
TMP451_REG_LOCAL_TEMPL
drivers/hwmon/ctf2301.c line 23
Quoted 1 line
drivers/hwmon/ctf2301.c new file +1
-0+#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 ?
Quoted 30 lines
drivers/hwmon/ctf2301.c new file +30
-0+#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)
drivers/hwmon/ctf2301.c line 63
Quoted 10 lines
drivers/hwmon/ctf2301.c new file +10
-0+ +/* 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.
drivers/hwmon/ctf2301.c line 65
Quoted 2 lines
drivers/hwmon/ctf2301.c new file +2
-0+struct ctf2301 { +struct i2c_client *client;
Not used anywhere.
drivers/hwmon/ctf2301.c line 69
Quoted 4 lines
drivers/hwmon/ctf2301.c new file +4
-0+ +struct regmap *regmap; + +unsigned int pwm_freq_code;
Unnecessary empty lines.
drivers/hwmon/ctf2301.c line 82
Quoted 13 lines
drivers/hwmon/ctf2301.c new file +13
-0+}; + +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.
drivers/hwmon/ctf2301.c line 90
Quoted 8 lines
drivers/hwmon/ctf2301.c new file +8
-0+ +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.
Quoted 4 lines
drivers/hwmon/ctf2301.c new file +4
-0+ + shift =5 ; + scale =1250 ; + reg_msb =CTF2301_RMT_TEMP_MSB ;
drivers/hwmon/ctf2301.c line 104
Quoted 10 lines
drivers/hwmon/ctf2301.c new file +10
-0+ 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.
drivers/hwmon/ctf2301.c line 106
Quoted 2 lines
drivers/hwmon/ctf2301.c new file +2
-0+ +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.
drivers/hwmon/ctf2301.c line 114
Quoted 8 lines
drivers/hwmon/ctf2301.c new file +8
-0+ + raw = (s16)((regval[0 ] <<8 ) | regval[1 ]); + + raw >>= shift; + + *val = raw * scale * flag; + +break ;
Drop empty lines.
drivers/hwmon/ctf2301.c line 122
Quoted 8 lines
drivers/hwmon/ctf2301.c new file +8
-0+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.
drivers/hwmon/ctf2301.c line 135
Quoted 13 lines
drivers/hwmon/ctf2301.c new file +13
-0+{ +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.
drivers/hwmon/ctf2301.c line 138
Quoted 3 lines
drivers/hwmon/ctf2301.c new file +3
-0+ + speed = (regval[0 ] <<8 ) | regval[1 ]; +
speed can be 0.
drivers/hwmon/ctf2301.c line 139
Quoted 1 line
drivers/hwmon/ctf2301.c new file +1
-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.
Quoted 4 lines
drivers/hwmon/ctf2301.c new file +4
-0+break ; +default : +return -EOPNOTSUPP ; + }
drivers/hwmon/ctf2301.c line 153
Quoted 10 lines
drivers/hwmon/ctf2301.c new file +10
-0+ +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.
drivers/hwmon/ctf2301.c line 157
Quoted 4 lines
drivers/hwmon/ctf2301.c new file +4
-0+ +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().
drivers/hwmon/ctf2301.c line 164
Quoted 7 lines
drivers/hwmon/ctf2301.c new file +7
-0+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.
Quoted 162 lines
drivers/hwmon/ctf2301.c new file +162
-0+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 <[email protected]>" ); +MODULE_DESCRIPTION ("ctf2301 driver" ); +MODULE_LICENSE ("GPL" );
Hi Guenter, Thanks for your review. There are many things to improve in this driver.
Quoted 1 line · Guenter Roeck
On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
[...]
Quoted 1 line
Quoted 1 line
drivers/hwmon/ctf2301.c +0
-0
[...]
Quoted 24 lines
drivers/hwmon/ctf2301.c
Quoted 2 lines
drivers/hwmon/ctf2301.c +2
-0+ +#define CTF2301_LOCAL_TEMP_MSB 0x00LM90_REG_LOCAL_TEMPQuoted 1 line
+#define
CTF2301_RMT_TEMP_MSB0x01
LM90_REG_REMOTE_TEMPHQuoted 1 line
+#define
CTF2301_ALERT_STATUS0x02LM90_REG_STATUSQuoted 1 line
+#define
CTF2301_GLOBAL_CFG0x03LM90_REG_CONFIG1Quoted 1 line
+#define
CTF2301_RMT_TEMP_LSB0x10
LM90_REG_REMOTE_TEMPLQuoted 1 line
+#define
CTF2301_LOCAL_TEMP_LSB0x15TMP451_REG_LOCAL_TEMPLQuoted 1 line
+#define
CTF2301_ALERT_MASK0x16TMP461_REG_CHENSo 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.
Quoted 3 lines
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
Quoted 373 lines
Quoted 31 lines
drivers/hwmon/ctf2301.c +30
-0+#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)drivers/hwmon/ctf2301.c line 63
Quoted 10 lines
drivers/hwmon/ctf2301.c +10
-0+ +/* 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 parenthesisare relevant and need to be fixed.
drivers/hwmon/ctf2301.c line 65
Quoted 2 lines
drivers/hwmon/ctf2301.c +2
-0+struct ctf2301 { +struct i2c_client *client;Not used anywhere.
drivers/hwmon/ctf2301.c line 69
Quoted 4 lines
drivers/hwmon/ctf2301.c +4
-0+ +struct regmap *regmap; + +unsigned int pwm_freq_code;Unnecessary empty lines.
drivers/hwmon/ctf2301.c line 82
Quoted 13 lines
drivers/hwmon/ctf2301.c +13
-0+}; + +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_visiblefunction. It is, therefore this check is unnecessary.drivers/hwmon/ctf2301.c line 90
Quoted 8 lines
drivers/hwmon/ctf2301.c +8
-0+ +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.Quoted 4 lines
drivers/hwmon/ctf2301.c +4
-0+ + shift =5 ; + scale =1250 ; + reg_msb =CTF2301_RMT_TEMP_MSB ;drivers/hwmon/ctf2301.c line 104
Quoted 10 lines
drivers/hwmon/ctf2301.c +10
-0+ 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.drivers/hwmon/ctf2301.c
Quoted 2 lines
drivers/hwmon/ctf2301.c +2
-0+ +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.
drivers/hwmon/ctf2301.c line 114
Quoted 8 lines
drivers/hwmon/ctf2301.c +8
-0+ + raw = (s16)((regval[0 ] <<8 ) | regval[1 ]); + + raw >>= shift; + + *val = raw * scale * flag; + +break ;Drop empty lines.
drivers/hwmon/ctf2301.c line 122
Quoted 8 lines
drivers/hwmon/ctf2301.c +8
-0+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.
drivers/hwmon/ctf2301.c line 135
Quoted 13 lines
drivers/hwmon/ctf2301.c +13
-0+{ +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_LSBandCTF2301_TACH_COUNT_MSBare consecutive registers, so it should be possible to useregmap_bulk_read(). If not, consider usingregmap_multi_reg_read()and explain whyregmap_bulk_read()does not work.drivers/hwmon/ctf2301.c line 138
Quoted 3 lines
drivers/hwmon/ctf2301.c +3
-0+ + speed = (regval[0 ] <<8 ) | regval[1 ]; +speed can be 0.
drivers/hwmon/ctf2301.c line 139
Quoted 1 line
drivers/hwmon/ctf2301.c +1
-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.
Quoted 4 lines
drivers/hwmon/ctf2301.c +4
-0+break ; +default : +return -EOPNOTSUPP ; + }drivers/hwmon/ctf2301.c line 153
Quoted 10 lines
drivers/hwmon/ctf2301.c +10
-0+ +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.
drivers/hwmon/ctf2301.c line 157
Quoted 4 lines
drivers/hwmon/ctf2301.c +4
-0+ +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
-EINVALif it is out of range. Also consider usingDIV_ROUND_CLOSEST().drivers/hwmon/ctf2301.c line 164
Quoted 7 lines
drivers/hwmon/ctf2301.c +7
-0+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.
Quoted 162 lines
drivers/hwmon/ctf2301.c +162
-0+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 <[email protected]>" ); +MODULE_DESCRIPTION ("ctf2301 driver" ); +MODULE_LICENSE ("GPL" );
Guenter Roeck
maintainerQuoted 32 lines · Troy Mitchell
Hi Guenter, Thanks for your review. There are many things to improve in this driver.
Quoted 1 line · Guenter Roeck
On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
[...]
Quoted 1 line
Quoted 1 line
drivers/hwmon/ctf2301.c +0
-0[...]
Quoted 24 lines
drivers/hwmon/ctf2301.c
Quoted 2 lines
drivers/hwmon/ctf2301.c +2
-0+ +#define CTF2301_LOCAL_TEMP_MSB 0x00LM90_REG_LOCAL_TEMPQuoted 1 line
+#define
CTF2301_RMT_TEMP_MSB0x01
LM90_REG_REMOTE_TEMPHQuoted 1 line
+#define
CTF2301_ALERT_STATUS0x02LM90_REG_STATUSQuoted 1 line
+#define
CTF2301_GLOBAL_CFG0x03LM90_REG_CONFIG1Quoted 1 line
+#define
CTF2301_RMT_TEMP_LSB0x10
LM90_REG_REMOTE_TEMPLQuoted 1 line
+#define
CTF2301_LOCAL_TEMP_LSB0x15TMP451_REG_LOCAL_TEMPLQuoted 1 line
+#define
CTF2301_ALERT_MASK0x16TMP461_REG_CHENSo 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.
Quoted 9 lines
Quoted 3 lines
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
Quoted 40 lines · Guenter Roeck
Quoted 39 lines · Troy Mitchell
Hi Guenter, Thanks for your review. There are many things to improve in this driver.
Quoted 1 line · Guenter Roeck
On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
[...]
Quoted 1 line
Quoted 1 line
drivers/hwmon/ctf2301.c +0
-0[...]
Quoted 24 lines
drivers/hwmon/ctf2301.c
Quoted 2 lines
drivers/hwmon/ctf2301.c +2
-0+ +#define CTF2301_LOCAL_TEMP_MSB 0x00LM90_REG_LOCAL_TEMPQuoted 1 line
+#define
CTF2301_RMT_TEMP_MSB0x01
LM90_REG_REMOTE_TEMPHQuoted 1 line
+#define
CTF2301_ALERT_STATUS0x02LM90_REG_STATUSQuoted 1 line
+#define
CTF2301_GLOBAL_CFG0x03LM90_REG_CONFIG1Quoted 1 line
+#define
CTF2301_RMT_TEMP_LSB0x10
LM90_REG_REMOTE_TEMPLQuoted 1 line
+#define
CTF2301_LOCAL_TEMP_LSB0x15TMP451_REG_LOCAL_TEMPLQuoted 1 line
+#define
CTF2301_ALERT_MASK0x16TMP461_REG_CHENSo 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.
Quoted 3 lines
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.
Quoted 7 lines
Quoted 2 lines
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?
Quoted 3 lines
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
Quoted 3 lines
Guenter
Guenter Roeck
maintainerQuoted 44 lines · Troy Mitchell
Quoted 43 lines · Guenter Roeck
Quoted 41 lines · Troy Mitchell
Hi Guenter, Thanks for your review. There are many things to improve in this driver.
Quoted 1 line · Guenter Roeck
On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
[...]
Quoted 1 line
Quoted 1 line
drivers/hwmon/ctf2301.c +0
-0[...]
Quoted 24 lines
drivers/hwmon/ctf2301.c
Quoted 2 lines
drivers/hwmon/ctf2301.c +2
-0+ +#define CTF2301_LOCAL_TEMP_MSB 0x00LM90_REG_LOCAL_TEMPQuoted 1 line
+#define
CTF2301_RMT_TEMP_MSB0x01
LM90_REG_REMOTE_TEMPHQuoted 1 line
+#define
CTF2301_ALERT_STATUS0x02LM90_REG_STATUSQuoted 1 line
+#define
CTF2301_GLOBAL_CFG0x03LM90_REG_CONFIG1Quoted 1 line
+#define
CTF2301_RMT_TEMP_LSB0x10
LM90_REG_REMOTE_TEMPLQuoted 1 line
+#define
CTF2301_LOCAL_TEMP_LSB0x15TMP451_REG_LOCAL_TEMPLQuoted 1 line
+#define
CTF2301_ALERT_MASK0x16TMP461_REG_CHENSo 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.
Quoted 3 lines
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?
Quoted 7 lines
Quoted 4 lines
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