Re: [PATCH 4/6] clocksource/drivers/davinci: Fix clock reference leak in of_davinci_timer_register

[Bartosz Golaszewski]

On Tue, 15 Sep 2026 07:19:48 +0200, Wentao Liang <vulab@xxxxxxxxxxx> said:
> of_davinci_timer_register() looks up the timer clock with of_clk_get(),
> which takes a reference on the clock, and passes it to
> davinci_timer_register(). That function enables the clock and keeps it
> enabled for the lifetime of the timer, but it never takes ownership of
> the reference and never calls clk_put(); only the error paths of
> of_davinci_timer_register() dropped the reference, so on success the
> reference obtained by of_clk_get() leaked.
>
> Drop the reference unconditionally after davinci_timer_register()
> returns: on failure the clock has already been disabled and unprepared
> by davinci_timer_register(), and on success the clock stays enabled
> while the consumer reference is released.
>
> Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Wentao Liang <vulab@xxxxxxxxxxx>
> ---
>  drivers/clocksource/timer-davinci.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> index 16ee0687ef31..54700738fcd4 100644
> --- a/drivers/clocksource/timer-davinci.c
> +++ b/drivers/clocksource/timer-davinci.c
> @@ -376,8 +376,11 @@ static int __init of_davinci_timer_register(struct device_node *np)
>  	}
>
>  	rv = davinci_timer_register(clk, &timer_cfg);
> -	if (rv)
> -		clk_put(clk);
> +	/*
> +	 * The clock is left enabled on success, but the consumer reference
> +	 * taken by of_clk_get() is not needed anymore in either case.
> +	 */
> +	clk_put(clk);
>
>  	return rv;
>  }

I'm not following. If we keep the clock enabled, we must keep the reference. We
just never disable it anywhere, so you correctly don't see the clk_put() in
success path.

Bart