Re: [PATCH 6.12.y-cip 04/17] clk: renesas: cpg-mssr: Add module reset support for RZ/T2H

[Pavel Machek <pavel@nabladev.com>]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

Hi!

There's a cleanup possible, see below.

> commit 3b37979dcbef0dc3fc1aaba75b21ff9a21799055 upstream.
> 
> Add support for module reset handling on the RZ/T2H SoC. Unlike earlier
> CPG/MSSR variants, RZ/T2H uses a unified set of Module Reset Control
> Registers (MRCR) where both reset and deassert actions are done via
> read-modify-write (RMW) to the same register.

> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> index 9ea544d00634b..4d2d945b3f5e2 100644
> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> +	/*
> +	 * For secure processing after release from a module reset, one must
> +	 * perform multiple dummy reads of the same register.
> +	 */
> +	for (i = 0; !set && i < RZT2H_RESET_REG_READ_COUNT; i++)
> +		readl(reg_addr);
> +
> +	/* Verify the operation */
> +	val = readl(reg_addr);
> +	if (set == !(bitmask & val)) {
> +		dev_err(priv->dev, "Reset register %u%02u operation failed\n", reg, bit);
> +		spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
> +		return -EIO;
> +	}
> +
> +	spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);

"(set == !(bitmask & val))" is only using local variables, so does not
need spinlock protection AFAICT.

And dev_err really should not be run under spinlock.

So I believe spin_unlock...() can be simply moved before the if() for
tiny bit faster and cleaner code.

Thanks and best regards,
								Pavel
-- 
In cooperation with Nabla.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]