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. > > > > +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? Cheers, Thilo
<<attachment: smime.p7s>>