[edk2,edk2-platforms,v1,10/16] Hisilicon/D06: Modify for M7 self-Adapte support

Message ID 20190201133436.10500-11-ming.huang@linaro.org
State Superseded
Headers show
Series
  • Fix issues and improve D0x
Related show

Commit Message

Ming Huang Feb. 1, 2019, 1:34 p.m.
As new M7(Cortex-M7) firmware support self-adapte, so do not
need BIOS to implement some function, remove useless funtions
and report CPU0/CPU1 Nic NCL offset to M7.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>

---
 Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------
 1 file changed, 45 insertions(+), 227 deletions(-)

-- 
2.9.5

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Feb. 11, 2019, 7:28 p.m. | #1
On Fri, Feb 01, 2019 at 09:34:30PM +0800, Ming Huang wrote:
> As new M7(Cortex-M7) firmware support self-adapte, so do not

> need BIOS to implement some function, remove useless funtions

> and report CPU0/CPU1 Nic NCL offset to M7.


I don't really care that some other device in the system is a
Cortex-A7. What is its function? Is it an SCP, an MCP, ?
Please describe its function rather than its implementation.

What are the external dependencies?
Is this addressed by one of the patches for edk2-non-osi?

More style issues below.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ming Huang <ming.huang@linaro.org>

> ---

>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------

>  1 file changed, 45 insertions(+), 227 deletions(-)

> 

> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

> index aaf990216982..9bf274e1b991 100644

> --- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

> @@ -21,44 +21,21 @@

>  #include <Library/OemNicLib.h>

>  

>  #define CPU2_SFP2_100G_CARD_OFFSET   0x25

> -#define CPU1_SFP1_LOCATE_OFFSET      0x16

> -#define CPU1_SFP0_LOCATE_OFFSET      0x12

> -#define CPU2_SFP1_LOCATE_OFFSET      0x21

> -#define CPU2_SFP0_LOCATE_OFFSET      0x19

> -#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25

>  

> -#define SFP_10G_SPEED   10

> -#define SFP_25G_SPEED   25

> -#define SFP_100G_SPEED  100

> -#define SFP_GE_SPEED    1

> -

> -#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C

> -#define SFP_GE_SPEED_VAL                0x0D

> -#define SFP_10G_SPEED_VAL               0x67

> -#define SFP_25G_SPEED_VAL               0xFF

> +#define SOCKET1_NET_PORT_100G         1

> +#define SOCKET0_NET_PORT_NUM          4

> +#define SOCKET1_NET_PORT_NUM          2

>  

>  #define CARD_PRESENT_100G               (BIT7)

> -#define CARD_PRESENT_10G                (BIT0)

> -#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))

> -#define SPF_SPEED_OFFSET                12

> -

> -#define SFP_DEVICE_ADDRESS 0x50

> -#define CPU1_9545_I2C_ADDR 0x70

> -#define CPU2_9545_I2C_ADDR 0x71

> -

> -#define FIBER_PRESENT     0

> -#define CARD_PRESENT      1

> -#define I2C_PORT_SFP      4

> -#define CPU2_I2C_PORT_SFP 5

> -

> -#define SOCKET_0                 0

> -#define SOCKET_1                 1

>  #define EEPROM_I2C_PORT          4

>  #define EEPROM_PAGE_SIZE         0x40

>  #define MAC_ADDR_LEN             6

>  #define I2C_OFFSET_EEPROM_ETH0   (0xc00)

>  #define I2C_SLAVEADDR_EEPROM     (0x52)

>  

> +#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0

> +#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4


Is this just a hard-coded address in SRAM? Where is it specified?
I don't think "_FLAG" is the correct name for this #define - this is
the actual offset value. So _OFFSET_ADDRESS would be more descriptive.

> +

>  #pragma pack(1)

>  typedef struct {

>    UINT16 Crc16;

> @@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {

>    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,

>  };

>  

> -EFI_STATUS

> -GetSfpSpeed (

> -  UINT16 Socket,

> -  UINT16 SfpNum,

> -  UINT8* FiberSpeed

> -  )

> -{

> -  EFI_STATUS  Status;

> -  I2C_DEVICE  SpdDev;

> -  UINT8       SfpSelect;

> -  UINT8       SfpSpeed;

> -  UINT32      RegAddr;

> -  UINT16      I2cAddr;

> -  UINT32      SfpPort;

> -

> -  SfpSpeed = 0x0;

> -  if (Socket == SOCKET_1) {

> -    I2cAddr = CPU2_9545_I2C_ADDR;

> -    SfpPort = CPU2_I2C_PORT_SFP;

> -  } else {

> -    I2cAddr = CPU1_9545_I2C_ADDR;

> -    SfpPort = I2C_PORT_SFP;

> -  }

> -

> -  Status = I2CInit (Socket, SfpPort, Normal);

> -  if (EFI_ERROR (Status)) {

> -    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",

> -            __FUNCTION__, __LINE__, Socket, Status));

> -    return Status;

> -  }

> -

> -  SpdDev.Socket = Socket;

> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;

> -  SpdDev.Port = SfpPort;

> -  SpdDev.SlaveDeviceAddress = I2cAddr;

> -  RegAddr = 0x0;

> -  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);

> -

> -  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);

> -  if (EFI_ERROR (Status)) {

> -    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));

> -    return Status;

> -  }

> -

> -  SpdDev.Socket = Socket;

> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;

> -  SpdDev.Port = SfpPort;

> -  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;

> -

> -  RegAddr = SPF_SPEED_OFFSET;

> -  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);

> -  if (EFI_ERROR (Status)) {

> -    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));

> -    return Status;

> -  }

> -

> -  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",

> -         SfpSpeed));

> -

> -  if (SfpSpeed == SFP_10G_SPEED_VAL) {

> -    *FiberSpeed = SFP_10G_SPEED;

> -  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {

> -    *FiberSpeed = SFP_25G_SPEED;

> -  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||

> -             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {

> -    *FiberSpeed = SFP_GE_SPEED;

> -  }

> -

> -  return EFI_SUCCESS;

> -}

> -

> -//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED

> -UINT32

> -GetCpu2FiberType (

> -  UINT8* Fiber1Type,

> -  UINT8* Fiber2Type,

> -  UINT8* Fiber100Ge

> -  )

> -{

> -  EFI_STATUS  Status;

> -  UINT16      SfpNum1;

> -  UINT8       SfpSpeed1;

> -  UINT16      SfpNum2;

> -  UINT8       SfpSpeed2;

> -

> -  SfpNum1 = 0x1;

> -  SfpSpeed1 = SFP_10G_SPEED;

> -  SfpNum2 = 0x2;

> -  SfpSpeed2 = SFP_10G_SPEED;

> -  *Fiber100Ge = 0x0;

> -  *Fiber1Type = SFP_10G_SPEED;

> -  *Fiber2Type = SFP_10G_SPEED;

> -

> -  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

> -    // 100 Ge card

> -    *Fiber1Type = SFP_10G_SPEED;

> -    *Fiber2Type = SFP_10G_SPEED;

> -    *Fiber100Ge = SFP_100G_SPEED;

> -    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));

> -  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {

> -    *Fiber100Ge = 0x0;

> -    *Fiber1Type = SFP_10G_SPEED;

> -    *Fiber2Type = SFP_10G_SPEED;

> -    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {

> -      // Fiber detected in CPU2 slot0, read speed via i2c

> -      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);

> -      if (EFI_ERROR (Status)) {

> -        DEBUG((DEBUG_ERROR,

> -               "Get Socket1 Sfp%d Speed Error: %r.\n",

> -               SfpNum1,

> -               Status));

> -        return Status;

> -      }

> -      if (SfpSpeed1 == SFP_25G_SPEED) {

> -        // P1 don't support 25G, so set speed to 10G

> -        *Fiber1Type = SFP_10G_SPEED;

> -      } else {

> -        *Fiber1Type = SfpSpeed1;

> -      }

> -    } else {

> -      // No fiber, set speed to 10G

> -      *Fiber1Type = SFP_10G_SPEED;

> -    }

> -

> -    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {

> -      // Fiber detected in CPU2 slot1, read speed via i2c

> -      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);

> -      if (EFI_ERROR (Status)) {

> -        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));

> -        return Status;

> -      }

> -      if (SfpSpeed2 == SFP_25G_SPEED) {

> -        *Fiber2Type = SFP_10G_SPEED;

> -      } else {

> -        *Fiber2Type = SfpSpeed2;

> -      }

> -    } else {

> -      // No fiber, set speed to 10G

> -      *Fiber2Type = SFP_10G_SPEED;

> -    }

> -  } else {

> -    // 100Ge/10Ge/Ge Fiber is not found.

> -    *Fiber1Type = SFP_10G_SPEED;

> -    *Fiber2Type = SFP_10G_SPEED;

> -    *Fiber100Ge = 0x0;

> -  }

> -

> -  return EFI_SUCCESS;

> -}

> -

> -//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED

> -UINT32

> -GetCpu1FiberType (

> -  UINT8* Fiber1Type,

> -  UINT8* Fiber2Type

> -  )

> -{

> -  EFI_STATUS  Status;

> -  UINT16      SfpNum1;

> -  UINT8       SfpSpeed1;

> -  UINT16      SfpNum2;

> -  UINT8       SfpSpeed2;

> -

> -  SfpNum1 = 0x1;

> -  SfpSpeed1 = SFP_10G_SPEED;

> -  SfpNum2 = 0x2;

> -  SfpSpeed2 = SFP_10G_SPEED;

> -  *Fiber1Type = SFP_10G_SPEED;

> -  *Fiber2Type = SFP_10G_SPEED;

> -  // Fiber detected in CPU1 slot0, read speed via i2c

> -  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {

> -    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);

> -    if (EFI_ERROR (Status)) {

> -      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",

> -              SfpNum1, Status));

> -      return Status;

> -    }

> -    *Fiber1Type = SfpSpeed1;

> -  } else {

> -    *Fiber1Type = SFP_10G_SPEED;

> -  }

> -

> -  // Fiber detected in CPU1 slot1, read speed via i2c

> -  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {

> -    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);

> -    if (EFI_ERROR (Status)) {

> -      *Fiber2Type = SFP_10G_SPEED;

> -      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));

> -      return Status;

> -    }

> -    *Fiber2Type = SfpSpeed2;

> -  } else {

> -    *Fiber2Type = SFP_10G_SPEED;

> -  }

> -

> -  return EFI_SUCCESS;

> -}

> -

>  UINT16 MakeCrcCheckSum (

>    UINT8 *Buffer,

>    UINT32 Length

> @@ -567,3 +346,42 @@ OemIsInitEth (

>  {

>    return TRUE;

>  }

> +

> +EFI_STATUS ConfigCDR(UINT32 Socket)

> +{

> +    return EFI_SUCCESS;

> +}

> +

> +UINT32 OemGetNclConfOffset (UINT32 Socket)

> +{

> +    UINT32           Cpu1NclConfOffet = 0;


Indentation is 2 spaces, not 4. (Please address throughout.)

> +    UINT32           Cpu2NclConfOffet = 0;


Also, no initialization on definition.
But I see no value in having two variables with complicated names.
Just a single one called ConfigurationOffset or whatever.

> +

> +    if (0 == Socket) {


No jeopardy-comparisons. Please flip that around.

> +      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);


This line can just write a 0 directly.
But it can also use a comment explaining what writing a 0 here achieves.

> +      return Cpu1NclConfOffet;


And this is effectively just an error return - so can just return 0
directly.

> +    } else {


No need for the else. You've returned is there was an error. The rest
is just the remainder of the function.

> +      //2P only


What is 2P?

> +      // P1


What is P1?

> +      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

> +        Cpu2NclConfOffet =  0x20000;


SIZE_128KB?

> +      } else {

> +        Cpu2NclConfOffet =  0x10000;


SIZE_64KB?

> +      }

> +      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);

> +      return Cpu2NclConfOffet;

> +    }

> +}

> +

> +UINT32 OemGetNetPortNum (UINT32 Socket)

> +{

> +    if (0 == Socket){


No jeopardy-comparisons. Please flip that around.

/
    Leif

> +    return SOCKET0_NET_PORT_NUM;

> +    }

> +

> +    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

> +      return SOCKET1_NET_PORT_100G;

> +    } else {

> +      return SOCKET1_NET_PORT_NUM;

> +    }

> +}

> -- 

> 2.9.5

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ming Huang Feb. 12, 2019, 3:14 p.m. | #2
On 2/12/2019 3:28 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:30PM +0800, Ming Huang wrote:

>> As new M7(Cortex-M7) firmware support self-adapte, so do not

>> need BIOS to implement some function, remove useless funtions

>> and report CPU0/CPU1 Nic NCL offset to M7.

> 

> I don't really care that some other device in the system is a

> Cortex-A7. What is its function? Is it an SCP, an MCP, ?

> Please describe its function rather than its implementation.


M7 is used for HNS(Hisilicon network system), cpu access the network
component via M7.

> 

> What are the external dependencies?

> Is this addressed by one of the patches for edk2-non-osi?


This is depend on M7 firmware which will include in hpm file.

> 

> More style issues below.

> 

>>

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Ming Huang <ming.huang@linaro.org>

>> ---

>>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------

>>  1 file changed, 45 insertions(+), 227 deletions(-)

>>

>> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

>> index aaf990216982..9bf274e1b991 100644

>> --- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

>> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

>> @@ -21,44 +21,21 @@

>>  #include <Library/OemNicLib.h>

>>  

>>  #define CPU2_SFP2_100G_CARD_OFFSET   0x25

>> -#define CPU1_SFP1_LOCATE_OFFSET      0x16

>> -#define CPU1_SFP0_LOCATE_OFFSET      0x12

>> -#define CPU2_SFP1_LOCATE_OFFSET      0x21

>> -#define CPU2_SFP0_LOCATE_OFFSET      0x19

>> -#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25

>>  

>> -#define SFP_10G_SPEED   10

>> -#define SFP_25G_SPEED   25

>> -#define SFP_100G_SPEED  100

>> -#define SFP_GE_SPEED    1

>> -

>> -#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C

>> -#define SFP_GE_SPEED_VAL                0x0D

>> -#define SFP_10G_SPEED_VAL               0x67

>> -#define SFP_25G_SPEED_VAL               0xFF

>> +#define SOCKET1_NET_PORT_100G         1

>> +#define SOCKET0_NET_PORT_NUM          4

>> +#define SOCKET1_NET_PORT_NUM          2

>>  

>>  #define CARD_PRESENT_100G               (BIT7)

>> -#define CARD_PRESENT_10G                (BIT0)

>> -#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))

>> -#define SPF_SPEED_OFFSET                12

>> -

>> -#define SFP_DEVICE_ADDRESS 0x50

>> -#define CPU1_9545_I2C_ADDR 0x70

>> -#define CPU2_9545_I2C_ADDR 0x71

>> -

>> -#define FIBER_PRESENT     0

>> -#define CARD_PRESENT      1

>> -#define I2C_PORT_SFP      4

>> -#define CPU2_I2C_PORT_SFP 5

>> -

>> -#define SOCKET_0                 0

>> -#define SOCKET_1                 1

>>  #define EEPROM_I2C_PORT          4

>>  #define EEPROM_PAGE_SIZE         0x40

>>  #define MAC_ADDR_LEN             6

>>  #define I2C_OFFSET_EEPROM_ETH0   (0xc00)

>>  #define I2C_SLAVEADDR_EEPROM     (0x52)

>>  

>> +#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0

>> +#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4

> 

> Is this just a hard-coded address in SRAM? Where is it specified?

> I don't think "_FLAG" is the correct name for this #define - this is

> the actual offset value. So _OFFSET_ADDRESS would be more descriptive.


Yes, M7 firmware will read this two sram addresses.

> 

>> +

>>  #pragma pack(1)

>>  typedef struct {

>>    UINT16 Crc16;

>> @@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {

>>    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,

>>  };

>>  

>> -EFI_STATUS

>> -GetSfpSpeed (

>> -  UINT16 Socket,

>> -  UINT16 SfpNum,

>> -  UINT8* FiberSpeed

>> -  )

>> -{

>> -  EFI_STATUS  Status;

>> -  I2C_DEVICE  SpdDev;

>> -  UINT8       SfpSelect;

>> -  UINT8       SfpSpeed;

>> -  UINT32      RegAddr;

>> -  UINT16      I2cAddr;

>> -  UINT32      SfpPort;

>> -

>> -  SfpSpeed = 0x0;

>> -  if (Socket == SOCKET_1) {

>> -    I2cAddr = CPU2_9545_I2C_ADDR;

>> -    SfpPort = CPU2_I2C_PORT_SFP;

>> -  } else {

>> -    I2cAddr = CPU1_9545_I2C_ADDR;

>> -    SfpPort = I2C_PORT_SFP;

>> -  }

>> -

>> -  Status = I2CInit (Socket, SfpPort, Normal);

>> -  if (EFI_ERROR (Status)) {

>> -    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",

>> -            __FUNCTION__, __LINE__, Socket, Status));

>> -    return Status;

>> -  }

>> -

>> -  SpdDev.Socket = Socket;

>> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;

>> -  SpdDev.Port = SfpPort;

>> -  SpdDev.SlaveDeviceAddress = I2cAddr;

>> -  RegAddr = 0x0;

>> -  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);

>> -

>> -  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);

>> -  if (EFI_ERROR (Status)) {

>> -    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));

>> -    return Status;

>> -  }

>> -

>> -  SpdDev.Socket = Socket;

>> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;

>> -  SpdDev.Port = SfpPort;

>> -  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;

>> -

>> -  RegAddr = SPF_SPEED_OFFSET;

>> -  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);

>> -  if (EFI_ERROR (Status)) {

>> -    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));

>> -    return Status;

>> -  }

>> -

>> -  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",

>> -         SfpSpeed));

>> -

>> -  if (SfpSpeed == SFP_10G_SPEED_VAL) {

>> -    *FiberSpeed = SFP_10G_SPEED;

>> -  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {

>> -    *FiberSpeed = SFP_25G_SPEED;

>> -  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||

>> -             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {

>> -    *FiberSpeed = SFP_GE_SPEED;

>> -  }

>> -

>> -  return EFI_SUCCESS;

>> -}

>> -

>> -//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED

>> -UINT32

>> -GetCpu2FiberType (

>> -  UINT8* Fiber1Type,

>> -  UINT8* Fiber2Type,

>> -  UINT8* Fiber100Ge

>> -  )

>> -{

>> -  EFI_STATUS  Status;

>> -  UINT16      SfpNum1;

>> -  UINT8       SfpSpeed1;

>> -  UINT16      SfpNum2;

>> -  UINT8       SfpSpeed2;

>> -

>> -  SfpNum1 = 0x1;

>> -  SfpSpeed1 = SFP_10G_SPEED;

>> -  SfpNum2 = 0x2;

>> -  SfpSpeed2 = SFP_10G_SPEED;

>> -  *Fiber100Ge = 0x0;

>> -  *Fiber1Type = SFP_10G_SPEED;

>> -  *Fiber2Type = SFP_10G_SPEED;

>> -

>> -  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

>> -    // 100 Ge card

>> -    *Fiber1Type = SFP_10G_SPEED;

>> -    *Fiber2Type = SFP_10G_SPEED;

>> -    *Fiber100Ge = SFP_100G_SPEED;

>> -    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));

>> -  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {

>> -    *Fiber100Ge = 0x0;

>> -    *Fiber1Type = SFP_10G_SPEED;

>> -    *Fiber2Type = SFP_10G_SPEED;

>> -    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {

>> -      // Fiber detected in CPU2 slot0, read speed via i2c

>> -      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);

>> -      if (EFI_ERROR (Status)) {

>> -        DEBUG((DEBUG_ERROR,

>> -               "Get Socket1 Sfp%d Speed Error: %r.\n",

>> -               SfpNum1,

>> -               Status));

>> -        return Status;

>> -      }

>> -      if (SfpSpeed1 == SFP_25G_SPEED) {

>> -        // P1 don't support 25G, so set speed to 10G

>> -        *Fiber1Type = SFP_10G_SPEED;

>> -      } else {

>> -        *Fiber1Type = SfpSpeed1;

>> -      }

>> -    } else {

>> -      // No fiber, set speed to 10G

>> -      *Fiber1Type = SFP_10G_SPEED;

>> -    }

>> -

>> -    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {

>> -      // Fiber detected in CPU2 slot1, read speed via i2c

>> -      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);

>> -      if (EFI_ERROR (Status)) {

>> -        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));

>> -        return Status;

>> -      }

>> -      if (SfpSpeed2 == SFP_25G_SPEED) {

>> -        *Fiber2Type = SFP_10G_SPEED;

>> -      } else {

>> -        *Fiber2Type = SfpSpeed2;

>> -      }

>> -    } else {

>> -      // No fiber, set speed to 10G

>> -      *Fiber2Type = SFP_10G_SPEED;

>> -    }

>> -  } else {

>> -    // 100Ge/10Ge/Ge Fiber is not found.

>> -    *Fiber1Type = SFP_10G_SPEED;

>> -    *Fiber2Type = SFP_10G_SPEED;

>> -    *Fiber100Ge = 0x0;

>> -  }

>> -

>> -  return EFI_SUCCESS;

>> -}

>> -

>> -//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED

>> -UINT32

>> -GetCpu1FiberType (

>> -  UINT8* Fiber1Type,

>> -  UINT8* Fiber2Type

>> -  )

>> -{

>> -  EFI_STATUS  Status;

>> -  UINT16      SfpNum1;

>> -  UINT8       SfpSpeed1;

>> -  UINT16      SfpNum2;

>> -  UINT8       SfpSpeed2;

>> -

>> -  SfpNum1 = 0x1;

>> -  SfpSpeed1 = SFP_10G_SPEED;

>> -  SfpNum2 = 0x2;

>> -  SfpSpeed2 = SFP_10G_SPEED;

>> -  *Fiber1Type = SFP_10G_SPEED;

>> -  *Fiber2Type = SFP_10G_SPEED;

>> -  // Fiber detected in CPU1 slot0, read speed via i2c

>> -  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {

>> -    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);

>> -    if (EFI_ERROR (Status)) {

>> -      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",

>> -              SfpNum1, Status));

>> -      return Status;

>> -    }

>> -    *Fiber1Type = SfpSpeed1;

>> -  } else {

>> -    *Fiber1Type = SFP_10G_SPEED;

>> -  }

>> -

>> -  // Fiber detected in CPU1 slot1, read speed via i2c

>> -  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {

>> -    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);

>> -    if (EFI_ERROR (Status)) {

>> -      *Fiber2Type = SFP_10G_SPEED;

>> -      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));

>> -      return Status;

>> -    }

>> -    *Fiber2Type = SfpSpeed2;

>> -  } else {

>> -    *Fiber2Type = SFP_10G_SPEED;

>> -  }

>> -

>> -  return EFI_SUCCESS;

>> -}

>> -

>>  UINT16 MakeCrcCheckSum (

>>    UINT8 *Buffer,

>>    UINT32 Length

>> @@ -567,3 +346,42 @@ OemIsInitEth (

>>  {

>>    return TRUE;

>>  }

>> +

>> +EFI_STATUS ConfigCDR(UINT32 Socket)

>> +{

>> +    return EFI_SUCCESS;

>> +}

>> +

>> +UINT32 OemGetNclConfOffset (UINT32 Socket)

>> +{

>> +    UINT32           Cpu1NclConfOffet = 0;

> 

> Indentation is 2 spaces, not 4. (Please address throughout.)

> 

>> +    UINT32           Cpu2NclConfOffet = 0;

> 

> Also, no initialization on definition.

> But I see no value in having two variables with complicated names.

> Just a single one called ConfigurationOffset or whatever.

> 

>> +

>> +    if (0 == Socket) {

> 

> No jeopardy-comparisons. Please flip that around.

> 

>> +      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);

> 

> This line can just write a 0 directly.

> But it can also use a comment explaining what writing a 0 here achieves.

> 

>> +      return Cpu1NclConfOffet;

> 

> And this is effectively just an error return - so can just return 0

> directly.

> 

>> +    } else {

> 

> No need for the else. You've returned is there was an error. The rest

> is just the remainder of the function.

> 

>> +      //2P only

> 

> What is 2P?


2 processors, or 2 sockets.

> 

>> +      // P1

> 

> What is P1?


The second processor.

> 

>> +      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

>> +        Cpu2NclConfOffet =  0x20000;

> 

> SIZE_128KB?


ok

> 

>> +      } else {

>> +        Cpu2NclConfOffet =  0x10000;

> 

> SIZE_64KB?


ok

> 

>> +      }

>> +      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);

>> +      return Cpu2NclConfOffet;

>> +    }

