From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22b.google.com (mail-wm0-x22b.google.com [IPv6:2a00:1450:400c:c09::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D1B3E82114 for ; Mon, 13 Feb 2017 04:26:52 -0800 (PST) Received: by mail-wm0-x22b.google.com with SMTP id v77so92317970wmv.0 for ; Mon, 13 Feb 2017 04:26:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=xKS0/OoV+/92ehhHxwDp/uIG/xZXB2orJFFxHTNKKWI=; b=jJ40GA949nX1X8stX5z2NU/UfHDmfc5ZKawpxuU6v8ecQBXPAHT0w+DTGrRhJ2X71G oamQ7O2DYPa9owTj4WWfDdkSoon+zMODBOOtO4vBFjYqdSF8yrFL2Z1MQon1gjVEftqP KMjdl50GvNEp9P6+31nlmTf20fA+BP/A04fcE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=xKS0/OoV+/92ehhHxwDp/uIG/xZXB2orJFFxHTNKKWI=; b=OZ/WP4mw8u3LWiOxNOd4eBJfRpBSSvNuLqrYgZjyp+DS/KzsEc2+aGY4N7f+TK2jb+ QZUlyaWpgpiWxysXzcT7bLLyJ86IYjOcwIuRyqG8xUqUyOOtdK9iifYt82mfrfNi77T5 vLnwDI0bSWSEOXrGMVEvoCBLNG/lEtFLA8XHL0+jRIcpSF4NzGJmdq974knF6Oda49dL 4QlVws9tUWfTPM/DuOh8oh3Oin8qqpx/ZUtGjEo7wXFKGalji1yvHeCtT6R3F62Yud/E z9A6vq1/TFeUQLDVLPwn0hnH4aGpBajH8SdU050BIllmt3zJmxrV+GtCjAwz3NRWh5qO iAMg== X-Gm-Message-State: AMke39k2iNQ0iNHlF722VpBFq0ljNlx7duwYXnvjF+1hAHoWZlwJOBQoM1UfGSRMl/RnUBeO X-Received: by 10.28.8.213 with SMTP id 204mr18866029wmi.100.1486988811401; Mon, 13 Feb 2017 04:26:51 -0800 (PST) Received: from [10.82.131.179] ([195.76.49.202]) by smtp.gmail.com with ESMTPSA id i189sm5219964wmg.7.2017.02.13.04.26.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 13 Feb 2017 04:26:50 -0800 (PST) Mime-Version: 1.0 (1.0) From: Ard Biesheuvel X-Mailer: iPhone Mail (14D27) In-Reply-To: <20170213122118.GV16034@bivouac.eciton.net> Date: Mon, 13 Feb 2017 13:26:49 +0100 Cc: evan.lloyd@arm.com, edk2-devel@ml01.01.org, Ryan Harkin Message-Id: <588E0A4A-89A9-4136-B8CE-59FEC94DDAE7@linaro.org> References: <20170209192623.262044-1-evan.lloyd@arm.com> <20170209192623.262044-3-evan.lloyd@arm.com> <20170213122118.GV16034@bivouac.eciton.net> To: Leif Lindholm Subject: Re: [PATCH 2/4] ArmPkg/ArmGicDxe: expose HardwareInterrupt2 protocol X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Feb 2017 12:26:53 -0000 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable > On 13 Feb 2017, at 13:21, Leif Lindholm wrote: >=20 >> On Thu, Feb 09, 2017 at 07:26:21PM +0000, evan.lloyd@arm.com wrote: >> From: Ard Biesheuvel >>=20 >=20 > Ard - can we have some more commit message, please? :) >=20 This patch was only a PoC, and needs to be merged with 4/4 imo. The STATICs should be kept, and the other cosmetic changes could also be dro= pped afaict >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel >> Signed-off-by: Girish Pathak >> Signed-off-by: Evan Lloyd >> Tested-by: Girish Pathak >> --- >> 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(-) >>=20 >> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/= ArmGicDxe.inf >> index e554301c4b28022c805f69242cf6ee979d19abc2..69390638a9afb6aeccad510e7= b572450568c1409 100644 >> --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf >> +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf >> @@ -48,6 +48,7 @@ [LibraryClasses] >>=20 >> [Protocols] >> gHardwareInterruptProtocolGuid >> + gHardwareInterrupt2ProtocolGuid >> gEfiCpuArchProtocolGuid >>=20 >> [Pcd.common] >> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h b/ArmPkg/Drivers/ArmGic/Ar= mGicDxe.h >> index af33aa90b00c6775e10a831d63ed707394862362..2633e1ea194fa67511861a416= 5d54dad99a6f39b 100644 >> --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h >> +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h >> @@ -24,6 +24,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITH= ER EXPRESS OR IMPLIED. >>=20 >> #include >> #include >> +#include >>=20 >> extern UINTN mGicNumInterrupts; >> extern HARDWARE_INTERRUPT_HANDLER *gRegisteredInterruptHandlers; >> @@ -34,6 +35,7 @@ extern HARDWARE_INTERRUPT_HANDLER *gRegisteredInterrupt= Handlers; >> 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/Arm= Gic/ArmGicCommonDxe.c >> index be77b8361c5af033fd2889cdb48902af867f321d..ef6746f1ad7afba5bba30fc17= 774987cf17121b6 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 =3D gBS->InstallMultipleProtocolInterfaces ( >> &gHardwareInterruptHandle, >> &gHardwareInterruptProtocolGuid, InterruptProtocol, >> + &gHardwareInterrupt2ProtocolGuid, Interrupt2Protocol, >> NULL >> ); >> if (EFI_ERROR (Status)) { >> diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c b/ArmPkg/Drivers/A= rmGic/GicV2/ArmGicV2Dxe.c >> index b9ecd5543a3e2e0b00fffbcf5543a60567bb5dde..8c4d66125e2e8c7af9898f336= ee742ed0aebf058 100644 >> --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c >> +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c >> @@ -193,6 +193,41 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2= Protocol =3D { >> GicV2EndOfInterrupt >> }; >>=20 >> +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 =3D= { >=20 > So, this one gets its STATIC revoked in 4/4 - should it just be left > out from the start? >=20 >> + (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 >>=20 >> @@ -311,7 +346,8 @@ GicV2DxeInitialize ( >> ArmGicEnableDistributor (mGicDistributorBase); >>=20 >> Status =3D InstallAndRegisterInterruptService ( >> - &gHardwareInterruptV2Protocol, GicV2IrqInterruptHandler, GicV2= ExitBootServicesEvent); >> + &gHardwareInterruptV2Protocol, &gHardwareInterrupt2V2Protoc= ol, >> + GicV2IrqInterruptHandler, GicV2ExitBootServicesEvent); >=20 > And arguably, since this is the functional change, you could do the > cosmetic change (1 per line) which Girish tried in 4/4. >=20 >>=20 >> return Status; >> } >> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/A= rmGic/GicV3/ArmGicV3Dxe.c >> index 8af97a93b1889b33978a7c7fb2a8417c83139142..02deeef78b6d7737172a5992c= 6decac43cfdd64a 100644 >> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >> @@ -184,6 +184,40 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3= Protocol =3D { >> GicV3EndOfInterrupt >> }; >>=20 >> +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 =3D= { >=20 > Same comment on STATIC. Leave out? >=20 >> + (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 >>=20 >> @@ -331,7 +365,8 @@ GicV3DxeInitialize ( >> ArmGicEnableDistributor (mGicDistributorBase); >>=20 >> Status =3D InstallAndRegisterInterruptService ( >> - &gHardwareInterruptV3Protocol, GicV3IrqInterruptHandler, GicV3= ExitBootServicesEvent); >> + &gHardwareInterruptV3Protocol, &gHardwareInterrupt2V3Protoc= ol, >> + GicV3IrqInterruptHandler, GicV3ExitBootServicesEvent); >=20 > And same comment on 1 per line. >=20 >>=20 >> return Status; >> } >> --=20 >> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") >>=20