On 08/17/17 03:02, Zeng, Star wrote: > Very good commit log, history information and code change. :) > > Reviewed-by: Star Zeng <[email protected]>
Thank you all for the comments; patch pushed as commit 5f0f5e90ae8c. Cheers, Laszlo > -----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 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
