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

Arch arm cortex m direct dynamic irqs #21516

Conversation

ioannisg
Copy link
Member

@ioannisg ioannisg commented Dec 19, 2019

DIrect Dynamic Interrupt Feature support for ARM Cortex-M

@ioannisg ioannisg added the Release Notes To be mentioned in the release notes label Dec 19, 2019
@ioannisg ioannisg added this to the v2.2.0 milestone Dec 19, 2019
@zephyrbot zephyrbot added area: ARM ARM (32-bit) Architecture area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test labels Dec 19, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Dec 19, 2019

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@ioannisg ioannisg force-pushed the arch_arm_cortex_m_direct_dynamic_irqs branch 2 times, most recently from c66ec9e to 2545b32 Compare December 19, 2019 12:05
@ioannisg ioannisg force-pushed the arch_arm_cortex_m_direct_dynamic_irqs branch 2 times, most recently from 5d7185d to c1f1bba Compare December 19, 2019 13:37
@ioannisg ioannisg marked this pull request as ready for review December 19, 2019 13:37
@ioannisg ioannisg self-assigned this Dec 19, 2019
include/arch/arm/irq.h Outdated Show resolved Hide resolved
@ioannisg ioannisg force-pushed the arch_arm_cortex_m_direct_dynamic_irqs branch 2 times, most recently from 3b84983 to c867075 Compare December 20, 2019 10:06
@carlescufi
Copy link
Member

@ioannisg please rebase

@ioannisg ioannisg force-pushed the arch_arm_cortex_m_direct_dynamic_irqs branch from bcd144f to c00f6d3 Compare December 22, 2019 23:00
@ioannisg
Copy link
Member Author

@ioannisg please rebase

@carlescufi rebased :)

@carlescufi
Copy link
Member

@ioannisg there seems to be an issue with shippable. Perhaps it just needs a rebase.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for doc changes

Copy link
Contributor

@andrewboie andrewboie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No complaints other than a suspicion that the Kconfig isn't quite ideal

arch/arm/core/aarch32/cortex_m/Kconfig Outdated Show resolved Hide resolved
help
Enable installation of interrupt service routines for Direct
interrupts at runtime.
Note: this enables support for dynamic interrupts in the kernel.
Copy link
Collaborator

@ulfalizer ulfalizer Jan 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could flesh out this help text a bit maybe. Could explain what direct interrupts are (and maybe lowercase 'direct'), give some links, mention any Zephyr-specifics, etc. In the Linux kernel, checkpatch.pl warns if the help text is less than four lines, but maybe our version is old.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. So, I've changed 'Direct' to lowercase, thanks.

Well, the provided API (see in this PR) has all the documentation for direct dynamic interrupts. So I've just copied here a minor phrase briefly explaining direct interrupts.

In general the help-text of Kconfig symbols does not need to explain in detail the feature/functionality; we've got documentation for that. :)

@ioannisg ioannisg force-pushed the arch_arm_cortex_m_direct_dynamic_irqs branch from cd4ac87 to 413144f Compare January 3, 2020 09:19
Copy link
Member Author

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ulfalizer comments addressed, pls, take another look

@ioannisg
Copy link
Member Author

ioannisg commented Jan 7, 2020

@ulfalizer @andrewboie could you re-visit your reviews, here, so we could resolve this one?

@andrewboie
Copy link
Contributor

Please change the Kconfig to have "depends on" and not "select". Users will need to enable both.

With this commit we add support for Dynamic Direct interrupts
for the ARM Cortex-M architecture. For that we introduce a new,
user-enabled, Kconfig symbol, DYNAMIC_DIRECT_INTERRUPTS.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
This commits implements the support for dynamic direct
interrupts for the ARM Cortex-M architecture, and exposes
the support to the user as an ARM-only API.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
Document that the Dynamic Direct interrupts feature is
implemented and supported as an ARM-only API.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
Move the zero-latency IRQ test into the new
arm_irq_advanced_features' test suite. Skip
running the test for non Mainline Cortex-M.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
We add a test-suite for the newly introduced feature of
ARM Dynamic Direct Interrupts.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg ioannisg force-pushed the arch_arm_cortex_m_direct_dynamic_irqs branch from 413144f to 2184cec Compare January 8, 2020 08:31
@ioannisg
Copy link
Member Author

ioannisg commented Jan 8, 2020

Please change the Kconfig to have "depends on" and not "select". Users will need to enable both.

That's alright then, @andrewboie , I' ve done that now, so using this feature requires both Kconfigs be set, thanks for looking at it.

@andrewboie andrewboie merged commit 80992d0 into zephyrproject-rtos:master Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: ARM ARM (32-bit) Architecture area: Documentation area: Tests Issues related to a particular existing or missing test Release Notes To be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants