2011-04-27 18:15:27

by James Nuss

[permalink] [raw]
Subject: [PATCH 2/2] pps: new client driver using IRQs

This client driver allows you to use raw IRQs as a source for PPS
signals.

This driver is based on the work by Ricardo Martins <[email protected]> who
submitted an initial implementation [1] of a PPS IRQ client driver to
the linuxpps mailing-list on Dec 3 2010.

[1] http://ml.enneenne.com/pipermail/linuxpps/2010-December/004155.html

Signed-off-by: James Nuss <[email protected]>
Reviewed-by: Ben Gardiner <[email protected]>
CC: Ricardo Martins <[email protected]>
---
NOTE: drivers/pps/clients/Kconfig does not have the obligatory statements
regarding building the drivers as modules, even though they are all tristates.
This was purposefully omitted from the new config item for consistency.
This could be addressed in a separate patch.

drivers/pps/clients/Kconfig | 8 ++
drivers/pps/clients/Makefile | 1 +
drivers/pps/clients/pps-irq.c | 169 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 178 insertions(+), 0 deletions(-)
create mode 100644 drivers/pps/clients/pps-irq.c

diff --git a/drivers/pps/clients/Kconfig b/drivers/pps/clients/Kconfig
index 8520a7f..94b2cc6 100644
--- a/drivers/pps/clients/Kconfig
+++ b/drivers/pps/clients/Kconfig
@@ -29,4 +29,12 @@ config PPS_CLIENT_PARPORT
If you say yes here you get support for a PPS source connected
with the interrupt pin of your parallel port.

+config PPS_CLIENT_IRQ
+ tristate "PPS client based on raw IRQs"
+ depends on PPS
+ help
+ If you say yes here you get support for a PPS source based
+ on raw IRQs. To be useful, you must also register a platform
+ device with an IRQ resource, usually in your board setup.
+
endif
diff --git a/drivers/pps/clients/Makefile b/drivers/pps/clients/Makefile
index 42517da..609a332 100644
--- a/drivers/pps/clients/Makefile
+++ b/drivers/pps/clients/Makefile
@@ -5,6 +5,7 @@
obj-$(CONFIG_PPS_CLIENT_KTIMER) += pps-ktimer.o
obj-$(CONFIG_PPS_CLIENT_LDISC) += pps-ldisc.o
obj-$(CONFIG_PPS_CLIENT_PARPORT) += pps_parport.o
+obj-$(CONFIG_PPS_CLIENT_IRQ) += pps-irq.o

