changes v6:
- rename it to interrupt-counter
- driver fixes
- device tree fixes
changes v5:
- rename it to event counter, since it support different event sources
- make it work with gpio-only or irq-only configuration
- update yaml binding
changes v4:
- use IRQ_NOAUTOEN to not enable IRQ by default
- rename gpio_ from name pattern and make this driver work any IRQ
source.
changes v3:
- convert counter to atomic_t
changes v2:
- add commas
- avoid possible unhandled interrupts in the enable path
- do not use of_ specific gpio functions
Add support for GPIO based pulse counter. For now it can only count
pulses. With counter char device support, we will be able to attach
timestamps and measure actual pulse frequency.
Never the less, it is better to mainline this driver now (before chardev
patches go mainline), to provide developers additional use case for the counter
framework with chardev support.
Oleksij Rempel (2):
dt-bindings: counter: add event-counter binding
counter: add IRQ or GPIO based event counter
.../bindings/counter/interrupt-counter.yaml | 62 +++++
MAINTAINERS | 7 +
drivers/counter/Kconfig | 10 +
drivers/counter/Makefile | 1 +
drivers/counter/interrupt-cnt.c | 249 ++++++++++++++++++
5 files changed, 329 insertions(+)
create mode 100644 Documentation/devicetree/bindings/counter/interrupt-counter.yaml
create mode 100644 drivers/counter/interrupt-cnt.c
--
2.29.2
Add binding for the event counter node
Signed-off-by: Oleksij Rempel <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---
.../bindings/counter/interrupt-counter.yaml | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/counter/interrupt-counter.yaml
diff --git a/Documentation/devicetree/bindings/counter/interrupt-counter.yaml b/Documentation/devicetree/bindings/counter/interrupt-counter.yaml
new file mode 100644
index 000000000000..fd075d104631
--- /dev/null
+++ b/Documentation/devicetree/bindings/counter/interrupt-counter.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/counter/interrupt-counter.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Interrupt counter
+
+maintainers:
+ - Oleksij Rempel <[email protected]>
+
+description: |
+ A generic interrupt counter to measure interrupt frequency. It was developed
+ and used for agricultural devices to measure rotation speed of wheels or
+ other tools. Since the direction of rotation is not important, only one
+ signal line is needed.
+ Interrupts or gpios are required. If both are defined, the interrupt will
+ take precedence for counting interrupts.
+
+properties:
+ compatible:
+ const: interrupt-counter
+
+ interrupts:
+ maxItems: 1
+
+ gpios:
+ maxItems: 1
+
+required:
+ - compatible
+
+anyOf:
+ - required: [ interrupts-extended ]
+ - required: [ interrupts ]
+ - required: [ gpios ]
+
+additionalProperties: false
+
+examples:
+ - |
+
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ counter-0 {
+ compatible = "interrupt-counter";
+ interrupts-extended = <&gpio 0 IRQ_TYPE_EDGE_RISING>;
+ };
+
+ counter-1 {
+ compatible = "interrupt-counter";
+ gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+ };
+
+ counter-2 {
+ compatible = "interrupt-counter";
+ interrupts-extended = <&gpio 2 IRQ_TYPE_EDGE_RISING>;
+ gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+ };
+
+...
--
2.29.2
Add simple IRQ or GPIO base event counter. This device is used to measure
rotation speed of some agricultural devices, so no high frequency on the
counter pin is expected.
The maximal measurement frequency depends on the CPU and system load. On
the idle iMX6S I was able to measure up to 20kHz without count drops.
Signed-off-by: Oleksij Rempel <[email protected]>
Reviewed-by: Ahmad Fatoum <[email protected]>
---
MAINTAINERS | 7 +
drivers/counter/Kconfig | 10 ++
drivers/counter/Makefile | 1 +
drivers/counter/interrupt-cnt.c | 249 ++++++++++++++++++++++++++++++++
4 files changed, 267 insertions(+)
create mode 100644 drivers/counter/interrupt-cnt.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 00836f6452f0..9cf87af41d7d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9246,6 +9246,13 @@ F: include/dt-bindings/interconnect/
F: include/linux/interconnect-provider.h
F: include/linux/interconnect.h
+INTERRUPT COUNTER DRIVER
+M: Oleksij Rempel <[email protected]>
+R: Pengutronix Kernel Team <[email protected]>
+L: [email protected]
+F: Documentation/devicetree/bindings/counter/interrupt-counter.yaml
+F: drivers/counter/interrupt-cnt.c
+
INVENSENSE ICM-426xx IMU DRIVER
M: Jean-Baptiste Maneyrol <[email protected]>
L: [email protected]
diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
index 2de53ab0dd25..dcad13229134 100644
--- a/drivers/counter/Kconfig
+++ b/drivers/counter/Kconfig
@@ -29,6 +29,16 @@ config 104_QUAD_8
The base port addresses for the devices may be configured via the base
array module parameter.
+config INTERRUPT_CNT
+ tristate "Interrupt counter driver"
+ depends on GPIOLIB
+ help
+ Select this option to enable interrupt counter driver. Any interrupt
+ source can be used by this driver as the event source.
+
+ To compile this driver as a module, choose M here: the
+ module will be called interrupt-cnt.
+
config STM32_TIMER_CNT
tristate "STM32 Timer encoder counter driver"
depends on MFD_STM32_TIMERS || COMPILE_TEST
diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
index 0a393f71e481..cb646ed2f039 100644
--- a/drivers/counter/Makefile
+++ b/drivers/counter/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_COUNTER) += counter.o
obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o
+obj-$(CONFIG_INTERRUPT_CNT) += interrupt-cnt.o
obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o
obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o
obj-$(CONFIG_TI_EQEP) += ti-eqep.o
diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
new file mode 100644
index 000000000000..16669924cdfa
--- /dev/null
+++ b/drivers/counter/interrupt-cnt.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021 Pengutronix, Oleksij Rempel <[email protected]>
+ */
+
+#include <linux/counter.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define INTERRUPT_CNT_NAME "interrupt-cnt"
+
+struct interrupt_cnt_priv {
+ atomic_t count;
+ struct counter_device counter;
+ struct gpio_desc *gpio;
+ int irq;
+ bool enabled;
+};
+
+static irqreturn_t interrupt_cnt_isr(int irq, void *dev_id)
+{
+ struct interrupt_cnt_priv *priv = dev_id;
+
+ atomic_inc(&priv->count);
+
+ return IRQ_HANDLED;
+}
+
+static ssize_t interrupt_cnt_enable_read(struct counter_device *counter,
+ struct counter_count *count,
+ void *private, char *buf)
+{
+ struct interrupt_cnt_priv *priv = counter->priv;
+
+ return sysfs_emit(buf, "%d\n", priv->enabled);
+}
+
+static ssize_t interrupt_cnt_enable_write(struct counter_device *counter,
+ struct counter_count *count,
+ void *private, const char *buf,
+ size_t len)
+{
+ struct interrupt_cnt_priv *priv = counter->priv;
+ bool enable;
+ ssize_t ret;
+
+ ret = kstrtobool(buf, &enable);
+ if (ret)
+ return ret;
+
+ if (priv->enabled == enable)
+ return len;
+
+ if (enable) {
+ priv->enabled = true;
+ enable_irq(priv->irq);
+ } else {
+ disable_irq(priv->irq);
+ priv->enabled = false;
+ }
+
+ return len;
+}
+
+static const struct counter_count_ext interrupt_cnt_ext[] = {
+ {
+ .name = "enable",
+ .read = interrupt_cnt_enable_read,
+ .write = interrupt_cnt_enable_write,
+ },
+};
+
+static enum counter_synapse_action interrupt_cnt_synapse_actionss[] = {
+ COUNTER_SYNAPSE_ACTION_RISING_EDGE,
+};
+
+static int interrupt_cnt_action_get(struct counter_device *counter,
+ struct counter_count *count,
+ struct counter_synapse *synapse,
+ size_t *action)
+{
+ *action = interrupt_cnt_synapse_actionss[0];
+
+ return 0;
+}
+
+static int interrupt_cnt_read(struct counter_device *counter,
+ struct counter_count *count, unsigned long *val)
+{
+ struct interrupt_cnt_priv *priv = counter->priv;
+
+ *val = atomic_read(&priv->count);
+
+ return 0;
+}
+
+static int interrupt_cnt_write(struct counter_device *counter,
+ struct counter_count *count,
+ const unsigned long val)
+{
+ struct interrupt_cnt_priv *priv = counter->priv;
+
+ atomic_set(&priv->count, val);
+
+ return 0;
+}
+
+static enum counter_count_function interrupt_cnt_functions[] = {
+ COUNTER_COUNT_FUNCTION_INCREASE,
+};
+
+static int interrupt_cnt_function_get(struct counter_device *counter,
+ struct counter_count *count,
+ size_t *function)
+{
+ *function = interrupt_cnt_functions[0];
+
+ return 0;
+}
+
+static int interrupt_cnt_signal_read(struct counter_device *counter,
+ struct counter_signal *signal,
+ enum counter_signal_value *val)
+{
+ struct interrupt_cnt_priv *priv = counter->priv;
+ int ret;
+
+ ret = gpiod_get_value(priv->gpio);
+ if (ret < 0)
+ return ret;
+
+ *val = ret ? COUNTER_SIGNAL_HIGH : COUNTER_SIGNAL_LOW;
+
+ return 0;
+}
+
+static struct counter_signal interrupt_cnt_signals[] = {
+ {
+ .id = 0,
+ .name = "Channel 0, GPIO line state",
+ },
+};
+
+static struct counter_synapse interrupt_cnt_synapses[] = {
+ {
+ .actions_list = interrupt_cnt_synapse_actionss,
+ .num_actions = ARRAY_SIZE(interrupt_cnt_synapse_actionss),
+ .signal = &interrupt_cnt_signals[0]
+ },
+};
+
+static struct counter_count interrupt_cnts[] = {
+ {
+ .id = 0,
+ .name = "Channel 0 Count",
+ .functions_list = interrupt_cnt_functions,
+ .num_functions = ARRAY_SIZE(interrupt_cnt_functions),
+ .synapses = interrupt_cnt_synapses,
+ .num_synapses = ARRAY_SIZE(interrupt_cnt_synapses),
+ .ext = interrupt_cnt_ext,
+ .num_ext = ARRAY_SIZE(interrupt_cnt_ext),
+ },
+};
+
+static const struct counter_ops interrupt_cnt_ops = {
+ .action_get = interrupt_cnt_action_get,
+ .count_read = interrupt_cnt_read,
+ .count_write = interrupt_cnt_write,
+ .function_get = interrupt_cnt_function_get,
+ .signal_read = interrupt_cnt_signal_read,
+};
+
+static int interrupt_cnt_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct interrupt_cnt_priv *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->irq = platform_get_irq_optional(pdev, 0);
+ if (priv->irq == -ENXIO)
+ priv->irq = 0;
+ else if (priv->irq < 0)
+ return dev_err_probe(dev, priv->irq, "failed to get IRQ\n");
+
+ priv->gpio = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
+ if (IS_ERR(priv->gpio))
+ return dev_err_probe(dev, PTR_ERR(priv->gpio), "failed to get GPIO\n");
+
+ if (!priv->irq && !priv->gpio) {
+ dev_err(dev, "IRQ and GPIO are not found. At least one source should be provided\n");
+ return -ENODEV;
+ }
+
+ if (!priv->irq) {
+ int irq = gpiod_to_irq(priv->gpio);
+
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "failed to get IRQ from GPIO\n");
+
+ priv->irq = irq;
+ }
+
+ priv->counter.priv = priv;
+ priv->counter.name = dev_name(dev);
+ priv->counter.parent = dev;
+ priv->counter.ops = &interrupt_cnt_ops;
+ priv->counter.counts = interrupt_cnts;
+ priv->counter.num_counts = ARRAY_SIZE(interrupt_cnts);
+ priv->counter.signals = interrupt_cnt_signals;
+ priv->counter.num_signals = priv->gpio ?
+ ARRAY_SIZE(interrupt_cnt_signals) : 0;
+
+ irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
+ ret = devm_request_irq(dev, priv->irq, interrupt_cnt_isr,
+ IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
+ INTERRUPT_CNT_NAME, priv);
+ if (ret)
+ return ret;
+
+ return devm_counter_register(dev, &priv->counter);
+}
+
+static const struct of_device_id interrupt_cnt_of_match[] = {
+ { .compatible = "interrupt-counter", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, interrupt_cnt_of_match);
+
+static struct platform_driver interrupt_cnt_driver = {
+ .probe = interrupt_cnt_probe,
+ .driver = {
+ .name = INTERRUPT_CNT_NAME,
+ .of_match_table = interrupt_cnt_of_match,
+ },
+};
+module_platform_driver(interrupt_cnt_driver);
+
+MODULE_ALIAS("platform:interrupt-counter");
+MODULE_AUTHOR("Oleksij Rempel <[email protected]>");
+MODULE_DESCRIPTION("Interrupt counter driver");
+MODULE_LICENSE("GPL v2");
--
2.29.2
On Tue, 16 Feb 2021 09:13:56 +0100
Oleksij Rempel <[email protected]> wrote:
> Add simple IRQ or GPIO base event counter. This device is used to measure
> rotation speed of some agricultural devices, so no high frequency on the
> counter pin is expected.
>
> The maximal measurement frequency depends on the CPU and system load. On
> the idle iMX6S I was able to measure up to 20kHz without count drops.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> Reviewed-by: Ahmad Fatoum <[email protected]>
A FYI note inline.
Driver looks good to me.
Jonathan
> ---
> MAINTAINERS | 7 +
> drivers/counter/Kconfig | 10 ++
> drivers/counter/Makefile | 1 +
> drivers/counter/interrupt-cnt.c | 249 ++++++++++++++++++++++++++++++++
> 4 files changed, 267 insertions(+)
> create mode 100644 drivers/counter/interrupt-cnt.c
>
...
> +static int interrupt_cnt_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct interrupt_cnt_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->irq = platform_get_irq_optional(pdev, 0);
> + if (priv->irq == -ENXIO)
> + priv->irq = 0;
> + else if (priv->irq < 0)
> + return dev_err_probe(dev, priv->irq, "failed to get IRQ\n");
> +
> + priv->gpio = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
> + if (IS_ERR(priv->gpio))
> + return dev_err_probe(dev, PTR_ERR(priv->gpio), "failed to get GPIO\n");
> +
> + if (!priv->irq && !priv->gpio) {
> + dev_err(dev, "IRQ and GPIO are not found. At least one source should be provided\n");
> + return -ENODEV;
> + }
> +
> + if (!priv->irq) {
> + int irq = gpiod_to_irq(priv->gpio);
> +
> + if (irq < 0)
> + return dev_err_probe(dev, irq, "failed to get IRQ from GPIO\n");
> +
> + priv->irq = irq;
> + }
> +
> + priv->counter.priv = priv;
> + priv->counter.name = dev_name(dev);
> + priv->counter.parent = dev;
> + priv->counter.ops = &interrupt_cnt_ops;
> + priv->counter.counts = interrupt_cnts;
> + priv->counter.num_counts = ARRAY_SIZE(interrupt_cnts);
> + priv->counter.signals = interrupt_cnt_signals;
> + priv->counter.num_signals = priv->gpio ?
> + ARRAY_SIZE(interrupt_cnt_signals) : 0;
> +
> + irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
Just as a side note, there is a series that cleans up this case, though no
idea if it will make the merge window or not.
https://lore.kernel.org/linux-input/[email protected]/
If it does we can tidy this up then.
> + ret = devm_request_irq(dev, priv->irq, interrupt_cnt_isr,
> + IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
> + INTERRUPT_CNT_NAME, priv);
> + if (ret)
> + return ret;
> +
> + return devm_counter_register(dev, &priv->counter);
> +}
> +
...
On Tue, Feb 16, 2021 at 09:13:54AM +0100, Oleksij Rempel wrote:
> changes v6:
> - rename it to interrupt-counter
Hi Oleksij,
Sorry to nitpick again, I think "irq-counter" as Jonathan suggested in
an earlier review would be a better name afterall. Would you be able to
rename this driver to use that name instead?
Sincerely,
William Breathitt Gray
> - driver fixes
> - device tree fixes
>
> changes v5:
> - rename it to event counter, since it support different event sources
> - make it work with gpio-only or irq-only configuration
> - update yaml binding
>
> changes v4:
> - use IRQ_NOAUTOEN to not enable IRQ by default
> - rename gpio_ from name pattern and make this driver work any IRQ
> source.
>
> changes v3:
> - convert counter to atomic_t
>
> changes v2:
> - add commas
> - avoid possible unhandled interrupts in the enable path
> - do not use of_ specific gpio functions
>
> Add support for GPIO based pulse counter. For now it can only count
> pulses. With counter char device support, we will be able to attach
> timestamps and measure actual pulse frequency.
>
> Never the less, it is better to mainline this driver now (before chardev
> patches go mainline), to provide developers additional use case for the counter
> framework with chardev support.
>
> Oleksij Rempel (2):
> dt-bindings: counter: add event-counter binding
> counter: add IRQ or GPIO based event counter
>
> .../bindings/counter/interrupt-counter.yaml | 62 +++++
> MAINTAINERS | 7 +
> drivers/counter/Kconfig | 10 +
> drivers/counter/Makefile | 1 +
> drivers/counter/interrupt-cnt.c | 249 ++++++++++++++++++
> 5 files changed, 329 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/counter/interrupt-counter.yaml
> create mode 100644 drivers/counter/interrupt-cnt.c
>
> --
> 2.29.2
>
Hi William,
On Mon, Feb 22, 2021 at 10:48:56AM +0900, William Breathitt Gray wrote:
> On Tue, Feb 16, 2021 at 09:13:54AM +0100, Oleksij Rempel wrote:
> > changes v6:
> > - rename it to interrupt-counter
>
> Hi Oleksij,
>
> Sorry to nitpick again, I think "irq-counter" as Jonathan suggested in
> an earlier review would be a better name afterall. Would you be able to
> rename this driver to use that name instead?
I would prefer not to rename it. IRQ (Interrupt Request) is a signal
outside of the system. Below some frequency rate, amount of counted
ISR (interrupt service routine) calls or events would be equal to the actual
IRQs. If frequency is too high, we will count ISR, but not IRQs. In
any case, interrupt-counter is more or leas neutral, without triggering
too many wrong assumptions.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Feb 23, 2021 at 08:16:30AM +0100, Oleksij Rempel wrote:
> Hi William,
>
> On Mon, Feb 22, 2021 at 10:48:56AM +0900, William Breathitt Gray wrote:
> > On Tue, Feb 16, 2021 at 09:13:54AM +0100, Oleksij Rempel wrote:
> > > changes v6:
> > > - rename it to interrupt-counter
> >
> > Hi Oleksij,
> >
> > Sorry to nitpick again, I think "irq-counter" as Jonathan suggested in
> > an earlier review would be a better name afterall. Would you be able to
> > rename this driver to use that name instead?
>
> I would prefer not to rename it. IRQ (Interrupt Request) is a signal
> outside of the system. Below some frequency rate, amount of counted
> ISR (interrupt service routine) calls or events would be equal to the actual
> IRQs. If frequency is too high, we will count ISR, but not IRQs. In
> any case, interrupt-counter is more or leas neutral, without triggering
> too many wrong assumptions.
>
> Regards,
> Oleksij
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
I suppose "interrupt-counter" should be fine in this context then, so
perhaps a rename isn't really necessary afterall.
William Breathitt Gray
On Tue, 16 Feb 2021 09:13:55 +0100, Oleksij Rempel wrote:
> Add binding for the event counter node
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> Reviewed-by: Linus Walleij <[email protected]>
> ---
> .../bindings/counter/interrupt-counter.yaml | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/counter/interrupt-counter.yaml
>
Reviewed-by: Rob Herring <[email protected]>