Am Dienstag, 18. November 2014, 09:59:56 schrieb Doug Anderson: > Hi, > > On Mon, Nov 17, 2014 at 1:14 PM, Mike Turquette <mturquette at linaro.org> wrote: > > Quoting Heiko St?bner (2014-11-14 10:06:47) > > > >> Hi Mike, > >> > >> Am Donnerstag, 13. November 2014, 17:41:02 schrieb Mike Turquette: > >> > Quoting Doug Anderson (2014-11-13 15:27:32) > >> > >> [...] > >> > >> > All of the above is to say that perhaps the solution to this problem > >> > belongs in the driver. In the end we're talking about details for > >> > correctly programming hardware, which sounds an awful lot like what > >> > drivers are supposed to do. > >> > > >> > Let me know if the ->init() callback holds any promise for you. If not > >> > we can figure something out. > >> > >> From my theoretical musings, ->init() sounds like a nice idea - but of > >> course it comes with a "but". > >> > >> I guess the general idea would be to have the pll clk-type simply reset > >> to the same rate but forcing it to use the parameters from its parameter > >> table - when the rate params differ [0]. > >> > >> The only problem would be the apll supplying the cpu cores. After all > >> clocks are registered, our armclk makes sure that the core clock gets > >> reparented before changing the underlying apll [dpll is safe, as it is > >> read-only currently]. > DPLL probably won't be read only forever... > > >> At the moment the order would be > >> clk_register(apll) > >> apll->init() > >> clk_register(armclk); > > > > Sorry, but I don't understand the problem. The at registration-time, > > apll is re-programmed to a correct value for its current rate. Then > > armclk is registered which might change apll's rate. Any change to the > > apll which is issued from armclk should insure that apll is programmed > > correctly. > > I think Heiko is worried that until the "armclk" is registered that > nobody is there to reparent the ARM core to GPLL while APLL changes > (that's armclk's job). This is potentially unsafe. > > NOTE: it actually might not be unsafe, just slow. I think we'll > actually swap the PLL into "slow" mode before changing it (24MHz) so > we won't die we'll just run at 24MHz at boot time while we wait for > APLL to re-lock. that is correct, we switch the pll to slow-mode (which simply passes through the xin24m) before touching its params, which is also the behaviour described in the manual for this. > One option would be to just add yet another per-pll parameter. We'll > only cleanup CPLL, GPLL, and NPLL. If APLL (ARM clock) and DPLL > (memory clock) are set differently by firmware then we're just SOL. > Of course if firmware boots us on GPLL then I guess we're back to > square one (would firmware really be that malicious?) I was talking with Kever about the same thing today. My best idea would be to give our plls a flag property - like all the other clocks have (divider_flags, mux_flags, etc) - because who knows if later pll designs will need other smaller adjustments. Then adding a ROCKCHIP_PLL_FLAG_SYNC_RATE triggering the init function to check the rate params. Heiko
