openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* EEPROM Validation issue in Fru Device.
@ 2021-09-29  5:49 Kumar Thangavel
  2021-09-29  5:53 ` Kumar Thangavel
  0 siblings, 1 reply; 5+ messages in thread
From: Kumar Thangavel @ 2021-09-29  5:49 UTC (permalink / raw)
  To: openbmc
  Cc: Jae Hyun Yoo, Vernon Mauery, Ed Tanous, Amithash Prasad,
	Velumani T-ERS, HCLTech

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

Classification: Confidential
Hi All,

           We are working in FRU module that has 16 bit EEPROM and Wanted to read eeprom data Via i2c.
           The below function in FruDevice.cpp in entity-manager validates 8bit or 16 bit device.

           This function returns "0" for 8 bit device.

           This function returns "0" for 16 bit device also. But it's supposed to return "1" for 16 bit device.

           We have tested with our 16bit device. It returns "0" only.

           If we read via i2c utility commands,  getting all 255 data in the first bytes.

           If we make code changes to return "1" for 16 bit device and called the function "i2cSmbusWriteThenRead",
           then we could able to read all the data perfectly for 16 bit device.

           Code snippet :

            static int isDevice16Bit(int file)
                {
                    /* Get first byte */
                    int byte1 = i2c_smbus_read_byte_data(file, 0);
                    if (byte1 < 0)
                    {
                        return byte1;
                    }
                    /* Read 7 more bytes, it will read same first byte in case of
                     * 8 bit but it will read next byte in case of 16 bit
                     */
                    for (int i = 0; i < 7; i++)
                    {
                        int byte2 = i2c_smbus_read_byte_data(file, 0);
                        if (byte2 < 0)
                        {
                            return byte2;
                        }
                        if (byte2 != byte1)
                        {
                            return 1;
                        }
                    }
                    return 0;
                }

    I2c utility command log :

    i2ctransfer 12 w2@0x51 0x00 0x00 r8
   WARNING! This program can confuse your I2C bus, cause data loss and worse!
   I will send the following messages to device file /dev/i2c-12:
   msg 0: addr 0x51, write, len 2, buf 0x00 0x00
   msg 1: addr 0x51, read, len 8
   Continue? [y/N] y
   0x01 0x00 0x00 0x01 0x0e 0x1c 0x00 0xd4

    Questions :

1.       How the function isDevice16Bit Validated for 16 bit device ?

2.       Is my validation and analysis is correct ?

     Could you please provide your suggestions on this.

Thanks,
Kumar.









::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: 14065 bytes --]

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

* RE: EEPROM Validation issue in Fru Device.
  2021-09-29  5:49 EEPROM Validation issue in Fru Device Kumar Thangavel
@ 2021-09-29  5:53 ` Kumar Thangavel
  2021-09-29 11:48   ` Patrick Williams
  0 siblings, 1 reply; 5+ messages in thread
From: Kumar Thangavel @ 2021-09-29  5:53 UTC (permalink / raw)
  To: openbmc
  Cc: Jae Hyun Yoo, Vernon Mauery, Ed Tanous, Amithash Prasad,
	Velumani T-ERS, HCLTech

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

Classification: Public
Classification changed as Public.

From: Kumar Thangavel
Sent: Wednesday, September 29, 2021 11:19 AM
To: openbmc@lists.ozlabs.org
Cc: Velumani T-ERS,HCLTech <velumanit@hcl.com>; Patrick Williams <patrick@stwcx.xyz>; Amithash Prasad <amithash@fb.com>; Sai Dasari <sdasari@fb.com>; Ed Tanous <ed@tanous.net>; Vernon Mauery <vernon.mauery@linux.intel.com>; Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Subject: EEPROM Validation issue in Fru Device.

Classification: Confidential
Hi All,

           We are working in FRU module that has 16 bit EEPROM and Wanted to read eeprom data Via i2c.

           The below function in FruDevice.cpp in entity-manager validates 8bit or 16 bit device.

           This function returns "0" for 8 bit device.

           This function returns "0" for 16 bit device also. But it's supposed to return "1" for 16 bit device.

           We have tested with our 16bit device. It returns "0" only.

           If we read via i2c utility commands,  getting all 255 data in the first bytes.

           If we make code changes to return "1" for 16 bit device and called the function "i2cSmbusWriteThenRead",
           then we could able to read all the data perfectly for 16 bit device.

           Code snippet :

            static int isDevice16Bit(int file)
                {
                    /* Get first byte */
                    int byte1 = i2c_smbus_read_byte_data(file, 0);
                    if (byte1 < 0)
                    {
                        return byte1;
                    }
                    /* Read 7 more bytes, it will read same first byte in case of
                     * 8 bit but it will read next byte in case of 16 bit
                     */
                    for (int i = 0; i < 7; i++)
                    {
                        int byte2 = i2c_smbus_read_byte_data(file, 0);
                        if (byte2 < 0)
                        {
                            return byte2;
                        }
                        if (byte2 != byte1)
                        {
                            return 1;
                        }
                    }
                    return 0;
                }

    I2c utility command log :

    i2ctransfer 12 w2@0x51 0x00 0x00 r8
   WARNING! This program can confuse your I2C bus, cause data loss and worse!
   I will send the following messages to device file /dev/i2c-12:
   msg 0: addr 0x51, write, len 2, buf 0x00 0x00
   msg 1: addr 0x51, read, len 8
   Continue? [y/N] y
   0x01 0x00 0x00 0x01 0x0e 0x1c 0x00 0xd4

  Note :

          We removed the EEPROM entry in dts file to skip the eeprom driver and trying to read via i2c.

    Questions :

1.       How the function isDevice16Bit Validated for 16 bit device ?

2.       Is my validation and analysis is correct ?

     Could you please provide your suggestions on this.

Thanks,
Kumar.









::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: 15763 bytes --]

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

* Re: EEPROM Validation issue in Fru Device.
  2021-09-29  5:53 ` Kumar Thangavel
