| Message ID | 20180724070922.63362-28-ming.huang@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Upload for D06 platform | expand |
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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
在 8/4/2018 5:59 PM, Leif Lindholm 写道: > 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. MN: Miscellaneous Node > > 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'. Agree, change it to 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 :) Maybe our comment is not accuracy, we want the PlatformArch.h should be in front of the below Library/*.h, not in top. > >> +#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). Sorry, I don't really understand here. This function is in .c file already. > >> + VOID >> + ) >> +{ >> + MmioWrite64 (FixedPcdGet64 (PcdMailBoxAddress), 0x0); >> + (void)WriteBackInvalidateDataCacheRange ( > > VOID > >> + (VOID *)FixedPcdGet64 (PcdMailBoxAddress), >> + 8 > > sizeof (UINT64)? Yes > >> + ); >> + >> + //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. OK > >> + } >> +} >> + >> + >> +EFI_STATUS >> +EFIAPI >> +EarlyConfigEntry ( > > No definitions in .h files. Move to .c file and make STATIC. Do you suggest add STATIC here? > >> + 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. OK, all comments will apply in v2. > > / > Leif > >> + >> +[Depex] >> +## As we will clean mailbox in this module, need to wait memory init complete >> + gEfiPeiMemoryDiscoveredPpiGuid >> -- >> 2.17.0 >>
On Thu, Aug 09, 2018 at 03:07:07PM +0800, Ming wrote: > 在 8/4/2018 5:59 PM, Leif Lindholm 写道: > > 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. > > MN: Miscellaneous Node > > > > > 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'. > > Agree, change it to BSP. Does that mean MN becomes AP? :) > >> 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 :) > > Maybe our comment is not accuracy, we want the PlatformArch.h should > be in front of the below Library/*.h, not in top. Why? I am happy for .h files to include other .h files that they need for their internal definitions - so that we don't need to worry about functional needs for include order. I would prefer for this to be resolved so that the comment is simply not needed. > > > >> +#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). > > Sorry, I don't really understand here. This function is in .c > file already. Ah, so it is. My apologies. In that case, please move these #defines to a .h file :) > > > > >> + VOID > >> + ) > >> +{ > >> + MmioWrite64 (FixedPcdGet64 (PcdMailBoxAddress), 0x0); > >> + (void)WriteBackInvalidateDataCacheRange ( > > > > VOID > > > >> + (VOID *)FixedPcdGet64 (PcdMailBoxAddress), > >> + 8 > > > > sizeof (UINT64)? > > Yes > > > > >> + ); > >> + > >> + //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. > > OK > > > > >> + } > >> +} > >> + > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +EarlyConfigEntry ( > > > > No definitions in .h files. Move to .c file and make STATIC. > > Do you suggest add STATIC here? If it is only used in this file, yes please. / Leif
在 8/9/2018 6:27 PM, Leif Lindholm 写道: > On Thu, Aug 09, 2018 at 03:07:07PM +0800, Ming wrote: >> 在 8/4/2018 5:59 PM, Leif Lindholm 写道: >>> 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. >> >> MN: Miscellaneous Node >> >>> >>> 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'. >> >> Agree, change it to BSP. > > Does that mean MN becomes AP? :) MN is not a processor. > >>>> 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 :) >> >> Maybe our comment is not accuracy, we want the PlatformArch.h should >> be in front of the below Library/*.h, not in top. > > Why? > I am happy for .h files to include other .h files that they need for > their internal definitions - so that we don't need to worry about > functional needs for include order. > > I would prefer for this to be resolved so that the comment is simply > not needed. As your suggestion, after adding PlatformArch.h to OemAddressMapLib.h, this line can be delete. > >>> >>>> +#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). >> >> Sorry, I don't really understand here. This function is in .c >> file already. > > Ah, so it is. My apologies. > In that case, please move these #defines to a .h file :) OK > >> >>> >>>> + VOID >>>> + ) >>>> +{ >>>> + MmioWrite64 (FixedPcdGet64 (PcdMailBoxAddress), 0x0); >>>> + (void)WriteBackInvalidateDataCacheRange ( >>> >>> VOID >>> >>>> + (VOID *)FixedPcdGet64 (PcdMailBoxAddress), >>>> + 8 >>> >>> sizeof (UINT64)? >> >> Yes >> >>> >>>> + ); >>>> + >>>> + //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. >> >> OK >> >>> >>>> + } >>>> +} >>>> + >>>> + >>>> +EFI_STATUS >>>> +EFIAPI >>>> +EarlyConfigEntry ( >>> >>> No definitions in .h files. Move to .c file and make STATIC. >> >> Do you suggest add STATIC here? > > If it is only used in this file, yes please.> > / > Leif >
在 8/9/2018 6:27 PM, Leif Lindholm 写道: > On Thu, Aug 09, 2018 at 03:07:07PM +0800, Ming wrote: >> 在 8/4/2018 5:59 PM, Leif Lindholm 写道: >>> 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. >> >> MN: Miscellaneous Node >> >>> >>> 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'. >> >> Agree, change it to BSP. > > Does that mean MN becomes AP? :) > >>>> 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 :) >> >> Maybe our comment is not accuracy, we want the PlatformArch.h should >> be in front of the below Library/*.h, not in top. > > Why? > I am happy for .h files to include other .h files that they need for > their internal definitions - so that we don't need to worry about > functional needs for include order. > > I would prefer for this to be resolved so that the comment is simply > not needed. > >>> >>>> +#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). >> >> Sorry, I don't really understand here. This function is in .c >> file already. > > Ah, so it is. My apologies. > In that case, please move these #defines to a .h file :) > >> >>> >>>> + VOID >>>> + ) >>>> +{ >>>> + MmioWrite64 (FixedPcdGet64 (PcdMailBoxAddress), 0x0); >>>> + (void)WriteBackInvalidateDataCacheRange ( >>> >>> VOID >>> >>>> + (VOID *)FixedPcdGet64 (PcdMailBoxAddress), >>>> + 8 >>> >>> sizeof (UINT64)? >> >> Yes >> >>> >>>> + ); >>>> + >>>> + //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. >> As this function is use in several platforms, if I rename this function, D03/D05 should be rename in edk2-platforms and many functions in HwPkg should be rename. How about use the original name? >> >>> >>>> + } >>>> +} >>>> + >>>> + >>>> +EFI_STATUS >>>> +EFIAPI >>>> +EarlyConfigEntry ( >>> >>> No definitions in .h files. Move to .c file and make STATIC. >> >> Do you suggest add STATIC here? > > If it is only used in this file, yes please. > > / > Leif >
On Tue, Aug 14, 2018 at 10:31:02AM +0800, Ming wrote: > >>> 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. > >> > > As this function is use in several platforms, if I rename this function, > D03/D05 should be rename in edk2-platforms and many functions in HwPkg > should be rename. > How about use the original name? I would prefer it changed. But it is not required for 18.08. We can discuss after that release. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 +#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 ( + VOID + ) +{ + MmioWrite64 (FixedPcdGet64 (PcdMailBoxAddress), 0x0); + (void)WriteBackInvalidateDataCacheRange ( + (VOID *)FixedPcdGet64 (PcdMailBoxAddress), + 8 + ); + + //SCCL A + if (!PcdGet64 (PcdTrustedFirmwareEnable)) + { + StartupAp (); + } +} + + +EFI_STATUS +EFIAPI +EarlyConfigEntry ( + 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 + +[Depex] +## As we will clean mailbox in this module, need to wait memory init complete + gEfiPeiMemoryDiscoveredPpiGuid