From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by mx.groups.io with SMTP id smtpd.web10.3462.1588760323363872335 for ; Wed, 06 May 2020 03:18:43 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=OYYtxvpo; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.66, mailfrom: pete@akeo.ie) Received: by mail-wm1-f66.google.com with SMTP id g12so1977075wmh.3 for ; Wed, 06 May 2020 03:18:43 -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=VKyQnAJQHePVNIFIOXs+h08tmWM5A/GmWqcwGZH8QF8=; b=OYYtxvpovViRdGtP2lSIeZ/jvFFmnMqfOEqvmMAqQ9TOw7JL23waW5fw8d/5OBQtra 4/EKuj1g+i1L69fHaZ+cHNddBCHUzHPqgUshd9MkXzuRPs46amn7sZPEOi1+OeOXcPmp 7JRmSlOfXakLs7+DZ78KEg+xwlIpEW0Gqgv0gDfbmW56yLpkb6TU+spH5Dn3wwP0jpXg q9UPIUdD1mm6LUzLyXShdH3VHKS88UaiYsKf/mI5bc1KL2IJvGkkh7+Yaz62jxqEKOUL 4P5bM/tcuEkzJM/mOrfFP9VTYPTPygoHvExmDQc+MqROsMMsAQ+CGdcYcYPerx0KpV9W M1ag== 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=VKyQnAJQHePVNIFIOXs+h08tmWM5A/GmWqcwGZH8QF8=; b=AcVa3g3RCgTHGM2NsdJVr46efeU6IV3YXGJLuugDpeDYhhnwB6mzBlokmd4yFVI/NV a9fhx4g8wW6mvFE+M9gXtLOCcNibloYQ+lkAi/RwI/AyLsdzYhHJZfvXNonYyAvUwCYg jNGntro2qaQ8qG4sUTLQrViL/Qu0aFjcsA1XGDOsy5s4gX8CteCwoDc3xTZHUNovxr6m D28jYJ8+NL4VdWVOuVYNpDkdsDYMDesElra9wuVj5d3Q0FrcLC8raSaj87C3mx8dHKyw fTbwjVsNjpU2GSxjpkXUpUzxeiMnHF5kkUXToM9viV3Xg5Vq6YHGRO3tc+tE08/XJ/VE 6okA== X-Gm-Message-State: AGi0PuYwDqKsUNSvurM4V17zaR6ZlUD/M/POnvPLuJsECiq0yAS+Hala fNeWiqYC6eQ8WpKeyTzo3i75esDQXGU= X-Google-Smtp-Source: APiQypIA/OmSx/WFv6d8TS/jtQW/REIY5sR+zh+UR4squF0OZn5iB+Ky4U/KaRZkPC4rw4sxvNH/1Q== X-Received: by 2002:a1c:3206:: with SMTP id y6mr3509687wmy.111.1588760321923; Wed, 06 May 2020 03:18:41 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.75.87]) by smtp.googlemail.com with ESMTPSA id v131sm2436033wmb.19.2020.05.06.03.18.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 06 May 2020 03:18:41 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH edk2-platforms 3/5] Platform/RaspberryPi: fix 16550 divisor calculation logic 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-4-ard.biesheuvel@arm.com> From: "Pete Batard" Message-ID: <956d2e2b-440e-9b04-1af1-a4a6a2dbe6ff@akeo.ie> Date: Wed, 6 May 2020 11:18:40 +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: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit One general remark below: On 2020.05.05 19:10, Ard Biesheuvel wrote: > On 5/5/20 4:50 PM, Ard Biesheuvel wrote: >> The 16550 'miniUART' on the Raspberry Pi gets its input clock from >> different sources on RPi3 and RPi3. Fix the logic that derives the > > This should be 'Rpi3 and RPi4' > >> divisor for the 16550 baud clock on the respective platforms. >> >> While at it, make the input clock PCD patchable for RPi3 so we can >> manipulate it at runtime in a future patch. For the sake of harmonization between platforms, wouldn't we want to also declare the PCD patchable on RPi4 as well (even though we obviously aren't going to patch it for the time being)? There is some plan of factorizing the DSC content, so if that doesn't hurt us, I'd propose we try to keep the Pi3 and Pi4 version as close as possible. This also ties in with my remark for the next patch. >> >> 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/RPi3/RPi3.dsc >> |  4 +++- >>   Platform/RaspberryPi/RPi4/RPi4.dsc >> |  2 +- >>   Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | >> 14 ++++++++------ >>   3 files changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc >> b/Platform/RaspberryPi/RPi3/RPi3.dsc >> index d7218219fc5a..96b27400eef8 100644 >> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc >> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc >> @@ -409,7 +409,6 @@ [PcdsFixedAtBuild.common] >>     gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000 >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4 >> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000 >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27 >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8 >>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200 >> @@ -441,6 +440,9 @@ [PcdsFixedAtBuild.common] >>     gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2" >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE >> +[PcdsPatchableInModule] >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|250000000 >> + >>   [PcdsDynamicHii.common.DEFAULT] >>     # >> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc >> b/Platform/RaspberryPi/RPi4/RPi4.dsc >> index 4fb015b077e6..5d8bd88d7e34 100644 >> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc >> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc >> @@ -420,7 +420,7 @@ [PcdsFixedAtBuild.common] >>     gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000 >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4 >> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000 >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000 >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27 >>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8 >>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200 >> diff --git >> a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c >> b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c >> index b1d17d3fa04a..5e83bbf022eb 100644 >> --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c >> +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c >> @@ -41,18 +41,20 @@ SerialPortGetDivisor ( >>     // 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) * 4; >> +  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 >>     // >> -  // Now calculate divisor for baud generator >> -  //    Ref_Clk_Rate / Baud_Rate / 16 >> +  // As per the BCM2xxx datasheets: >> +  // baudrate = system_clock_freq / (8 * (divisor + 1)). >>     // >> -  Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 16); >> -  if (((UINT32)BaseClockRate % (SerialBaudRate * 16)) >= >> SerialBaudRate * 8) { >> -    Divisor++; >> +  Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8); >> +  if (Divisor != 0) { >> +    Divisor--; >>     } >>     return Divisor; >>   } >> > > > > Reviewed-by: Pete Batard