From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from foss.arm.com (foss.arm.com [217.140.110.172])
 by mx.groups.io with SMTP id smtpd.web11.26377.1658147431964434945
 for <devel@edk2.groups.io>;
 Mon, 18 Jul 2022 05:30:32 -0700
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com)
Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14])
	by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 20FD71042;
	Mon, 18 Jul 2022 05:30:32 -0700 (PDT)
Received: from [10.34.100.102] (pierre123.nice.arm.com [10.34.100.102])
	by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 279893F70D;
	Mon, 18 Jul 2022 05:30:31 -0700 (PDT)
Message-ID: <686ccf50-5e9a-20e9-0ccb-39af74efe33c@arm.com>
Date: Mon, 18 Jul 2022 14:30:08 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [edk2-devel] [PATCH v1 1/1] DynamicTables: Fix DT PCI interrupt flags parsing
To: Sami Mujawar <sami.mujawar@arm.com>, devel@edk2.groups.io
References: <20220427144931.400413-1-Pierre.Gondois@arm.com>
 <17823.1658147248508568038@groups.io>
From: "PierreGondois" <pierre.gondois@arm.com>
In-Reply-To: <17823.1658147248508568038@groups.io>
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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
>