On 28 May 2016 at 14:54, <[email protected]> wrote: > From: Evan Lloyd <[email protected]> > > On some platforms the UART clock is not the same for all the serial > ports. The PL011 driver must be capable of handling serial ports with > different clock rates, so must not rely on a PCD for the clock rate. > > This patch allows the UART clock rate to be passed as a parameter > to PL011UartInitializePort(), which is called from the serial port > library. This patch also contains the corresponding changes in the > serial port library. > > The PCD in Drivers/PL011Uart is replaced by an extra parameter for > PL011UartInitializePort. The PCD is moved to Library/PL011SerialPortLib > to supply the value to pass. > > A corresponding patch to ArmVirtPkg is included in the same bundle to > align that with these changes. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Sami Mujawar <[email protected]> > Signed-off-by: Evan Lloyd <[email protected]>
Minor comment below. But I tested it on Juno R2 and FVP Foundation and AEMV8 models anyway as it won't effect functionality. I saw a hang on R2 that I know to be unrelated to this patch, but thought I'd report it as I've not seen this specific hang before: ------------------------------------------------------------------------ Installation of the FDT using the device path <VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/board.dtb> completed. PCI: The size 0x7F000000 of the region 0x80000000 has been increased to be a power of two for the AXI translation table. PCI: The size 0x180000000 of the region 0x880000000 has been increased to be a power of two for the AXI translation table. Synchronous Exception at 0x00000000FDFAFDCC /working/platforms/uefi/edk2/Build/ArmJuno/DEBUG_GCC49/AARCH64/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe/DEBUG/CpuIo2Dxe.dll loaded at 0x00000000FDFAE000 X0 0x0000000057F00000 X1 0x00000000FDFB4030 X2 0x0000000057F00000 X3 0x000000000000001A X4 0x00000000FD5ACD98 X5 0x00000000FDFAF4B4 X6 0x0000000000000008 X7 0x0000000000000020 X8 0x00000000FE113010 X9 0x0000004100000000 X10 0x0000000000040000 X11 0x0000000000000000 X12 0x00000000707FE07A X13 0x0000000000000000 X14 0x0000000000000000 X15 0x0000000000000000 X16 0x00000000FEFFFBF0 X17 0x0000000000000000 X18 0x0000000000000000 X19 0x00000000FE0C0018 X20 0x0000000000000000 X21 0x0000000000000000 X22 0x0000000000000000 X23 0x0000000000000000 X24 0x0000000000000000 X25 0x0000000000000000 X26 0x0000000000000000 X27 0x0000000000000000 X28 0x0000000000000000 FP 0x0000000000000000 LR 0x00000000FDFAF550 V0 0x3C83709547545153 BC990E9F897D4FA8 V1 0xBEBEF297C5EC0578 E1D99AE8C2B92607 V2 0x13242833C5F05BAB 101C24C521387481 V3 0x0A50ABCAD0BDDA12 996865483E880969 V4 0x8400848428600003 0000000008000538 V5 0x00010110001A4001 80000E0202A00001 V6 0xA010290482010102 4042000A02400100 V7 0x8CC7020884C87814 78A5636F748F82EE V8 0x0000000000000000 A4506CEB90BEFFFA V9 0x0000000000000000 88C1883495C7F76F V10 0x0000000000000000 3DB8D233CF470963 V11 0x0000000000000000 4416D9F479B08221 V12 0x0000000000000000 55B8AE51435C1A1A V13 0x0000000000000000 C95B757DA62B3A12 V14 0x0000000000000000 E08479A1B557DFA8 V15 0x0000000000000000 927A0019CDEC1A76 V16 0x21081A2640C0F178 0084C0048EC80F21 V17 0x1902182018B00C04 0001080284828304 V18 0x001080000004000C 5900108030000800 V19 0x8001804051402A9B 8000004000080405 V20 0x000000980800104D 82000C0000000282 V21 0x01B0A50000880048 38A0047001808220 V22 0x8200024108502200 1200252080001002 V23 0x000F0080006290A2 B1601010040BA110 V24 0x8401100100804000 0020010200812008 V25 0x2800000008A00108 2001000000E00C24 V26 0x0840000020668104 4810C11700008400 V27 0x9000000000E00002 A6201000C0028C04 V28 0x8020000281000200 890080400880A022 V29 0x6085000000000136 800489A520006304 V30 0x0801200082000020 0384E25502959220 V31 0x4064084112940202 1000459804200800 SP 0x00000000FEFFF3F0 ELR 0x00000000FDFAFDCC SPSR 0x60000309 FPSR 0x00000000 ESR 0x96000210 FAR 0x0000000057F00000 ESR : EC 0x25 IL 0x1 ISS 0x00000210 Data abort: Synchronous external abort ASSERT [ArmCpuDxe] /working/platforms/uefi/edk2/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(194): ((BOOLEAN)(0==1)) ------------------------------------------------------------------------ I reverted the patch and the problem still happens. I reverted the whole series and I can still reproduce the problem. Ho hum. Tested-by: Ryan Harkin <[email protected]> > --- > ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf | 1 - > ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf | 1 + > ArmPlatformPkg/Include/Drivers/PL011Uart.h | 43 > ++++++++++++-------- > ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c | 22 > +++++++--- > ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c | 30 > +++++++------- > 5 files changed, 60 insertions(+), 37 deletions(-) > > diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf > b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf > index > 5afce36d3935e7fd79c25c46360d72328b2a571f..0154f3bd2e3a3ab930227bde8e45d5e13eaf92ae > 100644 > --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf > +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf > @@ -37,6 +37,5 @@ [Packages] > [FixedPcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate > > - gArmPlatformTokenSpaceGuid.PL011UartClkInHz > gArmPlatformTokenSpaceGuid.PL011UartInteger > gArmPlatformTokenSpaceGuid.PL011UartFractional > diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf > b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf > index > 653c0b2dfc147f1d82155e4150812f0cb4c59e12..3683e06d27e1a084ba493b0bdf1bec4c0e8f117a > 100644 > --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf > +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf > @@ -41,3 +41,4 @@ [FixedPcd] > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits > + gArmPlatformTokenSpaceGuid.PL011UartClkInHz > diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h > b/ArmPlatformPkg/Include/Drivers/PL011Uart.h > index > 8c2616ede4131b7504088d656160ce184dadf914..6a97174c82fa8d3153af1327060a98e3b66a8a45 > 100644 > --- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h > +++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h > @@ -89,32 +89,43 @@ > #define PL011_UARTPID2_VER(X) (((X) >> 4) & 0xF) > #define PL011_VER_R1P4 0x2 > > -/* > +/** > > Initialise the serial port to the specified settings. > - Programmed hardware of Serial port. > - @param UartBase The base address of the serial device. > - @param BaudRate The baud rate of the serial device. If the > baud rate is not supported, > - the speed will be reduced down to the > nearest supported one and the > + All unspecified settings will be set to the default values. > + > + @param[in] UartBase The base address of the serial device. > + @param[in] UartClkInHz The clock in Hz for the serial device. > + Ignored if the PCD PL011UartInteger is not > 0 > + @param[in out] BaudRate The baud rate of the serial device. If the > + baud rate is not supported, the speed will > be > + reduced to the nearest supported one and > the > variable's value will be updated > accordingly. > - @param ReceiveFifoDepth The number of characters the device will > buffer on input. > - ReceiveFifoDepth value of 0 will use the > device's default FIFO depth. > - @param Parity If applicable, this is the EFI_PARITY_TYPE > that is computed or checked > - as each character is transmitted or > received. If the device does not > - support parity, the value is the default > parity value. > - @param DataBits The number of data bits in each character > - @param StopBits If applicable, the EFI_STOP_BITS_TYPE > number of stop bits per character. > - If the device does not support stop bits, > the value is the default stop > - bit value. > + @param[in out] ReceiveFifoDepth The number of characters the device will > + buffer on input. Value of 0 will use the > + device's default FIFO depth. > + @param[in out] Parity If applicable, this is the EFI_PARITY_TYPE > + that is computed or checked as each > character > + is transmitted or received. If the device > does > + not support parity, the value is the > default > + parity value. > + @param[in out] DataBits The number of data bits in each character. > + @param[in out] StopBits If applicable, the EFI_STOP_BITS_TYPE > number > + of stop bits per character. > + If the device does not support stop bits, > the > + value is the default stop bit value. > > - @retval RETURN_SUCCESS All attributes were set correctly on the > serial device. > - @retval RETURN_INVALID_PARAMETER One or more of the attributes has an > unsupported value. > + @retval RETURN_SUCCESS All attributes were set correctly on the > + serial device. > + @retval RETURN_INVALID_PARAMETER One or more of the attributes has an > + unsupported value. > > **/ > RETURN_STATUS > EFIAPI > PL011UartInitializePort ( > IN UINTN UartBase, > + IN UINT32 UartClkInHz, > IN OUT UINT64 *BaudRate, > IN OUT UINT32 *ReceiveFifoDepth, > IN OUT EFI_PARITY_TYPE *Parity, > diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > index > dd5f88d3d629d345a50af468ed394a269b35f52a..61b122a67ab7354714adb429e401126973ab6c8d > 100644 > --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > @@ -35,6 +35,8 @@ STATIC CONST UINT32 mInvalidControlBits = > EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE; > All unspecified settings will be set to the default values. > > @param UartBase The base address of the serial device. > + @param UartClkInHz The clock in Hz for the serial device. > + Ignored if the PCD PL011UartInteger is not > 0 > @param BaudRate The baud rate of the serial device. If the > baud rate is not supported, the speed will > be > reduced to the nearest supported one and > the > @@ -63,6 +65,7 @@ RETURN_STATUS > EFIAPI > PL011UartInitializePort ( > IN UINTN UartBase, > + IN UINT32 UartClkInHz, > IN OUT UINT64 *BaudRate, > IN OUT UINT32 *ReceiveFifoDepth, > IN OUT EFI_PARITY_TYPE *Parity, > @@ -72,6 +75,8 @@ PL011UartInitializePort ( > { > UINT32 LineControl; > UINT32 Divisor; > + UINT32 Integer; > + UINT32 Fractional; > > // The PL011 supports a buffer of 1, 16 or 32 chars. Therefore we can > accept > // 1 char buffer as the minimum FIFO size. Because everything can be > rounded > @@ -168,19 +173,24 @@ PL011UartInitializePort ( > > // If PL011 Integer value has been defined then always ignore the BAUD rate > if (FixedPcdGet32 (PL011UartInteger) != 0) { > - MmioWrite32 (UartBase + UARTIBRD, FixedPcdGet32 (PL011UartInteger)); > - MmioWrite32 (UartBase + UARTFBRD, FixedPcdGet32 (PL011UartFractional)); > + Integer = FixedPcdGet32 (PL011UartInteger); > + Fractional = FixedPcdGet32 (PL011UartFractional); > } else { > // If BAUD rate is zero then replace it with the system default value > if (*BaudRate == 0) { > *BaudRate = FixedPcdGet32 (PcdSerialBaudRate); > - ASSERT (*BaudRate != 0); > + if (*BaudRate == 0) { Should you check UartClkInHz at this point too? > + return RETURN_INVALID_PARAMETER; > + } > } > > - Divisor = (FixedPcdGet32 (PL011UartClkInHz) * 4) / *BaudRate; > - MmioWrite32 (UartBase + UARTIBRD, Divisor >> FRACTION_PART_SIZE_IN_BITS); > - MmioWrite32 (UartBase + UARTFBRD, Divisor & FRACTION_PART_MASK); > + Divisor = (UartClkInHz * 4) / *BaudRate; > + Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS; > + Fractional = Divisor & FRACTION_PART_MASK; > } > + // Set Baud Rate Registers > + MmioWrite32 (UartBase + UARTIBRD, Integer); > + MmioWrite32 (UartBase + UARTFBRD, Fractional); > > // No parity, 1 stop, no fifo, 8 data bits > MmioWrite32 (UartBase + UARTLCR_H, LineControl); > diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c > b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c > index > 9c998a63cfe4e1506e6c1f0aa25ab6566f6dbf65..b3dea6b3aca91b3bc7a296c28a02aaa6593e4cf2 > 100644 > --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c > +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c > @@ -50,13 +50,14 @@ SerialPortInitialize ( > StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits); > > return PL011UartInitializePort ( > - (UINTN)FixedPcdGet64 (PcdSerialRegisterBase), > - &BaudRate, > - &ReceiveFifoDepth, > - &Parity, > - &DataBits, > - &StopBits > - ); > + (UINTN)FixedPcdGet64 (PcdSerialRegisterBase), > + FixedPcdGet32 (PL011UartClkInHz), > + &BaudRate, > + &ReceiveFifoDepth, > + &Parity, > + &DataBits, > + &StopBits > + ); > } > > /** > @@ -150,13 +151,14 @@ SerialPortSetAttributes ( > ) > { > return PL011UartInitializePort ( > - (UINTN)FixedPcdGet64 (PcdSerialRegisterBase), > - BaudRate, > - ReceiveFifoDepth, > - Parity, > - DataBits, > - StopBits > - ); > + (UINTN)FixedPcdGet64 (PcdSerialRegisterBase), > + FixedPcdGet32 (PL011UartClkInHz), > + BaudRate, > + ReceiveFifoDepth, > + Parity, > + DataBits, > + StopBits > + ); > } > > /** > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") > > _______________________________________________ > 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
