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]
Message-ID: <84a0d46a-bca2-1000-a2a6-8890ee702dd3@gmail.com>
Date:   Fri, 9 Aug 2019 21:00:16 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Sowjanya Komatineni <skomatineni@...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
Cc:     pdeschrijver@...dia.com, pgaikwad@...dia.com, sboyd@...nel.org,
        linux-clk@...r.kernel.org, linux-gpio@...r.kernel.org,
        jckuo@...dia.com, josephl@...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, rjw@...ysocki.net,
        viresh.kumar@...aro.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v8 11/21] clk: tegra: clk-dfll: Add suspend and resume
 support

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 <skomatineni@...dia.com>
>>> ---
>>>   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.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