[PATCH 2/2] bsps/sparc: Add XtratuM BSP.

[Gedare Bloom]

On Wed, May 21, 2014 at 4:37 AM, Daniel Hellstrom <daniel at gaisler.com> wrote:
> On 05/21/2014 10:01 AM, Sebastian Huber wrote:
>>
>> Hello Gedare,
>>
>> thanks for your review.
>>
>> On 2014-05-20 16:58, Gedare Bloom wrote:
>>>>
>>>> @@ -54,7 +58,15 @@ SYM(_CPU_Context_switch):
>>>> >          ld      [%g6 + SPARC_PER_CPU_ISR_DISPATCH_DISABLE], %o4
>>>> >          ! save it a bit later so we do not waste a couple of cycles
>>>> >
>>>> >+#ifdef RTEMS_PARAVIRT_XTRATUM
>>>> >+        mov     %o0, %l6
>>>> >+        mov     sparc_get_psr_nr, %o0
>>>> >+        __XM_AHC
>>>
>>> I'm guessing these symbols are defined in xm.h?
>>>
>>>> >+        mov     %o0, %o2
>>>> >+        mov     %l6, %o0
>>>> >+#else /* RTEMS_PARAVIRT_XTRATUM */
>>>> >          rd      %psr, %o2
>>>> >+#endif /* RTEMS_PARAVIRT_XTRATUM */
>>>
>>> It would be cleaner code if you can write this get_psr as an assembler
>>> macro, e.g.
>>>
>>> .macro GET_PSR REG, TMP0
>>> #ifdef RTEMS_PARAVIRT_XTRATUM
>>>    mov %o0, \TMP
>>>    mov sparc_get_psr_nr, %o0
>>>    __XM_AHC
>>>    mov %o0, \REG
>>>    mov \TMP, %o0
>>> #else
>>>    rd %psr, \REG
>>> #endif
>>>
>>> Same for some of the other functions that get used multiple times in
>>> this file, such as set_psr, set_pil, and clear_pil. Putting these in a
>>> header file would make them useful in start.S also.
>>>
>>> [...]
>>>
>>
>> I also think that assembler macros could be useful here.
>>
>> Daniel, what is you opinion with respect to the usage of assembler macros
>> in this file?
>
>
> Personally I prefer C preprocessor macros/defines mixed with the ASM code
> instead. If the RTEMS project uses ASM macros I'm fine with it.
>
I noticed there was a macro in that file already, so I guess there is
precedence for using them. Possibly this issue needs to be taken up at
a higher level as a coding convention practice, as to whether to avoid
ASM macros or not. I can see reasons for and against.
-Gedare

> Thanks,
> Daniel
>
>