[PATCH edk2-platforms v1 00/16] Fix issues and improve D0x

[PATCH edk2-platforms v1 00/16] Fix issues and improve D0x

[Ming Huang]

Main Changes :
1 Use new flash layout;
2 Modify for M7 self-Adapte support;
3 Add PCI_OSC_SUPPORT for ACPI/DSDT;
4 Change HCCS speed from 30G to 26G;
5 Rename StartupAp() function;
6 Fix access variable fail issue;
7 Optimize SAS driver for reducing boot time;
8 Add DriverHealthManagerDxe;
9 Remove useless code;

Code can also be found in github:
https://github.com/hisilicon/OpenPlatformPkg.git
branch: 1902-platforms-v1


Jason Zhang (2):
  Hisilicon/D06: Fix access variable fail issue
  Hisilicon/D06: Add more PCIe port INT-x support

Ming Huang (13):
  Hisilicon/D0x: Remove SerdesLib
  Hisilicon/D0x: Add DriverHealthManagerDxe
  Hisilicon/D06: Optimize SAS driver for reducing boot time
  Hisilicon/D0x: Rename StartupAp() function
  Hisilicon/D06: Change HCCS speed from 30G to 26G
  Hisilicon/D06: Add PCI_OSC_SUPPORT
  Hisilicon/D06: Modify for M7 self-Adapte support
  Hisilicon/D06: Add Setup Item "Support DPC"
  Hisilicon/D06: Use new flash layout
  Hisilicon/D06: Remove SECURE_BOOT_ENABLE definition
  Hisilicon/D0x: Remove SP805 watchdog pcd
  Hisilicon/D06: Use CalculateCrc16 in BaseLib
  Hisilicon/D0x: Modify version to 19.02

xingjiang tang (1):
  Hisilicon/D06: Add OemGetCpuFreq to encapsulate difference

 Platform/Hisilicon/D03/D03.dsc                                       |   8 +-
 Platform/Hisilicon/D05/D05.dsc                                       |   8 +-
 Platform/Hisilicon/D06/D06.dsc                                       |  19 +-
 Platform/Hisilicon/D03/D03.fdf                                       |   1 +
 Platform/Hisilicon/D05/D05.fdf                                       |   1 +
 Platform/Hisilicon/D06/D06.fdf                                       |  18 +-
 Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf     |   1 -
 Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf |   1 -
 Platform/Hisilicon/D06/Include/Library/CpldD06.h                     |   4 +
 Silicon/Hisilicon/Include/Library/OemConfigData.h                    |   1 +
 Silicon/Hisilicon/Include/Library/OemMiscLib.h                       |  12 +
 Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h               |   2 +-
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr       |   4 +-
 Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c          |   2 +-
 Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c     |   2 +-
 Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c          |   2 +-
 Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c         |   3 +-
 Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c          |   2 +-
 Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c         |  24 ++
 Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c                 | 341 +++-----------------
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c            |   6 +-
 Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl         | 129 +++++++-
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr         | 197 +----------
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni  |   3 +-
 24 files changed, 238 insertions(+), 553 deletions(-)

-- 
2.9.5

[PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

[Ming Huang]

SerdesLib is useless for SmbiosMiscDxe and D06, so remove it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Platform/Hisilicon/D06/D06.dsc                                   | 2 --
 Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
 2 files changed, 3 deletions(-)

diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
index 396bd03c9d24..cbbd99e4a659 100644
--- a/Platform/Hisilicon/D06/D06.dsc
+++ b/Platform/Hisilicon/D06/D06.dsc
@@ -64,8 +64,6 @@ [LibraryClasses.common]
 
   CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
 
-  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
-
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
   RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
   OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
index 61cead7779b9..8e5c56fa41fd 100644
--- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
+++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
@@ -77,7 +77,6 @@ [LibraryClasses]
 
   IpmiCmdLib
 
-  SerdesLib
 
 [Protocols]
   gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED
-- 
2.9.5

Re: [PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:21PM +0800, Ming Huang wrote:
> SerdesLib is useless for SmbiosMiscDxe and D06, so remove it.

Should it not then also delete #include <Library/SerdesLib.h> from
Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
?

Meanwhile,
Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
and
Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
both include this header, but
Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
and 
Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
do not declare the dependency.

Can you investigate and submit an updated patch addressing all of the
unnecessary references?

Best Regards,

Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --
>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
>  2 files changed, 3 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 396bd03c9d24..cbbd99e4a659 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -64,8 +64,6 @@ [LibraryClasses.common]
>  
>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
>  
> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
> -
>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> index 61cead7779b9..8e5c56fa41fd 100644
> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> @@ -77,7 +77,6 @@ [LibraryClasses]
>  
>    IpmiCmdLib
>  
> -  SerdesLib
>  
>  [Protocols]
>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

[Ming Huang]

On 2/11/2019 11:05 PM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:21PM +0800, Ming Huang wrote:
>> SerdesLib is useless for SmbiosMiscDxe and D06, so remove it.
> 
> Should it not then also delete #include <Library/SerdesLib.h> from
> Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
> Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
> ?
> 
> Meanwhile,
> Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
> and
> Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
> both include this header, but
> Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
> and 
> Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
> do not declare the dependency.

OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in
SerdesLib.h, other .c files can not remove the header file.

> 
> Can you investigate and submit an updated patch addressing all of the
> unnecessary references?

This may takes a lot of time, as Hi1620(D06) is our important project,
maybe we should focus on D06.

Thanks

> 
> Best Regards,
> 
> Leif
> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --
>>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
>>  2 files changed, 3 deletions(-)
>>
>> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
>> index 396bd03c9d24..cbbd99e4a659 100644
>> --- a/Platform/Hisilicon/D06/D06.dsc
>> +++ b/Platform/Hisilicon/D06/D06.dsc
>> @@ -64,8 +64,6 @@ [LibraryClasses.common]
>>  
>>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
>>  
>> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
>> -
>>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
>>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
>> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>> index 61cead7779b9..8e5c56fa41fd 100644
>> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>> @@ -77,7 +77,6 @@ [LibraryClasses]
>>  
>>    IpmiCmdLib
>>  
>> -  SerdesLib
>>  
>>  [Protocols]
>>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED
>> -- 
>> 2.9.5
>>

Re: [PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

[Leif Lindholm]

On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:
> > Should it not then also delete #include <Library/SerdesLib.h> from
> > Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
> > Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
> > Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
> > ?
> > 
> > Meanwhile,
> > Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
> > and
> > Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
> > both include this header, but
> > Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
> > and 
> > Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
> > do not declare the dependency.
> 
> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in
> SerdesLib.h, other .c files can not remove the header file.

If they are using definitions from the library header, but not the
library itself, there is something suspicious about the code
structuring.

But in the meantime, if they are referencing library header files,
they need to list those libraryclasses in their .inf.

> > Can you investigate and submit an updated patch addressing all of the
> > unnecessary references?
> 
> This may takes a lot of time, as Hi1620(D06) is our important project,
> maybe we should focus on D06.

Feel free to submit deletions for all and any platforms you are
unwilling to maintain.

Best Regards,

Leif


> Thanks
> 
> > 
> > Best Regards,
> > 
> > Leif
> > 
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> >> ---
> >>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --
> >>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
> >>  2 files changed, 3 deletions(-)
> >>
> >> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> >> index 396bd03c9d24..cbbd99e4a659 100644
> >> --- a/Platform/Hisilicon/D06/D06.dsc
> >> +++ b/Platform/Hisilicon/D06/D06.dsc
> >> @@ -64,8 +64,6 @@ [LibraryClasses.common]
> >>  
> >>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
> >>  
> >> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
> >> -
> >>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
> >>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
> >>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
> >> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> >> index 61cead7779b9..8e5c56fa41fd 100644
> >> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> >> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> >> @@ -77,7 +77,6 @@ [LibraryClasses]
> >>  
> >>    IpmiCmdLib
> >>  
> >> -  SerdesLib
> >>  
> >>  [Protocols]
> >>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED
> >> -- 
> >> 2.9.5
> >>

Re: [PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

[Ming Huang]

On 2/13/2019 5:42 PM, Leif Lindholm wrote:
> On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:
>>> Should it not then also delete #include <Library/SerdesLib.h> from
>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
>>> Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
>>> ?
>>>
>>> Meanwhile,
>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
>>> and
>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
>>> both include this header, but
>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
>>> and 
>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
>>> do not declare the dependency.
>>
>> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in
>> SerdesLib.h, other .c files can not remove the header file.
> 
> If they are using definitions from the library header, but not the
> library itself, there is something suspicious about the code
> structuring.

Yes, there are maybe some unreasonable design in code structuring
for history reason.

> 
> But in the meantime, if they are referencing library header files,
> they need to list those libraryclasses in their .inf.

Do you mean add SerdesLib back to SmbiosMiscDxe.inf? If yes, maybe this
patch should be droped.

Thanks

> 
>>> Can you investigate and submit an updated patch addressing all of the
>>> unnecessary references?
>>
>> This may takes a lot of time, as Hi1620(D06) is our important project,
>> maybe we should focus on D06.
> 
> Feel free to submit deletions for all and any platforms you are
> unwilling to maintain.
> 
> Best Regards,
> 
> Leif
> 
> 
>> Thanks
>>
>>>
>>> Best Regards,
>>>
>>> Leif
>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>>>> ---
>>>>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --
>>>>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
>>>>  2 files changed, 3 deletions(-)
>>>>
>>>> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
>>>> index 396bd03c9d24..cbbd99e4a659 100644
>>>> --- a/Platform/Hisilicon/D06/D06.dsc
>>>> +++ b/Platform/Hisilicon/D06/D06.dsc
>>>> @@ -64,8 +64,6 @@ [LibraryClasses.common]
>>>>  
>>>>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
>>>>  
>>>> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
>>>> -
>>>>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
>>>>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>>>>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
>>>> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>>>> index 61cead7779b9..8e5c56fa41fd 100644
>>>> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>>>> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>>>> @@ -77,7 +77,6 @@ [LibraryClasses]
>>>>  
>>>>    IpmiCmdLib
>>>>  
>>>> -  SerdesLib
>>>>  
>>>>  [Protocols]
>>>>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED
>>>> -- 
>>>> 2.9.5
>>>>

Re: [PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

[Ming Huang]

On 2/13/2019 5:42 PM, Leif Lindholm wrote:
> On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:
>>> Should it not then also delete #include <Library/SerdesLib.h> from
>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
>>> Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
>>> ?
>>>
>>> Meanwhile,
>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
>>> and
>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
>>> both include this header, but
>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
>>> and 
>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
>>> do not declare the dependency.
>>
>> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in
>> SerdesLib.h, other .c files can not remove the header file.
> 
> If they are using definitions from the library header, but not the
> library itself, there is something suspicious about the code
> structuring.
> 
> But in the meantime, if they are referencing library header files,
> they need to list those libraryclasses in their .inf.
> 
>>> Can you investigate and submit an updated patch addressing all of the
>>> unnecessary references?
>>
>> This may takes a lot of time, as Hi1620(D06) is our important project,
>> maybe we should focus on D06.
> 
> Feel free to submit deletions for all and any platforms you are
> unwilling to maintain.

Maybe there are no enough time to investigate this. Is ok to do this for 19.06?
When should I send the v2 out?

Thanks

> 
> Best Regards,
> 
> Leif
> 
> 
>> Thanks
>>
>>>
>>> Best Regards,
>>>
>>> Leif
>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>>>> ---
>>>>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --
>>>>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
>>>>  2 files changed, 3 deletions(-)
>>>>
>>>> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
>>>> index 396bd03c9d24..cbbd99e4a659 100644
>>>> --- a/Platform/Hisilicon/D06/D06.dsc
>>>> +++ b/Platform/Hisilicon/D06/D06.dsc
>>>> @@ -64,8 +64,6 @@ [LibraryClasses.common]
>>>>  
>>>>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
>>>>  
>>>> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
>>>> -
>>>>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
>>>>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>>>>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
>>>> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>>>> index 61cead7779b9..8e5c56fa41fd 100644
>>>> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>>>> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>>>> @@ -77,7 +77,6 @@ [LibraryClasses]
>>>>  
>>>>    IpmiCmdLib
>>>>  
>>>> -  SerdesLib
>>>>  
>>>>  [Protocols]
>>>>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED
>>>> -- 
>>>> 2.9.5
>>>>

[PATCH edk2-platforms v1 02/16] Hisilicon/D0x: Add DriverHealthManagerDxe

[Ming Huang]

DriverHealthManagerDxe Collect driver health form of third party
drivers to repair no healthy card.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Platform/Hisilicon/D03/D03.dsc | 1 +
 Platform/Hisilicon/D05/D05.dsc | 1 +
 Platform/Hisilicon/D06/D06.dsc | 1 +
 Platform/Hisilicon/D03/D03.fdf | 1 +
 Platform/Hisilicon/D05/D05.fdf | 1 +
 Platform/Hisilicon/D06/D06.fdf | 1 +
 6 files changed, 6 insertions(+)

diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
index 3f59be22ec8e..fe443dd929ad 100644
--- a/Platform/Hisilicon/D03/D03.dsc
+++ b/Platform/Hisilicon/D03/D03.dsc
@@ -492,6 +492,7 @@ [Components.common]
 
   MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
   SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf {
     <LibraryClasses>
diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
index 25db1c38d287..0c4f21fbe056 100644
--- a/Platform/Hisilicon/D05/D05.dsc
+++ b/Platform/Hisilicon/D05/D05.dsc
@@ -638,6 +638,7 @@ [Components.common]
   MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
   MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
 
   SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf {
diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
index cbbd99e4a659..6d581337f199 100644
--- a/Platform/Hisilicon/D06/D06.dsc
+++ b/Platform/Hisilicon/D06/D06.dsc
@@ -435,6 +435,7 @@ [Components.common]
   Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.inf
   MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
   SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf {
     <LibraryClasses>
diff --git a/Platform/Hisilicon/D03/D03.fdf b/Platform/Hisilicon/D03/D03.fdf
index f453f9e46321..3f07b2e57778 100644
--- a/Platform/Hisilicon/D03/D03.fdf
+++ b/Platform/Hisilicon/D03/D03.fdf
@@ -295,6 +295,7 @@ [FV.FvMain]
   INF MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
   INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
 
 [FV.FVMAIN_COMPACT]
diff --git a/Platform/Hisilicon/D05/D05.fdf b/Platform/Hisilicon/D05/D05.fdf
index 85dd791564a4..9632aea4b00e 100644
--- a/Platform/Hisilicon/D05/D05.fdf
+++ b/Platform/Hisilicon/D05/D05.fdf
@@ -314,6 +314,7 @@ [FV.FvMain]
   INF MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
   INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
 
 [FV.FVMAIN_COMPACT]
diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
index fda29ab322e9..a937660a09e2 100644
--- a/Platform/Hisilicon/D06/D06.fdf
+++ b/Platform/Hisilicon/D06/D06.fdf
@@ -319,6 +319,7 @@ [FV.FvMain]
   INF MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
   INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
 
 [FV.FVMAIN_COMPACT]
-- 
2.9.5

Re: [PATCH edk2-platforms v1 02/16] Hisilicon/D0x: Add DriverHealthManagerDxe

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:22PM +0800, Ming Huang wrote:
> DriverHealthManagerDxe Collect driver health form of third party
> drivers to repair no healthy card.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Hisilicon/D03/D03.dsc | 1 +
>  Platform/Hisilicon/D05/D05.dsc | 1 +
>  Platform/Hisilicon/D06/D06.dsc | 1 +
>  Platform/Hisilicon/D03/D03.fdf | 1 +
>  Platform/Hisilicon/D05/D05.fdf | 1 +
>  Platform/Hisilicon/D06/D06.fdf | 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> index 3f59be22ec8e..fe443dd929ad 100644
> --- a/Platform/Hisilicon/D03/D03.dsc
> +++ b/Platform/Hisilicon/D03/D03.dsc
> @@ -492,6 +492,7 @@ [Components.common]
>  
>    MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
>    MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>    SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf {
>      <LibraryClasses>
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 25db1c38d287..0c4f21fbe056 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -638,6 +638,7 @@ [Components.common]
>    MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>    MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
>    MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>  
>    SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf {
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index cbbd99e4a659..6d581337f199 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -435,6 +435,7 @@ [Components.common]
>    Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.inf
>    MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
>    MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>    SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf {
>      <LibraryClasses>
> diff --git a/Platform/Hisilicon/D03/D03.fdf b/Platform/Hisilicon/D03/D03.fdf
> index f453f9e46321..3f07b2e57778 100644
> --- a/Platform/Hisilicon/D03/D03.fdf
> +++ b/Platform/Hisilicon/D03/D03.fdf
> @@ -295,6 +295,7 @@ [FV.FvMain]
>    INF MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>    INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
>    INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>  
>  [FV.FVMAIN_COMPACT]
> diff --git a/Platform/Hisilicon/D05/D05.fdf b/Platform/Hisilicon/D05/D05.fdf
> index 85dd791564a4..9632aea4b00e 100644
> --- a/Platform/Hisilicon/D05/D05.fdf
> +++ b/Platform/Hisilicon/D05/D05.fdf
> @@ -314,6 +314,7 @@ [FV.FvMain]
>    INF MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>    INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
>    INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>  
>  [FV.FVMAIN_COMPACT]
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index fda29ab322e9..a937660a09e2 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -319,6 +319,7 @@ [FV.FvMain]
>    INF MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>    INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
>    INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>  
>  [FV.FVMAIN_COMPACT]
> -- 
> 2.9.5
> 

[PATCH edk2-platforms v1 03/16] Hisilicon/D06: Optimize SAS driver for reducing boot time

[Ming Huang]

SAS controller is always existed, so accessing SAS register don't
depend on PciBusDxe (pci enumeration).
Move the SAS module early in D06.fdf for dispatching SAS driver
early. This can avoid wait in BDS normally and reduce boot time.

This patch is relative with SasDriverDxe in edk2-non-osi.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Platform/Hisilicon/D06/D06.fdf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
index a937660a09e2..d495ad7f264c 100644
--- a/Platform/Hisilicon/D06/D06.fdf
+++ b/Platform/Hisilicon/D06/D06.fdf
@@ -165,6 +165,7 @@ [FV.FvMain]
   INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 
   INF Platform/Hisilicon/D06/Drivers/IoInitDxe/IoInitDxe.inf
+  INF Platform/Hisilicon/D06/Drivers/Sas/SasDxeDriver.inf
   #
   # PI DXE Drivers producing Architectural Protocols (EFI Services)
   #
@@ -296,7 +297,6 @@ [FV.FvMain]
   #
   INF Platform/Hisilicon/D06/Drivers/Sm750Dxe/UefiSmi.inf
   INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
-  INF Platform/Hisilicon/D06/Drivers/Sas/SasDxeDriver.inf
   INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
   INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
   INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
-- 
2.9.5

Re: [PATCH edk2-platforms v1 03/16] Hisilicon/D06: Optimize SAS driver for reducing boot time

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:23PM +0800, Ming Huang wrote:
> SAS controller is always existed, so accessing SAS register don't
> depend on PciBusDxe (pci enumeration).
> Move the SAS module early in D06.fdf for dispatching SAS driver
> early. This can avoid wait in BDS normally and reduce boot time.
> 
> This patch is relative with SasDriverDxe in edk2-non-osi.

I think you are saying that this change is only valid after the
update to SasDriverDxe in edk2-non-osi has been applied?
Or does it mean that it only improves performance after that
edk2-non-osi patch has been applied?

Please be more explicit in the commit message.

Other than that. I'm OK with this patch.

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Platform/Hisilicon/D06/D06.fdf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index a937660a09e2..d495ad7f264c 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -165,6 +165,7 @@ [FV.FvMain]
>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>  
>    INF Platform/Hisilicon/D06/Drivers/IoInitDxe/IoInitDxe.inf
> +  INF Platform/Hisilicon/D06/Drivers/Sas/SasDxeDriver.inf
>    #
>    # PI DXE Drivers producing Architectural Protocols (EFI Services)
>    #
> @@ -296,7 +297,6 @@ [FV.FvMain]
>    #
>    INF Platform/Hisilicon/D06/Drivers/Sm750Dxe/UefiSmi.inf
>    INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
> -  INF Platform/Hisilicon/D06/Drivers/Sas/SasDxeDriver.inf
>    INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
>    INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
>    INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 03/16] Hisilicon/D06: Optimize SAS driver for reducing boot time

[Ming Huang]

On 2/12/2019 11:12 PM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:23PM +0800, Ming Huang wrote:
>> SAS controller is always existed, so accessing SAS register don't
>> depend on PciBusDxe (pci enumeration).
>> Move the SAS module early in D06.fdf for dispatching SAS driver
>> early. This can avoid wait in BDS normally and reduce boot time.
>>
>> This patch is relative with SasDriverDxe in edk2-non-osi.
> 
> I think you are saying that this change is only valid after the
> update to SasDriverDxe in edk2-non-osi has been applied?
> Or does it mean that it only improves performance after that
> edk2-non-osi patch has been applied?

This change is only valid after the update to SasDriverDxe in
edk2-non-osi has been applied.

Thanks

> 
> Please be more explicit in the commit message.
> 
> Other than that. I'm OK with this patch.
> 
> /
>     Leif
> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Platform/Hisilicon/D06/D06.fdf | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
>> index a937660a09e2..d495ad7f264c 100644
>> --- a/Platform/Hisilicon/D06/D06.fdf
>> +++ b/Platform/Hisilicon/D06/D06.fdf
>> @@ -165,6 +165,7 @@ [FV.FvMain]
>>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>>  
>>    INF Platform/Hisilicon/D06/Drivers/IoInitDxe/IoInitDxe.inf
>> +  INF Platform/Hisilicon/D06/Drivers/Sas/SasDxeDriver.inf
>>    #
>>    # PI DXE Drivers producing Architectural Protocols (EFI Services)
>>    #
>> @@ -296,7 +297,6 @@ [FV.FvMain]
>>    #
>>    INF Platform/Hisilicon/D06/Drivers/Sm750Dxe/UefiSmi.inf
>>    INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
>> -  INF Platform/Hisilicon/D06/Drivers/Sas/SasDxeDriver.inf
>>    INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
>>    INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
>>    INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
>> -- 
>> 2.9.5
>>

[PATCH edk2-platforms v1 04/16] Hisilicon/D06: Fix access variable fail issue

[Ming Huang]

From: Jason Zhang <zhangjinsong2@huawei.com>

BmcWdtEnable is a field of OemConfigData structure, need have
runtime service attribution if use it during exit boot service

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr | 2 +-
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
index 470e9ace3dcf..08236704fbfe 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
+++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
@@ -23,7 +23,7 @@ formset
   help      = STRING_TOKEN(STR_OEM_CONFIG),
   classguid = gEfiIfrFrontPageGuid,  // for MdeModule Bds.
   efivarstore OEM_CONFIG_DATA,
-    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS,
     name  = OemConfig,
     guid  = gOemConfigGuid;
 
diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
index 012d45bc0214..6668103af027 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
+++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
@@ -316,7 +316,7 @@ OemConfigUiLibConstructor (
       Status = gRT->SetVariable (
                       OEM_CONFIG_NAME,
                       &gOemConfigGuid,
-                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
                       sizeof (OEM_CONFIG_DATA),
                       &Configuration
                       );
-- 
2.9.5

Re: [PATCH edk2-platforms v1 04/16] Hisilicon/D06: Fix access variable fail issue

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:24PM +0800, Ming Huang wrote:
> From: Jason Zhang <zhangjinsong2@huawei.com>
> 
> BmcWdtEnable is a field of OemConfigData structure, need have
> runtime service attribution if use it during exit boot service

This sounds like a very shady thing to do.
Which module is seeing issues, and what issues are it seeing, during
ExitBootServices?

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr | 2 +-
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
> index 470e9ace3dcf..08236704fbfe 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
> @@ -23,7 +23,7 @@ formset
>    help      = STRING_TOKEN(STR_OEM_CONFIG),
>    classguid = gEfiIfrFrontPageGuid,  // for MdeModule Bds.
>    efivarstore OEM_CONFIG_DATA,
> -    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS,
>      name  = OemConfig,
>      guid  = gOemConfigGuid;
>  
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> index 012d45bc0214..6668103af027 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> @@ -316,7 +316,7 @@ OemConfigUiLibConstructor (
>        Status = gRT->SetVariable (
>                        OEM_CONFIG_NAME,
>                        &gOemConfigGuid,
> -                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
> +                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
>                        sizeof (OEM_CONFIG_DATA),
>                        &Configuration
>                        );
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 04/16] Hisilicon/D06: Fix access variable fail issue

[Ming Huang]

On 2/12/2019 11:17 PM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:24PM +0800, Ming Huang wrote:
>> From: Jason Zhang <zhangjinsong2@huawei.com>
>>
>> BmcWdtEnable is a field of OemConfigData structure, need have
>> runtime service attribution if use it during exit boot service
> 
> This sounds like a very shady thing to do.
> Which module is seeing issues, and what issues are it seeing, during
> ExitBootServices?

Yes,WatchDog module read the OemConfigData.BmcWdtEnable during ExitBootServices
and will get fail log before boot kernel:

Get Variable failed. Status Not Found
[    0.000000] Booting Linux on physical CPU 0x0000010000 [0x480fd010]

Thanks.

> 
> /
>     Leif
> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr | 2 +-
>>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c      | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
>> index 470e9ace3dcf..08236704fbfe 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
>> @@ -23,7 +23,7 @@ formset
>>    help      = STRING_TOKEN(STR_OEM_CONFIG),
>>    classguid = gEfiIfrFrontPageGuid,  // for MdeModule Bds.
>>    efivarstore OEM_CONFIG_DATA,
>> -    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS,
>>      name  = OemConfig,
>>      guid  = gOemConfigGuid;
>>  
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
>> index 012d45bc0214..6668103af027 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
>> @@ -316,7 +316,7 @@ OemConfigUiLibConstructor (
>>        Status = gRT->SetVariable (
>>                        OEM_CONFIG_NAME,
>>                        &gOemConfigGuid,
>> -                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
>> +                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
>>                        sizeof (OEM_CONFIG_DATA),
>>                        &Configuration
>>                        );
>> -- 
>> 2.9.5
>>

[PATCH edk2-platforms v1 05/16] Hisilicon/D06: Add more PCIe port INT-x support

[Ming Huang]

From: Jason Zhang <zhangjinsong2@huawei.com>

Since NVMe riser width is 6*X4, need add the related
port's INT-x support to match OS driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 65 +++++++++++++++-----
 1 file changed, 50 insertions(+), 15 deletions(-)

diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
index 27fde2e09bfe..4d9d9d95be68 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
+++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
@@ -41,11 +41,21 @@ Scope(_SB)
       // adding RPx INTx configure deponds on hardware board topology,
       // if UEFI enables RPx, RPy, RPz... related INTx configure
       // should be added
+      Package () {0x2FFFF,0,0,640},         // INT_A
+      Package () {0x2FFFF,1,0,641},         // INT_B
+      Package () {0x2FFFF,2,0,642},         // INT_C
+      Package () {0x2FFFF,3,0,643},         // INT_D
+
       Package () {0x4FFFF,0,0,640},         // INT_A
       Package () {0x4FFFF,1,0,641},         // INT_B
       Package () {0x4FFFF,2,0,642},         // INT_C
       Package () {0x4FFFF,3,0,643},         // INT_D
 
+      Package () {0x6FFFF,0,0,640},         // INT_A
+      Package () {0x6FFFF,1,0,641},         // INT_B
+      Package () {0x6FFFF,2,0,642},         // INT_C
+      Package () {0x6FFFF,3,0,643},         // INT_D
+
       Package () {0x8FFFF,0,0,640},         // INT_A
       Package () {0x8FFFF,1,0,641},         // INT_B
       Package () {0x8FFFF,2,0,642},         // INT_C
@@ -56,6 +66,11 @@ Scope(_SB)
       Package () {0xCFFFF,2,0,642},         // INT_C
       Package () {0xCFFFF,3,0,643},         // INT_D
 
+      Package () {0xEFFFF,0,0,640},         // INT_A
+      Package () {0xEFFFF,1,0,641},         // INT_B
+      Package () {0xEFFFF,2,0,642},         // INT_C
+      Package () {0xEFFFF,3,0,643},         // INT_D
+
       Package () {0x10FFFF,0,0,640},         // INT_A
       Package () {0x10FFFF,1,0,641},         // INT_B
       Package () {0x10FFFF,2,0,642},         // INT_C
@@ -759,26 +774,46 @@ Device (PCI6)
     // adding RPx INTx configure deponds on hardware board topology,
     // if UEFI enables RPx, RPy, RPz... related INTx configure
     // should be added
-    Package () {0x04FFFF,0,0,640},         // INT_A
-    Package () {0x04FFFF,1,0,641},         // INT_B
-    Package () {0x04FFFF,2,0,642},         // INT_C
-    Package () {0x04FFFF,3,0,643},         // INT_D
-
-    Package () {0x08FFFF,0,0,640},         // INT_A
-    Package () {0x08FFFF,1,0,641},         // INT_B
-    Package () {0x08FFFF,2,0,642},         // INT_C
-    Package () {0x08FFFF,3,0,643},         // INT_D
-
-    Package () {0x0CFFFF,0,0,640},         // INT_A
-    Package () {0x0CFFFF,1,0,641},         // INT_B
-    Package () {0x0CFFFF,2,0,642},         // INT_C
-    Package () {0x0CFFFF,3,0,643},         // INT_D
+    Package () {0x2FFFF,0,0,640},         // INT_A
+    Package () {0x2FFFF,1,0,641},         // INT_B
+    Package () {0x2FFFF,2,0,642},         // INT_C
+    Package () {0x2FFFF,3,0,643},         // INT_D
+
+    Package () {0x4FFFF,0,0,640},         // INT_A
+    Package () {0x4FFFF,1,0,641},         // INT_B
+    Package () {0x4FFFF,2,0,642},         // INT_C
+    Package () {0x4FFFF,3,0,643},         // INT_D
+
+    Package () {0x6FFFF,0,0,640},         // INT_A
+    Package () {0x6FFFF,1,0,641},         // INT_B
+    Package () {0x6FFFF,2,0,642},         // INT_C
+    Package () {0x6FFFF,3,0,643},         // INT_D
+
+    Package () {0x8FFFF,0,0,640},         // INT_A
+    Package () {0x8FFFF,1,0,641},         // INT_B
+    Package () {0x8FFFF,2,0,642},         // INT_C
+    Package () {0x8FFFF,3,0,643},         // INT_D
+
+    Package () {0xCFFFF,0,0,640},         // INT_A
+    Package () {0xCFFFF,1,0,641},         // INT_B
+    Package () {0xCFFFF,2,0,642},         // INT_C
+    Package () {0xCFFFF,3,0,643},         // INT_D
+
+    Package () {0xEFFFF,0,0,640},         // INT_A
+    Package () {0xEFFFF,1,0,641},         // INT_B
+    Package () {0xEFFFF,2,0,642},         // INT_C
+    Package () {0xEFFFF,3,0,643},         // INT_D
 
     Package () {0x10FFFF,0,0,640},         // INT_A
     Package () {0x10FFFF,1,0,641},         // INT_B
     Package () {0x10FFFF,2,0,642},         // INT_C
     Package () {0x10FFFF,3,0,643},         // INT_D
-  })
+
+    Package () {0x12FFFF,0,0,640},         // INT_A
+    Package () {0x12FFFF,1,0,641},         // INT_B
+    Package () {0x12FFFF,2,0,642},         // INT_C
+    Package () {0x12FFFF,3,0,643},         // INT_D
+    })
 
   Method (_CRS, 0, Serialized) {           // Root complex resources, _CRS: current resource setting
     Name (RBUF, ResourceTemplate () {      // Name: 19.6.87, ResourceTemplate: 19.6.111,
-- 
2.9.5

Re: [PATCH edk2-platforms v1 05/16] Hisilicon/D06: Add more PCIe port INT-x support

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:25PM +0800, Ming Huang wrote:
> From: Jason Zhang <zhangjinsong2@huawei.com>
> 
> Since NVMe riser width is 6*X4, need add the related
> port's INT-x support to match OS driver.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 65 +++++++++++++++-----
>  1 file changed, 50 insertions(+), 15 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> index 27fde2e09bfe..4d9d9d95be68 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> @@ -41,11 +41,21 @@ Scope(_SB)
>        // adding RPx INTx configure deponds on hardware board topology,
>        // if UEFI enables RPx, RPy, RPz... related INTx configure
>        // should be added
> +      Package () {0x2FFFF,0,0,640},         // INT_A
> +      Package () {0x2FFFF,1,0,641},         // INT_B
> +      Package () {0x2FFFF,2,0,642},         // INT_C
> +      Package () {0x2FFFF,3,0,643},         // INT_D
> +
>        Package () {0x4FFFF,0,0,640},         // INT_A
>        Package () {0x4FFFF,1,0,641},         // INT_B
>        Package () {0x4FFFF,2,0,642},         // INT_C
>        Package () {0x4FFFF,3,0,643},         // INT_D
>  
> +      Package () {0x6FFFF,0,0,640},         // INT_A
> +      Package () {0x6FFFF,1,0,641},         // INT_B
> +      Package () {0x6FFFF,2,0,642},         // INT_C
> +      Package () {0x6FFFF,3,0,643},         // INT_D
> +
>        Package () {0x8FFFF,0,0,640},         // INT_A
>        Package () {0x8FFFF,1,0,641},         // INT_B
>        Package () {0x8FFFF,2,0,642},         // INT_C
> @@ -56,6 +66,11 @@ Scope(_SB)
>        Package () {0xCFFFF,2,0,642},         // INT_C
>        Package () {0xCFFFF,3,0,643},         // INT_D
>  
> +      Package () {0xEFFFF,0,0,640},         // INT_A
> +      Package () {0xEFFFF,1,0,641},         // INT_B
> +      Package () {0xEFFFF,2,0,642},         // INT_C
> +      Package () {0xEFFFF,3,0,643},         // INT_D
> +
>        Package () {0x10FFFF,0,0,640},         // INT_A
>        Package () {0x10FFFF,1,0,641},         // INT_B
>        Package () {0x10FFFF,2,0,642},         // INT_C
> @@ -759,26 +774,46 @@ Device (PCI6)
>      // adding RPx INTx configure deponds on hardware board topology,
>      // if UEFI enables RPx, RPy, RPz... related INTx configure
>      // should be added
> -    Package () {0x04FFFF,0,0,640},         // INT_A
> -    Package () {0x04FFFF,1,0,641},         // INT_B
> -    Package () {0x04FFFF,2,0,642},         // INT_C
> -    Package () {0x04FFFF,3,0,643},         // INT_D
> -
> -    Package () {0x08FFFF,0,0,640},         // INT_A
> -    Package () {0x08FFFF,1,0,641},         // INT_B
> -    Package () {0x08FFFF,2,0,642},         // INT_C
> -    Package () {0x08FFFF,3,0,643},         // INT_D
> -
> -    Package () {0x0CFFFF,0,0,640},         // INT_A
> -    Package () {0x0CFFFF,1,0,641},         // INT_B
> -    Package () {0x0CFFFF,2,0,642},         // INT_C
> -    Package () {0x0CFFFF,3,0,643},         // INT_D

Please don't include the non-functional change of dropping the leading
0 (0x0 -> 0x) here together with the functional change of adding new
entries. Please submit as a separate patch.

/
    Leif

> +    Package () {0x2FFFF,0,0,640},         // INT_A
> +    Package () {0x2FFFF,1,0,641},         // INT_B
> +    Package () {0x2FFFF,2,0,642},         // INT_C
> +    Package () {0x2FFFF,3,0,643},         // INT_D
> +
> +    Package () {0x4FFFF,0,0,640},         // INT_A
> +    Package () {0x4FFFF,1,0,641},         // INT_B
> +    Package () {0x4FFFF,2,0,642},         // INT_C
> +    Package () {0x4FFFF,3,0,643},         // INT_D
> +
> +    Package () {0x6FFFF,0,0,640},         // INT_A
> +    Package () {0x6FFFF,1,0,641},         // INT_B
> +    Package () {0x6FFFF,2,0,642},         // INT_C
> +    Package () {0x6FFFF,3,0,643},         // INT_D
> +
> +    Package () {0x8FFFF,0,0,640},         // INT_A
> +    Package () {0x8FFFF,1,0,641},         // INT_B
> +    Package () {0x8FFFF,2,0,642},         // INT_C
> +    Package () {0x8FFFF,3,0,643},         // INT_D
> +
> +    Package () {0xCFFFF,0,0,640},         // INT_A
> +    Package () {0xCFFFF,1,0,641},         // INT_B
> +    Package () {0xCFFFF,2,0,642},         // INT_C
> +    Package () {0xCFFFF,3,0,643},         // INT_D
> +
> +    Package () {0xEFFFF,0,0,640},         // INT_A
> +    Package () {0xEFFFF,1,0,641},         // INT_B
> +    Package () {0xEFFFF,2,0,642},         // INT_C
> +    Package () {0xEFFFF,3,0,643},         // INT_D
>  
>      Package () {0x10FFFF,0,0,640},         // INT_A
>      Package () {0x10FFFF,1,0,641},         // INT_B
>      Package () {0x10FFFF,2,0,642},         // INT_C
>      Package () {0x10FFFF,3,0,643},         // INT_D
> -  })
> +
> +    Package () {0x12FFFF,0,0,640},         // INT_A
> +    Package () {0x12FFFF,1,0,641},         // INT_B
> +    Package () {0x12FFFF,2,0,642},         // INT_C
> +    Package () {0x12FFFF,3,0,643},         // INT_D
> +    })
>  
>    Method (_CRS, 0, Serialized) {           // Root complex resources, _CRS: current resource setting
>      Name (RBUF, ResourceTemplate () {      // Name: 19.6.87, ResourceTemplate: 19.6.111,
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 05/16] Hisilicon/D06: Add more PCIe port INT-x support

[Ming Huang]

On 2/12/2019 1:05 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:25PM +0800, Ming Huang wrote:
>> From: Jason Zhang <zhangjinsong2@huawei.com>
>>
>> Since NVMe riser width is 6*X4, need add the related
>> port's INT-x support to match OS driver.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 65 +++++++++++++++-----
>>  1 file changed, 50 insertions(+), 15 deletions(-)
>>
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> index 27fde2e09bfe..4d9d9d95be68 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> @@ -41,11 +41,21 @@ Scope(_SB)
>>        // adding RPx INTx configure deponds on hardware board topology,
>>        // if UEFI enables RPx, RPy, RPz... related INTx configure
>>        // should be added
>> +      Package () {0x2FFFF,0,0,640},         // INT_A
>> +      Package () {0x2FFFF,1,0,641},         // INT_B
>> +      Package () {0x2FFFF,2,0,642},         // INT_C
>> +      Package () {0x2FFFF,3,0,643},         // INT_D
>> +
>>        Package () {0x4FFFF,0,0,640},         // INT_A
>>        Package () {0x4FFFF,1,0,641},         // INT_B
>>        Package () {0x4FFFF,2,0,642},         // INT_C
>>        Package () {0x4FFFF,3,0,643},         // INT_D
>>  
>> +      Package () {0x6FFFF,0,0,640},         // INT_A
>> +      Package () {0x6FFFF,1,0,641},         // INT_B
>> +      Package () {0x6FFFF,2,0,642},         // INT_C
>> +      Package () {0x6FFFF,3,0,643},         // INT_D
>> +
>>        Package () {0x8FFFF,0,0,640},         // INT_A
>>        Package () {0x8FFFF,1,0,641},         // INT_B
>>        Package () {0x8FFFF,2,0,642},         // INT_C
>> @@ -56,6 +66,11 @@ Scope(_SB)
>>        Package () {0xCFFFF,2,0,642},         // INT_C
>>        Package () {0xCFFFF,3,0,643},         // INT_D
>>  
>> +      Package () {0xEFFFF,0,0,640},         // INT_A
>> +      Package () {0xEFFFF,1,0,641},         // INT_B
>> +      Package () {0xEFFFF,2,0,642},         // INT_C
>> +      Package () {0xEFFFF,3,0,643},         // INT_D
>> +
>>        Package () {0x10FFFF,0,0,640},         // INT_A
>>        Package () {0x10FFFF,1,0,641},         // INT_B
>>        Package () {0x10FFFF,2,0,642},         // INT_C
>> @@ -759,26 +774,46 @@ Device (PCI6)
>>      // adding RPx INTx configure deponds on hardware board topology,
>>      // if UEFI enables RPx, RPy, RPz... related INTx configure
>>      // should be added
>> -    Package () {0x04FFFF,0,0,640},         // INT_A
>> -    Package () {0x04FFFF,1,0,641},         // INT_B
>> -    Package () {0x04FFFF,2,0,642},         // INT_C
>> -    Package () {0x04FFFF,3,0,643},         // INT_D
>> -
>> -    Package () {0x08FFFF,0,0,640},         // INT_A
>> -    Package () {0x08FFFF,1,0,641},         // INT_B
>> -    Package () {0x08FFFF,2,0,642},         // INT_C
>> -    Package () {0x08FFFF,3,0,643},         // INT_D
>> -
>> -    Package () {0x0CFFFF,0,0,640},         // INT_A
>> -    Package () {0x0CFFFF,1,0,641},         // INT_B
>> -    Package () {0x0CFFFF,2,0,642},         // INT_C
>> -    Package () {0x0CFFFF,3,0,643},         // INT_D
> 
> Please don't include the non-functional change of dropping the leading
> 0 (0x0 -> 0x) here together with the functional change of adding new
> entries. Please submit as a separate patch.

Ok, do it in v2.

> 
> /
>     Leif
> 
>> +    Package () {0x2FFFF,0,0,640},         // INT_A
>> +    Package () {0x2FFFF,1,0,641},         // INT_B
>> +    Package () {0x2FFFF,2,0,642},         // INT_C
>> +    Package () {0x2FFFF,3,0,643},         // INT_D
>> +
>> +    Package () {0x4FFFF,0,0,640},         // INT_A
>> +    Package () {0x4FFFF,1,0,641},         // INT_B
>> +    Package () {0x4FFFF,2,0,642},         // INT_C
>> +    Package () {0x4FFFF,3,0,643},         // INT_D
>> +
>> +    Package () {0x6FFFF,0,0,640},         // INT_A
>> +    Package () {0x6FFFF,1,0,641},         // INT_B
>> +    Package () {0x6FFFF,2,0,642},         // INT_C
>> +    Package () {0x6FFFF,3,0,643},         // INT_D
>> +
>> +    Package () {0x8FFFF,0,0,640},         // INT_A
>> +    Package () {0x8FFFF,1,0,641},         // INT_B
>> +    Package () {0x8FFFF,2,0,642},         // INT_C
>> +    Package () {0x8FFFF,3,0,643},         // INT_D
>> +
>> +    Package () {0xCFFFF,0,0,640},         // INT_A
>> +    Package () {0xCFFFF,1,0,641},         // INT_B
>> +    Package () {0xCFFFF,2,0,642},         // INT_C
>> +    Package () {0xCFFFF,3,0,643},         // INT_D
>> +
>> +    Package () {0xEFFFF,0,0,640},         // INT_A
>> +    Package () {0xEFFFF,1,0,641},         // INT_B
>> +    Package () {0xEFFFF,2,0,642},         // INT_C
>> +    Package () {0xEFFFF,3,0,643},         // INT_D
>>  
>>      Package () {0x10FFFF,0,0,640},         // INT_A
>>      Package () {0x10FFFF,1,0,641},         // INT_B
>>      Package () {0x10FFFF,2,0,642},         // INT_C
>>      Package () {0x10FFFF,3,0,643},         // INT_D
>> -  })
>> +
>> +    Package () {0x12FFFF,0,0,640},         // INT_A
>> +    Package () {0x12FFFF,1,0,641},         // INT_B
>> +    Package () {0x12FFFF,2,0,642},         // INT_C
>> +    Package () {0x12FFFF,3,0,643},         // INT_D
>> +    })
>>  
>>    Method (_CRS, 0, Serialized) {           // Root complex resources, _CRS: current resource setting
>>      Name (RBUF, ResourceTemplate () {      // Name: 19.6.87, ResourceTemplate: 19.6.111,
>> -- 
>> 2.9.5
>>

[PATCH edk2-platforms v1 06/16] Hisilicon/D06: Add OemGetCpuFreq to encapsulate difference

[Ming Huang]

From: xingjiang tang <tangxingjiang@huawei.com>

Implementation OemGetCpuFreq() to get cpu frequency from cpld to
encapsulate project difference, for some projects don't support
get cpu frequency by this way.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Platform/Hisilicon/D06/Include/Library/CpldD06.h             |  4 ++++
 Silicon/Hisilicon/Include/Library/OemMiscLib.h               |  2 ++
 Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c | 16 ++++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/Platform/Hisilicon/D06/Include/Library/CpldD06.h b/Platform/Hisilicon/D06/Include/Library/CpldD06.h
index ec9b49f4e70d..4d07a8ab3741 100644
--- a/Platform/Hisilicon/D06/Include/Library/CpldD06.h
+++ b/Platform/Hisilicon/D06/Include/Library/CpldD06.h
@@ -36,4 +36,8 @@
 #define CPLD_X8_X8_X8_BOARD_ID            0x92
 #define CPLD_X16_X8_BOARD_ID              0x93
 
+#define CPLD_CLOCK_FLAG      0xFD
+#define CPLD_BOM_VER_FLAG    0x0B
+#define BRD_VER_4TH          0x4
+
 #endif /* __CPLDD06_H__ */
diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
index 86ea6a1b3deb..dfac87d635d9 100644
--- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
+++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
@@ -53,4 +53,6 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID);
 
 extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
 EFI_HII_HANDLE EFIAPI OemGetPackages ();
+UINTN OemGetCpuFreq (UINT8 Socket);
+
 #endif
diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
index 2a9db46d1ff9..8f2ac308c7b9 100644
--- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
+++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
@@ -207,3 +207,19 @@ OemIsNeedDisableExpanderBuffer (
 {
   return TRUE;
 }
+
+UINTN OemGetCpuFreq (UINT8 Socket)
+{
+  UINT8 BrdVerData;
+
+  BrdVerData = MmioRead8(CPLD_BASE_ADDRESS + CPLD_BOM_VER_FLAG);
+
+  if (BrdVerData >= BRD_VER_4TH){  //2.5G
+    return 2500000000;
+  }
+  else
+  {
+     return 2000000000;
+  }
+}
+
-- 
2.9.5

Re: [PATCH edk2-platforms v1 06/16] Hisilicon/D06: Add OemGetCpuFreq to encapsulate difference

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:26PM +0800, Ming Huang wrote:
> From: xingjiang tang <tangxingjiang@huawei.com>
> 
> Implementation OemGetCpuFreq() to get cpu frequency from cpld to
> encapsulate project difference, for some projects don't support
> get cpu frequency by this way.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Platform/Hisilicon/D06/Include/Library/CpldD06.h             |  4 ++++
>  Silicon/Hisilicon/Include/Library/OemMiscLib.h               |  2 ++
>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c | 16 ++++++++++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/Include/Library/CpldD06.h b/Platform/Hisilicon/D06/Include/Library/CpldD06.h
> index ec9b49f4e70d..4d07a8ab3741 100644
> --- a/Platform/Hisilicon/D06/Include/Library/CpldD06.h
> +++ b/Platform/Hisilicon/D06/Include/Library/CpldD06.h
> @@ -36,4 +36,8 @@
>  #define CPLD_X8_X8_X8_BOARD_ID            0x92
>  #define CPLD_X16_X8_BOARD_ID              0x93
>  
> +#define CPLD_CLOCK_FLAG      0xFD
> +#define CPLD_BOM_VER_FLAG    0x0B
> +#define BRD_VER_4TH          0x4

What is BRD_VER_4TH? Please write out full words.
Also, this macro needs a CPLD_ prefix.

> +
>  #endif /* __CPLDD06_H__ */
> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> index 86ea6a1b3deb..dfac87d635d9 100644
> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> @@ -53,4 +53,6 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID);
>  
>  extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
>  EFI_HII_HANDLE EFIAPI OemGetPackages ();
> +UINTN OemGetCpuFreq (UINT8 Socket);
> +
>  #endif
> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> index 2a9db46d1ff9..8f2ac308c7b9 100644
> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> @@ -207,3 +207,19 @@ OemIsNeedDisableExpanderBuffer (
>  {
>    return TRUE;
>  }
> +
> +UINTN OemGetCpuFreq (UINT8 Socket)
> +{
> +  UINT8 BrdVerData;

Write out full words.

> +
> +  BrdVerData = MmioRead8(CPLD_BASE_ADDRESS + CPLD_BOM_VER_FLAG);

Space before (.

> +
> +  if (BrdVerData >= BRD_VER_4TH){  //2.5G

What is the comment saying? The number below?
The number below is also saying the number below.
A useful comment would be
"// Board revision 4 and higher run at 2.5GHz
 // Earlier revisions run at 2GHz"

At that point you don't even need the #define.
And not really the temporary variable either.

> +    return 2500000000;
> +  }
> +  else
> +  {

} else {

/
    Leif

> +     return 2000000000;
> +  }
> +}
> +
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 06/16] Hisilicon/D06: Add OemGetCpuFreq to encapsulate difference

[Ming Huang]

On 2/12/2019 1:15 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:26PM +0800, Ming Huang wrote:
>> From: xingjiang tang <tangxingjiang@huawei.com>
>>
>> Implementation OemGetCpuFreq() to get cpu frequency from cpld to
>> encapsulate project difference, for some projects don't support
>> get cpu frequency by this way.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Platform/Hisilicon/D06/Include/Library/CpldD06.h             |  4 ++++
>>  Silicon/Hisilicon/Include/Library/OemMiscLib.h               |  2 ++
>>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c | 16 ++++++++++++++++
>>  3 files changed, 22 insertions(+)
>>
>> diff --git a/Platform/Hisilicon/D06/Include/Library/CpldD06.h b/Platform/Hisilicon/D06/Include/Library/CpldD06.h
>> index ec9b49f4e70d..4d07a8ab3741 100644
>> --- a/Platform/Hisilicon/D06/Include/Library/CpldD06.h
>> +++ b/Platform/Hisilicon/D06/Include/Library/CpldD06.h
>> @@ -36,4 +36,8 @@
>>  #define CPLD_X8_X8_X8_BOARD_ID            0x92
>>  #define CPLD_X16_X8_BOARD_ID              0x93
>>  
>> +#define CPLD_CLOCK_FLAG      0xFD
>> +#define CPLD_BOM_VER_FLAG    0x0B
>> +#define BRD_VER_4TH          0x4
> 
> What is BRD_VER_4TH? Please write out full words.
> Also, this macro needs a CPLD_ prefix.

BRD_VER_4TH: BOARD_REVISION_4TH
Modify in v2.

> 
>> +
>>  #endif /* __CPLDD06_H__ */
>> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> index 86ea6a1b3deb..dfac87d635d9 100644
>> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> @@ -53,4 +53,6 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID);
>>  
>>  extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
>>  EFI_HII_HANDLE EFIAPI OemGetPackages ();
>> +UINTN OemGetCpuFreq (UINT8 Socket);
>> +
>>  #endif
>> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> index 2a9db46d1ff9..8f2ac308c7b9 100644
>> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> @@ -207,3 +207,19 @@ OemIsNeedDisableExpanderBuffer (
>>  {
>>    return TRUE;
>>  }
>> +
>> +UINTN OemGetCpuFreq (UINT8 Socket)
>> +{
>> +  UINT8 BrdVerData;
> 
> Write out full words.
> 
>> +
>> +  BrdVerData = MmioRead8(CPLD_BASE_ADDRESS + CPLD_BOM_VER_FLAG);
> 
> Space before (.
> 
>> +
>> +  if (BrdVerData >= BRD_VER_4TH){  //2.5G
> 
> What is the comment saying? The number below?
> The number below is also saying the number below.
> A useful comment would be
> "// Board revision 4 and higher run at 2.5GHz
>  // Earlier revisions run at 2GHz"
> 
> At that point you don't even need the #define.
> And not really the temporary variable either.
> 
>> +    return 2500000000;
>> +  }
>> +  else
>> +  {
> 
> } else {

Modify in v2.

Thanks

> 
> /
>     Leif
> 
>> +     return 2000000000;
>> +  }
>> +}
>> +
>> -- 
>> 2.9.5
>>

[PATCH edk2-platforms v1 07/16] Hisilicon/D0x: Rename StartupAp() function

[Ming Huang]

As suggestion of community, 'AP' is a bit unfortunate to use in EDK2
context. PI specifies 'BSP' for Boot-strap Processor, as the one
executing all of the EDK2 code. It then uses 'AP' to refer to
Additional Processors, which can be assigned tasks using the
EFI_MP_SERVICES_PROTOCOL. In a TianoCore context, this should be 'BSP'.
So, Rename StartupAp() to StartUpBSP.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h           | 2 +-
 Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c      | 2 +-
 Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c | 2 +-
 Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c      | 2 +-
 Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c     | 3 ++-
 Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c      | 2 +-
 6 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h b/Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h
index a232e52ed719..712b77c44fc8 100644
--- a/Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h
+++ b/Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h
@@ -76,7 +76,7 @@ VOID MN_CONFIG (VOID);
 VOID SmmuConfigForOS (VOID);
 VOID SmmuConfigForBios (VOID);
 
-VOID StartupAp (VOID);
+VOID StartUpBSP (VOID);
 
 VOID LlcCleanInvalidate (VOID);
 
diff --git a/Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c b/Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c
index 97cf6b8d8757..dacd9e871faf 100644
--- a/Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c
+++ b/Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c
@@ -83,7 +83,7 @@ void QResetAp(VOID)
     //SCCL A
     if (!PcdGet64 (PcdTrustedFirmwareEnable))
     {
-        StartupAp();
+        StartUpBSP ();
     }
 }
 
diff --git a/Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c b/Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c
index b57fdfa68e45..c8a9da73bbca 100644
--- a/Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c
+++ b/Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c
@@ -133,7 +133,7 @@ VOID CoreSelectBoot(VOID)
 {
     if (!PcdGet64 (PcdTrustedFirmwareEnable))
     {
-        StartupAp ();
+        StartUpBSP ();
     }
 
     return;
diff --git a/Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c b/Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c
index 76a055cbe980..b374347e5c4d 100644
--- a/Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c
+++ b/Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c
@@ -35,7 +35,7 @@ QResetAp (
   (VOID)WriteBackInvalidateDataCacheRange((VOID *) FixedPcdGet64(PcdMailBoxAddress), 8);
 
   if (!PcdGet64 (PcdTrustedFirmwareEnable)) {
-    StartupAp();
+    StartUpBSP ();
   }
 }
 
diff --git a/Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c b/Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c
index 4c4c944dbead..a1458da7f0a3 100644
--- a/Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c
+++ b/Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c
@@ -96,7 +96,7 @@ UINTN OemGetDimmSlot(UINTN Socket, UINTN Channel)
 VOID CoreSelectBoot(VOID)
 {
   if (!PcdGet64 (PcdTrustedFirmwareEnable)) {
-      StartupAp ();
+      StartUpBSP ();
   }
 
   return;
@@ -128,3 +128,4 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID)
 {
   return TRUE;
 }
+
diff --git a/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c b/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c
index 0790f7941ae7..a8261d370626 100644
--- a/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c
+++ b/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c
@@ -78,7 +78,7 @@ QResetAp (
 
   //SCCL A
   if (!PcdGet64 (PcdTrustedFirmwareEnable)) {
-    StartupAp ();
+    StartUpBSP ();
   }
 }
 
-- 
2.9.5

Re: [PATCH edk2-platforms v1 07/16] Hisilicon/D0x: Rename StartupAp() function

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:27PM +0800, Ming Huang wrote:
> As suggestion of community, 'AP' is a bit unfortunate to use in EDK2
> context. PI specifies 'BSP' for Boot-strap Processor, as the one
> executing all of the EDK2 code. It then uses 'AP' to refer to
> Additional Processors, which can be assigned tasks using the
> EFI_MP_SERVICES_PROTOCOL. In a TianoCore context, this should be 'BSP'.
> So, Rename StartupAp() to StartUpBSP.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Thanks!

> ---
>  Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h           | 2 +-
>  Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c      | 2 +-
>  Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c | 2 +-
>  Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c      | 2 +-
>  Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c     | 3 ++-
>  Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c      | 2 +-
>  6 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h b/Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h
> index a232e52ed719..712b77c44fc8 100644
> --- a/Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h
> +++ b/Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h
> @@ -76,7 +76,7 @@ VOID MN_CONFIG (VOID);
>  VOID SmmuConfigForOS (VOID);
>  VOID SmmuConfigForBios (VOID);
>  
> -VOID StartupAp (VOID);
> +VOID StartUpBSP (VOID);
>  
>  VOID LlcCleanInvalidate (VOID);
>  
> diff --git a/Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c b/Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c
> index 97cf6b8d8757..dacd9e871faf 100644
> --- a/Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c
> +++ b/Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c
> @@ -83,7 +83,7 @@ void QResetAp(VOID)
>      //SCCL A
>      if (!PcdGet64 (PcdTrustedFirmwareEnable))
>      {
> -        StartupAp();
> +        StartUpBSP ();
>      }
>  }
>  
> diff --git a/Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c b/Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c
> index b57fdfa68e45..c8a9da73bbca 100644
> --- a/Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c
> +++ b/Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c
> @@ -133,7 +133,7 @@ VOID CoreSelectBoot(VOID)
>  {
>      if (!PcdGet64 (PcdTrustedFirmwareEnable))
>      {
> -        StartupAp ();
> +        StartUpBSP ();
>      }
>  
>      return;
> diff --git a/Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c b/Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c
> index 76a055cbe980..b374347e5c4d 100644
> --- a/Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c
> +++ b/Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c
> @@ -35,7 +35,7 @@ QResetAp (
>    (VOID)WriteBackInvalidateDataCacheRange((VOID *) FixedPcdGet64(PcdMailBoxAddress), 8);
>  
>    if (!PcdGet64 (PcdTrustedFirmwareEnable)) {
> -    StartupAp();
> +    StartUpBSP ();
>    }
>  }
>  
> diff --git a/Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c b/Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c
> index 4c4c944dbead..a1458da7f0a3 100644
> --- a/Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c
> +++ b/Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c
> @@ -96,7 +96,7 @@ UINTN OemGetDimmSlot(UINTN Socket, UINTN Channel)
>  VOID CoreSelectBoot(VOID)
>  {
>    if (!PcdGet64 (PcdTrustedFirmwareEnable)) {
> -      StartupAp ();
> +      StartUpBSP ();
>    }
>  
>    return;
> @@ -128,3 +128,4 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID)
>  {
>    return TRUE;
>  }
> +
> diff --git a/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c b/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c
> index 0790f7941ae7..a8261d370626 100644
> --- a/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c
> +++ b/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c
> @@ -78,7 +78,7 @@ QResetAp (
>  
>    //SCCL A
>    if (!PcdGet64 (PcdTrustedFirmwareEnable)) {
> -    StartupAp ();
> +    StartUpBSP ();
>    }
>  }
>  
> -- 
> 2.9.5
> 

[PATCH edk2-platforms v1 08/16] Hisilicon/D06: Change HCCS speed from 30G to 26G

[Ming Huang]

Follow chip team suggestion to change HCCS(Huawei Cache-Coherent
System) speed from 30G to 26G, this modification can avoid some
unstable stress issue.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Silicon/Hisilicon/Include/Library/OemMiscLib.h               | 10 ++++++++++
 Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c |  8 ++++++++
 2 files changed, 18 insertions(+)

diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
index dfac87d635d9..3c0cd0319122 100644
--- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
+++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
@@ -22,6 +22,11 @@
 #include <PlatformArch.h>
 #include <Library/I2CLib.h>
 
+#define HCCS_PLL_VALUE_3000  0x52240781
+#define HCCS_PLL_VALUE_2600  0x52240681
+#define HCCS_PLL_VALUE_2800  0x52240701
+
+
 #define PCIEDEVICE_REPORT_MAX      8
 #define MAX_PROCESSOR_SOCKETS      MAX_SOCKET
 #define MAX_MEMORY_CHANNELS        MAX_CHANNEL
@@ -55,4 +60,9 @@ extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
 EFI_HII_HANDLE EFIAPI OemGetPackages ();
 UINTN OemGetCpuFreq (UINT8 Socket);
 
+UINTN
+OemGetHccsFreq (
+  VOID
+  );
+
 #endif
diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
index 8f2ac308c7b9..83e53cfeb5dd 100644
--- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
+++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
@@ -223,3 +223,11 @@ UINTN OemGetCpuFreq (UINT8 Socket)
   }
 }
 
+UINTN
+OemGetHccsFreq (
+  VOID
+  )
+{
+  return HCCS_PLL_VALUE_2600;
+}
+
-- 
2.9.5

Re: [PATCH edk2-platforms v1 08/16] Hisilicon/D06: Change HCCS speed from 30G to 26G

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:28PM +0800, Ming Huang wrote:
> Follow chip team suggestion to change HCCS(Huawei Cache-Coherent
> System) speed from 30G to 26G, this modification can avoid some
> unstable stress issue.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Silicon/Hisilicon/Include/Library/OemMiscLib.h               | 10 ++++++++++
>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c |  8 ++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> index dfac87d635d9..3c0cd0319122 100644
> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> @@ -22,6 +22,11 @@
>  #include <PlatformArch.h>
>  #include <Library/I2CLib.h>
>  
> +#define HCCS_PLL_VALUE_3000  0x52240781
> +#define HCCS_PLL_VALUE_2600  0x52240681
> +#define HCCS_PLL_VALUE_2800  0x52240701

Could these be described by a proper macro instead of just values?
A cursory glance suggests that an increase of 0x80 in the lower half
means 200MHz.

If not, please sort them by frequency, ascending.

> +
> +
>  #define PCIEDEVICE_REPORT_MAX      8
>  #define MAX_PROCESSOR_SOCKETS      MAX_SOCKET
>  #define MAX_MEMORY_CHANNELS        MAX_CHANNEL
> @@ -55,4 +60,9 @@ extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
>  EFI_HII_HANDLE EFIAPI OemGetPackages ();
>  UINTN OemGetCpuFreq (UINT8 Socket);
>  
> +UINTN
> +OemGetHccsFreq (
> +  VOID
> +  );
> +
>  #endif
> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> index 8f2ac308c7b9..83e53cfeb5dd 100644
> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> @@ -223,3 +223,11 @@ UINTN OemGetCpuFreq (UINT8 Socket)
>    }
>  }
>  
> +UINTN
> +OemGetHccsFreq (

The commit message describes this patch as changing the frequency.
The actual code simply returns a value.
The name of the function returning this value suggests the value is a
frequency. 

> +  VOID
> +  )
> +{
> +  return HCCS_PLL_VALUE_2600;

But the constant returned is named suggesting a PLL configuration
value. And the frequency suggested by the name is many orders of
magnitude below that described by the commit message.

/
    Leif

> +}
> +
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 08/16] Hisilicon/D06: Change HCCS speed from 30G to 26G

[Ming Huang]

On 2/12/2019 2:36 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:28PM +0800, Ming Huang wrote:
>> Follow chip team suggestion to change HCCS(Huawei Cache-Coherent
>> System) speed from 30G to 26G, this modification can avoid some
>> unstable stress issue.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Silicon/Hisilicon/Include/Library/OemMiscLib.h               | 10 ++++++++++
>>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c |  8 ++++++++
>>  2 files changed, 18 insertions(+)
>>
>> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> index dfac87d635d9..3c0cd0319122 100644
>> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
>> @@ -22,6 +22,11 @@
>>  #include <PlatformArch.h>
>>  #include <Library/I2CLib.h>
>>  
>> +#define HCCS_PLL_VALUE_3000  0x52240781
>> +#define HCCS_PLL_VALUE_2600  0x52240681
>> +#define HCCS_PLL_VALUE_2800  0x52240701
> 
> Could these be described by a proper macro instead of just values?
> A cursory glance suggests that an increase of 0x80 in the lower half
> means 200MHz.
> 
> If not, please sort them by frequency, ascending.

As the macros have use in other files, I prefer sort them by frequency.

> 
>> +
>> +
>>  #define PCIEDEVICE_REPORT_MAX      8
>>  #define MAX_PROCESSOR_SOCKETS      MAX_SOCKET
>>  #define MAX_MEMORY_CHANNELS        MAX_CHANNEL
>> @@ -55,4 +60,9 @@ extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
>>  EFI_HII_HANDLE EFIAPI OemGetPackages ();
>>  UINTN OemGetCpuFreq (UINT8 Socket);
>>  
>> +UINTN
>> +OemGetHccsFreq (
>> +  VOID
>> +  );
>> +
>>  #endif
>> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> index 8f2ac308c7b9..83e53cfeb5dd 100644
>> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
>> @@ -223,3 +223,11 @@ UINTN OemGetCpuFreq (UINT8 Socket)
>>    }
>>  }
>>  
>> +UINTN
>> +OemGetHccsFreq (
> 
> The commit message describes this patch as changing the frequency.
> The actual code simply returns a value.
> The name of the function returning this value suggests the value is a
> frequency>
>> +  VOID
>> +  )
>> +{
>> +  return HCCS_PLL_VALUE_2600;
> 
> But the constant returned is named suggesting a PLL configuration
> value. And the frequency suggested by the name is many orders of
> magnitude below that described by the commit message.

Yes, the macros and function name are not very matched.
I plan to modify the commit title and message:
Hisilicon/D06: Use HCCS speed with 2.6G

Follow chip team suggestion, HCCS(Huawei Cache-Coherent System)
may be unstable while speed is 3.0G, so use 2.6G to avoid some
unstable stress issue.

Thanks.

> 
> /
>     Leif
> 
>> +}
>> +
>> -- 
>> 2.9.5
>>

Re: [PATCH edk2-platforms v1 08/16] Hisilicon/D06: Change HCCS speed from 30G to 26G

[Leif Lindholm]

On Tue, Feb 12, 2019 at 10:45:05PM +0800, Ming Huang wrote:
> 
> 
> On 2/12/2019 2:36 AM, Leif Lindholm wrote:
> > On Fri, Feb 01, 2019 at 09:34:28PM +0800, Ming Huang wrote:
> >> Follow chip team suggestion to change HCCS(Huawei Cache-Coherent
> >> System) speed from 30G to 26G, this modification can avoid some
> >> unstable stress issue.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> >> ---
> >>  Silicon/Hisilicon/Include/Library/OemMiscLib.h               | 10 ++++++++++
> >>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c |  8 ++++++++
> >>  2 files changed, 18 insertions(+)
> >>
> >> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> >> index dfac87d635d9..3c0cd0319122 100644
> >> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> >> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
> >> @@ -22,6 +22,11 @@
> >>  #include <PlatformArch.h>
> >>  #include <Library/I2CLib.h>
> >>  
> >> +#define HCCS_PLL_VALUE_3000  0x52240781
> >> +#define HCCS_PLL_VALUE_2600  0x52240681
> >> +#define HCCS_PLL_VALUE_2800  0x52240701
> > 
> > Could these be described by a proper macro instead of just values?
> > A cursory glance suggests that an increase of 0x80 in the lower half
> > means 200MHz.
> > 
> > If not, please sort them by frequency, ascending.
> 
> As the macros have use in other files, I prefer sort them by frequency.
> 
> > 
> >> +
> >> +
> >>  #define PCIEDEVICE_REPORT_MAX      8
> >>  #define MAX_PROCESSOR_SOCKETS      MAX_SOCKET
> >>  #define MAX_MEMORY_CHANNELS        MAX_CHANNEL
> >> @@ -55,4 +60,9 @@ extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
> >>  EFI_HII_HANDLE EFIAPI OemGetPackages ();
> >>  UINTN OemGetCpuFreq (UINT8 Socket);
> >>  
> >> +UINTN
> >> +OemGetHccsFreq (
> >> +  VOID
> >> +  );
> >> +
> >>  #endif
> >> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> >> index 8f2ac308c7b9..83e53cfeb5dd 100644
> >> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> >> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
> >> @@ -223,3 +223,11 @@ UINTN OemGetCpuFreq (UINT8 Socket)
> >>    }
> >>  }
> >>  
> >> +UINTN
> >> +OemGetHccsFreq (
> > 
> > The commit message describes this patch as changing the frequency.
> > The actual code simply returns a value.
> > The name of the function returning this value suggests the value is a
> > frequency>
> >> +  VOID
> >> +  )
> >> +{
> >> +  return HCCS_PLL_VALUE_2600;
> > 
> > But the constant returned is named suggesting a PLL configuration
> > value. And the frequency suggested by the name is many orders of
> > magnitude below that described by the commit message.
> 
> Yes, the macros and function name are not very matched.
> I plan to modify the commit title and message:
> Hisilicon/D06: Use HCCS speed with 2.6G
> 
> Follow chip team suggestion, HCCS(Huawei Cache-Coherent System)
> may be unstable while speed is 3.0G, so use 2.6G to avoid some
> unstable stress issue.

This all sounds good, thanks.

/
    Leif

[PATCH edk2-platforms v1 09/16] Hisilicon/D06: Add PCI_OSC_SUPPORT

[Ming Huang]

Add PCI_OSC_SUPPORT for remaining host bridges to remove fail
output in kernel:
[  103.478893] acpi PNP0A08:01: _OSC failed (AE_NOT_FOUND);

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 64 ++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
index 4d9d9d95be68..86d8728b82f2 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
+++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
@@ -17,6 +17,50 @@
 **/
 
 //#include "ArmPlatform.h"
+
+/*
+  See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5
+*/
+#define PCI_OSC_SUPPORT() \
+  Name(SUPP, Zero) /* PCI _OSC Support Field value */ \
+  Name(CTRL, Zero) /* PCI _OSC Control Field value */ \
+  Method(_OSC,4) { \
+    If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) { \
+      /* Create DWord-adressable fields from the Capabilities Buffer */ \
+      CreateDWordField(Arg3,0,CDW1) \
+      CreateDWordField(Arg3,4,CDW2) \
+      CreateDWordField(Arg3,8,CDW3) \
+      /* Save Capabilities DWord2 & 3 */ \
+      Store(CDW2,SUPP) \
+      Store(CDW3,CTRL) \
+      /* Only allow native hot plug control if OS supports: */ \
+      /* ASPM */ \
+      /* Clock PM */ \
+      /* MSI/MSI-X */ \
+      If(LNotEqual(And(SUPP, 0x16), 0x16)) { \
+        And(CTRL,0x1E,CTRL) \
+      }\
+      \
+      /* Do not allow native PME, AER */ \
+      /* Never allow SHPC (no SHPC controller in this system)*/ \
+      And(CTRL,0x10,CTRL) \
+      If(LNotEqual(Arg1,One)) { /* Unknown revision */ \
+        Or(CDW1,0x08,CDW1) \
+      } \
+      \
+      If(LNotEqual(CDW3,CTRL)) { /* Capabilities bits were masked */ \
+        Or(CDW1,0x10,CDW1) \
+      } \
+      \
+      /* Update DWORD3 in the buffer */ \
+      Store(CTRL,CDW3) \
+      Return(Arg3) \
+    } Else { \
+      Or(CDW1,4,CDW1) /* Unrecognized UUID */ \
+      Return(Arg3) \
+    } \
+  } // End _OSC
+
 Scope(_SB)
 {
   Device (PCI0)
@@ -270,6 +314,8 @@ Device (PCI1)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0xf)
@@ -333,6 +379,8 @@ Device (PCI2)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0xf)
@@ -382,6 +430,8 @@ Device (PCI3)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0xf)
@@ -431,6 +481,8 @@ Device (PCI4)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0x0F)
@@ -505,6 +557,8 @@ Device (PCI5)
     Return (RBUF)
   }                                    // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0xf)
