Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation

["Pete Batard" <pete@akeo.ie>]

On 2021.04.06 15:04, Mario Bălănică wrote:
> The VPU clock divisor has changed in this commit:
> https://github.com/raspberrypi/firmware/commit/1e5456a,
> thus breaking the mini UART clock divisor calculation on the Pi 4.
> 
> Fix this by reading the core clock from the mailbox instead.
> 
> Signed-off-by: Mario Bălănică <mariobalanica02@gmail.com>
> ---
>   Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c   | 15 +++------------
>   Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S |  4 ----
>   Platform/RaspberryPi/RPi4/RPi4.dsc                                   |  6 ++++--
>   3 files changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> index 5e83bbf022eb..d2f983bf0a9f 100644
> --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> @@ -34,25 +34,16 @@ SerialPortGetDivisor (
>     UINT32  SerialBaudRate
> 
>   )
> 
>   {
> 
> -  UINT64              BaseClockRate;
> 
> +  UINT32              BaseClockRate;
> 
>     UINT32              Divisor;
> 
>   
> 
> -  //
> 
> -  // On the Raspberry Pi, the clock to use for the 16650-compatible UART
> 
> -  // is the base clock divided by the 12.12 fixed point VPU clock divisor.
> 
> -  //
> 
> -  BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate);
> 
> -#if (RPI_MODEL == 4)
> 
> -  Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;
> 
> -  if (Divisor != 0)
> 
> -    BaseClockRate = (BaseClockRate << 12) / Divisor;
> 
> -#endif
> 
> +  BaseClockRate = PcdGet32 (PcdSerialClockRate);
> 
>   
> 
>     //
> 
>     // As per the BCM2xxx datasheets:
> 
>     // baudrate = system_clock_freq / (8 * (divisor + 1)).
> 
>     //
> 
> -  Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);
> 
> +  Divisor = BaseClockRate / (SerialBaudRate * 8);
> 
>     if (Divisor != 0) {
> 
>       Divisor--;
> 
>     }
> 
> diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
> index 58351e4fb8cc..7008aaf8aa4c 100644
> --- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
> +++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
> @@ -85,13 +85,11 @@ ASM_FUNC (ArmPlatformPeiBootAction)
>       adr     x2, mBoardRevision
> 
>       str     w0, [x2]
> 
>   
> 
> -#if (RPI_MODEL == 3)
> 
>       run     .Lclkinfo_buffer
> 
>   
> 
>       ldr     w0, .Lfrequency
> 
>       adr     x2, _gPcd_BinaryPatch_PcdSerialClockRate
> 
>       str     w0, [x2]
> 
> -#endif
> 
>   
> 
>       ret
> 
>   
> 
> @@ -135,7 +133,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
>       .long   0                           // end tag
> 
>       .set    .Lrevinfo_size, . - .Lrevinfo_buffer
> 
>   
> 
> -#if (RPI_MODEL == 3)
> 
>       .align  4
> 
>   .Lclkinfo_buffer:
> 
>       .long   .Lclkinfo_size
> 
> @@ -148,7 +145,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
>       .long   0                           // frequency
> 
>       .long   0                           // end tag
> 
>       .set    .Lclkinfo_size, . - .Lclkinfo_buffer
> 
> -#endif
> 
>   
> 
>   //UINTN
> 
>   //ArmPlatformGetPrimaryCoreMpId (
> 
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 2c05c31118d2..ff802d8347ea 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -429,7 +429,6 @@ [PcdsFixedAtBuild.common]
>     gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
> 
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
> 
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
> 
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000
> 
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
> 
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
> 
>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> 
> @@ -465,6 +464,9 @@ [PcdsFixedAtBuild.common]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"
> 
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> 
>   
> 
> +[PcdsPatchableInModule]
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
> 
> +
> 
>   [PcdsDynamicHii.common.DEFAULT]
> 
>   
> 
>     #
> 
> @@ -621,7 +623,7 @@ [Components.common]
>     MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> 
>     MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
> 
>       <LibraryClasses>
> 
> -      SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
> 
> +      SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf
> 
>     }
> 
>     Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf
> 
>     EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
> 

Reviewed-by: Pete Batard <pete@akeo.ie>
Tested-by: Pete Batard <pete@akeo.ie>