From: Sowjanya Komatineni <skomatineni@nvidia.com>
To: Dmitry Osipenko <digetx@gmail.com>,
thierry.reding@gmail.com, jonathanh@nvidia.com,
tglx@linutronix.de, jason@lakedaemon.net, marc.zyngier@arm.com,
linus.walleij@linaro.org, stefan@agner.ch, mark.rutland@arm.com
Cc: pdeschrijver@nvidia.com, pgaikwad@nvidia.com, sboyd@kernel.org,
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, rjw@rjwysocki.net,
viresh.kumar@linaro.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH v7 01/20] pinctrl: tegra: Add suspend and resume support
Date: Tue, 6 Aug 2019 14:54:18 -0700 [thread overview]
Message-ID: <961ceece-f42a-b933-9184-97e9d30ea381@nvidia.com> (raw)
In-Reply-To: <36351140-afd4-38c4-3722-4ee0894287fa@gmail.com>
On 8/6/19 10:59 AM, Dmitry Osipenko wrote:
> 05.08.2019 21:06, Sowjanya Komatineni пишет:
>> On 8/5/19 3:50 AM, Dmitry Osipenko wrote:
>>> 01.08.2019 0:10, Sowjanya Komatineni пишет:
>>>> This patch adds support for Tegra pinctrl driver suspend and resume.
>>>>
>>>> During suspend, context of all pinctrl registers are stored and
>>>> on resume they are all restored to have all the pinmux and pad
>>>> configuration for normal operation.
>>>>
>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 59
>>>> +++++++++++++++++++++++++++++++++++
>>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
>>>> 2 files changed, 62 insertions(+)
>>>>
>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> index 186ef98e7b2b..e3a237534281 100644
>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> @@ -631,6 +631,58 @@ static void
>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>> }
>>>> }
>>>> +static size_t tegra_pinctrl_get_bank_size(struct device *dev,
>>>> + unsigned int bank_id)
>>>> +{
>>>> + struct platform_device *pdev = to_platform_device(dev);
>>>> + struct resource *res;
>>>> +
>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, bank_id);
>>>> +
>>>> + return resource_size(res) / 4;
>>>> +}
>>>> +
>>>> +static int tegra_pinctrl_suspend(struct device *dev)
>>>> +{
>>>> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
>>>> + u32 *backup_regs = pmx->backup_regs;
>>>> + u32 *regs;
>>>> + size_t bank_size;
>>>> + unsigned int i, k;
>>>> +
>>>> + for (i = 0; i < pmx->nbanks; i++) {
>>>> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
>>>> + regs = pmx->regs[i];
>>>> + for (k = 0; k < bank_size; k++)
>>>> + *backup_regs++ = readl_relaxed(regs++);
>>>> + }
>>>> +
>>>> + return pinctrl_force_sleep(pmx->pctl);
>>>> +}
>>>> +
>>>> +static int tegra_pinctrl_resume(struct device *dev)
>>>> +{
>>>> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
>>>> + u32 *backup_regs = pmx->backup_regs;
>>>> + u32 *regs;
>>>> + size_t bank_size;
>>>> + unsigned int i, k;
>>>> +
>>>> + for (i = 0; i < pmx->nbanks; i++) {
>>>> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
>>>> + regs = pmx->regs[i];
>>>> + for (k = 0; k < bank_size; k++)
>>>> + writel_relaxed(*backup_regs++, regs++);
>>>> + }
>>> I'm now curious whether any kind of barrier is needed after the
>>> writings. The pmx_writel() doesn't insert a barrier after the write and
>>> seems it just misuses writel, which actually should be writel_relaxed()
>>> + barrier, IIUC.
>> pmx_writel uses writel and it has wmb before raw_write which complete
>> all writes initiated prior to this.
>>
>> By misusing writel, you mean to have barrier after register write?
> Yes, at least to me it doesn't make much sense for this driver to stall
> before the write. It's the pinctrl user which should be taking care
> about everything to be ready before making a change to the pinctrl's
> configuration.
>
>>> It's also not obvious whether PINCTRL HW has any kind of write-FIFO and
>>> thus maybe read-back + rmb() is needed in order ensure that writes are
>>> actually completed.
>> I believe adding write barrier wmb after writel_relaxed should be good
>> rather than doing readback + rmb
>>> The last thing which is not obvious is when the new configuration
>>> actually takes into effect, does it happen immediately or maybe some
>>> delay is needed?
>>>
>>> [snip]
>> Based on internal design there is no internal delay and it all depends
>> on APB rate that it takes to write to register.
>>
>> Pinmux value change to reflect internally might take couple of clock
>> cycles which is much faster than SW can read.
> Still not quite obvious if it's possible to have a case where some
> hardware is touched before necessary pinctrl change is fully completed
> and then to get into trouble because of it.
To be safer, will add write barrier after all writes in resume and also
will have separate patch for pmx_writel fix to use writel_relaxed
followed by write barrier.
Thanks
Sowjanya
next prev parent reply other threads:[~2019-08-06 21:54 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-31 21:10 [PATCH v7 00/20] SC7 entry and exit support for Tegra210 Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 01/20] pinctrl: tegra: Add suspend and resume support Sowjanya Komatineni
2019-08-05 9:20 ` Linus Walleij
2019-08-06 21:51 ` Sowjanya Komatineni
2019-08-07 3:40 ` Sowjanya Komatineni
2019-08-07 13:11 ` Linus Walleij
2019-08-05 10:50 ` Dmitry Osipenko
2019-08-05 18:06 ` Sowjanya Komatineni
2019-08-06 17:59 ` Dmitry Osipenko
2019-08-06 21:54 ` Sowjanya Komatineni [this message]
2019-07-31 21:10 ` [PATCH v7 02/20] pinctrl: tegra210: Add Tegra210 pinctrl pm ops Sowjanya Komatineni
2019-08-05 9:21 ` Linus Walleij
2019-07-31 21:10 ` [PATCH v7 03/20] clk: tegra: divider: Save and restore divider rate Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 04/20] clk: tegra: pllout: Save and restore pllout context Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 05/20] clk: tegra: pll: Save and restore pll context Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 08/20] clk: tegra: clk-super: Fix to enable PLLP branches to CPU Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 09/20] clk: tegra: clk-super: Add save and restore support Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 10/20] clk: tegra: clk-dfll: Add suspend and resume support Sowjanya Komatineni
2019-08-01 10:18 ` Dmitry Osipenko
2019-08-01 10:37 ` Dmitry Osipenko
2019-08-01 16:10 ` Sowjanya Komatineni
2019-08-01 17:10 ` Dmitry Osipenko
2019-08-01 17:53 ` Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 11/20] cpufreq: tegra124: " Sowjanya Komatineni
2019-08-01 5:40 ` Viresh Kumar
2019-08-01 17:51 ` Sowjanya Komatineni
2019-08-02 3:41 ` Viresh Kumar
2019-07-31 21:10 ` [PATCH v7 12/20] clk: tegra210: Use fence_udelay during PLLU init Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 13/20] clk: tegra210: Add suspend and resume support Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 14/20] soc/tegra: pmc: Allow to support more tegras wake Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 15/20] soc/tegra: pmc: Add pmc wake support for tegra210 Sowjanya Komatineni
2019-07-31 21:10 ` [PATCH v7 16/20] arm64: tegra: Enable wake from deep sleep on RTC alarm Sowjanya Komatineni
2019-07-31 21:11 ` [PATCH v7 17/20] soc/tegra: pmc: Configure core power request polarity Sowjanya Komatineni
2019-07-31 21:11 ` [PATCH v7 18/20] soc/tegra: pmc: Configure deep sleep control settings Sowjanya Komatineni
2019-07-31 21:11 ` [PATCH v7 19/20] arm64: dts: tegra210-p2180: Jetson TX1 SC7 timings Sowjanya Komatineni
2019-07-31 21:11 ` [PATCH v7 20/20] arm64: dts: tegra210-p3450: Jetson Nano " Sowjanya Komatineni
-- strict thread matches above, loose matches on Subject: below --
2019-07-31 0:20 [PATCH v7 00/20] SC7 entry and exit support for Tegra210 Sowjanya Komatineni
2019-07-31 0:20 ` [PATCH v7 01/20] pinctrl: tegra: Add suspend and resume support Sowjanya Komatineni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=961ceece-f42a-b933-9184-97e9d30ea381@nvidia.com \
--to=skomatineni@nvidia.com \
--cc=devicetree@vger.kernel.org \
--cc=digetx@gmail.com \
--cc=jason@lakedaemon.net \
--cc=jckuo@nvidia.com \
--cc=jonathanh@nvidia.com \
--cc=josephl@nvidia.com \
--cc=linus.walleij@linaro.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=mark.rutland@arm.com \
--cc=mperttunen@nvidia.com \
--cc=pdeschrijver@nvidia.com \
--cc=pgaikwad@nvidia.com \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
--cc=spatra@nvidia.com \
--cc=stefan@agner.ch \
--cc=talho@nvidia.com \
--cc=tglx@linutronix.de \
--cc=thierry.reding@gmail.com \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).