Re: [PATCH edk2-platforms v2 23/43] Silicon/Hisilicon/D06: Add I2C delay for HNS auto config

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

On Wed, Aug 22, 2018 at 11:16:26PM +0800, Ming wrote:
> 
> 
> On 8/22/2018 10:27 PM, Leif Lindholm wrote:
> > On Tue, Aug 14, 2018 at 04:08:43PM +0800, Ming Huang wrote:
> >> Because I2C Port5 salve device connect under I2C extender
> >> (9545 device), it will cost more time to access I2C slave
> >> device, so add delay time for HNS auto config.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> >> ---
> >>  Silicon/Hisilicon/Library/I2CLib/I2CHw.h  |  3 +++
> >>  Silicon/Hisilicon/Library/I2CLib/I2CLib.c | 21 +++++++++++++++-----
> >>  2 files changed, 19 insertions(+), 5 deletions(-)
> > 
> > Like previous patch, please change subject line - this affects d03,
> > d05, d06.
> > 
> > Silicon/Hisilicon: Add I2CLib delay for HNS auto config
> 
> Should this patch reorder before the first D06 patch ?

Yes please.

/
    Leif

> > 
> >> diff --git a/Silicon/Hisilicon/Library/I2CLib/I2CHw.h b/Silicon/Hisilicon/Library/I2CLib/I2CHw.h
> >> index fa954c7937..d77aea509e 100644
> >> --- a/Silicon/Hisilicon/Library/I2CLib/I2CHw.h
> >> +++ b/Silicon/Hisilicon/Library/I2CLib/I2CHw.h
> >> @@ -19,6 +19,9 @@
> >>  #include <Uefi.h>
> >>  #include <Library/IoLib.h>
> >>  
> >> +// The HNS I2C port 5 is under I2C extender
> >> +#define I2C_EXTENDER_PORT_HNS        5
> >> +
> >>  #define I2C_READ_TIMEOUT             500
> >>  #define I2C_DRV_ONCE_WRITE_BYTES_NUM 8
> >>  #define I2C_DRV_ONCE_READ_BYTES_NUM  8
> >> diff --git a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> >> index d67ddc7f9b..59633106ce 100644
> >> --- a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> >> +++ b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> >> @@ -258,8 +258,13 @@ CheckI2CTimeOut (
> >>    if (Transfer == I2CTx) {
> >>      Fifo = I2C_GetTxStatus (Socket, Port);
> >>      while (Fifo != 0) {
> >> -      // This is a empirical value for I2C delay. MemoryFance is no need here.
> >> -      I2C_Delay (2);
> >> +      if (Port == I2C_EXTENDER_PORT_HNS) {
> >> +        // This is a empirical value for I2C delay. MemoryFance is no need here.
> > 
> > MemoryFance ->
> > MemoryFence
> > 
> > (You may want to search and replace that on the whole set.)
> > 
> > With that:
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> 
> Replace they in v4.
> Thanks.
> 
> > 
> > /
> >     Leif
> > 
> >> +        I2C_Delay (1000);
> >> +      } else {
> >> +        // This is a empirical value for I2C delay. MemoryFance is no need here.
> >> +        I2C_Delay (2);
> >> +      }
> >>        if (++Times > I2C_READ_TIMEOUT) {
> >>          (VOID)I2C_Disable (Socket, Port);
> >>          return EFI_TIMEOUT;
> >> @@ -269,8 +274,13 @@ CheckI2CTimeOut (
> >>    } else {
> >>      Fifo = I2C_GetRxStatus (Socket, Port);
> >>      while (Fifo == 0) {
> >> -      // This is a empirical value for I2C delay. MemoryFance is no need here.
> >> -      I2C_Delay (2);
> >> +      if (Port == I2C_EXTENDER_PORT_HNS) {
> >> +        // This is a empirical value for I2C delay. MemoryFance is no need here.
> >> +        I2C_Delay (1000);
> >> +      } else {
> >> +        // This is a empirical value for I2C delay. MemoryFance is no need here.
> >> +        I2C_Delay (2);
> >> +      }
> >>        if (++Times > I2C_READ_TIMEOUT) {
> >>          (VOID)I2C_Disable (Socket, Port);
> >>          return EFI_TIMEOUT;
> >> @@ -369,7 +379,8 @@ I2CWrite(
> >>      Times = 0;
> >>      Fifo = I2C_GetTxStatus (I2cInfo->Socket, I2cInfo->Port);
> >>      while (Fifo > I2C_TXRX_THRESHOLD) {
> >> -      I2C_Delay (2);
> >> +      // This is a empirical value for I2C delay. MemoryFance is no need here.
> >> +      I2C_Delay (1000);
> >>        if (++Times > I2C_READ_TIMEOUT) {
> >>          (VOID)I2C_Disable (I2cInfo->Socket, I2cInfo->Port);
> >>          return EFI_TIMEOUT;
> >> -- 
> >> 2.17.0
> >>