On Thu, Nov 10, 2016 at 09:52:30PM +0800, Heyi Guo wrote:
We would get incorrect data when accessing multiple bytes at one time and going across EEPROM page boundary, so we split data when the length greater than page boundary.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Peicong Li lipeicong@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org
.../Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c | 35 ++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c b/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c index 994ed6a..eaabb1d 100644 --- a/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c +++ b/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c @@ -26,6 +26,7 @@ #include <Library/I2CLib.h> #define EEPROM_I2C_PORT 6 +#define EEPROM_PAGE_SIZE 0x40 EFI_STATUS EFIAPI OemGetMac2P (IN OUT EFI_MAC_ADDRESS *Mac, IN UINTN Port); @@ -110,6 +111,8 @@ EFI_STATUS OemGetMacE2prom(IN UINT32 Port, OUT UINT8 *pucAddr) UINT16 I2cOffset; UINT16 crc16; NIC_MAC_ADDRESS stMacDesc = {0};
- UINT16 RemainderMacOffset;
- UINT16 LessSizeOfPage;
Status = I2CInit(0, EEPROM_I2C_PORT, Normal); if (EFI_ERROR(Status)) @@ -124,7 +127,20 @@ EFI_STATUS OemGetMacE2prom(IN UINT32 Port, OUT UINT8 *pucAddr) stI2cDev.Port = EEPROM_I2C_PORT; stI2cDev.SlaveDeviceAddress = I2C_SLAVEADDR_EEPROM; stI2cDev.Socket = 0;
- Status = I2CRead(&stI2cDev, I2cOffset, sizeof(NIC_MAC_ADDRESS), (UINT8 *)&stMacDesc);
- RemainderMacOffset = I2cOffset % EEPROM_PAGE_SIZE;
- LessSizeOfPage = EEPROM_PAGE_SIZE - RemainderMacOffset;
- //The length of NIC_MAC_ADDRESS is 10 bytes long,
- //It surly less than EEPROM page size, so we could
- //code as bellow, check the address whether across the page boundary,
- //and split the data when across page boundary.
- if (sizeof(NIC_MAC_ADDRESS) <= LessSizeOfPage) {
Status |= I2CRead(&stI2cDev, I2cOffset, sizeof(NIC_MAC_ADDRESS), (UINT8 *)&stMacDesc);- } else {
Status |= I2CRead(&stI2cDev, I2cOffset, LessSizeOfPage, (UINT8 *)&stMacDesc);
No need for |=. Status is always clear if we get here.
if (!(EFI_ERROR(Status))) {Status |= I2CRead(&stI2cDev, I2cOffset + LessSizeOfPage, sizeof(NIC_MAC_ADDRESS) - LessSizeOfPage, (UINT8 *)&stMacDesc + LessSizeOfPage);}
Indentation. Also, would make sense to wrap this long line.
Could write as
if (sizeof(NIC_MAC_ADDRESS) <= LessSizeOfPage) { Status = I2CRead(&stI2cDev, I2cOffset, sizeof(NIC_MAC_ADDRESS), (UINT8 *)&stMacDesc); } else { Status = I2CRead(&stI2cDev, I2cOffset, LessSizeOfPage, (UINT8 *)&stMacDesc); Status |= I2CRead( &stI2cDev, I2cOffset + LessSizeOfPage, sizeof(NIC_MAC_ADDRESS) - LessSizeOfPage, (UINT8 *)&stMacDesc + LessSizeOfPage ); }
Could also keep the "if (!EFI_ERROR" bit, if there is a problem with performing a second access if the first failed. But there is no point with the |= then, since we know Status is 0 if the code is executed.
- } if (EFI_ERROR(Status)) { DEBUG((EFI_D_ERROR, "[%a]:[%dL] Call I2cRead failed! p1=0x%x.\n", __FUNCTION__, __LINE__, Status));
@@ -153,6 +169,8 @@ EFI_STATUS OemSetMacE2prom(IN UINT32 Port, IN UINT8 *pucAddr) stMacDesc.MacLen = MAC_ADDR_LEN;
- UINT16 RemainderMacOffset;
- UINT16 LessSizeOfPage; gBS->CopyMem((VOID *)(stMacDesc.Mac), (VOID *)pucAddr, MAC_ADDR_LEN);
stMacDesc.Crc16 = make_crc_checksum((UINT8 *)&(stMacDesc.MacLen), sizeof(stMacDesc.MacLen) + MAC_ADDR_LEN); @@ -170,7 +188,20 @@ EFI_STATUS OemSetMacE2prom(IN UINT32 Port, IN UINT8 *pucAddr) stI2cDev.Port = EEPROM_I2C_PORT; stI2cDev.SlaveDeviceAddress = I2C_SLAVEADDR_EEPROM; stI2cDev.Socket = 0;
- Status = I2CWrite(&stI2cDev, I2cOffset, sizeof(NIC_MAC_ADDRESS), (UINT8 *)&stMacDesc);
- RemainderMacOffset = I2cOffset % EEPROM_PAGE_SIZE;
- LessSizeOfPage = EEPROM_PAGE_SIZE - RemainderMacOffset;
- //The length of NIC_MAC_ADDRESS is 10 bytes long,
- //It surly less than EEPROM page size, so we could
- //code as bellow, check the address whether across the page boundary,
- //and split the data when across page boundary.
- if (sizeof(NIC_MAC_ADDRESS) <= LessSizeOfPage) {
Status |= I2CWrite(&stI2cDev, I2cOffset, sizeof(NIC_MAC_ADDRESS), (UINT8 *)&stMacDesc);- } else {
Status |= I2CWrite(&stI2cDev, I2cOffset, LessSizeOfPage, (UINT8 *)&stMacDesc);if (!(EFI_ERROR(Status))) {Status |= I2CWrite(&stI2cDev, I2cOffset + LessSizeOfPage, sizeof(NIC_MAC_ADDRESS) - LessSizeOfPage, (UINT8 *)&stMacDesc + LessSizeOfPage);
Identical comments as for the Get function.
}- } if (EFI_ERROR(Status)) { DEBUG((EFI_D_ERROR, "[%a]:[%dL] Call I2cWrite failed! p1=0x%x.\n", __FUNCTION__, __LINE__, Status));
-- 1.9.1