Re: [edk2-devel] [edk2-platforms][PATCH V3 4/5] Platform/Sgi: Initialize additional UART controllers

Wed, 26 Apr 2023 22:28:39 -0700 

Hi Sami,


On 4/26/23 21:47, Sami Mujawar wrote:

Hi Vivek,

Thank you for this patch.

I have some minor feedback that I will address before merging.

Reviewed-by: Sami Mujawar <sami.muja...@arm.com>
Thank you for the review of this patch and other patches in the series. Please find my comments inline below. 



Regards,

Sami Mujawar

On 24/03/2023 11:03 am, Vivek Gautam wrote:

From: Shriram K <shrira...@arm.com>

The IO virtualization block on reference design platforms allow
connecting SoC expansion devices such as PL011 UART. On platforms
that support this, initialize the UART controller connected to the
IO virtualization block.

Signed-off-by: Shriram K <shrira...@arm.com>
Signed-off-by: Vivek Gautam <vivek.gau...@arm.com>
---
  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf  |  9 ++-
  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf  |  6 +-
  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c    | 64 +++++++++++++++++++-   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c | 44 +++++++++++++- 
  4 files changed, 116 insertions(+), 7 deletions(-)
diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf 
index 9d89314a594e..3cd7e2329c22 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
@@ -1,5 +1,5 @@
  #
-#  Copyright (c) 2018, ARM Limited. All rights reserved.
+#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
  #
  #  SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -17,6 +17,7 @@
    VirtioDevices.c
  [Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
    EmbeddedPkg/EmbeddedPkg.dec
    MdePkg/MdePkg.dec
    OvmfPkg/OvmfPkg.dec
@@ -37,10 +38,16 @@
    gArmSgiTokenSpaceGuid.PcdVirtioNetSupported
  [FixedPcd]
+  gArmSgiTokenSpaceGuid.PcdChipCount
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
+  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
    gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
    gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
    gArmSgiTokenSpaceGuid.PcdVirtioNetBaseAddress
    gArmSgiTokenSpaceGuid.PcdVirtioNetSize
+  gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz
+
  [Depex]
    TRUE
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf 
index 1ca7679b4191..020bde0d1f56 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
@@ -1,5 +1,5 @@
  #
-#  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.
+#  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
  #
  #  SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -41,10 +41,12 @@
    gArmPlatformTokenSpaceGuid.PcdCoreCount
    gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase
-  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
    gArmSgiTokenSpaceGuid.PcdDramBlock2Base
    gArmSgiTokenSpaceGuid.PcdDramBlock2Size
    gArmSgiTokenSpaceGuid.PcdGicSize
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
+  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
    gArmTokenSpaceGuid.PcdSystemMemoryBase
    gArmTokenSpaceGuid.PcdSystemMemorySize
diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c 
index 2f72e7152ff3..b3a998bc1585 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
@@ -1,6 +1,6 @@
  /** @file
  *
-*  Copyright (c) 2018, ARM Limited. All rights reserved.
+*  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -9,6 +9,9 @@
  #include <Library/AcpiLib.h>
  #include <Library/DebugLib.h>
  #include <Library/HobLib.h>
+#include <Library/PL011UartLib.h>
+
+#include <IoVirtSoCExp.h>
  #include <SgiPlatform.h>
  VOID
@@ -16,6 +19,64 @@ InitVirtioDevices (
    VOID
    );
+/**
+  Initialize UART controllers connected to IO Virtualization block.
+
+  Use PL011UartLib Library to initialize UART controllers that are present in +  the SoC expansion block. This SoC expansion block is connected to the IO +  virtualization block on Arm infrastructure reference design (RD) platforms. 
+
+  @retval  None
+**/
+STATIC
+VOID
+InitIoVirtSocExpBlkUartControllers (VOID)
+{
+  EFI_STATUS                 Status;
+  EFI_PARITY_TYPE            Parity;
+  EFI_STOP_BITS_TYPE         StopBits;
+  UINT64                     BaudRate;
+  UINT32                     ReceiveFifoDepth;
+  UINT8                      DataBits;
+  UINT8                      UartIdx;
+  UINT32                     ChipIdx;
+  UINT64                     UartAddr;
+
+  if (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) == 0)
+    return;
[SAMI] edk2 coding standard requires to use opening and closing curly brackets even when the if condition has a single enclosing statement.
Also the function parameters must be on separate lines. However, I will address that locally before merging.
I will keep these things in mind for future. Thanks for taking care of these in this patch. 

Best regards
Vivek



[/SAMI]



[snip]


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103692): https://edk2.groups.io/g/devel/message/103692
Mute This Topic: https://groups.io/mt/97821142/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to