From: Leif Lindholm <leif.lindholm@linaro.org>
To: evan.lloyd@arm.com
Cc: edk2-devel@ml01.01.org,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Ryan Harkin <ryan.harkin@linaro.org>
Subject: Re: [PATCH 2/4] ArmPkg/ArmGicDxe: expose HardwareInterrupt2 protocol
Date: Mon, 13 Feb 2017 12:21:18 +0000 [thread overview]
Message-ID: <20170213122118.GV16034@bivouac.eciton.net> (raw)
In-Reply-To: <20170209192623.262044-3-evan.lloyd@arm.com>
On Thu, Feb 09, 2017 at 07:26:21PM +0000, evan.lloyd@arm.com wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
Ard - can we have some more commit message, please? :)
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Girish Pathak <girish.pathak@arm.com>
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
> Tested-by: Girish Pathak <girish.pathak@arm.com>
> ---
> ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 1 +
> ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 2 ++
> ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 2 ++
> ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 38 +++++++++++++++++++-
> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 37 ++++++++++++++++++-
> 5 files changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> index e554301c4b28022c805f69242cf6ee979d19abc2..69390638a9afb6aeccad510e7b572450568c1409 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> @@ -48,6 +48,7 @@ [LibraryClasses]
>
> [Protocols]
> gHardwareInterruptProtocolGuid
> + gHardwareInterrupt2ProtocolGuid
> gEfiCpuArchProtocolGuid
>
> [Pcd.common]
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
> index af33aa90b00c6775e10a831d63ed707394862362..2633e1ea194fa67511861a4165d54dad99a6f39b 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
> @@ -24,6 +24,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> #include <Protocol/Cpu.h>
> #include <Protocol/HardwareInterrupt.h>
> +#include <Protocol/HardwareInterrupt2.h>
>
> extern UINTN mGicNumInterrupts;
> extern HARDWARE_INTERRUPT_HANDLER *gRegisteredInterruptHandlers;
> @@ -34,6 +35,7 @@ extern HARDWARE_INTERRUPT_HANDLER *gRegisteredInterruptHandlers;
> EFI_STATUS
> InstallAndRegisterInterruptService (
> IN EFI_HARDWARE_INTERRUPT_PROTOCOL *InterruptProtocol,
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *Interrupt2Protocol,
> IN EFI_CPU_INTERRUPT_HANDLER InterruptHandler,
> IN EFI_EVENT_NOTIFY ExitBootServicesEvent
> );
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
> index be77b8361c5af033fd2889cdb48902af867f321d..ef6746f1ad7afba5bba30fc17774987cf17121b6 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
> @@ -88,6 +88,7 @@ RegisterInterruptSource (
> EFI_STATUS
> InstallAndRegisterInterruptService (
> IN EFI_HARDWARE_INTERRUPT_PROTOCOL *InterruptProtocol,
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *Interrupt2Protocol,
> IN EFI_CPU_INTERRUPT_HANDLER InterruptHandler,
> IN EFI_EVENT_NOTIFY ExitBootServicesEvent
> )
> @@ -104,6 +105,7 @@ InstallAndRegisterInterruptService (
> Status = gBS->InstallMultipleProtocolInterfaces (
> &gHardwareInterruptHandle,
> &gHardwareInterruptProtocolGuid, InterruptProtocol,
> + &gHardwareInterrupt2ProtocolGuid, Interrupt2Protocol,
> NULL
> );
> if (EFI_ERROR (Status)) {
> diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> index b9ecd5543a3e2e0b00fffbcf5543a60567bb5dde..8c4d66125e2e8c7af9898f336ee742ed0aebf058 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> @@ -193,6 +193,41 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol = {
> GicV2EndOfInterrupt
> };
>
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicV2GetTriggerType (
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE *TriggerType
> + )
> +{
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicV2SetTriggerType (
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + IN EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE TriggerType
> + )
> +{
> + return EFI_SUCCESS;
> +}
> +
> +STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol = {
So, this one gets its STATIC revoked in 4/4 - should it just be left
out from the start?
> + (HARDWARE_INTERRUPT2_REGISTER)RegisterInterruptSource,
> + (HARDWARE_INTERRUPT2_ENABLE)GicV2EnableInterruptSource,
> + (HARDWARE_INTERRUPT2_DISABLE)GicV2DisableInterruptSource,
> + (HARDWARE_INTERRUPT2_INTERRUPT_STATE)GicV2GetInterruptSourceState,
> + (HARDWARE_INTERRUPT2_END_OF_INTERRUPT)GicV2EndOfInterrupt,
> + GicV2GetTriggerType,
> + GicV2SetTriggerType
> +};
> +
> +
> /**
> Shutdown our hardware
>
> @@ -311,7 +346,8 @@ GicV2DxeInitialize (
> ArmGicEnableDistributor (mGicDistributorBase);
>
> Status = InstallAndRegisterInterruptService (
> - &gHardwareInterruptV2Protocol, GicV2IrqInterruptHandler, GicV2ExitBootServicesEvent);
> + &gHardwareInterruptV2Protocol, &gHardwareInterrupt2V2Protocol,
> + GicV2IrqInterruptHandler, GicV2ExitBootServicesEvent);
And arguably, since this is the functional change, you could do the
cosmetic change (1 per line) which Girish tried in 4/4.
>
> return Status;
> }
> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> index 8af97a93b1889b33978a7c7fb2a8417c83139142..02deeef78b6d7737172a5992c6decac43cfdd64a 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> @@ -184,6 +184,40 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol = {
> GicV3EndOfInterrupt
> };
>
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicV3GetTriggerType (
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE *TriggerType
> + )
> +{
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicV3SetTriggerType (
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + IN EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE TriggerType
> + )
> +{
> + return EFI_SUCCESS;
> +}
> +
> +STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol = {
Same comment on STATIC. Leave out?
> + (HARDWARE_INTERRUPT2_REGISTER)RegisterInterruptSource,
> + (HARDWARE_INTERRUPT2_ENABLE)GicV3EnableInterruptSource,
> + (HARDWARE_INTERRUPT2_DISABLE)GicV3DisableInterruptSource,
> + (HARDWARE_INTERRUPT2_INTERRUPT_STATE)GicV3GetInterruptSourceState,
> + (HARDWARE_INTERRUPT2_END_OF_INTERRUPT)GicV3EndOfInterrupt,
> + GicV3GetTriggerType,
> + GicV3SetTriggerType
> +};
> +
> /**
> Shutdown our hardware
>
> @@ -331,7 +365,8 @@ GicV3DxeInitialize (
> ArmGicEnableDistributor (mGicDistributorBase);
>
> Status = InstallAndRegisterInterruptService (
> - &gHardwareInterruptV3Protocol, GicV3IrqInterruptHandler, GicV3ExitBootServicesEvent);
> + &gHardwareInterruptV3Protocol, &gHardwareInterrupt2V3Protocol,
> + GicV3IrqInterruptHandler, GicV3ExitBootServicesEvent);
And same comment on 1 per line.
>
> return Status;
> }
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
next prev parent reply other threads:[~2017-02-13 12:21 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-09 19:26 [PATCH 0/4] HardwareInterrupt2 protocol evan.lloyd
2017-02-09 19:26 ` [PATCH 1/4] EmbeddedPkg: introduce " evan.lloyd
2017-02-13 12:26 ` Leif Lindholm
2017-02-09 19:26 ` [PATCH 2/4] ArmPkg/ArmGicDxe: expose " evan.lloyd
2017-02-13 12:21 ` Leif Lindholm [this message]
2017-02-13 12:26 ` Ard Biesheuvel
2017-02-09 19:26 ` [PATCH 3/4] ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type evan.lloyd
2017-02-13 12:30 ` Leif Lindholm
2017-02-09 19:26 ` [PATCH 4/4] ArmPkg:Provide GetTriggerType/SetTriggerType functions evan.lloyd
2017-02-13 12:15 ` Leif Lindholm
2017-02-16 20:27 ` Evan Lloyd
2017-02-16 20:42 ` Ryan Harkin
2017-02-17 12:06 ` Evan Lloyd
2017-02-17 12:30 ` Ryan Harkin
2017-02-17 15:08 ` Alexei Fedorov
2017-02-17 18:18 ` Ard Biesheuvel
2017-02-24 14:06 ` Leif Lindholm
2017-02-13 13:05 ` Ard Biesheuvel
2017-02-16 20:16 ` Evan Lloyd
2017-02-16 20:46 ` Ard Biesheuvel
2017-02-17 11:53 ` Evan Lloyd
2017-02-24 11:26 ` Leif Lindholm
2017-02-13 15:51 ` [PATCH 0/4] HardwareInterrupt2 protocol Evan Lloyd
2017-02-13 17:15 ` Leif Lindholm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170213122118.GV16034@bivouac.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox