On 08/05/2013 11:00 AM, Stephen Warren wrote:

On 08/05/2013 02:02 AM, Joseph Lo wrote:

quoted

On Sat, 2013-08-03 at 04:32 +0800, Stephen Warren wrote:

quoted

On 08/02/2013 02:09 AM, Joseph Lo wrote:

quoted

On Tue, 2013-07-30 at 06:51 +0800, Stephen Warren wrote:

quoted

On 07/26/2013 03:15 AM, Joseph Lo wrote:

quoted

When the system suspends to LP1, the clock of the CPU would be switched to
CLK_M (12MHz Oscillator) during suspend/resume flow. The clock driver
needs to restore the clock of CPU after LP1 resume.

It's unclear to me how the code change implements "restore the clock of
the CPU". A register name of CCLKG_BURST_POLICY doesn't sound like it's
anything to do with enabled/disabling the CPU clock, nor configuring its
rate. What exactly does this register do, and hence what does this new
code actually restore?

When system suspend to LP1, most of the PLLs was clock gated. Because we
didn't cut off the core power, the settings of PLL still keep there. But
we switch the clock source of CPU to CLK_M before shut off PLLs by
CCLKG_BURSY_POLICY register. So we need to resume it back to original
clock source by CCLKG_BURST_POLICY register. Or it would be keep in low
rate (CLK_M) after resume.

OK, I guess the register name was badly chosen by HW. I'd like to see
part of your description above in the patch description. How about
replacing the patch description with:

----------
When the system suspends to LP1, the CPU clock source is switched to
CLK_M (12MHz Oscillator) during suspend/resume flow[1]. The CPU clock
source is controlled by the CCLKG_BURST_POLICY register, and hence this
register must be restored during LP1 resume.
----------

[1] Question: where does this happen? This patch doesn't make that
change. I wonder why the suspend path can't save this register, rather
than implementing a separate suspend syscore op in the clock driver.
Does the HW auto-switch the value in the register itself?

If we switch CPU to CLK_M in clock driver, the system will become slowly
during the middle of suspending flow. We do this at the very end of the
LP1 suspending flow before the CPU disable all the PLL clocks.

I think you answered "why is the switch to CLK_M performed very late",
whereas I asked "where is the code that performs the switch to CLK_M".

To expand on this a bit more, I can't find any reference to register
CCLKG_BURST_POLICY in arch/arm/mach-tegra/ or drivers/clk/tegra/ except
for the definition of clock cclk_g, nor any reference to that clock in
those two directories. And that CCLKG_BURST_POLICY register is what this
patch saves/restores.

I do see function tegra30_switch_cpu_to_clk32k in patch 5/8 appears to
do something related to switching to clk_m and touches some other
burst-related registers, but not CCLKG_BURST_POLICY...

So, if this syscore_op is attempting to undo some change that happens to
CCLKG_BURST_POLICY during suspend, I still can't see what change is
happening to that register during suspemd, nor which piece of code
causes it.

If the issue is that the value in this register is simply lost during
LP1 because the power is turned off, then I wonder what sets up this
register during the original boot path?