09.08.2019 21:33, Sowjanya Komatineni пишет: > > On 8/9/19 11:00 AM, Dmitry Osipenko wrote: >> 09.08.2019 19:39, Sowjanya Komatineni пишет: >>> On 8/9/19 5:23 AM, Dmitry Osipenko wrote: >>>> 09.08.2019 2:46, Sowjanya Komatineni пишет: >>>>> This patch implements DFLL suspend and resume operation. >>>>> >>>>> During system suspend entry, CPU clock will switch CPU to safe >>>>> clock source of PLLP and disables DFLL clock output. >>>>> >>>>> DFLL driver suspend confirms DFLL disable state and errors out on >>>>> being active. >>>>> >>>>> DFLL is re-initialized during the DFLL driver resume as it goes >>>>> through complete reset during suspend entry. >>>>> >>>>> Signed-off-by: Sowjanya Komatineni <[email protected]> >>>>> --- >>>>> drivers/clk/tegra/clk-dfll.c | 56 >>>>> ++++++++++++++++++++++++++++++ >>>>> drivers/clk/tegra/clk-dfll.h | 2 ++ >>>>> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 1 + >>>>> 3 files changed, 59 insertions(+) >>>>> >>>>> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c >>>>> index f8688c2ddf1a..eb298a5d7be9 100644 >>>>> --- a/drivers/clk/tegra/clk-dfll.c >>>>> +++ b/drivers/clk/tegra/clk-dfll.c >>>>> @@ -1487,6 +1487,7 @@ static int dfll_init(struct tegra_dfll *td) >>>>> td->last_unrounded_rate = 0; >>>>> pm_runtime_enable(td->dev); >>>>> + pm_runtime_irq_safe(td->dev); >>>>> pm_runtime_get_sync(td->dev); >>>>> dfll_set_mode(td, DFLL_DISABLED); >>>>> @@ -1513,6 +1514,61 @@ static int dfll_init(struct tegra_dfll *td) >>>>> return ret; >>>>> } >>>>> +/** >>>>> + * tegra_dfll_suspend - check DFLL is disabled >>>>> + * @dev: DFLL device * >>>>> + * >>>>> + * DFLL clock should be disabled by the CPUFreq driver. So, make >>>>> + * sure it is disabled and disable all clocks needed by the DFLL. >>>>> + */ >>>>> +int tegra_dfll_suspend(struct device *dev) >>>>> +{ >>>>> + struct tegra_dfll *td = dev_get_drvdata(dev); >>>>> + >>>>> + if (dfll_is_running(td)) { >>>>> + dev_err(td->dev, "dfll is enabled while shouldn't be\n"); >>>>> + return -EBUSY; >>>>> + } >>>>> + >>>>> + reset_control_assert(td->dvco_rst); >>>>> + >>>>> + return 0; >>>>> +} >>>>> +EXPORT_SYMBOL(tegra_dfll_suspend); >>>>> + >>>>> +/** >>>>> + * tegra_dfll_resume - reinitialize DFLL on resume >>>>> + * @dev: DFLL instance >>>>> + * >>>>> + * DFLL is disabled and reset during suspend and resume. >>>>> + * So, reinitialize the DFLL IP block back for use. >>>>> + * DFLL clock is enabled later in closed loop mode by CPUFreq >>>>> + * driver before switching its clock source to DFLL output. >>>>> + */ >>>>> +int tegra_dfll_resume(struct device *dev) >>>>> +{ >>>>> + struct tegra_dfll *td = dev_get_drvdata(dev); >>>>> + >>>>> + reset_control_deassert(td->dvco_rst); >>>> This doesn't look right because I assume that DFLL resetting is >>>> synchronous and thus clk should be enabled in order for reset to >>>> propagate inside hardware. >>>> >>>>> + pm_runtime_get_sync(td->dev); >>>> Hence it will be better to remove the above reset_control_deassert() and >>>> add here: >>>> >>>> reset_control_reset(td->dvco_rst); >>> By the time dfll resume happens, dfll controller clock will already be >>> enabled. >>> >>> so doing reset de-assert before pm_runtime seems ok. >> I don't see what enables the DFLL clock because it should be enabled by the >> CPUFreq driver >> on resume from suspend and resume happens after resuming of the DFLL driver. > > dvco_rst is part of peripheral clocks and all peripheral clocks are restored > by clk-tegra210 > driver which happens before dfll driver resume. > > So dfll rst thru part of peripheral clock enable is set prior to dfll reset > deassertion
Ah, so that is DVCO resetting and not DFLL, which are different blocks. Looks correct then. Reviewed-by: Dmitry Osipenko <[email protected]>
