On 2/2/21 6:34 AM, Erik Rosen wrote: > On Mon, Feb 1, 2021 at 10:38 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: >> >> On 2/1/21 8:48 AM, Erik Rosen wrote: >>> Hi Guenter >>> >>> >>> On Fri, Jan 29, 2021 at 4:50 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: >>>> >>>> Hi Erik, >>>> >>>> On 1/29/21 5:07 AM, Erik Rosen wrote: >>>> [ ... ] >>>>>> >>>>>>> + break; >>>>>>> + case PMBUS_VOUT_OV_FAULT_LIMIT: >>>>>>> + case PMBUS_VOUT_UV_FAULT_LIMIT: >>>>>>> + ret = pmbus_read_word_data(client, page, phase, reg); >>>>>>> + if (ret < 0) >>>>>>> + return ret; >>>>>>> + ret &= 0x07ff; >>>>>> >>>>>> This needs explanation. The BMR481 datasheet does not suggest that only >>>>>> 11 bits are valid. >>>>> >>>>> Ok, I will add a clarification. These registers use LINEAR11 whereas >>>>> LINEAR16 is expected for vout-related registers. >>>>> >>>> Is that the correct fix, then ? LINEAR11 means that the exponent is flexible, >>>> while it is fixed for LINEAR16. Just assuming that the exponents always match >>>> seems risky. Also, bit 11 in LINEAR11 is the sign bit, meaning negative limits >>>> (which seem at least theoretically be possible) would be reported as large >>>> positive values. >>> >>> The chip actually uses fixed exponents for representing all linear values and >>> the specification explicitly states the values of the LSB for all registers. >>> It also states that the limits can be handled as linear format when >>> writing _if_ >>> the exponent is the fixed value used for that limit. This means I'll have to >>> convert all limit writes to use the expected exponent. >> >> I understand the datasheet a bit differently. You are correct, the exponent for >> VOUT limits is always the same (LSB=0.00390625V, exponent -8), as it should be. >> But it also seems to me that the data format is LINEAR16, not LINEAR11. >> The datasheet says that the vout limit values are in bit 0..15. >> >> For other sensors, the datasheet is a bit ambiguous. I can read it as always >> using a fixed exponent when reading, or that it expects a fixed exponent when >> writing. It might make sense to check this with an actual chip instead of >> guessing. In this context, it is a bit odd that the datasheet keeps talking >> about a "System Register IOUT_EXP" without actually specifying it. >> >> In this context, you might want to watch out for register MFR_SVID_SLOW_SR_SELECTOR. >> Its value seems to impact the exponents used for IOUT and POUT. >> >> Also, I am not sure about the scale of other registers. It almost seems as if >> limit registers are LINEAR11, but actual READ_xxx registers are LINEAR16. >> For example, READ_IOUT is supposed to be bit 15:0 with the LSB determined >> by IOUT_EXP, but IOUT_OC_WARN_LIMIT is in bit 9:0. This would be a problem >> if READ_IOUT does not include the exponent, since it is interpreted as >> LINEAR11 and expects the exponent in bit 11..15. With an expected exponent >> of -1, the reported value would always be wrong. The same might apply >> to pretty much all READ_xxx registers (what a mess). > > I have tested all commands on a real chip and the behavior is as follows: > > All linear (both limits and READ_XXX) registers return the exponent i.e. use > the LINEAR11 format (even the vout related ones that really should use LINEAR16 > according to the pmbus standard), so reading registers is quite straightforward. > > I got hold of an excel-document from ST that describes the pmbus > commands and system registers of the chip in greater detail. Unfortunately > it has got 'STMicroelectronics Confidential' stamped all over it so I > can't really > share it publicly. > > Iout and pout data and limits are described as: > [15:11]: Encoded/Decoded with N programmed by HC_SUPPORT bit or IOUT_EXP > [10]: 0b0 > [9:0]: Y mantissa > > Other data and limits (including vout) are described as: > [15:11]: Encoded/Decoded with N=X, LSB=XX > [10]: Don't care (returns 0 in read access) > [9:0]: Y; max XXX > > where X... are fixed values. The sign bit is always zero. > > You are correct in that it is possible to change the fixed exponents used for > iout and pout by manipulating the system registers accessible via > extended commands. > > However, in a footnote on the bottom of the page it says: > (***) Number format - When Linear, data need to be formatted with N specified. > Data sent with N different from what reported, will be decoded as if N > is the one > reported. > Some chip designers are really quite inventive. > I can only interpret this as that the chip expects data written to a > linear register > to be encoded with the same exponent as it uses when the same register is read. > > So the simplest way of ensuring that the correct exponent is used when > writing to > the chip seems to be to first read the value from the chip, extract the exponent > and then adjust the value to use this exponent before writing it. > Guess so, and the value written needs to be clamped to [0, 0x3ff]. Thanks, Guenter