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

Message ID 20181226132530.8445-4-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • synquacer multi-uart support
Related show

Commit Message

Ard Biesheuvel Dec. 26, 2018, 1:25 p.m.
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)

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(-)

-- 
2.19.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Jan. 11, 2019, 5:58 p.m. | #1
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

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Mark Kettenis Jan. 12, 2019, 6:18 p.m. | #2
> Date: Fri, 11 Jan 2019 17:58:44 +0000

> From: 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.)


The DesignWare component is (largely) 16550-compatible.  But the
FIFO's are optional and if they're not included you'll end up with
something that's probably closer to an 16450.  I suspect in most cases
SoC designers will include the FIFO's though since without them you
really can't use the port at anything but the slowest speeds.

Cheers,

Mark
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Jan. 14, 2019, 11:18 p.m. | #3
On Sat, 12 Jan 2019 at 19:25, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>

> > Date: Fri, 11 Jan 2019 17:58:44 +0000

> > From: 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.)

>

> The DesignWare component is (largely) 16550-compatible.  But the

> FIFO's are optional and if they're not included you'll end up with

> something that's probably closer to an 16450.  I suspect in most cases

> SoC designers will include the FIFO's though since without them you

> really can't use the port at anything but the slowest speeds.

>


Thanks Mark

Series pushed as e6fd447b23b6..307f7f5bfc4f (with Mark's clarification
added to the commit log of 3/3)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

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