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

[Sebastian Huber]

On 2014-05-21 15:44, Gedare Bloom wrote:
> 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.

This file is currently free of assembler macros (the one you probably have in 
mind disappeared with the commit 7c0bd74c87b141454ae17ee1cfeeba42dc4b0df2).

The PowerPC heavily uses assembler macros in the exceptions support.  The C 
pre-processor can be a bit tricky with multi-line statements since it produces 
a one-liner.  I don't have a strong opinion for using assembler macros or not.

-- 
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.