Hi Mike,
I will send an updated patch with the suggested changes. I will also drop the
IS_xxx macros.
Regards,
Sami Mujawar
-----Original Message-----
From: Kinney, Michael D <michael.d.kin...@intel.com>
Sent: 24 September 2020 11:07 PM
To: Sami Mujawar <sami.muja...@arm.com>; devel@edk2.groups.io; Kinney, Michael
D <michael.d.kin...@intel.com>
Cc: gaolim...@byosoft.com.cn; Liu, Zhiguang <zhiguang....@intel.com>; Alexei
Fedorov <alexei.fedo...@arm.com>; Pierre Gondois <pierre.gond...@arm.com>;
Matteo Carlini <matteo.carl...@arm.com>; Ben Adderson <ben.adder...@arm.com>;
nd <n...@arm.com>
Subject: RE: [PATCH v1 1/2] MdePkg: Definitions for Extended Interrupt Flags
Hi Sami,
The set of define values looks more complex than needed for single bit field
checks.
Perhaps we can simplify to just defining the bit locations and the IS_ macros
check
for 0 or non-zer0 results?
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER_CONSUMER_MASK BIT0
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_MODE_MASK BIT1
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_POLARITY_MASK BIT2
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARABLE_MASK BIT3
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLITY_MASK BIT4
#define IS_EXTENDED_INTERRUPT_CONSUMER(Flag) \
(((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER_CONSUMER_MASK) != 0)
#define IS_EXTENDED_INTERRUPT_EDGE_TRIGGERED(Flag) \
(((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_MODE_MASK) != 0)
Are the IS_ macros really required? I do not see those for other bit
fields in ACPI structs.
Thanks,
Mike
> -----Original Message-----
> From: Sami Mujawar <sami.muja...@arm.com>
> Sent: Tuesday, September 22, 2020 7:08 AM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar <sami.muja...@arm.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>; gaolim...@byosoft.com.cn; Liu, Zhiguang
> <zhiguang....@intel.com>; alexei.fedo...@arm.com; pierre.gond...@arm.com;
> matteo.carl...@arm.com; ben.adder...@arm.com; n...@arm.com
> Subject: [PATCH v1 1/2] MdePkg: Definitions for Extended Interrupt Flags
>
> Add Interrupt Vector Flag definitions for Extended Interrupt
> Descriptor, and macros to test the flags.
> Ref: ACPI specification 6.4.3.6
>
> Signed-off-by: Sami Mujawar <sami.muja...@arm.com>
> ---
> MdePkg/Include/IndustryStandard/Acpi10.h | 85 ++++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
> diff --git a/MdePkg/Include/IndustryStandard/Acpi10.h
> b/MdePkg/Include/IndustryStandard/Acpi10.h
> index
> adeb5ae8c219f31d2403fc7aa217bfb4e1e44694..fa3f0694b9cf80bf9c1a325099a970b9cf8c1426
> 100644
> --- a/MdePkg/Include/IndustryStandard/Acpi10.h
> +++ b/MdePkg/Include/IndustryStandard/Acpi10.h
> @@ -2,6 +2,7 @@
> ACPI 1.0b definitions from the ACPI Specification, revision 1.0b
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2020, Arm Limited. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
>
> @@ -377,6 +378,90 @@ typedef struct {
> #define EFI_ACPI_MEMORY_NON_WRITABLE 0x00
>
> //
> +// Interrupt Vector Flags definitions for Extended Interrupt Descriptor
> +// Ref ACPI specification 6.4.3.6
> +//
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER_CONSUMER_MASK BIT0
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER 0
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_CONSUMER BIT0
> +
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_MODE_MASK BIT1
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_LEVEL_TRIGGERED 0
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_EDGE_TRIGGERED BIT1
> +
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_POLARITY_MASK BIT2
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_ACTIVE_HIGH 0
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_ACTIVE_LOW BIT2
> +
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARABLE_MASK BIT3
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_EXCLUSIVE 0
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARED BIT3
> +
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLITY_MASK BIT4
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_NOT_WAKE_CAPABLE 0
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLE BIT4> +
> +/* Helper macros to test Extended Interrupt Resource descriptor flags.
> +*/
> +
> +/** Test the Extended Interrupt flags to determine if the Device
> + is an Interrupt Consumer or Producer.
> +
> + @param [in] Flag Extended Interrupt Resource descriptor flag.
> +
> + @retval TRUE Device is Interrupt Consumer.
> + @retval FALSE Device is Interrupt Producer.
> +*/
> +#define IS_EXTENDED_INTERRUPT_CONSUMER(Flag) \
> + (((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_CONSUMER) == \
> + EFI_ACPI_EXTENDED_INTERRUPT_FLAG_CONSUMER)
> +
> +/** Test if the Extended Interrupt is Edge or Level triggered.
> +
> + @param [in] Flag Extended Interrupt Resource descriptor flag.
> +
> + @retval TRUE Interrupt is Edge triggered.
> + @retval FALSE Interrupt is Level triggered.
> +*/
> +#define IS_EXTENDED_INTERRUPT_EDGE_TRIGGERED(Flag) \
> + (((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_EDGE_TRIGGERED) == \
> + EFI_ACPI_EXTENDED_INTERRUPT_FLAG_EDGE_TRIGGERED)> +
> +/** Test if the Extended Interrupt is Active Low or Active High.
> +
> + @param [in] Flag Extended Interrupt Resource descriptor flag.
> +
> + @retval TRUE Interrupt is Active Low.
> + @retval FALSE Interrupt is Active High.
> +*/
> +#define IS_EXTENDED_INTERRUPT_ACTIVE_LOW(Flag) \
> + (((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_ACTIVE_LOW) == \
> + EFI_ACPI_EXTENDED_INTERRUPT_FLAG_ACTIVE_LOW)
> +
> +/** Test if the Extended Interrupt is Shared or Exclusive.
> +
> + @param [in] Flag Extended Interrupt Resource descriptor flag.
> +
> + @retval TRUE Interrupt is Shared.
> + @retval FALSE Interrupt is Exclusive.
> +*/
> +#define IS_EXTENDED_INTERRUPT_SHARED(Flag) \
> + (((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARED) == \
> + EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARED)
> +
> +/** Test the Extended Interrupt flags to determine if the Device
> + is Wake capable or not.
> +
> + @param [in] Flag Extended Interrupt Resource descriptor flag.
> +
> + @retval TRUE Interrupt is Wake Capable.
> + @retval FALSE Interrupt is not Wake Capable.
> +*/
> +#define IS_EXTENDED_INTERRUPT_WAKE_CAPABLE(Flag) \
> + (((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLE) == \
> + EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLE)
> +
> +//
> // Ensure proper structure formats
> //
> #pragma pack(1)
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65613): https://edk2.groups.io/g/devel/message/65613
Mute This Topic: https://groups.io/mt/77013535/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
