[PATCH v2 00/15] MTU3 counter fixes and improvements

[PATCH v2 00/15] MTU3 counter fixes and improvements

[Cosmin Tanislav]

This series does some code quality improvements to the MTU3 counter
subdriver, while also fixing bugs and adding caching support so that
registers are not touched when the counter is disabled, as there is no
request API.

Patch 3 relies on the following series to enable runtime PM on the
parent device so that the PM domain handles it automatically.

https://lore.kernel.org/lkml/20261004174024.502949-1-cosmin-gabriel.tanislav.xa@renesas.com

V2:
 * remove clk.h include
 * remove struct rz_mtu3_cnt::clk documentation
 * fix struct rz_mtu3_cnt::ceiling documentation
 * add struct rz_mtu3_cnt::count documentation
 * add struct rz_mtu3_cnt::timer_mode documentation
 * add struct rz_mtu3_cnt::mtclkc_mtclkd documentation
 * add struct rz_mtu3_cnt::direction documentation
 * add struct rz_mtu3_cnt::cascade_counts_enable documentation
 * remove now-unused RZ_MTU3_MAX_HW_CNTR_CHANNELS macro
 * move rz_mtu3_set_lwa() call after rz_mtu3_disable()
 * mention the removed busy check
 * mask 16-bit counter value to preserve behavior

Cosmin Tanislav (15):
  counter: rz-mtu3-cnt: put runtime PM on initialization error path
  counter: rz-mtu3-cnt: do not assign struct rz_mtu3_channel::dev
  counter: rz-mtu3-cnt: remove manual runtime PM handling
  counter: rz-mtu3-cnt: use device-managed pm_runtime_enable()
  counter: rz-mtu3-cnt: read enable value from cache
  counter: rz-mtu3-cnt: disable channel before releasing
  counter: rz-mtu3-cnt: unify ceiling values
  counter: rz-mtu3-cnt: cache ceiling values
  counter: rz-mtu3-cnt: cache count values
  counter: rz-mtu3-cnt: cache function values
  counter: rz-mtu3-cnt: cache external input phase clock value
  counter: rz-mtu3-cnt: cache direction values
  counter: rz-mtu3-cnt: keep cascade_counts_enable in sync with usage
  counter: rz-mtu3-cnt: drop conditional locks
  counter: rz-mtu3-cnt: use pm_runtime_resume_and_get()

 drivers/counter/rz-mtu3-cnt.c | 458 +++++++++++-----------------------
 1 file changed, 149 insertions(+), 309 deletions(-)

-- 
2.55.0

[PATCH v2 01/15] counter: rz-mtu3-cnt: put runtime PM on initialization error path

[Cosmin Tanislav]

If rz_mtu3_initialize_counter() fails, the runtime PM usage count is not
decremented.

rz_mtu3_initialize_counter() will fail if the requested channel is busy.

Call pm_runtime_put() in the error path to decrement the usage count,
and flip the check to keep the success path straightforward.

Cc: stable@vger.kernel.org
Fixes: 0be8907359df ("counter: Add Renesas RZ/G2L MTU3a counter driver")
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * no changes

 drivers/counter/rz-mtu3-cnt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index 7bfb6979193c..48f183f0b54f 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -506,8 +506,12 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter,
 	if (enable) {
 		pm_runtime_get_sync(counter->parent);
 		ret = rz_mtu3_initialize_counter(counter, count->id);
-		if (ret == 0)
-			priv->count_is_enabled[count->id] = true;
+		if (ret) {
+			pm_runtime_put(counter->parent);
+			goto exit;
+		}
+
+		priv->count_is_enabled[count->id] = true;
 	} else {
 		rz_mtu3_terminate_counter(counter, count->id);
 		priv->count_is_enabled[count->id] = false;
-- 
2.55.0

[PATCH v2 02/15] counter: rz-mtu3-cnt: do not assign struct rz_mtu3_channel::dev

[Cosmin Tanislav]

struct rz_mtu3_channel::dev is now unused. Remove the assignment to
prepare for removing it entirely.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * no changes

 drivers/counter/rz-mtu3-cnt.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index 48f183f0b54f..4ad71ed3d1e1 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -843,7 +843,6 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 	struct rz_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent);
 	struct device *dev = &pdev->dev;
 	struct counter_device *counter;
-	struct rz_mtu3_channel *ch;
 	struct rz_mtu3_cnt *priv;
 	unsigned int i;
 	int ret;
@@ -856,12 +855,8 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 	priv->clk = ddata->clk;
 	priv->mtu_32bit_max = U32_MAX;
 	priv->ch = &ddata->channels[RZ_MTU3_CHAN_1];
-	ch = &priv->ch[0];
-	for (i = 0; i < RZ_MTU3_MAX_HW_CNTR_CHANNELS; i++) {
-		ch->dev = dev;
+	for (i = 0; i < RZ_MTU3_MAX_HW_CNTR_CHANNELS; i++)
 		priv->mtu_16bit_max[i] = U16_MAX;
-		ch++;
-	}
 
 	mutex_init(&priv->lock);
 	platform_set_drvdata(pdev, priv->clk);
-- 
2.55.0

[PATCH v2 03/15] counter: rz-mtu3-cnt: remove manual runtime PM handling

[Cosmin Tanislav]

The clock is already managed by the CPG power domain attached to the
parent device for all supported SoCs.

When runtime PM is enabled on the parent device, __device_attach()
temporarily resumes the parent for the duration of the child's probe and
drops the runtime PM reference afterwards. Runtime PM usage by the child
device also keeps the parent device active.

As a result, explicit runtime PM handling in the counter driver is not
necessary. Remove it.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * remove struct rz_mtu3_cnt::clk documentation
 * remove clk.h include

 drivers/counter/rz-mtu3-cnt.c | 44 +++--------------------------------
 1 file changed, 3 insertions(+), 41 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index 4ad71ed3d1e1..ecbb89c7e14a 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -5,7 +5,6 @@
  * Copyright (C) 2022 Renesas Electronics Corporation
  */
 
-#include <linux/clk.h>
 #include <linux/counter.h>
 #include <linux/mfd/rz-mtu3.h>
 #include <linux/module.h>
@@ -65,7 +64,6 @@
 /**
  * struct rz_mtu3_cnt - MTU3 counter private data
  *
- * @clk: MTU3 module clock
  * @lock: Lock to prevent concurrent access for ceiling and count
  * @ch: HW channels for the counters
  * @count_is_enabled: Enabled state of Counter value channel
@@ -73,7 +71,6 @@
  * @mtu_32bit_max: Cache for 32-bit counters
  */
 struct rz_mtu3_cnt {
-	struct clk *clk;
 	struct mutex lock;
 	struct rz_mtu3_channel *ch;
 	bool count_is_enabled[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
@@ -808,34 +805,11 @@ static struct counter_comp rz_mtu3_device_ext[] = {
 				 rz_mtu3_ext_input_phase_clock_select_enum),
 };
 
-static int rz_mtu3_cnt_pm_runtime_suspend(struct device *dev)
-{
-	struct clk *const clk = dev_get_drvdata(dev);
-
-	clk_disable_unprepare(clk);
-
-	return 0;
-}
-
-static int rz_mtu3_cnt_pm_runtime_resume(struct device *dev)
-{
-	struct clk *const clk = dev_get_drvdata(dev);
-
-	clk_prepare_enable(clk);
-
-	return 0;
-}
-
-static DEFINE_RUNTIME_DEV_PM_OPS(rz_mtu3_cnt_pm_ops,
-				 rz_mtu3_cnt_pm_runtime_suspend,
-				 rz_mtu3_cnt_pm_runtime_resume, NULL);
-
 static void rz_mtu3_cnt_pm_disable(void *data)
 {
 	struct device *dev = data;
 
 	pm_runtime_disable(dev);
-	pm_runtime_set_suspended(dev);
 }
 
 static int rz_mtu3_cnt_probe(struct platform_device *pdev)
@@ -852,20 +826,16 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv = counter_priv(counter);
-	priv->clk = ddata->clk;
 	priv->mtu_32bit_max = U32_MAX;
 	priv->ch = &ddata->channels[RZ_MTU3_CHAN_1];
 	for (i = 0; i < RZ_MTU3_MAX_HW_CNTR_CHANNELS; i++)
 		priv->mtu_16bit_max[i] = U16_MAX;
 
 	mutex_init(&priv->lock);
-	platform_set_drvdata(pdev, priv->clk);
-	clk_prepare_enable(priv->clk);
-	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 	ret = devm_add_action_or_reset(&pdev->dev, rz_mtu3_cnt_pm_disable, dev);
 	if (ret < 0)
-		goto disable_clock;
+		return ret;
 
 	counter->name = dev_name(dev);
 	counter->parent = dev;
@@ -879,24 +849,16 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 
 	/* Register Counter device */
 	ret = devm_counter_add(dev, counter);
-	if (ret < 0) {
-		dev_err_probe(dev, ret, "Failed to add counter\n");
-		goto disable_clock;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to add counter\n");
 
 	return 0;
-
-disable_clock:
-	clk_disable_unprepare(priv->clk);
-
-	return ret;
 }
 
 static struct platform_driver rz_mtu3_cnt_driver = {
 	.probe = rz_mtu3_cnt_probe,
 	.driver = {
 		.name = "rz-mtu3-counter",
-		.pm = pm_ptr(&rz_mtu3_cnt_pm_ops),
 	},
 };
 module_platform_driver(rz_mtu3_cnt_driver);
-- 
2.55.0

[PATCH v2 04/15] counter: rz-mtu3-cnt: use device-managed pm_runtime_enable()

[Cosmin Tanislav]

devm_pm_runtime_enable() has the same semantics as the current logic,
use it.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * no changes

 drivers/counter/rz-mtu3-cnt.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index ecbb89c7e14a..cc64ac363fa0 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -805,13 +805,6 @@ static struct counter_comp rz_mtu3_device_ext[] = {
 				 rz_mtu3_ext_input_phase_clock_select_enum),
 };
 
