From: "Pete Batard" <pete@akeo.ie>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: "Mario Bălănică" <mariobalanica02@gmail.com>,
devel@edk2.groups.io,
"Ard Biesheuvel" <ardb+tianocore@kernel.org>,
"Leif Lindholm" <leif@nuviainc.com>
Subject: Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation
Date: Thu, 8 Apr 2021 13:33:38 +0100 [thread overview]
Message-ID: <bbad5b5c-52f4-77c3-30bf-d2efdc499b63@akeo.ie> (raw)
In-Reply-To: <CAMj1kXEcG03NfDFZ6Qp+Jhg1E3UH7U+J7GoNJ2UzKecDkPbTEg@mail.gmail.com>
On 2021.04.08 13:26, Ard Biesheuvel wrote:
> On Thu, 8 Apr 2021 at 13:43, Pete Batard <pete@akeo.ie> wrote:
>>
>> On 2021.04.08 12:06, Ard Biesheuvel wrote:
>>> On Thu, 8 Apr 2021 at 11:47, Pete Batard <pete@akeo.ie> wrote:
>>>>
>>>> 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>
>>>
>>> Thanks Pete.
>>>
>>> Could anyone get me a version of this patch that is not whitespace
>>> mangled? This one does not apply with 'git am'
>>>
>>
>> Done.
>>
>> The version I sent is the one I applied & tested after I ran it through
>> my unmangling script to remove the extra lines.
>>
>> Not sure if it'll still not be re-mangled by the list processing though.
>>
>
> Yes, which is odd given that I am receiving this email directly.
>
> In any case, I still had to perform surgery on the patch to get it to apply.
>
> Pushed as 7fe9704893f1..d2339f3c5f9a; mind double checking if I did
> not break anything?
Just re-tested with edk2-platforms latest, and everything looks good.
Thanks.
/Pete
>
> Thanks,
> Ard.
>
next prev parent reply other threads:[~2021-04-08 12:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-06 14:04 [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation Mario Bălănică
2021-04-08 9:47 ` Pete Batard
2021-04-08 11:06 ` Ard Biesheuvel
2021-04-08 11:43 ` Pete Batard
2021-04-08 12:26 ` Ard Biesheuvel
2021-04-08 12:33 ` Pete Batard [this message]
-- strict thread matches above, loose matches on Subject: below --
2021-04-08 11:39 Pete Batard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bbad5b5c-52f4-77c3-30bf-d2efdc499b63@akeo.ie \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox