Very good commit log, history information and code change. :)

Reviewed-by: Star Zeng <[email protected]>

Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:[email protected]] 
Sent: Wednesday, August 16, 2017 8:11 PM
To: edk2-devel-01 <[email protected]>
Cc: Ard Biesheuvel <[email protected]>; Brijesh Singh 
<[email protected]>; Heyi Guo <[email protected]>; Zeng, Star 
<[email protected]>
Subject: [PATCH 1/1] ArmVirtPkg/FdtPL011SerialPortLib: call PL011UartLib in all 
SerialPortLib APIs

With the SerialDxe change in commit 4cf3f37c87ba ("MdeModulePkg SerialDxe:
Process timeout consistently in SerialRead", 2017-07-18), setting 
EFI_SERIAL_INPUT_BUFFER_EMPTY in the "Control" output parameter, in the
GetControl() SerialPortLib function, is no longer a "small optimization".
Namely, due to the SerialDxe change, the GetOneKeyFromSerial() call in 
TerminalDxe's TerminalConInTimerHandler() can take very long if the input queue 
is empty, even if GetOneKeyFromSerial()'s return value causes the loop to be 
exited right after, in the first iteration.

This issue causes a boot hang in ArmVirtQemu: with the input queue empty,
TerminalConInTimerHandler() takes so long to return that, by the time it 
returns, there's another execution queued already (due to the associated timer 
event being signaled meanwhile). The boot process is stuck in the timer event 
handler.

Therefore even the first GetOneKeyFromSerial() iteration must be prevented in 
TerminalConInTimerHandler() if the input queue is empty, and that requires 
implementing GetControl() for real.

Implement the SetAttributes(), SetControl() and GetControl() APIs (of 
SerialPortExtLib origin) in FdtPL011SerialPortLib with calls to matching 
PL011UartLib functions. This follows the example of 
"ArmPlatformPkg/Library/PL011SerialPortLib" and also matches Star's original 
idea under [1].

The patch can be considered a continuation of commit ad7f6bc2e116
("ArmVirtPkg: Use SerialDxe in MdeModulePkg instead of EmbeddedPkg", 
2015-11-26), based on the mailing list threads [1] [2] [3].

[1] 
http://mid.mail-archive.com/[email protected]
[2] 
http://mid.mail-archive.com/[email protected]
[3] [email protected]">http://mid.mail-archive.com/[email protected]

Cc: Ard Biesheuvel <[email protected]>
Cc: Brijesh Singh <[email protected]>
Cc: Heyi Guo <[email protected]>
Cc: Star Zeng <[email protected]>
Originally-suggested-by: Star Zeng <[email protected]>
Reported-by: Brijesh Singh <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <[email protected]>
---
 ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c | 38 
++++++++++++++++++--
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c 
b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
index 48a0530dcc2f..05d3547fda91 100644
--- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
+++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
@@ -200,7 +200,23 @@ SerialPortSetAttributes (
   IN OUT EFI_STOP_BITS_TYPE *StopBits
   )
 {
-  return RETURN_UNSUPPORTED;
+  RETURN_STATUS Status;
+
+  if (mSerialBaseAddress == 0) {
+    Status = RETURN_UNSUPPORTED;
+  } else {
+    Status = PL011UartInitializePort (
+               mSerialBaseAddress,
+               FixedPcdGet32 (PL011UartClkInHz),
+               BaudRate,
+               ReceiveFifoDepth,
+               Parity,
+               DataBits,
+               StopBits
+               );
+  }
+
+  return Status;
 }
 
 /**
@@ -219,7 +235,15 @@ SerialPortSetControl (
   IN UINT32 Control
   )
 {
-  return RETURN_UNSUPPORTED;
+  RETURN_STATUS Status;
+
+  if (mSerialBaseAddress == 0) {
+    Status = RETURN_UNSUPPORTED;
+  } else {
+    Status = PL011UartSetControl (mSerialBaseAddress, Control);  }
+
+  return Status;
 }
 
 /**
@@ -238,6 +262,14 @@ SerialPortGetControl (
   OUT UINT32 *Control
   )
 {
-  return RETURN_UNSUPPORTED;
+  RETURN_STATUS Status;
+
+  if (mSerialBaseAddress == 0) {
+    Status = RETURN_UNSUPPORTED;
+  } else {
+    Status = PL011UartGetControl (mSerialBaseAddress, Control);  }
+
+  return Status;
 }
 
--
2.14.1.3.gb7cf6e02401b

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to