[rtems commit] Using the generic atomic ops to implement UP mode atomic for all architectures . SMP atomic port will be later.

[Gedare Bloom]

I was looking some more at this.  I wonder if we can put the
"genericcpuatomic.h" directly into atomic.h?

Also, the file names could be grouped a little better, like:
atomic.h
atomicgenericops.h

I wonder if we should create a score/cpu/shared directory for these
generic/shared implementations? Then we could move
score/include/rtems/score/atomicgenericops.h to
cpu/shared/rtems/score/cpuatomic.h

-Gedare


On Wed, May 1, 2013 at 11:37 AM, wei.a.yang <wei.a.yang at gmail.com> wrote:
>
>
> 在 2013-5-1，23:19，Ralf Corsepius <ralf.corsepius at rtems.org> 写道：
>
>> On 05/01/2013 05:00 PM, Joel Sherrill wrote:
>>> On 5/1/2013 9:44 AM, Ralf Corsepius wrote:
>>>> On 05/01/2013 04:23 PM, Gedare Bloom wrote:
>>>>> Module:    rtems
>>>>> Branch:    master
>>>>> Commit:    9b605b28b497f398682a8ffd673f6860340957f8
>>>>> Changeset: http://git.rtems.org/rtems/commit/?id=9b605b28b497f398682a8ffd673f6860340957f8
>>>>>
>>>>> Author:    WeiY <wei.a.yang at gmail.com>
>>>>> Date:      Wed Apr 24 00:34:16 2013 +0800
>>>>>
>>>>> Using the generic atomic ops to implement UP mode atomic for all architectures. SMP atomic port will be later.
>>>>> +++ b/cpukit/score/cpu/arm/rtems/score/cpuatomic.h
>>>>> @@ -0,0 +1,40 @@
>>>>> +/**
>>>>> + * @file  rtems/score/cpuatomic.h
>>>>> + *
>>>>> + * This include file implements the atomic operations for arm and defines
>>>>> + * atomic data types which are used by the atomic operations API file. This
>>>>> + * file should use fixed name cpuatomic.h and should be included in atomic
>>>>> + * operations API file atomic.h. If the architecture works at the UP mode it
>>>>> + * will use a generic atomic ops using disable/enable-IRQ simulated. If the
>>>>> + * the architecture works at SMP mode, most of the parts of implementations
>>>>> + * are imported from FreeBSD kernel.
>>>>> + */
>>>>> +
>>>>> +#ifndef _RTEMS_SCORE_ATOMIC_CPU_H
>>>>> +#define _RTEMS_SCORE_ATOMIC_CPU_H
>>>>> +
>>>>> +#include <rtems/score/genericcpuatomic.h>
>>>>> +
>>>>> +#ifdef __cplusplus
>>>>> +extern "C" {
>>>>> +#endif
>>>>> +
>>>>> +/**
>>>>> + * @defgroup RTEMS atomic implementation
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +/**@{*/
>>>>> +
>>>>> +#if !defined(RTEMS_SMP)
>>>>> +#include <rtems/score/genericatomicops.h>
>>>>> +#else
>>>>> +#endif
>>>>> +
>>>>> +#ifdef __cplusplus
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> +/**@}*/
>>>>> +#endif
>>>>> +/*  end of include file */
>>>> Would somebody please explain the code above?
>>>>
>>>> The #ifdef __cplusplus as well as the #if !defined(RTEMS_SMP)
>>>> should be completely superflous. If not, they'd indicate bugs elsewhere.
>>> AFAIK The !defined(RTEMS_SMP) just says use the generic uniprocessor
>>> implementation when not SMP. What's the specific issue?
>> The code includes
>> <rtems/score/genericatomicops.h>
>> twice. Once unconditionally and once conditionally.
>> This is redundant.
>>
> No, the first time it includes genericcpuatomic.h instead of genericatomicops.h. So it is not redundant.
>>
>>>
>>> And I also don't see what the issue is with the C++ check.
>> The c++ guard is entirely superfluous in this case, because the file does not define any symbol.
>>
>>
>>> A quick
>>> grep of score shows 122 files or 333 .h files have the C++ guard.
>>> Do you have a rationale for when one should be present or not?
>> The guards are required when files defines/declares C symbols. Files which don't, don't have to.
>>
>> Finally, files should set c++ guards in ways they are self-contained. The construct above guards an included file, which means this guard is either redundant (in case the other file is self-guarded) or the included file isn't self-guarded (which would mean the include file is poorly implemented == broken).
>>
>> Ralf
>>
>> _______________________________________________
>> rtems-devel mailing list
>> rtems-devel at rtems.org
>> http://www.rtems.org/mailman/listinfo/rtems-devel
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel