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.conno...@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)