@ 2021-09-29 11:48   ` Patrick Williams
  2021-09-29 13:29     ` Kumar Thangavel
  2021-09-30  1:50     ` [External] " Lei Yu
  0 siblings, 2 replies; 5+ messages in thread
From: Patrick Williams @ 2021-09-29 11:48 UTC (permalink / raw)
  To: Kumar Thangavel
  Cc: Jae Hyun Yoo, Vernon Mauery, openbmc, Ed Tanous, Amithash Prasad,
	Velumani T-ERS, HCLTech

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

On Wed, Sep 29, 2021 at 05:53:33AM +0000, Kumar Thangavel wrote:
> 
> 1.       How the function isDevice16Bit Validated for 16 bit device ?

My understanding is that Vijay wrote this originally when he wrote the Tiogapass
port.  You should be able to confirm if it works there.

> 
> 2.       Is my validation and analysis is correct ?

Other people have complained (on Discord) that this current code doesn't work
for all eeproms.  If you have something that works better and doesn't break
Tiogapass support, I would expect it to be accepted as a change in fru-device.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* RE: EEPROM Validation issue in Fru Device.
  2021-09-29 11:48   ` Patrick Williams
@ 2021-09-29 13:29     ` Kumar Thangavel
  2021-09-30  1:50     ` [External] " Lei Yu
  1 sibling, 0 replies; 5+ messages in thread
From: Kumar Thangavel @ 2021-09-29 13:29 UTC (permalink / raw)
  To: Patrick Williams
  Cc: Jae Hyun Yoo, Vernon Mauery, openbmc, Ed Tanous, Amithash Prasad,
	Velumani T-ERS,HCLTech

Classification: Confidential

Thanks Patrick for your responses.

Please find my response inline below.

Thanks,
Kumar.

-----Original Message-----
From: Patrick Williams <patrick@stwcx.xyz>
Sent: Wednesday, September 29, 2021 5:19 PM
To: Kumar Thangavel <thangavel.k@hcl.com>
Cc: openbmc@lists.ozlabs.org; Velumani T-ERS,HCLTech <velumanit@hcl.com>; Amithash Prasad <amithash@fb.com>; Sai Dasari <sdasari@fb.com>; Ed Tanous <ed@tanous.net>; Vernon Mauery <vernon.mauery@linux.intel.com>; Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Subject: Re: EEPROM Validation issue in Fru Device.

On Wed, Sep 29, 2021 at 05:53:33AM +0000, Kumar Thangavel wrote:
>
> 1.       How the function isDevice16Bit Validated for 16 bit device ?

My understanding is that Vijay wrote this originally when he wrote the Tiogapass port.  You should be able to confirm if it works there.

Sure. I will confirm with Tiogapass.

>
> 2.       Is my validation and analysis is correct ?

Other people have complained (on Discord) that this current code doesn't work for all eeproms.  If you have something that works better and doesn't break Tiogapass support, I would expect it to be accepted as a change in fru-device.

OK. Will update the fru-device to make it work for all the platforms.

--
Patrick Williams
::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.
________________________________

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

* Re: [External] Re: EEPROM Validation issue in Fru Device.
  2021-09-29 11:48   ` Patrick Williams
  2021-09-29 13:29     ` Kumar Thangavel
@ 2021-09-30  1:50     ` Lei Yu
  1 sibling, 0 replies; 5+ messages in thread
From: Lei Yu @ 2021-09-30  1:50 UTC (permalink / raw)
  To: Patrick Williams
  Cc: Jae Hyun Yoo, Vernon Mauery, openbmc, Ed Tanous, Amithash Prasad,
	Kumar Thangavel, Velumani T-ERS, HCLTech

On Wed, Sep 29, 2021 at 7:59 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Wed, Sep 29, 2021 at 05:53:33AM +0000, Kumar Thangavel wrote:
> >
> > 1.       How the function isDevice16Bit Validated for 16 bit device ?
>
> My understanding is that Vijay wrote this originally when he wrote the Tiogapass
> port.  You should be able to confirm if it works there.
>
> >
> > 2.       Is my validation and analysis is correct ?
>
> Other people have complained (on Discord) that this current code doesn't work
> for all eeproms.  If you have something that works better and doesn't break
> Tiogapass support, I would expect it to be accepted as a change in fru-device.

We do hit the issue on this. The FRU in our system is 16-bit and
stored with an offset.
We have internal patches to handle the 16-bit and offset issue, but
the change is tricky and not generic for upstream.
It would be very appreciated to get a generic solution to handle such issues.

-- 
BRs,
Lei YU

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

end of thread, other threads:[~2021-09-30  1:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  5:49 EEPROM Validation issue in Fru Device Kumar Thangavel
2021-09-29  5:53 ` Kumar Thangavel
2021-09-29 11:48   ` Patrick Williams
2021-09-29 13:29     ` Kumar Thangavel
2021-09-30  1:50     ` [External] " Lei Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).