On 20 April 2018 at 16:14, Alexei Fedorov <[email protected]> wrote: > Leif, Ard > > > When do you plan to push this patch? >
I don't. There was some pushback from the Intel guys regarding the use of status codes instead of a runtime debug protocol, and nobody came forward to say that they wanted this. If you are interested in this functionality, please respond to the 0/4 cover letter and explain why this approach is preferred over status codes. Thanks, Ard. > ________________________________ > From: edk2-devel <[email protected]> on behalf of Leif > Lindholm <[email protected]> > Sent: 15 March 2018 12:49:27 > To: Ard Biesheuvel > Cc: [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected] > Subject: Re: [edk2] [PATCH 4/4] ArmPlatformPkg: add PL011 UART runtime debug > driver > > On Thu, Mar 01, 2018 at 06:11:42PM +0000, Ard Biesheuvel wrote: >> Implement the new runtime debug output protocol on top of a PL011 UART. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <[email protected]> > > LGTM > Reviewed-by: Leif Lindholm <[email protected]> > >> --- >> >> ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.c >> | 144 ++++++++++++++++++++ >> >> ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.inf >> | 62 +++++++++ >> 2 files changed, 206 insertions(+) >> >> diff --git >> a/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.c >> b/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.c >> new file mode 100644 >> index 000000000000..155b2c50d463 >> --- /dev/null >> +++ >> b/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.c >> @@ -0,0 +1,144 @@ >> +/** @file >> + Runtime driver to produce debug output on a PL011 UART >> + >> + Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR> >> + >> + This program and the accompanying materials are licensed and made >> available >> + under the terms and conditions of the BSD License which accompanies >> this >> + distribution. The full text of the license may be found at >> + http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >> IMPLIED. >> + >> +**/ >> + >> +#include <PiDxe.h> >> +#include <Library/DxeServicesTableLib.h> >> +#include <Library/PL011UartLib.h> >> +#include <Library/UefiDriverEntryPoint.h> >> +#include <Library/UefiBootServicesTableLib.h> >> +#include <Library/UefiRuntimeLib.h> >> +#include <Protocol/RuntimeDebugOutput.h> >> + >> +STATIC UINTN mUartBase; >> +STATIC EFI_EVENT mVirtualAddressChangeEvent; >> + >> +/** >> + Write data from buffer to debug output device >> + >> + Writes NumberOfBytes data bytes from Buffer to the debug output device. >> + The number of bytes actually written to the device is returned. >> + If the return value is less than NumberOfBytes, then the write >> operation >> + failed. >> + If NumberOfBytes is zero, then return 0. >> + >> + @param Buffer Pointer to the data buffer to be written. >> + @param NumberOfBytes Number of bytes to written to the device. >> + >> + @retval 0 NumberOfBytes is 0. >> + @retval >0 The number of bytes written to the serial >> device. >> + If this value is less than NumberOfBytes, then >> the >> + write operation failed. >> + >> +**/ >> +STATIC >> +UINTN >> +PL011RuntimeDebugOutputWrite ( >> + IN EDK2_RUNTIME_DEBUG_OUTPUT_PROTOCOL *This, >> + IN UINT8 *Buffer, >> + IN UINTN NumberOfBytes >> + ) >> +{ >> + return PL011UartWrite (mUartBase, Buffer, NumberOfBytes); >> +} >> + >> +STATIC EDK2_RUNTIME_DEBUG_OUTPUT_PROTOCOL mRuntimeDebugOutput = { >> + PL011RuntimeDebugOutputWrite >> +}; >> + >> +/** >> + Fixup internal data so that EFI can be called in virtual mode. >> + >> + @param[in] Event The Event that is being processed >> + @param[in] Context Event Context >> +**/ >> +STATIC >> +VOID >> +EFIAPI >> +VirtualNotifyEvent ( >> + IN EFI_EVENT Event, >> + IN VOID *Context >> + ) >> +{ >> + EfiConvertPointer (0x0, (VOID **)&mUartBase); >> +} >> + >> +EFI_STATUS >> +EFIAPI >> +PL011RuntimeDebugOutputDxeEntry ( >> + IN EFI_HANDLE ImageHandle, >> + IN EFI_SYSTEM_TABLE *SystemTable >> + ) >> +{ >> + EFI_STATUS Status; >> + EFI_HANDLE Handle; >> + UINT64 BaudRate; >> + UINT32 ReceiveFifoDepth; >> + EFI_PARITY_TYPE Parity; >> + UINT8 DataBits; >> + EFI_STOP_BITS_TYPE StopBits; >> + >> + mUartBase = (UINTN)FixedPcdGet64 (PcdSerialRegisterBase); >> + BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate); >> + ReceiveFifoDepth = 0; // Use default FIFO depth >> + Parity = (EFI_PARITY_TYPE)FixedPcdGet8 >> (PcdUartDefaultParity); >> + DataBits = FixedPcdGet8 (PcdUartDefaultDataBits); >> + StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 >> (PcdUartDefaultStopBits); >> + >> + Status = PL011UartInitializePort (mUartBase, FixedPcdGet32 >> (PL011UartClkInHz), >> + &BaudRate, &ReceiveFifoDepth, &Parity, &DataBits, >> &StopBits); >> + if (EFI_ERROR (Status)) { >> + return Status; >> + } >> + >> + // >> + // Register for the virtual address change event >> + // >> + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY, >> + VirtualNotifyEvent, NULL, >> &gEfiEventVirtualAddressChangeGuid, >> + &mVirtualAddressChangeEvent); >> + if (EFI_ERROR (Status)) { >> + return Status; >> + } >> + >> + // >> + // Declare the UART MMIO region as EFI_MEMORY_RUNTIME >> + // >> + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, >> mUartBase, >> + SIZE_4KB, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); >> + if (EFI_ERROR (Status)) { >> + goto CloseEvent; >> + } >> + >> + Status = gDS->SetMemorySpaceAttributes (mUartBase, SIZE_4KB, >> + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); >> + if (EFI_ERROR (Status)) { >> + goto CloseEvent; >> + } >> + >> + Handle = NULL; >> + Status = gBS->InstallMultipleProtocolInterfaces (&Handle, >> + &gEdkiiRuntimeDebugOutputProtocolGuid, >> &mRuntimeDebugOutput, >> + NULL); >> + if (EFI_ERROR (Status)) { >> + goto CloseEvent; >> + } >> + >> + return EFI_SUCCESS; >> + >> +CloseEvent: >> + gBS->CloseEvent (mVirtualAddressChangeEvent); >> + >> + return Status; >> +} >> diff --git >> a/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.inf >> b/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.inf >> new file mode 100644 >> index 000000000000..28a8e514552e >> --- /dev/null >> +++ >> b/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.inf >> @@ -0,0 +1,62 @@ >> +#/** @file >> +# Runtime driver to produce debug output on a PL011 UART >> +# >> +# Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR> >> +# >> +# This program and the accompanying materials are licensed and made >> available >> +# under the terms and conditions of the BSD License which accompanies >> this >> +# distribution. The full text of the license may be found at >> +# http://opensource.org/licenses/bsd-license.php >> +# >> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >> IMPLIED. >> +# >> +# >> +#**/ >> + >> +[Defines] >> + INF_VERSION = 0x0001001A >> + BASE_NAME = PL011RuntimeDebugOutputDxe >> + FILE_GUID = 494297ca-9205-463a-aae5-215ffd067cbb >> + MODULE_TYPE = DXE_RUNTIME_DRIVER >> + VERSION_STRING = 1.0 >> + ENTRY_POINT = PL011RuntimeDebugOutputDxeEntry >> + >> +# >> +# The following information is for reference only and not required by the >> build tools. >> +# >> +# VALID_ARCHITECTURES = AARCH64 ARM >> +# >> + >> +[Sources.common] >> + PL011RuntimeDebugOutputDxe.c >> + >> +[Packages] >> + ArmPlatformPkg/ArmPlatformPkg.dec >> + MdeModulePkg/MdeModulePkg.dec >> + MdePkg/MdePkg.dec >> + >> +[LibraryClasses] >> + DxeServicesTableLib >> + PcdLib >> + PL011UartLib >> + UefiBootServicesTableLib >> + UefiDriverEntryPoint >> + UefiRuntimeLib >> + >> +[Guids] >> + gEfiEventVirtualAddressChangeGuid ## CONSUMES # Event >> + >> +[Protocols] >> + gEdkiiRuntimeDebugOutputProtocolGuid ## PROTOCOL >> ALWAYS_PRODUCED >> + >> +[FixedPcd] >> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits >> + gArmPlatformTokenSpaceGuid.PL011UartClkInHz >> + >> +[Depex] >> + TRUE >> -- >> 2.11.0 >> > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
