lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list] 
Date:   Wed, 17 Jul 2019 18:15:22 -0700
From:   Sowjanya Komatineni <skomatineni@...dia.com>
To:     Dmitry Osipenko <digetx@...il.com>, <sboyd@...nel.org>,
        Michael Turquette <mturquette@...libre.com>
CC:     Peter De Schrijver <pdeschrijver@...dia.com>,
        Joseph Lo <josephl@...dia.com>, <thierry.reding@...il.com>,
        <jonathanh@...dia.com>, <tglx@...utronix.de>,
        <jason@...edaemon.net>, <marc.zyngier@....com>,
        <linus.walleij@...aro.org>, <stefan@...er.ch>,
        <mark.rutland@....com>, <pgaikwad@...dia.com>,
        <linux-clk@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
        <jckuo@...dia.com>, <talho@...dia.com>,
        <linux-tegra@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <mperttunen@...dia.com>, <spatra@...dia.com>, <robh+dt@...nel.org>,
        <devicetree@...r.kernel.org>
Subject: Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks


On 7/17/19 5:25 PM, Sowjanya Komatineni wrote:
>
> On 7/17/19 4:44 PM, Dmitry Osipenko wrote:
>> 18.07.2019 2:36, Sowjanya Komatineni пишет:
>>> On 7/17/19 3:48 PM, Dmitry Osipenko wrote:
>>>> 18.07.2019 0:57, Sowjanya Komatineni пишет:
>>>>> On 7/17/19 2:51 PM, Sowjanya Komatineni wrote:
>>>>>> On 7/17/19 2:30 PM, Dmitry Osipenko wrote:
>>>>>>> 17.07.2019 23:11, Sowjanya Komatineni пишет:
>>>>>>>> On 7/17/19 1:01 PM, Sowjanya Komatineni wrote:
>>>>>>>>> On 7/17/19 12:43 PM, Dmitry Osipenko wrote:
>>>>>>>>>> 17.07.2019 21:54, Sowjanya Komatineni пишет:
>>>>>>>>>>> On 7/17/19 11:51 AM, Sowjanya Komatineni wrote:
>>>>>>>>>>>> On 7/17/19 11:32 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>> 17.07.2019 20:29, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>> On 7/17/19 8:17 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>> 17.07.2019 9:36, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>> On 7/16/19 11:33 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>> В Tue, 16 Jul 2019 22:55:52 -0700
>>>>>>>>>>>>>>>>> Sowjanya Komatineni <skomatineni@...dia.com> пишет:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 7/16/19 10:42 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>> В Tue, 16 Jul 2019 22:25:25 -0700
>>>>>>>>>>>>>>>>>>> Sowjanya Komatineni <skomatineni@...dia.com> пишет:
>>>>>>>>>>>>>>>>>>>> On 7/16/19 9:11 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>>> В Tue, 16 Jul 2019 19:35:49 -0700
>>>>>>>>>>>>>>>>>>>>> Sowjanya Komatineni <skomatineni@...dia.com> пишет:
>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 7:18 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 3:06 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 3:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> 17.07.2019 0:35, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 2:21 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>> 17.07.2019 0:12, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 1:47 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 16.07.2019 22:26, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 11:43 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 16.07.2019 21:30, Sowjanya Komatineni 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> пишет:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 11:25 AM, Dmitry Osipenko 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 16.07.2019 21:19, Sowjanya Komatineni 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> пишет:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 9:50 AM, Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 7/16/19 8:00 AM, Dmitry Osipenko 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 16.07.2019 11:06, Peter De Schrijver
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> пишет:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, Jul 16, 2019 at 03:24:26PM
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> +0800,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Joseph
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Lo wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> OK, Will add to CPUFreq driver...
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The other thing that also need
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> attention is
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that T124 CPUFreq
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> driver
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> implicitly relies on DFLL driver
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> probed
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> first, which is
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> icky.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Should I add check for successful
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> dfll clk
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> register explicitly in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPUFreq driver probe and defer till
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> dfll
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> clk
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> registers?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Probably you should use the "device
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> links".
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> See
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [1][2] for the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> example.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://elixir.bootlin.com/linux/v5.2.1/source/drivers/gpu/drm/tegra/dc.c#L2383 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [2]
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://www.kernel.org/doc/html/latest/driver-api/device_link.html 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Return EPROBE_DEFER instead of 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> EINVAL if
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> device_link_add() fails.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> And
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> use of_find_device_by_node() to get 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL's
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> device, see [3].
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [3]
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/devfreq/tegra20-devfreq.c#n100 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Will go thru and add...
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Looks like I initially confused this case
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> getting
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> orphaned clock.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'm now seeing that the DFLL driver
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> registers the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> clock and then
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> clk_get(dfll) should be returning
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> EPROBE_DEFER
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> until
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL driver is
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> probed, hence everything should be fine
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> as-is and
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> there is no real
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> need
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for the 'device link'. Sorry for the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> confusion!
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Sorry, I didn't follow the mail
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> thread. Just
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> regarding the DFLL
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> part.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> As you know it, the DFLL clock is 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> one
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> of the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPU
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> clock sources and
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> integrated with DVFS control logic
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> regulator. We will not
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> switch
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPU to other clock sources once we
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> switched to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL. Because the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPU has
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> been regulated by the DFLL HW 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DVFS
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> table
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> (CVB or OPP
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> table
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> you see
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> in the driver.). We shouldn't 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> reparent
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> other sources with
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> unknew
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> freq/volt pair. That's not
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> guaranteed to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> work. We
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> allow switching to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> open-loop mode but different 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sources.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Okay, then the CPUFreq driver will
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> have to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> enforce
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL freq to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP's
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> rate before switching to PLLP in 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> order to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> have a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> proper CPU voltage.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP freq is safe to work for any CPU
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> voltage.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> So no
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> need to enforce
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL freq to PLLP rate before changing
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CCLK_G
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> source
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to PLLP during
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> suspend
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Sorry, please ignore my above comment.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> During
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> suspend, need to change
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CCLK_G source to PLLP when dfll is in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> closed
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> loop
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mode first and
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> dfll need to be set to open loop.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Okay.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> And I don't exactly understand 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> why we
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> need to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> switch to PLLP in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPU
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> idle
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> driver. Just keep it on CL-DVFS mode
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> time.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> In SC7 entry, the dfll suspend 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> function
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> moves it
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the open-loop
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mode. That's
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all. The sc7-entryfirmware will 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> handle
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the rest
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> of the sequence to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> turn off
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the CPU power.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> In SC7 resume, the warmboot code 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> will
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> handle
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sequence to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> turn on
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> regulator and power up the CPU
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> cluster. And
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> leave
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it on PLL_P.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> After
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> resuming to the kernel, we re-init
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> restore
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the CPU clock
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> policy (CPU
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> runs on DFLL open-loop mode) and 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> moving to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> close-loop mode.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The DFLL is re-inited after switching
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CCLK to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> parent during of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> early clocks-state restoring by CaR
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> driver.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hence
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> instead of having
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> odd
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> hacks in the CaR driver, it is much
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> nicer to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> have a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> proper suspend-resume sequencing of 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> device
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> drivers. In this case
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPUFreq
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> driver is the driver that enables DFLL
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> switches
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPU to that
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> source, which means that this 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> driver is
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> also
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> should
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> be responsible for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> management of the DFLL's state 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> during of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> suspend/resume process. If
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPUFreq driver disables DFLL during
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> suspend
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> re-enables it
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> resume, then looks like the CaR driver
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> hacks
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> around
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL are not
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> needed.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The DFLL part looks good to me. BTW,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> change the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> patch subject to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> "Add
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> suspend-resume support" seems more
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> appropriate to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> me.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> To clarify this, the sequences for 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> use
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> are as
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> follows (assuming
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> required DFLL hw configuration has 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> been
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> done)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Switch to DFLL:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 0) Save current parent and frequency
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 1) Program DFLL to open loop mode
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2) Enable DFLL
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3) Change cclk_g parent to DFLL
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> For OVR regulator:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 4) Change PWM output pin from
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> tristate to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> output
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 5) Enable DFLL PWM output
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> For I2C regulator:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 4) Enable DFLL I2C output
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 6) Program DFLL to closed loop mode
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Switch away from DFLL:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 0) Change cclk_g parent to PLLP so
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the CPU
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> frequency is ok for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> any
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> vdd_cpu voltage
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 1) Program DFLL to open loop mode
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I see during switch away from DFLL
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> (suspend),
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> cclk_g
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> parent is not
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> changed to PLLP before changing dfll to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> open
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> loop
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mode.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Will add this ...
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The CPUFreq driver switches parent to 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> probe, similar
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> should be done on suspend.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'm also wondering if it's always safe to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> switch to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP in the probe.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> If CPU is running on a lower freq than
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP, then
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> some
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> other more
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> appropriate intermediate parent should be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> selected.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CPU parents are PLL_X, PLL_P, and dfll. 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLL_X
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> always
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> runs at higher
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> rate
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> so switching to PLL_P during CPUFreq probe
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> prior to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> dfll clock enable
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> should be safe.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> AFAIK, PLLX could run at ~200MHz. There is
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> also a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> divided output of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> which CCLKG supports, the PLLP_OUT4.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Probably, realistically, CPU is always 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> running
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> off a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> fast PLLX during
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> boot, but I'm wondering what may happen on
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> KEXEC. I
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> guess ideally CPUFreq driver should also
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> have a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 'shutdown' callback to teardown DFLL
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> on a reboot, but likely that there are 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> other
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> clock-related problems as
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> well that may break KEXEC and thus it is 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> very
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> important at the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> moment.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [snip]
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> During bootup CPUG sources from PLL_X. By 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLL_P
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> source
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> above I meant
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLL_P_OUT4.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> As per clock policies, PLL_X is always used
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for high
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> freq
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> like
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 800Mhz
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> and for low frequency it will be sourced 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> from
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Alright, then please don't forget to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> pre-initialize
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP_OUT4 rate to a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> reasonable value using 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_init_table or
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> assigned-clocks.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> PLLP_OUT4 rate update is not needed as it is
>>>>>>>>>>>>>>>>>>>>>>>>>>>> safe to
>>>>>>>>>>>>>>>>>>>>>>>>>>>> run at
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 408Mhz because it is below fmax @ Vmin
>>>>>>>>>>>>>>>>>>>>>>>>>>> So even 204MHz CVB entries are having the same
>>>>>>>>>>>>>>>>>>>>>>>>>>> voltage as
>>>>>>>>>>>>>>>>>>>>>>>>>>> 408MHz, correct? It's not instantly obvious 
>>>>>>>>>>>>>>>>>>>>>>>>>>> to me
>>>>>>>>>>>>>>>>>>>>>>>>>>> from the
>>>>>>>>>>>>>>>>>>>>>>>>>>> DFLL driver's code where the fmax @ Vmin is
>>>>>>>>>>>>>>>>>>>>>>>>>>> defined,
>>>>>>>>>>>>>>>>>>>>>>>>>>> I see
>>>>>>>>>>>>>>>>>>>>>>>>>>> that there is the min_millivolts
>>>>>>>>>>>>>>>>>>>>>>>>>>> and frequency entries starting from 204MHZ 
>>>>>>>>>>>>>>>>>>>>>>>>>>> defined
>>>>>>>>>>>>>>>>>>>>>>>>>>> per-table.
>>>>>>>>>>>>>>>>>>>>>>>>>> Yes at Vmin CPU Fmax is ~800Mhz. So anything 
>>>>>>>>>>>>>>>>>>>>>>>>>> below
>>>>>>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>>>>>>> will
>>>>>>>>>>>>>>>>>>>>>>>>>> work at Vmin voltage and PLLP max is 408Mhz.
>>>>>>>>>>>>>>>>>>>>>>>>> Thank you for the clarification. It would be good
>>>>>>>>>>>>>>>>>>>>>>>>> to have
>>>>>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>>>>>> commented
>>>>>>>>>>>>>>>>>>>>>>>>> in the code as well.
>>>>>>>>>>>>>>>>>>>>>>>> OK, Will add...
>>>>>>>>>>>>>>>>>>>>>>> Regarding, adding suspend/resume to CPUFreq, 
>>>>>>>>>>>>>>>>>>>>>>> CPUFreq
>>>>>>>>>>>>>>>>>>>>>>> suspend
>>>>>>>>>>>>>>>>>>>>>>> happens very early even before disabling non-boot
>>>>>>>>>>>>>>>>>>>>>>> CPUs and
>>>>>>>>>>>>>>>>>>>>>>> also
>>>>>>>>>>>>>>>>>>>>>>> need to export clock driver APIs to CPUFreq.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Was thinking of below way of implementing this...
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Clock DFLL driver Suspend:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>                  - Save CPU clock policy 
>>>>>>>>>>>>>>>>>>>>>>> registers, and
>>>>>>>>>>>>>>>>>>>>>>> Perform
>>>>>>>>>>>>>>>>>>>>>>> dfll
>>>>>>>>>>>>>>>>>>>>>>> suspend which sets in open loop mode
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> CPU Freq driver Suspend: does nothing
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Clock DFLL driver Resume:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>                  - Re-init DFLL, Set in 
>>>>>>>>>>>>>>>>>>>>>>> Open-Loop mode,
>>>>>>>>>>>>>>>>>>>>>>> restore
>>>>>>>>>>>>>>>>>>>>>>> CPU
>>>>>>>>>>>>>>>>>>>>>>> Clock policy registers which actually sets 
>>>>>>>>>>>>>>>>>>>>>>> source to
>>>>>>>>>>>>>>>>>>>>>>> DFLL
>>>>>>>>>>>>>>>>>>>>>>> along
>>>>>>>>>>>>>>>>>>>>>>> with other CPU Policy register restore.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> CPU Freq driver Resume:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>                  - do clk_prepare_enable which 
>>>>>>>>>>>>>>>>>>>>>>> acutally
>>>>>>>>>>>>>>>>>>>>>>> sets
>>>>>>>>>>>>>>>>>>>>>>> DFLL in
>>>>>>>>>>>>>>>>>>>>>>> Closed loop mode
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Adding one more note: Switching CPU Clock to PLLP
>>>>>>>>>>>>>>>>>>>>>>> is not
>>>>>>>>>>>>>>>>>>>>>>> needed
>>>>>>>>>>>>>>>>>>>>>>> as CPU CLock can be from dfll in open-loop mode as
>>>>>>>>>>>>>>>>>>>>>>> DFLL
>>>>>>>>>>>>>>>>>>>>>>> is not
>>>>>>>>>>>>>>>>>>>>>>> disabled anywhere throught the suspend/resume path
>>>>>>>>>>>>>>>>>>>>>>> and SC7
>>>>>>>>>>>>>>>>>>>>>>> entry
>>>>>>>>>>>>>>>>>>>>>>> FW and Warm boot code will switch CPU source to 
>>>>>>>>>>>>>>>>>>>>>>> PLLP.
>>>>>>>>>>>>>>>>>>>>> Since CPU resumes on PLLP, it will be cleaner to 
>>>>>>>>>>>>>>>>>>>>> suspend
>>>>>>>>>>>>>>>>>>>>> it on
>>>>>>>>>>>>>>>>>>>>> PLLP as well. And besides, seems that currently
>>>>>>>>>>>>>>>>>>>>> disabling
>>>>>>>>>>>>>>>>>>>>> DFLL
>>>>>>>>>>>>>>>>>>>>> clock will disable DFLL completely and then you'd
>>>>>>>>>>>>>>>>>>>>> want to
>>>>>>>>>>>>>>>>>>>>> re-init
>>>>>>>>>>>>>>>>>>>>> the DFLL on resume any ways. So better to just 
>>>>>>>>>>>>>>>>>>>>> disable
>>>>>>>>>>>>>>>>>>>>> DFLL
>>>>>>>>>>>>>>>>>>>>> completely on suspend, which should happen on
>>>>>>>>>>>>>>>>>>>>> clk_disable(dfll).
>>>>>>>>>>>>>>>>>>>> Will switch to PLLP during CPUFreq suspend. With
>>>>>>>>>>>>>>>>>>>> decision of
>>>>>>>>>>>>>>>>>>>> using
>>>>>>>>>>>>>>>>>>>> clk_disable during suspend, its mandatory to switch to
>>>>>>>>>>>>>>>>>>>> PLLP as
>>>>>>>>>>>>>>>>>>>> DFLL
>>>>>>>>>>>>>>>>>>>> is completely disabled.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> My earlier concern was on restoring CPU policy as we
>>>>>>>>>>>>>>>>>>>> can't do
>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>> from CPUFreq driver and need export from clock driver.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Clear now and will do CPU clock policy restore in 
>>>>>>>>>>>>>>>>>>>> after
>>>>>>>>>>>>>>>>>>>> dfll
>>>>>>>>>>>>>>>>>>>> re-init.
>>>>>>>>>>>>>>>>>>> Why the policy can't be saved/restored by the CaR 
>>>>>>>>>>>>>>>>>>> driver
>>>>>>>>>>>>>>>>>>> as a
>>>>>>>>>>>>>>>>>>> context of any other clock?
>>>>>>>>>>>>>>>>>> restoring cpu clock policy involves programming 
>>>>>>>>>>>>>>>>>> source and
>>>>>>>>>>>>>>>>>> super_cclkg_divider.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> cclk_g is registered as clk_super_mux and it doesn't use
>>>>>>>>>>>>>>>>>> frac_div ops
>>>>>>>>>>>>>>>>>> to do save/restore its divider.
>>>>>>>>>>>>>>>>> That can be changed of course and I guess it also could
>>>>>>>>>>>>>>>>> be as
>>>>>>>>>>>>>>>>> simple as
>>>>>>>>>>>>>>>>> saving and restoring of two raw u32 values of the
>>>>>>>>>>>>>>>>> policy/divider
>>>>>>>>>>>>>>>>> registers.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Also, during clock context we cant restore cclk_g as 
>>>>>>>>>>>>>>>>>> cclk_g
>>>>>>>>>>>>>>>>>> source
>>>>>>>>>>>>>>>>>> will be dfll and dfll will not be resumed/re-initialized
>>>>>>>>>>>>>>>>>> by the
>>>>>>>>>>>>>>>>>> time
>>>>>>>>>>>>>>>>>> clk_super_mux save/restore happens.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> we can't use save/restore context for dfll clk_ops 
>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>> dfllCPU_out parent to CCLK_G is first in the clock 
>>>>>>>>>>>>>>>>>> tree and
>>>>>>>>>>>>>>>>>> dfll_ref
>>>>>>>>>>>>>>>>>> and dfll_soc peripheral clocks are not restored by the
>>>>>>>>>>>>>>>>>> time dfll
>>>>>>>>>>>>>>>>>> restore happens. Also dfll peripheral clock enables need
>>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>>> restored before dfll restore happens which involves
>>>>>>>>>>>>>>>>>> programming
>>>>>>>>>>>>>>>>>> dfll
>>>>>>>>>>>>>>>>>> controller for re-initialization.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> So dfll resume/re-init is done in clk-tegra210 at end of
>>>>>>>>>>>>>>>>>> all
>>>>>>>>>>>>>>>>>> clocks
>>>>>>>>>>>>>>>>>> restore in V5 series but instead of in clk-tegra210
>>>>>>>>>>>>>>>>>> driver I
>>>>>>>>>>>>>>>>>> moved
>>>>>>>>>>>>>>>>>> now to dfll-fcpu driver pm_ops as all dfll dependencies
>>>>>>>>>>>>>>>>>> will be
>>>>>>>>>>>>>>>>>> restored thru clk_restore_context by then. This will 
>>>>>>>>>>>>>>>>>> be in
>>>>>>>>>>>>>>>>>> V6.
>>>>>>>>>>>>>>>>> Since DFLL is now guaranteed to be disabled across CaR
>>>>>>>>>>>>>>>>> suspend/resume
>>>>>>>>>>>>>>>>> (hence it has nothing to do in regards to CCLK) and given
>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>> PLLs
>>>>>>>>>>>>>>>>> state is restored before the rest of the clocks, I don't
>>>>>>>>>>>>>>>>> see why
>>>>>>>>>>>>>>>>> not to
>>>>>>>>>>>>>>>>> implement CCLK save/restore in a generic fasion. CPU 
>>>>>>>>>>>>>>>>> policy
>>>>>>>>>>>>>>>>> wull be
>>>>>>>>>>>>>>>>> restored to either PLLP or PLLX (if CPUFreq driver is
>>>>>>>>>>>>>>>>> disabled).
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> CCLK_G save/restore should happen in clk_super_mux ops
>>>>>>>>>>>>>>>> save/context and
>>>>>>>>>>>>>>>> clk_super_mux save/restore happens very early as cclk_g is
>>>>>>>>>>>>>>>> first
>>>>>>>>>>>>>>>> in the
>>>>>>>>>>>>>>>> clock tree and save/restore traverses through the tree
>>>>>>>>>>>>>>>> top-bottom
>>>>>>>>>>>>>>>> order.
>>>>>>>>>>>>>>> If CCLK_G is restored before the PLLs, then just change the
>>>>>>>>>>>>>>> clocks
>>>>>>>>>>>>>>> order
>>>>>>>>>>>>>>> such that it won't happen.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I dont think we can change clocks order for CCLK_G.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> During bootup, cclk_g is registered after all pll's and
>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>> clocks which is the way we wanted, So cclk_g will be the 
>>>>>>>>>>>>>> first
>>>>>>>>>>>>>> one in
>>>>>>>>>>>>>> the clk list as clk_register adds new clock first in the 
>>>>>>>>>>>>>> list.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> When clk_save_context and clk_restore_context APIs iterates
>>>>>>>>>>>>>> over the
>>>>>>>>>>>>>> list, cclk_g is the first
>>>>>>>>>>>>> Looking at clk_core_restore_context(), I see that it walks up
>>>>>>>>>>>>> CLKs
>>>>>>>>>>>>> list
>>>>>>>>>>>>> from parent to children, hence I don't understand how it can
>>>>>>>>>>>>> ever
>>>>>>>>>>>>> happen
>>>>>>>>>>>>> that CCLK will be restored before the parent. The clocks
>>>>>>>>>>>>> registration
>>>>>>>>>>>>> order doesn't matter at all in that case.
>>>>>>>>>>>> yes from parent to children and dfllCPU_out is the top in the
>>>>>>>>>>>> list and
>>>>>>>>>>>> its child is cclk_g.
>>>>>>>>>>>>
>>>>>>>>>>>> the way clocks are registered is the order I see in the clock
>>>>>>>>>>>> list and
>>>>>>>>>>>> looking into clk_register API it adds new node first in the 
>>>>>>>>>>>> list.
>>>>>>>>>>>>
>>>>>>>>>>> cclkg_g & dfll register happens after all plls and peripheral
>>>>>>>>>>> clocks as
>>>>>>>>>>> it need ref, soc and peripheral clocks to be enabled.
>>>>>>>>>>>> So they are the last to get registered and so becomes first in
>>>>>>>>>>>> the
>>>>>>>>>>>> list.
>>>>>>>>>>>>
>>>>>>>>>>>> During save/restore context, it traverses thru this list and
>>>>>>>>>>>> first in
>>>>>>>>>>>> the list is dfllcpu_OUT (parent) and its child (cclk_g)
>>>>>>>>>>>>
>>>>>>>>>>>> saving should not be an issue at all but we cant restore
>>>>>>>>>>>> cclk_g/dfll
>>>>>>>>>>>> in normal way thru clk_ops restore as plls and peripherals
>>>>>>>>>>>> restore
>>>>>>>>>>>> doesn't happen by that time.
>>>>>>>>>>>>
>>>>>>>>>>> I was referring to clk_restore_context where it iterates thru
>>>>>>>>>>> root list
>>>>>>>>>>> and for each core from the root list clk_core_restore does
>>>>>>>>>>> restore of
>>>>>>>>>>> parent and children.
>>>>>>>>>>>
>>>>>>>>>>> dfllCPU_Out gets first in the list and its child is cclk_g
>>>>>>>>>>>
>>>>>>>>>>> https://elixir.bootlin.com/linux/v5.2.1/source/drivers/clk/clk.c#L1105 
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> What list you're talking about? clk_summary? It shows current
>>>>>>>>>> *active*
>>>>>>>>>> clocks configuration, if you'll try to disable CPUFreq driver 
>>>>>>>>>> then
>>>>>>>>>> the
>>>>>>>>>> parent of CCLK_G should be PLLX. Similarly when CPU is
>>>>>>>>>> reparented to
>>>>>>>>>> PLLP on driver's suspend, then PLLP is the parent.
>>>>>>>>>>
>>>>>>>>>>>>>>>> DFLL enable thru CPUFreq resume happens after all
>>>>>>>>>>>>>>>> clk_restore_context
>>>>>>>>>>>>>>>> happens. So during clk_restore_context, dfll re-init 
>>>>>>>>>>>>>>>> doesnt
>>>>>>>>>>>>>>>> happen
>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>> doing cpu clock policy restore during super_mux clk_ops 
>>>>>>>>>>>>>>>> will
>>>>>>>>>>>>>>>> crash as
>>>>>>>>>>>>>>>> DFLL is not initialized and its clock is not enabled 
>>>>>>>>>>>>>>>> but CPU
>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>> restore sets source to DFLL if we restore during
>>>>>>>>>>>>>>>> super_clk_mux
>>>>>>>>>>>>>>> If CPU was suspended on PLLP, then it will be restored on
>>>>>>>>>>>>>>> PLLP by
>>>>>>>>>>>>>>> CaR. I
>>>>>>>>>>>>>>> don't understand what DFLL has to do with the CCLK in that
>>>>>>>>>>>>>>> case
>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>> the clocks restore.
>>>>>>>>>>>>>> My above comment is in reference to your request of doing
>>>>>>>>>>>>>> save/restore
>>>>>>>>>>>>>> for cclk_g in normal fashion thru save/restore context. 
>>>>>>>>>>>>>> Because
>>>>>>>>>>>>>> of the
>>>>>>>>>>>>>> clk order I mentioned above, we cclk_g will be the first 
>>>>>>>>>>>>>> one to
>>>>>>>>>>>>>> go thru
>>>>>>>>>>>>>> save/context.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> During save_context of cclk_g, source can be from PLLX, 
>>>>>>>>>>>>>> dfll.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Issue will be when we do restore during 
>>>>>>>>>>>>>> clk_restore_context of
>>>>>>>>>>>>>> cclk_g as
>>>>>>>>>>>>>> by that time PLLX/dfll will not be restored.
>>>>>>>>>>>>>>
>>>>>>>>>>>>> Seems we already agreed that DFLL will be disabled by the
>>>>>>>>>>>>> CPUFreq
>>>>>>>>>>>>> driver
>>>>>>>>>>>>> on suspend. Hence CCLK can't be from DFLL if CPU is
>>>>>>>>>>>>> reparented to
>>>>>>>>>>>>> PLLP
>>>>>>>>>>>>> on CPUFreq driver's suspend, otherwise CPU keeps running 
>>>>>>>>>>>>> from a
>>>>>>>>>>>>> boot-state PLLX if CPUFreq driver is disabled.
>>>>>>>>>>>> Yes suspend should not be an issue but issue will be during
>>>>>>>>>>>> resume
>>>>>>>>>>>> where if we do cclk_g restore in normal way thru
>>>>>>>>>>>> clk_restore_context,
>>>>>>>>>>>> cclk_g restore happens very early as dfllCPU out is the first
>>>>>>>>>>>> one that
>>>>>>>>>>>> goes thru restore context and plls/peripherals are not 
>>>>>>>>>>>> resumed by
>>>>>>>>>>>> then.
>>>>>>>>>>>>
>>>>>>>>>>>> CPU runs from PLLX if dfll clock enable fails during boot. So
>>>>>>>>>>>> when it
>>>>>>>>>>>> gets to suspend, we save CPU running clock source as either
>>>>>>>>>>>> PLLX or
>>>>>>>>>>>> DFLL and then we switch to PLLP.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On resume, CPU runs from PLLP by warm boot code and we need to
>>>>>>>>>>>> restore
>>>>>>>>>>>> back its source to the one it was using from saved source 
>>>>>>>>>>>> context
>>>>>>>>>>>> (which can be either PLLX or DFLL)
>>>>>>>>>>>>
>>>>>>>>>>>> So PLLs & DFLL resume need to happen before CCLKG 
>>>>>>>>>>>> restore/resume.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> With all above discussions, we do DFLL disable in CPUFreq
>>>>>>>>>>>> driver on
>>>>>>>>>>>> suspend and on CPUFreq resume we enable DFLL back and 
>>>>>>>>>>>> restore CPU
>>>>>>>>>>>> clock source it was using during suspend (which will be either
>>>>>>>>>>>> PLLX if
>>>>>>>>>>>> dfll enable fails during probe or it will be using DFLL).
>>>>>>>>>> During suspend CPU's parent shall be PLLP and not DFLL (note 
>>>>>>>>>> that
>>>>>>>>>> it is
>>>>>>>>>> disabled) after reparenting to PLLP by the CPUFreq driver.
>>>>>>>>>>
>>>>>>>>> CPU source context should be saved before switching to safe
>>>>>>>>> source of
>>>>>>>>> PLLP as on resume we need to restore back to source it was using
>>>>>>>>> before we switch to safe source during suspend entry.
>>>>>>>>>
>>>>>>>>> So saved context for CPU Source will be either dfll or PLLX
>>>>>>>>>
>>>>>>>> PLLP reparenting is only during suspend/entry to have it as safe
>>>>>>>> source
>>>>>>>> but actual CPU source it was running from before suspending is 
>>>>>>>> either
>>>>>>>> dfll/pllx which should be the one to be restored on CPUFreq 
>>>>>>>> resume.
>>>>>>>> Resume happens with CPU running from PLLP till it gets to the
>>>>>>>> point of
>>>>>>>> restoring its original source (dfll or pllx)
>>>>>>> CaR should restore CPU to PLLP or PLLX, while CPUFreq driver 
>>>>>>> restores
>>>>>>> CPU to DFLL. Please see more comments below.
>>>>>>>
>>>>>>>>>>>> So i was trying to say dfll/cclk_g restore can't be done in
>>>>>>>>>>>> normal way
>>>>>>>>>>>> thru clk_ops save/restore context
>>>>>>>>>> Let's see what happens if CPUFreq is active:
>>>>>>>>>>
>>>>>>>>>> 1. CPUFreq driver probe happens
>>>>>>>>>>        2. CPU is reparented to PLLP
>>>>>>>>>>        3. DFLL inited
>>>>>>>>>>        4. CPU is reparented to DFLL
>>>>>>>>>>
>>>>>>>>>> 5. CPUFreq driver suspend happens
>>>>>>>>>>        6. CPU is reparented to PLLP
>>>>>>>>>>        7. DFLL is disabled
>>>>>>>>>>
>>>>>>>>>> 8. Car suspend happens
>>>>>>>>>>        9. DFLL context saved
>>>>>>>>>>        10. PLLP/PLLX context saved
>>>>>>>>>>        11. CCLK context saved
>>>>>>>>>>
>>>>>>>>>> 12. Car resume happens
>>>>>>>>>>        13. DFLL context restored
>>>>>>>>>>        14. PLLP/PLLX context restored
>>>>>>>>>>        15. CCLK context restored
>>>>>>>>>>
>>>>>>>>>> 16. CPUFreq driver resume happens
>>>>>>>>>>        17. DFLL re-inited
>>>>>>>>>>        18. CPU is reparented to DFLL
>>>>>>>>> Below is the order of sequence it should be based on the order of
>>>>>>>>> clk
>>>>>>>>> register.
>>>>>>>>>
>>>>>>>>> My comments inline in this sequence.
>>>>>>>>>
>>>>>>>>> 1. CPUFreq driver probe happens
>>>>>>>>>        2. CPU is reparented to PLLP
>>>>>>>>>        3. DFLL inited
>>>>>>>>>        4. CPU is reparented to DFLL
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 5. CPUFreq driver suspend happens
>>>>>>>>>        6. Save CPU source which could be either dfll or pllx
>>>>>>> Please see my next comment.
>>>>>>>
>>>>>>>>>        7. CPU is reparented to safe known source PLLP
>>>>>>>>>        8. DFLL is disabled
>>>>>>>>>
>>>>>>>>> 8. Car suspend happens
>>>>>>>>>        9. DFLL context saved (With DFLL disabled in CPUFreq 
>>>>>>>>> suspend,
>>>>>>>>> nothing to be saved here as last freq req will always be saved).
>>>>>>>>>        10. CCLK context saved (CPU clock source will be saved in
>>>>>>>>> CPUFreq
>>>>>>>>> driver suspend which could be either dfll or pllx)
>>>>>>> That I don't understand. The CPU's clock source state should be
>>>>>>> saved at
>>>>>>> the moment of the CaR's suspending (i.e. CCLK policy will be set to
>>>>>>> PLLP
>>>>>>> or PLLX) and then CCLK state should be also restored by the CaR in
>>>>>>> step 14.
>>>>>> CPU clock to be saved and restored should be the source used 
>>>>>> before we
>>>>>> switch it to safe PLLP for suspend/resume operation.
>>>>>>
>>>>>> This original source could be either PLLX or DFLL which it was using
>>>>>> before we disable DFLL during CPU Freq suspend.
>>>>>>
>>>>>> If we save CPU clock source at moment of CAR suspending, it will be
>>>>>> PLLP as we switch to safe PLLP in CPUFreq driver suspend.
>>>>>>
>>>>>> Infact, we dont need to restore CPU clock source to PLLP anywhere in
>>>>>> resume as it comes up with PLLP source from warm boot code itself.
>>>> You must always maintain proper suspend/resume encapsulation, 
>>>> otherwise
>>>> it's a total mess. It doesn't matter that CCLK is restored to PLLP 
>>>> even
>>>> that CPU is already running off PLLP after warmboot.
>>>>
>>>>>> But we need to restore CPU source to original source it was using
>>>>>> before we switch to safe PLLP source for suspend operation. This
>>>>>> original source could be PLLX/DFLL and this should be re-stored in
>>>>>> CPUFreq resume as by this time PLLs and peripherals are restored and
>>>>>> dfll is re-initialized.
>>>>>>
>>>>>> So saving actual CPU source before switching to intermediate safe 
>>>>>> PLLP
>>>>>> in CPUFreq driver and then restoring back during CPUFreq resume 
>>>>>> should
>>>>>> be good as CPUFreq resume happens right after all clocks (plls
>>>>>> restore, peripherals restore, dfll resume)>>
>>>>>>> CPUFreq driver should only switch CPU to PLLP and disable DFLL on
>>>>>>> suspend in step 5, that's it. On resume CPUFreq driver will restore
>>>>>>> CPU
>>>>>>> to DFLL in step 18.
>>>>>> Also I don't think we should hard-code to force CPU source to 
>>>>>> DFLL on
>>>>>> CPUFreq resume.
>>>>>>
>>>>>> Reason is during CPU Probe when it tries to switch to dfll 
>>>>>> source, for
>>>>>> some reason if dfll enable fails it sets CPU to its original source
>>>>>> which will be PLLX.
>>>> No!
>>>>
>>>> 1. CPU voltage could be too low for PLLX
>>>> 2. PLLX rate can't be changed without manual reparenting CPU to
>>>> intermediate clock
>>>> 3. CPUFreq can't manually manage CPU voltage
>>>>
>>>> DFLL-restoring failure is an extreme case. CPU must be kept on a safe
>>>> PLLP in that case and disable_cpufreq() must be invoked as well.
>>> OK, PLLX option was also in my mind. So If we just consider sources as
>>> DFLL or PLLP, then we can save source in CCLK save context and restore
>>> in CCLK restore basically it will be PLLP.
>>>
>>> Later during CPUFreq resume we can just switch to DFLL and if DFLL
>>> enable fails we will keep source as PLLP. Yes will invoke
>>> disable_cpufreq as well in case of dfll enable failure for some reason.
>> Sounds good!
>>
>>>>>> So CPU source could be either DFLL or PLLX in CPUFreq
>>>>>> tegra124_cpu_switch_to_dfll
>>>>>>
>>>>>>>>>        11. PLLP/PLLX context saved
>>>>>>>>>        12. Peripheral Clock saved
>>>>>>>>>
>>>>>>>>> 12. Car resume happens
>>>>>>>>>        13. DFLL context restored : No DFLL context to be restored
>>>>>>>>> and we
>>>>>>>>> only need to reinitialize DFLL and re-initialize can't be done
>>>>>>>>> here as
>>>>>>>>> this is the 1st to get restored and PLL/Peripheral clocks are not
>>>>>>>>> restored by this time. So we can't use clk_ops restore for DFLL
>>>>>>> It looks to me that clk_core_restore_context() should just do
>>>>>>> hlist_for_each_entry *_reverse*. Don't you think so?
>>>>>> Thought of that but this is in core driver and is used by other
>>>>>> non-tegra clock driver and not sure if that impacts for those.
>>>> The reverse ordering should be correct, I think it's just a 
>>>> shortcoming
>>>> of the CCF that need to be fixed. But will better to make a more
>>>> thorough research, asking Stephen and Michael for the clarification.
>>>>
>>>>>> But with decision of switching CPUFreq with dfll clock 
>>>>>> enable/disable
>>>>>> during CPUFreq suspend/resume, we can re-init dfll during dfll-fcpu
>>>>>> driver resume and we don't need CCLK save/restore.
>>>>>>
>> Actually CPUFreq driver should implement suspend/resume regardless of
>> CaR ability to restore DFLL or whatever, simply to properly handle
>> possible clock restoring failure on resume as we just found out.
>>
>>>>> the way of all clocks order is good except cclk_g which has 
>>>>> dependency
>>>>> on multiple clocks.
>>>> CCLK_G has a single parent at a time. What "multiple clocks" you're
>>>> talking about? Please explain.
>>> dependencies I am referring are dfll_ref, dfll_soc, and DVFS peripheral
>>> clocks which need to be restored prior to DFLL reinit.
>> Okay, but that shouldn't be a problem if clock dependencies are set up
>> properly.
>>
>>>>> reverse list order during restore might not work as all other 
>>>>> clocks are
>>>>> in proper order no with any ref clocks for plls getting restored 
>>>>> prior
>>>>> to their clients
>>>> Why? The ref clocks should be registered first and be the roots for 
>>>> PLLs
>>>> and the rest. If it's not currently the case, then this need to be
>>>> fixed. You need to ensure that each clock is modeled properly. If some
>>>> child clock really depends on multiple parents, then the parents 
>>>> need to
>>>> in the correct order or CCF need to be taught about such
>>>> multi-dependencies.
>>>>
>>>> If some required feature is missed, then you have to implement it
>>>> properly and for all, that's how things are done in upstream. 
>>>> Sometimes
>>>> it's quite a lot of extra work that everyone are benefiting from in
>>>> the end.
>>>>
>>>> [snip]
>>> Yes, we should register ref/parents before their clients.
>>>
>>> cclk_g clk is registered last after all pll and peripheral clocks are
>>> registers during clock init.
>>>
>>> dfllCPU_out clk is registered later during dfll-fcpu driver probe and
>>> gets added to the clock list.
>>>
>>> Probably the issue seems to be not linking dfll_ref and dfll_soc
>>> dependencies for dfllCPU_out thru clock list.
>>>
>>> clk-dfll driver during dfll_init_clks gets ref_clk and soc_clk 
>>> reference
>>> thru DT.
>> Please try to fix all missing dependencies and orderings.
>
> Peter,
>
> dfllCPU_OUT is the first one to go thru restore when 
> clk_restore_context traverses thru the list.
>
> dfllCPU_OUT has dependency on DFLL_ref and DFLL_SOC but this 
> dependency is unknown to clock-tree.
>
> We can add DFLL_REF and DFLL_SOC as parents to dfllCPU_OUT during 
> register so dfllCPU_OUT save/restore happens after their parents are 
> restored.
>
> But DFLL needs both of these to be restored before DFLLCPU_Out and as 
> DFLL_SOC restore always happens after the REF, thinking to add 
> DFLL_SOC as parent to dfllCPU_OUT so save/restore follows after their 
> dependencies.
>
> Please comment.
>
Did quick try and I see by adding dfll-soc as parent to dfllCPU_OUT, its 
in proper order after all its dependencies.

Can now add dfll save/restore to do dfll reinit during restore..

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