* [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM
@ 2017-09-21 16:23 evan.lloyd
2017-09-21 16:23 ` [PATCH 1/4] ArmPkg: Tidy GIC code before changes evan.lloyd
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: evan.lloyd @ 2017-09-21 16:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Ard Biesheuvel, Leif Lindholm, Matteo Carlini, nd
From: EvanLloyd <evan.lloyd@arm.com>
This v4 series of patches corrects a problem detected on the ARM Juno
platform that is actually generic (at least to ARM GIC platforms).
The HardwareInterrupt protocol had no means of handling characteristics
like Edge/Level triggered and polarity.
A new HardwareInterrupt2 protocol (provided by Ard) is added, and code
changed to utilise the new capabilities.
The code is available for examination on Github at:
https://github.com/EvanLloyd/tianocore/tree/376_irqtype_v4
v4 responds to further comments from maintainers, being mainly
cosmetic changes.
Note: Significant defects exist in the (original) Watchdog handling,
and a new patch will follow.
Ard Biesheuvel (3):
EmbeddedPkg: Introduce HardwareInterrupt2 protocol
ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type
Evan Lloyd (1):
ArmPkg: Tidy GIC code before changes.
EmbeddedPkg/EmbeddedPkg.dec | 1 +
ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 3 +-
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf | 6 +-
ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 31 ++-
ArmPkg/Include/Library/ArmGicLib.h | 34 +--
EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h | 182 ++++++++++++++++
ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 76 +++++--
ArmPkg/Drivers/ArmGic/ArmGicLib.c | 73 +++++--
ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 195 ++++++++++++++---
ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 218 +++++++++++++++++---
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 146 +++++++------
11 files changed, 779 insertions(+), 186 deletions(-)
create mode 100644 EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] ArmPkg: Tidy GIC code before changes.
2017-09-21 16:23 [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM evan.lloyd
@ 2017-09-21 16:23 ` evan.lloyd
2017-09-21 16:23 ` [PATCH 2/4] EmbeddedPkg: Introduce HardwareInterrupt2 protocol evan.lloyd
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: evan.lloyd @ 2017-09-21 16:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Ard Biesheuvel, Leif Lindholm, Matteo Carlini, nd
From: Evan Lloyd <evan.lloyd@arm.com>
This change is purely cosmetic, to tidy some code before change.
Mods involve:
Re-order #includes
Reformat comments.
Use ns consistently (always "100ns" not sometimes "100 nS")
Split overlength code lines.
Make protocol functions STATIC.
Remove "Horor vacui" comments.
Rationalize GIC register address calculations
Replace explicit test and assert with ASSERT_EFI_ERROR.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Girish Pathak <girish.pathak@arm.com>
Signed-off-by: Alexei Fedorov <alexei.fedorov@arm.com>
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
---
Notes:
376_irqtype_v4
[Leif Lindholm]
- Change SOB order
- Tidy // replacement
- Rework gRegisteredInterruptHandlers allocation
- Reformat CPU affinity comment
- Use ISENABLER_ADDRESS macro, etc
- moved stuff previously in ArmPkg: Tidy up GenericWatchdogDxe.c
- Did NOT include Move | to end of line before
ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 8 +-
ArmPkg/Include/Library/ArmGicLib.h | 23 ++--
ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 33 +++---
ArmPkg/Drivers/ArmGic/ArmGicLib.c | 73 +++++++++---
ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 58 ++++++----
ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 82 ++++++++-----
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 121 +++++++++-----------
7 files changed, 226 insertions(+), 172 deletions(-)
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
index af33aa90b00c6775e10a831d63ed707394862362..76945282d9b9bbf6da872a408a56387ee321a3b5 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
+++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.h
@@ -1,6 +1,6 @@
/*++
-Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
+Copyright (c) 2013-2017, ARM Ltd. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -28,9 +28,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
extern UINTN mGicNumInterrupts;
extern HARDWARE_INTERRUPT_HANDLER *gRegisteredInterruptHandlers;
-//
// Common API
-//
EFI_STATUS
InstallAndRegisterInterruptService (
IN EFI_HARDWARE_INTERRUPT_PROTOCOL *InterruptProtocol,
@@ -46,18 +44,14 @@ RegisterInterruptSource (
IN HARDWARE_INTERRUPT_HANDLER Handler
);
-//
// GicV2 API
-//
EFI_STATUS
GicV2DxeInitialize (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
);
-//
// GicV3 API
-//
EFI_STATUS
GicV3DxeInitialize (
IN EFI_HANDLE ImageHandle,
diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h
index 4364f3ffef464596f64cf59881d703cf54cf0ddd..a4ede82bc6c07733f880a12fff54001590a0518a 100644
--- a/ArmPkg/Include/Library/ArmGicLib.h
+++ b/ArmPkg/Include/Library/ArmGicLib.h
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+* Copyright (c) 2011-2017, ARM Limited. All rights reserved.
*
* This program and the accompanying materials
* are licensed and made available under the terms and conditions of the BSD License
@@ -17,9 +17,7 @@
#include <Library/ArmGicArchLib.h>
-//
// GIC Distributor
-//
#define ARM_GIC_ICDDCR 0x000 // Distributor Control Register
#define ARM_GIC_ICDICTR 0x004 // Interrupt Controller Type Register
#define ARM_GIC_ICDIIDR 0x008 // Implementer Identification Register
@@ -51,9 +49,7 @@
#define ARM_GIC_ICDDCR_ARE (1 << 4) // Affinity Routing Enable (ARE)
#define ARM_GIC_ICDDCR_DS (1 << 6) // Disable Security (DS)
-//
// GIC Redistributor
-//
#define ARM_GICR_CTLR_FRAME_SIZE SIZE_64KB
#define ARM_GICR_SGI_PPI_FRAME_SIZE SIZE_64KB
@@ -65,9 +61,7 @@
#define ARM_GICR_ISENABLER 0x0100 // Interrupt Set-Enable Registers
#define ARM_GICR_ICENABLER 0x0180 // Interrupt Clear-Enable Registers
-//
// GIC Cpu interface
-//
#define ARM_GIC_ICCICR 0x00 // CPU Interface Control Register
#define ARM_GIC_ICCPMR 0x04 // Interrupt Priority Mask Register
#define ARM_GIC_ICCBPR 0x08 // Binary Point Register
@@ -104,9 +98,7 @@ ArmGicGetInterfaceIdentification (
IN INTN GicInterruptInterfaceBase
);
-//
// GIC Secure interfaces
-//
VOID
EFIAPI
ArmGicSetupNonSecure (
@@ -170,7 +162,8 @@ ArmGicSendSgiTo (
* in the GICv3 the register value is only the InterruptId.
*
* @param GicInterruptInterfaceBase Base Address of the GIC CPU Interface
- * @param InterruptId InterruptId read from the Interrupt Acknowledge Register
+ * @param InterruptId InterruptId read from the Interrupt
+ * Acknowledge Register
*
* @retval value returned by the Interrupt Acknowledge Register
*
@@ -220,12 +213,12 @@ ArmGicIsInterruptEnabled (
IN UINTN Source
);
-//
// GIC revision 2 specific declarations
-//
-// Interrupts from 1020 to 1023 are considered as special interrupts (eg: spurious interrupts)
-#define ARM_GIC_IS_SPECIAL_INTERRUPTS(Interrupt) (((Interrupt) >= 1020) && ((Interrupt) <= 1023))
+// Interrupts from 1020 to 1023 are considered as special interrupts
+// (eg: spurious interrupts)
+#define ARM_GIC_IS_SPECIAL_INTERRUPTS(Interrupt) \
+ (((Interrupt) >= 1020) && ((Interrupt) <= 1023))
VOID
EFIAPI
@@ -260,9 +253,7 @@ ArmGicV2EndOfInterrupt (
IN UINTN Source
);
-//
// GIC revision 3 specific declarations
-//
#define ICC_SRE_EL2_SRE (1 << 0)
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
index be77b8361c5af033fd2889cdb48902af867f321d..7ca3ca8f52afe52c827ca82b749b665184813fc1 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
@@ -1,6 +1,6 @@
/*++
-Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
+Copyright (c) 2013-2017, ARM Ltd. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -28,14 +28,10 @@ ExitBootServicesEvent (
IN VOID *Context
);
-//
// Making this global saves a few bytes in image size
-//
EFI_HANDLE gHardwareInterruptHandle = NULL;
-//
// Notifications
-//
EFI_EVENT EfiExitBootServicesEvent = (EFI_EVENT)NULL;
// Maximum Number of Interrupts
@@ -94,48 +90,55 @@ InstallAndRegisterInterruptService (
{
EFI_STATUS Status;
EFI_CPU_ARCH_PROTOCOL *Cpu;
+ CONST UINTN RihArraySize =
+ (sizeof(HARDWARE_INTERRUPT_HANDLER) * mGicNumInterrupts);
// Initialize the array for the Interrupt Handlers
- gRegisteredInterruptHandlers = (HARDWARE_INTERRUPT_HANDLER*)AllocateZeroPool (sizeof(HARDWARE_INTERRUPT_HANDLER) * mGicNumInterrupts);
+ gRegisteredInterruptHandlers = AllocateZeroPool (RihArraySize);
if (gRegisteredInterruptHandlers == NULL) {
return EFI_OUT_OF_RESOURCES;
}
Status = gBS->InstallMultipleProtocolInterfaces (
&gHardwareInterruptHandle,
- &gHardwareInterruptProtocolGuid, InterruptProtocol,
+ &gHardwareInterruptProtocolGuid,
+ InterruptProtocol,
NULL
);
if (EFI_ERROR (Status)) {
return Status;
}
- //
// Get the CPU protocol that this driver requires.
- //
Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&Cpu);
if (EFI_ERROR (Status)) {
return Status;
}
- //
// Unregister the default exception handler.
- //
Status = Cpu->RegisterInterruptHandler (Cpu, ARM_ARCH_EXCEPTION_IRQ, NULL);
if (EFI_ERROR (Status)) {
return Status;
}
- //
// Register to receive interrupts
- //
- Status = Cpu->RegisterInterruptHandler (Cpu, ARM_ARCH_EXCEPTION_IRQ, InterruptHandler);
+ Status = Cpu->RegisterInterruptHandler (
+ Cpu,
+ ARM_ARCH_EXCEPTION_IRQ,
+ InterruptHandler
+ );
if (EFI_ERROR (Status)) {
return Status;
}
// Register for an ExitBootServicesEvent
- Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_NOTIFY, ExitBootServicesEvent, NULL, &EfiExitBootServicesEvent);
+ Status = gBS->CreateEvent (
+ EVT_SIGNAL_EXIT_BOOT_SERVICES,
+ TPL_NOTIFY,
+ ExitBootServicesEvent,
+ NULL,
+ &EfiExitBootServicesEvent
+ );
return Status;
}
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index e658e9bff5d8107b3914bdf1e9e1e51a4e4d4cd7..4d4dc940e150627a6e08aa61ad13278238cf0530 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+* Copyright (c) 2011-2017, ARM Limited. All rights reserved.
*
* This program and the accompanying materials
* are licensed and made available under the terms and conditions of the BSD License
@@ -19,6 +19,13 @@
#include <Library/IoLib.h>
#include <Library/PcdLib.h>
+
+#define ISENABLER_ADDRESS(base,offset) ((base) + \
+ ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ISENABLER + (4 * offset))
+
+#define ICENABLER_ADDRESS(base,offset) ((base) + \
+ ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ICENABLER + (4 * offset))
+
/**
*
* Return whether the Source interrupt index refers to a shared interrupt (SPI)
@@ -55,13 +62,17 @@ GicGetCpuRedistributorBase (
UINTN GicCpuRedistributorBase;
MpId = ArmReadMpidr ();
- // Define CPU affinity as Affinity0[0:8], Affinity1[9:15], Affinity2[16:23], Affinity3[24:32]
+ // Define CPU affinity as:
+ // Affinity0[0:8], Affinity1[9:15], Affinity2[16:23], Affinity3[24:32]
// whereas Affinity3 is defined at [32:39] in MPIDR
- CpuAffinity = (MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2)) | ((MpId & ARM_CORE_AFF3) >> 8);
+ CpuAffinity = (MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2))
+ | ((MpId & ARM_CORE_AFF3) >> 8);
if (Revision == ARM_GIC_ARCH_REVISION_3) {
- // 2 x 64KB frame: Redistributor control frame + SGI Control & Generation frame
- GicRedistributorGranularity = ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_SGI_PPI_FRAME_SIZE;
+ // 2 x 64KB frame:
+ // Redistributor control frame + SGI Control & Generation frame
+ GicRedistributorGranularity = ARM_GICR_CTLR_FRAME_SIZE
+ + ARM_GICR_SGI_PPI_FRAME_SIZE;
} else {
ASSERT_EFI_ERROR (EFI_UNSUPPORTED);
return 0;
@@ -112,7 +123,10 @@ ArmGicSendSgiTo (
IN INTN SgiId
)
{
- MmioWrite32 (GicDistributorBase + ARM_GIC_ICDSGIR, ((TargetListFilter & 0x3) << 24) | ((CPUTargetList & 0xFF) << 16) | SgiId);
+ MmioWrite32 (
+ GicDistributorBase + ARM_GIC_ICDSGIR,
+ ((TargetListFilter & 0x3) << 24) | ((CPUTargetList & 0xFF) << 16) | SgiId
+ );
}
/*
@@ -123,7 +137,8 @@ ArmGicSendSgiTo (
* in the GICv3 the register value is only the InterruptId.
*
* @param GicInterruptInterfaceBase Base Address of the GIC CPU Interface
- * @param InterruptId InterruptId read from the Interrupt Acknowledge Register
+ * @param InterruptId InterruptId read from the Interrupt
+ * Acknowledge Register
*
* @retval value returned by the Interrupt Acknowledge Register
*
@@ -200,16 +215,25 @@ ArmGicEnableInterrupt (
FeaturePcdGet (PcdArmGicV3WithV2Legacy) ||
SourceIsSpi (Source)) {
// Write set-enable register
- MmioWrite32 (GicDistributorBase + ARM_GIC_ICDISER + (4 * RegOffset), 1 << RegShift);
+ MmioWrite32 (
+ GicDistributorBase + ARM_GIC_ICDISER + (4 * RegOffset),
+ 1 << RegShift
+ );
} else {
- GicCpuRedistributorBase = GicGetCpuRedistributorBase (GicRedistributorBase, Revision);
+ GicCpuRedistributorBase = GicGetCpuRedistributorBase (
+ GicRedistributorBase,
+ Revision
+ );
if (GicCpuRedistributorBase == 0) {
ASSERT_EFI_ERROR (EFI_NOT_FOUND);
return;
}
// Write set-enable register
- MmioWrite32 (GicCpuRedistributorBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ISENABLER + (4 * RegOffset), 1 << RegShift);
+ MmioWrite32 (
+ ISENABLER_ADDRESS(GicCpuRedistributorBase, RegOffset),
+ 1 << RegShift
+ );
}
}
@@ -235,15 +259,24 @@ ArmGicDisableInterrupt (
FeaturePcdGet (PcdArmGicV3WithV2Legacy) ||
SourceIsSpi (Source)) {
// Write clear-enable register
- MmioWrite32 (GicDistributorBase + ARM_GIC_ICDICER + (4 * RegOffset), 1 << RegShift);
+ MmioWrite32 (
+ GicDistributorBase + ARM_GIC_ICDICER + (4 * RegOffset),
+ 1 << RegShift
+ );
} else {
- GicCpuRedistributorBase = GicGetCpuRedistributorBase (GicRedistributorBase, Revision);
+ GicCpuRedistributorBase = GicGetCpuRedistributorBase (
+ GicRedistributorBase,
+ Revision
+ );
if (GicCpuRedistributorBase == 0) {
return;
}
// Write clear-enable register
- MmioWrite32 (GicCpuRedistributorBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ICENABLER + (4 * RegOffset), 1 << RegShift);
+ MmioWrite32 (
+ ICENABLER_ADDRESS(GicCpuRedistributorBase, RegOffset),
+ 1 << RegShift
+ );
}
}
@@ -269,15 +302,23 @@ ArmGicIsInterruptEnabled (
if ((Revision == ARM_GIC_ARCH_REVISION_2) ||
FeaturePcdGet (PcdArmGicV3WithV2Legacy) ||
SourceIsSpi (Source)) {
- Interrupts = ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDISER + (4 * RegOffset)) & (1 << RegShift)) != 0);
+ Interrupts = ((MmioRead32 (
+ GicDistributorBase + ARM_GIC_ICDISER + (4 * RegOffset)
+ )
+ & (1 << RegShift)) != 0);
} else {
- GicCpuRedistributorBase = GicGetCpuRedistributorBase (GicRedistributorBase, Revision);
+ GicCpuRedistributorBase = GicGetCpuRedistributorBase (
+ GicRedistributorBase,
+ Revision
+ );
if (GicCpuRedistributorBase == 0) {
return 0;
}
// Read set-enable register
- Interrupts = MmioRead32 (GicCpuRedistributorBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ISENABLER + (4 * RegOffset));
+ Interrupts = MmioRead32 (
+ ISENABLER_ADDRESS(GicCpuRedistributorBase, RegOffset)
+ );
}
return ((Interrupts & (1 << RegShift)) != 0);
diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
index b9ecd5543a3e2e0b00fffbcf5543a60567bb5dde..3d5d034efbb1bf2e1e84841cab4175bc607845c2 100644
--- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
@@ -2,7 +2,7 @@
Copyright (c) 2009, Hewlett-Packard Company. All rights reserved.<BR>
Portions copyright (c) 2010, Apple Inc. All rights reserved.<BR>
-Portions copyright (c) 2011-2016, ARM Ltd. All rights reserved.<BR>
+Portions copyright (c) 2011-2017, ARM Ltd. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -43,6 +43,7 @@ STATIC UINT32 mGicDistributorBase;
@retval EFI_UNSUPPORTED Source interrupt is not supported
**/
+STATIC
EFI_STATUS
EFIAPI
GicV2EnableInterruptSource (
@@ -70,6 +71,7 @@ GicV2EnableInterruptSource (
@retval EFI_UNSUPPORTED Source interrupt is not supported
**/
+STATIC
EFI_STATUS
EFIAPI
GicV2DisableInterruptSource (
@@ -98,6 +100,7 @@ GicV2DisableInterruptSource (
@retval EFI_UNSUPPORTED Source interrupt is not supported
**/
+STATIC
EFI_STATUS
EFIAPI
GicV2GetInterruptSourceState (
@@ -127,6 +130,7 @@ GicV2GetInterruptSourceState (
@retval EFI_UNSUPPORTED Source interrupt is not supported
**/
+STATIC
EFI_STATUS
EFIAPI
GicV2EndOfInterrupt (
@@ -147,13 +151,15 @@ GicV2EndOfInterrupt (
EFI_CPU_INTERRUPT_HANDLER that is called when a processor interrupt occurs.
@param InterruptType Defines the type of interrupt or exception that
- occurred on the processor.This parameter is processor architecture specific.
+ occurred on the processor.This parameter is
+ processor architecture specific.
@param SystemContext A pointer to the processor context when
the interrupt occurred on the processor.
@return None
**/
+STATIC
VOID
EFIAPI
GicV2IrqInterruptHandler (
@@ -166,9 +172,10 @@ GicV2IrqInterruptHandler (
GicInterrupt = ArmGicV2AcknowledgeInterrupt (mGicInterruptInterfaceBase);
- // Special Interrupts (ID1020-ID1023) have an Interrupt ID greater than the number of interrupt (ie: Spurious interrupt).
+ // Special Interrupts (ID1020-ID1023) have an Interrupt ID greater than the
+ // number of interrupt (ie: Spurious interrupt).
if ((GicInterrupt & ARM_GIC_ICCIAR_ACKINTID) >= mGicNumInterrupts) {
- // The special interrupt do not need to be acknowledge
+ // The special interrupts do not need to be acknowledged
return;
}
@@ -177,14 +184,12 @@ GicV2IrqInterruptHandler (
// Call the registered interrupt handler.
InterruptHandler (GicInterrupt, SystemContext);
} else {
- DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));
+ DEBUG ((DEBUG_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));
GicV2EndOfInterrupt (&gHardwareInterruptV2Protocol, GicInterrupt);
}
}
-//
// The protocol instance produced by this driver
-//
EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol = {
RegisterInterruptSource,
GicV2EnableInterruptSource,
@@ -196,12 +201,13 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol = {
/**
Shutdown our hardware
- DXE Core will disable interrupts and turn off the timer and disable interrupts
- after all the event handlers have run.
+ DXE Core will disable interrupts and turn off the timer and disable
+ interrupts after all the event handlers have run.
@param[in] Event The Event that is being processed
@param[in] Context Event Context
**/
+STATIC
VOID
EFIAPI
GicV2ExitBootServicesEvent (
@@ -256,7 +262,8 @@ GicV2DxeInitialize (
UINTN RegShift;
UINT32 CpuTarget;
- // Make sure the Interrupt Controller Protocol is not already installed in the system.
+ // Make sure the Interrupt Controller Protocol is not already installed in
+ // the system.
ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gHardwareInterruptProtocolGuid);
mGicInterruptInterfaceBase = PcdGet64 (PcdGicInterruptInterfaceBase);
@@ -276,25 +283,27 @@ GicV2DxeInitialize (
);
}
- //
// Targets the interrupts to the Primary Cpu
- //
- // Only Primary CPU will run this code. We can identify our GIC CPU ID by reading
- // the GIC Distributor Target register. The 8 first GICD_ITARGETSRn are banked to each
- // connected CPU. These 8 registers hold the CPU targets fields for interrupts 0-31.
- // More Info in the GIC Specification about "Interrupt Processor Targets Registers"
- //
- // Read the first Interrupt Processor Targets Register (that corresponds to the 4
- // first SGIs)
+ // Only Primary CPU will run this code. We can identify our GIC CPU ID by
+ // reading the GIC Distributor Target register. The 8 first GICD_ITARGETSRn
+ // are banked to each connected CPU. These 8 registers hold the CPU targets
+ // fields for interrupts 0-31. More Info in the GIC Specification about
+ // "Interrupt Processor Targets Registers"
+
+ // Read the first Interrupt Processor Targets Register (that corresponds to
+ // the 4 first SGIs)
CpuTarget = MmioRead32 (mGicDistributorBase + ARM_GIC_ICDIPTR);
- // The CPU target is a bit field mapping each CPU to a GIC CPU Interface. This value
- // is 0 when we run on a uniprocessor platform.
+ // The CPU target is a bit field mapping each CPU to a GIC CPU Interface.
+ // This value is 0 when we run on a uniprocessor platform.
if (CpuTarget != 0) {
// The 8 first Interrupt Processor Targets Registers are read-only
for (Index = 8; Index < (mGicNumInterrupts / 4); Index++) {
- MmioWrite32 (mGicDistributorBase + ARM_GIC_ICDIPTR + (Index * 4), CpuTarget);
+ MmioWrite32 (
+ mGicDistributorBase + ARM_GIC_ICDIPTR + (Index * 4),
+ CpuTarget
+ );
}
}
@@ -311,7 +320,10 @@ GicV2DxeInitialize (
ArmGicEnableDistributor (mGicDistributorBase);
Status = InstallAndRegisterInterruptService (
- &gHardwareInterruptV2Protocol, GicV2IrqInterruptHandler, GicV2ExitBootServicesEvent);
+ &gHardwareInterruptV2Protocol,
+ GicV2IrqInterruptHandler,
+ GicV2ExitBootServicesEvent
+ );
return Status;
}
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
index 8af97a93b1889b33978a7c7fb2a8417c83139142..50b51ffcf57b23b110899c7c55237cf85b61d196 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2011-2016, ARM Limited. All rights reserved.
+* Copyright (c) 2011-2017, ARM Limited. All rights reserved.
*
* This program and the accompanying materials
* are licensed and made available under the terms and conditions of the BSD License
@@ -33,6 +33,7 @@ STATIC UINTN mGicRedistributorsBase;
@retval EFI_DEVICE_ERROR Hardware could not be programmed.
**/
+STATIC
EFI_STATUS
EFIAPI
GicV3EnableInterruptSource (
@@ -60,6 +61,7 @@ GicV3EnableInterruptSource (
@retval EFI_DEVICE_ERROR Hardware could not be programmed.
**/
+STATIC
EFI_STATUS
EFIAPI
GicV3DisableInterruptSource (
@@ -88,6 +90,7 @@ GicV3DisableInterruptSource (
@retval EFI_DEVICE_ERROR InterruptState is not valid
**/
+STATIC
EFI_STATUS
EFIAPI
GicV3GetInterruptSourceState (
@@ -101,7 +104,11 @@ GicV3GetInterruptSourceState (
return EFI_UNSUPPORTED;
}
- *InterruptState = ArmGicIsInterruptEnabled (mGicDistributorBase, mGicRedistributorsBase, Source);
+ *InterruptState = ArmGicIsInterruptEnabled (
+ mGicDistributorBase,
+ mGicRedistributorsBase,
+ Source
+ );
return EFI_SUCCESS;
}
@@ -117,6 +124,7 @@ GicV3GetInterruptSourceState (
@retval EFI_DEVICE_ERROR Hardware could not be programmed.
**/
+STATIC
EFI_STATUS
EFIAPI
GicV3EndOfInterrupt (
@@ -137,13 +145,15 @@ GicV3EndOfInterrupt (
EFI_CPU_INTERRUPT_HANDLER that is called when a processor interrupt occurs.
@param InterruptType Defines the type of interrupt or exception that
- occurred on the processor.This parameter is processor architecture specific.
+ occurred on the processor. This parameter is
+ processor architecture specific.
@param SystemContext A pointer to the processor context when
the interrupt occurred on the processor.
@return None
**/
+STATIC
VOID
EFIAPI
GicV3IrqInterruptHandler (
@@ -168,14 +178,12 @@ GicV3IrqInterruptHandler (
// Call the registered interrupt handler.
InterruptHandler (GicInterrupt, SystemContext);
} else {
- DEBUG ((EFI_D_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));
+ DEBUG ((DEBUG_ERROR, "Spurious GIC interrupt: 0x%x\n", GicInterrupt));
GicV3EndOfInterrupt (&gHardwareInterruptV3Protocol, GicInterrupt);
}
}
-//
// The protocol instance produced by this driver
-//
EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol = {
RegisterInterruptSource,
GicV3EnableInterruptSource,
@@ -242,17 +250,16 @@ GicV3DxeInitialize (
UINT64 CpuTarget;
UINT64 MpId;
- // Make sure the Interrupt Controller Protocol is not already installed in the system.
+ // Make sure the Interrupt Controller Protocol is not already installed in
+ // the system.
ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gHardwareInterruptProtocolGuid);
mGicDistributorBase = PcdGet64 (PcdGicDistributorBase);
mGicRedistributorsBase = PcdGet64 (PcdGicRedistributorsBase);
mGicNumInterrupts = ArmGicGetMaxNumInterrupts (mGicDistributorBase);
- //
// We will be driving this GIC in native v3 mode, i.e., with Affinity
// Routing enabled. So ensure that the ARE bit is set.
- //
if (!FeaturePcdGet (PcdArmGicV3WithV2Legacy)) {
MmioOr32 (mGicDistributorBase + ARM_GIC_ICDDCR, ARM_GIC_ICDDCR_ARE);
}
@@ -270,51 +277,65 @@ GicV3DxeInitialize (
);
}
- //
// Targets the interrupts to the Primary Cpu
- //
if (FeaturePcdGet (PcdArmGicV3WithV2Legacy)) {
- // Only Primary CPU will run this code. We can identify our GIC CPU ID by reading
- // the GIC Distributor Target register. The 8 first GICD_ITARGETSRn are banked to each
- // connected CPU. These 8 registers hold the CPU targets fields for interrupts 0-31.
- // More Info in the GIC Specification about "Interrupt Processor Targets Registers"
- //
- // Read the first Interrupt Processor Targets Register (that corresponds to the 4
- // first SGIs)
+ // Only Primary CPU will run this code. We can identify our GIC CPU ID by
+ // reading the GIC Distributor Target register. The 8 first
+ // GICD_ITARGETSRn are banked to each connected CPU. These 8 registers
+ // hold the CPU targets fields for interrupts 0-31. More Info in the GIC
+ // Specification about "Interrupt Processor Targets Registers"
+
+ // Read the first Interrupt Processor Targets Register (that corresponds
+ // to the 4 first SGIs)
CpuTarget = MmioRead32 (mGicDistributorBase + ARM_GIC_ICDIPTR);
- // The CPU target is a bit field mapping each CPU to a GIC CPU Interface. This value
- // is 0 when we run on a uniprocessor platform.
+ // The CPU target is a bit field mapping each CPU to a GIC CPU Interface.
+ // This value is 0 when we run on a uniprocessor platform.
if (CpuTarget != 0) {
// The 8 first Interrupt Processor Targets Registers are read-only
for (Index = 8; Index < (mGicNumInterrupts / 4); Index++) {
- MmioWrite32 (mGicDistributorBase + ARM_GIC_ICDIPTR + (Index * 4), CpuTarget);
+ MmioWrite32 (
+ mGicDistributorBase + ARM_GIC_ICDIPTR + (Index * 4),
+ CpuTarget
+ );
}
}
} else {
MpId = ArmReadMpidr ();
- CpuTarget = MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3);
+ CpuTarget = MpId &
+ (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3);
+
+ if ((MmioRead32 (
+ mGicDistributorBase + ARM_GIC_ICDDCR
+ ) & ARM_GIC_ICDDCR_DS) != 0) {
- if ((MmioRead32 (mGicDistributorBase + ARM_GIC_ICDDCR) & ARM_GIC_ICDDCR_DS) != 0) {
- //
// If the Disable Security (DS) control bit is set, we are dealing with a
// GIC that has only one security state. In this case, let's assume we are
// executing in non-secure state (which is appropriate for DXE modules)
// and that no other firmware has performed any configuration on the GIC.
// This means we need to reconfigure all interrupts to non-secure Group 1
// first.
- //
- MmioWrite32 (mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR, 0xffffffff);
+
+ MmioWrite32 (
+ mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR,
+ 0xffffffff
+ );
for (Index = 32; Index < mGicNumInterrupts; Index += 32) {
- MmioWrite32 (mGicDistributorBase + ARM_GIC_ICDISR + Index / 8, 0xffffffff);
+ MmioWrite32 (
+ mGicDistributorBase + ARM_GIC_ICDISR + Index / 8,
+ 0xffffffff
+ );
}
}
// Route the SPIs to the primary CPU. SPIs start at the INTID 32
for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) {
- MmioWrite32 (mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), CpuTarget | ARM_GICD_IROUTER_IRM);
+ MmioWrite32 (
+ mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8),
+ CpuTarget | ARM_GICD_IROUTER_IRM
+ );
}
}
@@ -331,7 +352,10 @@ GicV3DxeInitialize (
ArmGicEnableDistributor (mGicDistributorBase);
Status = InstallAndRegisterInterruptService (
- &gHardwareInterruptV3Protocol, GicV3IrqInterruptHandler, GicV3ExitBootServicesEvent);
+ &gHardwareInterruptV3Protocol,
+ GicV3IrqInterruptHandler,
+ GicV3ExitBootServicesEvent
+ );
return Status;
}
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 54a1625a32137556b58fa93ddf7fbe4d0f22c786..c5372056f3ea080c350ff67ce8f78f0a77663343 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+* Copyright (c) 2013-2017, ARM Limited. All rights reserved.
*
* This program and the accompanying materials
* are licensed and made available under the terms and conditions of the BSD
@@ -29,16 +29,16 @@
#include "GenericWatchdog.h"
-// The number of 100ns periods (the unit of time passed to these functions)
-// in a second
+/* The number of 100ns periods (the unit of time passed to these functions)
+ in a second */
#define TIME_UNITS_PER_SECOND 10000000
-// Tick frequency of the generic timer that is the basis of the generic watchdog
+// Tick frequency of the generic timer basis of the generic watchdog.
UINTN mTimerFrequencyHz = 0;
-// In cases where the compare register was set manually, information about
-// how long the watchdog was asked to wait cannot be retrieved from hardware.
-// It is therefore stored here. 0 means the timer is not running.
+/* In cases where the compare register was set manually, information about
+ how long the watchdog was asked to wait cannot be retrieved from hardware.
+ It is therefore stored here. 0 means the timer is not running. */
UINT64 mNumTimerTicks = 0;
EFI_HARDWARE_INTERRUPT_PROTOCOL *mInterruptProtocol;
@@ -75,8 +75,7 @@ WatchdogDisable (
return MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_DISABLED);
}
-/**
- On exiting boot services we must make sure the Watchdog Timer
+/** On exiting boot services we must make sure the Watchdog Timer
is stopped.
**/
VOID
@@ -90,9 +89,8 @@ WatchdogExitBootServicesEvent (
mNumTimerTicks = 0;
}
-/*
- This function is called when the watchdog's first signal (WS0) goes high.
- It uses the ResetSystem Runtime Service to reset the board.
+/* This function is called when the watchdog's first signal (WS0) goes high.
+ It uses the ResetSystem Runtime Service to reset the board.
*/
VOID
EFIAPI
@@ -101,7 +99,7 @@ WatchdogInterruptHandler (
IN EFI_SYSTEM_CONTEXT SystemContext
)
{
- STATIC CONST CHAR16 ResetString[] = L"The generic watchdog timer ran out.";
+ STATIC CONST CHAR16 ResetString[]= L"The generic watchdog timer ran out.";
WatchdogDisable ();
@@ -126,10 +124,10 @@ WatchdogInterruptHandler (
then the new handler is registered and EFI_SUCCESS is returned.
If NotifyFunction is NULL, and a handler is already registered,
then that handler is unregistered.
- If an attempt is made to register a handler when a handler is already registered,
- then EFI_ALREADY_STARTED is returned.
- If an attempt is made to unregister a handler when a handler is not registered,
- then EFI_INVALID_PARAMETER is returned.
+ If an attempt is made to register a handler when a handler is already
+ registered, then EFI_ALREADY_STARTED is returned.
+ If an attempt is made to unregister a handler when a handler is not
+ registered, then EFI_INVALID_PARAMETER is returned.
@param This The EFI_TIMER_ARCH_PROTOCOL instance.
@param NotifyFunction The function to call when a timer interrupt fires.
@@ -139,11 +137,7 @@ WatchdogInterruptHandler (
information is used to signal timer based events.
NULL will unregister the handler.
- @retval EFI_SUCCESS The watchdog timer handler was registered.
- @retval EFI_ALREADY_STARTED NotifyFunction is not NULL, and a handler is already
- registered.
- @retval EFI_INVALID_PARAMETER NotifyFunction is NULL, and a handler was not
- previously registered.
+ @retval EFI_UNSUPPORTED The code does not support NotifyFunction.
**/
EFI_STATUS
@@ -160,18 +154,18 @@ WatchdogRegisterHandler (
/**
This function sets the amount of time to wait before firing the watchdog
- timer to TimerPeriod 100 nS units. If TimerPeriod is 0, then the watchdog
+ timer to TimerPeriod 100ns units. If TimerPeriod is 0, then the watchdog
timer is disabled.
@param This The EFI_WATCHDOG_TIMER_ARCH_PROTOCOL instance.
- @param TimerPeriod The amount of time in 100 nS units to wait before the watchdog
- timer is fired. If TimerPeriod is zero, then the watchdog
- timer is disabled.
+ @param TimerPeriod The amount of time in 100ns units to wait before
+ the watchdog timer is fired. If TimerPeriod is zero,
+ then the watchdog timer is disabled.
- @retval EFI_SUCCESS The watchdog timer has been programmed to fire in Time
- 100 nS units.
- @retval EFI_DEVICE_ERROR A watchdog timer could not be programmed due to a device
- error.
+ @retval EFI_SUCCESS The watchdog timer has been programmed to fire
+ in Time 100ns units.
+ @retval EFI_DEVICE_ERROR A watchdog timer could not be programmed due
+ to a device error.
**/
EFI_STATUS
@@ -184,7 +178,7 @@ WatchdogSetTimerPeriod (
UINTN SystemCount;
EFI_STATUS Status;
- // if TimerPerdiod is 0, this is a request to stop the watchdog.
+ // if TimerPeriod is 0, this is a request to stop the watchdog.
if (TimerPeriod == 0) {
mNumTimerTicks = 0;
return WatchdogDisable ();
@@ -193,18 +187,14 @@ WatchdogSetTimerPeriod (
// Work out how many timer ticks will equate to TimerPeriod
mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
- //
- // If the number of required ticks is greater than the max number the
- // watchdog's offset register (WOR) can hold, we need to manually compute and
- // set the compare register (WCV)
- //
+ /* If the number of required ticks is greater than the max the watchdog's
+ offset register (WOR) can hold, we need to manually compute and set
+ the compare register (WCV) */
if (mNumTimerTicks > MAX_UINT32) {
- //
- // We need to enable the watchdog *before* writing to the compare register,
- // because enabling the watchdog causes an "explicit refresh", which
- // clobbers the compare register (WCV). In order to make sure this doesn't
- // trigger an interrupt, set the offset to max.
- //
+ /* We need to enable the watchdog *before* writing to the compare register,
+ because enabling the watchdog causes an "explicit refresh", which
+ clobbers the compare register (WCV). In order to make sure this doesn't
+ trigger an interrupt, set the offset to max. */
Status = WatchdogWriteOffsetRegister (MAX_UINT32);
if (EFI_ERROR (Status)) {
return Status;
@@ -221,14 +211,14 @@ WatchdogSetTimerPeriod (
}
/**
- This function retrieves the period of timer interrupts in 100 ns units,
+ This function retrieves the period of timer interrupts in 100ns units,
returns that value in TimerPeriod, and returns EFI_SUCCESS. If TimerPeriod
is NULL, then EFI_INVALID_PARAMETER is returned. If a TimerPeriod of 0 is
returned, then the timer is currently disabled.
@param This The EFI_TIMER_ARCH_PROTOCOL instance.
- @param TimerPeriod A pointer to the timer period to retrieve in 100
- ns units. If 0 is returned, then the timer is
+ @param TimerPeriod A pointer to the timer period to retrieve in
+ 100ns units. If 0 is returned, then the timer is
currently disabled.
@@ -275,19 +265,19 @@ WatchdogGetTimerPeriod (
this function will not have any chance of executing.
@param SetTimerPeriod
- Sets the period of the timer interrupt in 100 nS units.
+ Sets the period of the timer interrupt in 100ns units.
This function is optional, and may return EFI_UNSUPPORTED.
If this function is supported, then the timer period will
be rounded up to the nearest supported timer period.
@param GetTimerPeriod
- Retrieves the period of the timer interrupt in 100 nS units.
+ Retrieves the period of the timer interrupt in 100ns units.
**/
EFI_WATCHDOG_TIMER_ARCH_PROTOCOL gWatchdogTimer = {
- (EFI_WATCHDOG_TIMER_REGISTER_HANDLER) WatchdogRegisterHandler,
- (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD) WatchdogSetTimerPeriod,
- (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD) WatchdogGetTimerPeriod
+ (EFI_WATCHDOG_TIMER_REGISTER_HANDLER)WatchdogRegisterHandler,
+ (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD)WatchdogSetTimerPeriod,
+ (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD)WatchdogGetTimerPeriod
};
EFI_EVENT EfiExitBootServicesEvent = (EFI_EVENT)NULL;
@@ -302,11 +292,9 @@ GenericWatchdogEntry (
EFI_STATUS Status;
EFI_HANDLE Handle;
- //
- // Make sure the Watchdog Timer Architectural Protocol has not been installed
- // in the system yet.
- // This will avoid conflicts with the universal watchdog
- //
+ /* Make sure the Watchdog Timer Architectural Protocol has not been installed
+ in the system yet.
+ This will avoid conflicts with the universal watchdog */
ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEfiWatchdogTimerArchProtocolGuid);
mTimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
@@ -314,8 +302,11 @@ GenericWatchdogEntry (
// Register for an ExitBootServicesEvent
Status = gBS->CreateEvent (
- EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_NOTIFY,
- WatchdogExitBootServicesEvent, NULL, &EfiExitBootServicesEvent
+ EVT_SIGNAL_EXIT_BOOT_SERVICES,
+ TPL_NOTIFY,
+ WatchdogExitBootServicesEvent,
+ NULL,
+ &EfiExitBootServicesEvent
);
if (!EFI_ERROR (Status)) {
// Install interrupt handler
@@ -326,26 +317,24 @@ GenericWatchdogEntry (
);
if (!EFI_ERROR (Status)) {
Status = mInterruptProtocol->RegisterInterruptSource (
- mInterruptProtocol,
- FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
- WatchdogInterruptHandler
- );
+ mInterruptProtocol,
+ FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
+ WatchdogInterruptHandler
+ );
if (!EFI_ERROR (Status)) {
// Install the Timer Architectural Protocol onto a new handle
Handle = NULL;
Status = gBS->InstallMultipleProtocolInterfaces (
&Handle,
- &gEfiWatchdogTimerArchProtocolGuid, &gWatchdogTimer,
+ &gEfiWatchdogTimerArchProtocolGuid,
+ &gWatchdogTimer,
NULL
);
}
}
}
- if (EFI_ERROR (Status)) {
- // The watchdog failed to initialize
- ASSERT (FALSE);
- }
+ ASSERT_EFI_ERROR (Status);
mNumTimerTicks = 0;
WatchdogDisable ();
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] EmbeddedPkg: Introduce HardwareInterrupt2 protocol
2017-09-21 16:23 [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM evan.lloyd
2017-09-21 16:23 ` [PATCH 1/4] ArmPkg: Tidy GIC code before changes evan.lloyd
@ 2017-09-21 16:23 ` evan.lloyd
2017-09-21 16:23 ` [PATCH 3/4] ArmPkg/ArmGicDxe: Expose " evan.lloyd
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: evan.lloyd @ 2017-09-21 16:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Ard Biesheuvel, Leif Lindholm, Matteo Carlini, nd
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
The existing HardwareInterrupt protocol lacks the means to configure
the level/edge and polarity properties of an interrupt. So introduce a
new protocol HardwareInterrupt2, and add some new members that allow
manipulation of those properties.
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>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
EmbeddedPkg/EmbeddedPkg.dec | 1 +
EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h | 182 ++++++++++++++++++++
2 files changed, 183 insertions(+)
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 0be102ad9c767d81a004c757f40a65063aab1d7a..151b1d503dee463d529a173d2555b6c9208100e5 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -69,6 +69,7 @@ [Guids.common]
[Protocols.common]
gHardwareInterruptProtocolGuid = { 0x2890B3EA, 0x053D, 0x1643, { 0xAD, 0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
+ gHardwareInterrupt2ProtocolGuid = { 0x32898322, 0x2da1, 0x474a, { 0xba, 0xaa, 0xf3, 0xf7, 0xcf, 0x56, 0x94, 0x70 } }
gEfiDebugSupportPeriodicCallbackProtocolGuid = { 0x9546e07c, 0x2cbb, 0x4c88, { 0x98, 0x6c, 0xcd, 0x34, 0x10, 0x86, 0xf0, 0x44 } }
gEfiEblAddCommandProtocolGuid = { 0xaeda2428, 0x9a22, 0x4637, { 0x9b, 0x21, 0x54, 0x5e, 0x28, 0xfb, 0xb8, 0x29 } }
gEmbeddedDeviceGuid = { 0xbf4b9d10, 0x13ec, 0x43dd, { 0x88, 0x80, 0xe9, 0xb, 0x71, 0x8f, 0x27, 0xde } }
diff --git a/EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h b/EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h
new file mode 100644
index 0000000000000000000000000000000000000000..caa28d4b06c893d220efdbdb8814abaa1ddef1a6
--- /dev/null
+++ b/EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h
@@ -0,0 +1,182 @@
+/** @file
+
+ Copyright (c) 2016-2017, Linaro Ltd. All rights reserved.<BR>
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __HARDWARE_INTERRUPT2_H__
+#define __HARDWARE_INTERRUPT2_H__
+
+#include <Protocol/HardwareInterrupt.h>
+
+// 22838932-1a2d-4a47-aaba-f3f7cf569470
+
+#define EFI_HARDWARE_INTERRUPT2_PROTOCOL_GUID \
+ { 0x32898322, 0x2d1a, 0x474a, \
+ { 0xba, 0xaa, 0xf3, 0xf7, 0xcf, 0x56, 0x94, 0x70 } }
+
+typedef enum {
+ EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_LOW,
+ EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH,
+ EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_FALLING,
+ EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING,
+} EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE;
+
+typedef struct _EFI_HARDWARE_INTERRUPT2_PROTOCOL \
+ EFI_HARDWARE_INTERRUPT2_PROTOCOL;
+
+/**
+ Register Handler for the specified interrupt source.
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt
+ @param Handler Callback for interrupt. NULL to unregister
+
+ @retval EFI_SUCCESS Source was updated to support Handler.
+ @retval EFI_DEVICE_ERROR Hardware could not be programmed.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *HARDWARE_INTERRUPT2_REGISTER) (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ IN HARDWARE_INTERRUPT_HANDLER Handler
+ );
+
+
+/**
+ Enable interrupt source Source.
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt
+
+ @retval EFI_SUCCESS Source interrupt enabled.
+ @retval EFI_DEVICE_ERROR Hardware could not be programmed.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *HARDWARE_INTERRUPT2_ENABLE) (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source
+ );
+
+
+/**
+ Disable interrupt source Source.
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt
+
+ @retval EFI_SUCCESS Source interrupt disabled.
+ @retval EFI_DEVICE_ERROR Hardware could not be programmed.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *HARDWARE_INTERRUPT2_DISABLE) (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source
+ );
+
+
+/**
+ Return current state of interrupt source Source.
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt
+ @param InterruptState TRUE: source enabled, FALSE: source disabled.
+
+ @retval EFI_SUCCESS InterruptState is valid
+ @retval EFI_DEVICE_ERROR InterruptState is not valid
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *HARDWARE_INTERRUPT2_INTERRUPT_STATE) (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ IN BOOLEAN *InterruptState
+ );
+
+/**
+ Signal to the hardware that the End Of Interrupt state
+ has been reached.
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt
+
+ @retval EFI_SUCCESS Source interrupt EOI'ed.
+ @retval EFI_DEVICE_ERROR Hardware could not be programmed.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *HARDWARE_INTERRUPT2_END_OF_INTERRUPT) (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source
+ );
+
+/**
+ Return the configured trigger type for an interrupt source
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt
+ @param TriggerType The configured trigger type
+
+ @retval EFI_SUCCESS Operation successful
+ @retval EFI_DEVICE_ERROR Information could not be returned
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *HARDWARE_INTERRUPT2_GET_TRIGGER_TYPE) (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE *TriggerType
+ );
+
+
+/**
+ Configure the trigger type for an interrupt source
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt
+ @param TriggerType The trigger type to configure
+
+ @retval EFI_SUCCESS Operation successful
+ @retval EFI_DEVICE_ERROR Hardware could not be programmed.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *HARDWARE_INTERRUPT2_SET_TRIGGER_TYPE) (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ IN EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE TriggerType
+ );
+
+struct _EFI_HARDWARE_INTERRUPT2_PROTOCOL {
+ HARDWARE_INTERRUPT2_REGISTER RegisterInterruptSource;
+ HARDWARE_INTERRUPT2_ENABLE EnableInterruptSource;
+ HARDWARE_INTERRUPT2_DISABLE DisableInterruptSource;
+ HARDWARE_INTERRUPT2_INTERRUPT_STATE GetInterruptSourceState;
+ HARDWARE_INTERRUPT2_END_OF_INTERRUPT EndOfInterrupt;
+
+ // v2 members
+ HARDWARE_INTERRUPT2_GET_TRIGGER_TYPE GetTriggerType;
+ HARDWARE_INTERRUPT2_SET_TRIGGER_TYPE SetTriggerType;
+};
+
+extern EFI_GUID gHardwareInterrupt2ProtocolGuid;
+
+#endif
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
2017-09-21 16:23 [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM evan.lloyd
2017-09-21 16:23 ` [PATCH 1/4] ArmPkg: Tidy GIC code before changes evan.lloyd
2017-09-21 16:23 ` [PATCH 2/4] EmbeddedPkg: Introduce HardwareInterrupt2 protocol evan.lloyd
@ 2017-09-21 16:23 ` evan.lloyd
2017-10-05 20:15 ` Leif Lindholm
2017-09-21 16:23 ` [PATCH 4/4] ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type evan.lloyd
2017-09-21 17:56 ` [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM Leif Lindholm
4 siblings, 1 reply; 12+ messages in thread
From: evan.lloyd @ 2017-09-21 16:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Ard Biesheuvel, Leif Lindholm, Matteo Carlini, nd
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
The existing HardwareInterrupt protocol lacked a means to configure the
level/edge properties of an interrupt. The new HardwareInterrupt2
protocol introduced this capability.
This patch updates the GIC drivers to provide the new interfaces.
The changes comprise:
Update to use HardwareInterrupt2 protocol
Additions to register info in ArmGicLib.h
Added new functionality (GetTriggerType and SetTriggerType)
The requirement for this change derives from a problem detected on ARM
Juno boards, but the change is of generic (ARM) relevance.
This commit is in response to review on the mailing list and, as
suggested there, rolls Girish's updates onto Ard's original example.
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>
---
Notes:
376_irqtype_v4
[Leif Lindholm]
- Fixed continuation line indent
- Moved EFI_D_ERROR changes out
- Replaced MmioBitFieldRead32
- Fixed indent and parentheses
ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 3 +-
ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 23 ++++
ArmPkg/Include/Library/ArmGicLib.h | 11 +-
ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 43 ++++++
ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 137 ++++++++++++++++++++
ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 136 +++++++++++++++++++
6 files changed, 351 insertions(+), 2 deletions(-)
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
index e554301c4b28022c805f69242cf6ee979d19abc2..d5921533fb68fa32c3e0705b05930700ee81da07 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
@@ -1,7 +1,7 @@
#/** @file
#
# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
-# Copyright (c) 2012 - 2015, ARM Ltd. All rights reserved.<BR>
+# Copyright (c) 2012 - 2017, ARM Ltd. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -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 76945282d9b9bbf6da872a408a56387ee321a3b5..610ffacc7eb025011f87ad5e6e457a610bd570a9 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;
@@ -32,6 +33,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
);
@@ -58,4 +60,25 @@ GicV3DxeInitialize (
IN EFI_SYSTEM_TABLE *SystemTable
);
+
+// Shared code
+
+/**
+ Calculate GICD_ICFGRn base address and corresponding bit
+ field Int_config[1] of the GIC distributor register.
+
+ @param Source Hardware source of the interrupt.
+ @param RegAddress Corresponding GICD_ICFGRn base address.
+ @param Config1Bit Bit number of F Int_config[1] bit in the register.
+
+ @retval EFI_SUCCESS Source interrupt supported.
+ @retval EFI_UNSUPPORTED Source interrupt is not supported.
+**/
+EFI_STATUS
+GicGetDistributorIcfgBaseAndBit (
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ OUT UINTN *RegAddress,
+ OUT UINTN *Config1Bit
+ );
+
#endif
diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h
index a4ede82bc6c07733f880a12fff54001590a0518a..4b21ea9e4e76cb83c0c3421c1d9d88b456192687 100644
--- a/ArmPkg/Include/Library/ArmGicLib.h
+++ b/ArmPkg/Include/Library/ArmGicLib.h
@@ -49,8 +49,17 @@
#define ARM_GIC_ICDDCR_ARE (1 << 4) // Affinity Routing Enable (ARE)
#define ARM_GIC_ICDDCR_DS (1 << 6) // Disable Security (DS)
+// GICD_ICDICFR bits
+#define ARM_GIC_ICDICFR_WIDTH 32 // ICDICFR is a 32 bit register
+#define ARM_GIC_ICDICFR_BYTES (ARM_GIC_ICDICFR_WIDTH / 8)
+#define ARM_GIC_ICDICFR_F_WIDTH 2 // Each F field is 2 bits
+#define ARM_GIC_ICDICFR_F_STRIDE 16 // (32/2) F fields per register
+#define ARM_GIC_ICDICFR_F_CONFIG1_BIT 1 // Bit number within F field
+#define ARM_GIC_ICDICFR_LEVEL_TRIGGERED 0x0 // Level triggered interrupt
+#define ARM_GIC_ICDICFR_EDGE_TRIGGERED 0x1 // Edge triggered interrupt
+
+
// GIC Redistributor
-
#define ARM_GICR_CTLR_FRAME_SIZE SIZE_64KB
#define ARM_GICR_SGI_PPI_FRAME_SIZE SIZE_64KB
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
index 7ca3ca8f52afe52c827ca82b749b665184813fc1..bff8d983cf02f7fd1fa6165824cb6556cdea5c6f 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
@@ -39,6 +39,46 @@ UINTN mGicNumInterrupts = 0;
HARDWARE_INTERRUPT_HANDLER *gRegisteredInterruptHandlers = NULL;
+
+/**
+ Calculate GICD_ICFGRn base address and corresponding bit
+ field Int_config[1] of the GIC distributor register.
+
+ @param Source Hardware source of the interrupt.
+ @param RegAddress Corresponding GICD_ICFGRn base address.
+ @param Config1Bit Bit number of F Int_config[1] bit in the register.
+
+ @retval EFI_SUCCESS Source interrupt supported.
+ @retval EFI_UNSUPPORTED Source interrupt is not supported.
+**/
+EFI_STATUS
+GicGetDistributorIcfgBaseAndBit (
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ OUT UINTN *RegAddress,
+ OUT UINTN *Config1Bit
+ )
+{
+ UINTN RegIndex;
+ UINTN Field;
+
+ if (Source >= mGicNumInterrupts) {
+ ASSERT(Source < mGicNumInterrupts);
+ return EFI_UNSUPPORTED;
+ }
+
+ RegIndex = Source / ARM_GIC_ICDICFR_F_STRIDE; // NOTE: truncation is significant
+ Field = Source % ARM_GIC_ICDICFR_F_STRIDE;
+ *RegAddress = PcdGet64 (PcdGicDistributorBase)
+ + ARM_GIC_ICDICFR
+ + (ARM_GIC_ICDICFR_BYTES * RegIndex);
+ *Config1Bit = ((Field * ARM_GIC_ICDICFR_F_WIDTH)
+ + ARM_GIC_ICDICFR_F_CONFIG1_BIT);
+
+ return EFI_SUCCESS;
+}
+
+
+
/**
Register Handler for the specified interrupt source.
@@ -84,6 +124,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
)
@@ -103,6 +144,8 @@ InstallAndRegisterInterruptService (
&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 3d5d034efbb1bf2e1e84841cab4175bc607845c2..7717ebe75330562db61a30ab4e5c5ad559c5f201 100644
--- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
@@ -29,6 +29,7 @@ Abstract:
#define ARM_GIC_DEFAULT_PRIORITY 0x80
extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol;
+extern EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol;
STATIC UINT32 mGicInterruptInterfaceBase;
STATIC UINT32 mGicDistributorBase;
@@ -198,6 +199,141 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol = {
GicV2EndOfInterrupt
};
+/**
+ Get interrupt trigger type of an interrupt
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt.
+ @param TriggerType Returns interrupt trigger type.
+
+ @retval EFI_SUCCESS Source interrupt supported.
+ @retval EFI_UNSUPPORTED Source interrupt is not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicV2GetTriggerType (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE *TriggerType
+ )
+{
+ UINTN RegAddress;
+ UINTN Config1Bit;
+ EFI_STATUS Status;
+
+ Status = GicGetDistributorIcfgBaseAndBit (
+ Source,
+ &RegAddress,
+ &Config1Bit
+ );
+
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if ((MmioRead32 (RegAddress) & (1 << Config1Bit)) == 0) {
+ *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH;
+ } else {
+ *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Set interrupt trigger type of an interrupt
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt.
+ @param TriggerType Interrupt trigger type.
+
+ @retval EFI_SUCCESS Source interrupt supported.
+ @retval EFI_UNSUPPORTED Source interrupt is not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicV2SetTriggerType (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ IN EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE TriggerType
+ )
+{
+ UINTN RegAddress;
+ UINTN Config1Bit;
+ UINT32 Value;
+ EFI_STATUS Status;
+ BOOLEAN SourceEnabled;
+
+ if ( (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
+ && (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH)) {
+ DEBUG ((DEBUG_ERROR, "Invalid interrupt trigger type: %d\n", \
+ TriggerType));
+ ASSERT (FALSE);
+ return EFI_UNSUPPORTED;
+ }
+
+ Status = GicGetDistributorIcfgBaseAndBit (
+ Source,
+ &RegAddress,
+ &Config1Bit
+ );
+
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = GicV2GetInterruptSourceState (
+ (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+ Source,
+ &SourceEnabled
+ );
+
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Value = (TriggerType == EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
+ ? ARM_GIC_ICDICFR_EDGE_TRIGGERED
+ : ARM_GIC_ICDICFR_LEVEL_TRIGGERED;
+
+ // Before changing the value, we must disable the interrupt,
+ // otherwise GIC behavior is UNPREDICTABLE.
+ if (SourceEnabled) {
+ GicV2DisableInterruptSource (
+ (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+ Source
+ );
+ }
+
+ MmioAndThenOr32 (
+ RegAddress,
+ ~(0x1 << Config1Bit),
+ Value << Config1Bit
+ );
+
+ // Restore interrupt state
+ if (SourceEnabled) {
+ GicV2EnableInterruptSource (
+ (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+ Source
+ );
+ }
+
+ return EFI_SUCCESS;
+}
+
+EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol = {
+ (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
@@ -321,6 +457,7 @@ GicV2DxeInitialize (
Status = InstallAndRegisterInterruptService (
&gHardwareInterruptV2Protocol,
+ &gHardwareInterrupt2V2Protocol,
GicV2IrqInterruptHandler,
GicV2ExitBootServicesEvent
);
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
index 50b51ffcf57b23b110899c7c55237cf85b61d196..01154848f4439af3e87e89cf796f3d69ce4a0c4a 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
@@ -19,6 +19,7 @@
#define ARM_GIC_DEFAULT_PRIORITY 0x80
extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol;
+extern EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol;
STATIC UINTN mGicDistributorBase;
STATIC UINTN mGicRedistributorsBase;
@@ -192,6 +193,140 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol = {
GicV3EndOfInterrupt
};
+/**
+ Get interrupt trigger type of an interrupt
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt.
+ @param TriggerType Returns interrupt trigger type.
+
+ @retval EFI_SUCCESS Source interrupt supported.
+ @retval EFI_UNSUPPORTED Source interrupt is not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicV3GetTriggerType (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE *TriggerType
+ )
+{
+ UINTN RegAddress;
+ UINTN Config1Bit;
+ EFI_STATUS Status;
+
+ Status = GicGetDistributorIcfgBaseAndBit (
+ Source,
+ &RegAddress,
+ &Config1Bit
+ );
+
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if ((MmioRead32 (RegAddress) & (1 << Config1Bit)) == 0) {
+ *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH;
+ } else {
+ *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Set interrupt trigger type of an interrupt
+
+ @param This Instance pointer for this protocol
+ @param Source Hardware source of the interrupt.
+ @param TriggerType Interrupt trigger type.
+
+ @retval EFI_SUCCESS Source interrupt supported.
+ @retval EFI_UNSUPPORTED Source interrupt is not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GicV3SetTriggerType (
+ IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
+ IN HARDWARE_INTERRUPT_SOURCE Source,
+ IN EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE TriggerType
+ )
+{
+ UINTN RegAddress;
+ UINTN Config1Bit;
+ UINT32 Value;
+ EFI_STATUS Status;
+ BOOLEAN SourceEnabled;
+
+ if ( (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
+ && (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH)) {
+ DEBUG ((DEBUG_ERROR, "Invalid interrupt trigger type: %d\n", \
+ TriggerType));
+ ASSERT (FALSE);
+ return EFI_UNSUPPORTED;
+ }
+
+ Status = GicGetDistributorIcfgBaseAndBit (
+ Source,
+ &RegAddress,
+ &Config1Bit
+ );
+
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = GicV3GetInterruptSourceState (
+ (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+ Source,
+ &SourceEnabled
+ );
+
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Value = (TriggerType == EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
+ ? ARM_GIC_ICDICFR_EDGE_TRIGGERED
+ : ARM_GIC_ICDICFR_LEVEL_TRIGGERED;
+
+ // Before changing the value, we must disable the interrupt,
+ // otherwise GIC behavior is UNPREDICTABLE.
+ if (SourceEnabled) {
+ GicV3DisableInterruptSource (
+ (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+ Source
+ );
+ }
+
+ MmioAndThenOr32 (
+ RegAddress,
+ ~(0x1 << Config1Bit),
+ Value << Config1Bit
+ );
+ // Restore interrupt state
+ if (SourceEnabled) {
+ GicV3EnableInterruptSource (
+ (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
+ Source
+ );
+ }
+
+ return EFI_SUCCESS;
+}
+
+EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol = {
+ (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
@@ -353,6 +488,7 @@ GicV3DxeInitialize (
Status = InstallAndRegisterInterruptService (
&gHardwareInterruptV3Protocol,
+ &gHardwareInterrupt2V3Protocol,
GicV3IrqInterruptHandler,
GicV3ExitBootServicesEvent
);
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type
2017-09-21 16:23 [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM evan.lloyd
` (2 preceding siblings ...)
2017-09-21 16:23 ` [PATCH 3/4] ArmPkg/ArmGicDxe: Expose " evan.lloyd
@ 2017-09-21 16:23 ` evan.lloyd
2017-09-21 17:56 ` [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM Leif Lindholm
4 siblings, 0 replies; 12+ messages in thread
From: evan.lloyd @ 2017-09-21 16:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Ard Biesheuvel, Leif Lindholm, Matteo Carlini, nd
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Utilise the new HardwareInterrupt2 protocol to adjust the
Edge/Level characteristics of the Watchdog interrupt.
Contributed-under: TianoCore Contribution Agreement 1.1
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>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf | 6 ++--
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 29 ++++++++++++--------
2 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
index fece14cc18315cd15510680c438288687b60c018..ba0403d7fdc3589803c643c27a44918e73afa97e 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
@@ -1,5 +1,5 @@
#
-# Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+# Copyright (c) 2013-2017, ARM Limited. All rights reserved.
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -47,7 +47,7 @@ [Pcd.common]
[Protocols]
gEfiWatchdogTimerArchProtocolGuid
- gHardwareInterruptProtocolGuid
+ gHardwareInterrupt2ProtocolGuid
[Depex]
- gHardwareInterruptProtocolGuid
+ gHardwareInterrupt2ProtocolGuid
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index c5372056f3ea080c350ff67ce8f78f0a77663343..252ba5bf321e379ef440830cab468af7c55905b3 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -24,8 +24,8 @@
#include <Library/UefiLib.h>
#include <Library/ArmGenericTimerCounterLib.h>
+#include <Protocol/HardwareInterrupt2.h>
#include <Protocol/WatchdogTimer.h>
-#include <Protocol/HardwareInterrupt.h>
#include "GenericWatchdog.h"
@@ -41,7 +41,7 @@ UINTN mTimerFrequencyHz = 0;
It is therefore stored here. 0 means the timer is not running. */
UINT64 mNumTimerTicks = 0;
-EFI_HARDWARE_INTERRUPT_PROTOCOL *mInterruptProtocol;
+EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
EFI_STATUS
WatchdogWriteOffsetRegister (
@@ -311,7 +311,7 @@ GenericWatchdogEntry (
if (!EFI_ERROR (Status)) {
// Install interrupt handler
Status = gBS->LocateProtocol (
- &gHardwareInterruptProtocolGuid,
+ &gHardwareInterrupt2ProtocolGuid,
NULL,
(VOID **)&mInterruptProtocol
);
@@ -322,14 +322,21 @@ GenericWatchdogEntry (
WatchdogInterruptHandler
);
if (!EFI_ERROR (Status)) {
- // Install the Timer Architectural Protocol onto a new handle
- Handle = NULL;
- Status = gBS->InstallMultipleProtocolInterfaces (
- &Handle,
- &gEfiWatchdogTimerArchProtocolGuid,
- &gWatchdogTimer,
- NULL
- );
+ Status = mInterruptProtocol->SetTriggerType (
+ mInterruptProtocol,
+ FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
+ EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING
+ );
+ if (!EFI_ERROR (Status)) {
+ // Install the Timer Architectural Protocol onto a new handle
+ Handle = NULL;
+ Status = gBS->InstallMultipleProtocolInterfaces (
+ &Handle,
+ &gEfiWatchdogTimerArchProtocolGuid,
+ &gWatchdogTimer,
+ NULL
+ );
+ }
}
}
}
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM
2017-09-21 16:23 [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM evan.lloyd
` (3 preceding siblings ...)
2017-09-21 16:23 ` [PATCH 4/4] ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type evan.lloyd
@ 2017-09-21 17:56 ` Leif Lindholm
2017-09-21 18:39 ` Evan Lloyd
4 siblings, 1 reply; 12+ messages in thread
From: Leif Lindholm @ 2017-09-21 17:56 UTC (permalink / raw)
To: evan.lloyd
Cc: edk2-devel, Ard Biesheuvel, Matteo Carlini, nd, Arvind Chauhan,
Daniil Egranov, Thomas Panakamattam Abraham
On Thu, Sep 21, 2017 at 05:23:40PM +0100, evan.lloyd@arm.com wrote:
> From: EvanLloyd <evan.lloyd@arm.com>
>
> This v4 series of patches corrects a problem detected on the ARM Juno
> platform that is actually generic (at least to ARM GIC platforms).
> The HardwareInterrupt protocol had no means of handling characteristics
> like Edge/Level triggered and polarity.
>
> A new HardwareInterrupt2 protocol (provided by Ard) is added, and code
> changed to utilise the new capabilities.
>
> The code is available for examination on Github at:
> https://github.com/EvanLloyd/tianocore/tree/376_irqtype_v4
>
> v4 responds to further comments from maintainers, being mainly
> cosmetic changes.
>
> Note: Significant defects exist in the (original) Watchdog handling,
> and a new patch will follow.
If you let me move that | before pushing, I'm happy with this set.
Arvind, Daniil, Thomas - any comments?
/
Leif
> Ard Biesheuvel (3):
> EmbeddedPkg: Introduce HardwareInterrupt2 protocol
> ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
> ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type
>
> Evan Lloyd (1):
> ArmPkg: Tidy GIC code before changes.
>
> EmbeddedPkg/EmbeddedPkg.dec | 1 +
> ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 3 +-
> ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf | 6 +-
> ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 31 ++-
> ArmPkg/Include/Library/ArmGicLib.h | 34 +--
> EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h | 182 ++++++++++++++++
> ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 76 +++++--
> ArmPkg/Drivers/ArmGic/ArmGicLib.c | 73 +++++--
> ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 195 ++++++++++++++---
> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 218 +++++++++++++++++---
> ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 146 +++++++------
> 11 files changed, 779 insertions(+), 186 deletions(-)
> create mode 100644 EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h
>
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM
2017-09-21 17:56 ` [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM Leif Lindholm
@ 2017-09-21 18:39 ` Evan Lloyd
2017-09-21 19:15 ` Andrew Fish
0 siblings, 1 reply; 12+ messages in thread
From: Evan Lloyd @ 2017-09-21 18:39 UTC (permalink / raw)
To: Leif Lindholm
Cc: edk2-devel@lists.01.org, Ard Biesheuvel, Matteo Carlini, nd,
Arvind Chauhan, Daniil Egranov, Thomas Panakamattam Abraham
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: 21 September 2017 18:56
> To: Evan Lloyd <Evan.Lloyd@arm.com>
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>;
> Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>; Arvind
> Chauhan <Arvind.Chauhan@arm.com>; Daniil Egranov
> <Daniil.Egranov@arm.com>; Thomas Panakamattam Abraham
> <thomas.abraham@arm.com>
> Subject: Re: [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM
>
> On Thu, Sep 21, 2017 at 05:23:40PM +0100, evan.lloyd@arm.com wrote:
> > From: EvanLloyd <evan.lloyd@arm.com>
> >
> > This v4 series of patches corrects a problem detected on the ARM Juno
> > platform that is actually generic (at least to ARM GIC platforms).
> > The HardwareInterrupt protocol had no means of handling
> > characteristics like Edge/Level triggered and polarity.
> >
> > A new HardwareInterrupt2 protocol (provided by Ard) is added, and code
> > changed to utilise the new capabilities.
> >
> > The code is available for examination on Github at:
> > https://github.com/EvanLloyd/tianocore/tree/376_irqtype_v4
> >
> > v4 responds to further comments from maintainers, being mainly
> > cosmetic changes.
> >
> > Note: Significant defects exist in the (original) Watchdog handling,
> > and a new patch will follow.
>
> If you let me move that | before pushing, I'm happy with this set.
[[Evan Lloyd]] Very well, if you really feel that strongly.
How come you have a code style rooted in the 1960s before you were born, while I've move forward to at least the 1990s. Shouldn't I be the surly, hide-bound, old, luddite dinosaur?
>
> Arvind, Daniil, Thomas - any comments?
>
> /
> Leif
>
> > Ard Biesheuvel (3):
> > EmbeddedPkg: Introduce HardwareInterrupt2 protocol
> > ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
> > ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type
> >
> > Evan Lloyd (1):
> > ArmPkg: Tidy GIC code before changes.
> >
> > EmbeddedPkg/EmbeddedPkg.dec | 1 +
> > ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 3 +-
> > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf | 6 +-
> > ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 31 ++-
> > ArmPkg/Include/Library/ArmGicLib.h | 34 +--
> > EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h | 182
> ++++++++++++++++
> > ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 76 +++++--
> > ArmPkg/Drivers/ArmGic/ArmGicLib.c | 73 +++++--
> > ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 195
> ++++++++++++++---
> > ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 218
> +++++++++++++++++---
> > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 146
> +++++++------
> > 11 files changed, 779 insertions(+), 186 deletions(-) create mode
> > 100644 EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h
> >
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM
2017-09-21 18:39 ` Evan Lloyd
@ 2017-09-21 19:15 ` Andrew Fish
2017-09-21 20:53 ` Evan Lloyd
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Fish @ 2017-09-21 19:15 UTC (permalink / raw)
To: Evan Lloyd
Cc: Leif Lindholm, nd, Ard Biesheuvel, edk2-devel@lists.01.org,
Thomas Panakamattam Abraham, Arvind Chauhan
> On Sep 21, 2017, at 11:39 AM, Evan Lloyd <Evan.Lloyd@arm.com> wrote:
>
>
>
>> -----Original Message-----
>> From: Leif Lindholm [mailto:leif.lindholm@linaro.org <mailto:leif.lindholm@linaro.org>]
>> Sent: 21 September 2017 18:56
>> To: Evan Lloyd <Evan.Lloyd@arm.com <mailto:Evan.Lloyd@arm.com>>
>> Cc: edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org <mailto:ard.biesheuvel@linaro.org>>;
>> Matteo Carlini <Matteo.Carlini@arm.com <mailto:Matteo.Carlini@arm.com>>; nd <nd@arm.com <mailto:nd@arm.com>>; Arvind
>> Chauhan <Arvind.Chauhan@arm.com <mailto:Arvind.Chauhan@arm.com>>; Daniil Egranov
>> <Daniil.Egranov@arm.com <mailto:Daniil.Egranov@arm.com>>; Thomas Panakamattam Abraham
>> <thomas.abraham@arm.com <mailto:thomas.abraham@arm.com>>
>> Subject: Re: [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM
>>
>> On Thu, Sep 21, 2017 at 05:23:40PM +0100, evan.lloyd@arm.com wrote:
>>> From: EvanLloyd <evan.lloyd@arm.com>
>>>
>>> This v4 series of patches corrects a problem detected on the ARM Juno
>>> platform that is actually generic (at least to ARM GIC platforms).
>>> The HardwareInterrupt protocol had no means of handling
>>> characteristics like Edge/Level triggered and polarity.
>>>
>>> A new HardwareInterrupt2 protocol (provided by Ard) is added, and code
>>> changed to utilise the new capabilities.
>>>
>>> The code is available for examination on Github at:
>>> https://github.com/EvanLloyd/tianocore/tree/376_irqtype_v4
>>>
>>> v4 responds to further comments from maintainers, being mainly
>>> cosmetic changes.
>>>
>>> Note: Significant defects exist in the (original) Watchdog handling,
>>> and a new patch will follow.
>>
>> If you let me move that | before pushing, I'm happy with this set.
>
> [[Evan Lloyd]] Very well, if you really feel that strongly.
> How come you have a code style rooted in the 1960s before you were born, while I've move forward to at least the 1990s. Shouldn't I be the surly, hide-bound, old, luddite dinosaur?
>
Yikes. There is actually no scientific data that one coding standard is better than another. The data does show having a consistent coding style is what has value.
I wrote the original coding standard a long time ago.... I remember I was a 4 space indent person, and as a compromise we moved to 2 space indent. It drove me crazy for about 2 weeks and then my brain flipped over and now I prefer 2 spaces, and 4 spaces look strange. Same kind of thing happened when Apple flipped the scroll direction. Folks went crazy, but after a while your brain gets accustomed to the new normal.
Thanks,
Andrew Fish
>>
>> Arvind, Daniil, Thomas - any comments?
>>
>> /
>> Leif
>>
>>> Ard Biesheuvel (3):
>>> EmbeddedPkg: Introduce HardwareInterrupt2 protocol
>>> ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
>>> ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type
>>>
>>> Evan Lloyd (1):
>>> ArmPkg: Tidy GIC code before changes.
>>>
>>> EmbeddedPkg/EmbeddedPkg.dec | 1 +
>>> ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 3 +-
>>> ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf | 6 +-
>>> ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 31 ++-
>>> ArmPkg/Include/Library/ArmGicLib.h | 34 +--
>>> EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h | 182
>> ++++++++++++++++
>>> ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 76 +++++--
>>> ArmPkg/Drivers/ArmGic/ArmGicLib.c | 73 +++++--
>>> ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 195
>> ++++++++++++++---
>>> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 218
>> +++++++++++++++++---
>>> ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 146
>> +++++++------
>>> 11 files changed, 779 insertions(+), 186 deletions(-) create mode
>>> 100644 EmbeddedPkg/Include/Protocol/HardwareInterrupt2.h
>>>
>>> --
>>> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>>>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM
2017-09-21 19:15 ` Andrew Fish
@ 2017-09-21 20:53 ` Evan Lloyd
0 siblings, 0 replies; 12+ messages in thread
From: Evan Lloyd @ 2017-09-21 20:53 UTC (permalink / raw)
To: afish@apple.com
Cc: Leif Lindholm, nd, Ard Biesheuvel, edk2-devel@lists.01.org,
Thomas Panakamattam Abraham, Arvind Chauhan
Hi Andrew.
> From: afish@apple.com [mailto:afish@apple.com]
> Sent: 21 September 2017 20:15
> To: Evan Lloyd <Evan.Lloyd@arm.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>; nd <nd@arm.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org; Thomas Panakamattam Abraham <thomas.abraham@arm.com>; Arvind Chauhan <Arvind.Chauhan@arm.com>
> Subject: Re: [edk2] [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM
> ?
>
> If you let me move that | before pushing, I'm happy with this set.
>
> [[Evan Lloyd]] Very well, if you really feel that strongly.
> How come you have a code style rooted in the 1960s before you were born, while I've move forward to at least the 1990s. Shouldn't I be the surly, hide-bound, old, luddite dinosaur?
>
>
> Yikes. There is actually no scientific data that one coding standard is better than another. The data does show having a consistent coding style is what has value.
>
> I wrote the original coding standard a long time ago.... I remember I was a 4 space indent person, and as a compromise we moved to 2 space indent. It drove me crazy for about 2 weeks and then my brain flipped over and now I prefer 2 spaces, and 4 spaces look strange. Same kind of thing happened when Apple flipped the scroll direction. Folks went crazy, but after a while your brain gets accustomed to the new normal.
>
> Thanks,
>
> Andrew Fish
[[Evan Lloyd]] Reading my comment, one could infer that I overdid calling Leif names.
Now I do do that, frequently, but generally to his face, and, if possible, over beer.
He also knows me well enough to know that I WAS there in the 1960s.
The topic under discussion is the placement of operators when splitting lines,
and the standard has no rules but lots of examples that support my case,
which is that:
if ( ( (some long condition)
&& (Another long condition))
|| ( (something shortish || something shorter)
&& (another test)
&& (yet another check)))
is clearer than:
if (((some long condition) &&
(Another long condition)) ||
((something shortish || something shorter) &&
(another test) &&
(yet another check)))
A major reason for that is that aligning the operators at the start gives
an extra clue as to how they relate to each other. In the second example
the matching of operators can be obscured.
I hold that a similar case applies to ordinary operators:
variable = (a * (calculation + with))
/ sums;
rather than:
variable = (a * (calculation + with)) /
sums;
Regards,
Evan
PS Outlook insists on messing with the "plain text" format, so this may not look right, sorry.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
2017-09-21 16:23 ` [PATCH 3/4] ArmPkg/ArmGicDxe: Expose " evan.lloyd
@ 2017-10-05 20:15 ` Leif Lindholm
2017-10-05 20:15 ` Ard Biesheuvel
0 siblings, 1 reply; 12+ messages in thread
From: Leif Lindholm @ 2017-10-05 20:15 UTC (permalink / raw)
To: ard.biesheuvel; +Cc: edk2-devel, evan.lloyd, Matteo Carlini
On Thu, Sep 21, 2017 at 05:23:43PM +0100, evan.lloyd@arm.com wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> The existing HardwareInterrupt protocol lacked a means to configure the
> level/edge properties of an interrupt. The new HardwareInterrupt2
> protocol introduced this capability.
> This patch updates the GIC drivers to provide the new interfaces.
> The changes comprise:
> Update to use HardwareInterrupt2 protocol
> Additions to register info in ArmGicLib.h
> Added new functionality (GetTriggerType and SetTriggerType)
>
> The requirement for this change derives from a problem detected on ARM
> Juno boards, but the change is of generic (ARM) relevance.
>
> This commit is in response to review on the mailing list and, as
> suggested there, rolls Girish's updates onto Ard's original example.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
This patch has lagged: Ard, are you happy to confirm this one could be
updated to 1.1?
/
Leif
> 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>
> ---
>
> Notes:
> 376_irqtype_v4
> [Leif Lindholm]
> - Fixed continuation line indent
> - Moved EFI_D_ERROR changes out
> - Replaced MmioBitFieldRead32
> - Fixed indent and parentheses
>
> ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 3 +-
> ArmPkg/Drivers/ArmGic/ArmGicDxe.h | 23 ++++
> ArmPkg/Include/Library/ArmGicLib.h | 11 +-
> ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c | 43 ++++++
> ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 137 ++++++++++++++++++++
> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 136 +++++++++++++++++++
> 6 files changed, 351 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> index e554301c4b28022c805f69242cf6ee979d19abc2..d5921533fb68fa32c3e0705b05930700ee81da07 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> @@ -1,7 +1,7 @@
> #/** @file
> #
> # Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> -# Copyright (c) 2012 - 2015, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2012 - 2017, ARM Ltd. All rights reserved.<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> @@ -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 76945282d9b9bbf6da872a408a56387ee321a3b5..610ffacc7eb025011f87ad5e6e457a610bd570a9 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;
> @@ -32,6 +33,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
> );
> @@ -58,4 +60,25 @@ GicV3DxeInitialize (
> IN EFI_SYSTEM_TABLE *SystemTable
> );
>
> +
> +// Shared code
> +
> +/**
> + Calculate GICD_ICFGRn base address and corresponding bit
> + field Int_config[1] of the GIC distributor register.
> +
> + @param Source Hardware source of the interrupt.
> + @param RegAddress Corresponding GICD_ICFGRn base address.
> + @param Config1Bit Bit number of F Int_config[1] bit in the register.
> +
> + @retval EFI_SUCCESS Source interrupt supported.
> + @retval EFI_UNSUPPORTED Source interrupt is not supported.
> +**/
> +EFI_STATUS
> +GicGetDistributorIcfgBaseAndBit (
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + OUT UINTN *RegAddress,
> + OUT UINTN *Config1Bit
> + );
> +
> #endif
> diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h
> index a4ede82bc6c07733f880a12fff54001590a0518a..4b21ea9e4e76cb83c0c3421c1d9d88b456192687 100644
> --- a/ArmPkg/Include/Library/ArmGicLib.h
> +++ b/ArmPkg/Include/Library/ArmGicLib.h
> @@ -49,8 +49,17 @@
> #define ARM_GIC_ICDDCR_ARE (1 << 4) // Affinity Routing Enable (ARE)
> #define ARM_GIC_ICDDCR_DS (1 << 6) // Disable Security (DS)
>
> +// GICD_ICDICFR bits
> +#define ARM_GIC_ICDICFR_WIDTH 32 // ICDICFR is a 32 bit register
> +#define ARM_GIC_ICDICFR_BYTES (ARM_GIC_ICDICFR_WIDTH / 8)
> +#define ARM_GIC_ICDICFR_F_WIDTH 2 // Each F field is 2 bits
> +#define ARM_GIC_ICDICFR_F_STRIDE 16 // (32/2) F fields per register
> +#define ARM_GIC_ICDICFR_F_CONFIG1_BIT 1 // Bit number within F field
> +#define ARM_GIC_ICDICFR_LEVEL_TRIGGERED 0x0 // Level triggered interrupt
> +#define ARM_GIC_ICDICFR_EDGE_TRIGGERED 0x1 // Edge triggered interrupt
> +
> +
> // GIC Redistributor
> -
> #define ARM_GICR_CTLR_FRAME_SIZE SIZE_64KB
> #define ARM_GICR_SGI_PPI_FRAME_SIZE SIZE_64KB
>
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
> index 7ca3ca8f52afe52c827ca82b749b665184813fc1..bff8d983cf02f7fd1fa6165824cb6556cdea5c6f 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicCommonDxe.c
> @@ -39,6 +39,46 @@ UINTN mGicNumInterrupts = 0;
>
> HARDWARE_INTERRUPT_HANDLER *gRegisteredInterruptHandlers = NULL;
>
> +
> +/**
> + Calculate GICD_ICFGRn base address and corresponding bit
> + field Int_config[1] of the GIC distributor register.
> +
> + @param Source Hardware source of the interrupt.
> + @param RegAddress Corresponding GICD_ICFGRn base address.
> + @param Config1Bit Bit number of F Int_config[1] bit in the register.
> +
> + @retval EFI_SUCCESS Source interrupt supported.
> + @retval EFI_UNSUPPORTED Source interrupt is not supported.
> +**/
> +EFI_STATUS
> +GicGetDistributorIcfgBaseAndBit (
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + OUT UINTN *RegAddress,
> + OUT UINTN *Config1Bit
> + )
> +{
> + UINTN RegIndex;
> + UINTN Field;
> +
> + if (Source >= mGicNumInterrupts) {
> + ASSERT(Source < mGicNumInterrupts);
> + return EFI_UNSUPPORTED;
> + }
> +
> + RegIndex = Source / ARM_GIC_ICDICFR_F_STRIDE; // NOTE: truncation is significant
> + Field = Source % ARM_GIC_ICDICFR_F_STRIDE;
> + *RegAddress = PcdGet64 (PcdGicDistributorBase)
> + + ARM_GIC_ICDICFR
> + + (ARM_GIC_ICDICFR_BYTES * RegIndex);
> + *Config1Bit = ((Field * ARM_GIC_ICDICFR_F_WIDTH)
> + + ARM_GIC_ICDICFR_F_CONFIG1_BIT);
> +
> + return EFI_SUCCESS;
> +}
> +
> +
> +
> /**
> Register Handler for the specified interrupt source.
>
> @@ -84,6 +124,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
> )
> @@ -103,6 +144,8 @@ InstallAndRegisterInterruptService (
> &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 3d5d034efbb1bf2e1e84841cab4175bc607845c2..7717ebe75330562db61a30ab4e5c5ad559c5f201 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> @@ -29,6 +29,7 @@ Abstract:
> #define ARM_GIC_DEFAULT_PRIORITY 0x80
>
> extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol;
> +extern EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol;
>
> STATIC UINT32 mGicInterruptInterfaceBase;
> STATIC UINT32 mGicDistributorBase;
> @@ -198,6 +199,141 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol = {
> GicV2EndOfInterrupt
> };
>
> +/**
> + Get interrupt trigger type of an interrupt
> +
> + @param This Instance pointer for this protocol
> + @param Source Hardware source of the interrupt.
> + @param TriggerType Returns interrupt trigger type.
> +
> + @retval EFI_SUCCESS Source interrupt supported.
> + @retval EFI_UNSUPPORTED Source interrupt is not supported.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicV2GetTriggerType (
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE *TriggerType
> + )
> +{
> + UINTN RegAddress;
> + UINTN Config1Bit;
> + EFI_STATUS Status;
> +
> + Status = GicGetDistributorIcfgBaseAndBit (
> + Source,
> + &RegAddress,
> + &Config1Bit
> + );
> +
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if ((MmioRead32 (RegAddress) & (1 << Config1Bit)) == 0) {
> + *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH;
> + } else {
> + *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Set interrupt trigger type of an interrupt
> +
> + @param This Instance pointer for this protocol
> + @param Source Hardware source of the interrupt.
> + @param TriggerType Interrupt trigger type.
> +
> + @retval EFI_SUCCESS Source interrupt supported.
> + @retval EFI_UNSUPPORTED Source interrupt is not supported.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicV2SetTriggerType (
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + IN EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE TriggerType
> + )
> +{
> + UINTN RegAddress;
> + UINTN Config1Bit;
> + UINT32 Value;
> + EFI_STATUS Status;
> + BOOLEAN SourceEnabled;
> +
> + if ( (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
> + && (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH)) {
> + DEBUG ((DEBUG_ERROR, "Invalid interrupt trigger type: %d\n", \
> + TriggerType));
> + ASSERT (FALSE);
> + return EFI_UNSUPPORTED;
> + }
> +
> + Status = GicGetDistributorIcfgBaseAndBit (
> + Source,
> + &RegAddress,
> + &Config1Bit
> + );
> +
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Status = GicV2GetInterruptSourceState (
> + (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
> + Source,
> + &SourceEnabled
> + );
> +
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Value = (TriggerType == EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
> + ? ARM_GIC_ICDICFR_EDGE_TRIGGERED
> + : ARM_GIC_ICDICFR_LEVEL_TRIGGERED;
> +
> + // Before changing the value, we must disable the interrupt,
> + // otherwise GIC behavior is UNPREDICTABLE.
> + if (SourceEnabled) {
> + GicV2DisableInterruptSource (
> + (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
> + Source
> + );
> + }
> +
> + MmioAndThenOr32 (
> + RegAddress,
> + ~(0x1 << Config1Bit),
> + Value << Config1Bit
> + );
> +
> + // Restore interrupt state
> + if (SourceEnabled) {
> + GicV2EnableInterruptSource (
> + (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
> + Source
> + );
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V2Protocol = {
> + (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
>
> @@ -321,6 +457,7 @@ GicV2DxeInitialize (
>
> Status = InstallAndRegisterInterruptService (
> &gHardwareInterruptV2Protocol,
> + &gHardwareInterrupt2V2Protocol,
> GicV2IrqInterruptHandler,
> GicV2ExitBootServicesEvent
> );
> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> index 50b51ffcf57b23b110899c7c55237cf85b61d196..01154848f4439af3e87e89cf796f3d69ce4a0c4a 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> @@ -19,6 +19,7 @@
> #define ARM_GIC_DEFAULT_PRIORITY 0x80
>
> extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol;
> +extern EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol;
>
> STATIC UINTN mGicDistributorBase;
> STATIC UINTN mGicRedistributorsBase;
> @@ -192,6 +193,140 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV3Protocol = {
> GicV3EndOfInterrupt
> };
>
> +/**
> + Get interrupt trigger type of an interrupt
> +
> + @param This Instance pointer for this protocol
> + @param Source Hardware source of the interrupt.
> + @param TriggerType Returns interrupt trigger type.
> +
> + @retval EFI_SUCCESS Source interrupt supported.
> + @retval EFI_UNSUPPORTED Source interrupt is not supported.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicV3GetTriggerType (
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + OUT EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE *TriggerType
> + )
> +{
> + UINTN RegAddress;
> + UINTN Config1Bit;
> + EFI_STATUS Status;
> +
> + Status = GicGetDistributorIcfgBaseAndBit (
> + Source,
> + &RegAddress,
> + &Config1Bit
> + );
> +
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if ((MmioRead32 (RegAddress) & (1 << Config1Bit)) == 0) {
> + *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH;
> + } else {
> + *TriggerType = EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Set interrupt trigger type of an interrupt
> +
> + @param This Instance pointer for this protocol
> + @param Source Hardware source of the interrupt.
> + @param TriggerType Interrupt trigger type.
> +
> + @retval EFI_SUCCESS Source interrupt supported.
> + @retval EFI_UNSUPPORTED Source interrupt is not supported.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicV3SetTriggerType (
> + IN EFI_HARDWARE_INTERRUPT2_PROTOCOL *This,
> + IN HARDWARE_INTERRUPT_SOURCE Source,
> + IN EFI_HARDWARE_INTERRUPT2_TRIGGER_TYPE TriggerType
> + )
> +{
> + UINTN RegAddress;
> + UINTN Config1Bit;
> + UINT32 Value;
> + EFI_STATUS Status;
> + BOOLEAN SourceEnabled;
> +
> + if ( (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
> + && (TriggerType != EFI_HARDWARE_INTERRUPT2_TRIGGER_LEVEL_HIGH)) {
> + DEBUG ((DEBUG_ERROR, "Invalid interrupt trigger type: %d\n", \
> + TriggerType));
> + ASSERT (FALSE);
> + return EFI_UNSUPPORTED;
> + }
> +
> + Status = GicGetDistributorIcfgBaseAndBit (
> + Source,
> + &RegAddress,
> + &Config1Bit
> + );
> +
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Status = GicV3GetInterruptSourceState (
> + (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
> + Source,
> + &SourceEnabled
> + );
> +
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Value = (TriggerType == EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING)
> + ? ARM_GIC_ICDICFR_EDGE_TRIGGERED
> + : ARM_GIC_ICDICFR_LEVEL_TRIGGERED;
> +
> + // Before changing the value, we must disable the interrupt,
> + // otherwise GIC behavior is UNPREDICTABLE.
> + if (SourceEnabled) {
> + GicV3DisableInterruptSource (
> + (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
> + Source
> + );
> + }
> +
> + MmioAndThenOr32 (
> + RegAddress,
> + ~(0x1 << Config1Bit),
> + Value << Config1Bit
> + );
> + // Restore interrupt state
> + if (SourceEnabled) {
> + GicV3EnableInterruptSource (
> + (EFI_HARDWARE_INTERRUPT_PROTOCOL*)This,
> + Source
> + );
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +EFI_HARDWARE_INTERRUPT2_PROTOCOL gHardwareInterrupt2V3Protocol = {
> + (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
>
> @@ -353,6 +488,7 @@ GicV3DxeInitialize (
>
> Status = InstallAndRegisterInterruptService (
> &gHardwareInterruptV3Protocol,
> + &gHardwareInterrupt2V3Protocol,
> GicV3IrqInterruptHandler,
> GicV3ExitBootServicesEvent
> );
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
2017-10-05 20:15 ` Leif Lindholm
@ 2017-10-05 20:15 ` Ard Biesheuvel
2017-10-05 20:25 ` Leif Lindholm
0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2017-10-05 20:15 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Evan Lloyd, Matteo Carlini
On 5 October 2017 at 21:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Sep 21, 2017 at 05:23:43PM +0100, evan.lloyd@arm.com wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> The existing HardwareInterrupt protocol lacked a means to configure the
>> level/edge properties of an interrupt. The new HardwareInterrupt2
>> protocol introduced this capability.
>> This patch updates the GIC drivers to provide the new interfaces.
>> The changes comprise:
>> Update to use HardwareInterrupt2 protocol
>> Additions to register info in ArmGicLib.h
>> Added new functionality (GetTriggerType and SetTriggerType)
>>
>> The requirement for this change derives from a problem detected on ARM
>> Juno boards, but the change is of generic (ARM) relevance.
>>
>> This commit is in response to review on the mailing list and, as
>> suggested there, rolls Girish's updates onto Ard's original example.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>
> This patch has lagged: Ard, are you happy to confirm this one could be
> updated to 1.1?
>
Absolutely.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
2017-10-05 20:15 ` Ard Biesheuvel
@ 2017-10-05 20:25 ` Leif Lindholm
0 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2017-10-05 20:25 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, evan.lloyd, matteo.carlini
On Thu, Oct 05, 2017 at 09:15:41PM +0100, Ard Biesheuvel wrote:
> On 5 October 2017 at 21:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Thu, Sep 21, 2017 at 05:23:43PM +0100, evan.lloyd@arm.com wrote:
> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> The existing HardwareInterrupt protocol lacked a means to configure the
> >> level/edge properties of an interrupt. The new HardwareInterrupt2
> >> protocol introduced this capability.
> >> This patch updates the GIC drivers to provide the new interfaces.
> >> The changes comprise:
> >> Update to use HardwareInterrupt2 protocol
> >> Additions to register info in ArmGicLib.h
> >> Added new functionality (GetTriggerType and SetTriggerType)
> >>
> >> The requirement for this change derives from a problem detected on ARM
> >> Juno boards, but the change is of generic (ARM) relevance.
> >>
> >> This commit is in response to review on the mailing list and, as
> >> suggested there, rolls Girish's updates onto Ard's original example.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >
> > This patch has lagged: Ard, are you happy to confirm this one could be
> > updated to 1.1?
> >
>
> Absolutely.
Many thanks: series pushed as fe4049471b..baee8efb36
/
Leif
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-10-05 20:22 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-21 16:23 [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM evan.lloyd
2017-09-21 16:23 ` [PATCH 1/4] ArmPkg: Tidy GIC code before changes evan.lloyd
2017-09-21 16:23 ` [PATCH 2/4] EmbeddedPkg: Introduce HardwareInterrupt2 protocol evan.lloyd
2017-09-21 16:23 ` [PATCH 3/4] ArmPkg/ArmGicDxe: Expose " evan.lloyd
2017-10-05 20:15 ` Leif Lindholm
2017-10-05 20:15 ` Ard Biesheuvel
2017-10-05 20:25 ` Leif Lindholm
2017-09-21 16:23 ` [PATCH 4/4] ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type evan.lloyd
2017-09-21 17:56 ` [PATCH 0/4] [PATCH 0/5] Add HardwareInterrupt2 for ARM Leif Lindholm
2017-09-21 18:39 ` Evan Lloyd
2017-09-21 19:15 ` Andrew Fish
2017-09-21 20:53 ` Evan Lloyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox