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

2020-03-27 Thread Alexei Fedorov 
Reviewed by: Alexei Fedorov 

-=-=-=-=-=-=-=-=-=-=-=-
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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2019-11-21 Thread Philippe Mathieu-Daudé 

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



/
  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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2019-11-21 Thread Leif Lindholm 
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 
> > > > ---
> > > >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 
> 
> > 
> > /
> >  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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2019-11-21 Thread Philippe Mathieu-Daudé 

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



/
 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*)








-=-=-=-=-=-=-=-=-=-=-=-
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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2019-11-21 Thread Leif Lindholm 
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 
> > ---
> >   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*)
> > 
> 

-=-=-=-=-=-=-=-=-=-=-=-
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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2019-11-21 Thread Philippe Mathieu-Daudé 

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





-=-=-=-=-=-=-=-=-=-=-=-
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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2019-08-23 Thread Sami Mujawar 
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 
---
 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*)
-- 
'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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-