Testing the interrupt extension API?

[Gedare Bloom]

On Fri, Oct 4, 2019 at 12:34 AM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
> On 02/10/2019 19:44, Gedare Bloom wrote:
> > On Wed, Oct 2, 2019 at 1:28 AM Sebastian Huber
> > <sebastian.huber at embedded-brains.de> wrote:
> >>
> >> Hello,
> >>
> >> the interrupt extension API implementation is already quite complex and
> >> not covered by the test suite:
> >>
> >> https://git.rtems.org/rtems/tree/bsps/shared/irq
> >>
> >> In order to write generic tests for this we have to know which interrupt
> >> vector can be controlled by software. One approach would be to add an
> >> rtems_interrupt_get_capabilities() function:
> >>
> >> /**
> >>    * @brief A bit field indicating interrupt capabilities.
> >>    */
> > I don't care for the term 'capabilities' here, maybe 'attributes' is
> > OK? Can we overload these fields onto rtems_attribute?
>
> Yes, attributes is better.
>
> >
> >> typedef struct {
> >>     /**
> >>      * @brief Is set, if the interrupt can be enabled through
> >>      * rtems_interrupt_vector_enable(), otherwise cleared.
> >>      */
> >>     unsigned int can_enable : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt can be enabled through
> >>      * rtems_interrupt_vector_enable(), otherwise cleared.
> > disabled / disable()
> >>      */
> >>     unsigned int can_disable : 1;
> > Curious: are there vectors that can be enabled but not disabled (or vice versa)?
>
> Probably not.
Then you would only need one state bit, since these two would always
be identical? It's ok to leave it as is, but just may be wasteful.

>
> >
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt can be raised through
> >>      * rtems_interrupt_raise(), otherwise cleared.
> >>      */
> >>     unsigned int can_raise : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt can be cleared through
> >>      * rtems_interrupt_clear(), otherwise cleared.
> >>      */
> >>     unsigned int can_clear : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt supports a setting of its priority
> >> through
> >>      * rtems_interrupt_set_priority(), otherwise cleared.
> >>      */
> >>     unsigned int can_set_priority : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt supports a setting of its processor
> >>      * affinity through rtems_interrupt_set_affinity(), otherwise cleared.
> >>      */
> >>     unsigned int can_set_affinity : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt has a pending state which can be
> >> obtained
> >>      * through rtems_interrupt_is_pending(), otherwise cleared.
> >>      */
> >>     unsigned int has_pending_state : 1;
> > This is strange wording, because it implies a currently pending state.
> > Maybe, "can_be_pending"?
> >
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt has an active state which can be
> >> obtained
> >>      * through rtems_interrupt_is_active(), otherwise cleared.
> >>      */
> >>     unsigned int has_active_state : 1;
> > ditto: can_be_active?
>
> Yes, sounds good.
>
> >
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt is connected to a peripheral,
> >> otherwise
> >>      * cleared.
> >>      */
> >>     unsigned int has_peripheral : 1;
> >>
> > I guess this one is more a statement of fact.
>
> What do you mean by this?
>
> We have to know if an interrupt is connected to a peripheral function.
> Since these interrupts must not be used by a generic test. The generic
> test has no idea in which state a peripheral is.
>
Right, I was thinking about the previous fields, where
"can_have_peripheral" doesn't make a lot of sense, so this is an OK
flag name, except ....

> >
> >>     /**
> >>      * @brief Is set, if the interrupt must be raised through
> >>      * rtems_interrupt_raise_on(), otherwise cleared.
> >>      */
> >>     unsigned int needs_target : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt is triggered by a raising signal edge,
> >>      * otherwise cleared.
> >>      */
> >>     unsigned int is_raising_edge_triggered : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt is triggered by a falling signal edge,
> >>      * otherwise cleared.
> >>      */
> >>     unsigned int is_falling_edge_triggered : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt is triggered by a low signal level,
> >>      * otherwise cleared.
> >>      */
> >>     unsigned int is_low_level_triggered : 1;
> >>
> >>     /**
> >>      * @brief Is set, if the interrupt is triggered by a high signal level,
> >>      * otherwise cleared.
> >>      */
> >>     unsigned int is_high_level_triggered : 1;
> >> } rtems_interrupt_capabilities;
> >>
> >> /**
> >>    * @brief Gets the capabilities of the specified interrupt.
> >>    *
> >>    * @retval RTEMS_SUCCESSFUL Successful operation.
> >>    * @retval RTEMS_INVALID_ID The specified vector number is invalid.
> >>    */
> >> rtems_status_code rtems_interrupt_get_capabilities(
> >>     rtems_vector_number vector,
> >>     rtems_interrupt_capabilities *capabilities
> >> );
> >>
> >> Interrupts with cap.can_raise set and cap.has_peripheral cleared can be
> >> safely software controlled and used for tests.
> > Why not just have an "is_software_triggered"?
>
> As a replacement for has_peripheral?
>
yes, it seems that if an interrupt is software triggered, then it
cannot have a peripheral. I don't know if the opposite is true though,
I guess there can be interrupt lines that are not software triggered,
but don't have a peripheral attached to them, but then they are not
active lines they can't actually raise an interrupt.  I don't know if
that makes any sense.

> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax     : +49 89 189 47 41-09
> E-Mail  : sebastian.huber at embedded-brains.de
> PGP     : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.