[PATCHv2 0/8] arm: omap: clk/clkdm/pwrdm/voltdm usecounting changes

[PATCHv2 0/8] arm: omap: clk/clkdm/pwrdm/voltdm usecounting changes

[Tero Kristo]

Changes compared to previous version:

- moved voltdm_pwrdm_enable / disable hooks from idle code to
  pwrdm_pre_transition / pwrdm_post_transition functions (patch 4)
- added patches 7 and 8 to fix one issue that was visible with off-mode
  on omap3: attempting to disable wkdeps for per_clkdm causes crash
  dumps to be generated by omap_l3_smx (this was actually part of the
  original larger set but I dropped it accidentally from v1), this issue
  surfaces with this patch set as the kernel actually tries to manually
  idle per_clkdm (probably first time in history)
- added some parameter sanity checks (pwrdm/voltdm = null checks)

Similar tests done as with previous version, additionally off-mode tests
with omap3.

-Tero

[PATCHv2 1/8] arm: omap: clk: add support for omap_clk_for_each

[Tero Kristo]

This works similarly to e.g. pwrdm_for_each(). Needed by enhanced
usecounting debug functionality that will be added to pm-debug.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/clock.c              |   30 ++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/clock.h |    2 ++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 567e4b5..e17ee5e2 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -356,6 +356,36 @@ int omap_clk_enable_autoidle_all(void)
 	return 0;
 }
 
+/**
+ * omap_clk_for_each - call a function for each registered clock
+ * @fn: pointer to callback function
+ * @data: void * data to pass to callback function
+ *
+ * Call @fn for each registered clock, passing @data to each function.
+ * @fn must return 0 for success or any other value for failure. If
+ * @fn returns non-zero, the iteration across clocks will stop and
+ * the non-zero return value will be passed to the caller of
+ * omap_clk_for_each(). @fn is called with clockfw_lock held.
+ */
+int omap_clk_for_each(int (*fn)(struct clk *clk, void *user), void *user)
+{
+	struct clk *c;
+	unsigned long flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&clockfw_lock, flags);
+
+	list_for_each_entry(c, &clocks, node) {
+		ret = fn(c, user);
+		if (ret)
+			break;
+	}
+
+	spin_unlock_irqrestore(&clockfw_lock, flags);
+
+	return ret;
+}
+
 int omap_clk_disable_autoidle_all(void)
 {
 	struct clk *c;
diff --git a/arch/arm/plat-omap/include/plat/clock.h b/arch/arm/plat-omap/include/plat/clock.h
index 240a7b9..c84f2a4 100644
--- a/arch/arm/plat-omap/include/plat/clock.h
+++ b/arch/arm/plat-omap/include/plat/clock.h
@@ -300,6 +300,8 @@ extern void propagate_rate(struct clk *clk);
 extern void recalculate_root_clocks(void);
 extern unsigned long followparent_recalc(struct clk *clk);
 extern void clk_enable_init_clocks(void);
+extern int omap_clk_for_each(int (*fn)(struct clk *clk, void *user),
+				void *user);
 unsigned long omap_fixed_divisor_recalc(struct clk *clk);
 #ifdef CONFIG_CPU_FREQ
 extern void clk_init_cpufreq_table(struct cpufreq_frequency_table **table);
-- 
1.7.4.1

[PATCHv2 2/8] arm: omap3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking

[Tero Kristo]

This patch fixes the usecount tracking for omap3+, previously the
usecount numbers were rather bogus and were not really useful for
any purpose. Now usecount numbers track the number of really active
clients on each domain. This patch also adds support for usecount
tracking on powerdomain level and autoidle flag for clocks that
are hardware controlled and should be skipped in usecount
calculations.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/clkt_iclk.c         |   21 +++++++++++++++++++
 arch/arm/mach-omap2/clockdomain.c       |   34 +++++++++++++++++++++++++++++-
 arch/arm/mach-omap2/clockdomain.h       |    2 +
 arch/arm/mach-omap2/powerdomain.c       |   16 ++++++++++++++
 arch/arm/mach-omap2/powerdomain.h       |    5 ++++
 arch/arm/plat-omap/clock.c              |    6 +++++
 arch/arm/plat-omap/include/plat/clock.h |    2 +
 7 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clkt_iclk.c b/arch/arm/mach-omap2/clkt_iclk.c
index 3d43fba..f8c2e77 100644
--- a/arch/arm/mach-omap2/clkt_iclk.c
+++ b/arch/arm/mach-omap2/clkt_iclk.c
@@ -21,6 +21,7 @@
 #include "clock2xxx.h"
 #include "cm2xxx_3xxx.h"
 #include "cm-regbits-24xx.h"
+#include "clockdomain.h"
 
 /* Private functions */
 
@@ -34,6 +35,16 @@ void omap2_clkt_iclk_allow_idle(struct clk *clk)
 	v = __raw_readl((__force void __iomem *)r);
 	v |= (1 << clk->enable_bit);
 	__raw_writel(v, (__force void __iomem *)r);
+
+	/* Remove this clock from parent clockdomain usecounts */
+	if (clk->usecount && clk->clkdm)
+		clkdm_usecount_dec(clk->clkdm);
+
+	/*
+	 * Mark as autoidle, so we continue to ignore this clock in
+	 * parent clkdm usecount calculations
+	 */
+	clk->autoidle = true;
 }
 
 /* XXX */
@@ -46,6 +57,16 @@ void omap2_clkt_iclk_deny_idle(struct clk *clk)
 	v = __raw_readl((__force void __iomem *)r);
 	v &= ~(1 << clk->enable_bit);
 	__raw_writel(v, (__force void __iomem *)r);
+
+	/* Add clock back to parent clockdomain usecount */
+	if (clk->usecount && clk->clkdm)
+		clkdm_usecount_inc(clk->clkdm);
+
+	/*
+	 * Disable autoidle flag so further clkdm usecounts take this
+	 * clock into account
+	 */
+	clk->autoidle = false;
 }
 
 /* Public data */
diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index ad07689..c0b6187 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -907,6 +907,28 @@ bool clkdm_in_hwsup(struct clockdomain *clkdm)
 
 /* Clockdomain-to-clock/hwmod framework interface code */
 
+int clkdm_usecount_inc(struct clockdomain *clkdm)
+{
+	int usecount;
+
+	usecount = atomic_inc_return(&clkdm->usecount);
+
+	if (usecount == 1)
+		pwrdm_clkdm_enable(clkdm->pwrdm.ptr);
+	return usecount;
+}
+
+int clkdm_usecount_dec(struct clockdomain *clkdm)
+{
+	int usecount;
+
+	usecount = atomic_dec_return(&clkdm->usecount);
+
+	if (usecount == 0)
+		pwrdm_clkdm_disable(clkdm->pwrdm.ptr);
+	return usecount;
+}
+
 static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 {
 	unsigned long flags;
@@ -919,7 +941,7 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 	 * should be called for every clock instance or hwmod that is
 	 * enabled, so the clkdm can be force woken up.
 	 */
-	if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps)
+	if ((clkdm_usecount_inc(clkdm) > 1) && autodeps)
 		return 0;
 
 	spin_lock_irqsave(&clkdm->lock, flags);
@@ -945,7 +967,7 @@ static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
 		return -ERANGE;
 	}
 
-	if (atomic_dec_return(&clkdm->usecount) > 0)
+	if (clkdm_usecount_dec(clkdm) > 0)
 		return 0;
 
 	spin_lock_irqsave(&clkdm->lock, flags);
@@ -982,6 +1004,10 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
 	if (!clk)
 		return -EINVAL;
 
+	/* If autoidle clock, do not update clkdm usecounts */
+	if (clk->autoidle)
+		return 0;
+
 	return _clkdm_clk_hwmod_enable(clkdm);
 }
 
@@ -1008,6 +1034,10 @@ int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
 	if (!clk)
 		return -EINVAL;
 
+	/* If autoidle clock, do not update clkdm usecounts */
+	if (clk->autoidle)
+		return 0;
+
 	return _clkdm_clk_hwmod_disable(clkdm);
 }
 
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index f7b5860..373399a 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -191,6 +191,8 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk);
 int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk);
 int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh);
 int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh);
+int clkdm_usecount_inc(struct clockdomain *clkdm);
+int clkdm_usecount_dec(struct clockdomain *clkdm);
 
 extern void __init omap242x_clockdomains_init(void);
 extern void __init omap243x_clockdomains_init(void);
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 8a18d1b..3b70f69 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -985,6 +985,22 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm)
 	return -EINVAL;
 }
 
+void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
+{
+	if (!pwrdm)
+		return;
+
+	atomic_inc(&pwrdm->usecount);
+}
+
+void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
+{
+	if (!pwrdm)
+		return;
+
+	atomic_dec(&pwrdm->usecount);
+}
+
 int pwrdm_pre_transition(void)
 {
 	pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 0d72a8a..32f44c8 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -120,6 +120,7 @@ struct powerdomain {
 	unsigned state_counter[PWRDM_MAX_PWRSTS];
 	unsigned ret_logic_off_counter;
 	unsigned ret_mem_off_counter[PWRDM_MAX_MEM_BANKS];
+	atomic_t usecount;
 
 #ifdef CONFIG_PM_DEBUG
 	s64 timer;
@@ -216,6 +217,10 @@ int pwrdm_state_switch(struct powerdomain *pwrdm);
 int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
 int pwrdm_pre_transition(void);
 int pwrdm_post_transition(void);
+
+void pwrdm_clkdm_enable(struct powerdomain *pwrdm);
+void pwrdm_clkdm_disable(struct powerdomain *pwrdm);
+
 int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm);
 int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
 bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index e17ee5e2..1f80df8 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -283,6 +283,12 @@ int clk_register(struct clk *clk)
 		list_add(&clk->sibling, &root_clks);
 
 	list_add(&clk->node, &clocks);
+	/*
+	 * If clock has no ops, it is handled by hardware and thus will
+	 * idle automatically
+	 */
+	if (clk->ops == &clkops_null)
+		clk->autoidle = true;
 	if (clk->init)
 		clk->init(clk);
 	mutex_unlock(&clocks_mutex);
diff --git a/arch/arm/plat-omap/include/plat/clock.h b/arch/arm/plat-omap/include/plat/clock.h
index c84f2a4..14b44c0 100644
--- a/arch/arm/plat-omap/include/plat/clock.h
+++ b/arch/arm/plat-omap/include/plat/clock.h
@@ -208,6 +208,7 @@ struct dpll_data {
  * @init: fn ptr to do clock-specific initialization
  * @enable_bit: bitshift to write to enable/disable the clock (see @enable_reg)
  * @usecount: number of users that have requested this clock to be enabled
+ * @autoidle: indicates hardware controlled clock (not used in domain usecounts)
  * @fixed_div: when > 0, this clock's rate is its parent's rate / @fixed_div
  * @flags: see "struct clk.flags possibilities" above
  * @clksel_reg: for clksel clks, register va containing src/divisor select
@@ -254,6 +255,7 @@ struct clk {
 	void			(*init)(struct clk *);
 	u8			enable_bit;
 	s8			usecount;
+	bool			autoidle;
 	u8			fixed_div;
 	u8			flags;
 #ifdef CONFIG_ARCH_OMAP2PLUS
-- 
1.7.4.1

[PATCHv2 3/8] arm: omap3+: voltage: add support for voltagedomain usecounts

[Tero Kristo]

These are updated based on powerdomain usecounts. Also added support
for voltdm->sleep and voltdm->wakeup calls that will be invoked once
voltagedomain enters sleep or wakes up based on usecount numbers. These
will be used for controlling voltage scaling functionality.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |    6 +++-
 arch/arm/mach-omap2/voltage.c     |   54 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/voltage.h     |   11 +++++++
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 3b70f69..076eb42 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -990,7 +990,8 @@ void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
 	if (!pwrdm)
 		return;
 
-	atomic_inc(&pwrdm->usecount);
+	if (atomic_inc_return(&pwrdm->usecount) == 1)
+		voltdm_pwrdm_enable(pwrdm->voltdm.ptr);
 }
 
 void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
@@ -998,7 +999,8 @@ void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
 	if (!pwrdm)
 		return;
 
-	atomic_dec(&pwrdm->usecount);
+	if (!atomic_dec_return(&pwrdm->usecount))
+		voltdm_pwrdm_disable(pwrdm->voltdm.ptr);
 }
 
 int pwrdm_pre_transition(void)
diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 8a36342..bd5e413 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -38,6 +38,7 @@
 
 #include "voltage.h"
 #include "powerdomain.h"
+#include "smartreflex.h"
 
 #include "vc.h"
 #include "vp.h"
@@ -325,6 +326,59 @@ int voltdm_add_pwrdm(struct voltagedomain *voltdm, struct powerdomain *pwrdm)
 }
 
 /**
+ * voltdm_pwrdm_enable - increase usecount for a voltagedomain
+ * @voltdm: struct voltagedomain * to increase count for
+ *
+ * Increases usecount for a given voltagedomain. If the usecount reaches
+ * 1, the domain is awakened from idle and the function will call the
+ * voltagedomain->wakeup callback for this domain, and also enable the
+ * smartreflex for the domain.
+ */
+void voltdm_pwrdm_enable(struct voltagedomain *voltdm)
+{
+	if (!voltdm)
+		return;
+
+	if (atomic_inc_return(&voltdm->usecount) == 1) {
+		if (voltdm->wakeup)
+			voltdm->wakeup(voltdm);
+		omap_sr_enable(voltdm);
+	}
+}
+
+/**
+ * voltdm_pwrdm_disable - decrease usecount for a voltagedomain
+ * @voltdm: struct voltagedomain * to decrease count for
+ *
+ * Decreases the usecount for a given voltagedomain. If the usecount
+ * reaches zero, the domain can idle and the function will call the
+ * voltagedomain->sleep callback, disable smartreflex for the domain,
+ * and calculate the overall target state for the voltagedomain.
+ */
+void voltdm_pwrdm_disable(struct voltagedomain *voltdm)
+{
+	int target_state = -EINVAL;
+	int state;
+	struct powerdomain *pwrdm;
+
+	if (!voltdm)
+		return;
+
+	if (!atomic_dec_return(&voltdm->usecount)) {
+		omap_sr_disable(voltdm);
+		/* Determine target state for voltdm */
+		list_for_each_entry(pwrdm, &voltdm->pwrdm_list, voltdm_node) {
+			state = pwrdm_read_next_pwrst(pwrdm);
+			if (state > target_state)
+				target_state = state;
+		}
+		voltdm->target_state = target_state;
+		if (voltdm->sleep)
+			voltdm->sleep(voltdm);
+	}
+}
+
+/**
  * voltdm_for_each_pwrdm - call function for each pwrdm in a voltdm
  * @voltdm: struct voltagedomain * to iterate over
  * @fn: callback function *
diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
index 16a1b09..8829201 100644
--- a/arch/arm/mach-omap2/voltage.h
+++ b/arch/arm/mach-omap2/voltage.h
@@ -54,10 +54,14 @@ struct omap_vfsm_instance {
  * @pwrdm_list: list_head linking all powerdomains in this voltagedomain
  * @vc: pointer to VC channel associated with this voltagedomain
  * @vp: pointer to VP associated with this voltagedomain
+ * @usecount: number of users for this voltagedomain
+ * @target_state: calculated target state for the children of this domain
  * @read: read a VC/VP register
  * @write: write a VC/VP register
  * @read: read-modify-write a VC/VP register
  * @sys_clk: system clock name/frequency, used for various timing calculations
+ * @sleep: function to call once the domain enters idle
+ * @wakeup: function to call once the domain wakes up from idle
  * @scale: function used to scale the voltage of the voltagedomain
  * @nominal_volt: current nominal voltage for this voltage domain
  * @volt_data: voltage table having the distinct voltages supported
@@ -73,6 +77,9 @@ struct voltagedomain {
 	struct omap_vp_instance *vp;
 	struct omap_voltdm_pmic *pmic;
 
+	atomic_t usecount;
+	int target_state;
+
 	/* VC/VP register access functions: SoC specific */
 	u32 (*read) (u8 offset);
 	void (*write) (u32 val, u8 offset);
@@ -83,6 +90,8 @@ struct voltagedomain {
 		u32 rate;
 	} sys_clk;
 
+	void (*sleep) (struct voltagedomain *voltdm);
+	void (*wakeup) (struct voltagedomain *voltdm);
 	int (*scale) (struct voltagedomain *voltdm,
 		      unsigned long target_volt);
 
@@ -161,6 +170,8 @@ extern void omap44xx_voltagedomains_init(void);
 struct voltagedomain *voltdm_lookup(const char *name);
 void voltdm_init(struct voltagedomain **voltdm_list);
 int voltdm_add_pwrdm(struct voltagedomain *voltdm, struct powerdomain *pwrdm);
+void voltdm_pwrdm_enable(struct voltagedomain *voltdm);
+void voltdm_pwrdm_disable(struct voltagedomain *voltdm);
 int voltdm_for_each(int (*fn)(struct voltagedomain *voltdm, void *user),
 		    void *user);
 int voltdm_for_each_pwrdm(struct voltagedomain *voltdm,
-- 
1.7.4.1

[PATCHv2 4/8] arm: omap3: add manual control for mpu / core pwrdm usecounting

[Tero Kristo]

mpu / core powerdomain usecounts are now statically increased
by 1 during MPU activity. This allows the domains to reflect
actual usage, and will allow the usecount to reach 0 just before
entering wfi. Proper powerdomain usecounts are propageted to
voltagedomain level also, and will allow vc callbacks to be
triggered at right point of time.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |   40 +++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 076eb42..b0f9b29 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -47,6 +47,7 @@ enum {
 static LIST_HEAD(pwrdm_list);
 
 static struct pwrdm_ops *arch_pwrdm;
+static struct powerdomain *mpu_pwrdm, *core_pwrdm;
 
 /* Private functions */
 
@@ -1006,11 +1007,17 @@ void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
 int pwrdm_pre_transition(void)
 {
 	pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
+	/* Decrease mpu / core usecounts to indicate we are entering idle */
+	pwrdm_clkdm_disable(mpu_pwrdm);
+	pwrdm_clkdm_disable(core_pwrdm);
 	return 0;
 }
 
 int pwrdm_post_transition(void)
 {
+	/* Increase mpu / core usecounts to indicate we are leaving idle */
+	pwrdm_clkdm_enable(mpu_pwrdm);
+	pwrdm_clkdm_enable(core_pwrdm);
 	pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
 	return 0;
 }
@@ -1090,3 +1097,36 @@ bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm)
 
 	return 0;
 }
+
+/**
+ * pwrdm_usecount_init - initialize special powerdomain usecounts
+ *
+ * Initializes usecounts for the powerdomains that have static
+ * dependencies with MPU idle cycle, namely mpu_pwrdm and core_pwrdm.
+ * These powerdomains will get their usecounts increased / decreased
+ * each sleep cycle so that they reach 0 just before entering wfi,
+ * and are increased to 1 just after it. This allows the dependent
+ * voltage domains to follow idle cycle properly and trigger their
+ * callbacks for sleep / wakeup.
+ */
+static int __init pwrdm_usecount_init(void)
+{
+	mpu_pwrdm = pwrdm_lookup("mpu_pwrdm");
+	if (!mpu_pwrdm) {
+		pr_err("%s: failed to get mpu_pwrdm\n", __func__);
+		return -EINVAL;
+	}
+
+	pwrdm_clkdm_enable(mpu_pwrdm);
+
+	core_pwrdm = pwrdm_lookup("core_pwrdm");
+	if (!core_pwrdm) {
+		pr_err("%s: failed to get core_pwrdm\n", __func__);
+		return -EINVAL;
+	}
+
+	pwrdm_clkdm_enable(core_pwrdm);
+
+	return 0;
+}
+late_initcall(pwrdm_usecount_init);
-- 
1.7.4.1

[PATCHv2 5/8] arm: omap3: set autoidle flags for a few clocks

[Tero Kristo]

dpll3, dpll4 and sdrc_ick are controlled automatically by hardware.
Thus, reflect this with the autoidle flags, the clocks will no longer
show as active in usecount dumps and will allow the voltdm->sleep /
wakeup calls to work properly.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/clock3xxx_data.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index d75e5f6..f3ee983 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -432,6 +432,7 @@ static struct clk dpll3_ck = {
 	.round_rate	= &omap2_dpll_round_rate,
 	.clkdm_name	= "dpll3_clkdm",
 	.recalc		= &omap3_dpll_recalc,
+	.autoidle	= true,
 };
 
 /*
@@ -615,6 +616,7 @@ static struct clk dpll4_ck = {
 	.set_rate	= &omap3_dpll4_set_rate,
 	.clkdm_name	= "dpll4_clkdm",
 	.recalc		= &omap3_dpll_recalc,
+	.autoidle	= true,
 };
 
 /*
@@ -1744,6 +1746,7 @@ static struct clk sdrc_ick = {
 	.flags		= ENABLE_ON_INIT,
 	.clkdm_name	= "core_l3_clkdm",
 	.recalc		= &followparent_recalc,
+	.autoidle	= true,
 };
 
 static struct clk gpmc_fck = {
-- 
1.7.4.1

[PATCHv2 6/8] arm: omap: pm-debug: enhanced usecount debug support

[Tero Kristo]

Voltdm, pwrdm, clkdm, hwmod and clk usecounts are now separeted to
their own file, 'usecount'. This file shows the usecounts for every
active domain and their children recursively. 'count' file now only
shows power state counts for powerdomains.

This patch also provices a way to do printk dumps from kernel code,
by calling the pm_dbg_dump_X functions. The plan is to call these
functions once an error condition is detected, e.g. failed suspend.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/pm-debug.c |  128 ++++++++++++++++++++++++++++++++++------
 arch/arm/mach-omap2/pm.h       |    6 ++
 2 files changed, 115 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c
index 4411163..444588e 100644
--- a/arch/arm/mach-omap2/pm-debug.c
+++ b/arch/arm/mach-omap2/pm-debug.c
@@ -51,6 +51,7 @@ static int pm_dbg_init(void);
 enum {
 	DEBUG_FILE_COUNTERS = 0,
 	DEBUG_FILE_TIMERS,
+	DEBUG_FILE_USECOUNT,
 };
 
 static const char pwrdm_state_names[][PWRDM_MAX_PWRSTS] = {
@@ -75,23 +76,6 @@ void pm_dbg_update_time(struct powerdomain *pwrdm, int prev)
 	pwrdm->timer = t;
 }
 
-static int clkdm_dbg_show_counter(struct clockdomain *clkdm, void *user)
-{
-	struct seq_file *s = (struct seq_file *)user;
-
-	if (strcmp(clkdm->name, "emu_clkdm") == 0 ||
-		strcmp(clkdm->name, "wkup_clkdm") == 0 ||
-		strncmp(clkdm->name, "dpll", 4) == 0)
-		return 0;
-
-	seq_printf(s, "%s->%s (%d)", clkdm->name,
-			clkdm->pwrdm.ptr->name,
-			atomic_read(&clkdm->usecount));
-	seq_printf(s, "\n");
-
-	return 0;
-}
-
 static int pwrdm_dbg_show_counter(struct powerdomain *pwrdm, void *user)
 {
 	struct seq_file *s = (struct seq_file *)user;
@@ -145,11 +129,112 @@ static int pwrdm_dbg_show_timer(struct powerdomain *pwrdm, void *user)
 	return 0;
 }
 
+static struct voltagedomain *parent_voltdm;
+static struct powerdomain *parent_pwrdm;
+static struct clockdomain *parent_clkdm;
+
+#define PM_DBG_PRINT(s, fmt, args...)			\
+	{						\
+		if (s)					\
+			seq_printf(s, fmt, ## args);	\
+		else					\
+			pr_info(fmt, ## args);		\
+	}
+
+static int _pm_dbg_dump_clk(struct clk *clk, void *user)
+{
+	struct seq_file *s = user;
+
+	if (clk->clkdm == parent_clkdm && clk->usecount && !clk->autoidle)
+		PM_DBG_PRINT(s, "      ck:%s: %d\n", clk->name, clk->usecount);
+
+	return 0;
+}
+
+static int _pm_dbg_dump_hwmod(struct omap_hwmod *oh, void *user)
+{
+	struct seq_file *s = user;
+
+	if (oh->clkdm != parent_clkdm)
+		return 0;
+
+	if (oh->_state != _HWMOD_STATE_ENABLED)
+		return 0;
+
+	PM_DBG_PRINT(s, "      oh:%s: enabled\n", oh->name);
+
+	return 0;
+}
+
+static int _pm_dbg_dump_clkdm(struct clockdomain *clkdm, void *user)
+{
+	struct seq_file *s = user;
+	u32 usecount;
+
+	if (clkdm->pwrdm.ptr == parent_pwrdm) {
+		usecount = atomic_read(&clkdm->usecount);
+		if (usecount) {
+			PM_DBG_PRINT(s, "    cd:%s: %d\n", clkdm->name,
+				usecount);
+			parent_clkdm = clkdm;
+			omap_hwmod_for_each(_pm_dbg_dump_hwmod, s);
+			omap_clk_for_each(_pm_dbg_dump_clk, s);
+		}
+	}
+	return 0;
+}
+
+static int _pm_dbg_dump_pwrdm(struct powerdomain *pwrdm, void *user)
+{
+	struct seq_file *s = user;
+	u32 usecount;
+
+	if (pwrdm->voltdm.ptr == parent_voltdm) {
+		usecount = atomic_read(&pwrdm->usecount);
+		if (usecount) {
+			PM_DBG_PRINT(s, "  pd:%s: %d\n", pwrdm->name, usecount);
+			parent_pwrdm = pwrdm;
+			clkdm_for_each(_pm_dbg_dump_clkdm, s);
+		}
+	}
+	return 0;
+}
+
+void pm_dbg_dump_pwrdm(struct powerdomain *pwrdm)
+{
+	pr_info("pd:%s: %d\n", pwrdm->name, atomic_read(&pwrdm->usecount));
+	parent_pwrdm = pwrdm;
+	clkdm_for_each(_pm_dbg_dump_clkdm, NULL);
+}
+
+void pm_dbg_dump_voltdm(struct voltagedomain *voltdm)
+{
+	pr_info("vd:%s: %d\n", voltdm->name, atomic_read(&voltdm->usecount));
+	parent_voltdm = voltdm;
+	pwrdm_for_each(_pm_dbg_dump_pwrdm, NULL);
+}
+
+static int _voltdm_dbg_show_counters(struct voltagedomain *voltdm, void *user)
+{
+	struct seq_file *s = user;
+
+	seq_printf(s, "vd:%s: %d\n", voltdm->name,
+		atomic_read(&voltdm->usecount));
+
+	parent_voltdm = voltdm;
+	pwrdm_for_each(_pm_dbg_dump_pwrdm, s);
+	return 0;
+}
+
+static int pm_dbg_show_usecount(struct seq_file *s, void *unused)
+{
+	voltdm_for_each(_voltdm_dbg_show_counters, s);
+	return 0;
+}
+
 static int pm_dbg_show_counters(struct seq_file *s, void *unused)
 {
 	pwrdm_for_each(pwrdm_dbg_show_counter, s);
-	clkdm_for_each(clkdm_dbg_show_counter, s);
-
 	return 0;
 }
 
@@ -162,6 +247,9 @@ static int pm_dbg_show_timers(struct seq_file *s, void *unused)
 static int pm_dbg_open(struct inode *inode, struct file *file)
 {
 	switch ((int)inode->i_private) {
+	case DEBUG_FILE_USECOUNT:
+		return single_open(file, pm_dbg_show_usecount,
+			&inode->i_private);
 	case DEBUG_FILE_COUNTERS:
 		return single_open(file, pm_dbg_show_counters,
 			&inode->i_private);
@@ -271,6 +359,8 @@ static int __init pm_dbg_init(void)
 		d, (void *)DEBUG_FILE_COUNTERS, &debug_fops);
 	(void) debugfs_create_file("time", S_IRUGO,
 		d, (void *)DEBUG_FILE_TIMERS, &debug_fops);
+	(void) debugfs_create_file("usecount", S_IRUGO,
+		d, (void *)DEBUG_FILE_USECOUNT, &debug_fops);
 
 	pwrdm_for_each(pwrdms_setup, (void *)d);
 
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index b737b11..ec8f874 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -61,10 +61,16 @@ inline void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
 extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
 extern int omap3_pm_set_suspend_state(struct powerdomain *pwrdm, int state);
 
+struct clk;
+
 #ifdef CONFIG_PM_DEBUG
 extern u32 enable_off_mode;
+extern void pm_dbg_dump_pwrdm(struct powerdomain *pwrdm);
+extern void pm_dbg_dump_voltdm(struct voltagedomain *voltdm);
 #else
 #define enable_off_mode 0
+static inline void pm_dbg_dump_pwrdm(struct powerdomain *pwrdm) { }
+static inline void pm_dbg_dump_voltdm(struct voltagedomain *voltdm) { }
 #endif
 
 #if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS)
-- 
1.7.4.1

[PATCHv2 7/8] arm: omap: clockdomain: add support for preventing domain transitions

[Tero Kristo]

Some clockdomains can't support manual domain transitions triggered by
clock framework, and must be prevented from doing so. Added clkdm flag
CLKDM_NO_MANUAL_TRANS for doing this.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/clockdomain.c |    6 ++++++
 arch/arm/mach-omap2/clockdomain.h |    3 +++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index c0b6187..850665d 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -944,6 +944,9 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 	if ((clkdm_usecount_inc(clkdm) > 1) && autodeps)
 		return 0;
 
+	if (clkdm->flags & CLKDM_NO_MANUAL_TRANS)
+		return 0;
+
 	spin_lock_irqsave(&clkdm->lock, flags);
 	arch_clkdm->clkdm_clk_enable(clkdm);
 	pwrdm_wait_transition(clkdm->pwrdm.ptr);
@@ -970,6 +973,9 @@ static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
 	if (clkdm_usecount_dec(clkdm) > 0)
 		return 0;
 
+	if (clkdm->flags & CLKDM_NO_MANUAL_TRANS)
+		return 0;
+
 	spin_lock_irqsave(&clkdm->lock, flags);
 	arch_clkdm->clkdm_clk_disable(clkdm);
 	pwrdm_clkdm_state_switch(clkdm);
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 373399a..8693bec 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -31,12 +31,15 @@
  *
  * CLKDM_NO_AUTODEPS: Prevent "autodeps" from being added/removed from this
  *     clockdomain.  (Currently, this applies to OMAP3 clockdomains only.)
+ * CLKDM_NO_MANUAL_TRANS: Prevent clockdomain code from attempting to change
+ *     clockdomain state manually. Needed for PER domain on omap3.
  */
 #define CLKDM_CAN_FORCE_SLEEP			(1 << 0)
 #define CLKDM_CAN_FORCE_WAKEUP			(1 << 1)
 #define CLKDM_CAN_ENABLE_AUTO			(1 << 2)
 #define CLKDM_CAN_DISABLE_AUTO			(1 << 3)
 #define CLKDM_NO_AUTODEPS			(1 << 4)
+#define CLKDM_NO_MANUAL_TRANS			(1 << 5)
 
 #define CLKDM_CAN_HWSUP		(CLKDM_CAN_ENABLE_AUTO | CLKDM_CAN_DISABLE_AUTO)
 #define CLKDM_CAN_SWSUP		(CLKDM_CAN_FORCE_SLEEP | CLKDM_CAN_FORCE_WAKEUP)
-- 
1.7.4.1

[PATCHv2 7/8] arm: omap: clockdomain: add support for preventing domain transitions

[Kevin Hilman]

Tero Kristo <t-kristo@ti.com> writes:

> Some clockdomains can't support manual domain transitions triggered by
> clock framework, and must be prevented from doing so. Added clkdm flag
> CLKDM_NO_MANUAL_TRANS for doing this.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@ti.com>

Dumb Q: what's the difference between this new flag and CLKDM_CAN_SWSUP?

Kevin

[PATCHv2 7/8] arm: omap: clockdomain: add support for preventing domain transitions

[Tero Kristo]

On Wed, 2012-02-15 at 11:35 -0800, Kevin Hilman wrote:
> Tero Kristo <t-kristo@ti.com> writes:
> 
> > Some clockdomains can't support manual domain transitions triggered by
> > clock framework, and must be prevented from doing so. Added clkdm flag
> > CLKDM_NO_MANUAL_TRANS for doing this.
> >
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Kevin Hilman <khilman@ti.com>
> 
> Dumb Q: what's the difference between this new flag and CLKDM_CAN_SWSUP?

CAN_SWSUP controls software wakeup / sleep (clktrctrl values 1 and 2),
but it still allows hwsup based transitions during runtime (basically
using values 0 & 3 for the same register.) NO_MANUAL_TRANS prevents
both, but still allows hwauto mode being enabled in the first place.

I was thinking about adding a flag for preventing the autodep disabling
as this was the root cause for the problem I saw, but I wanted to
optimize the _clkdm_clk_hwmod_enable/disable for this case, as the
domain is not going through any transitions.

-Tero

[PATCHv2 7/8] arm: omap: clockdomain: add support for preventing domain transitions

[Shilimkar, Santosh]

Tero,
On Thu, Feb 16, 2012 at 2:09 PM, Tero Kristo <t-kristo@ti.com> wrote:
> On Wed, 2012-02-15 at 11:35 -0800, Kevin Hilman wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>>
>> > Some clockdomains can't support manual domain transitions triggered by
>> > clock framework, and must be prevented from doing so. Added clkdm flag
>> > CLKDM_NO_MANUAL_TRANS for doing this.
>> >
>> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> > Cc: Paul Walmsley <paul@pwsan.com>
>> > Cc: Kevin Hilman <khilman@ti.com>
>>
>> Dumb Q: what's the difference between this new flag and CLKDM_CAN_SWSUP?
>
> CAN_SWSUP controls software wakeup / sleep (clktrctrl values 1 and 2),
> but it still allows hwsup based transitions during runtime (basically
> using values 0 & 3 for the same register.) NO_MANUAL_TRANS prevents
> both, but still allows hwauto mode being enabled in the first place.
>
> I was thinking about adding a flag for preventing the autodep disabling
> as this was the root cause for the problem I saw, but I wanted to
> optimize the _clkdm_clk_hwmod_enable/disable for this case, as the
> domain is not going through any transitions.
>
Which clock-domain you are refering here which needs this
flag ?

Regards
santosh

[PATCHv2 7/8] arm: omap: clockdomain: add support for preventing domain transitions

[Tero Kristo]

On Thu, 2012-02-16 at 14:13 +0530, Shilimkar, Santosh wrote:
> Tero,
> On Thu, Feb 16, 2012 at 2:09 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > On Wed, 2012-02-15 at 11:35 -0800, Kevin Hilman wrote:
> >> Tero Kristo <t-kristo@ti.com> writes:
> >>
> >> > Some clockdomains can't support manual domain transitions triggered by
> >> > clock framework, and must be prevented from doing so. Added clkdm flag
> >> > CLKDM_NO_MANUAL_TRANS for doing this.
> >> >
> >> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> >> > Cc: Paul Walmsley <paul@pwsan.com>
> >> > Cc: Kevin Hilman <khilman@ti.com>
> >>
> >> Dumb Q: what's the difference between this new flag and CLKDM_CAN_SWSUP?
> >
> > CAN_SWSUP controls software wakeup / sleep (clktrctrl values 1 and 2),
> > but it still allows hwsup based transitions during runtime (basically
> > using values 0 & 3 for the same register.) NO_MANUAL_TRANS prevents
> > both, but still allows hwauto mode being enabled in the first place.
> >
> > I was thinking about adding a flag for preventing the autodep disabling
> > as this was the root cause for the problem I saw, but I wanted to
> > optimize the _clkdm_clk_hwmod_enable/disable for this case, as the
> > domain is not going through any transitions.
> >
> Which clock-domain you are refering here which needs this
> flag ?

per_clkdm, see patch 8 in this same set for comments. This happens with
omap3.

> 
> Regards
> santosh

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

Attempting this will cause problems especially with off-mode enabled.
Previously this issue was hidden by the fact that per_clkdm never
attempted manual idle by software, as the usecounts for the clockdomain
were broken.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/clockdomains3xxx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomains3xxx_data.c b/arch/arm/mach-omap2/clockdomains3xxx_data.c
index b84e138..db31bbf 100644
--- a/arch/arm/mach-omap2/clockdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/clockdomains3xxx_data.c
@@ -282,7 +282,7 @@ static struct clockdomain usbhost_clkdm = {
 static struct clockdomain per_clkdm = {
 	.name		= "per_clkdm",
 	.pwrdm		= { .name = "per_pwrdm" },
-	.flags		= CLKDM_CAN_HWSUP_SWSUP,
+	.flags		= CLKDM_CAN_HWSUP_SWSUP | CLKDM_NO_MANUAL_TRANS,
 	.dep_bit	= OMAP3430_EN_PER_SHIFT,
 	.wkdep_srcs	= per_wkdeps,
 	.sleepdep_srcs	= per_sleepdeps,
-- 
1.7.4.1

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Kevin Hilman]

Tero Kristo <t-kristo@ti.com> writes:

> Attempting this will cause problems especially with off-mode enabled.

Please be more verbose about the problems seen, and the root cause(s).

Kevin

> Previously this issue was hidden by the fact that per_clkdm never
> attempted manual idle by software, as the usecounts for the clockdomain
> were broken.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/mach-omap2/clockdomains3xxx_data.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clockdomains3xxx_data.c b/arch/arm/mach-omap2/clockdomains3xxx_data.c
> index b84e138..db31bbf 100644
> --- a/arch/arm/mach-omap2/clockdomains3xxx_data.c
> +++ b/arch/arm/mach-omap2/clockdomains3xxx_data.c
> @@ -282,7 +282,7 @@ static struct clockdomain usbhost_clkdm = {
>  static struct clockdomain per_clkdm = {
>  	.name		= "per_clkdm",
>  	.pwrdm		= { .name = "per_pwrdm" },
> -	.flags		= CLKDM_CAN_HWSUP_SWSUP,
> +	.flags		= CLKDM_CAN_HWSUP_SWSUP | CLKDM_NO_MANUAL_TRANS,
>  	.dep_bit	= OMAP3430_EN_PER_SHIFT,
>  	.wkdep_srcs	= per_wkdeps,
>  	.sleepdep_srcs	= per_sleepdeps,

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
> Tero Kristo <t-kristo@ti.com> writes:
> 
> > Attempting this will cause problems especially with off-mode enabled.
> 
> Please be more verbose about the problems seen, and the root cause(s).
> 

I was actually looking forward for some help with this commit message,
as I am still not quite sure what is going on in here. :) Here is the
log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
I think that is probably fixed by the patch from Paul,
omap_set_pwrdm_state() does not work properly.) The warning comes out
after wakeup from off-mode, and it is triggered by the disabling of
autodeps before off-mode entry.



[   79.010345] PM: Syncing filesystems ... done.
[   79.083801] Freezing user space processes ... (elapsed 0.01 seconds)
done.
[   79.110839] Freezing remaining freezable tasks ... (elapsed 0.02
seconds) don
e.
[   79.141845] Suspending console(s) (use no_console_suspend to debug)
[   79.266815] PM: suspend of devices complete after 115.551 msecs
[   79.269958] PM: late suspend of devices complete after 3.142 msecs
[   79.270050] Disabling non-boot CPUs ...
[   79.697235] Powerdomain (cam_pwrdm) didn't enter target state 0
[   79.697265] Could not enter target state in pm_suspend
[   79.699829] PM: early resume of devices complete after 2.257 msecs
[   79.699920] ------------[ cut here ]------------
[   79.699981] WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:161
omap3_l3_app_ir
q+0xd8/0x130()
[   79.699981] In-band Error seen by MPU  at address 0
[   79.699981] Modules linked in:
[   79.700042] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
[<c00428d0>] (warn_
slowpath_common+0x4c/0x64)
[   79.700073] [<c00428d0>] (warn_slowpath_common+0x4c/0x64) from
[<c004297c>] (
warn_slowpath_fmt+0x30/0x40)
[   79.700103] [<c004297c>] (warn_slowpath_fmt+0x30/0x40) from
[<c0034a2c>] (oma
p3_l3_app_irq+0xd8/0x130)
[   79.700103] [<c0034a2c>] (omap3_l3_app_irq+0xd8/0x130) from
[<c00a09e4>] (han
dle_irq_event_percpu+0x5c/0x22c)
[   79.700134] [<c00a09e4>] (handle_irq_event_percpu+0x5c/0x22c) from
[<c00a0bf0
>] (handle_irq_event+0x3c/0x5c)
[   79.700164] [<c00a0bf0>] (handle_irq_event+0x3c/0x5c) from
[<c00a3a70>] (hand
le_level_irq+0xac/0x118)
[   79.700195] [<c00a3a70>] (handle_level_irq+0xac/0x118) from
[<c00a0490>] (gen
eric_handle_irq+0x34/0x44)
[   79.700225] [<c00a0490>] (generic_handle_irq+0x34/0x44) from
[<c00151ec>] (ha
IdRQ+0x4c/0xac)
[   79.700256] [<c00151ec>] (handle_IRQ+0x4c/0xac) from [<c0008548>]
(omap3_intc
_handle_irq+0x44/0x4c)
[   79.700256] [<c0008548>] (omap3_intc_handle_irq+0x44/0x4c) from
[<c0476824>] 
(__irq_svc+0x44/0x60)
[   79.700286] Exception stack(0xcefe7e78 to 0xcefe7ec0)
[   79.700286] 7e60:
00007
a93 cedb25d0
[   79.700317] 7e80: 00000000 cedb2140 60000153 c0676994 00000000
c0676994 60000
153 00000000
[   79.700347] 7ea0: c0676940 000a3008 c0721d40 cefe7ec0 00007a94
c04765e4 20000
153 ffffffff
[   79.700347] [<c0476824>] (__irq_svc+0x44/0x60) from [<c04765e4>]
(_raw_spin_u
nlock_irqrestore+0x34/0x44)
[   79.700378] [<c04765e4>] (_raw_spin_unlock_irqrestore+0x34/0x44) from
[<c00a5
e18>] (resume_irqs+0x9c/0xbc)
[   79.700408] [<c00a5e18>] (resume_irqs+0x9c/0xbc) from [<c008061c>]
(suspend_d
evices_and_enter+0x114/0x2d8)
[   79.700439] [<c008061c>] (suspend_devices_and_enter+0x114/0x2d8) from
[<c0080
91c>] (enter_state+0x13c/0x184)
[   79.700469] [<c008091c>] (enter_state+0x13c/0x184) from [<c007f5fc>]
(state_s
tore+0xd0/0x170)
[   79.700500] [<c007f5fc>] (state_store+0xd0/0x170) from [<c02536a8>]
(kobj_att
r_store+0x18/0x1c)
[   79.700531] [<c02536a8>] (kobj_attr_store+0x18/0x1c) from
[<c0167a9c>] (sysfs
_write_file+0xfc/0x180)
[   79.700561] [<c0167a9c>] (sysfs_write_file+0xfc/0x180) from
[<c0107ed4>] (vfs
_write+0xb0/0x134)
[   79.700561] [<c0107ed4>] (vfs_write+0xb0/0x134) from [<c0108028>]
(sys_write+
0x40/0x70)
[   79.700592] [<c0108028>] (sys_write+0x40/0x70) from [<c0014160>]
(ret_fast_sy
scall+0x0/0x3c)
[   79.700622] ---[ end trace 338e34a6f123bc2b ]---
[   80.121765] PM: resume of devices complete after 420.012 msecs
[   80.414886] Restarting tasks ... done.


> Kevin
> 
> > Previously this issue was hidden by the fact that per_clkdm never
> > attempted manual idle by software, as the usecounts for the clockdomain
> > were broken.
> >
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Kevin Hilman <khilman@ti.com>
> > ---
> >  arch/arm/mach-omap2/clockdomains3xxx_data.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/clockdomains3xxx_data.c b/arch/arm/mach-omap2/clockdomains3xxx_data.c
> > index b84e138..db31bbf 100644
> > --- a/arch/arm/mach-omap2/clockdomains3xxx_data.c
> > +++ b/arch/arm/mach-omap2/clockdomains3xxx_data.c
> > @@ -282,7 +282,7 @@ static struct clockdomain usbhost_clkdm = {
> >  static struct clockdomain per_clkdm = {
> >  	.name		= "per_clkdm",
> >  	.pwrdm		= { .name = "per_pwrdm" },
> > -	.flags		= CLKDM_CAN_HWSUP_SWSUP,
> > +	.flags		= CLKDM_CAN_HWSUP_SWSUP | CLKDM_NO_MANUAL_TRANS,
> >  	.dep_bit	= OMAP3430_EN_PER_SHIFT,
> >  	.wkdep_srcs	= per_wkdeps,
> >  	.sleepdep_srcs	= per_sleepdeps,

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Shilimkar, Santosh]

On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>>
>> > Attempting this will cause problems especially with off-mode enabled.
>>
>> Please be more verbose about the problems seen, and the root cause(s).
>>
>
> I was actually looking forward for some help with this commit message,
> as I am still not quite sure what is going on in here. :) Here is the
> log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> I think that is probably fixed by the patch from Paul,
> omap_set_pwrdm_state() does not work properly.) The warning comes out
> after wakeup from off-mode, and it is triggered by the disabling of
> autodeps before off-mode entry.
>
This mostly indicates that one of the per clock-domain module
clock turning ON seems to be not working well with auto deps
disabled. This leads to interconnect violation.

if not tried already, can you put the per_clockdomain in SW_WKUP
in the low power code early resume path and see if this
error goes away.

regards
Santosh

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
> >> Tero Kristo <t-kristo@ti.com> writes:
> >>
> >> > Attempting this will cause problems especially with off-mode enabled.
> >>
> >> Please be more verbose about the problems seen, and the root cause(s).
> >>
> >
> > I was actually looking forward for some help with this commit message,
> > as I am still not quite sure what is going on in here. :) Here is the
> > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> > I think that is probably fixed by the patch from Paul,
> > omap_set_pwrdm_state() does not work properly.) The warning comes out
> > after wakeup from off-mode, and it is triggered by the disabling of
> > autodeps before off-mode entry.
> >
> This mostly indicates that one of the per clock-domain module
> clock turning ON seems to be not working well with auto deps
> disabled. This leads to interconnect violation.
> 
> if not tried already, can you put the per_clockdomain in SW_WKUP
> in the low power code early resume path and see if this
> error goes away.

This seems to get rid of the dump also. It looks like some driver resume
is not behaving nicely, I am trying to pinpoint the culprit currently
and see whether it can provide more info.

-Tero

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
> > >> Tero Kristo <t-kristo@ti.com> writes:
> > >>
> > >> > Attempting this will cause problems especially with off-mode enabled.
> > >>
> > >> Please be more verbose about the problems seen, and the root cause(s).
> > >>
> > >
> > > I was actually looking forward for some help with this commit message,
> > > as I am still not quite sure what is going on in here. :) Here is the
> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> > > I think that is probably fixed by the patch from Paul,
> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
> > > after wakeup from off-mode, and it is triggered by the disabling of
> > > autodeps before off-mode entry.
> > >
> > This mostly indicates that one of the per clock-domain module
> > clock turning ON seems to be not working well with auto deps
> > disabled. This leads to interconnect violation.
> > 
> > if not tried already, can you put the per_clockdomain in SW_WKUP
> > in the low power code early resume path and see if this
> > error goes away.
> 
> This seems to get rid of the dump also. It looks like some driver resume
> is not behaving nicely, I am trying to pinpoint the culprit currently
> and see whether it can provide more info.

Okay, I have some more info about this now.

What happens is that when entering off-mode, PER domain remains OFF even
during the execution of the exit phase from omap_sram_idle. Adding a
manual SW_WKUP it comes up and there are no issues. If autodeps are
enabled on the domain, it comes back from off mode as active.

Looking further in the code, we have this at the end of omap_sram_idle:

        if (per_next_state < PWRDM_POWER_ON) {
                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
                omap2_gpio_resume_after_idle();
                wake_per();
                if (per_prev_state == PWRDM_POWER_OFF)
                        omap3_per_restore_context();
        }

... which seems to assume that per domain is on. Gpio code does not
control any clocks currently, as it only requires the interface clock to
be on, and as this is autoidled....

Any comments how we should handle this? Shall we just keep these two
patches for handling this or add some different hackery for the gpio
issue?

-Tero

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Shilimkar, Santosh]

On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
> On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
>> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
>> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
>> > >> Tero Kristo <t-kristo@ti.com> writes:
>> > >>
>> > >> > Attempting this will cause problems especially with off-mode enabled.
>> > >>
>> > >> Please be more verbose about the problems seen, and the root cause(s).
>> > >>
>> > >
>> > > I was actually looking forward for some help with this commit message,
>> > > as I am still not quite sure what is going on in here. :) Here is the
>> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
>> > > I think that is probably fixed by the patch from Paul,
>> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
>> > > after wakeup from off-mode, and it is triggered by the disabling of
>> > > autodeps before off-mode entry.
>> > >
>> > This mostly indicates that one of the per clock-domain module
>> > clock turning ON seems to be not working well with auto deps
>> > disabled. This leads to interconnect violation.
>> >
>> > if not tried already, can you put the per_clockdomain in SW_WKUP
>> > in the low power code early resume path and see if this
>> > error goes away.
>>
>> This seems to get rid of the dump also. It looks like some driver resume
>> is not behaving nicely, I am trying to pinpoint the culprit currently
>> and see whether it can provide more info.
>
> Okay, I have some more info about this now.
>
> What happens is that when entering off-mode, PER domain remains OFF even
> during the execution of the exit phase from omap_sram_idle. Adding a
> manual SW_WKUP it comes up and there are no issues. If autodeps are
> enabled on the domain, it comes back from off mode as active.
>
> Looking further in the code, we have this at the end of omap_sram_idle:
>
> ? ? ? ?if (per_next_state < PWRDM_POWER_ON) {
> ? ? ? ? ? ? ? ?per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> ? ? ? ? ? ? ? ?omap2_gpio_resume_after_idle();
> ? ? ? ? ? ? ? ?wake_per();
> ? ? ? ? ? ? ? ?if (per_prev_state == PWRDM_POWER_OFF)
> ? ? ? ? ? ? ? ? ? ? ? ?omap3_per_restore_context();
> ? ? ? ?}
>
> ... which seems to assume that per domain is on. Gpio code does not
> control any clocks currently, as it only requires the interface clock to
> be on, and as this is autoidled....
>
> Any comments how we should handle this? Shall we just keep these two
> patches for handling this or add some different hackery for the gpio
> issue?
>
Good. I thought too that issue will disappear.
The issue is pretty clear. Technically every driver pm_runtime() code should
be able to manage a clock->clockdomain->power domain power up/down
sequence. That should work without auto deps.

Do you narrowed down which driver resume is creating the dump ?
UART , GPIO ?

