Skip to content

core: using native interrupt handlers - #215

Merged
jockebech merged 1 commit into
OP-TEE:masterfrom
etienne-lms:itrchip
Oct 18, 2023
Merged

core: using native interrupt handlers#215
jockebech merged 1 commit into
OP-TEE:masterfrom
etienne-lms:itrchip

Conversation

@etienne-lms

Copy link
Copy Markdown
Contributor

Adds a section in core architecture documentation to describe interrupt controllers API functions.
These commits are based on stalled P-R #192.

The 1st commit describes the API since OP-TEE/optee_os#6354 has been merged.
The 2nd commit is an RFC describing the API as per P-R OP-TEE/optee_os#6361.

@etienne-lms etienne-lms changed the title core: using native interrupt handlers #192 core: using native interrupt handlers Oct 11, 2023

@etienne-lms etienne-lms left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jbech-linaro, i think the 1st change should be merged before next tag. It descrides the existing implmentation. I'll remove the 2nd commit and will post it upon OP-TEE/optee_os#6361 completion.

Comment thread architecture/core.rst
@etienne-lms

Copy link
Copy Markdown
Contributor Author

I have remove the description related to P-R #6361, used a specific section for main controller info, rephrased a bit the description and add interrupt_call_handlers() info.
I think this change should in next tag documentation.

Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
@etienne-lms

Copy link
Copy Markdown
Contributor Author

comments addressed.

Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated

@jenswikl jenswikl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With my last comment addressed:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

Comment thread architecture/core.rst Outdated

@etienne-lms etienne-lms left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've sqaushed the fixup commits + the change below (missing quotes around function labels), and applied your review tag. Thanks for the review.

     - ``interrupt_mask()`` and ``interrupt_unmask()`` to respectively
-      mask or unmask an interrupt.
-      Masking of an enabled interrupt temporarily disables the interrupt
-      while unmasking enables a previously masked interrupt. interrpt_mask()
-      and interrpt_unmask() are allowed to be called from an interrupt
-      context, but interrupt_enable() and interrupt_disable() not so.
+      mask or unmask an interrupt. Masking of an enabled interrupt
+      temporarily disables the interrupt while unmasking enables a
+      previously masked interrupt. ``interrupt_mask()`` and
+      ``interrupt_unmask()`` are allowed to be called from an interrupt
+      context, but ``interrupt_enable()`` and ``interrupt_disable()`` not so.

Note: the last 'so' sound strange to me.

@jockebech

Copy link
Copy Markdown
Contributor

Not sure why this pass on readthedocs, since I see both Error and warnings when looking that the readthedocs log. I noticed that when building this locally:

/home/jyx/devel/optee_projects/optee_docs/architecture/core.rst:419: ERROR: Unexpected indentation.
/home/jyx/devel/optee_projects/optee_docs/architecture/core.rst:411: ERROR: Unknown target name: "interrupt.h".
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] index
/home/jyx/devel/optee_projects/optee_docs/architecture/core.rst:468: WARNING: undefined label: '_notifications'

@etienne-lms

Copy link
Copy Markdown
Contributor Author

Thanks @jbech-linaro. Indeed I forgot to test building the doc.
Now it should be ok with the appended fixup commit.

Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
Comment thread architecture/core.rst Outdated
@etienne-lms

Copy link
Copy Markdown
Contributor Author

comments addressed

@jockebech

Copy link
Copy Markdown
Contributor

Thanks @etienne-lms

Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

@etienne-lms

Copy link
Copy Markdown
Contributor Author

@jenswi-linaro, I guess your review tag still applies.

Adds a section in core architecture documentation on how to
implement and register interrupt controllers and handler functions.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
@jenswikl

Copy link
Copy Markdown
Contributor

@jenswi-linaro, I guess your review tag still applies.

Yes :-)

@etienne-lms

Copy link
Copy Markdown
Contributor Author

fixup commits squashed
review tags applied
thanks for the fixes

@jockebech
jockebech merged commit eda1e0f into OP-TEE:master Oct 18, 2023
@etienne-lms
etienne-lms deleted the itrchip branch October 31, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants