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

[Lee Jones]

On Thu, 07 May 2026, Cosmin-Gabriel Tanislav wrote:

> > 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?

If you want to be specific about what it's doing:

  rz_mtu3_remove_devices()
  
However, my advice would be to go with something more generic.

  rz_mtu3_remove()

[...] 

> > >  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?

Reverse Christmas-tree order doesn't make sense in quite a lot of cases.

I have no strong opinions, but _generally_ prefer to put the important
and physically large ones at the top (structs) and the little ones at
the bottom (ints and bools) - this also _mostly_ works out as RCT.

The really important ones here are the device driver 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

-- 
Lee Jones