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

Gedare Bloom gedare at rtems.org
Wed Apr 1 19:02:04 UTC 2015


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

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.

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


More information about the devel mailing list