On 4/1/2015 2:02 PM, Gedare Bloom wrote: > On Wed, Apr 1, 2015 at 2:54 PM, Sebastian Huber > <sebastian.hu...@embedded-brains.de> wrote: >> ----- Gedare Bloom <ged...@rtems.org> schrieb: >>> On Wed, Apr 1, 2015 at 11:58 AM, Joel Sherrill >>> <joel.sherr...@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.sherr...@oarcorp.com On-Line Applications Research Ask me about RTEMS: a free RTOS Huntsville AL 35805 Support Available (256) 722-9985 _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
