From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web12.7438.1617884831367690917 for ; Thu, 08 Apr 2021 05:27:11 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=PxPOQ1MA; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id C36F061166 for ; Thu, 8 Apr 2021 12:27:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1617884830; bh=JGK+7yQdP5eWDtKDpBax2xULWtsYORLK8ZaMK/Gs3ok=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=PxPOQ1MA8aIBL4wXu+5PNBdcnwCrMuNMhRuRnAoHXH3bdMQLLyPEo0BKkHAXFHBkI Mdj9z3i1HQE3hFMDE51VWWkghJ8ugziPck53Nq7uBwE3zljjLoQbLPDkd2y+u7Egtg Cv/LBqO3bqN0qpj/rcXCAeY8cu20ybYzSpnup3LE5Pse4YnJ8e6ax2g4Nq+okvaUdh IDYPGuCEsFzbz+XVD1TOgzZJQ/Ckn4zSQVQNXtwmMw09Yo+rzwZsukD8gPi5OD2JQl jRoK2uvy1tEc6a4+XeP6g3ffMj8Jld+vUSu4IwkIj4w5fg3H6YL1GA0FzRcVJQp4Hh VxhstonKDeCdw== Received: by mail-oi1-f171.google.com with SMTP id i81so1950492oif.6 for ; Thu, 08 Apr 2021 05:27:10 -0700 (PDT) X-Gm-Message-State: AOAM530uWyOic/wtbfddFpOjKYKhH3up2ET+3deqkJGpTsRxJxWiEHO+ ABt/AXy6pdwe3q4Jj0p96530MwlWT5ucBA7zKs0= X-Google-Smtp-Source: ABdhPJxkuizT9v1oYHUvjoZO6TMoDDgZvFO785OsYVqMRoAT+3kowpxMji4UGG++jR0DgLj2tIc3605PBIqz3oEtlNY= X-Received: by 2002:aca:1a01:: with SMTP id a1mr5880727oia.33.1617884830025; Thu, 08 Apr 2021 05:27:10 -0700 (PDT) MIME-Version: 1.0 References: <20210406140411.901-1-mariobalanica02@gmail.com> <45e921e6-edd7-a365-71c2-00383e76d5ea@akeo.ie> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 8 Apr 2021 14:26:58 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation To: Pete Batard Cc: =?UTF-8?B?TWFyaW8gQsSDbMSDbmljxIM=?= , devel@edk2.groups.io, Ard Biesheuvel , Leif Lindholm Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=C4=83l=C4=83nic=C4=83 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=C4=83l=C4=83nic=C4=83 > >>> --- > >>> 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/DualSeria= lPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortL= ib.c > >>> index 5e83bbf022eb..d2f983bf0a9f 100644 > >>> --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLi= b.c > >>> +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLi= b.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 di= visor. > >>> > >>> - // > >>> > >>> - BaseClockRate =3D (UINT64)PcdGet32 (PcdSerialClockRate); > >>> > >>> -#if (RPI_MODEL =3D=3D 4) > >>> > >>> - Divisor =3D MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVI= SOR) & 0xFFFFFF; > >>> > >>> - if (Divisor !=3D 0) > >>> > >>> - BaseClockRate =3D (BaseClockRate << 12) / Divisor; > >>> > >>> -#endif > >>> > >>> + BaseClockRate =3D PcdGet32 (PcdSerialClockRate); > >>> > >>> > >>> > >>> // > >>> > >>> // As per the BCM2xxx datasheets: > >>> > >>> // baudrate =3D system_clock_freq / (8 * (divisor + 1)). > >>> > >>> // > >>> > >>> - Divisor =3D (UINT32)BaseClockRate / (SerialBaudRate * 8); > >>> > >>> + Divisor =3D BaseClockRate / (SerialBaudRate * 8); > >>> > >>> if (Divisor !=3D 0) { > >>> > >>> Divisor--; > >>> > >>> } > >>> > >>> diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/Raspber= ryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPi= Helper.S > >>> index 58351e4fb8cc..7008aaf8aa4c 100644 > >>> --- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHel= per.S > >>> +++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHel= per.S > >>> @@ -85,13 +85,11 @@ ASM_FUNC (ArmPlatformPeiBootAction) > >>> adr x2, mBoardRevision > >>> > >>> str w0, [x2] > >>> > >>> > >>> > >>> -#if (RPI_MODEL =3D=3D 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 =3D=3D 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/RaspberryP= i/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/D= ualSerialPortLib.inf > >>> > >>> + SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/D= ualSerialPortDxeLib.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? Thanks, Ard.