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

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] The commit message description is not capitalized, violating MFD subsystem guidelines.
- [Low] Open-coding of `devm_mfd_add_devices()`.
- [Low] The newly renamed callback function 'rz_mtu3_mfd_remove' includes the string 'mfd' and the driver's name.
--

--- Patch [2]: [PATCH 02/11] mfd: rz-mtu3: use device-managed reset deassert ---
commit f1182ddc967373a6fde2ebff19ae553068ff4988
Author: Cosmin Tanislav <[email protected]>

mfd: rz-mtu3: use device-managed reset deassert

This commit replaces manual reset control handling with
devm_reset_control_get_exclusive_deasserted(). It also removes the manual
reset assert in the remove callback and renames it.

> mfd: rz-mtu3: use device-managed reset deassert

[Severity: Low]
This isn't a bug, but the MFD subsystem guidelines request that the
description after the subsystem prefix be capitalized. Could this be
updated to "mfd: rz-mtu3: Use device-managed reset deassert"?

> 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

[ ... ]

> @@ -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)

[Severity: Low]
This isn't a bug, but the MFD subsystem guidelines state we should avoid
including the string "mfd" or the driver's own filename in names. Does it
make sense to rename this callback to avoid these strings?

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

[ ... ]

> @@ -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);

[Severity: Low]
Since the custom reset assertion has been removed and this callback now only
calls mfd_remove_devices(), this essentially open-codes devm_mfd_add_devices().

Could this code be simplified by calling devm_mfd_add_devices() directly and
deleting the custom removal callback altogether?

> -
> -err_assert:
> -	reset_control_assert(priv->rstc);
> -	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2