@@ -1002,6 +1056,8 @@ Device (PCI7)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0xf)
@@ -1066,6 +1122,8 @@ Device (PCI8)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0xf)
@@ -1115,6 +1173,8 @@ Device (PCI9)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0xf)
@@ -1164,6 +1224,8 @@ Device (PCIA)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0x0F)
@@ -1238,6 +1300,8 @@ Device (PCIB)
     Return (RBUF)
   }                                         // Method(_CRS), this method return RBUF!
 
+  PCI_OSC_SUPPORT ()
+
   Method (_STA, 0x0, NotSerialized)
   {
     Return (0xf)
-- 
2.9.5

Re: [PATCH edk2-platforms v1 09/16] Hisilicon/D06: Add PCI_OSC_SUPPORT

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:29PM +0800, Ming Huang wrote:
> Add PCI_OSC_SUPPORT for remaining host bridges to remove fail
> output in kernel:
> [  103.478893] acpi PNP0A08:01: _OSC failed (AE_NOT_FOUND);
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 64 ++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> index 4d9d9d95be68..86d8728b82f2 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> @@ -17,6 +17,50 @@
>  **/
>  
>  //#include "ArmPlatform.h"
> +
> +/*
> +  See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5
> +*/
> +#define PCI_OSC_SUPPORT() \

PCI0 and PCI6 already have _OSC entries.
This macro ends up being used for 1-5 and 7-B.
So calling it PCI_OSC_SUPPORT seems somewhat misleading.

Then again, there is a lot of similarities between this macro and the
existing entries. Could the same macro be used for 0 and 6? Or could
the macro be split up into multiple parts and reused?

/
    Leif

> +  Name(SUPP, Zero) /* PCI _OSC Support Field value */ \
> +  Name(CTRL, Zero) /* PCI _OSC Control Field value */ \
> +  Method(_OSC,4) { \
> +    If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) { \
> +      /* Create DWord-adressable fields from the Capabilities Buffer */ \
> +      CreateDWordField(Arg3,0,CDW1) \
> +      CreateDWordField(Arg3,4,CDW2) \
> +      CreateDWordField(Arg3,8,CDW3) \
> +      /* Save Capabilities DWord2 & 3 */ \
> +      Store(CDW2,SUPP) \
> +      Store(CDW3,CTRL) \
> +      /* Only allow native hot plug control if OS supports: */ \
> +      /* ASPM */ \
> +      /* Clock PM */ \
> +      /* MSI/MSI-X */ \
> +      If(LNotEqual(And(SUPP, 0x16), 0x16)) { \
> +        And(CTRL,0x1E,CTRL) \
> +      }\
> +      \
> +      /* Do not allow native PME, AER */ \
> +      /* Never allow SHPC (no SHPC controller in this system)*/ \
> +      And(CTRL,0x10,CTRL) \
> +      If(LNotEqual(Arg1,One)) { /* Unknown revision */ \
> +        Or(CDW1,0x08,CDW1) \
> +      } \
> +      \
> +      If(LNotEqual(CDW3,CTRL)) { /* Capabilities bits were masked */ \
> +        Or(CDW1,0x10,CDW1) \
> +      } \
> +      \
> +      /* Update DWORD3 in the buffer */ \
> +      Store(CTRL,CDW3) \
> +      Return(Arg3) \
> +    } Else { \
> +      Or(CDW1,4,CDW1) /* Unrecognized UUID */ \
> +      Return(Arg3) \
> +    } \
> +  } // End _OSC
> +
>  Scope(_SB)
>  {
>    Device (PCI0)
> @@ -270,6 +314,8 @@ Device (PCI1)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0xf)
> @@ -333,6 +379,8 @@ Device (PCI2)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0xf)
> @@ -382,6 +430,8 @@ Device (PCI3)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0xf)
> @@ -431,6 +481,8 @@ Device (PCI4)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0x0F)
> @@ -505,6 +557,8 @@ Device (PCI5)
>      Return (RBUF)
>    }                                    // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0xf)
> @@ -1002,6 +1056,8 @@ Device (PCI7)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0xf)
> @@ -1066,6 +1122,8 @@ Device (PCI8)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0xf)
> @@ -1115,6 +1173,8 @@ Device (PCI9)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0xf)
> @@ -1164,6 +1224,8 @@ Device (PCIA)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0x0F)
> @@ -1238,6 +1300,8 @@ Device (PCIB)
>      Return (RBUF)
>    }                                         // Method(_CRS), this method return RBUF!
>  
> +  PCI_OSC_SUPPORT ()
> +
>    Method (_STA, 0x0, NotSerialized)
>    {
>      Return (0xf)
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 09/16] Hisilicon/D06: Add PCI_OSC_SUPPORT

[Ming Huang]

On 2/12/2019 2:51 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:29PM +0800, Ming Huang wrote:
>> Add PCI_OSC_SUPPORT for remaining host bridges to remove fail
>> output in kernel:
>> [  103.478893] acpi PNP0A08:01: _OSC failed (AE_NOT_FOUND);
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 64 ++++++++++++++++++++
>>  1 file changed, 64 insertions(+)
>>
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> index 4d9d9d95be68..86d8728b82f2 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> @@ -17,6 +17,50 @@
>>  **/
>>  
>>  //#include "ArmPlatform.h"
>> +
>> +/*
>> +  See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5
>> +*/
>> +#define PCI_OSC_SUPPORT() \
> 
> PCI0 and PCI6 already have _OSC entries.
> This macro ends up being used for 1-5 and 7-B.
> So calling it PCI_OSC_SUPPORT seems somewhat misleading.
> 
> Then again, there is a lot of similarities between this macro and the
> existing entries. Could the same macro be used for 0 and 6? Or could
> the macro be split up into multiple parts and reused?

When I make this patch, I try to rewrite PCI0/6 with the same macro, but
the macro don't support parameter. For spliting up multiple parts, if modify
something in future, the parts need to split up to smaller parts. So, if
need to rewrite PCI0/6 with macro, is it applicable to add another macro
PCI_OSC_SUPPORT_HOTPLUG?

Thanks

> 
> /
>     Leif
> 
>> +  Name(SUPP, Zero) /* PCI _OSC Support Field value */ \
>> +  Name(CTRL, Zero) /* PCI _OSC Control Field value */ \
>> +  Method(_OSC,4) { \
>> +    If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) { \
>> +      /* Create DWord-adressable fields from the Capabilities Buffer */ \
>> +      CreateDWordField(Arg3,0,CDW1) \
>> +      CreateDWordField(Arg3,4,CDW2) \
>> +      CreateDWordField(Arg3,8,CDW3) \
>> +      /* Save Capabilities DWord2 & 3 */ \
>> +      Store(CDW2,SUPP) \
>> +      Store(CDW3,CTRL) \
>> +      /* Only allow native hot plug control if OS supports: */ \
>> +      /* ASPM */ \
>> +      /* Clock PM */ \
>> +      /* MSI/MSI-X */ \
>> +      If(LNotEqual(And(SUPP, 0x16), 0x16)) { \
>> +        And(CTRL,0x1E,CTRL) \
>> +      }\
>> +      \
>> +      /* Do not allow native PME, AER */ \
>> +      /* Never allow SHPC (no SHPC controller in this system)*/ \
>> +      And(CTRL,0x10,CTRL) \
>> +      If(LNotEqual(Arg1,One)) { /* Unknown revision */ \
>> +        Or(CDW1,0x08,CDW1) \
>> +      } \
>> +      \
>> +      If(LNotEqual(CDW3,CTRL)) { /* Capabilities bits were masked */ \
>> +        Or(CDW1,0x10,CDW1) \
>> +      } \
>> +      \
>> +      /* Update DWORD3 in the buffer */ \
>> +      Store(CTRL,CDW3) \
>> +      Return(Arg3) \
>> +    } Else { \
>> +      Or(CDW1,4,CDW1) /* Unrecognized UUID */ \
>> +      Return(Arg3) \
>> +    } \
>> +  } // End _OSC
>> +
>>  Scope(_SB)
>>  {
>>    Device (PCI0)
>> @@ -270,6 +314,8 @@ Device (PCI1)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0xf)
>> @@ -333,6 +379,8 @@ Device (PCI2)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0xf)
>> @@ -382,6 +430,8 @@ Device (PCI3)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0xf)
>> @@ -431,6 +481,8 @@ Device (PCI4)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0x0F)
>> @@ -505,6 +557,8 @@ Device (PCI5)
>>      Return (RBUF)
>>    }                                    // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0xf)
>> @@ -1002,6 +1056,8 @@ Device (PCI7)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0xf)
>> @@ -1066,6 +1122,8 @@ Device (PCI8)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0xf)
>> @@ -1115,6 +1173,8 @@ Device (PCI9)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0xf)
>> @@ -1164,6 +1224,8 @@ Device (PCIA)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0x0F)
>> @@ -1238,6 +1300,8 @@ Device (PCIB)
>>      Return (RBUF)
>>    }                                         // Method(_CRS), this method return RBUF!
>>  
>> +  PCI_OSC_SUPPORT ()
>> +
>>    Method (_STA, 0x0, NotSerialized)
>>    {
>>      Return (0xf)
>> -- 
>> 2.9.5
>>

Re: [PATCH edk2-platforms v1 09/16] Hisilicon/D06: Add PCI_OSC_SUPPORT

[Leif Lindholm]

On Wed, Feb 13, 2019 at 10:59:17AM +0800, Ming Huang wrote:
> On 2/12/2019 2:51 AM, Leif Lindholm wrote:
> > On Fri, Feb 01, 2019 at 09:34:29PM +0800, Ming Huang wrote:
> >> Add PCI_OSC_SUPPORT for remaining host bridges to remove fail
> >> output in kernel:
> >> [  103.478893] acpi PNP0A08:01: _OSC failed (AE_NOT_FOUND);
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> >> ---
> >>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 64 ++++++++++++++++++++
> >>  1 file changed, 64 insertions(+)
> >>
> >> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> >> index 4d9d9d95be68..86d8728b82f2 100644
> >> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> >> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> >> @@ -17,6 +17,50 @@
> >>  **/
> >>  
> >>  //#include "ArmPlatform.h"
> >> +
> >> +/*
> >> +  See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5
> >> +*/
> >> +#define PCI_OSC_SUPPORT() \
> > 
> > PCI0 and PCI6 already have _OSC entries.
> > This macro ends up being used for 1-5 and 7-B.
> > So calling it PCI_OSC_SUPPORT seems somewhat misleading.
> > 
> > Then again, there is a lot of similarities between this macro and the
> > existing entries. Could the same macro be used for 0 and 6? Or could
> > the macro be split up into multiple parts and reused?
> 
> When I make this patch, I try to rewrite PCI0/6 with the same macro, but
> the macro don't support parameter. For spliting up multiple parts, if modify
> something in future, the parts need to split up to smaller parts. So, if
> need to rewrite PCI0/6 with macro, is it applicable to add another macro
> PCI_OSC_SUPPORT_HOTPLUG?

Yes, that sounds like a good solution to me.

Regards,

Leif

[PATCH edk2-platforms v1 10/16] Hisilicon/D06: Modify for M7 self-Adapte support

[Ming Huang]

As new M7(Cortex-M7) firmware support self-adapte, so do not
need BIOS to implement some function, remove useless funtions
and report CPU0/CPU1 Nic NCL offset to M7.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------
 1 file changed, 45 insertions(+), 227 deletions(-)

diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
index aaf990216982..9bf274e1b991 100644
--- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
+++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
@@ -21,44 +21,21 @@
 #include <Library/OemNicLib.h>
 
 #define CPU2_SFP2_100G_CARD_OFFSET   0x25
-#define CPU1_SFP1_LOCATE_OFFSET      0x16
-#define CPU1_SFP0_LOCATE_OFFSET      0x12
-#define CPU2_SFP1_LOCATE_OFFSET      0x21
-#define CPU2_SFP0_LOCATE_OFFSET      0x19
-#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25
 
-#define SFP_10G_SPEED   10
-#define SFP_25G_SPEED   25
-#define SFP_100G_SPEED  100
-#define SFP_GE_SPEED    1
-
-#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C
-#define SFP_GE_SPEED_VAL                0x0D
-#define SFP_10G_SPEED_VAL               0x67
-#define SFP_25G_SPEED_VAL               0xFF
+#define SOCKET1_NET_PORT_100G         1
+#define SOCKET0_NET_PORT_NUM          4
+#define SOCKET1_NET_PORT_NUM          2
 
 #define CARD_PRESENT_100G               (BIT7)
-#define CARD_PRESENT_10G                (BIT0)
-#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))
-#define SPF_SPEED_OFFSET                12
-
-#define SFP_DEVICE_ADDRESS 0x50
-#define CPU1_9545_I2C_ADDR 0x70
-#define CPU2_9545_I2C_ADDR 0x71
-
-#define FIBER_PRESENT     0
-#define CARD_PRESENT      1
-#define I2C_PORT_SFP      4
-#define CPU2_I2C_PORT_SFP 5
-
-#define SOCKET_0                 0
-#define SOCKET_1                 1
 #define EEPROM_I2C_PORT          4
 #define EEPROM_PAGE_SIZE         0x40
 #define MAC_ADDR_LEN             6
 #define I2C_OFFSET_EEPROM_ETH0   (0xc00)
 #define I2C_SLAVEADDR_EEPROM     (0x52)
 
+#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0
+#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4
+
 #pragma pack(1)
 typedef struct {
   UINT16 Crc16;
@@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {
   0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,
 };
 
-EFI_STATUS
-GetSfpSpeed (
-  UINT16 Socket,
-  UINT16 SfpNum,
-  UINT8* FiberSpeed
-  )
-{
-  EFI_STATUS  Status;
-  I2C_DEVICE  SpdDev;
-  UINT8       SfpSelect;
-  UINT8       SfpSpeed;
-  UINT32      RegAddr;
-  UINT16      I2cAddr;
-  UINT32      SfpPort;
-
-  SfpSpeed = 0x0;
-  if (Socket == SOCKET_1) {
-    I2cAddr = CPU2_9545_I2C_ADDR;
-    SfpPort = CPU2_I2C_PORT_SFP;
-  } else {
-    I2cAddr = CPU1_9545_I2C_ADDR;
-    SfpPort = I2C_PORT_SFP;
-  }
-
-  Status = I2CInit (Socket, SfpPort, Normal);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",
-            __FUNCTION__, __LINE__, Socket, Status));
-    return Status;
-  }
-
-  SpdDev.Socket = Socket;
-  SpdDev.DeviceType = DEVICE_TYPE_SPD;
-  SpdDev.Port = SfpPort;
-  SpdDev.SlaveDeviceAddress = I2cAddr;
-  RegAddr = 0x0;
-  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);
-
-  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));
-    return Status;
-  }
-
-  SpdDev.Socket = Socket;
-  SpdDev.DeviceType = DEVICE_TYPE_SPD;
-  SpdDev.Port = SfpPort;
-  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;
-
-  RegAddr = SPF_SPEED_OFFSET;
-  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));
-    return Status;
-  }
-
-  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",
-         SfpSpeed));
-
-  if (SfpSpeed == SFP_10G_SPEED_VAL) {
-    *FiberSpeed = SFP_10G_SPEED;
-  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {
-    *FiberSpeed = SFP_25G_SPEED;
-  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||
-             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {
-    *FiberSpeed = SFP_GE_SPEED;
-  }
-
-  return EFI_SUCCESS;
-}
-
-//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED
-UINT32
-GetCpu2FiberType (
-  UINT8* Fiber1Type,
-  UINT8* Fiber2Type,
-  UINT8* Fiber100Ge
-  )
-{
-  EFI_STATUS  Status;
-  UINT16      SfpNum1;
-  UINT8       SfpSpeed1;
-  UINT16      SfpNum2;
-  UINT8       SfpSpeed2;
-
-  SfpNum1 = 0x1;
-  SfpSpeed1 = SFP_10G_SPEED;
-  SfpNum2 = 0x2;
-  SfpSpeed2 = SFP_10G_SPEED;
-  *Fiber100Ge = 0x0;
-  *Fiber1Type = SFP_10G_SPEED;
-  *Fiber2Type = SFP_10G_SPEED;
-
-  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
-    // 100 Ge card
-    *Fiber1Type = SFP_10G_SPEED;
-    *Fiber2Type = SFP_10G_SPEED;
-    *Fiber100Ge = SFP_100G_SPEED;
-    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));
-  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {
-    *Fiber100Ge = 0x0;
-    *Fiber1Type = SFP_10G_SPEED;
-    *Fiber2Type = SFP_10G_SPEED;
-    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
-      // Fiber detected in CPU2 slot0, read speed via i2c
-      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);
-      if (EFI_ERROR (Status)) {
-        DEBUG((DEBUG_ERROR,
-               "Get Socket1 Sfp%d Speed Error: %r.\n",
-               SfpNum1,
-               Status));
-        return Status;
-      }
-      if (SfpSpeed1 == SFP_25G_SPEED) {
-        // P1 don't support 25G, so set speed to 10G
-        *Fiber1Type = SFP_10G_SPEED;
-      } else {
-        *Fiber1Type = SfpSpeed1;
-      }
-    } else {
-      // No fiber, set speed to 10G
-      *Fiber1Type = SFP_10G_SPEED;
-    }
-
-    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
-      // Fiber detected in CPU2 slot1, read speed via i2c
-      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);
-      if (EFI_ERROR (Status)) {
-        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
-        return Status;
-      }
-      if (SfpSpeed2 == SFP_25G_SPEED) {
-        *Fiber2Type = SFP_10G_SPEED;
-      } else {
-        *Fiber2Type = SfpSpeed2;
-      }
-    } else {
-      // No fiber, set speed to 10G
-      *Fiber2Type = SFP_10G_SPEED;
-    }
-  } else {
-    // 100Ge/10Ge/Ge Fiber is not found.
-    *Fiber1Type = SFP_10G_SPEED;
-    *Fiber2Type = SFP_10G_SPEED;
-    *Fiber100Ge = 0x0;
-  }
-
-  return EFI_SUCCESS;
-}
-
-//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED
-UINT32
-GetCpu1FiberType (
-  UINT8* Fiber1Type,
-  UINT8* Fiber2Type
-  )
-{
-  EFI_STATUS  Status;
-  UINT16      SfpNum1;
-  UINT8       SfpSpeed1;
-  UINT16      SfpNum2;
-  UINT8       SfpSpeed2;
-
-  SfpNum1 = 0x1;
-  SfpSpeed1 = SFP_10G_SPEED;
-  SfpNum2 = 0x2;
-  SfpSpeed2 = SFP_10G_SPEED;
-  *Fiber1Type = SFP_10G_SPEED;
-  *Fiber2Type = SFP_10G_SPEED;
-  // Fiber detected in CPU1 slot0, read speed via i2c
-  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
-    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);
-    if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",
-              SfpNum1, Status));
-      return Status;
-    }
-    *Fiber1Type = SfpSpeed1;
-  } else {
-    *Fiber1Type = SFP_10G_SPEED;
-  }
-
-  // Fiber detected in CPU1 slot1, read speed via i2c
-  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
-    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);
-    if (EFI_ERROR (Status)) {
-      *Fiber2Type = SFP_10G_SPEED;
-      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
-      return Status;
-    }
-    *Fiber2Type = SfpSpeed2;
-  } else {
-    *Fiber2Type = SFP_10G_SPEED;
-  }
-
-  return EFI_SUCCESS;
-}
-
 UINT16 MakeCrcCheckSum (
   UINT8 *Buffer,
   UINT32 Length
@@ -567,3 +346,42 @@ OemIsInitEth (
 {
   return TRUE;
 }
+
+EFI_STATUS ConfigCDR(UINT32 Socket)
+{
+    return EFI_SUCCESS;
+}
+
+UINT32 OemGetNclConfOffset (UINT32 Socket)
+{
+    UINT32           Cpu1NclConfOffet = 0;
+    UINT32           Cpu2NclConfOffet = 0;
+
+    if (0 == Socket) {
+      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);
+      return Cpu1NclConfOffet;
+    } else {
+      //2P only
+      // P1
+      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
+        Cpu2NclConfOffet =  0x20000;
+      } else {
+        Cpu2NclConfOffet =  0x10000;
+      }
+      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);
+      return Cpu2NclConfOffet;
+    }
+}
+
+UINT32 OemGetNetPortNum (UINT32 Socket)
+{
+    if (0 == Socket){
+    return SOCKET0_NET_PORT_NUM;
+    }
+
+    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
+      return SOCKET1_NET_PORT_100G;
+    } else {
+      return SOCKET1_NET_PORT_NUM;
+    }
+}
-- 
2.9.5

Re: [PATCH edk2-platforms v1 10/16] Hisilicon/D06: Modify for M7 self-Adapte support

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:30PM +0800, Ming Huang wrote:
> As new M7(Cortex-M7) firmware support self-adapte, so do not
> need BIOS to implement some function, remove useless funtions
> and report CPU0/CPU1 Nic NCL offset to M7.

I don't really care that some other device in the system is a
Cortex-A7. What is its function? Is it an SCP, an MCP, ?
Please describe its function rather than its implementation.

What are the external dependencies?
Is this addressed by one of the patches for edk2-non-osi?

More style issues below.

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------
>  1 file changed, 45 insertions(+), 227 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
> index aaf990216982..9bf274e1b991 100644
> --- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
> @@ -21,44 +21,21 @@
>  #include <Library/OemNicLib.h>
>  
>  #define CPU2_SFP2_100G_CARD_OFFSET   0x25
> -#define CPU1_SFP1_LOCATE_OFFSET      0x16
> -#define CPU1_SFP0_LOCATE_OFFSET      0x12
> -#define CPU2_SFP1_LOCATE_OFFSET      0x21
> -#define CPU2_SFP0_LOCATE_OFFSET      0x19
> -#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25
>  
> -#define SFP_10G_SPEED   10
> -#define SFP_25G_SPEED   25
> -#define SFP_100G_SPEED  100
> -#define SFP_GE_SPEED    1
> -
> -#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C
> -#define SFP_GE_SPEED_VAL                0x0D
> -#define SFP_10G_SPEED_VAL               0x67
> -#define SFP_25G_SPEED_VAL               0xFF
> +#define SOCKET1_NET_PORT_100G         1
> +#define SOCKET0_NET_PORT_NUM          4
> +#define SOCKET1_NET_PORT_NUM          2
>  
>  #define CARD_PRESENT_100G               (BIT7)
> -#define CARD_PRESENT_10G                (BIT0)
> -#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))
> -#define SPF_SPEED_OFFSET                12
> -
> -#define SFP_DEVICE_ADDRESS 0x50
> -#define CPU1_9545_I2C_ADDR 0x70
> -#define CPU2_9545_I2C_ADDR 0x71
> -
> -#define FIBER_PRESENT     0
> -#define CARD_PRESENT      1
> -#define I2C_PORT_SFP      4
> -#define CPU2_I2C_PORT_SFP 5
> -
> -#define SOCKET_0                 0
> -#define SOCKET_1                 1
>  #define EEPROM_I2C_PORT          4
>  #define EEPROM_PAGE_SIZE         0x40
>  #define MAC_ADDR_LEN             6
>  #define I2C_OFFSET_EEPROM_ETH0   (0xc00)
>  #define I2C_SLAVEADDR_EEPROM     (0x52)
>  
> +#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0
> +#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4

Is this just a hard-coded address in SRAM? Where is it specified?
I don't think "_FLAG" is the correct name for this #define - this is
the actual offset value. So _OFFSET_ADDRESS would be more descriptive.

