Re: [PATCH 4/5] clk: qcom: clk-rcg2: calculate timeout based on clock frequency

[Konrad Dybcio]

On 4/6/26 5:54 PM, Xilin Wu wrote:
> RCGs with extremely low rates (tens of Hz to low kHz) take much longer
> to update than the fixed 500 us timeout allows. A 1 kHz clock needs at
> least 3 ms (3 cycles) for the configuration handshake.
> 
> Instead of increasing the timeout to a huge fixed value for all clocks,
> dynamically compute the required timeout based on both the old and new
> clock rates, accounting for 3 cycles at each rate.
> 
> Based on a downstream patch by Mike Tipton:
> https://git.codelinaro.org/clo/la/kernel/qcom/-/commit/aa899c2d1fa31e247f04810f125ac9c60927c901
> 
> Fixes: bcd61c0f535a ("clk: qcom: Add support for root clock generators (RCGs)")
> Signed-off-by: Mike Tipton <quic_mdtipton@xxxxxxxxxxx>

Having Mike's s-o-b here is odd, given you've decided to go forward
without his "From:"

[...]
> +static int get_update_timeout(const struct clk_rcg2 *rcg)

Let's tack on a '_us'

> +{
> +	int timeout = 0;
> +	unsigned long current_freq;
> +
> +	/*
> +	 * The time it takes an RCG to update is roughly 3 clock cycles of the
> +	 * old and new clock rates.
> +	 */
> +	current_freq = clk_hw_get_rate(&rcg->clkr.hw);
> +	if (current_freq)
> +		timeout += 3 * (USEC_PER_SEC / current_freq);
> +	if (rcg->configured_freq)
> +		timeout += 3 * (USEC_PER_SEC / rcg->configured_freq);

I suppose both are nonzero if we end up in this path but a check for zerodiv
is always welcome

> +
> +	return max(timeout, 500);
> +}
> +
>  static int update_config(struct clk_rcg2 *rcg)
>  {
> -	int count, ret;
> +	int timeout, count, ret;
>  	u32 cmd;
>  	struct clk_hw *hw = &rcg->clkr.hw;
>  	const char *name = clk_hw_get_name(hw);
> @@ -123,8 +141,10 @@ static int update_config(struct clk_rcg2 *rcg)
>  	if (ret)
>  		return ret;
>  
> +	timeout = get_update_timeout(rcg);

You can just assign count = get_update_timeout() below since you're not
reusing this value

Konrad