From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by mx.groups.io with SMTP id smtpd.web12.7504.1617885222272103344 for ; Thu, 08 Apr 2021 05:33:42 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=qjLwBIrp; spf=pass (domain: akeo.ie, ip: 209.85.221.47, mailfrom: pete@akeo.ie) Received: by mail-wr1-f47.google.com with SMTP id b9so1976190wrs.1 for ; Thu, 08 Apr 2021 05:33:42 -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=zeOOjm1a7DDY/mG1499pr2FkowlNCYG+ixtxafFa1XQ=; b=qjLwBIrpBWE6kxzYMqDApD/q3hk67UfFBePViYY+vUNNFL/0AhlHKgz37vLAN7j51l HiJbAGJx64aNYSmPe+mTjkQObsJhdfG2yRnUp2pDppjEMaHBt/FFVBcPmCqPioH43V9u aeHEHWs0L8rXX22FdtM/VGqqjda8V9sKiXthLGI0AC1/DMiBpMcj650w9lPJDIsuzB+g IgxkEXnnOHKQmxrRuvp54LOlomNqGjyHoBMvLHNTCQlOEOAPWiDdcoXBZeVBAhV+QGJM eGH/27ics5Zqojer0yy8sX/2/tiRSoWF2GKspZtadg5+L0emuNIw0hc+kUl0XgWVtEzt 3xUw== 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=zeOOjm1a7DDY/mG1499pr2FkowlNCYG+ixtxafFa1XQ=; b=YPALYC9PrjlXTCZg7R+E/t6rakcZrcKu65ih2VnsV6WBiBtaPUNWn1/ijdop+Sburk mawLJeMZbISZxJF52pLHAoPHLU7VME7QeIVCneOdtrbWoP0QULDXnitVw2IBXZahEvig AB0ulwJ5tU7v4MN0jhPi3XY11AUu5JIQ1RsOFyxyBODBVxIxPzPWqUB8CP7k4pY+lg4r oX9PdxhF7FdOXv/UzEuMYRGGtPkYJe8izM/qP7E1MRoHAVjI8o/vUVbgIFEq914hB4kY qkxR0rQULUuAfdbY3MmCQs98AV6OfsbUqIrsFuZIOW+j1m8QXWOMeG7G1C7FA50mA+Mh V1CQ== X-Gm-Message-State: AOAM530os4aqdinSPWmWSOTGY3xks33iTq9S0cKCBu2ZCkxsybK2AcLP ROhY31DZOzDCd3Pn3SwYM9y9GA== X-Google-Smtp-Source: ABdhPJy2ayCOBBhlZXBLCHWqub3RMe9rFFCc9IFwWg0Qz1DoXpRibr8BzT8Mi4DKjXApZsIhoWW+Yw== X-Received: by 2002:a5d:5047:: with SMTP id h7mr11290056wrt.111.1617885220622; Thu, 08 Apr 2021 05:33:40 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.65.130]) by smtp.googlemail.com with ESMTPSA id f9sm13375256wmj.38.2021.04.08.05.33.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 08 Apr 2021 05:33:40 -0700 (PDT) Subject: Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation To: Ard Biesheuvel Cc: =?UTF-8?B?TWFyaW8gQsSDbMSDbmljxIM=?= , devel@edk2.groups.io, Ard Biesheuvel , Leif Lindholm References: <20210406140411.901-1-mariobalanica02@gmail.com> <45e921e6-edd7-a365-71c2-00383e76d5ea@akeo.ie> From: "Pete Batard" Message-ID: Date: Thu, 8 Apr 2021 13:33:38 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit On 2021.04.08 13:26, Ard Biesheuvel wrote: > On Thu, 8 Apr 2021 at 13:43, Pete Batard wrote: >> >> On 2021.04.08 12:06, Ard Biesheuvel wrote: >>> On Thu, 8 Apr 2021 at 11:47, Pete Batard 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ă >>>>> --- >>>>> 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 { >>>>> >>>>> >>>>> >>>>> - 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 >>>> Tested-by: Pete Batard >>> >>> 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. >