2024-03-07 13:34:50

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH v5 00/10] counter: Add stm32 timer events support

Improves the stm32 timer counter driver by introducing new signals,
e.g. counting frequency, and missing channels.

It also adds support for interrupt based events using the chrdev interface.
Two event types are added in this series: overflows and capture.

Up to now, stm32 timer counter driver focused mainly on quadrature
encoder feature on channel 1 & 2. It was limited to timers that has
the HW quadrature encoder.

With this series, all timer instances can be enabled for:
- Quadrature encoder on channel 1 and channel 2 (when supported in HW)
- Simple counting on internal clock
- Capture counter value when rising and falling edges are observed at
channel 1 to channel 4 input (number of channels varies depending on
the timer instance used)
- Push overflow and/or capture events through the chardev interface

Changes in v5:
- Rebase on counter-next, where COUNTER_COMP_FREQUENCY() has been applied
- Add collected Reviewed-by

Changes in v4:
- Introduce new COUNTER_COMP_FREQUENCY() macro
- Mainly update two last patchs, to address William's review comments
(details in each patch)

Changes in V3:
- This series has been split [1] and renamed.
[1] https://lore.kernel.org/lkml/[email protected]/
- Watch events tool patch applied, hence dropped from this series
- Refactor / split several patches
- Keep original signals order, to avoid breaking existing user space
- Don't adjust number of signals: adopt static configuration. rely on
error mechanism to manage diversity across STM32 Timers instances.

Changes in v2:
- counter fix and improvement patch applied, hence dropped in v2 series
- mfd patch applied, hence dropped in v2 series
- revisit watch events tool (mainly command line interface)
- add one patch to rename STM32 Timer counter
- various review comments from v1

Fabrice Gasnier (10):
counter: stm32-timer-cnt: rename quadrature signal
counter: stm32-timer-cnt: rename counter
counter: stm32-timer-cnt: adopt signal definitions
counter: stm32-timer-cnt: introduce clock signal
counter: stm32-timer-cnt: add counter prescaler extension
counter: stm32-timer-cnt: add checks on quadrature encoder capability
counter: stm32-timer-cnt: introduce channels
counter: stm32-timer-cnt: probe number of channels from registers
counter: stm32-timer-cnt: add support for overflow events
counter: stm32-timer-cnt: add support for capture events

drivers/counter/stm32-timer-cnt.c | 461 +++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 13 +
2 files changed, 460 insertions(+), 14 deletions(-)

--
2.25.1



2024-03-07 13:35:41

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH v5 05/10] counter: stm32-timer-cnt: add counter prescaler extension

There's a prescaler in between the selected input signal used for
counting (CK_PSC), and the counter input (CK_CNT).
So add the "prescaler" extension to the counter.

Reviewed-by: William Breathitt Gray <[email protected]>
Signed-off-by: Fabrice Gasnier <[email protected]>
---
Changes in v4:
- Add William's Reviewed-by tag
Changes in v3:
- New patch split from "counter: stm32-timer-cnt: introduce clock signal"
---
drivers/counter/stm32-timer-cnt.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index 65b447b42e75..b969d550e90a 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -220,11 +220,40 @@ static int stm32_count_enable_write(struct counter_device *counter,
return 0;
}

+static int stm32_count_prescaler_read(struct counter_device *counter,
+ struct counter_count *count, u64 *prescaler)
+{
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+ u32 psc;
+
+ regmap_read(priv->regmap, TIM_PSC, &psc);
+
+ *prescaler = psc + 1;
+
+ return 0;
+}
+
+static int stm32_count_prescaler_write(struct counter_device *counter,
+ struct counter_count *count, u64 prescaler)
+{
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+ u32 psc;
+
+ if (!prescaler || prescaler > MAX_TIM_PSC + 1)
+ return -ERANGE;
+
+ psc = prescaler - 1;
+
+ return regmap_write(priv->regmap, TIM_PSC, psc);
+}
+
static struct counter_comp stm32_count_ext[] = {
COUNTER_COMP_DIRECTION(stm32_count_direction_read),
COUNTER_COMP_ENABLE(stm32_count_enable_read, stm32_count_enable_write),
COUNTER_COMP_CEILING(stm32_count_ceiling_read,
stm32_count_ceiling_write),
+ COUNTER_COMP_COUNT_U64("prescaler", stm32_count_prescaler_read,
+ stm32_count_prescaler_write),
};

static const enum counter_synapse_action stm32_clock_synapse_actions[] = {
--
2.25.1


2024-03-07 13:35:44

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH v5 07/10] counter: stm32-timer-cnt: introduce channels

Simply add channels 3 and 4 that can be used for capture. Statically
add them, despite some timers doesn't have them. Rather rely on
stm32_action_read that will report "none" action for these currently.

Reviewed-by: William Breathitt Gray <[email protected]>
Signed-off-by: Fabrice Gasnier <[email protected]>
---
Changes in v4:
- Add William's Reviewed-by tag
Changes in v3:
- New patch split from:
"counter: stm32-timer-cnt: populate capture channels and check encoder"
---
drivers/counter/stm32-timer-cnt.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index 17f87ace450d..f63d0c3e3f22 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -25,6 +25,8 @@
#define STM32_CH1_SIG 0
#define STM32_CH2_SIG 1
#define STM32_CLOCK_SIG 2
+#define STM32_CH3_SIG 3
+#define STM32_CH4_SIG 4

struct stm32_timer_regs {
u32 cr1;
@@ -365,6 +367,14 @@ static struct counter_signal stm32_signals[] = {
.ext = stm32_count_clock_ext,
.num_ext = ARRAY_SIZE(stm32_count_clock_ext),
},
+ {
+ .id = STM32_CH3_SIG,
+ .name = "Channel 3"
+ },
+ {
+ .id = STM32_CH4_SIG,
+ .name = "Channel 4"
+ },
};

static struct counter_synapse stm32_count_synapses[] = {
@@ -383,6 +393,16 @@ static struct counter_synapse stm32_count_synapses[] = {
.num_actions = ARRAY_SIZE(stm32_clock_synapse_actions),
.signal = &stm32_signals[STM32_CLOCK_SIG]
},
+ {
+ .actions_list = stm32_synapse_actions,
+ .num_actions = ARRAY_SIZE(stm32_synapse_actions),
+ .signal = &stm32_signals[STM32_CH3_SIG]
+ },
+ {
+ .actions_list = stm32_synapse_actions,
+ .num_actions = ARRAY_SIZE(stm32_synapse_actions),
+ .signal = &stm32_signals[STM32_CH4_SIG]
+ },
};

static struct counter_count stm32_counts = {
--
2.25.1


2024-03-07 13:36:07

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH v5 06/10] counter: stm32-timer-cnt: add checks on quadrature encoder capability

This is a precursor patch to support capture channels on all possible
channels and stm32 timer types. Original driver was intended to be used
only as quadrature encoder and simple counter on internal clock.

So, add a check on encoder capability, so the driver may be probed for
timer instances without encoder feature. This way, all timers may be used
as simple counter on internal clock, starting from here.

Encoder capability is retrieved by using the timer index (originally in
stm32-timer-trigger driver and dt-bindings). The need to keep backward
compatibility with existing device tree lead to parse aside trigger node.

Reviewed-by: William Breathitt Gray <[email protected]>
Signed-off-by: Fabrice Gasnier <[email protected]>
---
Changes in v5:
- Add William's Reviewed-by tag.

Changes in v4:
- drop idx from struct stm32_timer_cnt as unused after probing
- directly use dev struct in stm32_timer_cnt_probe_encoder routine.

Changes in v3:
- New patch split from:
"counter: stm32-timer-cnt: populate capture channels and check encoder"
- return -EOPNOTSUPP when encoder function isn't supported by the timer
instance.
---
drivers/counter/stm32-timer-cnt.c | 55 +++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index b969d550e90a..17f87ace450d 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -11,6 +11,7 @@
#include <linux/mfd/stm32-timers.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/pinctrl/consumer.h>
#include <linux/platform_device.h>
#include <linux/types.h>
@@ -38,6 +39,7 @@ struct stm32_timer_cnt {
u32 max_arr;
bool enabled;
struct stm32_timer_regs bak;
+ bool has_encoder;
};

