All of lore.kernel.org
 help / color / mirror / Atom feed
* PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
@ 2019-03-13 12:31 Grönke, Christian
  2019-03-13 15:19 ` Guenter Roeck
  0 siblings, 1 reply; 14+ messages in thread
From: Grönke, Christian @ 2019-03-13 12:31 UTC (permalink / raw)
  To: linux-hwmon; +Cc: Guenter Roeck

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.

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.

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?

Kind regards
Christian

-- 

Christian GRÖNKE

INFODAS GmbH ~ Rhonestraße 2 ~ 50765 Köln ~ Germany
Phone: +49 221 70912 187 ~ Mail: c.groenke@infodas.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-13 12:31 PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16) Grönke, Christian
@ 2019-03-13 15:19 ` Guenter Roeck
  2019-03-13 16:20   ` AW: " Grönke, Christian
  2019-03-13 16:21   ` Guenter Roeck
  0 siblings, 2 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-03-13 15:19 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

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...

> 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.
What are those cases where a wrong value is reported ?

> 
> 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.

Thanks,
Guenter

P.s.: I asked for more information from the vendor, but I don't really expect to
get anything. Worth trying, though.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* AW: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-13 15:19 ` Guenter Roeck
@ 2019-03-13 16:20   ` Grönke, Christian
  2019-03-13 16:30     ` Guenter Roeck
  2019-03-13 16:21   ` Guenter Roeck
  1 sibling, 1 reply; 14+ messages in thread
From: Grönke, Christian @ 2019-03-13 16:20 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon

Hi,

Thanks for the fast answer.

> -----Ursprüngliche Nachricht-----
> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> Gesendet: Mittwoch, 13. März 2019 16:20
> An: Grönke, Christian <C.Groenke@infodas.de>
> Cc: linux-hwmon@vger.kernel.org
> 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.
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.

> 
> > 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.

> 
> >
> > 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.

> 
> Thanks,
> Guenter
> 
> P.s.: I asked for more information from the vendor, but I don't really
> expect to get anything. Worth trying, though.

I will do that. This was now the first round of poking around. I will have to go through our hardware supplier, so this might take some time.

Kind regards
Christian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-13 15:19 ` Guenter Roeck
  2019-03-13 16:20   ` AW: " Grönke, Christian
@ 2019-03-13 16:21   ` Guenter Roeck
  1 sibling, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-03-13 16:21 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

On Wed, Mar 13, 2019 at 08:19:41AM -0700, Guenter Roeck wrote:
> 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...
> 
> > 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.
> What are those cases where a wrong value is reported ?
> 
> > 
> > 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.
> 
As a follow-up: If the above is not feasible (eg the exponent changes
all the time, or the reported voltages can indeed be negative), an
alternative would be to implement support for IEEE half precision
floating point format, as specified in PMBus 1.3.1.

Downside is that a device using IEEE half precision floating point
values must not use LINEAR{11,16} at all. But that would still be
better than hacking the PMBus core code.

Guenter

> Thanks,
> Guenter
> 
> P.s.: I asked for more information from the vendor, but I don't really expect to
> get anything. Worth trying, though.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-13 16:20   ` AW: " Grönke, Christian
@ 2019-03-13 16:30     ` Guenter Roeck
  2019-03-13 17:35       ` AW: " Grönke, Christian
  0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2019-03-13 16:30 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

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 <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> > Gesendet: Mittwoch, 13. März 2019 16:20
> > An: Grönke, Christian <C.Groenke@infodas.de>
> > Cc: linux-hwmon@vger.kernel.org
> > 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* AW: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-13 16:30     ` Guenter Roeck
@ 2019-03-13 17:35       ` Grönke, Christian
  2019-03-14  3:18         ` Guenter Roeck
  0 siblings, 1 reply; 14+ messages in thread
From: Grönke, Christian @ 2019-03-13 17:35 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon

Hello Guenter,

> -----Ursprüngliche Nachricht-----
> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> Gesendet: Mittwoch, 13. März 2019 17:30
> An: Grönke, Christian <C.Groenke@infodas.de>
> Cc: linux-hwmon@vger.kernel.org
> Betreff: Re: PMBus driver for FSP/3Y Power device with non-standard VOUT
> values (LINEAR11 vs LINEAR16)
> 
> 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 <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> > > Gesendet: Mittwoch, 13. März 2019 16:20
> > > An: Grönke, Christian <C.Groenke@infodas.de>
> > > Cc: linux-hwmon@vger.kernel.org
> > > 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 ?

Maybe... I am forced to use Outlook. I was already happy that I got 
it to do proper commenting and text-only. However, I will check if I 
find something. Until then I try to manually format them.

> 
> > 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.

Yes, okay.

> 
> >
> > >
> > > > 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 off. 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 did a quick test with a faked VOUT_MODE and thus fixed exponent. 
This does work for the reported voltages. But as you said, only 
for positive values. Disregarding the negative line may be okay, 
as nothing is connected to it in my system. On the other handy, if 
it is reported, displaying it would be nice too.

The fixed exponent has its limits though. The values in eg. 
MFR_VOUT_[MIN|MAX] are not read correctly then. Of course,
I could add code to compensate this...

> 
> > >
> > > >
> > > > 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.

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.

Regards
Christian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-13 17:35       ` AW: " Grönke, Christian
@ 2019-03-14  3:18         ` Guenter Roeck
  2019-03-14 16:08           ` AW: " Grönke, Christian
  0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2019-03-14  3:18 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

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.

