Re: [PATCH v14 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] Exclusive acquisition of a shared bus reset blocks other devices on the bus from probing and may inadvertently reset the entire bus.
--

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

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

This commit updates the timer initialization to fetch and trigger all
reset lines listed in the device tree using the array variant of the
reset API. It intentionally holds exclusive references to these lines
to keep them deasserted while the timer runs.

> 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,14 @@ 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 the array variant to fetch and trigger all reset lines
> +	 * (e.g., timer and APB interface resets) together, without
> +	 * ordering requirements.
> +	 * The exclusive references are intentionally held to keep the
> +	 * resets deasserted while the timer is running.
>  	 */
> -	rstc = of_reset_control_get(np, NULL);
> +	rstc = of_reset_control_array_get_optional_exclusive(np);

[Severity: High]
If any of these reset lines are shared bus resets, does acquiring them
exclusively lock out other devices that share the same reset line?

The commit message mentions APB interface resets, which are typically shared
across multiple peripherals. If this driver holds a shared reset line
exclusively, subsequent reset_control_get() calls from other drivers sharing
the bus will fail with -EBUSY, preventing them from probing.

Furthermore, unconditionally asserting a shared bus reset here in
timer_get_base_and_rate() might inadvertently reset other peripherals on the
same bus that are already initialized. Does the timer driver need to use a
shared reset API to avoid disrupting other devices on the bus?

>  	if (!IS_ERR(rstc)) {
>  		reset_control_assert(rstc);
>  		reset_control_deassert(rstc);

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