From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by mx.groups.io with SMTP id smtpd.web10.3464.1588760341087270880 for ; Wed, 06 May 2020 03:19:01 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=gg7FcdK4; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.65, mailfrom: pete@akeo.ie) Received: by mail-wm1-f65.google.com with SMTP id z6so1990685wml.2 for ; Wed, 06 May 2020 03:19:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=cKhTjqdpdVt/1BAMAVgUyeqQsi+q+p2vUNbD1xtfBM0=; b=gg7FcdK4AGLcAPyRoOuncnDtfJ/QxSKQOnbL18PSJE84vz1Z6e9fWEW+lRFxXShkXM +f+togrmtrvXoUu4cR2Go4WNFSDgIL0ZRM5nz5JlmZjpYbcLY0g+SecMJR9JCEadeprN kF35E75kxhaRQOV6hLtns/vCelAYs3Jj8mxy8Rtj8bld3wase9ylAi4AGIDJhcDPNIuV e2shXCB2+MyHUuxcBbJ+/J7soTDN04DO4mLP7oMnYye+aDPdrcYuNohZIePrrLm+8i0i IOfnHdy8czJ9QPqDPFxLHF2FZR6oFW06pk5UNSiwly4igjtZGRgUDIDvHtiGwMjxeVKj K9JA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=cKhTjqdpdVt/1BAMAVgUyeqQsi+q+p2vUNbD1xtfBM0=; b=Wvxdq49yI4OMCWSAOejfZXf2zFJ+gI5EnGtFjd5f8aE0zzo9px7SI5e/KObySEPBXI 6ZcsSKEOWGwsdun9f5JI+iEswUvWRMXxlFww1S+IizvgE2L2k2DuDMNEgrCM+JXT9bEh wc0ZsSKIJ3zcBV7OFR/rs/9P10sj++MyD2tqpVoPvraQv2ZmE+XXgmp5V7H4vAASSPNn BLFNsPqB4eNH2P3EmZCVeeo1+dPcpAQQbYQbtKWVjnxIK7vA+ayPOr8knRBdYMlDUb9L ogXOTgnpbIVsz7CmjEcANZhh2O8UvqwBWh6I+Bf/wjSvkknSMA3NDMf4kDsBLs9rDQEJ QfNQ== X-Gm-Message-State: AGi0PuaqxxPd/UeACWvx5u5d1znfWlwG9QWV24z2UbXfjOr9T+vh920P DUM5zp/nlskpmGs6qKgkTW3hNXVk4Q0= X-Google-Smtp-Source: APiQypK6hgp6hu976g7jCqKQ8OGDOg3+DpeynOTxUiPBIHsyn7QhHEuGT7ziFaRMTO/6xkslutgMxw== X-Received: by 2002:a1c:7416:: with SMTP id p22mr3895661wmc.80.1588760339634; Wed, 06 May 2020 03:18:59 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.75.87]) by smtp.googlemail.com with ESMTPSA id a187sm2383549wmh.40.2020.05.06.03.18.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 06 May 2020 03:18:59 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH edk2-platforms 4/5] Platform/RaspberryPi3: query firmware for 16550 input clock at boot To: devel@edk2.groups.io, ard.biesheuvel@arm.com Cc: leif@nuviainc.com, andrey.warkentin@gmail.com References: <20200505145029.29826-1-ard.biesheuvel@arm.com> <20200505145029.29826-5-ard.biesheuvel@arm.com> <02940c75-5a92-9fca-449a-7a90ccfc8362@arm.com> From: "Pete Batard" Message-ID: <2144540f-1b10-de24-95fa-09a0b2601cbb@akeo.ie> Date: Wed, 6 May 2020 11:18:58 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <02940c75-5a92-9fca-449a-7a90ccfc8362@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit 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 >> Co-authored-by: Andrei Warkentin >> Co-authored-by: Ard Biesheuvel >> Signed-off-by: Pete Batard >> Signed-off-by: Ard Biesheuvel >> --- >>   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 >>    *  Copyright (c) 2019-2020, Pete Batard >>    *  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? 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"... >>       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