interrupt_controller: gic: Support PPIs #19860

carlocaione · 2019-10-16T13:20:55Z

The GIC-400 driver currently only supports SPIs because the (32) offset
for the INTIDs is hard-coded in the driver. At the driver level there is
no really difference between PPIs and SPIs so we can easily extend the
driver to support PPIs as well.

This is useful if we want to add support for the ARM Generic Timers that
use INTIDs in the PPI range.

SPI interrupts are in the range [0-987]. PPI interrupts are in the
range [0-15].

This patches introduces a new interrupt type (GIC_SPI | GIC_PPI) to be
defined in the DTS to mark the IRQ as SPI or PPI, following what's done
in the Linux kernel. When the IRQ is marked as SPI its INTID is
offsetted by 32, otherwise by 16 when marked as PPI. The fix is done
introducing a new macro to be used in the DTS fixup file.

Signed-off-by: Carlo Caione [email protected]

carlocaione · 2019-10-18T08:24:29Z

Gentle ping tagging @bbolen @stephanosio @ioannisg @andrewboie because previously involved with related stuff in the past.

include/dt-bindings/interrupt-controller/arm-gic.h

stephanosio · 2019-10-18T09:07:54Z

Also it may be worth noting that there was some major refactoring done on GIC driver and Cortex-R interrupt system in general here: 5772e52 82dd67b

carlocaione · 2019-10-18T09:12:23Z

@stephanosio damn, I was not aware of that. At this point I think the best way forward is just for you to incorporate these changes. Is that ok for you?

stephanosio · 2019-10-18T09:22:13Z

@stephanosio damn, I was not aware of that. At this point I think the best way forward is just for you to incorporate these changes. Is that ok for you?

Since we're already here (and #19698 has already become way too big for what it was initially intended for), I think it would be better if these changes get reviewed and merged here.

It is very likely that this PR will get merged before #19698, so once that happens, it will cause a merge conflict for #19698 and I will make the necessary adoptions there to incorporate these changes.

carlocaione · 2019-10-18T09:37:29Z

@stephanosio ACK. Pushed a new rev.

galak · 2019-10-18T14:03:04Z

Since you are reworking this can we get the dts for GIC to align with https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/tree/Bindings/interrupt-controller/arm,gic.yaml

carlocaione · 2019-10-18T14:11:49Z

@galak we cannot fully align if I follow the @stephanosio suggestion of using GIC_SPI / GIC_PPI as macros instead of flags for the type as done on the Linux kernel. Also on Linux there is no really support for IRQ priority.

stephanosio · 2019-10-18T14:30:00Z

Yea, the current Zephyr interrupt API won't be able to accommodate the Linux GIC dts without introducing many hacks (such as specifying dts_fixup for every GIC mapped interrupt, which is a major hassle).

The main reason is that Linux IRQ control API accepts struct irq_data, whereas Zephyr only accepts single unsigned int irq

carlocaione · 2019-10-21T10:13:58Z

agree. @galak given that is unlikely that we can align with the Linux kernel soon, I propose to merge the commit as is (with yours and @stephanosio ACK).

stephanosio

NOTE: Once merged, to be re-integrated into the refactored gic driver in #19698; unit tests for in-depth functional verification is to be implemented in the future.

stephanosio · 2019-10-23T01:15:00Z

cc @carlocaione @timork @vladimir-podbrezsky

carlocaione · 2019-10-26T17:56:56Z

@galak can you merge this? thanks.

carlocaione · 2019-10-30T13:48:19Z

@galak please, consider this blocked on #20243. So wait for that before merging this.

galak · 2019-10-30T15:49:18Z

Yea, the current Zephyr interrupt API won't be able to accommodate the Linux GIC dts without introducing many hacks (such as specifying dts_fixup for every GIC mapped interrupt, which is a major hassle).

The main reason is that Linux IRQ control API accepts struct irq_data, whereas Zephyr only accepts single unsigned int irq

What the issue w/regards to dts_fixup. I'd prefer if we were more consistent with the the linux binding. I get we'll need to differ for priority, but not sure I understand the other issues.

stephanosio · 2019-10-30T16:34:12Z

What the issue w/regards to dts_fixup.

For GIC, there are multiple interrupt types with different IRQ bases. If we follow the Linux dts and specify interrupt type in a separate property, for every single IRQ, we have to map/fix-up, the complex (interrupt type)+(type-dependent interrupt index) definition from dts to the actual linear IRQ number used by the Zephyr interrupt API.

