* [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation
@ 2021-04-06 14:04 Mario Bălănică
2021-04-08 9:47 ` Pete Batard
0 siblings, 1 reply; 7+ messages in thread
From: Mario Bălănică @ 2021-04-06 14:04 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, leif, pete
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
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation
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
0 siblings, 1 reply; 7+ messages in thread
From: Pete Batard @ 2021-04-08 9:47 UTC (permalink / raw)
To: Mario Bălănică, devel; +Cc: ardb+tianocore, leif
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation
2021-04-08 9:47 ` Pete Batard
@ 2021-04-08 11:06 ` Ard Biesheuvel
2021-04-08 11:43 ` Pete Batard
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2021-04-08 11:06 UTC (permalink / raw)
To: Pete Batard
Cc: Mario Bălănică, devel, Ard Biesheuvel,
Leif Lindholm
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'
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation
2021-04-08 11:06 ` Ard Biesheuvel
@ 2021-04-08 11:43 ` Pete Batard
2021-04-08 12:26 ` Ard Biesheuvel
0 siblings, 1 reply; 7+ messages in thread
From: Pete Batard @ 2021-04-08 11:43 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Mario Bălănică, devel, Ard Biesheuvel,
Leif Lindholm
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.
Regards,
/Pete
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation
2021-04-08 11:43 ` Pete Batard
@ 2021-04-08 12:26 ` Ard Biesheuvel
2021-04-08 12:33 ` Pete Batard
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2021-04-08 12:26 UTC (permalink / raw)
To: Pete Batard
Cc: Mario Bălănică, devel, Ard Biesheuvel,
Leif Lindholm
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?
Thanks,
Ard.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation
2021-04-08 12:26 ` Ard Biesheuvel
@ 2021-04-08 12:33 ` Pete Batard
0 siblings, 0 replies; 7+ messages in thread
From: Pete Batard @ 2021-04-08 12:33 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Mario Bălănică, devel, Ard Biesheuvel,
Leif Lindholm
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.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation
@ 2021-04-08 11:39 Pete Batard
0 siblings, 0 replies; 7+ messages in thread
From: Pete Batard @ 2021-04-08 11:39 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4735 bytes --]
From: Mario Bălănică <mariobalanica02@gmail.com>
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
-- 2.29.2.windows.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-08 12:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
-- strict thread matches above, loose matches on Subject: below --
2021-04-08 11:39 Pete Batard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox