Linux IIO development
 help / color / mirror / Atom feed
* [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
       [not found] <20221010145222.1047748-1-biju.das.jz@bp.renesas.com>
@ 2022-10-10 14:52 ` Biju Das
  2022-10-17 16:40   ` William Breathitt Gray
  2022-10-25 13:50   ` Geert Uytterhoeven
  0 siblings, 2 replies; 6+ messages in thread
From: Biju Das @ 2022-10-10 14:52 UTC (permalink / raw)
  To: William Breathitt Gray, Philipp Zabel
  Cc: Biju Das, Lee Jones, linux-iio, Geert Uytterhoeven,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
	linux-renesas-soc

Add RZ/G2L MTU3 counter driver. This IP supports the following
phase counting modes on MTU1 and MTU2 channels

1) 16-bit phase counting modes on MTU1 and MTU2 channels.
2) 32-bit phase counting mode by cascading MTU1 and MTU2.

This patch adds 3 counters by creating 3 logical channels
	counter0: 16-bit phase counter on MTU1 channel
	counter1: 16-bit phase counter on MTU2 channel
	counter2: 32-bit phase counter by cascading MTU1 and MTU2
		  channels.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * There is no resource associated with "rz-mtu3-counter" compatible
   and moved the code to mfd subsystem as it binds against "rz-mtu".
 * Removed struct platform_driver rz_mtu3_cnt_driver.
 * Updated commit description
 * Updated Kconfig description
 * Added macros RZ_MTU3_16_BIT_MTU{1,2}_CH for MTU1 and MTU2 channels
 * Added RZ_MTU3_GET_HW_CH macro for getting channel ID.
 * replaced priv->ch[id]->priv->ch[0] in rz_mtu3_count_read()
 * Cached counter max values
 * replaced cnt->tsr in rz_mtu3_count_direction_read()
 * Added comments for RZ_MTU3_TCR_CCLR_NONE
 * Replaced if with switch in rz_mtu3_initialize_counter() and
   rz_mtu3_count_ceiling_write()
 * Added locks in initialize, terminate and enable_read to prevent races.
 * Updated rz_mtu3_action_read to take care of MTU2 signals.
 * Added separate distinct array for each group of Synapse.
 * Moved pm handling to parent.

v1->v3:
 * Modelled as a counter device supporting 3 counters(2 16-bit and 
   32-bit)
 * Add kernel-doc comments to document struct rz_mtu3_cnt
 * Removed mmio variable from struct rz_mtu3_cnt
 * Removed cnt local variable from rz_mtu3_count_read()
 * Replaced -EINVAL->-ERANGE for out of range error conditions.
 * Removed explicit cast from write functions.
 * Removed local variable val from rz_mtu3_count_ceiling_read()
 * Added lock for RMW for counter/ceiling updates.
 * Added different synapses for counter0 and counter{1,2}
 * Used ARRAY for assigning num_counts.
 * Added PM runtime for managing clocks.
 * Add MODULE_IMPORT_NS(COUNTER) to import the COUNTER namespace.
---
 drivers/mfd/Kconfig       |   8 +
 drivers/mfd/Makefile      |   1 +
 drivers/mfd/rz-mtu3-cnt.c | 554 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 563 insertions(+)
 create mode 100644 drivers/mfd/rz-mtu3-cnt.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 7329971a3bdf..fa88056224c9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
 	  To compile this driver as a module, choose M here: the
 	  module will be called rz-mtu3.
 
+config MFD_RZ_MTU3_CNT
+	tristate "RZ/G2L MTU3 counter driver"
+	depends on MFD_RZ_MTU3 || COMPILE_TEST
+	help
+	  Enable support for MTU3 counter driver found on Renesas RZ/G2L alike
+	  SoCs. This IP supports both 16-bit and 32-bit phase counting mode
+	  support.
+
 config MFD_STM32_LPTIMER
 	tristate "Support for STM32 Low-Power Timer"
 	depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index cd492903411c..9d0d1fd22f99 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -252,6 +252,7 @@ obj-$(CONFIG_MFD_STPMIC1)	+= stpmic1.o
 obj-$(CONFIG_MFD_SUN4I_GPADC)	+= sun4i-gpadc.o
 
 rz-mtu3-objs			:= rz-mtu3-core.o
+rz-mtu3-$(CONFIG_MFD_RZ_MTU3_CNT)	+= rz-mtu3-cnt.o
 obj-$(CONFIG_MFD_RZ_MTU3) 	+= rz-mtu3.o
 obj-$(CONFIG_MFD_STM32_LPTIMER)	+= stm32-lptimer.o
 obj-$(CONFIG_MFD_STM32_TIMERS) 	+= stm32-timers.o
diff --git a/drivers/mfd/rz-mtu3-cnt.c b/drivers/mfd/rz-mtu3-cnt.c
new file mode 100644
index 000000000000..e4aef25be462
--- /dev/null
+++ b/drivers/mfd/rz-mtu3-cnt.c
@@ -0,0 +1,554 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L MTU3a Counter driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+#include <linux/counter.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/types.h>
+
+#include "rz-mtu3.h"
+
+#define RZ_MTU3_TSR_TCFD	BIT(7)
+#define RZ_MTU3_MAX_HW_CNTR_CHANNELS	(2)
+
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_1	(4)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_2	(5)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_3	(6)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_4	(7)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_5	(9)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_MASK	(0xf)
+
+#define RZ_MTU3_TCR_CCLR	GENMASK(7, 5)
+#define RZ_MTU3_TCR_CCLR_NONE	FIELD_PREP(RZ_MTU3_TCR_CCLR, 0)
+
+#define RZ_MTU3_TMDR3_LWA	BIT(0)
+#define RZ_MTU3_16_BIT_MTU1_CH	(0)
+#define RZ_MTU3_16_BIT_MTU2_CH	(1)
+#define RZ_MTU3_32_BIT_CH		(2)
+
+#define RZ_MTU3_TIOR_IC_BOTH	(10)
+
+#define RZ_MTU3_GET_HW_CH(id) (((id) == RZ_MTU3_32_BIT_CH) ? 0 : (id))
+
+/**
+ * struct rz_mtu3_cnt - MTU3 counter private data
+ *
+ * @lock: Lock to prevent concurrent access for ceiling and count
+ * @mtu_16bit_max: Cache for 16-bit counters
+ * @mtu_32bit_max: Cache for 32-bit counters
+ * @rz_mtu3_channel: HW channels for the counters
+ */
+struct rz_mtu3_cnt {
+	struct mutex lock;
+	u16 mtu_16bit_max[RZ_MTU3_MAX_HW_CNTR_CHANNELS];
+	u32 mtu_32bit_max;
+	struct rz_mtu3_channel *ch[RZ_MTU3_MAX_HW_CNTR_CHANNELS];
+};
+
+static const enum counter_function rz_mtu3_count_functions[] = {
+	COUNTER_FUNCTION_QUADRATURE_X4,
+	COUNTER_FUNCTION_PULSE_DIRECTION,
+	COUNTER_FUNCTION_QUADRATURE_X2_B,
+};
+
+static int rz_mtu3_count_read(struct counter_device *counter,
+			      struct counter_count *count, u64 *val)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+	if (count->id == RZ_MTU3_32_BIT_CH)
+		*val = rz_mtu3_32bit_ch_read(priv->ch[0], RZ_MTU3_TCNTLW);
+	else
+		*val = rz_mtu3_16bit_ch_read(priv->ch[count->id], RZ_MTU3_TCNT);
+
+	return 0;
+}
+
+static int rz_mtu3_count_write(struct counter_device *counter,
+			       struct counter_count *count, const u64 val)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+	u32 ceiling;
+
+	mutex_lock(&priv->lock);
+	if (count->id == RZ_MTU3_32_BIT_CH)
+		ceiling = priv->mtu_32bit_max;
+	else
+		ceiling = priv->mtu_16bit_max[ch_id];
+
+	if (val > ceiling) {
+		mutex_unlock(&priv->lock);
+		return -ERANGE;
+	}
+
+	if (count->id == RZ_MTU3_32_BIT_CH)
+		rz_mtu3_32bit_ch_write(priv->ch[0], RZ_MTU3_TCNTLW, val);
+	else
+		rz_mtu3_16bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCNT, val);
+
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int rz_mtu3_count_function_read(struct counter_device *counter,
+				       struct counter_count *count,
+				       enum counter_function *function)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+	u8 val;
+
+	val = rz_mtu3_8bit_ch_read(priv->ch[ch_id], RZ_MTU3_TMDR1);
+
+	switch (val & RZ_MTU3_TMDR1_PH_CNT_MODE_MASK) {
+	case RZ_MTU3_TMDR1_PH_CNT_MODE_1:
+		*function = COUNTER_FUNCTION_QUADRATURE_X4;
+		break;
+	case RZ_MTU3_TMDR1_PH_CNT_MODE_2:
+		*function = COUNTER_FUNCTION_PULSE_DIRECTION;
+		break;
+	case RZ_MTU3_TMDR1_PH_CNT_MODE_4:
+		*function = COUNTER_FUNCTION_QUADRATURE_X2_B;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int rz_mtu3_count_function_write(struct counter_device *counter,
+					struct counter_count *count,
+					enum counter_function function)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+	u8 mode;
+
+	switch (function) {
+	case COUNTER_FUNCTION_QUADRATURE_X4:
+		mode = RZ_MTU3_TMDR1_PH_CNT_MODE_1;
+		break;
+	case COUNTER_FUNCTION_PULSE_DIRECTION:
+		mode = RZ_MTU3_TMDR1_PH_CNT_MODE_2;
+		break;
+	case COUNTER_FUNCTION_QUADRATURE_X2_B:
+		mode = RZ_MTU3_TMDR1_PH_CNT_MODE_4;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TMDR1, mode);
+
+	return 0;
+}
+
+static int rz_mtu3_count_direction_read(struct counter_device *counter,
+					struct counter_count *count,
+					enum counter_count_direction *direction)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+	u8 tsr;
+
+	tsr = rz_mtu3_8bit_ch_read(priv->ch[ch_id], RZ_MTU3_TSR);
+
+	if (tsr & RZ_MTU3_TSR_TCFD)
+		*direction = COUNTER_COUNT_DIRECTION_FORWARD;
+	else
+		*direction = COUNTER_COUNT_DIRECTION_BACKWARD;
+
+	return 0;
+}
+
+static int rz_mtu3_count_ceiling_read(struct counter_device *counter,
+				      struct counter_count *count,
+				      u64 *ceiling)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+
+	if (count->id == RZ_MTU3_32_BIT_CH)
+		*ceiling = priv->mtu_32bit_max;
+	else
+		*ceiling = priv->mtu_16bit_max[ch_id];
+
+	return 0;
+}
+
+static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
+				       struct counter_count *count,
+				       u64 ceiling)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+
+	switch (count->id) {
+	case RZ_MTU3_16_BIT_MTU1_CH:
+	case RZ_MTU3_16_BIT_MTU2_CH:
+		if (ceiling > U16_MAX)
+			return -ERANGE;
+		priv->mtu_16bit_max[ch_id] = ceiling;
+		break;
+	case RZ_MTU3_32_BIT_CH:
+		if (ceiling > U32_MAX)
+			return -ERANGE;
+		priv->mtu_32bit_max = ceiling;
+		break;
+	}
+
+	mutex_lock(&priv->lock);
+	if (ceiling == 0) {
+		/* Disable counter clear source */
+		rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCR,
+				      RZ_MTU3_TCR_CCLR_NONE);
+	} else {
+		if (count->id == RZ_MTU3_32_BIT_CH)
+			rz_mtu3_32bit_ch_write(priv->ch[ch_id], RZ_MTU3_TGRALW,
+					       ceiling);
+		else
+			rz_mtu3_16bit_ch_write(priv->ch[ch_id], RZ_MTU3_TGRA,
+					       ceiling);
+
+		rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCR,
+				      RZ_MTU3_TCR_CCLR_TGRA);
+	}
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static void rz_mtu3_32bit_cnt_setting(struct counter_device *counter, int id)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+	/*
+	 * 32-bit phase counting need MTU1 and MTU2 to create 32-bit cascade
+	 * counter.
+	 */
+	priv->ch[0]->function = RZ_MTU3_32BIT_PHASE_COUNTING;
+	priv->ch[1]->function = RZ_MTU3_32BIT_PHASE_COUNTING;
+
+	rz_mtu3_shared_reg_write(priv->ch[0], RZ_MTU3_TMDR3, RZ_MTU3_TMDR3_LWA);
+
+	/* Phase counting mode 1 is used as default in initialization. */
+	rz_mtu3_8bit_ch_write(priv->ch[0], RZ_MTU3_TMDR1,
+			      RZ_MTU3_TMDR1_PH_CNT_MODE_1);
+
+	rz_mtu3_8bit_ch_write(priv->ch[0], RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+	rz_mtu3_8bit_ch_write(priv->ch[0], RZ_MTU3_TIOR, RZ_MTU3_TIOR_IC_BOTH);
+
+	rz_mtu3_enable(priv->ch[0]);
+	rz_mtu3_enable(priv->ch[1]);
+}
+
+static void rz_mtu3_16bit_cnt_setting(struct counter_device *counter, int id)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+	priv->ch[id]->function = RZ_MTU3_16BIT_PHASE_COUNTING;
+
+	rz_mtu3_shared_reg_write(priv->ch[0], RZ_MTU3_TMDR3, 0);
+	/* Phase counting mode 1 is used as default in initialization. */
+	rz_mtu3_8bit_ch_write(priv->ch[id], RZ_MTU3_TMDR1,
+			      RZ_MTU3_TMDR1_PH_CNT_MODE_1);
+
+	rz_mtu3_8bit_ch_write(priv->ch[id], RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+	rz_mtu3_enable(priv->ch[id]);
+}
+
+static int rz_mtu3_initialize_counter(struct counter_device *counter, int id)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+	mutex_lock(&priv->lock);
+	switch (id) {
+	case RZ_MTU3_16_BIT_MTU1_CH:
+	case RZ_MTU3_16_BIT_MTU2_CH:
+		if (priv->ch[id]->function != RZ_MTU3_NORMAL) {
+			mutex_unlock(&priv->lock);
+			return -EBUSY;
+		}
+
+		rz_mtu3_16bit_cnt_setting(counter, id);
+		break;
+	case RZ_MTU3_32_BIT_CH:
+		if (priv->ch[0]->function != RZ_MTU3_NORMAL ||
+		    priv->ch[1]->function != RZ_MTU3_NORMAL) {
+			mutex_unlock(&priv->lock);
+			return -EBUSY;
+		}
+		rz_mtu3_32bit_cnt_setting(counter, id);
+		break;
+	}
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static void rz_mtu3_terminate_counter(struct counter_device *counter, int id)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+	mutex_lock(&priv->lock);
+	if (id == RZ_MTU3_32_BIT_CH) {
+		priv->ch[0]->function = RZ_MTU3_NORMAL;
+		priv->ch[1]->function = RZ_MTU3_NORMAL;
+		rz_mtu3_shared_reg_write(priv->ch[0], RZ_MTU3_TMDR3, 0);
+		rz_mtu3_disable(priv->ch[1]);
+		rz_mtu3_disable(priv->ch[0]);
+	} else {
+		priv->ch[id]->function = RZ_MTU3_NORMAL;
+		rz_mtu3_disable(priv->ch[id]);
+	}
+	mutex_unlock(&priv->lock);
+}
+
+static int rz_mtu3_count_enable_read(struct counter_device *counter,
+				     struct counter_count *count, u8 *enable)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+	if (count->id == RZ_MTU3_32_BIT_CH) {
+		mutex_lock(&priv->lock);
+		*enable = rz_mtu3_is_enabled(priv->ch[0]) &&
+			rz_mtu3_is_enabled(priv->ch[1]);
+		mutex_unlock(&priv->lock);
+	} else {
+		*enable = rz_mtu3_is_enabled(priv->ch[count->id]);
+	}
+
+	return 0;
+}
+
+static int rz_mtu3_count_enable_write(struct counter_device *counter,
+				      struct counter_count *count, u8 enable)
+{
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+	struct rz_mtu3_channel *ch = priv->ch[ch_id];
+	int ret = 0;
+
+	if (enable) {
+		pm_runtime_get_sync(ch->dev);
+		ret = rz_mtu3_initialize_counter(counter, count->id);
+	} else {
+		rz_mtu3_terminate_counter(counter, count->id);
+		pm_runtime_put(ch->dev);
+	}
+
+	return ret;
+}
+
+static struct counter_comp rz_mtu3_count_ext[] = {
+	COUNTER_COMP_DIRECTION(rz_mtu3_count_direction_read),
+	COUNTER_COMP_ENABLE(rz_mtu3_count_enable_read,
+			    rz_mtu3_count_enable_write),
+	COUNTER_COMP_CEILING(rz_mtu3_count_ceiling_read,
+			     rz_mtu3_count_ceiling_write),
+};
+
+static const enum counter_synapse_action rz_mtu3_synapse_actions[] = {
+	COUNTER_SYNAPSE_ACTION_BOTH_EDGES,
+	COUNTER_SYNAPSE_ACTION_RISING_EDGE,
+	COUNTER_SYNAPSE_ACTION_NONE,
+};
+
+static int rz_mtu3_action_read(struct counter_device *counter,
+			       struct counter_count *count,
+			       struct counter_synapse *synapse,
+			       enum counter_synapse_action *action)
+{
+	const size_t signal_a_id = count->synapses[0].signal->id;
+	const size_t signal_b_id = count->synapses[1].signal->id;
+	size_t signal_c_id;
+	size_t signal_d_id;
+	enum counter_function function;
+	int err;
+
+	if (count->id != RZ_MTU3_16_BIT_MTU1_CH) {
+		signal_c_id = count->synapses[2].signal->id;
+		signal_d_id = count->synapses[3].signal->id;
+	}
+
+	err = rz_mtu3_count_function_read(counter, count, &function);
+	if (err)
+		return err;
+
+	/* Default action mode */
+	*action = COUNTER_SYNAPSE_ACTION_NONE;
+
+	switch (function) {
+	case COUNTER_FUNCTION_PULSE_DIRECTION:
+		/*
+		 * Rising edges on signal A updates the respective count.
+		 * The input level of signal B determines direction.
+		 */
+		if (synapse->signal->id == signal_a_id ||
+		    synapse->signal->id == signal_c_id)
+			*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+		break;
+	case COUNTER_FUNCTION_QUADRATURE_X2_B:
+		/*
+		 * Any state transition on quadrature pair signal B updates
+		 * the respective count.
+		 */
+		if (synapse->signal->id == signal_b_id ||
+		    synapse->signal->id == signal_d_id)
+			*action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	case COUNTER_FUNCTION_QUADRATURE_X4:
+		/* counts up/down on both edges of A and B signal*/
+		*action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct counter_ops rz_mtu3_cnt_ops = {
+	.count_read = rz_mtu3_count_read,
+	.count_write = rz_mtu3_count_write,
+	.function_read = rz_mtu3_count_function_read,
+	.function_write = rz_mtu3_count_function_write,
+	.action_read = rz_mtu3_action_read,
+};
+
+#define RZ_MTU3_PHASE_SIGNAL(_id, _name) {		\
+	.id = (_id),				\
+	.name = (_name),			\
+}
+
+static struct counter_signal rz_mtu3_signals[] = {
+	RZ_MTU3_PHASE_SIGNAL(0, "MTU1 MTCLKA"),
+	RZ_MTU3_PHASE_SIGNAL(1, "MTU1 MTCLKB"),
+	RZ_MTU3_PHASE_SIGNAL(2, "MTU2 MTCLKC"),
+	RZ_MTU3_PHASE_SIGNAL(3, "MTU2 MTCLKD"),
+};
+
+static struct counter_synapse rz_mtu3_mtu1_count_synapses[] = {
+	{
+		.actions_list = rz_mtu3_synapse_actions,
+		.num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+		.signal = rz_mtu3_signals,
+	},
+	{
+		.actions_list = rz_mtu3_synapse_actions,
+		.num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+		.signal = rz_mtu3_signals + 1,
+	}
+};
+
+static struct counter_synapse rz_mtu3_mtu2_count_synapses[] = {
+	{
+		.actions_list = rz_mtu3_synapse_actions,
+		.num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+		.signal = rz_mtu3_signals,
+	},
+	{
+		.actions_list = rz_mtu3_synapse_actions,
+		.num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+		.signal = rz_mtu3_signals + 1,
+	},
+	{
+		.actions_list = rz_mtu3_synapse_actions,
+		.num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+		.signal = rz_mtu3_signals + 2,
+	},
+	{
+		.actions_list = rz_mtu3_synapse_actions,
+		.num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+		.signal = rz_mtu3_signals + 3,
+	}
+};
+
+static struct counter_count rz_mtu3_counts[] = {
+	{
+		.id = RZ_MTU3_16_BIT_MTU1_CH,
+		.name = "Channel 1 phase counter",
+		.functions_list = rz_mtu3_count_functions,
+		.num_functions = ARRAY_SIZE(rz_mtu3_count_functions),
+		.synapses = rz_mtu3_mtu1_count_synapses,
+		.num_synapses = ARRAY_SIZE(rz_mtu3_mtu1_count_synapses),
+		.ext = rz_mtu3_count_ext,
+		.num_ext = ARRAY_SIZE(rz_mtu3_count_ext),
+	},
+	{
+		.id = RZ_MTU3_16_BIT_MTU2_CH,
+		.name = "Channel 2 phase counter",
+		.functions_list = rz_mtu3_count_functions,
+		.num_functions = ARRAY_SIZE(rz_mtu3_count_functions),
+		.synapses = rz_mtu3_mtu2_count_synapses,
+		.num_synapses = ARRAY_SIZE(rz_mtu3_mtu2_count_synapses),
+		.ext = rz_mtu3_count_ext,
+		.num_ext = ARRAY_SIZE(rz_mtu3_count_ext),
+	},
+	{
+		.id = RZ_MTU3_32_BIT_CH,
+		.name = "Channel 3 Cascaded phase counter",
+		.functions_list = rz_mtu3_count_functions,
+		.num_functions = ARRAY_SIZE(rz_mtu3_count_functions),
+		.synapses = rz_mtu3_mtu2_count_synapses,
+		.num_synapses = ARRAY_SIZE(rz_mtu3_mtu2_count_synapses),
+		.ext = rz_mtu3_count_ext,
+		.num_ext = ARRAY_SIZE(rz_mtu3_count_ext),
+	}
+};
+
+int rz_mtu3_cnt_probe(struct platform_device *pdev)
+{
+	struct rz_mtu3 *ddata = dev_get_drvdata(&pdev->dev);
+	struct device *dev = &pdev->dev;
+	struct counter_device *counter;
+	struct rz_mtu3_cnt *priv;
+	unsigned int i;
+	int ret;
+
+	counter = devm_counter_alloc(dev, sizeof(*priv));
+	if (!counter)
+		return -ENOMEM;
+
+	priv = counter_priv(counter);
+	ddata->cnt_priv = priv;
+
+	for (i = 0; i < RZ_MTU3_MAX_HW_CNTR_CHANNELS; i++) {
+		priv->ch[i] = &ddata->channels[RZ_MTU1 + i];
+		priv->ch[i]->dev = dev;
+		priv->mtu_16bit_max[i] = U16_MAX;
+	}
+
+	priv->mtu_32bit_max = U32_MAX;
+
+	mutex_init(&priv->lock);
+
+	counter->name = dev_name(dev);
+	counter->parent = dev;
+	counter->ops = &rz_mtu3_cnt_ops;
+	counter->counts = rz_mtu3_counts;
+	counter->num_counts = ARRAY_SIZE(rz_mtu3_counts);
+	counter->signals = rz_mtu3_signals;
+	counter->num_signals = ARRAY_SIZE(rz_mtu3_signals);
+
+	/* Register Counter device */
+	ret = devm_counter_add(dev, counter);
+	if (ret < 0)
+		dev_err_probe(dev, ret, "Failed to add counter\n");
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_cnt_probe);
+
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a counter driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(COUNTER);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
  2022-10-10 14:52 ` [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver Biju Das
@ 2022-10-17 16:40   ` William Breathitt Gray
  2022-10-17 19:58     ` Biju Das
  2022-10-25 13:50   ` Geert Uytterhoeven
  1 sibling, 1 reply; 6+ messages in thread
From: William Breathitt Gray @ 2022-10-17 16:40 UTC (permalink / raw)
  To: Biju Das
  Cc: Philipp Zabel, Lee Jones, linux-iio, Geert Uytterhoeven,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
	linux-renesas-soc

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

On Mon, Oct 10, 2022 at 03:52:21PM +0100, Biju Das wrote:
> Add RZ/G2L MTU3 counter driver. This IP supports the following
> phase counting modes on MTU1 and MTU2 channels
> 
> 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> 2) 32-bit phase counting mode by cascading MTU1 and MTU2.
> 
> This patch adds 3 counters by creating 3 logical channels
> 	counter0: 16-bit phase counter on MTU1 channel
> 	counter1: 16-bit phase counter on MTU2 channel
> 	counter2: 32-bit phase counter by cascading MTU1 and MTU2
> 		  channels.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Hello Biju,

We discussed some changes already for v5, but I have some additional
comments and questions below.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 7329971a3bdf..fa88056224c9 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called rz-mtu3.
>  
> +config MFD_RZ_MTU3_CNT
> +	tristate "RZ/G2L MTU3 counter driver"

This is a nitpick, but include the manufacturer name in the tristate
string for the sake of clarity: "Renesas RZ/G2L MTU3 counter driver".

> +	depends on MFD_RZ_MTU3 || COMPILE_TEST

I noticed you include <linux/of.h> in the rz-mtu3-cnt.c file; do you
need to depend on OF here in the Kconfig as well?

> +static int rz_mtu3_count_read(struct counter_device *counter,
> +			      struct counter_count *count, u64 *val)
> +{
> +	struct rz_mtu3_cnt *const priv = counter_priv(counter);
> +
> +	if (count->id == RZ_MTU3_32_BIT_CH)
> +		*val = rz_mtu3_32bit_ch_read(priv->ch[0], RZ_MTU3_TCNTLW);
> +	else
> +		*val = rz_mtu3_16bit_ch_read(priv->ch[count->id], RZ_MTU3_TCNT);

After considering this again, I think it'll be better to match the
structure of the rest of the functions in this driver for consistency.
Rather than hardcoding priv->ch[0], determine the ch_id first and pass
that instead::


    const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);

    if (count->id == RZ_MTU3_32_BIT_CH)
            *val = rz_mtu3_32bit_ch_read(priv->ch[ch_id], RZ_MTU3_TCNTLW);
    else
            *val = rz_mtu3_16bit_ch_read(priv->ch[ch_id], RZ_MTU3_TCNT);

> +static int rz_mtu3_count_write(struct counter_device *counter,
> +			       struct counter_count *count, const u64 val)
> +{
> +	struct rz_mtu3_cnt *const priv = counter_priv(counter);
> +	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> +	u32 ceiling;
> +
> +	mutex_lock(&priv->lock);
> +	if (count->id == RZ_MTU3_32_BIT_CH)
> +		ceiling = priv->mtu_32bit_max;
> +	else
> +		ceiling = priv->mtu_16bit_max[ch_id];
> +
> +	if (val > ceiling) {
> +		mutex_unlock(&priv->lock);
> +		return -ERANGE;
> +	}
> +
> +	if (count->id == RZ_MTU3_32_BIT_CH)
> +		rz_mtu3_32bit_ch_write(priv->ch[0], RZ_MTU3_TCNTLW, val);

Like in count_read(), use ch_id here instead of 0 for the sake of
consistency.

> +static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
> +				       struct counter_count *count,
> +				       u64 ceiling)
> +{
> +	struct rz_mtu3_cnt *const priv = counter_priv(counter);
> +	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> +
> +	switch (count->id) {
> +	case RZ_MTU3_16_BIT_MTU1_CH:
> +	case RZ_MTU3_16_BIT_MTU2_CH:
> +		if (ceiling > U16_MAX)
> +			return -ERANGE;
> +		priv->mtu_16bit_max[ch_id] = ceiling;
> +		break;
> +	case RZ_MTU3_32_BIT_CH:
> +		if (ceiling > U32_MAX)
> +			return -ERANGE;
> +		priv->mtu_32bit_max = ceiling;
> +		break;
> +	}
> +
> +	mutex_lock(&priv->lock);
> +	if (ceiling == 0) {
> +		/* Disable counter clear source */
> +		rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCR,
> +				      RZ_MTU3_TCR_CCLR_NONE);

Refer to our discussions in the v3 review thread regarding ceiling set
to 0; in particular, don't disable the count value ceiling but rather
limit it to a maximum value of 0.

> +static int rz_mtu3_count_enable_write(struct counter_device *counter,
> +				      struct counter_count *count, u8 enable)
> +{
> +	struct rz_mtu3_cnt *const priv = counter_priv(counter);
> +	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> +	struct rz_mtu3_channel *ch = priv->ch[ch_id];
> +	int ret = 0;
> +
> +	if (enable) {
> +		pm_runtime_get_sync(ch->dev);
> +		ret = rz_mtu3_initialize_counter(counter, count->id);
> +	} else {
> +		rz_mtu3_terminate_counter(counter, count->id);
> +		pm_runtime_put(ch->dev);
> +	}
Refer to our discussions in the v3 review thread regarding the "enable"
Count extension; in particular, "enable" pauses/unpauses counting.

> +static int rz_mtu3_action_read(struct counter_device *counter,
> +			       struct counter_count *count,
> +			       struct counter_synapse *synapse,
> +			       enum counter_synapse_action *action)
> +{
> +	const size_t signal_a_id = count->synapses[0].signal->id;
> +	const size_t signal_b_id = count->synapses[1].signal->id;
> +	size_t signal_c_id;
> +	size_t signal_d_id;
> +	enum counter_function function;
> +	int err;
> +
> +	if (count->id != RZ_MTU3_16_BIT_MTU1_CH) {
> +		signal_c_id = count->synapses[2].signal->id;
> +		signal_d_id = count->synapses[3].signal->id;
> +	}

The Signal ids are constants so you remove them from this function and
use preprocessor defines instead to represent SIGNAL_A_ID, SIGNAL_B_ID,
SIGNAL_C_ID, and SIGNAL_D_ID. Remember to set the Signal ids in the
rz_mtu3_signals[] array accordingly.

> +static struct counter_signal rz_mtu3_signals[] = {
> +	RZ_MTU3_PHASE_SIGNAL(0, "MTU1 MTCLKA"),
> +	RZ_MTU3_PHASE_SIGNAL(1, "MTU1 MTCLKB"),
> +	RZ_MTU3_PHASE_SIGNAL(2, "MTU2 MTCLKC"),
> +	RZ_MTU3_PHASE_SIGNAL(3, "MTU2 MTCLKD"),
> +};

The relationship of these Signals still has me somewhat confused so I'm
hoping you can help me properly ironed out my understanding. This is how
I currently understand it, so please point out any mistakes I have:

MTU1 (Channel 1):
 * Pulse-Direction mode:
   - MTCLKA updates count
   - MTCLKB determines direction
 * Quadrature x2 B:
   - MTCLKA is quadrature phase A
   - MTCLKB is quadrature phase B
   - Any state transition on MTCLKB updates count
 * Quadrature x4:
   - MTCLKA is quadrature phase A
   - MTCLKB is quadrature phase B
   - Any state transition on either MTCLKA or MTCLKB updates count

MTU2 (Channel 2):
 - Same as MTU1, but optionally can select MTCLKC and MTCLKD instead of
   MTCLKA and MTCLKB respectively
 * Pulse-Direction mode:
   - MTCLKA(C) updates count
   - MTCLKB(D) determines direction
 * Quadrature x2 B:
   - MTCLKA(C) is quadrature phase A
   - MTCLKB(D) is quadrature phase B
   - Any state transition on MTCLKB updates count
 * Quadrature x4:
   - MTCLKA(C) is quadrature phase A
   - MTCLKB(D) is quadrature phase B
   - Any state transition on either MTCLKA(C) or MTCLKB(D) updates count

MTU3 (Channel 1 and 2 cascading):
 - Same as MTU2 (but count is now 32-bit)
 * Pulse-Direction mode:
   - MTCLKA(C) updates count
   - MTCLKB(D) determines direction
 * Quadrature x2 B:
   - MTCLKA(C) is quadrature phase A
   - MTCLKB(D) is quadrature phase B
   - Any state transition on MTCLKB updates count
 * Quadrature x4:
   - MTCLKA(C) is quadrature phase A
   - MTCLKB(D) is quadrature phase B
   - Any state transition on either MTCLKA(C) or MTCLKB(D) updates count

Is my understanding correct here? Is the selection between MTCLKA/MTCLKB
and MTCLKC/MTCLKD done in software, and should we expose it in sysfs?

William Breathitt Gray

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
  2022-10-17 16:40   ` William Breathitt Gray
@ 2022-10-17 19:58     ` Biju Das
  2022-10-17 23:02       ` William Breathitt Gray
  0 siblings, 1 reply; 6+ messages in thread
From: Biju Das @ 2022-10-17 19:58 UTC (permalink / raw)
  To: William Breathitt Gray
  Cc: Philipp Zabel, Lee Jones, linux-iio@vger.kernel.org,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org

Hi William Breathitt Gray,

> Subject: Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
> 
> On Mon, Oct 10, 2022 at 03:52:21PM +0100, Biju Das wrote:
> > Add RZ/G2L MTU3 counter driver. This IP supports the following phase
> > counting modes on MTU1 and MTU2 channels
> >
> > 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> > 2) 32-bit phase counting mode by cascading MTU1 and MTU2.
> >
> > This patch adds 3 counters by creating 3 logical channels
> > 	counter0: 16-bit phase counter on MTU1 channel
> > 	counter1: 16-bit phase counter on MTU2 channel
> > 	counter2: 32-bit phase counter by cascading MTU1 and MTU2
> > 		  channels.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Hello Biju,
> 
> We discussed some changes already for v5, but I have some additional
> comments and questions below.
OK.

> 
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index
> > 7329971a3bdf..fa88056224c9 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called rz-mtu3.
> >
> > +config MFD_RZ_MTU3_CNT
> > +	tristate "RZ/G2L MTU3 counter driver"
> 
> This is a nitpick, but include the manufacturer name in the tristate
> string for the sake of clarity: "Renesas RZ/G2L MTU3 counter driver".
> 
> > +	depends on MFD_RZ_MTU3 || COMPILE_TEST
> 
> I noticed you include <linux/of.h> in the rz-mtu3-cnt.c file; do you
> need to depend on OF here in the Kconfig as well?

I will take out "of.h", as there is no compatible.

> 
> > +static int rz_mtu3_count_read(struct counter_device *counter,
> > +			      struct counter_count *count, u64 *val) {
> > +	struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > +
> > +	if (count->id == RZ_MTU3_32_BIT_CH)
> > +		*val = rz_mtu3_32bit_ch_read(priv->ch[0], RZ_MTU3_TCNTLW);
> > +	else
> > +		*val = rz_mtu3_16bit_ch_read(priv->ch[count->id],
> RZ_MTU3_TCNT);
> 
> After considering this again, I think it'll be better to match the
> structure of the rest of the functions in this driver for consistency.
> Rather than hardcoding priv->ch[0], determine the ch_id first and pass
> that instead::

OK.

> 
> 
>     const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> 
>     if (count->id == RZ_MTU3_32_BIT_CH)
>             *val = rz_mtu3_32bit_ch_read(priv->ch[ch_id],
> RZ_MTU3_TCNTLW);
>     else
>             *val = rz_mtu3_16bit_ch_read(priv->ch[ch_id],
> RZ_MTU3_TCNT);
> 
> > +static int rz_mtu3_count_write(struct counter_device *counter,
> > +			       struct counter_count *count, const u64 val) {
> > +	struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > +	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> > +	u32 ceiling;
> > +
> > +	mutex_lock(&priv->lock);
> > +	if (count->id == RZ_MTU3_32_BIT_CH)
> > +		ceiling = priv->mtu_32bit_max;
> > +	else
> > +		ceiling = priv->mtu_16bit_max[ch_id];
> > +
> > +	if (val > ceiling) {
> > +		mutex_unlock(&priv->lock);
> > +		return -ERANGE;
> > +	}
> > +
> > +	if (count->id == RZ_MTU3_32_BIT_CH)
> > +		rz_mtu3_32bit_ch_write(priv->ch[0], RZ_MTU3_TCNTLW, val);
> 
> Like in count_read(), use ch_id here instead of 0 for the sake of
> consistency.
> 
> > +static int rz_mtu3_count_ceiling_write(struct counter_device
> *counter,
> > +				       struct counter_count *count,
> > +				       u64 ceiling)
> > +{
> > +	struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > +	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> > +
> > +	switch (count->id) {
> > +	case RZ_MTU3_16_BIT_MTU1_CH:
> > +	case RZ_MTU3_16_BIT_MTU2_CH:
> > +		if (ceiling > U16_MAX)
> > +			return -ERANGE;
> > +		priv->mtu_16bit_max[ch_id] = ceiling;
> > +		break;
> > +	case RZ_MTU3_32_BIT_CH:
> > +		if (ceiling > U32_MAX)
> > +			return -ERANGE;
> > +		priv->mtu_32bit_max = ceiling;
> > +		break;
> > +	}
> > +
> > +	mutex_lock(&priv->lock);
> > +	if (ceiling == 0) {
> > +		/* Disable counter clear source */
> > +		rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCR,
> > +				      RZ_MTU3_TCR_CCLR_NONE);
> 
> Refer to our discussions in the v3 review thread regarding ceiling set
> to 0; in particular, don't disable the count value ceiling but rather
> limit it to a maximum value of 0.
> 
> > +static int rz_mtu3_count_enable_write(struct counter_device
> *counter,
> > +				      struct counter_count *count, u8 enable) {
> > +	struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > +	const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> > +	struct rz_mtu3_channel *ch = priv->ch[ch_id];
> > +	int ret = 0;
> > +
> > +	if (enable) {
> > +		pm_runtime_get_sync(ch->dev);
> > +		ret = rz_mtu3_initialize_counter(counter, count->id);
> > +	} else {
> > +		rz_mtu3_terminate_counter(counter, count->id);
> > +		pm_runtime_put(ch->dev);
> > +	}
> Refer to our discussions in the v3 review thread regarding the
> "enable"
> Count extension; in particular, "enable" pauses/unpauses counting.
> 
> > +static int rz_mtu3_action_read(struct counter_device *counter,
> > +			       struct counter_count *count,
> > +			       struct counter_synapse *synapse,
> > +			       enum counter_synapse_action *action) {
> > +	const size_t signal_a_id = count->synapses[0].signal->id;
> > +	const size_t signal_b_id = count->synapses[1].signal->id;
> > +	size_t signal_c_id;
> > +	size_t signal_d_id;
> > +	enum counter_function function;
> > +	int err;
> > +
> > +	if (count->id != RZ_MTU3_16_BIT_MTU1_CH) {
> > +		signal_c_id = count->synapses[2].signal->id;
> > +		signal_d_id = count->synapses[3].signal->id;
> > +	}
> 
> The Signal ids are constants so you remove them from this function and
> use preprocessor defines instead to represent SIGNAL_A_ID,
> SIGNAL_B_ID, SIGNAL_C_ID, and SIGNAL_D_ID. Remember to set the Signal
> ids in the rz_mtu3_signals[] array accordingly.
> 
> > +static struct counter_signal rz_mtu3_signals[] = {
> > +	RZ_MTU3_PHASE_SIGNAL(0, "MTU1 MTCLKA"),
> > +	RZ_MTU3_PHASE_SIGNAL(1, "MTU1 MTCLKB"),
> > +	RZ_MTU3_PHASE_SIGNAL(2, "MTU2 MTCLKC"),
> > +	RZ_MTU3_PHASE_SIGNAL(3, "MTU2 MTCLKD"), };
> 
> The relationship of these Signals still has me somewhat confused so
> I'm hoping you can help me properly ironed out my understanding. This
> is how I currently understand it, so please point out any mistakes I
> have:
> 
> MTU1 (Channel 1):
>  * Pulse-Direction mode:
>    - MTCLKA updates count
>    - MTCLKB determines direction
>  * Quadrature x2 B:
>    - MTCLKA is quadrature phase A
>    - MTCLKB is quadrature phase B
>    - Any state transition on MTCLKB updates count
>  * Quadrature x4:
>    - MTCLKA is quadrature phase A
>    - MTCLKB is quadrature phase B
>    - Any state transition on either MTCLKA or MTCLKB updates count
> 
> MTU2 (Channel 2):
>  - Same as MTU1, but optionally can select MTCLKC and MTCLKD instead
> of
>    MTCLKA and MTCLKB respectively
>  * Pulse-Direction mode:
>    - MTCLKA(C) updates count
>    - MTCLKB(D) determines direction
>  * Quadrature x2 B:
>    - MTCLKA(C) is quadrature phase A
>    - MTCLKB(D) is quadrature phase B
>    - Any state transition on MTCLKB updates count
>  * Quadrature x4:
>    - MTCLKA(C) is quadrature phase A
>    - MTCLKB(D) is quadrature phase B
>    - Any state transition on either MTCLKA(C) or MTCLKB(D) updates
> count
> 
> MTU3 (Channel 1 and 2 cascading):
>  - Same as MTU2 (but count is now 32-bit)
>  * Pulse-Direction mode:
>    - MTCLKA(C) updates count
>    - MTCLKB(D) determines direction
>  * Quadrature x2 B:
>    - MTCLKA(C) is quadrature phase A
>    - MTCLKB(D) is quadrature phase B
>    - Any state transition on MTCLKB updates count
>  * Quadrature x4:
>    - MTCLKA(C) is quadrature phase A
>    - MTCLKB(D) is quadrature phase B
>    - Any state transition on either MTCLKA(C) or MTCLKB(D) updates
> count
> 
> Is my understanding correct here? Is the selection between
> MTCLKA/MTCLKB and MTCLKC/MTCLKD done in software, and should we expose
> it in sysfs?

Yes, we need to expose this to sysfs. Is it same as phase mode?
Can you please provide an example, if possible how to handle this?

Cheers,
Biju


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
  2022-10-17 19:58     ` Biju Das
@ 2022-10-17 23:02       ` William Breathitt Gray
  0 siblings, 0 replies; 6+ messages in thread
From: William Breathitt Gray @ 2022-10-17 23:02 UTC (permalink / raw)
  To: Biju Das
  Cc: Philipp Zabel, Lee Jones, linux-iio@vger.kernel.org,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc@vger.kernel.org

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

On Mon, Oct 17, 2022 at 07:58:54PM +0000, Biju Das wrote:
> > Is my understanding correct here? Is the selection between
> > MTCLKA/MTCLKB and MTCLKC/MTCLKD done in software, and should we expose
> > it in sysfs?
> 
> Yes, we need to expose this to sysfs. Is it same as phase mode?
> Can you please provide an example, if possible how to handle this?
> 
> Cheers,
> Biju

Tentatively, I think you can handle it the same way as the phase
counting mode by creating a new Counter device extension; the code
should be pretty similar except the name you give it will be different.

William Breathitt Gray

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
  2022-10-10 14:52 ` [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver Biju Das
  2022-10-17 16:40   ` William Breathitt Gray
@ 2022-10-25 13:50   ` Geert Uytterhoeven
  2022-10-25 13:58     ` Biju Das
  1 sibling, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2022-10-25 13:50 UTC (permalink / raw)
  To: Biju Das
  Cc: William Breathitt Gray, Philipp Zabel, Lee Jones, linux-iio,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Biju,

On Mon, Oct 10, 2022 at 4:53 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add RZ/G2L MTU3 counter driver. This IP supports the following
> phase counting modes on MTU1 and MTU2 channels
>
> 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> 2) 32-bit phase counting mode by cascading MTU1 and MTU2.
>
> This patch adds 3 counters by creating 3 logical channels
>         counter0: 16-bit phase counter on MTU1 channel
>         counter1: 16-bit phase counter on MTU2 channel
>         counter2: 32-bit phase counter by cascading MTU1 and MTU2
>                   channels.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
>           To compile this driver as a module, choose M here: the
>           module will be called rz-mtu3.
>
> +config MFD_RZ_MTU3_CNT
> +       tristate "RZ/G2L MTU3 counter driver"

"depends on COUNTER", else it fails to link.

> +       depends on MFD_RZ_MTU3 || COMPILE_TEST
> +       help
> +         Enable support for MTU3 counter driver found on Renesas RZ/G2L alike
> +         SoCs. This IP supports both 16-bit and 32-bit phase counting mode
> +         support.
> +
>  config MFD_STM32_LPTIMER
>         tristate "Support for STM32 Low-Power Timer"
>         depends on (ARCH_STM32 && OF) || COMPILE_TEST

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
  2022-10-25 13:50   ` Geert Uytterhoeven
@ 2022-10-25 13:58     ` Biju Das
  0 siblings, 0 replies; 6+ messages in thread
From: Biju Das @ 2022-10-25 13:58 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: William Breathitt Gray, Philipp Zabel, Lee Jones,
	linux-iio@vger.kernel.org, Geert Uytterhoeven, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad,
	linux-renesas-soc@vger.kernel.org

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
> 
> Hi Biju,
> 
> On Mon, Oct 10, 2022 at 4:53 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add RZ/G2L MTU3 counter driver. This IP supports the following phase
> > counting modes on MTU1 and MTU2 channels
> >
> > 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> > 2) 32-bit phase counting mode by cascading MTU1 and MTU2.
> >
> > This patch adds 3 counters by creating 3 logical channels
> >         counter0: 16-bit phase counter on MTU1 channel
> >         counter1: 16-bit phase counter on MTU2 channel
> >         counter2: 32-bit phase counter by cascading MTU1 and MTU2
> >                   channels.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
> >           To compile this driver as a module, choose M here: the
> >           module will be called rz-mtu3.
> >
> > +config MFD_RZ_MTU3_CNT
> > +       tristate "RZ/G2L MTU3 counter driver"
> 
> "depends on COUNTER", else it fails to link.

OK, will add this dependency in counter/Kconfig.

As pwm/counter maintainers agreed to move the core to timer bindings.
and instantiate child devices using mfd_add_device().

Cheers,
Biju


> 
> > +       depends on MFD_RZ_MTU3 || COMPILE_TEST
> > +       help
> > +         Enable support for MTU3 counter driver found on Renesas
> RZ/G2L alike
> > +         SoCs. This IP supports both 16-bit and 32-bit phase
> counting mode
> > +         support.
> > +
> >  config MFD_STM32_LPTIMER
> >         tristate "Support for STM32 Low-Power Timer"
> >         depends on (ARCH_STM32 && OF) || COMPILE_TEST
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a
> hacker. But when I'm talking to journalists I just say "programmer" or
> something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-25 13:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20221010145222.1047748-1-biju.das.jz@bp.renesas.com>
2022-10-10 14:52 ` [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver Biju Das
2022-10-17 16:40   ` William Breathitt Gray
2022-10-17 19:58     ` Biju Das
2022-10-17 23:02       ` William Breathitt Gray
2022-10-25 13:50   ` Geert Uytterhoeven
2022-10-25 13:58     ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox