Re: [PATCH edk2-platforms 3/5] Platform/RaspberryPi: fix 16550 divisor calculation logic

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 5/5/20 4:50 PM, Ard Biesheuvel wrote:
> The 16550 'miniUART' on the Raspberry Pi gets its input clock from
> different sources on RPi3 and RPi3. Fix the logic that derives the

This should be 'Rpi3 and RPi4'

> divisor for the 16550 baud clock on the respective platforms.
> 
> While at it, make the input clock PCD patchable for RPi3 so we can
> manipulate it at runtime in a future patch.
> 
> Co-authored-by: Pete Batard <pete@akeo.ie>
> Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
> Co-authored-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   Platform/RaspberryPi/RPi3/RPi3.dsc                                 |  4 +++-
>   Platform/RaspberryPi/RPi4/RPi4.dsc                                 |  2 +-
>   Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 14 ++++++++------
>   3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index d7218219fc5a..96b27400eef8 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -409,7 +409,6 @@ [PcdsFixedAtBuild.common]
>     gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> @@ -441,6 +440,9 @@ [PcdsFixedAtBuild.common]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>   
> +[PcdsPatchableInModule]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|250000000
> +
>   [PcdsDynamicHii.common.DEFAULT]
>   
>     #
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 4fb015b077e6..5d8bd88d7e34 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -420,7 +420,7 @@ [PcdsFixedAtBuild.common]
>     gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> index b1d17d3fa04a..5e83bbf022eb 100644
> --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> @@ -41,18 +41,20 @@ SerialPortGetDivisor (
>     // 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) * 4;
> +  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
>   
>     //
> -  // Now calculate divisor for baud generator
> -  //    Ref_Clk_Rate / Baud_Rate / 16
> +  // As per the BCM2xxx datasheets:
> +  // baudrate = system_clock_freq / (8 * (divisor + 1)).
>     //
> -  Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 16);
> -  if (((UINT32)BaseClockRate % (SerialBaudRate * 16)) >= SerialBaudRate * 8) {
> -    Divisor++;
> +  Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);
> +  if (Divisor != 0) {
> +    Divisor--;
>     }
>     return Divisor;
>   }
>