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