* [edk2-platforms][PATCH V1 1/1] Platform/ARM/N1Sdp: Modify IRQ ID of Debug UART and routing to IOFPGA UART1 @ 2023-01-06 6:09 sahil 2023-01-25 19:53 ` [edk2-devel] " Thomas Abraham 0 siblings, 1 reply; 3+ messages in thread From: sahil @ 2023-01-06 6:09 UTC (permalink / raw) To: devel; +Cc: Ard Biesheuvel, Leif Lindholm, Sami Mujawar, Sahil From: Himanshu Sharma <Himanshu.Sharma@arm.com> In DBG2 table, IRQ ID was set as 0 for the UART. This overwrote the IPI0 trigger method to "level", which prevented SGI0 to be enabled again after a CPU offline/online cycle. This patch fixes the above issue by assigning a reserved IRQ ID for the Debug UART, other than 0 and also routing it to use IOFPGA UART1 by unsharing it from currently using serial terminal. Signed-off-by: Himanshu Sharma <Himanshu.Sharma@arm.com> --- Platform/ARM/N1Sdp/N1SdpPlatform.dsc | 8 ++++---- Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc index d04b22d3ef51..676ab677257a 100644 --- a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc @@ -4,7 +4,7 @@ # This provides platform specific component descriptions and libraries that # conform to EFI/Framework standards. # -# Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR> +# Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -136,9 +136,9 @@ [PcdsFixedAtBuild.common] gArmPlatformTokenSpaceGuid.PL011UartInterrupt|95 # PL011 Serial Debug UART (DBG2) - gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase - gArmPlatformTokenSpaceGuid.PcdSerialDbgUartBaudRate|gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate - gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz|50000000 + gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x1C0A0000 + gArmPlatformTokenSpaceGuid.PcdSerialDbgUartBaudRate|115200 + gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz|24000000 # SBSA Watchdog gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|93 diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c index b11c0425fe25..44046a0026bb 100644 --- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c +++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c @@ -320,7 +320,7 @@ EDKII_PLATFORM_REPOSITORY_INFO N1sdpRepositoryInfo = { // Debug Serial Port { FixedPcdGet64 (PcdSerialDbgRegisterBase), // BaseAddress - 0, // Interrupt -unused + 250, // Interrupt (reserved) FixedPcdGet64 (PcdSerialDbgUartBaudRate), // BaudRate FixedPcdGet32 (PcdSerialDbgUartClkInHz), // Clock EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART // Port subtype -- 2.25.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] Platform/ARM/N1Sdp: Modify IRQ ID of Debug UART and routing to IOFPGA UART1 2023-01-06 6:09 [edk2-platforms][PATCH V1 1/1] Platform/ARM/N1Sdp: Modify IRQ ID of Debug UART and routing to IOFPGA UART1 sahil @ 2023-01-25 19:53 ` Thomas Abraham 2023-01-26 10:34 ` Sami Mujawar 0 siblings, 1 reply; 3+ messages in thread From: Thomas Abraham @ 2023-01-25 19:53 UTC (permalink / raw) To: devel, sahil; +Cc: Ard Biesheuvel, Leif Lindholm, Sami Mujawar Hi Sahil, On 06/01/2023 06:09, sahil via groups.io wrote: > From: Himanshu Sharma <Himanshu.Sharma@arm.com> > > In DBG2 table, IRQ ID was set as 0 for the UART. This overwrote the > IPI0 trigger method to "level", which prevented SGI0 to be enabled > again after a CPU offline/online cycle. > > This patch fixes the above issue by assigning a reserved IRQ ID > for the Debug UART, other than 0 and also routing it to use IOFPGA > UART1 by unsharing it from currently using serial terminal. > > Signed-off-by: Himanshu Sharma <Himanshu.Sharma@arm.com> > --- > Platform/ARM/N1Sdp/N1SdpPlatform.dsc | 8 ++++---- > Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c | 2 +- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc > index d04b22d3ef51..676ab677257a 100644 > --- a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc > +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc > @@ -4,7 +4,7 @@ > # This provides platform specific component descriptions and libraries that > > # conform to EFI/Framework standards. > > # > > -# Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR> > > +# Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -136,9 +136,9 @@ [PcdsFixedAtBuild.common] > gArmPlatformTokenSpaceGuid.PL011UartInterrupt|95 > > > > # PL011 Serial Debug UART (DBG2) > > - gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase > > - gArmPlatformTokenSpaceGuid.PcdSerialDbgUartBaudRate|gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate > > - gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz|50000000 > > + gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x1C0A0000 > > + gArmPlatformTokenSpaceGuid.PcdSerialDbgUartBaudRate|115200 > > + gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz|24000000 > > > > # SBSA Watchdog > > gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|93 > > diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c > index b11c0425fe25..44046a0026bb 100644 > --- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c > +++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c > @@ -320,7 +320,7 @@ EDKII_PLATFORM_REPOSITORY_INFO N1sdpRepositoryInfo = { > // Debug Serial Port > > { > > FixedPcdGet64 (PcdSerialDbgRegisterBase), // BaseAddress > > - 0, // Interrupt -unused > > + 250, // Interrupt (reserved) I have not looked into ConfigurationManager but why would IRQ number be mandatory for debug UART? Isn't there a way to just avoid specifying a interrupt number? Thanks, Thomas. > > FixedPcdGet64 (PcdSerialDbgUartBaudRate), // BaudRate > > FixedPcdGet32 (PcdSerialDbgUartClkInHz), // Clock > > EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART // Port subtype > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] Platform/ARM/N1Sdp: Modify IRQ ID of Debug UART and routing to IOFPGA UART1 2023-01-25 19:53 ` [edk2-devel] " Thomas Abraham @ 2023-01-26 10:34 ` Sami Mujawar 0 siblings, 0 replies; 3+ messages in thread From: Sami Mujawar @ 2023-01-26 10:34 UTC (permalink / raw) To: Thomas Abraham, devel@edk2.groups.io, Sahil Cc: Ard Biesheuvel, Leif Lindholm, nd, Pierre Gondois, Andre Przywara Hi Sahil, Thomas, Please find my response inline marked [SAMI]. Regards, Sami Mujawar On 25/01/2023, 19:53, "Thomas Abraham" <thomas.abraham@arm.com> wrote: Hi Sahil, On 06/01/2023 06:09, sahil via groups.io wrote: > From: Himanshu Sharma <Himanshu.Sharma@arm.com> > > In DBG2 table, IRQ ID was set as 0 for the UART. This overwrote the [SAMI] The above line is incorrect. It is not the DBG2 table where the IRQ ID is set. It is the AML description for the UART that is configured as the serial debug port where the IRQ ID needs to be configured correctly. [/SAMI] > IPI0 trigger method to "level", which prevented SGI0 to be enabled > again after a CPU offline/online cycle. > > This patch fixes the above issue by assigning a reserved IRQ ID [SAMI] I am not sure what you mean here. Is the IRQ ID 250 (configured in code below) not for the UART? > for the Debug UART, other than 0 and also routing it to use IOFPGA > UART1 by unsharing it from currently using serial terminal. > > Signed-off-by: Himanshu Sharma <Himanshu.Sharma@arm.com> [SAMI] This should be signed off only by the person who sends the patch. > --- > Platform/ARM/N1Sdp/N1SdpPlatform.dsc | 8 ++++---- > Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c | 2 +- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc > index d04b22d3ef51..676ab677257a 100644 > --- a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc > +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc > @@ -4,7 +4,7 @@ > # This provides platform specific component descriptions and libraries that > > # conform to EFI/Framework standards. > > # > > -# Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR> > > +# Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -136,9 +136,9 @@ [PcdsFixedAtBuild.common] > gArmPlatformTokenSpaceGuid.PL011UartInterrupt|95 > > > > # PL011 Serial Debug UART (DBG2) > > - gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase > > - gArmPlatformTokenSpaceGuid.PcdSerialDbgUartBaudRate|gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate > > - gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz|50000000 > > + gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x1C0A0000 > > + gArmPlatformTokenSpaceGuid.PcdSerialDbgUartBaudRate|115200 > > + gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz|24000000 > > > > # SBSA Watchdog > > gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|93 > > diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c > index b11c0425fe25..44046a0026bb 100644 > --- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c > +++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c > @@ -320,7 +320,7 @@ EDKII_PLATFORM_REPOSITORY_INFO N1sdpRepositoryInfo = { > // Debug Serial Port > > { > > FixedPcdGet64 (PcdSerialDbgRegisterBase), // BaseAddress > > - 0, // Interrupt -unused > > + 250, // Interrupt (reserved) I have not looked into ConfigurationManager but why would IRQ number be mandatory for debug UART? Isn't there a way to just avoid specifying a interrupt number? [SAMI] This information is used for: 1. Generating the DBG2 table, which does not require the interrupt number (and is hence ignored). 2. Generating the AML description for the UART port which requires the Interrupt to be specified, see example below: // UART PL011 Device(COM0) { Name(_HID, "ARMH0011") Name(_UID, Zero) Name(_CRS, ResourceTemplate() { Memory32Fixed(ReadWrite, 0x7FF80000, 0x1000) Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 115 } }) } The commit message is misleading and needs to be fixed. [/SAMI] Thanks, Thomas. > > FixedPcdGet64 (PcdSerialDbgUartBaudRate), // BaudRate > > FixedPcdGet32 (PcdSerialDbgUartClkInHz), // Clock > > EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART // Port subtype > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-26 10:34 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-06 6:09 [edk2-platforms][PATCH V1 1/1] Platform/ARM/N1Sdp: Modify IRQ ID of Debug UART and routing to IOFPGA UART1 sahil 2023-01-25 19:53 ` [edk2-devel] " Thomas Abraham 2023-01-26 10:34 ` Sami Mujawar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox