Re: [PATCHv2 4/6] pps: add kernel consumer support

From: Rodolfo Giometti
Date: Mon Mar 01 2010 - 03:30:13 EST

Next message: tip-bot for Paul E. McKenney: "[tip:core/rcu] sched, rcu: Fix rcu_dereference() for RCU-lockdep"
Previous message: Sachin Sant: "Re: 2.6.33-git6 boot failure[x86_64] (WARN: at arch/x86/mm/ioremap.c:111)"
Next in thread: Alexander Gordeev: "Re: [PATCHv2 4/6] pps: add kernel consumer support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Feb 24, 2010 at 03:28:15PM +0300, Alexander Gordeev wrote:
> Add an optional feature of PPSAPI, kernel consumer support, which uses
> the added hardpps() function.
>
> Signed-off-by: Alexander Gordeev <lasaine@xxxxxxxxxxxxx>

Acked-by: Rodolfo Giometti <giometti@xxxxxxxx>

However see the note below.

> ---
> Documentation/ioctl/ioctl-number.txt | 2 +-
> drivers/pps/kapi.c | 26 +++++++++++++
> drivers/pps/pps.c | 67 +++++++++++++++++++++++++++++++++-
> include/linux/pps.h | 7 ++++
> include/linux/pps_kernel.h | 6 +++
> 5 files changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index 9473749..d68718b 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -152,7 +152,7 @@ Code Seq# Include File Comments
> 'p' 40-7F linux/nvram.h
> 'p' 80-9F user-space parport
> <mailto:tim@xxxxxxxxxxxx>
> -'p' a1-a4 linux/pps.h LinuxPPS
> +'p' a1-a5 linux/pps.h LinuxPPS
> <mailto:giometti@xxxxxxxx>
> 'q' 00-1F linux/serio.h
> 'q' 80-FF Internet PhoneJACK, Internet LineJACK
> diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
> index b87f699..5871744 100644
> --- a/drivers/pps/kapi.c
> +++ b/drivers/pps/kapi.c
> @@ -25,6 +25,7 @@
> #include <linux/init.h>
> #include <linux/sched.h>
> #include <linux/time.h>
> +#include <linux/timex.h>
> #include <linux/spinlock.h>
> #include <linux/idr.h>
> #include <linux/fs.h>
> @@ -37,6 +38,12 @@
> DEFINE_SPINLOCK(pps_idr_lock);
> DEFINE_IDR(pps_idr);
>
> +/* state variables to bind kernel consumer */
> +/* PPS API (RFC 2783): current source and mode for ``kernel consumer'' */
> +DEFINE_SPINLOCK(pps_kc_hardpps_lock);
> +void *pps_kc_hardpps_dev; /* some unique pointer to device */
> +int pps_kc_hardpps_mode; /* mode bits for kernel consumer */
> +
> /*
> * Local functions
> */
> @@ -248,6 +255,15 @@ void pps_unregister_source(int source)
> }
> spin_unlock_irq(&pps_idr_lock);
>
> + spin_lock_irq(&pps_kc_hardpps_lock);
> + if (pps == pps_kc_hardpps_dev) {
> + pps_kc_hardpps_mode = 0;
> + pps_kc_hardpps_dev = NULL;
> + spin_unlock_irq(&pps_kc_hardpps_lock);
> + pr_info("unbound kernel consumer on device removal\n");
> + } else
> + spin_unlock_irq(&pps_kc_hardpps_lock);
> +

I suppose you are using such if-else schema due the fact pr_info is
too slow to be executed with helded spinlock, arent't you? :)

