On Wed, Mar 13, 2019 at 04:20:28PM +0000, Grönke, Christian wrote: > Hi, > > Thanks for the fast answer. > > > -----Ursprüngliche Nachricht----- > > Von: Guenter Roeck <[email protected]> Im Auftrag von Guenter Roeck > > Gesendet: Mittwoch, 13. März 2019 16:20 > > An: Grönke, Christian <[email protected]> > > Cc: [email protected] > > Betreff: Re: PMBus driver for FSP/3Y Power device with non-standard VOUT > > values (LINEAR11 vs LINEAR16) > > > > On Wed, Mar 13, 2019 at 12:31:53PM +0000, Grönke, Christian wrote: > > > Hello all, > > > > > > I am currently working on a hwmon/pmbus driver for a PMBus capable 3Y > > Power/FSP power supply (YH5301-1EAR). > > > > > > The datasheet has limited information. But with the help of some > > online sources, other datasheets from the vendor and the pmbus sub- > > system I could figure out most of it. > > > > > > However, I did run in some troubles with the VOUT values. To my > > understanding (and from what I could validate) the device does encode > > _all_ values as "LINEAR11". Meaning all values have a mantissa of 11 bit > > and the exponent in the other 5 bits of the register word. This causes > > some troubles with the read out of the VOUT registers (e.g. READ_VOUT). > > The pmbus subsystem expects these values to be in "LINEAR16" encoding. > > Hence the full word register is the mantissa and the exponent is > > supposed to be in VOUT_MODE. > > > Sadly, the VOUT_MODE register isn't supported. It reads 0xFF. > > > > > ... violating its own specification... > > Well, yes. Judging by the spec of the FSP550-50ERS it also exceeds the > assumption that there are only 32 pages. The spec. of my current device does > not say anything with regards to the pages, but they seem to align with the > spec. of the FSP550-50ERS. Eg. page 0x22 shows the data for -12V.
Can you try to convince your e-mail system to split lines ? > But this is a different issue that I can come around with the hook in > read_word_data and a array that maps the page numbers. We could also increase the number of pages supported if it helps. > > > > > > In my first attempt to work around this, I provided a custom > > read_word_data function that would fixup the value. However, that did > > lead to problems with negative values and also had a precision loss > > (12,09V -> 12V). I tried to compensate by faking the values as 'direct' > > and adjusting the m/b/R values to match. This is also not perfect, as it > > is messy and also seems to report the wrong values in some cases. > > > > Messy is relative. Polluting generic code is just as messy. > > Indeed. > > > What are those cases where a wrong value is reported ? > > The values for lowest/highest seemed of. I don't have a specific example at > hand. I didn't save the read-outs during the testing. I could reproduce them. > However, they may also be wrong due to me doing the wrong conversion. > That is what I suspected. > > > > > > > > I think the best solution would be to prevent pmbus (more specifically > > 'pmbus_reg2data_linear') to treat the obtained value as LINEAR16. A > > quick hack shows me that this would work with all the reported values of > > the device. However, this means proving da vendor/device specific > > workaround by changing a generic component. > > > > > > I am not sure how you feel about this. Ultimately, I'd like to > > upstream the driver (and potential fix), so I would like to find a > > solution that is fine with you all. > > > > > > My current proposal would be to introduce a flag in > > pmbus_platform_data.flags that allows to disable the LINEAR16 switch in > > case the sensor has the class PSC_VOLTAGE_OUT. At least this seems the > > change with the smallest impact. I am not sure how to name the flag but > > to propose something I'd say 'PMBUS_VOUT_IS_LINEAR11' > > > > > > What do you think? > > > > Not sure if that is less messy. > > > > Have you tried to fake the VOUT_MODE command and to provide an exponent > > that makes sense ? While LINEAR16 does not support negative values, I > > don't immediately see that as a practical problem, unless the power > > supply indeed reports negative output voltages. > > Yes, I tried this at least partly. I remember that not all values looked > 'good'. Can't recall right now. The negative value of the -12V was definitely > a problem. > > However, writing this mail now casts a lot of doubt. I will revisit the code > and try the VOUT_MODE hack again. I know understand this stuff a bit better > than some days ago when I started. I will play around a bit more. > > That said, I still like the read outs I currently have with the hack in > place. They are a lot closer to what the other sensors on the board say. Makes sense, and in general I agree. 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. Thanks, Guenter
