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

[Cosmin-Gabriel Tanislav]

> From: Lee Jones <lee@xxxxxxxxxx>
> 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@xxxxxxxxxxx>
> > ---
> >
> > 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