Hello Guenter,
> -----Ursprüngliche Nachricht-----
> Von: Guenter Roeck <groeck7@xxxxxxxxx> Im Auftrag von Guenter Roeck
> Gesendet: Donnerstag, 14. März 2019 04:19
> An: Grönke, Christian <C.Groenke@xxxxxxxxxx>
> Cc: linux-hwmon@xxxxxxxxxxxxxxx
> Betreff: Re: PMBus driver for FSP/3Y Power device with non-standard VOUT
> values (LINEAR11 vs LINEAR16)
>
> Hi Christian,
>
> On Wed, Mar 13, 2019 at 05:35:38PM +0000, Grönke, Christian wrote:
> [ ... ]
> > >
> > > Our last e-mails overlapped; can you explore what it would take to
> > > add support for ieee754 half precision floating point ? We would
> > > need a new set of conversion functions in the core (ie
> > > pmbus_reg2data_ieee754 and pmbus_data2reg_ieee754), and your driver
> > > would have to implement the
> > > linear11 <-> ieee754 conversion. That may be a bit more work, but it
> > > would also be cleaner, and it should not affect precision.
> >
> > I give it a look tomorrow. If this presents a proper and clean way, to
> > avoid adding the hack in the generic code it might be worth it.
> >
>
> Please use the attached patch as starting point. Obviously I could not
> do much testing, but unit testing returned reasonable results. Note that
> we don't have to follow the pmbus spec to the letter - it is sufficient
> to declare the VOUT values to be in ieee754 format for your driver.
Thanks a lot for the patch. It did indeed help.
The framework code seemed to work fine. I used your code for the conversion:
linear11 -> 'scaled integer' -> ieee754
It provided a way to test the code and was easy for me as my tries to do
some other bit magic weren't successful. That means I partly tested the code
from pmbus_data2reg_ieee754 as my read_word function uses this for the
conversion. Of course not the module local function...
I have question wrt to the code:
[...]
> +static u16 pmbus_data2reg_ieee754(struct pmbus_data *data,
> + struct pmbus_sensor *sensor, long val) {
[...]
> +
> + /* Ensure that resulting number is within range */
> + if (mantissa > 0x7ff)
> + mantissa = 0x7ff;
> + else if (mantissa < 0x400)
> + mantissa = 0x400;
Setting the mantissa to 0x400...
> + /* Convert to sign, 5 bit exponent, 10 bit mantissa */
> + return sign | (mantissa & 0x3ff) | ((exponent << 10) & 0x7c00); }
... would mean it is cut to 0 here. I am not sure this is the intention.
I guess it should be 0x3FF then. Or in case it is supposed to be 0, then
'mantissa = 0;' would be easier to understand.
Regards
Christian
