public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [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

* [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

* 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

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