Re: [PATCH] added kernel module for FTS sensor chip "Teutates"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/07/2016 12:39 AM, thilo.cestonaro@xxxxxxxxxxxxxx wrote:
On Mo, 2016-06-06 at 20:18 -0700, Guenter Roeck wrote:
On 06/06/2016 02:21 AM, thilo.cestonaro@xxxxxxxxxxxxxx wrote:

Hey Guenter!

Thanks for your patience and very detailed review!
I will change the code to address all you hints where no discussion is needed.
The others follow down here.

On Fr, 2016-06-03 at 22:51 -0700, Guenter Roeck wrote:


+#define FTSTEUTATES_WATCHDOG_RESOLUTION		   60
+
60 seconds (minimum) resolution ? Really ? This is very unusual.
Will double check and talk to the hw guy.
There is a register which sets the resolution to 1sec. but it wasn't documented :(.
So next "version" will have 1sec. resolution.

Almost sounds like a common Super-IO watchdog. There is usually a register
to set the resolution (seconds or minutes) and another register to set the
value (0-255). Drivers then usually select minutes if the requested timeout
is larger than 255 seconds.


+static ssize_t show_fan_fault(struct device *dev,
+	struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct ftsteutates_data *data = ftsteutates_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->fan_present[index] == 1 ? 0 : 1);
A non-present fan does not indicate a fan fault.
For me a fan fault was a not existing fan, as the alarm is handled via fan_alarm.
What is the understanding of fan_fault and fan_alarm?

Some fan controllers can detect if a fan is faulty, for example if it consumes
power but does not turn. Unless you know for sure that a fan is faulty, don't
report that it is. A non-existing fan is definitely not faulty.
About the presence register from Teutates Spec:
The present-detection is done by checking the Fan-Speed.
If the fan-speed is 0 or below fan-fault-speed all the time,
the fan will be handled as not present. If the fan-speed is
greater than fan-fault-speed, the fan is handled as present.

So the presence is the faulty fan detection?


It pretty much means that you can not use the presence detect flag to indicate
a fan fault. I would suggest to just drop the flag.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux