Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

armv8-r/gicv3: support fiq #14929

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Conversation

jinliangli
Copy link
Contributor

support fiq decoding and dispatch

Summary

support armv8r gicv3 fiq

Impact

armv8r gicv3 irq/fiq handling

Testing

armv8r cortex-r52 cpu.

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Size: M The size of the change in this PR is medium labels Nov 25, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 25, 2024

[Experimental Bot, please feedback here]

No, this PR description does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details in several sections. Here's a breakdown:

Missing/Insufficient Information:

  • Summary:

    • Lacks a clear explanation of why FIQ support is necessary. Is it fixing a bug, adding a new feature, or improving performance?
    • "support armv8r gicv3 fiq" is too vague. What part of the code is changed? Mention specific files/modules (e.g., arch/arm/src/armv8-r/gicv3.c). Explain how the FIQ support is implemented. Does it involve new data structures, algorithms, or modifications to existing interrupt handling?
    • Missing related issue references. Even if there isn't a directly related issue, stating "N/A" is better than omitting it entirely.
  • Impact:

    • The impact descriptions are too brief. Instead of simply saying "armv8r gicv3 irq/fiq handling," specify the changes in handling. For example, "Added support for FIQ interrupts in the GICv3 driver, allowing faster handling of critical interrupts."
    • All the "NO / YES" questions are unanswered. Even if the answer is "NO," explicitly state it. If "YES," provide a detailed explanation as requested.
  • Testing:

    • "armv8r cortex-r52 cpu" is insufficient. Specify the exact build host details (OS, compiler version, etc.) and target configuration (board, config, etc.).
    • The "Testing logs before change" and "Testing logs after change" sections are empty. Provide actual logs demonstrating the change's effect. Even simple logs showing FIQ interrupts being handled are better than nothing. Explain what the logs demonstrate.

Example of Improved Sections:

Summary:

This PR adds support for FIQ (Fast Interrupt Request) handling on ARMv8-R processors using the GICv3 interrupt controller. This is necessary to enable faster response times for critical real-time operations. The change modifies the GICv3 driver (arch/arm/src/armv8-r/gicv3.c) to decode and dispatch FIQs, and updates the interrupt handling logic in the kernel (arch/arm/src/armv8-r/up_irq_dispatch.c). Related NuttX Issue: N/A. Related NuttX Apps Issue/PR: N/A.

Impact:

  • Is new feature added? YES (Added FIQ support for ARMv8-R GICv3).
  • Impact on user: YES (Users can now configure and utilize FIQs for time-critical interrupt handling).
  • Impact on build: NO
  • Impact on hardware: YES (Affects ARMv8-R architectures using the GICv3 interrupt controller).
  • Impact on documentation: YES (Documentation will be updated to describe FIQ configuration and usage).
  • Impact on security: NO
  • Impact on compatibility: NO
  • Anything else to consider: None.

Testing:

  • Build Host: Linux (Ubuntu 22.04), x86_64, GCC 11.2.0
  • Target: QEMU-emulated ARMv8-R Cortex-R52, board:qemu-r52_defconfig

Testing logs before change:

(Showing no FIQ handling)
...

Testing logs after change:

(Showing FIQ interrupts being handled correctly)
...

By providing more specific details and addressing all the required points, you can significantly improve the quality and acceptance rate of your NuttX PR.

arch/arm/src/armv8-r/arm_gicv3.c Outdated Show resolved Hide resolved
arch/arm/src/armv8-r/arm_gicv3.c Outdated Show resolved Hide resolved
arch/arm/src/armv8-r/arm_gicv3.c Outdated Show resolved Hide resolved
1. support fiq decoding and dispatch
2. replace CONFIG_ARMV8R_DECODEFIQ with CONFIG_ARCH_HIPRI_INTERRUPT

Signed-off-by: Jinliang Li <[email protected]>
@xiaoxiang781216 xiaoxiang781216 merged commit 37a0445 into apache:master Nov 27, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants