[PATCH v2 6/7] counter: Add rockchip-pwm-capture driver

Nicolas Frattaroli posted 7 patches 6 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 6/7] counter: Add rockchip-pwm-capture driver
Posted by Nicolas Frattaroli 6 months, 2 weeks ago
Among many other things, Rockchip's new PWMv4 IP in the RK3576 supports
PWM capture functionality.

Add a basic driver for this that works to capture period and duty cycle
values and return them as nanoseconds to the user. It's quite basic, but
works well enough to demonstrate the device function exclusion stuff
that mfpwm does, in order to eventually support all the functions of
this device in drivers within their appropriate subsystems, without them
interfering with each other.

Once enabled, the counter driver waits for enough high-to-low and
low-to-high interrupt signals to arrive, and then writes the cycle count
register values into some atomic members of the driver instance's state
struct. The read callback can then do the conversion from cycle count to
the more useful period and duty cycle nanosecond values, which require
knowledge of the clock rate, which requires a call that the interrupt
handler cannot make itself because said call may sleep.

To detect the condition of a PWM signal disappearing, i.e. turning off,
we modify the delay value of a delayed worker whose job it is to simply
set those atomic members to zero. Should the "timeout" so to speak be
reached, we assume the PWM signal is off. This isn't perfect; it
obviously introduces a latency between it being off and the counter
reporting it as such. Because there isn't a way to reset the internal
double-buffered cycle count in the hardware, we filter out unreliable
periods above the timeout value in the counter read callback.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 MAINTAINERS                            |   1 +
 drivers/counter/Kconfig                |  13 ++
 drivers/counter/Makefile               |   1 +
 drivers/counter/rockchip-pwm-capture.c | 352 +++++++++++++++++++++++++++++++++
 4 files changed, 367 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0014af1cbd3a7729a04bfdd0b0ed50e9df425693..9690f00053df17f8459aa2c6c8f0c62c6a25107e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21424,6 +21424,7 @@ L:	linux-rockchip@lists.infradead.org
 L:	linux-pwm@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/pwm/rockchip,rk3576-pwm.yaml
+F:	drivers/counter/rockchip-pwm-capture.c
 F:	drivers/pwm/pwm-rockchip-v4.c
 F:	drivers/soc/rockchip/mfpwm.c
 F:	include/soc/rockchip/mfpwm.h
diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
index d30d22dfe57741b145a45632b6325d5f9680590e..01b4f5c326478c73b518041830ee0d65b37f6833 100644
--- a/drivers/counter/Kconfig
+++ b/drivers/counter/Kconfig
@@ -90,6 +90,19 @@ config MICROCHIP_TCB_CAPTURE
 	  To compile this driver as a module, choose M here: the
 	  module will be called microchip-tcb-capture.
 
+config ROCKCHIP_PWM_CAPTURE
+	tristate "Rockchip PWM Counter Capture driver"
+	depends on ARCH_ROCKCHIP || COMPILE_TEST
+	depends on ROCKCHIP_MFPWM
+	depends on HAS_IOMEM
+	help
+	  Generic counter framework driver for the multi-function PWM on
+	  Rockchip SoCs such as the RK3576.
+
+	  Uses the Rockchip Multi-function PWM controller driver infrastructure
+	  to guarantee exclusive operation with other functions of the same
+	  device implemented by drivers in other subsystems.
+
 config RZ_MTU3_CNT
 	tristate "Renesas RZ/G2L MTU3a counter driver"
 	depends on RZ_MTU3
diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
index fa3c1d08f7068835aa912aa13bc92bcfd44d16fb..2bfcfc2c584bd174a9885064746a98f15b204aec 100644
--- a/drivers/counter/Makefile
+++ b/drivers/counter/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_FTM_QUADDEC)	+= ftm-quaddec.o
 obj-$(CONFIG_MICROCHIP_TCB_CAPTURE)	+= microchip-tcb-capture.o
 obj-$(CONFIG_INTEL_QEP)		+= intel-qep.o
 obj-$(CONFIG_TI_ECAP_CAPTURE)	+= ti-ecap-capture.o
+obj-$(CONFIG_ROCKCHIP_PWM_CAPTURE)	+= rockchip-pwm-capture.o
diff --git a/drivers/counter/rockchip-pwm-capture.c b/drivers/counter/rockchip-pwm-capture.c
new file mode 100644
index 0000000000000000000000000000000000000000..03fe9cf32e7f273c0091c4743642eda6bee76222
--- /dev/null
+++ b/drivers/counter/rockchip-pwm-capture.c
@@ -0,0 +1,352 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2025 Collabora Ltd.
+ *
+ * A counter driver for the Pulse-Width-Modulation (PWM) hardware found on
+ * Rockchip SoCs such as the RK3576, internally referred to as "PWM v4". It
+ * allows for measuring the period and duty cycle in nanoseconds through the
+ * generic counter framework, while guaranteeing exclusive use over the MFPWM
+ * device while the counter is enabled.
+ *
+ * Authors:
+ *     Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
+ */
+
+#include <linux/cleanup.h>
+#include <linux/counter.h>
+#include <linux/devm-helpers.h>
+#include <linux/interrupt.h>
+#include <linux/math64.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+#include <soc/rockchip/mfpwm.h>
+
+#define RKPWMC_INT_MASK			(PWMV4_INT_LPC | PWMV4_INT_HPC)
+/*
+ * amount of jiffies between no PWM signal change and us deciding PWM is off.
+ * PWM signals with a period longer than this won't be detected by us, which is
+ * the trade-off we make to have a faster response time when a signal is turned
+ * off.
+ */
+#define RKPWMC_CLEAR_DELAY		(max(msecs_to_jiffies(100), 1))
+
+struct rockchip_pwm_capture {
+	struct rockchip_mfpwm_func *pwmf;
+	bool is_enabled;
+	spinlock_t enable_lock;
+	atomic_t lpc;
+	atomic_t hpc;
+	atomic_t captures_left;
+	struct delayed_work clear_capture;
+};
+
+static struct counter_signal rkpwmc_signals[] = {
+	{
+		.id = 0,
+		.name = "Channel 1"
+	},
+};
+
+static const enum counter_synapse_action rkpwmc_hpc_lpc_actions[] = {
+	COUNTER_SYNAPSE_ACTION_BOTH_EDGES,
+
+};
+
+static struct counter_synapse rkpwmc_pwm_synapses[] = {
+	{
+		.actions_list = rkpwmc_hpc_lpc_actions,
+		.num_actions = ARRAY_SIZE(rkpwmc_hpc_lpc_actions),
+		.signal = &rkpwmc_signals[0]
+	},
+};
+
+static const enum counter_function rkpwmc_functions[] = {
+	COUNTER_FUNCTION_INCREASE,
+};
+
+static int rkpwmc_enable_read(struct counter_device *counter,
+			       struct counter_count *count,
+			       u8 *enable)
+{
+	struct rockchip_pwm_capture *pc = counter_priv(counter);
+
+	guard(spinlock)(&pc->enable_lock);
+
+	*enable = pc->is_enabled;
+
+	return 0;
+}
+
+static int rkpwmc_enable_write(struct counter_device *counter,
+			       struct counter_count *count,
+			       u8 enable)
+{
+	struct rockchip_pwm_capture *pc = counter_priv(counter);
+	int ret;
+
+	guard(spinlock)(&pc->enable_lock);
+
+	if (!!enable != pc->is_enabled) {
+		ret = mfpwm_acquire(pc->pwmf);
+		if (ret)
+			return ret;
+
+		if (enable) {
+			mfpwm_reg_write(pc->pwmf->base, PWMV4_REG_ENABLE,
+					 PWMV4_EN(false));
+			mfpwm_reg_write(pc->pwmf->base, PWMV4_REG_CTRL,
+					 PWMV4_CTRL_CAP_FLAGS);
+			mfpwm_reg_write(pc->pwmf->base, PWMV4_REG_INT_EN,
+					 PWMV4_INT_LPC_W(true) |
+					 PWMV4_INT_HPC_W(true));
+			mfpwm_reg_write(pc->pwmf->base, PWMV4_REG_ENABLE,
+					 PWMV4_EN(true) | PWMV4_CLK_EN(true));
+
+			ret = clk_enable(pc->pwmf->core);
+			if (ret)
+				goto err_release;
+
+			ret = clk_rate_exclusive_get(pc->pwmf->core);
+			if (ret)
+				goto err_disable_pwm_clk;
+
+			ret = mfpwm_acquire(pc->pwmf);
+			if (ret)
+				goto err_unprotect_pwm_clk;
+
+			atomic_set(&pc->captures_left, 4);
+			pc->is_enabled = true;
+		} else {
+			mfpwm_reg_write(pc->pwmf->base, PWMV4_REG_INT_EN,
+					 PWMV4_INT_LPC_W(false) |
+					 PWMV4_INT_HPC_W(false));
+			mfpwm_reg_write(pc->pwmf->base, PWMV4_REG_ENABLE,
+					 PWMV4_EN(false) | PWMV4_CLK_EN(false));
+			/*
+			 * Do not use cancel_delayed_work here unless you want
+			 * to cause the interrupt handler, which may still be
+			 * running at this point, to stall. Similarly, don't do
+			 * flush_delayed_work since it may sleep.
+			 */
+			mod_delayed_work(system_bh_wq, &pc->clear_capture, 0);
+			clk_rate_exclusive_put(pc->pwmf->core);
+			clk_disable(pc->pwmf->core);
+			pc->is_enabled = false;
+			mfpwm_release(pc->pwmf);
+		}
+
+		mfpwm_release(pc->pwmf);
+	}
+
+	return 0;
+
+err_unprotect_pwm_clk:
+	clk_rate_exclusive_put(pc->pwmf->core);
+err_disable_pwm_clk:
+	clk_disable(pc->pwmf->core);
+err_release:
+	mfpwm_release(pc->pwmf);
+
+	return ret;
+}
+
+static struct counter_comp rkpwmc_ext[] = {
+	COUNTER_COMP_ENABLE(rkpwmc_enable_read, rkpwmc_enable_write),
+};
+
+enum rkpwmc_count_id {
+	COUNT_PERIOD = 0,
+	COUNT_DUTY = 1,
+};
+
+static struct counter_count rkpwmc_counts[] = {
+	{
+		.id = COUNT_PERIOD,
+		.name = "Period in nanoseconds",
+		.functions_list = rkpwmc_functions,
+		.num_functions = ARRAY_SIZE(rkpwmc_functions),
+		.synapses = rkpwmc_pwm_synapses,
+		.num_synapses = ARRAY_SIZE(rkpwmc_pwm_synapses),
+		.ext = rkpwmc_ext,
+		.num_ext = ARRAY_SIZE(rkpwmc_ext),
+	},
+	{
+		.id = COUNT_DUTY,
+		.name = "Duty cycle in nanoseconds",
+		.functions_list = rkpwmc_functions,
+		.num_functions = ARRAY_SIZE(rkpwmc_functions),
+		.synapses = rkpwmc_pwm_synapses,
+		.num_synapses = ARRAY_SIZE(rkpwmc_pwm_synapses),
+		.ext = rkpwmc_ext,
+		.num_ext = ARRAY_SIZE(rkpwmc_ext),
+	},
+};
+
+static int rkpwmc_count_read(struct counter_device *counter,
+			     struct counter_count *count, u64 *value)
+{
+	struct rockchip_pwm_capture *pc = counter_priv(counter);
+	unsigned long rate;
+	u64 period;
+	u64 lpc;
+	u64 hpc;
+	int ret = 0;
+
+	if (count->id != COUNT_PERIOD && count->id != COUNT_DUTY)
+		return -EINVAL;
+
+	ret = mfpwm_acquire(pc->pwmf);
+	if (ret)
+		return ret;
+
+	rate = clk_get_rate(pc->pwmf->core);
+	if (!rate) {
+		ret = -EINVAL;
+		goto out_release;
+	}
+
+	hpc = (u32) atomic_read(&pc->hpc);
+	lpc = (u32) atomic_read(&pc->lpc);
+	period = mul_u64_u64_div_u64(hpc + lpc, NSEC_PER_SEC, rate);
+
+	if (period > jiffies_to_msecs(RKPWMC_CLEAR_DELAY) * NSEC_PER_MSEC) {
+		*value = 0;
+		goto out_release;
+	}
+
+	if (count->id == COUNT_PERIOD)
+		*value = period;
+	else
+		*value = mul_u64_u64_div_u64(hpc, NSEC_PER_SEC, rate);
+
+out_release:
+	mfpwm_release(pc->pwmf);
+
+	return ret;
+}
+
+static const struct counter_ops rkpwmc_ops = {
+	.count_read = rkpwmc_count_read,
+};
+
+static irqreturn_t rkpwmc_irq_handler(int irq, void *data)
+{
+	struct rockchip_pwm_capture *pc = data;
+	u32 intsts;
+	u32 clr = 0;
+	u32 lpc;
+	u32 hpc;
+
+	intsts = mfpwm_reg_read(pc->pwmf->base, PWMV4_REG_INTSTS);
+
+	if (!(intsts & RKPWMC_INT_MASK))
+		return IRQ_NONE;
+
+	if (intsts & PWMV4_INT_LPC) {
+		clr |= PWMV4_INT_LPC;
+		atomic_dec_if_positive(&pc->captures_left);
+	}
+
+	if (intsts & PWMV4_INT_HPC) {
+		clr |= PWMV4_INT_HPC;
+		atomic_dec_if_positive(&pc->captures_left);
+	}
+
+	/* After 4 interrupts, reset to 4 captures left and read the regs */
+	if (!atomic_cmpxchg(&pc->captures_left, 0, 4)) {
+		lpc = mfpwm_reg_read(pc->pwmf->base, PWMV4_REG_LPC);
+		hpc = mfpwm_reg_read(pc->pwmf->base, PWMV4_REG_HPC);
+
+		atomic_set(&pc->lpc, lpc);
+		atomic_set(&pc->hpc, hpc);
+		mod_delayed_work(system_bh_wq, &pc->clear_capture,
+				 RKPWMC_CLEAR_DELAY);
+	}
+
+	if (clr)
+		mfpwm_reg_write(pc->pwmf->base, PWMV4_REG_INTSTS, clr);
+
+	if (intsts ^ clr)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static void rkpwmc_clear_capture_worker(struct work_struct *work)
+{
+	struct rockchip_pwm_capture *pc = container_of(
+		work, struct rockchip_pwm_capture, clear_capture.work);
+
+	atomic_set(&pc->hpc, 0);
+	atomic_set(&pc->lpc, 0);
+}
+
+static int rockchip_pwm_capture_probe(struct platform_device *pdev)
+{
+	struct rockchip_mfpwm_func *pwmf = dev_get_platdata(&pdev->dev);
+	struct rockchip_pwm_capture *pc;
+	struct counter_device *counter;
+	int ret;
+
+	counter = devm_counter_alloc(&pdev->dev, sizeof(*pc));
+	if (IS_ERR(counter))
+		return PTR_ERR(counter);
+
+	pc = counter_priv(counter);
+	pc->pwmf = pwmf;
+	spin_lock_init(&pc->enable_lock);
+
+	platform_set_drvdata(pdev, pc);
+
+	ret = devm_request_irq(&pdev->dev, pwmf->irq, rkpwmc_irq_handler,
+			       IRQF_SHARED, pdev->name, pc);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed requesting IRQ\n");
+
+	ret = devm_delayed_work_autocancel(&pdev->dev, &pc->clear_capture,
+					   rkpwmc_clear_capture_worker);
+
+	counter->name = pdev->name;
+	counter->signals = rkpwmc_signals;
+	counter->num_signals = ARRAY_SIZE(rkpwmc_signals);
+	counter->ops = &rkpwmc_ops;
+	counter->counts = rkpwmc_counts;
+	counter->num_counts = ARRAY_SIZE(rkpwmc_counts);
+
+	ret = devm_counter_add(&pdev->dev, counter);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "Failed to add counter\n");
+
+	return 0;
+}
+
+static void rockchip_pwm_capture_remove(struct platform_device *pdev)
+{
+	struct rockchip_mfpwm_func *pwmf = dev_get_platdata(&pdev->dev);
+
+	mfpwm_remove_func(pwmf);
+}
+
+static const struct platform_device_id rockchip_pwm_capture_id_table[] = {
+	{ .name = "rockchip-pwm-capture", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, rockchip_pwm_capture_id_table);
+
+static struct platform_driver rockchip_pwm_capture_driver = {
+	.probe = rockchip_pwm_capture_probe,
+	.remove = rockchip_pwm_capture_remove,
+	.id_table = rockchip_pwm_capture_id_table,
+	.driver = {
+		.name = "rockchip-pwm-capture",
+	},
+};
+module_platform_driver(rockchip_pwm_capture_driver);
+
+MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
+MODULE_DESCRIPTION("Rockchip PWM Counter Capture Driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("ROCKCHIP_MFPWM");
+MODULE_IMPORT_NS("COUNTER");

-- 
2.49.0
Re: [PATCH v2 6/7] counter: Add rockchip-pwm-capture driver
Posted by William Breathitt Gray 5 months ago
On Mon, Jun 02, 2025 at 06:19:17PM +0200, Nicolas Frattaroli wrote:
> Among many other things, Rockchip's new PWMv4 IP in the RK3576 supports
> PWM capture functionality.
> 
> Add a basic driver for this that works to capture period and duty cycle
> values and return them as nanoseconds to the user. It's quite basic, but
> works well enough to demonstrate the device function exclusion stuff
> that mfpwm does, in order to eventually support all the functions of
> this device in drivers within their appropriate subsystems, without them
> interfering with each other.
> 
> Once enabled, the counter driver waits for enough high-to-low and
> low-to-high interrupt signals to arrive, and then writes the cycle count
> register values into some atomic members of the driver instance's state
> struct. The read callback can then do the conversion from cycle count to
> the more useful period and duty cycle nanosecond values, which require
> knowledge of the clock rate, which requires a call that the interrupt
> handler cannot make itself because said call may sleep.
> 
> To detect the condition of a PWM signal disappearing, i.e. turning off,
> we modify the delay value of a delayed worker whose job it is to simply
> set those atomic members to zero. Should the "timeout" so to speak be
> reached, we assume the PWM signal is off. This isn't perfect; it
> obviously introduces a latency between it being off and the counter
> reporting it as such. Because there isn't a way to reset the internal
> double-buffered cycle count in the hardware, we filter out unreliable
> periods above the timeout value in the counter read callback.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Hi Nicolas,

Would you help me understand the computations in this driver?

If I understand the purpose of this driver correctly, it's meant to
compute the period and duty cycle of a PWM signal. What do LPC and HPC
represent? I'm guessing they are the low period count (LPC) and the high
period count (HPC). So then you calculate the total period by adding
LPC and HPC, whereas the duty cycle derives from HPC.

Am I understanding the algorithm correctly? What are the units of HPC
and LPC; are they ticks from the core clock? Are PWMV4_INT_LPC and
PWM4_INT_HPC the change-of-state interrupts for LPC and HPC
respectively?

The Counter subsystem can be used to derive the period and duty cycle of
a signal, but I believe there's a more idiomatic way to implement this.
Existing counter drivers such as microchip-tcb-capture achieve this by
leveraging Counter events exposed via the Counter chrdev interface.

The basic idea would be:
    * Expose LPC and HPC as count0 and count1;
    * Push the PWMV4_INT_LPC and PWMV4_INT_HPC interrupts as
      COUNTER_EVENT_CHANGE_OF_STATE events on channel 0 and channel 1
      respectively;
    * Register Counter watches in userspace to capture LPC and HPC on
      each interrupt;

The Counter chrdev interface records a timestamp in nanoseconds with
each event capture. So to compute period and duty cycle, you would
subtract the difference between two HPC/LPC captures; the difference in
the timestamps gives you the elapsed time between the two captures in
nanoseconds.

Would that design work for your use case?

William Breathitt Gray
Re: [PATCH v2 6/7] counter: Add rockchip-pwm-capture driver
Posted by Nicolas Frattaroli 3 months, 3 weeks ago
On Sunday, 20 July 2025 02:20:15 Central European Summer Time William Breathitt Gray wrote:
> On Mon, Jun 02, 2025 at 06:19:17PM +0200, Nicolas Frattaroli wrote:
> > Among many other things, Rockchip's new PWMv4 IP in the RK3576 supports
> > PWM capture functionality.
> > 
> > Add a basic driver for this that works to capture period and duty cycle
> > values and return them as nanoseconds to the user. It's quite basic, but
> > works well enough to demonstrate the device function exclusion stuff
> > that mfpwm does, in order to eventually support all the functions of
> > this device in drivers within their appropriate subsystems, without them
> > interfering with each other.
> > 
> > Once enabled, the counter driver waits for enough high-to-low and
> > low-to-high interrupt signals to arrive, and then writes the cycle count
> > register values into some atomic members of the driver instance's state
> > struct. The read callback can then do the conversion from cycle count to
> > the more useful period and duty cycle nanosecond values, which require
> > knowledge of the clock rate, which requires a call that the interrupt
> > handler cannot make itself because said call may sleep.
> > 
> > To detect the condition of a PWM signal disappearing, i.e. turning off,
> > we modify the delay value of a delayed worker whose job it is to simply
> > set those atomic members to zero. Should the "timeout" so to speak be
> > reached, we assume the PWM signal is off. This isn't perfect; it
> > obviously introduces a latency between it being off and the counter
> > reporting it as such. Because there isn't a way to reset the internal
> > double-buffered cycle count in the hardware, we filter out unreliable
> > periods above the timeout value in the counter read callback.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> 
> Hi Nicolas,

Hi William,

> 
> Would you help me understand the computations in this driver?
> 
> If I understand the purpose of this driver correctly, it's meant to
> compute the period and duty cycle of a PWM signal. What do LPC and HPC
> represent? I'm guessing they are the low period count (LPC) and the high
> period count (HPC). So then you calculate the total period by adding
> LPC and HPC, whereas the duty cycle derives from HPC.
> 
> Am I understanding the algorithm correctly? What are the units of HPC
> and LPC; are they ticks from the core clock? Are PWMV4_INT_LPC and
> PWM4_INT_HPC the change-of-state interrupts for LPC and HPC
> respectively?

HPC = High Polarity Cycles, LPC = Low Polarity Cycles. They are counted
based on the pwm clock that the hardware runs at. PWMV4_INT_LPC and
PWMV4_INT_HPC are one-bit flags that are raised in the interrupt register
of this hardware when the interrupt is fired, to signal that LPC or HPC
changed state.

Your understanding of the algorithm appears to be correct, from my memory
of when I wrote the code. The 4 captures left logic is because the
hardware needs 4 level transitions before it can provide a useful
number for those two counts; thinking about it more now, I'm surprised it
can't do it in 3, so I'll need to double-check that next time I work on
this.

As an aside note, Rockchip has recently published the RK3506 Technical
Reference Manual, and the RK3506 SoC uses the same PWM IP as the RK3576
for which this driver is for. You can find it here in Chapter 31:

https://opensource.rock-chips.com/images/3/36/Rockchip_RK3506_TRM_Part_1_V1.2-20250811.pdf

This driver specifically implements "31.3.1 Capture Mode", later down
the line I may want to add "31.3.4 Clock Counter", "31.3.5 Clock
Frequency Meter" and "31.3.6 Biphasic Counter" but I lack a proper
signal generator so didn't want to bite off more than I could test.

> 
> The Counter subsystem can be used to derive the period and duty cycle of
> a signal, but I believe there's a more idiomatic way to implement this.
> Existing counter drivers such as microchip-tcb-capture achieve this by
> leveraging Counter events exposed via the Counter chrdev interface.
> 
> The basic idea would be:
>     * Expose LPC and HPC as count0 and count1;
>     * Push the PWMV4_INT_LPC and PWMV4_INT_HPC interrupts as
>       COUNTER_EVENT_CHANGE_OF_STATE events on channel 0 and channel 1
>       respectively;
>     * Register Counter watches in userspace to capture LPC and HPC on
>       each interrupt;
> 
> The Counter chrdev interface records a timestamp in nanoseconds with
> each event capture. So to compute period and duty cycle, you would
> subtract the difference between two HPC/LPC captures; the difference in
> the timestamps gives you the elapsed time between the two captures in
> nanoseconds.
> 
> Would that design work for your use case?

Basically, any design would work for me. I've only implemented the
counter driver to make sure the PWM reading part of the hardware works,
and Uwe advised me that new PWM drivers should use the counter
subsystem as opposed to implementing the PWM capture operation.

So I think your design makes more sense; any user of this driver would
likely want it to work like the other counter drivers, and exposing
LPC and HPC directly would also let me get rid of the quirky "is the
PWM actually off now" logic.

I'll do this when I get the chance to work on this patch series again,
as it needs a rewrite anyway to plug it into the MFD subsystem.

> 
> William Breathitt Gray
> 

Thank you for your review and suggestions!

Kind regards,
Nicolas Frattaroli