[PATCH 0/6] arm: omap: clk/clkdm/pwrdm/voltdm changes to usecounting

[PATCH 0/6] arm: omap: clk/clkdm/pwrdm/voltdm changes to usecounting

[Tero Kristo]

This set is split out from a larger set earlier circulated on linux-omap
list. The main purpose of this set is to provide useful recursive
usecounting support for clockdomains / powerdomains / voltagedomains
which can be used for debugging purposes and also to control certain
chip power features once a sleep condition is reached (voltdm usecount=0).

This set provides only the baseline support for the usecounting, and also
adds a pm-debug related patch that can be used to check out stuff (patch 6),
and should later prove rather useful for debugging. Rest of the
functionality will be posted in separate sets.

This set has been tested on omap3 beagle board with suspend / resume /
cpuidle. Also boot tested on omap4 panda (sleep is not functional yet
on that platform.)

Applies on mainline kernel.

-Tero

[PATCH 1/6] 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

[PATCH 2/6] 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       |   10 +++++++++
 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, 78 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..78bf3c0 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -985,6 +985,16 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm)
 	return -EINVAL;
 }
 
+void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
+{
+	atomic_inc(&pwrdm->usecount);
+}
+
+void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
+{
+	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

[PATCH 3/6] 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     |   48 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/voltage.h     |   11 ++++++++
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 78bf3c0..4ded0c8 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -987,12 +987,14 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm)
 
 void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
 {
-	atomic_inc(&pwrdm->usecount);
+	if (atomic_inc_return(&pwrdm->usecount) == 1)
+		voltdm_pwrdm_enable(pwrdm->voltdm.ptr);
 }
 
 void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
 {
-	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..88f0e98 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,53 @@ 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 (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 (!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

[PATCH 4/6] arm: omap3: add manual control for mpu_iva voltdm usecounting

[Tero Kristo]

mpu_iva voltdm usecount is now decreased during idle. This will
allow the voltagedomain to reflect actual usage, and will allow
proper usage of vc callbacks.

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/pm34xx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index fc69875..14defaf 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -74,6 +74,7 @@ void (*omap3_do_wfi_sram)(void);
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
 static struct powerdomain *cam_pwrdm;
+static struct voltagedomain *mpu_iva_voltdm;
 
 static inline void omap3_per_save_context(void)
 {
@@ -311,6 +312,8 @@ void omap_sram_idle(void)
 		return;
 	}
 
+	voltdm_pwrdm_disable(mpu_iva_voltdm);
+
 	/* NEON control */
 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
@@ -393,6 +396,8 @@ void omap_sram_idle(void)
 	}
 	omap3_intc_resume_idle();
 
+	voltdm_pwrdm_enable(mpu_iva_voltdm);
+
 	pwrdm_post_transition();
 
 	/* PER */
@@ -828,6 +833,9 @@ static int __init omap3_pm_init(void)
 
 	(void) clkdm_for_each(clkdms_setup, NULL);
 
+	mpu_iva_voltdm = voltdm_lookup("mpu_iva");
+	voltdm_pwrdm_enable(mpu_iva_voltdm);
+
 	mpu_pwrdm = pwrdm_lookup("mpu_pwrdm");
 	if (mpu_pwrdm == NULL) {
 		printk(KERN_ERR "Failed to get mpu_pwrdm\n");
-- 
1.7.4.1

[PATCH 4/6] arm: omap3: add manual control for mpu_iva voltdm usecounting

[Kevin Hilman]

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

> mpu_iva voltdm usecount is now decreased during idle. This will
> allow the voltagedomain to reflect actual usage, and will allow
> proper usage of vc callbacks.

I don't follow why this is needed in the idle path.

Why aren't the usecounts from clock/powerdomain disables (enables) causing this
voltdm disable (enable) to happen automatcially?

If these are needed, seems like they should be in the
pwrdm_[pre|post]_transition() hooks so they can be generic.

Kevin

> 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/pm34xx.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index fc69875..14defaf 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -74,6 +74,7 @@ void (*omap3_do_wfi_sram)(void);
>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>  static struct powerdomain *core_pwrdm, *per_pwrdm;
>  static struct powerdomain *cam_pwrdm;
> +static struct voltagedomain *mpu_iva_voltdm;
>  
>  static inline void omap3_per_save_context(void)
>  {
> @@ -311,6 +312,8 @@ void omap_sram_idle(void)
>  		return;
>  	}
>  
> +	voltdm_pwrdm_disable(mpu_iva_voltdm);
> +
>  	/* NEON control */
>  	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
>  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
> @@ -393,6 +396,8 @@ void omap_sram_idle(void)
>  	}
>  	omap3_intc_resume_idle();
>  
> +	voltdm_pwrdm_enable(mpu_iva_voltdm);
> +
>  	pwrdm_post_transition();
>  
>  	/* PER */
> @@ -828,6 +833,9 @@ static int __init omap3_pm_init(void)
>  
>  	(void) clkdm_for_each(clkdms_setup, NULL);
>  
> +	mpu_iva_voltdm = voltdm_lookup("mpu_iva");
> +	voltdm_pwrdm_enable(mpu_iva_voltdm);
> +
>  	mpu_pwrdm = pwrdm_lookup("mpu_pwrdm");
>  	if (mpu_pwrdm == NULL) {
>  		printk(KERN_ERR "Failed to get mpu_pwrdm\n");

[PATCH 4/6] arm: omap3: add manual control for mpu_iva voltdm usecounting

[Tero Kristo]

On Tue, 2012-02-14 at 11:35 -0800, Kevin Hilman wrote:
> Tero Kristo <t-kristo@ti.com> writes:
> 
> > mpu_iva voltdm usecount is now decreased during idle. This will
> > allow the voltagedomain to reflect actual usage, and will allow
> > proper usage of vc callbacks.
> 
> I don't follow why this is needed in the idle path.
> 
> Why aren't the usecounts from clock/powerdomain disables (enables) causing this
> voltdm disable (enable) to happen automatcially?

The main issue is mpu powerdomain which doesn't contain any clocks that
are controlled actively. Without adding a "virtual" usecount
enable/disable hook, the usecount for mpu_iva will always remain at 0
(or just follow the iva domain on omap3.) We want the usecount to
reflect the actual idling of this domain also, so that we can control
SR / auto-ret based on MPU activity.

> If these are needed, seems like they should be in the
> pwrdm_[pre|post]_transition() hooks so they can be generic.

This is actually a good idea, I could move them here. I could also add a
manual triggering for core domain here, as I have a feeling it might be
good to have so that core will also follow the idle cycle more tightly
(basically as a virtual follow for sdrc clock which is idled by HW
during wfi.)

-Tero

> 
> Kevin
> 
> > 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/pm34xx.c |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> > index fc69875..14defaf 100644
> > --- a/arch/arm/mach-omap2/pm34xx.c
> > +++ b/arch/arm/mach-omap2/pm34xx.c
> > @@ -74,6 +74,7 @@ void (*omap3_do_wfi_sram)(void);
> >  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
> >  static struct powerdomain *core_pwrdm, *per_pwrdm;
> >  static struct powerdomain *cam_pwrdm;
> > +static struct voltagedomain *mpu_iva_voltdm;
> >  
> >  static inline void omap3_per_save_context(void)
> >  {
> > @@ -311,6 +312,8 @@ void omap_sram_idle(void)
> >  		return;
> >  	}
> >  
> > +	voltdm_pwrdm_disable(mpu_iva_voltdm);
> > +
> >  	/* NEON control */
> >  	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
> >  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
> > @@ -393,6 +396,8 @@ void omap_sram_idle(void)
> >  	}
> >  	omap3_intc_resume_idle();
> >  
> > +	voltdm_pwrdm_enable(mpu_iva_voltdm);
> > +
> >  	pwrdm_post_transition();
> >  
> >  	/* PER */
> > @@ -828,6 +833,9 @@ static int __init omap3_pm_init(void)
> >  
> >  	(void) clkdm_for_each(clkdms_setup, NULL);
> >  
> > +	mpu_iva_voltdm = voltdm_lookup("mpu_iva");
> > +	voltdm_pwrdm_enable(mpu_iva_voltdm);
> > +
> >  	mpu_pwrdm = pwrdm_lookup("mpu_pwrdm");
> >  	if (mpu_pwrdm == NULL) {
> >  		printk(KERN_ERR "Failed to get mpu_pwrdm\n");

[PATCH 4/6] arm: omap3: add manual control for mpu_iva voltdm usecounting

[Kevin Hilman]

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

> On Tue, 2012-02-14 at 11:35 -0800, Kevin Hilman wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>> 
>> > mpu_iva voltdm usecount is now decreased during idle. This will
>> > allow the voltagedomain to reflect actual usage, and will allow
>> > proper usage of vc callbacks.
>> 
>> I don't follow why this is needed in the idle path.
>> 
>> Why aren't the usecounts from clock/powerdomain disables (enables) causing this
>> voltdm disable (enable) to happen automatcially?
>
> The main issue is mpu powerdomain which doesn't contain any clocks that
> are controlled actively. Without adding a "virtual" usecount
> enable/disable hook, the usecount for mpu_iva will always remain at 0
> (or just follow the iva domain on omap3.) 

Rather than faking the MPU clock gating by disabling the voltage/power
domains, why not just gate the MPU clock in idle?   That would trigger
all the rest and would also be much clearer what is happening in the
idle path.

> We want the usecount to reflect the actual idling of this domain also,
> so that we can control SR / auto-ret based on MPU activity.

Yes, makes sense now.  Thanks for the explanation.

>> If these are needed, seems like they should be in the
>> pwrdm_[pre|post]_transition() hooks so they can be generic.
>
> This is actually a good idea, I could move them here. I could also add a
> manual triggering for core domain here, as I have a feeling it might be
> good to have so that core will also follow the idle cycle more tightly
> (basically as a virtual follow for sdrc clock which is idled by HW
> during wfi.)

In the comments on the new version, you should also describe in detail
the reasons for the "manual" idling of CORE (due to clocks that are
hardware managed.)

Kevin

[PATCH 5/6] 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

[PATCH 6/6] 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

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

[Tony Lindgren]

* Tero Kristo <t-kristo@ti.com> [120214 08:19]:
> 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.

Why don't you replace this all with a userspace tool that deciphers
the registers for you?

Tony

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

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Tero Kristo <t-kristo@ti.com> [120214 08:19]:
>> 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.
>
> Why don't you replace this all with a userspace tool that deciphers
> the registers for you?

This patch isn't deciphering registers, it's just dumping usecounts, and
I think that is extremely useful to have in debugfs.

I've already removed all the register dumping from the kernel in the
hopes that someone will write a userspace tool for that.

Kevin

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

[Tony Lindgren]

* Kevin Hilman <khilman@ti.com> [120214 14:28]:
> Tony Lindgren <tony@atomide.com> writes:
> 
> > * Tero Kristo <t-kristo@ti.com> [120214 08:19]:
> >> 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.
> >
> > Why don't you replace this all with a userspace tool that deciphers
> > the registers for you?
> 
> This patch isn't deciphering registers, it's just dumping usecounts, and
> I think that is extremely useful to have in debugfs.
> 
> I've already removed all the register dumping from the kernel in the
> hopes that someone will write a userspace tool for that.

OK good to hear you're already considering it.

Tony

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

[Tero Kristo]

On Tue, 2012-02-14 at 15:52 -0800, Tony Lindgren wrote:
> * Kevin Hilman <khilman@ti.com> [120214 14:28]:
> > Tony Lindgren <tony@atomide.com> writes:
> > 
> > > * Tero Kristo <t-kristo@ti.com> [120214 08:19]:
> > >> 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.
> > >
> > > Why don't you replace this all with a userspace tool that deciphers
> > > the registers for you?
> > 
> > This patch isn't deciphering registers, it's just dumping usecounts, and
> > I think that is extremely useful to have in debugfs.
> > 
> > I've already removed all the register dumping from the kernel in the
> > hopes that someone will write a userspace tool for that.
> 
> OK good to hear you're already considering it.

Yes, register dumps are gone, and I am actually one of the persons who
is missing it.

I think there should still be some capability to get register snapshots
from certain points during kernel execution, this is useful for
debugging purposes. I don't know if it would be possible to do a
call_usermodehelper() or something from kernel space just before wfi to
read all (or part of) the PRCM registers, store them somewhere, and then
decipher this data later with another tool. Any comments to this?

-Tero

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

[Kevin Hilman]

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

> On Tue, 2012-02-14 at 15:52 -0800, Tony Lindgren wrote:
>> * Kevin Hilman <khilman@ti.com> [120214 14:28]:
>> > Tony Lindgren <tony@atomide.com> writes:
>> > 
>> > > * Tero Kristo <t-kristo@ti.com> [120214 08:19]:
>> > >> 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.
>> > >
>> > > Why don't you replace this all with a userspace tool that deciphers
>> > > the registers for you?
>> > 
>> > This patch isn't deciphering registers, it's just dumping usecounts, and
>> > I think that is extremely useful to have in debugfs.
>> > 
>> > I've already removed all the register dumping from the kernel in the
>> > hopes that someone will write a userspace tool for that.
>> 
>> OK good to hear you're already considering it.
>
> Yes, register dumps are gone, and I am actually one of the persons who
> is missing it.
>
> I think there should still be some capability to get register snapshots
> from certain points during kernel execution, this is useful for
> debugging purposes. I don't know if it would be possible to do a
> call_usermodehelper() or something from kernel space just before wfi to
> read all (or part of) the PRCM registers, store them somewhere, and then
> decipher this data later with another tool. Any comments to this?

You should look into the omapconf tool (TI internal only currently)

This tool already has the ability to use /dev/mem to read/decipher OMAP
PM related registers.

IMO, the one thing we're still missing is the ability to take register
snapshots (like immediately before and after WFI) and somehow feed those
to omapconf for deciphering.  

Kevin

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

[Menon, Nishanth]

On Wed, Feb 15, 2012 at 09:20, Kevin Hilman <khilman@ti.com> wrote:
> Tero Kristo <t-kristo@ti.com> writes:
>
>> On Tue, 2012-02-14 at 15:52 -0800, Tony Lindgren wrote:
>>> * Kevin Hilman <khilman@ti.com> [120214 14:28]:
>>> > Tony Lindgren <tony@atomide.com> writes:
>>> >
>>> > > * Tero Kristo <t-kristo@ti.com> [120214 08:19]:
>>> > >> 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.
>>> > >
>>> > > Why don't you replace this all with a userspace tool that deciphers
>>> > > the registers for you?
>>> >
>>> > This patch isn't deciphering registers, it's just dumping usecounts, and
>>> > I think that is extremely useful to have in debugfs.
>>> >
>>> > I've already removed all the register dumping from the kernel in the
>>> > hopes that someone will write a userspace tool for that.
>>>
>>> OK good to hear you're already considering it.
>>
>> Yes, register dumps are gone, and I am actually one of the persons who
>> is missing it.
>>
>> I think there should still be some capability to get register snapshots
>> from certain points during kernel execution, this is useful for
>> debugging purposes. I don't know if it would be possible to do a
>> call_usermodehelper() or something from kernel space just before wfi to
>> read all (or part of) the PRCM registers, store them somewhere, and then
>> decipher this data later with another tool. Any comments to this?
>
> You should look into the omapconf tool (TI internal only currently)
>
> This tool already has the ability to use /dev/mem to read/decipher OMAP
> PM related registers.
>
> IMO, the one thing we're still missing is the ability to take register
> snapshots (like immediately before and after WFI) and somehow feed those
> to omapconf for deciphering.
>
Something like this perhaps?
https://github.com/nmenon/linux-omap-ti-pm/commit/3cd1994fc0df9a8e3e0be74ec3f3add3ff3aef95

Regards,
Nishanth Menon

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

[Kevin Hilman]

"Menon, Nishanth" <nm@ti.com> writes:

> On Wed, Feb 15, 2012 at 09:20, Kevin Hilman <khilman@ti.com> wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>>
>>> On Tue, 2012-02-14 at 15:52 -0800, Tony Lindgren wrote:
>>>> * Kevin Hilman <khilman@ti.com> [120214 14:28]:
>>>> > Tony Lindgren <tony@atomide.com> writes:
>>>> >
>>>> > > * Tero Kristo <t-kristo@ti.com> [120214 08:19]:
>>>> > >> 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.
>>>> > >
>>>> > > Why don't you replace this all with a userspace tool that deciphers
>>>> > > the registers for you?
>>>> >
>>>> > This patch isn't deciphering registers, it's just dumping usecounts, and
>>>> > I think that is extremely useful to have in debugfs.
>>>> >
>>>> > I've already removed all the register dumping from the kernel in the
>>>> > hopes that someone will write a userspace tool for that.
>>>>
>>>> OK good to hear you're already considering it.
>>>
>>> Yes, register dumps are gone, and I am actually one of the persons who
>>> is missing it.
>>>
>>> I think there should still be some capability to get register snapshots
>>> from certain points during kernel execution, this is useful for
>>> debugging purposes. I don't know if it would be possible to do a
>>> call_usermodehelper() or something from kernel space just before wfi to
>>> read all (or part of) the PRCM registers, store them somewhere, and then
>>> decipher this data later with another tool. Any comments to this?
>>
>> You should look into the omapconf tool (TI internal only currently)
>>
>> This tool already has the ability to use /dev/mem to read/decipher OMAP
>> PM related registers.
>>
>> IMO, the one thing we're still missing is the ability to take register
>> snapshots (like immediately before and after WFI) and somehow feed those
>> to omapconf for deciphering.
>>
> Something like this perhaps?
> https://github.com/nmenon/linux-omap-ti-pm/commit/3cd1994fc0df9a8e3e0be74ec3f3add3ff3aef95

Yes, although that is targetted pretty narrowly at suspend and only PRM
registers.

Do you then use omapconf to display the results?

Kevin

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

[Menon, Nishanth]

On Thu, Feb 16, 2012 at 11:35, Kevin Hilman <khilman@ti.com> wrote:
>
>>> IMO, the one thing we're still missing is the ability to take register
>>> snapshots (like immediately before and after WFI) and somehow feed those
>>> to omapconf for deciphering.
>>>
>> Something like this perhaps?
>> https://github.com/nmenon/linux-omap-ti-pm/commit/3cd1994fc0df9a8e3e0be74ec3f3add3ff3aef95
>
> Yes, although that is targetted pretty narrowly at suspend and only PRM
> registers.
>
> Do you then use omapconf to display the results?
on the platform we worked on, not everyone had access to omapconf, it
was just 'cat' to take the dump.
omapconf support makes sense if this is a consistent interface cross
OMAP variants, even better if a similar
framework is available cross SoCs.

Regards,
Nishanth Menon

[PATCH 0/6] arm: omap: clk/clkdm/pwrdm/voltdm changes to usecounting

[Kevin Hilman]

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

> This set is split out from a larger set earlier circulated on linux-omap
> list. The main purpose of this set is to provide useful recursive
> usecounting support for clockdomains / powerdomains / voltagedomains
> which can be used for debugging purposes and also to control certain
> chip power features once a sleep condition is reached (voltdm usecount=0).
>
> This set provides only the baseline support for the usecounting, and also
> adds a pm-debug related patch that can be used to check out stuff (patch 6),
> and should later prove rather useful for debugging. Rest of the
> functionality will be posted in separate sets.
>
> This set has been tested on omap3 beagle board with suspend / resume /
> cpuidle. Also boot tested on omap4 panda (sleep is not functional yet
> on that platform.)

This is a great series, and results in pm_debug debugfs data that's
actually useful for real debug.  Thanks!

I had a minor comment on the idle path changes, but all the other stuff
is

Acked-by: Kevin Hilman <khilman@ti.com>

Paul, this should probably merge via you (when it's ready) since the
important changes are clock and powerdomain core changes.

Kevin