Refer to the following for what has to be done if we follow the Linux dts:
b761bd4#diff-8dec1584212c37fe3f73aa7c195ebde2L9-L16

This is simply not practical. If we really wanted to follow the Linux dts, we will have to redesign the Zephyr interrupt API to either accept interrupt type parameter alongside the IRQ number (which is somewhat interrupt controller-specific and should be avoided) or some kind struct representing an IRQ (as Linux is doing, but this is really an overkill for an OS that is intended to run on environments with very limited resources).

IMHO, following Linux dts is good while it makes sense; but, if you have to sacrifice tidiness of code and sanity of developers over following Linux dts, that is a very questionable approach.

stephanosio · 2019-10-31T18:47:23Z

Please merge this if there is no further comment. I have a quite few changes that are dependent on this.

stephanosio · 2019-11-04T17:37:45Z

Ping

ulfalizer · 2019-11-08T10:49:04Z

scripts/dts/gen_defines.py

@@ -453,7 +467,7 @@ def encode_zephyr_multi_level_irq(irq, irq_num):
        while irq_ctrl.interrupts:
            irq_num = (irq_num + 1) << 8
            if "irq" not in irq_ctrl.interrupts[0].data:
-                err("Expected binding for {!r} to have 'irq' in *-cells"
+                err("Expected binding for {!r} to have 'irq' in interrupt-cells"


Ah... had missed this one. :)

galak

Looks good, one change I'd like to see is swapping the order of flags and priority in the interrupt-cells. This keeps is a bit more compatible with the linux binding, and pretty minor change at this point before we have more dts files that have GICs in them :)

dts/bindings/interrupt-controller/arm,gic.yaml

galak · 2019-11-08T16:13:06Z

Thanks, I know this was a bit long to get ready.

bbolen

Sorry, I'm trying to catch up on all of the cortex changes.
I might suggest squashing this commit on the first one.

carlocaione · 2019-11-17T16:57:45Z

Squashed the commit as suggested by @bbolen and force-pushed again. We should actually squash all the commits together if we care about bisectability.

FWIW #20263 depends on this story.

ioannisg

Looks good to me

The ARM Generic Interrupt Controller (GIC) supports multiple interrupt types whose linear IRQ numbers are offset by a type-specific base number. This commit adds a function to automatically fix up ARM GIC interrupts in order to output a linear IRQ number that is offset by the interrupt type-specific base number. For more details, refer to the issue zephyrproject-rtos#19860. Signed-off-by: Stephanos Ioannidis <[email protected]>

The GIC-400 driver currently only supports SPIs because the (32) offset for the INTIDs is hard-coded in the driver. At the driver level there is no really difference between PPIs and SPIs so we can easily extend the driver to support PPIs as well. This is useful if we want to add support for the ARM Generic Timers that use INTIDs in the PPI range. SPI interrupts are in the range [0-987]. PPI interrupts are in the range [0-15]. This commit adds interrupt 'type' cell to the GIC device tree binding and changes the 'irq' cell to use interrupt type-specific index, rather than a linear IRQ number. The 'type'+'irq (index)' combo is automatically fixed up into a linear IRQ number by the scripts/dts/gen_defines.py script. Signed-off-by: Stephanos Ioannidis <[email protected]> Signed-off-by: Carlo Caione <[email protected]>

The ARM Generic Interrupt Controller (GIC) supports multiple interrupt types whose linear IRQ numbers are offset by a type-specific base number. This commit adds a function to automatically fix up ARM GIC interrupts in order to output a linear IRQ number that is offset by the interrupt type-specific base number. For more details, refer to the issue zephyrproject-rtos#19860. Signed-off-by: Stephanos Ioannidis <[email protected]>

carlocaione requested review from andrewboie, carlescufi, galak, ioannisg, MaureenHelm and nashif as code owners October 16, 2019 13:20

zephyrbot added area: Devicetree area: API Changes to public APIs labels Oct 16, 2019

stephanosio reviewed Oct 18, 2019

View reviewed changes

include/dt-bindings/interrupt-controller/arm-gic.h Outdated Show resolved Hide resolved

carlocaione force-pushed the PPI_GIC branch from b761bd4 to 376a091 Compare October 18, 2019 09:36

stephanosio approved these changes Oct 21, 2019

View reviewed changes

bbolen approved these changes Oct 22, 2019

View reviewed changes

This was referenced Oct 27, 2019

Add support for ARM architected timer #20175

Closed

irq: cortex-r: Fix wrong irq enabling #20243

Merged

stephanosio force-pushed the PPI_GIC branch from c27d440 to 120eb50 Compare November 8, 2019 10:48

ulfalizer reviewed Nov 8, 2019

View reviewed changes

galak requested changes Nov 8, 2019

View reviewed changes

dts/bindings/interrupt-controller/arm,gic.yaml Outdated Show resolved Hide resolved

stephanosio force-pushed the PPI_GIC branch from 120eb50 to a006ffc Compare November 8, 2019 16:10

stephanosio requested a review from galak November 8, 2019 16:11

galak approved these changes Nov 8, 2019

View reviewed changes

bbolen reviewed Nov 8, 2019

View reviewed changes

stephanosio mentioned this pull request Nov 12, 2019

Cortex-R Improvement #20594

Closed

25 tasks

carlocaione force-pushed the PPI_GIC branch from a006ffc to 20d8e98 Compare November 17, 2019 16:54

carlocaione mentioned this pull request Nov 18, 2019

Add Cortex-A53 (ARMv8, AArch64) support #20263

Merged

andrewboie approved these changes Nov 18, 2019

View reviewed changes

ioannisg approved these changes Nov 18, 2019

View reviewed changes

carlocaione and others added 2 commits December 9, 2019 15:56

carlocaione force-pushed the PPI_GIC branch from 20d8e98 to 7297e16 Compare December 9, 2019 15:56

ioannisg merged commit 432c93b into zephyrproject-rtos:master Dec 10, 2019

carlocaione deleted the PPI_GIC branch December 10, 2019 08:28

stephanosio mentioned this pull request May 20, 2020

dts: arm: Incorrect GIC interrupt spec order for AArch64 SoCs #25477

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

interrupt_controller: gic: Support PPIs #19860

interrupt_controller: gic: Support PPIs #19860

carlocaione commented Oct 16, 2019

carlocaione commented Oct 18, 2019

stephanosio commented Oct 18, 2019

carlocaione commented Oct 18, 2019

stephanosio commented Oct 18, 2019

carlocaione commented Oct 18, 2019

galak commented Oct 18, 2019

carlocaione commented Oct 18, 2019

stephanosio commented Oct 18, 2019

carlocaione commented Oct 21, 2019

stephanosio left a comment

stephanosio commented Oct 23, 2019

carlocaione commented Oct 26, 2019

carlocaione commented Oct 30, 2019

galak commented Oct 30, 2019

stephanosio commented Oct 30, 2019

stephanosio commented Oct 31, 2019

stephanosio commented Nov 4, 2019

ulfalizer Nov 8, 2019

galak left a comment

galak commented Nov 8, 2019

bbolen left a comment

carlocaione commented Nov 17, 2019

ioannisg left a comment •

edited

Loading

interrupt_controller: gic: Support PPIs #19860

interrupt_controller: gic: Support PPIs #19860

Conversation

carlocaione commented Oct 16, 2019

carlocaione commented Oct 18, 2019

stephanosio commented Oct 18, 2019

carlocaione commented Oct 18, 2019

stephanosio commented Oct 18, 2019

carlocaione commented Oct 18, 2019

galak commented Oct 18, 2019

carlocaione commented Oct 18, 2019

stephanosio commented Oct 18, 2019

carlocaione commented Oct 21, 2019

stephanosio left a comment

Choose a reason for hiding this comment

stephanosio commented Oct 23, 2019

carlocaione commented Oct 26, 2019

carlocaione commented Oct 30, 2019

galak commented Oct 30, 2019

stephanosio commented Oct 30, 2019

stephanosio commented Oct 31, 2019

stephanosio commented Nov 4, 2019

ulfalizer Nov 8, 2019

Choose a reason for hiding this comment

galak left a comment

Choose a reason for hiding this comment

galak commented Nov 8, 2019

bbolen left a comment

Choose a reason for hiding this comment

carlocaione commented Nov 17, 2019

ioannisg left a comment • edited Loading

Choose a reason for hiding this comment

ioannisg left a comment •

edited

Loading