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

2018-08-22 Thread Leif Lindholm 
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 
> >> ---
> >>  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 
> >>  #include 
> >>  
> >> +// The HNS I2C port 5 is under I2C extender
> >> +#define I2C_EXTENDER_PORT_HNS5
> >> +
> >>  #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 
> 
> 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
> >>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

2018-08-22 Thread Ming 



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 
>> ---
>>  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 ?

> 
>> 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 
>>  #include 
>>  
>> +// The HNS I2C port 5 is under I2C extender
>> +#define I2C_EXTENDER_PORT_HNS5
>> +
>>  #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 

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
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

2018-08-22 Thread Leif Lindholm 
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 
> ---
>  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

> 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 
>  #include 
>  
> +// The HNS I2C port 5 is under I2C extender
> +#define I2C_EXTENDER_PORT_HNS5
> +
>  #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

> +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
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

2018-08-14 Thread Ming Huang 
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 
---
 Silicon/Hisilicon/Library/I2CLib/I2CHw.h  |  3 +++
 Silicon/Hisilicon/Library/I2CLib/I2CLib.c | 21 +++-
 2 files changed, 19 insertions(+), 5 deletions(-)

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 
 #include 
 
+// The HNS I2C port 5 is under I2C extender
+#define I2C_EXTENDER_PORT_HNS5
+
 #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.
+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

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