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

[Gedare Bloom]

On Wed, May 21, 2014 at 9:53 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
> 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.
>
OK, the use of #ifdef..#endif is fine with me also. It can be left
as-is since Daniel prefers the CPP approach at least for this
particular case.

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