[PATCH 0/3] MTU3 MFD improvements

[PATCH 0/3] MTU3 MFD improvements

[Cosmin Tanislav]

This series does some code quality improvements to the MTU3 MFD driver,
and enables runtime PM to allow simplifications in the sub-drivers by
relying on PM domains rather than open-coding clock handling.

Cosmin Tanislav (3):
  mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev
  mfd: rz-mtu3: reuse shared_reg_update_bit() in start_stop_ch()
  mfd: rz-mtu3: enable runtime PM

 drivers/mfd/rz-mtu3.c | 71 ++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 41 deletions(-)

-- 
2.55.0

[PATCH 1/3] mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev

[Cosmin Tanislav]

Channels 1 and 2 are used by both the PWM and counter drivers. Both
subdrivers assign struct rz_mtu3_channel::dev to their own device
instance for those shared channels, so the value stored in it depends on
which subdriver binds last.

The MFD core helpers currently use ch->dev->parent to retrieve the
parent driver data, which makes them depend on subdriver-populated
state.

This is fragile for shared channels and unnecessarily couples the core
to child driver initialization.

Introduce a rz_mtu3_ch_to_priv() helper that takes in a pointer to a
struct rz_mtu3_channel and returns a pointer to the struct rz_mtu3_priv.

This is possible because the struct rz_mtu3_channel contains its index
(channel_number) in the struct rz_mtu3's channels array, allowing us to
retrieve the pointer of the first channel, and then its container.

By doing this, we remove the dependency on subdriver assigned data from
the parent driver.

Keep struct rz_mtu3_channel::dev for now, as it is still used by the PWM
and counter drivers.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---
 drivers/mfd/rz-mtu3.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
index 0a254e61ec0a..d809436e2124 100644
--- a/drivers/mfd/rz-mtu3.c
+++ b/drivers/mfd/rz-mtu3.c
@@ -60,10 +60,17 @@ static bool rz_mtu3_is_16bit_shared_reg(u16 offset)
 		offset == RZ_MTU3_TCNTSA || offset == RZ_MTU3_TCNTSB);
 }
 
+static inline struct rz_mtu3_priv *rz_mtu3_ch_to_priv(struct rz_mtu3_channel *ch)
+{
+	struct rz_mtu3_channel *first_ch = ch - ch->channel_number;
+	struct rz_mtu3 *mtu = container_of(first_ch, struct rz_mtu3, channels[0]);
+
+	return mtu->priv_data;
+}
+
 u16 rz_mtu3_shared_reg_read(struct rz_mtu3_channel *ch, u16 offset)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 
 	if (rz_mtu3_is_16bit_shared_reg(offset))
 		return readw(priv->mmio + offset);
@@ -74,8 +81,7 @@ EXPORT_SYMBOL_GPL(rz_mtu3_shared_reg_read);
 
 u8 rz_mtu3_8bit_ch_read(struct rz_mtu3_channel *ch, u16 offset)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	u16 ch_offs;
 
 	ch_offs = rz_mtu3_8bit_ch_reg_offs[ch->channel_number][offset];
@@ -86,8 +92,7 @@ EXPORT_SYMBOL_GPL(rz_mtu3_8bit_ch_read);
 
 u16 rz_mtu3_16bit_ch_read(struct rz_mtu3_channel *ch, u16 offset)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	u16 ch_offs;
 
 	/* MTU8 doesn't have 16-bit registers */
@@ -102,8 +107,7 @@ EXPORT_SYMBOL_GPL(rz_mtu3_16bit_ch_read);
 
 u32 rz_mtu3_32bit_ch_read(struct rz_mtu3_channel *ch, u16 offset)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	u16 ch_offs;
 
 	if (ch->channel_number != RZ_MTU3_CHAN_1 && ch->channel_number != RZ_MTU3_CHAN_8)
@@ -117,8 +121,7 @@ EXPORT_SYMBOL_GPL(rz_mtu3_32bit_ch_read);
 
 void rz_mtu3_8bit_ch_write(struct rz_mtu3_channel *ch, u16 offset, u8 val)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	u16 ch_offs;
 
 	ch_offs = rz_mtu3_8bit_ch_reg_offs[ch->channel_number][offset];
@@ -128,8 +131,7 @@ EXPORT_SYMBOL_GPL(rz_mtu3_8bit_ch_write);
 
 void rz_mtu3_16bit_ch_write(struct rz_mtu3_channel *ch, u16 offset, u16 val)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	u16 ch_offs;
 
 	/* MTU8 doesn't have 16-bit registers */
@@ -143,8 +145,7 @@ EXPORT_SYMBOL_GPL(rz_mtu3_16bit_ch_write);
 
 void rz_mtu3_32bit_ch_write(struct rz_mtu3_channel *ch, u16 offset, u32 val)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	u16 ch_offs;
 
 	if (ch->channel_number != RZ_MTU3_CHAN_1 && ch->channel_number != RZ_MTU3_CHAN_8)
@@ -157,8 +158,7 @@ EXPORT_SYMBOL_GPL(rz_mtu3_32bit_ch_write);
 
 void rz_mtu3_shared_reg_write(struct rz_mtu3_channel *ch, u16 offset, u16 value)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 
 	if (rz_mtu3_is_16bit_shared_reg(offset))
 		writew(value, priv->mmio + offset);
@@ -170,8 +170,7 @@ EXPORT_SYMBOL_GPL(rz_mtu3_shared_reg_write);
 void rz_mtu3_shared_reg_update_bit(struct rz_mtu3_channel *ch, u16 offset,
 				   u16 pos, u8 val)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	unsigned long tmdr, flags;
 
 	spin_lock_irqsave(&priv->lock, flags);
@@ -244,8 +243,7 @@ static u8 rz_mtu3_get_tstr_bit_pos(struct rz_mtu3_channel *ch)
 
 static void rz_mtu3_start_stop_ch(struct rz_mtu3_channel *ch, bool start)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	unsigned long flags, tstr;
 	u16 offset;
 	u8 bitpos;
@@ -265,8 +263,7 @@ static void rz_mtu3_start_stop_ch(struct rz_mtu3_channel *ch, bool start)
 
 bool rz_mtu3_is_enabled(struct rz_mtu3_channel *ch)
 {
-	struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
-	struct rz_mtu3_priv *priv = mtu->priv_data;
+	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
 	unsigned long flags, tstr;
 	u16 offset;
 	u8 bitpos;
-- 
2.55.0

[PATCH 2/3] mfd: rz-mtu3: reuse shared_reg_update_bit() in start_stop_ch()

[Cosmin Tanislav]

rz_mtu3_start_stop_ch() performs the same shared register update
sequence already implemented by rz_mtu3_shared_reg_update_bit().

Reuse the helper instead of open-coding the read-modify-write sequence.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---
 drivers/mfd/rz-mtu3.c | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
index d809436e2124..88aa1fedf1ef 100644
--- a/drivers/mfd/rz-mtu3.c
+++ b/drivers/mfd/rz-mtu3.c
@@ -171,12 +171,12 @@ void rz_mtu3_shared_reg_update_bit(struct rz_mtu3_channel *ch, u16 offset,
 				   u16 pos, u8 val)
 {
 	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
-	unsigned long tmdr, flags;
+	unsigned long reg_val, flags;
 
 	spin_lock_irqsave(&priv->lock, flags);
-	tmdr = rz_mtu3_shared_reg_read(ch, offset);
-	__assign_bit(pos, &tmdr, !!val);
-	rz_mtu3_shared_reg_write(ch, offset, tmdr);
+	reg_val = rz_mtu3_shared_reg_read(ch, offset);
+	__assign_bit(pos, &reg_val, !!val);
+	rz_mtu3_shared_reg_write(ch, offset, reg_val);
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 EXPORT_SYMBOL_GPL(rz_mtu3_shared_reg_update_bit);
@@ -243,22 +243,8 @@ static u8 rz_mtu3_get_tstr_bit_pos(struct rz_mtu3_channel *ch)
 
 static void rz_mtu3_start_stop_ch(struct rz_mtu3_channel *ch, bool start)
 {
-	struct rz_mtu3_priv *priv = rz_mtu3_ch_to_priv(ch);
-	unsigned long flags, tstr;
-	u16 offset;
-	u8 bitpos;
-
-	offset = rz_mtu3_get_tstr_offset(ch);
-	bitpos = rz_mtu3_get_tstr_bit_pos(ch);
-
-	/* start stop register shared by multiple timer channels */
-	spin_lock_irqsave(&priv->lock, flags);
-
-	tstr = rz_mtu3_shared_reg_read(ch, offset);
-	__assign_bit(bitpos, &tstr, start);
-	rz_mtu3_shared_reg_write(ch, offset, tstr);
-
-	spin_unlock_irqrestore(&priv->lock, flags);
+	rz_mtu3_shared_reg_update_bit(ch, rz_mtu3_get_tstr_offset(ch),
+				      rz_mtu3_get_tstr_bit_pos(ch), start);
 }
 
 bool rz_mtu3_is_enabled(struct rz_mtu3_channel *ch)
-- 
2.55.0

[PATCH 3/3] mfd: rz-mtu3: enable runtime PM

[Cosmin Tanislav]

The MTU3 child devices use runtime PM, but the PM domain is attached to
the parent MFD device rather than to the individual child devices, and
the child devices open-code their own clock handling instead of using
the PM domain logic.

Enable runtime PM on the parent device so runtime PM usage by the child
devices can propagate to the parent.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---
 drivers/mfd/rz-mtu3.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
index 88aa1fedf1ef..e24e67e7a66c 100644
--- a/drivers/mfd/rz-mtu3.c
+++ b/drivers/mfd/rz-mtu3.c
@@ -13,6 +13,7 @@
 #include <linux/mfd/rz-mtu3.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/spinlock.h>
 
@@ -298,6 +299,7 @@ static int rz_mtu3_probe(struct platform_device *pdev)
 	struct rz_mtu3 *ddata;
 	struct reset_control *rstc;
 	unsigned int i;
+	int ret;
 
 	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
@@ -330,6 +332,10 @@ static int rz_mtu3_probe(struct platform_device *pdev)
 		mutex_init(&ddata->channels[i].lock);
 	}
 
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
 	return devm_mfd_add_devices(dev, 0, rz_mtu3_devs,
 				    ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
 }
-- 
2.55.0

Re: [PATCH 2/3] mfd: rz-mtu3: reuse shared_reg_update_bit() in start_stop_ch()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Commit message description is not capitalized.
--

commit 67ee74ff3faf4f3c19662b6041b12e91d577cab9
Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

mfd: rz-mtu3: reuse shared_reg_update_bit() in start_stop_ch()

This commit simplifies the channel start and stop logic by reusing the
existing rz_mtu3_shared_reg_update_bit() helper instead of open-coding the
read-modify-write register sequence.

> mfd: rz-mtu3: reuse shared_reg_update_bit() in start_stop_ch()

[Severity: Low]
This isn't a bug, but the MFD subsystem formatting rules require the
description following the prefix to be capitalized. Should "reuse" be
changed to "Reuse" to match the subsystem conventions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914201456.2018403-1-cosmin-gabriel.tanislav.xa@renesas.com?part=2

Re: [PATCH 3/3] mfd: rz-mtu3: enable runtime PM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Parent device runtime PM is enabled without setting it to active, causing child devices' probe to fail PM activation and permanently leak their clocks.
- [Low] The commit message description is not capitalized.
--

commit e5d000ec993dddaf773352525f11aae34e95a812
Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

mfd: rz-mtu3: enable runtime PM

This commit enables runtime PM on the rz-mtu3 parent device. This allows
child devices to correctly propagate their runtime PM usage to the parent's
PM domain, replacing the open-coded clock handling within the child devices
themselves.

> mfd: rz-mtu3: enable runtime PM

[Severity: Low]
This isn't a bug, but the commit message subject violates the MFD subsystem
formatting rules which mandate capitalizing the description after the
subsystem prefix. Should it be "mfd: rz-mtu3: Enable runtime PM"?

> diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> index 88aa1fedf1ef5..e24e67e7a66cb 100644
> --- a/drivers/mfd/rz-mtu3.c
> +++ b/drivers/mfd/rz-mtu3.c
[ ... ]
> @@ -330,6 +332,10 @@ static int rz_mtu3_probe(struct platform_device *pdev)
>  		mutex_init(&ddata->channels[i].lock);
>  	}
>  
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;
> +

[Severity: High]
Does enabling runtime PM on the parent without calling
pm_runtime_set_active() leave the parent device in the RPM_SUSPENDED state?

During child driver probe, if a child calls pm_runtime_set_active(), it
would receive -EBUSY because the parent is suspended and ignore_children is
false. If the child driver ignores this error, its runtime PM status would
remain RPM_SUSPENDED.

Since the child considers itself already suspended, will subsequent calls to
pm_runtime_idle() or pm_runtime_put() in the child driver be ignored by the
PM core? If the runtime_suspend callback is never invoked to balance the
clk_prepare_enable() called during probe, could this result in a permanent
clock leak?

>  	return devm_mfd_add_devices(dev, 0, rz_mtu3_devs,
>  				    ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914201456.2018403-1-cosmin-gabriel.tanislav.xa@renesas.com?part=3

Re: [PATCH 1/3] mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch replaces a standard API call with a bespoke pointer arithmetic helper to retrieve parent data, cementing an MFD architectural guideline violation.
- [Low] The commit message description starts with a lowercase letter, violating MFD subsystem guidelines.
--

commit 8098c2550655f8bdfa6c7acd0a4b1dfbc4e57e2a
Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev

Channels 1 and 2 are used by both the PWM and counter drivers. Both
subdrivers assign struct rz_mtu3_channel::dev to their own device
instance for those shared channels, so the value stored in it depends on
which subdriver binds last.

> mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev

[Severity: Low]
Does this commit message subject violate the MFD subsystem formatting
conventions? The guidelines mandate capitalizing the description after the
subsystem prefix, e.g., "mfd: <Driver>: <Capitalized description>".

> diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> index 0a254e61ec0a5..d809436e2124c 100644
> --- a/drivers/mfd/rz-mtu3.c
> +++ b/drivers/mfd/rz-mtu3.c
> @@ -60,10 +60,17 @@ static bool rz_mtu3_is_16bit_shared_reg(u16 offset)
>  		offset == RZ_MTU3_TCNTSA || offset == RZ_MTU3_TCNTSB);
>  }
>  
> +static inline struct rz_mtu3_priv *rz_mtu3_ch_to_priv(struct rz_mtu3_channel *ch)
> +{
> +	struct rz_mtu3_channel *first_ch = ch - ch->channel_number;
> +	struct rz_mtu3 *mtu = container_of(first_ch, struct rz_mtu3, channels[0]);
> +
> +	return mtu->priv_data;
> +}

[Severity: Medium]
Does the introduction of rz_mtu3_ch_to_priv() violate MFD guidelines for
parent data access?

The MFD guidelines explicitly state that child drivers must directly access
parent data using standard APIs (like dev_get_drvdata) and to avoid writing
bespoke helpers in the parent. 

While this replaces dev_get_drvdata(ch->dev->parent) to avoid depending on
subdriver-populated state, substituting it with a custom helper using pointer
arithmetic (ch - ch->channel_number) tightly couples the implementation to
the physical layout of the channels array.

Could the child drivers be refactored to retrieve the parent data themselves
instead of introducing this bespoke accessor?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914201456.2018403-1-cosmin-gabriel.tanislav.xa@renesas.com?part=1

Re: [PATCH 1/3] mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev

[Lee Jones]

On Mon, 14 Sep 2026, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The patch replaces a standard API call with a bespoke pointer arithmetic helper to retrieve parent data, cementing an MFD architectural guideline violation.
> - [Low] The commit message description starts with a lowercase letter, violating MFD subsystem guidelines.

Please review, explain or fix the Sashiko review(s).

If this is a set, this request includes the other patches too.

> --
> 
> commit 8098c2550655f8bdfa6c7acd0a4b1dfbc4e57e2a
> Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> 
> mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev
> 
> Channels 1 and 2 are used by both the PWM and counter drivers. Both
> subdrivers assign struct rz_mtu3_channel::dev to their own device
> instance for those shared channels, so the value stored in it depends on
> which subdriver binds last.
> 
> > mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev
> 
> [Severity: Low]
> Does this commit message subject violate the MFD subsystem formatting
> conventions? The guidelines mandate capitalizing the description after the
> subsystem prefix, e.g., "mfd: <Driver>: <Capitalized description>".
> 
> > diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> > index 0a254e61ec0a5..d809436e2124c 100644
> > --- a/drivers/mfd/rz-mtu3.c
> > +++ b/drivers/mfd/rz-mtu3.c
> > @@ -60,10 +60,17 @@ static bool rz_mtu3_is_16bit_shared_reg(u16 offset)
> >  		offset == RZ_MTU3_TCNTSA || offset == RZ_MTU3_TCNTSB);
> >  }
> >  
> > +static inline struct rz_mtu3_priv *rz_mtu3_ch_to_priv(struct rz_mtu3_channel *ch)
> > +{
> > +	struct rz_mtu3_channel *first_ch = ch - ch->channel_number;
> > +	struct rz_mtu3 *mtu = container_of(first_ch, struct rz_mtu3, channels[0]);
> > +
> > +	return mtu->priv_data;
> > +}
> 
> [Severity: Medium]
> Does the introduction of rz_mtu3_ch_to_priv() violate MFD guidelines for
> parent data access?
> 
> The MFD guidelines explicitly state that child drivers must directly access
> parent data using standard APIs (like dev_get_drvdata) and to avoid writing
> bespoke helpers in the parent. 
> 
> While this replaces dev_get_drvdata(ch->dev->parent) to avoid depending on
> subdriver-populated state, substituting it with a custom helper using pointer
> arithmetic (ch - ch->channel_number) tightly couples the implementation to
> the physical layout of the channels array.
> 
> Could the child drivers be refactored to retrieve the parent data themselves
> instead of introducing this bespoke accessor?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260914201456.2018403-1-cosmin-gabriel.tanislav.xa@renesas.com?part=1

-- 
Lee Jones

RE: [PATCH 1/3] mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev

[Cosmin-Gabriel Tanislav]

> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Monday, September 14, 2026 11:28 PM
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The patch replaces a standard API call with a bespoke pointer arithmetic helper to retrieve
> parent data, cementing an MFD architectural guideline violation.
> - [Low] The commit message description starts with a lowercase letter, violating MFD subsystem
> guidelines.
> --
> 
> commit 8098c2550655f8bdfa6c7acd0a4b1dfbc4e57e2a
> Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> 
> mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev
> 
> Channels 1 and 2 are used by both the PWM and counter drivers. Both
> subdrivers assign struct rz_mtu3_channel::dev to their own device
> instance for those shared channels, so the value stored in it depends on
> which subdriver binds last.
> 
> > mfd: rz-mtu3: do not use struct rz_mtu3_channel::dev
> 
> [Severity: Low]
> Does this commit message subject violate the MFD subsystem formatting
> conventions? The guidelines mandate capitalizing the description after the
> subsystem prefix, e.g., "mfd: <Driver>: <Capitalized description>".
> 

Ack.

> > diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> > index 0a254e61ec0a5..d809436e2124c 100644
> > --- a/drivers/mfd/rz-mtu3.c
> > +++ b/drivers/mfd/rz-mtu3.c
> > @@ -60,10 +60,17 @@ static bool rz_mtu3_is_16bit_shared_reg(u16 offset)
> >  		offset == RZ_MTU3_TCNTSA || offset == RZ_MTU3_TCNTSB);
> >  }
> >
> > +static inline struct rz_mtu3_priv *rz_mtu3_ch_to_priv(struct rz_mtu3_channel *ch)
> > +{
> > +	struct rz_mtu3_channel *first_ch = ch - ch->channel_number;
> > +	struct rz_mtu3 *mtu = container_of(first_ch, struct rz_mtu3, channels[0]);
> > +
> > +	return mtu->priv_data;
> > +}
> 
> [Severity: Medium]
> Does the introduction of rz_mtu3_ch_to_priv() violate MFD guidelines for
> parent data access?
> 
> The MFD guidelines explicitly state that child drivers must directly access
> parent data using standard APIs (like dev_get_drvdata) and to avoid writing
> bespoke helpers in the parent.
> 
> While this replaces dev_get_drvdata(ch->dev->parent) to avoid depending on
> subdriver-populated state, substituting it with a custom helper using pointer
> arithmetic (ch - ch->channel_number) tightly couples the implementation to
> the physical layout of the channels array.
> 
> Could the child drivers be refactored to retrieve the parent data themselves
> instead of introducing this bespoke accessor?
> 

If we were to refactor the children to pass the parent's struct device
(or struct rz_mtu3 via dev_get_drvdata()), we'd have to change the API
and update all call sites.

What we could do rather than the container_of() solution I implemented
here is add a backpointer to struct rz_mtu3 in struct rz_mtu3_channel
and populate it in the MFD driver.

But container_of() gets the same data with nothing new to be stored,
and struct rz_mtu3_channel::dev can be removed entirely afterwards.

I would appreciate your feedback on the desired approach here, Lee.

RE: [PATCH 2/3] mfd: rz-mtu3: reuse shared_reg_update_bit() in start_stop_ch()

[Cosmin-Gabriel Tanislav]

> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Monday, September 14, 2026 11:25 PM
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Commit message description is not capitalized.
> --
> 
> commit 67ee74ff3faf4f3c19662b6041b12e91d577cab9
> Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> 
> mfd: rz-mtu3: reuse shared_reg_update_bit() in start_stop_ch()
> 
> This commit simplifies the channel start and stop logic by reusing the
> existing rz_mtu3_shared_reg_update_bit() helper instead of open-coding the
> read-modify-write register sequence.
> 
> > mfd: rz-mtu3: reuse shared_reg_update_bit() in start_stop_ch()
> 
> [Severity: Low]
> This isn't a bug, but the MFD subsystem formatting rules require the
> description following the prefix to be capitalized. Should "reuse" be
> changed to "Reuse" to match the subsystem conventions?
> 

Ack.

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260914201456.2018403-1-cosmin-
> gabriel.tanislav.xa@renesas.com?part=2

RE: [PATCH 3/3] mfd: rz-mtu3: enable runtime PM

[Cosmin-Gabriel Tanislav]

> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Monday, September 14, 2026 11:28 PM
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Parent device runtime PM is enabled without setting it to active, causing child devices'
> probe to fail PM activation and permanently leak their clocks.
> - [Low] The commit message description is not capitalized.
> --
> 
> commit e5d000ec993dddaf773352525f11aae34e95a812
> Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> 
> mfd: rz-mtu3: enable runtime PM
> 
> This commit enables runtime PM on the rz-mtu3 parent device. This allows
> child devices to correctly propagate their runtime PM usage to the parent's
> PM domain, replacing the open-coded clock handling within the child devices
> themselves.
> 
> > mfd: rz-mtu3: enable runtime PM
> 
> [Severity: Low]
> This isn't a bug, but the commit message subject violates the MFD subsystem
> formatting rules which mandate capitalizing the description after the
> subsystem prefix. Should it be "mfd: rz-mtu3: Enable runtime PM"?
> 

Ack.

> > diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> > index 88aa1fedf1ef5..e24e67e7a66cb 100644
> > --- a/drivers/mfd/rz-mtu3.c
> > +++ b/drivers/mfd/rz-mtu3.c
> [ ... ]
> > @@ -330,6 +332,10 @@ static int rz_mtu3_probe(struct platform_device *pdev)
> >  		mutex_init(&ddata->channels[i].lock);
> >  	}
> >
> > +	ret = devm_pm_runtime_enable(dev);
> > +	if (ret)
> > +		return ret;
> > +
> 
> [Severity: High]
> Does enabling runtime PM on the parent without calling
> pm_runtime_set_active() leave the parent device in the RPM_SUSPENDED state?
> 
> During child driver probe, if a child calls pm_runtime_set_active(), it
> would receive -EBUSY because the parent is suspended and ignore_children is
> false. If the child driver ignores this error, its runtime PM status would
> remain RPM_SUSPENDED.
> 
> Since the child considers itself already suspended, will subsequent calls to
> pm_runtime_idle() or pm_runtime_put() in the child driver be ignored by the
> PM core? If the runtime_suspend callback is never invoked to balance the
> clk_prepare_enable() called during probe, could this result in a permanent
> clock leak?
> 

devm_mfd_add_devices() ->
mfd_add_devices() ->
mfd_add_device() ->
platform_device_add() ->
device_add() ->
bus_probe_device() ->
device_initial_probe() ->
__device_attach()


__device_attach():
...
if (dev->parent)
	pm_runtime_get_sync(dev->parent);
...
__device_attach_driver()
...
if (dev->parent)
	pm_runtime_put(dev->parent);


__device_attach_driver() ->
driver_probe_device() ->
__driver_probe_device()


__driver_probe_device():
...
if (dev->parent)
	pm_runtime_get_sync(dev->parent);
...
really_probe()
...
if (dev->parent)
	pm_runtime_put(dev->parent);


really_probe() ->
call_driver_probe() ->
drv->probe()

So any time a child's probe runs, runtime PM is surely active on the
parent.