openbmc / entity-manager

Run-time JSON driven system configuration manager
Other
26 stars 48 forks source link

`isDevice16bit` function works incorrectly for the 24LC128 EEPROM #15

Closed Kostr closed 2 years ago

Kostr commented 3 years ago

AMD EthanolX CRB has 24LC128 EEPROM for its FRU data. This EEPROM is a 16-bit device. And it is not getting probed correctly if the EEPROM driver is not predeclared in the devicetree file or the driver is not initialized manually:

root@ethanolx:~# busctl tree xyz.openbmc_project.FruDevice
└─/xyz
  └─/xyz/openbmc_project
    └─/xyz/openbmc_project/FruDevice
      └─/xyz/openbmc_project/FruDevice/3_80
root@ethanolx:~# busctl introspect xyz.openbmc_project.FruDevice /xyz/openbmc_project/FruDevice/3_80
NAME                                         TYPE      SIGNATURE RESULT/VALUE FLAGS
...
xyz.openbmc_project.Inventory.Item.I2CDevice interface -         -            -
.Address                                     property  u         80           emits-change
.Bus                                         property  u         3            emits-change
root@ethanolx:~# echo 24c128 0x50 > /sys/class/i2c-dev/i2c-3/device/new_device
root@ethanolx:~# busctl call xyz.openbmc_project.FruDevice /xyz/openbmc_project/FruDevice xyz.openbmc_project.FruDeviceManager ReScanBus y 3
root@ethanolx:~# busctl tree xyz.openbmc_project.FruDevice
└─/xyz
  └─/xyz/openbmc_project
    └─/xyz/openbmc_project/FruDevice
      └─/xyz/openbmc_project/FruDevice/MY_PRODUCT
root@ethanolx:~# busctl introspect xyz.openbmc_project.FruDevice /xyz/openbmc_project/FruDevice/SP3
NAME                                TYPE      SIGNATURE RESULT/VALUE            FLAGS
...
xyz.openbmc_project.FruDevice       interface -         -                       -
.ADDRESS                            property  u         80                      emits-change
.BOARD_LANGUAGE_CODE                property  s         "25"                    emits-change
.BOARD_MANUFACTURER                 property  s         "MY_MANUFACTURER"       emits-change
.BOARD_MANUFACTURE_DATE             property  s         "2021-09-08 - 08:51:00" emits-change
.BOARD_PART_NUMBER                  property  s         "MY_PART_NUMBER"        emits-change
.BOARD_PRODUCT_NAME                 property  s         "MY_PRODUCT"            emits-change
.BOARD_SERIAL_NUMBER                property  s         "MY_SERIAL"             emits-change
.BUS                                property  u         3                       emits-change
.Common_Format_Version              property  s         "1"                     emits-change

The error comes from a fact that without a binded driver isDevice16Bit function (https://github.com/openbmc/entity-manager/blob/7af0c2709f98f47778beeb91175e1d125d370747/src/FruDevice.cpp#L197) returns 0 for this EEPROM and fru-device tries to parse it as a 8-bit EEPROM. The isDevice16Bit function was initially added in the commit https://github.com/openbmc/entity-manager/commit/2d681f6fdec02b2f56d9155117c4830703026cb0 But apparently it does not work well for all 16-bit EEPROMs.

iwoloschin commented 3 years ago

Isn't this basically the same issue as #1?

iwoloschin commented 2 years ago

I think this is fixed by https://github.com/openbmc/entity-manager/commit/f805bafb4dacc629c2b7da0fda90f3fd950d9b07.

edtanous commented 2 years ago

I've had multiple reports that the aforementioned commit has fixed this issue. I'm closing; If there are reports of systems for which this fix doesn't work, please comment here and I'll reopen, or file it as a new bug.