On Mon, Feb 08, 2021 at 02:53:47PM +0100, Oleksij Rempel 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 <o.rempel@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Hi Oleksij,

I haven't had a chance to do a proper review just yet, but a couple
things caught my eye as I skimmed so I want to mention them inline
below. I'll do a more complete review this weekend or soon after

The value pointed by this "action" parameter should be set to the index
of the action you want from your Synapse's ations[] array.

+
+	return 0;
+}
+
+static int event_cnt_read(struct counter_device *counter,
+				 struct counter_count *count,
+				 unsigned long *val)
+{
+	struct event_cnt_priv *priv = counter->priv;
+
+	*val = atomic_read(&priv->count);
+
+	return 0;
+}
+
+static int event_cnt_write(struct counter_device *counter,
+				  struct counter_count *count,
+				  const unsigned long val)
+{
+	struct event_cnt_priv *priv = counter->priv;
+
+	atomic_set(&priv->count, val);
+
+	return 0;
+}
+
+static int event_cnt_function_get(struct counter_device *counter,
+				  struct counter_count *count, size_t *function)
+{
+	*function = COUNTER_COUNT_FUNCTION_INCREASE;

Same problem here as before: should be set to the index of the function
you want from your functions[] array.

William Breathitt Gray

+
+	return 0;
+}
+
+static int event_cnt_signal_read(struct counter_device *counter,
+				 struct counter_signal *signal,
+				 enum counter_signal_value *val)
+{
+	struct event_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 event_cnt_signals[] = {
+	{
+		.id = 0,
+		.name = "Channel 0 signal",
+	},
+};
+
+static struct counter_synapse event_cnt_synapses[] = {
+	{
+		.actions_list = event_cnt_synapse_actionss,
+		.num_actions = ARRAY_SIZE(event_cnt_synapse_actionss),
+		.signal = &event_cnt_signals[0]
+	},
+};
+
+static enum counter_count_function event_cnt_functions[] = {
+	COUNTER_COUNT_FUNCTION_INCREASE,
+};
+
+static struct counter_count event_cnts[] = {
+	{
+		.id = 0,
+		.name = "Channel 1 Count",
+		.functions_list = event_cnt_functions,
+		.num_functions = ARRAY_SIZE(event_cnt_functions),
+		.synapses = event_cnt_synapses,
+		.num_synapses = ARRAY_SIZE(event_cnt_synapses),
+		.ext = event_cnt_ext,
+		.num_ext = ARRAY_SIZE(event_cnt_ext),
+	},
+};
+
+static int event_cnt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct event_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->ops.action_get = event_cnt_action_get;
+	priv->ops.count_read = event_cnt_read;
+	priv->ops.count_write = event_cnt_write;
+	priv->ops.function_get = event_cnt_function_get;
+	if (priv->gpio)
+		priv->ops.signal_read  = event_cnt_signal_read;
+
+	priv->counter.name = dev_name(dev);
+	priv->counter.parent = dev;
+	priv->counter.ops = &priv->ops;
+	priv->counter.counts = event_cnts;
+	priv->counter.num_counts = ARRAY_SIZE(event_cnts);
+	priv->counter.signals = event_cnt_signals;
+	priv->counter.num_signals = ARRAY_SIZE(event_cnt_signals);
+	priv->counter.priv = priv;
+
+	irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
+	ret = devm_request_irq(dev, priv->irq, event_cnt_isr,
+			       IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
+			       EVENT_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 event_cnt_of_match[] = {
+	{ .compatible = "event-counter", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, event_cnt_of_match);
+
+static struct platform_driver event_cnt_driver = {
+	.probe = event_cnt_probe,
+	.driver = {
+		.name = EVENT_CNT_NAME,
+		.of_match_table = event_cnt_of_match,
+	},
+};
+module_platform_driver(event_cnt_driver);
+
+MODULE_ALIAS("platform:event-counter");
+MODULE_AUTHOR("Oleksij Rempel [off-list ref]");
+MODULE_DESCRIPTION("Event counter driver");
+MODULE_LICENSE("GPL v2");
-- 
2.30.0