1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v1 00/19] Fix warnings reported by VS2017 compiler
  4. View patch

[edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation

Sami Mujawar posted 19 patches 6 years, 5 months ago
There is a newer version of this series
[edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
Posted by Sami Mujawar 6 years, 5 months ago 
The ARM DCC serial port subtype is an option that is
supported by the DBG2 generator. However, the serial
port initialisation should only be done for PL011/SBSA
compatible UARTs.

Add check to conditionally initialise the serial port.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
@@ -175,7 +175,7 @@ GET_OBJECT_LIST (
   CM_ARM_SERIAL_PORT_INFO
   );
 
-/** Initialize the PL011 UART with the parameters obtained from
+/** Initialize the PL011/SBSA UART with the parameters obtained from
     the Configuration Manager.
 
   @param [in]  SerialPortInfo Pointer to the Serial Port Information.
@@ -353,15 +353,22 @@ BuildDbg2Table (
   AcpiDbg2.Dbg2DeviceInfo[DBG_PORT_INDEX_PORT1].Dbg2Device.PortSubtype =
     SerialPortInfo->PortSubtype;
 
-  // Initialize the serial port
-  Status = SetupDebugUart (SerialPortInfo);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((
-      DEBUG_ERROR,
-      "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
-      Status
-      ));
-    goto error_handler;
+  if ((SerialPortInfo->PortSubtype ==
+      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)           ||
+      (SerialPortInfo->PortSubtype ==
+      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART_2X) ||
+      (SerialPortInfo->PortSubtype ==
+      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART)) {
+    // Initialize the serial port
+    Status = SetupDebugUart (SerialPortInfo);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
+        Status
+        ));
+      goto error_handler;
+    }
   }
 
   *Table = (EFI_ACPI_DESCRIPTION_HEADER*)&AcpiDbg2;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#46273): https://edk2.groups.io/g/devel/message/46273
Mute This Topic: https://groups.io/mt/32999793/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
Posted by Philippe Mathieu-Daudé 6 years, 2 months ago 
On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The ARM DCC serial port subtype is an option that is
> supported by the DBG2 generator. However, the serial
> port initialisation should only be done for PL011/SBSA
> compatible UARTs.
> 
> Add check to conditionally initialise the serial port.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>   DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
>   1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
>     CM_ARM_SERIAL_PORT_INFO
>     );
>   
> -/** Initialize the PL011 UART with the parameters obtained from
> +/** Initialize the PL011/SBSA UART with the parameters obtained from
>       the Configuration Manager.

Isn't the SBSA UART a PL011?

>   
>     @param [in]  SerialPortInfo Pointer to the Serial Port Information.
> @@ -353,15 +353,22 @@ BuildDbg2Table (
>     AcpiDbg2.Dbg2DeviceInfo[DBG_PORT_INDEX_PORT1].Dbg2Device.PortSubtype =
>       SerialPortInfo->PortSubtype;
>   
> -  // Initialize the serial port
> -  Status = SetupDebugUart (SerialPortInfo);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((
> -      DEBUG_ERROR,
> -      "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
> -      Status
> -      ));
> -    goto error_handler;
> +  if ((SerialPortInfo->PortSubtype ==
> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)           ||
> +      (SerialPortInfo->PortSubtype ==
> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART_2X) ||
> +      (SerialPortInfo->PortSubtype ==
> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART)) {
> +    // Initialize the serial port
> +    Status = SetupDebugUart (SerialPortInfo);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
> +        Status
> +        ));
> +      goto error_handler;
> +    }
>     }
>   
>     *Table = (EFI_ACPI_DESCRIPTION_HEADER*)&AcpiDbg2;
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51072): https://edk2.groups.io/g/devel/message/51072
Mute This Topic: https://groups.io/mt/32999793/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
Posted by Leif Lindholm 6 years, 2 months ago 
On Thu, Nov 21, 2019 at 16:20:31 +0100, Philippe Mathieu-Daudé wrote:
> On 8/23/19 12:55 PM, Sami Mujawar wrote:
> > The ARM DCC serial port subtype is an option that is
> > supported by the DBG2 generator. However, the serial
> > port initialisation should only be done for PL011/SBSA
> > compatible UARTs.
> > 
> > Add check to conditionally initialise the serial port.
> > 
> > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > ---
> >   DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
> >   1 file changed, 17 insertions(+), 10 deletions(-)
> > 
> > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
> > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
> >     CM_ARM_SERIAL_PORT_INFO
> >     );
> > -/** Initialize the PL011 UART with the parameters obtained from
> > +/** Initialize the PL011/SBSA UART with the parameters obtained from
> >       the Configuration Manager.
> 
> Isn't the SBSA UART a PL011?

No. It's a compatible subset.
So a PL011 can be used as an SBSA UART.

/
    Leif

> >     @param [in]  SerialPortInfo Pointer to the Serial Port Information.
> > @@ -353,15 +353,22 @@ BuildDbg2Table (
> >     AcpiDbg2.Dbg2DeviceInfo[DBG_PORT_INDEX_PORT1].Dbg2Device.PortSubtype =
> >       SerialPortInfo->PortSubtype;
> > -  // Initialize the serial port
> > -  Status = SetupDebugUart (SerialPortInfo);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((
> > -      DEBUG_ERROR,
> > -      "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
> > -      Status
> > -      ));
> > -    goto error_handler;
> > +  if ((SerialPortInfo->PortSubtype ==
> > +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)           ||
> > +      (SerialPortInfo->PortSubtype ==
> > +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART_2X) ||
> > +      (SerialPortInfo->PortSubtype ==
> > +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART)) {
> > +    // Initialize the serial port
> > +    Status = SetupDebugUart (SerialPortInfo);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((
> > +        DEBUG_ERROR,
> > +        "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
> > +        Status
> > +        ));
> > +      goto error_handler;
> > +    }
> >     }
> >     *Table = (EFI_ACPI_DESCRIPTION_HEADER*)&AcpiDbg2;
> > 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51075): https://edk2.groups.io/g/devel/message/51075
Mute This Topic: https://groups.io/mt/32999793/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
Posted by Philippe Mathieu-Daudé 6 years, 2 months ago 
On 11/21/19 4:23 PM, Leif Lindholm wrote:
> On Thu, Nov 21, 2019 at 16:20:31 +0100, Philippe Mathieu-Daudé wrote:
>> On 8/23/19 12:55 PM, Sami Mujawar wrote:
>>> The ARM DCC serial port subtype is an option that is
>>> supported by the DBG2 generator. However, the serial
>>> port initialisation should only be done for PL011/SBSA
>>> compatible UARTs.
>>>
>>> Add check to conditionally initialise the serial port.
>>>
>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>> ---
>>>    DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
>>>    1 file changed, 17 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>> index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
>>> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>> @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
>>>      CM_ARM_SERIAL_PORT_INFO
>>>      );
>>> -/** Initialize the PL011 UART with the parameters obtained from
>>> +/** Initialize the PL011/SBSA UART with the parameters obtained from
>>>        the Configuration Manager.
>>
>> Isn't the SBSA UART a PL011?
> 
> No. It's a compatible subset.
> So a PL011 can be used as an SBSA UART.

OK thanks.

Can you update the comment? Maybe:

"Initialize the PL011 compatible UART with the parameters ..."

Regardless:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> 
> /
>      Leif
> 
>>>      @param [in]  SerialPortInfo Pointer to the Serial Port Information.
>>> @@ -353,15 +353,22 @@ BuildDbg2Table (
>>>      AcpiDbg2.Dbg2DeviceInfo[DBG_PORT_INDEX_PORT1].Dbg2Device.PortSubtype =
>>>        SerialPortInfo->PortSubtype;
>>> -  // Initialize the serial port
>>> -  Status = SetupDebugUart (SerialPortInfo);
>>> -  if (EFI_ERROR (Status)) {
>>> -    DEBUG ((
>>> -      DEBUG_ERROR,
>>> -      "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
>>> -      Status
>>> -      ));
>>> -    goto error_handler;
>>> +  if ((SerialPortInfo->PortSubtype ==
>>> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)           ||
>>> +      (SerialPortInfo->PortSubtype ==
>>> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART_2X) ||
>>> +      (SerialPortInfo->PortSubtype ==
>>> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART)) {
>>> +    // Initialize the serial port
>>> +    Status = SetupDebugUart (SerialPortInfo);
>>> +    if (EFI_ERROR (Status)) {
>>> +      DEBUG ((
>>> +        DEBUG_ERROR,
>>> +        "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
>>> +        Status
>>> +        ));
>>> +      goto error_handler;
>>> +    }
>>>      }
>>>      *Table = (EFI_ACPI_DESCRIPTION_HEADER*)&AcpiDbg2;
>>>
>>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51076): https://edk2.groups.io/g/devel/message/51076
Mute This Topic: https://groups.io/mt/32999793/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
Posted by Leif Lindholm 6 years, 2 months ago 
On Thu, Nov 21, 2019 at 16:29:16 +0100, Philippe Mathieu-Daudé wrote:
> On 11/21/19 4:23 PM, Leif Lindholm wrote:
> > On Thu, Nov 21, 2019 at 16:20:31 +0100, Philippe Mathieu-Daudé wrote:
> > > On 8/23/19 12:55 PM, Sami Mujawar wrote:
> > > > The ARM DCC serial port subtype is an option that is
> > > > supported by the DBG2 generator. However, the serial
> > > > port initialisation should only be done for PL011/SBSA
> > > > compatible UARTs.
> > > > 
> > > > Add check to conditionally initialise the serial port.
> > > > 
> > > > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > > > ---
> > > >    DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
> > > >    1 file changed, 17 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > > > index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
> > > > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > > > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > > > @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
> > > >      CM_ARM_SERIAL_PORT_INFO
> > > >      );
> > > > -/** Initialize the PL011 UART with the parameters obtained from
> > > > +/** Initialize the PL011/SBSA UART with the parameters obtained from
> > > >        the Configuration Manager.
> > > 
> > > Isn't the SBSA UART a PL011?
> > 
> > No. It's a compatible subset.
> > So a PL011 can be used as an SBSA UART.
> 
> OK thanks.
> 
> Can you update the comment? Maybe:
> 
> "Initialize the PL011 compatible UART with the parameters ..."

The original is correct, the suggested alternative is not (an SBSA
UART is a subset, so not fully compatible).

If the comment was to change, I would suggest that dropping the model
name completely and simply refer to it as "the UART" would be
preferable.

/
    Leif

> Regardless:
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> 
> > 
> > /
> >      Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51080): https://edk2.groups.io/g/devel/message/51080
Mute This Topic: https://groups.io/mt/32999793/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
Posted by Philippe Mathieu-Daudé 6 years, 2 months ago 
On 11/21/19 4:48 PM, Leif Lindholm wrote:
> On Thu, Nov 21, 2019 at 16:29:16 +0100, Philippe Mathieu-Daudé wrote:
>> On 11/21/19 4:23 PM, Leif Lindholm wrote:
>>> On Thu, Nov 21, 2019 at 16:20:31 +0100, Philippe Mathieu-Daudé wrote:
>>>> On 8/23/19 12:55 PM, Sami Mujawar wrote:
>>>>> The ARM DCC serial port subtype is an option that is
>>>>> supported by the DBG2 generator. However, the serial
>>>>> port initialisation should only be done for PL011/SBSA
>>>>> compatible UARTs.
>>>>>
>>>>> Add check to conditionally initialise the serial port.
>>>>>
>>>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>>>> ---
>>>>>     DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
>>>>>     1 file changed, 17 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>>>> index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
>>>>> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>>>> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>>>> @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
>>>>>       CM_ARM_SERIAL_PORT_INFO
>>>>>       );
>>>>> -/** Initialize the PL011 UART with the parameters obtained from
>>>>> +/** Initialize the PL011/SBSA UART with the parameters obtained from
>>>>>         the Configuration Manager.
>>>>
>>>> Isn't the SBSA UART a PL011?
>>>
>>> No. It's a compatible subset.
>>> So a PL011 can be used as an SBSA UART.
>>
>> OK thanks.
>>
>> Can you update the comment? Maybe:
>>
>> "Initialize the PL011 compatible UART with the parameters ..."
> 
> The original is correct, the suggested alternative is not (an SBSA
> UART is a subset, so not fully compatible).

OK, thanks for clarifying.

> 
> If the comment was to change, I would suggest that dropping the model
> name completely and simply refer to it as "the UART" would be
> preferable.
> 
> /
>      Leif
> 
>> Regardless:
>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>
>>>
>>> /
>>>       Leif
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51087): https://edk2.groups.io/g/devel/message/51087
Mute This Topic: https://groups.io/mt/32999793/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
Posted by Alexei Fedorov 5 years, 10 months ago 
Reviewed by: Alexei Fedorov <Alexei.Fedorov@arm.com>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56486): https://edk2.groups.io/g/devel/message/56486
Mute This Topic: https://groups.io/mt/32999793/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.