Re: [PATCH v2 3/3] i3c: dw: Add apb reset support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The assertion and deassertion ordering of `core_rst` and `apb_rst` violates correct bus-peripheral hardware sequencing and LIFO resource management symmetry.
--

commit 9be063129d3014c8590f046b253447e4090e7f37
Author: Jisheng Zhang <jszhang@kernel.org>

i3c: dw: Add apb reset support

This commit adds support for the optional APB reset in the DesignWare I3C
master driver. The reset is acquired during probe and asserted or
deasserted during runtime PM suspend and resume cycles.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1596,6 +1596,11 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>  	if (IS_ERR(master->core_rst))
>  		return PTR_ERR(master->core_rst);
>  
> +	master->apb_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
> +									       "apb_rst");
> +	if (IS_ERR(master->apb_rst))
> +		return PTR_ERR(master->apb_rst);
> +

Should apb_rst be requested before core_rst?

Since devm handles cleanup in reverse allocation order, requesting core_rst
first means apb_rst will be asserted before core_rst during driver unload
or probe failure. This would shut down the bus interface while the IP core
is still active, contradicting the safe shutdown sequence correctly
implemented in dw_i3c_master_runtime_suspend().

Additionally, since devm_reset_control_get_optional_exclusive_deasserted()
automatically deasserts the reset, this ordering brings the IP core out of
reset before the APB bus interface is active.

[ ... ]

> @@ -1796,6 +1802,7 @@ static int __maybe_unused dw_i3c_master_runtime_resume(struct device *dev)
>  	pinctrl_pm_select_default_state(dev);
>  	dw_i3c_master_enable_clks(master);
>  	reset_control_deassert(master->core_rst);
> +	reset_control_deassert(master->apb_rst);

Is this deassertion order correct?

Deasserting core_rst before apb_rst brings the IP core out of reset before
its APB bus interface is active. Could this cause the IP to run while
disconnected from the bus, leading to unpredictable states?

Would it be safer to deassert apb_rst before core_rst?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511031945.3228-1-jszhang@kernel.org?part=3