[{"id":2196658,"web_url":"http://patchwork.ozlabs.org/comment/2196658/","msgid":"<20190618121607.GN28892@ulmo>","list_archive_url":null,"date":"2019-06-18T12:16:07","subject":"Re: [PATCH V3 11/17] clk: tegra210: support for Tegra210 clocks\n\tsuspend and resume","submitter":{"id":26234,"url":"http://patchwork.ozlabs.org/api/people/26234/","name":"Thierry Reding","email":"thierry.reding@gmail.com"},"content":"On Tue, Jun 18, 2019 at 12:46:25AM -0700, Sowjanya Komatineni wrote:\n> This patch adds system suspend and resume support for Tegra210\n> clocks.\n> \n> All the CAR controller settings are lost on suspend when core power\n> goes off.\n> \n> This patch has implementation for saving and restoring all the PLLs\n> and clocks context during system suspend and resume to have the\n> system back to operating state.\n> \n> Signed-off-by: Sowjanya Komatineni \n> ---\n> drivers/clk/tegra/clk-tegra210.c | 218 +++++++++++++++++++++++++++++++++++++--\n> 1 file changed, 211 insertions(+), 7 deletions(-)\n> \n> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c\n> index e1ba62d2b1a0..c34d92e871f4 100644\n> --- a/drivers/clk/tegra/clk-tegra210.c\n> +++ b/drivers/clk/tegra/clk-tegra210.c\n> @@ -9,10 +9,12 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -20,6 +22,7 @@\n> #include \n> \n> #include \"clk.h\"\n> +#include \"clk-dfll.h\"\n> #include \"clk-id.h\"\n> \n> /*\n> @@ -36,6 +39,8 @@\n> #define CLK_SOURCE_LA 0x1f8\n> #define CLK_SOURCE_SDMMC2 0x154\n> #define CLK_SOURCE_SDMMC4 0x164\n> +#define CLK_OUT_ENB_Y 0x298\n> +#define CLK_ENB_PLLP_OUT_CPU BIT(31)\n> \n> #define PLLC_BASE 0x80\n> #define PLLC_OUT 0x84\n> @@ -225,6 +230,7 @@\n> \n> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8\n> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac\n> +#define CPU_SOFTRST_CTRL 0x380\n> \n> #define LVL2_CLK_GATE_OVRA 0xf8\n> #define LVL2_CLK_GATE_OVRC 0x3a0\n> @@ -2820,6 +2826,7 @@ static int tegra210_enable_pllu(void)\n> \tstruct tegra_clk_pll_freq_table *fentry;\n> \tstruct tegra_clk_pll pllu;\n> \tu32 reg;\n> +\tint ret;\n> \n> \tfor (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {\n> \t\tif (fentry->input_rate == pll_ref_freq)\n> @@ -2836,7 +2843,7 @@ static int tegra210_enable_pllu(void)\n> \treg = readl_relaxed(clk_base + pllu.params->ext_misc_reg[0]);\n> \treg &= ~BIT(pllu.params->iddq_bit_idx);\n> \twritel_relaxed(reg, clk_base + pllu.params->ext_misc_reg[0]);\n> -\tudelay(5);\n> +\tfence_udelay(5, clk_base);\n> \n> \treg = readl_relaxed(clk_base + PLLU_BASE);\n> \treg &= ~GENMASK(20, 0);\n> @@ -2844,13 +2851,13 @@ static int tegra210_enable_pllu(void)\n> \treg |= fentry->n << 8;\n> \treg |= fentry->p << 16;\n> \twritel(reg, clk_base + PLLU_BASE);\n> -\tudelay(1);\n> +\tfence_udelay(1, clk_base);\n\nThese udelay() -> fence_udelay() seem like they should be a separate\npatch.\n\n> \treg |= PLL_ENABLE;\n> \twritel(reg, clk_base + PLLU_BASE);\n> +\tfence_udelay(1, clk_base);\n> \n> -\treadl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,\n> -\t\t\t\t\t reg & PLL_BASE_LOCK, 2, 1000);\n> -\tif (!(reg & PLL_BASE_LOCK)) {\n> +\tret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);\n> +\tif (ret) {\n> \t\tpr_err(\"Timed out waiting for PLL_U to lock\\n\");\n> \t\treturn -ETIMEDOUT;\n> \t}\n> @@ -2890,12 +2897,12 @@ static int tegra210_init_pllu(void)\n> \t\treg = readl_relaxed(clk_base + XUSB_PLL_CFG0);\n> \t\treg &= ~XUSB_PLL_CFG0_PLLU_LOCK_DLY_MASK;\n> \t\twritel_relaxed(reg, clk_base + XUSB_PLL_CFG0);\n> -\t\tudelay(1);\n> +\t\tfence_udelay(1, clk_base);\n> \n> \t\treg = readl_relaxed(clk_base + PLLU_HW_PWRDN_CFG0);\n> \t\treg |= PLLU_HW_PWRDN_CFG0_SEQ_ENABLE;\n> \t\twritel_relaxed(reg, clk_base + PLLU_HW_PWRDN_CFG0);\n> -\t\tudelay(1);\n> +\t\tfence_udelay(1, clk_base);\n> \n> \t\treg = readl_relaxed(clk_base + PLLU_BASE);\n> \t\treg &= ~PLLU_BASE_CLKENABLE_USB;\n> @@ -3282,6 +3289,188 @@ static void tegra210_disable_cpu_clock(u32 cpu)\n> }\n> \n> #ifdef CONFIG_PM_SLEEP\n> +static u32 cpu_softrst_ctx[3];\n> +static struct platform_device *dfll_pdev;\n> +static u32 *periph_clk_src_ctx;\n> +struct periph_source_bank {\n\nBlank line between the above two.\n\n> +\tu32 start;\n> +\tu32 end;\n> +};\n> +\n> +static struct periph_source_bank periph_srcs[] = {\n> +\t[0] = {\n> +\t\t.start = 0x100,\n> +\t\t.end = 0x198,\n> +\t},\n> +\t[1] = {\n> +\t\t.start = 0x1a0,\n> +\t\t.end = 0x1f8,\n> +\t},\n> +\t[2] = {\n> +\t\t.start = 0x3b4,\n> +\t\t.end = 0x42c,\n> +\t},\n> +\t[3] = {\n> +\t\t.start = 0x49c,\n> +\t\t.end = 0x4b4,\n> +\t},\n> +\t[4] = {\n> +\t\t.start = 0x560,\n> +\t\t.end = 0x564,\n> +\t},\n> +\t[5] = {\n> +\t\t.start = 0x600,\n> +\t\t.end = 0x678,\n> +\t},\n> +\t[6] = {\n> +\t\t.start = 0x694,\n> +\t\t.end = 0x6a0,\n> +\t},\n> +\t[7] = {\n> +\t\t.start = 0x6b8,\n> +\t\t.end = 0x718,\n> +\t},\n> +};\n> +\n> +/* This array lists the valid clocks for each periph clk bank */\n> +static u32 periph_clks_on[] = {\n> +\t0xdcd7dff9,\n> +\t0x87d1f3e7,\n> +\t0xf3fed3fa,\n> +\t0xffc18cfb,\n> +\t0x793fb7ff,\n> +\t0x3fe66fff,\n> +\t0xfc1fc7ff,\n> +};\n\nHm... this is a bunch of magic. Perhaps replace this by a list of the\nclock IDs? That's perhaps a little more verbose, but if we ever need to\ntweak the list of IDs in that periph_clks_on array, that'll be quite the\nchallenge.\n\nAlso, is this list a \"guess\" or are these all guaranteed to be always\non? What if some of these ended up getting disabled as part of suspend\nalready (by their users). If we force them on, won't their references\nbecome unbalanced if the driver later enables them again on resume?\n\n> +\n> +static struct platform_device *dfll_pdev;\n\nI think you already predeclared this one above.\n\n> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) + ((_off) * 4))\n> +#define car_writel(_val, _base, _off) \\\n> +\t\twritel_relaxed(_val, clk_base + (_base) + ((_off) * 4))\n> +\n> +static u32 * __init tegra210_init_suspend_ctx(void)\n> +{\n> +\tint i, size = 0;\n\nCan both be unsigned int.\n\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n> +\t\tsize += periph_srcs[i].end - periph_srcs[i].start + 4;\n> +\n> +\tperiph_clk_src_ctx = kmalloc(size, GFP_KERNEL);\n> +\n> +\treturn periph_clk_src_ctx;\n\nIt's somewhat wasteful to return a global variable since you can access\nit anyway. Perhaps it'd be more useful to make the function return a\nboolean?\n\n> +}\n> +\n> +static int tegra210_clk_suspend(void)\n> +{\n> +\tint i;\n\nunsigned int.\n\n> +\tunsigned long off;\n> +\tstruct device_node *node;\n> +\tu32 *clk_rst_ctx = periph_clk_src_ctx;\n> +\tu32 val;\n> +\n> +\ttegra_cclkg_burst_policy_save_context();\n> +\n> +\tif (!dfll_pdev) {\n> +\t\tnode = of_find_compatible_node(NULL, NULL,\n> +\t\t\t\t\t \"nvidia,tegra210-dfll\");\n> +\t\tif (node)\n> +\t\t\tdfll_pdev = of_find_device_by_node(node);\n> +\t\tof_node_put(node);\n> +\t\tif (!dfll_pdev)\n> +\t\t\tpr_err(\"dfll node not found. no suspend for dfll\\n\");\n> +\t}\n\nWouldn't it make sense to run this only once, perhaps as part of\ntegra210_init_suspend_ctx()?\n\n> +\n> +\tif (dfll_pdev)\n> +\t\ttegra_dfll_suspend(dfll_pdev);\n> +\n> +\t/* Enable PLLP_OUT_CPU after dfll suspend */\n> +\tval = car_readl(CLK_OUT_ENB_Y, 0);\n> +\tval |= CLK_ENB_PLLP_OUT_CPU;\n> +\tcar_writel(val, CLK_OUT_ENB_Y, 0);\n> +\n> +\ttegra_clk_periph_suspend(clk_base);\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n> +\t\tfor (off = periph_srcs[i].start; off <= periph_srcs[i].end;\n> +\t\t off += 4)\n> +\t\t\t*clk_rst_ctx++ = car_readl(off, 0);\n> +\n> +\ttegra_sclk_cclklp_burst_policy_save_context();\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)\n> +\t\tcpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);\n> +\n> +\tclk_save_context();\n> +\n> +\treturn 0;\n> +}\n> +\n> +static void tegra210_clk_resume(void)\n> +{\n> +\tint i;\n> +\tunsigned long off;\n> +\tu32 val;\n> +\tu32 *clk_rst_ctx = periph_clk_src_ctx;\n> +\tstruct clk_hw *parent;\n> +\tstruct clk *clk;\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)\n> +\t\tcar_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);\n> +\n> +\ttegra_clk_osc_resume(clk_base);\n> +\n> +\t/*\n> +\t * restore all the plls before configuring clocks and resetting\n> +\t * the devices.\n> +\t */\n> +\ttegra210_init_pllu();\n> +\ttegra_sclk_cpulp_burst_policy_restore_context();\n> +\tclk_restore_context();\n> +\n> +\t/* enable all clocks before configuring clock sources */\n> +\ttegra_clk_periph_force_on(periph_clks_on, ARRAY_SIZE(periph_clks_on),\n> +\t\t\t\t clk_base);\n> +\t/* wait for all writes to happen to have all the clocks enabled */\n> +\twmb();\n> +\tfence_udelay(2, clk_base);\n> +\n> +\t/* restore all the devices clock sources */\n> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n> +\t\tfor (off = periph_srcs[i].start; off <= periph_srcs[i].end;\n> +\t\t off += 4)\n> +\t\t\tcar_writel(*clk_rst_ctx++, off, 0);\n> +\n> +\t/* propagate and restore resets, restore clock state */\n> +\tfence_udelay(5, clk_base);\n> +\ttegra_clk_periph_resume(clk_base);\n> +\n> +\t/*\n> +\t * restore CPUG clocks:\n> +\t * - enable DFLL in open loop mode\n> +\t * - switch CPUG to DFLL clock source\n> +\t * - close DFLL loop\n> +\t * - sync PLLX state\n> +\t */\n> +\tif (dfll_pdev)\n> +\t\ttegra_dfll_resume(dfll_pdev, false);\n> +\n> +\ttegra_cclkg_burst_policy_restore_context();\n> +\tfence_udelay(2, clk_base);\n> +\n> +\tif (dfll_pdev)\n> +\t\ttegra_dfll_resume(dfll_pdev, true);\n> +\n> +\tparent = clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));\n> +\tclk = clks[TEGRA210_CLK_PLL_X];\n> +\tif (parent != __clk_get_hw(clk))\n> +\t\ttegra_clk_sync_state_pll(__clk_get_hw(clk));\n> +\n> +\t/* Disable PLL_OUT_CPU after DFLL resume */\n> +\tval = car_readl(CLK_OUT_ENB_Y, 0);\n> +\tval &= ~CLK_ENB_PLLP_OUT_CPU;\n> +\tcar_writel(val, CLK_OUT_ENB_Y, 0);\n> +}\n\nI'm surprised by the amount of work that we need to do here. I had hoped\nthat the clock framework's save/restore infrastructure would be enough.\nI suppose you do call clk_restore_context() somewhere in there, so maybe\nthis really is as good as it gets.\n\nThierry\n\n> +\n> static void tegra210_cpu_clock_suspend(void)\n> {\n> \t/* switch coresite to clk_m, save off original source */\n> @@ -3295,8 +3484,20 @@ static void tegra210_cpu_clock_resume(void)\n> \twritel(tegra210_cpu_clk_sctx.clk_csite_src,\n> \t\t\t\tclk_base + CLK_SOURCE_CSITE);\n> }\n> +#else\n> +#define tegra210_clk_suspend\tNULL\n> +#define tegra210_clk_resume\tNULL\n> +static inline u32 *tegra210_init_suspend_ctx(void)\n> +{\n> +\treturn NULL;\n> +}\n> #endif\n> \n> +static struct syscore_ops tegra_clk_syscore_ops = {\n> +\t.suspend = tegra210_clk_suspend,\n> +\t.resume = tegra210_clk_resume,\n> +};\n> +\n> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {\n> \t.wait_for_reset\t= tegra210_wait_cpu_in_reset,\n> \t.disable_clock\t= tegra210_disable_cpu_clock,\n> @@ -3580,5 +3781,8 @@ static void __init tegra210_clock_init(struct device_node *np)\n> \ttegra210_mbist_clk_init();\n> \n> \ttegra_cpu_car_ops = &tegra210_cpu_car_ops;\n> +\n> +\tif (tegra210_init_suspend_ctx())\n> +\t\tregister_syscore_ops(&tegra_clk_syscore_ops);\n> }\n> CLK_OF_DECLARE(tegra210, \"nvidia,tegra210-car\", tegra210_clock_init);\n> -- \n> 2.7.4\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdmarc=pass (p=none dis=none) header.from=gmail.com","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"LvauVUYw\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 45Sn9t1r26z9s3l\n\tfor ;\n\tTue, 18 Jun 2019 22:16:18 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1726640AbfFRMQO (ORCPT );\n\tTue, 18 Jun 2019 08:16:14 -0400","from mail-wr1-f68.google.com ([209.85.221.68]:46020 \"EHLO\n\tmail-wr1-f68.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1725913AbfFRMQN (ORCPT\n\t); Tue, 18 Jun 2019 08:16:13 -0400","by mail-wr1-f68.google.com with SMTP id f9so13662483wre.12;\n\tTue, 18 Jun 2019 05:16:09 -0700 (PDT)","from localhost (p2E5BEF36.dip0.t-ipconnect.de. [46.91.239.54])\n\tby smtp.gmail.com with ESMTPSA id\n\tt140sm3297550wmt.0.2019.06.18.05.16.07\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tTue, 18 Jun 2019 05:16:08 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=HMwhhkiVWNPMPXNcoEexQlnkoqgB9aDQdk5uXYLarRc=;\n\tb=LvauVUYw85TX2Y+CVPLrj2kglMLRXmw8CUHWuf196mbpysi7ey+2/1tES2Kw+I2n5i\n\tIDZIfGhtzJhUbUhQx8A3jiCMYexJcHQBH+6j7TINeDx47HZgMJkE8arEKY7jIYWBm34s\n\tsCDpnV9gsnEkHJ64tetsn8f0F5YR0fQQVIdsa8VJ3MKQX4Fy4S7Bybv6HQRztq7tsvGQ\n\tzCycRyPqe3raKKVaWylUYa6DCloo4O0wLI6AeJXaXtw312z73KMNz2SJn+9rR5FJ1gRs\n\t0bRywLX+5e9mwCHDFdOadJunbSO0o4ZdwbDZXkv02Pi2Wv7HOQ/TVWme59fc2uR18Tys\n\t9jCg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=HMwhhkiVWNPMPXNcoEexQlnkoqgB9aDQdk5uXYLarRc=;\n\tb=JQ1v4WQUWhzbJRifgLzuzT3mKB5fu2RbD0kuhc4dc5m+ThBfGvkRvTDJYC7E4wGNp4\n\t30gGg3xmLM+iSa6wfCvQP2c+NlftJlzcGq6QyJ5IqjqAE/6C2F6mOO6fi5LHy3/q85Ij\n\tvHC6kqwR1a34+2BAzhCBHHO3JL5C0D+Cl4gvaGFXvSWMeFPy0HhFXyw0meedQ0/cp8IN\n\t7Q2PB4Sx2Lfqa9/TOLqryYZkMqzhFzP3hPSRbC4zFAu5mQLJzDJlMe2PYs9hry9D3DG2\n\tShDVTPgU0NKTRejoB5ZyQwm9KpJVO11QyqrO50w+hAgjQipCK4MAfqlRbj3PEid3+Ouv\n\tv2jQ==","X-Gm-Message-State":"APjAAAUxMI8Ocpv2ceHtSy2o5V0D9JXoKeWX2LTj87cJK/qQERiwZSsp\n\tb3JKKyuAE8Iu2r4zVAKymNU=","X-Google-Smtp-Source":"APXvYqyVbOS0NbYVPFmkwYZmjGepni1A4WK8T4UqjFlWvBuJJgpaWGa7EEtjiyADK/MKd4LobAgtMg==","X-Received":"by 2002:a5d:6243:: with SMTP id m3mr53935093wrv.41.1560860169030;\n\tTue, 18 Jun 2019 05:16:09 -0700 (PDT)","Date":"Tue, 18 Jun 2019 14:16:07 +0200","From":"Thierry Reding ","To":"Sowjanya Komatineni ","Cc":"jonathanh@nvidia.com, tglx@linutronix.de, jason@lakedaemon.net,\n\tmarc.zyngier@arm.com, linus.walleij@linaro.org, stefan@agner.ch,\n\tmark.rutland@arm.com, pdeschrijver@nvidia.com, pgaikwad@nvidia.com,\n\tsboyd@kernel.org, linux-clk@vger.kernel.org,\n\tlinux-gpio@vger.kernel.org, jckuo@nvidia.com, josephl@nvidia.com,\n\ttalho@nvidia.com, linux-tegra@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, mperttunen@nvidia.com,\n\tspatra@nvidia.com, robh+dt@kernel.org, digetx@gmail.com,\n\tdevicetree@vger.kernel.org","Subject":"Re: [PATCH V3 11/17] clk: tegra210: support for Tegra210 clocks\n\tsuspend and resume","Message-ID":"<20190618121607.GN28892@ulmo>","References":"<1560843991-24123-1-git-send-email-skomatineni@nvidia.com>\n\t<1560843991-24123-12-git-send-email-skomatineni@nvidia.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"tfmLD+Hxjexp/STe\"","Content-Disposition":"inline","In-Reply-To":"<1560843991-24123-12-git-send-email-skomatineni@nvidia.com>","User-Agent":"Mutt/1.11.4 (2019-03-13)","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":2197030,"web_url":"http://patchwork.ozlabs.org/comment/2197030/","msgid":"<491e0b18-11e7-837c-4591-06ed30950e1d@nvidia.com>","list_archive_url":null,"date":"2019-06-18T17:58:40","subject":"Re: [PATCH V3 11/17] clk: tegra210: support for Tegra210 clocks\n\tsuspend and resume","submitter":{"id":75554,"url":"http://patchwork.ozlabs.org/api/people/75554/","name":"Sowjanya Komatineni","email":"skomatineni@nvidia.com"},"content":"On 6/18/19 5:16 AM, Thierry Reding wrote:\n> On Tue, Jun 18, 2019 at 12:46:25AM -0700, Sowjanya Komatineni wrote:\n>> This patch adds system suspend and resume support for Tegra210\n>> clocks.\n>>\n>> All the CAR controller settings are lost on suspend when core power\n>> goes off.\n>>\n>> This patch has implementation for saving and restoring all the PLLs\n>> and clocks context during system suspend and resume to have the\n>> system back to operating state.\n>>\n>> Signed-off-by: Sowjanya Komatineni \n>> ---\n>> drivers/clk/tegra/clk-tegra210.c | 218 +++++++++++++++++++++++++++++++++++++--\n>> 1 file changed, 211 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c\n>> index e1ba62d2b1a0..c34d92e871f4 100644\n>> --- a/drivers/clk/tegra/clk-tegra210.c\n>> +++ b/drivers/clk/tegra/clk-tegra210.c\n>> @@ -9,10 +9,12 @@\n>> #include \n>> #include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> #include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> #include \n>> @@ -20,6 +22,7 @@\n>> #include \n>> \n>> #include \"clk.h\"\n>> +#include \"clk-dfll.h\"\n>> #include \"clk-id.h\"\n>> \n>> /*\n>> @@ -36,6 +39,8 @@\n>> #define CLK_SOURCE_LA 0x1f8\n>> #define CLK_SOURCE_SDMMC2 0x154\n>> #define CLK_SOURCE_SDMMC4 0x164\n>> +#define CLK_OUT_ENB_Y 0x298\n>> +#define CLK_ENB_PLLP_OUT_CPU BIT(31)\n>> \n>> #define PLLC_BASE 0x80\n>> #define PLLC_OUT 0x84\n>> @@ -225,6 +230,7 @@\n>> \n>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8\n>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac\n>> +#define CPU_SOFTRST_CTRL 0x380\n>> \n>> #define LVL2_CLK_GATE_OVRA 0xf8\n>> #define LVL2_CLK_GATE_OVRC 0x3a0\n>> @@ -2820,6 +2826,7 @@ static int tegra210_enable_pllu(void)\n>> \tstruct tegra_clk_pll_freq_table *fentry;\n>> \tstruct tegra_clk_pll pllu;\n>> \tu32 reg;\n>> +\tint ret;\n>> \n>> \tfor (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {\n>> \t\tif (fentry->input_rate == pll_ref_freq)\n>> @@ -2836,7 +2843,7 @@ static int tegra210_enable_pllu(void)\n>> \treg = readl_relaxed(clk_base + pllu.params->ext_misc_reg[0]);\n>> \treg &= ~BIT(pllu.params->iddq_bit_idx);\n>> \twritel_relaxed(reg, clk_base + pllu.params->ext_misc_reg[0]);\n>> -\tudelay(5);\n>> +\tfence_udelay(5, clk_base);\n>> \n>> \treg = readl_relaxed(clk_base + PLLU_BASE);\n>> \treg &= ~GENMASK(20, 0);\n>> @@ -2844,13 +2851,13 @@ static int tegra210_enable_pllu(void)\n>> \treg |= fentry->n << 8;\n>> \treg |= fentry->p << 16;\n>> \twritel(reg, clk_base + PLLU_BASE);\n>> -\tudelay(1);\n>> +\tfence_udelay(1, clk_base);\n> These udelay() -> fence_udelay() seem like they should be a separate\n> patch.\n>\n>> \treg |= PLL_ENABLE;\n>> \twritel(reg, clk_base + PLLU_BASE);\n>> +\tfence_udelay(1, clk_base);\n>> \n>> -\treadl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,\n>> -\t\t\t\t\t reg & PLL_BASE_LOCK, 2, 1000);\n>> -\tif (!(reg & PLL_BASE_LOCK)) {\n>> +\tret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);\n>> +\tif (ret) {\n>> \t\tpr_err(\"Timed out waiting for PLL_U to lock\\n\");\n>> \t\treturn -ETIMEDOUT;\n>> \t}\n>> @@ -2890,12 +2897,12 @@ static int tegra210_init_pllu(void)\n>> \t\treg = readl_relaxed(clk_base + XUSB_PLL_CFG0);\n>> \t\treg &= ~XUSB_PLL_CFG0_PLLU_LOCK_DLY_MASK;\n>> \t\twritel_relaxed(reg, clk_base + XUSB_PLL_CFG0);\n>> -\t\tudelay(1);\n>> +\t\tfence_udelay(1, clk_base);\n>> \n>> \t\treg = readl_relaxed(clk_base + PLLU_HW_PWRDN_CFG0);\n>> \t\treg |= PLLU_HW_PWRDN_CFG0_SEQ_ENABLE;\n>> \t\twritel_relaxed(reg, clk_base + PLLU_HW_PWRDN_CFG0);\n>> -\t\tudelay(1);\n>> +\t\tfence_udelay(1, clk_base);\n>> \n>> \t\treg = readl_relaxed(clk_base + PLLU_BASE);\n>> \t\treg &= ~PLLU_BASE_CLKENABLE_USB;\n>> @@ -3282,6 +3289,188 @@ static void tegra210_disable_cpu_clock(u32 cpu)\n>> }\n>> \n>> #ifdef CONFIG_PM_SLEEP\n>> +static u32 cpu_softrst_ctx[3];\n>> +static struct platform_device *dfll_pdev;\n>> +static u32 *periph_clk_src_ctx;\n>> +struct periph_source_bank {\n> Blank line between the above two.\n>\n>> +\tu32 start;\n>> +\tu32 end;\n>> +};\n>> +\n>> +static struct periph_source_bank periph_srcs[] = {\n>> +\t[0] = {\n>> +\t\t.start = 0x100,\n>> +\t\t.end = 0x198,\n>> +\t},\n>> +\t[1] = {\n>> +\t\t.start = 0x1a0,\n>> +\t\t.end = 0x1f8,\n>> +\t},\n>> +\t[2] = {\n>> +\t\t.start = 0x3b4,\n>> +\t\t.end = 0x42c,\n>> +\t},\n>> +\t[3] = {\n>> +\t\t.start = 0x49c,\n>> +\t\t.end = 0x4b4,\n>> +\t},\n>> +\t[4] = {\n>> +\t\t.start = 0x560,\n>> +\t\t.end = 0x564,\n>> +\t},\n>> +\t[5] = {\n>> +\t\t.start = 0x600,\n>> +\t\t.end = 0x678,\n>> +\t},\n>> +\t[6] = {\n>> +\t\t.start = 0x694,\n>> +\t\t.end = 0x6a0,\n>> +\t},\n>> +\t[7] = {\n>> +\t\t.start = 0x6b8,\n>> +\t\t.end = 0x718,\n>> +\t},\n>> +};\n>> +\n>> +/* This array lists the valid clocks for each periph clk bank */\n>> +static u32 periph_clks_on[] = {\n>> +\t0xdcd7dff9,\n>> +\t0x87d1f3e7,\n>> +\t0xf3fed3fa,\n>> +\t0xffc18cfb,\n>> +\t0x793fb7ff,\n>> +\t0x3fe66fff,\n>> +\t0xfc1fc7ff,\n>> +};\n> Hm... this is a bunch of magic. Perhaps replace this by a list of the\n> clock IDs? That's perhaps a little more verbose, but if we ever need to\n> tweak the list of IDs in that periph_clks_on array, that'll be quite the\n> challenge.\n>\n> Also, is this list a \"guess\" or are these all guaranteed to be always\n> on? What if some of these ended up getting disabled as part of suspend\n> already (by their users). If we force them on, won't their references\n> become unbalanced if the driver later enables them again on resume?\n\nYes, will replace with list of peripheral clock names..\n\nThis list is not a guess. Each entry of this list maps to CLK_ENB set \nregister.\n\nTotal 7 registers are available and each bit of these registers is for \nenable/disable clock to corresponding peripheral.\n\nSome of the bits are off as those peripheral clocks don't need to be \nenabled as we are not changing source or not using them like MIPIBIF, \nPLLG_REF..\n\nThis list of peripheral clocks are enabled during resume before changing \nclock sources and after clock source update, they are restored back to \nthe state they were before suspend. So their references don't become \nunbalanced.\n\n>> +\n>> +static struct platform_device *dfll_pdev;\n> I think you already predeclared this one above.\n>\n>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) + ((_off) * 4))\n>> +#define car_writel(_val, _base, _off) \\\n>> +\t\twritel_relaxed(_val, clk_base + (_base) + ((_off) * 4))\n>> +\n>> +static u32 * __init tegra210_init_suspend_ctx(void)\n>> +{\n>> +\tint i, size = 0;\n> Can both be unsigned int.\n>\n>> +\n>> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n>> +\t\tsize += periph_srcs[i].end - periph_srcs[i].start + 4;\n>> +\n>> +\tperiph_clk_src_ctx = kmalloc(size, GFP_KERNEL);\n>> +\n>> +\treturn periph_clk_src_ctx;\n> It's somewhat wasteful to return a global variable since you can access\n> it anyway. Perhaps it'd be more useful to make the function return a\n> boolean?\n>\n>> +}\n>> +\n>> +static int tegra210_clk_suspend(void)\n>> +{\n>> +\tint i;\n> unsigned int.\n>\n>> +\tunsigned long off;\n>> +\tstruct device_node *node;\n>> +\tu32 *clk_rst_ctx = periph_clk_src_ctx;\n>> +\tu32 val;\n>> +\n>> +\ttegra_cclkg_burst_policy_save_context();\n>> +\n>> +\tif (!dfll_pdev) {\n>> +\t\tnode = of_find_compatible_node(NULL, NULL,\n>> +\t\t\t\t\t \"nvidia,tegra210-dfll\");\n>> +\t\tif (node)\n>> +\t\t\tdfll_pdev = of_find_device_by_node(node);\n>> +\t\tof_node_put(node);\n>> +\t\tif (!dfll_pdev)\n>> +\t\t\tpr_err(\"dfll node not found. no suspend for dfll\\n\");\n>> +\t}\n> Wouldn't it make sense to run this only once, perhaps as part of\n> tegra210_init_suspend_ctx()?\n>\n>> +\n>> +\tif (dfll_pdev)\n>> +\t\ttegra_dfll_suspend(dfll_pdev);\n>> +\n>> +\t/* Enable PLLP_OUT_CPU after dfll suspend */\n>> +\tval = car_readl(CLK_OUT_ENB_Y, 0);\n>> +\tval |= CLK_ENB_PLLP_OUT_CPU;\n>> +\tcar_writel(val, CLK_OUT_ENB_Y, 0);\n>> +\n>> +\ttegra_clk_periph_suspend(clk_base);\n>> +\n>> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n>> +\t\tfor (off = periph_srcs[i].start; off <= periph_srcs[i].end;\n>> +\t\t off += 4)\n>> +\t\t\t*clk_rst_ctx++ = car_readl(off, 0);\n>> +\n>> +\ttegra_sclk_cclklp_burst_policy_save_context();\n>> +\n>> +\tfor (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)\n>> +\t\tcpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);\n>> +\n>> +\tclk_save_context();\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +static void tegra210_clk_resume(void)\n>> +{\n>> +\tint i;\n>> +\tunsigned long off;\n>> +\tu32 val;\n>> +\tu32 *clk_rst_ctx = periph_clk_src_ctx;\n>> +\tstruct clk_hw *parent;\n>> +\tstruct clk *clk;\n>> +\n>> +\tfor (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)\n>> +\t\tcar_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);\n>> +\n>> +\ttegra_clk_osc_resume(clk_base);\n>> +\n>> +\t/*\n>> +\t * restore all the plls before configuring clocks and resetting\n>> +\t * the devices.\n>> +\t */\n>> +\ttegra210_init_pllu();\n>> +\ttegra_sclk_cpulp_burst_policy_restore_context();\n>> +\tclk_restore_context();\n>> +\n>> +\t/* enable all clocks before configuring clock sources */\n>> +\ttegra_clk_periph_force_on(periph_clks_on, ARRAY_SIZE(periph_clks_on),\n>> +\t\t\t\t clk_base);\n>> +\t/* wait for all writes to happen to have all the clocks enabled */\n>> +\twmb();\n>> +\tfence_udelay(2, clk_base);\n>> +\n>> +\t/* restore all the devices clock sources */\n>> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n>> +\t\tfor (off = periph_srcs[i].start; off <= periph_srcs[i].end;\n>> +\t\t off += 4)\n>> +\t\t\tcar_writel(*clk_rst_ctx++, off, 0);\n>> +\n>> +\t/* propagate and restore resets, restore clock state */\n>> +\tfence_udelay(5, clk_base);\n>> +\ttegra_clk_periph_resume(clk_base);\n>> +\n>> +\t/*\n>> +\t * restore CPUG clocks:\n>> +\t * - enable DFLL in open loop mode\n>> +\t * - switch CPUG to DFLL clock source\n>> +\t * - close DFLL loop\n>> +\t * - sync PLLX state\n>> +\t */\n>> +\tif (dfll_pdev)\n>> +\t\ttegra_dfll_resume(dfll_pdev, false);\n>> +\n>> +\ttegra_cclkg_burst_policy_restore_context();\n>> +\tfence_udelay(2, clk_base);\n>> +\n>> +\tif (dfll_pdev)\n>> +\t\ttegra_dfll_resume(dfll_pdev, true);\n>> +\n>> +\tparent = clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));\n>> +\tclk = clks[TEGRA210_CLK_PLL_X];\n>> +\tif (parent != __clk_get_hw(clk))\n>> +\t\ttegra_clk_sync_state_pll(__clk_get_hw(clk));\n>> +\n>> +\t/* Disable PLL_OUT_CPU after DFLL resume */\n>> +\tval = car_readl(CLK_OUT_ENB_Y, 0);\n>> +\tval &= ~CLK_ENB_PLLP_OUT_CPU;\n>> +\tcar_writel(val, CLK_OUT_ENB_Y, 0);\n>> +}\n> I'm surprised by the amount of work that we need to do here. I had hoped\n> that the clock framework's save/restore infrastructure would be enough.\n> I suppose you do call clk_restore_context() somewhere in there, so maybe\n> this really is as good as it gets.\n>\n> Thierry\n\nReason is there are dependencies b/w the clocks and DFLL resume and \nclocks resume order needed is not same as clock tree list.\n\nduring resume as per clock tree, CPU clock configs to use DFLL will \nhappen first as its first in the clock tree but DFLL resume should be \ndone prior to switching CPU to use from DFLL output.\n\nTo resume DFLL, peripheral clocks should be restored.\n\nConsidering these dependencies, performing peripheral and DFLL/CPU \nresume in Tegra210 clock driver rather than in corresponding peripheral \nclk_ops using save and restore context callback.\n\n>> +\n>> static void tegra210_cpu_clock_suspend(void)\n>> {\n>> \t/* switch coresite to clk_m, save off original source */\n>> @@ -3295,8 +3484,20 @@ static void tegra210_cpu_clock_resume(void)\n>> \twritel(tegra210_cpu_clk_sctx.clk_csite_src,\n>> \t\t\t\tclk_base + CLK_SOURCE_CSITE);\n>> }\n>> +#else\n>> +#define tegra210_clk_suspend\tNULL\n>> +#define tegra210_clk_resume\tNULL\n>> +static inline u32 *tegra210_init_suspend_ctx(void)\n>> +{\n>> +\treturn NULL;\n>> +}\n>> #endif\n>> \n>> +static struct syscore_ops tegra_clk_syscore_ops = {\n>> +\t.suspend = tegra210_clk_suspend,\n>> +\t.resume = tegra210_clk_resume,\n>> +};\n>> +\n>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {\n>> \t.wait_for_reset\t= tegra210_wait_cpu_in_reset,\n>> \t.disable_clock\t= tegra210_disable_cpu_clock,\n>> @@ -3580,5 +3781,8 @@ static void __init tegra210_clock_init(struct device_node *np)\n>> \ttegra210_mbist_clk_init();\n>> \n>> \ttegra_cpu_car_ops = &tegra210_cpu_car_ops;\n>> +\n>> +\tif (tegra210_init_suspend_ctx())\n>> +\t\tregister_syscore_ops(&tegra_clk_syscore_ops);\n>> }\n>> CLK_OF_DECLARE(tegra210, \"nvidia,tegra210-car\", tegra210_clock_init);\n>> -- \n>> 2.7.4\n>>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdmarc=pass (p=none dis=none) header.from=nvidia.com","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=nvidia.com header.i=@nvidia.com\n\theader.b=\"jR8Wquhk\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 45Swn23H2Fz9sN6\n\tfor ;\n\tWed, 19 Jun 2019 03:58:46 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1729681AbfFRR6p (ORCPT );\n\tTue, 18 Jun 2019 13:58:45 -0400","from hqemgate15.nvidia.com ([216.228.121.64]:12210 \"EHLO\n\thqemgate15.nvidia.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1729349AbfFRR6o (ORCPT\n\t); Tue, 18 Jun 2019 13:58:44 -0400","from hqpgpgate102.nvidia.com (Not Verified[216.228.121.13]) by\n\thqemgate15.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA)\n\tid ; Tue, 18 Jun 2019 10:58:43 -0700","from hqmail.nvidia.com ([172.20.161.6])\n\tby hqpgpgate102.nvidia.com (PGP Universal service);\n\tTue, 18 Jun 2019 10:58:42 -0700","from [10.2.168.217] (172.20.13.39) by HQMAIL107.nvidia.com\n\t(172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3;\n\tTue, 18 Jun 2019 17:58:39 +0000"],"X-PGP-Universal":"processed;\n\tby hqpgpgate102.nvidia.com on Tue, 18 Jun 2019 10:58:42 -0700","Subject":"Re: [PATCH V3 11/17] clk: tegra210: support for Tegra210 clocks\n\tsuspend and resume","To":"Thierry Reding ","CC":", ,\n\t, ,\n\t, ,\n\t, ,\n\t, ,\n\t, ,\n\t, , ,\n\t, ,\n\t, , ,\n\t, ","References":"<1560843991-24123-1-git-send-email-skomatineni@nvidia.com>\n\t<1560843991-24123-12-git-send-email-skomatineni@nvidia.com>\n\t<20190618121607.GN28892@ulmo>","From":"Sowjanya Komatineni ","Message-ID":"<491e0b18-11e7-837c-4591-06ed30950e1d@nvidia.com>","Date":"Tue, 18 Jun 2019 10:58:40 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190618121607.GN28892@ulmo>","X-Originating-IP":"[172.20.13.39]","X-ClientProxiedBy":"HQMAIL103.nvidia.com (172.20.187.11) To\n\tHQMAIL107.nvidia.com (172.20.187.13)","Content-Type":"text/plain; charset=\"windows-1252\"; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;\n\tt=1560880723; bh=EZsFDs+jzXTMfQC/Kn7gXDe1CR7sMkPUl6QccPbfO5Y=;\n\th=X-PGP-Universal:Subject:To:CC:References:From:Message-ID:Date:\n\tUser-Agent:MIME-Version:In-Reply-To:X-Originating-IP:\n\tX-ClientProxiedBy:Content-Type:Content-Transfer-Encoding:\n\tContent-Language;\n\tb=jR8WquhkrocPgWQ1vEIvs1EDxkSv75AdwJyVBJNuPl1kC/9pzufogNsvaaseFu7WD\n\t6AxOrtTWac3gf3FPbMrQJM/TIAMHrfWa6R+NwDfzfMr3wvBnbjaMgu1jE6pMJdbSxG\n\t4FknJ8+dScDMNzSsTlfiWeHYe1b0yZliwz2su55whwpOibpzMm3n1eDeqENGIGzAWS\n\tyOh6Yzk9MiXrTvgPaQUdUWXdWulqy1l64rx+I9yW8Rzzu5CvKiqozOG6wCU3m4bMWd\n\t63tg84riQcuQ9oTDCss8b8vS5417RULIb97BfxBjrdAcf2YfvgEUSl5w7+gWMGBGW2\n\t4NGHTJ96N+8ZQ==","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":2197463,"web_url":"http://patchwork.ozlabs.org/comment/2197463/","msgid":"<20190619081541.GA3187@ulmo>","list_archive_url":null,"date":"2019-06-19T08:15:41","subject":"Re: [PATCH V3 11/17] clk: tegra210: support for Tegra210 clocks\n\tsuspend and resume","submitter":{"id":26234,"url":"http://patchwork.ozlabs.org/api/people/26234/","name":"Thierry Reding","email":"thierry.reding@gmail.com"},"content":"On Tue, Jun 18, 2019 at 10:58:40AM -0700, Sowjanya Komatineni wrote:\n> \n> On 6/18/19 5:16 AM, Thierry Reding wrote:\n> > On Tue, Jun 18, 2019 at 12:46:25AM -0700, Sowjanya Komatineni wrote:\n> > > This patch adds system suspend and resume support for Tegra210\n> > > clocks.\n> > > \n> > > All the CAR controller settings are lost on suspend when core power\n> > > goes off.\n> > > \n> > > This patch has implementation for saving and restoring all the PLLs\n> > > and clocks context during system suspend and resume to have the\n> > > system back to operating state.\n> > > \n> > > Signed-off-by: Sowjanya Komatineni \n> > > ---\n> > > drivers/clk/tegra/clk-tegra210.c | 218 +++++++++++++++++++++++++++++++++++++--\n> > > 1 file changed, 211 insertions(+), 7 deletions(-)\n> > > \n> > > diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c\n> > > index e1ba62d2b1a0..c34d92e871f4 100644\n> > > --- a/drivers/clk/tegra/clk-tegra210.c\n> > > +++ b/drivers/clk/tegra/clk-tegra210.c\n> > > @@ -9,10 +9,12 @@\n> > > #include \n> > > #include \n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -20,6 +22,7 @@\n> > > #include \n> > > #include \"clk.h\"\n> > > +#include \"clk-dfll.h\"\n> > > #include \"clk-id.h\"\n> > > /*\n> > > @@ -36,6 +39,8 @@\n> > > #define CLK_SOURCE_LA 0x1f8\n> > > #define CLK_SOURCE_SDMMC2 0x154\n> > > #define CLK_SOURCE_SDMMC4 0x164\n> > > +#define CLK_OUT_ENB_Y 0x298\n> > > +#define CLK_ENB_PLLP_OUT_CPU BIT(31)\n> > > #define PLLC_BASE 0x80\n> > > #define PLLC_OUT 0x84\n> > > @@ -225,6 +230,7 @@\n> > > #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8\n> > > #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac\n> > > +#define CPU_SOFTRST_CTRL 0x380\n> > > #define LVL2_CLK_GATE_OVRA 0xf8\n> > > #define LVL2_CLK_GATE_OVRC 0x3a0\n> > > @@ -2820,6 +2826,7 @@ static int tegra210_enable_pllu(void)\n> > > \tstruct tegra_clk_pll_freq_table *fentry;\n> > > \tstruct tegra_clk_pll pllu;\n> > > \tu32 reg;\n> > > +\tint ret;\n> > > \tfor (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {\n> > > \t\tif (fentry->input_rate == pll_ref_freq)\n> > > @@ -2836,7 +2843,7 @@ static int tegra210_enable_pllu(void)\n> > > \treg = readl_relaxed(clk_base + pllu.params->ext_misc_reg[0]);\n> > > \treg &= ~BIT(pllu.params->iddq_bit_idx);\n> > > \twritel_relaxed(reg, clk_base + pllu.params->ext_misc_reg[0]);\n> > > -\tudelay(5);\n> > > +\tfence_udelay(5, clk_base);\n> > > \treg = readl_relaxed(clk_base + PLLU_BASE);\n> > > \treg &= ~GENMASK(20, 0);\n> > > @@ -2844,13 +2851,13 @@ static int tegra210_enable_pllu(void)\n> > > \treg |= fentry->n << 8;\n> > > \treg |= fentry->p << 16;\n> > > \twritel(reg, clk_base + PLLU_BASE);\n> > > -\tudelay(1);\n> > > +\tfence_udelay(1, clk_base);\n> > These udelay() -> fence_udelay() seem like they should be a separate\n> > patch.\n> > \n> > > \treg |= PLL_ENABLE;\n> > > \twritel(reg, clk_base + PLLU_BASE);\n> > > +\tfence_udelay(1, clk_base);\n> > > -\treadl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,\n> > > -\t\t\t\t\t reg & PLL_BASE_LOCK, 2, 1000);\n> > > -\tif (!(reg & PLL_BASE_LOCK)) {\n> > > +\tret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);\n> > > +\tif (ret) {\n> > > \t\tpr_err(\"Timed out waiting for PLL_U to lock\\n\");\n> > > \t\treturn -ETIMEDOUT;\n> > > \t}\n> > > @@ -2890,12 +2897,12 @@ static int tegra210_init_pllu(void)\n> > > \t\treg = readl_relaxed(clk_base + XUSB_PLL_CFG0);\n> > > \t\treg &= ~XUSB_PLL_CFG0_PLLU_LOCK_DLY_MASK;\n> > > \t\twritel_relaxed(reg, clk_base + XUSB_PLL_CFG0);\n> > > -\t\tudelay(1);\n> > > +\t\tfence_udelay(1, clk_base);\n> > > \t\treg = readl_relaxed(clk_base + PLLU_HW_PWRDN_CFG0);\n> > > \t\treg |= PLLU_HW_PWRDN_CFG0_SEQ_ENABLE;\n> > > \t\twritel_relaxed(reg, clk_base + PLLU_HW_PWRDN_CFG0);\n> > > -\t\tudelay(1);\n> > > +\t\tfence_udelay(1, clk_base);\n> > > \t\treg = readl_relaxed(clk_base + PLLU_BASE);\n> > > \t\treg &= ~PLLU_BASE_CLKENABLE_USB;\n> > > @@ -3282,6 +3289,188 @@ static void tegra210_disable_cpu_clock(u32 cpu)\n> > > }\n> > > #ifdef CONFIG_PM_SLEEP\n> > > +static u32 cpu_softrst_ctx[3];\n> > > +static struct platform_device *dfll_pdev;\n> > > +static u32 *periph_clk_src_ctx;\n> > > +struct periph_source_bank {\n> > Blank line between the above two.\n> > \n> > > +\tu32 start;\n> > > +\tu32 end;\n> > > +};\n> > > +\n> > > +static struct periph_source_bank periph_srcs[] = {\n> > > +\t[0] = {\n> > > +\t\t.start = 0x100,\n> > > +\t\t.end = 0x198,\n> > > +\t},\n> > > +\t[1] = {\n> > > +\t\t.start = 0x1a0,\n> > > +\t\t.end = 0x1f8,\n> > > +\t},\n> > > +\t[2] = {\n> > > +\t\t.start = 0x3b4,\n> > > +\t\t.end = 0x42c,\n> > > +\t},\n> > > +\t[3] = {\n> > > +\t\t.start = 0x49c,\n> > > +\t\t.end = 0x4b4,\n> > > +\t},\n> > > +\t[4] = {\n> > > +\t\t.start = 0x560,\n> > > +\t\t.end = 0x564,\n> > > +\t},\n> > > +\t[5] = {\n> > > +\t\t.start = 0x600,\n> > > +\t\t.end = 0x678,\n> > > +\t},\n> > > +\t[6] = {\n> > > +\t\t.start = 0x694,\n> > > +\t\t.end = 0x6a0,\n> > > +\t},\n> > > +\t[7] = {\n> > > +\t\t.start = 0x6b8,\n> > > +\t\t.end = 0x718,\n> > > +\t},\n> > > +};\n> > > +\n> > > +/* This array lists the valid clocks for each periph clk bank */\n> > > +static u32 periph_clks_on[] = {\n> > > +\t0xdcd7dff9,\n> > > +\t0x87d1f3e7,\n> > > +\t0xf3fed3fa,\n> > > +\t0xffc18cfb,\n> > > +\t0x793fb7ff,\n> > > +\t0x3fe66fff,\n> > > +\t0xfc1fc7ff,\n> > > +};\n> > Hm... this is a bunch of magic. Perhaps replace this by a list of the\n> > clock IDs? That's perhaps a little more verbose, but if we ever need to\n> > tweak the list of IDs in that periph_clks_on array, that'll be quite the\n> > challenge.\n> > \n> > Also, is this list a \"guess\" or are these all guaranteed to be always\n> > on? What if some of these ended up getting disabled as part of suspend\n> > already (by their users). If we force them on, won't their references\n> > become unbalanced if the driver later enables them again on resume?\n> \n> Yes, will replace with list of peripheral clock names..\n> \n> This list is not a guess. Each entry of this list maps to CLK_ENB set\n> register.\n> \n> Total 7 registers are available and each bit of these registers is for\n> enable/disable clock to corresponding peripheral.\n> \n> Some of the bits are off as those peripheral clocks don't need to be enabled\n> as we are not changing source or not using them like MIPIBIF, PLLG_REF..\n> \n> This list of peripheral clocks are enabled during resume before changing\n> clock sources and after clock source update, they are restored back to the\n> state they were before suspend. So their references don't become unbalanced.\n\nOkay, good. Can you maybe put a version of that explanation in a comment\non top of the periph_clks_on declaration? And perhaps also describe this\nin the commit message.\n\nOr maybe even better, add some comments in the main suspend/resume paths\nto sort of \"guide\" through what's happening.\n\n> > > +\n> > > +static struct platform_device *dfll_pdev;\n> > I think you already predeclared this one above.\n> > \n> > > +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) + ((_off) * 4))\n> > > +#define car_writel(_val, _base, _off) \\\n> > > +\t\twritel_relaxed(_val, clk_base + (_base) + ((_off) * 4))\n> > > +\n> > > +static u32 * __init tegra210_init_suspend_ctx(void)\n> > > +{\n> > > +\tint i, size = 0;\n> > Can both be unsigned int.\n> > \n> > > +\n> > > +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n> > > +\t\tsize += periph_srcs[i].end - periph_srcs[i].start + 4;\n> > > +\n> > > +\tperiph_clk_src_ctx = kmalloc(size, GFP_KERNEL);\n> > > +\n> > > +\treturn periph_clk_src_ctx;\n> > It's somewhat wasteful to return a global variable since you can access\n> > it anyway. Perhaps it'd be more useful to make the function return a\n> > boolean?\n> > \n> > > +}\n> > > +\n> > > +static int tegra210_clk_suspend(void)\n> > > +{\n> > > +\tint i;\n> > unsigned int.\n> > \n> > > +\tunsigned long off;\n> > > +\tstruct device_node *node;\n> > > +\tu32 *clk_rst_ctx = periph_clk_src_ctx;\n> > > +\tu32 val;\n> > > +\n> > > +\ttegra_cclkg_burst_policy_save_context();\n> > > +\n> > > +\tif (!dfll_pdev) {\n> > > +\t\tnode = of_find_compatible_node(NULL, NULL,\n> > > +\t\t\t\t\t \"nvidia,tegra210-dfll\");\n> > > +\t\tif (node)\n> > > +\t\t\tdfll_pdev = of_find_device_by_node(node);\n> > > +\t\tof_node_put(node);\n> > > +\t\tif (!dfll_pdev)\n> > > +\t\t\tpr_err(\"dfll node not found. no suspend for dfll\\n\");\n> > > +\t}\n> > Wouldn't it make sense to run this only once, perhaps as part of\n> > tegra210_init_suspend_ctx()?\n> > \n> > > +\n> > > +\tif (dfll_pdev)\n> > > +\t\ttegra_dfll_suspend(dfll_pdev);\n> > > +\n> > > +\t/* Enable PLLP_OUT_CPU after dfll suspend */\n> > > +\tval = car_readl(CLK_OUT_ENB_Y, 0);\n> > > +\tval |= CLK_ENB_PLLP_OUT_CPU;\n> > > +\tcar_writel(val, CLK_OUT_ENB_Y, 0);\n> > > +\n> > > +\ttegra_clk_periph_suspend(clk_base);\n> > > +\n> > > +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n> > > +\t\tfor (off = periph_srcs[i].start; off <= periph_srcs[i].end;\n> > > +\t\t off += 4)\n> > > +\t\t\t*clk_rst_ctx++ = car_readl(off, 0);\n> > > +\n> > > +\ttegra_sclk_cclklp_burst_policy_save_context();\n> > > +\n> > > +\tfor (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)\n> > > +\t\tcpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);\n> > > +\n> > > +\tclk_save_context();\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +static void tegra210_clk_resume(void)\n> > > +{\n> > > +\tint i;\n> > > +\tunsigned long off;\n> > > +\tu32 val;\n> > > +\tu32 *clk_rst_ctx = periph_clk_src_ctx;\n> > > +\tstruct clk_hw *parent;\n> > > +\tstruct clk *clk;\n> > > +\n> > > +\tfor (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)\n> > > +\t\tcar_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);\n> > > +\n> > > +\ttegra_clk_osc_resume(clk_base);\n> > > +\n> > > +\t/*\n> > > +\t * restore all the plls before configuring clocks and resetting\n> > > +\t * the devices.\n> > > +\t */\n> > > +\ttegra210_init_pllu();\n> > > +\ttegra_sclk_cpulp_burst_policy_restore_context();\n> > > +\tclk_restore_context();\n> > > +\n> > > +\t/* enable all clocks before configuring clock sources */\n> > > +\ttegra_clk_periph_force_on(periph_clks_on, ARRAY_SIZE(periph_clks_on),\n> > > +\t\t\t\t clk_base);\n> > > +\t/* wait for all writes to happen to have all the clocks enabled */\n> > > +\twmb();\n> > > +\tfence_udelay(2, clk_base);\n> > > +\n> > > +\t/* restore all the devices clock sources */\n> > > +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n> > > +\t\tfor (off = periph_srcs[i].start; off <= periph_srcs[i].end;\n> > > +\t\t off += 4)\n> > > +\t\t\tcar_writel(*clk_rst_ctx++, off, 0);\n> > > +\n> > > +\t/* propagate and restore resets, restore clock state */\n> > > +\tfence_udelay(5, clk_base);\n> > > +\ttegra_clk_periph_resume(clk_base);\n> > > +\n> > > +\t/*\n> > > +\t * restore CPUG clocks:\n> > > +\t * - enable DFLL in open loop mode\n> > > +\t * - switch CPUG to DFLL clock source\n> > > +\t * - close DFLL loop\n> > > +\t * - sync PLLX state\n> > > +\t */\n> > > +\tif (dfll_pdev)\n> > > +\t\ttegra_dfll_resume(dfll_pdev, false);\n> > > +\n> > > +\ttegra_cclkg_burst_policy_restore_context();\n> > > +\tfence_udelay(2, clk_base);\n> > > +\n> > > +\tif (dfll_pdev)\n> > > +\t\ttegra_dfll_resume(dfll_pdev, true);\n> > > +\n> > > +\tparent = clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));\n> > > +\tclk = clks[TEGRA210_CLK_PLL_X];\n> > > +\tif (parent != __clk_get_hw(clk))\n> > > +\t\ttegra_clk_sync_state_pll(__clk_get_hw(clk));\n> > > +\n> > > +\t/* Disable PLL_OUT_CPU after DFLL resume */\n> > > +\tval = car_readl(CLK_OUT_ENB_Y, 0);\n> > > +\tval &= ~CLK_ENB_PLLP_OUT_CPU;\n> > > +\tcar_writel(val, CLK_OUT_ENB_Y, 0);\n> > > +}\n> > I'm surprised by the amount of work that we need to do here. I had hoped\n> > that the clock framework's save/restore infrastructure would be enough.\n> > I suppose you do call clk_restore_context() somewhere in there, so maybe\n> > this really is as good as it gets.\n> > \n> > Thierry\n> \n> Reason is there are dependencies b/w the clocks and DFLL resume and clocks\n> resume order needed is not same as clock tree list.\n> \n> during resume as per clock tree, CPU clock configs to use DFLL will happen\n> first as its first in the clock tree but DFLL resume should be done prior to\n> switching CPU to use from DFLL output.\n> \n> To resume DFLL, peripheral clocks should be restored.\n> \n> Considering these dependencies, performing peripheral and DFLL/CPU resume in\n> Tegra210 clock driver rather than in corresponding peripheral clk_ops using\n> save and restore context callback.\n\nOkay makes sense. As mentioned above, I think it'd be great if you could\nadd more comments throughout the tegra210_clk_{suspend,resume}() code to\nguide the reader through what you're doing, given that this is far from\nobvious. You already do quite a bit of that, but it's perhaps better to\nexplain more what's going on and, perhaps more importantly, why. You're\ncurrently mostly repeating the code sequence in the code. It'd be great\nto have the general suspend/resume sequence detailed and highlight why\nthe sequence is the way it is and what the dependencies are, etc.\n\nThierry\n\n> \n> > > +\n> > > static void tegra210_cpu_clock_suspend(void)\n> > > {\n> > > \t/* switch coresite to clk_m, save off original source */\n> > > @@ -3295,8 +3484,20 @@ static void tegra210_cpu_clock_resume(void)\n> > > \twritel(tegra210_cpu_clk_sctx.clk_csite_src,\n> > > \t\t\t\tclk_base + CLK_SOURCE_CSITE);\n> > > }\n> > > +#else\n> > > +#define tegra210_clk_suspend\tNULL\n> > > +#define tegra210_clk_resume\tNULL\n> > > +static inline u32 *tegra210_init_suspend_ctx(void)\n> > > +{\n> > > +\treturn NULL;\n> > > +}\n> > > #endif\n> > > +static struct syscore_ops tegra_clk_syscore_ops = {\n> > > +\t.suspend = tegra210_clk_suspend,\n> > > +\t.resume = tegra210_clk_resume,\n> > > +};\n> > > +\n> > > static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {\n> > > \t.wait_for_reset\t= tegra210_wait_cpu_in_reset,\n> > > \t.disable_clock\t= tegra210_disable_cpu_clock,\n> > > @@ -3580,5 +3781,8 @@ static void __init tegra210_clock_init(struct device_node *np)\n> > > \ttegra210_mbist_clk_init();\n> > > \ttegra_cpu_car_ops = &tegra210_cpu_car_ops;\n> > > +\n> > > +\tif (tegra210_init_suspend_ctx())\n> > > +\t\tregister_syscore_ops(&tegra_clk_syscore_ops);\n> > > }\n> > > CLK_OF_DECLARE(tegra210, \"nvidia,tegra210-car\", tegra210_clock_init);\n> > > -- \n> > > 2.7.4\n> > >","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdmarc=pass (p=none dis=none) header.from=gmail.com","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"jX/dONXR\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 45THp21pdNz9sNR\n\tfor ;\n\tWed, 19 Jun 2019 18:15:54 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1731065AbfFSIPs (ORCPT );\n\tWed, 19 Jun 2019 04:15:48 -0400","from mail-wm1-f67.google.com ([209.85.128.67]:38855 \"EHLO\n\tmail-wm1-f67.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1731143AbfFSIPs (ORCPT\n\t); Wed, 19 Jun 2019 04:15:48 -0400","by mail-wm1-f67.google.com with SMTP id s15so732994wmj.3;\n\tWed, 19 Jun 2019 01:15:44 -0700 (PDT)","from localhost (p2E5BEF36.dip0.t-ipconnect.de. [46.91.239.54])\n\tby smtp.gmail.com with ESMTPSA id\n\ti25sm5198463wrc.91.2019.06.19.01.15.42\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tWed, 19 Jun 2019 01:15:42 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=ed+Vt6/WmkspgVX9Zm8df5w8dbSgvZ0uQmuzt2cB8Hc=;\n\tb=jX/dONXROXY7vDF0QBrC172jwPTV/xYyVIgj7bux7d/TDg7jawmlypF+6M+FGSxt4L\n\tODv0IjbiERq1euIZvVBYYeeQGyFGO4ZPYCl94HPPsypRUUxVi3fNiXfoSWLokkdHyGfo\n\tLNPv6UTVslWpAYoFMpiE5v7GSOoNNkkmaMNEJbPEemujd0uxElFEcRSyPIm6YF9zVHGT\n\tGvdbT/4+xBwSefA1A+pNh90kZRgZntnLeR4gmAr9jJ4I3UyxxlfhLoA7uRWMbJE5HuBI\n\t/iGGCq5regLKGwwXnW3iKoYWtr+SYH7jRmCQcci/ewk8Qardib6ruo+rs5LY8qWlLs0A\n\tEnGQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=ed+Vt6/WmkspgVX9Zm8df5w8dbSgvZ0uQmuzt2cB8Hc=;\n\tb=LHBqkHFONSbaDunSl1PSq/a4hZPB5sULJORvyWHT5FvRCCVpvNIil3lNtnDbI6Yw05\n\t0RUxKUX3N/A3DxcQprF+GHNfBzh6RGzIU0TIhGDU53IulcxwmVqUsHEelW1rEeNIgbFU\n\tybcn3bQBGaGhN4zZykFZcp2IAL6+EO4gut6Sq1r9Sk6rcHKLrBRr2Lf2Jt85GpBQPxkb\n\tBe17bPkEWDrk0f/qTgulXMVj9N39KyJ7SymfAte3spm8tNIqajhPOISoG3zA9wInfkrK\n\t0TbniLDcQNpCPz40HFZ/CWmYf7eL/MS05qMvCOWjgzal+4/AU4igLo+cazuW6rn4dAqu\n\tBMkA==","X-Gm-Message-State":"APjAAAUmWFAU4hYloY96rEK+r7Ba8rB6jd0SM1baD4eE+gS99mX6AU94\n\tGKd6PtN4FeVNJozkv7Jqnm6MxKbIZ48=","X-Google-Smtp-Source":"APXvYqzv+OMtKLeNVmr21mIQM6BLBXL4ULa9B3eHI8ETbfjXJ0PBcSYB0bSti23HD29FK/A0mLws+A==","X-Received":"by 2002:a1c:f515:: with SMTP id t21mr7475327wmh.39.1560932143750;\n\tWed, 19 Jun 2019 01:15:43 -0700 (PDT)","Date":"Wed, 19 Jun 2019 10:15:41 +0200","From":"Thierry Reding ","To":"Sowjanya Komatineni ","Cc":"jonathanh@nvidia.com, tglx@linutronix.de, jason@lakedaemon.net,\n\tmarc.zyngier@arm.com, linus.walleij@linaro.org, stefan@agner.ch,\n\tmark.rutland@arm.com, pdeschrijver@nvidia.com, pgaikwad@nvidia.com,\n\tsboyd@kernel.org, linux-clk@vger.kernel.org,\n\tlinux-gpio@vger.kernel.org, jckuo@nvidia.com, josephl@nvidia.com,\n\ttalho@nvidia.com, linux-tegra@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, mperttunen@nvidia.com,\n\tspatra@nvidia.com, robh+dt@kernel.org, digetx@gmail.com,\n\tdevicetree@vger.kernel.org","Subject":"Re: [PATCH V3 11/17] clk: tegra210: support for Tegra210 clocks\n\tsuspend and resume","Message-ID":"<20190619081541.GA3187@ulmo>","References":"<1560843991-24123-1-git-send-email-skomatineni@nvidia.com>\n\t<1560843991-24123-12-git-send-email-skomatineni@nvidia.com>\n\t<20190618121607.GN28892@ulmo>\n\t<491e0b18-11e7-837c-4591-06ed30950e1d@nvidia.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"bg08WKrSYDhXBjb5\"","Content-Disposition":"inline","In-Reply-To":"<491e0b18-11e7-837c-4591-06ed30950e1d@nvidia.com>","User-Agent":"Mutt/1.11.4 (2019-03-13)","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":2199965,"web_url":"http://patchwork.ozlabs.org/comment/2199965/","msgid":"<467eec6e-87fd-0b59-f2f6-75eae4a15a34@nvidia.com>","list_archive_url":null,"date":"2019-06-21T20:44:23","subject":"Re: [PATCH V3 11/17] clk: tegra210: support for Tegra210 clocks\n\tsuspend and resume","submitter":{"id":75554,"url":"http://patchwork.ozlabs.org/api/people/75554/","name":"Sowjanya Komatineni","email":"skomatineni@nvidia.com"},"content":"On 6/19/19 1:15 AM, Thierry Reding wrote:\n> On Tue, Jun 18, 2019 at 10:58:40AM -0700, Sowjanya Komatineni wrote:\n>> On 6/18/19 5:16 AM, Thierry Reding wrote:\n>>> On Tue, Jun 18, 2019 at 12:46:25AM -0700, Sowjanya Komatineni wrote:\n>>>> This patch adds system suspend and resume support for Tegra210\n>>>> clocks.\n>>>>\n>>>> All the CAR controller settings are lost on suspend when core power\n>>>> goes off.\n>>>>\n>>>> This patch has implementation for saving and restoring all the PLLs\n>>>> and clocks context during system suspend and resume to have the\n>>>> system back to operating state.\n>>>>\n>>>> Signed-off-by: Sowjanya Komatineni \n>>>> ---\n>>>> drivers/clk/tegra/clk-tegra210.c | 218 +++++++++++++++++++++++++++++++++++++--\n>>>> 1 file changed, 211 insertions(+), 7 deletions(-)\n>>>>\n>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c\n>>>> index e1ba62d2b1a0..c34d92e871f4 100644\n>>>> --- a/drivers/clk/tegra/clk-tegra210.c\n>>>> +++ b/drivers/clk/tegra/clk-tegra210.c\n>>>> @@ -9,10 +9,12 @@\n>>>> #include \n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> #include \n>>>> #include \n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> #include \n>>>> #include \n>>>> #include \n>>>> @@ -20,6 +22,7 @@\n>>>> #include \n>>>> #include \"clk.h\"\n>>>> +#include \"clk-dfll.h\"\n>>>> #include \"clk-id.h\"\n>>>> /*\n>>>> @@ -36,6 +39,8 @@\n>>>> #define CLK_SOURCE_LA 0x1f8\n>>>> #define CLK_SOURCE_SDMMC2 0x154\n>>>> #define CLK_SOURCE_SDMMC4 0x164\n>>>> +#define CLK_OUT_ENB_Y 0x298\n>>>> +#define CLK_ENB_PLLP_OUT_CPU BIT(31)\n>>>> #define PLLC_BASE 0x80\n>>>> #define PLLC_OUT 0x84\n>>>> @@ -225,6 +230,7 @@\n>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8\n>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac\n>>>> +#define CPU_SOFTRST_CTRL 0x380\n>>>> #define LVL2_CLK_GATE_OVRA 0xf8\n>>>> #define LVL2_CLK_GATE_OVRC 0x3a0\n>>>> @@ -2820,6 +2826,7 @@ static int tegra210_enable_pllu(void)\n>>>> \tstruct tegra_clk_pll_freq_table *fentry;\n>>>> \tstruct tegra_clk_pll pllu;\n>>>> \tu32 reg;\n>>>> +\tint ret;\n>>>> \tfor (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {\n>>>> \t\tif (fentry->input_rate == pll_ref_freq)\n>>>> @@ -2836,7 +2843,7 @@ static int tegra210_enable_pllu(void)\n>>>> \treg = readl_relaxed(clk_base + pllu.params->ext_misc_reg[0]);\n>>>> \treg &= ~BIT(pllu.params->iddq_bit_idx);\n>>>> \twritel_relaxed(reg, clk_base + pllu.params->ext_misc_reg[0]);\n>>>> -\tudelay(5);\n>>>> +\tfence_udelay(5, clk_base);\n>>>> \treg = readl_relaxed(clk_base + PLLU_BASE);\n>>>> \treg &= ~GENMASK(20, 0);\n>>>> @@ -2844,13 +2851,13 @@ static int tegra210_enable_pllu(void)\n>>>> \treg |= fentry->n << 8;\n>>>> \treg |= fentry->p << 16;\n>>>> \twritel(reg, clk_base + PLLU_BASE);\n>>>> -\tudelay(1);\n>>>> +\tfence_udelay(1, clk_base);\n>>> These udelay() -> fence_udelay() seem like they should be a separate\n>>> patch.\n>>>\n>>>> \treg |= PLL_ENABLE;\n>>>> \twritel(reg, clk_base + PLLU_BASE);\n>>>> +\tfence_udelay(1, clk_base);\n>>>> -\treadl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,\n>>>> -\t\t\t\t\t reg & PLL_BASE_LOCK, 2, 1000);\n>>>> -\tif (!(reg & PLL_BASE_LOCK)) {\n>>>> +\tret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);\n>>>> +\tif (ret) {\n>>>> \t\tpr_err(\"Timed out waiting for PLL_U to lock\\n\");\n>>>> \t\treturn -ETIMEDOUT;\n>>>> \t}\n>>>> @@ -2890,12 +2897,12 @@ static int tegra210_init_pllu(void)\n>>>> \t\treg = readl_relaxed(clk_base + XUSB_PLL_CFG0);\n>>>> \t\treg &= ~XUSB_PLL_CFG0_PLLU_LOCK_DLY_MASK;\n>>>> \t\twritel_relaxed(reg, clk_base + XUSB_PLL_CFG0);\n>>>> -\t\tudelay(1);\n>>>> +\t\tfence_udelay(1, clk_base);\n>>>> \t\treg = readl_relaxed(clk_base + PLLU_HW_PWRDN_CFG0);\n>>>> \t\treg |= PLLU_HW_PWRDN_CFG0_SEQ_ENABLE;\n>>>> \t\twritel_relaxed(reg, clk_base + PLLU_HW_PWRDN_CFG0);\n>>>> -\t\tudelay(1);\n>>>> +\t\tfence_udelay(1, clk_base);\n>>>> \t\treg = readl_relaxed(clk_base + PLLU_BASE);\n>>>> \t\treg &= ~PLLU_BASE_CLKENABLE_USB;\n>>>> @@ -3282,6 +3289,188 @@ static void tegra210_disable_cpu_clock(u32 cpu)\n>>>> }\n>>>> #ifdef CONFIG_PM_SLEEP\n>>>> +static u32 cpu_softrst_ctx[3];\n>>>> +static struct platform_device *dfll_pdev;\n>>>> +static u32 *periph_clk_src_ctx;\n>>>> +struct periph_source_bank {\n>>> Blank line between the above two.\n>>>\n>>>> +\tu32 start;\n>>>> +\tu32 end;\n>>>> +};\n>>>> +\n>>>> +static struct periph_source_bank periph_srcs[] = {\n>>>> +\t[0] = {\n>>>> +\t\t.start = 0x100,\n>>>> +\t\t.end = 0x198,\n>>>> +\t},\n>>>> +\t[1] = {\n>>>> +\t\t.start = 0x1a0,\n>>>> +\t\t.end = 0x1f8,\n>>>> +\t},\n>>>> +\t[2] = {\n>>>> +\t\t.start = 0x3b4,\n>>>> +\t\t.end = 0x42c,\n>>>> +\t},\n>>>> +\t[3] = {\n>>>> +\t\t.start = 0x49c,\n>>>> +\t\t.end = 0x4b4,\n>>>> +\t},\n>>>> +\t[4] = {\n>>>> +\t\t.start = 0x560,\n>>>> +\t\t.end = 0x564,\n>>>> +\t},\n>>>> +\t[5] = {\n>>>> +\t\t.start = 0x600,\n>>>> +\t\t.end = 0x678,\n>>>> +\t},\n>>>> +\t[6] = {\n>>>> +\t\t.start = 0x694,\n>>>> +\t\t.end = 0x6a0,\n>>>> +\t},\n>>>> +\t[7] = {\n>>>> +\t\t.start = 0x6b8,\n>>>> +\t\t.end = 0x718,\n>>>> +\t},\n>>>> +};\n>>>> +\n>>>> +/* This array lists the valid clocks for each periph clk bank */\n>>>> +static u32 periph_clks_on[] = {\n>>>> +\t0xdcd7dff9,\n>>>> +\t0x87d1f3e7,\n>>>> +\t0xf3fed3fa,\n>>>> +\t0xffc18cfb,\n>>>> +\t0x793fb7ff,\n>>>> +\t0x3fe66fff,\n>>>> +\t0xfc1fc7ff,\n>>>> +};\n>>> Hm... this is a bunch of magic. Perhaps replace this by a list of the\n>>> clock IDs? That's perhaps a little more verbose, but if we ever need to\n>>> tweak the list of IDs in that periph_clks_on array, that'll be quite the\n>>> challenge.\n>>>\n>>> Also, is this list a \"guess\" or are these all guaranteed to be always\n>>> on? What if some of these ended up getting disabled as part of suspend\n>>> already (by their users). If we force them on, won't their references\n>>> become unbalanced if the driver later enables them again on resume?\n>> Yes, will replace with list of peripheral clock names..\n>>\n>> This list is not a guess. Each entry of this list maps to CLK_ENB set\n>> register.\n>>\n>> Total 7 registers are available and each bit of these registers is for\n>> enable/disable clock to corresponding peripheral.\n>>\n>> Some of the bits are off as those peripheral clocks don't need to be enabled\n>> as we are not changing source or not using them like MIPIBIF, PLLG_REF..\n>>\n>> This list of peripheral clocks are enabled during resume before changing\n>> clock sources and after clock source update, they are restored back to the\n>> state they were before suspend. So their references don't become unbalanced.\n> Okay, good. Can you maybe put a version of that explanation in a comment\n> on top of the periph_clks_on declaration? And perhaps also describe this\n> in the commit message.\n>\n> Or maybe even better, add some comments in the main suspend/resume paths\n> to sort of \"guide\" through what's happening.\n>\n>>>> +\n>>>> +static struct platform_device *dfll_pdev;\n>>> I think you already predeclared this one above.\n>>>\n>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) + ((_off) * 4))\n>>>> +#define car_writel(_val, _base, _off) \\\n>>>> +\t\twritel_relaxed(_val, clk_base + (_base) + ((_off) * 4))\n>>>> +\n>>>> +static u32 * __init tegra210_init_suspend_ctx(void)\n>>>> +{\n>>>> +\tint i, size = 0;\n>>> Can both be unsigned int.\n>>>\n>>>> +\n>>>> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n>>>> +\t\tsize += periph_srcs[i].end - periph_srcs[i].start + 4;\n>>>> +\n>>>> +\tperiph_clk_src_ctx = kmalloc(size, GFP_KERNEL);\n>>>> +\n>>>> +\treturn periph_clk_src_ctx;\n>>> It's somewhat wasteful to return a global variable since you can access\n>>> it anyway. Perhaps it'd be more useful to make the function return a\n>>> boolean?\n>>>\n>>>> +}\n>>>> +\n>>>> +static int tegra210_clk_suspend(void)\n>>>> +{\n>>>> +\tint i;\n>>> unsigned int.\n>>>\n>>>> +\tunsigned long off;\n>>>> +\tstruct device_node *node;\n>>>> +\tu32 *clk_rst_ctx = periph_clk_src_ctx;\n>>>> +\tu32 val;\n>>>> +\n>>>> +\ttegra_cclkg_burst_policy_save_context();\n>>>> +\n>>>> +\tif (!dfll_pdev) {\n>>>> +\t\tnode = of_find_compatible_node(NULL, NULL,\n>>>> +\t\t\t\t\t \"nvidia,tegra210-dfll\");\n>>>> +\t\tif (node)\n>>>> +\t\t\tdfll_pdev = of_find_device_by_node(node);\n>>>> +\t\tof_node_put(node);\n>>>> +\t\tif (!dfll_pdev)\n>>>> +\t\t\tpr_err(\"dfll node not found. no suspend for dfll\\n\");\n>>>> +\t}\n>>> Wouldn't it make sense to run this only once, perhaps as part of\n>>> tegra210_init_suspend_ctx()?\n\ntegra210_init_suspend_ctx is invoked during tegra210_clock_init and as \nclock init happens earlier than dfll probe,\n\ndfll platform device will not be available at that time. So acquiring \ndfll pdev during 1st suspend.\n\n>>>> +\n>>>> +\tif (dfll_pdev)\n>>>> +\t\ttegra_dfll_suspend(dfll_pdev);\n>>>> +\n>>>> +\t/* Enable PLLP_OUT_CPU after dfll suspend */\n>>>> +\tval = car_readl(CLK_OUT_ENB_Y, 0);\n>>>> +\tval |= CLK_ENB_PLLP_OUT_CPU;\n>>>> +\tcar_writel(val, CLK_OUT_ENB_Y, 0);\n>>>> +\n>>>> +\ttegra_clk_periph_suspend(clk_base);\n>>>> +\n>>>> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n>>>> +\t\tfor (off = periph_srcs[i].start; off <= periph_srcs[i].end;\n>>>> +\t\t off += 4)\n>>>> +\t\t\t*clk_rst_ctx++ = car_readl(off, 0);\n>>>> +\n>>>> +\ttegra_sclk_cclklp_burst_policy_save_context();\n>>>> +\n>>>> +\tfor (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)\n>>>> +\t\tcpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);\n>>>> +\n>>>> +\tclk_save_context();\n>>>> +\n>>>> +\treturn 0;\n>>>> +}\n>>>> +\n>>>> +static void tegra210_clk_resume(void)\n>>>> +{\n>>>> +\tint i;\n>>>> +\tunsigned long off;\n>>>> +\tu32 val;\n>>>> +\tu32 *clk_rst_ctx = periph_clk_src_ctx;\n>>>> +\tstruct clk_hw *parent;\n>>>> +\tstruct clk *clk;\n>>>> +\n>>>> +\tfor (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)\n>>>> +\t\tcar_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);\n>>>> +\n>>>> +\ttegra_clk_osc_resume(clk_base);\n>>>> +\n>>>> +\t/*\n>>>> +\t * restore all the plls before configuring clocks and resetting\n>>>> +\t * the devices.\n>>>> +\t */\n>>>> +\ttegra210_init_pllu();\n>>>> +\ttegra_sclk_cpulp_burst_policy_restore_context();\n>>>> +\tclk_restore_context();\n>>>> +\n>>>> +\t/* enable all clocks before configuring clock sources */\n>>>> +\ttegra_clk_periph_force_on(periph_clks_on, ARRAY_SIZE(periph_clks_on),\n>>>> +\t\t\t\t clk_base);\n>>>> +\t/* wait for all writes to happen to have all the clocks enabled */\n>>>> +\twmb();\n>>>> +\tfence_udelay(2, clk_base);\n>>>> +\n>>>> +\t/* restore all the devices clock sources */\n>>>> +\tfor (i = 0; i < ARRAY_SIZE(periph_srcs); i++)\n>>>> +\t\tfor (off = periph_srcs[i].start; off <= periph_srcs[i].end;\n>>>> +\t\t off += 4)\n>>>> +\t\t\tcar_writel(*clk_rst_ctx++, off, 0);\n>>>> +\n>>>> +\t/* propagate and restore resets, restore clock state */\n>>>> +\tfence_udelay(5, clk_base);\n>>>> +\ttegra_clk_periph_resume(clk_base);\n>>>> +\n>>>> +\t/*\n>>>> +\t * restore CPUG clocks:\n>>>> +\t * - enable DFLL in open loop mode\n>>>> +\t * - switch CPUG to DFLL clock source\n>>>> +\t * - close DFLL loop\n>>>> +\t * - sync PLLX state\n>>>> +\t */\n>>>> +\tif (dfll_pdev)\n>>>> +\t\ttegra_dfll_resume(dfll_pdev, false);\n>>>> +\n>>>> +\ttegra_cclkg_burst_policy_restore_context();\n>>>> +\tfence_udelay(2, clk_base);\n>>>> +\n>>>> +\tif (dfll_pdev)\n>>>> +\t\ttegra_dfll_resume(dfll_pdev, true);\n>>>> +\n>>>> +\tparent = clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));\n>>>> +\tclk = clks[TEGRA210_CLK_PLL_X];\n>>>> +\tif (parent != __clk_get_hw(clk))\n>>>> +\t\ttegra_clk_sync_state_pll(__clk_get_hw(clk));\n>>>> +\n>>>> +\t/* Disable PLL_OUT_CPU after DFLL resume */\n>>>> +\tval = car_readl(CLK_OUT_ENB_Y, 0);\n>>>> +\tval &= ~CLK_ENB_PLLP_OUT_CPU;\n>>>> +\tcar_writel(val, CLK_OUT_ENB_Y, 0);\n>>>> +}\n>>> I'm surprised by the amount of work that we need to do here. I had hoped\n>>> that the clock framework's save/restore infrastructure would be enough.\n>>> I suppose you do call clk_restore_context() somewhere in there, so maybe\n>>> this really is as good as it gets.\n>>>\n>>> Thierry\n>> Reason is there are dependencies b/w the clocks and DFLL resume and clocks\n>> resume order needed is not same as clock tree list.\n>>\n>> during resume as per clock tree, CPU clock configs to use DFLL will happen\n>> first as its first in the clock tree but DFLL resume should be done prior to\n>> switching CPU to use from DFLL output.\n>>\n>> To resume DFLL, peripheral clocks should be restored.\n>>\n>> Considering these dependencies, performing peripheral and DFLL/CPU resume in\n>> Tegra210 clock driver rather than in corresponding peripheral clk_ops using\n>> save and restore context callback.\n> Okay makes sense. As mentioned above, I think it'd be great if you could\n> add more comments throughout the tegra210_clk_{suspend,resume}() code to\n> guide the reader through what you're doing, given that this is far from\n> obvious. You already do quite a bit of that, but it's perhaps better to\n> explain more what's going on and, perhaps more importantly, why. You're\n> currently mostly repeating the code sequence in the code. It'd be great\n> to have the general suspend/resume sequence detailed and highlight why\n> the sequence is the way it is and what the dependencies are, etc.\n>\n> Thierry\n>\nOK, Will add more comments in V4\n>>>> +\n>>>> static void tegra210_cpu_clock_suspend(void)\n>>>> {\n>>>> \t/* switch coresite to clk_m, save off original source */\n>>>> @@ -3295,8 +3484,20 @@ static void tegra210_cpu_clock_resume(void)\n>>>> \twritel(tegra210_cpu_clk_sctx.clk_csite_src,\n>>>> \t\t\t\tclk_base + CLK_SOURCE_CSITE);\n>>>> }\n>>>> +#else\n>>>> +#define tegra210_clk_suspend\tNULL\n>>>> +#define tegra210_clk_resume\tNULL\n>>>> +static inline u32 *tegra210_init_suspend_ctx(void)\n>>>> +{\n>>>> +\treturn NULL;\n>>>> +}\n>>>> #endif\n>>>> +static struct syscore_ops tegra_clk_syscore_ops = {\n>>>> +\t.suspend = tegra210_clk_suspend,\n>>>> +\t.resume = tegra210_clk_resume,\n>>>> +};\n>>>> +\n>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {\n>>>> \t.wait_for_reset\t= tegra210_wait_cpu_in_reset,\n>>>> \t.disable_clock\t= tegra210_disable_cpu_clock,\n>>>> @@ -3580,5 +3781,8 @@ static void __init tegra210_clock_init(struct device_node *np)\n>>>> \ttegra210_mbist_clk_init();\n>>>> \ttegra_cpu_car_ops = &tegra210_cpu_car_ops;\n>>>> +\n>>>> +\tif (tegra210_init_suspend_ctx())\n>>>> +\t\tregister_syscore_ops(&tegra_clk_syscore_ops);\n>>>> }\n>>>> CLK_OF_DECLARE(tegra210, \"nvidia,tegra210-car\", tegra210_clock_init);\n>>>> -- \n>>>> 2.7.4\n>>>>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdmarc=pass (p=none dis=none) header.from=nvidia.com","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=nvidia.com header.i=@nvidia.com\n\theader.b=\"dOPOOF91\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 45VrK00fcNz9s4Y\n\tfor ;\n\tSat, 22 Jun 2019 06:44:36 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1726059AbfFUUob (ORCPT );\n\tFri, 21 Jun 2019 16:44:31 -0400","from hqemgate15.nvidia.com ([216.228.121.64]:6721 \"EHLO\n\thqemgate15.nvidia.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1725985AbfFUUob (ORCPT\n\t); Fri, 21 Jun 2019 16:44:31 -0400","from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by\n\thqemgate15.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA)\n\tid ; Fri, 21 Jun 2019 13:44:29 -0700","from hqmail.nvidia.com ([172.20.161.6])\n\tby hqpgpgate101.nvidia.com (PGP Universal service);\n\tFri, 21 Jun 2019 13:44:27 -0700","from [10.2.174.126] (10.124.1.5) by HQMAIL107.nvidia.com\n\t(172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3;\n\tFri, 21 Jun 2019 20:44:23 +0000"],"X-PGP-Universal":"processed;\n\tby hqpgpgate101.nvidia.com on Fri, 21 Jun 2019 13:44:27 -0700","Subject":"Re: [PATCH V3 11/17] clk: tegra210: support for Tegra210 clocks\n\tsuspend and resume","To":"Thierry Reding ","CC":", ,\n\t, ,\n\t, ,\n\t, ,\n\t, ,\n\t, ,\n\t, , ,\n\t, ,\n\t, , ,\n\t, ","References":"<1560843991-24123-1-git-send-email-skomatineni@nvidia.com>\n\t<1560843991-24123-12-git-send-email-skomatineni@nvidia.com>\n\t<20190618121607.GN28892@ulmo>\n\t<491e0b18-11e7-837c-4591-06ed30950e1d@nvidia.com>\n\t<20190619081541.GA3187@ulmo>","From":"Sowjanya Komatineni ","Message-ID":"<467eec6e-87fd-0b59-f2f6-75eae4a15a34@nvidia.com>","Date":"Fri, 21 Jun 2019 13:44:23 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190619081541.GA3187@ulmo>","X-Originating-IP":"[10.124.1.5]","X-ClientProxiedBy":"HQMAIL105.nvidia.com (172.20.187.12) To\n\tHQMAIL107.nvidia.com (172.20.187.13)","Content-Type":"text/plain; charset=\"windows-1252\"; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;\n\tt=1561149869; bh=rMya3Lsh8UkESwXjmICg3L7PVGzynThgYAA0U3Q3z+s=;\n\th=X-PGP-Universal:Subject:To:CC:References:From:Message-ID:Date:\n\tUser-Agent:MIME-Version:In-Reply-To:X-Originating-IP:\n\tX-ClientProxiedBy:Content-Type:Content-Transfer-Encoding:\n\tContent-Language;\n\tb=dOPOOF916qaV1KpO9gd7faBpvrrghFMtQD9ge9fbbRSusg1fu+QAFB968lGK9VlGa\n\tattGAfjJBg9gbLLjFG9fL4VCi8JiYwEYWmW2fBlYoyq0p6dlWJrAMMLUk3WT0+bL3R\n\tuVn/72IcURycFECcFOfSnsWHCHwTdHh4Ugdf7P176JlTIIFsok1NAWrOreDihpkZng\n\tLvnmfEezHR7J8sagnlrFj2n1nCURIUj//1PlFtgeSz+/0Uuk5UqgmYc2QB1VZnGETc\n\tpfF84AV1PuoLYcwgmw4MP9vPKSHPqFdw/J5pqXQAVz8quN6XFHBoioJ9zDbcySeVli\n\thBjCtUUw/vppg==","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}}]