[Linuxpps] [PATCH] Diff against 2.6.17-rc5

[Rodolfo Giometti]

On Fri, May 26, 2006 at 10:06:31AM +0200, Udo van den Heuvel wrote:
> 
> Please have a look at http://bugs.ntp.isc.org/610, especially at comment
> #20, the attachment, etc.
> Can what is written there be realised? If so, those changes might make
> things easier for users.

I disagree with this sentence:

«If something needs to be done, then linuxpps needs to be fixed. There
 is no benefit in supporting every PPSAPI implementation in the
 world. PPSAPI was defined to REDUCE the system dependencies - not to
 start a race in creating many deviating interface interpretations
 that somewhat fit the PPSAPI spec.»

The _bad thing_ into current PPSAPI and that _should_ be fixed is that
it relays on the fact that _each_ PPS source is connected with a file
descriptor (see time_pps_create() which gets "filedes") and this may
be _not_ true.

In fact I have two systems where GPS antenna is connected with a
serial line but the PPS source is not! In this scenario is not easy to
manage the PPS code without _strongly_ modifying the serial driver.

Again, the current PPSAPI uses ioctl() interface to exchange data with
userland, this implies to _strongly_ modify _each_ char drivers (or
serial ones) who decide to implement it. See patch for 8250.c in both
cases. My patch is very small since all data exchanging code is
encapsulated into another file.

So about RFC 2783 I have only two notes:

1) RFC 2783 says that «pps_handle_t type is an opaque "scalar type"
   used to represent a PPS source within the API» but it whould be
   more generic if it says «pps_handle_t type is an opaque "variable"
   used to represent a PPS source within the API».

   Why the pps_handle_t type must be a scalar if it should be
   «opaque»? :)

2) RFC 2783 does _not_ say that programs should check the pps_handle_t
   variable before calling the time_pps_*() functions, then each
   function should do this job internally.

   This implies that drivers shouldn't do like this:

      if (handle) {
              time_pps_destroy(handle);
              handle = 0;
      }

   but just:

      time_pps_destroy(handle);

As you can see the main problem is just on point 1) where the RFC is
not so «elastic» in defining the pps_handle_t type and where it
_should_ be modified.

About the sentence:

«if you do the wildcard search time_pps_findsource(-1, ...) this is
 potentially wrong»

I agrre. In fact if you have only one PPS source in the system
everything works well but if not you may select a wrong PPS source.

A possible solution may be trying to find a "specified" PPS source by
checking the "name" or "path" arguments returned by the
time_pps_findsource(). As instance:

   for (num = 0; num < 16; num++) {
      ret = time_pps_findsource(num, path, STRING_LEN, id, STRING_LEN);
      if (ret < 0)
         continue;

      if (strncmp(path, "/dev/ttyS0", STRING_LEN) == 0)
         break;
   }
   if (num == 16) {
      fprintf(stderr, "no available PPS source in the system\n");
      exit(1);
   }

at this point you are sure that the "num" PPS source is connected with
serial line "/dev/ttyS0". Maybe this little code can be put into
timepps.h directly...

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: Digital signature
Url : http://ml.enneenne.com/pipermail/linuxpps/attachments/20060526/97ab1b09/attachment.pgp