Guenter

---
From 2f80cad52914c674afc5a80a84f4756e1b12d803 Mon Sep 17 00:00:00 2001
From: Guenter Roeck <linux@roeck-us.net>
Date: Wed, 13 Mar 2019 14:36:28 -0700
Subject: [PATCH] hwmon: (pmbus) Add IEEE 754 half precision support to PMBus
 core

PMBus v1.3.1 adds IEEE 754 half precision as additional data format.
Add support for it.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/pmbus.h      |   2 +-
 drivers/hwmon/pmbus/pmbus_core.c | 135 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 132 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 1d24397d36ec..6477eb433790 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -374,7 +374,7 @@ enum pmbus_sensor_classes {
 
 #define PMBUS_PAGE_VIRTUAL	BIT(31)
 
-enum pmbus_data_format { linear = 0, direct, vid };
+enum pmbus_data_format { linear = 0, ieee754, direct, vid };
 enum vrm_version { vr11 = 0, vr12, vr13 };
 
 struct pmbus_driver_info {
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 2e2b5851139c..e828160a1793 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -619,6 +619,66 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)
 }
 
 /*
+ * Convert ieee754 sensor values to milli- or micro-units
+ * depending on sensor type.
+ *
+ * ieee754 data format:
+ *	bit 15:		sign
+ *	bit 10..14:	exponent
+ *	bit 0..9:	mantissa
+ * exponent=0:
+ *	v=(−1)^signbit * 2^(−14) * 0.significantbits
+ * exponent=1..30:
+ *	v=(−1)^signbit * 2^(exponent - 15) * 1.significantbits
+ * exponent=31:
+ *	v=NaN
+ *
+ * Add the number mantissa bits into the calculations for simplicity.
+ * To do that, add '10' to the exponent. By doing that, we can just add
+ * 0x400 to normal values and get the expected result.
+ */
+static long pmbus_reg2data_ieee754(struct pmbus_data *data,
+				   struct pmbus_sensor *sensor)
+{
+	int exponent;
+	bool sign;
+	long val;
+
+	/* only support half precision for now */
+	sign = sensor->data & 0x8000;
+	exponent = (sensor->data >> 10) & 0x1f;
+	val = sensor->data & 0x3ff;
+
+	if (exponent == 0) {			/* subnormal */
+		exponent = -(14 + 10);
+	} else if (exponent ==  0x1f) {		/* NaN, convert to min/max */
+		exponent = 0;
+		val = 65504;
+	} else {
+		exponent -= (15 + 10);		/* normal */
+		val |= 0x400;
+	}
+
+	/* scale result to milli-units for all sensors except fans */
+	if (sensor->class != PSC_FAN)
+		val = val * 1000L;
+
+	/* scale result to micro-units for power sensors */
+	if (sensor->class == PSC_POWER)
+		val = val * 1000L;
+
+	if (exponent >= 0)
+		val <<= exponent;
+	else
+		val >>= -exponent;
+
+	if (sign)
+		val = -val;
+
+	return val;
+}
+
+/*
  * Convert linear sensor values to milli- or micro-units
  * depending on sensor type.
  */
