Re: [PATCH V6 14/21] clk: tegra210: Add suspend and resume support

From: Sowjanya Komatineni <skomatineni@nvidia.com>
To: Stephen Boyd <sboyd@kernel.org>,
	Dmitry Osipenko <digetx@gmail.com>, <jason@lakedaemon.net>,
	<jonathanh@nvidia.com>, <linus.walleij@linaro.org>,
	<marc.zyngier@arm.com>, <mark.rutland@arm.com>, <stefan@agner.ch>,
	<tglx@linutronix.de>, <thierry.reding@gmail.com>
Cc: <pdeschrijver@nvidia.com>, <pgaikwad@nvidia.com>,
	<linux-clk@vger.kernel.org>, <linux-gpio@vger.kernel.org>,
	<jckuo@nvidia.com>, <josephl@nvidia.com>, <talho@nvidia.com>,
	<linux-tegra@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<mperttunen@nvidia.com>, <spatra@nvidia.com>,
	<robh+dt@kernel.org>, <devicetree@vger.kernel.org>
Subject: Re: [PATCH V6 14/21] clk: tegra210: Add suspend and resume support
Date: Fri, 2 Aug 2019 13:39:57 -0700	[thread overview]
Message-ID: <641727e6-4796-f982-3b58-4c8d666de1a2@nvidia.com> (raw)
In-Reply-To: <20190802175119.1E401217F5@mail.kernel.org>


On 8/2/19 10:51 AM, Stephen Boyd wrote:
> Quoting Dmitry Osipenko (2019-07-22 00:12:17)
>> 22.07.2019 10:09, Dmitry Osipenko пишет:
>>> 22.07.2019 9:52, Sowjanya Komatineni пишет:
>>>> On 7/21/19 11:10 PM, Dmitry Osipenko wrote:
>>>>> 22.07.2019 1:45, Sowjanya Komatineni пишет:
>>>>>> On 7/21/19 2:38 PM, Dmitry Osipenko wrote:
>>>>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>>>>> @@ -2853,9 +2859,8 @@ static int tegra210_enable_pllu(void)
>>>>>>>>         reg |= PLL_ENABLE;
>>>>>>>>         writel(reg, clk_base + PLLU_BASE);
>>>>>>>>     -    readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>>>> -                      reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>>> -    if (!(reg & PLL_BASE_LOCK)) {
>>>>>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>>>> +    if (ret) {
>>>>>>> Why this is needed? Was there a bug?
>>>>>>>
>>>>>> during resume pllu init is needed and to use same terga210_init_pllu,
>>>>>> poll_timeout_atomic can't be used as its ony for atomic context.
>>>>>>
>>>>>> So changed to use wait_for_mask which should work in both cases.
>>>>> Atomic variant could be used from any context, not sure what do you
>>>>> mean. The 'atomic' part only means that function won't cause scheduling
>>>>> and that's it.
>>>> Sorry, replied incorrect. readx_poll_timeout_atomic uses ktime_get() and
>>>> during resume timekeeping suspend/resume happens later than clock
>>>> suspend/resume. So using tegra210_wait_for_mask.
>>>>
>>>> both timekeeping and clk-tegra210 drivers are registered as syscore but
>>>> not ordered.
>>> Okay, thank you for the clarification.
>>>
>>> [snip]
>>>
>> You should remove the 'iopoll.h' then, since it's not used anymore.
> And also add a comment to this location in the code because it's
> non-obvious that we can't use iopoll here.
>
Actually added comment during function usage instead of during include 
as iopoll.h is removed.

Will add additional comment in include section as well highlighting 
reason for removal of iopoll.h