Re: [PATCH edk2-platforms v1 14/38] Silicon/Hisilicon/D06: Fix I2C enable fail issue for D06

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Jul 24, 2018 at 03:08:58PM +0800, Ming Huang wrote:
> From: shaochangliang <shaochangliang@huawei.com>
> 
> I2C may enable failed in D06, so retry I2C enable while
> enable failed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shaochangliang <shaochangliang@huawei.com>
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> ---
>  Silicon/Hisilicon/Library/I2CLib/I2CLib.c | 22 ++++++++++++--------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> index b5b388d756..ecd2f07c4d 100644
> --- a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> +++ b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> @@ -83,6 +83,7 @@ I2C_Enable(UINT32 Socket,UINT8 Port)
>  {
>      I2C0_ENABLE_U           I2cEnableReg;

Ugh, indentation is incorrect in the original.
Can you correct that in a patch preceding this one?

>      I2C0_ENABLE_STATUS_U    I2cEnableStatusReg;
> +    UINT32                  ulTimeCnt = I2C_READ_TIMEOUT;

What is ul?

>  
>      UINTN Base = GetI2cBase(Socket, Port);
>  
> @@ -91,16 +92,19 @@ I2C_Enable(UINT32 Socket,UINT8 Port)
>      I2cEnableReg.bits.enable = 1;
>      I2C_REG_WRITE(Base + I2C_ENABLE_OFFSET, I2cEnableReg.Val32);
>  
> -
> -    I2C_REG_READ(Base + I2C_ENABLE_STATUS_OFFSET, I2cEnableStatusReg.Val32);
> -    if (1 == I2cEnableStatusReg.bits.ic_en)
> +    do
>      {

Move that brace up to the previous line.

> -        return EFI_SUCCESS;
> -    }
> -    else
> -    {
> -        return EFI_DEVICE_ERROR;
> -    }
> +        I2C_Delay(10000);

Why 10000?
Do we need a MemoryFence ()?

> +
> +        ulTimeCnt--;
> +        I2C_REG_READ(Base + I2C_ENABLE_STATUS_OFFSET, I2cEnableStatusReg.Val32);
> +        if (0 == ulTimeCnt)
> +        {

Move that brace up to previous line.

> +            return EFI_DEVICE_ERROR;
> +        }
> +    }while (0 == I2cEnableStatusReg.bits.ic_en);

Space after }

/
    Leif

> +
> +    return EFI_SUCCESS;
>  }
>  
>  void I2C_SetTarget(UINT32 Socket,UINT8 Port,UINT32 I2cDeviceAddr)
> -- 
> 2.17.0
>