> +
>  #pragma pack(1)
>  typedef struct {
>    UINT16 Crc16;
> @@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {
>    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,
>  };
>  
> -EFI_STATUS
> -GetSfpSpeed (
> -  UINT16 Socket,
> -  UINT16 SfpNum,
> -  UINT8* FiberSpeed
> -  )
> -{
> -  EFI_STATUS  Status;
> -  I2C_DEVICE  SpdDev;
> -  UINT8       SfpSelect;
> -  UINT8       SfpSpeed;
> -  UINT32      RegAddr;
> -  UINT16      I2cAddr;
> -  UINT32      SfpPort;
> -
> -  SfpSpeed = 0x0;
> -  if (Socket == SOCKET_1) {
> -    I2cAddr = CPU2_9545_I2C_ADDR;
> -    SfpPort = CPU2_I2C_PORT_SFP;
> -  } else {
> -    I2cAddr = CPU1_9545_I2C_ADDR;
> -    SfpPort = I2C_PORT_SFP;
> -  }
> -
> -  Status = I2CInit (Socket, SfpPort, Normal);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",
> -            __FUNCTION__, __LINE__, Socket, Status));
> -    return Status;
> -  }
> -
> -  SpdDev.Socket = Socket;
> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;
> -  SpdDev.Port = SfpPort;
> -  SpdDev.SlaveDeviceAddress = I2cAddr;
> -  RegAddr = 0x0;
> -  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);
> -
> -  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));
> -    return Status;
> -  }
> -
> -  SpdDev.Socket = Socket;
> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;
> -  SpdDev.Port = SfpPort;
> -  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;
> -
> -  RegAddr = SPF_SPEED_OFFSET;
> -  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));
> -    return Status;
> -  }
> -
> -  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",
> -         SfpSpeed));
> -
> -  if (SfpSpeed == SFP_10G_SPEED_VAL) {
> -    *FiberSpeed = SFP_10G_SPEED;
> -  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {
> -    *FiberSpeed = SFP_25G_SPEED;
> -  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||
> -             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {
> -    *FiberSpeed = SFP_GE_SPEED;
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
> -//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED
> -UINT32
> -GetCpu2FiberType (
> -  UINT8* Fiber1Type,
> -  UINT8* Fiber2Type,
> -  UINT8* Fiber100Ge
> -  )
> -{
> -  EFI_STATUS  Status;
> -  UINT16      SfpNum1;
> -  UINT8       SfpSpeed1;
> -  UINT16      SfpNum2;
> -  UINT8       SfpSpeed2;
> -
> -  SfpNum1 = 0x1;
> -  SfpSpeed1 = SFP_10G_SPEED;
> -  SfpNum2 = 0x2;
> -  SfpSpeed2 = SFP_10G_SPEED;
> -  *Fiber100Ge = 0x0;
> -  *Fiber1Type = SFP_10G_SPEED;
> -  *Fiber2Type = SFP_10G_SPEED;
> -
> -  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
> -    // 100 Ge card
> -    *Fiber1Type = SFP_10G_SPEED;
> -    *Fiber2Type = SFP_10G_SPEED;
> -    *Fiber100Ge = SFP_100G_SPEED;
> -    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));
> -  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {
> -    *Fiber100Ge = 0x0;
> -    *Fiber1Type = SFP_10G_SPEED;
> -    *Fiber2Type = SFP_10G_SPEED;
> -    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
> -      // Fiber detected in CPU2 slot0, read speed via i2c
> -      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);
> -      if (EFI_ERROR (Status)) {
> -        DEBUG((DEBUG_ERROR,
> -               "Get Socket1 Sfp%d Speed Error: %r.\n",
> -               SfpNum1,
> -               Status));
> -        return Status;
> -      }
> -      if (SfpSpeed1 == SFP_25G_SPEED) {
> -        // P1 don't support 25G, so set speed to 10G
> -        *Fiber1Type = SFP_10G_SPEED;
> -      } else {
> -        *Fiber1Type = SfpSpeed1;
> -      }
> -    } else {
> -      // No fiber, set speed to 10G
> -      *Fiber1Type = SFP_10G_SPEED;
> -    }
> -
> -    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
> -      // Fiber detected in CPU2 slot1, read speed via i2c
> -      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);
> -      if (EFI_ERROR (Status)) {
> -        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
> -        return Status;
> -      }
> -      if (SfpSpeed2 == SFP_25G_SPEED) {
> -        *Fiber2Type = SFP_10G_SPEED;
> -      } else {
> -        *Fiber2Type = SfpSpeed2;
> -      }
> -    } else {
> -      // No fiber, set speed to 10G
> -      *Fiber2Type = SFP_10G_SPEED;
> -    }
> -  } else {
> -    // 100Ge/10Ge/Ge Fiber is not found.
> -    *Fiber1Type = SFP_10G_SPEED;
> -    *Fiber2Type = SFP_10G_SPEED;
> -    *Fiber100Ge = 0x0;
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
> -//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED
> -UINT32
> -GetCpu1FiberType (
> -  UINT8* Fiber1Type,
> -  UINT8* Fiber2Type
> -  )
> -{
> -  EFI_STATUS  Status;
> -  UINT16      SfpNum1;
> -  UINT8       SfpSpeed1;
> -  UINT16      SfpNum2;
> -  UINT8       SfpSpeed2;
> -
> -  SfpNum1 = 0x1;
> -  SfpSpeed1 = SFP_10G_SPEED;
> -  SfpNum2 = 0x2;
> -  SfpSpeed2 = SFP_10G_SPEED;
> -  *Fiber1Type = SFP_10G_SPEED;
> -  *Fiber2Type = SFP_10G_SPEED;
> -  // Fiber detected in CPU1 slot0, read speed via i2c
> -  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
> -    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);
> -    if (EFI_ERROR (Status)) {
> -      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",
> -              SfpNum1, Status));
> -      return Status;
> -    }
> -    *Fiber1Type = SfpSpeed1;
> -  } else {
> -    *Fiber1Type = SFP_10G_SPEED;
> -  }
> -
> -  // Fiber detected in CPU1 slot1, read speed via i2c
> -  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
> -    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);
> -    if (EFI_ERROR (Status)) {
> -      *Fiber2Type = SFP_10G_SPEED;
> -      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
> -      return Status;
> -    }
> -    *Fiber2Type = SfpSpeed2;
> -  } else {
> -    *Fiber2Type = SFP_10G_SPEED;
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
>  UINT16 MakeCrcCheckSum (
>    UINT8 *Buffer,
>    UINT32 Length
> @@ -567,3 +346,42 @@ OemIsInitEth (
>  {
>    return TRUE;
>  }
> +
> +EFI_STATUS ConfigCDR(UINT32 Socket)
> +{
> +    return EFI_SUCCESS;
> +}
> +
> +UINT32 OemGetNclConfOffset (UINT32 Socket)
> +{
> +    UINT32           Cpu1NclConfOffet = 0;

Indentation is 2 spaces, not 4. (Please address throughout.)

> +    UINT32           Cpu2NclConfOffet = 0;

Also, no initialization on definition.
But I see no value in having two variables with complicated names.
Just a single one called ConfigurationOffset or whatever.

> +
> +    if (0 == Socket) {

No jeopardy-comparisons. Please flip that around.

> +      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);

This line can just write a 0 directly.
But it can also use a comment explaining what writing a 0 here achieves.

> +      return Cpu1NclConfOffet;

And this is effectively just an error return - so can just return 0
directly.

> +    } else {

No need for the else. You've returned is there was an error. The rest
is just the remainder of the function.

> +      //2P only

What is 2P?

> +      // P1

What is P1?

> +      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
> +        Cpu2NclConfOffet =  0x20000;

SIZE_128KB?

> +      } else {
> +        Cpu2NclConfOffet =  0x10000;

SIZE_64KB?

> +      }
> +      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);
> +      return Cpu2NclConfOffet;
> +    }
> +}
> +
> +UINT32 OemGetNetPortNum (UINT32 Socket)
> +{
> +    if (0 == Socket){

No jeopardy-comparisons. Please flip that around.

/
    Leif

> +    return SOCKET0_NET_PORT_NUM;
> +    }
> +
> +    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
> +      return SOCKET1_NET_PORT_100G;
> +    } else {
> +      return SOCKET1_NET_PORT_NUM;
> +    }
> +}
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 10/16] Hisilicon/D06: Modify for M7 self-Adapte support

[Ming Huang]

On 2/12/2019 3:28 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:30PM +0800, Ming Huang wrote:
>> As new M7(Cortex-M7) firmware support self-adapte, so do not
>> need BIOS to implement some function, remove useless funtions
>> and report CPU0/CPU1 Nic NCL offset to M7.
> 
> I don't really care that some other device in the system is a
> Cortex-A7. What is its function? Is it an SCP, an MCP, ?
> Please describe its function rather than its implementation.

M7 is used for HNS(Hisilicon network system), cpu access the network
component via M7.

> 
> What are the external dependencies?
> Is this addressed by one of the patches for edk2-non-osi?

This is depend on M7 firmware which will include in hpm file.

> 
> More style issues below.
> 
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------
>>  1 file changed, 45 insertions(+), 227 deletions(-)
>>
>> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
>> index aaf990216982..9bf274e1b991 100644
>> --- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
>> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
>> @@ -21,44 +21,21 @@
>>  #include <Library/OemNicLib.h>
>>  
>>  #define CPU2_SFP2_100G_CARD_OFFSET   0x25
>> -#define CPU1_SFP1_LOCATE_OFFSET      0x16
>> -#define CPU1_SFP0_LOCATE_OFFSET      0x12
>> -#define CPU2_SFP1_LOCATE_OFFSET      0x21
>> -#define CPU2_SFP0_LOCATE_OFFSET      0x19
>> -#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25
>>  
>> -#define SFP_10G_SPEED   10
>> -#define SFP_25G_SPEED   25
>> -#define SFP_100G_SPEED  100
>> -#define SFP_GE_SPEED    1
>> -
>> -#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C
>> -#define SFP_GE_SPEED_VAL                0x0D
>> -#define SFP_10G_SPEED_VAL               0x67
>> -#define SFP_25G_SPEED_VAL               0xFF
>> +#define SOCKET1_NET_PORT_100G         1
>> +#define SOCKET0_NET_PORT_NUM          4
>> +#define SOCKET1_NET_PORT_NUM          2
>>  
>>  #define CARD_PRESENT_100G               (BIT7)
>> -#define CARD_PRESENT_10G                (BIT0)
>> -#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))
>> -#define SPF_SPEED_OFFSET                12
>> -
>> -#define SFP_DEVICE_ADDRESS 0x50
>> -#define CPU1_9545_I2C_ADDR 0x70
>> -#define CPU2_9545_I2C_ADDR 0x71
>> -
>> -#define FIBER_PRESENT     0
>> -#define CARD_PRESENT      1
>> -#define I2C_PORT_SFP      4
>> -#define CPU2_I2C_PORT_SFP 5
>> -
>> -#define SOCKET_0                 0
>> -#define SOCKET_1                 1
>>  #define EEPROM_I2C_PORT          4
>>  #define EEPROM_PAGE_SIZE         0x40
>>  #define MAC_ADDR_LEN             6
>>  #define I2C_OFFSET_EEPROM_ETH0   (0xc00)
>>  #define I2C_SLAVEADDR_EEPROM     (0x52)
>>  
>> +#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0
>> +#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4
> 
> Is this just a hard-coded address in SRAM? Where is it specified?
> I don't think "_FLAG" is the correct name for this #define - this is
> the actual offset value. So _OFFSET_ADDRESS would be more descriptive.

Yes, M7 firmware will read this two sram addresses.

> 
>> +
>>  #pragma pack(1)
>>  typedef struct {
>>    UINT16 Crc16;
>> @@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {
>>    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,
>>  };
>>  
>> -EFI_STATUS
>> -GetSfpSpeed (
>> -  UINT16 Socket,
>> -  UINT16 SfpNum,
>> -  UINT8* FiberSpeed
>> -  )
>> -{
>> -  EFI_STATUS  Status;
>> -  I2C_DEVICE  SpdDev;
>> -  UINT8       SfpSelect;
>> -  UINT8       SfpSpeed;
>> -  UINT32      RegAddr;
>> -  UINT16      I2cAddr;
>> -  UINT32      SfpPort;
>> -
>> -  SfpSpeed = 0x0;
>> -  if (Socket == SOCKET_1) {
>> -    I2cAddr = CPU2_9545_I2C_ADDR;
>> -    SfpPort = CPU2_I2C_PORT_SFP;
>> -  } else {
>> -    I2cAddr = CPU1_9545_I2C_ADDR;
>> -    SfpPort = I2C_PORT_SFP;
>> -  }
>> -
>> -  Status = I2CInit (Socket, SfpPort, Normal);
>> -  if (EFI_ERROR (Status)) {
>> -    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",
>> -            __FUNCTION__, __LINE__, Socket, Status));
>> -    return Status;
>> -  }
>> -
>> -  SpdDev.Socket = Socket;
>> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;
>> -  SpdDev.Port = SfpPort;
>> -  SpdDev.SlaveDeviceAddress = I2cAddr;
>> -  RegAddr = 0x0;
>> -  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);
>> -
>> -  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);
>> -  if (EFI_ERROR (Status)) {
>> -    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));
>> -    return Status;
>> -  }
>> -
>> -  SpdDev.Socket = Socket;
>> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;
>> -  SpdDev.Port = SfpPort;
>> -  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;
>> -
>> -  RegAddr = SPF_SPEED_OFFSET;
>> -  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);
>> -  if (EFI_ERROR (Status)) {
>> -    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));
>> -    return Status;
>> -  }
>> -
>> -  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",
>> -         SfpSpeed));
>> -
>> -  if (SfpSpeed == SFP_10G_SPEED_VAL) {
>> -    *FiberSpeed = SFP_10G_SPEED;
>> -  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {
>> -    *FiberSpeed = SFP_25G_SPEED;
>> -  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||
>> -             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {
>> -    *FiberSpeed = SFP_GE_SPEED;
>> -  }
>> -
>> -  return EFI_SUCCESS;
>> -}
>> -
>> -//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED
>> -UINT32
>> -GetCpu2FiberType (
>> -  UINT8* Fiber1Type,
>> -  UINT8* Fiber2Type,
>> -  UINT8* Fiber100Ge
>> -  )
>> -{
>> -  EFI_STATUS  Status;
>> -  UINT16      SfpNum1;
>> -  UINT8       SfpSpeed1;
>> -  UINT16      SfpNum2;
>> -  UINT8       SfpSpeed2;
>> -
>> -  SfpNum1 = 0x1;
>> -  SfpSpeed1 = SFP_10G_SPEED;
>> -  SfpNum2 = 0x2;
>> -  SfpSpeed2 = SFP_10G_SPEED;
>> -  *Fiber100Ge = 0x0;
>> -  *Fiber1Type = SFP_10G_SPEED;
>> -  *Fiber2Type = SFP_10G_SPEED;
>> -
>> -  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
>> -    // 100 Ge card
>> -    *Fiber1Type = SFP_10G_SPEED;
>> -    *Fiber2Type = SFP_10G_SPEED;
>> -    *Fiber100Ge = SFP_100G_SPEED;
>> -    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));
>> -  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {
>> -    *Fiber100Ge = 0x0;
>> -    *Fiber1Type = SFP_10G_SPEED;
>> -    *Fiber2Type = SFP_10G_SPEED;
>> -    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
>> -      // Fiber detected in CPU2 slot0, read speed via i2c
>> -      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);
>> -      if (EFI_ERROR (Status)) {
>> -        DEBUG((DEBUG_ERROR,
>> -               "Get Socket1 Sfp%d Speed Error: %r.\n",
>> -               SfpNum1,
>> -               Status));
>> -        return Status;
>> -      }
>> -      if (SfpSpeed1 == SFP_25G_SPEED) {
>> -        // P1 don't support 25G, so set speed to 10G
>> -        *Fiber1Type = SFP_10G_SPEED;
>> -      } else {
>> -        *Fiber1Type = SfpSpeed1;
>> -      }
>> -    } else {
>> -      // No fiber, set speed to 10G
>> -      *Fiber1Type = SFP_10G_SPEED;
>> -    }
>> -
>> -    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
>> -      // Fiber detected in CPU2 slot1, read speed via i2c
>> -      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);
>> -      if (EFI_ERROR (Status)) {
>> -        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
>> -        return Status;
>> -      }
>> -      if (SfpSpeed2 == SFP_25G_SPEED) {
>> -        *Fiber2Type = SFP_10G_SPEED;
>> -      } else {
>> -        *Fiber2Type = SfpSpeed2;
>> -      }
>> -    } else {
>> -      // No fiber, set speed to 10G
>> -      *Fiber2Type = SFP_10G_SPEED;
>> -    }
>> -  } else {
>> -    // 100Ge/10Ge/Ge Fiber is not found.
>> -    *Fiber1Type = SFP_10G_SPEED;
>> -    *Fiber2Type = SFP_10G_SPEED;
>> -    *Fiber100Ge = 0x0;
>> -  }
>> -
>> -  return EFI_SUCCESS;
>> -}
>> -
>> -//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED
>> -UINT32
>> -GetCpu1FiberType (
>> -  UINT8* Fiber1Type,
>> -  UINT8* Fiber2Type
>> -  )
>> -{
>> -  EFI_STATUS  Status;
>> -  UINT16      SfpNum1;
>> -  UINT8       SfpSpeed1;
>> -  UINT16      SfpNum2;
>> -  UINT8       SfpSpeed2;
>> -
>> -  SfpNum1 = 0x1;
>> -  SfpSpeed1 = SFP_10G_SPEED;
>> -  SfpNum2 = 0x2;
>> -  SfpSpeed2 = SFP_10G_SPEED;
>> -  *Fiber1Type = SFP_10G_SPEED;
>> -  *Fiber2Type = SFP_10G_SPEED;
>> -  // Fiber detected in CPU1 slot0, read speed via i2c
>> -  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
>> -    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);
>> -    if (EFI_ERROR (Status)) {
>> -      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",
>> -              SfpNum1, Status));
>> -      return Status;
>> -    }
>> -    *Fiber1Type = SfpSpeed1;
>> -  } else {
>> -    *Fiber1Type = SFP_10G_SPEED;
>> -  }
>> -
>> -  // Fiber detected in CPU1 slot1, read speed via i2c
>> -  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
>> -    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);
>> -    if (EFI_ERROR (Status)) {
>> -      *Fiber2Type = SFP_10G_SPEED;
>> -      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
>> -      return Status;
>> -    }
>> -    *Fiber2Type = SfpSpeed2;
>> -  } else {
>> -    *Fiber2Type = SFP_10G_SPEED;
>> -  }
>> -
>> -  return EFI_SUCCESS;
>> -}
>> -
>>  UINT16 MakeCrcCheckSum (
>>    UINT8 *Buffer,
>>    UINT32 Length
>> @@ -567,3 +346,42 @@ OemIsInitEth (
>>  {
>>    return TRUE;
>>  }
>> +
>> +EFI_STATUS ConfigCDR(UINT32 Socket)
>> +{
>> +    return EFI_SUCCESS;
>> +}
>> +
>> +UINT32 OemGetNclConfOffset (UINT32 Socket)
>> +{
>> +    UINT32           Cpu1NclConfOffet = 0;
> 
> Indentation is 2 spaces, not 4. (Please address throughout.)
> 
>> +    UINT32           Cpu2NclConfOffet = 0;
> 
> Also, no initialization on definition.
> But I see no value in having two variables with complicated names.
> Just a single one called ConfigurationOffset or whatever.
> 
>> +
>> +    if (0 == Socket) {
> 
> No jeopardy-comparisons. Please flip that around.
> 
>> +      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);
> 
> This line can just write a 0 directly.
> But it can also use a comment explaining what writing a 0 here achieves.
> 
>> +      return Cpu1NclConfOffet;
> 
> And this is effectively just an error return - so can just return 0
> directly.
> 
>> +    } else {
> 
> No need for the else. You've returned is there was an error. The rest
> is just the remainder of the function.
> 
>> +      //2P only
> 
> What is 2P?

2 processors, or 2 sockets.

> 
>> +      // P1
> 
> What is P1?

The second processor.

> 
>> +      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
>> +        Cpu2NclConfOffet =  0x20000;
> 
> SIZE_128KB?

ok

> 
>> +      } else {
>> +        Cpu2NclConfOffet =  0x10000;
> 
> SIZE_64KB?

ok

> 
>> +      }
>> +      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);
>> +      return Cpu2NclConfOffet;
>> +    }
>> +}
>> +
>> +UINT32 OemGetNetPortNum (UINT32 Socket)
>> +{
>> +    if (0 == Socket){
> 
> No jeopardy-comparisons. Please flip that around.

All comments will be addressed.

Thanks

> 
> /
>     Leif
> 
>> +    return SOCKET0_NET_PORT_NUM;
>> +    }
>> +
>> +    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
>> +      return SOCKET1_NET_PORT_100G;
>> +    } else {
>> +      return SOCKET1_NET_PORT_NUM;
>> +    }
>> +}
>> -- 
>> 2.9.5
>>

Re: [PATCH edk2-platforms v1 10/16] Hisilicon/D06: Modify for M7 self-Adapte support

[Leif Lindholm]

On Tue, Feb 12, 2019 at 11:14:43PM +0800, Ming Huang wrote:
> 
> 
> On 2/12/2019 3:28 AM, Leif Lindholm wrote:
> > On Fri, Feb 01, 2019 at 09:34:30PM +0800, Ming Huang wrote:
> >> As new M7(Cortex-M7) firmware support self-adapte, so do not
> >> need BIOS to implement some function, remove useless funtions
> >> and report CPU0/CPU1 Nic NCL offset to M7.
> > 
> > I don't really care that some other device in the system is a
> > Cortex-A7. What is its function? Is it an SCP, an MCP, ?
> > Please describe its function rather than its implementation.
> 
> M7 is used for HNS(Hisilicon network system), cpu access the network
> component via M7.

Sure. But does customer documentation documentation refer to it as
"M7"?

> > 
> > What are the external dependencies?
> > Is this addressed by one of the patches for edk2-non-osi?
> 
> This is depend on M7 firmware which will include in hpm file.

So we don't get it when using Capsule Update?

What would be the implication of installing system firmware with the
below change on a system that had not had the corresponding M7
firmware update?

/
    Leif

