Re: [PATCH v2 02/10] mfd: rz-mtu3: use device-managed reset deassert

Re: [PATCH v2 02/10] mfd: rz-mtu3: use device-managed reset deassert

[Lee Jones]

On Fri, 10 Apr 2026, Cosmin Tanislav wrote:

> Replace devm_reset_control_get_exclusive() and the manual
> reset_control_deassert()/reset_control_assert() with handling by
> devm_reset_control_get_exclusive_deasserted().
> 
> While at it, remove struct rz_mtu3_priv::rstc and use a local variable
> for it as it is not needed inside rz_mtu3_reset_assert().
> 
> Rename rz_mtu3_reset_assert() to rz_mtu3_mfd_remove() to accurately
> describe its usage since it no longer calls reset_control_assert().
> 
> Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> ---
> 
> V2:
>  * no changes
> 
>  drivers/mfd/rz-mtu3.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> index 9cdfef610398f..6b9c6831dffa9 100644
> --- a/drivers/mfd/rz-mtu3.c
> +++ b/drivers/mfd/rz-mtu3.c
> @@ -21,7 +21,6 @@
>  
>  struct rz_mtu3_priv {
>  	void __iomem *mmio;
> -	struct reset_control *rstc;
>  	spinlock_t lock;
>  };
>  
> @@ -301,13 +300,9 @@ void rz_mtu3_disable(struct rz_mtu3_channel *ch)
>  }
>  EXPORT_SYMBOL_GPL(rz_mtu3_disable);
>  
> -static void rz_mtu3_reset_assert(void *data)
> +static void rz_mtu3_mfd_remove(void *data)

Remove any mention of "mfd".

