[PATCH v2 0/2] ECAP support on TI AM62x SoC

[PATCH v2 0/2] ECAP support on TI AM62x SoC

[Julien Panis]

The Enhanced Capture (ECAP) module can be used to timestamp events
detected on signal input pin. It can be used for time measurements
of pulse train signals.

ECAP module includes 4 timestamp capture registers. For all 4 sequenced
timestamp capture events (0->1->2->3->0->...), edge polarity (falling/rising
edge) can be selected. Moreover, input signal can be prescaled.

This driver leverages IIO subsystem to :
- select edge polarity for all 4 capture events (event mode)
- log both event index and timestamp for each capture event
Event polarity, event indexes, and timestamps give all the information
about the input pulse train. Further information can easily be computed :
period and/or duty cycle if frequency is constant, elapsed time between
pulses, etc...

Modifications since v1:
	- Fix yaml issues (id and example)
	- Use regmap instead of writel/readl
	- Enable/Disable clock explicitly
	- Simplify power management (suspend/resume)
	- Remove unnecessary using of ev_mode variable

Userspace commands :
	cd /sys/devices/platform/bus@f0000/23120000.capture/iio\:device2/

	# Configure/Enable data buffers
	echo 1 > scan_elements/in_index_en
	echo 1 > scan_elements/in_timestamp_en
	echo 100 > buffer/length
	echo 1 > buffer/enable

	# Set event mode in range [0 ; 15]
	# One bit for each CAPx register : 1 = falling edge / 0 = rising edge
	# e.g. mode = 13 = 0xd = 0b1101
	# -> falling edge for CAP1-3-4 / rising edge for CAP2
	echo 13 > events/change_falling_value

	# Run ECAP
	echo 1 > en

	# Get the number of available data
	cat buffer/data_available

	# Read data
	hexdump -v /dev/iio\:device2

	# Stop ECAP
	echo 0 > en

Julien Panis (2):
  dt-binding: iio: time: add capture-tiecap.yaml
  iio: time: capture-tiecap: capture driver support for ECAP

 .../bindings/iio/time/capture-tiecap.yaml     |  68 +++
 drivers/iio/Kconfig                           |   1 +
 drivers/iio/Makefile                          |   1 +
 drivers/iio/time/Kconfig                      |  22 +
 drivers/iio/time/Makefile                     |   6 +
 drivers/iio/time/capture-tiecap.c             | 521 ++++++++++++++++++
 6 files changed, 619 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
 create mode 100644 drivers/iio/time/Kconfig
 create mode 100644 drivers/iio/time/Makefile
 create mode 100644 drivers/iio/time/capture-tiecap.c

-- 
2.25.1

[PATCH v2 1/2] dt-binding: iio: time: add capture-tiecap.yaml

[Julien Panis]

This commit adds a YAML binding for TI ECAP used in capture operating mode.

Signed-off-by: Julien Panis <jpanis@baylibre.com>
---
 .../bindings/iio/time/capture-tiecap.yaml     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml

diff --git a/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml b/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
new file mode 100644
index 000000000000..5d66df90ba5d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/time/capture-tiecap.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments Enhanced Capture (eCAP) Module
+
+maintainers:
+  - Julien Panis <jpanis@baylibre.com>
+
+description: |
+  The eCAP module resources can be used to capture timestamps
+  on input signal events (falling/rising edges).
+
+properties:
+  compatible:
+    const: ti,am62-ecap-capture
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: fck
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        bus@f0000 {
+            compatible = "simple-bus";
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges = <0x00 0x20000000 0x00 0x20000000 0x00 0x0a008000>;
+
+            ecap0: capture@23100000 { /* eCAP in capture mode on am62x */
+                compatible = "ti,am62-ecap-capture";
+                reg = <0x00 0x23100000 0x00 0x100>;
+                interrupts = <GIC_SPI 113 IRQ_TYPE_EDGE_RISING>;
+                power-domains = <&k3_pds 51 TI_SCI_PD_EXCLUSIVE>;
+                clocks = <&k3_clks 51 0>;
+                clock-names = "fck";
+            };
+        };
+    };
-- 
2.25.1

[PATCH v2 2/2] iio: time: capture-tiecap: capture driver support for ECAP

[Julien Panis]

ECAP hardware on AM62x SoC supports capture feature. It can be used
to timestamp events (falling/rising edges) detected on signal input pin.

This commit adds capture driver support for ECAP hardware on AM62x SoC.

In the ECAP hardware, capture pin can also be configured to be in
PWM mode. Current implementation only supports capture operating mode.
Hardware also supports timebase sync between multiple instances, but
this driver supports simple independent capture functionality.

Signed-off-by: Julien Panis <jpanis@baylibre.com>
---
 drivers/iio/Kconfig               |   1 +
 drivers/iio/Makefile              |   1 +
 drivers/iio/time/Kconfig          |  22 ++
 drivers/iio/time/Makefile         |   6 +
 drivers/iio/time/capture-tiecap.c | 521 ++++++++++++++++++++++++++++++
 5 files changed, 551 insertions(+)
 create mode 100644 drivers/iio/time/Kconfig
 create mode 100644 drivers/iio/time/Makefile
 create mode 100644 drivers/iio/time/capture-tiecap.c

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index b190846c3dc2..ba11b8824071 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -99,5 +99,6 @@ source "drivers/iio/pressure/Kconfig"
 source "drivers/iio/proximity/Kconfig"
 source "drivers/iio/resolver/Kconfig"
 source "drivers/iio/temperature/Kconfig"
+source "drivers/iio/time/Kconfig"
 
 endif # IIO
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 3be08cdadd7e..09283402a2c6 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -42,4 +42,5 @@ obj-y += proximity/
 obj-y += resolver/
 obj-y += temperature/
 obj-y += test/
+obj-y += time/
 obj-y += trigger/
