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. > >> >> #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
