Linux-GPIO Archive on lore.kernel.org
 help / color / Atom feed
From: Sowjanya Komatineni <skomatineni@nvidia.com>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: <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>,
	<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 v8 14/21] clk: tegra210: Add suspend and resume support
Date: Mon, 12 Aug 2019 12:03:06 -0700
Message-ID: <cd685e84-c0de-6142-597e-f7c77604350e@nvidia.com> (raw)
In-Reply-To: <1779e92b-fa4d-68ab-9218-51970eee1ec5@gmail.com>


On 8/12/19 11:19 AM, Dmitry Osipenko wrote:
> 12.08.2019 20:28, Sowjanya Komatineni пишет:
>> On 8/12/19 9:25 AM, Dmitry Osipenko wrote:
>>> 11.08.2019 22:15, Sowjanya Komatineni пишет:
>>>> On 8/11/19 10:39 AM, Dmitry Osipenko wrote:
>>>>> 09.08.2019 21:40, Sowjanya Komatineni пишет:
>>>>>> On 8/9/19 11:18 AM, Dmitry Osipenko wrote:
>>>>>>> 09.08.2019 19:19, Sowjanya Komatineni пишет:
>>>>>>>> On 8/9/19 6:56 AM, Dmitry Osipenko wrote:
>>>>>>>>> 09.08.2019 2:46, Sowjanya Komatineni пишет:
>>>>>>>>>> This patch adds support for clk: tegra210: suspend-resume.
>>>>>>>>>>
>>>>>>>>>> All the CAR controller settings are lost on suspend when core
>>>>>>>>>> power goes off.
>>>>>>>>>>
>>>>>>>>>> This patch has implementation for saving and restoring all PLLs
>>>>>>>>>> and clocks context during system suspend and resume to have the
>>>>>>>>>> clocks back to same state for normal operation.
>>>>>>>>>>
>>>>>>>>>> Clock driver suspend and resume are registered as syscore_ops as clocks
>>>>>>>>>> restore need to happen before the other drivers resume to have all their
>>>>>>>>>> clocks back to the same state as before suspend.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>>> ---
>>>>>>>>>>      drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
>>>>>>>>>>      drivers/clk/tegra/clk.c          |  64 ++++++++++++++++++++++++
>>>>>>>>>>      drivers/clk/tegra/clk.h          |   3 ++
>>>>>>>>>>      3 files changed, 166 insertions(+), 4 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> index 998bf60b219a..8dd6f4f4debb 100644
>>>>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> @@ -9,13 +9,13 @@
>>>>>>>>>>      #include <linux/clkdev.h>
>>>>>>>>>>      #include <linux/of.h>
>>>>>>>>>>      #include <linux/of_address.h>
>>>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>>>      #include <linux/delay.h>
>>>>>>>>>>      #include <linux/export.h>
>>>>>>>>>>      #include <linux/mutex.h>
>>>>>>>>>>      #include <linux/clk/tegra.h>
>>>>>>>>>>      #include <dt-bindings/clock/tegra210-car.h>
>>>>>>>>>>      #include <dt-bindings/reset/tegra210-car.h>
>>>>>>>>>> -#include <linux/iopoll.h>
>>>>>>>>>>      #include <linux/sizes.h>
>>>>>>>>>>      #include <soc/tegra/pmc.h>
>>>>>>>>>>      @@ -220,11 +220,15 @@
>>>>>>>>>>      #define CLK_M_DIVISOR_SHIFT 2
>>>>>>>>>>      #define CLK_M_DIVISOR_MASK 0x3
>>>>>>>>>>      +#define CLK_MASK_ARM    0x44
>>>>>>>>>> +#define MISC_CLK_ENB    0x48
>>>>>>>>>> +
>>>>>>>>>>      #define RST_DFLL_DVCO 0x2f4
>>>>>>>>>>      #define DVFS_DFLL_RESET_SHIFT 0
>>>>>>>>>>        #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>>>>>      #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>>>>>        #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>>>>>      #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>>>>> @@ -2825,6 +2829,7 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>>          struct tegra_clk_pll_freq_table *fentry;
>>>>>>>>>>          struct tegra_clk_pll pllu;
>>>>>>>>>>          u32 reg;
>>>>>>>>>> +    int ret;
>>>>>>>>>>            for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>>>>>>>>>>              if (fentry->input_rate == pll_ref_freq)
>>>>>>>>>> @@ -2853,9 +2858,14 @@ 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)) {
>>>>>>>>>> +    /*
>>>>>>>>>> +     * During clocks resume, same PLLU init and enable sequence get
>>>>>>>>>> +     * executed. So, readx_poll_timeout_atomic can't be used here as it
>>>>>>>>>> +     * uses ktime_get() and timekeeping resume doesn't happen by that
>>>>>>>>>> +     * time. So, using tegra210_wait_for_mask for PLL LOCK.
>>>>>>>>>> +     */
>>>>>>>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>>>>>> +    if (ret) {
>>>>>>>>>>              pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>>>>>              return -ETIMEDOUT;
>>>>>>>>>>          }
>>>>>>>>>> @@ -3288,6 +3298,84 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>>>>>>>>>>      }
>>>>>>>>>>        #ifdef CONFIG_PM_SLEEP
>>>>>>>>>> +/*
>>>>>>>>>> + * This array lists mask values for each peripheral clk bank
>>>>>>>>>> + * to mask out reserved bits during the clocks state restore
>>>>>>>>>> + * on SC7 resume to prevent accidental writes to these reserved
>>>>>>>>>> + * bits.
>>>>>>>>>> + */
>>>>>>>>>> +static u32 periph_clk_rsvd_mask[TEGRA210_CAR_BANK_COUNT] = {
>>>>>>>>> Should be more natural to have a "valid_mask" instead of "rsvd_mask".
>>>>>>>>>
>>>>>>>>> What's actually wrong with touching of the reserved bits? They must be NO-OP.. or the
>>>>>>>>> reserved bits are actually some kind of "secret" bits? If those bits have some use-case
>>>>>>>>> outside of Silicon HW (like FPGA simulation), then this doesn't matter for upstream
>>>>>>>>> and you
>>>>>>>>> have to keep the workaround locally in the downstream kernel or whatever.
>>>>>>>> Will rename as valid_mask.
>>>>>>>>
>>>>>>>> some bits in these registers are undefined and is not good to write to these bits as
>>>>>>>> they
>>>>>>>> can cause pslverr.
>>>>>>> Okay, it should be explained in the comment.
>>>>>>>
>>>>>>> Is it possible to disable trapping of changing the undefined bits?
>>>>>> No its internal to design
>>>>> Okay.
>>>>>
>>>>> Also, what about to move the valid_mask into struct tegra_clk_periph_regs?
>>>> No, we cannot move to tegra_clk_periph_regs as its in tegra/clk.c and is common for all
>>>> tegra.
>>>>
>>>> Reserved bits are different on tegra chips so should come from Tegra chip specific clock
>>>> driver like
>>>>
>>>> clk-tegra210 for Tegra210.
>>> Could you please check whether the reserved bits are RAZ (read as zero)?
>>>
>>> [snip]
>> yes all reserved bits of clk_enb register is 0. This should not be set to 1.
>>
>> As I will be changing to variable name to valid_mask instead of reserved mask, will also
>> change values to valid mask so it can be used directly to write to clk_enb for enabling all
>> peripherals clks.
>>
> It looks to me that the tegra_clk_periph_force_on() could be made local to the
> clk-tegra210.c and then the raw clk_enb values could be written directly instead of having
> the clk_enb[] array, probably that will be a bit cleaner

All CLK_OUT_ENB* registers are already defined in clk driver and also 
periph_regs includes all of these to use.

To write value to enable all clocks directly without array, it need 
total 7 individual register writes for Tegra210. Also when 
suspend/resume is implemented for other prior tegras, they need to do 
same in tegra clock driver.

Reason I had this in clock driver is, this can be used by all tegra 
clock drivers and just can pass valid clocks values.

But doing individual register write with direct hard code values in 
corresponding tegra clock driver is preferred still, will update so in 
next revision and will move all the CLK_OUT_ENB* register defines to 
tegra/clk.h

Currently RST_DEVICES & CLK_OUT_ENB are all in tegra/clk.c

> .

  reply index

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-08 23:46 [PATCH v8 00/21] SC7 entry and exit support for Tegra210 Sowjanya Komatineni
2019-08-08 23:46 ` [PATCH v8 01/21] pinctrl: tegra: Fix write barrier placement in pmx_writel Sowjanya Komatineni
2019-08-09 11:38   ` Dmitry Osipenko
2019-08-12  9:20   ` Thierry Reding
2019-08-14  8:32   ` Linus Walleij
2019-08-08 23:46 ` [PATCH v8 02/21] pinctrl: tegra: Add write barrier after all pinctrl register writes Sowjanya Komatineni
2019-08-09 11:39   ` Dmitry Osipenko
2019-08-12  9:20   ` Thierry Reding
2019-08-14  8:33   ` Linus Walleij
2019-08-08 23:46 ` [PATCH v8 03/21] clk: tegra: divider: Save and restore divider rate Sowjanya Komatineni
2019-08-12  9:21   ` Thierry Reding
2019-08-08 23:46 ` [PATCH v8 04/21] clk: tegra: pllout: Save and restore pllout context Sowjanya Komatineni
2019-08-11 18:04   ` Dmitry Osipenko
2019-08-08 23:46 ` [PATCH v8 05/21] clk: tegra: pll: Save and restore pll context Sowjanya Komatineni
2019-08-09 11:33   ` Dmitry Osipenko
2019-08-09 17:39     ` Sowjanya Komatineni
2019-08-09 17:50       ` Dmitry Osipenko
2019-08-09 18:50         ` Sowjanya Komatineni
2019-08-11 17:24           ` Dmitry Osipenko
2019-08-09 12:46   ` Dmitry Osipenko
2019-08-08 23:46 ` [PATCH v8 06/21] clk: tegra: Support for OSC context save and restore Sowjanya Komatineni
2019-08-08 23:46 ` [PATCH v8 07/21] clk: Add API to get index of the clock parent Sowjanya Komatineni
2019-08-09 11:49   ` Dmitry Osipenko
2019-08-12  9:47   ` Thierry Reding
2019-08-08 23:46 ` [PATCH v8 08/21] clk: tegra: periph: Add restore_context support Sowjanya Komatineni
2019-08-09 11:55   ` Dmitry Osipenko
2019-08-09 12:20     ` Dmitry Osipenko
2019-08-09 16:55       ` Sowjanya Komatineni
2019-08-12  9:50   ` Thierry Reding
2019-08-08 23:46 ` [PATCH v8 09/21] clk: tegra: clk-super: Fix to enable PLLP branches to CPU Sowjanya Komatineni
2019-08-09 12:11   ` Dmitry Osipenko
2019-08-12  9:53   ` Thierry Reding
2019-08-08 23:46 ` [PATCH v8 10/21] clk: tegra: clk-super: Add restore-context support Sowjanya Komatineni
2019-08-09 12:17   ` Dmitry Osipenko
2019-08-09 17:08     ` Sowjanya Komatineni
2019-08-11 17:29       ` Dmitry Osipenko
2019-08-12  9:55   ` Thierry Reding
2019-08-08 23:46 ` [PATCH v8 11/21] clk: tegra: clk-dfll: Add suspend and resume support Sowjanya Komatineni
2019-08-09 12:23   ` Dmitry Osipenko
2019-08-09 16:39     ` Sowjanya Komatineni
2019-08-09 18:00       ` Dmitry Osipenko
2019-08-09 18:33         ` Sowjanya Komatineni
2019-08-09 18:52           ` Dmitry Osipenko
2019-08-12 10:01   ` Thierry Reding
2019-08-08 23:46 ` [PATCH v8 12/21] cpufreq: tegra124: " Sowjanya Komatineni
2019-08-12 10:07   ` Thierry Reding
2019-08-08 23:46 ` [PATCH v8 13/21] clk: tegra210: Use fence_udelay during PLLU init Sowjanya Komatineni
2019-08-11 18:02   ` Dmitry Osipenko
2019-08-11 19:16     ` Sowjanya Komatineni
2019-08-08 23:46 ` [PATCH v8 14/21] clk: tegra210: Add suspend and resume support Sowjanya Komatineni
2019-08-09 13:56   ` Dmitry Osipenko
2019-08-09 16:19     ` Sowjanya Komatineni
2019-08-09 18:18       ` Dmitry Osipenko
     [not found]         ` <cbe94f84-a17b-7e1a-811d-89db571784e1@nvidia.com>
2019-08-11 17:39           ` Dmitry Osipenko
2019-08-11 19:15             ` Sowjanya Komatineni
2019-08-12 16:25               ` Dmitry Osipenko
2019-08-12 17:28                 ` Sowjanya Komatineni
2019-08-12 18:19                   ` Dmitry Osipenko
2019-08-12 19:03                     ` Sowjanya Komatineni [this message]
2019-08-12 20:28                       ` Dmitry Osipenko
2019-08-12 10:17   ` Thierry Reding
2019-08-08 23:46 ` [PATCH v8 15/21] soc/tegra: pmc: Allow to support more tegras wake Sowjanya Komatineni
2019-08-11 17:52   ` Dmitry Osipenko
2019-08-08 23:46 ` [PATCH v8 16/21] soc/tegra: pmc: Add pmc wake support for tegra210 Sowjanya Komatineni
2019-08-09 13:28   ` Dmitry Osipenko
2019-08-08 23:46 ` [PATCH v8 17/21] arm64: tegra: Enable wake from deep sleep on RTC alarm Sowjanya Komatineni
2019-08-08 23:46 ` [PATCH v8 18/21] soc/tegra: pmc: Configure core power request polarity Sowjanya Komatineni
2019-08-09 13:13   ` Dmitry Osipenko
2019-08-08 23:46 ` [PATCH v8 19/21] soc/tegra: pmc: Configure deep sleep control settings Sowjanya Komatineni
2019-08-09 13:23   ` Dmitry Osipenko
2019-08-09 16:23     ` Sowjanya Komatineni
2019-08-09 17:24       ` Sowjanya Komatineni
2019-08-09 18:22         ` Dmitry Osipenko
2019-08-08 23:46 ` [PATCH v8 20/21] arm64: dts: tegra210-p2180: Jetson TX1 SC7 timings Sowjanya Komatineni
2019-08-08 23:47 ` [PATCH v8 21/21] arm64: dts: tegra210-p3450: Jetson Nano " Sowjanya Komatineni

Reply instructions:

You may reply publically 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=cd685e84-c0de-6142-597e-f7c77604350e@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

Linux-GPIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-gpio/0 linux-gpio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-gpio linux-gpio/ https://lore.kernel.org/linux-gpio \
		linux-gpio@vger.kernel.org linux-gpio@archiver.kernel.org
	public-inbox-index linux-gpio


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-gpio


AGPL code for this site: git clone https://public-inbox.org/ public-inbox