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

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

On 2021.04.03 16:17, Mario Bălănică wrote:
> I've submitted this patch because the mini UART serial console produces 
> garbage since UEFI v1.25 (due to the wrong baud rate).

OK.

When you fix something like this, please mention that there was a 
regression that requires fixing, in the commit message.

> It has been tested on the firmware shipped with UEFI v1.24 and it works 
> fine. I don't see any reason to test it on older versions than this.

Well, my worry is that you are removing this part:

 > -#if (RPI_MODEL == 4)
 > -  Divisor = MmioRead32(BCM2836_CM_BASE + 
BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;
 > -  if (Divisor != 0)
 > -    BaseClockRate = (BaseClockRate << 12) / Divisor;
 > -#endif

which was added due to serial issues with older versions of the firmware 
(much older than the one included in 1.24), to make it compatible with 
versions of start4.elf where the Pi Foundation had suffled the serial 
clock rates yet again.

I guess if we can't make both newer versions and these older versions 
work with the same code, we have to stick with what works for newer 
ones. But I'm still wondering what changed in the Pi firmware to make 
applying this divisor computation fail, especially as this was supposed 
to alleviate the precise serial baudrate issue you are trying to fix.

> The core clock is not fixed. I've made PcdSerialClockRate patchable here:
> 
>     @@ -465,6 +464,9 @@ [PcdsFixedAtBuild.common]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> 
>     +[PcdsPatchableInModule]
>     + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
>     +
> 
> 500 MHz is the default clock rate (assuming it hasn't been changed in 
> config.txt).
> The PCD gets patched in both the PEI and DXE phases to the value read 
> from mailbox.

Yeah, I had missed that.

I suppose we can go with what you suggest. But I'm still worried that 
the Pi Foundation are going to shuffle their clocks again in a future 
firmware, and that we'll be modifying this code yet again, as your patch 
is pretty much reverting the code to what we used to have... until we 
found it got broken by a newly introduced Pi Foundation firmware.

Regards,

/Pete