Re: [PATCH v13 5/5] clocksource: dw_apb_timer: Use reset array API to handle all resets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch dynamically allocates an exclusive reset control array using the unmanaged `of_` API but fails to free it, permanently locking shared APB resets and deadlocking other devices.
--

--- Patch [5]: [PATCH v13 5/5] clocksource: dw_apb_timer: Use reset array API to handle all resets ---
commit 015418d722b66a454000fd7a9c2606bea3f7e18b
Author: Xuyang Dong <dongxuyang@eswincomputing.com>

clocksource: dw_apb_timer: Use reset array API to handle all resets

This patch updates the timer initialization to fetch and pulse all reset lines
specified in the device tree by switching to the reset array API.

> Replace it with devm_reset_control_array_get_optional_exclusive() to
> fetch and control all reset lines listed in the device tree, ensuring
> every line is properly asserted and deasserted rather than only the first.

[Severity: Low]
This isn't a bug, but the commit message mentions using the managed
devm_reset_control_array_get_optional_exclusive function, while the code
below actually uses the unmanaged of_reset_control_array_get_optional_exclusive
function.

> diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
> --- a/drivers/clocksource/dw_apb_timer_of.c
> +++ b/drivers/clocksource/dw_apb_timer_of.c
[ ... ]
> @@ -29,9 +29,11 @@ static int __init timer_get_base_and_rate(struct device_node *np,
>  	/*
>  	 * Reset the timer if the reset control is available, wiping
> -	 * out the state the firmware may have left it
> +	 * out the state the firmware may have left it.
> +	 * Use array variant to handle multiple resets (e.g., timer
> +	 * and APB interface resets) if specified in the device tree.
>  	 */
> -	rstc = of_reset_control_get(np, NULL);
> +	rstc = of_reset_control_array_get_optional_exclusive(np);
>  	if (!IS_ERR(rstc)) {
>  		reset_control_assert(rstc);
>  		reset_control_deassert(rstc);
>  	}

[Severity: High]
Does this code leak the exclusive reset control array?

Since of_reset_control_array_get_optional_exclusive is used instead of the
managed devm variant, the returned rstc needs to be manually freed.

Because this API acquires exclusive references to the resets (which the
comment notes might include shared APB interface resets), exiting
timer_get_base_and_rate without calling reset_control_put(rstc) leaves
these resets permanently locked.

Could this prevent other devices on the bus from acquiring their shared
resets during probe by returning -EBUSY?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724084153.1473-1-dongxuyang@eswincomputing.com?part=5