[edk2-devel] [PATCH edk2-platforms 3/7] Platform/NXP: LX2160aRdbPkg: Add PlatformDxe driver

Wasim Khan posted 7 patches 5 years, 8 months ago
There is a newer version of this series
[edk2-devel] [PATCH edk2-platforms 3/7] Platform/NXP: LX2160aRdbPkg: Add PlatformDxe driver
Posted by Wasim Khan 5 years, 8 months ago
From: Wasim Khan <wasim.khan@nxp.com>

Add PlatformDxe to do platform specific work.
At present it perform platform specific Pci initialization.

Signed-off-by: Wasim Khan <wasim.khan@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
+
+**/
+VOID
+EnableCfgShift (
+  VOID
+  )
+{
+  UINT32 Svr;
+
+  Svr = SocGetSvr ();
+  if ((SVR_SOC_VER(Svr) == SVR_LX2160A) && (SVR_MAJOR(Svr) == 0x2)) {
+    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);
+  }
+}
+
+/**
+  Platfrom Specific PCI Initialization
+
+**/
+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 (#61053): https://edk2.groups.io/g/devel/message/61053
Mute This Topic: https://groups.io/mt/74793010/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms 3/7] Platform/NXP: LX2160aRdbPkg: Add PlatformDxe driver
Posted by Leif Lindholm 5 years, 7 months ago
On Wed, Jun 10, 2020 at 03:47:26 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> Add PlatformDxe to do platform specific work.
> At present it perform platform specific Pci initialization.
 
> Signed-off-by: Wasim Khan <wasim.khan@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-