diff mbox series

[RFC,6/6] hwmon: axi-fan-control: support temperature vs pwm points

Message ID 20210708120111.519444-7-nuno.sa@analog.com
State Accepted
Commit 2aee7e67bee7a5aa741bad6a0a472f108b29ad40
Headers show
Series AXI FAN new features and improvements | expand

Commit Message

Nuno Sa July 8, 2021, 12:01 p.m. UTC
The HW has some predefined points where it will associate a PWM value.
However some users might want to better set these points to their
usecases. This patch exposes these points as pwm auto_points:

 * pwm1_auto_point1_temp: temperature threshold below which PWM should be
   0%;
 * pwm1_auto_point2_temp: temperature threshold above which PWM should be
   25%;
 * pwm1_auto_point3_temp: temperature threshold below which PWM should be
   25%;
 * pwm1_auto_point4_temp: temperature threshold above which PWM should be
   50%;
 * pwm1_auto_point5_temp: temperature threshold below which PWM should be
   50%;
 * pwm1_auto_point6_temp: temperature threshold above which PWM should be
   75%;
 * pwm1_auto_point7_temp: temperature threshold below which PWM should be
   75%;
 * pwm1_auto_point8_temp: temperature threshold above which PWM should be
   100%;

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/hwmon/axi-fan-control.c | 74 ++++++++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 1 deletion(-)

Comments

Guenter Roeck July 17, 2021, 5:22 p.m. UTC | #1
On Thu, Jul 08, 2021 at 02:01:11PM +0200, Nuno Sá wrote:
> The HW has some predefined points where it will associate a PWM value.

> However some users might want to better set these points to their

> usecases. This patch exposes these points as pwm auto_points:

> 

>  * pwm1_auto_point1_temp: temperature threshold below which PWM should be

>    0%;

>  * pwm1_auto_point2_temp: temperature threshold above which PWM should be

>    25%;

>  * pwm1_auto_point3_temp: temperature threshold below which PWM should be

>    25%;

>  * pwm1_auto_point4_temp: temperature threshold above which PWM should be

>    50%;

>  * pwm1_auto_point5_temp: temperature threshold below which PWM should be

>    50%;

>  * pwm1_auto_point6_temp: temperature threshold above which PWM should be

>    75%;

>  * pwm1_auto_point7_temp: temperature threshold below which PWM should be

>    75%;

>  * pwm1_auto_point8_temp: temperature threshold above which PWM should be

>    100%;

> 


If I understand those correctly, half of those are really hysteresis points.
I think it would be better to express this with
	pwm1_auto_pointX_temp
	pwm1_auto_pointX_temp_hyst

where the hysteresis point is the temperature where the previous pwm value
is activated. In other words, change attribute names as follows:
    for 25%:
	pwm1_auto_point1_temp -> pwm1_auto_point1_temp_hyst
	pwm1_auto_point2_temp -> pwm1_auto_point1_temp
    for 50%:
	pwm1_auto_point3_temp -> pwm1_auto_point2_temp_hyst
	pwm1_auto_point4_temp -> pwm1_auto_point2_temp
    for 75%:
	pwm1_auto_point5_temp -> pwm1_auto_point3_temp_hyst
	pwm1_auto_point6_temp -> pwm1_auto_point3_temp
    for 100%:
	pwm1_auto_point7_temp -> pwm1_auto_point4_temp_hyst
	pwm1_auto_point8_temp -> pwm1_auto_point4_temp

Thanks,
Guenter
Nuno Sa July 19, 2021, 7:23 a.m. UTC | #2
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter

> Roeck

> Sent: Saturday, July 17, 2021 7:23 PM

> To: Sa, Nuno <Nuno.Sa@analog.com>

> Cc: linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; Rob

> Herring <robh+dt@kernel.org>; Jean Delvare <jdelvare@suse.com>

> Subject: Re: [RFC PATCH 6/6] hwmon: axi-fan-control: support

> temperature vs pwm points

> 

> [External]

> 

> On Thu, Jul 08, 2021 at 02:01:11PM +0200, Nuno Sá wrote:

> > The HW has some predefined points where it will associate a PWM

> value.

> > However some users might want to better set these points to their

> > usecases. This patch exposes these points as pwm auto_points:

> >

> >  * pwm1_auto_point1_temp: temperature threshold below which

> PWM should be

> >    0%;

> >  * pwm1_auto_point2_temp: temperature threshold above which

> PWM should be

> >    25%;

> >  * pwm1_auto_point3_temp: temperature threshold below which

> PWM should be

> >    25%;

> >  * pwm1_auto_point4_temp: temperature threshold above which

> PWM should be

> >    50%;

> >  * pwm1_auto_point5_temp: temperature threshold below which

> PWM should be

> >    50%;

> >  * pwm1_auto_point6_temp: temperature threshold above which

> PWM should be

> >    75%;

> >  * pwm1_auto_point7_temp: temperature threshold below which

> PWM should be

> >    75%;

> >  * pwm1_auto_point8_temp: temperature threshold above which

> PWM should be

> >    100%;

> >

> 

> If I understand those correctly, half of those are really hysteresis

> points.

> I think it would be better to express this with

> 	pwm1_auto_pointX_temp

> 	pwm1_auto_pointX_temp_hyst

> 

> where the hysteresis point is the temperature where the previous

> pwm value

> is activated. In other words, change attribute names as follows:

>     for 25%:

> 	pwm1_auto_point1_temp -> pwm1_auto_point1_temp_hyst

> 	pwm1_auto_point2_temp -> pwm1_auto_point1_temp

>     for 50%:

> 	pwm1_auto_point3_temp -> pwm1_auto_point2_temp_hyst

> 	pwm1_auto_point4_temp -> pwm1_auto_point2_temp

>     for 75%:

> 	pwm1_auto_point5_temp -> pwm1_auto_point3_temp_hyst

> 	pwm1_auto_point6_temp -> pwm1_auto_point3_temp

>     for 100%:

> 	pwm1_auto_point7_temp -> pwm1_auto_point4_temp_hyst

> 	pwm1_auto_point8_temp -> pwm1_auto_point4_temp

> 


Agree, it makes sense.

- Nuno Sá
diff mbox series

Patch

diff --git a/drivers/hwmon/axi-fan-control.c b/drivers/hwmon/axi-fan-control.c
index 59c9babb3850..80b216448f47 100644
--- a/drivers/hwmon/axi-fan-control.c
+++ b/drivers/hwmon/axi-fan-control.c
@@ -8,6 +8,7 @@ 
 #include <linux/clk.h>
 #include <linux/fpga/adi-axi-common.h>
 #include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -31,6 +32,14 @@ 
 #define ADI_REG_TACH_50_TOL	0x0154
 #define ADI_REG_TACH_75_TOL	0x0158
 #define ADI_REG_TACH_100_TOL	0x015c
+#define ADI_REG_TEMP_00_H	0x0100
+#define ADI_REG_TEMP_25_L	0x0104
+#define ADI_REG_TEMP_25_H	0x0108
+#define ADI_REG_TEMP_50_L	0x010c
+#define ADI_REG_TEMP_50_H	0x0110
+#define ADI_REG_TEMP_75_L	0x0114
+#define ADI_REG_TEMP_75_H	0x0118
+#define ADI_REG_TEMP_100_L	0x011c
 
 #define ADI_REG_IRQ_MASK	0x0040
 #define ADI_REG_IRQ_PENDING	0x0044
@@ -70,6 +79,39 @@  static inline u32 axi_ioread(const u32 reg,
 	return ioread32(ctl->base + reg);
 }
 
+/*
+ * The core calculates the temperature as:
+ *	T = /raw * 509.3140064 / 65535) - 280.2308787
+ */
+static ssize_t axi_fan_control_show(struct device *dev, struct device_attribute *da, char *buf)
+{
+	struct axi_fan_control_data *ctl = dev_get_drvdata(dev);
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	u32 temp = axi_ioread(attr->index, ctl);
+
+	temp = DIV_ROUND_CLOSEST_ULL(temp * 509314ULL, 65535) - 280230;
+
+	return sprintf(buf, "%u\n", temp);
+}
+
+static ssize_t axi_fan_control_store(struct device *dev, struct device_attribute *da,
+				     const char *buf, size_t count)
+{
+	struct axi_fan_control_data *ctl = dev_get_drvdata(dev);
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	u32 temp;
+	int ret;
+
+	ret = kstrtou32(buf, 10, &temp);
+	if (ret)
+		return ret;
+
+	temp = DIV_ROUND_CLOSEST_ULL((temp + 280230) * 65535ULL, 509314);
+	axi_iowrite(temp, attr->index, ctl);
+
+	return count;
+}
+
 static long axi_fan_control_get_pwm_duty(const struct axi_fan_control_data *ctl)
 {
 	u32 pwm_width = axi_ioread(ADI_REG_PWM_WIDTH, ctl);
@@ -418,6 +460,36 @@  static const struct hwmon_chip_info axi_chip_info = {
 	.info = axi_fan_control_info,
 };
 
+/* temperature threshold below which PWM should be 0% */
+static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point1_temp, axi_fan_control, ADI_REG_TEMP_00_H);
+/* temperature threshold above which PWM should be 25% */
+static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point2_temp, axi_fan_control, ADI_REG_TEMP_25_L);
+/* temperature threshold below which PWM should be 25% */
+static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point3_temp, axi_fan_control, ADI_REG_TEMP_25_H);
+/* temperature threshold above which PWM should be 50% */
+static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point4_temp, axi_fan_control, ADI_REG_TEMP_50_L);
+/* temperature threshold below which PWM should be 50% */
+static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point5_temp, axi_fan_control, ADI_REG_TEMP_50_H);
+/* temperature threshold above which PWM should be 75% */
+static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point6_temp, axi_fan_control, ADI_REG_TEMP_75_L);
+/* temperature threshold below which PWM should be 75% */
+static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point7_temp, axi_fan_control, ADI_REG_TEMP_75_H);
+/* temperature threshold above which PWM should be 100% */
+static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point8_temp, axi_fan_control, ADI_REG_TEMP_100_L);
+
+static struct attribute *axi_fan_control_attrs[] = {
+	&sensor_dev_attr_pwm1_auto_point1_temp.dev_attr.attr,
+	&sensor_dev_attr_pwm1_auto_point2_temp.dev_attr.attr,
+	&sensor_dev_attr_pwm1_auto_point3_temp.dev_attr.attr,
+	&sensor_dev_attr_pwm1_auto_point4_temp.dev_attr.attr,
+	&sensor_dev_attr_pwm1_auto_point5_temp.dev_attr.attr,
+	&sensor_dev_attr_pwm1_auto_point6_temp.dev_attr.attr,
+	&sensor_dev_attr_pwm1_auto_point7_temp.dev_attr.attr,
+	&sensor_dev_attr_pwm1_auto_point8_temp.dev_attr.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(axi_fan_control);
+
 static const u32 version_1_0_0 = ADI_AXI_PCORE_VER(1, 0, 'a');
 
 static const struct of_device_id axi_fan_control_of_match[] = {
@@ -502,7 +574,7 @@  static int axi_fan_control_probe(struct platform_device *pdev)
 							 name,
 							 ctl,
 							 &axi_chip_info,
-							 NULL);
+							 axi_fan_control_groups);
 
 	return PTR_ERR_OR_ZERO(ctl->hdev);
 }