Re: [PATCH] clk: tegra: Fix divider settings for Tegra210 pll_a

[Thierry Reding]

On Fri, Feb 05, 2016 at 11:03:04AM -0500, Rhyland Klein wrote:
> On 2/5/2016 9:01 AM, Jon Hunter wrote:
> > When setting the pll_a frequency, to what should be 368639844 Hz, the
> > actual output frequency of the pll is 737279687 Hz (as reported by the
> > debugfs clk_summary entry), which is double the expect frequency. The
> > calculations for the pll frequency appear to be correct and the problem
> > is caused by incorrect divider settings for the pll_a in the look-up
> > table of multipliers and dividers. The P dividers for all entries in the
> > table are all one less than they should be. Fix this by increasing the
> > value of the P dividers by 1 to get the expected rates.
> > 
> > The rates in the table have been verified using the following equations:
> > 
> > 1. Integer NDIV (SDM_DIN = 0)
> >    fout = (fref * n) / (m * p)
> > 
> > 2. Fractional NDIV
> >    fout = (fref * (n + 0.5 + (SDM_DIN/8192))) / (m * p)
> > 
> > Where SDM_DIN is a signed 16-bit value between 0xf000 and 0x1000.
> > 
> > Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> > ---
> > 
> > Thierry, Rhyland, please note that after this change, the pll_a settings
> > match those in the nvidia downstream 3.18 kernel for Tegra210.
> > 
> >
> 
> This was an issue we had before/around merging T210 driver. The tegra
> clock code was inconsistent in how it treated the pdiv. In some places
> it stored the direct HW value in the frequency table entry struct (like
> in the predefined frequencies like what you changed). Some places stored
> the translated (=_hw_to_p_div()). This is most commonly a difference in
> a factor of 2.
> 
> I found when initially doing the T210 that most clocks I requested rates
> for came out at either 1/2 or 2x what I wanted depending on the path.
> 
> I think when Thierry merged my series, he added some code to address
> this issue, as he saw it too I think. Depending on what his change was
> (to consistently store either pdiv or hw_val) then we may need to adjust
> ALL the predefined rates in the clk-tegra210 code.
> 
> Thierry, do you remember off-hand what you changed for that?

Yes. The problem was that we were inconsistently treating the P-divider
values in frequency tables. In some cases they were assumed to be the
real divider values, whereas in other cases they were treated as being
the direct register values.

This was changed to be consistent in commit 86c679a52294 ("clk: tegra:
pll: Fix _pll_ramp_calc_pll logic and _calc_dynamic_ramp_rate") but I
must have forgotten to update the Tegra210 frequency tables with a
corresponding change.

I've sent out a patch that should fix those all up since the same error
exists for many others.

Thierry

Attachment: signature.asc
Description: PGP signature