On 2020.05.06 11:31, Ard Biesheuvel wrote:
On 5/6/20 12:18 PM, Pete Batard wrote:
One remark below:

On 2020.05.05 19:11, Ard Biesheuvel wrote:
On 5/5/20 4:50 PM, Ard Biesheuvel wrote:
Query the firmware for the clock rate that is used to drive the
16550 baud clock, so that we can program the correct baud rate.

Co-authored-by: Pete Batard <p...@akeo.ie>
Co-authored-by: Andrei Warkentin <andrey.warken...@gmail.com>
Co-authored-by: Ard Biesheuvel <ard.biesheu...@arm.com>
Signed-off-by: Pete Batard <p...@akeo.ie>
Signed-off-by: Ard Biesheuvel <ard.biesheu...@arm.com>
---
Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 25 +++++++++++++++++++-
  1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
index 91dfe1bb981e..35580e4ed73a 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S +++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
@@ -3,7 +3,7 @@
   *  Copyright (c) 2020, Andrei Warkentin <andrey.warken...@gmail.com>
   *  Copyright (c) 2019-2020, Pete Batard <p...@akeo.ie>
   *  Copyright (c) 2016, Linaro Limited. All rights reserved.
- *  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+ *  Copyright (c) 2011-2020, ARM Limited. All rights reserved.
   *
   *  SPDX-License-Identifier: BSD-2-Clause-Patent
   *
@@ -85,6 +85,14 @@ ASM_FUNC (ArmPlatformPeiBootAction)
      adr     x2, mBoardRevision
      str     w0, [x2]
+#if (RPI_MODEL == 3)

As noted by Pete off-list, doing this doesn't work unless we add something like

GCC:*_*_*_PP_FLAGS          = -DRPI_MODEL=3

to the [BuildOptions] in RPi3.dsc



+    run     .Lclkinfo_buffer
+
+    ldr     w0, .Lfrequency
+    adrp    x2, _gPcd_BinaryPatch_PcdSerialClockRate
+    str     w0, [x2, :lo12:_gPcd_BinaryPatch_PcdSerialClockRate]
+#endif
+

Since we're modifying a patchable PCD here, shouldn't we add a:

[PatchPcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate

section in PlatformLib.inf?


We should add it, but we can add it to the [Pcd] section.

Alternatively, we could have different .INFs for RPi3 and RPi4, but that is really overkill IMO.

Making it patchable on both platforms just to patch it on one is also unnecessary I think. The current approach will ensure that we catch any issues at build time, without any major hacks,].

Yeah, I'm also fine with what we have at the moment, and I sure don't want separate .INFs for Pi3 and Pi4.


Of course, if we do that, we can't keep PcdSerialClockRate fixed for RPi4, as the build process will complain about PCD mismatch.

I also wouldn't mind a comment that explains how one arrives at figuring out that "_gPcd_BinaryPatch_PcdSerialClockRate" should be used to locate our address (and possibly the addition of :lo12:), because I don't think it's going to be that straightforward for people reading the code for the first time, though I fear that the explanation will boil down to "we need to do it this specific way for a gcc aarch64 relocation"...


We don't actually need the adrp/str pair with the lo12 here, I will replace it with adr. (Just muscle memory)

Sounds good, thanks!

/Pete




      ret
      .align  4
@@ -127,6 +135,21 @@ ASM_FUNC (ArmPlatformPeiBootAction)
      .long   0                           // end tag
      .set    .Lrevinfo_size, . - .Lrevinfo_buffer
+#if (RPI_MODEL == 3)
+    .align  4
+.Lclkinfo_buffer:
+    .long   .Lclkinfo_size
+    .long   0x0
+    .long   RPI_MBOX_GET_CLOCK_RATE
+    .long   8                           // buf size
+    .long   4                           // input len
+    .long   4                           // clock id: 0x04 = Core/VPU
+.Lfrequency:
+    .long   0                           // frequency
+    .long   0                           // end tag
+    .set    .Lclkinfo_size, . - .Lclkinfo_buffer
+#endif
+
  //UINTN
  //ArmPlatformGetPrimaryCoreMpId (
  //  VOID






With addition of "GCC:*_*_*_PP_FLAGS = -DRPI_MODEL=3" in the .dsc:
Reviewed-by: Pete Batard <p...@akeo.ie>



Thanks



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#58698): https://edk2.groups.io/g/devel/message/58698
Mute This Topic: https://groups.io/mt/73999457/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to