[LinuxPPS] [RFC] PPS: Implementing LinuxPPS API with new syscalls

[Rodolfo Giometti]

On Fri, Jun 08, 2007 at 12:59:09AM -0600, clemens at dwf.com wrote:
> OK, except for the fact that the previous implementation of the PPSAPI
> used IOCTLs rather than syscalls, these externals are looking more and
> more like the previous implementation.  You REALLY SHOULD look at
> what was done there, so you dont miss some of the subtle points.
> 
> For instance:
> 
>    static int time_pps_create(int source, pps_handle_t *handle)
>    {
>            /* In LinuxPPS there are no differences between a PPS source and
>             * a PPS handle so we return the same value. */
>            *handle = source;
>     
>            return 0;
>    }   
> 
> since the user may be passing source in the variable 'i' which 
> gets reused.  Rather you must malloc handle and place source in
> it so that it will be long lived.
> 
> In that case destroy DOES have something to do, release the space
> (be it only one word).

All the code I looked at defines the PPS handle as a variable and not
as a pointer, in fact in NTP tree we have:

   giometti at zaigor:~/Projects/linuxpps/ntp-4.2.2.p4+dfsg$ rgrep pps_handle_t * 
   ntpd/refclock_atom.c:	pps_handle_t handle;	/* pps handlebars */
   ntpd/refclock_jupiter.c:	pps_handle_t pps_handle;	/* pps handle */ntpd/refclock_mx4200.c:	pps_handle_t	pps_h;
   ntpd/refclock_nmea.c:	pps_handle_t handle;	/* pps handlebars */
   ntpd/refclock_oncore.c:	pps_handle_t pps_h;
   ntpd/refclock_parse.c:        pps_handle_t  ppshandle;        /* store PPSAPI handle */
   ntpd/refclock_ripencc.c:        pps_handle_t handle;            /* pps handlebars */
   Binary file ntpd/refclock_atom.o matches
   util/pps-api.c:	pps_handle_t ph;

in this case the time_pps_create() shouldn't allocate anything but
just fills the handle variable so time_pps_destroy(), in LinuxPPS, has
nothing to do.

> There are other tests that the previous code does that should be done
> here.  And in fact, there is no reason to invent new code just to invent
> it.  Other than changing the ioctl to syscalls (or just USING ioctl's)
> would maintain the current timepps.h, with your additions.

I cannot use ioctl() without defining a new char device and LinuxPPS
does NOT use any char device since a PPS source may be NOT connected
with any char device at all. In embedded systems a PPS source is
generally connected to a CPU's GPIO so the association between a file
descriptor and a PPS handle has no meaning.

> And am I missing something, or with this new structure, shouldnt
> previous code (as in ntp) just 'work' not knowing anything about
> the time_pps_findsource/time_pps_findpath calls?  They would be
> there if a person WANTED to use them, but would no longer be
> necessary.

You are right but the problem is that RFC forces the association
between a file descriptor and a PPS source and this is wrong for the
reason above. That's why LinuxPPS creates its PPS sources with the
kernel symbol pps_register_source().

So if I have a PPS source connected with a CPU's GPIO I have no file
descriptor to supply to time_pps_create(), in this case how I can find
my PPS source without using time_pps_findsource() or
time_pps_findpath()?

To be more generally RFC should defines the time_pps_create() as
follow:

   int time_pps_create(char *path, pps_handle_t *handle);

so users may use it in the following manner:

   char *path = "/dev/ttyS0";
   pps_handle_t handle'
   
   ...

   ret = time_pps_create(path, &handle);

in this case we don't need anymore time_pps_findsource() nor
time_pps_findpath() or we can use them in very rare cases.

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti at enneenne.com
Linux Device Driver                             giometti at gnudd.com
Embedded Systems                     		giometti at linux.it
UNIX programming                     phone:     +39 349 2432127