[PATCH v2 1/2] counter: microchip-tcb-capture: Add IRQ handling

[PATCH v2 1/2] counter: microchip-tcb-capture: Add IRQ handling

[Bence Csókás]

Add interrupt servicing to allow userspace to wait for a trigger event.

Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>
---

Notes:
    New in v2

 drivers/counter/microchip-tcb-capture.c | 60 +++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/counter/microchip-tcb-capture.c b/drivers/counter/microchip-tcb-capture.c
index 2f096a5b973d..316755c7659a 100644
--- a/drivers/counter/microchip-tcb-capture.c
+++ b/drivers/counter/microchip-tcb-capture.c
@@ -6,10 +6,12 @@
  */
 #include <linux/clk.h>
 #include <linux/counter.h>
+#include <linux/interrupt.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <soc/at91/atmel_tcb.h>
@@ -18,6 +20,8 @@
 				 ATMEL_TC_ETRGEDG_RISING | ATMEL_TC_LDBDIS | \
 				 ATMEL_TC_LDBSTOP)
 
+#define ATMEL_TC_DEF_IRQS	(ATMEL_TC_ETRGS | ATMEL_TC_COVFS)
+
 #define ATMEL_TC_QDEN			BIT(8)
 #define ATMEL_TC_POSEN			BIT(9)
 
@@ -27,6 +31,7 @@ struct mchp_tc_data {
 	int qdec_mode;
 	int num_channels;
 	int channel[2];
+	int irq;
 };
 
 static const enum counter_function mchp_tc_count_functions[] = {
@@ -294,6 +299,54 @@ static const struct of_device_id atmel_tc_of_match[] = {
 	{ /* sentinel */ }
 };
 
+static irqreturn_t mchp_tc_isr(int irq, void *dev_id)
+{
+	struct counter_device *const counter = dev_id;
+	struct mchp_tc_data *const priv = counter_priv(counter);
+	u32 sr, mask;
+
+	regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], SR), &sr);
+	regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], IMR), &mask);
+
+	sr &= mask;
+	if (!(sr & ATMEL_TC_ALL_IRQ))
+		return IRQ_NONE;
+
+	if (sr & ATMEL_TC_ETRGS)
+		counter_push_event(counter, COUNTER_EVENT_CHANGE_OF_STATE, 0);
+	if (sr & ATMEL_TC_COVFS)
+		counter_push_event(counter, COUNTER_EVENT_OVERFLOW, 0);
+
+	return IRQ_HANDLED;
+}
+
+static void mchp_tc_irq_remove(void *ptr)
+{
+	struct mchp_tc_data *priv = ptr;
+
+	regmap_write(priv->regmap, ATMEL_TC_REG(priv->channel[0], IDR), ATMEL_TC_DEF_IRQS);
+}
+
+static int mchp_tc_irq_enable(struct counter_device *const counter)
+{
+	struct mchp_tc_data *const priv = counter_priv(counter);
+	int ret = devm_request_irq(counter->parent, priv->irq, mchp_tc_isr, 0,
+				   dev_name(counter->parent), counter);
+
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(priv->regmap, ATMEL_TC_REG(priv->channel[0], IER), ATMEL_TC_DEF_IRQS);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_add_action_or_reset(counter->parent, mchp_tc_irq_remove, priv);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static void mchp_tc_clk_remove(void *ptr)
 {
 	clk_disable_unprepare((struct clk *)ptr);
@@ -378,6 +431,13 @@ static int mchp_tc_probe(struct platform_device *pdev)
 	counter->num_signals = ARRAY_SIZE(mchp_tc_count_signals);
 	counter->signals = mchp_tc_count_signals;
 
+	priv->irq = of_irq_get(np->parent, 0);
+	if (priv->irq > 0) {
+		ret = mchp_tc_irq_enable(counter);
+		if (ret < 0)
+			return dev_err_probe(&pdev->dev, ret, "Failed to set up IRQ");
+	}
+
 	ret = devm_counter_add(&pdev->dev, counter);
 	if (ret < 0)
 		return dev_err_probe(&pdev->dev, ret, "Failed to add counter\n");
-- 
2.48.1

[PATCH v2 2/2] counter: microchip-tcb-capture: Add capture extensions for registers RA-RC

[Bence Csókás]

TCB hardware is capable of capturing the timer value to registers RA and
RB. On top, it is capable of triggering on compare against a third
register, RC. Add these registers as extensions.

Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>
---
 drivers/counter/microchip-tcb-capture.c | 65 ++++++++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/counter/microchip-tcb-capture.c b/drivers/counter/microchip-tcb-capture.c
index 316755c7659a..a25b54493c7f 100644
--- a/drivers/counter/microchip-tcb-capture.c
+++ b/drivers/counter/microchip-tcb-capture.c
@@ -20,7 +20,8 @@
 				 ATMEL_TC_ETRGEDG_RISING | ATMEL_TC_LDBDIS | \
 				 ATMEL_TC_LDBSTOP)
 
-#define ATMEL_TC_DEF_IRQS	(ATMEL_TC_ETRGS | ATMEL_TC_COVFS)
+#define ATMEL_TC_DEF_IRQS	(ATMEL_TC_ETRGS | ATMEL_TC_COVFS | \
+				 ATMEL_TC_LDRAS | ATMEL_TC_LDRBS)
 
 #define ATMEL_TC_QDEN			BIT(8)
 #define ATMEL_TC_POSEN			BIT(9)
@@ -252,6 +253,62 @@ static int mchp_tc_count_read(struct counter_device *counter,
 	return 0;
 }
 
