Re: [PATCH edk2-platforms 3/3] Silicon/SynQuacer: add support for DEBUG output on second UART

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, Dec 26, 2018 at 02:25:30PM +0100, Ard Biesheuvel wrote:
> On headless server systems where the PL011 serial port is the primary
> console, having DEBUG output on the same port can be annoying, since
> DEBUG output gets lost when the console driver clears the screen or
> positions the cursor using control characters.
> 
> So add the ability to emit the DEBUG output on the DesignWare FUART
> (which is exposed via the LS connector on DeveloperBox)

>From what I can tell, the DesignWare component is 8250-compatible, yet
here we're using the 16550 driver. I presume this makes no difference
for how we're using it, but could you add a comment to this effect to
the commit message? (If the FUART is indeed a 16550 clone, please add
a statement to that effect instead.)

With that, for the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc                                          | 42 +++++++++++++++++---
>  Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c |  3 ++
>  2 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index ed11aed798b7..da450a132798 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -28,6 +28,8 @@
>    FLASH_DEFINITION               = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
>    BUILD_NUMBER                   = 1
>  
> +  DEFINE DEBUG_ON_UART1          = FALSE
> +
>  [BuildOptions]
>    RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
>  
> @@ -120,9 +122,17 @@
>    DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
>    DebugAgentTimerLib|EmbeddedPkg/Library/DebugAgentTimerLibNull/DebugAgentTimerLibNull.inf
>    PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
> -  SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>    PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>  
> +!if $(DEBUG_ON_UART1) == FALSE
> +  SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +!else
> +  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> +  PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
> +  PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
> +  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> +!endif
> +
>    HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
>    TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>  
> @@ -253,13 +263,26 @@
>  !endif
>  
>    ## PL011 - Serial Terminal
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x2a400000
> -  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> -  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
>    gArmPlatformTokenSpaceGuid.PL011UartInteger|0
>    gArmPlatformTokenSpaceGuid.PL011UartFractional|0
>    gArmPlatformTokenSpaceGuid.PL011UartClkInHz|62500000
>  
> +  ## DesignWare FUART
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl|FALSE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|62500000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
> +
> +  ## Shared UART settings
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
> +
> +!if $(DEBUG_ON_UART1) == FALSE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x2a400000
> +!else
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x51040000
> +!endif
> +
>    #
>    # ARM Generic Interrupt Controller
>    #
> @@ -505,7 +528,16 @@
>    }
>  
>    MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> -  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
> +!if $(DEBUG_ON_UART1) == TRUE
> +    <PcdsFixedAtBuild>
> +      gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x2a400000
> +    <LibraryClasses>
> +      SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +      # suppress debug output from SerialDxe itself which would go to the PL011
> +      DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> +!endif
> +  }
>    MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
>    MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
>    MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
> index 1402ecafce4a..e68997e05573 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
> @@ -118,6 +118,9 @@ STATIC CONST ARM_MEMORY_REGION_DESCRIPTOR mVirtualMemoryTable[] = {
>  
>    // NETSEC/eMMC SMMU
>    ARM_DEVICE_REGION (SYNQUACER_SCB_SMMU_BASE, SYNQUACER_SCB_SMMU_SIZE),
> +
> +  // DesignWare FUART
> +  ARM_DEVICE_REGION (SYNQUACER_UART1_BASE, SYNQUACER_UART1_SIZE),
>  };
>  
>  STATIC
> -- 
> 2.19.2
>