>  {
> -	struct rz_mtu3 *mtu = dev_get_drvdata(data);
> -	struct rz_mtu3_priv *priv = mtu->priv_data;
> -
>  	mfd_remove_devices(data);

Why not use devm_mfd_add_devices() instead?

> -	reset_control_assert(priv->rstc);
>  }
>  
>  static const struct mfd_cell rz_mtu3_devs[] = {
> @@ -321,6 +316,7 @@ static const struct mfd_cell rz_mtu3_devs[] = {
>  
>  static int rz_mtu3_probe(struct platform_device *pdev)
>  {
> +	struct reset_control *rstc;

This shouldn't go above the main device data structs.

>  	struct rz_mtu3_priv *priv;
>  	struct rz_mtu3 *ddata;
>  	unsigned int i;
> @@ -340,15 +336,14 @@ static int rz_mtu3_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->mmio))
>  		return PTR_ERR(priv->mmio);
>  
> -	priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> -	if (IS_ERR(priv->rstc))
> -		return PTR_ERR(priv->rstc);
> +	rstc = devm_reset_control_get_exclusive_deasserted(&pdev->dev, NULL);
> +	if (IS_ERR(rstc))
> +		return PTR_ERR(rstc);
>  
>  	ddata->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(ddata->clk))
>  		return PTR_ERR(ddata->clk);
>  
> -	reset_control_deassert(priv->rstc);
>  	spin_lock_init(&priv->lock);
>  	platform_set_drvdata(pdev, ddata);
>  
> @@ -361,14 +356,10 @@ static int rz_mtu3_probe(struct platform_device *pdev)
>  	ret = mfd_add_devices(&pdev->dev, 0, rz_mtu3_devs,
>  			      ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
>  	if (ret < 0)
> -		goto err_assert;
> +		return ret;
>  
> -	return devm_add_action_or_reset(&pdev->dev, rz_mtu3_reset_assert,
> +	return devm_add_action_or_reset(&pdev->dev, rz_mtu3_mfd_remove,
>  					&pdev->dev);
> -
> -err_assert:
> -	reset_control_assert(priv->rstc);
> -	return ret;
>  }
>  
>  static const struct of_device_id rz_mtu3_of_match[] = {
> -- 
> 2.53.0

-- 
Lee Jones

Re: [PATCH v2 03/10] mfd: rz-mtu3: use device-managed mfd_add_devices()

[Lee Jones]

On Fri, 10 Apr 2026, Cosmin Tanislav wrote:

> Replace mfd_add_devices() and the custom cleanup action with
> devm_mfd_add_devices().
> 
> Remove the ret variable as it is now unused.

Do this first, then the changes in the first patch make more sense.

> Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> ---
> 
> V2:
>  * no changes
> 
>  drivers/mfd/rz-mtu3.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> index 6b9c6831dffa9..3be6f6c900b82 100644
> --- a/drivers/mfd/rz-mtu3.c
> +++ b/drivers/mfd/rz-mtu3.c
> @@ -300,11 +300,6 @@ void rz_mtu3_disable(struct rz_mtu3_channel *ch)
>  }
>  EXPORT_SYMBOL_GPL(rz_mtu3_disable);
>  
> -static void rz_mtu3_mfd_remove(void *data)
> -{
> -	mfd_remove_devices(data);
> -}
> -
>  static const struct mfd_cell rz_mtu3_devs[] = {
>  	{
>  		.name = "rz-mtu3-counter",
> @@ -320,7 +315,6 @@ static int rz_mtu3_probe(struct platform_device *pdev)
>  	struct rz_mtu3_priv *priv;
>  	struct rz_mtu3 *ddata;
>  	unsigned int i;
> -	int ret;
>  
>  	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
>  	if (!ddata)
> @@ -353,13 +347,8 @@ static int rz_mtu3_probe(struct platform_device *pdev)
>  		mutex_init(&ddata->channels[i].lock);
>  	}
>  
> -	ret = mfd_add_devices(&pdev->dev, 0, rz_mtu3_devs,
> -			      ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
> -	if (ret < 0)
> -		return ret;
> -
> -	return devm_add_action_or_reset(&pdev->dev, rz_mtu3_mfd_remove,
> -					&pdev->dev);
> +	return devm_mfd_add_devices(&pdev->dev, 0, rz_mtu3_devs,
> +				    ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
>  }
>  
>  static const struct of_device_id rz_mtu3_of_match[] = {
> -- 
> 2.53.0

-- 
Lee Jones

Re: [PATCH v2 04/10] mfd: rz-mtu3: store &pdev->dev in local variable

[Lee Jones]

On Fri, 10 Apr 2026, Cosmin Tanislav wrote:

> &pdev->dev is accessed multiple times during probe. Store it in a local
> variable and use that to simplify the code.
> 
> Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> ---
> 
> V2:
>  * no changes
> 
>  drivers/mfd/rz-mtu3.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

This is fine.

> diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> index 3be6f6c900b82..37d12030e069c 100644
> --- a/drivers/mfd/rz-mtu3.c
> +++ b/drivers/mfd/rz-mtu3.c
> @@ -311,16 +311,17 @@ static const struct mfd_cell rz_mtu3_devs[] = {
>  
>  static int rz_mtu3_probe(struct platform_device *pdev)
>  {
> +	struct device *dev = &pdev->dev;
>  	struct reset_control *rstc;
>  	struct rz_mtu3_priv *priv;
>  	struct rz_mtu3 *ddata;
>  	unsigned int i;
>  
> -	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> +	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
>  	if (!ddata)
>  		return -ENOMEM;
>  
> -	ddata->priv_data = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	ddata->priv_data = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  	if (!ddata->priv_data)
>  		return -ENOMEM;
>  
> @@ -330,11 +331,11 @@ static int rz_mtu3_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->mmio))
>  		return PTR_ERR(priv->mmio);
>  
> -	rstc = devm_reset_control_get_exclusive_deasserted(&pdev->dev, NULL);
> +	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
>  	if (IS_ERR(rstc))
>  		return PTR_ERR(rstc);
>  
> -	ddata->clk = devm_clk_get(&pdev->dev, NULL);
> +	ddata->clk = devm_clk_get(dev, NULL);
>  	if (IS_ERR(ddata->clk))
>  		return PTR_ERR(ddata->clk);
>  
> @@ -347,7 +348,7 @@ static int rz_mtu3_probe(struct platform_device *pdev)
>  		mutex_init(&ddata->channels[i].lock);
>  	}
>  
> -	return devm_mfd_add_devices(&pdev->dev, 0, rz_mtu3_devs,
> +	return devm_mfd_add_devices(dev, 0, rz_mtu3_devs,
>  				    ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
>  }
>  
> -- 
> 2.53.0

-- 
Lee Jones

Re: [PATCH v2 05/10] mfd: rz-mtu3: make reset optional

[Lee Jones]

On Fri, 10 Apr 2026, Cosmin Tanislav wrote:

> The Renesas RZ/T2H (R9A09G077) and RZ/N2H (R9A09G087) SoCs do not have a
> reset line for the MTU3 block.
> 
> Prepare for them by making it optional.
> 
> Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> ---
> 
> V2:
>  * no changes
> 
>  drivers/mfd/rz-mtu3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Also fine.

> diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> index 37d12030e069c..689dbb181d305 100644
> --- a/drivers/mfd/rz-mtu3.c
> +++ b/drivers/mfd/rz-mtu3.c
> @@ -331,7 +331,7 @@ static int rz_mtu3_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->mmio))
>  		return PTR_ERR(priv->mmio);
>  
> -	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
> +	rstc = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
>  	if (IS_ERR(rstc))
>  		return PTR_ERR(rstc);
>  
> -- 
> 2.53.0

-- 
Lee Jones

RE: [PATCH v2 02/10] mfd: rz-mtu3: use device-managed reset deassert

[Cosmin-Gabriel Tanislav]

> From: Lee Jones <lee@kernel.org>
> Sent: Thursday, May 7, 2026 3:39 PM
> 
> On Fri, 10 Apr 2026, Cosmin Tanislav wrote:
> 
> > Replace devm_reset_control_get_exclusive() and the manual
> > reset_control_deassert()/reset_control_assert() with handling by
> > devm_reset_control_get_exclusive_deasserted().
> >
> > While at it, remove struct rz_mtu3_priv::rstc and use a local variable
> > for it as it is not needed inside rz_mtu3_reset_assert().
> >
> > Rename rz_mtu3_reset_assert() to rz_mtu3_mfd_remove() to accurately
> > describe its usage since it no longer calls reset_control_assert().
> >
> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> > ---
> >
> > V2:
> >  * no changes
> >
> >  drivers/mfd/rz-mtu3.c | 23 +++++++----------------
> >  1 file changed, 7 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> > index 9cdfef610398f..6b9c6831dffa9 100644
> > --- a/drivers/mfd/rz-mtu3.c
> > +++ b/drivers/mfd/rz-mtu3.c
> > @@ -21,7 +21,6 @@
> >
> >  struct rz_mtu3_priv {
> >  	void __iomem *mmio;
> > -	struct reset_control *rstc;
> >  	spinlock_t lock;
> >  };
> >
> > @@ -301,13 +300,9 @@ void rz_mtu3_disable(struct rz_mtu3_channel *ch)
> >  }
> >  EXPORT_SYMBOL_GPL(rz_mtu3_disable);
> >
> > -static void rz_mtu3_reset_assert(void *data)
> > +static void rz_mtu3_mfd_remove(void *data)
> 
> Remove any mention of "mfd".
> 

_mfd here is to describe that it removes MFD devices, not that it is
part of a MFD driver.

What name would you prefer to use in this case?

> >  {
> > -	struct rz_mtu3 *mtu = dev_get_drvdata(data);
> > -	struct rz_mtu3_priv *priv = mtu->priv_data;
> > -
> >  	mfd_remove_devices(data);
> 
> Why not use devm_mfd_add_devices() instead?
> 

Addressed on the following patch.

> > -	reset_control_assert(priv->rstc);
> >  }
> >
> >  static const struct mfd_cell rz_mtu3_devs[] = {
> > @@ -321,6 +316,7 @@ static const struct mfd_cell rz_mtu3_devs[] = {
> >
> >  static int rz_mtu3_probe(struct platform_device *pdev)
> >  {
> > +	struct reset_control *rstc;
> 
> This shouldn't go above the main device data structs.
> 

I was following reverse fir tree ordering, as required in some other
subsystems.

Is there an MFD-specific preferred style that I should follow?

Would you prefer having it right before unsigned int i?

> >  	struct rz_mtu3_priv *priv;
> >  	struct rz_mtu3 *ddata;
> >  	unsigned int i;
> > @@ -340,15 +336,14 @@ static int rz_mtu3_probe(struct platform_device *pdev)
> >  	if (IS_ERR(priv->mmio))
> >  		return PTR_ERR(priv->mmio);
> >
> > -	priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > -	if (IS_ERR(priv->rstc))
> > -		return PTR_ERR(priv->rstc);
> > +	rstc = devm_reset_control_get_exclusive_deasserted(&pdev->dev, NULL);
> > +	if (IS_ERR(rstc))
> > +		return PTR_ERR(rstc);
> >
> >  	ddata->clk = devm_clk_get(&pdev->dev, NULL);
> >  	if (IS_ERR(ddata->clk))
> >  		return PTR_ERR(ddata->clk);
> >
> > -	reset_control_deassert(priv->rstc);
> >  	spin_lock_init(&priv->lock);
> >  	platform_set_drvdata(pdev, ddata);
> >
> > @@ -361,14 +356,10 @@ static int rz_mtu3_probe(struct platform_device *pdev)
> >  	ret = mfd_add_devices(&pdev->dev, 0, rz_mtu3_devs,
> >  			      ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
> >  	if (ret < 0)
> > -		goto err_assert;
> > +		return ret;
> >
> > -	return devm_add_action_or_reset(&pdev->dev, rz_mtu3_reset_assert,
> > +	return devm_add_action_or_reset(&pdev->dev, rz_mtu3_mfd_remove,
> >  					&pdev->dev);
> > -
> > -err_assert:
> > -	reset_control_assert(priv->rstc);
> > -	return ret;
> >  }
> >
> >  static const struct of_device_id rz_mtu3_of_match[] = {
> > --
> > 2.53.0
> 
> --
> Lee Jones

RE: [PATCH v2 03/10] mfd: rz-mtu3: use device-managed mfd_add_devices()

[Cosmin-Gabriel Tanislav]

> From: Lee Jones <lee@kernel.org>
> Sent: Thursday, May 7, 2026 3:46 PM
> 
> On Fri, 10 Apr 2026, Cosmin Tanislav wrote:
> 
> > Replace mfd_add_devices() and the custom cleanup action with
> > devm_mfd_add_devices().
> >
> > Remove the ret variable as it is now unused.
> 
> Do this first, then the changes in the first patch make more sense.
> 

Doing the devm_mfd_add_devices() conversion first would not be a simple
replacement, as devm_add_action_or_reset() would also need to be moved
before it to keep the intermediate patch correct.

Otherwise, on release, the reset will be asserted before the MFD devices
are removed. Doing the reset conversion first fixes that without extra
logic changes.

Alternatively, we can squash the two conversions to avoid the
intermediate states entirely, and remove struct rz_mtu3_priv::rstc in a
new separate patch. What do you think about this variant?

> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> > ---
> >
> > V2:
> >  * no changes
> >
> >  drivers/mfd/rz-mtu3.c | 15 ++-------------
> >  1 file changed, 2 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c
> > index 6b9c6831dffa9..3be6f6c900b82 100644
> > --- a/drivers/mfd/rz-mtu3.c
> > +++ b/drivers/mfd/rz-mtu3.c
> > @@ -300,11 +300,6 @@ void rz_mtu3_disable(struct rz_mtu3_channel *ch)
> >  }
> >  EXPORT_SYMBOL_GPL(rz_mtu3_disable);
> >
> > -static void rz_mtu3_mfd_remove(void *data)
> > -{
> > -	mfd_remove_devices(data);
> > -}
> > -
> >  static const struct mfd_cell rz_mtu3_devs[] = {
> >  	{
> >  		.name = "rz-mtu3-counter",
> > @@ -320,7 +315,6 @@ static int rz_mtu3_probe(struct platform_device *pdev)
> >  	struct rz_mtu3_priv *priv;
> >  	struct rz_mtu3 *ddata;
> >  	unsigned int i;
> > -	int ret;
> >
> >  	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> >  	if (!ddata)
> > @@ -353,13 +347,8 @@ static int rz_mtu3_probe(struct platform_device *pdev)
> >  		mutex_init(&ddata->channels[i].lock);
> >  	}
> >
> > -	ret = mfd_add_devices(&pdev->dev, 0, rz_mtu3_devs,
> > -			      ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
> > -	if (ret < 0)
> > -		return ret;
> > -
> > -	return devm_add_action_or_reset(&pdev->dev, rz_mtu3_mfd_remove,
> > -					&pdev->dev);
> > +	return devm_mfd_add_devices(&pdev->dev, 0, rz_mtu3_devs,
> > +				    ARRAY_SIZE(rz_mtu3_devs), NULL, 0, NULL);
> >  }
> >
> >  static const struct of_device_id rz_mtu3_of_match[] = {
> > --
> > 2.53.0
> 
> --
> Lee Jones