diff mbox

[Linaro-uefi,v2,11/24] D03/D05: Change to access EEPROM data bytewise

Message ID 1476796207-94336-12-git-send-email-heyi.guo@linaro.org
State Superseded
Headers show

Commit Message

gary guo Oct. 18, 2016, 1:09 p.m. UTC 
From: Peicong Li <lipeicong@huawei.com>

We would get incorrect data when accessing multiple bytes at one time
and going across EEPROM page boundary, so we change to access EEPROM by
single byte each time.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Peicong Li <lipeicong@huawei.com>
---
 .../D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c   | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Comments

Leif Lindholm Oct. 27, 2016, 2:18 p.m. UTC | #1 
On Tue, Oct 18, 2016 at 09:09:55PM +0800, Heyi Guo wrote:
> From: Peicong Li <lipeicong@huawei.com>
> 
> We would get incorrect data when accessing multiple bytes at one time
> and going across EEPROM page boundary, so we change to access EEPROM by
> single byte each time.

I guess this is a one-off, so performance is not a big deal.

But would it not otherwise be cleaner to break the read up in I2C
driver? This sounds like something that could pop up in other
locations if the driver supports behaviour that the device does not.

Regards,

Leif

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Peicong Li <lipeicong@huawei.com>
> ---
>  .../D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c   | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c b/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c
> index 994ed6a..31fd333 100644
> --- a/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c
> +++ b/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c
> @@ -110,6 +110,7 @@ EFI_STATUS OemGetMacE2prom(IN UINT32 Port, OUT UINT8 *pucAddr)
>      UINT16     I2cOffset;
>      UINT16     crc16;
>      NIC_MAC_ADDRESS stMacDesc = {0};
> +    UINT32     Index;
>  
>      Status = I2CInit(0, EEPROM_I2C_PORT, Normal);
>      if (EFI_ERROR(Status))
> @@ -124,9 +125,10 @@ 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);
> -    if (EFI_ERROR(Status))
> -    {
> +    for (Index = 0; Index < sizeof(NIC_MAC_ADDRESS); Index++) {
> +        Status |= I2CRead(&stI2cDev, I2cOffset + Index, 1, (UINT8 *)&stMacDesc + Index);
> +    }
> +    if (EFI_ERROR(Status)) {
>          DEBUG((EFI_D_ERROR, "[%a]:[%dL] Call I2cRead failed! p1=0x%x.\n", __FUNCTION__, __LINE__, Status));
>          return Status;
>      }
> @@ -150,6 +152,7 @@ EFI_STATUS OemSetMacE2prom(IN UINT32 Port, IN UINT8 *pucAddr)
>      EFI_STATUS Status;
>      UINT16     I2cOffset;
>      NIC_MAC_ADDRESS stMacDesc = {0};
> +    UINTN      Index;
>  
>  
>      stMacDesc.MacLen = MAC_ADDR_LEN;
> @@ -170,9 +173,11 @@ 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);
> -    if (EFI_ERROR(Status))
> -    {
> +    for (Index = 0; Index < sizeof(NIC_MAC_ADDRESS); Index++) {
> +        Status |= I2CWrite(&stI2cDev, I2cOffset + Index, 1, (UINT8 *)&stMacDesc + Index);
> +    }
> +
> +    if (EFI_ERROR(Status)) {
>          DEBUG((EFI_D_ERROR, "[%a]:[%dL] Call I2cWrite failed! p1=0x%x.\n", __FUNCTION__, __LINE__, Status));
>          return Status;
>      }
> -- 
> 1.9.1
>
diff mbox

Patch

diff --git a/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c b/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c
index 994ed6a..31fd333 100644
--- a/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c
+++ b/Platforms/Hisilicon/D03/Drivers/OemNicConfig2PHi1610/OemNicConfig2P.c
@@ -110,6 +110,7 @@  EFI_STATUS OemGetMacE2prom(IN UINT32 Port, OUT UINT8 *pucAddr)
     UINT16     I2cOffset;
     UINT16     crc16;
     NIC_MAC_ADDRESS stMacDesc = {0};
+    UINT32     Index;
 
     Status = I2CInit(0, EEPROM_I2C_PORT, Normal);
     if (EFI_ERROR(Status))
@@ -124,9 +125,10 @@  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);
-    if (EFI_ERROR(Status))
-    {
+    for (Index = 0; Index < sizeof(NIC_MAC_ADDRESS); Index++) {
+        Status |= I2CRead(&stI2cDev, I2cOffset + Index, 1, (UINT8 *)&stMacDesc + Index);
+    }
+    if (EFI_ERROR(Status)) {
         DEBUG((EFI_D_ERROR, "[%a]:[%dL] Call I2cRead failed! p1=0x%x.\n", __FUNCTION__, __LINE__, Status));
         return Status;
     }
@@ -150,6 +152,7 @@  EFI_STATUS OemSetMacE2prom(IN UINT32 Port, IN UINT8 *pucAddr)
     EFI_STATUS Status;
     UINT16     I2cOffset;
     NIC_MAC_ADDRESS stMacDesc = {0};
+    UINTN      Index;
 
 
     stMacDesc.MacLen = MAC_ADDR_LEN;
@@ -170,9 +173,11 @@  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);
-    if (EFI_ERROR(Status))
-    {
+    for (Index = 0; Index < sizeof(NIC_MAC_ADDRESS); Index++) {
+        Status |= I2CWrite(&stI2cDev, I2cOffset + Index, 1, (UINT8 *)&stMacDesc + Index);
+    }
+
+    if (EFI_ERROR(Status)) {
         DEBUG((EFI_D_ERROR, "[%a]:[%dL] Call I2cWrite failed! p1=0x%x.\n", __FUNCTION__, __LINE__, Status));
         return Status;
     }