>> +}

>> +

>> +UINT32 OemGetNetPortNum (UINT32 Socket)

>> +{

>> +    if (0 == Socket){

> 

> No jeopardy-comparisons. Please flip that around.


All comments will be addressed.

Thanks

> 

> /

>     Leif

> 

>> +    return SOCKET0_NET_PORT_NUM;

>> +    }

>> +

>> +    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

>> +      return SOCKET1_NET_PORT_100G;

>> +    } else {

>> +      return SOCKET1_NET_PORT_NUM;

>> +    }

>> +}

>> -- 

>> 2.9.5

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Feb. 12, 2019, 3:46 p.m. | #3
On Tue, Feb 12, 2019 at 11:14:43PM +0800, Ming Huang wrote:
> 

> 

> On 2/12/2019 3:28 AM, Leif Lindholm wrote:

> > On Fri, Feb 01, 2019 at 09:34:30PM +0800, Ming Huang wrote:

> >> As new M7(Cortex-M7) firmware support self-adapte, so do not

> >> need BIOS to implement some function, remove useless funtions

> >> and report CPU0/CPU1 Nic NCL offset to M7.

> > 

> > I don't really care that some other device in the system is a

> > Cortex-A7. What is its function? Is it an SCP, an MCP, ?

> > Please describe its function rather than its implementation.

> 

> M7 is used for HNS(Hisilicon network system), cpu access the network

> component via M7.


Sure. But does customer documentation documentation refer to it as
"M7"?

> > 

> > What are the external dependencies?

> > Is this addressed by one of the patches for edk2-non-osi?

> 

> This is depend on M7 firmware which will include in hpm file.


So we don't get it when using Capsule Update?

What would be the implication of installing system firmware with the
below change on a system that had not had the corresponding M7
firmware update?

/
    Leif

> > 

> > More style issues below.

> > 

> >>

> >> Contributed-under: TianoCore Contribution Agreement 1.1

> >> Signed-off-by: Ming Huang <ming.huang@linaro.org>

> >> ---

> >>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------

> >>  1 file changed, 45 insertions(+), 227 deletions(-)

> >>

> >> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

> >> index aaf990216982..9bf274e1b991 100644

> >> --- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

> >> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

> >> @@ -21,44 +21,21 @@

> >>  #include <Library/OemNicLib.h>

> >>  

> >>  #define CPU2_SFP2_100G_CARD_OFFSET   0x25

> >> -#define CPU1_SFP1_LOCATE_OFFSET      0x16

> >> -#define CPU1_SFP0_LOCATE_OFFSET      0x12

> >> -#define CPU2_SFP1_LOCATE_OFFSET      0x21

> >> -#define CPU2_SFP0_LOCATE_OFFSET      0x19

> >> -#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25

> >>  

> >> -#define SFP_10G_SPEED   10

> >> -#define SFP_25G_SPEED   25

> >> -#define SFP_100G_SPEED  100

> >> -#define SFP_GE_SPEED    1

> >> -

> >> -#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C

> >> -#define SFP_GE_SPEED_VAL                0x0D

> >> -#define SFP_10G_SPEED_VAL               0x67

> >> -#define SFP_25G_SPEED_VAL               0xFF

> >> +#define SOCKET1_NET_PORT_100G         1

> >> +#define SOCKET0_NET_PORT_NUM          4

> >> +#define SOCKET1_NET_PORT_NUM          2

> >>  

> >>  #define CARD_PRESENT_100G               (BIT7)

> >> -#define CARD_PRESENT_10G                (BIT0)

> >> -#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))

> >> -#define SPF_SPEED_OFFSET                12

> >> -

> >> -#define SFP_DEVICE_ADDRESS 0x50

> >> -#define CPU1_9545_I2C_ADDR 0x70

> >> -#define CPU2_9545_I2C_ADDR 0x71

> >> -

> >> -#define FIBER_PRESENT     0

> >> -#define CARD_PRESENT      1

> >> -#define I2C_PORT_SFP      4

> >> -#define CPU2_I2C_PORT_SFP 5

> >> -

> >> -#define SOCKET_0                 0

> >> -#define SOCKET_1                 1

> >>  #define EEPROM_I2C_PORT          4

> >>  #define EEPROM_PAGE_SIZE         0x40

> >>  #define MAC_ADDR_LEN             6

> >>  #define I2C_OFFSET_EEPROM_ETH0   (0xc00)

> >>  #define I2C_SLAVEADDR_EEPROM     (0x52)

> >>  

> >> +#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0

> >> +#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4

> > 

> > Is this just a hard-coded address in SRAM? Where is it specified?

> > I don't think "_FLAG" is the correct name for this #define - this is

> > the actual offset value. So _OFFSET_ADDRESS would be more descriptive.

> 

> Yes, M7 firmware will read this two sram addresses.

> 

> > 

> >> +

> >>  #pragma pack(1)

> >>  typedef struct {

> >>    UINT16 Crc16;

> >> @@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {

> >>    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,

> >>  };

> >>  

> >> -EFI_STATUS

> >> -GetSfpSpeed (

> >> -  UINT16 Socket,

> >> -  UINT16 SfpNum,

> >> -  UINT8* FiberSpeed

> >> -  )

> >> -{

> >> -  EFI_STATUS  Status;

> >> -  I2C_DEVICE  SpdDev;

> >> -  UINT8       SfpSelect;

> >> -  UINT8       SfpSpeed;

> >> -  UINT32      RegAddr;

> >> -  UINT16      I2cAddr;

> >> -  UINT32      SfpPort;

> >> -

> >> -  SfpSpeed = 0x0;

> >> -  if (Socket == SOCKET_1) {

> >> -    I2cAddr = CPU2_9545_I2C_ADDR;

> >> -    SfpPort = CPU2_I2C_PORT_SFP;

> >> -  } else {

> >> -    I2cAddr = CPU1_9545_I2C_ADDR;

> >> -    SfpPort = I2C_PORT_SFP;

> >> -  }

> >> -

> >> -  Status = I2CInit (Socket, SfpPort, Normal);

> >> -  if (EFI_ERROR (Status)) {

> >> -    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",

> >> -            __FUNCTION__, __LINE__, Socket, Status));

> >> -    return Status;

> >> -  }

> >> -

> >> -  SpdDev.Socket = Socket;

> >> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;

> >> -  SpdDev.Port = SfpPort;

> >> -  SpdDev.SlaveDeviceAddress = I2cAddr;

> >> -  RegAddr = 0x0;

> >> -  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);

> >> -

> >> -  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);

> >> -  if (EFI_ERROR (Status)) {

> >> -    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));

> >> -    return Status;

> >> -  }

> >> -

> >> -  SpdDev.Socket = Socket;

> >> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;

> >> -  SpdDev.Port = SfpPort;

> >> -  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;

> >> -

> >> -  RegAddr = SPF_SPEED_OFFSET;

> >> -  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);

> >> -  if (EFI_ERROR (Status)) {

> >> -    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));

> >> -    return Status;

> >> -  }

> >> -

> >> -  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",

> >> -         SfpSpeed));

> >> -

