<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Nov 9, 2018 at 8:18 AM Sebastian Huber <<a href="mailto:sebastian.huber@embedded-brains.de">sebastian.huber@embedded-brains.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 09/11/2018 10:09, Sebastian Huber wrote:<br>
> Hello,<br>
><br>
> the set_vector() seems to be a pretty weird function. What is the <br>
> purpose of it? It is not clear from the implementations. </blockquote><div><br></div><div>It serves a very specific purpose. rtems_interrupt_catch did not address </div><div>the need for BSP specific actions on installation of an interrupt. In those </div><div>days, we followed the RTEID/ORKID specifications very closely and did </div><div>not deviate. </div><div><br></div><div>The specific case we saw multiple times was where boards had a </div><div>peripheral interrupt mask register which had to be modified as part</div><div>of installing the interrupt. set_vector() was added as a BSP/device</div><div>driver interface to allow a uniform way to do that. </div><div><br></div><div>I don't remember the precise argument for also allowing it to install</div><div>raw non-RTEMS interrupt handlers but imagine it derived from the</div><div>RTEID/ORKID and RTOS philosophy of the time to have small API</div><div>sets which often look strange viewed from the current philosophy</div><div>of having many special purpose APIs. Consider how rtems_task_set_priority()</div><div>can be used to both set and get a priority. That reflects the small API</div><div>philosophy and not the current many special purpose APIs philosophy.</div><div><br></div><div>RTEMS ISRs take an argument. Raw ISRs are attached to what we</div><div>now tend to call exceptions.</div><div><br></div><div>Yes. The abusive cast became necessary. I don't remember if it was</div><div>needed in C89 or not. When we started RTEMS C89 compilers didn't</div><div>exist so this all was developed in the days of using the AT&T UNIX K&R</div><div>compiler and targeting bare metal. By 1992, we were using the GNU tools</div><div>but I don't think C89 was supported until later by GCC.</div><div><br></div><div>A clean approach which keeps the functionality, meets the original requirements,</div><div>and doesn't add any overhead on BSPs which tend to be on small memory targets,</div><div>and doesn't do too much code damage would be to split the API into two:</div><div><br></div><div>+ bsp_set_rtems_vector(vector, handler)<br></div><div>+ bsp_set_raw_vector(vector, handler)<br></div><div><br></div><div>This removes the need that the current set_vector implementation has for </div><div>an if and you won't need to take two signatures for interrupt handler methods.</div><div><br></div><div>There is no distinction between CPU vector numbers and RTEMS vector</div><div>numbers on simple vectored architectures.</div><div><br></div><div>This is similar to what I did to rtems_clock_get() to get rid of the enum for </div><div>clock format requested and unsized buffer. I split it into a handful of strongly</div><div>typed methods and mechanically changed all uses. That was a pretty simple</div><div>change to implement.</div><div><br></div><div>Moving from a type overloaded API to multiple strongly typed specific APIs is </div><div>now recognized as a much better practice.</div><div><br></div><div>--joel</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">In particular <br>
> set_vector(..., ..., 0) is only used in five spots:<br>
><br>
> bsps/m68k/csb360/dev/timer.c: set_vector(timerisr, BSP_INTVEC_TMR2, <br>
> 0);<br>
> bsps/m68k/mcf5206elite/dev/timer.c: set_vector(timerisr, <br>
> BSP_INTVEC_TIMER2, 0);<br>
> bsps/m68k/mvme167/btimer/btimer.c: (void) set_vector( timerisr, <br>
> TIMER_VECTOR, 0 );<br>
> bsps/m68k/mvme147/btimer/btimer.c: (void) set_vector(timerisr, <br>
> TIMER_1_VECTOR, 0); /* install ISR */<br>
> bsps/m68k/mvme162/btimer/btimer.c: (void) set_vector( timerisr, VBR0 <br>
> * 0x10 + 0x8, 0 );<br>
><br>
> I think in most cases there is a type mismatch of the handler for type <br>
> 0 and not 0.<br>
><br>
<br>
I get a couple of warnings after the proc_ptr removal:<br>
<br>
bfin/bf537Stamp/../../../../../../bsps/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
bfin/eZKit533/../../../../../../bsps/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
bfin/TLL6527M/../../../../../../bsps/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
lm32/lm32_evr/../../../../../../bsps/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
m68k/av5282/../../../../../../bsps/shared/start/setvec.c:40:43: warning: <br>
passing argument 2 of '_CPU_ISR_install_raw_handler' from incompatible <br>
pointer type [-Wincompatible-pointer-types]<br>
m68k/csb360/../../../../../../bsps/shared/start/setvec.c:40:43: warning: <br>
passing argument 2 of '_CPU_ISR_install_raw_handler' from incompatible <br>
pointer type [-Wincompatible-pointer-types]<br>
m68k/gen68340/../../../../../../bsps/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
m68k/gen68360/../../../../../../bsps/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
m68k/genmcf548x/../../../../../../bsps/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
sh/gensh1/../../../../../../bsps/sh/gensh1/btimer/btimer.c:123:56: <br>
warning: passing argument 3 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
sh/gensh2/../../../../../../bsps/sh/gensh2/btimer/btimer.c:119:56: <br>
warning: passing argument 3 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
sh/gensh4/../../../../../../bsps/sh/gensh4/btimer/btimer.c:145:59: <br>
warning: passing argument 3 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
sparc64/niagara/../../../../../../bsps/sparc64/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
sparc64/usiii/../../../../../../bsps/sparc64/shared/start/setvec.c:40:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
sparc/erc32/../../../../../../bsps/sparc/erc32/start/setvec.c:46:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
sparc/leon2/../../../../../../bsps/sparc/leon2/start/setvec.c:52:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
sparc/leon3/../../../../../../bsps/sparc/leon3/start/setvec.c:51:43: <br>
warning: passing argument 2 of '_CPU_ISR_install_raw_handler' from <br>
incompatible pointer type [-Wincompatible-pointer-types]<br>
<br>
-- <br>
Sebastian Huber, embedded brains GmbH<br>
<br>
Address : Dornierstr. 4, D-82178 Puchheim, Germany<br>
Phone : +49 89 189 47 41-16<br>
Fax : +49 89 189 47 41-09<br>
E-Mail : <a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a><br>
PGP : Public key available on request.<br>
<br>
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.<br>
<br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a></blockquote></div></div>