@@ -740,6 +800,9 @@ static long pmbus_reg2data(struct pmbus_data *data, struct pmbus_sensor *sensor)
 	case vid:
 		val = pmbus_reg2data_vid(data, sensor);
 		break;
+	case ieee754:
+		val = pmbus_reg2data_ieee754(data, sensor);
+		break;
 	case linear:
 	default:
 		val = pmbus_reg2data_linear(data, sensor);
@@ -748,8 +811,65 @@ static long pmbus_reg2data(struct pmbus_data *data, struct pmbus_sensor *sensor)
 	return val;
 }
 
-#define MAX_MANTISSA	(1023 * 1000)
-#define MIN_MANTISSA	(511 * 1000)
+#define MAX_IEEE_MANTISSA	(0x7ff * 1000)
+#define MIN_IEEE_MANTISSA	(0x400 * 1000)
+
+static u16 pmbus_data2reg_ieee754(struct pmbus_data *data,
+				  struct pmbus_sensor *sensor, long val)
+{
+	u16 exponent = (15 + 10);
+	long mantissa;
+	u16 sign = 0;
+
+	/* simple case */
+	if (val == 0)
+		return 0;
+
+	if (val < 0) {
+		sign = 0x8000;
+		val = -val;
+	}
+
+	/* Power is in uW. Convert to mW before converting. */
+	if (sensor->class == PSC_POWER)
+		val = DIV_ROUND_CLOSEST(val, 1000L);
+
+	/*
+	 * For simplicity, convert fan data to milli-units
+	 * before calculating the exponent.
+	 */
+	if (sensor->class == PSC_FAN)
+		val = val * 1000;
+
+	/* Reduce large mantissa until it fits into 10 bit */
+	while (val > MAX_IEEE_MANTISSA && exponent < 30) {
+		exponent++;
+		val >>= 1;
+	}
+	/*
+	 * Increase small mantissa to generate valid 'normal'
+	 * number
+	 */
+	while (val < MIN_IEEE_MANTISSA && exponent > 1) {
+		exponent--;
+		val <<= 1;
+	}
+
+	/* Convert mantissa from milli-units to units */
+	mantissa = DIV_ROUND_CLOSEST(val, 1000);
+
+	/* Ensure that resulting number is within range */
+	if (mantissa > 0x7ff)
+		mantissa = 0x7ff;
+	else if (mantissa < 0x400)
+		mantissa = 0x400;
+
+	/* Convert to sign, 5 bit exponent, 10 bit mantissa */
+	return sign | (mantissa & 0x3ff) | ((exponent << 10) & 0x7c00);
+}
+
+#define MAX_LIN_MANTISSA	(1023 * 1000)
+#define MIN_LIN_MANTISSA	(511 * 1000)
 
 static u16 pmbus_data2reg_linear(struct pmbus_data *data,
 				 struct pmbus_sensor *sensor, long val)
@@ -795,12 +915,12 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
 		val = val * 1000;
 
 	/* Reduce large mantissa until it fits into 10 bit */
