From: "Ni, Ray" <ray.ni@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "Wu, Hao A" <hao.a.wu@intel.com>
Subject: Re: [edk2-devel] [Patch 1/1] MdeModulePkg/PciSioSerialDxe: Flush Tx before config change
Date: Fri, 11 Dec 2020 03:22:33 +0000 [thread overview]
Message-ID: <CO1PR11MB4930306934825C3D8BD7D11F8CCA0@CO1PR11MB4930.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20201211003255.967-1-michael.d.kinney@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney
> Sent: Friday, December 11, 2020 8:33 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [Patch 1/1] MdeModulePkg/PciSioSerialDxe: Flush Tx before config change
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=3114
>
> Add logic to flush all UART transmit buffers if there is a
> config change from Reset(), SetAttributes() or SetControl().
> Use a timeout in the flush operation, so the system can
> continue to boot if the transmit buffers can not be
> flushed for any reason.
>
> This change prevents lost characters on serial debug logs
> and serial consoles when a config change is made. It also
> prevents a UART from getting into a bad state or reporting
> error status due to characters being transmitted at the same
> time registers are updated with new communications settings.
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
> .../Bus/Pci/PciSioSerialDxe/SerialIo.c | 91 ++++++++++++++++++-
> 1 file changed, 90 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
> index 8377ffa13c7a..56c5faf5db1f 100644
> --- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
> @@ -1,7 +1,7 @@
> /** @file
> SerialIo implementation for PCI or SIO UARTs.
>
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -436,6 +436,68 @@ SerialReceiveTransmit (
> return EFI_SUCCESS;
> }
>
> +/**
> + Flush the serial hardware transmit FIFO, holding register, and shift register.
> +
> + @param SerialDevice The device to flush.
> +
> + @retval EFI_SUCCESS The transmit FIFO is completely flushed.
> + @retval EFI_TIMEOUT A timeout occured waiting for the transmit FIFO to flush.
> +**/
> +EFI_STATUS
> +SerialFlushTransmitFifo (
> + SERIAL_DEV *SerialDevice
> + )
> +{
> + SERIAL_PORT_LSR Lsr;
> + UINTN Timeout;
> + UINTN Elapsed;
> +
> + //
> + // If this is the first time the UART is being configured, then the current
> + // UART settings are not known, so compute a timeout to wait for the Tx FIFO
> + // assuming the worst case current settings.
> + //
> + // Timeout = (Max Bits per Char) * (Max Pending Chars) / (Slowest Baud Rate)
> + // Max Bits per Char = Start bit + 8 data bits + parity + 2 stop bits = 12
> + // Max Pending Chars = Largest Tx FIFO + hold + shift = 64 + 1 + 1 = 66
> + // Slowest Reasonable Baud Rate = 300 baud
> + // Timeout = 12 * 66 / 300 = 2.64 seconds = 2,640,000 uS
> + //
> + Timeout = 2640000;
> +
> + //
> + // Use the largest of the computed timeout, the default timeout, and the
> + // currently set timeout.
> + //
> + Timeout = MAX (Timeout, SERIAL_PORT_DEFAULT_TIMEOUT);
> + Timeout = MAX (Timeout, SerialDevice->SerialMode.Timeout);
> +
> + //
> + // Wait for the shortest time possible for the serial port to be ready making
> + // sure the transmit FIFO, holding register, and shift register are all
> + // empty. The actual wait time is expected to be very small because the
> + // number characters currently in the FIFO should be small when a
> + // configuration change is requested.
> + //
> + // NOTE: Do not use any DEBUG() or REPORT_STATUS_CODE() or any other calls
> + // in the rest of this function that may send additional characters to this
> + // UART device invalidating the flush operation.
> + //
> + Elapsed = 0;
> + Lsr.Data = READ_LSR (SerialDevice);
> + while (Lsr.Bits.Temt == 0 || Lsr.Bits.Thre == 0) {
> + if (Elapsed >= Timeout) {
> + return EFI_TIMEOUT;
> + }
> + gBS->Stall (TIMEOUT_STALL_INTERVAL);
> + Elapsed += TIMEOUT_STALL_INTERVAL;
> + Lsr.Data = READ_LSR (SerialDevice);
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> //
> // Interface Functions
> //
> @@ -476,6 +538,15 @@ SerialReset (
>
> Tpl = gBS->RaiseTPL (TPL_NOTIFY);
>
> + //
> + // Wait for all data to be transmitted before changing the UART configuration.
> + //
> + // NOTE: Do not use any DEBUG() or REPORT_STATUS_CODE() or any other calls
> + // that may send additional characters to this UART device until the UART
> + // configuration change is complete.
> + //
> + SerialFlushTransmitFifo (SerialDevice);
> +
> //
> // Make sure DLAB is 0.
> //
> @@ -654,6 +725,15 @@ SerialSetAttributes (
>
> Tpl = gBS->RaiseTPL (TPL_NOTIFY);
>
> + //
> + // Wait for all data to be transmitted before changing the UART configuration.
> + //
> + // NOTE: Do not use any DEBUG() or REPORT_STATUS_CODE() or any other calls
> + // that may send additional characters to this UART device until the UART
> + // configuration change is complete.
> + //
> + SerialFlushTransmitFifo (SerialDevice);
> +
> //
> // Put serial port on Divisor Latch Mode
> //
> @@ -826,6 +906,15 @@ SerialSetControl (
>
> Tpl = gBS->RaiseTPL (TPL_NOTIFY);
>
> + //
> + // Wait for all data to be transmitted before changing the UART configuration.
> + //
> + // NOTE: Do not use any DEBUG() or REPORT_STATUS_CODE() or any other calls
> + // that may send additional characters to this UART device until the UART
> + // configuration change is complete.
> + //
> + SerialFlushTransmitFifo (SerialDevice);
> +
> Mcr.Data = READ_MCR (SerialDevice);
> Mcr.Bits.DtrC = 0;
> Mcr.Bits.Rts = 0;
> --
> 2.29.2.windows.2
>
>
>
>
>
next prev parent reply other threads:[~2020-12-11 3:22 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-11 0:32 [Patch 1/1] MdeModulePkg/PciSioSerialDxe: Flush Tx before config change Michael D Kinney
2020-12-11 3:22 ` Ni, Ray [this message]
2020-12-11 5:56 ` [edk2-devel] " Wu, Hao A
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO1PR11MB4930306934825C3D8BD7D11F8CCA0@CO1PR11MB4930.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox