[PATCH 8/8] clk/qcom: fix rcg divider value

[Caleb Connolly]

On 24/10/2023 21:23, Caleb Connolly wrote:
> The RCG divider field takes a value of (2*h - 1) where h is the divisor.
> This allows fractional dividers to be supported by calculating them at
> compile time using a macro.
> 
> However, the clk_rcg_set_rate_mnd() function was also performing the
> calculation. Clean this all up and consistently use the F() macro to
> calculate these at compile time and properly support fractional divisors.
> 
> Additionally, improve clk_bcr_update() to timeout with a warning rather
> than hanging the board, and make the freq_tbl struct and helpers common
> so that they can be reused by future platforms.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>

[...]
>  
>  #define CFG_MODE_DUAL_EDGE (0x2 << 12) /* Counter mode */
>  
> -#define CFG_MASK 0x3FFF
> +// Disable the HW_CLK_CONTROL bit
> +#define CFG_MASK (0x3FFF | (1 << 20))

There seems to be a bug in this patch that causes the actual clock rate
to be wrong in some cases, most obvious with db845c UART. I had
initially thought it to be a board issue but upon further investigation
I think I'm wrong.

The UART clock frequency table in clock-sdm845.c is taken from Linux,
the 115200 baud rate corresponds to the lowest frequency (7372800Hz).
However, with the implementation changes here, the RCG configuration
actually results in a measured clock rate of 9085208Hz.

If the entry is changed as follows
-	F(7372800, CFG_CLK_SRC_GPLL0_EVEN, 1, 384, 15625)
+	F(7372800, CFG_CLK_SRC_GPLL0, 0.5, 192, 15625)

Then the resultant clock rate is 7372604Hz (within the tolerance)...

I will resolve this for v2.
>  
>  #define CFG_DIVIDER_MASK 0x1F
>  
> -/* root set rate for clocks with half integer and MND divider */
> +/*
> + * root set rate for clocks with half integer and MND divider
> + * div should be pre-calculated ((div * 2) - 1)
> + */
>  void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs,
>  			  int div, int m, int n, int source, u8 mnd_width)
>  {
> @@ -99,17 +125,15 @@ void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs,
>  	/* Program MND values */
>  	setbits_le32(base + regs->M, m_val & mask);
>  	setbits_le32(base + regs->N, n_val & mask);
> -	setbits_le32(base + regs->D, d_val & mask);
> +	setbits_le32(base + regs->D, (d_val & mask) == mask ? 0 : (d_val & mask));
>  
>  	/* setup src select and divider */
>  	cfg  = readl(base + regs->cfg_rcgr);
>  	cfg &= ~CFG_MASK;
>  	cfg |= source & CFG_CLK_SRC_MASK; /* Select clock source */
>  
> -	/* Set the divider; HW permits fraction dividers (+0.5), but
> -	   for simplicity, we will support integers only */
>  	if (div)
> -		cfg |= (2 * div - 1) & CFG_DIVIDER_MASK;
> +		cfg |= div & CFG_DIVIDER_MASK;
>  
>  	if (n_val)
>  		cfg |= CFG_MODE_DUAL_EDGE;

-- 
// Caleb (they/them)