On Thu, Jul 07, 2016 at 05:24:54PM +0100, Ryan Harkin wrote:
> On 7 July 2016 at 17:01, Ryan Harkin <[email protected]> wrote:
> > On 7 July 2016 at 16:59, Leif Lindholm <[email protected]> wrote:
> >> On Fri, Jul 01, 2016 at 12:52:06PM +0100, [email protected] wrote:
> >>> From: Evan Lloyd <[email protected]>
> >>>
> >>> Juno has several serial ports, one of which may be used for a remote
> >>> debug interface (e.g. gdb, WinDbg). The debug serial port needs to
> >>> be distinct from that used for UEFI trace to prevent corruption of
> >>> debugger protocol messaging.
> >>> The UEFI spec requires that serial devices be initialised to default
> >>> settings. (11.8 Serial I/O Protocol - "The default attributes for all
> >>> UART-style serial device interfaces are: 115,200 baud, ..."
> >>> and 17.3.3 EFI Debugport Variable - "These defaults must be used in the
> >>> absence of a DEBUGPORT variable...")
> >>>
> >>> This change adds initialization of the serial device reported in the
> >>> ACPI DBG2 table. The initialisation is done early in the boot to allow
> >>> the possibility of remote debug of UEFI itself.
> >>>
> >>> NOTE: This is functionally dependent on the DBG2 table being updated in
> >>> OpenPlatformPkg, but is required as a precursor to that change.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Sami Mujawar <[email protected]>
> >>> Signed-off-by: Evan Lloyd <[email protected]>
> >>
> >> I'd like a nod from Ryan on this one.
> >
> > As it happens, I've just finished testing it (on R0, R1 and R2):
> >
> > Tested-by: Ryan Harkin <[email protected]>
> >
> >
> >> One style comment below.
> >>
> >>> ---
> >>>
> >>> Notes:
> >>> v2
> >>> - No code change.
> >>> - Clarified commit message in response to comments [Ard Bieshevel]
> >>> Specific points include clarity that change is not OS specific,
> >>> and pertains to any debugger use, and justification for doing it
> >>> at an early point in the process.
> >>> Confirmed with Leif that update here (not OpenPlatformPkg) is
> >>> appropriate.
> >>>
> >>> ArmPlatformPkg/ArmPlatformPkg.dec | 5 +++
> >>> ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf | 15
> >>> ++++++++-
> >>> ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJuno.c | 35
> >>> ++++++++++++++++++--
> >>> 3 files changed, 52 insertions(+), 3 deletions(-)
> >>>
> >>> Code available at:
> >>> https://github.com/EvanLloyd/tianocore/tree/debug_serial_v2
> >>>
> >>> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> >>> b/ArmPlatformPkg/ArmPlatformPkg.dec
> >>> index
> >>> 1c05132cd625be75b0d9d1cd09950af983d1f049..5864d9835989382b79b1e6db6486fe67082df6a0
> >>> 100644
> >>> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> >>> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> >>> @@ -98,6 +98,11 @@ [PcdsFixedAtBuild.common]
> >>> gArmPlatformTokenSpaceGuid.PL011UartFractional|0|UINT32|0x0000002D
> >>>
> >>> gArmPlatformTokenSpaceGuid.PL011UartInterrupt|0x00000000|UINT32|0x0000002F
> >>>
> >>> + ## PL011 Serial Debug UART
> >>> +
> >>> gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x00000000|UINT64|0x00000030
> >>> +
> >>> gArmPlatformTokenSpaceGuid.PcdSerialDbgUartBaudRate|0x00000000|UINT64|0x00000031
> >>> +
> >>> gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz|0x00000000|UINT32|0x00000032
> >>> +
> >>> ## PL061 GPIO
> >>> gArmPlatformTokenSpaceGuid.PcdPL061GpioBase|0x0|UINT32|0x00000025
> >>>
> >>> diff --git a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> >>> b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> >>> index
> >>> 8c8b141c35a2693b618c1515cb665b9b1e81cc4e..77d18f855a833fff0d38518436ef331dfab2d7e1
> >>> 100644
> >>> --- a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> >>> +++ b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> >>> @@ -1,5 +1,5 @@
> >>> #
> >>> -# Copyright (c) 2013-2015, ARM Limited. All rights reserved.
> >>> +# Copyright (c) 2013-2016, ARM Limited. All rights reserved.
> >>> #
> >>> # This program and the accompanying materials
> >>> # are licensed and made available under the terms and conditions of the
> >>> BSD License
> >>> @@ -57,6 +57,19 @@ [FixedPcd]
> >>> gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> >>> gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
> >>>
> >>> +
> >>> + #
> >>> + # PL011 Serial Debug UART
> >>> + #
> >>> + gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
> >>> + gArmPlatformTokenSpaceGuid.PcdSerialDbgUartBaudRate
> >>> + gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz
> >>> +
> >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
> >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
> >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
> >>> +
> >>> [Pcd]
> >>> gArmTokenSpaceGuid.PcdPciMmio32Base
> >>> gArmTokenSpaceGuid.PcdPciMmio32Size
> >>> diff --git a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJuno.c
> >>> b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJuno.c
> >>> index
> >>> ffff5628247f61f4c2f31d3b9f7763b24f10641d..ae7f5e94acdaa5a37ba56c27fe0700c588dafcc9
> >>> 100644
> >>> --- a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJuno.c
> >>> +++ b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJuno.c
> >>> @@ -1,6 +1,6 @@
> >>> /** @file
> >>> *
> >>> -* Copyright (c) 2013-2014, ARM Limited. All rights reserved.
> >>> +* Copyright (c) 2013-2016, ARM Limited. All rights reserved.
> >>> *
> >>> * This program and the accompanying materials
> >>> * are licensed and made available under the terms and conditions of the
> >>> BSD License
> >>> @@ -18,6 +18,7 @@
> >>> #include <Library/PcdLib.h>
> >>>
> >>> #include <Ppi/ArmMpCoreInfo.h>
> >>> +#include <Drivers/PL011Uart.h>
> >>
> >> Is there an ordering requirement for the include file?
> >> If not, I would prefer it before the Library includes in order to keep
> >> the sort order.
> >>
> >> I can do this before pushing if someone tells me it's OK.
> >>
>
> I moved the new include to the top of the list and it built fine from
> clean, so go for it.
Thanks Ryan - and Evan.
Pushed.
> >>>
> >>> #include <ArmPlatform.h>
> >>>
> >>> @@ -112,7 +113,37 @@ ArmPlatformInitialize (
> >>> IN UINTN MpId
> >>> )
> >>> {
> >>> - return RETURN_SUCCESS;
> >>> + RETURN_STATUS Status;
> >>> + UINT64 BaudRate;
> >>> + UINT32 ReceiveFifoDepth;
> >>> + EFI_PARITY_TYPE Parity;
> >>> + UINT8 DataBits;
> >>> + EFI_STOP_BITS_TYPE StopBits;
> >>> +
> >>> + Status = RETURN_SUCCESS;
> >>> +
> >>> + //
> >>> + // Initialize the Serial Debug UART
> >>> + //
> >>> + if (FixedPcdGet64 (PcdSerialDbgRegisterBase)) {
> >>> + ReceiveFifoDepth = 0; // Use the default value for FIFO depth
> >>> + Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
> >>> + DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
> >>> + StopBits = (EFI_STOP_BITS_TYPE)FixedPcdGet8 (PcdUartDefaultStopBits);
> >>> +
> >>> + BaudRate = (UINTN)FixedPcdGet64 (PcdSerialDbgUartBaudRate);
> >>> + Status = PL011UartInitializePort (
> >>> + (UINTN)FixedPcdGet64 (PcdSerialDbgRegisterBase),
> >>> + FixedPcdGet32 (PcdSerialDbgUartClkInHz),
> >>> + &BaudRate,
> >>> + &ReceiveFifoDepth,
> >>> + &Parity,
> >>> + &DataBits,
> >>> + &StopBits
> >>> + );
> >>> + }
> >>> +
> >>> + return Status;
> >>> }
> >>>
> >>> /**
> >>> --
> >>> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >>>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
