Re: [PATCH] ArmPlatformPkg/PL011SerialPortLib: use untyped PCD for register base

[Leif Lindholm <leif.lindholm@linaro.org>]

On Mon, Dec 17, 2018 at 07:51:45PM +0100, Ard Biesheuvel wrote:
> Use an untyped PCD reference for PcdSerialRegisterBase, so that the
> library gets built without hardcoded values, permitting modules to
> override the default serial port. This allows SerialDxe to use a
> different serial port from the one used for DEBUG output (which
> often gets occluded due to the console driver clearing the screen)
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c   | 14 +++++++-------
>  ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf |  4 +++-
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> index 212991d63859..d576f79c3e6e 100644
> --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> @@ -48,7 +48,7 @@ SerialPortInitialize (
>    StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
>  
>    return PL011UartInitializePort (
> -           (UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
> +           (UINTN)PcdGet64 (PcdSerialRegisterBase),
>             PL011UartClockGetFreq(),
>             &BaudRate,
>             &ReceiveFifoDepth,
> @@ -75,7 +75,7 @@ SerialPortWrite (
>    IN UINTN     NumberOfBytes
>    )
>  {
> -  return PL011UartWrite ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase), Buffer, NumberOfBytes);
> +  return PL011UartWrite ((UINTN)PcdGet64 (PcdSerialRegisterBase), Buffer, NumberOfBytes);
>  }
>  
>  /**
> @@ -95,7 +95,7 @@ SerialPortRead (
>    IN  UINTN     NumberOfBytes
>  )
>  {
> -  return PL011UartRead ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase), Buffer, NumberOfBytes);
> +  return PL011UartRead ((UINTN)PcdGet64 (PcdSerialRegisterBase), Buffer, NumberOfBytes);
>  }
>  
>  /**
> @@ -111,7 +111,7 @@ SerialPortPoll (
>    VOID
>    )
>  {
> -  return PL011UartPoll ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase));
> +  return PL011UartPoll ((UINTN)PcdGet64 (PcdSerialRegisterBase));
>  }
>  /**
>    Set new attributes to PL011.
> @@ -156,7 +156,7 @@ SerialPortSetAttributes (
>    )
>  {
>    return PL011UartInitializePort (
> -           (UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
> +           (UINTN)PcdGet64 (PcdSerialRegisterBase),
>             PL011UartClockGetFreq(),
>             BaudRate,
>             ReceiveFifoDepth,
> @@ -198,7 +198,7 @@ SerialPortSetControl (
>    IN UINT32  Control
>    )
>  {
> -  return PL011UartSetControl ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase), Control);
> +  return PL011UartSetControl ((UINTN)PcdGet64 (PcdSerialRegisterBase), Control);
>  }
>  
>  /**
> @@ -239,5 +239,5 @@ SerialPortGetControl (
>    OUT UINT32  *Control
>    )
>  {
> -  return PL011UartGetControl ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase), Control);
> +  return PL011UartGetControl ((UINTN)PcdGet64 (PcdSerialRegisterBase), Control);
>  }
> diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> index 5ce5b2f5304c..bca7bed875c6 100644
> --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> @@ -36,8 +36,10 @@
>    MdeModulePkg/MdeModulePkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
>  
> -[FixedPcd]
> +[Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
> +
> +[FixedPcd]
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> -- 
> 2.17.1
>