Re: [PATCH v3 4/4] 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] Incorrect and asymmetric hardware reset sequencing for the APB bus and core.
--

commit 1f3afa3e9cfb230aef26a85d4d3a519a377255b1
Author: Jisheng Zhang <jszhang@kernel.org>

i3c: dw: Add apb reset support

This commit adds support for an APB interface reset control to the DesignWare
I3C master driver. The reset is obtained during probe and managed across
runtime suspend and resume cycles.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 6286dc5d09568..75608fbc3cbd1 100644
> --- 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");

Does allocating apb_rst after core_rst cause a devm cleanup ordering issue?

Because devm executes cleanups in reverse allocation order, apb_rst will be
asserted before core_rst during driver unbind or probe failure. This stops
the bus while the core is still running, which contradicts the assertion
order in dw_i3c_master_runtime_suspend().

Also, because devm_reset_control_get_optional_exclusive_deasserted()
immediately deasserts the reset, core_rst is deasserted before apb_rst
during probe. This brings the core out of reset while its bus interface is
still held in reset.

[ ... ]

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

Does this sequence fail to correctly reverse the suspend order?

In dw_i3c_master_runtime_suspend(), core_rst is asserted followed by
apb_rst, which correctly stops the core before the bus.

However, here core_rst is deasserted before apb_rst. This means the core
is brought out of reset while its APB bus interface is still held in reset,
which could trigger bus faults.

Should apb_rst be deasserted before core_rst to ensure the bus is available
before the core IP starts operating?

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