On Wed, Jun 10, 2020 at 03:47:26 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.k...@nxp.com>
>
> Add PlatformDxe to do platform specific work.
> At present it perform platform specific Pci initialization.
> Signed-off-by: Wasim Khan <wasim.k...@nxp.com>
> ---
> Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf | 35 +++++++++
> Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c | 78
> ++++++++++++++++++++
> 2 files changed, 113 insertions(+)
>
> diff --git a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> new file mode 100644
> index 000000000000..2514adf1d69d
> --- /dev/null
> +++ b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -0,0 +1,35 @@
> +## @file
> +#
> +# Copyright 2020 NXP
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010019
> + BASE_NAME = PlatformDxe
> + FILE_GUID = C4063EBA-7729-11EA-BC55-0232AC130003
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = PlatformDxeEntryPoint
> +
> +[Sources]
> + PlatformDxe.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + Silicon/NXP/Chassis3V2/Chassis3V2.dec
> + Silicon/NXP/LX2160A/LX2160A.dec
> + Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> + PcdLib
> + UefiDriverEntryPoint
> +
> +[Pcd]
> + gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable
> + gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl
> +
> +[Depex]
> + TRUE
> diff --git a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> new file mode 100644
> index 000000000000..73599aaeb7bf
> --- /dev/null
> +++ b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -0,0 +1,78 @@
> +/** @file
> +*
> +* Copyright 2020 NXP
> +*
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +#include <Library/PcdLib.h>
> +#include <Library/SocLib.h>
> +#include <Soc.h>
> +
> +/**
> + Enable PciCfgShift feature for LX2160-Rev2
... which means what?
> +
> +**/
> +VOID
> +EnableCfgShift (
But even more importantly, that isn't what this function does:
> + VOID
> + )
> +{
> + UINT32 Svr;
> +
> + Svr = SocGetSvr ();
> + if ((SVR_SOC_VER(Svr) == SVR_LX2160A) && (SVR_MAJOR(Svr) == 0x2)) {
... it performs variant/version detection and sets feature flags based
on it. The fact that it currently happens to be setting only a single
flag shouldn't mean that in order to understand what PlatformPciInit()
does even at a high level, I need to know what an "EnableCfgShift" is.
The function should be called something like DetectSocVersion,
EnableSocRevisionWorkarounds, or something other descriptive.
*Then* there should be a comment here describing what
PcdPciCfgShiftEnable is and what it does.
> + PcdSetBoolS (PcdPciCfgShiftEnable, TRUE);
> + }
> +}
> +
> +/**
> + Enable Layerscape Gen4 controller for LX2160A-Rev1
> +
> +**/
> +VOID
> +EnablePciController (
> + VOID
> + )
> +{
> + UINT32 Svr;
> +
> + Svr = SocGetSvr ();
> + if ((SVR_SOC_VER(Svr) == SVR_LX2160A) && (SVR_MAJOR(Svr) == 0x1)) {
> + PcdSetBoolS (PcdPciLsGen4Ctrl, TRUE);
Again, this doesn't *enable* a controller, which is what the function
name says it does. It sets a Pcd, which is then used by subsequent
code. I don't know if this function should be smashed together with
the previous one under an even more generic name, but this too
motivates a comment on what a PcdPciLsGen4Ctrl is.
> + }
> +}
> +
> +/**
> + Platfrom Specific PCI Initialization
Platform
/
Leif
> +
> +**/
> +VOID
> +PlatformPciInit (
> + VOID
> + )
> +{
> + EnableCfgShift ();
> + EnablePciController ();
> +}
> +
> +/**
> + The entry point for PlatformDxe driver. This driver
> + intends to perform platform specific initialization.
> +
> + @param[in] ImageHandle The image handle of the driver.
> + @param[in] SystemTable The system table.
> +
> + @retval EFI_SUCCESS Driver initialization success.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +PlatformDxeEntryPoint (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + // Platfrom Specific PCI Initialization
> + PlatformPciInit ();
> + return EFI_SUCCESS;
> +}
> --
> 2.7.4
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61482): https://edk2.groups.io/g/devel/message/61482
Mute This Topic: https://groups.io/mt/74793010/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