> > 
> > More style issues below.
> > 
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> >> ---
> >>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------
> >>  1 file changed, 45 insertions(+), 227 deletions(-)
> >>
> >> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
> >> index aaf990216982..9bf274e1b991 100644
> >> --- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
> >> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
> >> @@ -21,44 +21,21 @@
> >>  #include <Library/OemNicLib.h>
> >>  
> >>  #define CPU2_SFP2_100G_CARD_OFFSET   0x25
> >> -#define CPU1_SFP1_LOCATE_OFFSET      0x16
> >> -#define CPU1_SFP0_LOCATE_OFFSET      0x12
> >> -#define CPU2_SFP1_LOCATE_OFFSET      0x21
> >> -#define CPU2_SFP0_LOCATE_OFFSET      0x19
> >> -#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25
> >>  
> >> -#define SFP_10G_SPEED   10
> >> -#define SFP_25G_SPEED   25
> >> -#define SFP_100G_SPEED  100
> >> -#define SFP_GE_SPEED    1
> >> -
> >> -#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C
> >> -#define SFP_GE_SPEED_VAL                0x0D
> >> -#define SFP_10G_SPEED_VAL               0x67
> >> -#define SFP_25G_SPEED_VAL               0xFF
> >> +#define SOCKET1_NET_PORT_100G         1
> >> +#define SOCKET0_NET_PORT_NUM          4
> >> +#define SOCKET1_NET_PORT_NUM          2
> >>  
> >>  #define CARD_PRESENT_100G               (BIT7)
> >> -#define CARD_PRESENT_10G                (BIT0)
> >> -#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))
> >> -#define SPF_SPEED_OFFSET                12
> >> -
> >> -#define SFP_DEVICE_ADDRESS 0x50
> >> -#define CPU1_9545_I2C_ADDR 0x70
> >> -#define CPU2_9545_I2C_ADDR 0x71
> >> -
> >> -#define FIBER_PRESENT     0
> >> -#define CARD_PRESENT      1
> >> -#define I2C_PORT_SFP      4
> >> -#define CPU2_I2C_PORT_SFP 5
> >> -
> >> -#define SOCKET_0                 0
> >> -#define SOCKET_1                 1
> >>  #define EEPROM_I2C_PORT          4
> >>  #define EEPROM_PAGE_SIZE         0x40
> >>  #define MAC_ADDR_LEN             6
> >>  #define I2C_OFFSET_EEPROM_ETH0   (0xc00)
> >>  #define I2C_SLAVEADDR_EEPROM     (0x52)
> >>  
> >> +#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0
> >> +#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4
> > 
> > Is this just a hard-coded address in SRAM? Where is it specified?
> > I don't think "_FLAG" is the correct name for this #define - this is
> > the actual offset value. So _OFFSET_ADDRESS would be more descriptive.
> 
> Yes, M7 firmware will read this two sram addresses.
> 
> > 
> >> +
> >>  #pragma pack(1)
> >>  typedef struct {
> >>    UINT16 Crc16;
> >> @@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {
> >>    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,
> >>  };
> >>  
> >> -EFI_STATUS
> >> -GetSfpSpeed (
> >> -  UINT16 Socket,
> >> -  UINT16 SfpNum,
> >> -  UINT8* FiberSpeed
> >> -  )
> >> -{
> >> -  EFI_STATUS  Status;
> >> -  I2C_DEVICE  SpdDev;
> >> -  UINT8       SfpSelect;
> >> -  UINT8       SfpSpeed;
> >> -  UINT32      RegAddr;
> >> -  UINT16      I2cAddr;
> >> -  UINT32      SfpPort;
> >> -
> >> -  SfpSpeed = 0x0;
> >> -  if (Socket == SOCKET_1) {
> >> -    I2cAddr = CPU2_9545_I2C_ADDR;
> >> -    SfpPort = CPU2_I2C_PORT_SFP;
> >> -  } else {
> >> -    I2cAddr = CPU1_9545_I2C_ADDR;
> >> -    SfpPort = I2C_PORT_SFP;
> >> -  }
> >> -
> >> -  Status = I2CInit (Socket, SfpPort, Normal);
> >> -  if (EFI_ERROR (Status)) {
> >> -    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",
> >> -            __FUNCTION__, __LINE__, Socket, Status));
> >> -    return Status;
> >> -  }
> >> -
> >> -  SpdDev.Socket = Socket;
> >> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;
> >> -  SpdDev.Port = SfpPort;
> >> -  SpdDev.SlaveDeviceAddress = I2cAddr;
> >> -  RegAddr = 0x0;
> >> -  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);
> >> -
> >> -  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);
> >> -  if (EFI_ERROR (Status)) {
> >> -    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));
> >> -    return Status;
> >> -  }
> >> -
> >> -  SpdDev.Socket = Socket;
> >> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;
> >> -  SpdDev.Port = SfpPort;
> >> -  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;
> >> -
> >> -  RegAddr = SPF_SPEED_OFFSET;
> >> -  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);
> >> -  if (EFI_ERROR (Status)) {
> >> -    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));
> >> -    return Status;
> >> -  }
> >> -
> >> -  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",
> >> -         SfpSpeed));
> >> -
> >> -  if (SfpSpeed == SFP_10G_SPEED_VAL) {
> >> -    *FiberSpeed = SFP_10G_SPEED;
> >> -  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {
> >> -    *FiberSpeed = SFP_25G_SPEED;
> >> -  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||
> >> -             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {
> >> -    *FiberSpeed = SFP_GE_SPEED;
> >> -  }
> >> -
> >> -  return EFI_SUCCESS;
> >> -}
> >> -
> >> -//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED
> >> -UINT32
> >> -GetCpu2FiberType (
> >> -  UINT8* Fiber1Type,
> >> -  UINT8* Fiber2Type,
> >> -  UINT8* Fiber100Ge
> >> -  )
> >> -{
> >> -  EFI_STATUS  Status;
> >> -  UINT16      SfpNum1;
> >> -  UINT8       SfpSpeed1;
> >> -  UINT16      SfpNum2;
> >> -  UINT8       SfpSpeed2;
> >> -
> >> -  SfpNum1 = 0x1;
> >> -  SfpSpeed1 = SFP_10G_SPEED;
> >> -  SfpNum2 = 0x2;
> >> -  SfpSpeed2 = SFP_10G_SPEED;
> >> -  *Fiber100Ge = 0x0;
> >> -  *Fiber1Type = SFP_10G_SPEED;
> >> -  *Fiber2Type = SFP_10G_SPEED;
> >> -
> >> -  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
> >> -    // 100 Ge card
> >> -    *Fiber1Type = SFP_10G_SPEED;
> >> -    *Fiber2Type = SFP_10G_SPEED;
> >> -    *Fiber100Ge = SFP_100G_SPEED;
> >> -    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));
> >> -  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {
> >> -    *Fiber100Ge = 0x0;
> >> -    *Fiber1Type = SFP_10G_SPEED;
> >> -    *Fiber2Type = SFP_10G_SPEED;
> >> -    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
> >> -      // Fiber detected in CPU2 slot0, read speed via i2c
> >> -      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);
> >> -      if (EFI_ERROR (Status)) {
> >> -        DEBUG((DEBUG_ERROR,
> >> -               "Get Socket1 Sfp%d Speed Error: %r.\n",
> >> -               SfpNum1,
> >> -               Status));
> >> -        return Status;
> >> -      }
> >> -      if (SfpSpeed1 == SFP_25G_SPEED) {
> >> -        // P1 don't support 25G, so set speed to 10G
> >> -        *Fiber1Type = SFP_10G_SPEED;
> >> -      } else {
> >> -        *Fiber1Type = SfpSpeed1;
> >> -      }
> >> -    } else {
> >> -      // No fiber, set speed to 10G
> >> -      *Fiber1Type = SFP_10G_SPEED;
> >> -    }
> >> -
> >> -    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
> >> -      // Fiber detected in CPU2 slot1, read speed via i2c
> >> -      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);
> >> -      if (EFI_ERROR (Status)) {
> >> -        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
> >> -        return Status;
> >> -      }
> >> -      if (SfpSpeed2 == SFP_25G_SPEED) {
> >> -        *Fiber2Type = SFP_10G_SPEED;
> >> -      } else {
> >> -        *Fiber2Type = SfpSpeed2;
> >> -      }
> >> -    } else {
> >> -      // No fiber, set speed to 10G
> >> -      *Fiber2Type = SFP_10G_SPEED;
> >> -    }
> >> -  } else {
> >> -    // 100Ge/10Ge/Ge Fiber is not found.
> >> -    *Fiber1Type = SFP_10G_SPEED;
> >> -    *Fiber2Type = SFP_10G_SPEED;
> >> -    *Fiber100Ge = 0x0;
> >> -  }
> >> -
> >> -  return EFI_SUCCESS;
> >> -}
> >> -
> >> -//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED
> >> -UINT32
> >> -GetCpu1FiberType (
> >> -  UINT8* Fiber1Type,
> >> -  UINT8* Fiber2Type
> >> -  )
> >> -{
> >> -  EFI_STATUS  Status;
> >> -  UINT16      SfpNum1;
> >> -  UINT8       SfpSpeed1;
> >> -  UINT16      SfpNum2;
> >> -  UINT8       SfpSpeed2;
> >> -
> >> -  SfpNum1 = 0x1;
> >> -  SfpSpeed1 = SFP_10G_SPEED;
> >> -  SfpNum2 = 0x2;
> >> -  SfpSpeed2 = SFP_10G_SPEED;
> >> -  *Fiber1Type = SFP_10G_SPEED;
> >> -  *Fiber2Type = SFP_10G_SPEED;
> >> -  // Fiber detected in CPU1 slot0, read speed via i2c
> >> -  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
> >> -    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);
> >> -    if (EFI_ERROR (Status)) {
> >> -      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",
> >> -              SfpNum1, Status));
> >> -      return Status;
> >> -    }
> >> -    *Fiber1Type = SfpSpeed1;
> >> -  } else {
> >> -    *Fiber1Type = SFP_10G_SPEED;
> >> -  }
> >> -
> >> -  // Fiber detected in CPU1 slot1, read speed via i2c
> >> -  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
> >> -    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);
> >> -    if (EFI_ERROR (Status)) {
> >> -      *Fiber2Type = SFP_10G_SPEED;
> >> -      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
> >> -      return Status;
> >> -    }
> >> -    *Fiber2Type = SfpSpeed2;
> >> -  } else {
> >> -    *Fiber2Type = SFP_10G_SPEED;
> >> -  }
> >> -
> >> -  return EFI_SUCCESS;
> >> -}
> >> -
> >>  UINT16 MakeCrcCheckSum (
> >>    UINT8 *Buffer,
> >>    UINT32 Length
> >> @@ -567,3 +346,42 @@ OemIsInitEth (
> >>  {
> >>    return TRUE;
> >>  }
> >> +
> >> +EFI_STATUS ConfigCDR(UINT32 Socket)
> >> +{
> >> +    return EFI_SUCCESS;
> >> +}
> >> +
> >> +UINT32 OemGetNclConfOffset (UINT32 Socket)
> >> +{
> >> +    UINT32           Cpu1NclConfOffet = 0;
> > 
> > Indentation is 2 spaces, not 4. (Please address throughout.)
> > 
> >> +    UINT32           Cpu2NclConfOffet = 0;
> > 
> > Also, no initialization on definition.
> > But I see no value in having two variables with complicated names.
> > Just a single one called ConfigurationOffset or whatever.
> > 
> >> +
> >> +    if (0 == Socket) {
> > 
> > No jeopardy-comparisons. Please flip that around.
> > 
> >> +      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);
> > 
> > This line can just write a 0 directly.
> > But it can also use a comment explaining what writing a 0 here achieves.
> > 
> >> +      return Cpu1NclConfOffet;
> > 
> > And this is effectively just an error return - so can just return 0
> > directly.
> > 
> >> +    } else {
> > 
> > No need for the else. You've returned is there was an error. The rest
> > is just the remainder of the function.
> > 
> >> +      //2P only
> > 
> > What is 2P?
> 
> 2 processors, or 2 sockets.
> 
> > 
> >> +      // P1
> > 
> > What is P1?
> 
> The second processor.
> 
> > 
> >> +      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
> >> +        Cpu2NclConfOffet =  0x20000;
> > 
> > SIZE_128KB?
> 
> ok
> 
> > 
> >> +      } else {
> >> +        Cpu2NclConfOffet =  0x10000;
> > 
> > SIZE_64KB?
> 
> ok
> 
> > 
> >> +      }
> >> +      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);
> >> +      return Cpu2NclConfOffet;
> >> +    }
> >> +}
> >> +
> >> +UINT32 OemGetNetPortNum (UINT32 Socket)
> >> +{
> >> +    if (0 == Socket){
> > 
> > No jeopardy-comparisons. Please flip that around.
> 
> All comments will be addressed.
> 
> Thanks
> 
> > 
> > /
> >     Leif
> > 
> >> +    return SOCKET0_NET_PORT_NUM;
> >> +    }
> >> +
> >> +    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
> >> +      return SOCKET1_NET_PORT_100G;
> >> +    } else {
> >> +      return SOCKET1_NET_PORT_NUM;
> >> +    }
> >> +}
> >> -- 
> >> 2.9.5
> >>

Re: [PATCH edk2-platforms v1 10/16] Hisilicon/D06: Modify for M7 self-Adapte support

[Ming Huang]

On 2/12/2019 11:46 PM, Leif Lindholm wrote:
> On Tue, Feb 12, 2019 at 11:14:43PM +0800, Ming Huang wrote:
>>
>>
>> On 2/12/2019 3:28 AM, Leif Lindholm wrote:
>>> On Fri, Feb 01, 2019 at 09:34:30PM +0800, Ming Huang wrote:
>>>> As new M7(Cortex-M7) firmware support self-adapte, so do not
>>>> need BIOS to implement some function, remove useless funtions
>>>> and report CPU0/CPU1 Nic NCL offset to M7.
>>>
>>> I don't really care that some other device in the system is a
>>> Cortex-A7. What is its function? Is it an SCP, an MCP, ?
>>> Please describe its function rather than its implementation.
>>
>> M7 is used for HNS(Hisilicon network system), cpu access the network
>> component via M7.
> 
> Sure. But does customer documentation documentation refer to it as
> "M7"?

I check documentation just now, Integrated Management Processor(IMP) is used,
so, I will change commit titil and message M7 to IMP.

> 
>>>
>>> What are the external dependencies?
>>> Is this addressed by one of the patches for edk2-non-osi?
>>
>> This is depend on M7 firmware which will include in hpm file.
> 
> So we don't get it when using Capsule Update?

Yes.

> 
> What would be the implication of installing system firmware with the
> below change on a system that had not had the corresponding M7
> firmware update?

The HNS will not worked.

Thanks