+static int mchp_tc_count_cap_read(struct counter_device *counter,
+				  struct counter_count *count, size_t idx, u64 *val)
+{
+	struct mchp_tc_data *const priv = counter_priv(counter);
+	u32 cnt;
+
+	switch (idx) {
+	case 0:
+		regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], RA), &cnt);
+		break;
+	case 1:
+		regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], RB), &cnt);
+		break;
+	case 2:
+		regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], RC), &cnt);
+		break;
+	default:
+		return -EINVAL;
+	}
+	*val = cnt;
+
+	return 0;
+}
+
+static int mchp_tc_count_cap_write(struct counter_device *counter,
+				   struct counter_count *count, size_t idx, u64 val)
+{
+	struct mchp_tc_data *const priv = counter_priv(counter);
+
+	if (val > U32_MAX)
+		return -ERANGE;
+
+	switch (idx) {
+	case 0:
+		regmap_write(priv->regmap, ATMEL_TC_REG(priv->channel[0], RA), val);
+		break;
+	case 1:
+		regmap_write(priv->regmap, ATMEL_TC_REG(priv->channel[0], RB), val);
+		break;
+	case 2:
+		regmap_write(priv->regmap, ATMEL_TC_REG(priv->channel[0], RC), val);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static DEFINE_COUNTER_ARRAY_CAPTURE(mchp_tc_cnt_cap_array, 3);
+
+static struct counter_comp mchp_tc_count_ext[] = {
+	COUNTER_COMP_ARRAY_CAPTURE(mchp_tc_count_cap_read, mchp_tc_count_cap_write,
+				   mchp_tc_cnt_cap_array),
+};
+
 static struct counter_count mchp_tc_counts[] = {
 	{
 		.id = 0,
@@ -260,6 +317,8 @@ static struct counter_count mchp_tc_counts[] = {
 		.num_functions = ARRAY_SIZE(mchp_tc_count_functions),
 		.synapses = mchp_tc_count_synapses,
 		.num_synapses = ARRAY_SIZE(mchp_tc_count_synapses),
+		.ext = mchp_tc_count_ext,
+		.num_ext = ARRAY_SIZE(mchp_tc_count_ext),
 	},
 };
 
@@ -314,6 +373,10 @@ static irqreturn_t mchp_tc_isr(int irq, void *dev_id)
 
 	if (sr & ATMEL_TC_ETRGS)
 		counter_push_event(counter, COUNTER_EVENT_CHANGE_OF_STATE, 0);
+	if (sr & ATMEL_TC_LDRAS)
+		counter_push_event(counter, COUNTER_EVENT_CAPTURE, 0);
+	if (sr & ATMEL_TC_LDRBS)
+		counter_push_event(counter, COUNTER_EVENT_CAPTURE, 1);
 	if (sr & ATMEL_TC_COVFS)
 		counter_push_event(counter, COUNTER_EVENT_OVERFLOW, 0);
 
-- 
2.48.1

Re: [PATCH v2 1/2] counter: microchip-tcb-capture: Add IRQ handling

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

On Mon, Feb 03, 2025 at 05:29:52PM +0100, Bence Csókás wrote:
> Add interrupt servicing to allow userspace to wait for a trigger event.
> 
> Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>

Hi Bence,

Thank you for the patches. When you submit the next revision, please
include a cover letter as that will make it easier for us to separate
high-level discussions about the entire patchset from any individual
patch in particular.

Regarding this particular patch, update the description to mention the
specific types of events supported by this (i.e. Change-of-state and
Overflow events). Even though you expose the capture data in a
subsequent patch, you can push the Capture events in this patch so that
we introduce all the supported interrupt events at once.

Thanks,

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] counter: microchip-tcb-capture: Add capture extensions for registers RA-RC

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

On Mon, Feb 03, 2025 at 05:29:53PM +0100, Bence Csókás wrote:
> TCB hardware is capable of capturing the timer value to registers RA and
> RB. On top, it is capable of triggering on compare against a third
> register, RC. Add these registers as extensions.
> 
> Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>

What is the difference between RA and RB (your code looks like they
represent two distinct event channels, so I want to confirm)? Is RC a
threshold value set by the user, or is it another captured timer value?
The capture extension represents Count value specifically, so I want to
make sure we're using the right extension for these components.

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] counter: microchip-tcb-capture: Add capture extensions for registers RA-RC

[Csókás Bence]

Hi,

On 2025. 02. 05. 1:11, William Breathitt Gray wrote:
> On Mon, Feb 03, 2025 at 05:29:53PM +0100, Bence Csókás wrote:
>> TCB hardware is capable of capturing the timer value to registers RA and
>> RB. On top, it is capable of triggering on compare against a third
>> register, RC. Add these registers as extensions.
>>
>> Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>
> 
> What is the difference between RA and RB (your code looks like they
> represent two distinct event channels, so I want to confirm)? Is RC a
> threshold value set by the user, or is it another captured timer value?
> The capture extension represents Count value specifically, so I want to
> make sure we're using the right extension for these components.

RA, RB and RC are three R/W-able "general purpose" hardware registers. 
The HW additionally has the following capabilities:
* in PWM mode (not used by this driver), send an interrupt on compare to 
RA/B/C, and/or change output signals
* in Counter Capture mode (used here):
   * trigger interrupt on RC compare only
   * on external trigger rising/falling edge, sample Counter Value 
Register (CV) to RA/RB, optionally send an interrupt of this change
   * stop counting after sampling to RB
All of this is enable-able of course.

So yes, RA and RB can be thought of two near-identical, separate 
channels. Since they both sample CV, it made sense to use 
COUNTER_EVENT_CAPTURE. RC is indeed set by the user.

Bence