Re: [PATCH] LinuxPPS - definitive version

David Woodhouse
2007-07-23T13:35:44Z

On Tue, 2007-07-17 at 20:05 +0200, Rodolfo Giometti wrote:
> Hello,
>
> here my last patch for PPS support.
>
> In my opinion it should be ok for inclusion... please, let me know if
> something should be still changed.

s/Documentaion/Documentation/ in the last line of Documentation/pps/pps.txt

Please feed it to scripts/checkpatch.pl -- you can ignore all the
warnings about lines greater than 80 characters, and the complete crap
about "declaring multiple variables together should be avoided", but
some of what it points out is valid. Including the one about 'volatile'
-- your explanation lacked credibility. If you really need 'volatile'
then put it at the places you actually need it; not the declaration of
the structure.

You've also reverted to structures which vary between 32-bit and 64-bit
userspace, because they use 'long' and 'struct timespec', but you
haven't provided the compat_* routines which are then necessary.

+typedef int pps_handle_t; /* represents a PPS source */
+typedef unsigned long pps_seq_t; /* sequence number */
+typedef struct ntp_fp ntp_fp_t; /* NTP-compatible time stamp */
+typedef union pps_timeu pps_timeu_t; /* generic data type to represent time s
tamps */
+typedef struct pps_info pps_info_t;
+typedef struct pps_params pps_params_t;

Don't do this for the structures. It's dubious enough for the integer
types.

--
dwmw2