Re: [PATCH edk2-platforms v1 05/12] Hisilicon/D06: Move some functions to OemMiscLib

[Ming Huang <ming.huang@linaro.org>]

On 11/14/2018 8:04 AM, Leif Lindholm wrote:
> On Mon, Oct 29, 2018 at 11:32:42AM +0800, Ming Huang wrote:
>> As M41T83RealTimeClockLib is common library, so move two cpld
>> relative functions to OemMiscLib and rename this two functions.
> 
> This would be more clear as "platform specific" than "cpld relative".
> 
> I did not realise this wasn't a Hisilicon component when reviewing the
> original set.
> 
> I approve of this change, but can you tell me why it is included in
> this set? If the goal is to make the M41T83 support platform
> independent, should the library also move to Silicon/ST/?

This change had make in internal master and the next patch is base on
this patch, so I add this patch. Main gist is remove platform specific
functions from M41T83RealTimeClockLib.

> 
> /
>     Leif
> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf |  1 -
>>  Silicon/Hisilicon/Include/Library/OemMiscLib.h                              |  9 ++
>>  Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h      |  4 -
>>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c                | 82 ++++++++++++++++++
>>  Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c   | 90 ++------------------
>>  5 files changed, 98 insertions(+), 88 deletions(-)
>>
>> diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>> index e0bf6b3f24..4e963fd453 100644
>> --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>> +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>> @@ -27,7 +27,6 @@
>>  [Packages]
>>    EmbeddedPkg/EmbeddedPkg.dec
>>    MdePkg/MdePkg.dec
>> -  Platform/Hisilicon/D06/D06.dec
>>    Silicon/Hisilicon/HisiPkg.dec
>>  
>>  [LibraryClasses]
>> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> index 86ea6a1b3d..0d7bf71b17 100644
>> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> @@ -53,4 +53,13 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID);
>>  
>>  extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
>>  EFI_HII_HANDLE EFIAPI OemGetPackages ();
>> +
>> +VOID
>> +OemReleaseOwnershipOfRtc (
>> +  VOID
>> +  );
>> +EFI_STATUS
>> +OemSwitchRtcI2cChannelAndLock (
>> +  VOID
>> +  );
>>  #endif
>> diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h
>> index d985055d9b..f329108858 100644
>> --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h
>> +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h
>> @@ -16,12 +16,8 @@
>>  #ifndef __M41T83_REAL_TIME_CLOCK_H__
>>  #define __M41T83_REAL_TIME_CLOCK_H__
>>  
>> -// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need.
>> -#define RTC_DELAY_30_MS            30000
>>  // The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need.
>>  #define RTC_DELAY_1000_MACROSECOND 1000
>> -// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need.
>> -#define RTC_DELAY_2_MACROSECOND    2
>>  
>>  #define M41T83_REGADDR_DOTSECONDS       0x00
>>  #define M41T83_REGADDR_SECONDS          0x01
>> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> index 2a9db46d1f..64d167d18a 100644
>> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> @@ -17,6 +17,7 @@
>>  #include <PlatformArch.h>
>>  #include <Library/BaseMemoryLib.h>
>>  #include <Library/CpldD06.h>
>> +#include <Library/CpldIoLib.h>
>>  #include <Library/DebugLib.h>
>>  #include <Library/IoLib.h>
>>  #include <Library/LpcLib.h>
>> @@ -27,6 +28,12 @@
>>  #include <Library/SerdesLib.h>
>>  #include <Library/SerialPortLib.h>
>>  #include <Library/TimerLib.h>
>> +#include <Library/UefiRuntimeLib.h>
>> +
>> +// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need.
>> +#define RTC_DELAY_30_MS            30000
>> +// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need.
>> +#define RTC_DELAY_2_MACROSECOND    2
>>  
>>  REPORT_PCIEDIDVID2BMC PcieDeviceToReport[PCIEDEVICE_REPORT_MAX] = {
>>    {67,0,0,0},
>> @@ -207,3 +214,78 @@ OemIsNeedDisableExpanderBuffer (
>>  {
>>    return TRUE;
>>  }
>> +
>> +EFI_STATUS
>> +OemSwitchRtcI2cChannelAndLock (
>> +  VOID
>> +  )
>> +{
>> +  UINT8   Temp;
>> +  UINT8   Count;
>> +
>> +  for (Count = 0; Count < 100; Count++) {
>> +    // To get the other side's state is idle first
>> +    Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> +    if ((Temp & BIT3) != 0) {
>> +      (VOID) MicroSecondDelay (RTC_DELAY_30_MS);
>> +      // Try 100 times, if BMC has not released the bus, return preemption failed
>> +      if (Count == 99) {
>> +        if (!EfiAtRuntime ()) {
>> +          DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Clear cpu_i2c_rtc_state 100 times fail!\n",
>> +            __FUNCTION__, __LINE__));
>> +        }
>> +        return EFI_DEVICE_ERROR;
>> +      }
>> +      continue;
>> +    }
>> +
>> +    // if BMC free the bus, can be set 1 preemption
>> +    Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> +    Temp = Temp | CPU_GET_I2C_CONTROL;
>> +    // CPU occupied RTC I2C State
>> +    WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp);
>> +    (VOID) MicroSecondDelay (RTC_DELAY_2_MACROSECOND);
>> +    Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> +    // Is preempt success
>> +    if(CPU_GET_I2C_CONTROL == (Temp & CPU_GET_I2C_CONTROL)) {
>> +      break;
>> +    }
>> +    if (Count == 99) {
>> +      if (!EfiAtRuntime ()) {
>> +        DEBUG((DEBUG_ERROR, "[%a]:[%dL]  Clear cpu_i2c_rtc_state fail !!! \n",
>> +          __FUNCTION__, __LINE__));
>> +      }
>> +      return EFI_DEVICE_ERROR;
>> +    }
>> +    (VOID) MicroSecondDelay (RTC_DELAY_30_MS);
>> +  }
>> +
>> +  //Polling BMC RTC I2C status
>> +  for (Count = 0; Count < 100; Count++) {
>> +    Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> +    if ((Temp & BIT3) == 0) {
>> +      return EFI_SUCCESS;
>> +    }
>> +    (VOID) MicroSecondDelay (RTC_DELAY_30_MS);
>> +  }
>> +
>> +  //If the BMC occupies the RTC I2C Channel, write back the CPU side is idle
>> +  // or the subsequent BMC will not preempt
>> +  Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> +  Temp = Temp & (~CPU_GET_I2C_CONTROL);
>> +  WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp);
>> +
>> +  return EFI_NOT_READY;
>> +}
>> +
>> +VOID
>> +OemReleaseOwnershipOfRtc (
>> +  VOID
>> +  )
>> +{
>> +  UINT8   Temp;
>> +
>> +  Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> +  Temp = Temp & ~CPU_GET_I2C_CONTROL;
>> +  WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp);
>> +}
>> diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>> index 0670f9c5f4..1f50ad4b64 100644
>> --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>> +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>> @@ -17,10 +17,10 @@
>>  #include <PiDxe.h>
>>  #include <Library/BaseLib.h>
>>  #include <Library/BaseMemoryLib.h>
>> -#include <Library/CpldD06.h>
>>  #include <Library/CpldIoLib.h>
>>  #include <Library/DebugLib.h>
>>  #include <Library/I2CLib.h>
>> +#include <Library/OemMiscLib.h>
>>  #include <Library/TimeBaseLib.h>
>>  #include <Library/TimerLib.h>
>>  #include <Library/UefiLib.h>
>> @@ -32,70 +32,6 @@ extern I2C_DEVICE gRtcDevice;
>>  
>>  STATIC EFI_LOCK  mRtcLock;
>>  
>> -EFI_STATUS
>> -SwitchRtcI2cChannelAndLock (
>> -  VOID
>> -  )
>> -{
>> -  UINT8   Temp;
>> -  UINT8   Count;
>> -
>> -  for (Count = 0; Count < 100; Count++) {
>> -    // To get the other side's state is idle first
>> -    Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> -    if ((Temp & BIT3) != 0) {
>> -      (VOID) MicroSecondDelay (RTC_DELAY_30_MS);
>> -      // Try 100 times, if BMC has not released the bus, return preemption failed
>> -      if (Count == 99) {
>> -        if (!EfiAtRuntime ()) {
>> -          DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Clear cpu_i2c_rtc_state 100 times fail!\n",
>> -            __FUNCTION__, __LINE__));
>> -        }
>> -        return EFI_DEVICE_ERROR;
>> -      }
>> -      continue;
>> -    }
>> -
>> -    // if BMC free the bus, can be set 1 preemption
>> -    Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> -    Temp = Temp | CPU_GET_I2C_CONTROL;
>> -    // CPU occupied RTC I2C State
>> -    WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp);
>> -    (VOID) MicroSecondDelay (RTC_DELAY_2_MACROSECOND);
>> -    Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> -    // Is preempt success
>> -    if(CPU_GET_I2C_CONTROL == (Temp & CPU_GET_I2C_CONTROL)) {
>> -      break;
>> -    }
>> -    if (Count == 99) {
>> -      if (!EfiAtRuntime ()) {
>> -        DEBUG((DEBUG_ERROR, "[%a]:[%dL]  Clear cpu_i2c_rtc_state fail !!! \n",
>> -          __FUNCTION__, __LINE__));
>> -      }
>> -      return EFI_DEVICE_ERROR;
>> -    }
>> -    (VOID) MicroSecondDelay (RTC_DELAY_30_MS);
>> -  }
>> -
>> -  //Polling BMC RTC I2C status
>> -  for (Count = 0; Count < 100; Count++) {
>> -    Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> -    if ((Temp & BIT3) == 0) {
>> -      return EFI_SUCCESS;
>> -    }
>> -    (VOID) MicroSecondDelay (RTC_DELAY_30_MS);
>> -  }
>> -
>> -  //If the BMC occupies the RTC I2C Channel, write back the CPU side is idle
>> -  // or the subsequent BMC will not preempt
>> -  Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> -  Temp = Temp & (~CPU_GET_I2C_CONTROL);
>> -  WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp);
>> -
>> -  return EFI_NOT_READY;
>> -}
>> -
>> -
>>  /**
>>    Read RTC content through its registers.
>>  
>> @@ -142,18 +78,6 @@ RtcWrite (
>>    return Status;
>>  }
>>  
>> -VOID
>> -ReleaseOwnershipOfRtc (
>> -  VOID
>> -  )
>> -{
>> -  UINT8   Temp;
>> -
>> -  Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG);
>> -  Temp = Temp & ~CPU_GET_I2C_CONTROL;
>> -  WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp);
>> -}
>> -
>>  
>>  EFI_STATUS
>>  InitializeM41T83 (
>> @@ -178,7 +102,7 @@ InitializeM41T83 (
>>      return Status;
>>    }
>>  
>> -  Status = SwitchRtcI2cChannelAndLock ();
>> +  Status = OemSwitchRtcI2cChannelAndLock ();
>>    if (EFI_ERROR (Status)) {
>>      DEBUG ((DEBUG_ERROR, "Get i2c preemption failed: %r\n", Status));
>>      if (!EfiAtRuntime ()) {
>> @@ -231,7 +155,7 @@ InitializeM41T83 (
>>  
>>  Exit:
>>    // Release RTC Lock.
>> -  ReleaseOwnershipOfRtc ();
>> +  OemReleaseOwnershipOfRtc ();
>>    if (!EfiAtRuntime ()) {
>>      EfiReleaseLock (&mRtcLock);
>>    }
>> @@ -274,7 +198,7 @@ LibSetTime (
>>      return EFI_INVALID_PARAMETER;
>>    }
>>  
>> -  Status = SwitchRtcI2cChannelAndLock ();
>> +  Status = OemSwitchRtcI2cChannelAndLock ();
>>    if (EFI_ERROR (Status)) {
>>      return Status;
>>    }
>> @@ -332,7 +256,7 @@ LibSetTime (
>>    }
>>  
>>  Exit:
>> -  ReleaseOwnershipOfRtc ();
>> +  OemReleaseOwnershipOfRtc ();
>>    // Release RTC Lock.
>>    if (!EfiAtRuntime ()) {
>>      if (EFI_ERROR (Status)) {
>> @@ -377,7 +301,7 @@ LibGetTime (
>>      return EFI_INVALID_PARAMETER;
>>    }
>>  
>> -  Status = SwitchRtcI2cChannelAndLock ();
>> +  Status = OemSwitchRtcI2cChannelAndLock ();
>>    if (EFI_ERROR (Status)) {
>>      return Status;
>>    }
>> @@ -422,7 +346,7 @@ LibGetTime (
>>    }
>>  
>>  Exit:
>> -  ReleaseOwnershipOfRtc ();
>> +  OemReleaseOwnershipOfRtc ();
>>    // Release RTC Lock.
>>    if (!EfiAtRuntime ()) {
>>      if (EFI_ERROR (Status)) {
>> -- 
>> 2.18.0
>>