-static void rz_mtu3_cnt_pm_disable(void *data)
-{
-	struct device *dev = data;
-
-	pm_runtime_disable(dev);
-}
-
 static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 {
 	struct rz_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent);
@@ -832,9 +825,9 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 		priv->mtu_16bit_max[i] = U16_MAX;
 
 	mutex_init(&priv->lock);
-	pm_runtime_enable(&pdev->dev);
-	ret = devm_add_action_or_reset(&pdev->dev, rz_mtu3_cnt_pm_disable, dev);
-	if (ret < 0)
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
 		return ret;
 
 	counter->name = dev_name(dev);
-- 
2.55.0

[PATCH v2 05/15] counter: rz-mtu3-cnt: read enable value from cache

[Cosmin Tanislav]

Reading the enable value when PWM is in use returns -EINVAL because of
the checks inside rz_mtu3_lock_if_count_is_enabled(). Reading the enable
value should not be an issue even if PWM on that specific MTU channel is
enabled.

There is no need to check whether the underlying MTU3 channel is enabled
when determining whether a count is enabled, as we have a local
count_is_enabled array containing exactly that information, which cannot
be wrong.

Use the information in the local count_is_enabled array and allow
reading it even if PWM is in use.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * no changes

 drivers/counter/rz-mtu3-cnt.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index cc64ac363fa0..3e1323d40dbb 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -469,21 +469,10 @@ static void rz_mtu3_terminate_counter(struct counter_device *counter, int id)
 static int rz_mtu3_count_enable_read(struct counter_device *counter,
 				     struct counter_count *count, u8 *enable)
 {
-	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
-	struct rz_mtu3_channel *const ch1 = rz_mtu3_get_ch(counter, 0);
-	struct rz_mtu3_channel *const ch2 = rz_mtu3_get_ch(counter, 1);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
-
-	ret = rz_mtu3_lock_if_count_is_enabled(ch, priv, count->id);
-	if (ret)
-		return ret;
-
-	if (count->id == RZ_MTU3_32_BIT_CH)
-		*enable = rz_mtu3_is_enabled(ch1) && rz_mtu3_is_enabled(ch2);
-	else
-		*enable = rz_mtu3_is_enabled(ch);
 
+	mutex_lock(&priv->lock);
+	*enable = priv->count_is_enabled[count->id];
 	mutex_unlock(&priv->lock);
 
 	return 0;
-- 
2.55.0

[PATCH v2 06/15] counter: rz-mtu3-cnt: disable channel before releasing

[Cosmin Tanislav]

Releasing a channel marks it as not busy, allowing other threads to
claim it. With the current logic, it is possible for another thread to
claim the channel before disable is called, since we release the channel
before we disable it, causing the channel to possibly remain disabled
while it should have been enabled.

Disable the channel before releasing it.

Fixes: 0be8907359df ("counter: Add Renesas RZ/G2L MTU3a counter driver")
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * no changes

 drivers/counter/rz-mtu3-cnt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index 3e1323d40dbb..f3c4d857674f 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -456,13 +456,13 @@ static void rz_mtu3_terminate_counter(struct counter_device *counter, int id)
 	struct rz_mtu3_channel *const ch2 = rz_mtu3_get_ch(counter, 1);
 
 	if (id == RZ_MTU3_32_BIT_CH) {
-		rz_mtu3_release_channel(ch2);
-		rz_mtu3_release_channel(ch1);
 		rz_mtu3_disable(ch2);
 		rz_mtu3_disable(ch1);
+		rz_mtu3_release_channel(ch2);
+		rz_mtu3_release_channel(ch1);
 	} else {
-		rz_mtu3_release_channel(ch);
 		rz_mtu3_disable(ch);
+		rz_mtu3_release_channel(ch);
 	}
 }
 
-- 
2.55.0

[PATCH v2 07/15] counter: rz-mtu3-cnt: unify ceiling values

[Cosmin Tanislav]

Writing to the 32-bit ceiling value clobbers the 16-bit ceiling values
as both are stored in a union. This was probably done to save a few
bytes of memory, but that memory is then wasted on code to handle the
two separate variables.

Move them out of the union and use a single u32 array to hold ceiling
values for all channels to simplify the code.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * fix struct rz_mtu3_cnt::ceiling documentation
 * remove now-unused RZ_MTU3_MAX_HW_CNTR_CHANNELS macro

 drivers/counter/rz-mtu3-cnt.c | 36 +++++++----------------------------
 1 file changed, 7 insertions(+), 29 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index f3c4d857674f..dcc16f23f94e 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -58,7 +58,6 @@
 #define SIGNAL_C_ID	(2)
 #define SIGNAL_D_ID	(3)
 
-#define RZ_MTU3_MAX_HW_CNTR_CHANNELS	(2)
 #define RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS	(3)
 
 /**
@@ -67,17 +66,13 @@
  * @lock: Lock to prevent concurrent access for ceiling and count
  * @ch: HW channels for the counters
  * @count_is_enabled: Enabled state of Counter value channel
- * @mtu_16bit_max: Cache for 16-bit counters
- * @mtu_32bit_max: Cache for 32-bit counters
+ * @ceiling: Cache for ceiling values
  */
 struct rz_mtu3_cnt {
 	struct mutex lock;
 	struct rz_mtu3_channel *ch;
+	u32 ceiling[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	bool count_is_enabled[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
-	union {
-		u16 mtu_16bit_max[RZ_MTU3_MAX_HW_CNTR_CHANNELS];
-		u32 mtu_32bit_max;
-	};
 };
 
 static const enum counter_function rz_mtu3_count_functions[] = {
@@ -314,27 +309,13 @@ static int rz_mtu3_count_ceiling_read(struct counter_device *counter,
 {
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	const size_t ch_id = rz_mtu3_get_hw_ch(count->id);
 	int ret;
 
 	ret = rz_mtu3_lock_if_counter_is_valid(counter, ch, priv, count->id);
 	if (ret)
 		return ret;
 
-	switch (count->id) {
-	case RZ_MTU3_16_BIT_MTU1_CH:
-	case RZ_MTU3_16_BIT_MTU2_CH:
-		*ceiling = priv->mtu_16bit_max[ch_id];
-		break;
-	case RZ_MTU3_32_BIT_CH:
-		*ceiling = priv->mtu_32bit_max;
-		break;
-	default:
-		/* should never reach this path */
-		mutex_unlock(&priv->lock);
-		return -EINVAL;
-	}
-
+	*ceiling = priv->ceiling[count->id];
 	mutex_unlock(&priv->lock);
 	return 0;
 }
@@ -345,7 +326,6 @@ static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
 {
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	const size_t ch_id = rz_mtu3_get_hw_ch(count->id);
 	int ret;
 
 	ret = rz_mtu3_lock_if_counter_is_valid(counter, ch, priv, count->id);
@@ -359,14 +339,12 @@ static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
 			mutex_unlock(&priv->lock);
 			return -ERANGE;
 		}
-		priv->mtu_16bit_max[ch_id] = ceiling;
 		break;
 	case RZ_MTU3_32_BIT_CH:
 		if (ceiling > U32_MAX) {
 			mutex_unlock(&priv->lock);
 			return -ERANGE;
 		}
-		priv->mtu_32bit_max = ceiling;
 		break;
 	default:
 		/* should never reach this path */
@@ -381,6 +359,7 @@ static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
 		rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRA, ceiling);
 
 	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+	priv->ceiling[count->id] = ceiling;
 	pm_runtime_put(counter->parent);
 	mutex_unlock(&priv->lock);
 
@@ -800,7 +779,6 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 	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));
@@ -808,10 +786,10 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv = counter_priv(counter);
-	priv->mtu_32bit_max = U32_MAX;
+	priv->ceiling[RZ_MTU3_16_BIT_MTU1_CH] = U16_MAX;
+	priv->ceiling[RZ_MTU3_16_BIT_MTU2_CH] = U16_MAX;
+	priv->ceiling[RZ_MTU3_32_BIT_CH] = U32_MAX;
 	priv->ch = &ddata->channels[RZ_MTU3_CHAN_1];
-	for (i = 0; i < RZ_MTU3_MAX_HW_CNTR_CHANNELS; i++)
-		priv->mtu_16bit_max[i] = U16_MAX;
 
 	mutex_init(&priv->lock);
 
-- 
2.55.0

[PATCH v2 08/15] counter: rz-mtu3-cnt: cache ceiling values

[Cosmin Tanislav]

If an MTU3 channel is used in phase counting mode, and then in PWM mode,
when returning to counting mode the ceiling value is lost because PWM
mode clobbers the TGRA registers used to store the ceiling values.

Restore the ceiling value in the count initialization functions for both
16-bit and 32-bit channels.

To avoid writing the ceiling register when the count is disabled, and to
prepare for the removal of the rz_mtu3_lock_if_counter_is_valid()
function which is susceptible to TOCTOU races because of the ch->is_busy
check, only write ceiling if the count is enabled.

Since runtime PM is ensured to be resumed if the count is enabled, drop
the pm_runtime_get_sync() and pm_runtime_put() when writing the ceiling.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * no changes

 drivers/counter/rz-mtu3-cnt.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index dcc16f23f94e..485fde79a76a 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -146,6 +146,17 @@ static int rz_mtu3_lock_if_count_is_enabled(struct rz_mtu3_channel *const ch,
 	return 0;
 }
 
+static void rz_mtu3_set_ceiling(struct rz_mtu3_channel *const ch, int id,
+				u64 ceiling)
+{
+	if (id == RZ_MTU3_32_BIT_CH)
+		rz_mtu3_32bit_ch_write(ch, RZ_MTU3_TGRALW, ceiling);
+	else
+		rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRA, ceiling);
+
+	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+}
+
 static int rz_mtu3_count_read(struct counter_device *counter,
 			      struct counter_count *count, u64 *val)
 {
@@ -352,15 +363,9 @@ static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
 		return -EINVAL;
 	}
 
-	pm_runtime_get_sync(counter->parent);
-	if (count->id == RZ_MTU3_32_BIT_CH)
-		rz_mtu3_32bit_ch_write(ch, RZ_MTU3_TGRALW, ceiling);
-	else
-		rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRA, ceiling);
-
-	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+	if (priv->count_is_enabled[count->id])
+		rz_mtu3_set_ceiling(ch, count->id, ceiling);
 	priv->ceiling[count->id] = ceiling;
-	pm_runtime_put(counter->parent);
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -370,11 +375,14 @@ static void rz_mtu3_32bit_cnt_setting(struct counter_device *counter)
 {
 	struct rz_mtu3_channel *const ch1 = rz_mtu3_get_ch(counter, 0);
 	struct rz_mtu3_channel *const ch2 = rz_mtu3_get_ch(counter, 1);
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
 	/* Phase counting mode 1 is used as default in initialization. */
 	rz_mtu3_8bit_ch_write(ch1, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_PH_CNT_MODE_1);
 
-	rz_mtu3_8bit_ch_write(ch1, RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+	rz_mtu3_set_ceiling(ch1, RZ_MTU3_32_BIT_CH,
+			    priv->ceiling[RZ_MTU3_32_BIT_CH]);
+
 	rz_mtu3_8bit_ch_write(ch1, RZ_MTU3_TIOR, RZ_MTU3_TIOR_IC_BOTH);
 
 	rz_mtu3_enable(ch1);
@@ -384,11 +392,13 @@ static void rz_mtu3_32bit_cnt_setting(struct counter_device *counter)
 static void rz_mtu3_16bit_cnt_setting(struct counter_device *counter, int id)
 {
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, id);
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
 	/* Phase counting mode 1 is used as default in initialization. */
 	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_PH_CNT_MODE_1);
 
-	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+	rz_mtu3_set_ceiling(ch, id, priv->ceiling[id]);
+
 	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIOR, RZ_MTU3_TIOR_NO_OUTPUT);
 	rz_mtu3_enable(ch);
 }
-- 
2.55.0

[PATCH v2 09/15] counter: rz-mtu3-cnt: cache count values

[Cosmin Tanislav]

If an MTU3 channel is used in counting mode, and then in PWM mode, when
returning to counting mode the count value is lost because the TCNT
register is overwritten by PWM mode.

To handle this issue, save the count value on disable and restore it on
enable, and also save it inside rz_mtu3_count_write().

To avoid writing the count value register when the count is disabled,
and to prepare for the removal of the rz_mtu3_lock_if_counter_is_valid()
function which is susceptible to TOCTOU races because of the ch->is_busy
check, only read or write the count value if the count is enabled.

Mask the count value to the channel's width before caching it, so an
out-of-range write to a 16-bit channel is truncated the same way
a direct register write would have been.

Since runtime PM is ensured to be resumed if the count is enabled, drop
the pm_runtime_get_sync() and pm_runtime_put() calls.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * add struct rz_mtu3_cnt::count documentation
 * mask 16-bit counter value to preserve behavior

 drivers/counter/rz-mtu3-cnt.c | 47 ++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index 485fde79a76a..61cf26e20471 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -67,11 +67,13 @@
  * @ch: HW channels for the counters
  * @count_is_enabled: Enabled state of Counter value channel
  * @ceiling: Cache for ceiling values
+ * @count: Cache for count values
  */
 struct rz_mtu3_cnt {
 	struct mutex lock;
 	struct rz_mtu3_channel *ch;
 	u32 ceiling[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
+	u32 count[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	bool count_is_enabled[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 };
 
@@ -157,6 +159,23 @@ static void rz_mtu3_set_ceiling(struct rz_mtu3_channel *const ch, int id,
 	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
 }
 
+static u32 rz_mtu3_get_count(struct rz_mtu3_channel *const ch, int id)
+{
+	if (id == RZ_MTU3_32_BIT_CH)
+		return rz_mtu3_32bit_ch_read(ch, RZ_MTU3_TCNTLW);
+	else
+		return rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TCNT);
+}
+
+static void rz_mtu3_set_count(struct rz_mtu3_channel *const ch, int id,
+			      u32 count)
+{
+	if (id == RZ_MTU3_32_BIT_CH)
+		rz_mtu3_32bit_ch_write(ch, RZ_MTU3_TCNTLW, count);
+	else
+		rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TCNT, count);
+}
+
 static int rz_mtu3_count_read(struct counter_device *counter,
 			      struct counter_count *count, u64 *val)
 {
@@ -168,12 +187,9 @@ static int rz_mtu3_count_read(struct counter_device *counter,
 	if (ret)
 		return ret;
 
-	pm_runtime_get_sync(counter->parent);
-	if (count->id == RZ_MTU3_32_BIT_CH)
-		*val = rz_mtu3_32bit_ch_read(ch, RZ_MTU3_TCNTLW);
-	else
-		*val = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TCNT);
-	pm_runtime_put(counter->parent);
+	if (priv->count_is_enabled[count->id])
+		priv->count[count->id] = rz_mtu3_get_count(ch, count->id);
+	*val = priv->count[count->id];
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -184,18 +200,19 @@ static int rz_mtu3_count_write(struct counter_device *counter,
 {
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+	u32 count_val = val;
 	int ret;
 
+	if (count->id != RZ_MTU3_32_BIT_CH)
+		count_val &= U16_MAX;
+
 	ret = rz_mtu3_lock_if_counter_is_valid(counter, ch, priv, count->id);
 	if (ret)
 		return ret;
 
-	pm_runtime_get_sync(counter->parent);
-	if (count->id == RZ_MTU3_32_BIT_CH)
-		rz_mtu3_32bit_ch_write(ch, RZ_MTU3_TCNTLW, val);
-	else
-		rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TCNT, val);
-	pm_runtime_put(counter->parent);
+	if (priv->count_is_enabled[count->id])
+		rz_mtu3_set_count(ch, count->id, count_val);
+	priv->count[count->id] = count_val;
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -382,6 +399,8 @@ static void rz_mtu3_32bit_cnt_setting(struct counter_device *counter)
 
 	rz_mtu3_set_ceiling(ch1, RZ_MTU3_32_BIT_CH,
 			    priv->ceiling[RZ_MTU3_32_BIT_CH]);
+	rz_mtu3_set_count(ch1, RZ_MTU3_32_BIT_CH,
+			  priv->count[RZ_MTU3_32_BIT_CH]);
 
 	rz_mtu3_8bit_ch_write(ch1, RZ_MTU3_TIOR, RZ_MTU3_TIOR_IC_BOTH);
 
@@ -398,6 +417,7 @@ static void rz_mtu3_16bit_cnt_setting(struct counter_device *counter, int id)
 	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_PH_CNT_MODE_1);
 
 	rz_mtu3_set_ceiling(ch, id, priv->ceiling[id]);
+	rz_mtu3_set_count(ch, id, priv->count[id]);
 
 	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIOR, RZ_MTU3_TIOR_NO_OUTPUT);
 	rz_mtu3_enable(ch);
@@ -443,6 +463,9 @@ static void rz_mtu3_terminate_counter(struct counter_device *counter, int id)
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, id);
 	struct rz_mtu3_channel *const ch1 = rz_mtu3_get_ch(counter, 0);
 	struct rz_mtu3_channel *const ch2 = rz_mtu3_get_ch(counter, 1);
+	struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+	priv->count[id] = rz_mtu3_get_count(ch, id);
 
 	if (id == RZ_MTU3_32_BIT_CH) {
 		rz_mtu3_disable(ch2);
-- 
2.55.0

[PATCH v2 10/15] counter: rz-mtu3-cnt: cache function values

[Cosmin Tanislav]

If an MTU3 channel is used in a specific phase counting mode, when
disabling it and enabling it again, the user set mode is lost because
the timer mode register (TMDR1) is initialized to the default value.

Save the timer mode after setting it and restore it on enable.
Use the cached value to determine the function in
rz_mtu3_count_function_read_helper().

Avoid writing the TMDR1 register when the count is disabled to prevent
writing registers without actually owning the channel, as the busy check
inside ch->is_busy can race with PWM operations.

Since runtime PM is ensured to be resumed if the count is enabled, drop
the pm_runtime_get_sync() and pm_runtime_put() calls.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * add struct rz_mtu3_cnt::timer_mode documentation

 drivers/counter/rz-mtu3-cnt.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index 61cf26e20471..3557a524a1b3 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -68,12 +68,14 @@
  * @count_is_enabled: Enabled state of Counter value channel
  * @ceiling: Cache for ceiling values
  * @count: Cache for count values
+ * @timer_mode: Cache for timer mode values
  */
 struct rz_mtu3_cnt {
 	struct mutex lock;
 	struct rz_mtu3_channel *ch;
 	u32 ceiling[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	u32 count[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
+	u8 timer_mode[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	bool count_is_enabled[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 };
 
@@ -218,17 +220,11 @@ static int rz_mtu3_count_write(struct counter_device *counter,
 	return 0;
 }
 
-static int rz_mtu3_count_function_read_helper(struct rz_mtu3_channel *const ch,
-					      struct counter_device *const counter,
+static int rz_mtu3_count_function_read_helper(struct rz_mtu3_cnt *const priv,
+					      int id,
 					      enum counter_function *function)
 {
-	u8 timer_mode;
-
-	pm_runtime_get_sync(counter->parent);
-	timer_mode = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TMDR1);
-	pm_runtime_put(counter->parent);
-
-	switch (timer_mode & RZ_MTU3_TMDR1_PH_CNT_MODE_MASK) {
+	switch (priv->timer_mode[id]) {
 	case RZ_MTU3_TMDR1_PH_CNT_MODE_1:
 		*function = COUNTER_FUNCTION_QUADRATURE_X4;
 		return 0;
@@ -260,7 +256,7 @@ static int rz_mtu3_count_function_read(struct counter_device *counter,
 	if (ret)
 		return ret;
 
-	ret = rz_mtu3_count_function_read_helper(ch, counter, function);
+	ret = rz_mtu3_count_function_read_helper(priv, count->id, function);
 	mutex_unlock(&priv->lock);
 
 	return ret;
@@ -299,9 +295,9 @@ static int rz_mtu3_count_function_write(struct counter_device *counter,
 		return -EINVAL;
 	}
 
-	pm_runtime_get_sync(counter->parent);
-	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, timer_mode);
-	pm_runtime_put(counter->parent);
+	if (priv->count_is_enabled[count->id])
+		rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, timer_mode);
+	priv->timer_mode[count->id] = timer_mode;
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -394,8 +390,8 @@ static void rz_mtu3_32bit_cnt_setting(struct counter_device *counter)
 	struct rz_mtu3_channel *const ch2 = rz_mtu3_get_ch(counter, 1);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
-	/* Phase counting mode 1 is used as default in initialization. */
-	rz_mtu3_8bit_ch_write(ch1, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_PH_CNT_MODE_1);
+	rz_mtu3_8bit_ch_write(ch1, RZ_MTU3_TMDR1,
+			      priv->timer_mode[RZ_MTU3_32_BIT_CH]);
 
 	rz_mtu3_set_ceiling(ch1, RZ_MTU3_32_BIT_CH,
 			    priv->ceiling[RZ_MTU3_32_BIT_CH]);
@@ -413,9 +409,7 @@ static void rz_mtu3_16bit_cnt_setting(struct counter_device *counter, int id)
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
-	/* Phase counting mode 1 is used as default in initialization. */
-	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_PH_CNT_MODE_1);
-
+	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, priv->timer_mode[id]);
 	rz_mtu3_set_ceiling(ch, id, priv->ceiling[id]);
 	rz_mtu3_set_count(ch, id, priv->count[id]);
 
@@ -645,7 +639,7 @@ static int rz_mtu3_action_read(struct counter_device *counter,
 	if (ret)
 		return ret;
 
-	ret = rz_mtu3_count_function_read_helper(ch, counter, &function);
+	ret = rz_mtu3_count_function_read_helper(priv, count->id, &function);
 	if (ret) {
 		mutex_unlock(&priv->lock);
 		return ret;
@@ -812,6 +806,7 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 	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));
@@ -823,6 +818,8 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 	priv->ceiling[RZ_MTU3_16_BIT_MTU2_CH] = U16_MAX;
 	priv->ceiling[RZ_MTU3_32_BIT_CH] = U32_MAX;
 	priv->ch = &ddata->channels[RZ_MTU3_CHAN_1];
+	for (i = 0; i < RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS; i++)
+		priv->timer_mode[i] = RZ_MTU3_TMDR1_PH_CNT_MODE_1;
 
 	mutex_init(&priv->lock);
 
-- 
2.55.0

[PATCH v2 11/15] counter: rz-mtu3-cnt: cache external input phase clock value

[Cosmin Tanislav]

PHCKSEL selects between MTCLKA-MTCLKB and MTCLKC-MTCLKD pins for the
external input phase clock.

PHCKSEL only affects the 16-bit count backed by MTU2 and the 32-bit
count.

Cache the value written to external_input_phase_clock_select and restore
it on enable to bring the external_input_phase_clock_select sysfs entry
in line with the other entries.

Initialize the cached value to true, matching PHCKSEL's reset value so
behavior is unchanged for a count that gets enabled without
external_input_phase_clock_select being written explicitly.

Always use the cached PHCKSEL value to remove the need to access
registers while counts might not be enabled.

Only restore it for the count backed by MTU2 as changing PHCKSEL for
MTU1 does not affect the pins used for the external input phase clock.

Check if any of these counts are enabled before writing the new PHCKSEL
value.

This prevents writing registers without actually owning the channel,
as the busy check inside ch->is_busy can race with PWM operations.

Since runtime PM is ensured to be resumed if the count is enabled, drop
the pm_runtime_get_sync() and pm_runtime_put() calls.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * add struct rz_mtu3_cnt::mtclkc_mtclkd documentation

 drivers/counter/rz-mtu3-cnt.c | 36 +++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index 3557a524a1b3..b1714ab1f7db 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -69,6 +69,7 @@
  * @ceiling: Cache for ceiling values
  * @count: Cache for count values
  * @timer_mode: Cache for timer mode values
+ * @mtclkc_mtclkd: Cache for external input phase clock select value
  */
 struct rz_mtu3_cnt {
 	struct mutex lock;
@@ -77,6 +78,7 @@ struct rz_mtu3_cnt {
 	u32 count[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	u8 timer_mode[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	bool count_is_enabled[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
+	bool mtclkc_mtclkd;
 };
 
 static const enum counter_function rz_mtu3_count_functions[] = {
@@ -178,6 +180,12 @@ static void rz_mtu3_set_count(struct rz_mtu3_channel *const ch, int id,
 		rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TCNT, count);
 }
 
+static void rz_mtu3_set_phcksel(struct rz_mtu3_channel *const ch, bool enable)
+{
+	rz_mtu3_shared_reg_update_bit(ch, RZ_MTU3_TMDR3, RZ_MTU3_TMDR3_PHCKSEL,
+				      enable);
+}
+
 static int rz_mtu3_count_read(struct counter_device *counter,
 			      struct counter_count *count, u64 *val)
 {
@@ -390,6 +398,8 @@ static void rz_mtu3_32bit_cnt_setting(struct counter_device *counter)
 	struct rz_mtu3_channel *const ch2 = rz_mtu3_get_ch(counter, 1);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
+	rz_mtu3_set_phcksel(priv->ch, priv->mtclkc_mtclkd);
+
 	rz_mtu3_8bit_ch_write(ch1, RZ_MTU3_TMDR1,
 			      priv->timer_mode[RZ_MTU3_32_BIT_CH]);
 
@@ -409,6 +419,8 @@ static void rz_mtu3_16bit_cnt_setting(struct counter_device *counter, int id)
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
+	if (id == RZ_MTU3_16_BIT_MTU2_CH)
+		rz_mtu3_set_phcksel(priv->ch, priv->mtclkc_mtclkd);
 	rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, priv->timer_mode[id]);
 	rz_mtu3_set_ceiling(ch, id, priv->ceiling[id]);
 	rz_mtu3_set_count(ch, id, priv->count[id]);
@@ -571,17 +583,13 @@ static int rz_mtu3_ext_input_phase_clock_select_get(struct counter_device *count
 						    u32 *ext_input_phase_clock_select)
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	unsigned long tmdr;
 	int ret;
 
 	ret = rz_mtu3_lock_if_ch0_is_enabled(priv);
 	if (ret)
 		return ret;
 
-	pm_runtime_get_sync(counter->parent);
-	tmdr = rz_mtu3_shared_reg_read(priv->ch, RZ_MTU3_TMDR3);
-	pm_runtime_put(counter->parent);
-	*ext_input_phase_clock_select = test_bit(RZ_MTU3_TMDR3_PHCKSEL, &tmdr);
+	*ext_input_phase_clock_select = priv->mtclkc_mtclkd;
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -597,11 +605,10 @@ static int rz_mtu3_ext_input_phase_clock_select_set(struct counter_device *count
 	if (ret)
 		return ret;
 
-	pm_runtime_get_sync(counter->parent);
-	rz_mtu3_shared_reg_update_bit(priv->ch, RZ_MTU3_TMDR3,
-				      RZ_MTU3_TMDR3_PHCKSEL,
-				      ext_input_phase_clock_select);
-	pm_runtime_put(counter->parent);
+	if (priv->count_is_enabled[RZ_MTU3_16_BIT_MTU2_CH] ||
+	    priv->count_is_enabled[RZ_MTU3_32_BIT_CH])
+		rz_mtu3_set_phcksel(priv->ch, ext_input_phase_clock_select);
+	priv->mtclkc_mtclkd = ext_input_phase_clock_select;
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -631,8 +638,6 @@ static int rz_mtu3_action_read(struct counter_device *counter,
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 	enum counter_function function;
-	bool mtclkc_mtclkd;
-	unsigned long tmdr;
 	int ret;
 
 	ret = rz_mtu3_lock_if_count_is_enabled(ch, priv, count->id);
@@ -649,10 +654,8 @@ static int rz_mtu3_action_read(struct counter_device *counter,
 	*action = COUNTER_SYNAPSE_ACTION_NONE;
 
 	if (count->id != RZ_MTU3_16_BIT_MTU1_CH) {
-		tmdr = rz_mtu3_shared_reg_read(priv->ch, RZ_MTU3_TMDR3);
-		mtclkc_mtclkd = test_bit(RZ_MTU3_TMDR3_PHCKSEL, &tmdr);
-		if ((mtclkc_mtclkd && is_signal_ab) ||
-		    (!mtclkc_mtclkd && !is_signal_ab)) {
+		if ((priv->mtclkc_mtclkd && is_signal_ab) ||
+		    (!priv->mtclkc_mtclkd && !is_signal_ab)) {
 			mutex_unlock(&priv->lock);
 			return 0;
 		}
@@ -814,6 +817,7 @@ static int rz_mtu3_cnt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv = counter_priv(counter);
+	priv->mtclkc_mtclkd = true;
 	priv->ceiling[RZ_MTU3_16_BIT_MTU1_CH] = U16_MAX;
 	priv->ceiling[RZ_MTU3_16_BIT_MTU2_CH] = U16_MAX;
 	priv->ceiling[RZ_MTU3_32_BIT_CH] = U32_MAX;
-- 
2.55.0

[PATCH v2 12/15] counter: rz-mtu3-cnt: cache direction values

[Cosmin Tanislav]

Reading registers without the count being enabled is an issue as the
ch->is_busy check inside rz_mtu3_lock_if_count_is_enabled() can race
with PWM operations.

Cache the direction value on every read and on disable, and return the
cached value if the count is disabled.

Since runtime PM is ensured to be resumed if the count is enabled, drop
the pm_runtime_get_sync() and pm_runtime_put() calls.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * add struct rz_mtu3_cnt::direction documentation

 drivers/counter/rz-mtu3-cnt.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index b1714ab1f7db..e36c61ae35d1 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -70,6 +70,7 @@
  * @count: Cache for count values
  * @timer_mode: Cache for timer mode values
  * @mtclkc_mtclkd: Cache for external input phase clock select value
+ * @direction: Cache for direction values
  */
 struct rz_mtu3_cnt {
 	struct mutex lock;
@@ -78,6 +79,7 @@ struct rz_mtu3_cnt {
 	u32 count[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	u8 timer_mode[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	bool count_is_enabled[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
+	bool direction[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	bool mtclkc_mtclkd;
 };
 
@@ -186,6 +188,13 @@ static void rz_mtu3_set_phcksel(struct rz_mtu3_channel *const ch, bool enable)
 				      enable);
 }
 
+static bool rz_mtu3_get_direction(struct rz_mtu3_channel *const ch)
+{
+	u8 tsr = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TSR);
+
+	return !!(tsr & RZ_MTU3_TSR_TCFD);
+}
+
 static int rz_mtu3_count_read(struct counter_device *counter,
 			      struct counter_count *count, u64 *val)
 {
@@ -318,18 +327,17 @@ static int rz_mtu3_count_direction_read(struct counter_device *counter,
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 	int ret;
-	u8 tsr;
 
 	ret = rz_mtu3_lock_if_count_is_enabled(ch, priv, count->id);
 	if (ret)
 		return ret;
 
-	pm_runtime_get_sync(counter->parent);
-	tsr = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TSR);
-	pm_runtime_put(counter->parent);
+	if (priv->count_is_enabled[count->id])
+		priv->direction[count->id] = rz_mtu3_get_direction(ch);
 
-	*direction = (tsr & RZ_MTU3_TSR_TCFD) ?
+	*direction = priv->direction[count->id] ?
 		COUNTER_COUNT_DIRECTION_FORWARD : COUNTER_COUNT_DIRECTION_BACKWARD;
+
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -472,6 +480,7 @@ static void rz_mtu3_terminate_counter(struct counter_device *counter, int id)
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
 	priv->count[id] = rz_mtu3_get_count(ch, id);
+	priv->direction[id] = rz_mtu3_get_direction(ch);
 
 	if (id == RZ_MTU3_32_BIT_CH) {
 		rz_mtu3_disable(ch2);
-- 
2.55.0

[PATCH v2 13/15] counter: rz-mtu3-cnt: keep cascade_counts_enable in sync with usage

[Cosmin Tanislav]

LWA is enabled/disabled by writing 1/0 to the cascade_counts_enable
sysfs entry.

The 32-bit channel cannot function without LWA being enabled, and the
16-bit channels cannot function with LWA enabled.

PWM mode cannot function with LWA enabled.

It is possible to accidentally leave LWA enabled while trying to use PWM
mode, leading to unintended access to the TGRA or TGRB registers of
MTU1 and MTU2, which should not be accessed while LWA is enabled.

It is also possible to toggle LWA while PWM is enabled on MTU2, as the
checks inside rz_mtu3_lock_if_ch0_is_enabled() do not check if MTU2 is
busy, even if this setting affects MTU2.

To handle this, write 1 to the LWA bit when enabling the 32-bit channel,
and automatically write 0 to it when disabling the 32-bit channel, to
ensure that LWA is always in the correct state for any possible usage.

Remove rz_mtu3_is_counter_invalid() checks as it is now guaranteed
that LWA has the proper value if a count is enabled.

This also drops the busy check that rz_mtu3_lock_if_counter_is_valid()
carried. A count cannot be enabled without the underlying MTU channel
being claimed as busy by the count driver itself, so the check is
redundant.

Keep the cascade_counts_enable sysfs entry for ABI compatibility.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * add struct rz_mtu3_cnt::cascade_counts_enable documentation
 * move rz_mtu3_set_lwa() call after rz_mtu3_disable()
 * mention the removed busy check in the message

 drivers/counter/rz-mtu3-cnt.c | 95 +++++++----------------------------
 1 file changed, 18 insertions(+), 77 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index e36c61ae35d1..9be65df799af 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -71,6 +71,7 @@
  * @timer_mode: Cache for timer mode values
  * @mtclkc_mtclkd: Cache for external input phase clock select value
  * @direction: Cache for direction values
+ * @cascade_enable: Cache for cascade_counts_enable value
  */
 struct rz_mtu3_cnt {
 	struct mutex lock;
@@ -81,6 +82,7 @@ struct rz_mtu3_cnt {
 	bool count_is_enabled[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	bool direction[RZ_MTU3_MAX_LOGICAL_CNTR_CHANNELS];
 	bool mtclkc_mtclkd;
+	bool cascade_enable;
 };
 
 static const enum counter_function rz_mtu3_count_functions[] = {
@@ -102,44 +104,6 @@ static inline struct rz_mtu3_channel *rz_mtu3_get_ch(struct counter_device *coun
 	return &priv->ch[ch_id];
 }
 
-static bool rz_mtu3_is_counter_invalid(struct counter_device *counter, int id)
-{
-	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	unsigned long tmdr;
-
-	pm_runtime_get_sync(counter->parent);
-	tmdr = rz_mtu3_shared_reg_read(priv->ch, RZ_MTU3_TMDR3);
-	pm_runtime_put(counter->parent);
-
-	if (id == RZ_MTU3_32_BIT_CH && test_bit(RZ_MTU3_TMDR3_LWA, &tmdr))
-		return false;
-
-	if (id != RZ_MTU3_32_BIT_CH && !test_bit(RZ_MTU3_TMDR3_LWA, &tmdr))
-		return false;
-
-	return true;
-}
-
-static int rz_mtu3_lock_if_counter_is_valid(struct counter_device *counter,
-					    struct rz_mtu3_channel *const ch,
-					    struct rz_mtu3_cnt *const priv,
-					    int id)
-{
-	mutex_lock(&priv->lock);
-
-	if (ch->is_busy && !priv->count_is_enabled[id]) {
-		mutex_unlock(&priv->lock);
-		return -EINVAL;
-	}
-
-	if (rz_mtu3_is_counter_invalid(counter, id)) {
-		mutex_unlock(&priv->lock);
-		return -EBUSY;
-	}
-
-	return 0;
-}
-
 static int rz_mtu3_lock_if_count_is_enabled(struct rz_mtu3_channel *const ch,
 					    struct rz_mtu3_cnt *const priv,
 					    int id)
@@ -195,17 +159,19 @@ static bool rz_mtu3_get_direction(struct rz_mtu3_channel *const ch)
 	return !!(tsr & RZ_MTU3_TSR_TCFD);
 }
 
+static void rz_mtu3_set_lwa(struct rz_mtu3_channel *const ch, bool enable)
+{
+	rz_mtu3_shared_reg_update_bit(ch, RZ_MTU3_TMDR3, RZ_MTU3_TMDR3_LWA,
+				      enable);
+}
+
 static int rz_mtu3_count_read(struct counter_device *counter,
 			      struct counter_count *count, u64 *val)
 {
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
-
-	ret = rz_mtu3_lock_if_counter_is_valid(counter, ch, priv, count->id);
-	if (ret)
-		return ret;
 
+	mutex_lock(&priv->lock);
 	if (priv->count_is_enabled[count->id])
 		priv->count[count->id] = rz_mtu3_get_count(ch, count->id);
 	*val = priv->count[count->id];
@@ -220,15 +186,11 @@ static int rz_mtu3_count_write(struct counter_device *counter,
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 	u32 count_val = val;
-	int ret;
 
 	if (count->id != RZ_MTU3_32_BIT_CH)
 		count_val &= U16_MAX;
 
-	ret = rz_mtu3_lock_if_counter_is_valid(counter, ch, priv, count->id);
-	if (ret)
-		return ret;
-
+	mutex_lock(&priv->lock);
 	if (priv->count_is_enabled[count->id])
 		rz_mtu3_set_count(ch, count->id, count_val);
 	priv->count[count->id] = count_val;
@@ -347,14 +309,9 @@ static int rz_mtu3_count_ceiling_read(struct counter_device *counter,
 				      struct counter_count *count,
 				      u64 *ceiling)
 {
-	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
-
-	ret = rz_mtu3_lock_if_counter_is_valid(counter, ch, priv, count->id);
-	if (ret)
-		return ret;
 
+	mutex_lock(&priv->lock);
 	*ceiling = priv->ceiling[count->id];
 	mutex_unlock(&priv->lock);
 	return 0;
@@ -366,11 +323,8 @@ static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
 {
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
 
-	ret = rz_mtu3_lock_if_counter_is_valid(counter, ch, priv, count->id);
-	if (ret)
-		return ret;
+	mutex_lock(&priv->lock);
 
 	switch (count->id) {
 	case RZ_MTU3_16_BIT_MTU1_CH:
@@ -407,6 +361,7 @@ static void rz_mtu3_32bit_cnt_setting(struct counter_device *counter)
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
 	rz_mtu3_set_phcksel(priv->ch, priv->mtclkc_mtclkd);
+	rz_mtu3_set_lwa(ch1, true);
 
 	rz_mtu3_8bit_ch_write(ch1, RZ_MTU3_TMDR1,
 			      priv->timer_mode[RZ_MTU3_32_BIT_CH]);
@@ -485,6 +440,7 @@ static void rz_mtu3_terminate_counter(struct counter_device *counter, int id)
 	if (id == RZ_MTU3_32_BIT_CH) {
 		rz_mtu3_disable(ch2);
 		rz_mtu3_disable(ch1);
+		rz_mtu3_set_lwa(ch1, false);
 		rz_mtu3_release_channel(ch2);
 		rz_mtu3_release_channel(ch1);
 	} else {
@@ -553,17 +509,9 @@ static int rz_mtu3_cascade_counts_enable_get(struct counter_device *counter,
 					     u8 *cascade_enable)
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	unsigned long tmdr;
-	int ret;
 
-	ret = rz_mtu3_lock_if_ch0_is_enabled(priv);
-	if (ret)
-		return ret;
-
-	pm_runtime_get_sync(counter->parent);
-	tmdr = rz_mtu3_shared_reg_read(priv->ch, RZ_MTU3_TMDR3);
-	pm_runtime_put(counter->parent);
-	*cascade_enable = test_bit(RZ_MTU3_TMDR3_LWA, &tmdr);
+	mutex_lock(&priv->lock);
+	*cascade_enable = priv->cascade_enable;
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -573,16 +521,9 @@ static int rz_mtu3_cascade_counts_enable_set(struct counter_device *counter,
 					     u8 cascade_enable)
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
 
-	ret = rz_mtu3_lock_if_ch0_is_enabled(priv);
-	if (ret)
-		return ret;
-
-	pm_runtime_get_sync(counter->parent);
-	rz_mtu3_shared_reg_update_bit(priv->ch, RZ_MTU3_TMDR3,
-				      RZ_MTU3_TMDR3_LWA, cascade_enable);
-	pm_runtime_put(counter->parent);
+	mutex_lock(&priv->lock);
+	priv->cascade_enable = cascade_enable;
 	mutex_unlock(&priv->lock);
 
 	return 0;
-- 
2.55.0

[PATCH v2 14/15] counter: rz-mtu3-cnt: drop conditional locks

[Cosmin Tanislav]

All read and write operations now only access the hardware if the
respective count is enabled.

As a count cannot be enabled without the underlying MTU channel(s) being
claimed as busy by the count driver, drop conditional locks and replace
them with simple locks.

Use guard() where possible.

Remove the conditional locking functions as there are no users left.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * no changes

 drivers/counter/rz-mtu3-cnt.c | 120 ++++++++--------------------------
 1 file changed, 26 insertions(+), 94 deletions(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index 9be65df799af..d1d955ee67f1 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -104,20 +104,6 @@ static inline struct rz_mtu3_channel *rz_mtu3_get_ch(struct counter_device *coun
 	return &priv->ch[ch_id];
 }
 
-static int rz_mtu3_lock_if_count_is_enabled(struct rz_mtu3_channel *const ch,
-					    struct rz_mtu3_cnt *const priv,
-					    int id)
-{
-	mutex_lock(&priv->lock);
-
-	if (ch->is_busy && !priv->count_is_enabled[id]) {
-		mutex_unlock(&priv->lock);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static void rz_mtu3_set_ceiling(struct rz_mtu3_channel *const ch, int id,
 				u64 ceiling)
 {
@@ -171,11 +157,11 @@ static int rz_mtu3_count_read(struct counter_device *counter,
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
-	mutex_lock(&priv->lock);
+	guard(mutex)(&priv->lock);
+
 	if (priv->count_is_enabled[count->id])
 		priv->count[count->id] = rz_mtu3_get_count(ch, count->id);
 	*val = priv->count[count->id];
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -190,11 +176,11 @@ static int rz_mtu3_count_write(struct counter_device *counter,
 	if (count->id != RZ_MTU3_32_BIT_CH)
 		count_val &= U16_MAX;
 
-	mutex_lock(&priv->lock);
+	guard(mutex)(&priv->lock);
+
 	if (priv->count_is_enabled[count->id])
 		rz_mtu3_set_count(ch, count->id, count_val);
 	priv->count[count->id] = count_val;
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -227,18 +213,11 @@ static int rz_mtu3_count_function_read(struct counter_device *counter,
 				       struct counter_count *count,
 				       enum counter_function *function)
 {
-	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
 
-	ret = rz_mtu3_lock_if_count_is_enabled(ch, priv, count->id);
-	if (ret)
-		return ret;
+	guard(mutex)(&priv->lock);
 
-	ret = rz_mtu3_count_function_read_helper(priv, count->id, function);
-	mutex_unlock(&priv->lock);
-
-	return ret;
+	return rz_mtu3_count_function_read_helper(priv, count->id, function);
 }
 
 static int rz_mtu3_count_function_write(struct counter_device *counter,
@@ -248,11 +227,6 @@ static int rz_mtu3_count_function_write(struct counter_device *counter,
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 	u8 timer_mode;
-	int ret;
-
-	ret = rz_mtu3_lock_if_count_is_enabled(ch, priv, count->id);
-	if (ret)
-		return ret;
 
 	switch (function) {
 	case COUNTER_FUNCTION_QUADRATURE_X4:
@@ -270,14 +244,14 @@ static int rz_mtu3_count_function_write(struct counter_device *counter,
 		 *  - need to add RZ_MTU3_TMDR1_PH_CNT_MODE_3
 		 *  - need to add RZ_MTU3_TMDR1_PH_CNT_MODE_5
 		 */
-		mutex_unlock(&priv->lock);
 		return -EINVAL;
 	}
 
+	guard(mutex)(&priv->lock);
+
 	if (priv->count_is_enabled[count->id])
 		rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, timer_mode);
 	priv->timer_mode[count->id] = timer_mode;
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -288,11 +262,8 @@ static int rz_mtu3_count_direction_read(struct counter_device *counter,
 {
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
 
-	ret = rz_mtu3_lock_if_count_is_enabled(ch, priv, count->id);
-	if (ret)
-		return ret;
+	guard(mutex)(&priv->lock);
 
 	if (priv->count_is_enabled[count->id])
 		priv->direction[count->id] = rz_mtu3_get_direction(ch);
@@ -300,8 +271,6 @@ static int rz_mtu3_count_direction_read(struct counter_device *counter,
 	*direction = priv->direction[count->id] ?
 		COUNTER_COUNT_DIRECTION_FORWARD : COUNTER_COUNT_DIRECTION_BACKWARD;
 
-	mutex_unlock(&priv->lock);
-
 	return 0;
 }
 
@@ -311,9 +280,9 @@ static int rz_mtu3_count_ceiling_read(struct counter_device *counter,
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
-	mutex_lock(&priv->lock);
+	guard(mutex)(&priv->lock);
 	*ceiling = priv->ceiling[count->id];
-	mutex_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -324,32 +293,26 @@ static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
 	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
-	mutex_lock(&priv->lock);
-
 	switch (count->id) {
 	case RZ_MTU3_16_BIT_MTU1_CH:
 	case RZ_MTU3_16_BIT_MTU2_CH:
-		if (ceiling > U16_MAX) {
-			mutex_unlock(&priv->lock);
+		if (ceiling > U16_MAX)
 			return -ERANGE;
-		}
 		break;
 	case RZ_MTU3_32_BIT_CH:
-		if (ceiling > U32_MAX) {
-			mutex_unlock(&priv->lock);
+		if (ceiling > U32_MAX)
 			return -ERANGE;
-		}
 		break;
 	default:
 		/* should never reach this path */
-		mutex_unlock(&priv->lock);
 		return -EINVAL;
 	}
 
+	guard(mutex)(&priv->lock);
+
 	if (priv->count_is_enabled[count->id])
 		rz_mtu3_set_ceiling(ch, count->id, ceiling);
 	priv->ceiling[count->id] = ceiling;
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -454,9 +417,8 @@ static int rz_mtu3_count_enable_read(struct counter_device *counter,
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
-	mutex_lock(&priv->lock);
+	guard(mutex)(&priv->lock);
 	*enable = priv->count_is_enabled[count->id];
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -467,17 +429,17 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter,
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 	int ret = 0;
 
-	mutex_lock(&priv->lock);
+	guard(mutex)(&priv->lock);
 
 	if (priv->count_is_enabled[count->id] == enable)
-		goto exit;
+		return 0;
 
 	if (enable) {
 		pm_runtime_get_sync(counter->parent);
 		ret = rz_mtu3_initialize_counter(counter, count->id);
 		if (ret) {
 			pm_runtime_put(counter->parent);
-			goto exit;
+			return ret;
 		}
 
 		priv->count_is_enabled[count->id] = true;
@@ -487,32 +449,16 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter,
 		pm_runtime_put(counter->parent);
 	}
 
-exit:
-	mutex_unlock(&priv->lock);
-
 	return ret;
 }
 
-static int rz_mtu3_lock_if_ch0_is_enabled(struct rz_mtu3_cnt *const priv)
-{
-	mutex_lock(&priv->lock);
-	if (priv->ch->is_busy && !(priv->count_is_enabled[RZ_MTU3_16_BIT_MTU1_CH] ||
-				   priv->count_is_enabled[RZ_MTU3_32_BIT_CH])) {
-		mutex_unlock(&priv->lock);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int rz_mtu3_cascade_counts_enable_get(struct counter_device *counter,
 					     u8 *cascade_enable)
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
-	mutex_lock(&priv->lock);
+	guard(mutex)(&priv->lock);
 	*cascade_enable = priv->cascade_enable;
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -522,9 +468,8 @@ static int rz_mtu3_cascade_counts_enable_set(struct counter_device *counter,
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 
-	mutex_lock(&priv->lock);
+	guard(mutex)(&priv->lock);
 	priv->cascade_enable = cascade_enable;
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -533,14 +478,9 @@ static int rz_mtu3_ext_input_phase_clock_select_get(struct counter_device *count
 						    u32 *ext_input_phase_clock_select)
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
-
-	ret = rz_mtu3_lock_if_ch0_is_enabled(priv);
-	if (ret)
-		return ret;
 
+	guard(mutex)(&priv->lock);
 	*ext_input_phase_clock_select = priv->mtclkc_mtclkd;
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -549,17 +489,13 @@ static int rz_mtu3_ext_input_phase_clock_select_set(struct counter_device *count
 						    u32 ext_input_phase_clock_select)
 {
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
-	int ret;
 
-	ret = rz_mtu3_lock_if_ch0_is_enabled(priv);
-	if (ret)
-		return ret;
+	guard(mutex)(&priv->lock);
 
 	if (priv->count_is_enabled[RZ_MTU3_16_BIT_MTU2_CH] ||
 	    priv->count_is_enabled[RZ_MTU3_32_BIT_CH])
 		rz_mtu3_set_phcksel(priv->ch, ext_input_phase_clock_select);
 	priv->mtclkc_mtclkd = ext_input_phase_clock_select;
-	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -585,14 +521,11 @@ static int rz_mtu3_action_read(struct counter_device *counter,
 {
 	const bool is_signal_ab = (synapse->signal->id == SIGNAL_A_ID) ||
 				  (synapse->signal->id == SIGNAL_B_ID);
-	struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
 	struct rz_mtu3_cnt *const priv = counter_priv(counter);
 	enum counter_function function;
 	int ret;
 
-	ret = rz_mtu3_lock_if_count_is_enabled(ch, priv, count->id);
-	if (ret)
-		return ret;
+	mutex_lock(&priv->lock);
 
 	ret = rz_mtu3_count_function_read_helper(priv, count->id, &function);
 	if (ret) {
@@ -611,6 +544,8 @@ static int rz_mtu3_action_read(struct counter_device *counter,
 		}
 	}
 
+	mutex_unlock(&priv->lock);
+
 	switch (function) {
 	case COUNTER_FUNCTION_PULSE_DIRECTION:
 		/*
@@ -637,12 +572,9 @@ static int rz_mtu3_action_read(struct counter_device *counter,
 		break;
 	default:
 		/* should never reach this path */
-		mutex_unlock(&priv->lock);
 		return -EINVAL;
 	}
 
-	mutex_unlock(&priv->lock);
-
 	return 0;
 }
 
-- 
2.55.0

[PATCH v2 15/15] counter: rz-mtu3-cnt: use pm_runtime_resume_and_get()

[Cosmin Tanislav]

pm_runtime_get_sync() may increment the runtime PM usage count even if
the resume fails, which requires an explicit pm_runtime_put_noidle() to
balance it.

This driver ignores the return value of pm_runtime_get_sync(), risking
accessing device registers even if resume failed.

Replace it with pm_runtime_resume_and_get() and handle errors properly.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * no changes

 drivers/counter/rz-mtu3-cnt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c
index d1d955ee67f1..83bdda57bf35 100644
--- a/drivers/counter/rz-mtu3-cnt.c
+++ b/drivers/counter/rz-mtu3-cnt.c
@@ -435,7 +435,10 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter,
 		return 0;
 
 	if (enable) {
-		pm_runtime_get_sync(counter->parent);
+		ret = pm_runtime_resume_and_get(counter->parent);
+		if (ret)
+			return ret;
+
 		ret = rz_mtu3_initialize_counter(counter, count->id);
 		if (ret) {
 			pm_runtime_put(counter->parent);
-- 
2.55.0

RE: [PATCH v2 00/15] MTU3 counter fixes and improvements

[Biju Das]

Hi Cosmin,

Thanks for the series.

> -----Original Message-----
> From: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> Sent: 04 October 2026 18:48
> Subject: [PATCH v2 00/15] MTU3 counter fixes and improvements
> 
> This series does some code quality improvements to the MTU3 counter subdriver, while also fixing bugs and
> adding caching support so that registers are not touched when the counter is disabled, as there is no
> request API.

I believe you need to rebase this series on top of[1].

Since this patch series is caching userspace values in memory, 
It can support suspend to RAM as well as in suspend to RAM,we lose
the register contents, this patch series can address that limitation.
Can you also please prepare a follow up patches for STR support?

[1] https://lore.kernel.org/all/20260903090719.3498955-1-dayou5941@163.com/

Cheers,
Biju

> 
> Patch 3 relies on the following series to enable runtime PM on the parent device so that the PM domain
> handles it automatically.
> 
> https://lore.kernel.org/lkml/20261004174024.502949-1-cosmin-gabriel.tanislav.xa@renesas.com
> 
> V2:
>  * remove clk.h include
>  * remove struct rz_mtu3_cnt::clk documentation
>  * fix struct rz_mtu3_cnt::ceiling documentation
>  * add struct rz_mtu3_cnt::count documentation
>  * add struct rz_mtu3_cnt::timer_mode documentation
>  * add struct rz_mtu3_cnt::mtclkc_mtclkd documentation
>  * add struct rz_mtu3_cnt::direction documentation
>  * add struct rz_mtu3_cnt::cascade_counts_enable documentation
>  * remove now-unused RZ_MTU3_MAX_HW_CNTR_CHANNELS macro
>  * move rz_mtu3_set_lwa() call after rz_mtu3_disable()
>  * mention the removed busy check
>  * mask 16-bit counter value to preserve behavior
> 
> Cosmin Tanislav (15):
>   counter: rz-mtu3-cnt: put runtime PM on initialization error path
>   counter: rz-mtu3-cnt: do not assign struct rz_mtu3_channel::dev
>   counter: rz-mtu3-cnt: remove manual runtime PM handling
>   counter: rz-mtu3-cnt: use device-managed pm_runtime_enable()
>   counter: rz-mtu3-cnt: read enable value from cache
>   counter: rz-mtu3-cnt: disable channel before releasing
>   counter: rz-mtu3-cnt: unify ceiling values
>   counter: rz-mtu3-cnt: cache ceiling values
>   counter: rz-mtu3-cnt: cache count values
>   counter: rz-mtu3-cnt: cache function values
>   counter: rz-mtu3-cnt: cache external input phase clock value
>   counter: rz-mtu3-cnt: cache direction values
>   counter: rz-mtu3-cnt: keep cascade_counts_enable in sync with usage
>   counter: rz-mtu3-cnt: drop conditional locks
>   counter: rz-mtu3-cnt: use pm_runtime_resume_and_get()
> 
>  drivers/counter/rz-mtu3-cnt.c | 458 +++++++++++-----------------------
>  1 file changed, 149 insertions(+), 309 deletions(-)
> 
> --
> 2.55.0

RE: [PATCH v2 00/15] MTU3 counter fixes and improvements

[Cosmin-Gabriel Tanislav]

> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Monday, October 5, 2026 11:45 AM
> 
> Hi Cosmin,
> 
> Thanks for the series.
> 
> > -----Original Message-----
> > From: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> > Sent: 04 October 2026 18:48
> > Subject: [PATCH v2 00/15] MTU3 counter fixes and improvements
> >
> > This series does some code quality improvements to the MTU3 counter subdriver, while also fixing
> bugs and
> > adding caching support so that registers are not touched when the counter is disabled, as there is
> no
> > request API.
> 
> I believe you need to rebase this series on top of[1].
> 
> Since this patch series is caching userspace values in memory,
> It can support suspend to RAM as well as in suspend to RAM,we lose
> the register contents, this patch series can address that limitation.
> Can you also please prepare a follow up patches for STR support?
> 
> [1] https://lore.kernel.org/all/20260903090719.3498955-1-dayou5941@163.com/
> 

Hi Biju.

The changes from patch 1 of Li's series will simply be dropped by us if
I rebase on top of it.

The changes from patch 2 of Li's series are superseded by our cleanups.

The changes from patch 3 of Li's series depend on his patch 2 changes,
and are equivalent to our patch 1.

I think it would be best if Li re-submitted just patch 3 alone without
any dependencies, so that it can be accepted, since he also submitted
it before we did and we duplicated his work accidentally.

After his patch is accepted I will re-submit this series without patch
1.

What do you think, William, Li?

Re:RE: [PATCH v2 00/15] MTU3 counter fixes and improvements

[李佑鸿]

At 2026-10-05 19:47:04, "Cosmin-Gabriel Tanislav" <cosmin-gabriel.tanislav.xa@renesas.com> wrote:
>> From: Biju Das <biju.das.jz@bp.renesas.com>
>> Sent: Monday, October 5, 2026 11:45 AM
>> 
>> Hi Cosmin,
>> 
>> Thanks for the series.
>> 
>> > -----Original Message-----
>> > From: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
>> > Sent: 04 October 2026 18:48
>> > Subject: [PATCH v2 00/15] MTU3 counter fixes and improvements
>> >
>> > This series does some code quality improvements to the MTU3 counter subdriver, while also fixing
>> bugs and
>> > adding caching support so that registers are not touched when the counter is disabled, as there is
>> no
>> > request API.
>> 
>> I believe you need to rebase this series on top of[1].
>> 
>> Since this patch series is caching userspace values in memory,
>> It can support suspend to RAM as well as in suspend to RAM,we lose
>> the register contents, this patch series can address that limitation.
>> Can you also please prepare a follow up patches for STR support?
>> 
>> [1] https://lore.kernel.org/all/20260903090719.3498955-1-dayou5941@163.com/
>> 
>
>Hi Biju.
>
>The changes from patch 1 of Li's series will simply be dropped by us if
>I rebase on top of it.
>
>The changes from patch 2 of Li's series are superseded by our cleanups.
>
>The changes from patch 3 of Li's series depend on his patch 2 changes,
>and are equivalent to our patch 1.
>
>I think it would be best if Li re-submitted just patch 3 alone without
>any dependencies, so that it can be accepted, since he also submitted
>it before we did and we duplicated his work accidentally.
>
>After his patch is accepted I will re-submit this series without patch
>1.
>

>What do you think, William, Li?


Hi Cosmin, William,


Agreed. I'll resend just the runtime PM put on the initialization error path,
based on the current pm_runtime_get_sync() code. It no longer depends on the
other two patches.


Thanks,
Li