Regards
Santosh

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
> >> > >> Tero Kristo <t-kristo@ti.com> writes:
> >> > >>
> >> > >> > Attempting this will cause problems especially with off-mode enabled.
> >> > >>
> >> > >> Please be more verbose about the problems seen, and the root cause(s).
> >> > >>
> >> > >
> >> > > I was actually looking forward for some help with this commit message,
> >> > > as I am still not quite sure what is going on in here. :) Here is the
> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> >> > > I think that is probably fixed by the patch from Paul,
> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
> >> > > after wakeup from off-mode, and it is triggered by the disabling of
> >> > > autodeps before off-mode entry.
> >> > >
> >> > This mostly indicates that one of the per clock-domain module
> >> > clock turning ON seems to be not working well with auto deps
> >> > disabled. This leads to interconnect violation.
> >> >
> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
> >> > in the low power code early resume path and see if this
> >> > error goes away.
> >>
> >> This seems to get rid of the dump also. It looks like some driver resume
> >> is not behaving nicely, I am trying to pinpoint the culprit currently
> >> and see whether it can provide more info.
> >
> > Okay, I have some more info about this now.
> >
> > What happens is that when entering off-mode, PER domain remains OFF even
> > during the execution of the exit phase from omap_sram_idle. Adding a
> > manual SW_WKUP it comes up and there are no issues. If autodeps are
> > enabled on the domain, it comes back from off mode as active.
> >
> > Looking further in the code, we have this at the end of omap_sram_idle:
> >
> >        if (per_next_state < PWRDM_POWER_ON) {
> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> >                omap2_gpio_resume_after_idle();
> >                wake_per();
> >                if (per_prev_state == PWRDM_POWER_OFF)
> >                        omap3_per_restore_context();
> >        }
> >
> > ... which seems to assume that per domain is on. Gpio code does not
> > control any clocks currently, as it only requires the interface clock to
> > be on, and as this is autoidled....
> >
> > Any comments how we should handle this? Shall we just keep these two
> > patches for handling this or add some different hackery for the gpio
> > issue?
> >
> Good. I thought too that issue will disappear.
> The issue is pretty clear. Technically every driver pm_runtime() code should
> be able to manage a clock->clockdomain->power domain power up/down
> sequence. That should work without auto deps.
> 
> Do you narrowed down which driver resume is creating the dump ?
> UART , GPIO ?

It is the gpio base stuff called from omap_sram_idle(), basically the
restore context part. If I force enable per domain before the code
snippet before, there is no dump, but if it is done after, I get the
dump.

The thing is that gpio driver doesn't currently have this kind of
mechanism for the restore context part, at least not on omap3 due to
above clocking issue (only autoidled interface clock is used.)

-Tero

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Kevin Hilman]

Tero Kristo <t-kristo@ti.com> writes:

> On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
>> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
>> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
>> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
>> >> > >> Tero Kristo <t-kristo@ti.com> writes:
>> >> > >>
>> >> > >> > Attempting this will cause problems especially with off-mode enabled.
>> >> > >>
>> >> > >> Please be more verbose about the problems seen, and the root cause(s).
>> >> > >>
>> >> > >
>> >> > > I was actually looking forward for some help with this commit message,
>> >> > > as I am still not quite sure what is going on in here. :) Here is the
>> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
>> >> > > I think that is probably fixed by the patch from Paul,
>> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
>> >> > > after wakeup from off-mode, and it is triggered by the disabling of
>> >> > > autodeps before off-mode entry.
>> >> > >
>> >> > This mostly indicates that one of the per clock-domain module
>> >> > clock turning ON seems to be not working well with auto deps
>> >> > disabled. This leads to interconnect violation.
>> >> >
>> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
>> >> > in the low power code early resume path and see if this
>> >> > error goes away.
>> >>
>> >> This seems to get rid of the dump also. It looks like some driver resume
>> >> is not behaving nicely, I am trying to pinpoint the culprit currently
>> >> and see whether it can provide more info.
>> >
>> > Okay, I have some more info about this now.
>> >
>> > What happens is that when entering off-mode, PER domain remains OFF even
>> > during the execution of the exit phase from omap_sram_idle. Adding a
>> > manual SW_WKUP it comes up and there are no issues. If autodeps are
>> > enabled on the domain, it comes back from off mode as active.
>> >
>> > Looking further in the code, we have this at the end of omap_sram_idle:
>> >
>> >        if (per_next_state < PWRDM_POWER_ON) {
>> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
>> >                omap2_gpio_resume_after_idle();
>> >                wake_per();
>> >                if (per_prev_state == PWRDM_POWER_OFF)
>> >                        omap3_per_restore_context();
>> >        }
>> >
>> > ... which seems to assume that per domain is on. Gpio code does not
>> > control any clocks currently, as it only requires the interface clock to
>> > be on, and as this is autoidled....
>> >
>> > Any comments how we should handle this? Shall we just keep these two
>> > patches for handling this or add some different hackery for the gpio
>> > issue?
>> >
>> Good. I thought too that issue will disappear.
>> The issue is pretty clear. Technically every driver pm_runtime() code should
>> be able to manage a clock->clockdomain->power domain power up/down
>> sequence. That should work without auto deps.
>> 
>> Do you narrowed down which driver resume is creating the dump ?
>> UART , GPIO ?
>
> It is the gpio base stuff called from omap_sram_idle(), basically the
> restore context part. If I force enable per domain before the code
> snippet before, there is no dump, but if it is done after, I get the
> dump.
>
> The thing is that gpio driver doesn't currently have this kind of
> mechanism for the restore context part, at least not on omap3 due to
> above clocking issue (only autoidled interface clock is used.)

I'm not sure if it fully addresses this, but Tarun's series converts
GPIO to runtime PM.

Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
branch here:
git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git

Kevin

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Thu, 2012-02-16 at 09:31 -0800, Kevin Hilman wrote:
> Tero Kristo <t-kristo@ti.com> writes:
> 
> > On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
> >> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
> >> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> >> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
> >> >> > >> Tero Kristo <t-kristo@ti.com> writes:
> >> >> > >>
> >> >> > >> > Attempting this will cause problems especially with off-mode enabled.
> >> >> > >>
> >> >> > >> Please be more verbose about the problems seen, and the root cause(s).
> >> >> > >>
> >> >> > >
> >> >> > > I was actually looking forward for some help with this commit message,
> >> >> > > as I am still not quite sure what is going on in here. :) Here is the
> >> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> >> >> > > I think that is probably fixed by the patch from Paul,
> >> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
> >> >> > > after wakeup from off-mode, and it is triggered by the disabling of
> >> >> > > autodeps before off-mode entry.
> >> >> > >
> >> >> > This mostly indicates that one of the per clock-domain module
> >> >> > clock turning ON seems to be not working well with auto deps
> >> >> > disabled. This leads to interconnect violation.
> >> >> >
> >> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
> >> >> > in the low power code early resume path and see if this
> >> >> > error goes away.
> >> >>
> >> >> This seems to get rid of the dump also. It looks like some driver resume
> >> >> is not behaving nicely, I am trying to pinpoint the culprit currently
> >> >> and see whether it can provide more info.
> >> >
> >> > Okay, I have some more info about this now.
> >> >
> >> > What happens is that when entering off-mode, PER domain remains OFF even
> >> > during the execution of the exit phase from omap_sram_idle. Adding a
> >> > manual SW_WKUP it comes up and there are no issues. If autodeps are
> >> > enabled on the domain, it comes back from off mode as active.
> >> >
> >> > Looking further in the code, we have this at the end of omap_sram_idle:
> >> >
> >> >        if (per_next_state < PWRDM_POWER_ON) {
> >> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> >> >                omap2_gpio_resume_after_idle();
> >> >                wake_per();
> >> >                if (per_prev_state == PWRDM_POWER_OFF)
> >> >                        omap3_per_restore_context();
> >> >        }
> >> >
> >> > ... which seems to assume that per domain is on. Gpio code does not
> >> > control any clocks currently, as it only requires the interface clock to
> >> > be on, and as this is autoidled....
> >> >
> >> > Any comments how we should handle this? Shall we just keep these two
> >> > patches for handling this or add some different hackery for the gpio
> >> > issue?
> >> >
> >> Good. I thought too that issue will disappear.
> >> The issue is pretty clear. Technically every driver pm_runtime() code should
> >> be able to manage a clock->clockdomain->power domain power up/down
> >> sequence. That should work without auto deps.
> >> 
> >> Do you narrowed down which driver resume is creating the dump ?
> >> UART , GPIO ?
> >
> > It is the gpio base stuff called from omap_sram_idle(), basically the
> > restore context part. If I force enable per domain before the code
> > snippet before, there is no dump, but if it is done after, I get the
> > dump.
> >
> > The thing is that gpio driver doesn't currently have this kind of
> > mechanism for the restore context part, at least not on omap3 due to
> > above clocking issue (only autoidled interface clock is used.)
> 
> I'm not sure if it fully addresses this, but Tarun's series converts
> GPIO to runtime PM.
> 
> Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
> branch here:
> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git

It does something for the issue, but I still get this during suspend to
off:

[   11.284973] PM: Syncing filesystems ... done.
[   11.379241] Freezing user space processes ... (elapsed 0.02 seconds)
done.
[   11.408233] Freezing remaining freezable tasks ... (elapsed 0.02
seconds) don
e.
[   11.439239] Suspending console(s) (use no_console_suspend to debug)
[   11.564178] PM: suspend of devices complete after 115.506 msecs
[   11.567382] PM: late suspend of devices complete after 3.204 msecs
[   11.567443] Disabling non-boot CPUs ...
[   12.004089] Powerdomain (cam_pwrdm) didn't enter target state 0
[   12.004119] Could not enter target state in pm_suspend
[   12.009368] PM: early resume of devices complete after 4.944 msecs
[   12.436645] PM: resume of devices complete after 426.086 msecs
[   12.480285] Restarting tasks ... done.
/sys/kernel/debu[   12.488433] ------------[ cut here ]------------
[   12.494415] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:1604 _idle
+0x164/0x1
7c()
[   12.502258] omap_hwmod: gpio6: idle state can only be entered from
enabled st
ate
[   12.509979] Modules linked in:
[   12.513214] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
[<c0042968>] (warn_
slowpath_common+0x4c/0x64)
[   12.523071] [<c0042968>] (warn_slowpath_common+0x4c/0x64) from
[<c0042a14>] (
warn_slowpath_fmt+0x30/0x40)
[   12.533081] [<c0042a14>] (warn_slowpath_fmt+0x30/0x40) from
[<c0028d68>] (_id
le+0x164/0x17c)
[   12.541931] [<c0028d68>] (_idle+0x164/0x17c) from [<c00298b0>]
(omap_hwmod_id
le+0x28/0x3c)
[   12.550567] [<c00298b0>] (omap_hwmod_idle+0x28/0x3c) from
[<c003c664>] (omap_
device_idle_hwmods+0x24/0x3c)
[   12.560699] [<c003c664>] (omap_device_idle_hwmods+0x24/0x3c) from
[<c003c898>
] (_omap_device_deactivate+0xa4/0x138)
[   12.571594] [<c003c898>] (_omap_device_deactivate+0xa4/0x138) from
[<c003c954
>] (omap_device_idle+0x28/0x54)
[   12.581909] [<c003c954>] (omap_device_idle+0x28/0x54) from
[<c003c99c>] (_od_
runtime_suspend+0x1c/0x24)
[   12.591735] [<c003c99c>] (_od_runtime_suspend+0x1c/0x24) from
[<c02c5010>] (_
_rpm_callback+0x2c/0x78)
[   12.601379] [<c02c5010>] (__rpm_callback+0x2c/0x78) from [<c02c54d0>]
(rpm_su
spend+0x264/0x6c4)
[   12.610504] [<c02c54d0>] (rpm_suspend+0x264/0x6c4) from [<c02c6998>]
(__pm_ru
ntime_suspend+0x5c/0x74)
[   12.620178] [<c02c6998>] (__pm_runtime_suspend+0x5c/0x74) from
[<c02731a4>] (
omap2_gpio_prepare_for_idle+0x50/0x64)
[   12.631103] [<c02731a4>] (omap2_gpio_prepare_for_idle+0x50/0x64) from
[<c002b
d30>] (omap_sram_idle+0xa0/0x3b0)
[   12.641571] [<c002bd30>] (omap_sram_idle+0xa0/0x3b0) from
[<c002c1d8>] (omap3
_pm_idle+0x60/0x178)
[   12.650848] [<c002c1d8>] (omap3_pm_idle+0x60/0x178) from [<c0015c10>]
(cpu_id
le+0xc4/0x108)
[   12.659606] [<c0015c10>] (cpu_idle+0xc4/0x108) from [<c0626828>]
(start_kerne
l+0x2b0/0x304)
[   12.668365] ---[ end trace 441b8fea2b56dcb1 ]---


Also, this goes away if I manually force wakeup for the per domain, so
this might be caused by some additional latency.

-Tero

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Kevin Hilman]

Tero Kristo <t-kristo@ti.com> writes:

> On Thu, 2012-02-16 at 09:31 -0800, Kevin Hilman wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>> 
>> > On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
>> >> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> >> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
>> >> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
>> >> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> >> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
>> >> >> > >> Tero Kristo <t-kristo@ti.com> writes:
>> >> >> > >>
>> >> >> > >> > Attempting this will cause problems especially with off-mode enabled.
>> >> >> > >>
>> >> >> > >> Please be more verbose about the problems seen, and the root cause(s).
>> >> >> > >>
>> >> >> > >
>> >> >> > > I was actually looking forward for some help with this commit message,
>> >> >> > > as I am still not quite sure what is going on in here. :) Here is the
>> >> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
>> >> >> > > I think that is probably fixed by the patch from Paul,
>> >> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
>> >> >> > > after wakeup from off-mode, and it is triggered by the disabling of
>> >> >> > > autodeps before off-mode entry.
>> >> >> > >
>> >> >> > This mostly indicates that one of the per clock-domain module
>> >> >> > clock turning ON seems to be not working well with auto deps
>> >> >> > disabled. This leads to interconnect violation.
>> >> >> >
>> >> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
>> >> >> > in the low power code early resume path and see if this
>> >> >> > error goes away.
>> >> >>
>> >> >> This seems to get rid of the dump also. It looks like some driver resume
>> >> >> is not behaving nicely, I am trying to pinpoint the culprit currently
>> >> >> and see whether it can provide more info.
>> >> >
>> >> > Okay, I have some more info about this now.
>> >> >
>> >> > What happens is that when entering off-mode, PER domain remains OFF even
>> >> > during the execution of the exit phase from omap_sram_idle. Adding a
>> >> > manual SW_WKUP it comes up and there are no issues. If autodeps are
>> >> > enabled on the domain, it comes back from off mode as active.
>> >> >
>> >> > Looking further in the code, we have this at the end of omap_sram_idle:
>> >> >
>> >> >        if (per_next_state < PWRDM_POWER_ON) {
>> >> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
>> >> >                omap2_gpio_resume_after_idle();
>> >> >                wake_per();
>> >> >                if (per_prev_state == PWRDM_POWER_OFF)
>> >> >                        omap3_per_restore_context();
>> >> >        }
>> >> >
>> >> > ... which seems to assume that per domain is on. Gpio code does not
>> >> > control any clocks currently, as it only requires the interface clock to
>> >> > be on, and as this is autoidled....
>> >> >
>> >> > Any comments how we should handle this? Shall we just keep these two
>> >> > patches for handling this or add some different hackery for the gpio
>> >> > issue?
>> >> >
>> >> Good. I thought too that issue will disappear.
>> >> The issue is pretty clear. Technically every driver pm_runtime() code should
>> >> be able to manage a clock->clockdomain->power domain power up/down
>> >> sequence. That should work without auto deps.
>> >> 
>> >> Do you narrowed down which driver resume is creating the dump ?
>> >> UART , GPIO ?
>> >
>> > It is the gpio base stuff called from omap_sram_idle(), basically the
>> > restore context part. If I force enable per domain before the code
>> > snippet before, there is no dump, but if it is done after, I get the
>> > dump.
>> >
>> > The thing is that gpio driver doesn't currently have this kind of
>> > mechanism for the restore context part, at least not on omap3 due to
>> > above clocking issue (only autoidled interface clock is used.)
>> 
>> I'm not sure if it fully addresses this, but Tarun's series converts
>> GPIO to runtime PM.
>> 
>> Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
>> branch here:
>> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git
>
> It does something for the issue, but I still get this during suspend to
> off:
>
> [   11.284973] PM: Syncing filesystems ... done.
> [   11.379241] Freezing user space processes ... (elapsed 0.02 seconds)
> done.
> [   11.408233] Freezing remaining freezable tasks ... (elapsed 0.02
> seconds) don
> e.
> [   11.439239] Suspending console(s) (use no_console_suspend to debug)
> [   11.564178] PM: suspend of devices complete after 115.506 msecs
> [   11.567382] PM: late suspend of devices complete after 3.204 msecs
> [   11.567443] Disabling non-boot CPUs ...
> [   12.004089] Powerdomain (cam_pwrdm) didn't enter target state 0
> [   12.004119] Could not enter target state in pm_suspend
> [   12.009368] PM: early resume of devices complete after 4.944 msecs
> [   12.436645] PM: resume of devices complete after 426.086 msecs
> [   12.480285] Restarting tasks ... done.
> /sys/kernel/debu[   12.488433] ------------[ cut here ]------------
> [   12.494415] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:1604 _idle
> +0x164/0x1
> 7c()
> [   12.502258] omap_hwmod: gpio6: idle state can only be entered from
> enabled st
> ate
> [   12.509979] Modules linked in:
> [   12.513214] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
> [<c0042968>] (warn_
> slowpath_common+0x4c/0x64)
> [   12.523071] [<c0042968>] (warn_slowpath_common+0x4c/0x64) from
> [<c0042a14>] (
> warn_slowpath_fmt+0x30/0x40)
> [   12.533081] [<c0042a14>] (warn_slowpath_fmt+0x30/0x40) from
> [<c0028d68>] (_id
> le+0x164/0x17c)
> [   12.541931] [<c0028d68>] (_idle+0x164/0x17c) from [<c00298b0>]
> (omap_hwmod_id
> le+0x28/0x3c)
> [   12.550567] [<c00298b0>] (omap_hwmod_idle+0x28/0x3c) from
> [<c003c664>] (omap_
> device_idle_hwmods+0x24/0x3c)
> [   12.560699] [<c003c664>] (omap_device_idle_hwmods+0x24/0x3c) from
> [<c003c898>
> ] (_omap_device_deactivate+0xa4/0x138)
> [   12.571594] [<c003c898>] (_omap_device_deactivate+0xa4/0x138) from
> [<c003c954
>>] (omap_device_idle+0x28/0x54)
> [   12.581909] [<c003c954>] (omap_device_idle+0x28/0x54) from
> [<c003c99c>] (_od_
> runtime_suspend+0x1c/0x24)
> [   12.591735] [<c003c99c>] (_od_runtime_suspend+0x1c/0x24) from
> [<c02c5010>] (_
> _rpm_callback+0x2c/0x78)
> [   12.601379] [<c02c5010>] (__rpm_callback+0x2c/0x78) from [<c02c54d0>]
> (rpm_su
> spend+0x264/0x6c4)
> [   12.610504] [<c02c54d0>] (rpm_suspend+0x264/0x6c4) from [<c02c6998>]
> (__pm_ru
> ntime_suspend+0x5c/0x74)
> [   12.620178] [<c02c6998>] (__pm_runtime_suspend+0x5c/0x74) from
> [<c02731a4>] (
> omap2_gpio_prepare_for_idle+0x50/0x64)
> [   12.631103] [<c02731a4>] (omap2_gpio_prepare_for_idle+0x50/0x64) from
> [<c002b
> d30>] (omap_sram_idle+0xa0/0x3b0)
> [   12.641571] [<c002bd30>] (omap_sram_idle+0xa0/0x3b0) from
> [<c002c1d8>] (omap3
> _pm_idle+0x60/0x178)
> [   12.650848] [<c002c1d8>] (omap3_pm_idle+0x60/0x178) from [<c0015c10>]
> (cpu_id
> le+0xc4/0x108)
> [   12.659606] [<c0015c10>] (cpu_idle+0xc4/0x108) from [<c0626828>]
> (start_kerne
> l+0x2b0/0x304)
> [   12.668365] ---[ end trace 441b8fea2b56dcb1 ]---
>
>
> Also, this goes away if I manually force wakeup for the per domain, so
> this might be caused by some additional latency.
>

What platform are you testing on?

When I test Tarun's series with off-mode on 3430/n900 suspend/resume
works fine.  But when I add your v2 series, it never comes out of
suspend, and I don't get a kernel dump either.

Can you debug this a little further so we can explain what's going on
here?

Thanks,

Kevin

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Wed, 2012-02-22 at 14:37 -0800, Kevin Hilman wrote:
> Tero Kristo <t-kristo@ti.com> writes:
> 
> > On Thu, 2012-02-16 at 09:31 -0800, Kevin Hilman wrote:
> >> Tero Kristo <t-kristo@ti.com> writes:
> >> 
> >> > On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
> >> >> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
> >> >> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> >> >> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
> >> >> >> > >> Tero Kristo <t-kristo@ti.com> writes:
> >> >> >> > >>
> >> >> >> > >> > Attempting this will cause problems especially with off-mode enabled.
> >> >> >> > >>
> >> >> >> > >> Please be more verbose about the problems seen, and the root cause(s).
> >> >> >> > >>
> >> >> >> > >
> >> >> >> > > I was actually looking forward for some help with this commit message,
> >> >> >> > > as I am still not quite sure what is going on in here. :) Here is the
> >> >> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> >> >> >> > > I think that is probably fixed by the patch from Paul,
> >> >> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
> >> >> >> > > after wakeup from off-mode, and it is triggered by the disabling of
> >> >> >> > > autodeps before off-mode entry.
> >> >> >> > >
> >> >> >> > This mostly indicates that one of the per clock-domain module
> >> >> >> > clock turning ON seems to be not working well with auto deps
> >> >> >> > disabled. This leads to interconnect violation.
> >> >> >> >
> >> >> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
> >> >> >> > in the low power code early resume path and see if this
> >> >> >> > error goes away.
> >> >> >>
> >> >> >> This seems to get rid of the dump also. It looks like some driver resume
> >> >> >> is not behaving nicely, I am trying to pinpoint the culprit currently
> >> >> >> and see whether it can provide more info.
> >> >> >
> >> >> > Okay, I have some more info about this now.
> >> >> >
> >> >> > What happens is that when entering off-mode, PER domain remains OFF even
> >> >> > during the execution of the exit phase from omap_sram_idle. Adding a
> >> >> > manual SW_WKUP it comes up and there are no issues. If autodeps are
> >> >> > enabled on the domain, it comes back from off mode as active.
> >> >> >
> >> >> > Looking further in the code, we have this at the end of omap_sram_idle:
> >> >> >
> >> >> >        if (per_next_state < PWRDM_POWER_ON) {
> >> >> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> >> >> >                omap2_gpio_resume_after_idle();
> >> >> >                wake_per();
> >> >> >                if (per_prev_state == PWRDM_POWER_OFF)
> >> >> >                        omap3_per_restore_context();
> >> >> >        }
> >> >> >
> >> >> > ... which seems to assume that per domain is on. Gpio code does not
> >> >> > control any clocks currently, as it only requires the interface clock to
> >> >> > be on, and as this is autoidled....
> >> >> >
> >> >> > Any comments how we should handle this? Shall we just keep these two
> >> >> > patches for handling this or add some different hackery for the gpio
> >> >> > issue?
> >> >> >
> >> >> Good. I thought too that issue will disappear.
> >> >> The issue is pretty clear. Technically every driver pm_runtime() code should
> >> >> be able to manage a clock->clockdomain->power domain power up/down
> >> >> sequence. That should work without auto deps.
> >> >> 
> >> >> Do you narrowed down which driver resume is creating the dump ?
> >> >> UART , GPIO ?
> >> >
> >> > It is the gpio base stuff called from omap_sram_idle(), basically the
> >> > restore context part. If I force enable per domain before the code
> >> > snippet before, there is no dump, but if it is done after, I get the
> >> > dump.
> >> >
> >> > The thing is that gpio driver doesn't currently have this kind of
> >> > mechanism for the restore context part, at least not on omap3 due to
> >> > above clocking issue (only autoidled interface clock is used.)
> >> 
> >> I'm not sure if it fully addresses this, but Tarun's series converts
> >> GPIO to runtime PM.
> >> 
> >> Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
> >> branch here:
> >> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git
> >
> > It does something for the issue, but I still get this during suspend to
> > off:
> >
> > [   11.284973] PM: Syncing filesystems ... done.
> > [   11.379241] Freezing user space processes ... (elapsed 0.02 seconds)
> > done.
> > [   11.408233] Freezing remaining freezable tasks ... (elapsed 0.02
> > seconds) don
> > e.
> > [   11.439239] Suspending console(s) (use no_console_suspend to debug)
> > [   11.564178] PM: suspend of devices complete after 115.506 msecs
> > [   11.567382] PM: late suspend of devices complete after 3.204 msecs
> > [   11.567443] Disabling non-boot CPUs ...
> > [   12.004089] Powerdomain (cam_pwrdm) didn't enter target state 0
> > [   12.004119] Could not enter target state in pm_suspend
> > [   12.009368] PM: early resume of devices complete after 4.944 msecs
> > [   12.436645] PM: resume of devices complete after 426.086 msecs
> > [   12.480285] Restarting tasks ... done.
> > /sys/kernel/debu[   12.488433] ------------[ cut here ]------------
> > [   12.494415] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:1604 _idle
> > +0x164/0x1
> > 7c()
> > [   12.502258] omap_hwmod: gpio6: idle state can only be entered from
> > enabled st
> > ate
> > [   12.509979] Modules linked in:
> > [   12.513214] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
> > [<c0042968>] (warn_
> > slowpath_common+0x4c/0x64)
> > [   12.523071] [<c0042968>] (warn_slowpath_common+0x4c/0x64) from
> > [<c0042a14>] (
> > warn_slowpath_fmt+0x30/0x40)
> > [   12.533081] [<c0042a14>] (warn_slowpath_fmt+0x30/0x40) from
> > [<c0028d68>] (_id
> > le+0x164/0x17c)
> > [   12.541931] [<c0028d68>] (_idle+0x164/0x17c) from [<c00298b0>]
> > (omap_hwmod_id
> > le+0x28/0x3c)
> > [   12.550567] [<c00298b0>] (omap_hwmod_idle+0x28/0x3c) from
> > [<c003c664>] (omap_
> > device_idle_hwmods+0x24/0x3c)
> > [   12.560699] [<c003c664>] (omap_device_idle_hwmods+0x24/0x3c) from
> > [<c003c898>
> > ] (_omap_device_deactivate+0xa4/0x138)
> > [   12.571594] [<c003c898>] (_omap_device_deactivate+0xa4/0x138) from
> > [<c003c954
> >>] (omap_device_idle+0x28/0x54)
> > [   12.581909] [<c003c954>] (omap_device_idle+0x28/0x54) from
> > [<c003c99c>] (_od_
> > runtime_suspend+0x1c/0x24)
> > [   12.591735] [<c003c99c>] (_od_runtime_suspend+0x1c/0x24) from
> > [<c02c5010>] (_
> > _rpm_callback+0x2c/0x78)
> > [   12.601379] [<c02c5010>] (__rpm_callback+0x2c/0x78) from [<c02c54d0>]
> > (rpm_su
> > spend+0x264/0x6c4)
> > [   12.610504] [<c02c54d0>] (rpm_suspend+0x264/0x6c4) from [<c02c6998>]
> > (__pm_ru
> > ntime_suspend+0x5c/0x74)
> > [   12.620178] [<c02c6998>] (__pm_runtime_suspend+0x5c/0x74) from
> > [<c02731a4>] (
> > omap2_gpio_prepare_for_idle+0x50/0x64)
> > [   12.631103] [<c02731a4>] (omap2_gpio_prepare_for_idle+0x50/0x64) from
> > [<c002b
> > d30>] (omap_sram_idle+0xa0/0x3b0)
> > [   12.641571] [<c002bd30>] (omap_sram_idle+0xa0/0x3b0) from
> > [<c002c1d8>] (omap3
> > _pm_idle+0x60/0x178)
> > [   12.650848] [<c002c1d8>] (omap3_pm_idle+0x60/0x178) from [<c0015c10>]
> > (cpu_id
> > le+0xc4/0x108)
> > [   12.659606] [<c0015c10>] (cpu_idle+0xc4/0x108) from [<c0626828>]
> > (start_kerne
> > l+0x2b0/0x304)
> > [   12.668365] ---[ end trace 441b8fea2b56dcb1 ]---
> >
> >
> > Also, this goes away if I manually force wakeup for the per domain, so
> > this might be caused by some additional latency.
> >
> 
> What platform are you testing on?

This is with omap3530 beagle.

> When I test Tarun's series with off-mode on 3430/n900 suspend/resume
> works fine.  But when I add your v2 series, it never comes out of
> suspend, and I don't get a kernel dump either.

Did you try without Tarun's series and see what happens in that case?

> Can you debug this a little further so we can explain what's going on
> here?

I don't have capability to test with any other omap3 device than beagle,
so don't know what I can accomplish here. I just got omap4 cswr working
yesterday with mainline, so I can try with that one and see how it
behaves (omap4 requires some additional patching for the usecounting
though.)

-Tero

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Wed, 2012-02-22 at 14:37 -0800, Kevin Hilman wrote:
> Tero Kristo <t-kristo@ti.com> writes:
> 
> > On Thu, 2012-02-16 at 09:31 -0800, Kevin Hilman wrote:
> >> Tero Kristo <t-kristo@ti.com> writes:
> >> 
> >> > On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
> >> >> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
> >> >> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> >> >> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
> >> >> >> > >> Tero Kristo <t-kristo@ti.com> writes:
> >> >> >> > >>
> >> >> >> > >> > Attempting this will cause problems especially with off-mode enabled.
> >> >> >> > >>
> >> >> >> > >> Please be more verbose about the problems seen, and the root cause(s).
> >> >> >> > >>
> >> >> >> > >
> >> >> >> > > I was actually looking forward for some help with this commit message,
> >> >> >> > > as I am still not quite sure what is going on in here. :) Here is the
> >> >> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> >> >> >> > > I think that is probably fixed by the patch from Paul,
> >> >> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
> >> >> >> > > after wakeup from off-mode, and it is triggered by the disabling of
> >> >> >> > > autodeps before off-mode entry.
> >> >> >> > >
> >> >> >> > This mostly indicates that one of the per clock-domain module
> >> >> >> > clock turning ON seems to be not working well with auto deps
> >> >> >> > disabled. This leads to interconnect violation.
> >> >> >> >
> >> >> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
> >> >> >> > in the low power code early resume path and see if this
> >> >> >> > error goes away.
> >> >> >>
> >> >> >> This seems to get rid of the dump also. It looks like some driver resume
> >> >> >> is not behaving nicely, I am trying to pinpoint the culprit currently
> >> >> >> and see whether it can provide more info.
> >> >> >
> >> >> > Okay, I have some more info about this now.
> >> >> >
> >> >> > What happens is that when entering off-mode, PER domain remains OFF even
> >> >> > during the execution of the exit phase from omap_sram_idle. Adding a
> >> >> > manual SW_WKUP it comes up and there are no issues. If autodeps are
> >> >> > enabled on the domain, it comes back from off mode as active.
> >> >> >
> >> >> > Looking further in the code, we have this at the end of omap_sram_idle:
> >> >> >
> >> >> >        if (per_next_state < PWRDM_POWER_ON) {
> >> >> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> >> >> >                omap2_gpio_resume_after_idle();
> >> >> >                wake_per();
> >> >> >                if (per_prev_state == PWRDM_POWER_OFF)
> >> >> >                        omap3_per_restore_context();
> >> >> >        }
> >> >> >
> >> >> > ... which seems to assume that per domain is on. Gpio code does not
> >> >> > control any clocks currently, as it only requires the interface clock to
> >> >> > be on, and as this is autoidled....
> >> >> >
> >> >> > Any comments how we should handle this? Shall we just keep these two
> >> >> > patches for handling this or add some different hackery for the gpio
> >> >> > issue?
> >> >> >
> >> >> Good. I thought too that issue will disappear.
> >> >> The issue is pretty clear. Technically every driver pm_runtime() code should
> >> >> be able to manage a clock->clockdomain->power domain power up/down
> >> >> sequence. That should work without auto deps.
> >> >> 
> >> >> Do you narrowed down which driver resume is creating the dump ?
> >> >> UART , GPIO ?
> >> >
> >> > It is the gpio base stuff called from omap_sram_idle(), basically the
> >> > restore context part. If I force enable per domain before the code
> >> > snippet before, there is no dump, but if it is done after, I get the
> >> > dump.
> >> >
> >> > The thing is that gpio driver doesn't currently have this kind of
> >> > mechanism for the restore context part, at least not on omap3 due to
> >> > above clocking issue (only autoidled interface clock is used.)
> >> 
> >> I'm not sure if it fully addresses this, but Tarun's series converts
> >> GPIO to runtime PM.
> >> 
> >> Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
> >> branch here:
> >> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git
> >
> > It does something for the issue, but I still get this during suspend to
> > off:
> >
> > [   11.284973] PM: Syncing filesystems ... done.
> > [   11.379241] Freezing user space processes ... (elapsed 0.02 seconds)
> > done.
> > [   11.408233] Freezing remaining freezable tasks ... (elapsed 0.02
> > seconds) don
> > e.
> > [   11.439239] Suspending console(s) (use no_console_suspend to debug)
> > [   11.564178] PM: suspend of devices complete after 115.506 msecs
> > [   11.567382] PM: late suspend of devices complete after 3.204 msecs
> > [   11.567443] Disabling non-boot CPUs ...
> > [   12.004089] Powerdomain (cam_pwrdm) didn't enter target state 0
> > [   12.004119] Could not enter target state in pm_suspend
> > [   12.009368] PM: early resume of devices complete after 4.944 msecs
> > [   12.436645] PM: resume of devices complete after 426.086 msecs
> > [   12.480285] Restarting tasks ... done.
> > /sys/kernel/debu[   12.488433] ------------[ cut here ]------------
> > [   12.494415] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:1604 _idle
> > +0x164/0x1
> > 7c()
> > [   12.502258] omap_hwmod: gpio6: idle state can only be entered from
> > enabled st
> > ate
> > [   12.509979] Modules linked in:
> > [   12.513214] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
> > [<c0042968>] (warn_
> > slowpath_common+0x4c/0x64)
> > [   12.523071] [<c0042968>] (warn_slowpath_common+0x4c/0x64) from
> > [<c0042a14>] (
> > warn_slowpath_fmt+0x30/0x40)
> > [   12.533081] [<c0042a14>] (warn_slowpath_fmt+0x30/0x40) from
> > [<c0028d68>] (_id
> > le+0x164/0x17c)
> > [   12.541931] [<c0028d68>] (_idle+0x164/0x17c) from [<c00298b0>]
> > (omap_hwmod_id
> > le+0x28/0x3c)
> > [   12.550567] [<c00298b0>] (omap_hwmod_idle+0x28/0x3c) from
> > [<c003c664>] (omap_
> > device_idle_hwmods+0x24/0x3c)
> > [   12.560699] [<c003c664>] (omap_device_idle_hwmods+0x24/0x3c) from
> > [<c003c898>
> > ] (_omap_device_deactivate+0xa4/0x138)
> > [   12.571594] [<c003c898>] (_omap_device_deactivate+0xa4/0x138) from
> > [<c003c954
> >>] (omap_device_idle+0x28/0x54)
> > [   12.581909] [<c003c954>] (omap_device_idle+0x28/0x54) from
> > [<c003c99c>] (_od_
> > runtime_suspend+0x1c/0x24)
> > [   12.591735] [<c003c99c>] (_od_runtime_suspend+0x1c/0x24) from
> > [<c02c5010>] (_
> > _rpm_callback+0x2c/0x78)
> > [   12.601379] [<c02c5010>] (__rpm_callback+0x2c/0x78) from [<c02c54d0>]
> > (rpm_su
> > spend+0x264/0x6c4)
> > [   12.610504] [<c02c54d0>] (rpm_suspend+0x264/0x6c4) from [<c02c6998>]
> > (__pm_ru
> > ntime_suspend+0x5c/0x74)
> > [   12.620178] [<c02c6998>] (__pm_runtime_suspend+0x5c/0x74) from
> > [<c02731a4>] (
> > omap2_gpio_prepare_for_idle+0x50/0x64)
> > [   12.631103] [<c02731a4>] (omap2_gpio_prepare_for_idle+0x50/0x64) from
> > [<c002b
> > d30>] (omap_sram_idle+0xa0/0x3b0)
> > [   12.641571] [<c002bd30>] (omap_sram_idle+0xa0/0x3b0) from
> > [<c002c1d8>] (omap3
> > _pm_idle+0x60/0x178)
> > [   12.650848] [<c002c1d8>] (omap3_pm_idle+0x60/0x178) from [<c0015c10>]
> > (cpu_id
> > le+0xc4/0x108)
> > [   12.659606] [<c0015c10>] (cpu_idle+0xc4/0x108) from [<c0626828>]
> > (start_kerne
> > l+0x2b0/0x304)
> > [   12.668365] ---[ end trace 441b8fea2b56dcb1 ]---
> >
> >
> > Also, this goes away if I manually force wakeup for the per domain, so
> > this might be caused by some additional latency.
> >
> 
> What platform are you testing on?
> 
> When I test Tarun's series with off-mode on 3430/n900 suspend/resume
> works fine.  But when I add your v2 series, it never comes out of
> suspend, and I don't get a kernel dump either.
> 
> Can you debug this a little further so we can explain what's going on
> here?

Attached one trial patch you could try to use to see if it fixes the
issues you are seeing with 3430/n900. I'll look at Tarun's set and see
if I can figure out what is causing the warning above.

-Tero
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-arm-omap3-prevent-dpll4-manual-enable-disable-preven.patch
Type: text/x-patch
Size: 2116 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120224/143dd71b/attachment.bin>

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Wed, 2012-02-22 at 14:37 -0800, Kevin Hilman wrote:
> Tero Kristo <t-kristo@ti.com> writes:
> 
> > On Thu, 2012-02-16 at 09:31 -0800, Kevin Hilman wrote:
> >> Tero Kristo <t-kristo@ti.com> writes:
> >> 
> >> > On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
> >> >> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
> >> >> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> >> >> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote:
> >> >> >> > >> Tero Kristo <t-kristo@ti.com> writes:
> >> >> >> > >>
> >> >> >> > >> > Attempting this will cause problems especially with off-mode enabled.
> >> >> >> > >>
> >> >> >> > >> Please be more verbose about the problems seen, and the root cause(s).
> >> >> >> > >>
> >> >> >> > >
> >> >> >> > > I was actually looking forward for some help with this commit message,
> >> >> >> > > as I am still not quite sure what is going on in here. :) Here is the
> >> >> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> >> >> >> > > I think that is probably fixed by the patch from Paul,
> >> >> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
> >> >> >> > > after wakeup from off-mode, and it is triggered by the disabling of
> >> >> >> > > autodeps before off-mode entry.
> >> >> >> > >
> >> >> >> > This mostly indicates that one of the per clock-domain module
> >> >> >> > clock turning ON seems to be not working well with auto deps
> >> >> >> > disabled. This leads to interconnect violation.
> >> >> >> >
> >> >> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
> >> >> >> > in the low power code early resume path and see if this
> >> >> >> > error goes away.
> >> >> >>
> >> >> >> This seems to get rid of the dump also. It looks like some driver resume
> >> >> >> is not behaving nicely, I am trying to pinpoint the culprit currently
> >> >> >> and see whether it can provide more info.
> >> >> >
> >> >> > Okay, I have some more info about this now.
> >> >> >
> >> >> > What happens is that when entering off-mode, PER domain remains OFF even
> >> >> > during the execution of the exit phase from omap_sram_idle. Adding a
> >> >> > manual SW_WKUP it comes up and there are no issues. If autodeps are
> >> >> > enabled on the domain, it comes back from off mode as active.
> >> >> >
> >> >> > Looking further in the code, we have this at the end of omap_sram_idle:
> >> >> >
> >> >> >        if (per_next_state < PWRDM_POWER_ON) {
> >> >> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> >> >> >                omap2_gpio_resume_after_idle();
> >> >> >                wake_per();
> >> >> >                if (per_prev_state == PWRDM_POWER_OFF)
> >> >> >                        omap3_per_restore_context();
> >> >> >        }
> >> >> >
> >> >> > ... which seems to assume that per domain is on. Gpio code does not
> >> >> > control any clocks currently, as it only requires the interface clock to
> >> >> > be on, and as this is autoidled....
> >> >> >
> >> >> > Any comments how we should handle this? Shall we just keep these two
> >> >> > patches for handling this or add some different hackery for the gpio
> >> >> > issue?
> >> >> >
> >> >> Good. I thought too that issue will disappear.
> >> >> The issue is pretty clear. Technically every driver pm_runtime() code should
> >> >> be able to manage a clock->clockdomain->power domain power up/down
> >> >> sequence. That should work without auto deps.
> >> >> 
> >> >> Do you narrowed down which driver resume is creating the dump ?
> >> >> UART , GPIO ?
> >> >
> >> > It is the gpio base stuff called from omap_sram_idle(), basically the
> >> > restore context part. If I force enable per domain before the code
> >> > snippet before, there is no dump, but if it is done after, I get the
> >> > dump.
> >> >
> >> > The thing is that gpio driver doesn't currently have this kind of
> >> > mechanism for the restore context part, at least not on omap3 due to
> >> > above clocking issue (only autoidled interface clock is used.)
> >> 
> >> I'm not sure if it fully addresses this, but Tarun's series converts
> >> GPIO to runtime PM.
> >> 
> >> Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
> >> branch here:
> >> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git
> >
> > It does something for the issue, but I still get this during suspend to
> > off:
> >
> > [   11.284973] PM: Syncing filesystems ... done.
> > [   11.379241] Freezing user space processes ... (elapsed 0.02 seconds)
> > done.
> > [   11.408233] Freezing remaining freezable tasks ... (elapsed 0.02
> > seconds) don
> > e.
> > [   11.439239] Suspending console(s) (use no_console_suspend to debug)
> > [   11.564178] PM: suspend of devices complete after 115.506 msecs
> > [   11.567382] PM: late suspend of devices complete after 3.204 msecs
> > [   11.567443] Disabling non-boot CPUs ...
> > [   12.004089] Powerdomain (cam_pwrdm) didn't enter target state 0
> > [   12.004119] Could not enter target state in pm_suspend
> > [   12.009368] PM: early resume of devices complete after 4.944 msecs
> > [   12.436645] PM: resume of devices complete after 426.086 msecs
> > [   12.480285] Restarting tasks ... done.
> > /sys/kernel/debu[   12.488433] ------------[ cut here ]------------
> > [   12.494415] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:1604 _idle
> > +0x164/0x1
> > 7c()
> > [   12.502258] omap_hwmod: gpio6: idle state can only be entered from
> > enabled st
> > ate
> > [   12.509979] Modules linked in:
> > [   12.513214] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
> > [<c0042968>] (warn_
> > slowpath_common+0x4c/0x64)
> > [   12.523071] [<c0042968>] (warn_slowpath_common+0x4c/0x64) from
> > [<c0042a14>] (
> > warn_slowpath_fmt+0x30/0x40)
> > [   12.533081] [<c0042a14>] (warn_slowpath_fmt+0x30/0x40) from
> > [<c0028d68>] (_id
> > le+0x164/0x17c)
> > [   12.541931] [<c0028d68>] (_idle+0x164/0x17c) from [<c00298b0>]
> > (omap_hwmod_id
> > le+0x28/0x3c)
> > [   12.550567] [<c00298b0>] (omap_hwmod_idle+0x28/0x3c) from
> > [<c003c664>] (omap_
> > device_idle_hwmods+0x24/0x3c)
> > [   12.560699] [<c003c664>] (omap_device_idle_hwmods+0x24/0x3c) from
> > [<c003c898>
> > ] (_omap_device_deactivate+0xa4/0x138)
> > [   12.571594] [<c003c898>] (_omap_device_deactivate+0xa4/0x138) from
> > [<c003c954
> >>] (omap_device_idle+0x28/0x54)
> > [   12.581909] [<c003c954>] (omap_device_idle+0x28/0x54) from
> > [<c003c99c>] (_od_
> > runtime_suspend+0x1c/0x24)
> > [   12.591735] [<c003c99c>] (_od_runtime_suspend+0x1c/0x24) from
> > [<c02c5010>] (_
> > _rpm_callback+0x2c/0x78)
> > [   12.601379] [<c02c5010>] (__rpm_callback+0x2c/0x78) from [<c02c54d0>]
> > (rpm_su
> > spend+0x264/0x6c4)
> > [   12.610504] [<c02c54d0>] (rpm_suspend+0x264/0x6c4) from [<c02c6998>]
> > (__pm_ru
> > ntime_suspend+0x5c/0x74)
> > [   12.620178] [<c02c6998>] (__pm_runtime_suspend+0x5c/0x74) from
> > [<c02731a4>] (
> > omap2_gpio_prepare_for_idle+0x50/0x64)
> > [   12.631103] [<c02731a4>] (omap2_gpio_prepare_for_idle+0x50/0x64) from
> > [<c002b
> > d30>] (omap_sram_idle+0xa0/0x3b0)
> > [   12.641571] [<c002bd30>] (omap_sram_idle+0xa0/0x3b0) from
> > [<c002c1d8>] (omap3
> > _pm_idle+0x60/0x178)
> > [   12.650848] [<c002c1d8>] (omap3_pm_idle+0x60/0x178) from [<c0015c10>]
> > (cpu_id
> > le+0xc4/0x108)
> > [   12.659606] [<c0015c10>] (cpu_idle+0xc4/0x108) from [<c0626828>]
> > (start_kerne
> > l+0x2b0/0x304)
> > [   12.668365] ---[ end trace 441b8fea2b56dcb1 ]---
> >
> >
> > Also, this goes away if I manually force wakeup for the per domain, so
> > this might be caused by some additional latency.
> >
> 
> What platform are you testing on?
> 
> When I test Tarun's series with off-mode on 3430/n900 suspend/resume
> works fine.  But when I add your v2 series, it never comes out of
> suspend, and I don't get a kernel dump either.
> 
> Can you debug this a little further so we can explain what's going on
> here?
> 

After some debugging, it looks like the root cause for the WARN dump I
see with GPIO PM runtime + usecounting is the fact that PER domain
enters SW assisted idle and remains OFF after wakeup from OFF mode, and
it will remain there until some functional clock is enabled. GPIO6
module is only enabling its interface clock, but as this is autoidled,
it will not wake up PER domain. This will cause the initial
_wait_target_ready(oh) to fail with EBUSY, leaving the hwmod into idle
state. Next cpuidle entry will attempt to disable the already idle hwmod
causing this dump.

This can be avoided by preventing PER domain idle on omap3530. I believe
on omap3430 you also need to prevent similar stuff for CORE domain.

Did you try the patch I posted last week for the core domain + dplls?
Does it help with 3430? I guess n900 could hang in the ROM code if PER
domain is idle during wakeup, as it is a HS device also and behaves
differently during wakeup from OFF.

-Tero

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Kevin Hilman]

+Paul

Tero Kristo <t-kristo@ti.com> writes:

> On Wed, 2012-02-22 at 14:37 -0800, Kevin Hilman wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>> 
>> > On Thu, 2012-02-16 at 09:31 -0800, Kevin Hilman wrote:
>> >> Tero Kristo <t-kristo@ti.com> writes:
>> >> 
>> >> > On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
>> >> >> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> >> >> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
>> >> >> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
>> >> >> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> >> >> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote: 
>> >> >> >> > >> Tero Kristo <t-kristo@ti.com> writes:
>> >> >> >> > >> P
>> >> >> >> > >> > Attempting this will cause problems especially with off-mode enabled.
>> >> >> >> > >>
>> >> >> >> > >> Please be more verbose about the problems seen, and the root cause(s).
>> >> >> >> > >>
>> >> >> >> > >
>> >> >> >> > > I was actually looking forward for some help with this commit message,
>> >> >> >> > > as I am still not quite sure what is going on in here. :) Here is the
>> >> >> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
>> >> >> >> > > I think that is probably fixed by the patch from Paul,
>> >> >> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
>> >> >> >> > > after wakeup from off-mode, and it is triggered by the disabling of
>> >> >> >> > > autodeps before off-mode entry.
>> >> >> >> > >
>> >> >> >> > This mostly indicates that one of the per clock-domain module
>> >> >> >> > clock turning ON seems to be not working well with auto deps
>> >> >> >> > disabled. This leads to interconnect violation.
>> >> >> >> >
>> >> >> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
>> >> >> >> > in the low power code early resume path and see if this
>> >> >> >> > error goes away.
>> >> >> >>
>> >> >> >> This seems to get rid of the dump also. It looks like some driver resume
>> >> >> >> is not behaving nicely, I am trying to pinpoint the culprit currently
>> >> >> >> and see whether it can provide more info.
>> >> >> >
>> >> >> > Okay, I have some more info about this now.
>> >> >> >
>> >> >> > What happens is that when entering off-mode, PER domain remains OFF even
>> >> >> > during the execution of the exit phase from omap_sram_idle. Adding a
>> >> >> > manual SW_WKUP it comes up and there are no issues. If autodeps are
>> >> >> > enabled on the domain, it comes back from off mode as active.
>> >> >> >
>> >> >> > Looking further in the code, we have this at the end of omap_sram_idle:
>> >> >> >
>> >> >> >        if (per_next_state < PWRDM_POWER_ON) {
>> >> >> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
>> >> >> >                omap2_gpio_resume_after_idle();
>> >> >> >                wake_per();
>> >> >> >                if (per_prev_state == PWRDM_POWER_OFF)
>> >> >> >                        omap3_per_restore_context();
>> >> >> >        }
>> >> >> >
>> >> >> > ... which seems to assume that per domain is on. Gpio code does not
>> >> >> > control any clocks currently, as it only requires the interface clock to
>> >> >> > be on, and as this is autoidled....
>> >> >> >
>> >> >> > Any comments how we should handle this? Shall we just keep these two
>> >> >> > patches for handling this or add some different hackery for the gpio
>> >> >> > issue?
>> >> >> >
>> >> >> Good. I thought too that issue will disappear.
>> >> >> The issue is pretty clear. Technically every driver pm_runtime() code should
>> >> >> be able to manage a clock->clockdomain->power domain power up/down
>> >> >> sequence. That should work without auto deps.
>> >> >> 
>> >> >> Do you narrowed down which driver resume is creating the dump ?
>> >> >> UART , GPIO ?
>> >> >
>> >> > It is the gpio base stuff called from omap_sram_idle(), basically the
>> >> > restore context part. If I force enable per domain before the code
>> >> > snippet before, there is no dump, but if it is done after, I get the
>> >> > dump.
>> >> >
>> >> > The thing is that gpio driver doesn't currently have this kind of
>> >> > mechanism for the restore context part, at least not on omap3 due to
>> >> > above clocking issue (only autoidled interface clock is used.)
>> >> 
>> >> I'm not sure if it fully addresses this, but Tarun's series converts
>> >> GPIO to runtime PM.
>> >> 
>> >> Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
>> >> branch here:
>> >> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git
>> >
>> > It does something for the issue, but I still get this during suspend to
>> > off:
>> >
>> > [   11.284973] PM: Syncing filesystems ... done.
>> > [   11.379241] Freezing user space processes ... (elapsed 0.02 seconds)
>> > done.
>> > [   11.408233] Freezing remaining freezable tasks ... (elapsed 0.02
>> > seconds) don
>> > e.
>> > [   11.439239] Suspending console(s) (use no_console_suspend to debug)
>> > [   11.564178] PM: suspend of devices complete after 115.506 msecs
>> > [   11.567382] PM: late suspend of devices complete after 3.204 msecs
>> > [   11.567443] Disabling non-boot CPUs ...
>> > [   12.004089] Powerdomain (cam_pwrdm) didn't enter target state 0
>> > [   12.004119] Could not enter target state in pm_suspend
>> > [   12.009368] PM: early resume of devices complete after 4.944 msecs
>> > [   12.436645] PM: resume of devices complete after 426.086 msecs
>> > [   12.480285] Restarting tasks ... done.
>> > /sys/kernel/debu[   12.488433] ------------[ cut here ]------------
>> > [   12.494415] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:1604 _idle
>> > +0x164/0x1
>> > 7c()
>> > [   12.502258] omap_hwmod: gpio6: idle state can only be entered from
>> > enabled st
>> > ate
>> > [   12.509979] Modules linked in:
>> > [   12.513214] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
>> > [<c0042968>] (warn_
>> > slowpath_common+0x4c/0x64)
>> > [   12.523071] [<c0042968>] (warn_slowpath_common+0x4c/0x64) from
>> > [<c0042a14>] (
>> > warn_slowpath_fmt+0x30/0x40)
>> > [   12.533081] [<c0042a14>] (warn_slowpath_fmt+0x30/0x40) from
>> > [<c0028d68>] (_id
>> > le+0x164/0x17c)
>> > [   12.541931] [<c0028d68>] (_idle+0x164/0x17c) from [<c00298b0>]
>> > (omap_hwmod_id
>> > le+0x28/0x3c)
>> > [   12.550567] [<c00298b0>] (omap_hwmod_idle+0x28/0x3c) from
>> > [<c003c664>] (omap_
>> > device_idle_hwmods+0x24/0x3c)
>> > [   12.560699] [<c003c664>] (omap_device_idle_hwmods+0x24/0x3c) from
>> > [<c003c898>
>> > ] (_omap_device_deactivate+0xa4/0x138)
>> > [   12.571594] [<c003c898>] (_omap_device_deactivate+0xa4/0x138) from
>> > [<c003c954
>> >>] (omap_device_idle+0x28/0x54)
>> > [   12.581909] [<c003c954>] (omap_device_idle+0x28/0x54) from
>> > [<c003c99c>] (_od_
>> > runtime_suspend+0x1c/0x24)
>> > [   12.591735] [<c003c99c>] (_od_runtime_suspend+0x1c/0x24) from
>> > [<c02c5010>] (_
>> > _rpm_callback+0x2c/0x78)
>> > [   12.601379] [<c02c5010>] (__rpm_callback+0x2c/0x78) from [<c02c54d0>]
>> > (rpm_su
>> > spend+0x264/0x6c4)
>> > [   12.610504] [<c02c54d0>] (rpm_suspend+0x264/0x6c4) from [<c02c6998>]
>> > (__pm_ru
>> > ntime_suspend+0x5c/0x74)
>> > [   12.620178] [<c02c6998>] (__pm_runtime_suspend+0x5c/0x74) from
>> > [<c02731a4>] (
>> > omap2_gpio_prepare_for_idle+0x50/0x64)
>> > [   12.631103] [<c02731a4>] (omap2_gpio_prepare_for_idle+0x50/0x64) from
>> > [<c002b
>> > d30>] (omap_sram_idle+0xa0/0x3b0)
>> > [   12.641571] [<c002bd30>] (omap_sram_idle+0xa0/0x3b0) from
>> > [<c002c1d8>] (omap3
>> > _pm_idle+0x60/0x178)
>> > [   12.650848] [<c002c1d8>] (omap3_pm_idle+0x60/0x178) from [<c0015c10>]
>> > (cpu_id
>> > le+0xc4/0x108)
>> > [   12.659606] [<c0015c10>] (cpu_idle+0xc4/0x108) from [<c0626828>]
>> > (start_kerne
>> > l+0x2b0/0x304)
>> > [   12.668365] ---[ end trace 441b8fea2b56dcb1 ]---
>> >
>> >
>> > Also, this goes away if I manually force wakeup for the per domain, so
>> > this might be caused by some additional latency.
>> >
>> 
>> What platform are you testing on?
>> 
>> When I test Tarun's series with off-mode on 3430/n900 suspend/resume
>> works fine.  But when I add your v2 series, it never comes out of
>> suspend, and I don't get a kernel dump either.
>> 
>> Can you debug this a little further so we can explain what's going on
>> here?
>> 
>
> After some debugging, it looks like the root cause for the WARN dump I
> see with GPIO PM runtime + usecounting is the fact that PER domain
> enters SW assisted idle and remains OFF after wakeup from OFF mode, and
> it will remain there until some functional clock is enabled. 

OK, but before your usecounting changes, the runtime PM enable was
successfully bringing GPIO out of idle.

In the case of GPIO, the iclk is also the functional clock (it's the
main_clk in the hwmod.)  

> GPIO6 module is only enabling its interface clock, but as this is
> autoidled, it will not wake up PER domain. This will cause the initial
> _wait_target_ready(oh) to fail with EBUSY, leaving the hwmod into idle
> state. Next cpuidle entry will attempt to disable the already idle
> hwmod causing this dump.

I see.  Thanks for tracking it down.

To me, this suggests that the autoidle changes in your usecounting
series aren't quite right, especially for modules like GPIO where the
iclk is also the functional clock.

I added Paul to this discussion to see what his thoughts are on this.

> This can be avoided by preventing PER domain idle on omap3530. 

That doesn't sound quite right either.

> I believe on omap3430 you also need to prevent similar stuff for CORE
> domain.
>
> Did you try the patch I posted last week for the core domain + dplls?

I didn't try it until just now and it' doesn't compile because the new
flag you're using doesn't seem to be defined anywhere.

Kevin

> Does it help with 3430? I guess n900 could hang in the ROM code if PER
> domain is idle during wakeup, as it is a HS device also and behaves
> differently during wakeup from OFF.
>
> -Tero
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Tue, 2012-02-28 at 15:05 -0800, Kevin Hilman wrote:
> +Paul
> 
> Tero Kristo <t-kristo@ti.com> writes:
> 
> > On Wed, 2012-02-22 at 14:37 -0800, Kevin Hilman wrote:
> >> Tero Kristo <t-kristo@ti.com> writes:
> >> 
> >> > On Thu, 2012-02-16 at 09:31 -0800, Kevin Hilman wrote:
> >> >> Tero Kristo <t-kristo@ti.com> writes:
> >> >> 
> >> >> > On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
> >> >> >> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> >> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
> >> >> >> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> >> >> >> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> >> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote: 
> >> >> >> >> > >> Tero Kristo <t-kristo@ti.com> writes:
> >> >> >> >> > >> P
> >> >> >> >> > >> > Attempting this will cause problems especially with off-mode enabled.
> >> >> >> >> > >>
> >> >> >> >> > >> Please be more verbose about the problems seen, and the root cause(s).
> >> >> >> >> > >>
> >> >> >> >> > >
> >> >> >> >> > > I was actually looking forward for some help with this commit message,
> >> >> >> >> > > as I am still not quite sure what is going on in here. :) Here is the
> >> >> >> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> >> >> >> >> > > I think that is probably fixed by the patch from Paul,
> >> >> >> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
> >> >> >> >> > > after wakeup from off-mode, and it is triggered by the disabling of
> >> >> >> >> > > autodeps before off-mode entry.
> >> >> >> >> > >
> >> >> >> >> > This mostly indicates that one of the per clock-domain module
> >> >> >> >> > clock turning ON seems to be not working well with auto deps
> >> >> >> >> > disabled. This leads to interconnect violation.
> >> >> >> >> >
> >> >> >> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
> >> >> >> >> > in the low power code early resume path and see if this
> >> >> >> >> > error goes away.
> >> >> >> >>
> >> >> >> >> This seems to get rid of the dump also. It looks like some driver resume
> >> >> >> >> is not behaving nicely, I am trying to pinpoint the culprit currently
> >> >> >> >> and see whether it can provide more info.
> >> >> >> >
> >> >> >> > Okay, I have some more info about this now.
> >> >> >> >
> >> >> >> > What happens is that when entering off-mode, PER domain remains OFF even
> >> >> >> > during the execution of the exit phase from omap_sram_idle. Adding a
> >> >> >> > manual SW_WKUP it comes up and there are no issues. If autodeps are
> >> >> >> > enabled on the domain, it comes back from off mode as active.
> >> >> >> >
> >> >> >> > Looking further in the code, we have this at the end of omap_sram_idle:
> >> >> >> >
> >> >> >> >        if (per_next_state < PWRDM_POWER_ON) {
> >> >> >> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> >> >> >> >                omap2_gpio_resume_after_idle();
> >> >> >> >                wake_per();
> >> >> >> >                if (per_prev_state == PWRDM_POWER_OFF)
> >> >> >> >                        omap3_per_restore_context();
> >> >> >> >        }
> >> >> >> >
> >> >> >> > ... which seems to assume that per domain is on. Gpio code does not
> >> >> >> > control any clocks currently, as it only requires the interface clock to
> >> >> >> > be on, and as this is autoidled....
> >> >> >> >
> >> >> >> > Any comments how we should handle this? Shall we just keep these two
> >> >> >> > patches for handling this or add some different hackery for the gpio
> >> >> >> > issue?
> >> >> >> >
> >> >> >> Good. I thought too that issue will disappear.
> >> >> >> The issue is pretty clear. Technically every driver pm_runtime() code should
> >> >> >> be able to manage a clock->clockdomain->power domain power up/down
> >> >> >> sequence. That should work without auto deps.
> >> >> >> 
> >> >> >> Do you narrowed down which driver resume is creating the dump ?
> >> >> >> UART , GPIO ?
> >> >> >
> >> >> > It is the gpio base stuff called from omap_sram_idle(), basically the
> >> >> > restore context part. If I force enable per domain before the code
> >> >> > snippet before, there is no dump, but if it is done after, I get the
> >> >> > dump.
> >> >> >
> >> >> > The thing is that gpio driver doesn't currently have this kind of
> >> >> > mechanism for the restore context part, at least not on omap3 due to
> >> >> > above clocking issue (only autoidled interface clock is used.)
> >> >> 
> >> >> I'm not sure if it fully addresses this, but Tarun's series converts
> >> >> GPIO to runtime PM.
> >> >> 
> >> >> Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
> >> >> branch here:
> >> >> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git
> >> >
> >> > It does something for the issue, but I still get this during suspend to
> >> > off:
> >> >
> >> > [   11.284973] PM: Syncing filesystems ... done.
> >> > [   11.379241] Freezing user space processes ... (elapsed 0.02 seconds)
> >> > done.
> >> > [   11.408233] Freezing remaining freezable tasks ... (elapsed 0.02
> >> > seconds) don
> >> > e.
> >> > [   11.439239] Suspending console(s) (use no_console_suspend to debug)
> >> > [   11.564178] PM: suspend of devices complete after 115.506 msecs
> >> > [   11.567382] PM: late suspend of devices complete after 3.204 msecs
> >> > [   11.567443] Disabling non-boot CPUs ...
> >> > [   12.004089] Powerdomain (cam_pwrdm) didn't enter target state 0
> >> > [   12.004119] Could not enter target state in pm_suspend
> >> > [   12.009368] PM: early resume of devices complete after 4.944 msecs
> >> > [   12.436645] PM: resume of devices complete after 426.086 msecs
> >> > [   12.480285] Restarting tasks ... done.
> >> > /sys/kernel/debu[   12.488433] ------------[ cut here ]------------
> >> > [   12.494415] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:1604 _idle
> >> > +0x164/0x1
> >> > 7c()
> >> > [   12.502258] omap_hwmod: gpio6: idle state can only be entered from
> >> > enabled st
> >> > ate
> >> > [   12.509979] Modules linked in:
> >> > [   12.513214] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
> >> > [<c0042968>] (warn_
> >> > slowpath_common+0x4c/0x64)
> >> > [   12.523071] [<c0042968>] (warn_slowpath_common+0x4c/0x64) from
> >> > [<c0042a14>] (
> >> > warn_slowpath_fmt+0x30/0x40)
> >> > [   12.533081] [<c0042a14>] (warn_slowpath_fmt+0x30/0x40) from
> >> > [<c0028d68>] (_id
> >> > le+0x164/0x17c)
> >> > [   12.541931] [<c0028d68>] (_idle+0x164/0x17c) from [<c00298b0>]
> >> > (omap_hwmod_id
> >> > le+0x28/0x3c)
> >> > [   12.550567] [<c00298b0>] (omap_hwmod_idle+0x28/0x3c) from
> >> > [<c003c664>] (omap_
> >> > device_idle_hwmods+0x24/0x3c)
> >> > [   12.560699] [<c003c664>] (omap_device_idle_hwmods+0x24/0x3c) from
> >> > [<c003c898>
> >> > ] (_omap_device_deactivate+0xa4/0x138)
> >> > [   12.571594] [<c003c898>] (_omap_device_deactivate+0xa4/0x138) from
> >> > [<c003c954
> >> >>] (omap_device_idle+0x28/0x54)
> >> > [   12.581909] [<c003c954>] (omap_device_idle+0x28/0x54) from
> >> > [<c003c99c>] (_od_
> >> > runtime_suspend+0x1c/0x24)
> >> > [   12.591735] [<c003c99c>] (_od_runtime_suspend+0x1c/0x24) from
> >> > [<c02c5010>] (_
> >> > _rpm_callback+0x2c/0x78)
> >> > [   12.601379] [<c02c5010>] (__rpm_callback+0x2c/0x78) from [<c02c54d0>]
> >> > (rpm_su
> >> > spend+0x264/0x6c4)
> >> > [   12.610504] [<c02c54d0>] (rpm_suspend+0x264/0x6c4) from [<c02c6998>]
> >> > (__pm_ru
> >> > ntime_suspend+0x5c/0x74)
> >> > [   12.620178] [<c02c6998>] (__pm_runtime_suspend+0x5c/0x74) from
> >> > [<c02731a4>] (
> >> > omap2_gpio_prepare_for_idle+0x50/0x64)
> >> > [   12.631103] [<c02731a4>] (omap2_gpio_prepare_for_idle+0x50/0x64) from
> >> > [<c002b
> >> > d30>] (omap_sram_idle+0xa0/0x3b0)
> >> > [   12.641571] [<c002bd30>] (omap_sram_idle+0xa0/0x3b0) from
> >> > [<c002c1d8>] (omap3
> >> > _pm_idle+0x60/0x178)
> >> > [   12.650848] [<c002c1d8>] (omap3_pm_idle+0x60/0x178) from [<c0015c10>]
> >> > (cpu_id
> >> > le+0xc4/0x108)
> >> > [   12.659606] [<c0015c10>] (cpu_idle+0xc4/0x108) from [<c0626828>]
> >> > (start_kerne
> >> > l+0x2b0/0x304)
> >> > [   12.668365] ---[ end trace 441b8fea2b56dcb1 ]---
> >> >
> >> >
> >> > Also, this goes away if I manually force wakeup for the per domain, so
> >> > this might be caused by some additional latency.
> >> >
> >> 
> >> What platform are you testing on?
> >> 
> >> When I test Tarun's series with off-mode on 3430/n900 suspend/resume
> >> works fine.  But when I add your v2 series, it never comes out of
> >> suspend, and I don't get a kernel dump either.
> >> 
> >> Can you debug this a little further so we can explain what's going on
> >> here?
> >> 
> >
> > After some debugging, it looks like the root cause for the WARN dump I
> > see with GPIO PM runtime + usecounting is the fact that PER domain
> > enters SW assisted idle and remains OFF after wakeup from OFF mode, and
> > it will remain there until some functional clock is enabled. 
> 
> OK, but before your usecounting changes, the runtime PM enable was
> successfully bringing GPIO out of idle.

Well, the thing is in this case, PER never idles by SW, it is only
following MPU to idle and back. PER awakens immediately by wakeup from
OFF, which is not the case with the usecounting set.

> In the case of GPIO, the iclk is also the functional clock (it's the
> main_clk in the hwmod.)  
> 
> > GPIO6 module is only enabling its interface clock, but as this is
> > autoidled, it will not wake up PER domain. This will cause the initial
> > _wait_target_ready(oh) to fail with EBUSY, leaving the hwmod into idle
> > state. Next cpuidle entry will attempt to disable the already idle
> > hwmod causing this dump.
> 
> I see.  Thanks for tracking it down.
> 
> To me, this suggests that the autoidle changes in your usecounting
> series aren't quite right, especially for modules like GPIO where the
> iclk is also the functional clock.

Hmm, it might be possible to change the behavior to mark a few of the
iclks as functional clocks. Or alternatively take a hwmod based approach
that will just track active hwmods instead of clocks + hwmods and leave
the clocks as they are. Still the PER domain will require additional
tweaks due to the wake dep stuff. I think I'll experiment with this a
bit and see.

> 
> I added Paul to this discussion to see what his thoughts are on this.

Yea, would be nice to get Paul's comments how he thinks this should be
handled.

> 
> > This can be avoided by preventing PER domain idle on omap3530. 
> 
> That doesn't sound quite right either.

I think I was somewhat confusing with this comment. PER domain idle is
not prevented, we just prevent the manual SW triggered transitions for
PER.

> 
> > I believe on omap3430 you also need to prevent similar stuff for CORE
> > domain.
> >
> > Did you try the patch I posted last week for the core domain + dplls?
> 
> I didn't try it until just now and it' doesn't compile because the new
> flag you're using doesn't seem to be defined anywhere.

You need to use that patch in addition to the usecount set.

-Tero

> 
> Kevin
> 
> > Does it help with 3430? I guess n900 could hang in the ROM code if PER
> > domain is idle during wakeup, as it is a HS device also and behaves
> > differently during wakeup from OFF.
> >
> > -Tero
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 8/8] arm: omap3: prevent per_clkdm from attempting manual domain transitions

[Tero Kristo]

On Wed, 2012-02-29 at 10:01 +0200, Tero Kristo wrote:
> On Tue, 2012-02-28 at 15:05 -0800, Kevin Hilman wrote:
> > +Paul
> > 
> > Tero Kristo <t-kristo@ti.com> writes:
> > 
> > > On Wed, 2012-02-22 at 14:37 -0800, Kevin Hilman wrote:
> > >> Tero Kristo <t-kristo@ti.com> writes:
> > >> 
> > >> > On Thu, 2012-02-16 at 09:31 -0800, Kevin Hilman wrote:
> > >> >> Tero Kristo <t-kristo@ti.com> writes:
> > >> >> 
> > >> >> > On Thu, 2012-02-16 at 21:15 +0530, Shilimkar, Santosh wrote:
> > >> >> >> On Thu, Feb 16, 2012 at 8:53 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > >> >> >> > On Thu, 2012-02-16 at 15:15 +0200, Tero Kristo wrote:
> > >> >> >> >> On Thu, 2012-02-16 at 15:27 +0530, Shilimkar, Santosh wrote:
> > >> >> >> >> > On Thu, Feb 16, 2012 at 2:27 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > >> >> >> >> > > On Wed, 2012-02-15 at 11:37 -0800, Kevin Hilman wrote: 
> > >> >> >> >> > >> Tero Kristo <t-kristo@ti.com> writes:
> > >> >> >> >> > >> P
> > >> >> >> >> > >> > Attempting this will cause problems especially with off-mode enabled.
> > >> >> >> >> > >>
> > >> >> >> >> > >> Please be more verbose about the problems seen, and the root cause(s).
> > >> >> >> >> > >>
> > >> >> >> >> > >
> > >> >> >> >> > > I was actually looking forward for some help with this commit message,
> > >> >> >> >> > > as I am still not quite sure what is going on in here. :) Here is the
> > >> >> >> >> > > log for suspend (btw, cam_pwrdm does not go to off in mainline yet, but
> > >> >> >> >> > > I think that is probably fixed by the patch from Paul,
> > >> >> >> >> > > omap_set_pwrdm_state() does not work properly.) The warning comes out
> > >> >> >> >> > > after wakeup from off-mode, and it is triggered by the disabling of
> > >> >> >> >> > > autodeps before off-mode entry.
> > >> >> >> >> > >
> > >> >> >> >> > This mostly indicates that one of the per clock-domain module
> > >> >> >> >> > clock turning ON seems to be not working well with auto deps
> > >> >> >> >> > disabled. This leads to interconnect violation.
> > >> >> >> >> >
> > >> >> >> >> > if not tried already, can you put the per_clockdomain in SW_WKUP
> > >> >> >> >> > in the low power code early resume path and see if this
> > >> >> >> >> > error goes away.
> > >> >> >> >>
> > >> >> >> >> This seems to get rid of the dump also. It looks like some driver resume
> > >> >> >> >> is not behaving nicely, I am trying to pinpoint the culprit currently
> > >> >> >> >> and see whether it can provide more info.
> > >> >> >> >
> > >> >> >> > Okay, I have some more info about this now.
> > >> >> >> >
> > >> >> >> > What happens is that when entering off-mode, PER domain remains OFF even
> > >> >> >> > during the execution of the exit phase from omap_sram_idle. Adding a
> > >> >> >> > manual SW_WKUP it comes up and there are no issues. If autodeps are
> > >> >> >> > enabled on the domain, it comes back from off mode as active.
> > >> >> >> >
> > >> >> >> > Looking further in the code, we have this at the end of omap_sram_idle:
> > >> >> >> >
> > >> >> >> >        if (per_next_state < PWRDM_POWER_ON) {
> > >> >> >> >                per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> > >> >> >> >                omap2_gpio_resume_after_idle();
> > >> >> >> >                wake_per();
> > >> >> >> >                if (per_prev_state == PWRDM_POWER_OFF)
> > >> >> >> >                        omap3_per_restore_context();
> > >> >> >> >        }
> > >> >> >> >
> > >> >> >> > ... which seems to assume that per domain is on. Gpio code does not
> > >> >> >> > control any clocks currently, as it only requires the interface clock to
> > >> >> >> > be on, and as this is autoidled....
> > >> >> >> >
> > >> >> >> > Any comments how we should handle this? Shall we just keep these two
> > >> >> >> > patches for handling this or add some different hackery for the gpio
> > >> >> >> > issue?
> > >> >> >> >
> > >> >> >> Good. I thought too that issue will disappear.
> > >> >> >> The issue is pretty clear. Technically every driver pm_runtime() code should
> > >> >> >> be able to manage a clock->clockdomain->power domain power up/down
> > >> >> >> sequence. That should work without auto deps.
> > >> >> >> 
> > >> >> >> Do you narrowed down which driver resume is creating the dump ?
> > >> >> >> UART , GPIO ?
> > >> >> >
> > >> >> > It is the gpio base stuff called from omap_sram_idle(), basically the
> > >> >> > restore context part. If I force enable per domain before the code
> > >> >> > snippet before, there is no dump, but if it is done after, I get the
> > >> >> > dump.
> > >> >> >
> > >> >> > The thing is that gpio driver doesn't currently have this kind of
> > >> >> > mechanism for the restore context part, at least not on omap3 due to
> > >> >> > above clocking issue (only autoidled interface clock is used.)
> > >> >> 
> > >> >> I'm not sure if it fully addresses this, but Tarun's series converts
> > >> >> GPIO to runtime PM.
> > >> >> 
> > >> >> Can you try with Tarun's series.  See the for_3.4/gpio_cleanup_fixes_v9
> > >> >> branch here:
> > >> >> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git
> > >> >
> > >> > It does something for the issue, but I still get this during suspend to
> > >> > off:
> > >> >
> > >> > [   11.284973] PM: Syncing filesystems ... done.
> > >> > [   11.379241] Freezing user space processes ... (elapsed 0.02 seconds)
> > >> > done.
> > >> > [   11.408233] Freezing remaining freezable tasks ... (elapsed 0.02
> > >> > seconds) don
> > >> > e.
> > >> > [   11.439239] Suspending console(s) (use no_console_suspend to debug)
> > >> > [   11.564178] PM: suspend of devices complete after 115.506 msecs
> > >> > [   11.567382] PM: late suspend of devices complete after 3.204 msecs
> > >> > [   11.567443] Disabling non-boot CPUs ...
> > >> > [   12.004089] Powerdomain (cam_pwrdm) didn't enter target state 0
> > >> > [   12.004119] Could not enter target state in pm_suspend
> > >> > [   12.009368] PM: early resume of devices complete after 4.944 msecs
> > >> > [   12.436645] PM: resume of devices complete after 426.086 msecs
> > >> > [   12.480285] Restarting tasks ... done.
> > >> > /sys/kernel/debu[   12.488433] ------------[ cut here ]------------
> > >> > [   12.494415] WARNING: at arch/arm/mach-omap2/omap_hwmod.c:1604 _idle
> > >> > +0x164/0x1
> > >> > 7c()
> > >> > [   12.502258] omap_hwmod: gpio6: idle state can only be entered from
> > >> > enabled st
> > >> > ate
> > >> > [   12.509979] Modules linked in:
> > >> > [   12.513214] [<c001bcd0>] (unwind_backtrace+0x0/0xf4) from
> > >> > [<c0042968>] (warn_
> > >> > slowpath_common+0x4c/0x64)
> > >> > [   12.523071] [<c0042968>] (warn_slowpath_common+0x4c/0x64) from
> > >> > [<c0042a14>] (
> > >> > warn_slowpath_fmt+0x30/0x40)
> > >> > [   12.533081] [<c0042a14>] (warn_slowpath_fmt+0x30/0x40) from
> > >> > [<c0028d68>] (_id
> > >> > le+0x164/0x17c)
> > >> > [   12.541931] [<c0028d68>] (_idle+0x164/0x17c) from [<c00298b0>]
> > >> > (omap_hwmod_id
> > >> > le+0x28/0x3c)
> > >> > [   12.550567] [<c00298b0>] (omap_hwmod_idle+0x28/0x3c) from
> > >> > [<c003c664>] (omap_
> > >> > device_idle_hwmods+0x24/0x3c)
> > >> > [   12.560699] [<c003c664>] (omap_device_idle_hwmods+0x24/0x3c) from
> > >> > [<c003c898>
> > >> > ] (_omap_device_deactivate+0xa4/0x138)
> > >> > [   12.571594] [<c003c898>] (_omap_device_deactivate+0xa4/0x138) from
> > >> > [<c003c954
> > >> >>] (omap_device_idle+0x28/0x54)
> > >> > [   12.581909] [<c003c954>] (omap_device_idle+0x28/0x54) from
> > >> > [<c003c99c>] (_od_
> > >> > runtime_suspend+0x1c/0x24)
> > >> > [   12.591735] [<c003c99c>] (_od_runtime_suspend+0x1c/0x24) from
> > >> > [<c02c5010>] (_
> > >> > _rpm_callback+0x2c/0x78)
> > >> > [   12.601379] [<c02c5010>] (__rpm_callback+0x2c/0x78) from [<c02c54d0>]
> > >> > (rpm_su
> > >> > spend+0x264/0x6c4)
> > >> > [   12.610504] [<c02c54d0>] (rpm_suspend+0x264/0x6c4) from [<c02c6998>]
> > >> > (__pm_ru
> > >> > ntime_suspend+0x5c/0x74)
> > >> > [   12.620178] [<c02c6998>] (__pm_runtime_suspend+0x5c/0x74) from
> > >> > [<c02731a4>] (
> > >> > omap2_gpio_prepare_for_idle+0x50/0x64)
> > >> > [   12.631103] [<c02731a4>] (omap2_gpio_prepare_for_idle+0x50/0x64) from
> > >> > [<c002b
> > >> > d30>] (omap_sram_idle+0xa0/0x3b0)
> > >> > [   12.641571] [<c002bd30>] (omap_sram_idle+0xa0/0x3b0) from
> > >> > [<c002c1d8>] (omap3
> > >> > _pm_idle+0x60/0x178)
> > >> > [   12.650848] [<c002c1d8>] (omap3_pm_idle+0x60/0x178) from [<c0015c10>]
> > >> > (cpu_id
> > >> > le+0xc4/0x108)
> > >> > [   12.659606] [<c0015c10>] (cpu_idle+0xc4/0x108) from [<c0626828>]
> > >> > (start_kerne
> > >> > l+0x2b0/0x304)
> > >> > [   12.668365] ---[ end trace 441b8fea2b56dcb1 ]---
> > >> >
> > >> >
> > >> > Also, this goes away if I manually force wakeup for the per domain, so
> > >> > this might be caused by some additional latency.
> > >> >
> > >> 
> > >> What platform are you testing on?
> > >> 
> > >> When I test Tarun's series with off-mode on 3430/n900 suspend/resume
> > >> works fine.  But when I add your v2 series, it never comes out of
> > >> suspend, and I don't get a kernel dump either.
> > >> 
> > >> Can you debug this a little further so we can explain what's going on
> > >> here?
> > >> 
> > >
> > > After some debugging, it looks like the root cause for the WARN dump I
> > > see with GPIO PM runtime + usecounting is the fact that PER domain
> > > enters SW assisted idle and remains OFF after wakeup from OFF mode, and
> > > it will remain there until some functional clock is enabled. 
> > 
> > OK, but before your usecounting changes, the runtime PM enable was
> > successfully bringing GPIO out of idle.
> 
> Well, the thing is in this case, PER never idles by SW, it is only
> following MPU to idle and back. PER awakens immediately by wakeup from
> OFF, which is not the case with the usecounting set.
> 
> > In the case of GPIO, the iclk is also the functional clock (it's the
> > main_clk in the hwmod.)  
> > 
> > > GPIO6 module is only enabling its interface clock, but as this is
> > > autoidled, it will not wake up PER domain. This will cause the initial
> > > _wait_target_ready(oh) to fail with EBUSY, leaving the hwmod into idle
> > > state. Next cpuidle entry will attempt to disable the already idle
> > > hwmod causing this dump.
> > 
> > I see.  Thanks for tracking it down.
> > 
> > To me, this suggests that the autoidle changes in your usecounting
> > series aren't quite right, especially for modules like GPIO where the
> > iclk is also the functional clock.
> 
> Hmm, it might be possible to change the behavior to mark a few of the
> iclks as functional clocks. Or alternatively take a hwmod based approach
> that will just track active hwmods instead of clocks + hwmods and leave
> the clocks as they are. Still the PER domain will require additional
> tweaks due to the wake dep stuff. I think I'll experiment with this a
> bit and see.

If I change the gpio*_ick clkops from clkops_omap2_iclk_dflt_wait to
clkops_omap2_dflt_wait, the issue is gone, this basically prevents
autoidle for these clocks. The hwmod approach does not work for omap3
currently, as most of the hwmods don't have clkdm defined for them (only
a couple of USB hwmods have this.)

Anyway, I think we still need to prevent the manual transition tries for
per / core domains to avoid any trouble. There are even a couple of
omap3 erratas related to per wkdeps and how they should not be disabled
when going to idle.

-Tero

> 
> > 
> > I added Paul to this discussion to see what his thoughts are on this.
> 
> Yea, would be nice to get Paul's comments how he thinks this should be
> handled.
> 
> > 
> > > This can be avoided by preventing PER domain idle on omap3530. 
> > 
> > That doesn't sound quite right either.
> 
> I think I was somewhat confusing with this comment. PER domain idle is
> not prevented, we just prevent the manual SW triggered transitions for
> PER.
> 
> > 
> > > I believe on omap3430 you also need to prevent similar stuff for CORE
> > > domain.
> > >
> > > Did you try the patch I posted last week for the core domain + dplls?
> > 
> > I didn't try it until just now and it' doesn't compile because the new
> > flag you're using doesn't seem to be defined anywhere.
> 
> You need to use that patch in addition to the usecount set.
> 
> -Tero
> 
> > 
> > Kevin
> > 
> > > Does it help with 3430? I guess n900 could hang in the ROM code if PER
> > > domain is idle during wakeup, as it is a HS device also and behaves
> > > differently during wakeup from OFF.
> > >
> > > -Tero
> > >
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > > the body of a message to majordomo at vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCHv2 0/8] arm: omap: clk/clkdm/pwrdm/voltdm usecounting changes

[Jean Pihet]

Hi Tero,

Great to see those changes merged in.
FWIW:
Reviewed-by: Jean Pihet <j-pihet@ti.com>

Jean

On Wed, Feb 15, 2012 at 7:37 AM, Tero Kristo <t-kristo@ti.com> wrote:
> Changes compared to previous version:
>
> - moved voltdm_pwrdm_enable / disable hooks from idle code to
> ?pwrdm_pre_transition / pwrdm_post_transition functions (patch 4)
> - added patches 7 and 8 to fix one issue that was visible with off-mode
> ?on omap3: attempting to disable wkdeps for per_clkdm causes crash
> ?dumps to be generated by omap_l3_smx (this was actually part of the
> ?original larger set but I dropped it accidentally from v1), this issue
> ?surfaces with this patch set as the kernel actually tries to manually
> ?idle per_clkdm (probably first time in history)
> - added some parameter sanity checks (pwrdm/voltdm = null checks)
>
> Similar tests done as with previous version, additionally off-mode tests
> with omap3.
>
> -Tero
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html