| Message ID | 1464443656-12108-7-git-send-email-evan.lloyd@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
On 28 May 2016 at 14:54, <evan.lloyd@arm.com> wrote: > From: Evan Lloyd <evan.lloyd@arm.com> > > This change corrects 3 problems in the PL011 driver. > 1. The TRM states "The UARTLCR_H, UARTIBRD, and UARTFBRD registers must > not be changed:...when the UART is enabled" > 2. The TRM (3.3.8) describes logic requiring the UART to be disabled and > flushed before adjusting UARTCR. > 3. Several redundant calls get made to PL011UartInitializePort, where > the characteristics do not change, but updating the registers can > cause glitches in the output stream. > > The parameters are compared to the current state and no action taken if > no change of state is required. > Where an update is required, the specified logic is followed, and the > register updates only made when the UART is disabled. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> > Signed-off-by: Evan Lloyd <evan.lloyd@arm.com> Tested on Juno R2 and FVP Foundation and AEMv8 models. Tested-by: Ryan Harkin <ryan.harkin@linaro.org> Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org> > --- > ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c | 24 +++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > index 61b122a67ab7354714adb429e401126973ab6c8d..ec1abdd10c4ab2fdf59576af6eb3c92b8728d81a 100644 > --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > @@ -29,9 +29,11 @@ > // > STATIC CONST UINT32 mInvalidControlBits = EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE; > > -/* > +/** > > Initialise the serial port to the specified settings. > + The serial port is re-configured only if the specified settings > + are different from the current settings. > All unspecified settings will be set to the default values. > > @param UartBase The base address of the serial device. > @@ -188,6 +190,26 @@ PL011UartInitializePort ( > Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS; > Fractional = Divisor & FRACTION_PART_MASK; > } > + > + // > + // If PL011 is already initialized, check the current settings > + // and re-initialize only if the settings are different. > + // > + if (((MmioRead32 (UartBase + UARTCR) & PL011_UARTCR_UARTEN) != 0) && > + (MmioRead32 (UartBase + UARTLCR_H) == LineControl) && > + (MmioRead32 (UartBase + UARTIBRD) == Integer) && > + (MmioRead32 (UartBase + UARTFBRD) == Fractional)) { > + // Nothing to do - already initialized with correct attributes > + return RETURN_SUCCESS; > + } > + > + // Wait for the end of transmission > + while ((MmioRead32 (UartBase + UARTFR) & PL011_UARTFR_TXFE) == 0); > + > + // Disable UART: "The UARTLCR_H, UARTIBRD, and UARTFBRD registers must not be changed > + // when the UART is enabled" > + MmioWrite32 (UartBase + UARTCR, 0); > + > // Set Baud Rate Registers > MmioWrite32 (UartBase + UARTIBRD, Integer); > MmioWrite32 (UartBase + UARTFBRD, Fractional); > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c index 61b122a67ab7354714adb429e401126973ab6c8d..ec1abdd10c4ab2fdf59576af6eb3c92b8728d81a 100644 --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c @@ -29,9 +29,11 @@ // STATIC CONST UINT32 mInvalidControlBits = EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE; -/* +/** Initialise the serial port to the specified settings. + The serial port is re-configured only if the specified settings + are different from the current settings. All unspecified settings will be set to the default values. @param UartBase The base address of the serial device. @@ -188,6 +190,26 @@ PL011UartInitializePort ( Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS; Fractional = Divisor & FRACTION_PART_MASK; } + + // + // If PL011 is already initialized, check the current settings + // and re-initialize only if the settings are different. + // + if (((MmioRead32 (UartBase + UARTCR) & PL011_UARTCR_UARTEN) != 0) && + (MmioRead32 (UartBase + UARTLCR_H) == LineControl) && + (MmioRead32 (UartBase + UARTIBRD) == Integer) && + (MmioRead32 (UartBase + UARTFBRD) == Fractional)) { + // Nothing to do - already initialized with correct attributes + return RETURN_SUCCESS; + } + + // Wait for the end of transmission + while ((MmioRead32 (UartBase + UARTFR) & PL011_UARTFR_TXFE) == 0); + + // Disable UART: "The UARTLCR_H, UARTIBRD, and UARTFBRD registers must not be changed + // when the UART is enabled" + MmioWrite32 (UartBase + UARTCR, 0); + // Set Baud Rate Registers MmioWrite32 (UartBase + UARTIBRD, Integer); MmioWrite32 (UartBase + UARTFBRD, Fractional);