Re: [PATCH edk2-platforms v1 27/38] Platform/Hisilicon/D06: Add EarlyConfigPeim peim

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

On Tue, Jul 24, 2018 at 03:09:11PM +0800, Ming Huang wrote:
> This peim configuare SMMU,AP,MN.

configuare -> configure

I know SMMU and AP, but I don't know MN.

Hmm, also, '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.

So I think in a TianoCore context, this should be 'BSP'. 

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> ---
>  Platform/Hisilicon/D06/D06.dsc                                |   1 +
>  Platform/Hisilicon/D06/D06.fdf                                |   1 +
>  Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c   | 108 ++++++++++++++++++++
>  Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.inf |  50 +++++++++
>  4 files changed, 160 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 49322f8304..9e4f961116 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -267,6 +267,7 @@
>    MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
>    MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>  
> +  Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.inf
>    Silicon/Hisilicon/Drivers/VersionInfoPeim/VersionInfoPeim.inf
>  
>    MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index e65dddd4e9..ec424d49ed 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -359,6 +359,7 @@ READ_LOCK_STATUS   = TRUE
>    INF ArmPkg/Drivers/CpuPei/CpuPei.inf
>    INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>    INF IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
> +  INF Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.inf
>  
>    INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>  
> diff --git a/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c b/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c
> new file mode 100644
> index 0000000000..606cdf926a
> --- /dev/null
> +++ b/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.c
> @@ -0,0 +1,108 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD License
> +*  which accompanies this distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +
> +#include <Uefi.h>
> +#include <PlatformArch.h> // This header file should be on ahead

Then please move it down instead of leaving a comment :)

> +#include <Library/ArmLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/OemAddressMapLib.h>
> +#include <Library/OemMiscLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PlatformSysCtrlLib.h>
> +#include <PiPei.h>
> +
> +#define PERI_SUBCTRL_BASE                               (0x40000000)
> +#define MDIO_SUBCTRL_BASE                               (0x60000000)
> +#define PCIE2_SUBCTRL_BASE                              (0xA0000000)
> +#define PCIE0_SUBCTRL_BASE                              (0xB0000000)
> +#define ALG_BASE                                        (0xD0000000)
> +
> +#define SC_BROADCAST_EN_REG                             (0x16220)
> +#define SC_BROADCAST_SCL1_ADDR0_REG                     (0x16230)
> +#define SC_BROADCAST_SCL1_ADDR1_REG                     (0x16234)
> +#define SC_BROADCAST_SCL2_ADDR0_REG                     (0x16238)
> +#define SC_BROADCAST_SCL2_ADDR1_REG                     (0x1623C)
> +#define SC_BROADCAST_SCL3_ADDR0_REG                     (0x16240)
> +#define SC_BROADCAST_SCL3_ADDR1_REG                     (0x16244)
> +#define PCIE_SUBCTRL_SC_DISP_DAW_EN_REG                 (0x1000)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY3_REG         (0x1010)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY4_REG         (0x1014)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY5_REG         (0x1018)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY6_REG         (0x101C)
> +#define PCIE_SUBCTRL_SC_REMAP_CTRL_REG                  (0x1200)
> +#define SC_ITS_M3_INT_MUX_SEL_REG                       (0x21F0)
> +#define SC_TM_CLKEN0_REG                                (0x2050)
> +
> +#define SC_TM_CLKEN0_REG_VALUE                          (0x3)
> +#define SC_BROADCAST_EN_REG_VALUE                       (0x7)
> +#define SC_BROADCAST_SCLx_ADDRx_REG_VALUE0              (0x0)
> +#define SC_BROADCAST_SCLx_ADDRx_REG_VALUE1              (0x40016260)
> +#define SC_BROADCAST_SCLx_ADDRx_REG_VALUE2              (0x60016260)
> +#define SC_BROADCAST_SCLx_ADDRx_REG_VALUE3              (0x400)
> +#define SC_ITS_M3_INT_MUX_SEL_REG_VALUE                 (0x7)
> +#define PCIE_SUBCTRL_SC_REMAP_CTRL_REG_VALUE0           (0x0)
> +#define PCIE_SUBCTRL_SC_DISP_DAW_EN_REG_VALUE0          (0x27)
> +#define PCIE_SUBCTRL_SC_DISP_DAW_EN_REG_VALUE1          (0x2F)
> +#define PCIE_SUBCTRL_SC_DISP_DAW_EN_REG_VALUE2          (0x77)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY3_REG_VALUE0  (0x178033)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY4_REG_VALUE0  (0x17003c)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY5_REG_VALUE0  (0x15003d)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY5_REG_VALUE1  (0x170035)
> +#define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY6_REG_VALUE0  (0x16003e)
> +
> +VOID
> +QResetAp (

Hmm. No function definitions in header files please.
Move to .c file (and make STATIC).

> +  VOID
> +  )
> +{
> +  MmioWrite64 (FixedPcdGet64 (PcdMailBoxAddress), 0x0);
> +  (void)WriteBackInvalidateDataCacheRange (

VOID

> +          (VOID *)FixedPcdGet64 (PcdMailBoxAddress),
> +          8

sizeof (UINT64)?

> +          );
> +
> +  //SCCL A
> +  if (!PcdGet64 (PcdTrustedFirmwareEnable))
> +  {

That { goes at the end of the previous line.

> +    StartupAp ();

Hmm. At some point, we want to rename that function, to align with my
comments above.

> +  }
> +}
> +
> +
> +EFI_STATUS
> +EFIAPI
> +EarlyConfigEntry (

No definitions in .h files. Move to .c file and make STATIC.

> +  IN       EFI_PEI_FILE_HANDLE  FileHandle,
> +  IN CONST EFI_PEI_SERVICES     **PeiServices
> +  )
> +{
> +  DEBUG ((DEBUG_INFO,"SMMU CONFIG........."));
> +  (VOID)SmmuConfigForBios ();
> +  DEBUG ((DEBUG_INFO,"Done\n"));
> +
> +  DEBUG ((DEBUG_INFO,"AP CONFIG........."));
> +  (VOID)QResetAp ();
> +  DEBUG ((DEBUG_INFO,"Done\n"));
> +
> +  DEBUG ((DEBUG_INFO,"MN CONFIG........."));
> +  (VOID)MN_CONFIG ();
> +  DEBUG ((DEBUG_INFO,"Done\n"));
> +
> +  return EFI_SUCCESS;
> +}
> +
> diff --git a/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.inf b/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.inf
> new file mode 100644
> index 0000000000..58ee5537c2
> --- /dev/null
> +++ b/Platform/Hisilicon/D06/EarlyConfigPeim/EarlyConfigPeimD06.inf
> @@ -0,0 +1,50 @@
> +#/** @file
> +#
> +#    Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +#    Copyright (c) 2017, Linaro Limited. All rights reserved.
> +#
> +#    This program and the accompanying materials
> +#    are licensed and made available under the terms and conditions of the BSD License
> +#    which accompanies this distribution. The full text of the license may be found at
> +#    http://opensource.org/licenses/bsd-license.php
> +#
> +#    THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#    WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#**/
> +
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = EarlyConfigPeimD06
> +  FILE_GUID                      = FB8C65EB-0199-40C3-A82B-029921A9E9B3
> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = EarlyConfigEntry
> +
> +[Sources.common]
> +  EarlyConfigPeimD06.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/Hisilicon/HisiPkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  CacheMaintenanceLib
> +  DebugLib
> +  IoLib
> +  PcdLib
> +  PeimEntryPoint
> +  PlatformSysCtrlLib
> +
> +[Pcd]
> +  gHisiTokenSpaceGuid.PcdMailBoxAddress
> +  gHisiTokenSpaceGuid.PcdTrustedFirmwareEnable
> +  gHisiTokenSpaceGuid.PcdPeriSubctrlAddress

Swap previous two lines around.

/
    Leif

> +
> +[Depex]
> +## As we will clean mailbox in this module, need to wait memory init complete
> +  gEfiPeiMemoryDiscoveredPpiGuid
> -- 
> 2.17.0
>