Re: hwmon: (nct7802) buggy VSEN1/2/3 alarm

[Gilles Buloz <Gilles.Buloz@kontron.com>]

Le 25/11/2019 15:31, Guenter Roeck a écrit :
> On 11/25/19 5:13 AM, Gilles Buloz wrote:
>> Hi Guenter,
>>
>> According to the NCT7802Y datasheet, the REG_VOLTAGE_LIMIT_LSB definition is wrong and leads to wrong threshold registers used. It
>> should be :
>> static const u8 REG_VOLTAGE_LIMIT_LSB[2][5] = {
>>           { 0x46, 0x00, 0x40, 0x42, 0x44 },
>>           { 0x45, 0x00, 0x3f, 0x41, 0x43 },
>> };
>> With this definition, the right bit is set in "Voltage SMI Status Register @0x1e" for each threshold reached (using i2cget to check)
>>
>
> Good catch. Care to send a patch ?
As a fix for this is only useful with a fix for the problem below, maybe a single patch for both would be better.
>> But I'm unable to get any "ALARM" reported by the command "sensors" for VSEN1/2/3 = in2,in3,in4 because status for in0 is read
>> before (unless I set "ignore in0" in sensors file). The problem is that status bits in "Voltage SMI Status Register @0x1e" are
>> cleared when reading, and a read is done for each inX processed, so only the first inX has a chance to get its alarm bit set.
>> For this problem I don't see how to fix this easily; just to let you know ...
>>
> One possible fix would be to cache each alarm register and to clear the cache
> either after reading it (bitwise) or after a timeout. The latter is probably
> better to avoid stale information.
As we have status registers cleared at byte level and we want them to be cleared at bit level when each bit is read, I think a cache 
would be better. I suggest this :
- have a cached value for each status register, by default at 0x00
- when reading a register to get a bit, "OR" its byte value with its cached value, then use its cached value for processing.
- then clear the bit that has been processed from the cached value.

I think a timeout would not be obvious to set : at least the time for sensors to read all info (including when terminal is a serial 
line and output is slower) and to deal with possible latencies, but not too long...
>
> Guenter
> .
>