diff --git a/drivers/iio/time/Kconfig b/drivers/iio/time/Kconfig
new file mode 100644
index 000000000000..02f6cf7ff79e
--- /dev/null
+++ b/drivers/iio/time/Kconfig
@@ -0,0 +1,22 @@
+#
+# Time drivers
+#
+
+menu "Time"
+
+config CAPTURE_TIECAP
+	tristate "ECAP capture support"
+	depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
+	depends on HAS_IOMEM
+	select IIO_BUFFER
+	select IIO_KFIFO_BUF
+	help
+	  IIO driver support for the ECAP capture hardware found on TI SoCs.
+
+	  It can be used to timestamp events (falling/rising edges) detected
+	  on signal input pin.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called capture-tiecap.
+
+endmenu
diff --git a/drivers/iio/time/Makefile b/drivers/iio/time/Makefile
new file mode 100644
index 000000000000..3a27f3557d1e
--- /dev/null
+++ b/drivers/iio/time/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for industrial I/O Time drivers
+#
+
+obj-$(CONFIG_CAPTURE_TIECAP) += capture-tiecap.o
diff --git a/drivers/iio/time/capture-tiecap.c b/drivers/iio/time/capture-tiecap.c
new file mode 100644
index 000000000000..1044e0d36b86
--- /dev/null
+++ b/drivers/iio/time/capture-tiecap.c
@@ -0,0 +1,521 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * ECAP Capture driver
+ *
+ * Copyright (C) 2022 Julien Panis <jpanis@baylibre.com>
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/pm_runtime.h>
+#include <linux/of_device.h>
+#include <linux/bitfield.h>
+#include <linux/regmap.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/kfifo_buf.h>
+
+/* Registers */
+#define ECAP_NB_CAP			4
+
+#define ECAP_TSCNT_REG			0x00
+
+#define ECAP_CAP_REG(i)		(((i) << 2) + 0x08)
+
+#define ECAP_ECCTL_REG			0x28
+#define ECAP_CAPPOL_BIT(i)		BIT((i) << 1)
+#define ECAP_EV_MODE_MASK		GENMASK(7, 0)
+#define ECAP_CAPLDEN_BIT		BIT(8)
+#define ECAP_EVTFLTPS_MASK		GENMASK(13, 9)
+#define ECAP_PS_DEFAULT_VAL		0
+#define ECAP_PS_MAX_VAL		31
+#define ECAP_CONT_ONESHT_BIT		BIT(16)
+#define ECAP_STOPVALUE_MASK		GENMASK(18, 17)
+#define ECAP_REARM_RESET_BIT		BIT(19)
+#define ECAP_TSCNTSTP_BIT		BIT(20)
+#define ECAP_SYNCO_DIS_MASK		GENMASK(23, 22)
+#define ECAP_CAP_APWM_BIT		BIT(25)
+#define ECAP_ECCTL_EN_MASK		(ECAP_CAPLDEN_BIT | ECAP_TSCNTSTP_BIT)
+#define ECAP_ECCTL_CFG_MASK		(ECAP_EVTFLTPS_MASK | ECAP_SYNCO_DIS_MASK	\
+					| ECAP_STOPVALUE_MASK | ECAP_ECCTL_EN_MASK	\
+					| ECAP_CAP_APWM_BIT | ECAP_CONT_ONESHT_BIT	\
+					| ECAP_REARM_RESET_BIT)
+
+#define ECAP_ECINT_EN_FLG_REG		0x2c
+#define ECAP_NB_CEVT			(ECAP_NB_CAP + 1)
+#define ECAP_CEVT_EN_MASK		GENMASK(ECAP_NB_CEVT, 1)
+#define ECAP_CEVT_FLG_BIT(i)		BIT((i) + 17)
+#define ECAP_OVF_VAL			0xff
+
+#define ECAP_ECINT_CLR_FRC_REG	0x30
+#define ECAP_INT_CLR_BIT		BIT(0)
+#define ECAP_CEVT_CLR_BIT(i)		BIT((i) + 1)
+#define ECAP_CEVT_CLR_MASK		GENMASK(ECAP_NB_CEVT, 0)
+
+#define ECAP_PID_REG			0x5c
+
+/*
+ * Event modes
+ * One bit for each CAPx register : 1 = falling edge / 0 = rising edge
+ * e.g. mode = 13 = 0xd = 0b1101
+ * -> falling edge for CAP1-3-4 / rising edge for CAP2
+ */
+#define ECAP_NB_EV_MODES		GENMASK(ECAP_NB_CAP - 1, 0)
+#define ECAP_EV_MODE_BIT(i)		BIT(i)
+
+static unsigned int prescaler = ECAP_PS_DEFAULT_VAL;
+module_param(prescaler, uint, 0644);
+MODULE_PARM_DESC(prescaler, "Input capture signal prescaler from 0 to "
+		 __MODULE_STRING(ECAP_PS_MAX_VAL)", default "
+		 __MODULE_STRING(ECAP_PS_DEFAULT_VAL));
+
+static const struct regmap_config ecap_iio_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = ECAP_PID_REG,
+};
+
+/*
+ * struct ecap_iio_context - IIO device context
+ * @ev_mode:   event mode describing falling/rising edges for captures 1 to 4
+ * @time_cntr: timestamp counter value
+ */
+struct ecap_iio_context {
+	u8 ev_mode;
+	unsigned int time_cntr;
+};
+
+/*
+ * struct ecap_iio_data - IIO device data
+ * @ev_idx:  event index (0 to 3 for CAP1 to CAP4)
+ * @ev_time: falling/rising edge timestamp
+ */
+struct ecap_iio_data {
+	u8 ev_idx;
+	s64 ev_time __aligned(sizeof(s64));
+};
+
+/*
+ * struct ecap_iio_dev - IIO device private data structure
+ * @enabled:  device state
+ * @clk:      device clock
+ * @clk_rate: device clock rate
+ * @regmap:   device register map
+ * @pm_ctx:   device context for PM operations
+ * @data:     device data
+ */
+struct ecap_iio_dev {
+	bool enabled;
+	struct clk *clk;
+	unsigned long clk_rate;
+	struct regmap *regmap;
+	struct ecap_iio_context pm_ctx;
+	struct ecap_iio_data data;
+};
+
+static u8 ecap_iio_capture_get_evmode(struct iio_dev *indio_dev)
+{
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+	u8 ev_mode = 0;
+	unsigned int regval;
+	int i;
+
+	pm_runtime_get_sync(indio_dev->dev.parent);
+	regmap_read(ecap_dev->regmap, ECAP_ECCTL_REG, &regval);
+	pm_runtime_put_sync(indio_dev->dev.parent);
+
+	for (i = 0 ; i < ECAP_NB_CAP ; i++) {
+		if (regval & ECAP_CAPPOL_BIT(i))
+			ev_mode |= ECAP_EV_MODE_BIT(i);
+	}
+
+	return ev_mode;
+}
+
+static void ecap_iio_capture_set_evmode(struct iio_dev *indio_dev, u8 ev_mode)
+{
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+	unsigned int regval = 0;
+	int i;
+
+	for (i = 0 ; i < ECAP_NB_CAP ; i++) {
+		if (ev_mode & ECAP_EV_MODE_BIT(i))
+			regval |= ECAP_CAPPOL_BIT(i);
+	}
+
+	pm_runtime_get_sync(indio_dev->dev.parent);
+	regmap_update_bits(ecap_dev->regmap, ECAP_ECCTL_REG, ECAP_EV_MODE_MASK, regval);
+	pm_runtime_put_sync(indio_dev->dev.parent);
+}
+
+static void ecap_iio_capture_enable(struct iio_dev *indio_dev, bool rearm)
+{
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+	unsigned int regval = 0;
+
+	pm_runtime_get_sync(indio_dev->dev.parent);
+
+	/* Enable interrupts on events */
+	regmap_update_bits(ecap_dev->regmap, ECAP_ECINT_EN_FLG_REG,
+			   ECAP_CEVT_EN_MASK, ECAP_CEVT_EN_MASK);
+
+	/* Run counter */
+	regval |= FIELD_PREP(ECAP_EVTFLTPS_MASK, prescaler)
+		| ECAP_SYNCO_DIS_MASK | ECAP_STOPVALUE_MASK | ECAP_ECCTL_EN_MASK;
+	if (rearm) {
+		regmap_write(ecap_dev->regmap, ECAP_TSCNT_REG, 0);
+		regval |= ECAP_REARM_RESET_BIT;
+	}
+	regmap_update_bits(ecap_dev->regmap, ECAP_ECCTL_REG, ECAP_ECCTL_CFG_MASK, regval);
+}
+
+static void ecap_iio_capture_disable(struct iio_dev *indio_dev)
+{
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+
+	/* Disable interrupts on events */
+	regmap_update_bits(ecap_dev->regmap, ECAP_ECINT_EN_FLG_REG, ECAP_CEVT_EN_MASK, 0);
+
+	/* Stop counter */
+	regmap_update_bits(ecap_dev->regmap, ECAP_ECCTL_REG, ECAP_ECCTL_EN_MASK, 0);
+
+	pm_runtime_put_sync(indio_dev->dev.parent);
+}
+
+static int ecap_iio_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_ENABLE:
+		*val = ecap_dev->enabled;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ecap_iio_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long info)
+{
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_ENABLE:
+		if (val < 0 || val > 1)
+			return -EINVAL;
+		if (val == ecap_dev->enabled)
+			return 0;
+		if (val)
+			ecap_iio_capture_enable(indio_dev, true);
+		else
+			ecap_iio_capture_disable(indio_dev);
+		ecap_dev->enabled = val;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ecap_iio_read_event_value(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int *val, int *val2)
+{
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (dir) {
+		case IIO_EV_DIR_FALLING:
+			*val = ecap_iio_capture_get_evmode(indio_dev);
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ecap_iio_write_event_value(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      enum iio_event_info info,
+				      int val, int val2)
+{
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (dir) {
+		case IIO_EV_DIR_FALLING:
+			if (val < 0 || val > ECAP_NB_EV_MODES)
+				return -EINVAL;
+			if (ecap_dev->enabled)
+				return -EBUSY;
+			ecap_iio_capture_set_evmode(indio_dev, val);
+			return 0;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info ecap_iio_info = {
+	.read_raw = ecap_iio_read_raw,
+	.write_raw = ecap_iio_write_raw,
+	.read_event_value = ecap_iio_read_event_value,
+	.write_event_value = ecap_iio_write_event_value,
+};
+
+static const struct iio_event_spec ecap_iio_events[] = {
+	{
+		.type = IIO_EV_TYPE_CHANGE,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_shared_by_all = BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
+static const struct iio_chan_spec ecap_iio_channels[] = {
+	{
+		.scan_index = 0,
+		.type = IIO_INDEX,
+		.address = 0,
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_ENABLE),
+		.modified = 0,
+		.event_spec = ecap_iio_events,
+		.num_event_specs = ARRAY_SIZE(ecap_iio_events),
+		.scan_type = {
+			.sign = 'u',
+			.endianness = IIO_LE,
+			.realbits = 2,
+			.storagebits = 8,
+			.shift = 0,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+static irqreturn_t ecap_iio_isr(int irq, void *dev_id)
+{
+	struct iio_dev *indio_dev = dev_id;
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+	struct ecap_iio_data *ecap_data = &ecap_dev->data;
+	unsigned int clr = 0;
+	unsigned int flg;
+	unsigned int cap_time;
+	int i;
+
+	regmap_read(ecap_dev->regmap, ECAP_ECINT_EN_FLG_REG, &flg);
+
+	for (i = 0 ; i < ECAP_NB_CEVT ; i++) {
+		if (flg & ECAP_CEVT_FLG_BIT(i)) {
+			if (i < ECAP_NB_CAP) {
+				/*
+				 * Input signal edge detected
+				 * time_ns = 10^9 * time_cycles / clk_rate
+				 */
+				ecap_data->ev_idx = i;
+				regmap_read(ecap_dev->regmap, ECAP_CAP_REG(i), &cap_time);
+				ecap_data->ev_time = cap_time * NSEC_PER_SEC;
+				do_div(ecap_data->ev_time, ecap_dev->clk_rate);
+			} else {
+				/* Counter overflow */
+				ecap_data->ev_idx = ECAP_OVF_VAL;
+				ecap_data->ev_time = 0;
+			}
+			iio_push_to_buffers(indio_dev, ecap_data);
+
+			clr |= ECAP_CEVT_CLR_BIT(i);
+		}
+	}
+
+	clr |= ECAP_INT_CLR_BIT;
+	regmap_update_bits(ecap_dev->regmap, ECAP_ECINT_CLR_FRC_REG, ECAP_CEVT_CLR_MASK, clr);
+
+	return IRQ_HANDLED;
+}
+
+static void ecap_iio_clk_disable(void *clk)
+{
+	clk_disable_unprepare(clk);
+}
+
+static int ecap_iio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ecap_iio_dev *ecap_dev;
+	struct iio_dev *indio_dev;
+	void __iomem *mmio_base;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*ecap_dev));
+	if (IS_ERR(indio_dev)) {
+		dev_err(dev, "failed to allocate memory for iio device\n");
+		return PTR_ERR(indio_dev);
+	}
+
+	ecap_dev = iio_priv(indio_dev);
+
+	ecap_dev->clk = devm_clk_get(dev, "fck");
+	if (IS_ERR(ecap_dev->clk)) {
+		dev_err(dev, "failed to get clock\n");
+		return PTR_ERR(ecap_dev->clk);
+	}
+
+	ret = clk_prepare_enable(ecap_dev->clk);
+	if (ret) {
+		dev_err(dev, "failed to enable clock\n");
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, ecap_iio_clk_disable, ecap_dev->clk);
+	if (ret) {
+		dev_err(dev, "failed to add clock disable action\n");
+		return ret;
+	}
+
+	ecap_dev->clk_rate = clk_get_rate(ecap_dev->clk);
+	if (!ecap_dev->clk_rate) {
+		dev_err(dev, "failed to get clock rate\n");
+		return -EINVAL;
+	}
+
+	if (prescaler > ECAP_PS_MAX_VAL) {
+		prescaler = ECAP_PS_MAX_VAL;
+		dev_warn(dev, "prescaler out of range, forced to %d\n", prescaler);
+	}
+
+	mmio_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(mmio_base)) {
+		dev_err(dev, "failed to remap io\n");
+		return PTR_ERR(mmio_base);
+	}
+
+	ecap_dev->regmap = regmap_init_mmio(dev, mmio_base, &ecap_iio_regmap_config);
+	if (IS_ERR(ecap_dev->regmap)) {
+		dev_err(dev, "failed to init regmap\n");
+		return PTR_ERR(ecap_dev->regmap);
+	}
+
+	indio_dev->name = devm_kasprintf(dev, GFP_KERNEL,
+					 "ecap-iio-%p", mmio_base);
+	indio_dev->info = &ecap_iio_info;
+	indio_dev->channels = ecap_iio_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ecap_iio_channels);
+	indio_dev->modes = INDIO_BUFFER_SOFTWARE;
+
+	ret = devm_iio_kfifo_buffer_setup_ext(dev, indio_dev, NULL, NULL);
+	if (ret) {
+		dev_err(dev, "failed to setup iio buffer\n");
+		return ret;
+	}
+
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0) {
+		dev_err(dev, "failed to get irq\n");
+		return ret;
+	}
+
+	ret = devm_request_irq(dev, ret, ecap_iio_isr, 0, pdev->name, indio_dev);
+	if (ret) {
+		dev_err(dev, "failed to request irq\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, indio_dev);
+	pm_runtime_enable(&pdev->dev);
+
+	ecap_dev->enabled = 0;
+	ecap_iio_capture_set_evmode(indio_dev, 0);
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static int ecap_iio_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+
+	if (ecap_dev->enabled)
+		ecap_iio_capture_disable(indio_dev);
+
+	regmap_exit(ecap_dev->regmap);
+
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static __maybe_unused int ecap_iio_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+
+	/* If eCAP is running, stop capture then save timestamp counter */
+	if (ecap_dev->enabled) {
+		ecap_iio_capture_disable(indio_dev);
+
+		pm_runtime_get_sync(indio_dev->dev.parent);
+		regmap_read(ecap_dev->regmap, ECAP_TSCNT_REG, &ecap_dev->pm_ctx.time_cntr);
+		pm_runtime_put_sync(indio_dev->dev.parent);
+	}
+
+	ecap_dev->pm_ctx.ev_mode = ecap_iio_capture_get_evmode(indio_dev);
+
+	return 0;
+}
+
+static __maybe_unused int ecap_iio_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct ecap_iio_dev *ecap_dev = iio_priv(indio_dev);
+
+	ecap_iio_capture_set_evmode(indio_dev, ecap_dev->pm_ctx.ev_mode);
+
+	/* If eCAP was running, restore timestamp counter then run capture */
+	if (ecap_dev->enabled) {
+		pm_runtime_get_sync(indio_dev->dev.parent);
+		regmap_write(ecap_dev->regmap, ECAP_TSCNT_REG, ecap_dev->pm_ctx.time_cntr);
+		pm_runtime_put_sync(indio_dev->dev.parent);
+
+		ecap_iio_capture_enable(indio_dev, false);
+	}
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ecap_iio_pm_ops, ecap_iio_suspend, ecap_iio_resume);
+
+static const struct of_device_id ecap_iio_of_match[] = {
+	{ .compatible	= "ti,am62-ecap-capture" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ecap_iio_of_match);
+
+static struct platform_driver ecap_iio_driver = {
+	.probe = ecap_iio_probe,
+	.remove = ecap_iio_remove,
+	.driver = {
+		.name = "ecap-capture",
+		.of_match_table = of_match_ptr(ecap_iio_of_match),
+		.pm = &ecap_iio_pm_ops,
+	},
+};
+module_platform_driver(ecap_iio_driver);
+
+MODULE_DESCRIPTION("ECAP Capture driver");
+MODULE_AUTHOR("Julien Panis <jpanis@baylibre.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v2 1/2] dt-binding: iio: time: add capture-tiecap.yaml

[Krzysztof Kozlowski]

On 28/07/2022 14:52, Julien Panis wrote:
> This commit adds a YAML binding for TI ECAP used in capture operating mode.
> 
> Signed-off-by: Julien Panis <jpanis@baylibre.com>
> ---
>  .../bindings/iio/time/capture-tiecap.yaml     | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml b/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
> new file mode 100644
> index 000000000000..5d66df90ba5d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/time/capture-tiecap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

Still wrong file name.

Do not ignore comments you received. Either you implement them or you
keep the discussion going.


> +
> +title: Texas Instruments Enhanced Capture (eCAP) Module
> +
> +maintainers:
> +  - Julien Panis <jpanis@baylibre.com>
> +

(...)

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        bus@f0000 {
> +            compatible = "simple-bus";
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +            ranges = <0x00 0x20000000 0x00 0x20000000 0x00 0x0a008000>;

A bit too many nodes. Just keep one, bus or soc. Look at other examples.


Best regards,
Krzysztof

Re: [PATCH v2 2/2] iio: time: capture-tiecap: capture driver support for ECAP

[Krzysztof Kozlowski]

On 28/07/2022 14:52, Julien Panis wrote:
> ECAP hardware on AM62x SoC supports capture feature. It can be used
> to timestamp events (falling/rising edges) detected on signal input pin.
> 
> This commit adds capture driver support for ECAP hardware on AM62x SoC.
> 
> In the ECAP hardware, capture pin can also be configured to be in

Thank you for your patch. There is something to discuss/improve.

(...)

> +static int ecap_iio_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ecap_iio_dev *ecap_dev;
> +	struct iio_dev *indio_dev;
> +	void __iomem *mmio_base;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*ecap_dev));
> +	if (IS_ERR(indio_dev)) {
> +		dev_err(dev, "failed to allocate memory for iio device\n");

Do not print messages, which core takes care of.

> +		return PTR_ERR(indio_dev);
> +	}
> +
> +	ecap_dev = iio_priv(indio_dev);
> +
> +	ecap_dev->clk = devm_clk_get(dev, "fck");
> +	if (IS_ERR(ecap_dev->clk)) {
> +		dev_err(dev, "failed to get clock\n");
> +		return PTR_ERR(ecap_dev->clk);
> +	}
> +
> +	ret = clk_prepare_enable(ecap_dev->clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable clock\n");
> +		return ret;
> +	}
> +
> +	ret = devm_add_action_or_reset(dev, ecap_iio_clk_disable, ecap_dev->clk);
> +	if (ret) {
> +		dev_err(dev, "failed to add clock disable action\n");
> +		return ret;
> +	}
> +
> +	ecap_dev->clk_rate = clk_get_rate(ecap_dev->clk);
> +	if (!ecap_dev->clk_rate) {
> +		dev_err(dev, "failed to get clock rate\n");
> +		return -EINVAL;
> +	}
> +
> +	if (prescaler > ECAP_PS_MAX_VAL) {
> +		prescaler = ECAP_PS_MAX_VAL;
> +		dev_warn(dev, "prescaler out of range, forced to %d\n", prescaler);
> +	}
> +
> +	mmio_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(mmio_base)) {
> +		dev_err(dev, "failed to remap io\n");

No need for msg.

> +		return PTR_ERR(mmio_base);
> +	}
> +
> +	ecap_dev->regmap = regmap_init_mmio(dev, mmio_base, &ecap_iio_regmap_config);
> +	if (IS_ERR(ecap_dev->regmap)) {
> +		dev_err(dev, "failed to init regmap\n");
> +		return PTR_ERR(ecap_dev->regmap);
> +	}
> +
> +	indio_dev->name = devm_kasprintf(dev, GFP_KERNEL,
> +					 "ecap-iio-%p", mmio_base);
> +	indio_dev->info = &ecap_iio_info;
> +	indio_dev->channels = ecap_iio_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ecap_iio_channels);
> +	indio_dev->modes = INDIO_BUFFER_SOFTWARE;
> +
> +	ret = devm_iio_kfifo_buffer_setup_ext(dev, indio_dev, NULL, NULL);
> +	if (ret) {
> +		dev_err(dev, "failed to setup iio buffer\n");
> +		return ret;
> +	}
> +
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get irq\n");
> +		return ret;
> +	}
> +
> +	ret = devm_request_irq(dev, ret, ecap_iio_isr, 0, pdev->name, indio_dev);
> +	if (ret) {
> +		dev_err(dev, "failed to request irq\n");
> +		return ret;
> +	}

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-binding: iio: time: add capture-tiecap.yaml

[Julien Panis]

On 28/07/2022 15:38, Krzysztof Kozlowski wrote:
> On 28/07/2022 14:52, Julien Panis wrote:
>> This commit adds a YAML binding for TI ECAP used in capture operating mode.
>>
>> Signed-off-by: Julien Panis <jpanis@baylibre.com>
>> ---
>>   .../bindings/iio/time/capture-tiecap.yaml     | 68 +++++++++++++++++++
>>   1 file changed, 68 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml b/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
>> new file mode 100644
>> index 000000000000..5d66df90ba5d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/time/capture-tiecap.yaml
>> @@ -0,0 +1,68 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/time/capture-tiecap.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> Still wrong file name.
>
> Do not ignore comments you received. Either you implement them or you
> keep the discussion going.


Sorry, that was not intentional. I have prepared a new version which 
takes into account your last comments.

Julien