Negative value returns for sensor in tiogapass

Negative value returns for sensor in tiogapass

[Jayashree D]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

Classification: Public
Hi Team,

Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
After analysing the code in the dbus-sensors repo, I found the following issue.

dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors (github.com)<https://github.com/openbmc/dbus-sensors/blob/master/src/IpmbSensor.cpp#L267>

From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.

int16_t value = ((data[4] << 8) | data[3]);

Since the below logic is added, the values get shifted and getting negative values as output.

constexpr const size_t shift = 16 - 11; // 11bit into 16bit
value <<= shift;
value >>= shift;

Could you please suggest any idea to resolve this issue.

Regards,
Jayashree

::DISCLAIMER::
________________________________
The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
________________________________

[-- Attachment #2: Type: text/html, Size: 4559 bytes --]

Re: Negative value returns for sensor in tiogapass

[Ed Tanous]

On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
>
> Classification: Public
>
> Hi Team,
>
>
>
> Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
>
> After analysing the code in the dbus-sensors repo, I found the following issue.
>
>
>
> dbus-sensors/IpmbSensor.cpp at master · openbmc/dbus-sensors (github.com)
>
>
>
> From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
>
>
>
> int16_t value = ((data[4] << 8) | data[3]);
>
>
>
> Since the below logic is added, the values get shifted and getting negative values as output.
>
>
>
> constexpr const size_t shift = 16 - 11; // 11bit into 16bit
>
> value <<= shift;
>
> value >>= shift;
>
>
>
> Could you please suggest any idea to resolve this issue.

I haven't looked at this in detail, but we should follow whatever the
spec says here.  If whomever wrote this originally put in the wrong
math (which seems likely, given they were implementing 4 types and
probably only using one) then we should just get it fixed and do what
the spec says.

>
>
>
> Regards,
>
> Jayashree
>
>
>
> ::DISCLAIMER::
> ________________________________
> The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> ________________________________

RE: Negative value returns for sensor in tiogapass

[Jayashree D]

Classification: Confidential

Thanks Ed for your response.
I'll look into the specs.

-----Original Message-----
From: Ed Tanous <edtanous@google.com> 
Sent: Friday, February 26, 2021 9:57 PM
To: Jayashree D <jayashree-d@hcl.com>
Cc: openbmc@lists.ozlabs.org
Subject: Re: Negative value returns for sensor in tiogapass

[CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don’t click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]

On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
>
> Classification: Public
>
> Hi Team,
>
>
>
> Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
>
> After analysing the code in the dbus-sensors repo, I found the following issue.
>
>
>
> dbus-sensors/IpmbSensor.cpp at master · openbmc/dbus-sensors 
> (github.com)
>
>
>
> From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
>
>
>
> int16_t value = ((data[4] << 8) | data[3]);
>
>
>
> Since the below logic is added, the values get shifted and getting negative values as output.
>
>
>
> constexpr const size_t shift = 16 - 11; // 11bit into 16bit
>
> value <<= shift;
>
> value >>= shift;
>
>
>
> Could you please suggest any idea to resolve this issue.

I haven't looked at this in detail, but we should follow whatever the spec says here.  If whomever wrote this originally put in the wrong math (which seems likely, given they were implementing 4 types and probably only using one) then we should just get it fixed and do what the spec says.

>
>
>
> Regards,
>
> Jayashree
>
>
>
> ::DISCLAIMER::
> ________________________________
> The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> ________________________________

RE: Negative value returns for sensor in tiogapass

[Jayashree D]

Classification: Public

Hi Ed,

In the below link, PXE1410CVR and ADM1278HSC are using the same reading format.
I am not able to find any information on PXE1410CVR. If there is any spec available, could you please share it.
It will be helpful to analyze both the specs and fix the problem.

https://github.com/openbmc/dbus-sensors/blob/master/src/IpmbSensor.cpp#L144

Regards,
Jayashree


-----Original Message-----
From: Ed Tanous <edtanous@google.com> 
Sent: Friday, February 26, 2021 9:57 PM
To: Jayashree D <jayashree-d@hcl.com>
Cc: openbmc@lists.ozlabs.org
Subject: Re: Negative value returns for sensor in tiogapass

[CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don’t click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]

On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
>
> Classification: Public
>
> Hi Team,
>
>
>
> Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
>
> After analysing the code in the dbus-sensors repo, I found the following issue.
>
>
>
> dbus-sensors/IpmbSensor.cpp at master · openbmc/dbus-sensors 
> (github.com)
>
>
>
> From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
>
>
>
> int16_t value = ((data[4] << 8) | data[3]);
>
>
>
> Since the below logic is added, the values get shifted and getting negative values as output.
>
>
>
> constexpr const size_t shift = 16 - 11; // 11bit into 16bit
>
> value <<= shift;
>
> value >>= shift;
>
>
>
> Could you please suggest any idea to resolve this issue.

I haven't looked at this in detail, but we should follow whatever the spec says here.  If whomever wrote this originally put in the wrong math (which seems likely, given they were implementing 4 types and probably only using one) then we should just get it fixed and do what the spec says.

>
>
>
> Regards,
>
> Jayashree
>
>
>
> ::DISCLAIMER::
> ________________________________
> The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> ________________________________

Re: Negative value returns for sensor in tiogapass

[Ed Tanous]

On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
>
> Classification: Public
>
> Hi Ed,
>
> In the below link, PXE1410CVR and ADM1278HSC are using the same reading format.
> I am not able to find any information on PXE1410CVR. If there is any spec available, could you please share it.
> It will be helpful to analyze both the specs and fix the problem.

I don't have any specs available for those.  I would assume they
follow the pmbus spec though, you might start there.

>
> https://github.com/openbmc/dbus-sensors/blob/master/src/IpmbSensor.cpp#L144
>
> Regards,
> Jayashree
>
>
> -----Original Message-----
> From: Ed Tanous <edtanous@google.com>
> Sent: Friday, February 26, 2021 9:57 PM
> To: Jayashree D <jayashree-d@hcl.com>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: Negative value returns for sensor in tiogapass
>
> [CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don’t click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]
>
> On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
> >
> > Classification: Public
> >
> > Hi Team,
> >
> >
> >
> > Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
> >
> > After analysing the code in the dbus-sensors repo, I found the following issue.
> >
> >
> >
> > dbus-sensors/IpmbSensor.cpp at master · openbmc/dbus-sensors
> > (github.com)
> >
> >
> >
> > From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
> >
> >
> >
> > int16_t value = ((data[4] << 8) | data[3]);
> >
> >
> >
> > Since the below logic is added, the values get shifted and getting negative values as output.
> >
> >
> >
> > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >
> > value <<= shift;
> >
> > value >>= shift;
> >
> >
> >
> > Could you please suggest any idea to resolve this issue.
>
> I haven't looked at this in detail, but we should follow whatever the spec says here.  If whomever wrote this originally put in the wrong math (which seems likely, given they were implementing 4 types and probably only using one) then we should just get it fixed and do what the spec says.
>
> >
> >
> >
> > Regards,
> >
> > Jayashree
> >
> >
> >
> > ::DISCLAIMER::
> > ________________________________
> > The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> > ________________________________

RE: Negative value returns for sensor in tiogapass

[Jayashree D]

Classification: Public

Thanks Ed, I'll check it out.

-----Original Message-----
From: Ed Tanous <ed@tanous.net> 
Sent: Monday, March 8, 2021 9:37 PM
To: Jayashree D <jayashree-d@hcl.com>
Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
Subject: Re: Negative value returns for sensor in tiogapass

[CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don't click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]

On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
>
> Classification: Public
>
> Hi Ed,
>
> In the below link, PXE1410CVR and ADM1278HSC are using the same reading format.
> I am not able to find any information on PXE1410CVR. If there is any spec available, could you please share it.
> It will be helpful to analyze both the specs and fix the problem.

I don't have any specs available for those.  I would assume they follow the pmbus spec though, you might start there.

>
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fopenbmc%2Fdbus-sensors%2Fblob%2Fmaster%2Fsrc%2FIpmbSensor.cpp
> %23L144&amp;data=04%7C01%7Cjayashree-d%40hcl.com%7C8676d30f4d3a4dda1e0
> e08d8e24c4957%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C63750816456
> 8775248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5lhuUdfI%2BG75C8I1HDAaEH
> VP46%2Bz1r3nJV0ek3CiiR4%3D&amp;reserved=0
>
> Regards,
> Jayashree
>
>
> -----Original Message-----
> From: Ed Tanous <edtanous@google.com>
> Sent: Friday, February 26, 2021 9:57 PM
> To: Jayashree D <jayashree-d@hcl.com>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: Negative value returns for sensor in tiogapass
>
> [CAUTION: This Email is from outside the Organization. Unless you 
> trust the sender, Don't click links or open attachments as it may be a 
> Phishing email, which can steal your Information and compromise your 
> Computer.]
>
> On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
> >
> > Classification: Public
> >
> > Hi Team,
> >
> >
> >
> > Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
> >
> > After analysing the code in the dbus-sensors repo, I found the following issue.
> >
> >
> >
> > dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors
> > (github.com)
> >
> >
> >
> > From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
> >
> >
> >
> > int16_t value = ((data[4] << 8) | data[3]);
> >
> >
> >
> > Since the below logic is added, the values get shifted and getting negative values as output.
> >
> >
> >
> > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >
> > value <<= shift;
> >
> > value >>= shift;
> >
> >
> >
> > Could you please suggest any idea to resolve this issue.
>
> I haven't looked at this in detail, but we should follow whatever the spec says here.  If whomever wrote this originally put in the wrong math (which seems likely, given they were implementing 4 types and probably only using one) then we should just get it fixed and do what the spec says.
>
> >
> >
> >
> > Regards,
> >
> > Jayashree
> >
> >
> >
> > ::DISCLAIMER::
> > ________________________________
> > The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> > ________________________________

RE: Negative value returns for sensor in tiogapass

[Jayashree D]

Classification: Public

Hi Ed,

PMBus spec only have read and write format. In the below link, PXE VR uses 11 bit format. Also sign extend the 11bit
reading so that negatives show correctly.

https://github.com/openbmc/dbus-sensors/commit/e4a970d9aea97c7c1a11c63215e7d3cda2124e54#diff-135678dd2046935c5dd0be8e5a5a529d33231203149e786d57b15a3cc0cc1240

            constexpr const size_t shift = 16 - 11; // 11bit into 16bit
            value <<= shift;
            value >>= shift;

Could anyone from the intel team can clarify the need of above logic used in IpmbSensor.

Regards,
Jayashree

-----Original Message-----
From: Jayashree D 
Sent: Tuesday, March 9, 2021 4:48 PM
To: Ed Tanous <ed@tanous.net>
Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
Subject: RE: Negative value returns for sensor in tiogapass

Classification: Public

Thanks Ed, I'll check it out.

-----Original Message-----
From: Ed Tanous <ed@tanous.net>
Sent: Monday, March 8, 2021 9:37 PM
To: Jayashree D <jayashree-d@hcl.com>
Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
Subject: Re: Negative value returns for sensor in tiogapass

[CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don't click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]

On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
>
> Classification: Public
>
> Hi Ed,
>
> In the below link, PXE1410CVR and ADM1278HSC are using the same reading format.
> I am not able to find any information on PXE1410CVR. If there is any spec available, could you please share it.
> It will be helpful to analyze both the specs and fix the problem.

I don't have any specs available for those.  I would assume they follow the pmbus spec though, you might start there.

>
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fopenbmc%2Fdbus-sensors%2Fblob%2Fmaster%2Fsrc%2FIpmbSensor.cpp
> %23L144&amp;data=04%7C01%7Cjayashree-d%40hcl.com%7C8676d30f4d3a4dda1e0
> e08d8e24c4957%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C63750816456
> 8775248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5lhuUdfI%2BG75C8I1HDAaEH
> VP46%2Bz1r3nJV0ek3CiiR4%3D&amp;reserved=0
>
> Regards,
> Jayashree
>
>
> -----Original Message-----
> From: Ed Tanous <edtanous@google.com>
> Sent: Friday, February 26, 2021 9:57 PM
> To: Jayashree D <jayashree-d@hcl.com>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: Negative value returns for sensor in tiogapass
>
> [CAUTION: This Email is from outside the Organization. Unless you 
> trust the sender, Don't click links or open attachments as it may be a 
> Phishing email, which can steal your Information and compromise your 
> Computer.]
>
> On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
> >
> > Classification: Public
> >
> > Hi Team,
> >
> >
> >
> > Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
> >
> > After analysing the code in the dbus-sensors repo, I found the following issue.
> >
> >
> >
> > dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors
> > (github.com)
> >
> >
> >
> > From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
> >
> >
> >
> > int16_t value = ((data[4] << 8) | data[3]);
> >
> >
> >
> > Since the below logic is added, the values get shifted and getting negative values as output.
> >
> >
> >
> > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >
> > value <<= shift;
> >
> > value >>= shift;
> >
> >
> >
> > Could you please suggest any idea to resolve this issue.
>
> I haven't looked at this in detail, but we should follow whatever the spec says here.  If whomever wrote this originally put in the wrong math (which seems likely, given they were implementing 4 types and probably only using one) then we should just get it fixed and do what the spec says.
>
> >
> >
> >
> > Regards,
> >
> > Jayashree
> >
> >
> >
> > ::DISCLAIMER::
> > ________________________________
> > The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> > ________________________________

RE: Negative value returns for sensor in tiogapass

[Ren, Zhikui]

-----Original Message-----
From: openbmc <openbmc-bounces+zhikui.ren=intel.com@lists.ozlabs.org> On Behalf Of Jayashree D
Sent: Wednesday, March 17, 2021 12:39 AM
To: Ed Tanous <edtanous@google.com>
Cc: openbmc@lists.ozlabs.org
Subject: RE: Negative value returns for sensor in tiogapass

Classification: Public

Hi Ed,

PMBus spec only have read and write format. In the below link, PXE VR uses 11 bit format. Also sign extend the 11bit reading so that negatives show correctly.

https://github.com/openbmc/dbus-sensors/commit/e4a970d9aea97c7c1a11c63215e7d3cda2124e54#diff-135678dd2046935c5dd0be8e5a5a529d33231203149e786d57b15a3cc0cc1240

            constexpr const size_t shift = 16 - 11; // 11bit into 16bit
            value <<= shift;
            value >>= shift;

Could anyone from the intel team can clarify the need of above logic used in IpmbSensor.

[Ren, Zhikui]  This change was made to allow negative numbers be reported correctly.  This can happen during test. Without the change, 255degree will be reported and trip threshold event incorrectly. 
Exponent data is not used because it is always 0. 
 
Regards,
Jayashree

-----Original Message-----
From: Jayashree D
Sent: Tuesday, March 9, 2021 4:48 PM
To: Ed Tanous <ed@tanous.net>
Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
Subject: RE: Negative value returns for sensor in tiogapass

Classification: Public

Thanks Ed, I'll check it out.

-----Original Message-----
From: Ed Tanous <ed@tanous.net>
Sent: Monday, March 8, 2021 9:37 PM
To: Jayashree D <jayashree-d@hcl.com>
Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
Subject: Re: Negative value returns for sensor in tiogapass

[CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don't click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]

On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
>
> Classification: Public
>
> Hi Ed,
>
> In the below link, PXE1410CVR and ADM1278HSC are using the same reading format.
> I am not able to find any information on PXE1410CVR. If there is any spec available, could you please share it.
> It will be helpful to analyze both the specs and fix the problem.

I don't have any specs available for those.  I would assume they follow the pmbus spec though, you might start there.

>
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fopenbmc%2Fdbus-sensors%2Fblob%2Fmaster%2Fsrc%2FIpmbSensor.cpp
> %23L144&amp;data=04%7C01%7Cjayashree-d%40hcl.com%7C8676d30f4d3a4dda1e0
> e08d8e24c4957%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C63750816456
> 8775248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5lhuUdfI%2BG75C8I1HDAaEH
> VP46%2Bz1r3nJV0ek3CiiR4%3D&amp;reserved=0
>
> Regards,
> Jayashree
>
>
> -----Original Message-----
> From: Ed Tanous <edtanous@google.com>
> Sent: Friday, February 26, 2021 9:57 PM
> To: Jayashree D <jayashree-d@hcl.com>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: Negative value returns for sensor in tiogapass
>
> [CAUTION: This Email is from outside the Organization. Unless you 
> trust the sender, Don't click links or open attachments as it may be a 
> Phishing email, which can steal your Information and compromise your 
> Computer.]
>
> On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
> >
> > Classification: Public
> >
> > Hi Team,
> >
> >
> >
> > Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
> >
> > After analysing the code in the dbus-sensors repo, I found the following issue.
> >
> >
> >
> > dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors
> > (github.com)
> >
> >
> >
> > From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
> >
> >
> >
> > int16_t value = ((data[4] << 8) | data[3]);
> >
> >
> >
> > Since the below logic is added, the values get shifted and getting negative values as output.
> >
> >
> >
> > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >
> > value <<= shift;
> >
> > value >>= shift;
> >
> >
> >
> > Could you please suggest any idea to resolve this issue.
>
> I haven't looked at this in detail, but we should follow whatever the spec says here.  If whomever wrote this originally put in the wrong math (which seems likely, given they were implementing 4 types and probably only using one) then we should just get it fixed and do what the spec says.
>
> >
> >
> >
> > Regards,
> >
> > Jayashree
> >
> >
> >
> > ::DISCLAIMER::
> > ________________________________
> > The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> > ________________________________

RE: Negative value returns for sensor in tiogapass

[Jayashree D]

Classification: Public



-----Original Message-----
From: Ren, Zhikui <zhikui.ren@intel.com> 
Sent: Wednesday, March 17, 2021 11:48 PM
To: Jayashree D <jayashree-d@hcl.com>; Ed Tanous <edtanous@google.com>
Cc: openbmc@lists.ozlabs.org
Subject: RE: Negative value returns for sensor in tiogapass

[CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don't click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]

-----Original Message-----
From: openbmc <openbmc-bounces+zhikui.ren=intel.com@lists.ozlabs.org> On Behalf Of Jayashree D
Sent: Wednesday, March 17, 2021 12:39 AM
To: Ed Tanous <edtanous@google.com>
Cc: openbmc@lists.ozlabs.org
Subject: RE: Negative value returns for sensor in tiogapass

Classification: Public

Hi Ed,

PMBus spec only have read and write format. In the below link, PXE VR uses 11 bit format. Also sign extend the 11bit reading so that negatives show correctly.

https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fopenbmc%2Fdbus-sensors%2Fcommit%2Fe4a970d9aea97c7c1a11c63215e7d3cda2124e54%23diff-135678dd2046935c5dd0be8e5a5a529d33231203149e786d57b15a3cc0cc1240&amp;data=04%7C01%7Cjayashree-d%40hcl.com%7C1f0d1622ed8e4cfdd79108d8e9710e18%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C637516019071748468%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Jz4749uSr3qy7opyujowf98PmaVOwEViBuzKIT%2BK1v4%3D&amp;reserved=0

            constexpr const size_t shift = 16 - 11; // 11bit into 16bit
            value <<= shift;
            value >>= shift;

Could anyone from the intel team can clarify the need of above logic used in IpmbSensor.

[Ren, Zhikui]  This change was made to allow negative numbers be reported correctly.  This can happen during test. Without the change, 255degree will be reported and trip threshold event incorrectly.
Exponent data is not used because it is always 0.

Jayashree]
We need only below line in the code to process the HSC sensors value for tiogapass.
	int16_t value = ((data[4] << 8) | data[3]);

 Since the below logic is added, the values get shifted and getting negative values as output.
	 constexpr const size_t shift = 16 - 11; // 11bit into 16bit
	 value <<= shift;
	 value >>= shift;

Could you please suggest any idea to resolve this issue.

Regards,
Jayashree

-----Original Message-----
From: Jayashree D
Sent: Tuesday, March 9, 2021 4:48 PM
To: Ed Tanous <ed@tanous.net>
Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
Subject: RE: Negative value returns for sensor in tiogapass

Classification: Public

Thanks Ed, I'll check it out.

-----Original Message-----
From: Ed Tanous <ed@tanous.net>
Sent: Monday, March 8, 2021 9:37 PM
To: Jayashree D <jayashree-d@hcl.com>
Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
Subject: Re: Negative value returns for sensor in tiogapass

[CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don't click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]

On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
>
> Classification: Public
>
> Hi Ed,
>
> In the below link, PXE1410CVR and ADM1278HSC are using the same reading format.
> I am not able to find any information on PXE1410CVR. If there is any spec available, could you please share it.
> It will be helpful to analyze both the specs and fix the problem.

I don't have any specs available for those.  I would assume they follow the pmbus spec though, you might start there.

>
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fopenbmc%2Fdbus-sensors%2Fblob%2Fmaster%2Fsrc%2FIpmbSensor.cpp
> %23L144&amp;data=04%7C01%7Cjayashree-d%40hcl.com%7C8676d30f4d3a4dda1e0
> e08d8e24c4957%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C63750816456
> 8775248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5lhuUdfI%2BG75C8I1HDAaEH
> VP46%2Bz1r3nJV0ek3CiiR4%3D&amp;reserved=0
>
> Regards,
> Jayashree
>
>
> -----Original Message-----
> From: Ed Tanous <edtanous@google.com>
> Sent: Friday, February 26, 2021 9:57 PM
> To: Jayashree D <jayashree-d@hcl.com>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: Negative value returns for sensor in tiogapass
>
> [CAUTION: This Email is from outside the Organization. Unless you 
> trust the sender, Don't click links or open attachments as it may be a 
> Phishing email, which can steal your Information and compromise your 
> Computer.]
>
> On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
> >
> > Classification: Public
> >
> > Hi Team,
> >
> >
> >
> > Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
> >
> > After analysing the code in the dbus-sensors repo, I found the following issue.
> >
> >
> >
> > dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors
> > (github.com)
> >
> >
> >
> > From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
> >
> >
> >
> > int16_t value = ((data[4] << 8) | data[3]);
> >
> >
> >
> > Since the below logic is added, the values get shifted and getting negative values as output.
> >
> >
> >
> > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >
> > value <<= shift;
> >
> > value >>= shift;
> >
> >
> >
> > Could you please suggest any idea to resolve this issue.
>
> I haven't looked at this in detail, but we should follow whatever the spec says here.  If whomever wrote this originally put in the wrong math (which seems likely, given they were implementing 4 types and probably only using one) then we should just get it fixed and do what the spec says.
>
> >
> >
> >
> > Regards,
> >
> > Jayashree
> >
> >
> >
> > ::DISCLAIMER::
> > ________________________________
> > The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> > ________________________________

Re: Negative value returns for sensor in tiogapass

[Ed Tanous]

On Wed, Mar 17, 2021 at 11:18 AM Ren, Zhikui <zhikui.ren@intel.com> wrote:
>
>
>
> -----Original Message-----
> From: openbmc <openbmc-bounces+zhikui.ren=intel.com@lists.ozlabs.org> On Behalf Of Jayashree D
> Sent: Wednesday, March 17, 2021 12:39 AM
> To: Ed Tanous <edtanous@google.com>
> Cc: openbmc@lists.ozlabs.org
> Subject: RE: Negative value returns for sensor in tiogapass
>
> Classification: Public
>
> Hi Ed,
>
> PMBus spec only have read and write format. In the below link, PXE VR uses 11 bit format. Also sign extend the 11bit reading so that negatives show correctly.
>
> https://github.com/openbmc/dbus-sensors/commit/e4a970d9aea97c7c1a11c63215e7d3cda2124e54#diff-135678dd2046935c5dd0be8e5a5a529d33231203149e786d57b15a3cc0cc1240
>
>             constexpr const size_t shift = 16 - 11; // 11bit into 16bit
>             value <<= shift;
>             value >>= shift;
>
> Could anyone from the intel team can clarify the need of above logic used in IpmbSensor.
>
> [Ren, Zhikui]  This change was made to allow negative numbers be reported correctly.  This can happen during test. Without the change, 255degree will be reported and trip threshold event incorrectly.

Can you walk through what your test was?  It's sounding like Jayashree
was seeing incorrect values in a real world application.  Is it
possible we fixed something in a test by breaking something in the
real world?  Can you think of any other reasons why others would be
seeing different behavior?

> Exponent data is not used because it is always 0.
>
> Regards,
> Jayashree
>
> -----Original Message-----
> From: Jayashree D
> Sent: Tuesday, March 9, 2021 4:48 PM
> To: Ed Tanous <ed@tanous.net>
> Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
> Subject: RE: Negative value returns for sensor in tiogapass
>
> Classification: Public
>
> Thanks Ed, I'll check it out.
>
> -----Original Message-----
> From: Ed Tanous <ed@tanous.net>
> Sent: Monday, March 8, 2021 9:37 PM
> To: Jayashree D <jayashree-d@hcl.com>
> Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
> Subject: Re: Negative value returns for sensor in tiogapass
>
> [CAUTION: This Email is from outside the Organization. Unless you trust the sender, Don't click links or open attachments as it may be a Phishing email, which can steal your Information and compromise your Computer.]
>
> On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
> >
> > Classification: Public
> >
> > Hi Ed,
> >
> > In the below link, PXE1410CVR and ADM1278HSC are using the same reading format.
> > I am not able to find any information on PXE1410CVR. If there is any spec available, could you please share it.
> > It will be helpful to analyze both the specs and fix the problem.
>
> I don't have any specs available for those.  I would assume they follow the pmbus spec though, you might start there.
>
> >
> > https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Fopenbmc%2Fdbus-sensors%2Fblob%2Fmaster%2Fsrc%2FIpmbSensor.cpp
> > %23L144&amp;data=04%7C01%7Cjayashree-d%40hcl.com%7C8676d30f4d3a4dda1e0
> > e08d8e24c4957%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C63750816456
> > 8775248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> > JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5lhuUdfI%2BG75C8I1HDAaEH
> > VP46%2Bz1r3nJV0ek3CiiR4%3D&amp;reserved=0
> >
> > Regards,
> > Jayashree
> >
> >
> > -----Original Message-----
> > From: Ed Tanous <edtanous@google.com>
> > Sent: Friday, February 26, 2021 9:57 PM
> > To: Jayashree D <jayashree-d@hcl.com>
> > Cc: openbmc@lists.ozlabs.org
> > Subject: Re: Negative value returns for sensor in tiogapass
> >
> > [CAUTION: This Email is from outside the Organization. Unless you
> > trust the sender, Don't click links or open attachments as it may be a
> > Phishing email, which can steal your Information and compromise your
> > Computer.]
> >
> > On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com> wrote:
> > >
> > > Classification: Public
> > >
> > > Hi Team,
> > >
> > >
> > >
> > > Recently, I have tested sensors for tiogapass, in which one sensor returns negative value.
> > >
> > > After analysing the code in the dbus-sensors repo, I found the following issue.
> > >
> > >
> > >
> > > dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors
> > > (github.com)
> > >
> > >
> > >
> > > From the above link, We need only below line in the code to process the HSC sensors value for tiogapass.
> > >
> > >
> > >
> > > int16_t value = ((data[4] << 8) | data[3]);
> > >
> > >
> > >
> > > Since the below logic is added, the values get shifted and getting negative values as output.
> > >
> > >
> > >
> > > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> > >
> > > value <<= shift;
> > >
> > > value >>= shift;
> > >
> > >
> > >
> > > Could you please suggest any idea to resolve this issue.
> >
> > I haven't looked at this in detail, but we should follow whatever the spec says here.  If whomever wrote this originally put in the wrong math (which seems likely, given they were implementing 4 types and probably only using one) then we should just get it fixed and do what the spec says.
> >
> > >
> > >
> > >
> > > Regards,
> > >
> > > Jayashree
> > >
> > >
> > >
> > > ::DISCLAIMER::
> > > ________________________________
> > > The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. E-mail transmission is not guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in transmission. The e mail and its contents (with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates. Views or opinions, if any, presented in this email are solely those of the author and may not necessarily reflect the views or opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of authorized representative of HCL is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any email and/or attachments, please check them for viruses and other defects.
> > > ________________________________

RE: Negative value returns for sensor in tiogapass

[Ren, Zhikui]

>-----Original Message-----
>From: Ed Tanous <edtanous@google.com>
>Sent: Thursday, March 18, 2021 9:11 AM
>To: Ren, Zhikui <zhikui.ren@intel.com>
>Cc: Jayashree D <jayashree-d@hcl.com>; openbmc@lists.ozlabs.org
>Subject: Re: Negative value returns for sensor in tiogapass
>
>On Wed, Mar 17, 2021 at 11:18 AM Ren, Zhikui <zhikui.ren@intel.com> wrote:
>>
>>
>>
>> -----Original Message-----
>> From: openbmc <openbmc-bounces+zhikui.ren=intel.com@lists.ozlabs.org>
>> On Behalf Of Jayashree D
>> Sent: Wednesday, March 17, 2021 12:39 AM
>> To: Ed Tanous <edtanous@google.com>
>> Cc: openbmc@lists.ozlabs.org
>> Subject: RE: Negative value returns for sensor in tiogapass
>>
>> Classification: Public
>>
>> Hi Ed,
>>
>> PMBus spec only have read and write format. In the below link, PXE VR uses
>11 bit format. Also sign extend the 11bit reading so that negatives show
>correctly.
>>
>> https://github.com/openbmc/dbus-
>sensors/commit/e4a970d9aea97c7c1a11c63
>> 215e7d3cda2124e54#diff-
>135678dd2046935c5dd0be8e5a5a529d33231203149e786
>> d57b15a3cc0cc1240
>>
>>             constexpr const size_t shift = 16 - 11; // 11bit into 16bit
>>             value <<= shift;
>>             value >>= shift;
>>
>> Could anyone from the intel team can clarify the need of above logic used in
>IpmbSensor.
>>
>> [Ren, Zhikui]  This change was made to allow negative numbers be reported
>correctly.  This can happen during test. Without the change, 255degree will be
>reported and trip threshold event incorrectly.
>
>Can you walk through what your test was?  It's sounding like Jayashree was
>seeing incorrect values in a real world application.  Is it possible we fixed
>something in a test by breaking something in the real world?  Can you think of
>any other reasons why others would be seeing different behavior?
>
[Ren, Zhikui] The issue here is the two device's temperatures are not of the same format.
PXE1410 uses PMBus LINEAR11 format, where the mantissa  is a signed 11-bit 2’s complement integer.
ADM1278 temperature is 11 bit, but not with LINEAR11 format. 
We should rename the current elevenBit to  linearElevenBit and add elevenBit without the sign extension for ADM1278. 

>> Exponent data is not used because it is always 0.
>>
>> Regards,
>> Jayashree
>>
>> -----Original Message-----
>> From: Jayashree D
>> Sent: Tuesday, March 9, 2021 4:48 PM
>> To: Ed Tanous <ed@tanous.net>
>> Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
>> Subject: RE: Negative value returns for sensor in tiogapass
>>
>> Classification: Public
>>
>> Thanks Ed, I'll check it out.
>>
>> -----Original Message-----
>> From: Ed Tanous <ed@tanous.net>
>> Sent: Monday, March 8, 2021 9:37 PM
>> To: Jayashree D <jayashree-d@hcl.com>
>> Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
>> Subject: Re: Negative value returns for sensor in tiogapass
>>
>> [CAUTION: This Email is from outside the Organization. Unless you
>> trust the sender, Don't click links or open attachments as it may be a
>> Phishing email, which can steal your Information and compromise your
>> Computer.]
>>
>> On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
>> >
>> > Classification: Public
>> >
>> > Hi Ed,
>> >
>> > In the below link, PXE1410CVR and ADM1278HSC are using the same
>reading format.
>> > I am not able to find any information on PXE1410CVR. If there is any spec
>available, could you please share it.
>> > It will be helpful to analyze both the specs and fix the problem.
>>
>> I don't have any specs available for those.  I would assume they follow the
>pmbus spec though, you might start there.
>>
>> >
>> > https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
>> > th
>> > ub.com%2Fopenbmc%2Fdbus-
>sensors%2Fblob%2Fmaster%2Fsrc%2FIpmbSensor.c
>> > pp
>> > %23L144&amp;data=04%7C01%7Cjayashree-
>d%40hcl.com%7C8676d30f4d3a4dda1
>> > e0
>> >
>e08d8e24c4957%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C637508
>164
>> > 56
>> >
>8775248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
>luMzIi
>> > LC
>> >
>JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5lhuUdfI%2BG75C8I1
>HDAa
>> > EH
>> > VP46%2Bz1r3nJV0ek3CiiR4%3D&amp;reserved=0
>> >
>> > Regards,
>> > Jayashree
>> >
>> >
>> > -----Original Message-----
>> > From: Ed Tanous <edtanous@google.com>
>> > Sent: Friday, February 26, 2021 9:57 PM
>> > To: Jayashree D <jayashree-d@hcl.com>
>> > Cc: openbmc@lists.ozlabs.org
>> > Subject: Re: Negative value returns for sensor in tiogapass
>> >
>> > [CAUTION: This Email is from outside the Organization. Unless you
>> > trust the sender, Don't click links or open attachments as it may be
>> > a Phishing email, which can steal your Information and compromise
>> > your Computer.]
>> >
>> > On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com>
>wrote:
>> > >
>> > > Classification: Public
>> > >
>> > > Hi Team,
>> > >
>> > >
>> > >
>> > > Recently, I have tested sensors for tiogapass, in which one sensor
>returns negative value.
>> > >
>> > > After analysing the code in the dbus-sensors repo, I found the following
>issue.
>> > >
>> > >
>> > >
>> > > dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors
>> > > (github.com)
>> > >
>> > >
>> > >
>> > > From the above link, We need only below line in the code to process the
>HSC sensors value for tiogapass.
>> > >
>> > >
>> > >
>> > > int16_t value = ((data[4] << 8) | data[3]);
>> > >
>> > >
>> > >
>> > > Since the below logic is added, the values get shifted and getting
>negative values as output.
>> > >
>> > >
>> > >
>> > > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
>> > >
>> > > value <<= shift;
>> > >
>> > > value >>= shift;
>> > >
>> > >
>> > >
>> > > Could you please suggest any idea to resolve this issue.
>> >
>> > I haven't looked at this in detail, but we should follow whatever the spec
>says here.  If whomever wrote this originally put in the wrong math (which
>seems likely, given they were implementing 4 types and probably only using
>one) then we should just get it fixed and do what the spec says.
>> >
>> > >
>> > >
>> > >
>> > > Regards,
>> > >
>> > > Jayashree
>> > >
>> > >
>> > >
>> > > ::DISCLAIMER::
>> > > ________________________________
>> > > The contents of this e-mail and any attachment(s) are confidential and
>intended for the named recipient(s) only. E-mail transmission is not
>guaranteed to be secure or error-free as information could be intercepted,
>corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in
>transmission. The e mail and its contents (with or without referred errors)
>shall therefore not attach any liability on the originator or HCL or its affiliates.
>Views or opinions, if any, presented in this email are solely those of the
>author and may not necessarily reflect the views or opinions of HCL or its
>affiliates. Any form of reproduction, dissemination, copying, disclosure,
>modification, distribution and / or publication of this message without the
>prior written consent of authorized representative of HCL is strictly prohibited.
>If you have received this email in error please delete it and notify the sender
>immediately. Before opening any email and/or attachments, please check
>them for viruses and other defects.
>> > > ________________________________

Re: Negative value returns for sensor in tiogapass

[Ed Tanous]

On Thu, Mar 18, 2021 at 10:27 AM Ren, Zhikui <zhikui.ren@intel.com> wrote:
>
>
>
> >-----Original Message-----
> >From: Ed Tanous <edtanous@google.com>
> >Sent: Thursday, March 18, 2021 9:11 AM
> >To: Ren, Zhikui <zhikui.ren@intel.com>
> >Cc: Jayashree D <jayashree-d@hcl.com>; openbmc@lists.ozlabs.org
> >Subject: Re: Negative value returns for sensor in tiogapass
> >
> >On Wed, Mar 17, 2021 at 11:18 AM Ren, Zhikui <zhikui.ren@intel.com> wrote:
> >>
> >>
> >>
> >> -----Original Message-----
> >> From: openbmc <openbmc-bounces+zhikui.ren=intel.com@lists.ozlabs.org>
> >> On Behalf Of Jayashree D
> >> Sent: Wednesday, March 17, 2021 12:39 AM
> >> To: Ed Tanous <edtanous@google.com>
> >> Cc: openbmc@lists.ozlabs.org
> >> Subject: RE: Negative value returns for sensor in tiogapass
> >>
> >> Classification: Public
> >>
> >> Hi Ed,
> >>
> >> PMBus spec only have read and write format. In the below link, PXE VR uses
> >11 bit format. Also sign extend the 11bit reading so that negatives show
> >correctly.
> >>
> >> https://github.com/openbmc/dbus-
> >sensors/commit/e4a970d9aea97c7c1a11c63
> >> 215e7d3cda2124e54#diff-
> >135678dd2046935c5dd0be8e5a5a529d33231203149e786
> >> d57b15a3cc0cc1240
> >>
> >>             constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >>             value <<= shift;
> >>             value >>= shift;
> >>
> >> Could anyone from the intel team can clarify the need of above logic used in
> >IpmbSensor.
> >>
> >> [Ren, Zhikui]  This change was made to allow negative numbers be reported
> >correctly.  This can happen during test. Without the change, 255degree will be
> >reported and trip threshold event incorrectly.
> >
> >Can you walk through what your test was?  It's sounding like Jayashree was
> >seeing incorrect values in a real world application.  Is it possible we fixed
> >something in a test by breaking something in the real world?  Can you think of
> >any other reasons why others would be seeing different behavior?
> >
> [Ren, Zhikui] The issue here is the two device's temperatures are not of the same format.
> PXE1410 uses PMBus LINEAR11 format, where the mantissa  is a signed 11-bit 2’s complement integer.
> ADM1278 temperature is 11 bit, but not with LINEAR11 format.
> We should rename the current elevenBit to  linearElevenBit and add elevenBit without the sign extension for ADM1278.

I'll leave it between you and Jayashree to figure out who will push
the patch, but that approach sounds reasonable to me.

>
> >> Exponent data is not used because it is always 0.
> >>
> >> Regards,
> >> Jayashree
> >>
> >> -----Original Message-----
> >> From: Jayashree D
> >> Sent: Tuesday, March 9, 2021 4:48 PM
> >> To: Ed Tanous <ed@tanous.net>
> >> Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
> >> Subject: RE: Negative value returns for sensor in tiogapass
> >>
> >> Classification: Public
> >>
> >> Thanks Ed, I'll check it out.
> >>
> >> -----Original Message-----
> >> From: Ed Tanous <ed@tanous.net>
> >> Sent: Monday, March 8, 2021 9:37 PM
> >> To: Jayashree D <jayashree-d@hcl.com>
> >> Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
> >> Subject: Re: Negative value returns for sensor in tiogapass
> >>
> >> [CAUTION: This Email is from outside the Organization. Unless you
> >> trust the sender, Don't click links or open attachments as it may be a
> >> Phishing email, which can steal your Information and compromise your
> >> Computer.]
> >>
> >> On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
> >> >
> >> > Classification: Public
> >> >
> >> > Hi Ed,
> >> >
> >> > In the below link, PXE1410CVR and ADM1278HSC are using the same
> >reading format.
> >> > I am not able to find any information on PXE1410CVR. If there is any spec
> >available, could you please share it.
> >> > It will be helpful to analyze both the specs and fix the problem.
> >>
> >> I don't have any specs available for those.  I would assume they follow the
> >pmbus spec though, you might start there.
> >>
> >> >
> >> > https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> >> > th
> >> > ub.com%2Fopenbmc%2Fdbus-
> >sensors%2Fblob%2Fmaster%2Fsrc%2FIpmbSensor.c
> >> > pp
> >> > %23L144&amp;data=04%7C01%7Cjayashree-
> >d%40hcl.com%7C8676d30f4d3a4dda1
> >> > e0
> >> >
> >e08d8e24c4957%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C637508
> >164
> >> > 56
> >> >
> >8775248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
> >luMzIi
> >> > LC
> >> >
> >JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5lhuUdfI%2BG75C8I1
> >HDAa
> >> > EH
> >> > VP46%2Bz1r3nJV0ek3CiiR4%3D&amp;reserved=0
> >> >
> >> > Regards,
> >> > Jayashree
> >> >
> >> >
> >> > -----Original Message-----
> >> > From: Ed Tanous <edtanous@google.com>
> >> > Sent: Friday, February 26, 2021 9:57 PM
> >> > To: Jayashree D <jayashree-d@hcl.com>
> >> > Cc: openbmc@lists.ozlabs.org
> >> > Subject: Re: Negative value returns for sensor in tiogapass
> >> >
> >> > [CAUTION: This Email is from outside the Organization. Unless you
> >> > trust the sender, Don't click links or open attachments as it may be
> >> > a Phishing email, which can steal your Information and compromise
> >> > your Computer.]
> >> >
> >> > On Fri, Feb 26, 2021 at 12:55 AM Jayashree D <jayashree-d@hcl.com>
> >wrote:
> >> > >
> >> > > Classification: Public
> >> > >
> >> > > Hi Team,
> >> > >
> >> > >
> >> > >
> >> > > Recently, I have tested sensors for tiogapass, in which one sensor
> >returns negative value.
> >> > >
> >> > > After analysing the code in the dbus-sensors repo, I found the following
> >issue.
> >> > >
> >> > >
> >> > >
> >> > > dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors
> >> > > (github.com)
> >> > >
> >> > >
> >> > >
> >> > > From the above link, We need only below line in the code to process the
> >HSC sensors value for tiogapass.
> >> > >
> >> > >
> >> > >
> >> > > int16_t value = ((data[4] << 8) | data[3]);
> >> > >
> >> > >
> >> > >
> >> > > Since the below logic is added, the values get shifted and getting
> >negative values as output.
> >> > >
> >> > >
> >> > >
> >> > > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >> > >
> >> > > value <<= shift;
> >> > >
> >> > > value >>= shift;
> >> > >
> >> > >
> >> > >
> >> > > Could you please suggest any idea to resolve this issue.
> >> >
> >> > I haven't looked at this in detail, but we should follow whatever the spec
> >says here.  If whomever wrote this originally put in the wrong math (which
> >seems likely, given they were implementing 4 types and probably only using
> >one) then we should just get it fixed and do what the spec says.
> >> >
> >> > >
> >> > >
> >> > >
> >> > > Regards,
> >> > >
> >> > > Jayashree
> >> > >
> >> > >
> >> > >
> >> > > ::DISCLAIMER::
> >> > > ________________________________
> >> > > The contents of this e-mail and any attachment(s) are confidential and
> >intended for the named recipient(s) only. E-mail transmission is not
> >guaranteed to be secure or error-free as information could be intercepted,
> >corrupted, lost, destroyed, arrive late or incomplete, or may contain viruses in
> >transmission. The e mail and its contents (with or without referred errors)
> >shall therefore not attach any liability on the originator or HCL or its affiliates.
> >Views or opinions, if any, presented in this email are solely those of the
> >author and may not necessarily reflect the views or opinions of HCL or its
> >affiliates. Any form of reproduction, dissemination, copying, disclosure,
> >modification, distribution and / or publication of this message without the
> >prior written consent of authorized representative of HCL is strictly prohibited.
> >If you have received this email in error please delete it and notify the sender
> >immediately. Before opening any email and/or attachments, please check
> >them for viruses and other defects.
> >> > > ________________________________

RE: Negative value returns for sensor in tiogapass

[Jayashree D]

Classification: Public


> >> -----Original Message-----
> >> From: openbmc 
> >> <openbmc-bounces+zhikui.ren=intel.com@lists.ozlabs.org>
> >> On Behalf Of Jayashree D
> >> Sent: Wednesday, March 17, 2021 12:39 AM
> >> To: Ed Tanous <edtanous@google.com>
> >> Cc: openbmc@lists.ozlabs.org
> >> Subject: RE: Negative value returns for sensor in tiogapass
> >>
> >> Classification: Public
> >>
> >> Hi Ed,
> >>
> >> PMBus spec only have read and write format. In the below link, PXE 
> >> VR uses
> >11 bit format. Also sign extend the 11bit reading so that negatives 
> >show correctly.
> >>
> >> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> >> ithub.com%2Fopenbmc%2Fdbus-&amp;data=04%7C01%7Cjayashree-d%40hcl.co
> >> m%7C1b6d7a1501e44b43ee9908d8ea33afb0%7C189de737c93a4f5a8b686f4ca994
> >> 1912%7C0%7C0%7C637516854997063723%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> >> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&am
> >> p;sdata=5kyigWCboGaq2b2IEfJ1LNWEzB9arVYz3GgF5o0JxMA%3D&amp;reserved
> >> =0
> >sensors/commit/e4a970d9aea97c7c1a11c63
> >> 215e7d3cda2124e54#diff-
> >135678dd2046935c5dd0be8e5a5a529d33231203149e786
> >> d57b15a3cc0cc1240
> >>
> >>             constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >>             value <<= shift;
> >>             value >>= shift;
> >>
> >> Could anyone from the intel team can clarify the need of above 
> >> logic used in
> >IpmbSensor.
> >>
> >> [Ren, Zhikui]  This change was made to allow negative numbers be 
> >> reported
> >correctly.  This can happen during test. Without the change, 
> >255degree will be reported and trip threshold event incorrectly.
> >
> >Can you walk through what your test was?  It's sounding like 
> >Jayashree was seeing incorrect values in a real world application.  
> >Is it possible we fixed something in a test by breaking something in 
> >the real world?  Can you think of any other reasons why others would be seeing different behavior?
> >
> [Ren, Zhikui] The issue here is the two device's temperatures are not of the same format.
> PXE1410 uses PMBus LINEAR11 format, where the mantissa  is a signed 11-bit 2's complement integer.
> ADM1278 temperature is 11 bit, but not with LINEAR11 format.
> We should rename the current elevenBit to  linearElevenBit and add elevenBit without the sign extension for ADM1278.

I'll leave it between you and Jayashree to figure out who will push the patch, but that approach sounds reasonable to me.

[Jayashree]
I have pushed the changed in gerrit, Please review the patch and provide your comments.
https://gerrit.openbmc-project.xyz/c/openbmc/dbus-sensors/+/41433


>
> >> Exponent data is not used because it is always 0.
> >>
> >> Regards,
> >> Jayashree
> >>
> >> -----Original Message-----
> >> From: Jayashree D
> >> Sent: Tuesday, March 9, 2021 4:48 PM
> >> To: Ed Tanous <ed@tanous.net>
> >> Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
> >> Subject: RE: Negative value returns for sensor in tiogapass
> >>
> >> Classification: Public
> >>
> >> Thanks Ed, I'll check it out.
> >>
> >> -----Original Message-----
> >> From: Ed Tanous <ed@tanous.net>
> >> Sent: Monday, March 8, 2021 9:37 PM
> >> To: Jayashree D <jayashree-d@hcl.com>
> >> Cc: Ed Tanous <edtanous@google.com>; openbmc@lists.ozlabs.org
> >> Subject: Re: Negative value returns for sensor in tiogapass
> >>
> >> [CAUTION: This Email is from outside the Organization. Unless you 
> >> trust the sender, Don't click links or open attachments as it may 
> >> be a Phishing email, which can steal your Information and 
> >> compromise your Computer.]
> >>
> >> On Sun, Mar 7, 2021 at 10:17 PM Jayashree D <jayashree-d@hcl.com> wrote:
> >> >
> >> > Classification: Public
> >> >
> >> > Hi Ed,
> >> >
> >> > In the below link, PXE1410CVR and ADM1278HSC are using the same
> >reading format.
> >> > I am not able to find any information on PXE1410CVR. If there is 
> >> > any spec
> >available, could you please share it.
> >> > It will be helpful to analyze both the specs and fix the problem.
> >>
> >> I don't have any specs available for those.  I would assume they 
> >> follow the
> >pmbus spec though, you might start there.
> >>
> >> >
> >> > https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> >> > Fgi
> >> > th
> >> > ub.com%2Fopenbmc%2Fdbus-
> >sensors%2Fblob%2Fmaster%2Fsrc%2FIpmbSensor.c
> >> > pp
> >> > %23L144&amp;data=04%7C01%7Cjayashree-
> >d%40hcl.com%7C8676d30f4d3a4dda1
> >> > e0
> >> >
> >e08d8e24c4957%7C189de737c93a4f5a8b686f4ca9941912%7C0%7C0%7C637508
> >164
> >> > 56
> >> >
> >8775248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
> >luMzIi
> >> > LC
> >> >
> >JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5lhuUdfI%2BG75C8I1
> >HDAa
> >> > EH
> >> > VP46%2Bz1r3nJV0ek3CiiR4%3D&amp;reserved=0
> >> >
> >> > Regards,
> >> > Jayashree
> >> >
> >> >
> >> > -----Original Message-----
> >> > From: Ed Tanous <edtanous@google.com>
> >> > Sent: Friday, February 26, 2021 9:57 PM
> >> > To: Jayashree D <jayashree-d@hcl.com>
> >> > Cc: openbmc@lists.ozlabs.org
> >> > Subject: Re: Negative value returns for sensor in tiogapass
> >> >
> >> > [CAUTION: This Email is from outside the Organization. Unless you 
> >> > trust the sender, Don't click links or open attachments as it may 
> >> > be a Phishing email, which can steal your Information and 
> >> > compromise your Computer.]
> >> >
> >> > On Fri, Feb 26, 2021 at 12:55 AM Jayashree D 
> >> > <jayashree-d@hcl.com>
> >wrote:
> >> > >
> >> > > Classification: Public
> >> > >
> >> > > Hi Team,
> >> > >
> >> > >
> >> > >
> >> > > Recently, I have tested sensors for tiogapass, in which one 
> >> > > sensor
> >returns negative value.
> >> > >
> >> > > After analysing the code in the dbus-sensors repo, I found the 
> >> > > following
> >issue.
> >> > >
> >> > >
> >> > >
> >> > > dbus-sensors/IpmbSensor.cpp at master * openbmc/dbus-sensors
> >> > > (github.com)
> >> > >
> >> > >
> >> > >
> >> > > From the above link, We need only below line in the code to 
> >> > > process the
> >HSC sensors value for tiogapass.
> >> > >
> >> > >
> >> > >
> >> > > int16_t value = ((data[4] << 8) | data[3]);
> >> > >
> >> > >
> >> > >
> >> > > Since the below logic is added, the values get shifted and 
> >> > > getting
> >negative values as output.
> >> > >
> >> > >
> >> > >
> >> > > constexpr const size_t shift = 16 - 11; // 11bit into 16bit
> >> > >
> >> > > value <<= shift;
> >> > >
> >> > > value >>= shift;
> >> > >
> >> > >
> >> > >
> >> > > Could you please suggest any idea to resolve this issue.
> >> >
> >> > I haven't looked at this in detail, but we should follow whatever 
> >> > the spec
> >says here.  If whomever wrote this originally put in the wrong math 
> >(which seems likely, given they were implementing 4 types and 
> >probably only using
> >one) then we should just get it fixed and do what the spec says.
> >> >
> >> > >
> >> > >
> >> > >
> >> > > Regards,
> >> > >
> >> > > Jayashree
> >> > >
> >> > >
> >> > >
> >> > > ::DISCLAIMER::
> >> > > ________________________________ The contents of this e-mail 
> >> > > and any attachment(s) are confidential and
> >intended for the named recipient(s) only. E-mail transmission is not 
> >guaranteed to be secure or error-free as information could be 
> >intercepted, corrupted, lost, destroyed, arrive late or incomplete, 
> >or may contain viruses in transmission. The e mail and its contents 
> >(with or without referred errors) shall therefore not attach any liability on the originator or HCL or its affiliates.
> >Views or opinions, if any, presented in this email are solely those 
> >of the author and may not necessarily reflect the views or opinions 
> >of HCL or its affiliates. Any form of reproduction, dissemination, 
> >copying, disclosure, modification, distribution and / or publication 
> >of this message without the prior written consent of authorized representative of HCL is strictly prohibited.
> >If you have received this email in error please delete it and notify 
> >the sender immediately. Before opening any email and/or attachments, 
> >please check them for viruses and other defects.
> >> > > ________________________________