* [PATCH v1 1/1] DynamicTables: Fix DT PCI interrupt flags parsing
@ 2022-04-27 14:49 PierreGondois
2022-07-18 12:27 ` [edk2-devel] " Sami Mujawar
2022-07-18 16:26 ` Sami Mujawar
0 siblings, 2 replies; 5+ messages in thread
From: PierreGondois @ 2022-04-27 14:49 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar
From: Pierre Gondois <pierre.gondois@arm.com>
Device Tree PCI interrupt flags use the convention described at
linux/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
The 3rd cell is the flags, encoded as follows:
bits[3:0] trigger type and level flags.
1 = low-to-high edge triggered
2 = high-to-low edge triggered (invalid for SPIs)
4 = active high level-sensitive
8 = active low level-sensitive (invalid for SPIs).
Fix the incorrect code.
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 2 +-
DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index a34018151f2d..d5b1c153e98f 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -449,7 +449,7 @@ GeneratePrt (
if ((Index > 0) &&
(IrqMapInfo->IntcInterrupt.Interrupt >= 32) &&
(IrqMapInfo->IntcInterrupt.Interrupt < 1020) &&
- ((IrqMapInfo->IntcInterrupt.Flags & 0x3) != BIT0))
+ ((IrqMapInfo->IntcInterrupt.Flags & 0x3) != 0))
{
Status = EFI_INVALID_PARAMETER;
ASSERT_EFI_ERROR (Status);
diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h b/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h
index f2f425632b10..3f5d131d9ae5 100644
--- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h
+++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h
@@ -60,7 +60,7 @@
#define SPI_OFFSET (32U)
#define DT_PPI_IRQ (1U)
#define DT_SPI_IRQ (0U)
-#define DT_IRQ_IS_EDGE_TRIGGERED(x) ((((x) & (BIT0 | BIT2)) != 0))
+#define DT_IRQ_IS_EDGE_TRIGGERED(x) ((((x) & (BIT0 | BIT1)) != 0))
#define DT_IRQ_IS_ACTIVE_LOW(x) ((((x) & (BIT1 | BIT3)) != 0))
#define IRQ_TYPE_OFFSET (0U)
#define IRQ_NUMBER_OFFSET (1U)
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [edk2-devel] [PATCH v1 1/1] DynamicTables: Fix DT PCI interrupt flags parsing
2022-04-27 14:49 [PATCH v1 1/1] DynamicTables: Fix DT PCI interrupt flags parsing PierreGondois
@ 2022-07-18 12:27 ` Sami Mujawar
2022-07-18 12:30 ` PierreGondois
2022-07-18 16:26 ` Sami Mujawar
1 sibling, 1 reply; 5+ messages in thread
From: Sami Mujawar @ 2022-07-18 12:27 UTC (permalink / raw)
To: PierreGondois, devel
[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]
Hi Pierre,
Please find my response inline marked [SAMI]
Regards,
Sami Mujawar
On Wed, Apr 27, 2022 at 07:49 AM, PierreGondois wrote:
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPci=
>
> eGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtP=
>
> cieGenerator.c
> index a34018151f2d..d5b1c153e98f 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera=
>
> tor.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera=
>
> tor.c
> @@ -449,7 +449,7 @@ GeneratePrt (
> if ((Index > 0) &&
> (IrqMapInfo->IntcInterrupt.Interrupt >=3D 32) &&
> (IrqMapInfo->IntcInterrupt.Interrupt < 1020) &&
> - ((IrqMapInfo->IntcInterrupt.Flags & 0x3) !=3D BIT0))
> + ((IrqMapInfo->IntcInterrupt.Flags & 0x3) !=3D 0))
[SAMI] The BSA 1.0 says " Each legacy interrupt SPI must be programmed as level-sensitive ".
Considering this, I think the above check should be ((IrqMapInfo->IntcInterrupt.Flags & 0xB) != 0).
If you agree, please let me know I will make that change locally before pushing.
[/SAMI]
>
> {
> Status =3D EFI_INVALID_PARAMETER;
> ASSERT_EFI_ERROR (Status);
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h b/D=
>
> ynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h
[-- Attachment #2: Type: text/html, Size: 2074 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [edk2-devel] [PATCH v1 1/1] DynamicTables: Fix DT PCI interrupt flags parsing
2022-07-18 12:27 ` [edk2-devel] " Sami Mujawar
@ 2022-07-18 12:30 ` PierreGondois
2022-07-18 12:35 ` Sami Mujawar
0 siblings, 1 reply; 5+ messages in thread
From: PierreGondois @ 2022-07-18 12:30 UTC (permalink / raw)
To: Sami Mujawar, devel
Hi Sami
On 7/18/22 14:27, Sami Mujawar wrote:
> Hi Pierre,
>
> Please find my response inline marked [SAMI]
>
> Regards,
>
> Sami Mujawar
>
> On Wed, Apr 27, 2022 at 07:49 AM, PierreGondois wrote:
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPci=
> eGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtP=
> cieGenerator.c
> index a34018151f2d..d5b1c153e98f 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera=
> tor.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenera=
> tor.c
> @@ -449,7 +449,7 @@ GeneratePrt (
> if ((Index > 0) &&
> (IrqMapInfo->IntcInterrupt.Interrupt >=3D 32) &&
> (IrqMapInfo->IntcInterrupt.Interrupt < 1020) &&
> - ((IrqMapInfo->IntcInterrupt.Flags & 0x3) !=3D BIT0))
> + ((IrqMapInfo->IntcInterrupt.Flags & 0x3) !=3D 0))
>
> [SAMI] The BSA 1.0 says "Each legacy interrupt SPI must be programmed as level-sensitive".
> Considering this, I think the above check should be ((IrqMapInfo->IntcInterrupt.Flags & 0xB) != 0).
> If you agree, please let me know I will make that change locally before pushing.
> [/SAMI]
[Pierre]
Yes indeed,
Thanks for the review and changing this without requiring a v2,
Regards,
Pierre
[/Pierre]
>
> {
> Status =3D EFI_INVALID_PARAMETER;
> ASSERT_EFI_ERROR (Status);
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h b/D=
> ynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] DynamicTables: Fix DT PCI interrupt flags parsing
2022-04-27 14:49 [PATCH v1 1/1] DynamicTables: Fix DT PCI interrupt flags parsing PierreGondois
2022-07-18 12:27 ` [edk2-devel] " Sami Mujawar
@ 2022-07-18 16:26 ` Sami Mujawar
1 sibling, 0 replies; 5+ messages in thread
From: Sami Mujawar @ 2022-07-18 16:26 UTC (permalink / raw)
To: Pierre.Gondois, devel, nd
Pushed as 039bdb4d3e96..fc4a132c0e9d
Regards,
Sami Mujawar
On 27/04/2022 03:49 pm, Pierre.Gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> Device Tree PCI interrupt flags use the convention described at
> linux/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
>
> The 3rd cell is the flags, encoded as follows:
> bits[3:0] trigger type and level flags.
> 1 = low-to-high edge triggered
> 2 = high-to-low edge triggered (invalid for SPIs)
> 4 = active high level-sensitive
> 8 = active low level-sensitive (invalid for SPIs).
>
> Fix the incorrect code.
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
> .../Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 2 +-
> DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> index a34018151f2d..d5b1c153e98f 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> @@ -449,7 +449,7 @@ GeneratePrt (
> if ((Index > 0) &&
> (IrqMapInfo->IntcInterrupt.Interrupt >= 32) &&
> (IrqMapInfo->IntcInterrupt.Interrupt < 1020) &&
> - ((IrqMapInfo->IntcInterrupt.Flags & 0x3) != BIT0))
> + ((IrqMapInfo->IntcInterrupt.Flags & 0x3) != 0))
> {
> Status = EFI_INVALID_PARAMETER;
> ASSERT_EFI_ERROR (Status);
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h b/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h
> index f2f425632b10..3f5d131d9ae5 100644
> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h
> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtUtility.h
> @@ -60,7 +60,7 @@
> #define SPI_OFFSET (32U)
> #define DT_PPI_IRQ (1U)
> #define DT_SPI_IRQ (0U)
> -#define DT_IRQ_IS_EDGE_TRIGGERED(x) ((((x) & (BIT0 | BIT2)) != 0))
> +#define DT_IRQ_IS_EDGE_TRIGGERED(x) ((((x) & (BIT0 | BIT1)) != 0))
> #define DT_IRQ_IS_ACTIVE_LOW(x) ((((x) & (BIT1 | BIT3)) != 0))
> #define IRQ_TYPE_OFFSET (0U)
> #define IRQ_NUMBER_OFFSET (1U)
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-18 16:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-27 14:49 [PATCH v1 1/1] DynamicTables: Fix DT PCI interrupt flags parsing PierreGondois
2022-07-18 12:27 ` [edk2-devel] " Sami Mujawar
2022-07-18 12:30 ` PierreGondois
2022-07-18 12:35 ` Sami Mujawar
2022-07-18 16:26 ` Sami Mujawar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox