2019-05-28 23:10:45

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 00/12] LP0 entry and exit support for Tegra210

This patch series includes Tegra210 deepsleep/LP0 support with
deep sleep exit through RTC alarm wake and power button wake events.

Note: Wake on power button is through gpio-keys node in device tree.

This series also includes save and restore of PLLs, clocks, OSC contexts
for basic LP0 exit.

This patch series doesn't support 100% suspend/resume to allow fully
functional state upon resume and we are working on some more drivers suspend
and resume implementations.

[V2] : V1 feedback fixes
Patch 0002: This version still using syscore. Thierry suggest not to
use syscore and waiting on suggestion from Linux Walleij for any better
way of storing current state of pins before suspend entry and restoring
them on resume at very early stage. So left this the same way as V1 and
will address once I get more feedback on this.
Also need to findout and implement proper way of forcing resume order
between pinctrl and gpio driver.


Sowjanya Komatineni (12):
irqchip: tegra: do not disable COP IRQ during suspend
pinctrl: tegra: add suspend and resume support
clk: tegra: save and restore PLLs state for system
clk: tegra: add support for peripheral clock suspend and resume
clk: tegra: add support for OSC clock resume
clk: tegra: add suspend resume support for DFLL clock
clk: tegra: support for Tegra210 clocks suspend-resume
soc/tegra: pmc: allow support for more tegra wake models
soc/tegra: pmc: add pmc wake support for tegra210
gpio: tegra: implement wake event support for Tegra210 and prior GPIO
arm64: tegra: enable wake from deep sleep on RTC alarm.
soc/tegra: pmc: configure tegra deep sleep control settings

arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 7 +
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 +-
drivers/clk/tegra/clk-dfll.c | 82 ++++++
drivers/clk/tegra/clk-dfll.h | 2 +
drivers/clk/tegra/clk-divider.c | 19 ++
drivers/clk/tegra/clk-pll-out.c | 25 ++
drivers/clk/tegra/clk-pll.c | 99 +++++--
drivers/clk/tegra/clk-tegra-fixed.c | 16 ++
drivers/clk/tegra/clk-tegra210.c | 382 +++++++++++++++++++++++++
drivers/clk/tegra/clk.c | 74 ++++-
drivers/clk/tegra/clk.h | 13 +
drivers/gpio/gpio-tegra.c | 116 +++++++-
drivers/irqchip/irq-tegra.c | 22 +-
drivers/pinctrl/tegra/pinctrl-tegra.c | 68 ++++-
drivers/pinctrl/tegra/pinctrl-tegra.h | 3 +
drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra210.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
drivers/soc/tegra/pmc.c | 150 +++++++++-
21 files changed, 1053 insertions(+), 35 deletions(-)

--
2.7.4


2019-05-28 23:10:45

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 05/12] clk: tegra: add support for OSC clock resume

This patch adds support for restoring OSC control context on resume.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/clk/tegra/clk-tegra-fixed.c | 16 ++++++++++++++++
drivers/clk/tegra/clk.h | 1 +
2 files changed, 17 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra-fixed.c b/drivers/clk/tegra/clk-tegra-fixed.c
index 91c38f1666c1..de94333e800c 100644
--- a/drivers/clk/tegra/clk-tegra-fixed.c
+++ b/drivers/clk/tegra/clk-tegra-fixed.c
@@ -28,7 +28,10 @@
#define OSC_CTRL 0x50
#define OSC_CTRL_OSC_FREQ_SHIFT 28
#define OSC_CTRL_PLL_REF_DIV_SHIFT 26
+#define OSC_CTRL_MASK (0x3f2 | \
+ (0xf << OSC_CTRL_OSC_FREQ_SHIFT))

+static u32 osc_ctrl_ctx;
int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
unsigned long *input_freqs, unsigned int num,
unsigned int clk_m_div, unsigned long *osc_freq,
@@ -40,6 +43,7 @@ int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
unsigned osc_idx;

val = readl_relaxed(clk_base + OSC_CTRL);
+ osc_ctrl_ctx = val & OSC_CTRL_MASK;
osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;

if (osc_idx < num)
@@ -107,3 +111,15 @@ void __init tegra_fixed_clk_init(struct tegra_clk *tegra_clks)
*dt_clk = clk;
}
}
+
+#ifdef CONFIG_PM_SLEEP
+void tegra_clk_osc_resume(void __iomem *clk_base)
+{
+ u32 val;
+
+ val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
+ val |= osc_ctrl_ctx;
+ writel_relaxed(val, clk_base + OSC_CTRL);
+ fence_udelay(2, clk_base);
+}
+#endif
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index ab238b2c3125..666b9d907951 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -851,6 +851,7 @@ void tegra_clk_sync_state_pll_out(struct clk *clk);
void tegra_clk_periph_suspend(void __iomem *clk_base);
void tegra_clk_periph_resume(void __iomem *clk_base);
void tegra_clk_periph_force_on(u32 *clks_on, int count, void __iomem *clk_base);
+void tegra_clk_osc_resume(void __iomem *clk_base);
#endif


--
2.7.4

2019-05-28 23:11:05

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 03/12] clk: tegra: save and restore PLLs state for system

This patch has implementation of saving and restoring PLL's state to
support system suspend and resume operations.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/clk/tegra/clk-divider.c | 19 ++++++++
drivers/clk/tegra/clk-pll-out.c | 25 +++++++++++
drivers/clk/tegra/clk-pll.c | 99 ++++++++++++++++++++++++++++++++---------
drivers/clk/tegra/clk.h | 9 ++++
4 files changed, 132 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
index 2a1822a22740..718694727042 100644
--- a/drivers/clk/tegra/clk-divider.c
+++ b/drivers/clk/tegra/clk-divider.c
@@ -14,6 +14,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

+#include <linux/clk.h>
#include <linux/kernel.h>
#include <linux/io.h>
#include <linux/err.h>
@@ -179,3 +180,21 @@ struct clk *tegra_clk_register_mc(const char *name, const char *parent_name,
reg, 16, 1, CLK_DIVIDER_READ_ONLY,
mc_div_table, lock);
}
+
+#if defined(CONFIG_PM_SLEEP)
+void tegra_clk_divider_resume(struct clk_hw *hw, unsigned long rate)
+{
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ unsigned long parent_rate;
+
+ if (IS_ERR(parent)) {
+ WARN_ON(1);
+ return;
+ }
+
+ parent_rate = clk_hw_get_rate(parent);
+
+ if (clk_frac_div_set_rate(hw, rate, parent_rate) < 0)
+ WARN_ON(1);
+}
+#endif
diff --git a/drivers/clk/tegra/clk-pll-out.c b/drivers/clk/tegra/clk-pll-out.c
index 257cae0c1488..8b8c3b77d243 100644
--- a/drivers/clk/tegra/clk-pll-out.c
+++ b/drivers/clk/tegra/clk-pll-out.c
@@ -14,6 +14,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

+#include <linux/clk.h>
#include <linux/kernel.h>
#include <linux/io.h>
#include <linux/err.h>
@@ -120,3 +121,27 @@ struct clk *tegra_clk_register_pll_out(const char *name,

return clk;
}
+
+#if defined(CONFIG_PM_SLEEP)
+void tegra_clk_pll_out_resume(struct clk *clk, unsigned long rate)
+{
+ struct clk_hw *hw = __clk_get_hw(clk);
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+
+ if (IS_ERR(parent)) {
+ WARN_ON(1);
+ return;
+ }
+
+ tegra_clk_divider_resume(parent, rate);
+ clk_pll_out_enable(hw);
+}
+
+void tegra_clk_sync_state_pll_out(struct clk *clk)
+{
+ struct clk_hw *hw = __clk_get_hw(clk);
+
+ if (!__clk_get_enable_count(clk))
+ clk_pll_out_disable(hw);
+}
+#endif
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 6b976b2514f7..b363b6c6f600 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -20,6 +20,7 @@
#include <linux/err.h>
#include <linux/clk.h>
#include <linux/clk-provider.h>
+#include <linux/clk/tegra.h>

#include "clk.h"

@@ -1813,6 +1814,28 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw)

return ret;
}
+
+static void _clk_plle_tegra_init_parent(struct tegra_clk_pll *pll)
+{
+ u32 val, val_aux;
+
+ /* ensure parent is set to pll_ref */
+
+ val = pll_readl_base(pll);
+ val_aux = pll_readl(pll->params->aux_reg, pll);
+
+ if (val & PLL_BASE_ENABLE) {
+ if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
+ (val_aux & PLLE_AUX_PLLP_SEL))
+ WARN(1, "pll_e enabled with unsupported parent %s\n",
+ (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
+ "pll_re_vco");
+ } else {
+ val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
+ pll_writel(val_aux, pll->params->aux_reg, pll);
+ fence_udelay(1, pll->clk_base);
+ }
+}
#endif

static struct tegra_clk_pll *_tegra_init_pll(void __iomem *clk_base,
@@ -2289,6 +2312,21 @@ static const struct clk_ops tegra_clk_pllss_ops = {
.set_rate = clk_pllxc_set_rate,
};

+static void _pllss_set_defaults(struct tegra_clk_pll *pll)
+{
+ u32 val;
+
+ pll_writel_misc(PLLSS_MISC_DEFAULT, pll);
+ pll_writel(PLLSS_CFG_DEFAULT, pll->params->ext_misc_reg[0], pll);
+ pll_writel(PLLSS_CTRL1_DEFAULT, pll->params->ext_misc_reg[1], pll);
+ pll_writel(PLLSS_CTRL2_DEFAULT, pll->params->ext_misc_reg[2], pll);
+
+ val = pll_readl_base(pll);
+ val &= ~PLLSS_LOCK_OVERRIDE;
+ pll_writel_base(val, pll);
+}
+
struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,
void __iomem *clk_base, unsigned long flags,
struct tegra_clk_pll_params *pll_params,
@@ -2339,10 +2377,7 @@ struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,

_update_pll_mnp(pll, &cfg);

- pll_writel_misc(PLLSS_MISC_DEFAULT, pll);
- pll_writel(PLLSS_CFG_DEFAULT, pll_params->ext_misc_reg[0], pll);
- pll_writel(PLLSS_CTRL1_DEFAULT, pll_params->ext_misc_reg[1], pll);
- pll_writel(PLLSS_CTRL1_DEFAULT, pll_params->ext_misc_reg[2], pll);
+ _pllss_set_defaults(pll);

val = pll_readl_base(pll);
val_iddq = readl_relaxed(clk_base + pll_params->iddq_reg);
@@ -2546,27 +2581,12 @@ struct clk *tegra_clk_register_plle_tegra210(const char *name,
{
struct tegra_clk_pll *pll;
struct clk *clk;
- u32 val, val_aux;

pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
if (IS_ERR(pll))
return ERR_CAST(pll);

- /* ensure parent is set to pll_re_vco */
-
- val = pll_readl_base(pll);
- val_aux = pll_readl(pll_params->aux_reg, pll);
-
- if (val & PLLE_BASE_ENABLE) {
- if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
- (val_aux & PLLE_AUX_PLLP_SEL))
- WARN(1, "pll_e enabled with unsupported parent %s\n",
- (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
- "pll_re_vco");
- } else {
- val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
- pll_writel(val_aux, pll_params->aux_reg, pll);
- }
+ _clk_plle_tegra_init_parent(pll);

clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
&tegra_clk_plle_tegra210_ops);
@@ -2710,3 +2730,42 @@ struct clk *tegra_clk_register_pllmb(const char *name, const char *parent_name,
}

#endif
+
+#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARCH_TEGRA_210_SOC)
+void tegra_clk_pll_resume(struct clk *c, unsigned long rate)
+{
+ struct clk_hw *hw = __clk_get_hw(c);
+ struct tegra_clk_pll *pll = to_clk_pll(hw);
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+
+ if (clk_pll_is_enabled(hw))
+ return;
+
+ if (IS_ERR(parent)) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (pll->params->set_defaults)
+ pll->params->set_defaults(pll);
+
+ clk_set_rate(c, rate);
+ clk_pll_enable(hw);
+}
+
+void tegra_clk_sync_state_pll(struct clk *c)
+{
+ struct clk_hw *hw = __clk_get_hw(c);
+
+ if (!__clk_get_enable_count(c))
+ clk_pll_disable(hw);
+}
+
+void tegra_clk_plle_tegra210_resume(struct clk *c)
+{
+ struct clk_hw *hw = __clk_get_hw(c);
+ struct tegra_clk_pll *pll = to_clk_pll(hw);
+
+ _clk_plle_tegra_init_parent(pll);
+}
+#endif
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 09bccbb9640c..e4d124cc5657 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -841,6 +841,15 @@ int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
u8 frac_width, u8 flags);

+#ifdef CONFIG_PM_SLEEP
+void tegra_clk_pll_resume(struct clk *c, unsigned long rate);
+void tegra_clk_divider_resume(struct clk_hw *hw, unsigned long rate);
+void tegra_clk_pll_out_resume(struct clk *clk, unsigned long rate);
+void tegra_clk_plle_tegra210_resume(struct clk *c);
+void tegra_clk_sync_state_pll(struct clk *c);
+void tegra_clk_sync_state_pll_out(struct clk *clk);
+#endif
+

/* Combined read fence with delay */
#define fence_udelay(delay, reg) \
--
2.7.4

2019-05-28 23:11:06

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 07/12] clk: tegra: support for Tegra210 clocks suspend-resume

This patch adds system suspend and resume support for Tegra210
clocks.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/clk/tegra/clk-tegra210.c | 382 +++++++++++++++++++++++++++++++++++++++
1 file changed, 382 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index ed3c7df75d1e..d012b53ca132 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -20,10 +20,12 @@
#include <linux/clkdev.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_platform.h>
#include <linux/delay.h>
#include <linux/export.h>
#include <linux/mutex.h>
#include <linux/clk/tegra.h>
+#include <linux/syscore_ops.h>
#include <dt-bindings/clock/tegra210-car.h>
#include <dt-bindings/reset/tegra210-car.h>
#include <linux/iopoll.h>
@@ -31,6 +33,7 @@
#include <soc/tegra/pmc.h>

#include "clk.h"
+#include "clk-dfll.h"
#include "clk-id.h"

/*
@@ -48,6 +51,9 @@
#define CLK_SOURCE_SDMMC2 0x154
#define CLK_SOURCE_SDMMC4 0x164

+#define CLK_OUT_ENB_Y 0x298
+#define CLK_ENB_PLLP_OUT_CPU BIT(31)
+
#define PLLC_BASE 0x80
#define PLLC_OUT 0x84
#define PLLC_MISC0 0x88
@@ -71,6 +77,8 @@
#define PLLM_MISC1 0x98
#define PLLM_MISC2 0x9c
#define PLLP_BASE 0xa0
+#define PLLP_OUTA 0xa4
+#define PLLP_OUTB 0xa8
#define PLLP_MISC0 0xac
#define PLLP_MISC1 0x680
#define PLLA_BASE 0xb0
@@ -227,9 +235,18 @@
#define XUSB_PLL_CFG0_UTMIPLL_LOCK_DLY 0x3ff
#define XUSB_PLL_CFG0_PLLU_LOCK_DLY_MASK (0x3ff << 14)

+#define SCLK_BURST_POLICY 0x28
+#define SYSTEM_CLK_RATE 0x30
+#define CLK_MASK_ARM 0x44
+#define MISC_CLK_ENB 0x48
+#define CCLKG_BURST_POLICY 0x368
+#define CCLKLP_BURST_POLICY 0x370
+#define CPU_SOFTRST_CTRL 0x380
+#define SYS_CLK_DIV 0x400
#define SPARE_REG0 0x55c
#define CLK_M_DIVISOR_SHIFT 2
#define CLK_M_DIVISOR_MASK 0x3
+#define BURST_POLICY_REG_SIZE 2

#define RST_DFLL_DVCO 0x2f4
#define DVFS_DFLL_RESET_SHIFT 0
@@ -3381,6 +3398,367 @@ static struct tegra_clk_init_table init_table[] __initdata = {
{ TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 },
};

+#ifdef CONFIG_PM_SLEEP
+static unsigned long pll_c_rate, pll_c2_rate, pll_c3_rate, pll_x_rate;
+static unsigned long pll_c4_rate, pll_d2_rate, pll_dp_rate;
+static unsigned long pll_re_vco_rate, pll_d_rate, pll_a_rate, pll_a1_rate;
+static unsigned long pll_c_out1_rate;
+static unsigned long pll_a_out0_rate, pll_c4_out3_rate;
+static unsigned long pll_p_out_rate[5];
+static unsigned long pll_u_out1_rate, pll_u_out2_rate;
+static unsigned long pll_mb_rate;
+static u32 pll_m_v;
+static u32 pll_p_outa, pll_p_outb;
+static u32 pll_re_out_div, pll_re_out_1;
+static u32 cpu_softrst_ctx[3];
+static u32 cclkg_burst_policy_ctx[2];
+static u32 cclklp_burst_policy_ctx[2];
+static u32 sclk_burst_policy_ctx[3];
+static u32 sclk_ctx, spare_ctx, misc_clk_enb_ctx, clk_arm_ctx;
+
+static struct platform_device *dfll_pdev;
+#define car_readl(_base, _off) \
+ readl_relaxed(clk_base + (_base) + ((_off) * 4))
+#define car_writel(_val, _base, _off) \
+ writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
+
+static u32 *periph_clk_src_ctx;
+struct periph_source_bank {
+ u32 start;
+ u32 end;
+};
+
+static struct periph_source_bank periph_srcs[] = {
+ [0] = {
+ .start = 0x100,
+ .end = 0x198,
+ },
+ [1] = {
+ .start = 0x1a0,
+ .end = 0x1f8,
+ },
+ [2] = {
+ .start = 0x3b4,
+ .end = 0x42c,
+ },
+ [3] = {
+ .start = 0x49c,
+ .end = 0x4b4,
+ },
+ [4] = {
+ .start = 0x560,
+ .end = 0x564,
+ },
+ [5] = {
+ .start = 0x600,
+ .end = 0x678,
+ },
+ [6] = {
+ .start = 0x694,
+ .end = 0x6a0,
+ },
+ [7] = {
+ .start = 0x6b8,
+ .end = 0x718,
+ },
+};
+
+/* This array lists the valid clocks for each periph clk bank */
+static u32 periph_clks_on[] = {
+ 0xdcd7dff9,
+ 0x87d1f3e7,
+ 0xf3fed3fa,
+ 0xffc18cfb,
+ 0x793fb7ff,
+ 0x3fe66fff,
+ 0xfc1fc7ff,
+};
+
+static inline unsigned long clk_get_rate_nolock(struct clk *clk)
+{
+ if (IS_ERR_OR_NULL(clk)) {
+ WARN_ON(1);
+ return 0;
+ }
+
+ return clk_hw_get_rate(__clk_get_hw(clk));
+}
+
+static inline struct clk *pll_p_clk(unsigned int x)
+{
+ if (x < 4) {
+ return clks[TEGRA210_CLK_PLL_P_OUT1 + x];
+ } else if (x != 4) {
+ WARN_ON(1);
+ return NULL;
+ } else {
+ return clks[TEGRA210_CLK_PLL_P_OUT5];
+ }
+}
+
+static u32 * __init tegra210_init_suspend_ctx(void)
+{
+ int i, size = 0;
+
+ for (i = 0; i < ARRAY_SIZE(periph_srcs); i++)
+ size += periph_srcs[i].end - periph_srcs[i].start + 4;
+
+ periph_clk_src_ctx = kmalloc(size, GFP_KERNEL);
+
+ return periph_clk_src_ctx;
+}
+
+static int tegra210_clk_suspend(void)
+{
+ int i;
+ unsigned long off;
+ struct device_node *node;
+ u32 *clk_rst_ctx = periph_clk_src_ctx;
+ u32 val;
+
+ pll_a_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_A]);
+ pll_a_out0_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_A_OUT0]);
+ pll_a1_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_A1]);
+ pll_c2_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_C2]);
+ pll_c3_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_C3]);
+ pll_c_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_C]);
+ pll_c_out1_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_C_OUT1]);
+ pll_x_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_X]);
+ pll_c4_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_C4]);
+ pll_c4_out3_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_C4_OUT3]);
+ pll_dp_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_DP]);
+ pll_d_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_D]);
+ pll_d2_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_D2]);
+ pll_re_vco_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_RE_VCO]);
+ pll_re_out_div = car_readl(PLLRE_BASE, 0) & (0xf << 16);
+ pll_re_out_1 = car_readl(PLLRE_OUT1, 0);
+ pll_u_out1_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_U_OUT1]);
+ pll_u_out2_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_U_OUT2]);
+ pll_mb_rate = clk_get_rate_nolock(clks[TEGRA210_CLK_PLL_MB]);
+ pll_m_v = car_readl(PLLM_BASE, 0);
+ pll_p_outa = car_readl(PLLP_OUTA, 0);
+ pll_p_outb = car_readl(PLLP_OUTB, 0);
+
+ for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
+ cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
+
+ for (i = 0; i < ARRAY_SIZE(pll_p_out_rate); i++)
+ pll_p_out_rate[i] = clk_get_rate_nolock(pll_p_clk(i));
+
+ for (i = 0; i < BURST_POLICY_REG_SIZE; i++) {
+ cclkg_burst_policy_ctx[i] = car_readl(CCLKG_BURST_POLICY, i);
+ cclklp_burst_policy_ctx[i] = car_readl(CCLKLP_BURST_POLICY, i);
+ sclk_burst_policy_ctx[i] = car_readl(SCLK_BURST_POLICY, i);
+ }
+ sclk_burst_policy_ctx[i] = car_readl(SYS_CLK_DIV, 0);
+
+ sclk_ctx = car_readl(SYSTEM_CLK_RATE, 0);
+ spare_ctx = car_readl(SPARE_REG0, 0);
+ misc_clk_enb_ctx = car_readl(MISC_CLK_ENB, 0);
+ clk_arm_ctx = car_readl(CLK_MASK_ARM, 0);
+
+ for (i = 0; i < ARRAY_SIZE(periph_srcs); i++)
+ for (off = periph_srcs[i].start; off <= periph_srcs[i].end;
+ off += 4)
+ *clk_rst_ctx++ = car_readl(off, 0);
+
+ if (!dfll_pdev) {
+ node = of_find_compatible_node(NULL, NULL,
+ "nvidia,tegra210-dfll");
+ if (node)
+ dfll_pdev = of_find_device_by_node(node);
+ of_node_put(node);
+ if (!dfll_pdev)
+ pr_err("dfll node not found. no suspend for dfll\n");
+ }
+
+ if (dfll_pdev)
+ tegra_dfll_suspend(dfll_pdev);
+
+ /* Enable PLLP_OUT_CPU after dfll suspend */
+ val = car_readl(CLK_OUT_ENB_Y, 0);
+ val |= CLK_ENB_PLLP_OUT_CPU;
+ car_writel(val, CLK_OUT_ENB_Y, 0);
+
+ tegra_clk_periph_suspend(clk_base);
+ return 0;
+}
+
+static void tegra210_clk_resume(void)
+{
+ int i;
+ unsigned long off;
+ u32 val;
+ u32 *clk_rst_ctx = periph_clk_src_ctx;
+ struct clk_hw *parent;
+
+ tegra_clk_osc_resume(clk_base);
+
+ for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
+ car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
+
+ /*
+ * Since we are going to reset devices and switch clock sources in this
+ * function, plls and secondary dividers is required to be enabled. The
+ * actual value will be restored back later.
+ */
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_P_OUT1],
+ pll_p_out_rate[0]);
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_P_OUT3],
+ pll_p_out_rate[2]);
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_P_OUT4],
+ pll_p_out_rate[3]);
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_P_OUT5],
+ pll_p_out_rate[4]);
+
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_A1], pll_a1_rate);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_C2], pll_c2_rate);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_C3], pll_c3_rate);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_C], pll_c_rate);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_X], pll_x_rate);
+
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_A], pll_a_rate);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_D], pll_d_rate);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_D2], pll_d2_rate);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_DP], pll_dp_rate);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_C4], pll_c4_rate);
+
+ /* enable the PLLD */
+ val = car_readl(PLLD_MISC0, 0);
+ val |= PLLD_MISC0_DSI_CLKENABLE;
+ car_writel(val, PLLD_MISC0, 0);
+
+ /* reprogram PLLRE post divider, VCO, 2ndary divider (in this order) */
+ if (!__clk_is_enabled(clks[TEGRA210_CLK_PLL_RE_VCO])) {
+ val = car_readl(PLLRE_BASE, 0);
+ val &= ~(0xf << 16);
+ car_writel(val | pll_re_out_div, PLLRE_BASE, 0);
+ }
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_RE_VCO], pll_re_vco_rate);
+
+ car_writel(pll_re_out_1, PLLRE_OUT1, 0);
+
+ /* resume PLLU */
+ tegra210_init_pllu();
+
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_U_OUT1],
+ pll_u_out1_rate);
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_U_OUT2],
+ pll_u_out2_rate);
+
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_C_OUT1],
+ pll_c_out1_rate);
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_A_OUT0],
+ pll_a_out0_rate);
+ tegra_clk_pll_out_resume(clks[TEGRA210_CLK_PLL_C4_OUT3],
+ pll_c4_out3_rate);
+ /*
+ * resume SCLK and CPULP clocks
+ * for SCLk 1st set safe dividers values, then restore source,
+ * then restore dividers
+ */
+ car_writel(0x1, SYSTEM_CLK_RATE, 0);
+ val = car_readl(SYS_CLK_DIV, 0);
+ i = BURST_POLICY_REG_SIZE;
+ if (val < sclk_burst_policy_ctx[i])
+ car_writel(sclk_burst_policy_ctx[i], SYS_CLK_DIV, 0);
+ fence_udelay(2, clk_base);
+ for (i = 0; i < BURST_POLICY_REG_SIZE; i++) {
+ car_writel(cclklp_burst_policy_ctx[i], CCLKLP_BURST_POLICY, i);
+ car_writel(sclk_burst_policy_ctx[i], SCLK_BURST_POLICY, i);
+ }
+ car_writel(sclk_burst_policy_ctx[i], SYS_CLK_DIV, 0);
+
+ car_writel(sclk_ctx, SYSTEM_CLK_RATE, 0);
+ car_writel(spare_ctx, SPARE_REG0, 0);
+ car_writel(misc_clk_enb_ctx, MISC_CLK_ENB, 0);
+ car_writel(clk_arm_ctx, CLK_MASK_ARM, 0);
+
+ /* enable all clocks before configuring clock sources */
+ tegra_clk_periph_force_on(periph_clks_on, ARRAY_SIZE(periph_clks_on),
+ clk_base);
+
+ wmb();
+ fence_udelay(2, clk_base);
+
+ for (i = 0; i < ARRAY_SIZE(periph_srcs); i++)
+ for (off = periph_srcs[i].start; off <= periph_srcs[i].end;
+ off += 4)
+ car_writel(*clk_rst_ctx++, off, 0);
+
+ /* propagate and restore resets, restore clock state */
+ fence_udelay(5, clk_base);
+ tegra_clk_periph_resume(clk_base);
+
+ /* restore (sync) the actual PLL and secondary divider values */
+ car_writel(pll_p_outa, PLLP_OUTA, 0);
+ car_writel(pll_p_outb, PLLP_OUTB, 0);
+
+ tegra_clk_sync_state_pll_out(clks[TEGRA210_CLK_PLL_U_OUT1]);
+ tegra_clk_sync_state_pll_out(clks[TEGRA210_CLK_PLL_U_OUT2]);
+
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_A1]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_C2]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_C3]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_C]);
+
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_RE_VCO]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_C4]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_D2]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_DP]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_A]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_D]);
+
+ tegra_clk_sync_state_pll_out(clks[TEGRA210_CLK_PLL_C_OUT1]);
+ tegra_clk_sync_state_pll_out(clks[TEGRA210_CLK_PLL_A_OUT0]);
+
+ /*
+ * restore CPUG clocks:
+ * - enable DFLL in open loop mode
+ * - switch CPUG to DFLL clock source
+ * - close DFLL loop
+ * - sync PLLX state
+ */
+ if (dfll_pdev)
+ tegra_dfll_resume(dfll_pdev, false);
+
+ for (i = 0; i < BURST_POLICY_REG_SIZE; i++)
+ car_writel(cclkg_burst_policy_ctx[i], CCLKG_BURST_POLICY, i);
+ fence_udelay(2, clk_base);
+
+ if (dfll_pdev)
+ tegra_dfll_resume(dfll_pdev, true);
+
+ parent = clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
+ if (parent != __clk_get_hw(clks[TEGRA210_CLK_PLL_X]))
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_X]);
+
+ /* Disable PLL_OUT_CPU after DFLL resume */
+ val = car_readl(CLK_OUT_ENB_Y, 0);
+ val &= ~CLK_ENB_PLLP_OUT_CPU;
+ car_writel(val, CLK_OUT_ENB_Y, 0);
+
+ car_writel(pll_m_v, PLLM_BASE, 0);
+ tegra_clk_pll_resume(clks[TEGRA210_CLK_PLL_MB], pll_mb_rate);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_M]);
+ tegra_clk_sync_state_pll(clks[TEGRA210_CLK_PLL_MB]);
+
+ tegra_clk_plle_tegra210_resume(clks[TEGRA210_CLK_PLL_E]);
+}
+#else
+#define tegra210_clk_suspend NULL
+#define tegra210_clk_resume NULL
+static inline u32 *tegra210_init_suspend_ctx(void)
+{
+ return NULL;
+}
+#endif
+
+static struct syscore_ops tegra_clk_syscore_ops = {
+ .suspend = tegra210_clk_suspend,
+ .resume = tegra210_clk_resume,
+};
+
/**
* tegra210_clock_apply_init_table - initialize clocks on Tegra210 SoCs
*
@@ -3591,5 +3969,9 @@ static void __init tegra210_clock_init(struct device_node *np)
tegra210_mbist_clk_init();

tegra_cpu_car_ops = &tegra210_cpu_car_ops;
+
+ if (tegra210_init_suspend_ctx())
+ register_syscore_ops(&tegra_clk_syscore_ops);
+
}
CLK_OF_DECLARE(tegra210, "nvidia,tegra210-car", tegra210_clock_init);
--
2.7.4

2019-05-28 23:11:18

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 06/12] clk: tegra: add suspend resume support for DFLL clock

This patch adds support for suspend and resume for DFLL clock.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/clk/tegra/clk-dfll.c | 82 ++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/tegra/clk-dfll.h | 2 ++
2 files changed, 84 insertions(+)

diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
index 1fc71baae13b..d92a5a05fbbc 100644
--- a/drivers/clk/tegra/clk-dfll.c
+++ b/drivers/clk/tegra/clk-dfll.c
@@ -286,6 +286,7 @@ struct tegra_dfll {
unsigned long dvco_rate_min;

enum dfll_ctrl_mode mode;
+ enum dfll_ctrl_mode resume_mode;
enum dfll_tune_range tune_range;
struct dentry *debugfs_dir;
struct clk_hw dfll_clk_hw;
@@ -1873,6 +1874,87 @@ static int dfll_fetch_common_params(struct tegra_dfll *td)
}

/*
+ * tegra_dfll_suspend
+ * @pdev: DFLL instance
+ *
+ * dfll controls clock/voltage to other devices, including CPU. Therefore,
+ * dfll driver pm suspend callback does not stop cl-dvfs operations. It is
+ * only used to enforce cold voltage limit, since SoC may cool down during
+ * suspend without waking up. The correct temperature zone after suspend will
+ * be updated via dfll cooling device interface during resume of temperature
+ * sensor.
+ */
+void tegra_dfll_suspend(struct platform_device *pdev)
+{
+ struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+
+ if (!td)
+ return;
+
+ if (td->mode <= DFLL_DISABLED)
+ return;
+
+ td->resume_mode = td->mode;
+ switch (td->mode) {
+ case DFLL_CLOSED_LOOP:
+ dfll_set_mode(td, DFLL_CLOSED_LOOP);
+ dfll_set_frequency_request(td, &td->last_req);
+
+ dfll_unlock(td);
+ break;
+ default:
+ break;
+ }
+}
+
+/**
+ * tegra_dfll_resume - reprogram the DFLL after context-loss
+ * @pdev: DFLL instance
+ *
+ * Re-initialize and enable target device clock in open loop mode. Called
+ * directly from SoC clock resume syscore operation. Closed loop will be
+ * re-entered in platform syscore ops as well after CPU clock source is
+ * switched to DFLL in open loop.
+ */
+void tegra_dfll_resume(struct platform_device *pdev, bool on_dfll)
+{
+ struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+
+ if (!td)
+ return;
+
+ if (on_dfll) {
+ if (td->resume_mode == DFLL_CLOSED_LOOP)
+ dfll_lock(td);
+ td->resume_mode = DFLL_DISABLED;
+ return;
+ }
+
+ reset_control_deassert(td->dvco_rst);
+
+ pm_runtime_get(td->dev);
+
+ /* Re-init DFLL */
+ dfll_init_out_if(td);
+ dfll_set_default_params(td);
+ dfll_set_open_loop_config(td);
+
+ pm_runtime_put(td->dev);
+
+ /* Restore last request and mode up to open loop */
+ switch (td->resume_mode) {
+ case DFLL_CLOSED_LOOP:
+ case DFLL_OPEN_LOOP:
+ dfll_set_mode(td, DFLL_OPEN_LOOP);
+ if (td->pmu_if == TEGRA_DFLL_PMU_I2C)
+ dfll_i2c_set_output_enabled(td, false);
+ break;
+ default:
+ break;
+ }
+}
+
+/*
* API exported to per-SoC platform drivers
*/

diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
index 85d0d95223f3..44160b0495fe 100644
--- a/drivers/clk/tegra/clk-dfll.h
+++ b/drivers/clk/tegra/clk-dfll.h
@@ -48,6 +48,8 @@ struct tegra_dfll_soc_data {
int tegra_dfll_register(struct platform_device *pdev,
struct tegra_dfll_soc_data *soc);
struct tegra_dfll_soc_data *tegra_dfll_unregister(struct platform_device *pdev);
+void tegra_dfll_suspend(struct platform_device *pdev);
+void tegra_dfll_resume(struct platform_device *pdev, bool on_dfll);
int tegra_dfll_runtime_suspend(struct device *dev);
int tegra_dfll_runtime_resume(struct device *dev);

--
2.7.4

2019-05-28 23:11:21

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 09/12] soc/tegra: pmc: add pmc wake support for tegra210

This patch implements PMC wakeup sequence for Tegra210 and defines
common used wake events of RTC alarm and power key.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/soc/tegra/pmc.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 113 insertions(+)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 974b4c9f6ada..54dc8409e353 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -57,6 +57,7 @@
#include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
#include <dt-bindings/gpio/tegra186-gpio.h>
#include <dt-bindings/gpio/tegra194-gpio.h>
+#include <dt-bindings/gpio/tegra-gpio.h>

#define PMC_CNTRL 0x0
#define PMC_CNTRL_INTR_POLARITY BIT(17) /* inverts INTR polarity */
@@ -66,6 +67,12 @@
#define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
#define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
#define PMC_CNTRL_MAIN_RST BIT(4)
+#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
+
+#define PMC_WAKE_MASK 0x0c
+#define PMC_WAKE_LEVEL 0x10
+#define PMC_WAKE_STATUS 0x14
+#define PMC_SW_WAKE_STATUS 0x18

#define DPD_SAMPLE 0x020
#define DPD_SAMPLE_ENABLE BIT(0)
@@ -96,6 +103,11 @@

#define PMC_SCRATCH41 0x140

+#define PMC_WAKE2_MASK 0x160
+#define PMC_WAKE2_LEVEL 0x164
+#define PMC_WAKE2_STATUS 0x168
+#define PMC_SW_WAKE2_STATUS 0x16c
+
#define PMC_SENSOR_CTRL 0x1b0
#define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
#define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
@@ -245,6 +257,7 @@ struct tegra_pmc_soc {

const struct tegra_wake_event *wake_events;
unsigned int num_wake_events;
+ unsigned int max_supported_wake_events;
};

static const char * const tegra186_reset_sources[] = {
@@ -1917,6 +1930,54 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
.alloc = tegra_pmc_irq_alloc,
};

+static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+ struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
+ unsigned int offset, bit;
+ u32 value;
+
+ if (data->hwirq == ULONG_MAX)
+ return 0;
+
+ offset = data->hwirq / 32;
+ bit = data->hwirq % 32;
+
+ /*
+ * latch wakeups to SW_WAKE_STATUS register to capture events
+ * that would not make it into wakeup event register during LP0 exit.
+ */
+ value = tegra_pmc_readl(pmc, PMC_CNTRL);
+ value |= PMC_CNTRL_LATCH_WAKEUPS;
+ tegra_pmc_writel(pmc, value, PMC_CNTRL);
+ usleep_range(110, 120);
+
+ value &= ~PMC_CNTRL_LATCH_WAKEUPS;
+ tegra_pmc_writel(pmc, value, PMC_CNTRL);
+ usleep_range(110, 120);
+
+ tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
+ if (pmc->soc->max_supported_wake_events > 32)
+ tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
+
+ tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
+ if (pmc->soc->max_supported_wake_events > 32)
+ tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
+
+ /* enable PMC wake */
+ if (data->hwirq >= 32)
+ offset = PMC_WAKE2_MASK;
+ else
+ offset = PMC_WAKE_MASK;
+ value = tegra_pmc_readl(pmc, offset);
+ if (on)
+ value |= 1 << bit;
+ else
+ value &= ~(1 << bit);
+ tegra_pmc_writel(pmc, value, offset);
+
+ return 0;
+}
+
static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
@@ -1948,6 +2009,48 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
return 0;
}

+static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
+ unsigned int offset, bit;
+ u32 value;
+
+ if (data->hwirq == ULONG_MAX)
+ return 0;
+
+ offset = data->hwirq / 32;
+ bit = data->hwirq % 32;
+
+ if (data->hwirq >= 32)
+ offset = PMC_WAKE2_LEVEL;
+ else
+ offset = PMC_WAKE_LEVEL;
+ value = tegra_pmc_readl(pmc, offset);
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ case IRQ_TYPE_LEVEL_HIGH:
+ value |= 1 << bit;
+ break;
+
+ case IRQ_TYPE_EDGE_FALLING:
+ case IRQ_TYPE_LEVEL_LOW:
+ value &= ~(1 << bit);
+ break;
+
+ case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
+ value ^= 1 << bit;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ tegra_pmc_writel(pmc, value, offset);
+
+ return 0;
+}
+
static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
@@ -2535,6 +2638,11 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
};

+static const struct tegra_wake_event tegra210_wake_events[] = {
+ TEGRA_WAKE_GPIO("power", 24, 0, 189),
+ TEGRA_WAKE_IRQ("rtc", 16, 2),
+};
+
static const struct tegra_pmc_soc tegra210_pmc_soc = {
.num_powergates = ARRAY_SIZE(tegra210_powergates),
.powergates = tegra210_powergates,
@@ -2552,10 +2660,15 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
.regs = &tegra20_pmc_regs,
.init = tegra20_pmc_init,
.setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
+ .irq_set_wake = tegra210_pmc_irq_set_wake,
+ .irq_set_type = tegra210_pmc_irq_set_type,
.reset_sources = tegra210_reset_sources,
.num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
.reset_levels = NULL,
.num_reset_levels = 0,
+ .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
+ .wake_events = tegra210_wake_events,
+ .max_supported_wake_events = 64,
};

#define TEGRA186_IO_PAD_TABLE(_pad) \
--
2.7.4

2019-05-28 23:11:48

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 04/12] clk: tegra: add support for peripheral clock suspend and resume

This patch implements peripheral clock context save and restore
to support system suspend and resume operation.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/clk/tegra/clk.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++-
drivers/clk/tegra/clk.h | 3 ++
2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
index 6f2862eddad7..08b788766564 100644
--- a/drivers/clk/tegra/clk.c
+++ b/drivers/clk/tegra/clk.c
@@ -81,6 +81,10 @@ static struct clk **clks;
static int clk_num;
static struct clk_onecell_data clk_data;

+#ifdef CONFIG_PM_SLEEP
+static u32 *periph_ctx;
+#endif
+
/* Handlers for SoC-specific reset lines */
static int (*special_reset_assert)(unsigned long);
static int (*special_reset_deassert)(unsigned long);
@@ -210,6 +214,65 @@ const struct tegra_clk_periph_regs *get_reg_bank(int clkid)
}
}

+#ifdef CONFIG_PM_SLEEP
+void tegra_clk_periph_suspend(void __iomem *clk_base)
+{
+ int i, idx;
+
+ idx = 0;
+ for (i = 0; i < periph_banks; i++, idx++)
+ periph_ctx[idx] =
+ readl_relaxed(clk_base + periph_regs[i].rst_reg);
+
+ for (i = 0; i < periph_banks; i++, idx++)
+ periph_ctx[idx] =
+ readl_relaxed(clk_base + periph_regs[i].enb_reg);
+}
+
+void tegra_clk_periph_force_on(u32 *clks_on, int count, void __iomem *clk_base)
+{
+ int i;
+
+ WARN_ON(count != periph_banks);
+
+ for (i = 0; i < count; i++)
+ writel_relaxed(clks_on[i], clk_base + periph_regs[i].enb_reg);
+}
+
+void tegra_clk_periph_resume(void __iomem *clk_base)
+{
+ int i, idx;
+
+ idx = 0;
+ for (i = 0; i < periph_banks; i++, idx++)
+ writel_relaxed(periph_ctx[idx],
+ clk_base + periph_regs[i].rst_reg);
+
+ /* ensure all resets have propagated */
+ fence_udelay(2, clk_base);
+ tegra_read_chipid();
+
+ for (i = 0; i < periph_banks; i++, idx++)
+ writel_relaxed(periph_ctx[idx],
+ clk_base + periph_regs[i].enb_reg);
+
+ /* ensure all enables have propagated */
+ fence_udelay(2, clk_base);
+ tegra_read_chipid();
+}
+
+static int tegra_clk_suspend_ctx_init(int banks)
+{
+ int err = 0;
+
+ periph_ctx = kzalloc(2 * banks * sizeof(*periph_ctx), GFP_KERNEL);
+ if (!periph_ctx)
+ err = -ENOMEM;
+
+ return err;
+}
+#endif
+
struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
{
clk_base = regs;
@@ -226,11 +289,20 @@ struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
periph_banks = banks;

clks = kcalloc(num, sizeof(struct clk *), GFP_KERNEL);
- if (!clks)
+ if (!clks) {
kfree(periph_clk_enb_refcnt);
+ return NULL;
+ }

clk_num = num;

+#ifdef CONFIG_PM_SLEEP
+ if (tegra_clk_suspend_ctx_init(banks)) {
+ kfree(periph_clk_enb_refcnt);
+ kfree(clks);
+ return NULL;
+ }
+#endif
return clks;
}

diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index e4d124cc5657..ab238b2c3125 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -848,6 +848,9 @@ void tegra_clk_pll_out_resume(struct clk *clk, unsigned long rate);
void tegra_clk_plle_tegra210_resume(struct clk *c);
void tegra_clk_sync_state_pll(struct clk *c);
void tegra_clk_sync_state_pll_out(struct clk *clk);
+void tegra_clk_periph_suspend(void __iomem *clk_base);
+void tegra_clk_periph_resume(void __iomem *clk_base);
+void tegra_clk_periph_force_on(u32 *clks_on, int count, void __iomem *clk_base);
#endif


--
2.7.4

2019-05-28 23:12:04

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 11/12] arm64: tegra: enable wake from deep sleep on RTC alarm.

This patch updates device tree for RTC and PMC to allow system wake
from deep sleep on RTC alarm.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index a550c0a4d572..cf5c215efb04 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -763,7 +763,8 @@
rtc@7000e000 {
compatible = "nvidia,tegra210-rtc", "nvidia,tegra20-rtc";
reg = <0x0 0x7000e000 0x0 0x100>;
- interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&pmc>;
clocks = <&tegra_car TEGRA210_CLK_RTC>;
clock-names = "rtc";
};
@@ -773,6 +774,8 @@
reg = <0x0 0x7000e400 0x0 0x400>;
clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
clock-names = "pclk", "clk32k_in";
+ #interrupt-cells = <2>;
+ interrupt-controller;

powergates {
pd_audio: aud {
--
2.7.4

2019-05-28 23:12:11

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 08/12] soc/tegra: pmc: allow support for more tegra wake models

This patch allows to create separate irq_set_wake and irq_set_type
implementations for different tegra designs PMC that has different
wake models which require difference wake registers and different
programming sequence.

AOWAKE model support is available for Tegra186 and Tegra194 only
and it resides within PMC and supports tiered wake architecture.

Tegra210 and prior tegra designs uses PMC directly to receive wake
events and coordinate the wake sequence.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/soc/tegra/pmc.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 5648e5c09ef5..974b4c9f6ada 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -235,6 +235,8 @@ struct tegra_pmc_soc {
void (*setup_irq_polarity)(struct tegra_pmc *pmc,
struct device_node *np,
bool invert);
+ int (*irq_set_wake)(struct irq_data *data, unsigned int on);
+ int (*irq_set_type)(struct irq_data *data, unsigned int type);

const char * const *reset_sources;
unsigned int num_reset_sources;
@@ -1915,12 +1917,15 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
.alloc = tegra_pmc_irq_alloc,
};

-static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
+static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
unsigned int offset, bit;
u32 value;

+ if (data->hwirq == ULONG_MAX)
+ return 0;
+
offset = data->hwirq / 32;
bit = data->hwirq % 32;

@@ -1943,7 +1948,7 @@ static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
return 0;
}

-static int tegra_pmc_irq_set_type(struct irq_data *data, unsigned int type)
+static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
u32 value;
@@ -1996,8 +2001,10 @@ static int tegra_pmc_irq_init(struct tegra_pmc *pmc)
pmc->irq.irq_unmask = irq_chip_unmask_parent;
pmc->irq.irq_eoi = irq_chip_eoi_parent;
pmc->irq.irq_set_affinity = irq_chip_set_affinity_parent;
- pmc->irq.irq_set_type = tegra_pmc_irq_set_type;
- pmc->irq.irq_set_wake = tegra_pmc_irq_set_wake;
+ if (pmc->soc->irq_set_type)
+ pmc->irq.irq_set_type = pmc->soc->irq_set_type;
+ if (pmc->soc->irq_set_wake)
+ pmc->irq.irq_set_wake = pmc->soc->irq_set_wake;

pmc->domain = irq_domain_add_hierarchy(parent, 0, 96, pmc->dev->of_node,
&tegra_pmc_irq_domain_ops, pmc);
@@ -2670,6 +2677,8 @@ static const struct tegra_pmc_soc tegra186_pmc_soc = {
.regs = &tegra186_pmc_regs,
.init = NULL,
.setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
+ .irq_set_wake = tegra186_pmc_irq_set_wake,
+ .irq_set_type = tegra186_pmc_irq_set_type,
.reset_sources = tegra186_reset_sources,
.num_reset_sources = ARRAY_SIZE(tegra186_reset_sources),
.reset_levels = tegra186_reset_levels,
@@ -2748,6 +2757,8 @@ static const struct tegra_pmc_soc tegra194_pmc_soc = {
.regs = &tegra186_pmc_regs,
.init = NULL,
.setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
+ .irq_set_wake = tegra186_pmc_irq_set_wake,
+ .irq_set_type = tegra186_pmc_irq_set_type,
.num_wake_events = ARRAY_SIZE(tegra194_wake_events),
.wake_events = tegra194_wake_events,
};
--
2.7.4

2019-05-28 23:12:16

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 10/12] gpio: tegra: implement wake event support for Tegra210 and prior GPIO

The GPIO controller doesn't have any controls to enable the system to
wake up from low power states based on activity on GPIO pins. An extra
hardware block that is part of the power management controller (PMC)
contains these controls. In order for the GPIO controller to be able
to cooperate with the PMC, obtain a reference to the PMC's IRQ domain
and make it a parent to the GPIO controller's IRQ domain. This way the
PMC gets an opportunity to program the additional registers required
to enable wakeup sources on suspend.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/gpio/gpio-tegra.c | 116 +++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 110 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 6d9b6906b9d0..5190129668d3 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -32,6 +32,8 @@
#include <linux/pinctrl/consumer.h>
#include <linux/pm.h>

+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
#define GPIO_BANK(x) ((x) >> 5)
#define GPIO_PORT(x) (((x) >> 3) & 0x3)
#define GPIO_BIT(x) ((x) & 0x7)
@@ -275,8 +277,22 @@ static int tegra_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
{
struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
+ struct irq_domain *domain = tgi->irq_domain;
+
+ if (!gpiochip_irqchip_irq_valid(chip, offset))
+ return -ENXIO;
+
+ if (irq_domain_is_hierarchy(domain)) {
+ struct irq_fwspec spec;
+
+ spec.fwnode = domain->fwnode;
+ spec.param_count = 2;
+ spec.param[0] = offset;
+ spec.param[1] = IRQ_TYPE_NONE;
+ return irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, &spec);
+ }

- return irq_find_mapping(tgi->irq_domain, offset);
+ return irq_find_mapping(domain, offset);
}

static void tegra_gpio_irq_ack(struct irq_data *d)
@@ -365,7 +381,10 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type)
else if (type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
irq_set_handler_locked(d, handle_edge_irq);

- return 0;
+ if (d->parent_data)
+ return irq_chip_set_type_parent(d, type);
+ else
+ return 0;
}

static void tegra_gpio_irq_shutdown(struct irq_data *d)
@@ -503,6 +522,7 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
struct tegra_gpio_bank *bank = irq_data_get_irq_chip_data(d);
unsigned int gpio = d->hwirq;
u32 port, bit, mask;
+ int ret;

port = GPIO_PORT(gpio);
bit = GPIO_BIT(gpio);
@@ -513,7 +533,14 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
else
bank->wake_enb[port] &= ~mask;

- return irq_set_irq_wake(bank->irq, enable);
+ ret = irq_set_irq_wake(bank->irq, enable);
+ if (ret < 0)
+ return ret;
+
+ if (d->parent_data)
+ return irq_chip_set_wake_parent(d, enable);
+ else
+ return 0;
}
#endif

@@ -566,10 +593,78 @@ static const struct dev_pm_ops tegra_gpio_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(tegra_gpio_suspend, tegra_gpio_resume)
};

+static int tegra_gpio_irq_domain_translate(struct irq_domain *domain,
+ struct irq_fwspec *fwspec,
+ unsigned long *hwirq,
+ unsigned int *type)
+{
+ if (WARN_ON(fwspec->param_count < 2))
+ return -EINVAL;
+
+ *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
+ *hwirq = fwspec->param[0];
+
+ return 0;
+}
+
+static int tegra_gpio_irq_domain_alloc(struct irq_domain *domain,
+ unsigned int virq,
+ unsigned int num_irqs, void *data)
+{
+ struct tegra_gpio_info *tgi = gpiochip_get_data(domain->host_data);
+ struct irq_fwspec *fwspec = data, spec;
+ struct tegra_gpio_bank *bank;
+ unsigned long hwirq;
+ unsigned int type;
+ int err = 0;
+
+ if (WARN_ON(num_irqs != 1))
+ return -EINVAL;
+
+ if (WARN_ON(fwspec->param_count < 2))
+ return -EINVAL;
+
+ err = tegra_gpio_irq_domain_translate(domain, fwspec, &hwirq, &type);
+ if (err)
+ return err;
+
+ bank = &tgi->bank_info[GPIO_BANK(hwirq)];
+ err = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+ &tgi->ic, bank);
+
+ if (err < 0)
+ return err;
+
+ spec.fwnode = domain->parent->fwnode;
+ spec.param_count = 3;
+ spec.param[0] = GIC_SPI;
+ spec.param[1] = fwspec->param[0];
+ spec.param[2] = fwspec->param[1];
+
+ return irq_domain_alloc_irqs_parent(domain, virq, 1, &spec);
+}
+
+static const struct irq_domain_ops tegra_gpio_irq_domain_ops = {
+ .translate = tegra_gpio_irq_domain_translate,
+ .alloc = tegra_gpio_irq_domain_alloc,
+};
+
+static const struct of_device_id tegra_pmc_of_match[] = {
+ { .compatible = "nvidia,tegra210-pmc" },
+ { .compatible = "nvidia,tegra132-pmc" },
+ { .compatible = "nvidia,tegra124-pmc" },
+ { .compatible = "nvidia,tegra114-pmc" },
+ { .compatible = "nvidia,tegra30-pmc" },
+ { .compatible = "nvidia,tegra20-pmc" },
+ { }
+};
+
static int tegra_gpio_probe(struct platform_device *pdev)
{
struct tegra_gpio_info *tgi;
struct tegra_gpio_bank *bank;
+ struct device_node *np;
+ struct irq_domain *parent_domain = NULL;
unsigned int gpio, i, j;
int ret;

@@ -614,6 +709,13 @@ static int tegra_gpio_probe(struct platform_device *pdev)
#ifdef CONFIG_PM_SLEEP
tgi->ic.irq_set_wake = tegra_gpio_irq_set_wake;
#endif
+ np = of_find_matching_node(NULL, tegra_pmc_of_match);
+ if (np) {
+ parent_domain = irq_find_host(np);
+ of_node_put(np);
+ if (!parent_domain)
+ return -EPROBE_DEFER;
+ }

platform_set_drvdata(pdev, tgi);

@@ -625,9 +727,11 @@ static int tegra_gpio_probe(struct platform_device *pdev)
if (!tgi->bank_info)
return -ENOMEM;

- tgi->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
- tgi->gc.ngpio,
- &irq_domain_simple_ops, NULL);
+ tgi->irq_domain = irq_domain_add_hierarchy(parent_domain, 0,
+ tgi->gc.ngpio,
+ pdev->dev.of_node,
+ &tegra_gpio_irq_domain_ops,
+ &tgi->gc);
if (!tgi->irq_domain)
return -ENODEV;

--
2.7.4

2019-05-28 23:12:21

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support

This patch adds suspend and resume support for Tegra pinctrl driver
and registers them to syscore so the pinmux settings are restored
before the devices resume.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/pinctrl/tegra/pinctrl-tegra.c | 68 +++++++++++++++++++++++++++++++-
drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra210.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
7 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index a5008c066bac..bdc47e62c457 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -28,11 +28,18 @@
#include <linux/pinctrl/pinmux.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/slab.h>
+#include <linux/syscore_ops.h>

#include "../core.h"
#include "../pinctrl-utils.h"
#include "pinctrl-tegra.h"

+#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
+#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
+#define EMMC_DPD_PARKING (0x1fff << 14)
+
+static struct tegra_pmx *pmx;
+
static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
{
return readl(pmx->regs[bank] + reg);
@@ -629,6 +636,50 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
}
}

+static int __maybe_unused tegra_pinctrl_suspend(void)
+{
+ u32 *backup_regs = pmx->backup_regs;
+ u32 *regs;
+ int i, j;
+
+ for (i = 0; i < pmx->nbanks; i++) {
+ regs = pmx->regs[i];
+ for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
+ *backup_regs++ = readl(regs++);
+ }
+
+ return pinctrl_force_sleep(pmx->pctl);
+}
+
+static void __maybe_unused tegra_pinctrl_resume(void)
+{
+ u32 *backup_regs = pmx->backup_regs;
+ u32 *regs;
+ u32 val;
+ int i, j;
+
+ for (i = 0; i < pmx->nbanks; i++) {
+ regs = pmx->regs[i];
+ for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
+ writel(*backup_regs++, regs++);
+ }
+
+ if (pmx->soc->has_park_padcfg) {
+ val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
+ val &= ~EMMC_DPD_PARKING;
+ pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
+
+ val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
+ val &= ~EMMC_DPD_PARKING;
+ pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
+ }
+}
+
+static struct syscore_ops pinctrl_syscore_ops = {
+ .suspend = tegra_pinctrl_suspend,
+ .resume = tegra_pinctrl_resume,
+};
+
static bool gpio_node_has_range(const char *compatible)
{
struct device_node *np;
@@ -648,11 +699,11 @@ static bool gpio_node_has_range(const char *compatible)
int tegra_pinctrl_probe(struct platform_device *pdev,
const struct tegra_pinctrl_soc_data *soc_data)
{
- struct tegra_pmx *pmx;
struct resource *res;
int i;
const char **group_pins;
int fn, gn, gfn;
+ unsigned long backup_regs_size = 0;

pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
if (!pmx)
@@ -705,6 +756,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
if (!res)
break;
+ backup_regs_size += resource_size(res);
}
pmx->nbanks = i;

@@ -713,11 +765,24 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
if (!pmx->regs)
return -ENOMEM;

+ pmx->reg_bank_size = devm_kcalloc(&pdev->dev, pmx->nbanks,
+ sizeof(*pmx->reg_bank_size),
+ GFP_KERNEL);
+ if (!pmx->reg_bank_size)
+ return -ENOMEM;
+
+ pmx->backup_regs = devm_kzalloc(&pdev->dev, backup_regs_size,
+ GFP_KERNEL);
+ if (!pmx->backup_regs)
+ return -ENOMEM;
+
for (i = 0; i < pmx->nbanks; i++) {
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
pmx->regs[i] = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(pmx->regs[i]))
return PTR_ERR(pmx->regs[i]);
+
+ pmx->reg_bank_size[i] = resource_size(res);
}

pmx->pctl = devm_pinctrl_register(&pdev->dev, &tegra_pinctrl_desc, pmx);
@@ -732,6 +797,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
pinctrl_add_gpio_range(pmx->pctl, &tegra_pinctrl_gpio_range);

platform_set_drvdata(pdev, pmx);
+ register_syscore_ops(&pinctrl_syscore_ops);

dev_dbg(&pdev->dev, "Probed Tegra pinctrl driver\n");

diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
index 44c71941b5f8..b405df6fd390 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.h
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
@@ -25,6 +25,8 @@ struct tegra_pmx {

int nbanks;
void __iomem **regs;
+ size_t *reg_bank_size;
+ u32 *backup_regs;
};

enum tegra_pinconf_param {
@@ -199,6 +201,7 @@ struct tegra_pinctrl_soc_data {
bool hsm_in_mux;
bool schmitt_in_mux;
bool drvtype_in_mux;
+ bool has_park_padcfg;
};

int tegra_pinctrl_probe(struct platform_device *pdev,
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra114.c b/drivers/pinctrl/tegra/pinctrl-tegra114.c
index d43c209e9c30..4ac44f34dccf 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra114.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra114.c
@@ -1849,6 +1849,7 @@ static const struct tegra_pinctrl_soc_data tegra114_pinctrl = {
.hsm_in_mux = false,
.schmitt_in_mux = false,
.drvtype_in_mux = false,
+ .has_park_padcfg = false,
};

static int tegra114_pinctrl_probe(struct platform_device *pdev)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra124.c b/drivers/pinctrl/tegra/pinctrl-tegra124.c
index 5b07a5834d15..1dac7648b41f 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra124.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra124.c
@@ -2061,6 +2061,7 @@ static const struct tegra_pinctrl_soc_data tegra124_pinctrl = {
.hsm_in_mux = false,
.schmitt_in_mux = false,
.drvtype_in_mux = false,
+ .has_park_padcfg = false,
};

static int tegra124_pinctrl_probe(struct platform_device *pdev)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra20.c b/drivers/pinctrl/tegra/pinctrl-tegra20.c
index 1fc82a9576e0..9d2b25200f32 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra20.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra20.c
@@ -2231,6 +2231,7 @@ static const struct tegra_pinctrl_soc_data tegra20_pinctrl = {
.hsm_in_mux = false,
.schmitt_in_mux = false,
.drvtype_in_mux = false,
+ .has_park_padcfg = false,
};

static const char *cdev1_parents[] = {
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
index 3e77f5474dd8..dc06c36e698a 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
@@ -1563,6 +1563,7 @@ static const struct tegra_pinctrl_soc_data tegra210_pinctrl = {
.hsm_in_mux = true,
.schmitt_in_mux = true,
.drvtype_in_mux = true,
+ .has_park_padcfg = true,
};

static int tegra210_pinctrl_probe(struct platform_device *pdev)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra30.c b/drivers/pinctrl/tegra/pinctrl-tegra30.c
index 10e617003e9c..42182d714950 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra30.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra30.c
@@ -2484,6 +2484,7 @@ static const struct tegra_pinctrl_soc_data tegra30_pinctrl = {
.hsm_in_mux = false,
.schmitt_in_mux = false,
.drvtype_in_mux = false,
+ .has_park_padcfg = false,
};

static int tegra30_pinctrl_probe(struct platform_device *pdev)
--
2.7.4

2019-05-28 23:35:12

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH V2 12/12] soc/tegra: pmc: configure tegra deep sleep control settings

Tegra210 and prior Tegra chips have power request signal polarity,
deep sleep entry and wake related timings which are platform specific
that should be configured before entering into deep sleep.

Below are the timings specific configurations for deep sleep and wake.
- Core rail power-on stabilization timer
- OSC clock stabilization timer after SOC rail power is stabilized.
- Core power off time is the minimum wake delay to keep the system
in deep sleep state irrespective of any quick wake event.

These values depends on the discharge time of regulators and turn OFF
time of the PMIC to allow the complete system to finish entering into
deep sleep state.

These values vary based on the platform design and are specified
through the device tree.

This patch has implementation to configure these configurations which
are must to have for deep sleep state.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 7 +++++++
drivers/soc/tegra/pmc.c | 18 ++++++++++++++++++
2 files changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
index 4dcd0d36189a..7ac5e55a30aa 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
@@ -266,6 +266,13 @@

pmc@7000e400 {
nvidia,invert-interrupt;
+ nvidia,suspend-mode = <0>;
+ nvidia,cpu-pwr-good-time = <0>;
+ nvidia,cpu-pwr-off-time = <0>;
+ nvidia,core-pwr-good-time = <4587 3876>;
+ nvidia,core-pwr-off-time = <39065>;
+ nvidia,core-power-req-active-high;
+ nvidia,sys-clock-req-active-high;
};

/* eMMC */
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 54dc8409e353..d7e0f5057f16 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -66,6 +66,7 @@
#define PMC_CNTRL_SIDE_EFFECT_LP0 BIT(14) /* LP0 when CPU pwr gated */
#define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
#define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
+#define PMC_CNTRL_PWRREQ_POLARITY BIT(8)
#define PMC_CNTRL_MAIN_RST BIT(4)
#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)

@@ -98,6 +99,8 @@

#define PMC_CPUPWRGOOD_TIMER 0xc8
#define PMC_CPUPWROFF_TIMER 0xcc
+#define PMC_COREPWRGOOD_TIMER 0x3c
+#define PMC_COREPWROFF_TIMER 0xe0

#define PMC_PWR_DET_VALUE 0xe4

@@ -2285,6 +2288,7 @@ static const struct tegra_pmc_regs tegra20_pmc_regs = {
static void tegra20_pmc_init(struct tegra_pmc *pmc)
{
u32 value;
+ unsigned long osc, pmu, off;

/* Always enable CPU power request */
value = tegra_pmc_readl(pmc, PMC_CNTRL);
@@ -2298,6 +2302,11 @@ static void tegra20_pmc_init(struct tegra_pmc *pmc)
else
value |= PMC_CNTRL_SYSCLK_POLARITY;

+ if (pmc->corereq_high)
+ value &= ~PMC_CNTRL_PWRREQ_POLARITY;
+ else
+ value |= PMC_CNTRL_PWRREQ_POLARITY;
+
/* configure the output polarity while the request is tristated */
tegra_pmc_writel(pmc, value, PMC_CNTRL);

@@ -2305,6 +2314,15 @@ static void tegra20_pmc_init(struct tegra_pmc *pmc)
value = tegra_pmc_readl(pmc, PMC_CNTRL);
value |= PMC_CNTRL_SYSCLK_OE;
tegra_pmc_writel(pmc, value, PMC_CNTRL);
+
+ osc = DIV_ROUND_UP_ULL(pmc->core_osc_time * 8192, 1000000);
+ pmu = DIV_ROUND_UP_ULL(pmc->core_pmu_time * 32768, 1000000);
+ off = DIV_ROUND_UP_ULL(pmc->core_off_time * 32768, 1000000);
+ if (osc && pmu)
+ tegra_pmc_writel(pmc, ((osc << 8) & 0xff00) | (pmu & 0xff),
+ PMC_COREPWRGOOD_TIMER);
+ if (off)
+ tegra_pmc_writel(pmc, off, PMC_COREPWROFF_TIMER);
}

static void tegra20_pmc_setup_irq_polarity(struct tegra_pmc *pmc,
--
2.7.4

2019-05-29 05:44:06

by JC Kuo

[permalink] [raw]
Subject: Re: [PATCH V2 09/12] soc/tegra: pmc: add pmc wake support for tegra210

Hi Sowjanya,

usleep_range() in tegra210_pmc_irq_set_wake() should be replaced with
udelay() because caller irq_set_irq_wake() acquired spinlock and made
this context atomic.

Thanks,

JC

On 5/29/19 7:08 AM, Sowjanya Komatineni wrote:
> This patch implements PMC wakeup sequence for Tegra210 and defines
> common used wake events of RTC alarm and power key.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/soc/tegra/pmc.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 113 insertions(+)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 974b4c9f6ada..54dc8409e353 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -57,6 +57,7 @@
> #include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
> #include <dt-bindings/gpio/tegra186-gpio.h>
> #include <dt-bindings/gpio/tegra194-gpio.h>
> +#include <dt-bindings/gpio/tegra-gpio.h>
>
> #define PMC_CNTRL 0x0
> #define PMC_CNTRL_INTR_POLARITY BIT(17) /* inverts INTR polarity */
> @@ -66,6 +67,12 @@
> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
> #define PMC_CNTRL_MAIN_RST BIT(4)
> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
> +
> +#define PMC_WAKE_MASK 0x0c
> +#define PMC_WAKE_LEVEL 0x10
> +#define PMC_WAKE_STATUS 0x14
> +#define PMC_SW_WAKE_STATUS 0x18
>
> #define DPD_SAMPLE 0x020
> #define DPD_SAMPLE_ENABLE BIT(0)
> @@ -96,6 +103,11 @@
>
> #define PMC_SCRATCH41 0x140
>
> +#define PMC_WAKE2_MASK 0x160
> +#define PMC_WAKE2_LEVEL 0x164
> +#define PMC_WAKE2_STATUS 0x168
> +#define PMC_SW_WAKE2_STATUS 0x16c
> +
> #define PMC_SENSOR_CTRL 0x1b0
> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
> @@ -245,6 +257,7 @@ struct tegra_pmc_soc {
>
> const struct tegra_wake_event *wake_events;
> unsigned int num_wake_events;
> + unsigned int max_supported_wake_events;
> };
>
> static const char * const tegra186_reset_sources[] = {
> @@ -1917,6 +1930,54 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
> .alloc = tegra_pmc_irq_alloc,
> };
>
> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> +{
> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> + unsigned int offset, bit;
> + u32 value;
> +
> + if (data->hwirq == ULONG_MAX)
> + return 0;
> +
> + offset = data->hwirq / 32;
> + bit = data->hwirq % 32;
> +
> + /*
> + * latch wakeups to SW_WAKE_STATUS register to capture events
> + * that would not make it into wakeup event register during LP0 exit.
> + */
> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
> + value |= PMC_CNTRL_LATCH_WAKEUPS;
> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
> + usleep_range(110, 120);
> +
> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
> + usleep_range(110, 120);
> +
> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
> + if (pmc->soc->max_supported_wake_events > 32)
> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
> +
> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
> + if (pmc->soc->max_supported_wake_events > 32)
> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
> +
> + /* enable PMC wake */
> + if (data->hwirq >= 32)
> + offset = PMC_WAKE2_MASK;
> + else
> + offset = PMC_WAKE_MASK;
> + value = tegra_pmc_readl(pmc, offset);
> + if (on)
> + value |= 1 << bit;
> + else
> + value &= ~(1 << bit);
> + tegra_pmc_writel(pmc, value, offset);
> +
> + return 0;
> +}
> +
> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> {
> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> @@ -1948,6 +2009,48 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> return 0;
> }
>
> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> + unsigned int offset, bit;
> + u32 value;
> +
> + if (data->hwirq == ULONG_MAX)
> + return 0;
> +
> + offset = data->hwirq / 32;
> + bit = data->hwirq % 32;
> +
> + if (data->hwirq >= 32)
> + offset = PMC_WAKE2_LEVEL;
> + else
> + offset = PMC_WAKE_LEVEL;
> + value = tegra_pmc_readl(pmc, offset);
> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + case IRQ_TYPE_LEVEL_HIGH:
> + value |= 1 << bit;
> + break;
> +
> + case IRQ_TYPE_EDGE_FALLING:
> + case IRQ_TYPE_LEVEL_LOW:
> + value &= ~(1 << bit);
> + break;
> +
> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
> + value ^= 1 << bit;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + tegra_pmc_writel(pmc, value, offset);
> +
> + return 0;
> +}
> +
> static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> {
> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> @@ -2535,6 +2638,11 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
> TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
> };
>
> +static const struct tegra_wake_event tegra210_wake_events[] = {
> + TEGRA_WAKE_GPIO("power", 24, 0, 189),
> + TEGRA_WAKE_IRQ("rtc", 16, 2),
> +};
> +
> static const struct tegra_pmc_soc tegra210_pmc_soc = {
> .num_powergates = ARRAY_SIZE(tegra210_powergates),
> .powergates = tegra210_powergates,
> @@ -2552,10 +2660,15 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
> .regs = &tegra20_pmc_regs,
> .init = tegra20_pmc_init,
> .setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
> + .irq_set_wake = tegra210_pmc_irq_set_wake,
> + .irq_set_type = tegra210_pmc_irq_set_type,
> .reset_sources = tegra210_reset_sources,
> .num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
> .reset_levels = NULL,
> .num_reset_levels = 0,
> + .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
> + .wake_events = tegra210_wake_events,
> + .max_supported_wake_events = 64,
> };
>
> #define TEGRA186_IO_PAD_TABLE(_pad) \

2019-05-29 13:54:59

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH V2 09/12] soc/tegra: pmc: add pmc wake support for tegra210

On Tue, May 28, 2019 at 04:08:53PM -0700, Sowjanya Komatineni wrote:
> This patch implements PMC wakeup sequence for Tegra210 and defines
> common used wake events of RTC alarm and power key.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/soc/tegra/pmc.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 113 insertions(+)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 974b4c9f6ada..54dc8409e353 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -57,6 +57,7 @@
> #include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
> #include <dt-bindings/gpio/tegra186-gpio.h>
> #include <dt-bindings/gpio/tegra194-gpio.h>
> +#include <dt-bindings/gpio/tegra-gpio.h>
>
> #define PMC_CNTRL 0x0
> #define PMC_CNTRL_INTR_POLARITY BIT(17) /* inverts INTR polarity */
> @@ -66,6 +67,12 @@
> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
> #define PMC_CNTRL_MAIN_RST BIT(4)
> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
> +
> +#define PMC_WAKE_MASK 0x0c
> +#define PMC_WAKE_LEVEL 0x10
> +#define PMC_WAKE_STATUS 0x14
> +#define PMC_SW_WAKE_STATUS 0x18
>
> #define DPD_SAMPLE 0x020
> #define DPD_SAMPLE_ENABLE BIT(0)
> @@ -96,6 +103,11 @@
>
> #define PMC_SCRATCH41 0x140
>
> +#define PMC_WAKE2_MASK 0x160
> +#define PMC_WAKE2_LEVEL 0x164
> +#define PMC_WAKE2_STATUS 0x168
> +#define PMC_SW_WAKE2_STATUS 0x16c
> +
> #define PMC_SENSOR_CTRL 0x1b0
> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
> @@ -245,6 +257,7 @@ struct tegra_pmc_soc {
>
> const struct tegra_wake_event *wake_events;
> unsigned int num_wake_events;
> + unsigned int max_supported_wake_events;

Do we really need this? It's only used in Tegra210 specific code and
it's always 64 on Tegra210. Can't we always hard-code it?

> };
>
> static const char * const tegra186_reset_sources[] = {
> @@ -1917,6 +1930,54 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
> .alloc = tegra_pmc_irq_alloc,
> };
>
> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> +{
> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> + unsigned int offset, bit;
> + u32 value;
> +
> + if (data->hwirq == ULONG_MAX)
> + return 0;
> +
> + offset = data->hwirq / 32;
> + bit = data->hwirq % 32;
> +
> + /*
> + * latch wakeups to SW_WAKE_STATUS register to capture events
> + * that would not make it into wakeup event register during LP0 exit.
> + */
> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
> + value |= PMC_CNTRL_LATCH_WAKEUPS;
> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
> + usleep_range(110, 120);
> +
> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
> + usleep_range(110, 120);
> +
> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
> + if (pmc->soc->max_supported_wake_events > 32)
> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
> +
> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
> + if (pmc->soc->max_supported_wake_events > 32)
> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
> +
> + /* enable PMC wake */
> + if (data->hwirq >= 32)
> + offset = PMC_WAKE2_MASK;
> + else
> + offset = PMC_WAKE_MASK;
> + value = tegra_pmc_readl(pmc, offset);

Blank line before and after the "value = ..." line, for readability.

> + if (on)
> + value |= 1 << bit;
> + else
> + value &= ~(1 << bit);
> + tegra_pmc_writel(pmc, value, offset);

Same here, leave a blank line before the tegra_pmc_writel(...) call to
improve readability.

> +
> + return 0;
> +}
> +
> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> {
> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> @@ -1948,6 +2009,48 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> return 0;
> }
>
> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> + unsigned int offset, bit;
> + u32 value;
> +
> + if (data->hwirq == ULONG_MAX)
> + return 0;
> +
> + offset = data->hwirq / 32;
> + bit = data->hwirq % 32;
> +
> + if (data->hwirq >= 32)
> + offset = PMC_WAKE2_LEVEL;
> + else
> + offset = PMC_WAKE_LEVEL;
> + value = tegra_pmc_readl(pmc, offset);

Same comment as above.

Otherwise, looking good, thanks.

Thierry

> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + case IRQ_TYPE_LEVEL_HIGH:
> + value |= 1 << bit;
> + break;
> +
> + case IRQ_TYPE_EDGE_FALLING:
> + case IRQ_TYPE_LEVEL_LOW:
> + value &= ~(1 << bit);
> + break;
> +
> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
> + value ^= 1 << bit;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + tegra_pmc_writel(pmc, value, offset);
> +
> + return 0;
> +}
> +
> static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> {
> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> @@ -2535,6 +2638,11 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
> TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
> };
>
> +static const struct tegra_wake_event tegra210_wake_events[] = {
> + TEGRA_WAKE_GPIO("power", 24, 0, 189),
> + TEGRA_WAKE_IRQ("rtc", 16, 2),
> +};
> +
> static const struct tegra_pmc_soc tegra210_pmc_soc = {
> .num_powergates = ARRAY_SIZE(tegra210_powergates),
> .powergates = tegra210_powergates,
> @@ -2552,10 +2660,15 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
> .regs = &tegra20_pmc_regs,
> .init = tegra20_pmc_init,
> .setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
> + .irq_set_wake = tegra210_pmc_irq_set_wake,
> + .irq_set_type = tegra210_pmc_irq_set_type,
> .reset_sources = tegra210_reset_sources,
> .num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
> .reset_levels = NULL,
> .num_reset_levels = 0,
> + .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
> + .wake_events = tegra210_wake_events,
> + .max_supported_wake_events = 64,
> };
>
> #define TEGRA186_IO_PAD_TABLE(_pad) \
> --
> 2.7.4
>


Attachments:
(No filename) (6.23 kB)
signature.asc (849.00 B)
Download all attachments

2019-05-29 14:06:32

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH V2 10/12] gpio: tegra: implement wake event support for Tegra210 and prior GPIO

On Tue, May 28, 2019 at 04:08:54PM -0700, Sowjanya Komatineni wrote:
> The GPIO controller doesn't have any controls to enable the system to
> wake up from low power states based on activity on GPIO pins. An extra
> hardware block that is part of the power management controller (PMC)
> contains these controls. In order for the GPIO controller to be able
> to cooperate with the PMC, obtain a reference to the PMC's IRQ domain
> and make it a parent to the GPIO controller's IRQ domain. This way the
> PMC gets an opportunity to program the additional registers required
> to enable wakeup sources on suspend.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/gpio/gpio-tegra.c | 116 +++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 110 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> index 6d9b6906b9d0..5190129668d3 100644
> --- a/drivers/gpio/gpio-tegra.c
> +++ b/drivers/gpio/gpio-tegra.c
> @@ -32,6 +32,8 @@
> #include <linux/pinctrl/consumer.h>
> #include <linux/pm.h>
>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> #define GPIO_BANK(x) ((x) >> 5)
> #define GPIO_PORT(x) (((x) >> 3) & 0x3)
> #define GPIO_BIT(x) ((x) & 0x7)
> @@ -275,8 +277,22 @@ static int tegra_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> {
> struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
> + struct irq_domain *domain = tgi->irq_domain;
> +
> + if (!gpiochip_irqchip_irq_valid(chip, offset))
> + return -ENXIO;
> +
> + if (irq_domain_is_hierarchy(domain)) {
> + struct irq_fwspec spec;
> +
> + spec.fwnode = domain->fwnode;
> + spec.param_count = 2;
> + spec.param[0] = offset;
> + spec.param[1] = IRQ_TYPE_NONE;
> + return irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, &spec);

This looks like it was copied from the equivalent Tegra186 patch. I have
since then changed the implementation, based on feedback by Linus, to
not call irq_domain_alloc_irqs() here and instead call
irq_create_fwspec_mapping(). This has the advantage of not requiring the
irq_domain_alloc_irqs() function to be exported. It ends up calling that
function internally, but as discussed with Linus it's also a nicer way
to create these mappings.

> + }
>
> - return irq_find_mapping(tgi->irq_domain, offset);
> + return irq_find_mapping(domain, offset);
> }
>
> static void tegra_gpio_irq_ack(struct irq_data *d)
> @@ -365,7 +381,10 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> else if (type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
> irq_set_handler_locked(d, handle_edge_irq);
>
> - return 0;
> + if (d->parent_data)
> + return irq_chip_set_type_parent(d, type);
> + else
> + return 0;

There's no need for this final else. Just make it a regular "return 0;"
at the end of the function, without the extra else branch.

> }
>
> static void tegra_gpio_irq_shutdown(struct irq_data *d)
> @@ -503,6 +522,7 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
> struct tegra_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> unsigned int gpio = d->hwirq;
> u32 port, bit, mask;
> + int ret;
>
> port = GPIO_PORT(gpio);
> bit = GPIO_BIT(gpio);
> @@ -513,7 +533,14 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
> else
> bank->wake_enb[port] &= ~mask;
>
> - return irq_set_irq_wake(bank->irq, enable);
> + ret = irq_set_irq_wake(bank->irq, enable);
> + if (ret < 0)
> + return ret;
> +
> + if (d->parent_data)
> + return irq_chip_set_wake_parent(d, enable);
> + else
> + return 0;

Same here.

Thierry


Attachments:
(No filename) (3.75 kB)
signature.asc (849.00 B)
Download all attachments

2019-05-29 14:08:30

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH V2 12/12] soc/tegra: pmc: configure tegra deep sleep control settings

On Tue, May 28, 2019 at 04:08:56PM -0700, Sowjanya Komatineni wrote:
> Tegra210 and prior Tegra chips have power request signal polarity,
> deep sleep entry and wake related timings which are platform specific
> that should be configured before entering into deep sleep.
>
> Below are the timings specific configurations for deep sleep and wake.
> - Core rail power-on stabilization timer
> - OSC clock stabilization timer after SOC rail power is stabilized.
> - Core power off time is the minimum wake delay to keep the system
> in deep sleep state irrespective of any quick wake event.
>
> These values depends on the discharge time of regulators and turn OFF
> time of the PMIC to allow the complete system to finish entering into
> deep sleep state.
>
> These values vary based on the platform design and are specified
> through the device tree.
>
> This patch has implementation to configure these configurations which
> are must to have for deep sleep state.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 7 +++++++
> drivers/soc/tegra/pmc.c | 18 ++++++++++++++++++
> 2 files changed, 25 insertions(+)

Please split up the DT and driver changes into separate patches.

Thierry


Attachments:
(No filename) (1.29 kB)
signature.asc (849.00 B)
Download all attachments

2019-05-29 14:14:47

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH V2 00/12] LP0 entry and exit support for Tegra210

On Tue, May 28, 2019 at 04:08:44PM -0700, Sowjanya Komatineni wrote:
> This patch series includes Tegra210 deepsleep/LP0 support with
> deep sleep exit through RTC alarm wake and power button wake events.
>
> Note: Wake on power button is through gpio-keys node in device tree.

On the Jetson TX2 patches for LP0 support we added a couple of other
properties that we don't have in the Jetson TX1 device tree yet. For
example:

linux,input-type = <EV_KEY>;

which is probably harmless, but we may want to add it on Jetson TX1
eventually anyway.

debounce-interval = <10>;

May be good an Jetson TX1 as well.

wakeup-event-action = <EV_ACT_ASSERTED>;

I vaguely recall this to be necessary for some reason, but I may be
wrong.

Thierry

>
> This series also includes save and restore of PLLs, clocks, OSC contexts
> for basic LP0 exit.
>
> This patch series doesn't support 100% suspend/resume to allow fully
> functional state upon resume and we are working on some more drivers suspend
> and resume implementations.
>
> [V2] : V1 feedback fixes
> Patch 0002: This version still using syscore. Thierry suggest not to
> use syscore and waiting on suggestion from Linux Walleij for any better
> way of storing current state of pins before suspend entry and restoring
> them on resume at very early stage. So left this the same way as V1 and
> will address once I get more feedback on this.
> Also need to findout and implement proper way of forcing resume order
> between pinctrl and gpio driver.
>
>
> Sowjanya Komatineni (12):
> irqchip: tegra: do not disable COP IRQ during suspend
> pinctrl: tegra: add suspend and resume support
> clk: tegra: save and restore PLLs state for system
> clk: tegra: add support for peripheral clock suspend and resume
> clk: tegra: add support for OSC clock resume
> clk: tegra: add suspend resume support for DFLL clock
> clk: tegra: support for Tegra210 clocks suspend-resume
> soc/tegra: pmc: allow support for more tegra wake models
> soc/tegra: pmc: add pmc wake support for tegra210
> gpio: tegra: implement wake event support for Tegra210 and prior GPIO
> arm64: tegra: enable wake from deep sleep on RTC alarm.
> soc/tegra: pmc: configure tegra deep sleep control settings
>
> arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 7 +
> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 +-
> drivers/clk/tegra/clk-dfll.c | 82 ++++++
> drivers/clk/tegra/clk-dfll.h | 2 +
> drivers/clk/tegra/clk-divider.c | 19 ++
> drivers/clk/tegra/clk-pll-out.c | 25 ++
> drivers/clk/tegra/clk-pll.c | 99 +++++--
> drivers/clk/tegra/clk-tegra-fixed.c | 16 ++
> drivers/clk/tegra/clk-tegra210.c | 382 +++++++++++++++++++++++++
> drivers/clk/tegra/clk.c | 74 ++++-
> drivers/clk/tegra/clk.h | 13 +
> drivers/gpio/gpio-tegra.c | 116 +++++++-
> drivers/irqchip/irq-tegra.c | 22 +-
> drivers/pinctrl/tegra/pinctrl-tegra.c | 68 ++++-
> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 +
> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra210.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
> drivers/soc/tegra/pmc.c | 150 +++++++++-
> 21 files changed, 1053 insertions(+), 35 deletions(-)
>
> --
> 2.7.4
>


Attachments:
(No filename) (3.61 kB)
signature.asc (849.00 B)
Download all attachments

2019-05-29 14:32:34

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH V2 08/12] soc/tegra: pmc: allow support for more tegra wake models

On Tue, May 28, 2019 at 04:08:52PM -0700, Sowjanya Komatineni wrote:
> This patch allows to create separate irq_set_wake and irq_set_type
> implementations for different tegra designs PMC that has different
> wake models which require difference wake registers and different
> programming sequence.
>
> AOWAKE model support is available for Tegra186 and Tegra194 only
> and it resides within PMC and supports tiered wake architecture.
>
> Tegra210 and prior tegra designs uses PMC directly to receive wake
> events and coordinate the wake sequence.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/soc/tegra/pmc.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 5648e5c09ef5..974b4c9f6ada 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -235,6 +235,8 @@ struct tegra_pmc_soc {
> void (*setup_irq_polarity)(struct tegra_pmc *pmc,
> struct device_node *np,
> bool invert);
> + int (*irq_set_wake)(struct irq_data *data, unsigned int on);
> + int (*irq_set_type)(struct irq_data *data, unsigned int type);
>
> const char * const *reset_sources;
> unsigned int num_reset_sources;
> @@ -1915,12 +1917,15 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
> .alloc = tegra_pmc_irq_alloc,
> };
>
> -static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> +static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> {
> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> unsigned int offset, bit;
> u32 value;
>
> + if (data->hwirq == ULONG_MAX)
> + return 0;
> +
> offset = data->hwirq / 32;
> bit = data->hwirq % 32;
>

I've submitted this hunk as a separate patch because I think we may end
up needing to backport that to v5.0.

No need for you to worry about that, though. I'll take care of it when I
apply this patch.

Thierry

> @@ -1943,7 +1948,7 @@ static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> return 0;
> }
>
> -static int tegra_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> +static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> {
> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> u32 value;
> @@ -1996,8 +2001,10 @@ static int tegra_pmc_irq_init(struct tegra_pmc *pmc)
> pmc->irq.irq_unmask = irq_chip_unmask_parent;
> pmc->irq.irq_eoi = irq_chip_eoi_parent;
> pmc->irq.irq_set_affinity = irq_chip_set_affinity_parent;
> - pmc->irq.irq_set_type = tegra_pmc_irq_set_type;
> - pmc->irq.irq_set_wake = tegra_pmc_irq_set_wake;
> + if (pmc->soc->irq_set_type)
> + pmc->irq.irq_set_type = pmc->soc->irq_set_type;
> + if (pmc->soc->irq_set_wake)
> + pmc->irq.irq_set_wake = pmc->soc->irq_set_wake;
>
> pmc->domain = irq_domain_add_hierarchy(parent, 0, 96, pmc->dev->of_node,
> &tegra_pmc_irq_domain_ops, pmc);
> @@ -2670,6 +2677,8 @@ static const struct tegra_pmc_soc tegra186_pmc_soc = {
> .regs = &tegra186_pmc_regs,
> .init = NULL,
> .setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
> + .irq_set_wake = tegra186_pmc_irq_set_wake,
> + .irq_set_type = tegra186_pmc_irq_set_type,
> .reset_sources = tegra186_reset_sources,
> .num_reset_sources = ARRAY_SIZE(tegra186_reset_sources),
> .reset_levels = tegra186_reset_levels,
> @@ -2748,6 +2757,8 @@ static const struct tegra_pmc_soc tegra194_pmc_soc = {
> .regs = &tegra186_pmc_regs,
> .init = NULL,
> .setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
> + .irq_set_wake = tegra186_pmc_irq_set_wake,
> + .irq_set_type = tegra186_pmc_irq_set_type,
> .num_wake_events = ARRAY_SIZE(tegra194_wake_events),
> .wake_events = tegra194_wake_events,
> };
> --
> 2.7.4
>


Attachments:
(No filename) (3.84 kB)
signature.asc (849.00 B)
Download all attachments

2019-05-29 15:33:42

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support

29.05.2019 2:08, Sowjanya Komatineni пишет:
> This patch adds suspend and resume support for Tegra pinctrl driver
> and registers them to syscore so the pinmux settings are restored
> before the devices resume.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra.c | 68 +++++++++++++++++++++++++++++++-
> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra210.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
> 7 files changed, 75 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index a5008c066bac..bdc47e62c457 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -28,11 +28,18 @@
> #include <linux/pinctrl/pinmux.h>
> #include <linux/pinctrl/pinconf.h>
> #include <linux/slab.h>
> +#include <linux/syscore_ops.h>
>
> #include "../core.h"
> #include "../pinctrl-utils.h"
> #include "pinctrl-tegra.h"
>
> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
> +#define EMMC_DPD_PARKING (0x1fff << 14)
> +
> +static struct tegra_pmx *pmx;
> +
> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
> {
> return readl(pmx->regs[bank] + reg);
> @@ -629,6 +636,50 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> }
> }
>
> +static int __maybe_unused tegra_pinctrl_suspend(void)
> +{
> + u32 *backup_regs = pmx->backup_regs;
> + u32 *regs;
> + int i, j;
> +
> + for (i = 0; i < pmx->nbanks; i++) {
> + regs = pmx->regs[i];
> + for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
> + *backup_regs++ = readl(regs++);
> + }
> +
> + return pinctrl_force_sleep(pmx->pctl);
> +}
> +
> +static void __maybe_unused tegra_pinctrl_resume(void)
> +{
> + u32 *backup_regs = pmx->backup_regs;
> + u32 *regs;
> + u32 val;
> + int i, j;
> +
> + for (i = 0; i < pmx->nbanks; i++) {
> + regs = pmx->regs[i];
> + for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
> + writel(*backup_regs++, regs++);
> + }
> +
> + if (pmx->soc->has_park_padcfg) {
> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
> + val &= ~EMMC_DPD_PARKING;
> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
> +
> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
> + val &= ~EMMC_DPD_PARKING;
> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
> + }
> +}
>

But the CFGPADCTRL registers are already programmed by restoring the
backup_regs and hence the relevant EMMC's are already unparked. Hence
why do you need to force-unpark both of the EMMC's? What if EMMC is
unpopulated on a board, why do you need to unpark it then?

--
Dmitry

2019-05-29 18:17:29

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support


On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>> This patch adds suspend and resume support for Tegra pinctrl driver
>> and registers them to syscore so the pinmux settings are restored
>> before the devices resume.
>>
>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>> ---
>> drivers/pinctrl/tegra/pinctrl-tegra.c | 68 +++++++++++++++++++++++++++++++-
>> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 1 +
>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
>> 7 files changed, 75 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> index a5008c066bac..bdc47e62c457 100644
>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> @@ -28,11 +28,18 @@
>> #include <linux/pinctrl/pinmux.h>
>> #include <linux/pinctrl/pinconf.h>
>> #include <linux/slab.h>
>> +#include <linux/syscore_ops.h>
>>
>> #include "../core.h"
>> #include "../pinctrl-utils.h"
>> #include "pinctrl-tegra.h"
>>
>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
>> +#define EMMC_DPD_PARKING (0x1fff << 14)
>> +
>> +static struct tegra_pmx *pmx;
>> +
>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
>> {
>> return readl(pmx->regs[bank] + reg);
>> @@ -629,6 +636,50 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>> }
>> }
>>
>> +static int __maybe_unused tegra_pinctrl_suspend(void)
>> +{
>> + u32 *backup_regs = pmx->backup_regs;
>> + u32 *regs;
>> + int i, j;
>> +
>> + for (i = 0; i < pmx->nbanks; i++) {
>> + regs = pmx->regs[i];
>> + for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>> + *backup_regs++ = readl(regs++);
>> + }
>> +
>> + return pinctrl_force_sleep(pmx->pctl);
>> +}
>> +
>> +static void __maybe_unused tegra_pinctrl_resume(void)
>> +{
>> + u32 *backup_regs = pmx->backup_regs;
>> + u32 *regs;
>> + u32 val;
>> + int i, j;
>> +
>> + for (i = 0; i < pmx->nbanks; i++) {
>> + regs = pmx->regs[i];
>> + for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>> + writel(*backup_regs++, regs++);
>> + }
>> +
>> + if (pmx->soc->has_park_padcfg) {
>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>> + val &= ~EMMC_DPD_PARKING;
>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>> +
>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>> + val &= ~EMMC_DPD_PARKING;
>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>> + }
>> +}
>>
> But the CFGPADCTRL registers are already programmed by restoring the
> backup_regs and hence the relevant EMMC's are already unparked. Hence
> why do you need to force-unpark both of the EMMC's? What if EMMC is
> unpopulated on a board, why do you need to unpark it then?

PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and EMMC4_PAD_CFGPADCTRL)
are not part of pinmux.

They are part of CFGPADCTRL register so pinctrl driver pingroup doesn't
include these registers.

backup_regs doesn't take care of this and need to handled separately for
Tegra210.


During resume we have to clear PARK bit for the pads on Tegra210 and
this is not related to presence/absence of eMMC on the board.

PAD is parked during LP0 entry to have it in DPD mode and it stays in
DPD till its cleared by SW on resume.

2019-05-29 19:35:14

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support

29.05.2019 21:14, Sowjanya Komatineni пишет:
>
> On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
>> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>> and registers them to syscore so the pinmux settings are restored
>>> before the devices resume.
>>>
>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>> ---
>>>   drivers/pinctrl/tegra/pinctrl-tegra.c    | 68
>>> +++++++++++++++++++++++++++++++-
>>>   drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
>>>   drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>   drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>   drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>   drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
>>>   drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>   7 files changed, 75 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> index a5008c066bac..bdc47e62c457 100644
>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> @@ -28,11 +28,18 @@
>>>   #include <linux/pinctrl/pinmux.h>
>>>   #include <linux/pinctrl/pinconf.h>
>>>   #include <linux/slab.h>
>>> +#include <linux/syscore_ops.h>
>>>     #include "../core.h"
>>>   #include "../pinctrl-utils.h"
>>>   #include "pinctrl-tegra.h"
>>>   +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>> +
>>> +static struct tegra_pmx *pmx;
>>> +
>>>   static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
>>>   {
>>>       return readl(pmx->regs[bank] + reg);
>>> @@ -629,6 +636,50 @@ static void
>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>       }
>>>   }
>>>   +static int __maybe_unused tegra_pinctrl_suspend(void)
>>> +{
>>> +    u32 *backup_regs = pmx->backup_regs;
>>> +    u32 *regs;
>>> +    int i, j;
>>> +
>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>> +        regs = pmx->regs[i];
>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>> +            *backup_regs++ = readl(regs++);
>>> +    }
>>> +
>>> +    return pinctrl_force_sleep(pmx->pctl);
>>> +}
>>> +
>>> +static void __maybe_unused tegra_pinctrl_resume(void)
>>> +{
>>> +    u32 *backup_regs = pmx->backup_regs;
>>> +    u32 *regs;
>>> +    u32 val;
>>> +    int i, j;
>>> +
>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>> +        regs = pmx->regs[i];
>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>> +            writel(*backup_regs++, regs++);
>>> +    }
>>> +
>>> +    if (pmx->soc->has_park_padcfg) {
>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>> +        val &= ~EMMC_DPD_PARKING;
>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>> +
>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>> +        val &= ~EMMC_DPD_PARKING;
>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>> +    }
>>> +}
>>>
>> But the CFGPADCTRL registers are already programmed by restoring the
>> backup_regs and hence the relevant EMMC's are already unparked. Hence
>> why do you need to force-unpark both of the EMMC's? What if EMMC is
>> unpopulated on a board, why do you need to unpark it then?
>
> PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and EMMC4_PAD_CFGPADCTRL)
> are not part of pinmux.
>
> They are part of CFGPADCTRL register so pinctrl driver pingroup doesn't
> include these registers.

I'm looking at the tegra210_groups and it clearly has these both
registers as a part of pinctrl setup because the rest of the bits
configure drive of the pads.

From pinctrl-tegra210.c:

#define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */

DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2),
DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2),

...

0xa9c - 0x8d4 = 0x1c8
0xab4 - 0x8d4 = 0x1e0

Hence the PARK bits are already getting unset by restoring the
backup_regs because the CFGPADCTRL registers are a part of the "bank 0"
registers.

Am I still missing something?

> backup_regs doesn't take care of this and need to handled separately for
> Tegra210.
>
>
> During resume we have to clear PARK bit for the pads on Tegra210 and
> this is not related to presence/absence of eMMC on the board.

Okay, thank you for the clarification.

> PAD is parked during LP0 entry to have it in DPD mode and it stays in
> DPD till its cleared by SW on resume.

Yes, this is documented in the public TRM. My main point is that it
looks like the PARK bits are unneedlessly getting unset twice in your
code (and it still looks like that to me).

2019-05-29 20:14:40

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support


On 5/29/19 12:32 PM, Dmitry Osipenko wrote:
> 29.05.2019 21:14, Sowjanya Komatineni пишет:
>> On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
>>> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>> and registers them to syscore so the pinmux settings are restored
>>>> before the devices resume.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>> ---
>>>>   drivers/pinctrl/tegra/pinctrl-tegra.c    | 68
>>>> +++++++++++++++++++++++++++++++-
>>>>   drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
>>>>   drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>>   drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>>   drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>>   drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
>>>>   drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>>   7 files changed, 75 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> index a5008c066bac..bdc47e62c457 100644
>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> @@ -28,11 +28,18 @@
>>>>   #include <linux/pinctrl/pinmux.h>
>>>>   #include <linux/pinctrl/pinconf.h>
>>>>   #include <linux/slab.h>
>>>> +#include <linux/syscore_ops.h>
>>>>     #include "../core.h"
>>>>   #include "../pinctrl-utils.h"
>>>>   #include "pinctrl-tegra.h"
>>>>   +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>>> +
>>>> +static struct tegra_pmx *pmx;
>>>> +
>>>>   static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
>>>>   {
>>>>       return readl(pmx->regs[bank] + reg);
>>>> @@ -629,6 +636,50 @@ static void
>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>       }
>>>>   }
>>>>   +static int __maybe_unused tegra_pinctrl_suspend(void)
>>>> +{
>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>> +    u32 *regs;
>>>> +    int i, j;
>>>> +
>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>> +        regs = pmx->regs[i];
>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>> +            *backup_regs++ = readl(regs++);
>>>> +    }
>>>> +
>>>> +    return pinctrl_force_sleep(pmx->pctl);
>>>> +}
>>>> +
>>>> +static void __maybe_unused tegra_pinctrl_resume(void)
>>>> +{
>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>> +    u32 *regs;
>>>> +    u32 val;
>>>> +    int i, j;
>>>> +
>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>> +        regs = pmx->regs[i];
>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>> +            writel(*backup_regs++, regs++);
>>>> +    }
>>>> +
>>>> +    if (pmx->soc->has_park_padcfg) {
>>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>> +        val &= ~EMMC_DPD_PARKING;
>>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>> +
>>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>> +        val &= ~EMMC_DPD_PARKING;
>>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>> +    }
>>>> +}
>>>>
>>> But the CFGPADCTRL registers are already programmed by restoring the
>>> backup_regs and hence the relevant EMMC's are already unparked. Hence
>>> why do you need to force-unpark both of the EMMC's? What if EMMC is
>>> unpopulated on a board, why do you need to unpark it then?
>> PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and EMMC4_PAD_CFGPADCTRL)
>> are not part of pinmux.
>>
>> They are part of CFGPADCTRL register so pinctrl driver pingroup doesn't
>> include these registers.
> I'm looking at the tegra210_groups and it clearly has these both
> registers as a part of pinctrl setup because the rest of the bits
> configure drive of the pads.
>
> From pinctrl-tegra210.c:
>
> #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */
>
> DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2),
> DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2),
>
> ...
>
> 0xa9c - 0x8d4 = 0x1c8
> 0xab4 - 0x8d4 = 0x1e0
>
> Hence the PARK bits are already getting unset by restoring the
> backup_regs because the CFGPADCTRL registers are a part of the "bank 0"
> registers.
>
> Am I still missing something?

DRV_PINGROUP parked_bit is -1 and will not be programmed so store and
restore will not take care of it.

Also EMMC PADCFG is the only padcfg register which has parked bit and
for other IO pads its part of pinmux

>> backup_regs doesn't take care of this and need to handled separately for
>> Tegra210.
>>
>>
>> During resume we have to clear PARK bit for the pads on Tegra210 and
>> this is not related to presence/absence of eMMC on the board.
> Okay, thank you for the clarification.
>
>> PAD is parked during LP0 entry to have it in DPD mode and it stays in
>> DPD till its cleared by SW on resume.
> Yes, this is documented in the public TRM. My main point is that it
> looks like the PARK bits are unneedlessly getting unset twice in your
> code (and it still looks like that to me).

2019-05-29 20:48:44

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support

29.05.2019 23:11, Sowjanya Komatineni пишет:
>
> On 5/29/19 12:32 PM, Dmitry Osipenko wrote:
>> 29.05.2019 21:14, Sowjanya Komatineni пишет:
>>> On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
>>>> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>> and registers them to syscore so the pinmux settings are restored
>>>>> before the devices resume.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>> ---
>>>>>    drivers/pinctrl/tegra/pinctrl-tegra.c    | 68
>>>>> +++++++++++++++++++++++++++++++-
>>>>>    drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
>>>>>    drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>>>    drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>>>    drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>>>    drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
>>>>>    drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>>>    7 files changed, 75 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> index a5008c066bac..bdc47e62c457 100644
>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> @@ -28,11 +28,18 @@
>>>>>    #include <linux/pinctrl/pinmux.h>
>>>>>    #include <linux/pinctrl/pinconf.h>
>>>>>    #include <linux/slab.h>
>>>>> +#include <linux/syscore_ops.h>
>>>>>      #include "../core.h"
>>>>>    #include "../pinctrl-utils.h"
>>>>>    #include "pinctrl-tegra.h"
>>>>>    +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>>>> +
>>>>> +static struct tegra_pmx *pmx;
>>>>> +
>>>>>    static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32
>>>>> reg)
>>>>>    {
>>>>>        return readl(pmx->regs[bank] + reg);
>>>>> @@ -629,6 +636,50 @@ static void
>>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>        }
>>>>>    }
>>>>>    +static int __maybe_unused tegra_pinctrl_suspend(void)
>>>>> +{
>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>> +    u32 *regs;
>>>>> +    int i, j;
>>>>> +
>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>> +        regs = pmx->regs[i];
>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>> +            *backup_regs++ = readl(regs++);
>>>>> +    }
>>>>> +
>>>>> +    return pinctrl_force_sleep(pmx->pctl);
>>>>> +}
>>>>> +
>>>>> +static void __maybe_unused tegra_pinctrl_resume(void)
>>>>> +{
>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>> +    u32 *regs;
>>>>> +    u32 val;
>>>>> +    int i, j;
>>>>> +
>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>> +        regs = pmx->regs[i];
>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>> +            writel(*backup_regs++, regs++);
>>>>> +    }
>>>>> +
>>>>> +    if (pmx->soc->has_park_padcfg) {
>>>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>> +
>>>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>> +    }
>>>>> +}
>>>>>
>>>> But the CFGPADCTRL registers are already programmed by restoring the
>>>> backup_regs and hence the relevant EMMC's are already unparked. Hence
>>>> why do you need to force-unpark both of the EMMC's? What if EMMC is
>>>> unpopulated on a board, why do you need to unpark it then?
>>> PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and EMMC4_PAD_CFGPADCTRL)
>>> are not part of pinmux.
>>>
>>> They are part of CFGPADCTRL register so pinctrl driver pingroup doesn't
>>> include these registers.
>> I'm looking at the tegra210_groups and it clearly has these both
>> registers as a part of pinctrl setup because the rest of the bits
>> configure drive of the pads.
>>
>>  From pinctrl-tegra210.c:
>>
>> #define DRV_PINGROUP_REG_A        0x8d4    /* bank 0 */
>>
>> DRV_PINGROUP(sdmmc2, 0xa9c, 2,  6,  8,  6,  28, 2,  30, 2),
>> DRV_PINGROUP(sdmmc4, 0xab4, 2,  6,  8,  6,  28, 2,  30, 2),
>>
>> ...
>>
>> 0xa9c - 0x8d4 = 0x1c8
>> 0xab4 - 0x8d4 = 0x1e0
>>
>> Hence the PARK bits are already getting unset by restoring the
>> backup_regs because the CFGPADCTRL registers are a part of the "bank 0"
>> registers.
>>
>> Am I still missing something?
>
> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and
> restore will not take care of it.
>
> Also EMMC PADCFG is the only padcfg register which has parked bit and
> for other IO pads its part of pinmux

You're storing raw values of all of the PINCTRL registers and then
restoring the raw values (if I'm not misreading that part on the patch),
it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK bits.

In a result, the backup_regs array contains raw CFGPADCTRL value with
the PARK bits being unset on store, that value is written out on the
restore as-is and hence the PARK bits are getting unset as well.

And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a
driver's drawback that need to be addressed.

2019-05-29 20:59:45

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support


On 5/29/19 1:47 PM, Dmitry Osipenko wrote:
> 29.05.2019 23:11, Sowjanya Komatineni пишет:
>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote:
>>> 29.05.2019 21:14, Sowjanya Komatineni пишет:
>>>> On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
>>>>> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>>> and registers them to syscore so the pinmux settings are restored
>>>>>> before the devices resume.
>>>>>>
>>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>>> ---
>>>>>>    drivers/pinctrl/tegra/pinctrl-tegra.c    | 68
>>>>>> +++++++++++++++++++++++++++++++-
>>>>>>    drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
>>>>>>    drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>>>>    drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>>>>    drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>>>>    drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
>>>>>>    drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>>>>    7 files changed, 75 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>> index a5008c066bac..bdc47e62c457 100644
>>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>> @@ -28,11 +28,18 @@
>>>>>>    #include <linux/pinctrl/pinmux.h>
>>>>>>    #include <linux/pinctrl/pinconf.h>
>>>>>>    #include <linux/slab.h>
>>>>>> +#include <linux/syscore_ops.h>
>>>>>>      #include "../core.h"
>>>>>>    #include "../pinctrl-utils.h"
>>>>>>    #include "pinctrl-tegra.h"
>>>>>>    +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>>>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>>>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>>>>> +
>>>>>> +static struct tegra_pmx *pmx;
>>>>>> +
>>>>>>    static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32
>>>>>> reg)
>>>>>>    {
>>>>>>        return readl(pmx->regs[bank] + reg);
>>>>>> @@ -629,6 +636,50 @@ static void
>>>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>>        }
>>>>>>    }
>>>>>>    +static int __maybe_unused tegra_pinctrl_suspend(void)
>>>>>> +{
>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>> +    u32 *regs;
>>>>>> +    int i, j;
>>>>>> +
>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>> +        regs = pmx->regs[i];
>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>> +            *backup_regs++ = readl(regs++);
>>>>>> +    }
>>>>>> +
>>>>>> +    return pinctrl_force_sleep(pmx->pctl);
>>>>>> +}
>>>>>> +
>>>>>> +static void __maybe_unused tegra_pinctrl_resume(void)
>>>>>> +{
>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>> +    u32 *regs;
>>>>>> +    u32 val;
>>>>>> +    int i, j;
>>>>>> +
>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>> +        regs = pmx->regs[i];
>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>> +            writel(*backup_regs++, regs++);
>>>>>> +    }
>>>>>> +
>>>>>> +    if (pmx->soc->has_park_padcfg) {
>>>>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>> +
>>>>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>> +    }
>>>>>> +}
>>>>>>
>>>>> But the CFGPADCTRL registers are already programmed by restoring the
>>>>> backup_regs and hence the relevant EMMC's are already unparked. Hence
>>>>> why do you need to force-unpark both of the EMMC's? What if EMMC is
>>>>> unpopulated on a board, why do you need to unpark it then?
>>>> PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and EMMC4_PAD_CFGPADCTRL)
>>>> are not part of pinmux.
>>>>
>>>> They are part of CFGPADCTRL register so pinctrl driver pingroup doesn't
>>>> include these registers.
>>> I'm looking at the tegra210_groups and it clearly has these both
>>> registers as a part of pinctrl setup because the rest of the bits
>>> configure drive of the pads.
>>>
>>>  From pinctrl-tegra210.c:
>>>
>>> #define DRV_PINGROUP_REG_A        0x8d4    /* bank 0 */
>>>
>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2,  6,  8,  6,  28, 2,  30, 2),
>>> DRV_PINGROUP(sdmmc4, 0xab4, 2,  6,  8,  6,  28, 2,  30, 2),
>>>
>>> ...
>>>
>>> 0xa9c - 0x8d4 = 0x1c8
>>> 0xab4 - 0x8d4 = 0x1e0
>>>
>>> Hence the PARK bits are already getting unset by restoring the
>>> backup_regs because the CFGPADCTRL registers are a part of the "bank 0"
>>> registers.
>>>
>>> Am I still missing something?
>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and
>> restore will not take care of it.
>>
>> Also EMMC PADCFG is the only padcfg register which has parked bit and
>> for other IO pads its part of pinmux
> You're storing raw values of all of the PINCTRL registers and then
> restoring the raw values (if I'm not misreading that part on the patch),
> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK bits.
>
> In a result, the backup_regs array contains raw CFGPADCTRL value with
> the PARK bits being unset on store, that value is written out on the
> restore as-is and hence the PARK bits are getting unset as well.
>
> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a
> driver's drawback that need to be addressed.

Parked bits from padcfg are available only for couple of EMMC registers.

default PARK bits are set so stored value contains park bit set. on
resume, after restoring park bit is cleared.

on subsequence DPD entry, stored value contains park bit 0 and HW clamps
park bit to logic 1 during DPD entry and cleared again on resume.


2019-05-29 21:09:28

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support


On 5/29/19 1:56 PM, Sowjanya Komatineni wrote:
>
> On 5/29/19 1:47 PM, Dmitry Osipenko wrote:
>> 29.05.2019 23:11, Sowjanya Komatineni пишет:
>>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote:
>>>> 29.05.2019 21:14, Sowjanya Komatineni пишет:
>>>>> On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
>>>>>> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>>>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>>>> and registers them to syscore so the pinmux settings are restored
>>>>>>> before the devices resume.
>>>>>>>
>>>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>>>> ---
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra.c    | 68
>>>>>>> +++++++++++++++++++++++++++++++-
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>>>>>     7 files changed, 75 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> index a5008c066bac..bdc47e62c457 100644
>>>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> @@ -28,11 +28,18 @@
>>>>>>>     #include <linux/pinctrl/pinmux.h>
>>>>>>>     #include <linux/pinctrl/pinconf.h>
>>>>>>>     #include <linux/slab.h>
>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>       #include "../core.h"
>>>>>>>     #include "../pinctrl-utils.h"
>>>>>>>     #include "pinctrl-tegra.h"
>>>>>>>     +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>>>>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>>>>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>>>>>> +
>>>>>>> +static struct tegra_pmx *pmx;
>>>>>>> +
>>>>>>>     static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank,
>>>>>>> u32
>>>>>>> reg)
>>>>>>>     {
>>>>>>>         return readl(pmx->regs[bank] + reg);
>>>>>>> @@ -629,6 +636,50 @@ static void
>>>>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>>>         }
>>>>>>>     }
>>>>>>>     +static int __maybe_unused tegra_pinctrl_suspend(void)
>>>>>>> +{
>>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>>> +    u32 *regs;
>>>>>>> +    int i, j;
>>>>>>> +
>>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>>> +        regs = pmx->regs[i];
>>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>>> +            *backup_regs++ = readl(regs++);
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    return pinctrl_force_sleep(pmx->pctl);
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void __maybe_unused tegra_pinctrl_resume(void)
>>>>>>> +{
>>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>>> +    u32 *regs;
>>>>>>> +    u32 val;
>>>>>>> +    int i, j;
>>>>>>> +
>>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>>> +        regs = pmx->regs[i];
>>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>>> +            writel(*backup_regs++, regs++);
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    if (pmx->soc->has_park_padcfg) {
>>>>>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>> +
>>>>>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>> +    }
>>>>>>> +}
>>>>>>>
>>>>>> But the CFGPADCTRL registers are already programmed by restoring the
>>>>>> backup_regs and hence the relevant EMMC's are already unparked.
>>>>>> Hence
>>>>>> why do you need to force-unpark both of the EMMC's? What if EMMC is
>>>>>> unpopulated on a board, why do you need to unpark it then?
>>>>> PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and
>>>>> EMMC4_PAD_CFGPADCTRL)
>>>>> are not part of pinmux.
>>>>>
>>>>> They are part of CFGPADCTRL register so pinctrl driver pingroup
>>>>> doesn't
>>>>> include these registers.
>>>> I'm looking at the tegra210_groups and it clearly has these both
>>>> registers as a part of pinctrl setup because the rest of the bits
>>>> configure drive of the pads.
>>>>
>>>>   From pinctrl-tegra210.c:
>>>>
>>>> #define DRV_PINGROUP_REG_A        0x8d4    /* bank 0 */
>>>>
>>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2,  6,  8,  6,  28, 2,  30, 2),
>>>> DRV_PINGROUP(sdmmc4, 0xab4, 2,  6,  8,  6,  28, 2,  30, 2),
>>>>
>>>> ...
>>>>
>>>> 0xa9c - 0x8d4 = 0x1c8
>>>> 0xab4 - 0x8d4 = 0x1e0
>>>>
>>>> Hence the PARK bits are already getting unset by restoring the
>>>> backup_regs because the CFGPADCTRL registers are a part of the
>>>> "bank 0"
>>>> registers.
>>>>
>>>> Am I still missing something?
>>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and
>>> restore will not take care of it.
>>>
>>> Also EMMC PADCFG is the only padcfg register which has parked bit and
>>> for other IO pads its part of pinmux
>> You're storing raw values of all of the PINCTRL registers and then
>> restoring the raw values (if I'm not misreading that part on the patch),
>> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK
>> bits.
>>
>> In a result, the backup_regs array contains raw CFGPADCTRL value with
>> the PARK bits being unset on store, that value is written out on the
>> restore as-is and hence the PARK bits are getting unset as well.
>>
>> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a
>> driver's drawback that need to be addressed.
>
> Parked bits from padcfg are available only for couple of EMMC registers.
>
> default PARK bits are set so stored value contains park bit set. on
> resume, after restoring park bit is cleared.
>
> on subsequence DPD entry, stored value contains park bit 0 and HW
> clamps park bit to logic 1 during DPD entry and cleared again on resume.
>
>
Other IOs park bit in pinmux gets cleared thru
tegra_pinctrl_clear_parked_bits on probe and during suspend register
values saved contains park bit = 0 which is same when restored on DPD
resume.

clearing park bit during resume for EMMC pads is same as clearing it
during probe which is then saved during suspend and restored on resume
similar to pinmux registers.

So for more readability, probably can clear parked bit for EMMC during
pinctrl_clear_parked_bits instead of on resume.

2019-05-29 21:29:08

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support

29.05.2019 23:56, Sowjanya Komatineni пишет:
>
> On 5/29/19 1:47 PM, Dmitry Osipenko wrote:
>> 29.05.2019 23:11, Sowjanya Komatineni пишет:
>>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote:
>>>> 29.05.2019 21:14, Sowjanya Komatineni пишет:
>>>>> On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
>>>>>> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>>>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>>>> and registers them to syscore so the pinmux settings are restored
>>>>>>> before the devices resume.
>>>>>>>
>>>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>>>> ---
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra.c    | 68
>>>>>>> +++++++++++++++++++++++++++++++-
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>>>>>     7 files changed, 75 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> index a5008c066bac..bdc47e62c457 100644
>>>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> @@ -28,11 +28,18 @@
>>>>>>>     #include <linux/pinctrl/pinmux.h>
>>>>>>>     #include <linux/pinctrl/pinconf.h>
>>>>>>>     #include <linux/slab.h>
>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>       #include "../core.h"
>>>>>>>     #include "../pinctrl-utils.h"
>>>>>>>     #include "pinctrl-tegra.h"
>>>>>>>     +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>>>>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>>>>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>>>>>> +
>>>>>>> +static struct tegra_pmx *pmx;
>>>>>>> +
>>>>>>>     static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32
>>>>>>> reg)
>>>>>>>     {
>>>>>>>         return readl(pmx->regs[bank] + reg);
>>>>>>> @@ -629,6 +636,50 @@ static void
>>>>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>>>         }
>>>>>>>     }
>>>>>>>     +static int __maybe_unused tegra_pinctrl_suspend(void)
>>>>>>> +{
>>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>>> +    u32 *regs;
>>>>>>> +    int i, j;
>>>>>>> +
>>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>>> +        regs = pmx->regs[i];
>>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>>> +            *backup_regs++ = readl(regs++);
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    return pinctrl_force_sleep(pmx->pctl);
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void __maybe_unused tegra_pinctrl_resume(void)
>>>>>>> +{
>>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>>> +    u32 *regs;
>>>>>>> +    u32 val;
>>>>>>> +    int i, j;
>>>>>>> +
>>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>>> +        regs = pmx->regs[i];
>>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>>> +            writel(*backup_regs++, regs++);
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    if (pmx->soc->has_park_padcfg) {
>>>>>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>> +
>>>>>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>> +    }
>>>>>>> +}
>>>>>>>
>>>>>> But the CFGPADCTRL registers are already programmed by restoring the
>>>>>> backup_regs and hence the relevant EMMC's are already unparked. Hence
>>>>>> why do you need to force-unpark both of the EMMC's? What if EMMC is
>>>>>> unpopulated on a board, why do you need to unpark it then?
>>>>> PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and
>>>>> EMMC4_PAD_CFGPADCTRL)
>>>>> are not part of pinmux.
>>>>>
>>>>> They are part of CFGPADCTRL register so pinctrl driver pingroup
>>>>> doesn't
>>>>> include these registers.
>>>> I'm looking at the tegra210_groups and it clearly has these both
>>>> registers as a part of pinctrl setup because the rest of the bits
>>>> configure drive of the pads.
>>>>
>>>>   From pinctrl-tegra210.c:
>>>>
>>>> #define DRV_PINGROUP_REG_A        0x8d4    /* bank 0 */
>>>>
>>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2,  6,  8,  6,  28, 2,  30, 2),
>>>> DRV_PINGROUP(sdmmc4, 0xab4, 2,  6,  8,  6,  28, 2,  30, 2),
>>>>
>>>> ...
>>>>
>>>> 0xa9c - 0x8d4 = 0x1c8
>>>> 0xab4 - 0x8d4 = 0x1e0
>>>>
>>>> Hence the PARK bits are already getting unset by restoring the
>>>> backup_regs because the CFGPADCTRL registers are a part of the "bank 0"
>>>> registers.
>>>>
>>>> Am I still missing something?
>>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and
>>> restore will not take care of it.
>>>
>>> Also EMMC PADCFG is the only padcfg register which has parked bit and
>>> for other IO pads its part of pinmux
>> You're storing raw values of all of the PINCTRL registers and then
>> restoring the raw values (if I'm not misreading that part on the patch),
>> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK
>> bits.
>>
>> In a result, the backup_regs array contains raw CFGPADCTRL value with
>> the PARK bits being unset on store, that value is written out on the
>> restore as-is and hence the PARK bits are getting unset as well.
>>
>> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a
>> driver's drawback that need to be addressed.
>
> Parked bits from padcfg are available only for couple of EMMC registers.
>
> default PARK bits are set so stored value contains park bit set. on
> resume, after restoring park bit is cleared.

TRM says that the PARK bits are set on HW reset (and on DPD IIUC) and
then SW should unset the bits. I assume that the PARK bits of the EMMC
pads are unset by bootloader and that's why it doesn't cause problems
for kernel, in oppose to PARK bits of the rest of pinmux that are unset
by the driver in tegra_pinctrl_clear_parked_bits() on driver's probe.

> on subsequence DPD entry, stored value contains park bit 0 and HW clamps
> park bit to logic 1 during DPD entry and cleared again on resume.

I assume that EMMC won't work properly if pads are "parked" and the PARK
bits should be in unset state on kernel boot-up as I wrote above, hence
stored value should always contain 0 for the EMMC PARK bits. No?

2019-05-29 21:31:34

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support


On 5/29/19 2:25 PM, Dmitry Osipenko wrote:
> 29.05.2019 23:56, Sowjanya Komatineni пишет:
>> On 5/29/19 1:47 PM, Dmitry Osipenko wrote:
>>> 29.05.2019 23:11, Sowjanya Komatineni пишет:
>>>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote:
>>>>> 29.05.2019 21:14, Sowjanya Komatineni пишет:
>>>>>> On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
>>>>>>> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>>>>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>>>>> and registers them to syscore so the pinmux settings are restored
>>>>>>>> before the devices resume.
>>>>>>>>
>>>>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>>>>> ---
>>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra.c    | 68
>>>>>>>> +++++++++++++++++++++++++++++++-
>>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
>>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
>>>>>>>>     drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>>>>>>     7 files changed, 75 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>> index a5008c066bac..bdc47e62c457 100644
>>>>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>> @@ -28,11 +28,18 @@
>>>>>>>>     #include <linux/pinctrl/pinmux.h>
>>>>>>>>     #include <linux/pinctrl/pinconf.h>
>>>>>>>>     #include <linux/slab.h>
>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>       #include "../core.h"
>>>>>>>>     #include "../pinctrl-utils.h"
>>>>>>>>     #include "pinctrl-tegra.h"
>>>>>>>>     +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>>>>>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>>>>>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>>>>>>> +
>>>>>>>> +static struct tegra_pmx *pmx;
>>>>>>>> +
>>>>>>>>     static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32
>>>>>>>> reg)
>>>>>>>>     {
>>>>>>>>         return readl(pmx->regs[bank] + reg);
>>>>>>>> @@ -629,6 +636,50 @@ static void
>>>>>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>>>>         }
>>>>>>>>     }
>>>>>>>>     +static int __maybe_unused tegra_pinctrl_suspend(void)
>>>>>>>> +{
>>>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>>>> +    u32 *regs;
>>>>>>>> +    int i, j;
>>>>>>>> +
>>>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>>>> +        regs = pmx->regs[i];
>>>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>>>> +            *backup_regs++ = readl(regs++);
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    return pinctrl_force_sleep(pmx->pctl);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void __maybe_unused tegra_pinctrl_resume(void)
>>>>>>>> +{
>>>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>>>> +    u32 *regs;
>>>>>>>> +    u32 val;
>>>>>>>> +    int i, j;
>>>>>>>> +
>>>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>>>> +        regs = pmx->regs[i];
>>>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>>>> +            writel(*backup_regs++, regs++);
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    if (pmx->soc->has_park_padcfg) {
>>>>>>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>>> +
>>>>>>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>>> +    }
>>>>>>>> +}
>>>>>>>>
>>>>>>> But the CFGPADCTRL registers are already programmed by restoring the
>>>>>>> backup_regs and hence the relevant EMMC's are already unparked. Hence
>>>>>>> why do you need to force-unpark both of the EMMC's? What if EMMC is
>>>>>>> unpopulated on a board, why do you need to unpark it then?
>>>>>> PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and
>>>>>> EMMC4_PAD_CFGPADCTRL)
>>>>>> are not part of pinmux.
>>>>>>
>>>>>> They are part of CFGPADCTRL register so pinctrl driver pingroup
>>>>>> doesn't
>>>>>> include these registers.
>>>>> I'm looking at the tegra210_groups and it clearly has these both
>>>>> registers as a part of pinctrl setup because the rest of the bits
>>>>> configure drive of the pads.
>>>>>
>>>>>   From pinctrl-tegra210.c:
>>>>>
>>>>> #define DRV_PINGROUP_REG_A        0x8d4    /* bank 0 */
>>>>>
>>>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2,  6,  8,  6,  28, 2,  30, 2),
>>>>> DRV_PINGROUP(sdmmc4, 0xab4, 2,  6,  8,  6,  28, 2,  30, 2),
>>>>>
>>>>> ...
>>>>>
>>>>> 0xa9c - 0x8d4 = 0x1c8
>>>>> 0xab4 - 0x8d4 = 0x1e0
>>>>>
>>>>> Hence the PARK bits are already getting unset by restoring the
>>>>> backup_regs because the CFGPADCTRL registers are a part of the "bank 0"
>>>>> registers.
>>>>>
>>>>> Am I still missing something?
>>>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and
>>>> restore will not take care of it.
>>>>
>>>> Also EMMC PADCFG is the only padcfg register which has parked bit and
>>>> for other IO pads its part of pinmux
>>> You're storing raw values of all of the PINCTRL registers and then
>>> restoring the raw values (if I'm not misreading that part on the patch),
>>> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK
>>> bits.
>>>
>>> In a result, the backup_regs array contains raw CFGPADCTRL value with
>>> the PARK bits being unset on store, that value is written out on the
>>> restore as-is and hence the PARK bits are getting unset as well.
>>>
>>> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a
>>> driver's drawback that need to be addressed.
>> Parked bits from padcfg are available only for couple of EMMC registers.
>>
>> default PARK bits are set so stored value contains park bit set. on
>> resume, after restoring park bit is cleared.
> TRM says that the PARK bits are set on HW reset (and on DPD IIUC) and
> then SW should unset the bits. I assume that the PARK bits of the EMMC
> pads are unset by bootloader and that's why it doesn't cause problems
> for kernel, in oppose to PARK bits of the rest of pinmux that are unset
> by the driver in tegra_pinctrl_clear_parked_bits() on driver's probe.
>
>> on subsequence DPD entry, stored value contains park bit 0 and HW clamps
>> park bit to logic 1 during DPD entry and cleared again on resume.
> I assume that EMMC won't work properly if pads are "parked" and the PARK
> bits should be in unset state on kernel boot-up as I wrote above, hence
> stored value should always contain 0 for the EMMC PARK bits. No?

On bootup, pads still works. PARK bit is to keep pads in DPD once they
enter into DPD (LP0) and on resume they need to be cleared to be out of DPD


2019-05-29 21:35:52

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support

30.05.2019 0:27, Sowjanya Komatineni пишет:
>
> On 5/29/19 2:25 PM, Dmitry Osipenko wrote:
>> 29.05.2019 23:56, Sowjanya Komatineni пишет:
>>> On 5/29/19 1:47 PM, Dmitry Osipenko wrote:
>>>> 29.05.2019 23:11, Sowjanya Komatineni пишет:
>>>>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote:
>>>>>> 29.05.2019 21:14, Sowjanya Komatineni пишет:
>>>>>>> On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
>>>>>>>> 29.05.2019 2:08, Sowjanya Komatineni пишет:
>>>>>>>>> This patch adds suspend and resume support for Tegra pinctrl
>>>>>>>>> driver
>>>>>>>>> and registers them to syscore so the pinmux settings are restored
>>>>>>>>> before the devices resume.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>>>>>> ---
>>>>>>>>>      drivers/pinctrl/tegra/pinctrl-tegra.c    | 68
>>>>>>>>> +++++++++++++++++++++++++++++++-
>>>>>>>>>      drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
>>>>>>>>>      drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>>>>>>>      drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>>>>>>>      drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>>>>>>>      drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
>>>>>>>>>      drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>>>>>>>      7 files changed, 75 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>>> index a5008c066bac..bdc47e62c457 100644
>>>>>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>>> @@ -28,11 +28,18 @@
>>>>>>>>>      #include <linux/pinctrl/pinmux.h>
>>>>>>>>>      #include <linux/pinctrl/pinconf.h>
>>>>>>>>>      #include <linux/slab.h>
>>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>>        #include "../core.h"
>>>>>>>>>      #include "../pinctrl-utils.h"
>>>>>>>>>      #include "pinctrl-tegra.h"
>>>>>>>>>      +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>>>>>>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>>>>>>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>>>>>>>> +
>>>>>>>>> +static struct tegra_pmx *pmx;
>>>>>>>>> +
>>>>>>>>>      static inline u32 pmx_readl(struct tegra_pmx *pmx, u32
>>>>>>>>> bank, u32
>>>>>>>>> reg)
>>>>>>>>>      {
>>>>>>>>>          return readl(pmx->regs[bank] + reg);
>>>>>>>>> @@ -629,6 +636,50 @@ static void
>>>>>>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>>>>>          }
>>>>>>>>>      }
>>>>>>>>>      +static int __maybe_unused tegra_pinctrl_suspend(void)
>>>>>>>>> +{
>>>>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>>>>> +    u32 *regs;
>>>>>>>>> +    int i, j;
>>>>>>>>> +
>>>>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>>>>> +        regs = pmx->regs[i];
>>>>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>>>>> +            *backup_regs++ = readl(regs++);
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>> +    return pinctrl_force_sleep(pmx->pctl);
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void __maybe_unused tegra_pinctrl_resume(void)
>>>>>>>>> +{
>>>>>>>>> +    u32 *backup_regs = pmx->backup_regs;
>>>>>>>>> +    u32 *regs;
>>>>>>>>> +    u32 val;
>>>>>>>>> +    int i, j;
>>>>>>>>> +
>>>>>>>>> +    for (i = 0; i < pmx->nbanks; i++) {
>>>>>>>>> +        regs = pmx->regs[i];
>>>>>>>>> +        for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
>>>>>>>>> +            writel(*backup_regs++, regs++);
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>> +    if (pmx->soc->has_park_padcfg) {
>>>>>>>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>>>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>>>> +
>>>>>>>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>>>>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>>>> +    }
>>>>>>>>> +}
>>>>>>>>>
>>>>>>>> But the CFGPADCTRL registers are already programmed by restoring
>>>>>>>> the
>>>>>>>> backup_regs and hence the relevant EMMC's are already unparked.
>>>>>>>> Hence
>>>>>>>> why do you need to force-unpark both of the EMMC's? What if EMMC is
>>>>>>>> unpopulated on a board, why do you need to unpark it then?
>>>>>>> PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and
>>>>>>> EMMC4_PAD_CFGPADCTRL)
>>>>>>> are not part of pinmux.
>>>>>>>
>>>>>>> They are part of CFGPADCTRL register so pinctrl driver pingroup
>>>>>>> doesn't
>>>>>>> include these registers.
>>>>>> I'm looking at the tegra210_groups and it clearly has these both
>>>>>> registers as a part of pinctrl setup because the rest of the bits
>>>>>> configure drive of the pads.
>>>>>>
>>>>>>    From pinctrl-tegra210.c:
>>>>>>
>>>>>> #define DRV_PINGROUP_REG_A        0x8d4    /* bank 0 */
>>>>>>
>>>>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2,  6,  8,  6,  28, 2,  30, 2),
>>>>>> DRV_PINGROUP(sdmmc4, 0xab4, 2,  6,  8,  6,  28, 2,  30, 2),
>>>>>>
>>>>>> ...
>>>>>>
>>>>>> 0xa9c - 0x8d4 = 0x1c8
>>>>>> 0xab4 - 0x8d4 = 0x1e0
>>>>>>
>>>>>> Hence the PARK bits are already getting unset by restoring the
>>>>>> backup_regs because the CFGPADCTRL registers are a part of the
>>>>>> "bank 0"
>>>>>> registers.
>>>>>>
>>>>>> Am I still missing something?
>>>>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and
>>>>> restore will not take care of it.
>>>>>
>>>>> Also EMMC PADCFG is the only padcfg register which has parked bit and
>>>>> for other IO pads its part of pinmux
>>>> You're storing raw values of all of the PINCTRL registers and then
>>>> restoring the raw values (if I'm not misreading that part on the
>>>> patch),
>>>> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK
>>>> bits.
>>>>
>>>> In a result, the backup_regs array contains raw CFGPADCTRL value with
>>>> the PARK bits being unset on store, that value is written out on the
>>>> restore as-is and hence the PARK bits are getting unset as well.
>>>>
>>>> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a
>>>> driver's drawback that need to be addressed.
>>> Parked bits from padcfg are available only for couple of EMMC registers.
>>>
>>> default PARK bits are set so stored value contains park bit set. on
>>> resume, after restoring park bit is cleared.
>> TRM says that the PARK bits are set on HW reset (and on DPD IIUC) and
>> then SW should unset the bits. I assume that the PARK bits of the EMMC
>> pads are unset by bootloader and that's why it doesn't cause problems
>> for kernel, in oppose to PARK bits of the rest of pinmux that are unset
>> by the driver in tegra_pinctrl_clear_parked_bits() on driver's probe.
>>
>>> on subsequence DPD entry, stored value contains park bit 0 and HW clamps
>>> park bit to logic 1 during DPD entry and cleared again on resume.
>> I assume that EMMC won't work properly if pads are "parked" and the PARK
>> bits should be in unset state on kernel boot-up as I wrote above, hence
>> stored value should always contain 0 for the EMMC PARK bits. No?
>
> On bootup, pads still works. PARK bit is to keep pads in DPD once they
> enter into DPD (LP0) and on resume they need to be cleared to be out of DPD
>
>

Ah okay, thank you for the clarification.

Indeed, it will be better to unset the PARK bits in
pinctrl_clear_parked_bits, as you suggested in the other email. Seems it
should be simple to turn parked_bit into parked_bitmask.

2019-05-29 23:29:28

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH V2 03/12] clk: tegra: save and restore PLLs state for system

Quoting Sowjanya Komatineni (2019-05-28 16:08:47)
> This patch has implementation of saving and restoring PLL's state to
> support system suspend and resume operations.

Can you provide some more background on _why_ this patch should exist?
That's typically what gets written in the commit text.

>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/clk/tegra/clk-divider.c | 19 ++++++++
> drivers/clk/tegra/clk-pll-out.c | 25 +++++++++++
> drivers/clk/tegra/clk-pll.c | 99 ++++++++++++++++++++++++++++++++---------
> drivers/clk/tegra/clk.h | 9 ++++
> 4 files changed, 132 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
> index 2a1822a22740..718694727042 100644
> --- a/drivers/clk/tegra/clk-divider.c
> +++ b/drivers/clk/tegra/clk-divider.c
> @@ -14,6 +14,7 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/clk.h>
> #include <linux/kernel.h>
> #include <linux/io.h>
> #include <linux/err.h>
> @@ -179,3 +180,21 @@ struct clk *tegra_clk_register_mc(const char *name, const char *parent_name,
> reg, 16, 1, CLK_DIVIDER_READ_ONLY,
> mc_div_table, lock);
> }
> +
> +#if defined(CONFIG_PM_SLEEP)
> +void tegra_clk_divider_resume(struct clk_hw *hw, unsigned long rate)
> +{
> + struct clk_hw *parent = clk_hw_get_parent(hw);
> + unsigned long parent_rate;
> +
> + if (IS_ERR(parent)) {

Will this ever happen? Collapse the WARN_ON into the if please:

if (WARN_ON(IS_ERR(parent)))

> + WARN_ON(1);
> + return;
> + }
> +
> + parent_rate = clk_hw_get_rate(parent);
> +
> + if (clk_frac_div_set_rate(hw, rate, parent_rate) < 0)
> + WARN_ON(1);
> +}
> +#endif
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index 09bccbb9640c..e4d124cc5657 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -841,6 +841,15 @@ int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
> u8 frac_width, u8 flags);
>
> +#ifdef CONFIG_PM_SLEEP

Can you remove this ifdef? It just complicates compilation testing.

> +void tegra_clk_pll_resume(struct clk *c, unsigned long rate);
> +void tegra_clk_divider_resume(struct clk_hw *hw, unsigned long rate);
> +void tegra_clk_pll_out_resume(struct clk *clk, unsigned long rate);
> +void tegra_clk_plle_tegra210_resume(struct clk *c);
> +void tegra_clk_sync_state_pll(struct clk *c);
> +void tegra_clk_sync_state_pll_out(struct clk *clk);

Do these APIs need to operate on struct clk? Why can't they operate on
clk_hw or why can't we drive the suspend/resume sequence from the clk
provider driver itself?

2019-05-29 23:32:22

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH V2 04/12] clk: tegra: add support for peripheral clock suspend and resume

Quoting Sowjanya Komatineni (2019-05-28 16:08:48)
> This patch implements peripheral clock context save and restore
> to support system suspend and resume operation.

Again, why?

> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> index 6f2862eddad7..08b788766564 100644
> --- a/drivers/clk/tegra/clk.c
> +++ b/drivers/clk/tegra/clk.c
> @@ -81,6 +81,10 @@ static struct clk **clks;
> static int clk_num;
> static struct clk_onecell_data clk_data;
>
> +#ifdef CONFIG_PM_SLEEP
> +static u32 *periph_ctx;
> +#endif

Please move this into the ifdef below.

> +
> /* Handlers for SoC-specific reset lines */
> static int (*special_reset_assert)(unsigned long);
> static int (*special_reset_deassert)(unsigned long);
> @@ -210,6 +214,65 @@ const struct tegra_clk_periph_regs *get_reg_bank(int clkid)
> }
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +void tegra_clk_periph_suspend(void __iomem *clk_base)
> +{
> + int i, idx;
> +
> + idx = 0;
> + for (i = 0; i < periph_banks; i++, idx++)
> + periph_ctx[idx] =
> + readl_relaxed(clk_base + periph_regs[i].rst_reg);
> +
> + for (i = 0; i < periph_banks; i++, idx++)
> + periph_ctx[idx] =
> + readl_relaxed(clk_base + periph_regs[i].enb_reg);
> +}
> +
> +void tegra_clk_periph_force_on(u32 *clks_on, int count, void __iomem *clk_base)
> +{
> + int i;
> +
> + WARN_ON(count != periph_banks);
> +
> + for (i = 0; i < count; i++)
> + writel_relaxed(clks_on[i], clk_base + periph_regs[i].enb_reg);
> +}
> +
> +void tegra_clk_periph_resume(void __iomem *clk_base)
> +{
> + int i, idx;
> +
> + idx = 0;
> + for (i = 0; i < periph_banks; i++, idx++)
> + writel_relaxed(periph_ctx[idx],
> + clk_base + periph_regs[i].rst_reg);
> +
> + /* ensure all resets have propagated */
> + fence_udelay(2, clk_base);
> + tegra_read_chipid();
> +
> + for (i = 0; i < periph_banks; i++, idx++)
> + writel_relaxed(periph_ctx[idx],
> + clk_base + periph_regs[i].enb_reg);
> +
> + /* ensure all enables have propagated */
> + fence_udelay(2, clk_base);
> + tegra_read_chipid();
> +}
> +
> +static int tegra_clk_suspend_ctx_init(int banks)
> +{
> + int err = 0;
> +
> + periph_ctx = kzalloc(2 * banks * sizeof(*periph_ctx), GFP_KERNEL);

Is this kcalloc(2 * banks, sizeof(*periph_ctx)... ?

> + if (!periph_ctx)
> + err = -ENOMEM;
> +
> + return err;
> +}
> +#endif
> +
> struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
> {
> clk_base = regs;
> @@ -226,11 +289,20 @@ struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
> periph_banks = banks;
>
> clks = kcalloc(num, sizeof(struct clk *), GFP_KERNEL);
> - if (!clks)
> + if (!clks) {
> kfree(periph_clk_enb_refcnt);
> + return NULL;
> + }
>
> clk_num = num;
>
> +#ifdef CONFIG_PM_SLEEP

Can you use if (IS_ENABLED(CONFIG_PM_SLEEP)) here?

> + if (tegra_clk_suspend_ctx_init(banks)) {
> + kfree(periph_clk_enb_refcnt);
> + kfree(clks);
> + return NULL;
> + }
> +#endif
> return clks;
> }
>

2019-05-31 19:54:14

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 03/12] clk: tegra: save and restore PLLs state for system


On 5/29/19 4:28 PM, Stephen Boyd wrote:
> Quoting Sowjanya Komatineni (2019-05-28 16:08:47)
>> This patch has implementation of saving and restoring PLL's state to
>> support system suspend and resume operations.
> Can you provide some more background on _why_ this patch should exist?
> That's typically what gets written in the commit text.
Will add more in next version of this series.
>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>> ---
>> drivers/clk/tegra/clk-divider.c | 19 ++++++++
>> drivers/clk/tegra/clk-pll-out.c | 25 +++++++++++
>> drivers/clk/tegra/clk-pll.c | 99 ++++++++++++++++++++++++++++++++---------
>> drivers/clk/tegra/clk.h | 9 ++++
>> 4 files changed, 132 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
>> index 2a1822a22740..718694727042 100644
>> --- a/drivers/clk/tegra/clk-divider.c
>> +++ b/drivers/clk/tegra/clk-divider.c
>> @@ -14,6 +14,7 @@
>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> */
>>
>> +#include <linux/clk.h>
>> #include <linux/kernel.h>
>> #include <linux/io.h>
>> #include <linux/err.h>
>> @@ -179,3 +180,21 @@ struct clk *tegra_clk_register_mc(const char *name, const char *parent_name,
>> reg, 16, 1, CLK_DIVIDER_READ_ONLY,
>> mc_div_table, lock);
>> }
>> +
>> +#if defined(CONFIG_PM_SLEEP)
>> +void tegra_clk_divider_resume(struct clk_hw *hw, unsigned long rate)
>> +{
>> + struct clk_hw *parent = clk_hw_get_parent(hw);
>> + unsigned long parent_rate;
>> +
>> + if (IS_ERR(parent)) {
> Will this ever happen? Collapse the WARN_ON into the if please:
>
> if (WARN_ON(IS_ERR(parent)))
>
Will fix in next version of this series.
>> + WARN_ON(1);
>> + return;
>> + }
>> +
>> + parent_rate = clk_hw_get_rate(parent);
>> +
>> + if (clk_frac_div_set_rate(hw, rate, parent_rate) < 0)
>> + WARN_ON(1);
>> +}
>> +#endif
>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>> index 09bccbb9640c..e4d124cc5657 100644
>> --- a/drivers/clk/tegra/clk.h
>> +++ b/drivers/clk/tegra/clk.h
>> @@ -841,6 +841,15 @@ int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
>> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
>> u8 frac_width, u8 flags);
>>
>> +#ifdef CONFIG_PM_SLEEP
> Can you remove this ifdef? It just complicates compilation testing.
OK, Will fix in next version
>> +void tegra_clk_pll_resume(struct clk *c, unsigned long rate);
>> +void tegra_clk_divider_resume(struct clk_hw *hw, unsigned long rate);
>> +void tegra_clk_pll_out_resume(struct clk *clk, unsigned long rate);
>> +void tegra_clk_plle_tegra210_resume(struct clk *c);
>> +void tegra_clk_sync_state_pll(struct clk *c);
>> +void tegra_clk_sync_state_pll_out(struct clk *clk);
> Do these APIs need to operate on struct clk? Why can't they operate on
> clk_hw or why can't we drive the suspend/resume sequence from the clk
> provider driver itself?
>
Yes can change to use clk_hw.

By clk provider driver, are you referring to clk-tegra210?

clk-terga210 driver has suspend/resume implementation. These API's are
for corresponding clock specific implementations (clk-pll, clk-pll-out,
clk-divider) for enabling and restoring to proper rate and are invoked
during clk-tegra210 driver resume.

thanks

Sowjanya

2019-05-31 19:57:24

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 04/12] clk: tegra: add support for peripheral clock suspend and resume

DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;
t=1559332560; bh=fyUgksf15TgB7D/+ZMOAO7M7CVG31MW/WSQCDBy4pVQ=;
h=X-PGP-Universal:Subject:To:CC:References:From:Message-ID:Date:
User-Agent:MIME-Version:In-Reply-To:X-Originating-IP:
X-ClientProxiedBy:Content-Type:Content-Transfer-Encoding:
Content-Language;
b=iqAjo2NdebEX6IQgtE+dZ1EoyeCEIZAqRf22p21EapkVX0OBJC3q4Gq7gRPrH2fJC
uhLbg6vyTpA7tw+SL/Q6OC76foZllVOZDlC8m02oSU3qttQyJX5rCD59shHR+I4y1U
EO5f5ynsViWBR2kJNtgRO2yTrKQCYOWgX2VSfb1c3OX347L/TSCQDDFU/bocKSDD6L
cjVCbddyiIHj6HtpiRzWKIezGprf40oa5Lsd7heEqGTFe2IPIuIAcrtSjmg455iUvU
qyKwofgEpOyXkvlW1Jy4Xm+1v3hRH1W5bgxWWMWL7BWkGQQDwHLZXwNVTA0iT/JH6L
nz/b3Hyar6Dwg==

On 5/29/19 4:30 PM, Stephen Boyd wrote:
> Quoting Sowjanya Komatineni (2019-05-28 16:08:48)
>> This patch implements peripheral clock context save and restore
>> to support system suspend and resume operation.
> Again, why?
Will add more in commit in next version of this series.
>> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
>> index 6f2862eddad7..08b788766564 100644
>> --- a/drivers/clk/tegra/clk.c
>> +++ b/drivers/clk/tegra/clk.c
>> @@ -81,6 +81,10 @@ static struct clk **clks;
>> static int clk_num;
>> static struct clk_onecell_data clk_data;
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +static u32 *periph_ctx;
>> +#endif
> Please move this into the ifdef below.
>
WIll fix in V3
>> +
>> /* Handlers for SoC-specific reset lines */
>> static int (*special_reset_assert)(unsigned long);
>> static int (*special_reset_deassert)(unsigned long);
>> @@ -210,6 +214,65 @@ const struct tegra_clk_periph_regs *get_reg_bank(int clkid)
>> }
>> }
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +void tegra_clk_periph_suspend(void __iomem *clk_base)
>> +{
>> + int i, idx;
>> +
>> + idx = 0;
>> + for (i = 0; i < periph_banks; i++, idx++)
>> + periph_ctx[idx] =
>> + readl_relaxed(clk_base + periph_regs[i].rst_reg);
>> +
>> + for (i = 0; i < periph_banks; i++, idx++)
>> + periph_ctx[idx] =
>> + readl_relaxed(clk_base + periph_regs[i].enb_reg);
>> +}
>> +
>> +void tegra_clk_periph_force_on(u32 *clks_on, int count, void __iomem *clk_base)
>> +{
>> + int i;
>> +
>> + WARN_ON(count != periph_banks);
>> +
>> + for (i = 0; i < count; i++)
>> + writel_relaxed(clks_on[i], clk_base + periph_regs[i].enb_reg);
>> +}
>> +
>> +void tegra_clk_periph_resume(void __iomem *clk_base)
>> +{
>> + int i, idx;
>> +
>> + idx = 0;
>> + for (i = 0; i < periph_banks; i++, idx++)
>> + writel_relaxed(periph_ctx[idx],
>> + clk_base + periph_regs[i].rst_reg);
>> +
>> + /* ensure all resets have propagated */
>> + fence_udelay(2, clk_base);
>> + tegra_read_chipid();
>> +
>> + for (i = 0; i < periph_banks; i++, idx++)
>> + writel_relaxed(periph_ctx[idx],
>> + clk_base + periph_regs[i].enb_reg);
>> +
>> + /* ensure all enables have propagated */
>> + fence_udelay(2, clk_base);
>> + tegra_read_chipid();
>> +}
>> +
>> +static int tegra_clk_suspend_ctx_init(int banks)
>> +{
>> + int err = 0;
>> +
>> + periph_ctx = kzalloc(2 * banks * sizeof(*periph_ctx), GFP_KERNEL);
> Is this kcalloc(2 * banks, sizeof(*periph_ctx)... ?
Will fix in V3
>> + if (!periph_ctx)
>> + err = -ENOMEM;
>> +
>> + return err;
>> +}
>> +#endif
>> +
>> struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
>> {
>> clk_base = regs;
>> @@ -226,11 +289,20 @@ struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
>> periph_banks = banks;
>>
>> clks = kcalloc(num, sizeof(struct clk *), GFP_KERNEL);
>> - if (!clks)
>> + if (!clks) {
>> kfree(periph_clk_enb_refcnt);
>> + return NULL;
>> + }
>>
>> clk_num = num;
>>
>> +#ifdef CONFIG_PM_SLEEP
> Can you use if (IS_ENABLED(CONFIG_PM_SLEEP)) here?
Will fix in V3
>
>> + if (tegra_clk_suspend_ctx_init(banks)) {
>> + kfree(periph_clk_enb_refcnt);
>> + kfree(clks);
>> + return NULL;
>> + }
>> +#endif
>> return clks;
>> }
>>

2019-06-01 08:30:19

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 10/12] gpio: tegra: implement wake event support for Tegra210 and prior GPIO


On 5/29/19 7:03 AM, Thierry Reding wrote:
> On Tue, May 28, 2019 at 04:08:54PM -0700, Sowjanya Komatineni wrote:
>> The GPIO controller doesn't have any controls to enable the system to
>> wake up from low power states based on activity on GPIO pins. An extra
>> hardware block that is part of the power management controller (PMC)
>> contains these controls. In order for the GPIO controller to be able
>> to cooperate with the PMC, obtain a reference to the PMC's IRQ domain
>> and make it a parent to the GPIO controller's IRQ domain. This way the
>> PMC gets an opportunity to program the additional registers required
>> to enable wakeup sources on suspend.
>>
>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>> ---
>> drivers/gpio/gpio-tegra.c | 116 +++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 110 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
>> index 6d9b6906b9d0..5190129668d3 100644
>> --- a/drivers/gpio/gpio-tegra.c
>> +++ b/drivers/gpio/gpio-tegra.c
>> @@ -32,6 +32,8 @@
>> #include <linux/pinctrl/consumer.h>
>> #include <linux/pm.h>
>>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> #define GPIO_BANK(x) ((x) >> 5)
>> #define GPIO_PORT(x) (((x) >> 3) & 0x3)
>> #define GPIO_BIT(x) ((x) & 0x7)
>> @@ -275,8 +277,22 @@ static int tegra_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
>> static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
>> {
>> struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
>> + struct irq_domain *domain = tgi->irq_domain;
>> +
>> + if (!gpiochip_irqchip_irq_valid(chip, offset))
>> + return -ENXIO;
>> +
>> + if (irq_domain_is_hierarchy(domain)) {
>> + struct irq_fwspec spec;
>> +
>> + spec.fwnode = domain->fwnode;
>> + spec.param_count = 2;
>> + spec.param[0] = offset;
>> + spec.param[1] = IRQ_TYPE_NONE;
>> + return irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, &spec);
> This looks like it was copied from the equivalent Tegra186 patch. I have
> since then changed the implementation, based on feedback by Linus, to
> not call irq_domain_alloc_irqs() here and instead call
> irq_create_fwspec_mapping(). This has the advantage of not requiring the
> irq_domain_alloc_irqs() function to be exported. It ends up calling that
> function internally, but as discussed with Linus it's also a nicer way
> to create these mappings.
>
existing gpio-tegra driver maps hwirq to virtual interrupt number using
irq_create_mapping during gpio probe

and irq_create_fwspec_mapping() will always return virq number as virq
number exists already and irq_domain_alloc_irqs doesn't happen.

So I was using irq_domain_alloc_irqs().


>> + }
>>
>> - return irq_find_mapping(tgi->irq_domain, offset);
>> + return irq_find_mapping(domain, offset);
>> }
>>
>> static void tegra_gpio_irq_ack(struct irq_data *d)
>> @@ -365,7 +381,10 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>> else if (type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
>> irq_set_handler_locked(d, handle_edge_irq);
>>
>> - return 0;
>> + if (d->parent_data)
>> + return irq_chip_set_type_parent(d, type);
>> + else
>> + return 0;
> There's no need for this final else. Just make it a regular "return 0;"
> at the end of the function, without the extra else branch.
>
>> }
>>
>> static void tegra_gpio_irq_shutdown(struct irq_data *d)
>> @@ -503,6 +522,7 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
>> struct tegra_gpio_bank *bank = irq_data_get_irq_chip_data(d);
>> unsigned int gpio = d->hwirq;
>> u32 port, bit, mask;
>> + int ret;
>>
>> port = GPIO_PORT(gpio);
>> bit = GPIO_BIT(gpio);
>> @@ -513,7 +533,14 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
>> else
>> bank->wake_enb[port] &= ~mask;
>>
>> - return irq_set_irq_wake(bank->irq, enable);
>> + ret = irq_set_irq_wake(bank->irq, enable);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (d->parent_data)
>> + return irq_chip_set_wake_parent(d, enable);
>> + else
>> + return 0;
> Same here.
>
> Thierry

2019-06-04 12:42:57

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH V2 06/12] clk: tegra: add suspend resume support for DFLL clock

On Tue, May 28, 2019 at 04:08:50PM -0700, Sowjanya Komatineni wrote:
> This patch adds support for suspend and resume for DFLL clock.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/clk/tegra/clk-dfll.c | 82 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/clk/tegra/clk-dfll.h | 2 ++
> 2 files changed, 84 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> index 1fc71baae13b..d92a5a05fbbc 100644
> --- a/drivers/clk/tegra/clk-dfll.c
> +++ b/drivers/clk/tegra/clk-dfll.c
> @@ -286,6 +286,7 @@ struct tegra_dfll {
> unsigned long dvco_rate_min;
>
> enum dfll_ctrl_mode mode;
> + enum dfll_ctrl_mode resume_mode;
> enum dfll_tune_range tune_range;
> struct dentry *debugfs_dir;
> struct clk_hw dfll_clk_hw;
> @@ -1873,6 +1874,87 @@ static int dfll_fetch_common_params(struct tegra_dfll *td)
> }
>
> /*
> + * tegra_dfll_suspend
> + * @pdev: DFLL instance
> + *
> + * dfll controls clock/voltage to other devices, including CPU. Therefore,
> + * dfll driver pm suspend callback does not stop cl-dvfs operations. It is
> + * only used to enforce cold voltage limit, since SoC may cool down during
> + * suspend without waking up. The correct temperature zone after suspend will
> + * be updated via dfll cooling device interface during resume of temperature
> + * sensor.

Temperature dependent cl-dvfs is not yet implemented in upstream, so
leave out the part about cold voltage limits and temperature zones.

Peter.

2019-06-04 13:48:58

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH V2 00/12] LP0 entry and exit support for Tegra210

On Tue, May 28, 2019 at 04:08:44PM -0700, Sowjanya Komatineni wrote:
> This patch series includes Tegra210 deepsleep/LP0 support with
> deep sleep exit through RTC alarm wake and power button wake events.
>

We have been calling this SC7 (system C-state 7) for quite a while now.

Peter.

> Note: Wake on power button is through gpio-keys node in device tree.
>
> This series also includes save and restore of PLLs, clocks, OSC contexts
> for basic LP0 exit.
>
> This patch series doesn't support 100% suspend/resume to allow fully
> functional state upon resume and we are working on some more drivers suspend
> and resume implementations.
>
> [V2] : V1 feedback fixes
> Patch 0002: This version still using syscore. Thierry suggest not to
> use syscore and waiting on suggestion from Linux Walleij for any better
> way of storing current state of pins before suspend entry and restoring
> them on resume at very early stage. So left this the same way as V1 and
> will address once I get more feedback on this.
> Also need to findout and implement proper way of forcing resume order
> between pinctrl and gpio driver.
>
>
> Sowjanya Komatineni (12):
> irqchip: tegra: do not disable COP IRQ during suspend
> pinctrl: tegra: add suspend and resume support
> clk: tegra: save and restore PLLs state for system
> clk: tegra: add support for peripheral clock suspend and resume
> clk: tegra: add support for OSC clock resume
> clk: tegra: add suspend resume support for DFLL clock
> clk: tegra: support for Tegra210 clocks suspend-resume
> soc/tegra: pmc: allow support for more tegra wake models
> soc/tegra: pmc: add pmc wake support for tegra210
> gpio: tegra: implement wake event support for Tegra210 and prior GPIO
> arm64: tegra: enable wake from deep sleep on RTC alarm.
> soc/tegra: pmc: configure tegra deep sleep control settings
>
> arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 7 +
> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 +-
> drivers/clk/tegra/clk-dfll.c | 82 ++++++
> drivers/clk/tegra/clk-dfll.h | 2 +
> drivers/clk/tegra/clk-divider.c | 19 ++
> drivers/clk/tegra/clk-pll-out.c | 25 ++
> drivers/clk/tegra/clk-pll.c | 99 +++++--
> drivers/clk/tegra/clk-tegra-fixed.c | 16 ++
> drivers/clk/tegra/clk-tegra210.c | 382 +++++++++++++++++++++++++
> drivers/clk/tegra/clk.c | 74 ++++-
> drivers/clk/tegra/clk.h | 13 +
> drivers/gpio/gpio-tegra.c | 116 +++++++-
> drivers/irqchip/irq-tegra.c | 22 +-
> drivers/pinctrl/tegra/pinctrl-tegra.c | 68 ++++-
> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 +
> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra210.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
> drivers/soc/tegra/pmc.c | 150 +++++++++-
> 21 files changed, 1053 insertions(+), 35 deletions(-)
>
> --
> 2.7.4
>

2019-06-05 23:32:46

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH V2 03/12] clk: tegra: save and restore PLLs state for system

Quoting Sowjanya Komatineni (2019-05-31 12:52:44)
>
> On 5/29/19 4:28 PM, Stephen Boyd wrote:
> > Quoting Sowjanya Komatineni (2019-05-28 16:08:47)
> >> + WARN_ON(1);
> >> + return;
> >> + }
> >> +
> >> + parent_rate = clk_hw_get_rate(parent);
> >> +
> >> + if (clk_frac_div_set_rate(hw, rate, parent_rate) < 0)
> >> + WARN_ON(1);
> >> +}
> >> +#endif
> >> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> >> index 09bccbb9640c..e4d124cc5657 100644
> >> --- a/drivers/clk/tegra/clk.h
> >> +++ b/drivers/clk/tegra/clk.h
> >> @@ -841,6 +841,15 @@ int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
> >> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
> >> u8 frac_width, u8 flags);
> >>
> >> +#ifdef CONFIG_PM_SLEEP
> > Can you remove this ifdef? It just complicates compilation testing.
> OK, Will fix in next version
> >> +void tegra_clk_pll_resume(struct clk *c, unsigned long rate);
> >> +void tegra_clk_divider_resume(struct clk_hw *hw, unsigned long rate);
> >> +void tegra_clk_pll_out_resume(struct clk *clk, unsigned long rate);
> >> +void tegra_clk_plle_tegra210_resume(struct clk *c);
> >> +void tegra_clk_sync_state_pll(struct clk *c);
> >> +void tegra_clk_sync_state_pll_out(struct clk *clk);
> > Do these APIs need to operate on struct clk? Why can't they operate on
> > clk_hw or why can't we drive the suspend/resume sequence from the clk
> > provider driver itself?
> >
> Yes can change to use clk_hw.
>
> By clk provider driver, are you referring to clk-tegra210?

I guess so.

>
> clk-terga210 driver has suspend/resume implementation. These API's are
> for corresponding clock specific implementations (clk-pll, clk-pll-out,
> clk-divider) for enabling and restoring to proper rate and are invoked
> during clk-tegra210 driver resume.

Yes, so when the clk provider suspends it needs to do something? Our
handling of clk rates and other state like enable/disable over
suspend/resume isn't really well thought out or implemented so far. TI
has some code to do some stuff, but otherwise I haven't seen drivers
handling this. Ideally it would be something generic in the framework so
that drivers don't have to work around stuff.

2019-06-06 20:18:29

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH V2 07/12] clk: tegra: support for Tegra210 clocks suspend-resume


On 6/6/19 11:17 AM, Stephen Boyd wrote:
> Quoting Sowjanya Komatineni (2019-05-28 16:08:51)
>> @@ -3381,6 +3398,367 @@ static struct tegra_clk_init_table init_table[] __initdata = {
>> { TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 },
>> };
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +static unsigned long pll_c_rate, pll_c2_rate, pll_c3_rate, pll_x_rate;
>> +static unsigned long pll_c4_rate, pll_d2_rate, pll_dp_rate;
>> +static unsigned long pll_re_vco_rate, pll_d_rate, pll_a_rate, pll_a1_rate;
>> +static unsigned long pll_c_out1_rate;
>> +static unsigned long pll_a_out0_rate, pll_c4_out3_rate;
>> +static unsigned long pll_p_out_rate[5];
>> +static unsigned long pll_u_out1_rate, pll_u_out2_rate;
>> +static unsigned long pll_mb_rate;
>> +static u32 pll_m_v;
>> +static u32 pll_p_outa, pll_p_outb;
>> +static u32 pll_re_out_div, pll_re_out_1;
>> +static u32 cpu_softrst_ctx[3];
>> +static u32 cclkg_burst_policy_ctx[2];
>> +static u32 cclklp_burst_policy_ctx[2];
>> +static u32 sclk_burst_policy_ctx[3];
>> +static u32 sclk_ctx, spare_ctx, misc_clk_enb_ctx, clk_arm_ctx;
> This is a lot of state to maintain globally. Can it go into a container
> struct so we can get docs and understand what's going on a little
> better?
>
Will revisit and change in next version along with using clk driver
save_context and restore_context callbacks.
>> +
>> +static struct platform_device *dfll_pdev;
>> +#define car_readl(_base, _off) \
>> + readl_relaxed(clk_base + (_base) + ((_off) * 4))
>> +#define car_writel(_val, _base, _off) \
>> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>> +
>> +static u32 *periph_clk_src_ctx;
>> +struct periph_source_bank {
>> + u32 start;
>> + u32 end;
> Do these need to be u32 or could they be u16?
WIll update in next version of this patch
>> +};
>> +
>> +static struct periph_source_bank periph_srcs[] = {
> Can this be const?
WIll update in next version of this patch
>
>> + [0] = {
>> + .start = 0x100,
>> + .end = 0x198,
>> + },
>> + [1] = {
>> + .start = 0x1a0,
>> + .end = 0x1f8,
>> + },
>> + [2] = {
>> + .start = 0x3b4,
>> + .end = 0x42c,
>> + },
>> + [3] = {
>> + .start = 0x49c,
>> + .end = 0x4b4,
>> + },
>> + [4] = {
>> + .start = 0x560,
>> + .end = 0x564,
>> + },
>> + [5] = {
>> + .start = 0x600,
>> + .end = 0x678,
>> + },
>> + [6] = {
>> + .start = 0x694,
>> + .end = 0x6a0,
>> + },
>> + [7] = {
>> + .start = 0x6b8,
>> + .end = 0x718,
>> + },
>> +};
>> +
>> +/* This array lists the valid clocks for each periph clk bank */
>> +static u32 periph_clks_on[] = {
> const?
WIll update in next version of this patch
>> + 0xdcd7dff9,
>> + 0x87d1f3e7,
>> + 0xf3fed3fa,
>> + 0xffc18cfb,
>> + 0x793fb7ff,
>> + 0x3fe66fff,
>> + 0xfc1fc7ff,
> What are these magic numbers?

These are the hard coded values for peripheral clock enable register
where it enables all clocks before restoring clock sources to proper rate.


>> +};
>> +
>> +static inline unsigned long clk_get_rate_nolock(struct clk *clk)
>> +{
>> + if (IS_ERR_OR_NULL(clk)) {
> NULL is a valid clk pointer. Typically usage of IS_ERR_OR_NULL() is
> wrong.
>
>> + WARN_ON(1);
>> + return 0;
>> + }
>> +
>> + return clk_hw_get_rate(__clk_get_hw(clk));
>> +}
>> +
>> +static inline struct clk *pll_p_clk(unsigned int x)
>> +{
>> + if (x < 4) {
> What is magic value 4?

PLLP outs OUT1 through OUT4 are defined sequential in tegra210-car.h and
OUT5  is not so retrieving value from clks using sequential indexing for
up to OUT4.

Will update in next version to use define

>> + return clks[TEGRA210_CLK_PLL_P_OUT1 + x];
>> + } else if (x != 4) {
>> + WARN_ON(1);
>> + return NULL;
>> + } else {
>> + return clks[TEGRA210_CLK_PLL_P_OUT5];
>> + }
>> +}
>> +
> [..]
>> +
>> +static void tegra210_clk_resume(void)
>> +{
> [..]
>> + fence_udelay(2, clk_base);
>> + for (i = 0; i < BURST_POLICY_REG_SIZE; i++) {
>> + car_writel(cclklp_burst_policy_ctx[i], CCLKLP_BURST_POLICY, i);
>> + car_writel(sclk_burst_policy_ctx[i], SCLK_BURST_POLICY, i);
>> + }
>> + car_writel(sclk_burst_policy_ctx[i], SYS_CLK_DIV, 0);
>> +
>> + car_writel(sclk_ctx, SYSTEM_CLK_RATE, 0);
>> + car_writel(spare_ctx, SPARE_REG0, 0);
>> + car_writel(misc_clk_enb_ctx, MISC_CLK_ENB, 0);
>> + car_writel(clk_arm_ctx, CLK_MASK_ARM, 0);
>> +
>> + /* enable all clocks before configuring clock sources */
>> + tegra_clk_periph_force_on(periph_clks_on, ARRAY_SIZE(periph_clks_on),
>> + clk_base);
>> +
>> + wmb();
> Please add a comment before barriers so we know what they're for.
Will do in next version of this patch.
>> + fence_udelay(2, clk_base);
>> +

2019-06-06 21:28:19

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH V2 07/12] clk: tegra: support for Tegra210 clocks suspend-resume

Quoting Sowjanya Komatineni (2019-05-28 16:08:51)
> @@ -3381,6 +3398,367 @@ static struct tegra_clk_init_table init_table[] __initdata = {
> { TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 },
> };
>
> +#ifdef CONFIG_PM_SLEEP
> +static unsigned long pll_c_rate, pll_c2_rate, pll_c3_rate, pll_x_rate;
> +static unsigned long pll_c4_rate, pll_d2_rate, pll_dp_rate;
> +static unsigned long pll_re_vco_rate, pll_d_rate, pll_a_rate, pll_a1_rate;
> +static unsigned long pll_c_out1_rate;
> +static unsigned long pll_a_out0_rate, pll_c4_out3_rate;
> +static unsigned long pll_p_out_rate[5];
> +static unsigned long pll_u_out1_rate, pll_u_out2_rate;
> +static unsigned long pll_mb_rate;
> +static u32 pll_m_v;
> +static u32 pll_p_outa, pll_p_outb;
> +static u32 pll_re_out_div, pll_re_out_1;
> +static u32 cpu_softrst_ctx[3];
> +static u32 cclkg_burst_policy_ctx[2];
> +static u32 cclklp_burst_policy_ctx[2];
> +static u32 sclk_burst_policy_ctx[3];
> +static u32 sclk_ctx, spare_ctx, misc_clk_enb_ctx, clk_arm_ctx;

This is a lot of state to maintain globally. Can it go into a container
struct so we can get docs and understand what's going on a little
better?

> +
> +static struct platform_device *dfll_pdev;
> +#define car_readl(_base, _off) \
> + readl_relaxed(clk_base + (_base) + ((_off) * 4))
> +#define car_writel(_val, _base, _off) \
> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
> +
> +static u32 *periph_clk_src_ctx;
> +struct periph_source_bank {
> + u32 start;
> + u32 end;

Do these need to be u32 or could they be u16?

> +};
> +
> +static struct periph_source_bank periph_srcs[] = {

Can this be const?

> + [0] = {
> + .start = 0x100,
> + .end = 0x198,
> + },
> + [1] = {
> + .start = 0x1a0,
> + .end = 0x1f8,
> + },
> + [2] = {
> + .start = 0x3b4,
> + .end = 0x42c,
> + },
> + [3] = {
> + .start = 0x49c,
> + .end = 0x4b4,
> + },
> + [4] = {
> + .start = 0x560,
> + .end = 0x564,
> + },
> + [5] = {
> + .start = 0x600,
> + .end = 0x678,
> + },
> + [6] = {
> + .start = 0x694,
> + .end = 0x6a0,
> + },
> + [7] = {
> + .start = 0x6b8,
> + .end = 0x718,
> + },
> +};
> +
> +/* This array lists the valid clocks for each periph clk bank */
> +static u32 periph_clks_on[] = {

const?

> + 0xdcd7dff9,
> + 0x87d1f3e7,
> + 0xf3fed3fa,
> + 0xffc18cfb,
> + 0x793fb7ff,
> + 0x3fe66fff,
> + 0xfc1fc7ff,

What are these magic numbers?

> +};
> +
> +static inline unsigned long clk_get_rate_nolock(struct clk *clk)
> +{
> + if (IS_ERR_OR_NULL(clk)) {

NULL is a valid clk pointer. Typically usage of IS_ERR_OR_NULL() is
wrong.

> + WARN_ON(1);
> + return 0;
> + }
> +
> + return clk_hw_get_rate(__clk_get_hw(clk));
> +}
> +
> +static inline struct clk *pll_p_clk(unsigned int x)
> +{
> + if (x < 4) {

What is magic value 4?

> + return clks[TEGRA210_CLK_PLL_P_OUT1 + x];
> + } else if (x != 4) {
> + WARN_ON(1);
> + return NULL;
> + } else {
> + return clks[TEGRA210_CLK_PLL_P_OUT5];
> + }
> +}
> +
[..]
> +
> +static void tegra210_clk_resume(void)
> +{
[..]
> + fence_udelay(2, clk_base);
> + for (i = 0; i < BURST_POLICY_REG_SIZE; i++) {
> + car_writel(cclklp_burst_policy_ctx[i], CCLKLP_BURST_POLICY, i);
> + car_writel(sclk_burst_policy_ctx[i], SCLK_BURST_POLICY, i);
> + }
> + car_writel(sclk_burst_policy_ctx[i], SYS_CLK_DIV, 0);
> +
> + car_writel(sclk_ctx, SYSTEM_CLK_RATE, 0);
> + car_writel(spare_ctx, SPARE_REG0, 0);
> + car_writel(misc_clk_enb_ctx, MISC_CLK_ENB, 0);
> + car_writel(clk_arm_ctx, CLK_MASK_ARM, 0);
> +
> + /* enable all clocks before configuring clock sources */
> + tegra_clk_periph_force_on(periph_clks_on, ARRAY_SIZE(periph_clks_on),
> + clk_base);
> +
> + wmb();

Please add a comment before barriers so we know what they're for.

> + fence_udelay(2, clk_base);
> +