Re: [PATCH 1/1] Add ST STPDDC60 pmbus driver

[Guenter Roeck <linux@roeck-us.net>]

On 2/2/21 6:34 AM, Erik Rosen wrote:
> On Mon, Feb 1, 2021 at 10:38 PM Guenter Roeck <linux@roeck-us.net> 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@roeck-us.net> 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