What is the purpose of set_vector()?

Fri Nov 9 14:41:18 UTC 2018

On Fri, Nov 9, 2018 at 8:18 AM Sebastian Huber <
sebastian.huber at embedded-brains.de> wrote:

> On 09/11/2018 10:09, Sebastian Huber wrote:
> > Hello,
> >
> > the set_vector() seems to be a pretty weird function. What is the
> > purpose of it? It is not clear from the implementations.

It serves a very specific purpose. rtems_interrupt_catch did not address
the need for BSP specific actions on installation of an interrupt. In those
days, we followed the RTEID/ORKID specifications very closely and did
not deviate.

The specific case we saw multiple times was where boards had a
peripheral interrupt mask register which had to be modified as part
of installing the interrupt. set_vector() was added as a BSP/device
driver interface to allow a uniform way to do that.

I don't remember the precise argument for also allowing it to install
raw non-RTEMS interrupt handlers but imagine it derived from the
RTEID/ORKID and RTOS philosophy of the time to have small API
sets which often look strange viewed from the current philosophy
of having many special purpose APIs. Consider how rtems_task_set_priority()
can be used to both set and get a priority. That reflects the small API
philosophy and not the current many special purpose APIs philosophy.

RTEMS ISRs take an argument. Raw ISRs are attached to what we
now tend to call exceptions.

Yes. The abusive cast became necessary. I don't remember if it was
needed in C89 or not. When we started RTEMS C89 compilers didn't
exist so this all was developed in the days of using the AT&T UNIX K&R
compiler and targeting bare metal. By 1992, we were using the GNU tools
but I don't think C89 was supported until later by GCC.

A clean approach which keeps the functionality, meets the original
requirements,
and doesn't add any overhead on BSPs which tend to be on small memory
targets,
and doesn't do too much code damage would be to split the API into two:

+ bsp_set_rtems_vector(vector, handler)
+ bsp_set_raw_vector(vector, handler)

This removes the need that the current set_vector implementation has for
an if and you won't need to take two signatures for interrupt handler
methods.

There is no distinction between CPU vector numbers and RTEMS vector
numbers on simple vectored architectures.

This is similar to what I did to rtems_clock_get() to get rid of the enum
for
clock format requested and unsized buffer. I split it into a handful of
strongly
typed methods and mechanically changed all uses. That was a pretty simple
change to implement.

Moving from a type overloaded API to multiple strongly typed specific APIs
is
now recognized as a much better practice.

--joel

> In particular
> > set_vector(..., ..., 0) is only used in five spots:
> >
> > bsps/m68k/csb360/dev/timer.c:    set_vector(timerisr, BSP_INTVEC_TMR2,
> > 0);
> > bsps/m68k/mcf5206elite/dev/timer.c:    set_vector(timerisr,
> > BSP_INTVEC_TIMER2, 0);
> > bsps/m68k/mvme167/btimer/btimer.c:  (void) set_vector( timerisr,
> > TIMER_VECTOR, 0 );
> > bsps/m68k/mvme147/btimer/btimer.c:  (void) set_vector(timerisr,
> > TIMER_1_VECTOR, 0); /* install ISR */
> > bsps/m68k/mvme162/btimer/btimer.c:  (void) set_vector( timerisr, VBR0
> > * 0x10 + 0x8, 0 );
> >
> > I think in most cases there is a type mismatch of the handler for type
> > 0 and not 0.
> >
>
> I get a couple of warnings after the proc_ptr removal:
>
> bfin/bf537Stamp/../../../../../../bsps/shared/start/setvec.c:40:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> bfin/eZKit533/../../../../../../bsps/shared/start/setvec.c:40:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> bfin/TLL6527M/../../../../../../bsps/shared/start/setvec.c:40:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> lm32/lm32_evr/../../../../../../bsps/shared/start/setvec.c:40:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> m68k/av5282/../../../../../../bsps/shared/start/setvec.c:40:43: warning:
> passing argument 2 of '_CPU_ISR_install_raw_handler' from incompatible
> pointer type [-Wincompatible-pointer-types]
> m68k/csb360/../../../../../../bsps/shared/start/setvec.c:40:43: warning:
> passing argument 2 of '_CPU_ISR_install_raw_handler' from incompatible
> pointer type [-Wincompatible-pointer-types]
> m68k/gen68340/../../../../../../bsps/shared/start/setvec.c:40:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> m68k/gen68360/../../../../../../bsps/shared/start/setvec.c:40:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> m68k/genmcf548x/../../../../../../bsps/shared/start/setvec.c:40:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> sh/gensh1/../../../../../../bsps/sh/gensh1/btimer/btimer.c:123:56:
> warning: passing argument 3 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> sh/gensh2/../../../../../../bsps/sh/gensh2/btimer/btimer.c:119:56:
> warning: passing argument 3 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> sh/gensh4/../../../../../../bsps/sh/gensh4/btimer/btimer.c:145:59:
> warning: passing argument 3 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> sparc64/niagara/../../../../../../bsps/sparc64/shared/start/setvec.c:40:43:
>
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> sparc64/usiii/../../../../../../bsps/sparc64/shared/start/setvec.c:40:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> sparc/erc32/../../../../../../bsps/sparc/erc32/start/setvec.c:46:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> sparc/leon2/../../../../../../bsps/sparc/leon2/start/setvec.c:52:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
> sparc/leon3/../../../../../../bsps/sparc/leon3/start/setvec.c:51:43:
> warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from
> incompatible pointer type [-Wincompatible-pointer-types]
>
> --
> 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.
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20181109/eac6604d/attachment-0002.html>