> >> -  if (SfpSpeed == SFP_10G_SPEED_VAL) {

> >> -    *FiberSpeed = SFP_10G_SPEED;

> >> -  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {

> >> -    *FiberSpeed = SFP_25G_SPEED;

> >> -  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||

> >> -             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {

> >> -    *FiberSpeed = SFP_GE_SPEED;

> >> -  }

> >> -

> >> -  return EFI_SUCCESS;

> >> -}

> >> -

> >> -//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED

> >> -UINT32

> >> -GetCpu2FiberType (

> >> -  UINT8* Fiber1Type,

> >> -  UINT8* Fiber2Type,

> >> -  UINT8* Fiber100Ge

> >> -  )

> >> -{

> >> -  EFI_STATUS  Status;

> >> -  UINT16      SfpNum1;

> >> -  UINT8       SfpSpeed1;

> >> -  UINT16      SfpNum2;

> >> -  UINT8       SfpSpeed2;

> >> -

> >> -  SfpNum1 = 0x1;

> >> -  SfpSpeed1 = SFP_10G_SPEED;

> >> -  SfpNum2 = 0x2;

> >> -  SfpSpeed2 = SFP_10G_SPEED;

> >> -  *Fiber100Ge = 0x0;

> >> -  *Fiber1Type = SFP_10G_SPEED;

> >> -  *Fiber2Type = SFP_10G_SPEED;

> >> -

> >> -  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

> >> -    // 100 Ge card

> >> -    *Fiber1Type = SFP_10G_SPEED;

> >> -    *Fiber2Type = SFP_10G_SPEED;

> >> -    *Fiber100Ge = SFP_100G_SPEED;

> >> -    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));

> >> -  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {

> >> -    *Fiber100Ge = 0x0;

> >> -    *Fiber1Type = SFP_10G_SPEED;

> >> -    *Fiber2Type = SFP_10G_SPEED;

> >> -    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {

> >> -      // Fiber detected in CPU2 slot0, read speed via i2c

> >> -      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);

> >> -      if (EFI_ERROR (Status)) {

> >> -        DEBUG((DEBUG_ERROR,

> >> -               "Get Socket1 Sfp%d Speed Error: %r.\n",

> >> -               SfpNum1,

> >> -               Status));

> >> -        return Status;

> >> -      }

> >> -      if (SfpSpeed1 == SFP_25G_SPEED) {

> >> -        // P1 don't support 25G, so set speed to 10G

> >> -        *Fiber1Type = SFP_10G_SPEED;

> >> -      } else {

> >> -        *Fiber1Type = SfpSpeed1;

> >> -      }

> >> -    } else {

> >> -      // No fiber, set speed to 10G

> >> -      *Fiber1Type = SFP_10G_SPEED;

> >> -    }

> >> -

> >> -    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {

> >> -      // Fiber detected in CPU2 slot1, read speed via i2c

> >> -      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);

> >> -      if (EFI_ERROR (Status)) {

> >> -        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));

> >> -        return Status;

> >> -      }

> >> -      if (SfpSpeed2 == SFP_25G_SPEED) {

> >> -        *Fiber2Type = SFP_10G_SPEED;

> >> -      } else {

> >> -        *Fiber2Type = SfpSpeed2;

> >> -      }

> >> -    } else {

> >> -      // No fiber, set speed to 10G

> >> -      *Fiber2Type = SFP_10G_SPEED;

> >> -    }

> >> -  } else {

> >> -    // 100Ge/10Ge/Ge Fiber is not found.

> >> -    *Fiber1Type = SFP_10G_SPEED;

> >> -    *Fiber2Type = SFP_10G_SPEED;

> >> -    *Fiber100Ge = 0x0;

> >> -  }

> >> -

> >> -  return EFI_SUCCESS;

> >> -}

> >> -

> >> -//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED

> >> -UINT32

> >> -GetCpu1FiberType (

> >> -  UINT8* Fiber1Type,

> >> -  UINT8* Fiber2Type

> >> -  )

> >> -{

> >> -  EFI_STATUS  Status;

> >> -  UINT16      SfpNum1;

> >> -  UINT8       SfpSpeed1;

> >> -  UINT16      SfpNum2;

> >> -  UINT8       SfpSpeed2;

> >> -

> >> -  SfpNum1 = 0x1;

> >> -  SfpSpeed1 = SFP_10G_SPEED;

> >> -  SfpNum2 = 0x2;

> >> -  SfpSpeed2 = SFP_10G_SPEED;

> >> -  *Fiber1Type = SFP_10G_SPEED;

> >> -  *Fiber2Type = SFP_10G_SPEED;

> >> -  // Fiber detected in CPU1 slot0, read speed via i2c

> >> -  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {

> >> -    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);

> >> -    if (EFI_ERROR (Status)) {

> >> -      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",

> >> -              SfpNum1, Status));

> >> -      return Status;

> >> -    }

> >> -    *Fiber1Type = SfpSpeed1;

> >> -  } else {

> >> -    *Fiber1Type = SFP_10G_SPEED;

> >> -  }

> >> -

> >> -  // Fiber detected in CPU1 slot1, read speed via i2c

> >> -  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {

> >> -    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);

> >> -    if (EFI_ERROR (Status)) {

> >> -      *Fiber2Type = SFP_10G_SPEED;

> >> -      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));

> >> -      return Status;

> >> -    }

> >> -    *Fiber2Type = SfpSpeed2;

> >> -  } else {

> >> -    *Fiber2Type = SFP_10G_SPEED;

> >> -  }

> >> -

> >> -  return EFI_SUCCESS;

> >> -}

> >> -

> >>  UINT16 MakeCrcCheckSum (

> >>    UINT8 *Buffer,

> >>    UINT32 Length

> >> @@ -567,3 +346,42 @@ OemIsInitEth (

> >>  {

> >>    return TRUE;

> >>  }

> >> +

> >> +EFI_STATUS ConfigCDR(UINT32 Socket)

> >> +{

> >> +    return EFI_SUCCESS;

> >> +}

> >> +

> >> +UINT32 OemGetNclConfOffset (UINT32 Socket)

> >> +{

> >> +    UINT32           Cpu1NclConfOffet = 0;

> > 

> > Indentation is 2 spaces, not 4. (Please address throughout.)

> > 

> >> +    UINT32           Cpu2NclConfOffet = 0;

> > 

> > Also, no initialization on definition.

> > But I see no value in having two variables with complicated names.

> > Just a single one called ConfigurationOffset or whatever.

> > 

> >> +

> >> +    if (0 == Socket) {

> > 

> > No jeopardy-comparisons. Please flip that around.

> > 

> >> +      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);

> > 

> > This line can just write a 0 directly.

> > But it can also use a comment explaining what writing a 0 here achieves.

> > 

> >> +      return Cpu1NclConfOffet;

> > 

> > And this is effectively just an error return - so can just return 0

> > directly.

> > 

> >> +    } else {

> > 

> > No need for the else. You've returned is there was an error. The rest

> > is just the remainder of the function.

> > 

> >> +      //2P only

> > 

> > What is 2P?

> 

> 2 processors, or 2 sockets.

> 

> > 

> >> +      // P1

> > 

> > What is P1?

> 

> The second processor.

> 

> > 

> >> +      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

> >> +        Cpu2NclConfOffet =  0x20000;

> > 

> > SIZE_128KB?

> 

> ok

> 

> > 

> >> +      } else {

> >> +        Cpu2NclConfOffet =  0x10000;

> > 

> > SIZE_64KB?

> 

> ok

> 

> > 

> >> +      }

> >> +      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);

> >> +      return Cpu2NclConfOffet;

> >> +    }

> >> +}

> >> +

> >> +UINT32 OemGetNetPortNum (UINT32 Socket)

> >> +{

> >> +    if (0 == Socket){

> > 

> > No jeopardy-comparisons. Please flip that around.

> 

> All comments will be addressed.

> 

> Thanks

> 

> > 

> > /

> >     Leif

> > 

> >> +    return SOCKET0_NET_PORT_NUM;

> >> +    }

> >> +

> >> +    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

> >> +      return SOCKET1_NET_PORT_100G;

> >> +    } else {

> >> +      return SOCKET1_NET_PORT_NUM;

> >> +    }

> >> +}

> >> -- 

> >> 2.9.5

> >>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ming Huang Feb. 13, 2019, 4:38 a.m. | #4
On 2/12/2019 11:46 PM, Leif Lindholm wrote:
> On Tue, Feb 12, 2019 at 11:14:43PM +0800, Ming Huang wrote:

>>

>>

>> On 2/12/2019 3:28 AM, Leif Lindholm wrote:

>>> On Fri, Feb 01, 2019 at 09:34:30PM +0800, Ming Huang wrote:

>>>> As new M7(Cortex-M7) firmware support self-adapte, so do not

>>>> need BIOS to implement some function, remove useless funtions

>>>> and report CPU0/CPU1 Nic NCL offset to M7.

>>>

>>> I don't really care that some other device in the system is a

>>> Cortex-A7. What is its function? Is it an SCP, an MCP, ?

>>> Please describe its function rather than its implementation.

>>

>> M7 is used for HNS(Hisilicon network system), cpu access the network

>> component via M7.

> 

> Sure. But does customer documentation documentation refer to it as

> "M7"?


I check documentation just now, Integrated Management Processor(IMP) is used,
so, I will change commit titil and message M7 to IMP.

> 

>>>

>>> What are the external dependencies?

>>> Is this addressed by one of the patches for edk2-non-osi?

>>

>> This is depend on M7 firmware which will include in hpm file.

> 

> So we don't get it when using Capsule Update?


Yes.

> 

> What would be the implication of installing system firmware with the

> below change on a system that had not had the corresponding M7

> firmware update?


The HNS will not worked.

Thanks

> 

> /

>     Leif

> 

>>>

>>> More style issues below.

>>>

>>>>

>>>> Contributed-under: TianoCore Contribution Agreement 1.1

>>>> Signed-off-by: Ming Huang <ming.huang@linaro.org>

>>>> ---

>>>>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------

>>>>  1 file changed, 45 insertions(+), 227 deletions(-)

>>>>

>>>> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

>>>> index aaf990216982..9bf274e1b991 100644

>>>> --- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

>>>> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c

>>>> @@ -21,44 +21,21 @@

>>>>  #include <Library/OemNicLib.h>

>>>>  

>>>>  #define CPU2_SFP2_100G_CARD_OFFSET   0x25

>>>> -#define CPU1_SFP1_LOCATE_OFFSET      0x16

>>>> -#define CPU1_SFP0_LOCATE_OFFSET      0x12

>>>> -#define CPU2_SFP1_LOCATE_OFFSET      0x21

>>>> -#define CPU2_SFP0_LOCATE_OFFSET      0x19

>>>> -#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25

>>>>  

>>>> -#define SFP_10G_SPEED   10

>>>> -#define SFP_25G_SPEED   25

>>>> -#define SFP_100G_SPEED  100

>>>> -#define SFP_GE_SPEED    1

>>>> -

>>>> -#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C

>>>> -#define SFP_GE_SPEED_VAL                0x0D

>>>> -#define SFP_10G_SPEED_VAL               0x67

>>>> -#define SFP_25G_SPEED_VAL               0xFF

>>>> +#define SOCKET1_NET_PORT_100G         1

>>>> +#define SOCKET0_NET_PORT_NUM          4

>>>> +#define SOCKET1_NET_PORT_NUM          2

>>>>  

>>>>  #define CARD_PRESENT_100G               (BIT7)

>>>> -#define CARD_PRESENT_10G                (BIT0)

>>>> -#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))

>>>> -#define SPF_SPEED_OFFSET                12

>>>> -

>>>> -#define SFP_DEVICE_ADDRESS 0x50

>>>> -#define CPU1_9545_I2C_ADDR 0x70

>>>> -#define CPU2_9545_I2C_ADDR 0x71

>>>> -

>>>> -#define FIBER_PRESENT     0

>>>> -#define CARD_PRESENT      1

>>>> -#define I2C_PORT_SFP      4

>>>> -#define CPU2_I2C_PORT_SFP 5

>>>> -

>>>> -#define SOCKET_0                 0

>>>> -#define SOCKET_1                 1

>>>>  #define EEPROM_I2C_PORT          4

>>>>  #define EEPROM_PAGE_SIZE         0x40

>>>>  #define MAC_ADDR_LEN             6

>>>>  #define I2C_OFFSET_EEPROM_ETH0   (0xc00)

>>>>  #define I2C_SLAVEADDR_EEPROM     (0x52)

>>>>  

>>>> +#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0

>>>> +#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4

>>>

>>> Is this just a hard-coded address in SRAM? Where is it specified?

>>> I don't think "_FLAG" is the correct name for this #define - this is

>>> the actual offset value. So _OFFSET_ADDRESS would be more descriptive.

>>

>> Yes, M7 firmware will read this two sram addresses.

>>

>>>

>>>> +

>>>>  #pragma pack(1)

>>>>  typedef struct {

>>>>    UINT16 Crc16;

>>>> @@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {

>>>>    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,

>>>>  };

>>>>  

>>>> -EFI_STATUS

>>>> -GetSfpSpeed (

>>>> -  UINT16 Socket,

>>>> -  UINT16 SfpNum,

>>>> -  UINT8* FiberSpeed

>>>> -  )

>>>> -{

>>>> -  EFI_STATUS  Status;

>>>> -  I2C_DEVICE  SpdDev;

>>>> -  UINT8       SfpSelect;

>>>> -  UINT8       SfpSpeed;

>>>> -  UINT32      RegAddr;

>>>> -  UINT16      I2cAddr;

>>>> -  UINT32      SfpPort;

>>>> -

>>>> -  SfpSpeed = 0x0;

>>>> -  if (Socket == SOCKET_1) {

>>>> -    I2cAddr = CPU2_9545_I2C_ADDR;

>>>> -    SfpPort = CPU2_I2C_PORT_SFP;

>>>> -  } else {

>>>> -    I2cAddr = CPU1_9545_I2C_ADDR;

>>>> -    SfpPort = I2C_PORT_SFP;

>>>> -  }

>>>> -

>>>> -  Status = I2CInit (Socket, SfpPort, Normal);

>>>> -  if (EFI_ERROR (Status)) {

>>>> -    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",

>>>> -            __FUNCTION__, __LINE__, Socket, Status));

>>>> -    return Status;

>>>> -  }

>>>> -

>>>> -  SpdDev.Socket = Socket;

>>>> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;

>>>> -  SpdDev.Port = SfpPort;

>>>> -  SpdDev.SlaveDeviceAddress = I2cAddr;

>>>> -  RegAddr = 0x0;

>>>> -  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);

>>>> -

>>>> -  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);

>>>> -  if (EFI_ERROR (Status)) {

>>>> -    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));

>>>> -    return Status;

>>>> -  }

>>>> -

>>>> -  SpdDev.Socket = Socket;

>>>> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;

>>>> -  SpdDev.Port = SfpPort;

>>>> -  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;

>>>> -

>>>> -  RegAddr = SPF_SPEED_OFFSET;

>>>> -  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);

>>>> -  if (EFI_ERROR (Status)) {

>>>> -    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));

>>>> -    return Status;

>>>> -  }

>>>> -

>>>> -  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",

>>>> -         SfpSpeed));

>>>> -

>>>> -  if (SfpSpeed == SFP_10G_SPEED_VAL) {

>>>> -    *FiberSpeed = SFP_10G_SPEED;

>>>> -  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {

>>>> -    *FiberSpeed = SFP_25G_SPEED;

>>>> -  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||

>>>> -             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {

>>>> -    *FiberSpeed = SFP_GE_SPEED;

>>>> -  }

>>>> -

>>>> -  return EFI_SUCCESS;

>>>> -}

>>>> -

>>>> -//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED

>>>> -UINT32

>>>> -GetCpu2FiberType (

>>>> -  UINT8* Fiber1Type,

>>>> -  UINT8* Fiber2Type,

>>>> -  UINT8* Fiber100Ge

>>>> -  )

>>>> -{

>>>> -  EFI_STATUS  Status;

>>>> -  UINT16      SfpNum1;

>>>> -  UINT8       SfpSpeed1;

>>>> -  UINT16      SfpNum2;

>>>> -  UINT8       SfpSpeed2;

>>>> -

>>>> -  SfpNum1 = 0x1;

>>>> -  SfpSpeed1 = SFP_10G_SPEED;

>>>> -  SfpNum2 = 0x2;

>>>> -  SfpSpeed2 = SFP_10G_SPEED;

>>>> -  *Fiber100Ge = 0x0;

>>>> -  *Fiber1Type = SFP_10G_SPEED;

>>>> -  *Fiber2Type = SFP_10G_SPEED;

>>>> -

>>>> -  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

>>>> -    // 100 Ge card

>>>> -    *Fiber1Type = SFP_10G_SPEED;

>>>> -    *Fiber2Type = SFP_10G_SPEED;

>>>> -    *Fiber100Ge = SFP_100G_SPEED;

>>>> -    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));

