diff mbox series

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

Message ID 20181217185145.24618-1-ard.biesheuvel@linaro.org
State Accepted
Commit 5a9b3eb8e5fbbc0a49f80630039d58712aacfab8
Headers show
Series [edk2] ArmPlatformPkg/PL011SerialPortLib: use untyped PCD for register base | expand

Commit Message

Ard Biesheuvel Dec. 17, 2018, 6:51 p.m. UTC 
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>

---
 ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c   | 14 +++++++-------
 ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf |  4 +++-
 2 files changed, 10 insertions(+), 8 deletions(-)

-- 
2.17.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Philippe Mathieu-Daudé Dec. 18, 2018, 10:01 a.m. UTC | #1 
On 12/17/18 7:51 PM, 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)

You missed the trailing '.' :)

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  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
>
Leif Lindholm Dec. 20, 2018, 3:20 p.m. UTC | #2 
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

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 20, 2018, 5:35 p.m. UTC | #3 
On Thu, 20 Dec 2018 at 16:20, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>

> 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>

>


Thanks

Pushed as 6f42f9a54bf0..5a9b3eb8e5fb

> > ---

> >  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

> >

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

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