Re: [PATCH v5 03/17] i3c: renesas: Follow the reset deassert order used in probe

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The resume sequence deasserts resets before enabling clocks, which contradicts the probe sequence and violates standard hardware initialization ordering.
--

--- Patch [3]: [PATCH v5 03/17] i3c: renesas: Follow the reset deassert order used in probe ---
commit 1f6a035090d6207c3bc14c55b1513d8d3cf0afcd
Author: Claudiu Beznea <[email protected]>

i3c: renesas: Follow the reset deassert order used in probe

This commit reorders the deassertion of the presetn and tresetn resets
during resume to match the order used in the probe path. This prevents
potential failures caused by ordering differences.

> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index cd9928649c7f7..ccf55afcdedc7 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c

[ ... ]

> @@ -1455,17 +1455,17 @@ static int renesas_i3c_resume_noirq(struct device *dev)
>  	struct renesas_i3c *i3c = dev_get_drvdata(dev);
>  	int i, ret;
>  
> -	ret = reset_control_deassert(i3c->presetn);
> +	ret = reset_control_deassert(i3c->tresetn);
>  	if (ret)
>  		return ret;
>  
> -	ret = reset_control_deassert(i3c->tresetn);
> +	ret = reset_control_deassert(i3c->presetn);
>  	if (ret)
> -		goto err_presetn;
> +		goto err_tresetn;
>  
>  	ret = clk_bulk_enable(i3c->num_clks, i3c->clks);
>  	if (ret)
> -		goto err_tresetn;
> +		goto err_presetn;

[Severity: High]
This is a pre-existing issue, but is it safe to deassert the synchronous 
resets before the clocks are enabled in renesas_i3c_resume_noirq()?

Deasserting synchronous resets while the peripheral's clock is disabled can
prevent the internal state machine from correctly exiting the reset state.

Subsequent register accesses to a peripheral that has failed to initialize 
or is unclocked can cause an asynchronous external abort (system hang) or 
leave the I3C bus non-functional.

Could the clock enablement step be moved before the reset deassertion to
fully mirror the correct initialization order?

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

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c