ifeq ($(CONFIG_PPS_DEBUG),y)
EXTRA_CFLAGS += -DDEBUG
diff --git a/drivers/pps/clients/pps-irq.c b/drivers/pps/clients/pps-irq.c
new file mode 100644
index 0000000..33c1bf4
--- /dev/null
+++ b/drivers/pps/clients/pps-irq.c
@@ -0,0 +1,169 @@
+/*
+ * pps-irq.c -- PPS IRQ client
+ *
+ *
+ * Copyright (C) 2010 Ricardo Martins <[email protected]>
+ * Copyright (C) 2011 James Nuss <[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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/pps_kernel.h>
+#include <linux/gpio.h>
+#include <linux/list.h>
+
+#define PPS_IRQ_NAME "pps-irq"
+#define PPS_IRQ_VERSION "1.1.0"
+
+/* Info for each registered platform device */
+struct pps_irq_data {
+ int irq; /* IRQ used as PPS source */
+ struct pps_device *pps; /* PPS source device */
+ struct pps_source_info info; /* PPS source information */
+};
+
+/*
+ * Report the PPS event
+ */
+
+static irqreturn_t pps_irq_irq_handler(int irq, void *data)
+{
+ struct pps_irq_data *info;
+ struct pps_event_time ts;
+
+ /* Get the time stamp first */
+ pps_get_ts(&ts);
+
+ info = (struct pps_irq_data *)data;
+ pps_event(info->pps, &ts, PPS_CAPTUREASSERT, NULL);
+
+ return IRQ_HANDLED;
+}
+
+static int pps_irq_probe(struct platform_device *pdev)
+{
+ struct pps_irq_data *data;
+ struct resource *res;
+ int irq;
+ int ret;
+
+ /* Validate resource. */
+ if (pdev->num_resources != 1) {
+ pr_err(PPS_IRQ_NAME ": this driver handles only one IRQ resource");
+ return -EINVAL;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (res == NULL) {
+ pr_err(PPS_IRQ_NAME ": no IRQ resource was given");
+ return -EINVAL;
+ }
+
+ if (!(res->flags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
+ pr_err(PPS_IRQ_NAME ": given IRQ resource must be edge triggered");
+ return -EINVAL;
+ }
+
+ /* Allocate space for device info */
+ data = kzalloc(sizeof(struct pps_irq_data), GFP_KERNEL);
+ if (data == NULL)
+ return -ENOMEM;
+
+ /* Initialize PPS specific parts of the bookkeeping data structure. */
+ data->info.mode = PPS_CAPTUREASSERT | PPS_OFFSETASSERT |
+ PPS_ECHOASSERT | PPS_CANWAIT | PPS_TSFMT_TSPEC;
+ data->info.owner = THIS_MODULE;
+ snprintf(data->info.name, PPS_MAX_NAME_LEN - 1, "%s.%d",
+ pdev->name, pdev->id);
+
+ /* Register PPS source. */
+ irq = platform_get_irq(pdev, 0);
+ pr_info(PPS_IRQ_NAME ": registering IRQ %d as PPS source", irq);
+ data->pps = pps_register_source(&data->info, PPS_CAPTUREASSERT
+ | PPS_OFFSETASSERT);
+ if (data->pps == NULL) {
+ kfree(data);
+ pr_err(PPS_IRQ_NAME ": failed to register IRQ %d as PPS source",
+ irq);
+ return -1;
+ }
+
+ /* Request IRQ. */
+ pr_info(PPS_IRQ_NAME ": requesting IRQ %d", irq);
+ ret = request_irq(irq, pps_irq_irq_handler,
+ res->flags & IRQF_TRIGGER_MASK,
+ data->info.name, data);
+ if (ret) {
+ pps_unregister_source(data->pps);
+ kfree(data);
+ pr_err(PPS_IRQ_NAME ": failed to acquire IRQ %d\n", irq);
+ return ret;
+ }
+ data->irq = irq;
+
+ platform_set_drvdata(pdev, data);
+ dev_info(data->pps->dev, "Registered IRQ %d as PPS source", data->irq);
+
+ return 0;
+}
+
+static int pps_irq_remove(struct platform_device *pdev)
+{
+ struct pps_irq_data *data = platform_get_drvdata(pdev);
+ platform_set_drvdata(pdev, NULL);
+ free_irq(data->irq, data);
+ pps_unregister_source(data->pps);
+ pr_info(PPS_IRQ_NAME ": removed IRQ %d as PPS source", data->irq);
+ kfree(data);
+ return 0;
+}
+
+static struct platform_driver pps_irq_driver = {
+ .probe = pps_irq_probe,
+ .remove = __devexit_p(pps_irq_remove),
+ .driver = {
+ .name = PPS_IRQ_NAME,
+ .owner = THIS_MODULE
+ },
+};
+
+static int __init pps_irq_init(void)
+{
+ int ret = platform_driver_register(&pps_irq_driver);
+ if (ret < 0)
+ pr_err(PPS_IRQ_NAME ": failed to register platform driver");
+ return ret;
+}
+
+static void __exit pps_irq_exit(void)
+{
+ platform_driver_unregister(&pps_irq_driver);
+ pr_debug(PPS_IRQ_NAME ": unregistered platform driver");
+}
+
+module_init(pps_irq_init);
+module_exit(pps_irq_exit);
+
+MODULE_AUTHOR("Ricardo Martins <[email protected]>");
+MODULE_AUTHOR("James Nuss <[email protected]>");
+MODULE_DESCRIPTION("Use raw IRQs as PPS source");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(PPS_IRQ_VERSION);
--
1.7.1


2011-04-27 19:21:17

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 2/2] pps: new client driver using IRQs

On Wed, Apr 27, 2011 at 02:14:14PM -0400, James Nuss wrote:
> This client driver allows you to use raw IRQs as a source for PPS
> signals.
>
> This driver is based on the work by Ricardo Martins <[email protected]> who
> submitted an initial implementation [1] of a PPS IRQ client driver to
> the linuxpps mailing-list on Dec 3 2010.
>
> [1] http://ml.enneenne.com/pipermail/linuxpps/2010-December/004155.html
>
> Signed-off-by: James Nuss <[email protected]>
> Reviewed-by: Ben Gardiner <[email protected]>
> CC: Ricardo Martins <[email protected]>

Acked-by: Rodolfo Giometti <[email protected]>

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2011-04-28 11:31:23

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [LinuxPPS] [PATCH 2/2] pps: new client driver using IRQs

Hi James,

В Wed, 27 Apr 2011 14:14:14 -0400
James Nuss <[email protected]> пишет:

> This client driver allows you to use raw IRQs as a source for PPS
> signals.
>
> This driver is based on the work by Ricardo Martins <[email protected]> who
> submitted an initial implementation [1] of a PPS IRQ client driver to
> the linuxpps mailing-list on Dec 3 2010.
>
> [1] http://ml.enneenne.com/pipermail/linuxpps/2010-December/004155.html
>
> Signed-off-by: James Nuss <[email protected]>
> Reviewed-by: Ben Gardiner <[email protected]>
> CC: Ricardo Martins <[email protected]>
> ---
> NOTE: drivers/pps/clients/Kconfig does not have the obligatory statements
> regarding building the drivers as modules, even though they are all tristates.
> This was purposefully omitted from the new config item for consistency.
> This could be addressed in a separate patch.
>
> drivers/pps/clients/Kconfig | 8 ++
> drivers/pps/clients/Makefile | 1 +
> drivers/pps/clients/pps-irq.c | 169 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 178 insertions(+), 0 deletions(-)
> create mode 100644 drivers/pps/clients/pps-irq.c
>
> diff --git a/drivers/pps/clients/Kconfig b/drivers/pps/clients/Kconfig
> index 8520a7f..94b2cc6 100644
> --- a/drivers/pps/clients/Kconfig
> +++ b/drivers/pps/clients/Kconfig
> @@ -29,4 +29,12 @@ config PPS_CLIENT_PARPORT
> If you say yes here you get support for a PPS source connected
> with the interrupt pin of your parallel port.
>
> +config PPS_CLIENT_IRQ
> + tristate "PPS client based on raw IRQs"
> + depends on PPS
> + help
> + If you say yes here you get support for a PPS source based
> + on raw IRQs. To be useful, you must also register a platform
> + device with an IRQ resource, usually in your board setup.
> +
> endif
> diff --git a/drivers/pps/clients/Makefile b/drivers/pps/clients/Makefile
> index 42517da..609a332 100644
> --- a/drivers/pps/clients/Makefile
> +++ b/drivers/pps/clients/Makefile
> @@ -5,6 +5,7 @@
> obj-$(CONFIG_PPS_CLIENT_KTIMER) += pps-ktimer.o
> obj-$(CONFIG_PPS_CLIENT_LDISC) += pps-ldisc.o
> obj-$(CONFIG_PPS_CLIENT_PARPORT) += pps_parport.o
> +obj-$(CONFIG_PPS_CLIENT_IRQ) += pps-irq.o
>
> ifeq ($(CONFIG_PPS_DEBUG),y)
> EXTRA_CFLAGS += -DDEBUG
> diff --git a/drivers/pps/clients/pps-irq.c b/drivers/pps/clients/pps-irq.c
> new file mode 100644
> index 0000000..33c1bf4
> --- /dev/null
> +++ b/drivers/pps/clients/pps-irq.c
> @@ -0,0 +1,169 @@
> +/*
> + * pps-irq.c -- PPS IRQ client
> + *
> + *
> + * Copyright (C) 2010 Ricardo Martins <[email protected]>
> + * Copyright (C) 2011 James Nuss <[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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/pps_kernel.h>
> +#include <linux/gpio.h>
> +#include <linux/list.h>
> +
> +#define PPS_IRQ_NAME "pps-irq"
> +#define PPS_IRQ_VERSION "1.1.0"
> +
> +/* Info for each registered platform device */
> +struct pps_irq_data {
> + int irq; /* IRQ used as PPS source */
> + struct pps_device *pps; /* PPS source device */
> + struct pps_source_info info; /* PPS source information */
> +};
> +
> +/*
> + * Report the PPS event
> + */
> +
> +static irqreturn_t pps_irq_irq_handler(int irq, void *data)
> +{
> + struct pps_irq_data *info;
> + struct pps_event_time ts;
> +
> + /* Get the time stamp first */
> + pps_get_ts(&ts);
> +
> + info = (struct pps_irq_data *)data;
> + pps_event(info->pps, &ts, PPS_CAPTUREASSERT, NULL);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int pps_irq_probe(struct platform_device *pdev)
> +{
> + struct pps_irq_data *data;
> + struct resource *res;
> + int irq;
> + int ret;
> +
> + /* Validate resource. */
> + if (pdev->num_resources != 1) {
> + pr_err(PPS_IRQ_NAME ": this driver handles only one IRQ resource");

Shouldn't the format string in pr_*/dev_* call be ended with a '\n' or I
missed something?
Also I suggest you what I was once suggested: to define pr_fmt() macro
with a module name as in drivers/pps/clients/pps_parport.c to omit
it's use in every pr_* call. :)

> + return -EINVAL;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (res == NULL) {
> + pr_err(PPS_IRQ_NAME ": no IRQ resource was given");
> + return -EINVAL;
> + }
> +
> + if (!(res->flags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> + pr_err(PPS_IRQ_NAME ": given IRQ resource must be edge triggered");
> + return -EINVAL;
> + }

I think it doesn't actually expect that both flags are set because it
always treats it as assert in the irq handler. What does your signal
look like?

> + /* Allocate space for device info */
> + data = kzalloc(sizeof(struct pps_irq_data), GFP_KERNEL);
> + if (data == NULL)
> + return -ENOMEM;
> +
> + /* Initialize PPS specific parts of the bookkeeping data structure. */
> + data->info.mode = PPS_CAPTUREASSERT | PPS_OFFSETASSERT |
> + PPS_ECHOASSERT | PPS_CANWAIT | PPS_TSFMT_TSPEC;
> + data->info.owner = THIS_MODULE;
> + snprintf(data->info.name, PPS_MAX_NAME_LEN - 1, "%s.%d",
> + pdev->name, pdev->id);
> +
> + /* Register PPS source. */
> + irq = platform_get_irq(pdev, 0);
> + pr_info(PPS_IRQ_NAME ": registering IRQ %d as PPS source", irq);
> + data->pps = pps_register_source(&data->info, PPS_CAPTUREASSERT
> + | PPS_OFFSETASSERT);
> + if (data->pps == NULL) {
> + kfree(data);
> + pr_err(PPS_IRQ_NAME ": failed to register IRQ %d as PPS source",
> + irq);
> + return -1;
> + }
> +
> + /* Request IRQ. */
> + pr_info(PPS_IRQ_NAME ": requesting IRQ %d", irq);
> + ret = request_irq(irq, pps_irq_irq_handler,
> + res->flags & IRQF_TRIGGER_MASK,
> + data->info.name, data);
> + if (ret) {
> + pps_unregister_source(data->pps);
> + kfree(data);
> + pr_err(PPS_IRQ_NAME ": failed to acquire IRQ %d\n", irq);
> + return ret;
> + }
> + data->irq = irq;
> +
> + platform_set_drvdata(pdev, data);
> + dev_info(data->pps->dev, "Registered IRQ %d as PPS source", data->irq);
> +
> + return 0;
> +}
> +
> +static int pps_irq_remove(struct platform_device *pdev)
> +{
> + struct pps_irq_data *data = platform_get_drvdata(pdev);
> + platform_set_drvdata(pdev, NULL);
> + free_irq(data->irq, data);
> + pps_unregister_source(data->pps);
> + pr_info(PPS_IRQ_NAME ": removed IRQ %d as PPS source", data->irq);
> + kfree(data);
> + return 0;
> +}
> +
> +static struct platform_driver pps_irq_driver = {
> + .probe = pps_irq_probe,
> + .remove = __devexit_p(pps_irq_remove),
> + .driver = {
> + .name = PPS_IRQ_NAME,
> + .owner = THIS_MODULE
> + },
> +};
> +
> +static int __init pps_irq_init(void)
> +{
> + int ret = platform_driver_register(&pps_irq_driver);
> + if (ret < 0)
> + pr_err(PPS_IRQ_NAME ": failed to register platform driver");
> + return ret;
> +}
> +
> +static void __exit pps_irq_exit(void)
> +{
> + platform_driver_unregister(&pps_irq_driver);
> + pr_debug(PPS_IRQ_NAME ": unregistered platform driver");
> +}
> +
> +module_init(pps_irq_init);
> +module_exit(pps_irq_exit);
> +
> +MODULE_AUTHOR("Ricardo Martins <[email protected]>");
> +MODULE_AUTHOR("James Nuss <[email protected]>");
> +MODULE_DESCRIPTION("Use raw IRQs as PPS source");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(PPS_IRQ_VERSION);


--
Alexander


Attachments:
signature.asc (490.00 B)

2011-04-28 20:04:04

by James Nuss

[permalink] [raw]
Subject: Re: [LinuxPPS] [PATCH 2/2] pps: new client driver using IRQs

Hi Alexander,

Thanks for reviewing the code.

On Thu, Apr 28, 2011 at 7:22 AM, Alexander Gordeev
<[email protected]> wrote:
> Hi James,
>
> В Wed, 27 Apr 2011 14:14:14 -0400
> James Nuss <[email protected]> пишет:
>
>> This client driver allows you to use raw IRQs as a source for PPS
>> signals.
>>
>> This driver is based on the work by Ricardo Martins <[email protected]> who
>> submitted an initial implementation [1] of a PPS IRQ client driver to
>> the linuxpps mailing-list on Dec 3 2010.
>>
>> [1] http://ml.enneenne.com/pipermail/linuxpps/2010-December/004155.html
>>
>> Signed-off-by: James Nuss <[email protected]>
>> Reviewed-by: Ben Gardiner <[email protected]>
>> CC: Ricardo Martins <[email protected]>
>> ---
>> NOTE: drivers/pps/clients/Kconfig does not have the obligatory statements
>> regarding building the drivers as modules, even though they are all tristates.
>> This was purposefully omitted from the new config item for consistency.
>> This could be addressed in a separate patch.
>>
>>  drivers/pps/clients/Kconfig   |    8 ++
>>  drivers/pps/clients/Makefile  |    1 +
>>  drivers/pps/clients/pps-irq.c |  169 +++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 178 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/pps/clients/pps-irq.c
>>
>> diff --git a/drivers/pps/clients/Kconfig b/drivers/pps/clients/Kconfig
>> index 8520a7f..94b2cc6 100644
>> --- a/drivers/pps/clients/Kconfig
>> +++ b/drivers/pps/clients/Kconfig
>> @@ -29,4 +29,12 @@ config PPS_CLIENT_PARPORT
>>         If you say yes here you get support for a PPS source connected
>>         with the interrupt pin of your parallel port.
>>
>> +config PPS_CLIENT_IRQ
>> +     tristate "PPS client based on raw IRQs"
>> +     depends on PPS
>> +     help
>> +       If you say yes here you get support for a PPS source based
>> +       on raw IRQs. To be useful, you must also register a platform
>> +       device with an IRQ resource, usually in your board setup.
>> +
>>  endif
>> diff --git a/drivers/pps/clients/Makefile b/drivers/pps/clients/Makefile
>> index 42517da..609a332 100644
>> --- a/drivers/pps/clients/Makefile
>> +++ b/drivers/pps/clients/Makefile
>> @@ -5,6 +5,7 @@
>>  obj-$(CONFIG_PPS_CLIENT_KTIMER)      += pps-ktimer.o
>>  obj-$(CONFIG_PPS_CLIENT_LDISC)       += pps-ldisc.o
>>  obj-$(CONFIG_PPS_CLIENT_PARPORT) += pps_parport.o
>> +obj-$(CONFIG_PPS_CLIENT_IRQ) += pps-irq.o
>>
>>  ifeq ($(CONFIG_PPS_DEBUG),y)
>>  EXTRA_CFLAGS += -DDEBUG
>> diff --git a/drivers/pps/clients/pps-irq.c b/drivers/pps/clients/pps-irq.c
>> new file mode 100644
>> index 0000000..33c1bf4
>> --- /dev/null
>> +++ b/drivers/pps/clients/pps-irq.c
>> @@ -0,0 +1,169 @@
>> +/*
>> + * pps-irq.c -- PPS IRQ client
>> + *
>> + *
>> + * Copyright (C) 2010 Ricardo Martins <[email protected]>
>> + * Copyright (C) 2011 James Nuss <[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.
>> + *
>> + *   You should have received a copy of the GNU General Public License
>> + *   along with this program; if not, write to the Free Software
>> + *   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/pps_kernel.h>
>> +#include <linux/gpio.h>
>> +#include <linux/list.h>
>> +
>> +#define PPS_IRQ_NAME            "pps-irq"
>> +#define PPS_IRQ_VERSION         "1.1.0"
>> +
>> +/* Info for each registered platform device */
>> +struct pps_irq_data {
>> +     int irq;                        /* IRQ used as PPS source */
>> +     struct pps_device *pps;         /* PPS source device */
>> +     struct pps_source_info info;    /* PPS source information */
>> +};
>> +
>> +/*
>> + * Report the PPS event
>> + */
>> +
>> +static irqreturn_t pps_irq_irq_handler(int irq, void *data)
>> +{
>> +     struct pps_irq_data *info;
>> +     struct pps_event_time ts;
>> +
>> +     /* Get the time stamp first */
>> +     pps_get_ts(&ts);
>> +
>> +     info = (struct pps_irq_data *)data;
>> +     pps_event(info->pps, &ts, PPS_CAPTUREASSERT, NULL);
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static int pps_irq_probe(struct platform_device *pdev)
>> +{
>> +     struct pps_irq_data *data;
>> +     struct resource *res;
>> +     int irq;
>> +     int ret;
>> +
>> +     /* Validate resource. */
>> +     if (pdev->num_resources != 1) {
>> +             pr_err(PPS_IRQ_NAME ": this driver handles only one IRQ resource");
>
> Shouldn't the format string in pr_*/dev_* call be ended with a '\n' or I
> missed something?
> Also I suggest you what I was once suggested: to define pr_fmt() macro
> with a module name as in drivers/pps/clients/pps_parport.c to omit
> it's use in every pr_* call. :)

You are right with the '\n'. My mistake.
Good suggestion on the pr_fmt() macro - that's much cleaner.

>
>> +             return -EINVAL;
>> +     }
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> +     if (res == NULL) {
>> +             pr_err(PPS_IRQ_NAME ": no IRQ resource was given");
>> +             return -EINVAL;
>> +     }
>> +
>> +     if (!(res->flags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>> +             pr_err(PPS_IRQ_NAME ": given IRQ resource must be edge triggered");
>> +             return -EINVAL;
>> +     }
>
> I think it doesn't actually expect that both flags are set because it
> always treats it as assert in the irq handler. What does your signal
> look like?

The conditional logic is that one of either IRQF_TRIGGER_RISING or
IRQF_TRIGGER_FALLING must be set. It doesn't make much sense to have
neither set for PPS signals.
My intention is that the driver is generic enough so you can register
an IRQ resource with either IRQF_TRIGGER_RISING or
IRQF_TRIGGER_FALLING and you will get and assert event for that edge.
Clear events are not generated as you suggest but I believe this is
OK.
My signal is a simple low-to-high transition indicating the PPS. But I
believe you could register a device using this driver referencing the
other edge if required.

>
>> +     /* Allocate space for device info */
>> +     data = kzalloc(sizeof(struct pps_irq_data), GFP_KERNEL);
>> +     if (data == NULL)
>> +             return -ENOMEM;
>> +
>> +     /* Initialize PPS specific parts of the bookkeeping data structure. */
>> +     data->info.mode = PPS_CAPTUREASSERT | PPS_OFFSETASSERT |
>> +                     PPS_ECHOASSERT | PPS_CANWAIT | PPS_TSFMT_TSPEC;
>> +     data->info.owner = THIS_MODULE;
>> +     snprintf(data->info.name, PPS_MAX_NAME_LEN - 1, "%s.%d",
>> +              pdev->name, pdev->id);
>> +
>> +     /* Register PPS source. */
>> +     irq = platform_get_irq(pdev, 0);
>> +     pr_info(PPS_IRQ_NAME ": registering IRQ %d as PPS source", irq);
>> +     data->pps = pps_register_source(&data->info, PPS_CAPTUREASSERT
>> +                                     | PPS_OFFSETASSERT);
>> +     if (data->pps == NULL) {
>> +             kfree(data);
>> +             pr_err(PPS_IRQ_NAME ": failed to register IRQ %d as PPS source",
>> +                             irq);
>> +             return -1;
>> +     }
>> +
>> +     /* Request IRQ. */
>> +     pr_info(PPS_IRQ_NAME ": requesting IRQ %d", irq);
>> +     ret = request_irq(irq, pps_irq_irq_handler,
>> +                      res->flags & IRQF_TRIGGER_MASK,
>> +                      data->info.name, data);
>> +     if (ret) {
>> +             pps_unregister_source(data->pps);
>> +             kfree(data);
>> +             pr_err(PPS_IRQ_NAME ": failed to acquire IRQ %d\n", irq);
>> +             return ret;
>> +     }
>> +     data->irq = irq;
>> +
>> +     platform_set_drvdata(pdev, data);
>> +     dev_info(data->pps->dev, "Registered IRQ %d as PPS source", data->irq);
>> +
>> +     return 0;
>> +}
>> +
>> +static int pps_irq_remove(struct platform_device *pdev)
>> +{
>> +     struct pps_irq_data *data = platform_get_drvdata(pdev);
>> +     platform_set_drvdata(pdev, NULL);
>> +     free_irq(data->irq, data);
>> +     pps_unregister_source(data->pps);
>> +     pr_info(PPS_IRQ_NAME ": removed IRQ %d as PPS source", data->irq);
>> +     kfree(data);
>> +     return 0;
>> +}
>> +
>> +static struct platform_driver pps_irq_driver = {
>> +     .probe          = pps_irq_probe,
>> +     .remove         =  __devexit_p(pps_irq_remove),
>> +     .driver         = {
>> +             .name   = PPS_IRQ_NAME,
>> +             .owner  = THIS_MODULE
>> +     },
>> +};
>> +
>> +static int __init pps_irq_init(void)
>> +{
>> +     int ret = platform_driver_register(&pps_irq_driver);
>> +     if (ret < 0)
>> +             pr_err(PPS_IRQ_NAME ": failed to register platform driver");
>> +     return ret;
>> +}
>> +
>> +static void __exit pps_irq_exit(void)
>> +{
>> +     platform_driver_unregister(&pps_irq_driver);
>> +     pr_debug(PPS_IRQ_NAME ": unregistered platform driver");
>> +}
>> +
>> +module_init(pps_irq_init);
>> +module_exit(pps_irq_exit);
>> +
>> +MODULE_AUTHOR("Ricardo Martins <[email protected]>");
>> +MODULE_AUTHOR("James Nuss <[email protected]>");
>> +MODULE_DESCRIPTION("Use raw IRQs as PPS source");
>> +MODULE_LICENSE("GPL");
>> +MODULE_VERSION(PPS_IRQ_VERSION);
>
>
> --
>  Alexander
>

2011-04-28 20:55:39

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [LinuxPPS] [PATCH 2/2] pps: new client driver using IRQs

В Thu, 28 Apr 2011 16:03:59 -0400
James Nuss <[email protected]> пишет:

> Hi Alexander,
>
> Thanks for reviewing the code.
>
> On Thu, Apr 28, 2011 at 7:22 AM, Alexander Gordeev
> <[email protected]> wrote:
> > Hi James,
> >
> > В Wed, 27 Apr 2011 14:14:14 -0400
> > James Nuss <[email protected]> пишет:
> >
> >> This client driver allows you to use raw IRQs as a source for PPS
> >> signals.
> >>
> >> This driver is based on the work by Ricardo Martins <[email protected]> who
> >> submitted an initial implementation [1] of a PPS IRQ client driver to
> >> the linuxpps mailing-list on Dec 3 2010.
> >>
> >> [1] http://ml.enneenne.com/pipermail/linuxpps/2010-December/004155.html
> >>
> >> Signed-off-by: James Nuss <[email protected]>
> >> Reviewed-by: Ben Gardiner <[email protected]>
> >> CC: Ricardo Martins <[email protected]>
> >> ---
> >> NOTE: drivers/pps/clients/Kconfig does not have the obligatory statements
> >> regarding building the drivers as modules, even though they are all tristates.
> >> This was purposefully omitted from the new config item for consistency.
> >> This could be addressed in a separate patch.
> >>
> >>  drivers/pps/clients/Kconfig   |    8 ++
> >>  drivers/pps/clients/Makefile  |    1 +
> >>  drivers/pps/clients/pps-irq.c |  169 +++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 178 insertions(+), 0 deletions(-)
> >>  create mode 100644 drivers/pps/clients/pps-irq.c
> >>
> >> diff --git a/drivers/pps/clients/Kconfig b/drivers/pps/clients/Kconfig
> >> index 8520a7f..94b2cc6 100644
> >> --- a/drivers/pps/clients/Kconfig
> >> +++ b/drivers/pps/clients/Kconfig
> >> @@ -29,4 +29,12 @@ config PPS_CLIENT_PARPORT
> >>         If you say yes here you get support for a PPS source connected
> >>         with the interrupt pin of your parallel port.
> >>
> >> +config PPS_CLIENT_IRQ
> >> +     tristate "PPS client based on raw IRQs"
> >> +     depends on PPS
> >> +     help
> >> +       If you say yes here you get support for a PPS source based
> >> +       on raw IRQs. To be useful, you must also register a platform
> >> +       device with an IRQ resource, usually in your board setup.
> >> +
> >>  endif
> >> diff --git a/drivers/pps/clients/Makefile b/drivers/pps/clients/Makefile
> >> index 42517da..609a332 100644
> >> --- a/drivers/pps/clients/Makefile
> >> +++ b/drivers/pps/clients/Makefile
> >> @@ -5,6 +5,7 @@
> >>  obj-$(CONFIG_PPS_CLIENT_KTIMER)      += pps-ktimer.o
> >>  obj-$(CONFIG_PPS_CLIENT_LDISC)       += pps-ldisc.o
> >>  obj-$(CONFIG_PPS_CLIENT_PARPORT) += pps_parport.o
> >> +obj-$(CONFIG_PPS_CLIENT_IRQ) += pps-irq.o
> >>
> >>  ifeq ($(CONFIG_PPS_DEBUG),y)
> >>  EXTRA_CFLAGS += -DDEBUG
> >> diff --git a/drivers/pps/clients/pps-irq.c b/drivers/pps/clients/pps-irq.c
> >> new file mode 100644
> >> index 0000000..33c1bf4
> >> --- /dev/null
> >> +++ b/drivers/pps/clients/pps-irq.c
> >> @@ -0,0 +1,169 @@
> >> +/*
> >> + * pps-irq.c -- PPS IRQ client
> >> + *
> >> + *
> >> + * Copyright (C) 2010 Ricardo Martins <[email protected]>
> >> + * Copyright (C) 2011 James Nuss <[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.
> >> + *
> >> + *   You should have received a copy of the GNU General Public License
> >> + *   along with this program; if not, write to the Free Software
> >> + *   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> >> + */
> >> +
> >> +#include <linux/init.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/module.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/pps_kernel.h>
> >> +#include <linux/gpio.h>
> >> +#include <linux/list.h>
> >> +
> >> +#define PPS_IRQ_NAME            "pps-irq"
> >> +#define PPS_IRQ_VERSION         "1.1.0"
> >> +
> >> +/* Info for each registered platform device */
> >> +struct pps_irq_data {
> >> +     int irq;                        /* IRQ used as PPS source */
> >> +     struct pps_device *pps;         /* PPS source device */
> >> +     struct pps_source_info info;    /* PPS source information */
> >> +};
> >> +
> >> +/*
> >> + * Report the PPS event
> >> + */
> >> +
> >> +static irqreturn_t pps_irq_irq_handler(int irq, void *data)
> >> +{
> >> +     struct pps_irq_data *info;
> >> +     struct pps_event_time ts;
> >> +
> >> +     /* Get the time stamp first */
> >> +     pps_get_ts(&ts);
> >> +
> >> +     info = (struct pps_irq_data *)data;
> >> +     pps_event(info->pps, &ts, PPS_CAPTUREASSERT, NULL);
> >> +
> >> +     return IRQ_HANDLED;
> >> +}
> >> +
> >> +static int pps_irq_probe(struct platform_device *pdev)
> >> +{
> >> +     struct pps_irq_data *data;
> >> +     struct resource *res;
> >> +     int irq;
> >> +     int ret;
> >> +
> >> +     /* Validate resource. */
> >> +     if (pdev->num_resources != 1) {
> >> +             pr_err(PPS_IRQ_NAME ": this driver handles only one IRQ resource");
> >
> > Shouldn't the format string in pr_*/dev_* call be ended with a '\n' or I
> > missed something?
> > Also I suggest you what I was once suggested: to define pr_fmt() macro
> > with a module name as in drivers/pps/clients/pps_parport.c to omit
> > it's use in every pr_* call. :)
>
> You are right with the '\n'. My mistake.
> Good suggestion on the pr_fmt() macro - that's much cleaner.

Ok, good.

> >
> >> +             return -EINVAL;
> >> +     }
> >> +
> >> +     res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> >> +     if (res == NULL) {
> >> +             pr_err(PPS_IRQ_NAME ": no IRQ resource was given");
> >> +             return -EINVAL;
> >> +     }
> >> +
> >> +     if (!(res->flags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> >> +             pr_err(PPS_IRQ_NAME ": given IRQ resource must be edge triggered");
> >> +             return -EINVAL;
> >> +     }
> >
> > I think it doesn't actually expect that both flags are set because it
> > always treats it as assert in the irq handler. What does your signal
> > look like?
>
> The conditional logic is that one of either IRQF_TRIGGER_RISING or
> IRQF_TRIGGER_FALLING must be set. It doesn't make much sense to have
> neither set for PPS signals.
> My intention is that the driver is generic enough so you can register
> an IRQ resource with either IRQF_TRIGGER_RISING or
> IRQF_TRIGGER_FALLING and you will get and assert event for that edge.
> Clear events are not generated as you suggest but I believe this is
> OK.
> My signal is a simple low-to-high transition indicating the PPS. But I
> believe you could register a device using this driver referencing the
> other edge if required.

Ok, but is there a way one can register an IRQ resource with both flags
set? If yes, then it would be nice to have a stricter check here to
prevent two situations:
1. none flag is set (it is already in place)
2. both flags are set

The latter will definitely mess things up, right?

> >
> >> +     /* Allocate space for device info */
> >> +     data = kzalloc(sizeof(struct pps_irq_data), GFP_KERNEL);
> >> +     if (data == NULL)
> >> +             return -ENOMEM;
> >> +
> >> +     /* Initialize PPS specific parts of the bookkeeping data structure. */
> >> +     data->info.mode = PPS_CAPTUREASSERT | PPS_OFFSETASSERT |
> >> +                     PPS_ECHOASSERT | PPS_CANWAIT | PPS_TSFMT_TSPEC;
> >> +     data->info.owner = THIS_MODULE;
> >> +     snprintf(data->info.name, PPS_MAX_NAME_LEN - 1, "%s.%d",
> >> +              pdev->name, pdev->id);
> >> +
> >> +     /* Register PPS source. */
> >> +     irq = platform_get_irq(pdev, 0);
> >> +     pr_info(PPS_IRQ_NAME ": registering IRQ %d as PPS source", irq);
> >> +     data->pps = pps_register_source(&data->info, PPS_CAPTUREASSERT
> >> +                                     | PPS_OFFSETASSERT);
> >> +     if (data->pps == NULL) {
> >> +             kfree(data);
> >> +             pr_err(PPS_IRQ_NAME ": failed to register IRQ %d as PPS source",
> >> +                             irq);
> >> +             return -1;
> >> +     }
> >> +
> >> +     /* Request IRQ. */
> >> +     pr_info(PPS_IRQ_NAME ": requesting IRQ %d", irq);
> >> +     ret = request_irq(irq, pps_irq_irq_handler,
> >> +                      res->flags & IRQF_TRIGGER_MASK,
> >> +                      data->info.name, data);
> >> +     if (ret) {
> >> +             pps_unregister_source(data->pps);
> >> +             kfree(data);
> >> +             pr_err(PPS_IRQ_NAME ": failed to acquire IRQ %d\n", irq);
> >> +             return ret;
> >> +     }
> >> +     data->irq = irq;
> >> +
> >> +     platform_set_drvdata(pdev, data);
> >> +     dev_info(data->pps->dev, "Registered IRQ %d as PPS source", data->irq);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static int pps_irq_remove(struct platform_device *pdev)
> >> +{
> >> +     struct pps_irq_data *data = platform_get_drvdata(pdev);
> >> +     platform_set_drvdata(pdev, NULL);
> >> +     free_irq(data->irq, data);
> >> +     pps_unregister_source(data->pps);
> >> +     pr_info(PPS_IRQ_NAME ": removed IRQ %d as PPS source", data->irq);
> >> +     kfree(data);
> >> +     return 0;
> >> +}
> >> +
> >> +static struct platform_driver pps_irq_driver = {
> >> +     .probe          = pps_irq_probe,
> >> +     .remove         =  __devexit_p(pps_irq_remove),
> >> +     .driver         = {
> >> +             .name   = PPS_IRQ_NAME,
> >> +             .owner  = THIS_MODULE
> >> +     },
> >> +};
> >> +
> >> +static int __init pps_irq_init(void)
> >> +{
> >> +     int ret = platform_driver_register(&pps_irq_driver);
> >> +     if (ret < 0)
> >> +             pr_err(PPS_IRQ_NAME ": failed to register platform driver");
> >> +     return ret;
> >> +}
> >> +
> >> +static void __exit pps_irq_exit(void)
> >> +{
> >> +     platform_driver_unregister(&pps_irq_driver);
> >> +     pr_debug(PPS_IRQ_NAME ": unregistered platform driver");
> >> +}
> >> +
> >> +module_init(pps_irq_init);
> >> +module_exit(pps_irq_exit);
> >> +
> >> +MODULE_AUTHOR("Ricardo Martins <[email protected]>");
> >> +MODULE_AUTHOR("James Nuss <[email protected]>");
> >> +MODULE_DESCRIPTION("Use raw IRQs as PPS source");
> >> +MODULE_LICENSE("GPL");
> >> +MODULE_VERSION(PPS_IRQ_VERSION);
> >
> >
> > --
> >  Alexander
> >


--
Alexander


Attachments:
signature.asc (490.00 B)

2011-04-28 21:28:02

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [LinuxPPS] [PATCH 2/2] pps: new client driver using IRQs

В Fri, 29 Apr 2011 00:55:24 +0400
Alexander Gordeev <[email protected]> пишет:

> В Thu, 28 Apr 2011 16:03:59 -0400
> James Nuss <[email protected]> пишет:
>
> > Hi Alexander,
> >
> > Thanks for reviewing the code.
> >
> > On Thu, Apr 28, 2011 at 7:22 AM, Alexander Gordeev
> > <[email protected]> wrote:
> > > Hi James,
> > >
> > > В Wed, 27 Apr 2011 14:14:14 -0400
> > > James Nuss <[email protected]> пишет:
> > >
> > >> +             return -EINVAL;
> > >> +     }
> > >> +
> > >> +     res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > >> +     if (res == NULL) {
> > >> +             pr_err(PPS_IRQ_NAME ": no IRQ resource was given");
> > >> +             return -EINVAL;
> > >> +     }
> > >> +
> > >> +     if (!(res->flags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> > >> +             pr_err(PPS_IRQ_NAME ": given IRQ resource must be edge triggered");
> > >> +             return -EINVAL;
> > >> +     }
> > >
> > > I think it doesn't actually expect that both flags are set because it
> > > always treats it as assert in the irq handler. What does your signal
> > > look like?
> >
> > The conditional logic is that one of either IRQF_TRIGGER_RISING or
> > IRQF_TRIGGER_FALLING must be set. It doesn't make much sense to have
> > neither set for PPS signals.
> > My intention is that the driver is generic enough so you can register
> > an IRQ resource with either IRQF_TRIGGER_RISING or
> > IRQF_TRIGGER_FALLING and you will get and assert event for that edge.
> > Clear events are not generated as you suggest but I believe this is
> > OK.
> > My signal is a simple low-to-high transition indicating the PPS. But I
> > believe you could register a device using this driver referencing the
> > other edge if required.
>
> Ok, but is there a way one can register an IRQ resource with both flags
> set? If yes, then it would be nice to have a stricter check here to
> prevent two situations:
> 1. none flag is set (it is already in place)
> 2. both flags are set
>
> The latter will definitely mess things up, right?

I mean, one surely can register an IRQ resource with both flags set. And
if the underlying hardware works as it is described (i.e. raises an irq
on both edges) then it will be a problem.

--
Alexander


Attachments:
signature.asc (490.00 B)

2011-04-29 04:31:20

by Igor Plyatov

[permalink] [raw]
Subject: Re: [LinuxPPS] [PATCH 2/2] pps: new client driver using IRQs

Hi!

> В Fri, 29 Apr 2011 00:55:24 +0400
> Alexander Gordeev<[email protected]> пишет:
>
>> В Thu, 28 Apr 2011 16:03:59 -0400
>> James Nuss<[email protected]> пишет:
>>
>>> Hi Alexander,
>>>
>>> Thanks for reviewing the code.
>>>
>>> On Thu, Apr 28, 2011 at 7:22 AM, Alexander Gordeev
>>> <[email protected]> wrote:
>>>> Hi James,
>>>>
>>>> В Wed, 27 Apr 2011 14:14:14 -0400
>>>> James Nuss<[email protected]> пишет:
>>>>
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>>>> + if (res == NULL) {
>>>>> + pr_err(PPS_IRQ_NAME ": no IRQ resource was given");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + if (!(res->flags& (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>>>>> + pr_err(PPS_IRQ_NAME ": given IRQ resource must be edge triggered");
>>>>> + return -EINVAL;
>>>>> + }
>>>> I think it doesn't actually expect that both flags are set because it
>>>> always treats it as assert in the irq handler. What does your signal
>>>> look like?
>>> The conditional logic is that one of either IRQF_TRIGGER_RISING or
>>> IRQF_TRIGGER_FALLING must be set. It doesn't make much sense to have
>>> neither set for PPS signals.
>>> My intention is that the driver is generic enough so you can register
>>> an IRQ resource with either IRQF_TRIGGER_RISING or
>>> IRQF_TRIGGER_FALLING and you will get and assert event for that edge.
>>> Clear events are not generated as you suggest but I believe this is
>>> OK.
>>> My signal is a simple low-to-high transition indicating the PPS. But I
>>> believe you could register a device using this driver referencing the
>>> other edge if required.
>> Ok, but is there a way one can register an IRQ resource with both flags
>> set? If yes, then it would be nice to have a stricter check here to
>> prevent two situations:
>> 1. none flag is set (it is already in place)
>> 2. both flags are set
>>
>> The latter will definitely mess things up, right?
> I mean, one surely can register an IRQ resource with both flags set. And
> if the underlying hardware works as it is described (i.e. raises an irq
> on both edges) then it will be a problem.

Please don't try to abandon one of ASSERT or CLEAR events!
It is very useful to register both of them, because in this case its
possible to measure pulse width and decode PPS signals like DCF77.

I write similar driver (pps-client-gpio.c) for Linux-2.6.36 with helper
drivers (virtual-gps.c and pps-decoders.c) which allows to receive
GeoSIG Ltd. T1PPS (DCF77 alike) time signal, register both events, then
decode them and provide a virtual GPS (NMEA time source) device.

These drivers operates very well together with standard NTP server.

>
> _______________________________________________
> LinuxPPS mailing list
> [email protected]
> http://ml.enneenne.com/cgi-bin/mailman/listinfo/linuxpps
> Wiki:http://wiki.enneenne.com/index.php/LinuxPPS_support

Best regards!

--
Igor Plyatov

2011-04-29 08:15:59

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [LinuxPPS] [PATCH 2/2] pps: new client driver using IRQs

On Thu, Apr 28, 2011 at 04:03:59PM -0400, James Nuss wrote:
> >> +
> >> + ? ? if (!(res->flags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> >> + ? ? ? ? ? ? pr_err(PPS_IRQ_NAME ": given IRQ resource must be edge triggered");
> >> + ? ? ? ? ? ? return -EINVAL;
> >> + ? ? }
> >
> > I think it doesn't actually expect that both flags are set because it
> > always treats it as assert in the irq handler. What does your signal
> > look like?
>
> The conditional logic is that one of either IRQF_TRIGGER_RISING or
> IRQF_TRIGGER_FALLING must be set. It doesn't make much sense to have
> neither set for PPS signals.
> My intention is that the driver is generic enough so you can register
> an IRQ resource with either IRQF_TRIGGER_RISING or
> IRQF_TRIGGER_FALLING and you will get and assert event for that edge.
> Clear events are not generated as you suggest but I believe this is
> OK.
> My signal is a simple low-to-high transition indicating the PPS. But I
> believe you could register a device using this driver referencing the
> other edge if required.

The driver is ok for me but if you say that your ?intention is that
the driver is generic enough? you should consider adding CLEAR events
since some GPIOs controller can manager IRQs for both edges... ;)

Ciao,

Rodolfo

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it