>>>> -  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {

>>>> -    *Fiber100Ge = 0x0;

>>>> -    *Fiber1Type = SFP_10G_SPEED;

>>>> -    *Fiber2Type = SFP_10G_SPEED;

>>>> -    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {

>>>> -      // Fiber detected in CPU2 slot0, read speed via i2c

>>>> -      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);

>>>> -      if (EFI_ERROR (Status)) {

>>>> -        DEBUG((DEBUG_ERROR,

>>>> -               "Get Socket1 Sfp%d Speed Error: %r.\n",

>>>> -               SfpNum1,

>>>> -               Status));

>>>> -        return Status;

>>>> -      }

>>>> -      if (SfpSpeed1 == SFP_25G_SPEED) {

>>>> -        // P1 don't support 25G, so set speed to 10G

>>>> -        *Fiber1Type = SFP_10G_SPEED;

>>>> -      } else {

>>>> -        *Fiber1Type = SfpSpeed1;

>>>> -      }

>>>> -    } else {

>>>> -      // No fiber, set speed to 10G

>>>> -      *Fiber1Type = SFP_10G_SPEED;

>>>> -    }

>>>> -

>>>> -    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {

>>>> -      // Fiber detected in CPU2 slot1, read speed via i2c

>>>> -      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);

>>>> -      if (EFI_ERROR (Status)) {

>>>> -        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));

>>>> -        return Status;

>>>> -      }

>>>> -      if (SfpSpeed2 == SFP_25G_SPEED) {

>>>> -        *Fiber2Type = SFP_10G_SPEED;

>>>> -      } else {

>>>> -        *Fiber2Type = SfpSpeed2;

>>>> -      }

>>>> -    } else {

>>>> -      // No fiber, set speed to 10G

>>>> -      *Fiber2Type = SFP_10G_SPEED;

>>>> -    }

>>>> -  } else {

>>>> -    // 100Ge/10Ge/Ge Fiber is not found.

>>>> -    *Fiber1Type = SFP_10G_SPEED;

>>>> -    *Fiber2Type = SFP_10G_SPEED;

>>>> -    *Fiber100Ge = 0x0;

>>>> -  }

>>>> -

>>>> -  return EFI_SUCCESS;

>>>> -}

>>>> -

>>>> -//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED

>>>> -UINT32

>>>> -GetCpu1FiberType (

>>>> -  UINT8* Fiber1Type,

>>>> -  UINT8* Fiber2Type

>>>> -  )

>>>> -{

>>>> -  EFI_STATUS  Status;

>>>> -  UINT16      SfpNum1;

>>>> -  UINT8       SfpSpeed1;

>>>> -  UINT16      SfpNum2;

>>>> -  UINT8       SfpSpeed2;

>>>> -

>>>> -  SfpNum1 = 0x1;

>>>> -  SfpSpeed1 = SFP_10G_SPEED;

>>>> -  SfpNum2 = 0x2;

>>>> -  SfpSpeed2 = SFP_10G_SPEED;

>>>> -  *Fiber1Type = SFP_10G_SPEED;

>>>> -  *Fiber2Type = SFP_10G_SPEED;

>>>> -  // Fiber detected in CPU1 slot0, read speed via i2c

>>>> -  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {

>>>> -    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);

>>>> -    if (EFI_ERROR (Status)) {

>>>> -      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",

>>>> -              SfpNum1, Status));

>>>> -      return Status;

>>>> -    }

>>>> -    *Fiber1Type = SfpSpeed1;

>>>> -  } else {

>>>> -    *Fiber1Type = SFP_10G_SPEED;

>>>> -  }

>>>> -

>>>> -  // Fiber detected in CPU1 slot1, read speed via i2c

>>>> -  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {

>>>> -    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);

>>>> -    if (EFI_ERROR (Status)) {

>>>> -      *Fiber2Type = SFP_10G_SPEED;

>>>> -      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));

>>>> -      return Status;

>>>> -    }

>>>> -    *Fiber2Type = SfpSpeed2;

>>>> -  } else {

>>>> -    *Fiber2Type = SFP_10G_SPEED;

>>>> -  }

>>>> -

>>>> -  return EFI_SUCCESS;

>>>> -}

>>>> -

>>>>  UINT16 MakeCrcCheckSum (

>>>>    UINT8 *Buffer,

>>>>    UINT32 Length

>>>> @@ -567,3 +346,42 @@ OemIsInitEth (

>>>>  {

>>>>    return TRUE;

>>>>  }

>>>> +

>>>> +EFI_STATUS ConfigCDR(UINT32 Socket)

>>>> +{

>>>> +    return EFI_SUCCESS;

>>>> +}

>>>> +

>>>> +UINT32 OemGetNclConfOffset (UINT32 Socket)

>>>> +{

>>>> +    UINT32           Cpu1NclConfOffet = 0;

>>>

>>> Indentation is 2 spaces, not 4. (Please address throughout.)

>>>

>>>> +    UINT32           Cpu2NclConfOffet = 0;

>>>

>>> Also, no initialization on definition.

>>> But I see no value in having two variables with complicated names.

>>> Just a single one called ConfigurationOffset or whatever.

>>>

>>>> +

>>>> +    if (0 == Socket) {

>>>

>>> No jeopardy-comparisons. Please flip that around.

>>>

>>>> +      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);

>>>

>>> This line can just write a 0 directly.

>>> But it can also use a comment explaining what writing a 0 here achieves.

>>>

>>>> +      return Cpu1NclConfOffet;

>>>

>>> And this is effectively just an error return - so can just return 0

>>> directly.

>>>

>>>> +    } else {

>>>

>>> No need for the else. You've returned is there was an error. The rest

>>> is just the remainder of the function.

>>>

>>>> +      //2P only

>>>

>>> What is 2P?

>>

>> 2 processors, or 2 sockets.

>>

>>>

>>>> +      // P1

>>>

>>> What is P1?

>>

>> The second processor.

>>

>>>

>>>> +      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

>>>> +        Cpu2NclConfOffet =  0x20000;

>>>

>>> SIZE_128KB?

>>

>> ok

>>

>>>

>>>> +      } else {

>>>> +        Cpu2NclConfOffet =  0x10000;

>>>

>>> SIZE_64KB?

>>

>> ok

>>

>>>

>>>> +      }

>>>> +      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);

>>>> +      return Cpu2NclConfOffet;

>>>> +    }

>>>> +}

>>>> +

>>>> +UINT32 OemGetNetPortNum (UINT32 Socket)

>>>> +{

>>>> +    if (0 == Socket){

>>>

>>> No jeopardy-comparisons. Please flip that around.

>>

>> All comments will be addressed.

>>

>> Thanks

>>

>>>

>>> /

>>>     Leif

>>>

>>>> +    return SOCKET0_NET_PORT_NUM;

>>>> +    }

>>>> +

>>>> +    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {

>>>> +      return SOCKET1_NET_PORT_100G;

>>>> +    } else {

>>>> +      return SOCKET1_NET_PORT_NUM;

>>>> +    }

>>>> +}

>>>> -- 

>>>> 2.9.5

>>>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
index aaf990216982..9bf274e1b991 100644
--- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
+++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
@@ -21,44 +21,21 @@ 
 #include <Library/OemNicLib.h>
 
 #define CPU2_SFP2_100G_CARD_OFFSET   0x25
-#define CPU1_SFP1_LOCATE_OFFSET      0x16
-#define CPU1_SFP0_LOCATE_OFFSET      0x12
-#define CPU2_SFP1_LOCATE_OFFSET      0x21
-#define CPU2_SFP0_LOCATE_OFFSET      0x19
-#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25
 
-#define SFP_10G_SPEED   10
-#define SFP_25G_SPEED   25
-#define SFP_100G_SPEED  100
-#define SFP_GE_SPEED    1
-
-#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C
-#define SFP_GE_SPEED_VAL                0x0D
-#define SFP_10G_SPEED_VAL               0x67
-#define SFP_25G_SPEED_VAL               0xFF
+#define SOCKET1_NET_PORT_100G         1
+#define SOCKET0_NET_PORT_NUM          4
+#define SOCKET1_NET_PORT_NUM          2
 
 #define CARD_PRESENT_100G               (BIT7)
-#define CARD_PRESENT_10G                (BIT0)
-#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))
-#define SPF_SPEED_OFFSET                12
-
-#define SFP_DEVICE_ADDRESS 0x50
-#define CPU1_9545_I2C_ADDR 0x70
-#define CPU2_9545_I2C_ADDR 0x71
-
-#define FIBER_PRESENT     0
-#define CARD_PRESENT      1
-#define I2C_PORT_SFP      4
-#define CPU2_I2C_PORT_SFP 5
-
-#define SOCKET_0                 0
-#define SOCKET_1                 1
 #define EEPROM_I2C_PORT          4
 #define EEPROM_PAGE_SIZE         0x40
 #define MAC_ADDR_LEN             6
 #define I2C_OFFSET_EEPROM_ETH0   (0xc00)
 #define I2C_SLAVEADDR_EEPROM     (0x52)
 
+#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0
+#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4
+
 #pragma pack(1)
 typedef struct {
   UINT16 Crc16;
@@ -114,204 +91,6 @@  UINT16 CrcTable16[256] = {
   0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,
 };
 
-EFI_STATUS
-GetSfpSpeed (
-  UINT16 Socket,
-  UINT16 SfpNum,
-  UINT8* FiberSpeed
-  )
-{
-  EFI_STATUS  Status;
-  I2C_DEVICE  SpdDev;
-  UINT8       SfpSelect;
-  UINT8       SfpSpeed;
-  UINT32      RegAddr;
-  UINT16      I2cAddr;
-  UINT32      SfpPort;
-
-  SfpSpeed = 0x0;
-  if (Socket == SOCKET_1) {
-    I2cAddr = CPU2_9545_I2C_ADDR;
-    SfpPort = CPU2_I2C_PORT_SFP;
-  } else {
-    I2cAddr = CPU1_9545_I2C_ADDR;
-    SfpPort = I2C_PORT_SFP;
-  }
-
-  Status = I2CInit (Socket, SfpPort, Normal);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",
-            __FUNCTION__, __LINE__, Socket, Status));
-    return Status;
-  }
-
-  SpdDev.Socket = Socket;
-  SpdDev.DeviceType = DEVICE_TYPE_SPD;
-  SpdDev.Port = SfpPort;
-  SpdDev.SlaveDeviceAddress = I2cAddr;
-  RegAddr = 0x0;
-  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);
-
-  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));
-    return Status;
-  }
-
-  SpdDev.Socket = Socket;
-  SpdDev.DeviceType = DEVICE_TYPE_SPD;
-  SpdDev.Port = SfpPort;
-  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;
-
-  RegAddr = SPF_SPEED_OFFSET;
-  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));
-    return Status;
-  }
-
-  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",
-         SfpSpeed));
-
-  if (SfpSpeed == SFP_10G_SPEED_VAL) {
-    *FiberSpeed = SFP_10G_SPEED;
-  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {
-    *FiberSpeed = SFP_25G_SPEED;
-  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||
-             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {
-    *FiberSpeed = SFP_GE_SPEED;
-  }
-
-  return EFI_SUCCESS;
-}
-
-//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED
-UINT32
-GetCpu2FiberType (
-  UINT8* Fiber1Type,
-  UINT8* Fiber2Type,
-  UINT8* Fiber100Ge
-  )
-{
-  EFI_STATUS  Status;
-  UINT16      SfpNum1;
-  UINT8       SfpSpeed1;
-  UINT16      SfpNum2;
-  UINT8       SfpSpeed2;
-
-  SfpNum1 = 0x1;
-  SfpSpeed1 = SFP_10G_SPEED;
-  SfpNum2 = 0x2;
-  SfpSpeed2 = SFP_10G_SPEED;
-  *Fiber100Ge = 0x0;
-  *Fiber1Type = SFP_10G_SPEED;
-  *Fiber2Type = SFP_10G_SPEED;
-
-  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
-    // 100 Ge card
-    *Fiber1Type = SFP_10G_SPEED;
-    *Fiber2Type = SFP_10G_SPEED;
-    *Fiber100Ge = SFP_100G_SPEED;
-    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));
-  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {
-    *Fiber100Ge = 0x0;
-    *Fiber1Type = SFP_10G_SPEED;
-    *Fiber2Type = SFP_10G_SPEED;
-    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
-      // Fiber detected in CPU2 slot0, read speed via i2c
-      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);
-      if (EFI_ERROR (Status)) {
-        DEBUG((DEBUG_ERROR,
-               "Get Socket1 Sfp%d Speed Error: %r.\n",
-               SfpNum1,
-               Status));
-        return Status;
-      }
-      if (SfpSpeed1 == SFP_25G_SPEED) {
-        // P1 don't support 25G, so set speed to 10G
-        *Fiber1Type = SFP_10G_SPEED;
-      } else {
-        *Fiber1Type = SfpSpeed1;
-      }
-    } else {
-      // No fiber, set speed to 10G
-      *Fiber1Type = SFP_10G_SPEED;
-    }
-
-    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
-      // Fiber detected in CPU2 slot1, read speed via i2c
-      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);
-      if (EFI_ERROR (Status)) {
-        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
-        return Status;
-      }
-      if (SfpSpeed2 == SFP_25G_SPEED) {
-        *Fiber2Type = SFP_10G_SPEED;
-      } else {
-        *Fiber2Type = SfpSpeed2;
-      }
-    } else {
-      // No fiber, set speed to 10G
-      *Fiber2Type = SFP_10G_SPEED;
-    }
-  } else {
-    // 100Ge/10Ge/Ge Fiber is not found.
-    *Fiber1Type = SFP_10G_SPEED;
-    *Fiber2Type = SFP_10G_SPEED;
-    *Fiber100Ge = 0x0;
-  }
-
-  return EFI_SUCCESS;
-}
-
-//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED
-UINT32
-GetCpu1FiberType (
-  UINT8* Fiber1Type,
-  UINT8* Fiber2Type
-  )
-{
-  EFI_STATUS  Status;
-  UINT16      SfpNum1;
-  UINT8       SfpSpeed1;
-  UINT16      SfpNum2;
-  UINT8       SfpSpeed2;
-
-  SfpNum1 = 0x1;
-  SfpSpeed1 = SFP_10G_SPEED;
-  SfpNum2 = 0x2;
-  SfpSpeed2 = SFP_10G_SPEED;
-  *Fiber1Type = SFP_10G_SPEED;
-  *Fiber2Type = SFP_10G_SPEED;
-  // Fiber detected in CPU1 slot0, read speed via i2c
-  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
-    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);
-    if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",
-              SfpNum1, Status));
-      return Status;
-    }
-    *Fiber1Type = SfpSpeed1;
-  } else {
-    *Fiber1Type = SFP_10G_SPEED;
-  }
-
-  // Fiber detected in CPU1 slot1, read speed via i2c
-  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
-    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);
-    if (EFI_ERROR (Status)) {
-      *Fiber2Type = SFP_10G_SPEED;
-      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
-      return Status;
-    }
-    *Fiber2Type = SfpSpeed2;
-  } else {
-    *Fiber2Type = SFP_10G_SPEED;
-  }
-
-  return EFI_SUCCESS;
-}
-
 UINT16 MakeCrcCheckSum (
   UINT8 *Buffer,
   UINT32 Length
@@ -567,3 +346,42 @@  OemIsInitEth (
 {
   return TRUE;
 }
+
+EFI_STATUS ConfigCDR(UINT32 Socket)
+{
+    return EFI_SUCCESS;
+}
+
+UINT32 OemGetNclConfOffset (UINT32 Socket)
+{
+    UINT32           Cpu1NclConfOffet = 0;
+    UINT32           Cpu2NclConfOffet = 0;
+
+    if (0 == Socket) {
+      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);
+      return Cpu1NclConfOffet;
+    } else {
+      //2P only
+      // P1
+      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
+        Cpu2NclConfOffet =  0x20000;
+      } else {
+        Cpu2NclConfOffet =  0x10000;
+      }
+      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);
+      return Cpu2NclConfOffet;
+    }
+}
+
+UINT32 OemGetNetPortNum (UINT32 Socket)
+{
+    if (0 == Socket){
+    return SOCKET0_NET_PORT_NUM;
+    }
+
+    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
+      return SOCKET1_NET_PORT_100G;
+    } else {
+      return SOCKET1_NET_PORT_NUM;
+    }
+}