在 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.hu...@linaro.org>
>>>> Signed-off-by: Heyi Guo <heyi....@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
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
