Add simple IRQ or GPIO base pulse 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]>
---
drivers/counter/Kconfig | 10 ++
drivers/counter/Makefile | 1 +
drivers/counter/pulse-cnt.c | 235 ++++++++++++++++++++++++++++++++++++
3 files changed, 246 insertions(+)
create mode 100644 drivers/counter/pulse-cnt.c
diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
index 2de53ab0dd25..73107a861648 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 PULSE_CNT
+ tristate "Pulse counter driver"
+ depends on GPIOLIB
+ help
+ Select this option to enable pulse counter driver. Any interrupt source
+ can be used by this driver as the pulse source.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-pulse-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..73999e39e984 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_PULSE_CNT) += pulse-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/pulse-cnt.c b/drivers/counter/pulse-cnt.c
new file mode 100644
index 000000000000..4128b4b3661e
--- /dev/null
+++ b/drivers/counter/pulse-cnt.c
@@ -0,0 +1,235 @@
+// 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/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define PULSE_CNT_NAME "pulse-cnt"
+
+struct pulse_cnt_priv {
+ struct counter_device counter;
+ struct counter_ops ops;
+ struct gpio_desc *gpio;
+ int irq;
+ bool enabled;
+ atomic_t count;
+};
+
+static irqreturn_t pulse_cnt_isr(int irq, void *dev_id)
+{
+ struct pulse_cnt_priv *priv = dev_id;
+
+ atomic_inc(&priv->count);
+
+ return IRQ_HANDLED;
+}
+
+static ssize_t pulse_cnt_enable_read(struct counter_device *counter,
+ struct counter_count *count, void *private,
+ char *buf)
+{
+ struct pulse_cnt_priv *priv = counter->priv;
+
+ return sysfs_emit(buf, "%d\n", priv->enabled);
+}
+
+static ssize_t pulse_cnt_enable_write(struct counter_device *counter,
+ struct counter_count *count,
+ void *private, const char *buf,
+ size_t len)
+{
+ struct pulse_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 = enable;
+ enable_irq(priv->irq);
+ } else {
+ disable_irq(priv->irq);
+ priv->enabled = enable;
+ }
+
+ return len;
+}
+
+static const struct counter_count_ext pulse_cnt_ext[] = {
+ {
+ .name = "enable",
+ .read = pulse_cnt_enable_read,
+ .write = pulse_cnt_enable_write,
+ },
+};
+
+static enum counter_synapse_action pulse_cnt_synapse_actionss[] = {
+ COUNTER_SYNAPSE_ACTION_RISING_EDGE,
+};
+
+static int pulse_cnt_action_get(struct counter_device *counter,
+ struct counter_count *count,
+ struct counter_synapse *synapse,
+ size_t *action)
+{
+ *action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+
+ return 0;
+}
+
+static int pulse_cnt_read(struct counter_device *counter,
+ struct counter_count *count,
+ unsigned long *val)
+{
+ struct pulse_cnt_priv *priv = counter->priv;
+
+ *val = atomic_read(&priv->count);
+
+ return 0;
+}
+
+static int pulse_cnt_write(struct counter_device *counter,
+ struct counter_count *count,
+ const unsigned long val)
+{
+ struct pulse_cnt_priv *priv = counter->priv;
+
+ atomic_set(&priv->count, val);
+
+ return 0;
+}
+
+static int pulse_cnt_function_get(struct counter_device *counter,
+ struct counter_count *count, size_t *function)
+{
+ *function = COUNTER_COUNT_FUNCTION_INCREASE;
+
+ return 0;
+}
+
+static int pulse_cnt_signal_read(struct counter_device *counter,
+ struct counter_signal *signal,
+ enum counter_signal_value *val)
+{
+ struct pulse_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 pulse_cnt_signals[] = {
+ {
+ .id = 0,
+ .name = "Channel 0 signal",
+ },
+};
+
+static struct counter_synapse pulse_cnt_synapses[] = {
+ {
+ .actions_list = pulse_cnt_synapse_actionss,
+ .num_actions = ARRAY_SIZE(pulse_cnt_synapse_actionss),
+ .signal = &pulse_cnt_signals[0]
+ },
+};
+
+static enum counter_count_function pulse_cnt_functions[] = {
+ COUNTER_COUNT_FUNCTION_INCREASE,
+};
+
+static struct counter_count pulse_cnts[] = {
+ {
+ .id = 0,
+ .name = "Channel 1 Count",
+ .functions_list = pulse_cnt_functions,
+ .num_functions = ARRAY_SIZE(pulse_cnt_functions),
+ .synapses = pulse_cnt_synapses,
+ .num_synapses = ARRAY_SIZE(pulse_cnt_synapses),
+ .ext = pulse_cnt_ext,
+ .num_ext = ARRAY_SIZE(pulse_cnt_ext),
+ },
+};
+
+static int pulse_cnt_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct pulse_cnt_priv *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->irq = platform_get_irq(pdev, 0);
+ if (priv->irq < 0) {
+ dev_err(dev, "failed to map GPIO to IRQ: %d\n", priv->irq);
+ return priv->irq;
+ }
+
+ 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");
+
+ priv->ops.action_get = pulse_cnt_action_get;
+ priv->ops.count_read = pulse_cnt_read;
+ priv->ops.count_write = pulse_cnt_write;
+ priv->ops.function_get = pulse_cnt_function_get;
+ if (priv->gpio)
+ priv->ops.signal_read = pulse_cnt_signal_read;
+
+ priv->counter.name = dev_name(dev);
+ priv->counter.parent = dev;
+ priv->counter.ops = &priv->ops;
+ priv->counter.counts = pulse_cnts;
+ priv->counter.num_counts = ARRAY_SIZE(pulse_cnts);
+ priv->counter.signals = pulse_cnt_signals;
+ priv->counter.num_signals = ARRAY_SIZE(pulse_cnt_signals);
+ priv->counter.priv = priv;
+
+ irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
+ ret = devm_request_irq(dev, priv->irq, pulse_cnt_isr,
+ IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
+ PULSE_CNT_NAME, priv);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, priv);
+
+ return devm_counter_register(dev, &priv->counter);
+}
+
+static const struct of_device_id pulse_cnt_of_match[] = {
+ { .compatible = "virtual,pulse-counter", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pulse_cnt_of_match);
+
+static struct platform_driver pulse_cnt_driver = {
+ .probe = pulse_cnt_probe,
+ .driver = {
+ .name = PULSE_CNT_NAME,
+ .of_match_table = pulse_cnt_of_match,
+ },
+};
+module_platform_driver(pulse_cnt_driver);
+
+MODULE_ALIAS("platform:pulse-counter");
+MODULE_AUTHOR("Oleksij Rempel <[email protected]>");
+MODULE_DESCRIPTION("Pulse counter driver");
+MODULE_LICENSE("GPL v2");
--
2.30.0
Hi Oleksij,
I love your patch! Yet something to improve:
[auto build test ERROR on robh/for-next]
[also build test ERROR on v5.11-rc5 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Oleksij-Rempel/add-support-for-GPIO-based-counter/20210127-085034
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/39e2c0023dba4e0e0f2bb9bfa1caeadb40df6356
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Oleksij-Rempel/add-support-for-GPIO-based-counter/20210127-085034
git checkout 39e2c0023dba4e0e0f2bb9bfa1caeadb40df6356
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/counter/pulse-cnt.c: In function 'pulse_cnt_probe':
>> drivers/counter/pulse-cnt.c:205:2: error: implicit declaration of function 'irq_set_status_flags' [-Werror=implicit-function-declaration]
205 | irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
| ^~~~~~~~~~~~~~~~~~~~
>> drivers/counter/pulse-cnt.c:205:34: error: 'IRQ_NOAUTOEN' undeclared (first use in this function)
205 | irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
| ^~~~~~~~~~~~
drivers/counter/pulse-cnt.c:205:34: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
vim +/irq_set_status_flags +205 drivers/counter/pulse-cnt.c
168
169 static int pulse_cnt_probe(struct platform_device *pdev)
170 {
171 struct device *dev = &pdev->dev;
172 struct pulse_cnt_priv *priv;
173 int ret;
174
175 priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
176 if (!priv)
177 return -ENOMEM;
178
179 priv->irq = platform_get_irq(pdev, 0);
180 if (priv->irq < 0) {
181 dev_err(dev, "failed to map GPIO to IRQ: %d\n", priv->irq);
182 return priv->irq;
183 }
184
185 priv->gpio = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
186 if (IS_ERR(priv->gpio))
187 return dev_err_probe(dev, PTR_ERR(priv->gpio), "failed to get gpio\n");
188
189 priv->ops.action_get = pulse_cnt_action_get;
190 priv->ops.count_read = pulse_cnt_read;
191 priv->ops.count_write = pulse_cnt_write;
192 priv->ops.function_get = pulse_cnt_function_get;
193 if (priv->gpio)
194 priv->ops.signal_read = pulse_cnt_signal_read;
195
196 priv->counter.name = dev_name(dev);
197 priv->counter.parent = dev;
198 priv->counter.ops = &priv->ops;
199 priv->counter.counts = pulse_cnts;
200 priv->counter.num_counts = ARRAY_SIZE(pulse_cnts);
201 priv->counter.signals = pulse_cnt_signals;
202 priv->counter.num_signals = ARRAY_SIZE(pulse_cnt_signals);
203 priv->counter.priv = priv;
204
> 205 irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
206 ret = devm_request_irq(dev, priv->irq, pulse_cnt_isr,
207 IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
208 PULSE_CNT_NAME, priv);
209 if (ret)
210 return ret;
211
212 platform_set_drvdata(pdev, priv);
213
214 return devm_counter_register(dev, &priv->counter);
215 }
216
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi Oleksij,
thanks for your patch!
On Tue, Jan 26, 2021 at 2:15 PM Oleksij Rempel <[email protected]> wrote:
> + priv->irq = platform_get_irq(pdev, 0);
> + if (priv->irq < 0) {
> + dev_err(dev, "failed to map GPIO to IRQ: %d\n", priv->irq);
> + return priv->irq;
> + }
> +
> + 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");
I would attempt to get the IRQ from the GPIO if not defined explicitly
in the device tree.
priv->gpio = devm_gpiod_get_optional(...)
if (priv->gpio) {
/* Attempt to look up IRQ */
irq = gpiod_to_irq(priv->irq);
}
priv->irq = platfform_get_irq(...)
if (priv->irq < 0 && irq > 0) {
/* Use the GPIO-related IRQ */
priv->irq = irq;
} else if (priv->irq < 0) {
/* Error */
}
This way the example in the device tree binding which only defines
a GPIO and no interrupt will work if the GPIO chip provides an
IRQ mapping.
Yours.
Linus Walleij
On Thu, Jan 28, 2021 at 09:24:08AM +0100, Linus Walleij wrote:
> Hi Oleksij,
>
> thanks for your patch!
>
> On Tue, Jan 26, 2021 at 2:15 PM Oleksij Rempel <[email protected]> wrote:
>
> > + priv->irq = platform_get_irq(pdev, 0);
> > + if (priv->irq < 0) {
> > + dev_err(dev, "failed to map GPIO to IRQ: %d\n", priv->irq);
> > + return priv->irq;
> > + }
> > +
> > + 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");
>
> I would attempt to get the IRQ from the GPIO if not defined explicitly
> in the device tree.
>
> priv->gpio = devm_gpiod_get_optional(...)
> if (priv->gpio) {
> /* Attempt to look up IRQ */
> irq = gpiod_to_irq(priv->irq);
> }
> priv->irq = platfform_get_irq(...)
> if (priv->irq < 0 && irq > 0) {
> /* Use the GPIO-related IRQ */
> priv->irq = irq;
> } else if (priv->irq < 0) {
> /* Error */
> }
>
> This way the example in the device tree binding which only defines
> a GPIO and no interrupt will work if the GPIO chip provides an
> IRQ mapping.
>
Ok, thx!
I'll send updated version after dt-binding discussion
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 |