static const enum counter_function stm32_count_functions[] = {
@@ -111,12 +113,18 @@ static int stm32_count_function_write(struct counter_device *counter,
sms = TIM_SMCR_SMS_SLAVE_MODE_DISABLED;
break;
case COUNTER_FUNCTION_QUADRATURE_X2_A:
+ if (!priv->has_encoder)
+ return -EOPNOTSUPP;
sms = TIM_SMCR_SMS_ENCODER_MODE_1;
break;
case COUNTER_FUNCTION_QUADRATURE_X2_B:
+ if (!priv->has_encoder)
+ return -EOPNOTSUPP;
sms = TIM_SMCR_SMS_ENCODER_MODE_2;
break;
case COUNTER_FUNCTION_QUADRATURE_X4:
+ if (!priv->has_encoder)
+ return -EOPNOTSUPP;
sms = TIM_SMCR_SMS_ENCODER_MODE_3;
break;
default:
@@ -388,6 +396,49 @@ static struct counter_count stm32_counts = {
.num_ext = ARRAY_SIZE(stm32_count_ext)
};

+/* encoder supported on TIM1 TIM2 TIM3 TIM4 TIM5 TIM8 */
+#define STM32_TIM_ENCODER_SUPPORTED (BIT(0) | BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(7))
+
+static const char * const stm32_timer_trigger_compat[] = {
+ "st,stm32-timer-trigger",
+ "st,stm32h7-timer-trigger",
+};
+
+static int stm32_timer_cnt_probe_encoder(struct device *dev,
+ struct stm32_timer_cnt *priv)
+{
+ struct device *parent = dev->parent;
+ struct device_node *tnode = NULL, *pnode = parent->of_node;
+ int i, ret;
+ u32 idx;
+
+ /*
+ * Need to retrieve the trigger node index from DT, to be able
+ * to determine if the counter supports encoder mode. It also
+ * enforce backward compatibility, and allow to support other
+ * counter modes in this driver (when the timer doesn't support
+ * encoder).
+ */
+ for (i = 0; i < ARRAY_SIZE(stm32_timer_trigger_compat) && !tnode; i++)
+ tnode = of_get_compatible_child(pnode, stm32_timer_trigger_compat[i]);
+ if (!tnode) {
+ dev_err(dev, "Can't find trigger node\n");
+ return -ENODATA;
+ }
+
+ ret = of_property_read_u32(tnode, "reg", &idx);
+ if (ret) {
+ dev_err(dev, "Can't get index (%d)\n", ret);
+ return ret;
+ }
+
+ priv->has_encoder = !!(STM32_TIM_ENCODER_SUPPORTED & BIT(idx));
+
+ dev_dbg(dev, "encoder support: %s\n", priv->has_encoder ? "yes" : "no");
+
+ return 0;
+}
+
static int stm32_timer_cnt_probe(struct platform_device *pdev)
{
struct stm32_timers *ddata = dev_get_drvdata(pdev->dev.parent);
@@ -409,6 +460,10 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
priv->clk = ddata->clk;
priv->max_arr = ddata->max_arr;

+ ret = stm32_timer_cnt_probe_encoder(dev, priv);
+ if (ret)
+ return ret;
+
counter->name = dev_name(dev);
counter->parent = dev;
counter->ops = &stm32_timer_cnt_ops;
--
2.25.1


2024-03-07 13:36:29

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH v5 09/10] counter: stm32-timer-cnt: add support for overflow events

Add support overflow events. Also add the related validation and
configuration routine. Register and enable interrupts to push events.
STM32 Timers can have either 1 global interrupt, or 4 dedicated interrupt
lines. Request only the necessary interrupt, e.g. either global interrupt
that can report all event types, or update interrupt only for overflow
event.

Reviewed-by: William Breathitt Gray <[email protected]>
Signed-off-by: Fabrice Gasnier <[email protected]>
---
Changes in v5:
- Add William's Reviewed-by tag.

Changes in v4:
- drop *irq from stm32_timer_cnt, as only used at probe time.
- Declare nb_ovf as u64 instead of atomic_t, to fit the API, and avoid
unsuitable range check. This also avoid checking for negative value.
Use a spin_lock to protect it, as it is updated in interrupt context.
- use regmap_test_bits() to avoid intermediate variable.
- fix error message in probe (ddata->irq[0] instead of ddata->irq[i])
- move mfd header to subsequent patch, where the defines are actually
used.

Changes in v3:
- patch split from: "counter: stm32-timer-cnt: add support for events", to
focus on the overflow events only here. Move the capture event support
to a separate patch.
- simplify the patch, by removing diversity introduced by the number of
possible channels. All channels are now exposed instead.
Report an error when trying to access a channel that doesn't exist.

Changes in v2:
- fix warnings (kernel test robot)
- fix a typo
- add collected ack from Lee
---
drivers/counter/stm32-timer-cnt.c | 138 +++++++++++++++++++++++++++++-
1 file changed, 137 insertions(+), 1 deletion(-)

diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index e1c0a502b74c..9fcafec682b7 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -8,6 +8,7 @@
*
*/
#include <linux/counter.h>
+#include <linux/interrupt.h>
#include <linux/mfd/stm32-timers.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
@@ -43,6 +44,9 @@ struct stm32_timer_cnt {
struct stm32_timer_regs bak;
bool has_encoder;
unsigned int nchannels;
+ unsigned int nr_irqs;
+ spinlock_t lock; /* protects nb_ovf */
+ u64 nb_ovf;
};

static const enum counter_function stm32_count_functions[] = {
@@ -258,6 +262,32 @@ static int stm32_count_prescaler_write(struct counter_device *counter,
return regmap_write(priv->regmap, TIM_PSC, psc);
}

+static int stm32_count_nb_ovf_read(struct counter_device *counter,
+ struct counter_count *count, u64 *val)
+{
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+ unsigned long irqflags;
+
+ spin_lock_irqsave(&priv->lock, irqflags);
+ *val = priv->nb_ovf;
+ spin_unlock_irqrestore(&priv->lock, irqflags);
+
+ return 0;
+}
+
+static int stm32_count_nb_ovf_write(struct counter_device *counter,
+ struct counter_count *count, u64 val)
+{
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+ unsigned long irqflags;
+
+ spin_lock_irqsave(&priv->lock, irqflags);
+ priv->nb_ovf = val;
+ spin_unlock_irqrestore(&priv->lock, irqflags);
+
+ return 0;
+}
+
static struct counter_comp stm32_count_ext[] = {
COUNTER_COMP_DIRECTION(stm32_count_direction_read),
COUNTER_COMP_ENABLE(stm32_count_enable_read, stm32_count_enable_write),
@@ -265,6 +295,7 @@ static struct counter_comp stm32_count_ext[] = {
stm32_count_ceiling_write),
COUNTER_COMP_COUNT_U64("prescaler", stm32_count_prescaler_read,
stm32_count_prescaler_write),
+ COUNTER_COMP_COUNT_U64("num_overflows", stm32_count_nb_ovf_read, stm32_count_nb_ovf_write),
};

static const enum counter_synapse_action stm32_clock_synapse_actions[] = {
@@ -322,12 +353,57 @@ static int stm32_action_read(struct counter_device *counter,
}
}

+static int stm32_count_events_configure(struct counter_device *counter)
+{
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+ struct counter_event_node *event_node;
+ u32 dier = 0;
+
+ list_for_each_entry(event_node, &counter->events_list, l) {
+ switch (event_node->event) {
+ case COUNTER_EVENT_OVERFLOW_UNDERFLOW:
+ /* first clear possibly latched UIF before enabling */
+ if (!regmap_test_bits(priv->regmap, TIM_DIER, TIM_DIER_UIE))
+ regmap_write(priv->regmap, TIM_SR, (u32)~TIM_SR_UIF);
+ dier |= TIM_DIER_UIE;
+ break;
+ default:
+ /* should never reach this path */
+ return -EINVAL;
+ }
+ }
+
+ /* Enable / disable all events at once, from events_list, so write all DIER bits */
+ regmap_write(priv->regmap, TIM_DIER, dier);
+
+ return 0;
+}
+
+static int stm32_count_watch_validate(struct counter_device *counter,
+ const struct counter_watch *watch)
+{
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+
+ /* Interrupts are optional */
+ if (!priv->nr_irqs)
+ return -EOPNOTSUPP;
+
+ switch (watch->event) {
+ case COUNTER_EVENT_OVERFLOW_UNDERFLOW:
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
static const struct counter_ops stm32_timer_cnt_ops = {
.count_read = stm32_count_read,
.count_write = stm32_count_write,
.function_read = stm32_count_function_read,
.function_write = stm32_count_function_write,
.action_read = stm32_action_read,
+ .events_configure = stm32_count_events_configure,
+ .watch_validate = stm32_count_watch_validate,
};

static int stm32_count_clk_get_freq(struct counter_device *counter,
@@ -417,6 +493,37 @@ static struct counter_count stm32_counts = {
.num_ext = ARRAY_SIZE(stm32_count_ext)
};

+static irqreturn_t stm32_timer_cnt_isr(int irq, void *ptr)
+{
+ struct counter_device *counter = ptr;
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+ u32 clr = GENMASK(31, 0); /* SR flags can be cleared by writing 0 (wr 1 has no effect) */
+ u32 sr, dier;
+
+ regmap_read(priv->regmap, TIM_SR, &sr);
+ regmap_read(priv->regmap, TIM_DIER, &dier);
+ /*
+ * Some status bits in SR don't match with the enable bits in DIER. Only take care of
+ * the possibly enabled bits in DIER (that matches in between SR and DIER).
+ */
+ dier &= TIM_DIER_UIE;
+ sr &= dier;
+
+ if (sr & TIM_SR_UIF) {
+ spin_lock(&priv->lock);
+ priv->nb_ovf++;
+ spin_unlock(&priv->lock);
+ counter_push_event(counter, COUNTER_EVENT_OVERFLOW_UNDERFLOW, 0);
+ dev_dbg(counter->parent, "COUNTER_EVENT_OVERFLOW_UNDERFLOW\n");
+ /* SR flags can be cleared by writing 0, only clear relevant flag */
+ clr &= ~TIM_SR_UIF;
+ }
+
+ regmap_write(priv->regmap, TIM_SR, clr);
+
+ return IRQ_HANDLED;
+};
+
static void stm32_timer_cnt_detect_channels(struct device *dev,
struct stm32_timer_cnt *priv)
{
@@ -480,7 +587,7 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct stm32_timer_cnt *priv;
struct counter_device *counter;
- int ret;
+ int i, ret;

if (IS_ERR_OR_NULL(ddata))
return -EINVAL;
@@ -494,6 +601,7 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
priv->regmap = ddata->regmap;
priv->clk = ddata->clk;
priv->max_arr = ddata->max_arr;
+ priv->nr_irqs = ddata->nr_irqs;

ret = stm32_timer_cnt_probe_encoder(dev, priv);
if (ret)
@@ -509,8 +617,36 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
counter->signals = stm32_signals;
counter->num_signals = ARRAY_SIZE(stm32_signals);

+ spin_lock_init(&priv->lock);
+
platform_set_drvdata(pdev, priv);

+ /* STM32 Timers can have either 1 global, or 4 dedicated interrupts (optional) */
+ if (priv->nr_irqs == 1) {
+ /* All events reported through the global interrupt */
+ ret = devm_request_irq(&pdev->dev, ddata->irq[0], stm32_timer_cnt_isr,
+ 0, dev_name(dev), counter);
+ if (ret) {
+ dev_err(dev, "Failed to request irq %d (err %d)\n",
+ ddata->irq[0], ret);
+ return ret;
+ }
+ } else {
+ for (i = 0; i < priv->nr_irqs; i++) {
+ /* Only take care of update IRQ for overflow events */
+ if (i != STM32_TIMERS_IRQ_UP)
+ continue;
+
+ ret = devm_request_irq(&pdev->dev, ddata->irq[i], stm32_timer_cnt_isr,
+ 0, dev_name(dev), counter);
+ if (ret) {
+ dev_err(dev, "Failed to request irq %d (err %d)\n",
+ ddata->irq[i], ret);
+ return ret;
+ }
+ }
+ }
+
/* Reset input selector to its default input */
regmap_write(priv->regmap, TIM_TISEL, 0x0);

--
2.25.1


2024-03-07 13:37:36

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH v5 08/10] counter: stm32-timer-cnt: probe number of channels from registers

Probe the number of capture compare channels, by writing CCER register bits
and read them back. Take care to restore the register original value.

This is a precursor patch to support capture channels.

Reviewed-by: William Breathitt Gray <[email protected]>
Signed-off-by: Fabrice Gasnier <[email protected]>
---
Changes in v5:
- Add William's Reviewed-by tag.

Changes in v4:
- directly use dev struct in stm32_timer_cnt_detect_channels routine.

Changes in v3:
- New patch split from:
"counter: stm32-timer-cnt: populate capture channels and check encoder"
---
drivers/counter/stm32-timer-cnt.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index f63d0c3e3f22..e1c0a502b74c 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -42,6 +42,7 @@ struct stm32_timer_cnt {
bool enabled;
struct stm32_timer_regs bak;
bool has_encoder;
+ unsigned int nchannels;
};

static const enum counter_function stm32_count_functions[] = {
@@ -416,6 +417,20 @@ static struct counter_count stm32_counts = {
.num_ext = ARRAY_SIZE(stm32_count_ext)
};

+static void stm32_timer_cnt_detect_channels(struct device *dev,
+ struct stm32_timer_cnt *priv)
+{
+ u32 ccer, ccer_backup;
+
+ regmap_read(priv->regmap, TIM_CCER, &ccer_backup);
+ regmap_set_bits(priv->regmap, TIM_CCER, TIM_CCER_CCXE);
+ regmap_read(priv->regmap, TIM_CCER, &ccer);
+ regmap_write(priv->regmap, TIM_CCER, ccer_backup);
+ priv->nchannels = hweight32(ccer & TIM_CCER_CCXE);
+
+ dev_dbg(dev, "has %d cc channels\n", priv->nchannels);
+}
+
/* encoder supported on TIM1 TIM2 TIM3 TIM4 TIM5 TIM8 */
#define STM32_TIM_ENCODER_SUPPORTED (BIT(0) | BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(7))

@@ -484,6 +499,8 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
if (ret)
return ret;

+ stm32_timer_cnt_detect_channels(dev, priv);
+
counter->name = dev_name(dev);
counter->parent = dev;
counter->ops = &stm32_timer_cnt_ops;
--
2.25.1


2024-03-07 13:38:45

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH v5 10/10] counter: stm32-timer-cnt: add support for capture events

Add support for capture events. Captured counter value for each channel
can be retrieved through CCRx register.
STM32 timers can have up to 4 capture channels (on input channel 1 to
channel 4), hence need to check the number of channels before reading
the capture data.
The capture configuration is hard-coded to capture signals on both edges
(non-inverted). Interrupts are used to report events independently for
each channel.

Reviewed-by: William Breathitt Gray <[email protected]>
Acked-by: Lee Jones <[email protected]>
Signed-off-by: Fabrice Gasnier <[email protected]>
---
Changes in v5:
- Add William's Reviewed-by tag.

Changes in v4:
- move registers definition from previous patch to here. That's where
the defines are actually used. So move the collected Acked-by: Lee
for the mfd header here.
- drop *irq from stm32_timer_cnt, as only used at probe time.
- adopt some of the suggestions from William: introduce cc local variable,
use regmap_test_bits().

Changes in v3:
- patch split from: "counter: stm32-timer-cnt: add support for events", to
focus on the capture events only here.
- only get relevant interrupt line
---
drivers/counter/stm32-timer-cnt.c | 134 +++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 13 +++
2 files changed, 144 insertions(+), 3 deletions(-)

diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index 9fcafec682b7..0664ef969f79 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -262,6 +262,40 @@ static int stm32_count_prescaler_write(struct counter_device *counter,
return regmap_write(priv->regmap, TIM_PSC, psc);
}

+static int stm32_count_cap_read(struct counter_device *counter,
+ struct counter_count *count,
+ size_t ch, u64 *cap)
+{
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+ u32 ccrx;
+
+ if (ch >= priv->nchannels)
+ return -EOPNOTSUPP;
+
+ switch (ch) {
+ case 0:
+ regmap_read(priv->regmap, TIM_CCR1, &ccrx);
+ break;
+ case 1:
+ regmap_read(priv->regmap, TIM_CCR2, &ccrx);
+ break;
+ case 2:
+ regmap_read(priv->regmap, TIM_CCR3, &ccrx);
+ break;
+ case 3:
+ regmap_read(priv->regmap, TIM_CCR4, &ccrx);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ dev_dbg(counter->parent, "CCR%zu: 0x%08x\n", ch + 1, ccrx);
+
+ *cap = ccrx;
+
+ return 0;
+}
+
static int stm32_count_nb_ovf_read(struct counter_device *counter,
struct counter_count *count, u64 *val)
{
@@ -288,6 +322,8 @@ static int stm32_count_nb_ovf_write(struct counter_device *counter,
return 0;
}

+static DEFINE_COUNTER_ARRAY_CAPTURE(stm32_count_cap_array, 4);
+
static struct counter_comp stm32_count_ext[] = {
COUNTER_COMP_DIRECTION(stm32_count_direction_read),
COUNTER_COMP_ENABLE(stm32_count_enable_read, stm32_count_enable_write),
@@ -295,6 +331,7 @@ static struct counter_comp stm32_count_ext[] = {
stm32_count_ceiling_write),
COUNTER_COMP_COUNT_U64("prescaler", stm32_count_prescaler_read,
stm32_count_prescaler_write),
+ COUNTER_COMP_ARRAY_CAPTURE(stm32_count_cap_read, NULL, stm32_count_cap_array),
COUNTER_COMP_COUNT_U64("num_overflows", stm32_count_nb_ovf_read, stm32_count_nb_ovf_write),
};

@@ -353,11 +390,68 @@ static int stm32_action_read(struct counter_device *counter,
}
}

+struct stm32_count_cc_regs {
+ u32 ccmr_reg;
+ u32 ccmr_mask;
+ u32 ccmr_bits;
+ u32 ccer_bits;
+};
+
+static const struct stm32_count_cc_regs stm32_cc[] = {
+ { TIM_CCMR1, TIM_CCMR_CC1S, TIM_CCMR_CC1S_TI1,
+ TIM_CCER_CC1E | TIM_CCER_CC1P | TIM_CCER_CC1NP },
+ { TIM_CCMR1, TIM_CCMR_CC2S, TIM_CCMR_CC2S_TI2,
+ TIM_CCER_CC2E | TIM_CCER_CC2P | TIM_CCER_CC2NP },
+ { TIM_CCMR2, TIM_CCMR_CC3S, TIM_CCMR_CC3S_TI3,
+ TIM_CCER_CC3E | TIM_CCER_CC3P | TIM_CCER_CC3NP },
+ { TIM_CCMR2, TIM_CCMR_CC4S, TIM_CCMR_CC4S_TI4,
+ TIM_CCER_CC4E | TIM_CCER_CC4P | TIM_CCER_CC4NP },
+};
+
+static int stm32_count_capture_configure(struct counter_device *counter, unsigned int ch,
+ bool enable)
+{
+ struct stm32_timer_cnt *const priv = counter_priv(counter);
+ const struct stm32_count_cc_regs *cc;
+ u32 ccmr, ccer;
+
+ if (ch >= ARRAY_SIZE(stm32_cc) || ch >= priv->nchannels) {
+ dev_err(counter->parent, "invalid ch: %d\n", ch);
+ return -EINVAL;
+ }
+
+ cc = &stm32_cc[ch];
+
+ /*
+ * configure channel in input capture mode, map channel 1 on TI1, channel2 on TI2...
+ * Select both edges / non-inverted to trigger a capture.
+ */
+ if (enable) {
+ /* first clear possibly latched capture flag upon enabling */
+ if (!regmap_test_bits(priv->regmap, TIM_CCER, cc->ccer_bits))
+ regmap_write(priv->regmap, TIM_SR, ~TIM_SR_CC_IF(ch));
+ regmap_update_bits(priv->regmap, cc->ccmr_reg, cc->ccmr_mask,
+ cc->ccmr_bits);
+ regmap_set_bits(priv->regmap, TIM_CCER, cc->ccer_bits);
+ } else {
+ regmap_clear_bits(priv->regmap, TIM_CCER, cc->ccer_bits);
+ regmap_clear_bits(priv->regmap, cc->ccmr_reg, cc->ccmr_mask);
+ }
+
+ regmap_read(priv->regmap, cc->ccmr_reg, &ccmr);
+ regmap_read(priv->regmap, TIM_CCER, &ccer);
+ dev_dbg(counter->parent, "%s(%s) ch%d 0x%08x 0x%08x\n", __func__, enable ? "ena" : "dis",
+ ch, ccmr, ccer);
+
+ return 0;
+}
+
static int stm32_count_events_configure(struct counter_device *counter)
{
struct stm32_timer_cnt *const priv = counter_priv(counter);
struct counter_event_node *event_node;
u32 dier = 0;
+ int i, ret;

list_for_each_entry(event_node, &counter->events_list, l) {
switch (event_node->event) {
@@ -367,6 +461,12 @@ static int stm32_count_events_configure(struct counter_device *counter)
regmap_write(priv->regmap, TIM_SR, (u32)~TIM_SR_UIF);
dier |= TIM_DIER_UIE;
break;
+ case COUNTER_EVENT_CAPTURE:
+ ret = stm32_count_capture_configure(counter, event_node->channel, true);
+ if (ret)
+ return ret;
+ dier |= TIM_DIER_CC_IE(event_node->channel);
+ break;
default:
/* should never reach this path */
return -EINVAL;
@@ -376,6 +476,15 @@ static int stm32_count_events_configure(struct counter_device *counter)
/* Enable / disable all events at once, from events_list, so write all DIER bits */
regmap_write(priv->regmap, TIM_DIER, dier);

+ /* check for disabled capture events */
+ for (i = 0 ; i < priv->nchannels; i++) {
+ if (!(dier & TIM_DIER_CC_IE(i))) {
+ ret = stm32_count_capture_configure(counter, i, false);
+ if (ret)
+ return ret;
+ }
+ }
+
return 0;
}

@@ -389,6 +498,12 @@ static int stm32_count_watch_validate(struct counter_device *counter,
return -EOPNOTSUPP;

switch (watch->event) {
+ case COUNTER_EVENT_CAPTURE:
+ if (watch->channel >= priv->nchannels) {
+ dev_err(counter->parent, "Invalid channel %d\n", watch->channel);
+ return -EINVAL;
+ }
+ return 0;
case COUNTER_EVENT_OVERFLOW_UNDERFLOW:
return 0;
default:
@@ -499,6 +614,7 @@ static irqreturn_t stm32_timer_cnt_isr(int irq, void *ptr)
struct stm32_timer_cnt *const priv = counter_priv(counter);
u32 clr = GENMASK(31, 0); /* SR flags can be cleared by writing 0 (wr 1 has no effect) */
u32 sr, dier;
+ int i;

regmap_read(priv->regmap, TIM_SR, &sr);
regmap_read(priv->regmap, TIM_DIER, &dier);
@@ -506,7 +622,7 @@ static irqreturn_t stm32_timer_cnt_isr(int irq, void *ptr)
* Some status bits in SR don't match with the enable bits in DIER. Only take care of
* the possibly enabled bits in DIER (that matches in between SR and DIER).
*/
- dier &= TIM_DIER_UIE;
+ dier &= (TIM_DIER_UIE | TIM_DIER_CC1IE | TIM_DIER_CC2IE | TIM_DIER_CC3IE | TIM_DIER_CC4IE);
sr &= dier;

if (sr & TIM_SR_UIF) {
@@ -519,6 +635,15 @@ static irqreturn_t stm32_timer_cnt_isr(int irq, void *ptr)
clr &= ~TIM_SR_UIF;
}

+ /* Check capture events */
+ for (i = 0 ; i < priv->nchannels; i++) {
+ if (sr & TIM_SR_CC_IF(i)) {
+ counter_push_event(counter, COUNTER_EVENT_CAPTURE, i);
+ clr &= ~TIM_SR_CC_IF(i);
+ dev_dbg(counter->parent, "COUNTER_EVENT_CAPTURE, %d\n", i);
+ }
+ }
+
regmap_write(priv->regmap, TIM_SR, clr);

return IRQ_HANDLED;
@@ -633,8 +758,11 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
}
} else {
for (i = 0; i < priv->nr_irqs; i++) {
- /* Only take care of update IRQ for overflow events */
- if (i != STM32_TIMERS_IRQ_UP)
+ /*
+ * Only take care of update IRQ for overflow events, and cc for
+ * capture events.
+ */
+ if (i != STM32_TIMERS_IRQ_UP && i != STM32_TIMERS_IRQ_CC)
continue;

ret = devm_request_irq(&pdev->dev, ddata->irq[i], stm32_timer_cnt_isr,
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index ca35af30745f..9eb17481b07f 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -41,6 +41,11 @@
#define TIM_SMCR_SMS (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
#define TIM_SMCR_TS (BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
#define TIM_DIER_UIE BIT(0) /* Update interrupt */
+#define TIM_DIER_CC1IE BIT(1) /* CC1 Interrupt Enable */
+#define TIM_DIER_CC2IE BIT(2) /* CC2 Interrupt Enable */
+#define TIM_DIER_CC3IE BIT(3) /* CC3 Interrupt Enable */
+#define TIM_DIER_CC4IE BIT(4) /* CC4 Interrupt Enable */
+#define TIM_DIER_CC_IE(x) BIT((x) + 1) /* CC1, CC2, CC3, CC4 interrupt enable */
#define TIM_DIER_UDE BIT(8) /* Update DMA request Enable */
#define TIM_DIER_CC1DE BIT(9) /* CC1 DMA request Enable */
#define TIM_DIER_CC2DE BIT(10) /* CC2 DMA request Enable */
@@ -49,6 +54,7 @@
#define TIM_DIER_COMDE BIT(13) /* COM DMA request Enable */
#define TIM_DIER_TDE BIT(14) /* Trigger DMA request Enable */
#define TIM_SR_UIF BIT(0) /* Update interrupt flag */
+#define TIM_SR_CC_IF(x) BIT((x) + 1) /* CC1, CC2, CC3, CC4 interrupt flag */
#define TIM_EGR_UG BIT(0) /* Update Generation */
#define TIM_CCMR_PE BIT(3) /* Channel Preload Enable */
#define TIM_CCMR_M1 (BIT(6) | BIT(5)) /* Channel PWM Mode 1 */
@@ -60,16 +66,23 @@
#define TIM_CCMR_CC1S_TI2 BIT(1) /* IC1/IC3 selects TI2/TI4 */
#define TIM_CCMR_CC2S_TI2 BIT(8) /* IC2/IC4 selects TI2/TI4 */
#define TIM_CCMR_CC2S_TI1 BIT(9) /* IC2/IC4 selects TI1/TI3 */
+#define TIM_CCMR_CC3S (BIT(0) | BIT(1)) /* Capture/compare 3 sel */
+#define TIM_CCMR_CC4S (BIT(8) | BIT(9)) /* Capture/compare 4 sel */
+#define TIM_CCMR_CC3S_TI3 BIT(0) /* IC3 selects TI3 */
+#define TIM_CCMR_CC4S_TI4 BIT(8) /* IC4 selects TI4 */
#define TIM_CCER_CC1E BIT(0) /* Capt/Comp 1 out Ena */
#define TIM_CCER_CC1P BIT(1) /* Capt/Comp 1 Polarity */
#define TIM_CCER_CC1NE BIT(2) /* Capt/Comp 1N out Ena */
#define TIM_CCER_CC1NP BIT(3) /* Capt/Comp 1N Polarity */
#define TIM_CCER_CC2E BIT(4) /* Capt/Comp 2 out Ena */
#define TIM_CCER_CC2P BIT(5) /* Capt/Comp 2 Polarity */
+#define TIM_CCER_CC2NP BIT(7) /* Capt/Comp 2N Polarity */
#define TIM_CCER_CC3E BIT(8) /* Capt/Comp 3 out Ena */
#define TIM_CCER_CC3P BIT(9) /* Capt/Comp 3 Polarity */
+#define TIM_CCER_CC3NP BIT(11) /* Capt/Comp 3N Polarity */
#define TIM_CCER_CC4E BIT(12) /* Capt/Comp 4 out Ena */
#define TIM_CCER_CC4P BIT(13) /* Capt/Comp 4 Polarity */
+#define TIM_CCER_CC4NP BIT(15) /* Capt/Comp 4N Polarity */
#define TIM_CCER_CCXE (BIT(0) | BIT(4) | BIT(8) | BIT(12))
#define TIM_BDTR_BKE(x) BIT(12 + (x) * 12) /* Break input enable */
#define TIM_BDTR_BKP(x) BIT(13 + (x) * 12) /* Break input polarity */
--
2.25.1


2024-03-07 16:10:20

by William Breathitt Gray

[permalink] [raw]
Subject: Re: [PATCH v5 00/10] counter: Add stm32 timer events support


On Thu, 07 Mar 2024 14:32:56 +0100, Fabrice Gasnier wrote:
> Improves the stm32 timer counter driver by introducing new signals,
> e.g. counting frequency, and missing channels.
>
> It also adds support for interrupt based events using the chrdev interface.
> Two event types are added in this series: overflows and capture.
>
> Up to now, stm32 timer counter driver focused mainly on quadrature
> encoder feature on channel 1 & 2. It was limited to timers that has
> the HW quadrature encoder.
>
> [...]

Applied, thanks!

[01/10] counter: stm32-timer-cnt: rename quadrature signal
commit: 74326ec28e2794e352284f619faa967b4744b79f
[02/10] counter: stm32-timer-cnt: rename counter
commit: 017edec6629109d14e8d0caf2539998f7912ec80
[03/10] counter: stm32-timer-cnt: adopt signal definitions
commit: 2017a23fc03c9c6d0aab7775ae6a91b4b0a7b84e
[04/10] counter: stm32-timer-cnt: introduce clock signal
commit: 70f8e3252192451bdb5575dc6f931655a45764a3
[05/10] counter: stm32-timer-cnt: add counter prescaler extension
commit: 18766e036b33ea2cd017cf49cb0be6504a8821f6
[06/10] counter: stm32-timer-cnt: add checks on quadrature encoder capability
commit: 395db2e2de1434c9f361db5a8ebcae07086826a0
[07/10] counter: stm32-timer-cnt: introduce channels
commit: ed54da4ae5e10ea104aca4feab9a7f413ec4ace6
[08/10] counter: stm32-timer-cnt: probe number of channels from registers
commit: 1f4552e70ca8ff36c59b7ffe2a7c277c23ea274a
[09/10] counter: stm32-timer-cnt: add support for overflow events
commit: b30011f67e90076476d843f416085be7dead3bf1
[10/10] counter: stm32-timer-cnt: add support for capture events
commit: a297b4a46ffd195a87532d37a2c344240d7d5ee4

William Breathitt Gray <[email protected]>