> 
> /
>     Leif
> 
>>>
>>> More style issues below.
>>>
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>>>> ---
>>>>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c | 272 ++++----------------
>>>>  1 file changed, 45 insertions(+), 227 deletions(-)
>>>>
>>>> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
>>>> index aaf990216982..9bf274e1b991 100644
>>>> --- a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
>>>> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
>>>> @@ -21,44 +21,21 @@
>>>>  #include <Library/OemNicLib.h>
>>>>  
>>>>  #define CPU2_SFP2_100G_CARD_OFFSET   0x25
>>>> -#define CPU1_SFP1_LOCATE_OFFSET      0x16
>>>> -#define CPU1_SFP0_LOCATE_OFFSET      0x12
>>>> -#define CPU2_SFP1_LOCATE_OFFSET      0x21
>>>> -#define CPU2_SFP0_LOCATE_OFFSET      0x19
>>>> -#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25
>>>>  
>>>> -#define SFP_10G_SPEED   10
>>>> -#define SFP_25G_SPEED   25
>>>> -#define SFP_100G_SPEED  100
>>>> -#define SFP_GE_SPEED    1
>>>> -
>>>> -#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C
>>>> -#define SFP_GE_SPEED_VAL                0x0D
>>>> -#define SFP_10G_SPEED_VAL               0x67
>>>> -#define SFP_25G_SPEED_VAL               0xFF
>>>> +#define SOCKET1_NET_PORT_100G         1
>>>> +#define SOCKET0_NET_PORT_NUM          4
>>>> +#define SOCKET1_NET_PORT_NUM          2
>>>>  
>>>>  #define CARD_PRESENT_100G               (BIT7)
>>>> -#define CARD_PRESENT_10G                (BIT0)
>>>> -#define SELECT_SFP_BY_INDEX(index)      (1 << (index - 1))
>>>> -#define SPF_SPEED_OFFSET                12
>>>> -
>>>> -#define SFP_DEVICE_ADDRESS 0x50
>>>> -#define CPU1_9545_I2C_ADDR 0x70
>>>> -#define CPU2_9545_I2C_ADDR 0x71
>>>> -
>>>> -#define FIBER_PRESENT     0
>>>> -#define CARD_PRESENT      1
>>>> -#define I2C_PORT_SFP      4
>>>> -#define CPU2_I2C_PORT_SFP 5
>>>> -
>>>> -#define SOCKET_0                 0
>>>> -#define SOCKET_1                 1
>>>>  #define EEPROM_I2C_PORT          4
>>>>  #define EEPROM_PAGE_SIZE         0x40
>>>>  #define MAC_ADDR_LEN             6
>>>>  #define I2C_OFFSET_EEPROM_ETH0   (0xc00)
>>>>  #define I2C_SLAVEADDR_EEPROM     (0x52)
>>>>  
>>>> +#define SRAM_NIC_NCL1_OFFSET_FLAG   0xA0E87FE0
>>>> +#define SRAM_NIC_NCL2_OFFSET_FLAG   0xA0E87FE4
>>>
>>> Is this just a hard-coded address in SRAM? Where is it specified?
>>> I don't think "_FLAG" is the correct name for this #define - this is
>>> the actual offset value. So _OFFSET_ADDRESS would be more descriptive.
>>
>> Yes, M7 firmware will read this two sram addresses.
>>
>>>
>>>> +
>>>>  #pragma pack(1)
>>>>  typedef struct {
>>>>    UINT16 Crc16;
>>>> @@ -114,204 +91,6 @@ UINT16 CrcTable16[256] = {
>>>>    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0,
>>>>  };
>>>>  
>>>> -EFI_STATUS
>>>> -GetSfpSpeed (
>>>> -  UINT16 Socket,
>>>> -  UINT16 SfpNum,
>>>> -  UINT8* FiberSpeed
>>>> -  )
>>>> -{
>>>> -  EFI_STATUS  Status;
>>>> -  I2C_DEVICE  SpdDev;
>>>> -  UINT8       SfpSelect;
>>>> -  UINT8       SfpSpeed;
>>>> -  UINT32      RegAddr;
>>>> -  UINT16      I2cAddr;
>>>> -  UINT32      SfpPort;
>>>> -
>>>> -  SfpSpeed = 0x0;
>>>> -  if (Socket == SOCKET_1) {
>>>> -    I2cAddr = CPU2_9545_I2C_ADDR;
>>>> -    SfpPort = CPU2_I2C_PORT_SFP;
>>>> -  } else {
>>>> -    I2cAddr = CPU1_9545_I2C_ADDR;
>>>> -    SfpPort = I2C_PORT_SFP;
>>>> -  }
>>>> -
>>>> -  Status = I2CInit (Socket, SfpPort, Normal);
>>>> -  if (EFI_ERROR (Status)) {
>>>> -    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Socket%d Call I2CInit failed! p1=0x%x.\n",
>>>> -            __FUNCTION__, __LINE__, Socket, Status));
>>>> -    return Status;
>>>> -  }
>>>> -
>>>> -  SpdDev.Socket = Socket;
>>>> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;
>>>> -  SpdDev.Port = SfpPort;
>>>> -  SpdDev.SlaveDeviceAddress = I2cAddr;
>>>> -  RegAddr = 0x0;
>>>> -  SfpSelect = SELECT_SFP_BY_INDEX (SfpNum);
>>>> -
>>>> -  Status = I2CWrite (&SpdDev, RegAddr, 1, &SfpSelect);
>>>> -  if (EFI_ERROR (Status)) {
>>>> -    DEBUG ((DEBUG_ERROR, "I2CWrite Error =%r.\n", Status));
>>>> -    return Status;
>>>> -  }
>>>> -
>>>> -  SpdDev.Socket = Socket;
>>>> -  SpdDev.DeviceType = DEVICE_TYPE_SPD;
>>>> -  SpdDev.Port = SfpPort;
>>>> -  SpdDev.SlaveDeviceAddress = SFP_DEVICE_ADDRESS;
>>>> -
>>>> -  RegAddr = SPF_SPEED_OFFSET;
>>>> -  Status = I2CRead (&SpdDev, RegAddr, 1, &SfpSpeed);
>>>> -  if (EFI_ERROR (Status)) {
>>>> -    DEBUG ((DEBUG_ERROR, "I2CRead Error =%r.\n", Status));
>>>> -    return Status;
>>>> -  }
>>>> -
>>>> -  DEBUG ((DEBUG_INFO, "BR, Nominal, Nominal signalling rate, SfpSpeed:    0x%x\n",
>>>> -         SfpSpeed));
>>>> -
>>>> -  if (SfpSpeed == SFP_10G_SPEED_VAL) {
>>>> -    *FiberSpeed = SFP_10G_SPEED;
>>>> -  } else if (SfpSpeed == SFP_25G_SPEED_VAL) {
>>>> -    *FiberSpeed = SFP_25G_SPEED;
>>>> -  } else if ((SfpSpeed == SFP_GE_SPEED_VAL) ||
>>>> -             (SfpSpeed == SFP_GE_SPEED_VAL_VENDOR_FINISAR)) {
>>>> -    *FiberSpeed = SFP_GE_SPEED;
>>>> -  }
>>>> -
>>>> -  return EFI_SUCCESS;
>>>> -}
>>>> -
>>>> -//Fiber1Type/Fiber2Type/Fiber3Type return: SFP_10G_SPEED, SFP_100G_SPEED, SFP_GE_SPEED
>>>> -UINT32
>>>> -GetCpu2FiberType (
>>>> -  UINT8* Fiber1Type,
>>>> -  UINT8* Fiber2Type,
>>>> -  UINT8* Fiber100Ge
>>>> -  )
>>>> -{
>>>> -  EFI_STATUS  Status;
>>>> -  UINT16      SfpNum1;
>>>> -  UINT8       SfpSpeed1;
>>>> -  UINT16      SfpNum2;
>>>> -  UINT8       SfpSpeed2;
>>>> -
>>>> -  SfpNum1 = 0x1;
>>>> -  SfpSpeed1 = SFP_10G_SPEED;
>>>> -  SfpNum2 = 0x2;
>>>> -  SfpSpeed2 = SFP_10G_SPEED;
>>>> -  *Fiber100Ge = 0x0;
>>>> -  *Fiber1Type = SFP_10G_SPEED;
>>>> -  *Fiber2Type = SFP_10G_SPEED;
>>>> -
>>>> -  if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
>>>> -    // 100 Ge card
>>>> -    *Fiber1Type = SFP_10G_SPEED;
>>>> -    *Fiber2Type = SFP_10G_SPEED;
>>>> -    *Fiber100Ge = SFP_100G_SPEED;
>>>> -    DEBUG ((DEBUG_ERROR,"Detect Fiber SFP_100G is Present, Set 100Ge\n"));
>>>> -  } else if ((ReadCpldReg (CPU2_SFP2_10G_GE_CARD_OFFSET) & CARD_PRESENT_10G) != 0) {
>>>> -    *Fiber100Ge = 0x0;
>>>> -    *Fiber1Type = SFP_10G_SPEED;
>>>> -    *Fiber2Type = SFP_10G_SPEED;
>>>> -    if (ReadCpldReg (CPU2_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
>>>> -      // Fiber detected in CPU2 slot0, read speed via i2c
>>>> -      Status = GetSfpSpeed (SOCKET_1, SfpNum1, &SfpSpeed1);
>>>> -      if (EFI_ERROR (Status)) {
>>>> -        DEBUG((DEBUG_ERROR,
>>>> -               "Get Socket1 Sfp%d Speed Error: %r.\n",
>>>> -               SfpNum1,
>>>> -               Status));
>>>> -        return Status;
>>>> -      }
>>>> -      if (SfpSpeed1 == SFP_25G_SPEED) {
>>>> -        // P1 don't support 25G, so set speed to 10G
>>>> -        *Fiber1Type = SFP_10G_SPEED;
>>>> -      } else {
>>>> -        *Fiber1Type = SfpSpeed1;
>>>> -      }
>>>> -    } else {
>>>> -      // No fiber, set speed to 10G
>>>> -      *Fiber1Type = SFP_10G_SPEED;
>>>> -    }
>>>> -
>>>> -    if (ReadCpldReg (CPU2_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
>>>> -      // Fiber detected in CPU2 slot1, read speed via i2c
>>>> -      Status = GetSfpSpeed (SOCKET_1, SfpNum2, &SfpSpeed2);
>>>> -      if (EFI_ERROR (Status)) {
>>>> -        DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
>>>> -        return Status;
>>>> -      }
>>>> -      if (SfpSpeed2 == SFP_25G_SPEED) {
>>>> -        *Fiber2Type = SFP_10G_SPEED;
>>>> -      } else {
>>>> -        *Fiber2Type = SfpSpeed2;
>>>> -      }
>>>> -    } else {
>>>> -      // No fiber, set speed to 10G
>>>> -      *Fiber2Type = SFP_10G_SPEED;
>>>> -    }
>>>> -  } else {
>>>> -    // 100Ge/10Ge/Ge Fiber is not found.
>>>> -    *Fiber1Type = SFP_10G_SPEED;
>>>> -    *Fiber2Type = SFP_10G_SPEED;
>>>> -    *Fiber100Ge = 0x0;
>>>> -  }
>>>> -
>>>> -  return EFI_SUCCESS;
>>>> -}
>>>> -
>>>> -//Fiber1Type/Fiber2Type return: SFP_10G_SPEED, SFP_25G_SPEED, SFP_GE_SPEED
>>>> -UINT32
>>>> -GetCpu1FiberType (
>>>> -  UINT8* Fiber1Type,
>>>> -  UINT8* Fiber2Type
>>>> -  )
>>>> -{
>>>> -  EFI_STATUS  Status;
>>>> -  UINT16      SfpNum1;
>>>> -  UINT8       SfpSpeed1;
>>>> -  UINT16      SfpNum2;
>>>> -  UINT8       SfpSpeed2;
>>>> -
>>>> -  SfpNum1 = 0x1;
>>>> -  SfpSpeed1 = SFP_10G_SPEED;
>>>> -  SfpNum2 = 0x2;
>>>> -  SfpSpeed2 = SFP_10G_SPEED;
>>>> -  *Fiber1Type = SFP_10G_SPEED;
>>>> -  *Fiber2Type = SFP_10G_SPEED;
>>>> -  // Fiber detected in CPU1 slot0, read speed via i2c
>>>> -  if (ReadCpldReg (CPU1_SFP0_LOCATE_OFFSET) == FIBER_PRESENT) {
>>>> -    Status = GetSfpSpeed (SOCKET_0, SfpNum1, &SfpSpeed1);
>>>> -    if (EFI_ERROR (Status)) {
>>>> -      DEBUG ((DEBUG_ERROR, "Get Socket0 Sfp%d Speed Error: %r.\n",
>>>> -              SfpNum1, Status));
>>>> -      return Status;
>>>> -    }
>>>> -    *Fiber1Type = SfpSpeed1;
>>>> -  } else {
>>>> -    *Fiber1Type = SFP_10G_SPEED;
>>>> -  }
>>>> -
>>>> -  // Fiber detected in CPU1 slot1, read speed via i2c
>>>> -  if (ReadCpldReg (CPU1_SFP1_LOCATE_OFFSET) == FIBER_PRESENT) {
>>>> -    Status = GetSfpSpeed (SOCKET_0, SfpNum2, &SfpSpeed2);
>>>> -    if (EFI_ERROR (Status)) {
>>>> -      *Fiber2Type = SFP_10G_SPEED;
>>>> -      DEBUG ((DEBUG_ERROR, "Get Sfp%d Speed Error: %r.\n", SfpNum2, Status));
>>>> -      return Status;
>>>> -    }
>>>> -    *Fiber2Type = SfpSpeed2;
>>>> -  } else {
>>>> -    *Fiber2Type = SFP_10G_SPEED;
>>>> -  }
>>>> -
>>>> -  return EFI_SUCCESS;
>>>> -}
>>>> -
>>>>  UINT16 MakeCrcCheckSum (
>>>>    UINT8 *Buffer,
>>>>    UINT32 Length
>>>> @@ -567,3 +346,42 @@ OemIsInitEth (
>>>>  {
>>>>    return TRUE;
>>>>  }
>>>> +
>>>> +EFI_STATUS ConfigCDR(UINT32 Socket)
>>>> +{
>>>> +    return EFI_SUCCESS;
>>>> +}
>>>> +
>>>> +UINT32 OemGetNclConfOffset (UINT32 Socket)
>>>> +{
>>>> +    UINT32           Cpu1NclConfOffet = 0;
>>>
>>> Indentation is 2 spaces, not 4. (Please address throughout.)
>>>
>>>> +    UINT32           Cpu2NclConfOffet = 0;
>>>
>>> Also, no initialization on definition.
>>> But I see no value in having two variables with complicated names.
>>> Just a single one called ConfigurationOffset or whatever.
>>>
>>>> +
>>>> +    if (0 == Socket) {
>>>
>>> No jeopardy-comparisons. Please flip that around.
>>>
>>>> +      MmioWrite32 (SRAM_NIC_NCL1_OFFSET_FLAG, Cpu1NclConfOffet);
>>>
>>> This line can just write a 0 directly.
>>> But it can also use a comment explaining what writing a 0 here achieves.
>>>
>>>> +      return Cpu1NclConfOffet;
>>>
>>> And this is effectively just an error return - so can just return 0
>>> directly.
>>>
>>>> +    } else {
>>>
>>> No need for the else. You've returned is there was an error. The rest
>>> is just the remainder of the function.
>>>
>>>> +      //2P only
>>>
>>> What is 2P?
>>
>> 2 processors, or 2 sockets.
>>
>>>
>>>> +      // P1
>>>
>>> What is P1?
>>
>> The second processor.
>>
>>>
>>>> +      if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
>>>> +        Cpu2NclConfOffet =  0x20000;
>>>
>>> SIZE_128KB?
>>
>> ok
>>
>>>
>>>> +      } else {
>>>> +        Cpu2NclConfOffet =  0x10000;
>>>
>>> SIZE_64KB?
>>
>> ok
>>
>>>
>>>> +      }
>>>> +      MmioWrite32 (SRAM_NIC_NCL2_OFFSET_FLAG, Cpu2NclConfOffet);
>>>> +      return Cpu2NclConfOffet;
>>>> +    }
>>>> +}
>>>> +
>>>> +UINT32 OemGetNetPortNum (UINT32 Socket)
>>>> +{
>>>> +    if (0 == Socket){
>>>
>>> No jeopardy-comparisons. Please flip that around.
>>
>> All comments will be addressed.
>>
>> Thanks
>>
>>>
>>> /
>>>     Leif
>>>
>>>> +    return SOCKET0_NET_PORT_NUM;
>>>> +    }
>>>> +
>>>> +    if ((ReadCpldReg (CPU2_SFP2_100G_CARD_OFFSET) & CARD_PRESENT_100G) != 0) {
>>>> +      return SOCKET1_NET_PORT_100G;
>>>> +    } else {
>>>> +      return SOCKET1_NET_PORT_NUM;
>>>> +    }
>>>> +}
>>>> -- 
>>>> 2.9.5
>>>>

[PATCH edk2-platforms v1 11/16] Hisilicon/D06: Add Setup Item "Support DPC"

[Ming Huang]

Add setup item "Support DPC" to enable or disable PCIe DPC
(Downstream Port Containment).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Silicon/Hisilicon/Include/Library/OemConfigData.h                   |   1 +
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr      |   2 -
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c           |   4 +
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr        | 197 +-------------------
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni |   3 +-
 5 files changed, 10 insertions(+), 197 deletions(-)

diff --git a/Silicon/Hisilicon/Include/Library/OemConfigData.h b/Silicon/Hisilicon/Include/Library/OemConfigData.h
index f120e3123c83..c0097d0829f0 100644
--- a/Silicon/Hisilicon/Include/Library/OemConfigData.h
+++ b/Silicon/Hisilicon/Include/Library/OemConfigData.h
@@ -49,6 +49,7 @@ typedef struct {
   UINT8         OSWdtAction;
   /*PCIe Config*/
   UINT8         PcieSRIOVSupport;
+  UINT8         PcieDPCSupport;
   UINT8         PciePort[PCIE_MAX_TOTAL_PORTS];
   UINT8         PcieLinkSpeedPort[PCIE_MAX_TOTAL_PORTS];
   UINT8         PcieLinkDeEmphasisPort[PCIE_MAX_TOTAL_PORTS];
diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
index 08236704fbfe..93ccb99bdc67 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
+++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
@@ -62,11 +62,9 @@ formset
       prompt = STRING_TOKEN(STR_IBMC_CONFIG_FORM_TITLE),
       help   = STRING_TOKEN(STR_IBMC_CONFIG_FORM_HELP);
 
-    suppressif TRUE;
     goto PCIE_CONFIG_FORM_ID,
       prompt  = STRING_TOKEN(STR_PCIE_CONFIG_FORM_TITLE),
       help    = STRING_TOKEN(STR_PCIE_CONFIG_FORM_HELP);
-    endif;
 
     goto MISC_CONFIG_FORM_ID,
       prompt  = STRING_TOKEN(STR_MISC_CONFIG_FORM_TITLE),
diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
index 6668103af027..be4ce8820f73 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
+++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
@@ -290,6 +290,10 @@ OemConfigUiLibConstructor (
       Configuration.OSWdtTimeout = 5;
       Configuration.OSWdtAction = 1;
       //
+      //Set the default value of the PCIe option
+      //
+      Configuration.PcieDPCSupport = 0;
+      //
       //Set the default value of the Misc option
       //
       Configuration.EnableSmmu = 1;
diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
index 7cf7cdd29ba2..c65907fe846e 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
+++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
@@ -17,203 +17,12 @@
 form formid = PCIE_CONFIG_FORM_ID,
   title   = STRING_TOKEN (STR_PCIE_CONFIG_FORM_TITLE);
 
-  goto VFR_FORMID_PCIE_SOCKET0,
-    prompt  = STRING_TOKEN (STR_PCIE_CPU_0_PROMPT),
-    help    = STRING_TOKEN (STR_PCIE_CPU_PROMPT_HELP);
-
-  goto VFR_FORMID_PCIE_SOCKET1,
-    prompt  = STRING_TOKEN (STR_PCIE_CPU_1_PROMPT),
-    help    = STRING_TOKEN (STR_PCIE_CPU_PROMPT_HELP);
-
-  oneof varid  = OEM_CONFIG_DATA.PcieSRIOVSupport,
-        prompt   = STRING_TOKEN (STR_SRIOV_SUPPORT_PROMPT),
-        help     = STRING_TOKEN (STR_SRIOV_SUPPORT_HELP),
+  oneof varid  = OEM_CONFIG_DATA.PcieDPCSupport,
+        prompt   = STRING_TOKEN (STR_DPC_SUPPORT_PROMPT),
+        help     = STRING_TOKEN (STR_DPC_SUPPORT_HELP),
         option text = STRING_TOKEN (STR_DISABLE), value = 0, flags = MANUFACTURING | DEFAULT | RESET_REQUIRED;
         option text = STRING_TOKEN (STR_ENABLE),  value = 1, flags = RESET_REQUIRED;
   endoneof;
 
 endform;
 
-form formid = VFR_FORMID_PCIE_SOCKET0,
-  title = STRING_TOKEN(STR_PCIE_CPU_0_PROMPT);
-
-  goto VFR_FORMID_PCIE_PORT2,
-    prompt  = STRING_TOKEN(STR_PCIE_PORT_2_PROMPT),
-    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
-
-  goto VFR_FORMID_PCIE_PORT4,
-    prompt  = STRING_TOKEN(STR_PCIE_PORT_4_PROMPT),
-    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
-
-  goto VFR_FORMID_PCIE_PORT5,
-    prompt  = STRING_TOKEN(STR_PCIE_PORT_5_PROMPT),
-    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
-
-  goto VFR_FORMID_PCIE_PORT6,
-    prompt  = STRING_TOKEN(STR_PCIE_PORT_6_PROMPT),
-    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
-
-  goto VFR_FORMID_PCIE_PORT7,
-    prompt  = STRING_TOKEN(STR_PCIE_PORT_7_PROMPT),
-    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
-
-endform;
-
-form formid = VFR_FORMID_PCIE_SOCKET1,
-  title = STRING_TOKEN(STR_PCIE_CPU_1_PROMPT);
-  goto VFR_FORMID_PCIE_PORT10,
-    prompt  = STRING_TOKEN(STR_PCIE_PORT_10_PROMPT),
-    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
-
-  goto VFR_FORMID_PCIE_PORT12,
-    prompt  = STRING_TOKEN(STR_PCIE_PORT_12_PROMPT),
-    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
-
-  goto VFR_FORMID_PCIE_PORT13,
-    prompt  = STRING_TOKEN(STR_PCIE_PORT_13_PROMPT),
-    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT0,
-  title = STRING_TOKEN(STR_PCIE_PORT_0_PROMPT);
-  #undef  INDEX
-  #define INDEX   0
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT1,
-  title = STRING_TOKEN(STR_PCIE_PORT_1_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   1
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT2,
-  title = STRING_TOKEN(STR_PCIE_PORT_2_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   2
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT3,
-  title = STRING_TOKEN(STR_PCIE_PORT_3_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   3
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT4,
-  title = STRING_TOKEN(STR_PCIE_PORT_4_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   4
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT5,
-  title = STRING_TOKEN(STR_PCIE_PORT_5_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   5
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT6,
-  title = STRING_TOKEN(STR_PCIE_PORT_6_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   6
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT7,
-  title = STRING_TOKEN(STR_PCIE_PORT_7_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   7
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT8,
-  title = STRING_TOKEN(STR_PCIE_PORT_8_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   8
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT9,
-  title = STRING_TOKEN(STR_PCIE_PORT_9_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   9
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT10,
-  title = STRING_TOKEN(STR_PCIE_PORT_10_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   10
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT11,
-  title = STRING_TOKEN(STR_PCIE_PORT_11_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   11
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT12,
-  title = STRING_TOKEN(STR_PCIE_PORT_12_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   12
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT13,
-  title = STRING_TOKEN(STR_PCIE_PORT_13_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   13
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT14,
-  title = STRING_TOKEN(STR_PCIE_PORT_14_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   14
-  #include "PciePortConfig.hfr"
-
-endform;
-
-form formid = VFR_FORMID_PCIE_PORT15,
-  title = STRING_TOKEN(STR_PCIE_PORT_15_PROMPT);
-
-  #undef  INDEX
-  #define INDEX   15
-  #include "PciePortConfig.hfr"
-
-endform;
-
diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
index d87d30f975b8..0127ea952dee 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
+++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
@@ -26,7 +26,8 @@
 #string STR_PCIE_CPU_1_PROMPT           #language en-US "CPU 1 PCIE Configuration"
 #string STR_SRIOV_SUPPORT_PROMPT        #language en-US "SRIOV"
 #string STR_SRIOV_SUPPORT_HELP          #language en-US "This option enables / disables the SRIOV function"
-
+#string STR_DPC_SUPPORT_PROMPT          #language en-US "Support DPC"
+#string STR_DPC_SUPPORT_HELP            #language en-US "This option enables / disables the DPC function"
 #string STR_PCIE_PORT_PROMPT_HELP       #language en-US "Press <Enter> to config this port."
 #string STR_PCIE_PORT_0_NULL_PROMPT     #language en-US ""
 #string STR_PCIE_PORT_0_PROMPT          #language en-US "CPU 0 Pcie - Port 0"
-- 
2.9.5

Re: [PATCH edk2-platforms v1 11/16] Hisilicon/D06: Add Setup Item "Support DPC"

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:31PM +0800, Ming Huang wrote:
> Add setup item "Support DPC" to enable or disable PCIe DPC
> (Downstream Port Containment).

This patch also seems to disable the SRIOV configuration and delete a
lot of ports. Can you explain how this is related?

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Silicon/Hisilicon/Include/Library/OemConfigData.h                   |   1 +
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr      |   2 -
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c           |   4 +
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr        | 197 +-------------------
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni |   3 +-
>  5 files changed, 10 insertions(+), 197 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Include/Library/OemConfigData.h b/Silicon/Hisilicon/Include/Library/OemConfigData.h
> index f120e3123c83..c0097d0829f0 100644
> --- a/Silicon/Hisilicon/Include/Library/OemConfigData.h
> +++ b/Silicon/Hisilicon/Include/Library/OemConfigData.h
> @@ -49,6 +49,7 @@ typedef struct {
>    UINT8         OSWdtAction;
>    /*PCIe Config*/
>    UINT8         PcieSRIOVSupport;
> +  UINT8         PcieDPCSupport;
>    UINT8         PciePort[PCIE_MAX_TOTAL_PORTS];
>    UINT8         PcieLinkSpeedPort[PCIE_MAX_TOTAL_PORTS];
>    UINT8         PcieLinkDeEmphasisPort[PCIE_MAX_TOTAL_PORTS];
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
> index 08236704fbfe..93ccb99bdc67 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
> @@ -62,11 +62,9 @@ formset
>        prompt = STRING_TOKEN(STR_IBMC_CONFIG_FORM_TITLE),
>        help   = STRING_TOKEN(STR_IBMC_CONFIG_FORM_HELP);
>  
> -    suppressif TRUE;
>      goto PCIE_CONFIG_FORM_ID,
>        prompt  = STRING_TOKEN(STR_PCIE_CONFIG_FORM_TITLE),
>        help    = STRING_TOKEN(STR_PCIE_CONFIG_FORM_HELP);
> -    endif;
>  
>      goto MISC_CONFIG_FORM_ID,
>        prompt  = STRING_TOKEN(STR_MISC_CONFIG_FORM_TITLE),
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> index 6668103af027..be4ce8820f73 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> @@ -290,6 +290,10 @@ OemConfigUiLibConstructor (
>        Configuration.OSWdtTimeout = 5;
>        Configuration.OSWdtAction = 1;
>        //
> +      //Set the default value of the PCIe option
> +      //
> +      Configuration.PcieDPCSupport = 0;
> +      //
>        //Set the default value of the Misc option
>        //
>        Configuration.EnableSmmu = 1;
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
> index 7cf7cdd29ba2..c65907fe846e 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
> @@ -17,203 +17,12 @@
>  form formid = PCIE_CONFIG_FORM_ID,
>    title   = STRING_TOKEN (STR_PCIE_CONFIG_FORM_TITLE);
>  
> -  goto VFR_FORMID_PCIE_SOCKET0,
> -    prompt  = STRING_TOKEN (STR_PCIE_CPU_0_PROMPT),
> -    help    = STRING_TOKEN (STR_PCIE_CPU_PROMPT_HELP);
> -
> -  goto VFR_FORMID_PCIE_SOCKET1,
> -    prompt  = STRING_TOKEN (STR_PCIE_CPU_1_PROMPT),
> -    help    = STRING_TOKEN (STR_PCIE_CPU_PROMPT_HELP);
> -
> -  oneof varid  = OEM_CONFIG_DATA.PcieSRIOVSupport,
> -        prompt   = STRING_TOKEN (STR_SRIOV_SUPPORT_PROMPT),
> -        help     = STRING_TOKEN (STR_SRIOV_SUPPORT_HELP),
> +  oneof varid  = OEM_CONFIG_DATA.PcieDPCSupport,
> +        prompt   = STRING_TOKEN (STR_DPC_SUPPORT_PROMPT),
> +        help     = STRING_TOKEN (STR_DPC_SUPPORT_HELP),
>          option text = STRING_TOKEN (STR_DISABLE), value = 0, flags = MANUFACTURING | DEFAULT | RESET_REQUIRED;
>          option text = STRING_TOKEN (STR_ENABLE),  value = 1, flags = RESET_REQUIRED;
>    endoneof;
>  
>  endform;
>  
> -form formid = VFR_FORMID_PCIE_SOCKET0,
> -  title = STRING_TOKEN(STR_PCIE_CPU_0_PROMPT);
> -
> -  goto VFR_FORMID_PCIE_PORT2,
> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_2_PROMPT),
> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
> -
> -  goto VFR_FORMID_PCIE_PORT4,
> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_4_PROMPT),
> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
> -
> -  goto VFR_FORMID_PCIE_PORT5,
> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_5_PROMPT),
> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
> -
> -  goto VFR_FORMID_PCIE_PORT6,
> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_6_PROMPT),
> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
> -
> -  goto VFR_FORMID_PCIE_PORT7,
> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_7_PROMPT),
> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_SOCKET1,
> -  title = STRING_TOKEN(STR_PCIE_CPU_1_PROMPT);
> -  goto VFR_FORMID_PCIE_PORT10,
> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_10_PROMPT),
> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
> -
> -  goto VFR_FORMID_PCIE_PORT12,
> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_12_PROMPT),
> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
> -
> -  goto VFR_FORMID_PCIE_PORT13,
> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_13_PROMPT),
> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT0,
> -  title = STRING_TOKEN(STR_PCIE_PORT_0_PROMPT);
> -  #undef  INDEX
> -  #define INDEX   0
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT1,
> -  title = STRING_TOKEN(STR_PCIE_PORT_1_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   1
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT2,
> -  title = STRING_TOKEN(STR_PCIE_PORT_2_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   2
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT3,
> -  title = STRING_TOKEN(STR_PCIE_PORT_3_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   3
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT4,
> -  title = STRING_TOKEN(STR_PCIE_PORT_4_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   4
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT5,
> -  title = STRING_TOKEN(STR_PCIE_PORT_5_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   5
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT6,
> -  title = STRING_TOKEN(STR_PCIE_PORT_6_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   6
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT7,
> -  title = STRING_TOKEN(STR_PCIE_PORT_7_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   7
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT8,
> -  title = STRING_TOKEN(STR_PCIE_PORT_8_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   8
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT9,
> -  title = STRING_TOKEN(STR_PCIE_PORT_9_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   9
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT10,
> -  title = STRING_TOKEN(STR_PCIE_PORT_10_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   10
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT11,
> -  title = STRING_TOKEN(STR_PCIE_PORT_11_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   11
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT12,
> -  title = STRING_TOKEN(STR_PCIE_PORT_12_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   12
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT13,
> -  title = STRING_TOKEN(STR_PCIE_PORT_13_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   13
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT14,
> -  title = STRING_TOKEN(STR_PCIE_PORT_14_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   14
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> -form formid = VFR_FORMID_PCIE_PORT15,
> -  title = STRING_TOKEN(STR_PCIE_PORT_15_PROMPT);
> -
> -  #undef  INDEX
> -  #define INDEX   15
> -  #include "PciePortConfig.hfr"
> -
> -endform;
> -
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
> index d87d30f975b8..0127ea952dee 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
> @@ -26,7 +26,8 @@
>  #string STR_PCIE_CPU_1_PROMPT           #language en-US "CPU 1 PCIE Configuration"
>  #string STR_SRIOV_SUPPORT_PROMPT        #language en-US "SRIOV"
>  #string STR_SRIOV_SUPPORT_HELP          #language en-US "This option enables / disables the SRIOV function"
> -
> +#string STR_DPC_SUPPORT_PROMPT          #language en-US "Support DPC"
> +#string STR_DPC_SUPPORT_HELP            #language en-US "This option enables / disables the DPC function"
>  #string STR_PCIE_PORT_PROMPT_HELP       #language en-US "Press <Enter> to config this port."
>  #string STR_PCIE_PORT_0_NULL_PROMPT     #language en-US ""
>  #string STR_PCIE_PORT_0_PROMPT          #language en-US "CPU 0 Pcie - Port 0"
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 11/16] Hisilicon/D06: Add Setup Item "Support DPC"

[Ming Huang]

On 2/12/2019 3:46 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:31PM +0800, Ming Huang wrote:
>> Add setup item "Support DPC" to enable or disable PCIe DPC
>> (Downstream Port Containment).
> 
> This patch also seems to disable the SRIOV configuration and delete a
> lot of ports. Can you explain how this is related?

The pcie menu is suppressed for original code as these menus are not ready,
this patch remove the suppression for pcie menu, so delete these menus for now.

Thanks.

> 
> /
>     Leif
> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Silicon/Hisilicon/Include/Library/OemConfigData.h                   |   1 +
>>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr      |   2 -
>>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c           |   4 +
>>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr        | 197 +-------------------
>>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni |   3 +-
>>  5 files changed, 10 insertions(+), 197 deletions(-)
>>
>> diff --git a/Silicon/Hisilicon/Include/Library/OemConfigData.h b/Silicon/Hisilicon/Include/Library/OemConfigData.h
>> index f120e3123c83..c0097d0829f0 100644
>> --- a/Silicon/Hisilicon/Include/Library/OemConfigData.h
>> +++ b/Silicon/Hisilicon/Include/Library/OemConfigData.h
>> @@ -49,6 +49,7 @@ typedef struct {
>>    UINT8         OSWdtAction;
>>    /*PCIe Config*/
>>    UINT8         PcieSRIOVSupport;
>> +  UINT8         PcieDPCSupport;
>>    UINT8         PciePort[PCIE_MAX_TOTAL_PORTS];
>>    UINT8         PcieLinkSpeedPort[PCIE_MAX_TOTAL_PORTS];
>>    UINT8         PcieLinkDeEmphasisPort[PCIE_MAX_TOTAL_PORTS];
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
>> index 08236704fbfe..93ccb99bdc67 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
>> @@ -62,11 +62,9 @@ formset
>>        prompt = STRING_TOKEN(STR_IBMC_CONFIG_FORM_TITLE),
>>        help   = STRING_TOKEN(STR_IBMC_CONFIG_FORM_HELP);
>>  
>> -    suppressif TRUE;
>>      goto PCIE_CONFIG_FORM_ID,
>>        prompt  = STRING_TOKEN(STR_PCIE_CONFIG_FORM_TITLE),
>>        help    = STRING_TOKEN(STR_PCIE_CONFIG_FORM_HELP);
>> -    endif;
>>  
>>      goto MISC_CONFIG_FORM_ID,
>>        prompt  = STRING_TOKEN(STR_MISC_CONFIG_FORM_TITLE),
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
>> index 6668103af027..be4ce8820f73 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
>> @@ -290,6 +290,10 @@ OemConfigUiLibConstructor (
>>        Configuration.OSWdtTimeout = 5;
>>        Configuration.OSWdtAction = 1;
>>        //
>> +      //Set the default value of the PCIe option
>> +      //
>> +      Configuration.PcieDPCSupport = 0;
>> +      //
>>        //Set the default value of the Misc option
>>        //
>>        Configuration.EnableSmmu = 1;
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
>> index 7cf7cdd29ba2..c65907fe846e 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr
>> @@ -17,203 +17,12 @@
>>  form formid = PCIE_CONFIG_FORM_ID,
>>    title   = STRING_TOKEN (STR_PCIE_CONFIG_FORM_TITLE);
>>  
>> -  goto VFR_FORMID_PCIE_SOCKET0,
>> -    prompt  = STRING_TOKEN (STR_PCIE_CPU_0_PROMPT),
>> -    help    = STRING_TOKEN (STR_PCIE_CPU_PROMPT_HELP);
>> -
>> -  goto VFR_FORMID_PCIE_SOCKET1,
>> -    prompt  = STRING_TOKEN (STR_PCIE_CPU_1_PROMPT),
>> -    help    = STRING_TOKEN (STR_PCIE_CPU_PROMPT_HELP);
>> -
>> -  oneof varid  = OEM_CONFIG_DATA.PcieSRIOVSupport,
>> -        prompt   = STRING_TOKEN (STR_SRIOV_SUPPORT_PROMPT),
>> -        help     = STRING_TOKEN (STR_SRIOV_SUPPORT_HELP),
>> +  oneof varid  = OEM_CONFIG_DATA.PcieDPCSupport,
>> +        prompt   = STRING_TOKEN (STR_DPC_SUPPORT_PROMPT),
>> +        help     = STRING_TOKEN (STR_DPC_SUPPORT_HELP),
>>          option text = STRING_TOKEN (STR_DISABLE), value = 0, flags = MANUFACTURING | DEFAULT | RESET_REQUIRED;
>>          option text = STRING_TOKEN (STR_ENABLE),  value = 1, flags = RESET_REQUIRED;
>>    endoneof;
>>  
>>  endform;
>>  
>> -form formid = VFR_FORMID_PCIE_SOCKET0,
>> -  title = STRING_TOKEN(STR_PCIE_CPU_0_PROMPT);
>> -
>> -  goto VFR_FORMID_PCIE_PORT2,
>> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_2_PROMPT),
>> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
>> -
>> -  goto VFR_FORMID_PCIE_PORT4,
>> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_4_PROMPT),
>> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
>> -
>> -  goto VFR_FORMID_PCIE_PORT5,
>> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_5_PROMPT),
>> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
>> -
>> -  goto VFR_FORMID_PCIE_PORT6,
>> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_6_PROMPT),
>> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
>> -
>> -  goto VFR_FORMID_PCIE_PORT7,
>> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_7_PROMPT),
>> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_SOCKET1,
>> -  title = STRING_TOKEN(STR_PCIE_CPU_1_PROMPT);
>> -  goto VFR_FORMID_PCIE_PORT10,
>> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_10_PROMPT),
>> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
>> -
>> -  goto VFR_FORMID_PCIE_PORT12,
>> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_12_PROMPT),
>> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
>> -
>> -  goto VFR_FORMID_PCIE_PORT13,
>> -    prompt  = STRING_TOKEN(STR_PCIE_PORT_13_PROMPT),
>> -    help    = STRING_TOKEN(STR_PCIE_PORT_PROMPT_HELP);
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT0,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_0_PROMPT);
>> -  #undef  INDEX
>> -  #define INDEX   0
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT1,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_1_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   1
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT2,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_2_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   2
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT3,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_3_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   3
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT4,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_4_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   4
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT5,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_5_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   5
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT6,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_6_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   6
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT7,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_7_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   7
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT8,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_8_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   8
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT9,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_9_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   9
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT10,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_10_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   10
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT11,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_11_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   11
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT12,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_12_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   12
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT13,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_13_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   13
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT14,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_14_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   14
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> -form formid = VFR_FORMID_PCIE_PORT15,
>> -  title = STRING_TOKEN(STR_PCIE_PORT_15_PROMPT);
>> -
>> -  #undef  INDEX
>> -  #define INDEX   15
>> -  #include "PciePortConfig.hfr"
>> -
>> -endform;
>> -
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
>> index d87d30f975b8..0127ea952dee 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni
>> @@ -26,7 +26,8 @@
>>  #string STR_PCIE_CPU_1_PROMPT           #language en-US "CPU 1 PCIE Configuration"
>>  #string STR_SRIOV_SUPPORT_PROMPT        #language en-US "SRIOV"
>>  #string STR_SRIOV_SUPPORT_HELP          #language en-US "This option enables / disables the SRIOV function"
>> -
>> +#string STR_DPC_SUPPORT_PROMPT          #language en-US "Support DPC"
>> +#string STR_DPC_SUPPORT_HELP            #language en-US "This option enables / disables the DPC function"
>>  #string STR_PCIE_PORT_PROMPT_HELP       #language en-US "Press <Enter> to config this port."
>>  #string STR_PCIE_PORT_0_NULL_PROMPT     #language en-US ""
>>  #string STR_PCIE_PORT_0_PROMPT          #language en-US "CPU 0 Pcie - Port 0"
>> -- 
>> 2.9.5
>>

Re: [PATCH edk2-platforms v1 11/16] Hisilicon/D06: Add Setup Item "Support DPC"

[Leif Lindholm]

On Tue, Feb 12, 2019 at 11:22:24PM +0800, Ming Huang wrote:
> On 2/12/2019 3:46 AM, Leif Lindholm wrote:
> > On Fri, Feb 01, 2019 at 09:34:31PM +0800, Ming Huang wrote:
> >> Add setup item "Support DPC" to enable or disable PCIe DPC
> >> (Downstream Port Containment).
> > 
> > This patch also seems to disable the SRIOV configuration and delete a
> > lot of ports. Can you explain how this is related?
> 
> The pcie menu is suppressed for original code as these menus are not ready,
> this patch remove the suppression for pcie menu, so delete these menus for now.

OK. Please update subject and commit message to reflect these
additional changes.

/
    Leif

[PATCH edk2-platforms v1 12/16] Hisilicon/D06: Use new flash layout

[Ming Huang]

In new flash layout, BIOS fd change from offset 1M to 8M in 16M
spi flash. Use the new CustomData.Fv which indicate the offset
of fd and which flash area can be updated for BMC.

This patch is relative with patch "Use new flash layout" in
edk2-non-osi.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Platform/Hisilicon/D06/D06.fdf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
index d495ad7f264c..f72b513352fb 100644
--- a/Platform/Hisilicon/D06/D06.fdf
+++ b/Platform/Hisilicon/D06/D06.fdf
@@ -29,7 +29,7 @@ [DEFINES]
 ################################################################################
 [FD.D06]
 
-BaseAddress   = 0x204100000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
+BaseAddress   = 0x204800000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
 
 Size          = 0x00400000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
 ErasePolarity = 1
@@ -124,7 +124,7 @@ [FD.D06]
 0x003E0000|0x00010000
 
 0x003F0000|0x00010000
-FILE = Platform/Hisilicon/D0x-CustomData.Fv
+FILE = Platform/Hisilicon/D06/CustomData.Fv
 
 ################################################################################
 #
-- 
2.9.5

Re: [PATCH edk2-platforms v1 12/16] Hisilicon/D06: Use new flash layout

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:32PM +0800, Ming Huang wrote:
> In new flash layout, BIOS fd change from offset 1M to 8M in 16M
> spi flash.

This bit

> Use the new CustomData.Fv which indicate the offset
> of fd and which flash area can be updated for BMC.

is of critical importance. Should be its own paragraph.

How does this change affect variable storage? Will the server maintain
state after a firmware upgrade, or will the operator need to rescue it
manually via the BMC?

/
    Leif

> 
> This patch is relative with patch "Use new flash layout" in
> edk2-non-osi.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Platform/Hisilicon/D06/D06.fdf | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index d495ad7f264c..f72b513352fb 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -29,7 +29,7 @@ [DEFINES]
>  ################################################################################
>  [FD.D06]
>  
> -BaseAddress   = 0x204100000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
> +BaseAddress   = 0x204800000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
>  
>  Size          = 0x00400000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
>  ErasePolarity = 1
> @@ -124,7 +124,7 @@ [FD.D06]
>  0x003E0000|0x00010000
>  
>  0x003F0000|0x00010000
> -FILE = Platform/Hisilicon/D0x-CustomData.Fv
> +FILE = Platform/Hisilicon/D06/CustomData.Fv
>  
>  ################################################################################
>  #
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 12/16] Hisilicon/D06: Use new flash layout

[Ming Huang]

On 2/11/2019 10:54 PM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:32PM +0800, Ming Huang wrote:
>> In new flash layout, BIOS fd change from offset 1M to 8M in 16M
>> spi flash.
> 
> This bit
> 
>> Use the new CustomData.Fv which indicate the offset
>> of fd and which flash area can be updated for BMC.
> 
> is of critical importance. Should be its own paragraph.
> 
> How does this change affect variable storage? Will the server maintain
> state after a firmware upgrade, or will the operator need to rescue it
> manually via the BMC?

As the address of variable is change, need to update via the BMC for 19.02.

Thanks

> 
> /
>     Leif
> 
>>
>> This patch is relative with patch "Use new flash layout" in
>> edk2-non-osi.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Platform/Hisilicon/D06/D06.fdf | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
>> index d495ad7f264c..f72b513352fb 100644
>> --- a/Platform/Hisilicon/D06/D06.fdf
>> +++ b/Platform/Hisilicon/D06/D06.fdf
>> @@ -29,7 +29,7 @@ [DEFINES]
>>  ################################################################################
>>  [FD.D06]
>>  
>> -BaseAddress   = 0x204100000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
>> +BaseAddress   = 0x204800000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
>>  
>>  Size          = 0x00400000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
>>  ErasePolarity = 1
>> @@ -124,7 +124,7 @@ [FD.D06]
>>  0x003E0000|0x00010000
>>  
>>  0x003F0000|0x00010000
>> -FILE = Platform/Hisilicon/D0x-CustomData.Fv
>> +FILE = Platform/Hisilicon/D06/CustomData.Fv
>>  
>>  ################################################################################
>>  #
>> -- 
>> 2.9.5
>>

[PATCH edk2-platforms v1 13/16] Hisilicon/D06: Remove SECURE_BOOT_ENABLE definition

[Ming Huang]

As secure boot is not ready, remove SECURE_BOOT_ENABLE and
relative code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Platform/Hisilicon/D06/D06.dsc | 12 ------------
 Platform/Hisilicon/D06/D06.fdf | 11 -----------
 2 files changed, 23 deletions(-)

diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
index 6d581337f199..a3a01bfb1e23 100644
--- a/Platform/Hisilicon/D06/D06.dsc
+++ b/Platform/Hisilicon/D06/D06.dsc
@@ -30,7 +30,6 @@ [Defines]
   FLASH_DEFINITION               = Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
   DEFINE NETWORK_IP6_ENABLE      = FALSE
   DEFINE HTTP_BOOT_ENABLE        = FALSE
-  DEFINE SECURE_BOOT_ENABLE      = FALSE
 
 !include Silicon/Hisilicon/Hisilicon.dsc.inc
 
@@ -87,9 +86,6 @@ [LibraryClasses.common]
   LpcLib|Silicon/Hisilicon/Hi1620/Library/LpcLibHi1620/LpcLib.inf
   SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
   OemNicLib|Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
-!endif
   PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
   PciPlatformLib|Silicon/Hisilicon/Hi1620/Library/Hi1620PciPlatformLib/Hi1620PciPlatformLib.inf
 
@@ -290,15 +286,7 @@ [Components.common]
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
   Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.inf
 
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
-    <LibraryClasses>
-      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
-  }
-  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
-!else
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
-!endif
   Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
     <LibraryClasses>
diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
index f72b513352fb..e402628a1b35 100644
--- a/Platform/Hisilicon/D06/D06.fdf
+++ b/Platform/Hisilicon/D06/D06.fdf
@@ -88,17 +88,10 @@ [FD.D06]
   #Blockmap[1]: End
   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
   ## This is the VARIABLE_STORE_HEADER
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  #Signature: gEfiAuthenticatedVariableGuid =
-  #  { 0xaaf32c78, 0x947b, 0x439a, { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
-  0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
-  0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
-!else
   #Signature: gEfiVariableGuid =
   #  { 0xddcf3616, 0x3275, 0x4164, { 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d }}
   0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
   0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
-!endif
   #Size: 0xe000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0xdFB8
   0xB8, 0xdF, 0x00, 0x00,
   #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
@@ -183,10 +176,6 @@ [FV.FvMain]
   INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
   INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
 
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
-!endif
-
   INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
   INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
   INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
-- 
2.9.5

Re: [PATCH edk2-platforms v1 13/16] Hisilicon/D06: Remove SECURE_BOOT_ENABLE definition

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:33PM +0800, Ming Huang wrote:
> As secure boot is not ready, remove SECURE_BOOT_ENABLE and
> relative code.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Hisilicon/D06/D06.dsc | 12 ------------
>  Platform/Hisilicon/D06/D06.fdf | 11 -----------
>  2 files changed, 23 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 6d581337f199..a3a01bfb1e23 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -30,7 +30,6 @@ [Defines]
>    FLASH_DEFINITION               = Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
>    DEFINE NETWORK_IP6_ENABLE      = FALSE
>    DEFINE HTTP_BOOT_ENABLE        = FALSE
> -  DEFINE SECURE_BOOT_ENABLE      = FALSE
>  
>  !include Silicon/Hisilicon/Hisilicon.dsc.inc
>  
> @@ -87,9 +86,6 @@ [LibraryClasses.common]
>    LpcLib|Silicon/Hisilicon/Hi1620/Library/LpcLibHi1620/LpcLib.inf
>    SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>    OemNicLib|Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.inf
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> -  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> -!endif
>    PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
>    PciPlatformLib|Silicon/Hisilicon/Hi1620/Library/Hi1620PciPlatformLib/Hi1620PciPlatformLib.inf
>  
> @@ -290,15 +286,7 @@ [Components.common]
>    MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>    Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.inf
>  
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> -  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> -    <LibraryClasses>
> -      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> -  }
> -  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> -!else
>    MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> -!endif
>    Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
>      <LibraryClasses>
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index f72b513352fb..e402628a1b35 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -88,17 +88,10 @@ [FD.D06]
>    #Blockmap[1]: End
>    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>    ## This is the VARIABLE_STORE_HEADER
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> -  #Signature: gEfiAuthenticatedVariableGuid =
> -  #  { 0xaaf32c78, 0x947b, 0x439a, { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
> -  0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
> -  0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
> -!else
>    #Signature: gEfiVariableGuid =
>    #  { 0xddcf3616, 0x3275, 0x4164, { 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d }}
>    0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
>    0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
> -!endif
>    #Size: 0xe000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0xdFB8
>    0xB8, 0xdF, 0x00, 0x00,
>    #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
> @@ -183,10 +176,6 @@ [FV.FvMain]
>    INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
>    INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
>  
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> -  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> -!endif
> -
>    INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
>    INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
>    INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
> -- 
> 2.9.5
> 

[PATCH edk2-platforms v1 14/16] Hisilicon/D0x: Remove SP805 watchdog pcd

[Ming Huang]

SP805 watchdog is no used for D0x, so remove it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Platform/Hisilicon/D03/D03.dsc                                       | 3 ---
 Platform/Hisilicon/D05/D05.dsc                                       | 3 ---
 Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf | 1 -
 3 files changed, 7 deletions(-)

diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
index fe443dd929ad..35b54f8c83be 100644
--- a/Platform/Hisilicon/D03/D03.dsc
+++ b/Platform/Hisilicon/D03/D03.dsc
@@ -149,9 +149,6 @@ [PcdsFixedAtBuild.common]
 
   gHisiTokenSpaceGuid.PcdPcieRootBridgeMask|0x7 # bit0:HB0RB0,bit1:HB0RB1,bit2:HB0RB2,bit3:HB0RB3,bit4:HB1RB0,bit5:HB1RB1,bit6:HB1RB2,bit7:HB1RB3
 
-  ## SP805 Watchdog - Motherboard Watchdog
-  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x601e0000
-
   ## Serial Terminal
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x2F8
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
index 0c4f21fbe056..49bd5b37ea34 100644
--- a/Platform/Hisilicon/D05/D05.dsc
+++ b/Platform/Hisilicon/D05/D05.dsc
@@ -163,9 +163,6 @@ [PcdsFixedAtBuild.common]
   gHisiTokenSpaceGuid.PcdPcieRootBridgeMask2P|0x34F4 # bit0:HB0RB0,bit1:HB0RB1,bit2:HB0RB2,bit3:HB0RB3,bit4:HB0RB4,bit5:HB0RB5,bit6:HB0RB6,bit7:HB0RB7
                                                 # bit8:HB1RB0,bit9:HB1RB1,bit10:HB1RB2,bit11:HB1RB3,bit12:HB1RB4,bit13:HB1RB5,bit14:HB1RB6,bit14:HB1RB15
 
-  ## SP805 Watchdog - Motherboard Watchdog
-  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x601e0000
-
   ## Serial Terminal
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x602B0000
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
diff --git a/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf b/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf
index 3563df6e10d1..4ce5f5fea1f3 100644
--- a/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf
+++ b/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf
@@ -61,5 +61,4 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
   gHisiTokenSpaceGuid.PcdSysControlBaseAddress
   gHisiTokenSpaceGuid.PcdPeriSubctrlAddress
-  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase
 
-- 
2.9.5

Re: [PATCH edk2-platforms v1 14/16] Hisilicon/D0x: Remove SP805 watchdog pcd

[Leif Lindholm]

On Fri, Feb 01, 2019 at 09:34:34PM +0800, Ming Huang wrote:
> SP805 watchdog is no used for D0x, so remove it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Hisilicon/D03/D03.dsc                                       | 3 ---
>  Platform/Hisilicon/D05/D05.dsc                                       | 3 ---
>  Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf | 1 -
>  3 files changed, 7 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> index fe443dd929ad..35b54f8c83be 100644
> --- a/Platform/Hisilicon/D03/D03.dsc
> +++ b/Platform/Hisilicon/D03/D03.dsc
> @@ -149,9 +149,6 @@ [PcdsFixedAtBuild.common]
>  
>    gHisiTokenSpaceGuid.PcdPcieRootBridgeMask|0x7 # bit0:HB0RB0,bit1:HB0RB1,bit2:HB0RB2,bit3:HB0RB3,bit4:HB1RB0,bit5:HB1RB1,bit6:HB1RB2,bit7:HB1RB3
>  
> -  ## SP805 Watchdog - Motherboard Watchdog
> -  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x601e0000
> -
>    ## Serial Terminal
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x2F8
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 0c4f21fbe056..49bd5b37ea34 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -163,9 +163,6 @@ [PcdsFixedAtBuild.common]
>    gHisiTokenSpaceGuid.PcdPcieRootBridgeMask2P|0x34F4 # bit0:HB0RB0,bit1:HB0RB1,bit2:HB0RB2,bit3:HB0RB3,bit4:HB0RB4,bit5:HB0RB5,bit6:HB0RB6,bit7:HB0RB7
>                                                  # bit8:HB1RB0,bit9:HB1RB1,bit10:HB1RB2,bit11:HB1RB3,bit12:HB1RB4,bit13:HB1RB5,bit14:HB1RB6,bit14:HB1RB15
>  
> -  ## SP805 Watchdog - Motherboard Watchdog
> -  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x601e0000
> -
>    ## Serial Terminal
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x602B0000
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> diff --git a/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf b/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf
> index 3563df6e10d1..4ce5f5fea1f3 100644
> --- a/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf
> +++ b/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf
> @@ -61,5 +61,4 @@ [FixedPcd]
>    gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
>    gHisiTokenSpaceGuid.PcdSysControlBaseAddress
>    gHisiTokenSpaceGuid.PcdPeriSubctrlAddress
> -  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase
>  
> -- 
> 2.9.5
> 

Re: [PATCH edk2-platforms v1 00/16] Fix issues and improve D0x

[Ming Huang]

Hi Leif,

Please have a look for several outstanding patches.

Thanks

On 2/1/2019 9:34 PM, Ming Huang wrote:
> Main Changes :
> 1 Use new flash layout;
> 2 Modify for M7 self-Adapte support;
> 3 Add PCI_OSC_SUPPORT for ACPI/DSDT;
> 4 Change HCCS speed from 30G to 26G;
> 5 Rename StartupAp() function;
> 6 Fix access variable fail issue;
> 7 Optimize SAS driver for reducing boot time;
> 8 Add DriverHealthManagerDxe;
> 9 Remove useless code;
> 
> Code can also be found in github:
> https://github.com/hisilicon/OpenPlatformPkg.git
> branch: 1902-platforms-v1
> 
> 
> Jason Zhang (2):
>   Hisilicon/D06: Fix access variable fail issue
>   Hisilicon/D06: Add more PCIe port INT-x support
> 
> Ming Huang (13):
>   Hisilicon/D0x: Remove SerdesLib
>   Hisilicon/D0x: Add DriverHealthManagerDxe
>   Hisilicon/D06: Optimize SAS driver for reducing boot time
>   Hisilicon/D0x: Rename StartupAp() function
>   Hisilicon/D06: Change HCCS speed from 30G to 26G
>   Hisilicon/D06: Add PCI_OSC_SUPPORT
>   Hisilicon/D06: Modify for M7 self-Adapte support
>   Hisilicon/D06: Add Setup Item "Support DPC"
>   Hisilicon/D06: Use new flash layout
>   Hisilicon/D06: Remove SECURE_BOOT_ENABLE definition
>   Hisilicon/D0x: Remove SP805 watchdog pcd
>   Hisilicon/D06: Use CalculateCrc16 in BaseLib
>   Hisilicon/D0x: Modify version to 19.02
> 
> xingjiang tang (1):
>   Hisilicon/D06: Add OemGetCpuFreq to encapsulate difference
> 
>  Platform/Hisilicon/D03/D03.dsc                                       |   8 +-
>  Platform/Hisilicon/D05/D05.dsc                                       |   8 +-
>  Platform/Hisilicon/D06/D06.dsc                                       |  19 +-
>  Platform/Hisilicon/D03/D03.fdf                                       |   1 +
>  Platform/Hisilicon/D05/D05.fdf                                       |   1 +
>  Platform/Hisilicon/D06/D06.fdf                                       |  18 +-
>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf     |   1 -
>  Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf |   1 -
>  Platform/Hisilicon/D06/Include/Library/CpldD06.h                     |   4 +
>  Silicon/Hisilicon/Include/Library/OemConfigData.h                    |   1 +
>  Silicon/Hisilicon/Include/Library/OemMiscLib.h                       |  12 +
>  Silicon/Hisilicon/Include/Library/PlatformSysCtrlLib.h               |   2 +-
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr       |   4 +-
>  Platform/Hisilicon/D03/EarlyConfigPeim/EarlyConfigPeimD03.c          |   2 +-
>  Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.c     |   2 +-
>  Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.c          |   2 +-
>  Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.c         |   3 +-
>  Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c          |   2 +-
>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c         |  24 ++
>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c                 | 341 +++-----------------
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c            |   6 +-
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl         | 129 +++++++-
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr         | 197 +----------
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni  |   3 +-
>  24 files changed, 238 insertions(+), 553 deletions(-)
>