On 7/5/24 03:59, Quentin Schulz wrote:[...]
Hi Guenter,
On 7/4/24 7:52 PM, Guenter Roeck wrote:
+ err = regmap_read(regmap,
+ channel ? AMC6821_REG_RTEMP_FAN_CTRL : AMC6821_REG_LTEMP_FAN_CTRL,
+ ®val);
+ if (err)
+ return err;
+ temps[1] = (regval & 0xF8) >> 1;
I think we want to use AMC6821_TEMP_LIMIT_MASK here instead of 0xF8?
I guess we could also use FIELD_GET?
Yes. The value in the register is in °C * 4, so that is going to be
temps[1] = FIELD_GET(regval, AMC6821_TEMP_LIMIT_MASK) * 4;
which improves readability and should also clarify the units a bit
better.
Note hat
(regval & 0xF8) >> 1;
resulted in the temperature in °C (shift right 1 instead of 3).
+ /*
+ * Passive cooling temperature. Range limit against low limit
+ * of both channels.
+ */
+ val = DIV_ROUND_CLOSEST(clamp_val(val, 0, 63000), 1000);
This was already in the original code, but I think 64°C should be doable as well? The datasheet says:
"""
The PSV ranges from 0°C to +64°C.
"""
Yes, but I am sure the datasheet is wrong here. The register has 6 active bits,
which means the highest possible value is 0x3f or 63.
And there's a PSV8 bit we can write, meaning we can do (1 << 8) with a step of 4°C which gives us 64°C? In a separate commit though, to not mix too many fixes into one, making it easier for people to identify and possibly revert them if necessary.Not sure I understand. Can you clarify ?
Temperature bit assignments in the datasheet are confusing. PSV3
means full degrees C, PSV8 means 32 degrees C. That is all in one register.
On the other side, L-TEMP0 reflects _4_ degrees C.
Am I missing something ?