New API directives to enable/disable interrupt vectors

[Chris Johns]

On 18/6/21 1:28 am, Gedare Bloom wrote:
> On Wed, Jun 16, 2021 at 11:40 PM Sebastian Huber
> <sebastian.huber at embedded-brains.de> wrote:
>>
>> On 16/06/2021 20:36, Gedare Bloom wrote:
>>> Looks like the existing irq-extension.h uses 'vector', so
>>> rtems_interrupt_disable_vector() is a possibility, or else
>>> rtems_interrupt_controller_disable_vector() is more wordy but if we
>>> want to treat 'interrupt controller' as its own category of API
>>> separate from 'interrupt'.
>>
>> Yes, mixing "vector" into the name a good idea.  What about
>> rtems_interrupt_vector_enable() and rtems_interrupt_vector_disable()?
>>
>> I am not that fond of using "interrupt controller" since I don't want to
>> change the existing API and it would result in very long directive names.
>>
> I'm fine with the rtems_interrupt_vector_* to distinguish between the
> score/cpu level support, and the bsp/irq controllers. We already use
> this for example as "bsp_interrupt_vector_enable()". I think it will
> be great to lift this "bsp" interface up to the "rtems" level.

I have no objection to any of this but it raised my interest in what is the
whole rtems_interrupt_* API and how consistent it is ...

I have managed to filter the cpukit headers [1] to get:

rtems_interrupt_catch
rtems_interrupt_cause
rtems_interrupt_clear
rtems_interrupt_disable
rtems_interrupt_enable
rtems_interrupt_flash
rtems_interrupt_is_in_progress
rtems_interrupt_level_body

rtems_interrupt_local_disable
rtems_interrupt_local_enable

rtems_interrupt_handler_install
rtems_interrupt_handler_iterate
rtems_interrupt_handler_remove

rtems_interrupt_get_affinity
rtems_interrupt_set_affinity

rtems_interrupt_lock_acquire
rtems_interrupt_lock_acquire_isr
rtems_interrupt_lock_destroy
rtems_interrupt_lock_initialize
rtems_interrupt_lock_interrupt_disable
rtems_interrupt_lock_release
rtems_interrupt_lock_release_isr

rtems_interrupt_server_action_prepend
rtems_interrupt_server_create
rtems_interrupt_server_delete
rtems_interrupt_server_entry_destroy
rtems_interrupt_server_entry_initialize
rtems_interrupt_server_entry_move
rtems_interrupt_server_entry_submit
rtems_interrupt_server_handler_install
rtems_interrupt_server_handler_iterate
rtems_interrupt_server_handler_remove
rtems_interrupt_server_initialize
rtems_interrupt_server_move
rtems_interrupt_server_request_destroy
rtems_interrupt_server_request_initialize
rtems_interrupt_server_request_set_vector
rtems_interrupt_server_request_submit
rtems_interrupt_server_resume
rtems_interrupt_server_set_affinity
rtems_interrupt_server_suspend

The list does not capture things like rtems_interrupt_mask which is a typedef.

My concern is:

rtems_interrupt_disable
rtems_interrupt_enable

rtems_interrupt_local_disable
rtems_interrupt_local_enable

and then we add:

rtems_interrupt_vector_disable
rtems_interrupt_vector_enable

How do each of these enable/disable pairs interact with the other parts of the API?

This also seems to pull back to Sebastian's initial comment about the
enable/disable being occupied and I wonder if that concern highlights the need
for some cleaning in the API? It is does it may take a release or 2 to migrate
users but it may be worth it in the long run if the API is made clearer.

I am not across enough of the detail to comment specifically about this and I am
happy to see what others think and follow their lead.

Chris

[1] egrep -r 'rtems_interrupt_.*\(' cpukit/include | sed -e 's/.*
\(rtems_interrupt_.*\).*/\1/' | sed -e 's/(.*//' | grep '^rtems' | sort | uniq