[PATCH] hwmon: (nct7802) Add time step attributes for tweaking responsiveness

[PATCH] hwmon: (nct7802) Add time step attributes for tweaking responsiveness

[Ronan Dalton]

The nct7802 chip exposes two registers that allow setting the time
interval between successive duty increases or decreases in Smart Fan
mode. The units are intervals of 0.1 second. The default value at power
on is 10, so 1 second.

Add sysfs attributes for step_up_time and step_down_time to allow
controlling the responsiveness of the fan speed. Values are represented
as milliseconds to the user. When set, the value is clamped to the valid
range of 100 to 25500 (0.1 to 25.5 seconds), and rounded to the nearest
multiple of 100.

Signed-off-by: Ronan Dalton <ronan.dalton@alliedtelesis.co.nz>
Cc: linux-kernel@vger.kernel.org
Cc: linux-hwmon@vger.kernel.org
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 Documentation/hwmon/nct7802.rst | 16 ++++++
 drivers/hwmon/nct7802.c         | 91 +++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/Documentation/hwmon/nct7802.rst b/Documentation/hwmon/nct7802.rst
index 8b7365a7cb32..366050ea595c 100644
--- a/Documentation/hwmon/nct7802.rst
+++ b/Documentation/hwmon/nct7802.rst
@@ -24,6 +24,22 @@ speed sensors.
 
 Smart Fan™ speed control is available via pwmX_auto_point attributes.
 
+Sysfs Attributes
+----------------
+
+Sysfs attributes unique to this chip are documented below. For common
+attributes, see Documentation/hwmon/sysfs-interface.rst.
+
+step_up_time
+			Time interval between successive duty cycle increases
+			when in Smart Fan mode. Specified in milliseconds and
+			rounded to intervals of 100 in the range 100-25500.
+
+step_down_time
+			Time interval between successive duty cycle decreases
+			when in Smart Fan mode. Specified in milliseconds and
+			rounded to intervals of 100 in the range 100-25500.
+
 Tested Boards and BIOS Versions
 -------------------------------
 
diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
index 8c9351da12c6..f30123fd908c 100644
--- a/drivers/hwmon/nct7802.c
+++ b/drivers/hwmon/nct7802.c
@@ -47,6 +47,8 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
 #define REG_PWM(x)		(0x60 + (x))
 #define REG_SMARTFAN_EN(x)      (0x64 + (x) / 2)
 #define SMARTFAN_EN_SHIFT(x)    ((x) % 2 * 4)
+#define REG_SMARTFAN_STEP_UP_TIME	0x6e
+#define REG_SMARTFAN_STEP_DOWN_TIME	0x6f
 #define REG_VENDOR_ID		0xfd
 #define REG_CHIP_ID		0xfe
 #define REG_VERSION_ID		0xff
@@ -560,6 +562,77 @@ beep_store(struct device *dev, struct device_attribute *attr, const char *buf,
 	return err ? : count;
 }
 
+static ssize_t step_time_show(struct device *dev, struct device_attribute *attr,
+			      char *buf, bool step_up)
+{
+	struct nct7802_data *data = dev_get_drvdata(dev);
+	unsigned int reg, val;
+	int ret;
+
+	if (step_up)
+		reg = REG_SMARTFAN_STEP_UP_TIME;
+	else
+		reg = REG_SMARTFAN_STEP_DOWN_TIME;
+
+	ret = regmap_read(data->regmap, reg, &val);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%u\n", val * 100); /* Convert from ds to ms */
+}
+
+static ssize_t step_up_time_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	return step_time_show(dev, attr, buf, true);
+}
+
+static ssize_t step_down_time_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	return step_time_show(dev, attr, buf, false);
+}
+
+static ssize_t step_time_store(struct device *dev,
+			       struct device_attribute *attr, const char *buf,
+			       size_t count, bool step_up)
+{
+	struct nct7802_data *data = dev_get_drvdata(dev);
+	unsigned long val;
+	unsigned int reg;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	/* Clamp range, and convert from ms to ds */
+	val = DIV_ROUND_CLOSEST(clamp_val(val, 100, 25500), 100);
+
+	if (step_up)
+		reg = REG_SMARTFAN_STEP_UP_TIME;
+	else
+		reg = REG_SMARTFAN_STEP_DOWN_TIME;
+
+	ret = regmap_write(data->regmap, reg, val);
+
+	return ret ? : count;
+}
+
+static ssize_t step_up_time_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	return step_time_store(dev, attr, buf, count, true);
+}
+
+static ssize_t step_down_time_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	return step_time_store(dev, attr, buf, count, false);
+}
+
 static SENSOR_DEVICE_ATTR_RW(temp1_type, temp_type, 0);
 static SENSOR_DEVICE_ATTR_2_RO(temp1_input, temp, 0x01, REG_TEMP_LSB);
 static SENSOR_DEVICE_ATTR_2_RW(temp1_min, temp, 0x31, 0);
@@ -975,12 +1048,30 @@ static const struct attribute_group nct7802_auto_point_group = {
 	.attrs = nct7802_auto_point_attrs,
 };
 
+/* 7.2.102 0x6E FANCTL Step Up Time Register */
+static SENSOR_DEVICE_ATTR_RW(step_up_time, step_up_time, 0);
+
+/* 7.2.103 0x6F FANCTL Step Down Time Register */
+static SENSOR_DEVICE_ATTR_RW(step_down_time, step_down_time, 0);
+
+static struct attribute *nct7802_step_time_attrs[] = {
+	&sensor_dev_attr_step_up_time.dev_attr.attr,
+	&sensor_dev_attr_step_down_time.dev_attr.attr,
+
+	NULL
+};
+
+static const struct attribute_group nct7802_step_time_group = {
+	.attrs = nct7802_step_time_attrs,
+};
+
 static const struct attribute_group *nct7802_groups[] = {
 	&nct7802_temp_group,
 	&nct7802_in_group,
 	&nct7802_fan_group,
 	&nct7802_pwm_group,
 	&nct7802_auto_point_group,
+	&nct7802_step_time_group,
 	NULL
 };
 
-- 
2.53.0

Re: [PATCH] hwmon: (nct7802) Add time step attributes for tweaking responsiveness

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] kstrtoul is called with base 0 for a time interval, allowing unintended octal parsing.
--

commit 3cecff14f185e191b2b2e66dc9e9e9cb80c25244
Author: Ronan Dalton <ronan.dalton@alliedtelesis.co.nz>

hwmon: (nct7802) Add time step attributes for tweaking responsiveness

This commit adds sysfs attributes to configure the step up and step down
time intervals for the Smart Fan mode on the nct7802 chip. The user-facing
values are specified in milliseconds and clamped to valid ranges.

> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
> index 8c9351da12c6e..f30123fd908cd 100644
> --- a/drivers/hwmon/nct7802.c
> +++ b/drivers/hwmon/nct7802.c
[ ... ]
> +static ssize_t step_time_store(struct device *dev,
> +			       struct device_attribute *attr, const char *buf,
> +			       size_t count, bool step_up)
> +{
> +	struct nct7802_data *data = dev_get_drvdata(dev);
> +	unsigned long val;
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);

Does this allow unintended octal parsing?

Using base 0 means that a zero-padded string written from a userspace script
(e.g., echo 0150 > step_up_time) will be parsed as an octal number rather
than decimal.

Should this use base 10 instead to prevent surprising behavior and match
other standard sysfs attribute parsers in this driver?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512041607.988078-2-ronan.dalton@alliedtelesis.co.nz?part=1

Re: [PATCH] hwmon: (nct7802) Add time step attributes for tweaking responsiveness

[Guenter Roeck]

On 5/13/26 16:31, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] kstrtoul is called with base 0 for a time interval, allowing unintended octal parsing.
> --
> 
> commit 3cecff14f185e191b2b2e66dc9e9e9cb80c25244
> Author: Ronan Dalton <ronan.dalton@alliedtelesis.co.nz>
> 
> hwmon: (nct7802) Add time step attributes for tweaking responsiveness
> 
> This commit adds sysfs attributes to configure the step up and step down
> time intervals for the Smart Fan mode on the nct7802 chip. The user-facing
> values are specified in milliseconds and clamped to valid ranges.
> 
>> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
>> index 8c9351da12c6e..f30123fd908cd 100644
>> --- a/drivers/hwmon/nct7802.c
>> +++ b/drivers/hwmon/nct7802.c
> [ ... ]
>> +static ssize_t step_time_store(struct device *dev,
>> +			       struct device_attribute *attr, const char *buf,
>> +			       size_t count, bool step_up)
>> +{
>> +	struct nct7802_data *data = dev_get_drvdata(dev);
>> +	unsigned long val;
>> +	unsigned int reg;
>> +	int ret;
>> +
>> +	ret = kstrtoul(buf, 0, &val);
> 
> Does this allow unintended octal parsing?
> 
> Using base 0 means that a zero-padded string written from a userspace script
> (e.g., echo 0150 > step_up_time) will be parsed as an octal number rather
> than decimal.
> 
> Should this use base 10 instead to prevent surprising behavior and match
> other standard sysfs attribute parsers in this driver?
> 
Agreed. Please fix.

Thanks,
Guenter

Re: [PATCH] hwmon: (nct7802) Add time step attributes for tweaking responsiveness

[Ronan Dalton]

Hi.

On Wed, 2026-05-13 at 23:31 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Low] kstrtoul is called with base 0 for a time interval, allowing
> unintended octal parsing.
> [ ... ]
> > +static ssize_t step_time_store(struct device *dev,
> > +                              struct device_attribute *attr, const
> > char *buf,
> > +                              size_t count, bool step_up)
> > +{
> > +       struct nct7802_data *data = dev_get_drvdata(dev);
> > +       unsigned long val;
> > +       unsigned int reg;
> > +       int ret;
> > +
> > +       ret = kstrtoul(buf, 0, &val);
> 
> Does this allow unintended octal parsing?
> 
> Using base 0 means that a zero-padded string written from a userspace
> script
> (e.g., echo 0150 > step_up_time) will be parsed as an octal number
> rather
> than decimal.
> 
> Should this use base 10 instead to prevent surprising behavior and
> match
> other standard sysfs attribute parsers in this driver?
> 
Currently in this driver the following functions accept any number
format (base 0) when parsing an input number:

- temp_type_store
- pwm_store
- pwm_enable_store

The following functions interpret input numbers as base 10:

- in_store
- temp_store
- fan_min_store
- beep_store

So the driver isn't currently consistent in this regard.

Looking at the other usages of the *strto* functions in drivers/hwmon
does show a strong preference for enforcing base 10, and the example in
hwmon/sysfs-interface.rst does the same.

As such, I will change the line to the following:

ret = kstrtoul(buf, 10, &val);

Regards,
Ronan.

[PATCH] hwmon: (nct7802) Add time step attributes for tweaking responsiveness

[Ronan Dalton]

The nct7802 chip exposes two registers that allow setting the time
interval between successive duty increases or decreases in Smart Fan
mode. The units are intervals of 0.1 second. The default value at power
on is 10, so 1 second.

Add sysfs attributes for step_up_time and step_down_time to allow
controlling the responsiveness of the fan speed. Values are represented
as milliseconds to the user. When set, the value is clamped to the valid
range of 100 to 25500 (0.1 to 25.5 seconds), and rounded to the nearest
multiple of 100.

Signed-off-by: Ronan Dalton <ronan.dalton@alliedtelesis.co.nz>
Cc: linux-kernel@vger.kernel.org
Cc: linux-hwmon@vger.kernel.org
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- Require base 10 when parsing input number in step_time_store

 Documentation/hwmon/nct7802.rst | 16 ++++++
 drivers/hwmon/nct7802.c         | 91 +++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/Documentation/hwmon/nct7802.rst b/Documentation/hwmon/nct7802.rst
index 8b7365a7cb32..366050ea595c 100644
--- a/Documentation/hwmon/nct7802.rst
+++ b/Documentation/hwmon/nct7802.rst
@@ -24,6 +24,22 @@ speed sensors.
 
 Smart Fan™ speed control is available via pwmX_auto_point attributes.
 
+Sysfs Attributes
+----------------
+
+Sysfs attributes unique to this chip are documented below. For common
+attributes, see Documentation/hwmon/sysfs-interface.rst.
+
+step_up_time
+			Time interval between successive duty cycle increases
+			when in Smart Fan mode. Specified in milliseconds and
+			rounded to intervals of 100 in the range 100-25500.
+
+step_down_time
+			Time interval between successive duty cycle decreases
+			when in Smart Fan mode. Specified in milliseconds and
+			rounded to intervals of 100 in the range 100-25500.
+
 Tested Boards and BIOS Versions
 -------------------------------
 
diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
index 8c9351da12c6..faa2988e40f8 100644
--- a/drivers/hwmon/nct7802.c
+++ b/drivers/hwmon/nct7802.c
@@ -47,6 +47,8 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
 #define REG_PWM(x)		(0x60 + (x))
 #define REG_SMARTFAN_EN(x)      (0x64 + (x) / 2)
 #define SMARTFAN_EN_SHIFT(x)    ((x) % 2 * 4)
+#define REG_SMARTFAN_STEP_UP_TIME	0x6e
+#define REG_SMARTFAN_STEP_DOWN_TIME	0x6f
 #define REG_VENDOR_ID		0xfd
 #define REG_CHIP_ID		0xfe
 #define REG_VERSION_ID		0xff
@@ -560,6 +562,77 @@ beep_store(struct device *dev, struct device_attribute *attr, const char *buf,
 	return err ? : count;
 }
 
+static ssize_t step_time_show(struct device *dev, struct device_attribute *attr,
+			      char *buf, bool step_up)
+{
+	struct nct7802_data *data = dev_get_drvdata(dev);
+	unsigned int reg, val;
+	int ret;
+
+	if (step_up)
+		reg = REG_SMARTFAN_STEP_UP_TIME;
+	else
+		reg = REG_SMARTFAN_STEP_DOWN_TIME;
+
+	ret = regmap_read(data->regmap, reg, &val);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%u\n", val * 100); /* Convert from ds to ms */
+}
+
+static ssize_t step_up_time_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	return step_time_show(dev, attr, buf, true);
+}
+
+static ssize_t step_down_time_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	return step_time_show(dev, attr, buf, false);
+}
+
+static ssize_t step_time_store(struct device *dev,
+			       struct device_attribute *attr, const char *buf,
+			       size_t count, bool step_up)
+{
+	struct nct7802_data *data = dev_get_drvdata(dev);
+	unsigned long val;
+	unsigned int reg;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &val);
+	if (ret < 0)
+		return ret;
+
+	/* Clamp range, and convert from ms to ds */
+	val = DIV_ROUND_CLOSEST(clamp_val(val, 100, 25500), 100);
+
+	if (step_up)
+		reg = REG_SMARTFAN_STEP_UP_TIME;
+	else
+		reg = REG_SMARTFAN_STEP_DOWN_TIME;
+
+	ret = regmap_write(data->regmap, reg, val);
+
+	return ret ? : count;
+}
+
+static ssize_t step_up_time_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	return step_time_store(dev, attr, buf, count, true);
+}
+
+static ssize_t step_down_time_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	return step_time_store(dev, attr, buf, count, false);
+}
+
 static SENSOR_DEVICE_ATTR_RW(temp1_type, temp_type, 0);
 static SENSOR_DEVICE_ATTR_2_RO(temp1_input, temp, 0x01, REG_TEMP_LSB);
 static SENSOR_DEVICE_ATTR_2_RW(temp1_min, temp, 0x31, 0);