-	while (val >= MAX_MANTISSA && exponent < 15) {
+	while (val >= MAX_LIN_MANTISSA && exponent < 15) {
 		exponent++;
 		val >>= 1;
 	}
 	/* Increase small mantissa to improve precision */
-	while (val < MIN_MANTISSA && exponent > -15) {
+	while (val < MIN_LIN_MANTISSA && exponent > -15) {
 		exponent--;
 		val <<= 1;
 	}
@@ -878,6 +998,9 @@ static u16 pmbus_data2reg(struct pmbus_data *data,
 	case vid:
 		regval = pmbus_data2reg_vid(data, sensor, val);
 		break;
+	case ieee754:
+		regval = pmbus_data2reg_ieee754(data, sensor, val);
+		break;
 	case linear:
 	default:
 		regval = pmbus_data2reg_linear(data, sensor, val);
@@ -1967,6 +2090,10 @@ static int pmbus_identify_common(struct i2c_client *client,
 			if (data->info->format[PSC_VOLTAGE_OUT] != direct)
 				return -ENODEV;
 			break;
+		case 3:	/* ieee 754 half precision */
+			if (data->info->format[PSC_VOLTAGE_OUT] != ieee754)
+				return -ENODEV;
+			break;
 		default:
 			return -ENODEV;
 		}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* AW: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-14  3:18         ` Guenter Roeck
@ 2019-03-14 16:08           ` Grönke, Christian
  2019-03-14 16:22             ` Guenter Roeck
  2019-03-14 16:53             ` Guenter Roeck
  0 siblings, 2 replies; 14+ messages in thread
From: Grönke, Christian @ 2019-03-14 16:08 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon

Hello Guenter,

> -----Ursprüngliche Nachricht-----
> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> Gesendet: Donnerstag, 14. März 2019 04:19
> An: Grönke, Christian <C.Groenke@infodas.de>
> Cc: linux-hwmon@vger.kernel.org
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-14 16:08           ` AW: " Grönke, Christian
@ 2019-03-14 16:22             ` Guenter Roeck
  2019-03-14 16:53             ` Guenter Roeck
  1 sibling, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-03-14 16:22 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

On Thu, Mar 14, 2019 at 04:08:32PM +0000, Grönke, Christian wrote:
> Hello Guenter,
> 
> > -----Ursprüngliche Nachricht-----
> > Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> > Gesendet: Donnerstag, 14. März 2019 04:19
> > An: Grönke, Christian <C.Groenke@infodas.de>
> > Cc: linux-hwmon@vger.kernel.org
> > 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.
> 

It is supposed to end up being 0 at the end. The valid range is
0x400..0x7ff, where bit 10 represents the '1' in the normalized number.
Bit 10 has to be removed when applying the value to the final ieee754
representation. I thought that was easier to understand than setting
the values to 0x3ff and 0x0 (which would look even more confusing
to the casual reader). I think I'll add a comment at the top
explaining the logic.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-14 16:08           ` AW: " Grönke, Christian
  2019-03-14 16:22             ` Guenter Roeck
@ 2019-03-14 16:53             ` Guenter Roeck
  2019-03-15 10:19               ` AW: " Grönke, Christian
  1 sibling, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2019-03-14 16:53 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

Hi Christian,
On Thu, Mar 14, 2019 at 04:08:32PM +0000, Grönke, Christian wrote:
> 
> 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...
> 

Wondering ... I would have thought that it should be possible to implement
a simplified linear11 <--> ieee754 conversion, without converting to a
scaled integer first. Have you tried that ?

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 14+ messages in thread

* AW: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-14 16:53             ` Guenter Roeck
@ 2019-03-15 10:19               ` Grönke, Christian
  2019-03-15 18:29                 ` Guenter Roeck
  0 siblings, 1 reply; 14+ messages in thread
From: Grönke, Christian @ 2019-03-15 10:19 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon

Hello Guenter,

> -----Ursprüngliche Nachricht-----
> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> Gesendet: Donnerstag, 14. März 2019 17:53
> An: Grönke, Christian <C.Groenke@infodas.de>
> Cc: linux-hwmon@vger.kernel.org
> Betreff: Re: PMBus driver for FSP/3Y Power device with non-standard VOUT
> values (LINEAR11 vs LINEAR16)
> 
> Hi Christian,
> On Thu, Mar 14, 2019 at 04:08:32PM +0000, Grönke, Christian wrote:
> >
> > 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...
> >
> 
> Wondering ... I would have thought that it should be possible to
> implement a simplified linear11 <--> ieee754 conversion, without
> converting to a scaled integer first. Have you tried that ?

There might be a more efficient and elegant way. I toyed around with
the conversion for a moment but didn't figure out a working way 
quickly.
Right now I need to integrate the driver and get it working in the
System so that the depended application could can be worked on.
I might revisit the current conversion code after that and try to
find a "better" way.

Kind regards
Christian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-15 10:19               ` AW: " Grönke, Christian
@ 2019-03-15 18:29                 ` Guenter Roeck
  2019-03-15 19:38                   ` Guenter Roeck
  0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2019-03-15 18:29 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

On Fri, Mar 15, 2019 at 10:19:33AM +0000, Grönke, Christian wrote:
> Hello Guenter,
> 
> > -----Ursprüngliche Nachricht-----
> > Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> > Gesendet: Donnerstag, 14. März 2019 17:53
> > An: Grönke, Christian <C.Groenke@infodas.de>
> > Cc: linux-hwmon@vger.kernel.org
> > Betreff: Re: PMBus driver for FSP/3Y Power device with non-standard VOUT
> > values (LINEAR11 vs LINEAR16)
> > 
> > Hi Christian,
> > On Thu, Mar 14, 2019 at 04:08:32PM +0000, Grönke, Christian wrote:
> > >
> > > 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...
> > >
> > 
> > Wondering ... I would have thought that it should be possible to
> > implement a simplified linear11 <--> ieee754 conversion, without
> > converting to a scaled integer first. Have you tried that ?
> 
> There might be a more efficient and elegant way. I toyed around with
> the conversion for a moment but didn't figure out a working way 
> quickly.

This should work.

u16 lin2ieee(u16 lin11)
{
	s16 mantissa = ((s16)((lin11 & 0x7ff) << 5)) >> 5;
	int exponent = (((s16)lin11) >> 11) + 25;
	u16 sign = 0;

	/* simple case */
	if (mantissa == 0)
		return 0;

	if (mantissa < 0) {
		sign = 0x8000;
		mantissa = -mantissa;
	}

	while (mantissa < 0x400 && exponent > 1) {
		mantissa <<= 1;
		exponent--;
	}

	/* boundary checks */
	if (exponent > 30)
		return sign | 0x7bff;
	if (mantissa < 0x400)
		return sign | 0x0400;

	return sign | (exponent << 10) | (mantissa & 0x3ff);
}

Guenter

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-15 18:29                 ` Guenter Roeck
@ 2019-03-15 19:38                   ` Guenter Roeck
  2019-03-16 15:27                     ` Guenter Roeck
  0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2019-03-15 19:38 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

On Fri, Mar 15, 2019 at 11:29:45AM -0700, Guenter Roeck wrote:
> On Fri, Mar 15, 2019 at 10:19:33AM +0000, Grönke, Christian wrote:
> > Hello Guenter,
> > 
> > > -----Ursprüngliche Nachricht-----
> > > Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> > > Gesendet: Donnerstag, 14. März 2019 17:53
> > > An: Grönke, Christian <C.Groenke@infodas.de>
> > > Cc: linux-hwmon@vger.kernel.org
> > > Betreff: Re: PMBus driver for FSP/3Y Power device with non-standard VOUT
> > > values (LINEAR11 vs LINEAR16)
> > > 
> > > Hi Christian,
> > > On Thu, Mar 14, 2019 at 04:08:32PM +0000, Grönke, Christian wrote:
> > > >
> > > > 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...
> > > >
> > > 
> > > Wondering ... I would have thought that it should be possible to
> > > implement a simplified linear11 <--> ieee754 conversion, without
> > > converting to a scaled integer first. Have you tried that ?
> > 
> > There might be a more efficient and elegant way. I toyed around with
> > the conversion for a moment but didn't figure out a working way 
> > quickly.
> 
> This should work.
> 
> u16 lin2ieee(u16 lin11)
> {
> 	s16 mantissa = ((s16)((lin11 & 0x7ff) << 5)) >> 5;
> 	int exponent = (((s16)lin11) >> 11) + 25;
> 	u16 sign = 0;
> 
> 	/* simple case */
> 	if (mantissa == 0)
> 		return 0;
> 
> 	if (mantissa < 0) {
> 		sign = 0x8000;
> 		mantissa = -mantissa;
> 	}
> 
> 	while (mantissa < 0x400 && exponent > 1) {
> 		mantissa <<= 1;
> 		exponent--;
> 	}
> 
> 	/* boundary checks */
> 	if (exponent > 30)
> 		return sign | 0x7bff;
> 	if (mantissa < 0x400)
> 		return sign | 0x0400;
> 
> 	return sign | (exponent << 10) | (mantissa & 0x3ff);
> }
> 
Plus, just in case, the other direction.

u16 ieee2lin(u16 ieee)
{
	s16 mantissa = (ieee & 0x3ff) + 0x400;
	int exponent = ((ieee >> 10) & 0x1f) - 25; 
	int sign = ieee & 0x8000;

	/* simple case */
	if (ieee == sign)
		return 0;

	/* map into lin11 number space */
	mantissa >>= 1;
	exponent++;

	while (mantissa && exponent < -15) {
		exponent++;
		mantissa >>= 1;
	}

	/* boundary checks */
	if (exponent < -15)	/* number too small */
		return 0;

	if (sign)
		mantissa = -mantissa;

	return (exponent << 11) | (mantissa & 0x7ff);
}

Guenter

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16)
  2019-03-15 19:38                   ` Guenter Roeck
@ 2019-03-16 15:27                     ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-03-16 15:27 UTC (permalink / raw)
  To: Grönke, Christian; +Cc: linux-hwmon

On Fri, Mar 15, 2019 at 12:38:59PM -0700, Guenter Roeck wrote:
> On Fri, Mar 15, 2019 at 11:29:45AM -0700, Guenter Roeck wrote:
> > On Fri, Mar 15, 2019 at 10:19:33AM +0000, Grönke, Christian wrote:
> > > Hello Guenter,
> > > 
> > > > -----Ursprüngliche Nachricht-----
> > > > Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> > > > Gesendet: Donnerstag, 14. März 2019 17:53
> > > > An: Grönke, Christian <C.Groenke@infodas.de>
> > > > Cc: linux-hwmon@vger.kernel.org
> > > > Betreff: Re: PMBus driver for FSP/3Y Power device with non-standard VOUT
> > > > values (LINEAR11 vs LINEAR16)
> > > > 
> > > > Hi Christian,
> > > > On Thu, Mar 14, 2019 at 04:08:32PM +0000, Grönke, Christian wrote:
> > > > >
> > > > > 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...
> > > > >
> > > > 
> > > > Wondering ... I would have thought that it should be possible to
> > > > implement a simplified linear11 <--> ieee754 conversion, without
> > > > converting to a scaled integer first. Have you tried that ?
> > > 
> > > There might be a more efficient and elegant way. I toyed around with
> > > the conversion for a moment but didn't figure out a working way 
> > > quickly.
> > 
> > This should work.
> > 

Somewhat optimized versions, with additional boundary checks, kernel types,
and one less while loop.

Guenter

---
u16 lin2ieee(u16 lin11)
{
	s16 mantissa = ((s16)((lin11 & 0x7ff) << 5)) >> 5;
	int exponent = (((s16)lin11) >> 11) + 25;
	u16 sign = 0;

	/* simple case */
	if (mantissa == 0)
		return 0;

	if (mantissa < 0) {
		sign = 0x8000;
		mantissa = -mantissa;
	}
	while (mantissa < 0x400 && exponent > 1) {
		mantissa <<= 1;
		exponent--;
	}

	/* boundary checks */
	if (exponent > 30)
		return sign | 0x7bff;
	if (mantissa < 0x400)
		return sign | 0x0400;

	return sign | (exponent << 10) | (mantissa & 0x3ff);
}

u16 ieee2lin(u16 ieee)
{
	s16 mantissa = (ieee & 0x3ff) | 0x400;
	int exponent = ((ieee >> 10) & 0x1f); 
	u16 sign = ieee & 0x8000;
	int shift;

	/* 0 or too small */
	if (ieee == sign || exponent == 0)
		return 0;

	/* NaN */
	if (exponent == 0x1f)
		return sign ? 0xffff : 0xfbff;

	exponent -= 25;

	/* map into lin11 number space */
	mantissa >>= 1;
	exponent++;

	shift = -15 - exponent;
	if (shift > 0) {
		exponent = -15;
		mantissa >>= shift;
	}

	/* boundary checks */
	if (mantissa == 0)	/* number too small */
		return 0;

	if (sign)
		mantissa = -mantissa;

	return (exponent << 11) | (mantissa & 0x7ff);
}

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-03-16 15:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-13 12:31 PMBus driver for FSP/3Y Power device with non-standard VOUT values (LINEAR11 vs LINEAR16) Grönke, Christian
2019-03-13 15:19 ` Guenter Roeck
2019-03-13 16:20   ` AW: " Grönke, Christian
2019-03-13 16:30     ` Guenter Roeck
2019-03-13 17:35       ` AW: " Grönke, Christian
2019-03-14  3:18         ` Guenter Roeck
2019-03-14 16:08           ` AW: " Grönke, Christian
2019-03-14 16:22             ` Guenter Roeck
2019-03-14 16:53             ` Guenter Roeck
2019-03-15 10:19               ` AW: " Grönke, Christian
2019-03-15 18:29                 ` Guenter Roeck
2019-03-15 19:38                   ` Guenter Roeck
2019-03-16 15:27                     ` Guenter Roeck
2019-03-13 16:21   ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.