LinuxPPS (with new syscalls API)
Subsystems (6)
Rodolfo Giometti
maintainerPATCH 1/1 LinuxPPS (with new syscalls API)
Hello,
here my new version of LinuxPPS with a new syscalls API.
Please take a look at this new patch and report any suggestions/modifications so I can prepare a proper patch for kernel inclusion.
Thanks in advance,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Documentation/pps.txt new file +183 -0
@@ -0,0 +1,183 @@ + + PPS - Pulse Per Second + ---------------------- + +(C) Copyright 2007 Rodolfo Giometti <[email protected]> + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + + + +Overview +-------- + +LinuxPPS provides a programming interface (API) to define into the +system several PPS sources. + +PPS means "pulse per second" and a PPS source is just a device which +provides a high precision signal each second so that an application +can use it to adjust system clock time. + +A PPS source can be connected to a serial port (usually to the Data +Carrier Detect pin) or to a parallel port (ACK-pin) or to a special +CPU's GPIOs (this is the common case in embedded systems) but in each +case when a new pulse comes the system must apply to it a timestamp +and record it for the userland. + +Common use is the combination of the NTPD as userland program with a +GPS receiver as PPS source to obtain a wallclock-time with +sub-millisecond synchronisation to UTC. + + +RFC considerations +------------------ + +While implementing a PPS API as RFC 2783 defines and using an embedded +CPU GPIO-Pin as physical link to the signal, I encountered a deeper +problem: + + At startup it needs a file descriptor as argument for the function + time_pps_create(). + +This implies that the source has a /dev/... entry. This assumption is +ok for the serial and parallel port, where you can do something +useful besides(!) the gathering of timestamps as it is the central +task for a PPS-API. But this assumption does not work for a single +purpose GPIO line. In this case even basic file-related functionality +(like read() and write()) makes no sense at all and should not be a +precondition for the use of a PPS-API. + +The problem can be simply solved if you change the original RFC 2783: + + pps_handle_t type is an opaque __scalar type__ used to represent a + PPS source within the API + +into a modified: + + pps_handle_t type is an opaque __variable__ used to represent a PPS + source within the API and programs should not access it directly to + it due to its opacity. + +This change seems to be neglibile because even the original RFC 2783 +does not encourage programs to check (read: use) the pps_handle_t +variable before calling the time_pps_*() functions, since each +function should do this job internally. + +If I intentionally separate the concept of "file descriptor" from the +concept of the "PPS source" I'm obliged to provide a solution to find +and register a PPS-source without using a file descriptor: it's done +by the functions time_pps_findsource() and time_pps_findpath() now. + +According to this current NTPD drivers' code should be modified as +follows: + ++#ifdef PPS_HAVE_FINDPATH ++ /* Get the PPS source's real name */ ++ fd = readlink(link, path, STRING_LEN-1); ++ if (fd <= 0) ++ strncpy(path, link, STRING_LEN); ++ else ++ path[fd] = '\0'; ++ ++ /* Try to find the source */ ++ fd = time_pps_findpath(path, STRING_LEN, id, STRING_LEN); ++ if (fd < 0) { ++ msyslog(LOG_ERR, "refclock: cannot find PPS source \"%s\" in the system", path); ++ return 1; ++ } ++ msyslog(LOG_INFO, "refclock: found PPS source #%d \"%s\" on \"%s\"", fd, path, id); ++#endif /* PPS_HAVE_FINDPATH */ ++ ++ + if (time_pps_create(fd, &pps_handle) < 0) { +- pps_handle = 0; + msyslog(LOG_ERR, "refclock: time_pps_create failed: %m"); + } + + +Coding example +-------------- + +To register a PPS source into the kernel you should define a struct +pps_source_info_s as follow: + + static struct pps_source_info_s pps_ktimer_info = { + name : "ktimer", + path : "", + mode : PPS_CAPTUREASSERT|PPS_OFFSETASSERT|PPS_ECHOASSERT| \ + PPS_CANWAIT|PPS_TSFMT_TSPEC, + echo : pps_ktimer_echo, + }; + +and then calling the function pps_register_source() in your +intialization routine as follow: + + source = pps_register_source(&pps_ktimer_info, + PPS_CAPTUREASSERT|PPS_OFFSETASSERT, + -1 /* is up to the system */); + +The pps_register_source() prototype is: + + int pps_register_source(struct pps_source_info_s *info, int default_params, int try_id) + +where "info" is a pointer to a structure that describes a particular +PPS source, "default_params" tells the system what the initial default +parameters for the device should be (is obvious that these parameters +must be a subset of ones defined into the struct +pps_source_info_s which describe the capabilities of the driver) +and "try_id" can be used to force a particular ID for your device into +the system (just use -1 if you wish the system chooses one for you). + +Once you have registered a new PPS source into the system you can +signal an assert event (for example in the interrupt handler routine) +just using: + + pps_event(source, PPS_CAPTUREASSERT, ptr); + +The same function may also run the defined echo function +(pps_ktimer_echo(), passing to it the "ptr" pointer) if the user +asked for that... etc.. + +Please see the file drivers/pps/clients/ktimer.c for an example code. + + +SYSFS support +------------- + +The SYSFS support is enabled by default if the SYSFS filesystem is +enabled in the kernel and it provides a new class: + + $ ls /sys/class/pps/ + 00/ 01/ 02/ + +Every directory is the ID of a PPS sources defined into the system and +inside you find several files: + + $ ls /sys/class/pps/00/ + assert clear echo mode name path subsystem@ uevent + +Inside each "assert" and "clear" file you can find the timestamp and a +sequence number: + + $ cat /sys/class/pps/00/assert + 1170026870.983207967#8 + +Where before the "#" is the timestamp in seconds and after it is the +sequence number. Other files are: + +* echo: reports if the PPS source has an echo function or not; + +* mode: reports available PPS functioning modes; + +* name: reports the PPS source's name; + +* path: reports the PPS source's device path, that is the device the + PPS source is connected to (if it exists).
MAINTAINERS +7 -0
@@ -2860,6 +2860,13 @@ P: Michal Ostrowski M: [email protected] S: Maintained +PPS SUPPORT +P: Rodolfo Giometti +M: [email protected] +W: http://wiki.enneenne.com/index.php/LinuxPPS_support +L: [email protected] +S: Maintained + PREEMPTIBLE KERNEL P: Robert Love M: [email protected]
arch/i386/kernel/syscall_table.S +5 -0
@@ -320,3 +320,8 @@ ENTRY(sys_call_table) .long sys_getcpu .long sys_epoll_pwait .long sys_utimensat /* 320 */ + .long sys_time_pps_cmd + .long sys_time_pps_getparams + .long sys_time_pps_setparams + .long sys_time_pps_getcap + .long sys_time_pps_fetch /* 325 */
drivers/Kconfig +2 -0
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig" source "drivers/spi/Kconfig" +source "drivers/pps/Kconfig" + source "drivers/w1/Kconfig" source "drivers/hwmon/Kconfig"
drivers/Makefile +1 -0
@@ -59,6 +59,7 @@ obj-$(CONFIG_INPUT) += input/ obj-$(CONFIG_I2O) += message/ obj-$(CONFIG_RTC_LIB) += rtc/ obj-y += i2c/ +obj-$(CONFIG_PPS) += pps/ obj-$(CONFIG_W1) += w1/ obj-$(CONFIG_HWMON) += hwmon/ obj-$(CONFIG_PHONE) += telephony/
drivers/char/lp.c +58 -0
76 lines not on this page · raw message
drivers/pps/Kconfig new file +34 -0
@@ -0,0 +1,34 @@ +# +# Character device configuration +# + +menu "PPS support" + +config PPS + bool "PPS support" + depends on EXPERIMENTAL + ---help--- + PPS (Pulse Per Second) is a special pulse provided by some GPS + antennas. Userland can use it to get an high time reference. + + Some antennas' PPS signals are connected with the CD (Carrier + Detect) pin of the serial line they use to communicate with the + host. In this case use the SERIAL_LINE client support. + + Some antennas' PPS signals are connected with some special host + inputs so you have to enable the corresponding client support. + + This PPS support can also be built as a module. If so, the module + will be called pps-core. + +config PPS_DEBUG + bool "PPS debugging messages" + depends on PPS + help + Say Y here if you want the PPS support to produce a bunch of debug + messages to the system log. Select this if you are having a + problem with PPS support and want to see more of what is going on. + +source drivers/pps/clients/Kconfig + +endmenu
drivers/pps/Makefile new file +7 -0
@@ -0,0 +1,7 @@ +# +# Makefile for the PPS core. +# + +pps_core-objs += pps.o kapi.o sysfs.o +obj-$(CONFIG_PPS) += pps_core.o +obj-y += clients/
drivers/pps/clients/Kconfig new file +30 -0
@@ -0,0 +1,30 @@ +# +# LinuxPPS clients configuration +# + +if PPS + +comment "PPS clients support" + +config PPS_CLIENT_KTIMER + tristate "Kernel timer client (Testing client, use for debug)" + help + If you say yes here you get support for a PPS debugging client + which uses a kernel timer to generate the PPS signal. + + This driver can also be built as a module. If so, the module + will be called ktimer.o. + +config PPS_CLIENT_UART + bool "UART serial support" + help + If you say yes here you get support for a PPS source connected + with the CD (Carrier Detect) pin of your serial port. + +config PPS_CLIENT_LP + bool "Parallel printer support" + help + If you say yes here you get support for a PPS source connected + with the interrupt pin of your parallel port. + +endif
drivers/pps/clients/Makefile new file +6 -0
@@ -0,0 +1,6 @@ +# +# Makefile for miscellaneous I2C chip drivers. +# + +obj-$(CONFIG_PPS_CLIENT_KTIMER) += ktimer.o +
drivers/pps/clients/ktimer.c new file +113 -0
113 lines not on this page · raw message
drivers/pps/kapi.c new file +202 -0
202 lines not on this page · raw message
drivers/pps/pps.c new file +406 -0
406 lines not on this page · raw message
drivers/pps/sysfs.c new file +219 -0
219 lines not on this page · raw message
drivers/serial/8250.c +2 -0
@@ -2101,6 +2101,8 @@ serial8250_set_termios(struct uart_port *port, struct ktermios *termios, up->ier |=UART_IER_MSI ;if (up->capabilities &UART_CAP_UUE ) up->ier |=UART_IER_UUE |UART_IER_RTOIE ; +if (up->port.flags &UPF_HARDPPS_CD ) + up->ier |=UART_IER_MSI ;/* enable interrupts */ serial_out (up,UART_IER , up->ier);
drivers/serial/serial_core.c +68 -1
103 lines not on this page · raw message
include/asm-i386/unistd.h +6 -1
@@ -326,10 +326,15 @@#define __NR_getcpu 318#define __NR_epoll_pwait 319#define __NR_utimensat 320 +#define __NR_time_pps_cmd 321 +#define __NR_time_pps_getparams 322 +#define __NR_time_pps_setparams 323 +#define __NR_time_pps_getcap 324 +#define __NR_time_pps_fetch 325#ifdef __KERNEL__ -#define NR_syscalls 321 +#define NR_syscalls 326#define __ARCH_WANT_IPC_PARSE_VERSION#define __ARCH_WANT_OLD_READDIR
include/linux/parport.h +12 -0
@@ -100,6 +100,7 @@ typedef enum {#include <linux/proc_fs.h> #include <linux/spinlock.h> #include <linux/wait.h> +#include <linux/pps.h> #include <asm/system.h> #include <asm/ptrace.h> #include <asm/semaphore.h>
@@ -327,6 +328,11 @@ struct parport {struct list_head full_list;struct parport *slaves[3 ]; + +#ifdef CONFIG_PPS_CLIENT_LP +struct pps_source_info_s pps_info; +int pps_source;/* PPS source ID */ +#endif };#define DEFAULT_SPIN_TIME 500/* us */
@@ -517,6 +523,12 @@ extern int parport_daisy_select (struct parport *port, int daisy, int mode);/* Lowlevel drivers _can_ call this support function to handle irqs. */ static __inline__void parport_generic_irq (int irq,struct parport *port) { +#ifdef CONFIG_PPS_CLIENT_LP +pps_event (port->pps_source,PPS_CAPTUREASSERT , port); +pps_dbg ("parport_pc: PPS assert event at %lu on source #%d" , + jiffies, port->pps_source); +#endif +parport_ieee1284_interrupt (irq, port);read_lock (&port->cad_lock);if (port->cad && port->cad->irq_func)
include/linux/pps.h new file +211 -0
211 lines not on this page · raw message
include/linux/serial_core.h +25 -6
@@ -148,6 +148,7 @@#include <linux/sched.h> #include <linux/tty.h> #include <linux/mutex.h> +#include <linux/pps.h> struct uart_port ;struct uart_info ;
@@ -227,6 +228,9 @@ struct uart_port {unsigned char regshift;/* reg offset shift */ unsigned char iotype;/* io access style */ unsigned char unused1; +#ifdef CONFIG_PPS_CLIENT_UART +int pps_source;/* PPS source ID */ +#endif #define UPIO_PORT (0)#define UPIO_HUB6 (1)
@@ -271,7 +275,8 @@ struct uart_port {#define UPF_IOREMAP ((__force upf_t) (1 << 31))#define UPF_CHANGE_MASK ((__force upf_t) (0x17fff)) -#define UPF_USR_MASK ((__force upf_t) (UPF_SPD_MASK|UPF_LOW_LATENCY)) +#define UPF_USR_MASK ((__force upf_t) (UPF_SPD_MASK|UPF_LOW_LATENCY\ + |UPF_HARDPPS_CD))unsigned int mctrl;/* current modem ctrl settings */ unsigned int timeout;/* character-based timeout */
@@ -303,6 +308,10 @@ struct uart_state {struct uart_info *info;struct uart_port *port; +#ifdef CONFIG_PPS_CLIENT_UART +struct pps_source_info_s pps_info; +#endif +struct mutex mutex; };
@@ -467,13 +476,23 @@ uart_handle_dcd_change(struct uart_port *port, unsigned int status) {struct uart_info *info = port->info; - port->icount.dcd++; - -#ifdef CONFIG_HARD_PPS -if ((port->flags &UPF_HARDPPS_CD ) && status) -hardpps (); +#ifdef CONFIG_PPS_CLIENT_UART +if (port->flags &UPF_HARDPPS_CD ) { +if (status) { +pps_event (port->pps_source,PPS_CAPTUREASSERT , port); +pps_dbg ("serial8250: PPS assert event at %lu on source #%d" , + jiffies, port->pps_source); + } +else { +pps_event (port->pps_source,PPS_CAPTURECLEAR , port); +pps_dbg ("serial8250: PPS clear event at %lu on source #%d" , + jiffies, port->pps_source); + } + }#endif + port->icount.dcd++; +if (info->flags &UIF_CHECK_CD ) {if (status)wake_up_interruptible (&info->open_wait);
include/linux/syscalls.h +11 -0
@@ -65,6 +65,7 @@ struct getcpu_cache;#include <asm/signal.h> #include <linux/quota.h> #include <linux/key.h> +#include <linux/pps.h> asmlinkage long sys_time (time_t __user *tloc);asmlinkage long sys_stime (time_t __user *tptr);
@@ -605,6 +606,16 @@ asmlinkage long sys_set_robust_list(struct robust_list_head __user *head, size_t len);asmlinkage long sys_getcpu (unsigned __user *cpu,unsigned __user *node,struct getcpu_cache __user *cache); +asmlinkage long sys_time_pps_cmd (int cmd,void __user *arg); +asmlinkage long sys_time_pps_getparams (int source, +struct pps_params __user *params); +asmlinkage long sys_time_pps_setparams (int source, +const struct pps_params __user *params); +asmlinkage long sys_time_pps_getcap (int source,int __user *mode); +asmlinkage long sys_time_pps_fetch (int source,const int tsformat, +struct pps_info __user *info, +const struct timespec __user *timeout); +int kernel_execve (const char *filename,char *const argv[],char *const envp[]);#endif
kernel/sys_ni.c +7 -0
@@ -141,3 +141,10 @@ cond_syscall(compat_sys_migrate_pages);cond_syscall (sys_bdflush);cond_syscall (sys_ioprio_set);cond_syscall (sys_ioprio_get); + +/* PPS dependent */ +cond_syscall (sys_time_pps_find); +cond_syscall (sys_time_pps_getparams); +cond_syscall (sys_time_pps_setparams); +cond_syscall (sys_time_pps_getcap); +cond_syscall (sys_time_pps_fetch);
Quoted 7 lines · Rodolfo Giometti
Hello,
here my new version of LinuxPPS with a new syscalls API.
Please take a look at this new patch and report any suggestions/modifications so I can prepare a proper patch for kernel inclusion.
Your syscalls blindly dereference userspace pointers instead of using copy_{to,from} user.
Why did you split all your syscalls into two functions?
s/__FUNCTION__/__func__/
s/antennas/antennae/
You seem to have added debugging messages mentioning 'serial8250' into serial_core.h
You added <linux/pps.h> with #ifdef __KERNEL__ in it, but didn't export it to userspace. Why?
Your structures for userspace communication look OK -- I don't think you need special 32/64 compatibility for them. You do need it for the 'struct timespec' in sys_time_pps_fetch() though.
Must we have the ioctl-like interface to sys_time_pps_cmd()? If the second argument is always 'struct pps_source_data_s *', why does the syscall pretend it's 'void *'?
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 3 lines · David Woodhouse
Your syscalls blindly dereference userspace pointers instead of using copy_{to,from} user.
I use access_ok() to test userspace addresses. It should be ok, shouldn't it?
Quoted 3 lines
Why did you split all your syscalls into two functions?
s/__FUNCTION__/__func__/
Just for an easy management of mutex locking.
Quoted 1 line
s/antennas/antennae/
Done. However I found other files in the kernel code with the same error... ;)
Quoted 2 lines
You seem to have added debugging messages mentioning 'serial8250' into
serial_core.h
Yes! Fixed.
Quoted 2 lines
You added <
linux/pps.h> with #ifdef__KERNEL__in it, but didn't export it to userspace. Why?
This file is called by timepps.h who exports the userland data.
Quoted 3 lines
Your structures for userspace communication look OK -- I don't think you need special 32/64 compatibility for them. You do need it for the 'struct timespec' in
sys_time_pps_fetch()though.
Mmm... can you please explain a bit what do you mean? Maybe just a link...
Quoted 1 line
Must we have the ioctl-like interface to
sys_time_pps_cmd()? If the
It seems to me stronger then other solutions...
Quoted 2 lines
second argument is always 'struct
pps_source_data_s*', why does the syscall pretend it's 'void *'?
Just to keep sys_time_pps_cmd() generic for future new commands.
Thanks,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Quoted 7 lines · Rodolfo Giometti
Quoted 3 lines · David Woodhouse
Your syscalls blindly dereference userspace pointers instead of using copy_{to,from} user.
I use
access_ok()to test userspace addresses. It should be ok, shouldn't it?
No; it's racy. You must use copy_from_user() and copy_to_user().
Quoted 5 lines
Quoted 3 lines
Why did you split all your syscalls into two functions?
s/__FUNCTION__/__func__/
Just for an easy management of mutex locking.
That sounds like you're scared of using goto. Don't be :)
Quoted 4 lines
Quoted 1 line
s/antennas/antennae/
Done. However I found other files in the kernel code with the same error... ;)
This is often true of anything which gets pointed out during review. :)
Quoted 9 lines
Quoted 2 lines
You seem to have added debugging messages mentioning 'serial8250' into
serial_core.hYes! Fixed.
Quoted 2 lines
You added <
linux/pps.h> with #ifdef__KERNEL__in it, but didn't export it to userspace. Why?This file is called by
timepps.hwho exports the userland data.
I don't see this timepps.h of which you speak. If it's a _userspace_ file, it cannot include <linux/pps.h> unless you actually add <linux/pps.h> to the list of files which are exported.
Run 'make headers_install' and observe that there is no file usr/include/linux/pps.h -- so there would be no /usr/include/linux/pps.h generated from your kernel tree. You need to add 'unifdef-y += pps.h' to include/linux/Kbuild for that to happen.
Quoted 6 lines
Quoted 3 lines
Your structures for userspace communication look OK -- I don't think you need special 32/64 compatibility for them. You do need it for the 'struct timespec' in
sys_time_pps_fetch()though.Mmm... can you please explain a bit what do you mean? Maybe just a link...
64-bit kernels can run 32-bit userspace programs. But some structures come out _differently_ between 32-bit and 64-bit compilation, so the system call needs a special 'compat' handler instead of just running the normal 64-bit system call.
The 'struct timespec' is one structure which is sometimes different for 32-bit vs. 64-bit, so any system call taking a 'struct timespec' must have a separate compat_sys_xxxx() to handle that. See something like compat_sys_clock_settime() in kernel/compat.c for an example (but don't use set_fs() like it does; just see how it handles the compat_timespec).
Quoted 8 lines
Quoted 1 line
Must we have the ioctl-like interface to
sys_time_pps_cmd()? If theIt seems to me stronger then other solutions...
Quoted 2 lines
second argument is always 'struct
pps_source_data_s*', why does the syscall pretend it's 'void *'?Just to keep
sys_time_pps_cmd()generic for future new commands.
Hm. I'll let other people complain at you about that :)
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 1 line · David Woodhouse
That sounds like you're scared of using goto. Don't be :)
But it's not wrong... should I change it or not?
Quoted 8 lines
I don't see this
timepps.hof which you speak. If it's a _userspace_ file, it cannot include <linux/pps.h> unless you actually add <linux/pps.h> to the list of files which are exported.Run 'make
headers_install' and observe that there is no fileusr/include/linux/pps.h-- so there would be no/usr/include/linux/pps.hgenerated from your kernel tree. You need to add 'unifdef-y +=pps.h' toinclude/linux/Kbuildfor that to happen.
Is this right?
include/linux/Kbuild +1 -0
@@ -290,6 +290,7 @@ unifdef-y += pmu.h unifdef-y += poll.h unifdef-y += ppp_defs.h unifdef-y += ppp-comp.h +unifdef-y += pps.h unifdef-y += ptrace.h unifdef-y += qnx4_fs.h unifdef-y += quota.h
Quoted 10 lines
64-bit kernels can run 32-bit userspace programs. But some structures come out _differently_ between 32-bit and 64-bit compilation, so the system call needs a special 'compat' handler instead of just running the normal 64-bit system call.
The 'struct timespec' is one structure which is sometimes different for 32-bit vs. 64-bit, so any system call taking a 'struct timespec' must have a separate
compat_sys_xxxx()to handle that. See something likecompat_sys_clock_settime()inkernel/compat.cfor an example (but don't useset_fs()like it does; just see how it handles thecompat_timespec).
Ok, I'll take a look at it.
Thanks a lot,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Quoted 5 lines · Rodolfo Giometti
Quoted 1 line · David Woodhouse
That sounds like you're scared of using goto. Don't be :)
But it's not wrong... should I change it or not?
I would suggest changing it. It makes it more obvious to the casual that there are no other callers of the 'internal' functions. It's a matter of taste though; if you feel _very_ strongly about it then feel free to ignore me.
Quoted 2 lines
Is this right?
+unifdef-y +=pps.h
Yes, that looks right. As a matter of course you should also run 'make headers_check' before submitting your new patch, to make sure everything is sane. That'll tell you if your exported pps.h is trying to include other header files which aren't also exported.
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 11 lines · David Woodhouse
64-bit kernels can run 32-bit userspace programs. But some structures come out _differently_ between 32-bit and 64-bit compilation, so the system call needs a special 'compat' handler instead of just running the normal 64-bit system call.
The 'struct timespec' is one structure which is sometimes different for 32-bit vs. 64-bit, so any system call taking a 'struct timespec' must have a separate
compat_sys_xxxx()to handle that. See something likecompat_sys_clock_settime()inkernel/compat.cfor an example (but don't useset_fs()like it does; just see how it handles thecompat_timespec).
Did you mean something like this?
drivers/pps/pps.c +13 -2
@@ -26,6 +26,7 @@#include <linux/init.h> #include <linux/linkage.h> #include <linux/sched.h> +#include <linux/compat.h> #include <linux/pps.h> #include <asm/uaccess.h>
@@ -284,9 +285,15 @@ sys_time_pps_getcap_exit:return ret; } +#ifdef CONFIG_COMPAT asmlinkage long sys_time_pps_fetch (int source,const int tsformat, -struct pps_info __user *info, -const struct timespec __user *timeout) +struct pps_info __user *info, + const struct compat_timespec __user *timeout) +#else +asmlinkage long sys_time_pps_fetch (int source,const int tsformat, +struct pps_info __user *info, + const struct timespec __user *timeout) +#endif {unsigned long ticks;struct pps_info pi;
@@ -318,7 +325,11 @@ asmlinkage long sys_time_pps_fetch(int source, const int ts/* Manage the timeout */ if (timeout) { +#ifdef CONFIG_COMPAT + ret =get_compat_timespec (&to, timeout); +#else ret =copy_from_user (&to, timeout,sizeof (struct timespec )); +#endif if (ret) goto sys_time_pps_fetch_exit;if (to.tv_sec !=-1 ) {
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Quoted 7 lines · Rodolfo Giometti
Quoted 6 lines · David Woodhouse
64-bit kernels can run 32-bit userspace programs. But some structures come out _differently_ between 32-bit and 64-bit compilation, so the system call needs a special 'compat' handler instead of just running the normal 64-bit system call.
Quoted 7 lines
Quoted 5 lines
The 'struct timespec' is one structure which is sometimes different for 32-bit vs. 64-bit, so any system call taking a 'struct timespec' must have a separate
compat_sys_xxxx()to handle that. See something likecompat_sys_clock_settime()inkernel/compat.cfor an example (but don't useset_fs()like it does; just see how it handles thecompat_timespec).Did you mean something like this?
How will 64-bit system calls work if you do it like that? You need to provide _both_ sys_time_pps_fetch() and compat_sys_time_pps_fetch().
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 14 lines · David Woodhouse
Quoted 13 lines · Rodolfo Giometti
Quoted 11 lines · David Woodhouse
64-bit kernels can run 32-bit userspace programs. But some structures come out _differently_ between 32-bit and 64-bit compilation, so the system call needs a special 'compat' handler instead of just running the normal 64-bit system call.
The 'struct timespec' is one structure which is sometimes different for 32-bit vs. 64-bit, so any system call taking a 'struct timespec' must have a separate
compat_sys_xxxx()to handle that. See something likecompat_sys_clock_settime()inkernel/compat.cfor an example (but don't useset_fs()like it does; just see how it handles thecompat_timespec).
Quoted 4 lines
Quoted 1 line
Did you mean something like this?
How will 64-bit system calls work if you do it like that? You need to provide _both_
sys_time_pps_fetch()andcompat_sys_time_pps_fetch().
Sorry, I'm new to this 32/64 bits issues...
Now is it correct?
drivers/pps/pps.c +17 -0
@@ -26,6 +26,7 @@#include <linux/init.h> #include <linux/linkage.h> #include <linux/sched.h> +#include <linux/compat.h> #include <linux/pps.h> #include <asm/uaccess.h>
@@ -362,6 +363,22 @@ sys_time_pps_fetch_exit:return ret; } +#ifdef CONFIG_COMPAT +asmlinkage long compat_sys_time_pps_fetch (int source,const int tsformat, +struct pps_info __user *info, +const struct compat_timespec __user *timeout) +{ +int ret; +struct timespec to; + + ret =get_compat_timespec (&to, timeout); +if (ret) +return -EFAULT ; + +return sys_time_pps_fetch (source, tsformat, info, &to); +} +#endif +/* * Module staff */
Since I have no way to test this code maybe is better add no function at all and simply using a warning message if someone try compiling this code with CONFIG_COMPAT enabled...
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Quoted 15 lines · Rodolfo Giometti
Quoted 14 lines · David Woodhouse
Quoted 13 lines · Rodolfo Giometti
Quoted 11 lines · David Woodhouse
64-bit kernels can run 32-bit userspace programs. But some structures come out _differently_ between 32-bit and 64-bit compilation, so the system call needs a special 'compat' handler instead of just running the normal 64-bit system call.
The 'struct timespec' is one structure which is sometimes different for 32-bit vs. 64-bit, so any system call taking a 'struct timespec' must have a separate
compat_sys_xxxx()to handle that. See something likecompat_sys_clock_settime()inkernel/compat.cfor an example (but don't useset_fs()like it does; just see how it handles thecompat_timespec).
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Did you mean something like this?
How will 64-bit system calls work if you do it like that? You need to provide _both_
sys_time_pps_fetch()andcompat_sys_time_pps_fetch().Sorry, I'm new to this 32/64 bits issues...
Now is it correct?
No, because you're passing a _kernel_ pointer to sys_time_pps_fetch() where it expects a userspace pointer. Use compat_alloc_user_space() to find somewhere to put it in user space, instead. Or change your internal __sys_time_pps_fetch() function to take a number of ticks instead of a pointer to a timespec, then call that directly with appropriate arguments, from both the normal and compat syscall routines.
Quoted 4 lines
Since I have no way to test this code maybe is better add no function at all and simply using a warning message if someone try compiling this code with
CONFIG_COMPATenabled...
No. The fact that you cannot test it is no excuse for submitting something which is _known_ to be broken. Once you have it to the point where a casual observer can't point out errors, then people can test it for you.
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 6 lines · David Woodhouse
No, because you're passing a _kernel_ pointer to
sys_time_pps_fetch()where it expects a userspace pointer. Usecompat_alloc_user_space()to find somewhere to put it in user space, instead. Or change your internal__sys_time_pps_fetch()function to take a number of ticks instead of a pointer to a timespec, then call that directly with appropriate arguments, from both the normal and compat syscall routines.
Ok. Please see the attached patch.
Thanks a lot,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
drivers/pps/pps.c +54 -21
@@ -26,6 +26,7 @@#include <linux/init.h> #include <linux/linkage.h> #include <linux/sched.h> +#include <linux/compat.h> #include <linux/pps.h> #include <asm/uaccess.h>
@@ -284,13 +285,12 @@ sys_time_pps_getcap_exit:return ret; } -asmlinkage long sys_time_pps_fetch (int source,const int tsformat, +static long __sys_time_pps_fetch (int source,const int tsformat,struct pps_info __user *info, -const struct timespec __user *timeout) +const struct timespec *timeout) {unsigned long ticks;struct pps_info pi; -struct timespec to;int ret;pps_dbg ("%s: source %d" , __FUNCTION__, source);
@@ -317,24 +317,19 @@ asmlinkage long sys_time_pps_fetch(int source, const int tsformat, pps_source[source].go =0 ;/* Manage the timeout */ -if (timeout) { - ret =copy_from_user (&to, timeout,sizeof (struct timespec )); -if (ret) - goto sys_time_pps_fetch_exit; -if (to.tv_sec !=-1 ) { -pps_dbg ("timeout %ld.%09ld" , to.tv_sec, to.tv_nsec); - ticks = to.tv_sec *HZ ; - ticks += to.tv_nsec / (NSEC_PER_SEC /HZ ); - -if (ticks !=0 ) { - ret =wait_event_interruptible_timeout ( - pps_source[source].queue, - pps_source[source].go, ticks); -if (ret ==0 ) { -pps_dbg ("timeout expired" ); - ret = -ETIMEDOUT ; - goto sys_time_pps_fetch_exit; - } +if (timeout->tv_sec !=-1 ) { +pps_dbg ("timeout %ld.%09ld" , timeout->tv_sec, timeout->tv_nsec); + ticks = timeout->tv_sec *HZ ; + ticks += timeout->tv_nsec / (NSEC_PER_SEC /HZ ); + +if (ticks !=0 ) { + ret =wait_event_interruptible_timeout ( + pps_source[source].queue, + pps_source[source].go, ticks); +if (ret ==0 ) { +pps_dbg ("timeout expired" ); + ret = -ETIMEDOUT ; + goto sys_time_pps_fetch_exit; } } }else
@@ -362,6 +357,44 @@ sys_time_pps_fetch_exit:return ret; } +asmlinkage long sys_time_pps_fetch (int source,const int tsformat, +struct pps_info __user *info, +const struct timespec __user *timeout) +{ +int ret; +struct timespec to; + +if (timeout) { + ret =copy_from_user (&to, timeout,sizeof (struct timespec )); +if (ret) +return ret; + } +else + to.tv_sec =-1 ; + +return __sys_time_pps_fetch (source, tsformat, info, timeout); +} + +#ifdef CONFIG_COMPAT +asmlinkage long compat_sys_time_pps_fetch (int source,const int tsformat, +struct pps_info __user *info, +const struct compat_timespec __user *timeout) +{ +int ret; +struct timespec to; + +if (timeout) { + ret =get_compat_timespec (&to, timeout); +if (ret) +return -EFAULT ; + } +else + to.tv_sec =-1 ; + +return __sys_time_pps_fetch (source, tsformat, info, &to); +} +#endif +/* * Module staff */
Quoted 1 line · Rodolfo Giometti
Ok. Please see the attached patch.
Looks better. All I can find to complain about is the fact that you return whatever copy_from_user() returns. Don't -- that's the number of bytes left to copy. It should be if (copy_from_user(..)) return -EFAULT;
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 4 lines · David Woodhouse
Looks better. All I can find to complain about is the fact that you return whatever
copy_from_user()returns. Don't -- that's the number of bytes left to copy. It should be if (copy_from_user(..)) return-EFAULT;
Ok, I'll fix it.
Just last question: I still don't well understand where I should declare the new compat_sys_time_pps_fetch() syscall... it's automagically defined by the system when CONFIG_COMPAT is enabled? :-o
Thanks for your help,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Quoted 3 lines · Rodolfo Giometti
Just last question: I still don't well understand where I should declare the new
compat_sys_time_pps_fetch()syscall... it's automagically defined by the system whenCONFIG_COMPATis enabled? :-o
It isn't used on i386. On a 64-bit architecture, you need to put compat_sys_time_pps_fetch() into the syscall table for 32-bit processes.
On PowerPC you do this by using COMPAT_SYS_SPU(time_pps_fetch) instead of SYSCALL_SPU(..) in include/asm-powerpc/systbl.h. On x86_64 you'd put it into the ia32_sys_call_table in arch/x86_64/ia32/ia32entry.S
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 5 lines · David Woodhouse
Quoted 3 lines · Rodolfo Giometti
Just last question: I still don't well understand where I should declare the new
compat_sys_time_pps_fetch()syscall... it's automagically defined by the system whenCONFIG_COMPATis enabled? :-o
Quoted 7 lines
It isn't used on i386. On a 64-bit architecture, you need to put
compat_sys_time_pps_fetch()into the syscall table for 32-bit processes.On PowerPC you do this by using
COMPAT_SYS_SPU(time_pps_fetch)instead ofSYSCALL_SPU(..)ininclude/asm-powerpc/systbl.h. Onx86_64you'd put it into theia32_sys_call_tableinarch/x86_64/ia32/ia32entry.S
I see.
Do you think I should add these functions into my patch, even if I cannot test it, or it's enought providing just the compat_sys_time_pps_fetch() function?
Thanks,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Quoted 3 lines · Rodolfo Giometti
Do you think I should add these functions into my patch, even if I cannot test it, or it's enought providing just the
compat_sys_time_pps_fetch()function?
Probably best to put them in. That way, you make it easier for people to test. Ideally, you should also provide a simple program that other people can use to test it, preferably without needing hardware (or at least just a nullmodem cable and another test program at the other end of it).
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 6 lines · David Woodhouse
Quoted 3 lines · Rodolfo Giometti
Do you think I should add these functions into my patch, even if I cannot test it, or it's enought providing just the
compat_sys_time_pps_fetch()function?Probably best to put them in. That way, you make it easier for people to
Mmm... so I should provide new syscalls for _all_
architectures... gulp! :)
It could be acceptable, just for the first release, to provide the support for x86 only?
In this manner we can have a first release of LinuxPPS in the main line just for x86 and then me, or other people, may add support for the several supported architectures.
Quoted 4 lines
test. Ideally, you should also provide a simple program that other people can use to test it, preferably without needing hardware (or at least just a nullmodem cable and another test program at the other end of it).
I already have a basic testing program... I can add it into Documentation/pps directory, can't I?
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Quoted 10 lines · Rodolfo Giometti
Quoted 6 lines · David Woodhouse
Quoted 3 lines · Rodolfo Giometti
Do you think I should add these functions into my patch, even if I cannot test it, or it's enought providing just the
compat_sys_time_pps_fetch()function?Probably best to put them in. That way, you make it easier for people to
Mmm... so I should provide new syscalls for _all_
architectures... gulp! :)
It's nice if you can do so, but I wouldn't suggest that you _have_ to. I have to admit that I rarely bother actually wiring new system calls up on anything but PowerPC to start with.
The important thing is that you've _considered_ the other architectures, and the 32/64 compatibility implications. As long as the API of your new system call is sensible and takes that kind of thing into account, it should be fine.
Had you considered changing the API so that you don't need the compatibility wrapper at all? Could you take an integer number of µS or ms instead of a struct timespec?
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerQuoted 9 lines · David Woodhouse
It's nice if you can do so, but I wouldn't suggest that you _have_ to. I have to admit that I rarely bother actually wiring new system calls up on anything but PowerPC to start with.
The important thing is that you've _considered_ the other architectures, and the 32/64 compatibility implications. As long as the API of your new system call is sensible and takes that kind of thing into account, it should be fine.
Ok. :)
Quoted 3 lines
Had you considered changing the API so that you don't need the compatibility wrapper at all? Could you take an integer number of µS or ms instead of a struct timespec?
Not before now, but I followed the API specified into RFC 2783 who specifies struct timespec...
Thanks for your suggestions! I'll send a new patch ASAP!
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Rodolfo Giometti
maintainerPATCH 1/1 LinuxPPS (with new syscalls API) - new version
In reply to David WoodhouseHello,
here my new LinuxPPS patch.
What to do now for kernel inclusion? Should I provide several patches? If so how should I divide them?
Thanks a lot,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Documentation/pps/Makefile new file +27 -0
@@ -0,0 +1,27 @@ +TARGETS = ppstest ppsctl + +CFLAGS += -Wall -O2 -D_GNU_SOURCE +CFLAGS += -I . +CFLAGS += -ggdb + +# -- Actions section ---------------------------------------------------------- + +.PHONY : all depend dep + +all : .depend $(TARGETS) + +.depend depend dep : + $(CC) $(CFLAGS) -M $(TARGETS:=.c) > .depend + +ifeq (.depend,$(wildcard .depend)) +include .depend +endif + + +# -- Clean section ------------------------------------------------------------ + +.PHONY : clean + +clean : + rm -f *.o *~ core .depend + rm -f ${TARGETS}
Documentation/pps/pps.txt new file +211 -0
212 lines not on this page · raw message
Documentation/pps/ppsctl.c new file +61 -0
61 lines not on this page · raw message
Documentation/pps/ppstest.c new file +199 -0
199 lines not on this page · raw message
Documentation/pps/timepps.h new file +161 -0
161 lines not on this page · raw message
MAINTAINERS +7 -0
@@ -2860,6 +2860,13 @@ P: Michal Ostrowski M: [email protected] S: Maintained +PPS SUPPORT +P: Rodolfo Giometti +M: [email protected] +W: http://wiki.enneenne.com/index.php/LinuxPPS_support +L: [email protected] +S: Maintained + PREEMPTIBLE KERNEL P: Robert Love M: [email protected]
arch/i386/kernel/syscall_table.S +5 -0
@@ -320,3 +320,8 @@ ENTRY(sys_call_table) .long sys_getcpu .long sys_epoll_pwait .long sys_utimensat /* 320 */ + .long sys_time_pps_cmd + .long sys_time_pps_getparams + .long sys_time_pps_setparams + .long sys_time_pps_getcap + .long sys_time_pps_fetch /* 325 */
drivers/Kconfig +2 -0
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig" source "drivers/spi/Kconfig" +source "drivers/pps/Kconfig" + source "drivers/w1/Kconfig" source "drivers/hwmon/Kconfig"
drivers/Makefile +1 -0
@@ -59,6 +59,7 @@ obj-$(CONFIG_INPUT) += input/ obj-$(CONFIG_I2O) += message/ obj-$(CONFIG_RTC_LIB) += rtc/ obj-y += i2c/ +obj-$(CONFIG_PPS) += pps/ obj-$(CONFIG_W1) += w1/ obj-$(CONFIG_HWMON) += hwmon/ obj-$(CONFIG_PHONE) += telephony/
drivers/char/lp.c +58 -0
76 lines not on this page · raw message
drivers/pps/Kconfig new file +34 -0
@@ -0,0 +1,34 @@ +# +# Character device configuration +# + +menu "PPS support" + +config PPS + bool "PPS support" + depends on EXPERIMENTAL + ---help--- + PPS (Pulse Per Second) is a special pulse provided by some GPS + antennae. Userland can use it to get an high time reference. + + Some antennae's PPS signals are connected with the CD (Carrier + Detect) pin of the serial line they use to communicate with the + host. In this case use the SERIAL_LINE client support. + + Some antennae's PPS signals are connected with some special host + inputs so you have to enable the corresponding client support. + + This PPS support can also be built as a module. If so, the module + will be called pps-core. + +config PPS_DEBUG + bool "PPS debugging messages" + depends on PPS + help + Say Y here if you want the PPS support to produce a bunch of debug + messages to the system log. Select this if you are having a + problem with PPS support and want to see more of what is going on. + +source drivers/pps/clients/Kconfig + +endmenu
drivers/pps/Makefile new file +7 -0
@@ -0,0 +1,7 @@ +# +# Makefile for the PPS core. +# + +pps_core-objs += pps.o kapi.o sysfs.o +obj-$(CONFIG_PPS) += pps_core.o +obj-y += clients/
drivers/pps/clients/Kconfig new file +30 -0
@@ -0,0 +1,30 @@ +# +# LinuxPPS clients configuration +# + +if PPS + +comment "PPS clients support" + +config PPS_CLIENT_KTIMER + tristate "Kernel timer client (Testing client, use for debug)" + help + If you say yes here you get support for a PPS debugging client + which uses a kernel timer to generate the PPS signal. + + This driver can also be built as a module. If so, the module + will be called ktimer.o. + +config PPS_CLIENT_UART + bool "UART serial support" + help + If you say yes here you get support for a PPS source connected + with the CD (Carrier Detect) pin of your serial port. + +config PPS_CLIENT_LP + bool "Parallel printer support" + help + If you say yes here you get support for a PPS source connected + with the interrupt pin of your parallel port. + +endif
drivers/pps/clients/Makefile new file +6 -0
@@ -0,0 +1,6 @@ +# +# Makefile for miscellaneous I2C chip drivers. +# + +obj-$(CONFIG_PPS_CLIENT_KTIMER) += ktimer.o +
drivers/pps/clients/ktimer.c new file +113 -0
113 lines not on this page · raw message
drivers/pps/kapi.c new file +202 -0
202 lines not on this page · raw message
drivers/pps/pps.c new file +451 -0
451 lines not on this page · raw message
drivers/pps/sysfs.c new file +219 -0
219 lines not on this page · raw message
drivers/serial/8250.c +2 -0
@@ -2101,6 +2101,8 @@ serial8250_set_termios(struct uart_port *port, struct ktermios *termios, up->ier |=UART_IER_MSI ;if (up->capabilities &UART_CAP_UUE ) up->ier |=UART_IER_UUE |UART_IER_RTOIE ; +if (up->port.flags &UPF_HARDPPS_CD ) + up->ier |=UART_IER_MSI ;/* enable interrupts */ serial_out (up,UART_IER , up->ier);
drivers/serial/serial_core.c +68 -1
103 lines not on this page · raw message
include/asm-i386/unistd.h +6 -1
@@ -326,10 +326,15 @@#define __NR_getcpu 318#define __NR_epoll_pwait 319#define __NR_utimensat 320 +#define __NR_time_pps_cmd 321 +#define __NR_time_pps_getparams 322 +#define __NR_time_pps_setparams 323 +#define __NR_time_pps_getcap 324 +#define __NR_time_pps_fetch 325#ifdef __KERNEL__ -#define NR_syscalls 321 +#define NR_syscalls 326#define __ARCH_WANT_IPC_PARSE_VERSION#define __ARCH_WANT_OLD_READDIR
include/linux/Kbuild +1 -0
@@ -290,6 +290,7 @@ unifdef-y += pmu.h unifdef-y += poll.h unifdef-y += ppp_defs.h unifdef-y += ppp-comp.h +unifdef-y += pps.h unifdef-y += ptrace.h unifdef-y += qnx4_fs.h unifdef-y += quota.h
include/linux/parport.h +12 -0
@@ -100,6 +100,7 @@ typedef enum {#include <linux/proc_fs.h> #include <linux/spinlock.h> #include <linux/wait.h> +#include <linux/pps.h> #include <asm/system.h> #include <asm/ptrace.h> #include <asm/semaphore.h>
@@ -327,6 +328,11 @@ struct parport {struct list_head full_list;struct parport *slaves[3 ]; + +#ifdef CONFIG_PPS_CLIENT_LP +struct pps_source_info_s pps_info; +int pps_source;/* PPS source ID */ +#endif };#define DEFAULT_SPIN_TIME 500/* us */
@@ -517,6 +523,12 @@ extern int parport_daisy_select (struct parport *port, int daisy, int mode);/* Lowlevel drivers _can_ call this support function to handle irqs. */ static __inline__void parport_generic_irq (int irq,struct parport *port) { +#ifdef CONFIG_PPS_CLIENT_LP +pps_event (port->pps_source,PPS_CAPTUREASSERT , port); +pps_dbg ("parport_pc: PPS assert event at %lu on source #%d" , + jiffies, port->pps_source); +#endif +parport_ieee1284_interrupt (irq, port);read_lock (&port->cad_lock);if (port->cad && port->cad->irq_func)
include/linux/pps.h new file +211 -0
211 lines not on this page · raw message
include/linux/serial_core.h +28 -5
@@ -148,6 +148,7 @@#include <linux/sched.h> #include <linux/tty.h> #include <linux/mutex.h> +#include <linux/pps.h> struct uart_port ;struct uart_info ;
@@ -227,6 +228,9 @@ struct uart_port {unsigned char regshift;/* reg offset shift */ unsigned char iotype;/* io access style */ unsigned char unused1; +#ifdef CONFIG_PPS_CLIENT_UART +int pps_source;/* PPS source ID */ +#endif #define UPIO_PORT (0)#define UPIO_HUB6 (1)
@@ -271,7 +275,8 @@ struct uart_port {#define UPF_IOREMAP ((__force upf_t) (1 << 31))#define UPF_CHANGE_MASK ((__force upf_t) (0x17fff)) -#define UPF_USR_MASK ((__force upf_t) (UPF_SPD_MASK|UPF_LOW_LATENCY)) +#define UPF_USR_MASK ((__force upf_t) (UPF_SPD_MASK|UPF_LOW_LATENCY\ + |UPF_HARDPPS_CD))unsigned int mctrl;/* current modem ctrl settings */ unsigned int timeout;/* character-based timeout */
@@ -303,6 +308,10 @@ struct uart_state {struct uart_info *info;struct uart_port *port; +#ifdef CONFIG_PPS_CLIENT_UART +struct pps_source_info_s pps_info; +#endif +struct mutex mutex; };
@@ -467,13 +476,27 @@ uart_handle_dcd_change(struct uart_port *port, unsigned int status) {struct uart_info *info = port->info; - port->icount.dcd++; +#ifdef CONFIG_PPS_CLIENT_UART +struct tty_driver *drv = port->info->tty->driver; -#ifdef CONFIG_HARD_PPS -if ((port->flags &UPF_HARDPPS_CD ) && status) -hardpps (); +if (port->flags &UPF_HARDPPS_CD ) { +if (status) { +pps_event (port->pps_source,PPS_CAPTUREASSERT , port); +pps_dbg ("%s%d: PPS assert event at %lu on source #%d" , + drv->driver_name, port->line, + jiffies, port->pps_source); + } +else { +pps_event (port->pps_source,PPS_CAPTURECLEAR , port); +pps_dbg ("%s%d: PPS clear event at %lu on source #%d" , + drv->driver_name, port->line, + jiffies, port->pps_source); + } + }#endif + port->icount.dcd++; +if (info->flags &UIF_CHECK_CD ) {if (status)wake_up_interruptible (&info->open_wait);
include/linux/syscalls.h +11 -0
@@ -65,6 +65,7 @@ struct getcpu_cache;#include <asm/signal.h> #include <linux/quota.h> #include <linux/key.h> +#include <linux/pps.h> asmlinkage long sys_time (time_t __user *tloc);asmlinkage long sys_stime (time_t __user *tptr);
@@ -605,6 +606,16 @@ asmlinkage long sys_set_robust_list(struct robust_list_head __user *head, size_t len);asmlinkage long sys_getcpu (unsigned __user *cpu,unsigned __user *node,struct getcpu_cache __user *cache); +asmlinkage long sys_time_pps_cmd (int cmd,void __user *arg); +asmlinkage long sys_time_pps_getparams (int source, +struct pps_params __user *params); +asmlinkage long sys_time_pps_setparams (int source, +const struct pps_params __user *params); +asmlinkage long sys_time_pps_getcap (int source,int __user *mode); +asmlinkage long sys_time_pps_fetch (int source,const int tsformat, +struct pps_info __user *info, +const struct timespec __user *timeout); +int kernel_execve (const char *filename,char *const argv[],char *const envp[]);#endif
kernel/sys_ni.c +7 -0
@@ -141,3 +141,10 @@ cond_syscall(compat_sys_migrate_pages);cond_syscall (sys_bdflush);cond_syscall (sys_ioprio_set);cond_syscall (sys_ioprio_get); + +/* PPS dependent */ +cond_syscall (sys_time_pps_find); +cond_syscall (sys_time_pps_getparams); +cond_syscall (sys_time_pps_setparams); +cond_syscall (sys_time_pps_getcap); +cond_syscall (sys_time_pps_fetch);
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 4 lines · Rodolfo Giometti
here my new LinuxPPS patch.
What to do now for kernel inclusion? Should I provide several patches? If so how should I divide them?
It doesn't apply to the current git tree, which has already had some new system calls added.
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 3 lines · David Woodhouse
It doesn't apply to the current git tree, which has already had some new system calls added.
Ok, here the patch against latest git commit.
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Documentation/pps/Makefile new file +27 -0
@@ -0,0 +1,27 @@ +TARGETS = ppstest ppsctl + +CFLAGS += -Wall -O2 -D_GNU_SOURCE +CFLAGS += -I . +CFLAGS += -ggdb + +# -- Actions section ---------------------------------------------------------- + +.PHONY : all depend dep + +all : .depend $(TARGETS) + +.depend depend dep : + $(CC) $(CFLAGS) -M $(TARGETS:=.c) > .depend + +ifeq (.depend,$(wildcard .depend)) +include .depend +endif + + +# -- Clean section ------------------------------------------------------------ + +.PHONY : clean + +clean : + rm -f *.o *~ core .depend + rm -f ${TARGETS}
Documentation/pps/pps.txt new file +211 -0
212 lines not on this page · raw message
Documentation/pps/ppsctl.c new file +61 -0
61 lines not on this page · raw message
Documentation/pps/ppstest.c new file +199 -0
199 lines not on this page · raw message
Documentation/pps/timepps.h new file +161 -0
161 lines not on this page · raw message
MAINTAINERS +7 -0
@@ -2903,6 +2903,13 @@ P: Michal Ostrowski M: [email protected] S: Maintained +PPS SUPPORT +P: Rodolfo Giometti +M: [email protected] +W: http://wiki.enneenne.com/index.php/LinuxPPS_support +L: [email protected] +S: Maintained + PREEMPTIBLE KERNEL P: Robert Love M: [email protected]
arch/i386/kernel/syscall_table.S +5 -0
@@ -323,3 +323,8 @@ ENTRY(sys_call_table) .long sys_signalfd .long sys_timerfd .long sys_eventfd + .long sys_time_pps_cmd + .long sys_time_pps_getparams /* 325 */ + .long sys_time_pps_setparams + .long sys_time_pps_getcap + .long sys_time_pps_fetch
drivers/Kconfig +2 -0
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig" source "drivers/spi/Kconfig" +source "drivers/pps/Kconfig" + source "drivers/w1/Kconfig" source "drivers/hwmon/Kconfig"
drivers/Makefile +1 -0
@@ -60,6 +60,7 @@ obj-$(CONFIG_INPUT) += input/ obj-$(CONFIG_I2O) += message/ obj-$(CONFIG_RTC_LIB) += rtc/ obj-y += i2c/ +obj-$(CONFIG_PPS) += pps/ obj-$(CONFIG_W1) += w1/ obj-$(CONFIG_HWMON) += hwmon/ obj-$(CONFIG_PHONE) += telephony/
drivers/char/lp.c +58 -0
76 lines not on this page · raw message
drivers/pps/Kconfig new file +34 -0
@@ -0,0 +1,34 @@ +# +# Character device configuration +# + +menu "PPS support" + +config PPS + bool "PPS support" + depends on EXPERIMENTAL + ---help--- + PPS (Pulse Per Second) is a special pulse provided by some GPS + antennae. Userland can use it to get an high time reference. + + Some antennae's PPS signals are connected with the CD (Carrier + Detect) pin of the serial line they use to communicate with the + host. In this case use the SERIAL_LINE client support. + + Some antennae's PPS signals are connected with some special host + inputs so you have to enable the corresponding client support. + + This PPS support can also be built as a module. If so, the module + will be called pps-core. + +config PPS_DEBUG + bool "PPS debugging messages" + depends on PPS + help + Say Y here if you want the PPS support to produce a bunch of debug + messages to the system log. Select this if you are having a + problem with PPS support and want to see more of what is going on. + +source drivers/pps/clients/Kconfig + +endmenu
drivers/pps/Makefile new file +7 -0
@@ -0,0 +1,7 @@ +# +# Makefile for the PPS core. +# + +pps_core-objs += pps.o kapi.o sysfs.o +obj-$(CONFIG_PPS) += pps_core.o +obj-y += clients/
drivers/pps/clients/Kconfig new file +30 -0
@@ -0,0 +1,30 @@ +# +# LinuxPPS clients configuration +# + +if PPS + +comment "PPS clients support" + +config PPS_CLIENT_KTIMER + tristate "Kernel timer client (Testing client, use for debug)" + help + If you say yes here you get support for a PPS debugging client + which uses a kernel timer to generate the PPS signal. + + This driver can also be built as a module. If so, the module + will be called ktimer.o. + +config PPS_CLIENT_UART + bool "UART serial support" + help + If you say yes here you get support for a PPS source connected + with the CD (Carrier Detect) pin of your serial port. + +config PPS_CLIENT_LP + bool "Parallel printer support" + help + If you say yes here you get support for a PPS source connected + with the interrupt pin of your parallel port. + +endif
drivers/pps/clients/Makefile new file +6 -0
@@ -0,0 +1,6 @@ +# +# Makefile for miscellaneous I2C chip drivers. +# + +obj-$(CONFIG_PPS_CLIENT_KTIMER) += ktimer.o +
drivers/pps/clients/ktimer.c new file +113 -0
113 lines not on this page · raw message
drivers/pps/kapi.c new file +202 -0
202 lines not on this page · raw message
drivers/pps/pps.c new file +451 -0
451 lines not on this page · raw message
drivers/pps/sysfs.c new file +219 -0
219 lines not on this page · raw message
drivers/serial/8250.c +2 -0
@@ -2101,6 +2101,8 @@ serial8250_set_termios(struct uart_port *port, struct ktermios *termios, up->ier |=UART_IER_MSI ;if (up->capabilities &UART_CAP_UUE ) up->ier |=UART_IER_UUE |UART_IER_RTOIE ; +if (up->port.flags &UPF_HARDPPS_CD ) + up->ier |=UART_IER_MSI ;/* enable interrupts */ serial_out (up,UART_IER , up->ier);
drivers/serial/serial_core.c +68 -1
103 lines not on this page · raw message
include/asm-i386/unistd.h +6 -1
@@ -329,10 +329,15 @@#define __NR_signalfd 321#define __NR_timerfd 322#define __NR_eventfd 323 +#define __NR_time_pps_cmd 324 +#define __NR_time_pps_getparams 325 +#define __NR_time_pps_setparams 326 +#define __NR_time_pps_getcap 327 +#define __NR_time_pps_fetch 328#ifdef __KERNEL__ -#define NR_syscalls 324 +#define NR_syscalls 329#define __ARCH_WANT_IPC_PARSE_VERSION#define __ARCH_WANT_OLD_READDIR
include/linux/Kbuild +1 -0
@@ -293,6 +293,7 @@ unifdef-y += pmu.h unifdef-y += poll.h unifdef-y += ppp_defs.h unifdef-y += ppp-comp.h +unifdef-y += pps.h unifdef-y += ptrace.h unifdef-y += qnx4_fs.h unifdef-y += quota.h
include/linux/parport.h +12 -0
@@ -100,6 +100,7 @@ typedef enum {#include <linux/proc_fs.h> #include <linux/spinlock.h> #include <linux/wait.h> +#include <linux/pps.h> #include <asm/system.h> #include <asm/ptrace.h> #include <asm/semaphore.h>
@@ -327,6 +328,11 @@ struct parport {struct list_head full_list;struct parport *slaves[3 ]; + +#ifdef CONFIG_PPS_CLIENT_LP +struct pps_source_info_s pps_info; +int pps_source;/* PPS source ID */ +#endif };#define DEFAULT_SPIN_TIME 500/* us */
@@ -517,6 +523,12 @@ extern int parport_daisy_select (struct parport *port, int daisy, int mode);/* Lowlevel drivers _can_ call this support function to handle irqs. */ static __inline__void parport_generic_irq (int irq,struct parport *port) { +#ifdef CONFIG_PPS_CLIENT_LP +pps_event (port->pps_source,PPS_CAPTUREASSERT , port); +pps_dbg ("parport_pc: PPS assert event at %lu on source #%d" , + jiffies, port->pps_source); +#endif +parport_ieee1284_interrupt (irq, port);read_lock (&port->cad_lock);if (port->cad && port->cad->irq_func)
include/linux/pps.h new file +211 -0
211 lines not on this page · raw message
include/linux/serial_core.h +28 -5
@@ -153,6 +153,7 @@#include <linux/tty.h> #include <linux/mutex.h> #include <linux/sysrq.h> +#include <linux/pps.h> struct uart_port ;struct uart_info ;
@@ -232,6 +233,9 @@ struct uart_port {unsigned char regshift;/* reg offset shift */ unsigned char iotype;/* io access style */ unsigned char unused1; +#ifdef CONFIG_PPS_CLIENT_UART +int pps_source;/* PPS source ID */ +#endif #define UPIO_PORT (0)#define UPIO_HUB6 (1)
@@ -276,7 +280,8 @@ struct uart_port {#define UPF_IOREMAP ((__force upf_t) (1 << 31))#define UPF_CHANGE_MASK ((__force upf_t) (0x17fff)) -#define UPF_USR_MASK ((__force upf_t) (UPF_SPD_MASK|UPF_LOW_LATENCY)) +#define UPF_USR_MASK ((__force upf_t) (UPF_SPD_MASK|UPF_LOW_LATENCY\ + |UPF_HARDPPS_CD))unsigned int mctrl;/* current modem ctrl settings */ unsigned int timeout;/* character-based timeout */
@@ -308,6 +313,10 @@ struct uart_state {struct uart_info *info;struct uart_port *port; +#ifdef CONFIG_PPS_CLIENT_UART +struct pps_source_info_s pps_info; +#endif +struct mutex mutex; };
@@ -472,13 +481,27 @@ uart_handle_dcd_change(struct uart_port *port, unsigned int status) {struct uart_info *info = port->info; - port->icount.dcd++; +#ifdef CONFIG_PPS_CLIENT_UART +struct tty_driver *drv = port->info->tty->driver; -#ifdef CONFIG_HARD_PPS -if ((port->flags &UPF_HARDPPS_CD ) && status) -hardpps (); +if (port->flags &UPF_HARDPPS_CD ) { +if (status) { +pps_event (port->pps_source,PPS_CAPTUREASSERT , port); +pps_dbg ("%s%d: PPS assert event at %lu on source #%d" , + drv->driver_name, port->line, + jiffies, port->pps_source); + } +else { +pps_event (port->pps_source,PPS_CAPTURECLEAR , port); +pps_dbg ("%s%d: PPS clear event at %lu on source #%d" , + drv->driver_name, port->line, + jiffies, port->pps_source); + } + }#endif + port->icount.dcd++; +if (info->flags &UIF_CHECK_CD ) {if (status)wake_up_interruptible (&info->open_wait);
include/linux/syscalls.h +11 -0
@@ -65,6 +65,7 @@ struct getcpu_cache;#include <asm/signal.h> #include <linux/quota.h> #include <linux/key.h> +#include <linux/pps.h> asmlinkage long sys_time (time_t __user *tloc);asmlinkage long sys_stime (time_t __user *tptr);
@@ -611,6 +612,16 @@ asmlinkage long sys_timerfd(int ufd, int clockid, int flags, const struct itimerspec __user *utmr);asmlinkage long sys_eventfd (unsigned int count); +asmlinkage long sys_time_pps_cmd (int cmd,void __user *arg); +asmlinkage long sys_time_pps_getparams (int source, +struct pps_params __user *params); +asmlinkage long sys_time_pps_setparams (int source, +const struct pps_params __user *params); +asmlinkage long sys_time_pps_getcap (int source,int __user *mode); +asmlinkage long sys_time_pps_fetch (int source,const int tsformat, +struct pps_info __user *info, +const struct timespec __user *timeout); +int kernel_execve (const char *filename,char *const argv[],char *const envp[]);#endif
kernel/sys_ni.c +7 -0
@@ -148,3 +148,10 @@ cond_syscall(sys_timerfd);cond_syscall (compat_sys_signalfd);cond_syscall (compat_sys_timerfd);cond_syscall (sys_eventfd); + +/* PPS dependent */ +cond_syscall (sys_time_pps_find); +cond_syscall (sys_time_pps_getparams); +cond_syscall (sys_time_pps_setparams); +cond_syscall (sys_time_pps_getcap); +cond_syscall (sys_time_pps_fetch);
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 6 lines · Rodolfo Giometti
Quoted 3 lines · David Woodhouse
It doesn't apply to the current git tree, which has already had some new system calls added.
Ok, here the patch against latest git commit.
CC fs/fcntl.o
In file included from include/linux/syscalls.h:69,
from fs/fcntl.c:8:
include/linux/pps.h:51: error: field ‘tspec’ has incomplete type make[1]: *** [fs/fcntl.o] Error 1
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 6 lines · David Woodhouse
Quoted 5 lines · Rodolfo Giometti
Quoted 3 lines · David Woodhouse
It doesn't apply to the current git tree, which has already had some new system calls added.
Quoted 7 lines
Quoted 1 line
Ok, here the patch against latest git commit.
CC
fs/fcntl.o
In file included frominclude/linux/syscalls.h:69,from fs/fcntl.c:8:
include/linux/pps.h:51: error: field ???tspec??? has incomplete type make[1]: *** [fs/fcntl.o] Error 1
Gulp! =:-o
On my system I get:
giometti@zaigor:~/Projects/linuxpps/kernel$ touch fs/fcntl.c
giometti@zaigor:~/Projects/linuxpps/kernel$ make
CHK include/linux/version.h
CHK include/linux/utsrelease.h
CALL scripts/checksyscalls.sh
CHK include/linux/compile.h
CC fs/fcntl.o
LD fs/built-in.o
GEN .version
CHK include/linux/compile.h
UPD include/linux/compile.h
CC init/version.o
LD init/built-in.o
LD .tmp_vmlinux1
KSYM .tmp_kallsyms1.S
AS .tmp_kallsyms1.o
LD .tmp_vmlinux2
KSYM .tmp_kallsyms2.S
AS .tmp_kallsyms2.o
LD vmlinux
SYSMAP System.map
SYSMAP .tmp_System.map
MODPOST vmlinux
WARNING: arch/i386/kernel/built-in.o(.exit.text+0x18): Section mismatch: reference to .init.text: (after 'cache_remove_dev')
WARNING: kernel/built-in.o(.text+0x13826): Section mismatch: reference to .init.text: (between 'kthreadd' and 'init_waitqueue_head')
AS arch/i386/boot/setup.o
LD arch/i386/boot/setup
OBJCOPY arch/i386/boot/compressed/vmlinux.bin
GZIP arch/i386/boot/compressed/vmlinux.bin.gz
LD arch/i386/boot/compressed/piggy.o
LD arch/i386/boot/compressed/vmlinux
OBJCOPY arch/i386/boot/vmlinux.bin
BUILD arch/i386/boot/bzImage
Root device is (9, 0)
Boot sector 512 bytes.
Setup is 7057 bytes.
System is 1348 kB
Kernel: arch/i386/boot/bzImage is ready (#135)
Building modules, stage 2.
MODPOST 69 modulesHow is that possible??? I just git pull the linux code... maybe you have a bit older version?
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 2 lines · Rodolfo Giometti
How is that possible??? I just git pull the linux code... maybe you have a bit older version?
<asm-i386/signal.h> includes <linux/time.h>, for some reason. <asm-powerpc/signal.h> doesn't.
You shouldn't rely on <linux/time.h> being pulled in like that -- you either need a forward declaration of struct timespec, or to include <linux/time.h> for yourself from <linux/pps.h>
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 7 lines · David Woodhouse
<
asm-i386/signal.h> includes <linux/time.h>, for some reason. <asm-powerpc/signal.h> doesn't.You shouldn't rely on <
linux/time.h> being pulled in like that -- you either need a forward declaration of struct timespec, or to include <linux/time.h> for yourself from <linux/pps.h>
Can you please check if this patch resolve your problem?
include/linux/pps.h +1 -0
@@ -33,6 +33,7 @@#ifndef __KERNEL__#include <asm/types.h> +#include <linux/time.h> #include <sys/time.h> #endif
Thanks,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 16 lines · Rodolfo Giometti
Quoted 7 lines · David Woodhouse
<
asm-i386/signal.h> includes <linux/time.h>, for some reason. <asm-powerpc/signal.h> doesn't.You shouldn't rely on <
linux/time.h> being pulled in like that -- you either need a forward declaration of struct timespec, or to include <linux/time.h> for yourself from <linux/pps.h>Can you please check if this patch resolve your problem?
include/linux/pps.h +0
-0
include/linux/pps.h line 38
Quoted 6 lines
include/linux/pps.h +1
-0@@ -33,6 +33,7 @@#ifndef __KERNEL__#include <asm/types.h> +#include <linux/time.h> #include <sys/time.h> #endif
You'll need to put it in an #else case, not in #ifndef __KERNEL__.
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 2 lines · David Woodhouse
You'll need to put it in an #else case, not in #ifndef
__KERNEL__.
Sorry. :)
include/linux/pps.h +2 -0
@@ -34,6 +34,8 @@#ifndef __KERNEL__#include <asm/types.h> #include <sys/time.h> +#else +#include <linux/time.h> #endif #define PPS_API_VERS_2 2/* LinuxPPS proposal, dated 2006-05 */
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 5 lines · Rodolfo Giometti
Quoted 2 lines · David Woodhouse
You'll need to put it in an #else case, not in #ifndef
__KERNEL__.Sorry. :)
That matches what I built with earlier.
Signature · 1 line
dwmw2
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to Rodolfo Giomettidrivers/pps/pps.c line 382
Quoted 12 lines · Rodolfo Giometti
drivers/pps/pps.c new file +11
-0+asmlinkage long sys_time_pps_fetch (int source,const int tsformat, +struct pps_info __user *info, +const struct timespec __user *timeout) +{ +int ret; +struct timespec to; + +if (timeout) { + ret =copy_from_user (&to, timeout,sizeof (struct timespec )); +if (ret) +return ret;
You missed one. This should be -EFAULT too. And there's not a huge amount of point in keeping the access_ok() checks elsewhere, since copy_to_user() does that for itself.
Oh, and I think you do need compat magic for 'struct pps_info' and 'struct pps_params' too -- there's a struct timespec hidden deep in there, as well as 'unsigned long longpad[3]'.
Can you explain the 'union pps_timeu'? It seems very odd. How do we know which member of the union should be used?
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 3 lines · David Woodhouse
You missed one. This should be
-EFAULTtoo. And there's not a huge amount of point in keeping theaccess_ok()checks elsewhere, sincecopy_to_user()does that for itself.
Ok, fixed.
Quoted 3 lines
Oh, and I think you do need compat magic for 'struct
pps_info' and 'structpps_params' too -- there's a struct timespec hidden deep in there, as well as 'unsigned long longpad[3]'.
Gulp! Can you please give me some advices in order to solve also this problem? Should I use some "ifdef CONFIG_COMPAT" into those structures? :-o
Quoted 2 lines
Can you explain the 'union
pps_timeu'? It seems very odd. How do we know which member of the union should be used?
This union is defined by the RFC 2783... we can know which member of the union should be used by using define PPS_TSFMT_TSPEC for variable tsformat into function time_pps_fetch().
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 3 lines · Rodolfo Giometti
Gulp! Can you please give me some advices in order to solve also this problem? Should I use some "ifdef
CONFIG_COMPAT" into those structures? :-o
Remember you have to support _both_ 32-bit and 64-bit system calls. You need to define struct compat_pps_info and struct compat_pps_params, and you'll have to provide a compat wrapper for sys_time_pps_getparams() and sys_time_pps_setparams(). You'll also need to extend your compat_sys_time_pps_fetch() wrapper to handle the struct pps_info too.
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 6 lines · David Woodhouse
Remember you have to support _both_ 32-bit and 64-bit system calls. You need to define struct
compat_pps_infoand structcompat_pps_params, and you'll have to provide a compat wrapper forsys_time_pps_getparams()andsys_time_pps_setparams(). You'll also need to extend yourcompat_sys_time_pps_fetch()wrapper to handle the structpps_infotoo.
At this point I'm seriously considfering your previous suggestion:
Had you considered changing the API so that you don't need the compatibility wrapper at all? Could you take an integer number of µS or ms instead of a struct timespec?
Maybe I can define a special struct for exchanging time data as:
struct pps_timedata_s {
long sec;
long nsec;
}and managing time data conversions at userland...
What do you think about that? :)
Thanks,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 11 lines · On Sat, 30 Jun 2007 19:13:40 +0200 Rodolfo Giometti <[email protected]>…
Maybe I can define a special struct for exchanging time data as:
struct pps_timedata_s { long sec; long nsec; }and managing time data conversions at userland...
What do you think about that? :)
"long" is one of the datatypes that changes size between 32 bit and 64 bit, so not such a good choice. You could use __u32 or __64 (or whatever), that makes it very clear that these are fixed size data types.
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 9 lines · Stephen Rothwell
Quoted 8 lines · On Sat, 30 Jun 2007 19:13:40 +0200 Rodolfo Giometti <[email protected]>…
Maybe I can define a special struct for exchanging time data as:
struct pps_timedata_s { long sec; long nsec; }
Quoted 7 lines
Quoted 3 lines
and managing time data conversions at userland...
What do you think about that? :)
"long" is one of the datatypes that changes size between 32 bit and 64 bit, so not such a good choice. You could use
__u32or __64 (or whatever), that makes it very clear that these are fixed size data types.
struct pps_timedata_s {
__32 sec;
__32 nsec;
}Ok? I think 32 bits are enought for keeping seconds... :)
If you have no more advices I think I can start changing this part in order to avoid using struct timespec.
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 6 lines · Rodolfo Giometti
struct pps_timedata_s { __32 sec; __32 nsec; }Ok? I think 32 bits are enought for keeping seconds... :)
You want to purposely define an API that will break in 23 years (or is that 83 years since you made it unsigned potentially)? Why not 64bit for seconds and 32bit for nsec. That should cover it for long enough. When the unix time format was created 37 years ago, I could see thinking 32bit seemed reasonable, but why do it now. We have ram enough for 64bits.
--
Len Sorensen
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 6 lines · Lennart Sorensen
Quoted 5 lines · Rodolfo Giometti
struct pps_timedata_s { __32 sec; __32 nsec; }
Quoted 8 lines
Quoted 1 line
Ok? I think 32 bits are enought for keeping seconds... :)
You want to purposely define an API that will break in 23 years (or is that 83 years since you made it unsigned potentially)? Why not 64bit for seconds and 32bit for nsec. That should cover it for long enough. When the unix time format was created 37 years ago, I could see thinking 32bit seemed reasonable, but why do it now. We have ram enough for 64bits.
Sorry I wrote wrong. I meant __u32.
I can use __u64 for seconds but doing this there could be problems for 32 bits platforms? =:-o
Rodolfo (not a 64 bits guru :)
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 1 line · Rodolfo Giometti
Sorry I wrote wrong. I meant
__u32.
We guessed that much.
Quoted 2 lines
I can use
__u64for seconds but doing this there could be problems for 32 bits platforms? =:-o
Why would there be problems?
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 2 lines · David Woodhouse
Why would there be problems?
I'm just asking since I don't know well 64 bits architectures. :)
However, how can I fix 64 bits seconds into struct timespec?
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to Rodolfo GiomettiQuoted 4 lines · Rodolfo Giometti
Sorry I wrote wrong. I meant
__u32.I can use
__u64for seconds but doing this there could be problems for 32 bits platforms? =:-o
32bit platforms can still work with 64bit values, they may just not be quite as efficient about it.
After all we support files with more than 2^32 bytes in them, so the file access structures must have more than 32bit values in them.
--
Len Sorensen
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 6 lines · Lennart Sorensen
32bit platforms can still work with 64bit values, they may just not be quite as efficient about it.
After all we support files with more than 2^32 bytes in them, so the file access structures must have more than 32bit values in them.
My question is: how can I fit a 64 bits number of seconds into timespec structure which, for 32 bits architetures, has a 32 bits bits number of seconds?
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 3 lines · Rodolfo Giometti
My question is: how can I fit a 64 bits number of seconds into timespec structure which, for 32 bits architetures, has a 32 bits bits number of seconds?
I imagine you can't. I have no idea if there are any intensions to change the definition of timespec on 32bit architectures, or if the thought is that they will all be dead before it becomes a problem. :)
I still think the API you define for PPS should support 64bit seconds, even if on some systems you can't take advantage of it.
--
Len Sorensen
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 11 lines · Lennart Sorensen
Quoted 3 lines · Rodolfo Giometti
My question is: how can I fit a 64 bits number of seconds into timespec structure which, for 32 bits architetures, has a 32 bits bits number of seconds?
I imagine you can't. I have no idea if there are any intensions to change the definition of timespec on 32bit architectures, or if the thought is that they will all be dead before it becomes a problem. :)
I still think the API you define for PPS should support 64bit seconds, even if on some systems you can't take advantage of it.
So on 32 bits systems I simply drop the high part of the second data... is that right?
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to Lennart SorensenHi,
Quoted 10 lines · Lennart Sorensen
Quoted 6 lines · Rodolfo Giometti
struct pps_timedata_s { __32 sec; __32 nsec; }Ok? I think 32 bits are enought for keeping seconds... :)
You want to purposely define an API that will break in 23 years (or is that 83 years since you made it unsigned potentially)?
This is really not an API that needs to deal with such large time range.
bye, Roman
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 1 line · Roman Zippel
This is really not an API that needs to deal with such large time range.
No one will want to use PPS API to keep accurate time past 2030? Why not? Or should we be forced to invent a new PPS API before then to fix this one?
Actually, come to think of it, is this passing a current time, or it it passing an interval since last PPS? If it is an interval, then yes never mind, we don't need that many seconds (usually 1 or 2 at most). For some reason I thought this was for passing the current time of a stamp.
--
Len Sorensen
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 2 lines · Lennart Sorensen
For some reason I thought this was for passing the current time of a stamp.
And you was right. :)
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to Lennart SorensenHi,
Quoted 6 lines · Lennart Sorensen
Quoted 1 line · Roman Zippel
This is really not an API that needs to deal with such large time range.
No one will want to use PPS API to keep accurate time past 2030? Why not? Or should we be forced to invent a new PPS API before then to fix this one?
That's not necessary.
Quoted 5 lines
Actually, come to think of it, is this passing a current time, or it it passing an interval since last PPS? If it is an interval, then yes never mind, we don't need that many seconds (usually 1 or 2 at most). For some reason I thought this was for passing the current time of a stamp.
It's an absolute time stamp, but users of this interface are more interested in the time difference between events and how they change over time. E.g. NTP time stamps are 64bit fixed point values with a 32bit integral part.
This is not an interface to read the current system time, so 32bit will continue to work.
bye, Roman
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to Rodolfo GiomettiQuoted 6 lines · Rodolfo Giometti
Maybe I can define a special struct for exchanging time data as:
struct pps_timedata_s { long sec; long nsec; }
Seems reasonable enough in principle -- but whatever you do, don't use "long" for it. That would definitely need different behaviour for 32-bit vs. 64-bit. Use explicitly sized types such as uint32_t or uint64_t.
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 4 lines · David Woodhouse
Seems reasonable enough in principle -- but whatever you do, don't use "long" for it. That would definitely need different behaviour for 32-bit vs. 64-bit. Use explicitly sized types such as
uint32_toruint64_t.
Which is the difference in using __u32 or uint32_t? Maybe is better defining the new struct as follow?
struct pps_timedata_s {
uint32_t sec;
uint32_t nsec;
}or as:
struct pps_timedata_s {
__u32 sec;
__u32 nsec;
}?
Thanks,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to David WoodhouseQuoted 4 lines · David Woodhouse
Seems reasonable enough in principle -- but whatever you do, don't use "long" for it. That would definitely need different behaviour for 32-bit vs. 64-bit. Use explicitly sized types such as
uint32_toruint64_t.
Here the patch to convert LinuxPPS data structs into fixed ones.
Please, take a look at it and report possible modifications.
Thanks for your time,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Documentation/pps/timepps.h +81 -7
@@ -28,6 +28,32 @@/* --- 3.2 New data structures --------------------------------------------- */ +struct ntp_fp { +unsigned int integral; +unsigned int fractional; +}; + +union pps_timeu { +struct timespec tspec; +struct ntp_fp ntpfp; +unsigned long longpad[3 ]; +}; + +struct pps_info { +unsigned long assert_sequence;/* seq. num. of assert event */ +unsigned long clear_sequence;/* seq. num. of clear event */ +union pps_timeu assert_tu;/* time of assert event */ +union pps_timeu clear_tu;/* time of clear event */ +int current_mode;/* current mode bits */ +}; + +struct pps_params { +int api_version;/* API version # */ +int mode;/* mode bits */ +union pps_timeu assert_off_tu;/* offset compensation for assert */ +union pps_timeu clear_off_tu;/* offset compensation for clear */ +}; +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 */
@@ -86,7 +112,6 @@ void time_pps_readlink(char *link, int linklen, char *path, int pathlen)/* Return the file name where "link" points to */ path[i] ='\0' ; -return ; }#define PPS_HAVE_FINDPATH 1
@@ -108,8 +133,10 @@ int time_pps_findpath(char *path, int pathlen, char *idstring, int idlen)int time_pps_create (int source,pps_handle_t *handle) { -if (!handle) -return -EINVAL ; +if (!handle) { + errno = -EINVAL ; +return -1 ; + }/* In LinuxPPS there are no differences between a PPS source and * a PPS handle so we return the same value. */
@@ -128,12 +155,33 @@ int time_pps_destroy(pps_handle_t handle)int time_pps_getparams (pps_handle_t handle,pps_params_t *ppsparams) { -return syscall (__NR_time_pps_getparams, handle, ppsparams); +int ret; +struct pps_kparams __ppsparams; + + ret =syscall (__NR_time_pps_getparams, handle, &__ppsparams); + + ppsparams->api_version = __ppsparams.api_version; + ppsparams->mode = __ppsparams.mode; + ppsparams->assert_off_tu.tspec.tv_sec = __ppsparams.assert_off_tu.sec; + ppsparams->assert_off_tu.tspec.tv_nsec = __ppsparams.assert_off_tu.nsec; + ppsparams->clear_off_tu.tspec.tv_sec = __ppsparams.clear_off_tu.sec; + ppsparams->clear_off_tu.tspec.tv_nsec = __ppsparams.clear_off_tu.nsec; + +return ret; }int time_pps_setparams (pps_handle_t handle,const pps_params_t *ppsparams) { +struct pps_kparams __ppsparams; + + __ppsparams.api_version = ppsparams->api_version; + __ppsparams.mode = ppsparams->mode; + __ppsparams.assert_off_tu.sec = ppsparams->assert_off_tu.tspec.tv_sec; + __ppsparams.assert_off_tu.nsec = ppsparams->assert_off_tu.tspec.tv_nsec; + __ppsparams.clear_off_tu.sec = ppsparams->clear_off_tu.tspec.tv_sec; + __ppsparams.clear_off_tu.nsec = ppsparams->clear_off_tu.tspec.tv_nsec; +return syscall (__NR_time_pps_getparams, handle, ppsparams); }
@@ -147,15 +195,41 @@ int time_pps_fetch(pps_handle_t handle, const int tsformat,pps_info_t *ppsinfobuf, const struct timespec *timeout) { - return syscall(__NR_time_pps_fetch, handle, -tsformat ,ppsinfobuf ,timeout ); + struct pps_kinfo __ppsinfobuf; +struct pps_ktime __timeout; +int ret; + +/* Sanity checks */ +if (tsformat !=PPS_TSFMT_TSPEC ) { + errno = -EINVAL ; +return -1 ; + } + +if (timeout) { + __timeout.sec = timeout->tv_sec; + __timeout.nsec = timeout->tv_nsec; + } + + ret =syscall (__NR_time_pps_fetch, handle, &__ppsinfobuf, + timeout ? &__timeout :NULL ); + + ppsinfobuf->assert_sequence = __ppsinfobuf.assert_sequence; + ppsinfobuf->clear_sequence = __ppsinfobuf.clear_sequence; + ppsinfobuf->assert_tu.tspec.tv_sec = __ppsinfobuf.assert_tu.sec; + ppsinfobuf->assert_tu.tspec.tv_nsec = __ppsinfobuf.assert_tu.nsec; + ppsinfobuf->clear_tu.tspec.tv_sec = __ppsinfobuf.clear_tu.sec; + ppsinfobuf->clear_tu.tspec.tv_nsec = __ppsinfobuf.clear_tu.nsec; + ppsinfobuf->current_mode = __ppsinfobuf.current_mode; + +return ret; }int time_pps_kcbind (pps_handle_t handle,const int kernel_consumer,const int edge,const int tsformat) {/* LinuxPPS doesn't implement kernel consumer feature */ -return -EOPNOTSUPP ; + errno = -EOPNOTSUPP ; +return -1 ; }#endif /* _SYS_TIMEPPS_H_ */
drivers/pps/kapi.c +23 -18
81 lines not on this page · raw message
drivers/pps/pps.c +14 -20
@@ -168,7 +168,7 @@ sys_time_pps_cmd_exit: }asmlinkage long sys_time_pps_getparams (int source, -struct pps_params __user *params) +struct pps_kparams __user *params) {int ret =0 ;
@@ -189,7 +189,7 @@ asmlinkage long sys_time_pps_getparams(int source,/* Return current parameters */ ret =copy_to_user (params, &pps_source[source].params, -sizeof (struct pps_params )); +sizeof (struct pps_kparams ));if (ret) ret = -EFAULT ;
@@ -200,7 +200,7 @@ sys_time_pps_getparams_exit: }asmlinkage long sys_time_pps_setparams (int source, -const struct pps_params __user *params) +const struct pps_kparams __user *params) {int ret;
@@ -233,7 +233,7 @@ asmlinkage long sys_time_pps_setparams(int source,/* Save the new parameters */ ret =copy_from_user (&pps_source[source].params, params, -sizeof (struct pps_params )); +sizeof (struct pps_kparams ));if (ret) { ret = -EFAULT ; goto sys_time_pps_setparams_exit;
@@ -282,22 +282,16 @@ sys_time_pps_getcap_exit:return ret; } -asmlinkage long sys_time_pps_fetch (int source,const int tsformat, -struct pps_info __user *info, -const struct timespec __user *timeout) +asmlinkage long sys_time_pps_fetch (int source,struct pps_kinfo __user *info, +const struct pps_ktime __user *timeout) {unsigned long ticks; -struct pps_info pi; -struct timespec to; +struct pps_kinfo pi; +struct pps_ktime to;int ret;pps_dbg ("%s: source %d" , __FUNCTION__, source); -/* Sanity checks */ -if (tsformat !=PPS_TSFMT_TSPEC ) { -pps_dbg ("unsupported time format" ); -return -EINVAL ; - }if (!info)return -EINVAL ;
@@ -314,15 +308,15 @@ asmlinkage long sys_time_pps_fetch(int source, const int tsformat,/* Manage the timeout */ if (timeout) { - ret =copy_from_user (&to, timeout,sizeof (struct timespec )); + ret =copy_from_user (&to, timeout,sizeof (struct pps_ktime ));if (ret) { goto sys_time_pps_fetch_exit; ret = -EFAULT ; } -if (to.tv_sec !=-1 ) { -pps_dbg ("timeout %ld.%09ld" , to.tv_sec, to.tv_nsec); - ticks = to.tv_sec *HZ ; - ticks += to.tv_nsec / (NSEC_PER_SEC /HZ ); +if (to.sec !=-1 ) { +pps_dbg ("timeout %d.%09d" , to.sec, to.nsec); + ticks = to.sec *HZ ; + ticks += to.nsec / (NSEC_PER_SEC /HZ );if (ticks !=0 ) { ret = wait_event_interruptible_timeout(
@@ -352,7 +346,7 @@ asmlinkage long sys_time_pps_fetch(int source, const int tsformat, pi.assert_tu = pps_source[source].assert_tu; pi.clear_tu = pps_source[source].clear_tu; pi.current_mode = pps_source[source].current_mode; - ret =copy_to_user (info, &pi,sizeof (struct pps_info )); + ret =copy_to_user (info, &pi,sizeof (struct pps_kinfo ));if (ret) ret = -EFAULT ;
drivers/pps/sysfs.c +4 -6
@@ -34,9 +34,8 @@ static ssize_t pps_show_assert(struct class_device *cdev, char *buf) {struct pps_s *dev =class_get_devdata (cdev); -return sprintf (buf,"%ld.%09ld#%ld\n" , - dev->assert_tu.tspec.tv_sec, - dev->assert_tu.tspec.tv_nsec, +return sprintf (buf,"%d.%09d#%d\n" , + dev->assert_tu.sec, dev->assert_tu.nsec, dev->assert_sequence); }
@@ -44,9 +43,8 @@ static ssize_t pps_show_clear(struct class_device *cdev, char *buf) {struct pps_s *dev =class_get_devdata (cdev); -return sprintf (buf,"%ld.%09ld#%ld\n" , - dev->clear_tu.tspec.tv_sec, - dev->clear_tu.tspec.tv_nsec, +return sprintf (buf,"%d.%09d#%d\n" , + dev->clear_tu.sec, dev->clear_tu.nsec, dev->clear_sequence); }
include/linux/pps.h +16 -22
@@ -44,30 +44,24 @@#define PPS_MAX_NAME_LEN 32 -struct ntp_fp { -unsigned int integral; -unsigned int fractional; +struct pps_ktime { +__u32 sec; +__u32 nsec; }; -union pps_timeu { -struct timespec tspec; -struct ntp_fp ntpfp; -unsigned long longpad[3 ]; -}; - -struct pps_info { -unsigned long assert_sequence;/* seq. num. of assert event */ -unsigned long clear_sequence;/* seq. num. of clear event */ -union pps_timeu assert_tu;/* time of assert event */ -union pps_timeu clear_tu;/* time of clear event */ +struct pps_kinfo { +__u32 assert_sequence;/* seq. num. of assert event */ +__u32 clear_sequence;/* seq. num. of clear event */ +struct pps_ktime assert_tu;/* time of assert event */ +struct pps_ktime clear_tu;/* time of clear event */ int current_mode;/* current mode bits */ }; -struct pps_params { +struct pps_kparams {int api_version;/* API version # */ int mode;/* mode bits */ -union pps_timeu assert_off_tu;/* offset compensation for assert */ -union pps_timeu clear_off_tu;/* offset compensation for clear */ +struct pps_ktime assert_off_tu;/* offset compensation for assert */ +struct pps_ktime clear_off_tu;/* offset compensation for clear */ };/*
@@ -162,12 +156,12 @@ struct pps_source_info_s {struct pps_s {struct pps_source_info_s *info;/* PSS source info */ -struct pps_params params;/* PPS's current params */ +struct pps_kparams params;/* PPS's current params */ -volatile unsigned long assert_sequence;/* PPS' assert event seq # */ -volatile unsigned long clear_sequence;/* PPS' clear event seq # */ -volatile union pps_timeu assert_tu; -volatile union pps_timeu clear_tu; +volatile uint32_t assert_sequence;/* PPS' assert event seq # */ +volatile uint32_t clear_sequence;/* PPS' clear event seq # */ +volatile struct pps_ktime assert_tu; +volatile struct pps_ktime clear_tu;int current_mode;/* PPS mode at event time */ int go;/* PPS event is arrived? */
include/linux/syscalls.h +4 -5
@@ -614,13 +614,12 @@ asmlinkage long sys_eventfd(unsigned int count);asmlinkage long sys_time_pps_cmd (int cmd,void __user *arg);asmlinkage long sys_time_pps_getparams (int source, -struct pps_params __user *params); +struct pps_kparams __user *params);asmlinkage long sys_time_pps_setparams (int source, -const struct pps_params __user *params); +const struct pps_kparams __user *params);asmlinkage long sys_time_pps_getcap (int source,int __user *mode); -asmlinkage long sys_time_pps_fetch (int source,const int tsformat, -struct pps_info __user *info, -const struct timespec __user *timeout); +asmlinkage long sys_time_pps_fetch (int source,struct pps_kinfo __user *info, +const struct pps_ktime __user *timeout);int kernel_execve (const char *filename,char *const argv[],char *const envp[]);
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 9 lines · Rodolfo Giometti
Quoted 4 lines · David Woodhouse
Seems reasonable enough in principle -- but whatever you do, don't use "long" for it. That would definitely need different behaviour for 32-bit vs. 64-bit. Use explicitly sized types such as
uint32_toruint64_t.Here the patch to convert LinuxPPS data structs into fixed ones.
Please, take a look at it and report possible modifications.
Looks relatively sane at first glance; busy this week so haven't looked very hard yet. Two thing though... you're mixing proper C types (uint32_t) and the Linux-specific legacy crap types (__u32). Pick one. I won't recommend _which_ one, because if I do I'll make Andrew unhappy. But pick one; don't use both at the same time.
Also read Documentation/volatile-considered-harmful.txt and ponder deeply your use of 'volatile' on certain members of struct pps_s.
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 11 lines · David Woodhouse
Quoted 9 lines · Rodolfo Giometti
Quoted 4 lines · David Woodhouse
Seems reasonable enough in principle -- but whatever you do, don't use "long" for it. That would definitely need different behaviour for 32-bit vs. 64-bit. Use explicitly sized types such as
uint32_toruint64_t.Here the patch to convert LinuxPPS data structs into fixed ones.
Please, take a look at it and report possible modifications.
Quoted 8 lines
Looks relatively sane at first glance; busy this week so haven't looked very hard yet. Two thing though... you're mixing proper C types (
uint32_t) and the Linux-specific legacy crap types (__u32). Pick one. I won't recommend _which_ one, because if I do I'll make Andrew unhappy. But pick one; don't use both at the same time.Also read
Documentation/volatile-considered-harmful.txtand ponder deeply your use of 'volatile' on certain members of structpps_s.
I will! Thanks a lot.
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to David WoodhouseQuoted 6 lines · David Woodhouse
Looks relatively sane at first glance; busy this week so haven't looked very hard yet. Two thing though... you're mixing proper C types (
uint32_t) and the Linux-specific legacy crap types (__u32). Pick one. I won't recommend _which_ one, because if I do I'll make Andrew unhappy. But pick one; don't use both at the same time.
Ok. I choose __u32. :)
Quoted 2 lines
Also read
Documentation/volatile-considered-harmful.txtand ponder deeply your use of 'volatile' on certain members of structpps_s.
I read such document but I'm still convinced that the attribute volatile is needed for {assert,clear}_sequence and {assert,clear}_tu since inside pps_event() they are updated without any locks at all thanks to the dummy_info variable which is used for unallocated PPS sources.
Here you can find my last patch.
Thanks again,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Documentation/pps/Makefile +0 -0
Documentation/pps/timepps.h +76 -4
103 lines not on this page · raw message
drivers/pps/kapi.c +23 -18
81 lines not on this page · raw message
drivers/pps/pps.c +22 -30
104 lines not on this page · raw message
drivers/pps/sysfs.c +4 -6
@@ -34,9 +34,8 @@ static ssize_t pps_show_assert(struct class_device *cdev, char *buf) {struct pps_s *dev =class_get_devdata (cdev); -return sprintf (buf,"%ld.%09ld#%ld\n" , - dev->assert_tu.tspec.tv_sec, - dev->assert_tu.tspec.tv_nsec, +return sprintf (buf,"%d.%09d#%d\n" , + dev->assert_tu.sec, dev->assert_tu.nsec, dev->assert_sequence); }
@@ -44,9 +43,8 @@ static ssize_t pps_show_clear(struct class_device *cdev, char *buf) {struct pps_s *dev =class_get_devdata (cdev); -return sprintf (buf,"%ld.%09ld#%ld\n" , - dev->clear_tu.tspec.tv_sec, - dev->clear_tu.tspec.tv_nsec, +return sprintf (buf,"%d.%09d#%d\n" , + dev->clear_tu.sec, dev->clear_tu.nsec, dev->clear_sequence); }
include/linux/pps.h +16 -22
@@ -44,30 +44,24 @@#define PPS_MAX_NAME_LEN 32 -struct ntp_fp { -unsigned int integral; -unsigned int fractional; +struct pps_ktime { +__u32 sec; +__u32 nsec; }; -union pps_timeu { -struct timespec tspec; -struct ntp_fp ntpfp; -unsigned long longpad[3 ]; -}; - -struct pps_info { -unsigned long assert_sequence;/* seq. num. of assert event */ -unsigned long clear_sequence;/* seq. num. of clear event */ -union pps_timeu assert_tu;/* time of assert event */ -union pps_timeu clear_tu;/* time of clear event */ +struct pps_kinfo { +__u32 assert_sequence;/* seq. num. of assert event */ +__u32 clear_sequence;/* seq. num. of clear event */ +struct pps_ktime assert_tu;/* time of assert event */ +struct pps_ktime clear_tu;/* time of clear event */ int current_mode;/* current mode bits */ }; -struct pps_params { +struct pps_kparams {int api_version;/* API version # */ int mode;/* mode bits */ -union pps_timeu assert_off_tu;/* offset compensation for assert */ -union pps_timeu clear_off_tu;/* offset compensation for clear */ +struct pps_ktime assert_off_tu;/* offset compensation for assert */ +struct pps_ktime clear_off_tu;/* offset compensation for clear */ };/*
@@ -162,12 +156,12 @@ struct pps_source_info_s {struct pps_s {struct pps_source_info_s *info;/* PSS source info */ -struct pps_params params;/* PPS's current params */ +struct pps_kparams params;/* PPS's current params */ -volatile unsigned long assert_sequence;/* PPS' assert event seq # */ -volatile unsigned long clear_sequence;/* PPS' clear event seq # */ -volatile union pps_timeu assert_tu; -volatile union pps_timeu clear_tu; +volatile __u32 assert_sequence;/* PPS' assert event seq # */ +volatile __u32 clear_sequence;/* PPS' clear event seq # */ +volatile struct pps_ktime assert_tu; +volatile struct pps_ktime clear_tu;int current_mode;/* PPS mode at event time */ int go;/* PPS event is arrived? */
include/linux/syscalls.h +4 -5
@@ -614,13 +614,12 @@ asmlinkage long sys_eventfd(unsigned int count);asmlinkage long sys_time_pps_cmd (int cmd,void __user *arg);asmlinkage long sys_time_pps_getparams (int source, -struct pps_params __user *params); +struct pps_kparams __user *params);asmlinkage long sys_time_pps_setparams (int source, -const struct pps_params __user *params); +const struct pps_kparams __user *params);asmlinkage long sys_time_pps_getcap (int source,int __user *mode); -asmlinkage long sys_time_pps_fetch (int source,const int tsformat, -struct pps_info __user *info, -const struct timespec __user *timeout); +asmlinkage long sys_time_pps_fetch (int source,struct pps_kinfo __user *info, +const struct pps_ktime __user *timeout);int kernel_execve (const char *filename,char *const argv[],char *const envp[]);
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 9 lines · Rodolfo Giometti
Quoted 3 lines
Also read
Documentation/volatile-considered-harmful.txtand ponder deeply your use of 'volatile' on certain members of structpps_s.I read such document but I'm still convinced that the attribute volatile is needed for {assert,clear}_sequence and {assert,clear}_tu since inside
pps_event()they are updated without any locks at all thanks to thedummy_infovariable which is used for unallocated PPS sources.
I'm sure the version with 'volatile' will also be broken then. Sounds like the right answer is to fix the locking.
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 3 lines · David Woodhouse
I'm sure the version with 'volatile' will also be broken then. Sounds like the right answer is to fix the locking.
But wish avoiding locking at all since this may delay the time stamp recording. We (the LinuxPPS guys) niticed that just a single instruction my degrade the time setting about 50%!
I avoid locking just using a pointer and setting it to a dummy structure when not used... is that wrong? :-o
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 11 lines · Rodolfo Giometti
Quoted 3 lines · David Woodhouse
I'm sure the version with 'volatile' will also be broken then. Sounds like the right answer is to fix the locking.
But wish avoiding locking at all since this may delay the time stamp recording. We (the LinuxPPS guys) niticed that just a single instruction my degrade the time setting about 50%!
I avoid locking just using a pointer and setting it to a dummy structure when not used... is that wrong? :-o
Almost certainly (admittedly said without looking).
Signature · 1 line
dwmw2
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 6 lines · David Woodhouse
Quoted 5 lines · Rodolfo Giometti
Quoted 3 lines · David Woodhouse
I'm sure the version with 'volatile' will also be broken then. Sounds like the right answer is to fix the locking.
Quoted 8 lines
Quoted 6 lines
But wish avoiding locking at all since this may delay the time stamp recording. We (the LinuxPPS guys) niticed that just a single instruction my degrade the time setting about 50%!
I avoid locking just using a pointer and setting it to a dummy structure when not used... is that wrong? :-o
Almost certainly (admittedly said without looking).
Please take a look at the code. I'm quite sure that it's ok. :)
I just set a pointer to a dummy struct or a valid one. By doing like this I don't need a lock in the irq handler since it writes always into a valid area. At non irq time I simply use a mutex lock.
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to Rodolfo GiomettiQuoted 8 lines · Rodolfo Giometti
Hello,
here my new LinuxPPS patch.
What to do now for kernel inclusion? Should I provide several patches? If so how should I divide them?
Thanks a lot,
Sorry for coming in that late, but using syscalls for something as periphal sounds like a very bad idea to me, and the syscalls aren't defined nicely either (e.g. you have an ioctl lookalike). I'd say back to the drawingboard.
And yes, even ioctls are nicer than badly designed syscalls :)
Also code seems to be odd at least in a few places, e.g. all the access_oks and double checks of the ioctl-lookalike commands should go.
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 5 lines · Christoph Hellwig
Sorry for coming in that late, but using syscalls for something as periphal sounds like a very bad idea to me, and the syscalls aren't defined nicely either (e.g. you have an ioctl lookalike). I'd say back to the drawingboard.
PPS API is not only a periphal class. RFC 2783 defines new «functions» that allow accesso to internal system data collected by some periferals but these devices are never managed as a tipical char or block device.
I think implementing parts or full RFC 2783 PPS's functions as syscalls are not so wrong... IMHO, at least! :)
Quoted 1 line
And yes, even ioctls are nicer than badly designed syscalls :)
I see, but consider that some PPS devices cannot be always connected with a filedescriptor since, for example, some PPS devices are connected by serial lines, other by parallel ports and other (expecially on embedded systems) are connected by CPU's GPIOs.
An userland programs shouldn't know whenever these devices are connected, they should only know how to collect PPS data from the system.
Quoted 2 lines
Also code seems to be odd at least in a few places, e.g. all the
access_oksand double checks of the ioctl-lookalike commands should go.
I already removed such functions in my latest patches. :)
Thanks,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Re: [PATCH] LinuxPPS (with new syscalls API) - new version
In reply to Rodolfo Giometti* Rodolfo Giometti (Thu, 28 Jun 2007 18:14:50 +0200)
* Organization: GNU/Linux Device Drivers, Embedded Systems and Courses
Documentation/pps/Makefile line 14
Quoted 6 lines
+.PHONY : all depend dep
Documentation/pps/Makefile new file +5
-0+ +all : .depend $(TARGETS) + +.depend depend dep : + $(CC) $(CFLAGS) -M $(TARGETS:=.c) > .depend
[]
Documentation/pps/Makefile line 26
Quoted 6 lines
Documentation/pps/Makefile new file +6
-0+# -- Clean section ------------------------------------------------------------ + +.PHONY : clean + +clean : + rm -f *.o *~ core .depend
Please see how currently .PHONY targets are used correctly, e.g. in linux/Makefile.
____
Rodolfo Giometti
maintainerRe: [PATCH] LinuxPPS (with new syscalls API) - new version
Quoted 10 lines · Oleg Verych
* Rodolfo Giometti (Thu, 28 Jun 2007 18:14:50 +0200)
* Organization: GNU/Linux Device Drivers, Embedded Systems and CoursesDocumentation/pps/Makefile line 14
Quoted 6 lines
+.PHONY : all depend dep
Documentation/pps/Makefile new file +5
-0+ +all : .depend $(TARGETS) + +.depend depend dep : + $(CC) $(CFLAGS) -M $(TARGETS:=.c) > .depend[]
Quoted 9 lines
Documentation/pps/Makefile line 26
Quoted 6 lines
Documentation/pps/Makefile new file +6
-0+# -- Clean section ------------------------------------------------------------ + +.PHONY : clean + +clean : + rm -f *.o *~ core .dependPlease see how currently .PHONY targets are used correctly, e.g. in linux/Makefile.
I don't understand why you wish I use linux PHONY targets management in a documentation Makefile... I suppose you are talking about linux/Documentation/pps/Makefile which is just an example about people may compile some testing/debugging programs. :-o
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Makefiles for GNU make (Re: [PATCH] LinuxPPS (with new syscalls API) - new version)
Quoted 21 lines · Rodolfo Giometti
Quoted 10 lines · Oleg Verych
* Rodolfo Giometti (Thu, 28 Jun 2007 18:14:50 +0200)
* Organization: GNU/Linux Device Drivers, Embedded Systems and CoursesDocumentation/pps/Makefile line 14
Quoted 6 lines
+.PHONY : all depend dep
Documentation/pps/Makefile new file +5
-0+ +all : .depend $(TARGETS) + +.depend depend dep : + $(CC) $(CFLAGS) -M $(TARGETS:=.c) > .depend[]
Quoted 9 lines
Documentation/pps/Makefile line 26
Quoted 6 lines
Documentation/pps/Makefile new file +6
-0+# -- Clean section ------------------------------------------------------------ + +.PHONY : clean + +clean : + rm -f *.o *~ core .dependPlease see how currently .PHONY targets are used correctly, e.g. in linux/Makefile.
Quoted 1 line
I don't understand why you wish I use linux PHONY targets management
Sorry i was not clear. Problem is `GNU make'[0], not management.
[0] <http://article.gmane.org/gmane.comp.gnu.make.bugs/2481>
Quoted 3 lines
in a documentation
Makefile... I suppose you are talking about linux/Documentation/pps/Makefile which is just an example about people may compile some testing/debugging programs. :-o
So, after this example you may write correct Makefiles for `GNU make'.
Quoted 4 lines
Ciao,
Rodolfo
____
Rodolfo Giometti
maintainerRe: Makefiles for GNU make (Re: [PATCH] LinuxPPS (with new syscalls API) - new version)
Quoted 26 lines · Oleg Verych
Quoted 21 lines · Rodolfo Giometti
Quoted 10 lines · Oleg Verych
* Rodolfo Giometti (Thu, 28 Jun 2007 18:14:50 +0200)
* Organization: GNU/Linux Device Drivers, Embedded Systems and CoursesDocumentation/pps/Makefile line 14
Quoted 6 lines
+.PHONY : all depend dep
Documentation/pps/Makefile new file +5
-0+ +all : .depend $(TARGETS) + +.depend depend dep : + $(CC) $(CFLAGS) -M $(TARGETS:=.c) > .depend[]
Quoted 9 lines
Documentation/pps/Makefile line 26
Quoted 6 lines
Documentation/pps/Makefile new file +6
-0+# -- Clean section ------------------------------------------------------------ + +.PHONY : clean + +clean : + rm -f *.o *~ core .dependPlease see how currently .PHONY targets are used correctly, e.g. in linux/Makefile.
Quoted 1 line
I don't understand why you wish I use linux PHONY targets management
Sorry i was not clear. Problem is `GNU make'[0], not management.
Quoted 7 lines
[0] <http://article.gmane.org/gmane.comp.gnu.make.bugs/2481>
Quoted 3 lines
in a documentation
Makefile... I suppose you are talking about linux/Documentation/pps/Makefile which is just an example about people may compile some testing/debugging programs. :-oSo, after this example you may write correct Makefiles for `GNU make'.
I see... but I'm still a bit confused... if I well understand I should change the Makefile as follow:
diff --git a/Documentation/pps/Makefile b/Documentation/pps/Makefile index a2660a2..d766447 100644 --- a/Documentation/pps/Makefile +++ b/Documentation/pps/Makefile @@ -6,7 +6,7 @@ CFLAGS += -ggdb
# -- Actions section ----------------------------------------------------------
-.PHONY : all depend dep
+.PHONY : depend dep
all : .depend $(TARGETS)
@@ -20,8 +20,6 @@ endif
# -- Clean section ------------------------------------------------------------
-.PHONY : clean
-
clean :
rm -f *.o *~ core .depend
rm -f ${TARGETS}
Is that right?
Thanks,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127