@@ -975,12 +1048,30 @@ static const struct attribute_group nct7802_auto_point_group = {
 	.attrs = nct7802_auto_point_attrs,
 };
 
+/* 7.2.102 0x6E FANCTL Step Up Time Register */
+static SENSOR_DEVICE_ATTR_RW(step_up_time, step_up_time, 0);
+
+/* 7.2.103 0x6F FANCTL Step Down Time Register */
+static SENSOR_DEVICE_ATTR_RW(step_down_time, step_down_time, 0);
+
+static struct attribute *nct7802_step_time_attrs[] = {
+	&sensor_dev_attr_step_up_time.dev_attr.attr,
+	&sensor_dev_attr_step_down_time.dev_attr.attr,
+
+	NULL
+};
+
+static const struct attribute_group nct7802_step_time_group = {
+	.attrs = nct7802_step_time_attrs,
+};
+
 static const struct attribute_group *nct7802_groups[] = {
 	&nct7802_temp_group,
 	&nct7802_in_group,
 	&nct7802_fan_group,
 	&nct7802_pwm_group,
 	&nct7802_auto_point_group,
+	&nct7802_step_time_group,
 	NULL
 };
 
-- 
2.53.0

Re: [PATCH] hwmon: (nct7802) Add time step attributes for tweaking responsiveness

[Ronan Dalton]

Oops, I accidentally sent the patch without (v2).

Please disregard.