[PATCH 3/4] new rtems environment with the implementation of FreeBSD timecounters; modifications of certain tests in the testsuite New test: timecounter02

[Joel Sherrill]

On 4/1/2015 2:02 PM, Gedare Bloom wrote:
> On Wed, Apr 1, 2015 at 2:54 PM, Sebastian Huber
> <sebastian.huber at embedded-brains.de> wrote:
>> ----- Gedare Bloom <gedare at rtems.org> schrieb:
>>> On Wed, Apr 1, 2015 at 11:58 AM, Joel Sherrill
>>> <joel.sherrill at oarcorp.com> wrote:
>>>>
>>>> On 4/1/2015 10:01 AM, Gedare Bloom wrote:
>>>>> I didn't read much of this, but it needs doxygen, and probably part of
>>>>> the previous patch should be merged in here, or some better
>>>>> splitting/recombining of patches so I don't have to review code that
>>>>> gets fixed. It's worth repeating, the "rtems_*time" functions are not
>>>>> following the API conventions of rtems_package_method, it should be
>>>>> rtems_timecounter_*time like rtems_timecounter_bintime.
>>>>>
>>>>> Please use a short git-commit message for the first line, and longer
>>>>> git commit message after a blank line. This will avoid really long
>>>>> lines in git log, and long subjects in git-send-email. I guess this
>>>>> advice is in the Git page
>>>>> (https://devel.rtems.org/wiki/Developer/Git).
>>>> It is already combined so I am not going to complain but I see a method
>>>> renamed
>>>> in here (_TOD_Get). Personally I like a series of small patches and that
>>>> is an
>>>> example of something that could have been done independently.  Remember
>>>> smaller patches are easier for everyone to review. It is often hard to
>>>> think that
>>>> way as you are working and knees deep in it, but it is important.
>>>>
>>>> Add an _ after _Timecount_Get and rtems_get_ in various API methods. But the
>>>> rtems_get methods are (as Gedare pointed out) named incorrectly per the
>>>> RTEMS API naming patterns.
>>>>
>>> The _ after get may not be needed if there is a reason such as wanting
>>> to be close to an upstream interface.
>> The names are derived from the FreeBSD <sys/time.h> kernel space API, e.g. bintime() -> rtems_bintime().  These functions have per se nothing to do with timecounters.  The timecounters are just one way to implement this API.  So maybe the <rtems/timecounter.h> header file is wrongly named, what about <rtems/time.h>?
>>
> Do you prefer to nest it under rtems/sys/time.h? I'd worry about a
> clash some day in the header names otherwise. (I'm also not sure what
> the status of the includes will be after the source code reorg, but
> one problem at a time.)
+1 As long as in the union of all .h files it stays unique.
> Should we consider making the "namespace" somehow point back to the
> bsd/freebsd instead? Rather than come up with a useful namespace for
> every function we mimic from the bsd kernel, something like:
> rtems_bsd_bintime() makes sense to me.
That sounds reasonable. Once you put the _bsd_ in front of it, that puts
it in a "call it what BSD does" box.
>> The driver interface function is _Timecounter_Install().  Should we provide a rtems_timecounter_install()?
> If the function is expected to be called from user code, then yes.
Or provided by user code. The _ should be used by RTEMS internal methods.

-- 
Joel Sherrill, Ph.D.             Director of Research & Development
joel.sherrill at OARcorp.com        On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
Support Available                (256) 722-9985