[Linaro-uefi,v4,01/56] D03: enhance RTC lock acquiring

We would acquire I2C lock only when I2C status in CPLD shows idle,
however, acquiring lock will still fail for BMC might acquire the lock
at exactly the same time.
So we add additional check to see if we really get the lock. Timeout process
is also added to avoid system hang due to possible deadlock or device error.
Code style is improved as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Peicong Li <lipeicong@huawei.com>
Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
---
 Platforms/Hisilicon/D03/Include/Library/CpldD03.h  |  4 ++
 .../DS3231RealTimeClockLib.c                       | 77 ++++++++++++++++------
 .../DS3231RealTimeClockLib.inf                     |  2 +
 3 files changed, 64 insertions(+), 19 deletions(-)

On Sat, Nov 19, 2016 at 04:37:16PM +0800, Heyi Guo wrote:
> We would acquire I2C lock only when I2C status in CPLD shows idle,
> however, acquiring lock will still fail for BMC might acquire the lock
> at exactly the same time.
> So we add additional check to see if we really get the lock. Timeout process
> is also added to avoid system hang due to possible deadlock or device error.
> Code style is improved as well.

This looks like a completely new patch (at least the gmail web client
can find no other patch modifying these files).

Can previously unsubmitted patches be added only at the end of the
patch set, please?

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Peicong Li <lipeicong@huawei.com>
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> ---
>  Platforms/Hisilicon/D03/Include/Library/CpldD03.h  |  4 ++
>  .../DS3231RealTimeClockLib.c                       | 77 ++++++++++++++++------
>  .../DS3231RealTimeClockLib.inf                     |  2 +
>  3 files changed, 64 insertions(+), 19 deletions(-)
> 
> diff --git a/Platforms/Hisilicon/D03/Include/Library/CpldD03.h b/Platforms/Hisilicon/D03/Include/Library/CpldD03.h
> index 78aec2f..456bf4b 100644
> --- a/Platforms/Hisilicon/D03/Include/Library/CpldD03.h
> +++ b/Platforms/Hisilicon/D03/Include/Library/CpldD03.h
> @@ -17,5 +17,9 @@
>  #define __CPLD_D03_H__
>  
>  #define CPLD_BIOSINDICATE_FLAG  0x09
> +#define CPLD_I2C_SWITCH_FLAG    0x17
> +#define CPU_GET_I2C_CONTROL     BIT2
> +#define BMC_I2C_STATUS          BIT3
> +
>  
>  #endif
> diff --git a/Platforms/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c b/Platforms/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
> index fa63027..89afa08 100644
> --- a/Platforms/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
> +++ b/Platforms/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
> @@ -39,6 +39,7 @@
>  #include <Library/I2CLib.h>
>  #include "DS3231RealTimeClock.h"
>  #include <Library/CpldIoLib.h>
> +#include <Library/CpldD03.h>

Please insert before Library/CpldIoLib.h, to improve sorting?

>  
>  extern I2C_DEVICE gDS3231RtcDevice;
>  
> @@ -56,6 +57,42 @@ IdentifyDS3231 (
>  }
>  
>  EFI_STATUS
> +SwitchRtcI2cChannelAndLock(VOID)
> +{
> +  UINT8   Temp;
> +  UINT8   Count;
> +
> +  for (Count = 0; Count < 20; Count++){
> +    Temp = ReadCpldReg(CPLD_I2C_SWITCH_FLAG);
> +
> +    if ((Temp & BMC_I2C_STATUS) != 0){

Can we have a comment with this delay, please?

> +      MicroSecondDelay(30000);
> +
> +      continue;
> +    }
> +
> +    Temp = ReadCpldReg(CPLD_I2C_SWITCH_FLAG);
> +    Temp = Temp | CPU_GET_I2C_CONTROL;
> +    WriteCpldReg(CPLD_I2C_SWITCH_FLAG, Temp);

Can we have a comment with this delay, please?

> +    MicroSecondDelay(2);
> +    Temp = ReadCpldReg(CPLD_I2C_SWITCH_FLAG);
> +
> +    if ((Temp & CPU_GET_I2C_CONTROL) == CPU_GET_I2C_CONTROL){
> +      return EFI_SUCCESS;
> +    }
> +

Can we have a comment with this delay, please?

> +    MicroSecondDelay(30000);
> +  }
> +
> +  Temp = ReadCpldReg(CPLD_I2C_SWITCH_FLAG);
> +  Temp = Temp & ~CPU_GET_I2C_CONTROL;
> +  WriteCpldReg(CPLD_I2C_SWITCH_FLAG, Temp);
> +
> +  return EFI_NOT_READY;
> +}
> +
> +
> +EFI_STATUS
>  InitializeDS3231 (
>    VOID
>    )
> @@ -136,19 +173,17 @@ LibGetTime (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -
> -
> -  Temp = ReadCpldReg(0x17);
> -  while( (Temp & BIT3) != 0)
> -  {
> -      Temp = ReadCpldReg(0x17);
> +  Status = SwitchRtcI2cChannelAndLock();
> +  if(EFI_ERROR (Status)) {
> +    return Status;
>    }
> -  WriteCpldReg(0x17,0x4);
> +
>    // Initialize the hardware if not already done
>    if (!mDS3231Initialized) {
>      Status = InitializeDS3231 ();
>      if (EFI_ERROR (Status)) {
> -      return EFI_NOT_READY;
> +      Status = EFI_NOT_READY;
> +      goto GExit;
>      }
>    }
>  
> @@ -175,7 +210,8 @@ LibGetTime (
>  
>    BaseHour = 0;
>    if((Temp&0x30) == 0x30){
> -    return EFI_DEVICE_ERROR;
> +    Status = EFI_DEVICE_ERROR;
> +    goto GExit;
>    }else if(Temp&0x20){
>      BaseHour = 20;
>    }else if(Temp&0x10){
> @@ -196,11 +232,15 @@ LibGetTime (
>    Time->TimeZone    = EFI_UNSPECIFIED_TIMEZONE;
>  
>    if((EFI_ERROR(Status)) || (!IsTimeValid(Time)) || ((Time->Year - BaseYear) > 99)) {
> -    return EFI_DEVICE_ERROR;
> +    Status = EFI_UNSUPPORTED;
>    }
>  
> -  WriteCpldReg(0x17,0x0);
> -  return EFI_SUCCESS;
> +GExit:
> +  Temp = ReadCpldReg(CPLD_I2C_SWITCH_FLAG);
> +  Temp = Temp & ~CPU_GET_I2C_CONTROL;
> +  WriteCpldReg(CPLD_I2C_SWITCH_FLAG, Temp);
> +
> +  return Status;
>  
>  }
>  
> @@ -234,13 +274,10 @@ LibSetTime (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -
> -  Temp = ReadCpldReg(0x17);
> -  while( (Temp & BIT3) != 0)
> -  {
> -      Temp = ReadCpldReg(0x17);
> +  Status = SwitchRtcI2cChannelAndLock();
> +  if(EFI_ERROR (Status)) {
> +    return Status;
>    }
> -  WriteCpldReg(0x17,0x4);
>  
>    // Initialize the hardware if not already done
>    if (!mDS3231Initialized) {
> @@ -313,7 +350,9 @@ LibSetTime (
>  
>    EXIT:
>  
> -  WriteCpldReg(0x17,0x0);
> +  Temp = ReadCpldReg(CPLD_I2C_SWITCH_FLAG);
> +  Temp = Temp & ~CPU_GET_I2C_CONTROL;
> +  WriteCpldReg(CPLD_I2C_SWITCH_FLAG, Temp);
>  
>    return Status;
>  }
> diff --git a/Platforms/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf b/Platforms/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf
> index 8121f37..a28a975 100644
> --- a/Platforms/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf
> +++ b/Platforms/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf
> @@ -31,6 +31,7 @@
>    EmbeddedPkg/EmbeddedPkg.dec
>    OpenPlatformPkg/OpenPlatformPkg.dec
>    OpenPlatformPkg/Chips/Hisilicon/HisiPkg.dec
> +  OpenPlatformPkg/Platforms/Hisilicon/D03/D03.dec

Can this move before OpenPlatformPkg/Chips/Hisilicon/HisiPkg.dec?

>  
>  [LibraryClasses]
>    IoLib
> @@ -42,6 +43,7 @@
>  # Use EFiAtRuntime to check stage
>    UefiRuntimeLib
>    EfiTimeBaseLib
> +  CpldIoLib

And can this be inserted before EfiTimeBaseLib?

Regards,

Leif

>  
>  [Pcd]
>  
> -- 
> 1.9.1
>