> pps_unregister_cdev(pps);
> pps_put_source(pps);
> }
> @@ -325,6 +341,16 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
>
> captured = ~0;
> }
> + spin_lock(&pps_kc_hardpps_lock);
> + if (pps == pps_kc_hardpps_dev && event & pps_kc_hardpps_mode) {
> + struct timespec p_ts = {
> + .tv_sec = ts->ts_real.sec,
> + .tv_nsec = ts->ts_real.nsec
> + };
> +
> + hardpps(&p_ts, &ts->ts_raw);
> + }
> + spin_unlock(&pps_kc_hardpps_lock);
>
> /* Wake up iif captured somthing */
> if (captured) {
> diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
> index ca5183b..22fdcfe 100644
> --- a/drivers/pps/pps.c
> +++ b/drivers/pps/pps.c
> @@ -62,6 +62,8 @@ static long pps_cdev_ioctl(struct file *file,
> struct pps_device *pps = file->private_data;
> struct pps_kparams params;
> struct pps_fdata fdata;
> + struct pps_bind_args bind_args;
> + unsigned long irq_flags;
> unsigned long ticks;
> void __user *uarg = (void __user *) arg;
> int __user *iuarg = (int __user *) arg;
> @@ -186,9 +188,72 @@ static long pps_cdev_ioctl(struct file *file,
>
> break;
>
> + case PPS_KC_BIND:
> + pr_info("PPS_KC_BIND: source %d\n", pps->id);
> +
> + /* Check the capabilities */
> + if (!capable(CAP_SYS_TIME))
> + return -EPERM;
> +
> + if (copy_from_user(&bind_args, uarg,
> + sizeof(struct pps_bind_args)))
> + return -EFAULT;
> +
> + /* Check for supported capabilities */
> + if ((bind_args.edge & ~pps->info.mode) != 0) {
> + pr_err("PPS_KC_BIND: "
> + "unsupported capabilities (%x)\n",
> + bind_args.edge);
> + return -EINVAL;
> + }
> +
> + /* Validate parameters roughly */
> + if (bind_args.tsformat != PPS_TSFMT_TSPEC ||
> + (bind_args.edge & ~PPS_CAPTUREBOTH) != 0 ||
> + bind_args.consumer != PPS_KC_HARDPPS) {
> + pr_err("PPS_KC_BIND: "
> + "invalid kcbind parameters (%x)\n",
> + bind_args.edge);
> + return -EINVAL;
> + }
> +
> + /* Check if another consumer is already bound */
> + spin_lock_irqsave(&pps_kc_hardpps_lock, irq_flags);
> +
> + if (bind_args.edge == 0)
> + if (pps_kc_hardpps_dev == pps) {
> + pps_kc_hardpps_mode = 0;
> + pps_kc_hardpps_dev = NULL;
> + spin_unlock_irqrestore(&pps_kc_hardpps_lock,
> + irq_flags);
> + pr_info("unbound kernel consumer\n");
> + } else {
> + spin_unlock_irqrestore(&pps_kc_hardpps_lock,
> + irq_flags);
> + pr_err("selected kernel consumer"
> + " is not bound\n");
> + return -EINVAL;
> + }
> + else
> + if (pps_kc_hardpps_dev == NULL ||
> + pps_kc_hardpps_dev == pps) {
> + pps_kc_hardpps_mode = bind_args.edge;
> + pps_kc_hardpps_dev = pps;
> + spin_unlock_irqrestore(&pps_kc_hardpps_lock,
> + irq_flags);
> + pr_info("bound kernel consumer: dev=%p, "
> + "edge=0x%x\n", pps, bind_args.edge);
> + } else {
> + spin_unlock_irqrestore(&pps_kc_hardpps_lock,
> + irq_flags);
> + pr_err("another kernel consumer"
> + " is already bound\n");
> + return -EINVAL;
> + }
> + break;
> +
> default:
> return -ENOTTY;
> - break;
> }
>
> return 0;
> diff --git a/include/linux/pps.h b/include/linux/pps.h
> index 0194ab0..a9bb1d9 100644
> --- a/include/linux/pps.h
> +++ b/include/linux/pps.h
> @@ -114,11 +114,18 @@ struct pps_fdata {
> struct pps_ktime timeout;
> };
>
> +struct pps_bind_args {
> + int tsformat; /* format of time stamps */
> + int edge; /* selected event type */
> + int consumer; /* selected kernel consumer */
> +};
> +
> #include <linux/ioctl.h>
>
> #define PPS_GETPARAMS _IOR('p', 0xa1, struct pps_kparams *)
> #define PPS_SETPARAMS _IOW('p', 0xa2, struct pps_kparams *)
> #define PPS_GETCAP _IOR('p', 0xa3, int *)
> #define PPS_FETCH _IOWR('p', 0xa4, struct pps_fdata *)
> +#define PPS_KC_BIND _IOW('p', 0xa5, struct pps_bind_args *)
>
> #endif /* _PPS_H_ */
> diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
> index 9bda892..2b203cd 100644
> --- a/include/linux/pps_kernel.h
> +++ b/include/linux/pps_kernel.h
> @@ -80,6 +80,12 @@ extern struct timespec pps_irq_ts[];
>
> extern struct device_attribute pps_attrs[];
>
> +/* state variables to bind kernel consumer */
> +/* PPS API (RFC 2783): current source and mode for ``kernel consumer'' */
> +extern spinlock_t pps_kc_hardpps_lock;
> +extern void *pps_kc_hardpps_dev; /* some unique pointer to device */
> +extern int pps_kc_hardpps_mode; /* mode bits for kernel consumer */
> +
> /*
> * Exported functions
> */
> --
> 1.6.6.1
>

--

GNU/Linux Solutions e-mail: giometti@xxxxxxxxxxxx
Linux Device Driver giometti@xxxxxxxx
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: tip-bot for Paul E. McKenney: "[tip:core/rcu] sched, rcu: Fix rcu_dereference() for RCU-lockdep"
Previous message: Sachin Sant: "Re: 2.6.33-git6 boot failure[x86_64] (WARN: at arch/x86/mm/ioremap.c:111)"
Next in thread: Alexander Gordeev: "Re: [PATCHv2 4/6] pps: add kernel consumer support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]