On 2023-02-10 18:24, Stephen Boyd wrote:

Quoting Kevin Groeneveld (2022-12-10 12:38:35)

quoted

kdiv is a signed 16 bit value in the DEV_CTL1 register. Commit
53990cf9d5b4 ("clk: imx: pll14xx: consolidate rate calculation") changed
the kdiv variable from a short int to just int. When the value read from
the DIV_CTL1 register is assigned directly to an int the sign of the value
is lost resulting in incorrect results when the value is negative. Adding

Can the field be negative?

Yes it can be. That is how I found the issue as I had a negative value 
in imx_pll14xx_rate_table which used to work but broke with the above 
mentioned commit.

The i.MX8MM reference manual states:
 >Formula for Fraction PLLOUT:
 >* FOUT=((m+k/65536) x FIN) /(p x 2s)
 >* Where, 1 ≤ p ≤ 63, 64 ≤ m ≤ 1023, 0 ≤ s ≤ 6, -32768 ≤ k ≤ 32767
 >* p, m, and s are unsigned integers. k is a two's complement integer.

The comments in the imx_pll14xx_calc_settings function also mention the 
same range for kdiv. And this function also uses negative values. For 
example:

 >/* Then see if we can get the desired rate by only adjusting kdiv 
(glitch free) */
 >rate_min = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, KDIV_MIN, prate);
 >rate_max = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, KDIV_MAX, prate);

Where KDIV_MIN is defined as SHRT_MIN which is negative.


Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel