Skip to contentSkip to conversation

ARM: OMAP3+: pwrdm changes for usecounting

PATCH Superseded Tero Kristo 5 participants 19 messages May 31, 2012 – Jul 12, 2012

Subsystems (5)

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v3 cover ARM: OMAP3+: pwrdm changes for usecounting

Hi,

Refreshed the patches against latest mainline kernel, and did some updates mainly proposed by Nishanth Menon <[email protected]>. Changes compared to previous version:

patch 1:
- added check against null pointer

patch 2:
- added BUG_ON in case clkdm / pwrdm usecount goes negative

patch 3:
- added BUG_ON in case voltdm usecount goes negative
- removed smartreflex enable / disable from voltdm code (this should be added in a separate patch once this is deemed safe)
- changed target_state from int to u8

patch 4:
- changed idle loop count initializations to happen synchronously with PM init, this avoids potential trouble in a case where a CPU would enter idle before usecount init for the idle has been done
- added functions pwrdm_cpu_wakeup + pwrdm_cpu_idle to be called from idle cycle

patch 7:
- changed flag name from CLKDM_NO_MANUAL_TRANS to CLKDM_SKIP_MANUAL_TRANS

patch 9:
- added as new patch, would appreciate testing for this one!

Tested on omap3 beagle + boot tested on omap4460 panda es. Previous version of this set had some issues with omap3430 devices (n900), which might have been caused by improper clock handling or security on these devices. I can't test these patches myself on n900, so if someone could do this that would be extra nice.

git tree also available:
tree: git://gitorious.org/~kristo/omap-pm/omap-pm-work.git
branch: mainline-3.4-pwrdm-changes-v3

-Tero

#1 lore raw

PATCH v3 1/9 ARM: OMAP: clk: add support for omap_clk_for_each

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

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/plat-omap/clock.c              |   33 +++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/clock.h |    2 +
 2 files changed, 35 insertions(+), 0 deletions(-)
arch/arm/plat-omap/clock.c +33 -0
index 62ec5c4..0a0775a 100644
@@ -355,6 +355,39 @@ 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;
+
+	if (!fn)
+		return -EINVAL;
+
+	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;
arch/arm/plat-omap/include/plat/clock.h +2 -0
index d0ef57c..998947e 100644
@@ -294,6 +294,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);
 extern struct clk *omap_clk_get_by_name(const char *name);
 extern int omap_clk_enable_autoidle_all(void);
Signature · 1 line
1.7.4.1
#2 lore raw

PATCH v3 2/9 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking

In reply to 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.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/clkt_iclk.c         |   21 +++++++++++++++++
 arch/arm/mach-omap2/clockdomain.c       |   38 +++++++++++++++++++++++++++++-
 arch/arm/mach-omap2/clockdomain.h       |    2 +
 arch/arm/mach-omap2/powerdomain.c       |   20 ++++++++++++++++
 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, 92 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clkt_iclk.c +21 -0
index 3d43fba..f8c2e77 100644
@@ -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 */
arch/arm/mach-omap2/clockdomain.c +36 -2
index 8664f5a..c665348 100644
@@ -907,6 +907,32 @@ 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);
+
+	BUG_ON(usecount < 0);
+
+	return usecount;
+}
+
 static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 {
 	unsigned long flags;
@@ -919,7 +945,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);
@@ -944,7 +970,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);
@@ -981,6 +1007,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);
 }
 
@@ -1007,6 +1037,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);
 }
 
arch/arm/mach-omap2/clockdomain.h +2 -0
index f7b5860..373399a 100644
@@ -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);
arch/arm/mach-omap2/powerdomain.c +20 -0
index 9611490..4800483 100644
@@ -981,6 +981,26 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)
 	return ret;
 }
 
+void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
+{
+	if (!pwrdm)
+		return;
+
+	atomic_inc(&pwrdm->usecount);
+}
+
+void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
+{
+	int val;
+
+	if (!pwrdm)
+		return;
+
+	val = atomic_dec_return(&pwrdm->usecount);
+
+	BUG_ON(val < 0);
+}
+
 int pwrdm_pre_transition(void)
 {
 	pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
arch/arm/mach-omap2/powerdomain.h +5 -0
index 8f88d65..705b983 100644
@@ -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;
@@ -215,6 +216,10 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm);
 int pwrdm_state_switch(struct powerdomain *pwrdm);
 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);
arch/arm/plat-omap/clock.c +6 -0
index 0a0775a..bf4249e 100644
@@ -282,6 +282,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);
arch/arm/plat-omap/include/plat/clock.h +2 -0
index 998947e..6fee8c3 100644
@@ -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
Signature · 1 line
1.7.4.1
#3 lore raw

PATCH v3 2/9 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking

On Thu, May 31, 2012 at 8:28 AM, Tero Kristo <[email protected]> wrote:
minor comment:

Quoted 1 line

+void pwrdm_clkdm_enable(struct powerdomain *pwrdm)

<snip>

Quoted 1 line

+void pwrdm_clkdm_disable(struct powerdomain *pwrdm)

I know the understand that rest of the code lacks kernel-doc, but at least can we ensure that the new functions does?

Regards,
Nishanth Menon

maintainer
#4 lore raw

PATCH v3 2/9 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking

Quoted 7 lines · Nishanth

On Thu, May 31, 2012 at 8:28 AM, Tero Kristo <[email protected]> wrote:
minor comment:

Quoted 1 line

+void pwrdm_clkdm_enable(struct powerdomain *pwrdm)

<snip>

Quoted 1 line

+void pwrdm_clkdm_disable(struct powerdomain *pwrdm)

I know the understand that rest of the code lacks kernel-doc,

Almost all of the powerdomain code has kerneldoc present.

Quoted 1 line

but at least can we ensure that the new functions does?

Indeed:

http://www.spinics.net/lists/arm-kernel/msg164548.html

- Paul

#5 lore raw

PATCH v3 2/9 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking

Quoted 10 lines · Paul Walmsley
Quoted 7 lines · Nishanth

On Thu, May 31, 2012 at 8:28 AM, Tero Kristo <[email protected]> wrote:
minor comment:

Quoted 1 line

+void pwrdm_clkdm_enable(struct powerdomain *pwrdm)

<snip>

Quoted 1 line

+void pwrdm_clkdm_disable(struct powerdomain *pwrdm)

I know the understand that rest of the code lacks kernel-doc,

Quoted 7 lines

Almost all of the powerdomain code has kerneldoc present.

Quoted 1 line

but at least can we ensure that the new functions does?

Indeed:

http://www.spinics.net/lists/arm-kernel/msg164548.html

Mmm yea, I'll add those for next rev.

-Tero

#6 lore raw

PATCH v3 3/9 ARM: OMAP3+: voltage: add support for voltagedomain usecounts

In reply to 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.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/powerdomain.c |    6 +++-
 arch/arm/mach-omap2/voltage.c     |   56 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/voltage.h     |   11 +++++++
 3 files changed, 72 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/powerdomain.c +5 -1
index 4800483..317bea1 100644
@@ -986,7 +986,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,6 +999,9 @@ void pwrdm_clkdm_disable(struct powerdomain *pwrdm)
 
 	val = atomic_dec_return(&pwrdm->usecount);
 
+	if (!val)
+		voltdm_pwrdm_disable(pwrdm->voltdm.ptr);
+
 	BUG_ON(val < 0);
 }
 
arch/arm/mach-omap2/voltage.c +56 -0

68 lines not on this page · raw message

arch/arm/mach-omap2/voltage.h +11 -0
index 16a1b09..c1f4ae8 100644
@@ -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;
+	u8 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,
Signature · 1 line
1.7.4.1
#7 lore raw

PATCH v3 4/9 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting

In reply to 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 all CPUs are ready to idle. Proper powerdomain usecounts are propageted to voltagedomain level also, and will allow vc callbacks to be triggered at right point of time.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/pm34xx.c      |    3 ++
 arch/arm/mach-omap2/pm44xx.c      |    3 ++
 arch/arm/mach-omap2/powerdomain.c |   54 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/powerdomain.h |    3 ++
 4 files changed, 63 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/pm34xx.c +3 -0
index a34023d..b746fe0 100644
@@ -757,6 +757,9 @@ int __init omap3_pm_init(void)
 	omap_pm_suspend = omap3_pm_suspend;
 #endif
 
+	/* Notify pwrdm usecounters about active CPU */
+	pwrdm_cpu_wakeup();
+
 	arm_pm_idle = omap3_pm_idle;
 	omap3_idle_init();
 
arch/arm/mach-omap2/pm44xx.c +3 -0
index ea24174..45eef2d 100644
@@ -206,6 +206,9 @@ int __init omap4_pm_init(void)
 	omap_pm_suspend = omap4_pm_suspend;
 #endif
 
+	/* Notify pwrdm usecounters about active CPU */
+	pwrdm_cpu_wakeup();
+
 	/* Overwrite the default cpu_do_idle() */
 	arm_pm_idle = omap_default_idle;
 
arch/arm/mach-omap2/powerdomain.c +54 -0

74 lines not on this page · raw message

arch/arm/mach-omap2/powerdomain.h +3 -0
index 705b983..ecf7d3d 100644
@@ -220,6 +220,9 @@ int pwrdm_post_transition(void);
 void pwrdm_clkdm_enable(struct powerdomain *pwrdm);
 void pwrdm_clkdm_disable(struct powerdomain *pwrdm);
 
+void pwrdm_cpu_wakeup(void);
+void pwrdm_cpu_idle(void);
+
 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);
Signature · 1 line
1.7.4.1
#8 lore raw

PATCH v3 5/9 ARM: OMAP3: set autoidle flags for a few clocks

In reply to 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.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/clock3xxx_data.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock3xxx_data.c +3 -0
index 4e1a3b0..ca0de28 100644
@@ -434,6 +434,7 @@ static struct clk dpll3_ck = {
 	.round_rate	= &omap2_dpll_round_rate,
 	.clkdm_name	= "dpll3_clkdm",
 	.recalc		= &omap3_dpll_recalc,
+	.autoidle	= true,
 };
 
 /*
@@ -617,6 +618,7 @@ static struct clk dpll4_ck = {
 	.set_rate	= &omap3_dpll4_set_rate,
 	.clkdm_name	= "dpll4_clkdm",
 	.recalc		= &omap3_dpll_recalc,
+	.autoidle	= true,
 };
 
 /*
@@ -1751,6 +1753,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 = {
Signature · 1 line
1.7.4.1
#9 lore raw

PATCH v3 5/9 ARM: OMAP3: set autoidle flags for a few clocks

Hi Tero,

Quoted 1 line · Tero Kristo

dpll3, dpll4 and sdrc_ick are controlled automatically by hardware.

sdrc_ick does not seem to have a software control to enable/disable this automatic control in hardware, so what you are doing in the patch below seems fine.
However for dpll3 and dpll4, there is indeed a software control which is enabled/disabled through omap3_dpll_allow_idle/omap3_dpll_deny_idle functions respectively. So shouldn't the 'autoidle' flag for them be handled similar to what you do in omap2_clkt_iclk_allow_idle/ omap2_clkt_iclk_deny_idle functions in your PATCH 2/9, instead of setting them to 'true' statically?

regards,
Rajendra

Quoted 39 lines

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.

Cc Paul Walmsley<[email protected]>
Cc Kevin Hilman<[email protected]>
---
  arch/arm/mach-omap2/clock3xxx_data.c |    3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock3xxx_data.c +3 -0
index 4e1a3b0..ca0de28 100644
@@ -434,6 +434,7 @@ static struct clk dpll3_ck = {
  	.round_rate	=&omap2_dpll_round_rate,
  	.clkdm_name	= "dpll3_clkdm",
  	.recalc		=&omap3_dpll_recalc,
+	.autoidle	= true,
  };
 
  /*
@@ -617,6 +618,7 @@ static struct clk dpll4_ck = {
  	.set_rate	=&omap3_dpll4_set_rate,
  	.clkdm_name	= "dpll4_clkdm",
  	.recalc		=&omap3_dpll_recalc,
+	.autoidle	= true,
  };
 
  /*
@@ -1751,6 +1753,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 = {
#10 lore raw

PATCH v3 5/9 ARM: OMAP3: set autoidle flags for a few clocks

Quoted 5 lines · Rajendra Nayak

Hi Tero,

Quoted 1 line · Tero Kristo

dpll3, dpll4 and sdrc_ick are controlled automatically by hardware.

Quoted 9 lines

sdrc_ick does not seem to have a software control to enable/disable this automatic control in hardware, so what you are doing in the patch below seems fine.
However for dpll3 and dpll4, there is indeed a software control which is enabled/disabled through omap3_dpll_allow_idle/omap3_dpll_deny_idle functions respectively. So shouldn't the 'autoidle' flag for them be handled similar to what you do in omap2_clkt_iclk_allow_idle/ omap2_clkt_iclk_deny_idle functions in your PATCH 2/9, instead of setting them to 'true' statically?

Yea, this is a good idea, and seems to work also. I'll add this change to the next rev.

-Tero

Quoted 44 lines

regards,
Rajendra

Quoted 39 lines

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.

Cc Paul Walmsley<[email protected]>
Cc Kevin Hilman<[email protected]>
---
  arch/arm/mach-omap2/clock3xxx_data.c |    3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock3xxx_data.c +3 -0
index 4e1a3b0..ca0de28 100644
@@ -434,6 +434,7 @@ static struct clk dpll3_ck = {
  	.round_rate	=&omap2_dpll_round_rate,
  	.clkdm_name	= "dpll3_clkdm",
  	.recalc		=&omap3_dpll_recalc,
+	.autoidle	= true,
  };
 
  /*
@@ -617,6 +618,7 @@ static struct clk dpll4_ck = {
  	.set_rate	=&omap3_dpll4_set_rate,
  	.clkdm_name	= "dpll4_clkdm",
  	.recalc		=&omap3_dpll_recalc,
+	.autoidle	= true,
  };
 
  /*
@@ -1751,6 +1753,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 = {
#11 lore raw

PATCH v3 6/9 ARM: OMAP: pm-debug: enhanced usecount debug support

In reply to 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.

Cc Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/pm-debug.c |  128 ++++++++++++++++++++++++++++++++++------
 arch/arm/mach-omap2/pm.h       |    6 ++
 2 files changed, 115 insertions(+), 19 deletions(-)
arch/arm/mach-omap2/pm-debug.c +109 -19
index 814bcd9..cfc46a8 100644
@@ -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);
 
arch/arm/mach-omap2/pm.h +6 -0
index 7856489..312f2de 100644
@@ -41,10 +41,16 @@ static inline int omap4_opp_init(void)
 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)
Signature · 1 line
1.7.4.1
#12 lore raw

PATCH v3 7/9 ARM: OMAP: clockdomain: add support for preventing domain transitions

In reply to 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_SKIP_MANUAL_TRANS for doing this.

---
 arch/arm/mach-omap2/clockdomain.c |    6 ++++++
 arch/arm/mach-omap2/clockdomain.h |    3 +++
 2 files changed, 9 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clockdomain.c +6 -0
index c665348..fa652b5 100644
@@ -948,6 +948,9 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 	if ((clkdm_usecount_inc(clkdm) > 1) && autodeps)
 		return 0;
 
+	if (clkdm->flags & CLKDM_SKIP_MANUAL_TRANS)
+		return 0;
+
 	spin_lock_irqsave(&clkdm->lock, flags);
 	arch_clkdm->clkdm_clk_enable(clkdm);
 	pwrdm_state_switch(clkdm->pwrdm.ptr);
@@ -973,6 +976,9 @@ static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
 	if (clkdm_usecount_dec(clkdm) > 0)
 		return 0;
 
+	if (clkdm->flags & CLKDM_SKIP_MANUAL_TRANS)
+		return 0;
+
 	spin_lock_irqsave(&clkdm->lock, flags);
 	arch_clkdm->clkdm_clk_disable(clkdm);
 	pwrdm_state_switch(clkdm->pwrdm.ptr);
arch/arm/mach-omap2/clockdomain.h +3 -0
index 373399a..35ac7ee 100644
@@ -31,12 +31,15 @@
  *
  * CLKDM_NO_AUTODEPS: Prevent "autodeps" from being added/removed from this
  *     clockdomain.  (Currently, this applies to OMAP3 clockdomains only.)
+ * CLKDM_SKIP_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_SKIP_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)
Signature · 1 line
1.7.4.1
#13 lore raw

PATCH v3 8/9 ARM: OMAP3: prevent per_clkdm from attempting manual domain transitions

In reply to Tero Kristo

Previously, PER clock domain was always enabled, as the usecounts for this domain incorrectly always showed positive value. On HW level though, the domain enters idle as it is set in HW supervised mode. Now, when the usecounts reflect real values, PER domain will attempt to enter software supervised idle, which is not supported by the hardware/kernel that well, and causes multiple problems. First of all, coming back from idle, PER domain remains idle as the wakedeps have been disabled for the domain, and this causes a crash with the GPIO code, as the resume code attempts to access domain which is not active. Just enabling the interface clocks for the GPIO does not help, as they are autoidled and don't bring the domain out of idle. Secondly, there are multiple erratas for omap3, which say that the wakedeps should be enabled for the PER domain, see e.g. errata i582 for omap3630.

---
 arch/arm/mach-omap2/clockdomains3xxx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/clockdomains3xxx_data.c +1 -1
index 6038adb..0dae4c8 100644
@@ -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_SKIP_MANUAL_TRANS,
 	.dep_bit	= OMAP3430_EN_PER_SHIFT,
 	.wkdep_srcs	= per_wkdeps,
 	.sleepdep_srcs	= per_sleepdeps,
Signature · 1 line
1.7.4.1
#14 lore raw

PATCH v3 8/9 ARM: OMAP3: prevent per_clkdm from attempting manual domain transitions

Hi Tero,

Quoted 24 lines · Tero Kristo

Previously, PER clock domain was always enabled, as the usecounts for this domain incorrectly always showed positive value. On HW level though, the domain enters idle as it is set in HW supervised mode. Now, when the usecounts reflect real values, PER domain will attempt to enter software supervised idle, which is not supported by the hardware/kernel that well, and causes multiple problems. First of all, coming back from idle, PER domain remains idle as the wakedeps have been disabled for the domain, and this causes a crash with the GPIO code, as the resume code attempts to access domain which is not active. Just enabling the interface clocks for the GPIO does not help, as they are autoidled and don't bring the domain out of idle. Secondly, there are multiple erratas for omap3, which say that the wakedeps should be enabled for the PER domain, see e.g. errata i582 for omap3630.

---
 arch/arm/mach-omap2/clockdomains3xxx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/clockdomains3xxx_data.c +0 -0
index 6038adb..0dae4c8 100644

arch/arm/mach-omap2/clockdomains3xxx_data.c static struct clockdomain usbhost_clkdm = { · line 288

Quoted 8 lines
arch/arm/mach-omap2/clockdomains3xxx_data.c +1 -1
@@ -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_SKIP_MANUAL_TRANS,
 	.dep_bit	= OMAP3430_EN_PER_SHIFT,
 	.wkdep_srcs	= per_wkdeps,
 	.sleepdep_srcs	= per_sleepdeps,

So this change is just preventing software from clearing the wakeup-deps and keeping the PER clock domain always in HW_AUTO?

Sorry for the dumb question, but what is the circumstance under which we want to clear the wakeup-deps when we disable the clock domain? I am just curious why this is being done in the first place.

Cheers
Jon

#15 lore raw

PATCH v3 8/9 ARM: OMAP3: prevent per_clkdm from attempting manual domain transitions

Quoted 22 lines · Jon Hunter

Hi Tero,

Quoted 19 lines · Tero Kristo

Previously, PER clock domain was always enabled, as the usecounts for this domain incorrectly always showed positive value. On HW level though, the domain enters idle as it is set in HW supervised mode. Now, when the usecounts reflect real values, PER domain will attempt to enter software supervised idle, which is not supported by the hardware/kernel that well, and causes multiple problems. First of all, coming back from idle, PER domain remains idle as the wakedeps have been disabled for the domain, and this causes a crash with the GPIO code, as the resume code attempts to access domain which is not active. Just enabling the interface clocks for the GPIO does not help, as they are autoidled and don't bring the domain out of idle. Secondly, there are multiple erratas for omap3, which say that the wakedeps should be enabled for the PER domain, see e.g. errata i582 for omap3630.

---
 arch/arm/mach-omap2/clockdomains3xxx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 16 lines

arch/arm/mach-omap2/clockdomains3xxx_data.c static struct clockdomain usbhost_clkdm = { · line 288

Quoted 13 lines
arch/arm/mach-omap2/clockdomains3xxx_data.c +1 -1
index 6038adb..0dae4c8 100644
@@ -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_SKIP_MANUAL_TRANS,
 	.dep_bit	= OMAP3430_EN_PER_SHIFT,
 	.wkdep_srcs	= per_wkdeps,
 	.sleepdep_srcs	= per_sleepdeps,

So this change is just preventing software from clearing the wakeup-deps and keeping the PER clock domain always in HW_AUTO?

Basically yes.

Quoted 3 lines

Sorry for the dumb question, but what is the circumstance under which we want to clear the wakeup-deps when we disable the clock domain? I am just curious why this is being done in the first place.

Not sure, maybe Paul can answer that question. Personally I don't see much point in fiddling with the wakedeps.

-Tero

#16 lore raw

PATCH v3 8/9 ARM: OMAP3: prevent per_clkdm from attempting manual domain transitions

In reply to Tero Kristo
Quoted 5 lines · Tero Kristo

Previously, PER clock domain was always enabled, as the usecounts for this domain incorrectly always showed positive value. On HW level though, the domain enters idle as it is set in HW supervised mode. Now, when the usecounts reflect real values, PER domain will attempt to enter software supervised idle, which is not supported by the

Quoted 8 lines

hardware/kernel that well, and causes multiple problems. First of all, coming back from idle, PER domain remains idle as the wakedeps have been disabled for the domain, and this causes a crash with the GPIO code, as the resume code attempts to access domain which is not active. Just enabling the interface clocks for the GPIO does not help, as they are autoidled and don't bring the domain out of idle. Secondly, there are multiple erratas for omap3, which say that the wakedeps should be enabled for the PER domain, see e.g. errata i582 for omap3630.

If PER software supervised idle is so *buggy* then shouldn't the flags just reflect that PER *does not* support software supervised idle? Using a flag which reflects that the clkdm supports both hardware and software supervised idle ('CLKDM_CAN_HWSUP_SWSUP') and then using another one to actually skip putting it in software supervised sounds a little round about, no?
Does updating the flag to 'CLKDM_CAN_HWSUP' also fix all the issues for you? without adding the additional 'CLKDM_SKIP_MANUAL_TRANS' flag.

Quoted 19 lines
---
  arch/arm/mach-omap2/clockdomains3xxx_data.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/clockdomains3xxx_data.c +1 -1
index 6038adb..0dae4c8 100644
@@ -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_SKIP_MANUAL_TRANS,
  	.dep_bit	= OMAP3430_EN_PER_SHIFT,
  	.wkdep_srcs	= per_wkdeps,
  	.sleepdep_srcs	= per_sleepdeps,
#17 lore raw

PATCH v3 8/9 ARM: OMAP3: prevent per_clkdm from attempting manual domain transitions

Quoted 15 lines · Rajendra Nayak
Quoted 5 lines · Tero Kristo

Previously, PER clock domain was always enabled, as the usecounts for this domain incorrectly always showed positive value. On HW level though, the domain enters idle as it is set in HW supervised mode. Now, when the usecounts reflect real values, PER domain will attempt to enter software supervised idle, which is not supported by the

Quoted 8 lines

hardware/kernel that well, and causes multiple problems. First of all, coming back from idle, PER domain remains idle as the wakedeps have been disabled for the domain, and this causes a crash with the GPIO code, as the resume code attempts to access domain which is not active. Just enabling the interface clocks for the GPIO does not help, as they are autoidled and don't bring the domain out of idle. Secondly, there are multiple erratas for omap3, which say that the wakedeps should be enabled for the PER domain, see e.g. errata i582 for omap3630.

Quoted 8 lines

If PER software supervised idle is so *buggy* then shouldn't the flags just reflect that PER *does not* support software supervised idle? Using a flag which reflects that the clkdm supports both hardware and software supervised idle ('CLKDM_CAN_HWSUP_SWSUP') and then using another one to actually skip putting it in software supervised sounds a little round about, no?
Does updating the flag to 'CLKDM_CAN_HWSUP' also fix all the issues for you? without adding the additional 'CLKDM_SKIP_MANUAL_TRANS' flag.

Simple CAN_HWSUP doesn't work, as the whole problem is with HWSUP mode and manual enabling / disabling of autodeps by kernel. I can rename the flag to something like CLKDM_NO_AUTODEP_DISABLE and move the checking of flag from the generic clockdomain code to omap3 clockdomain part though.

-Tero

Quoted 21 lines
Quoted 20 lines
---
  arch/arm/mach-omap2/clockdomains3xxx_data.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/clockdomains3xxx_data.c +1 -1
index 6038adb..0dae4c8 100644
@@ -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_SKIP_MANUAL_TRANS,
  	.dep_bit	= OMAP3430_EN_PER_SHIFT,
  	.wkdep_srcs	= per_wkdeps,
  	.sleepdep_srcs	= per_sleepdeps,
#18 lore raw

PATCH v3 9/9 TEMP: ARM: OMAP3: prevent dpll4 manual enable / disable + prevent core clkdm idle

In reply to Tero Kristo

DPLL4 (PER DPLL) disable can cause issues on omap3, thus prevent disable / enable by setting the clkops as core_dpll_ops. Also, prevent l3 / l4 core clkdomain manual transitions as these can cause issues also.

---
 arch/arm/mach-omap2/clock3xxx_data.c        |    2 +-
 arch/arm/mach-omap2/clockdomains3xxx_data.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/mach-omap2/clock3xxx_data.c +1 -1
index ca0de28..bbc1a36 100644
@@ -611,7 +611,7 @@ static struct dpll_data dpll4_dd_3630 __initdata = {
 
 static struct clk dpll4_ck = {
 	.name		= "dpll4_ck",
-	.ops		= &clkops_omap3_noncore_dpll_ops,
+	.ops		= &clkops_omap3_core_dpll_ops,
 	.parent		= &sys_ck,
 	.dpll_data	= &dpll4_dd,
 	.round_rate	= &omap2_dpll_round_rate,
arch/arm/mach-omap2/clockdomains3xxx_data.c +2 -2
index 0dae4c8..52bf48c 100644
@@ -232,7 +232,7 @@ static struct clockdomain d2d_clkdm = {
 static struct clockdomain core_l3_3xxx_clkdm = {
 	.name		= "core_l3_clkdm",
 	.pwrdm		= { .name = "core_pwrdm" },
-	.flags		= CLKDM_CAN_HWSUP,
+	.flags		= CLKDM_CAN_HWSUP | CLKDM_SKIP_MANUAL_TRANS,
 	.dep_bit	= OMAP3430_EN_CORE_SHIFT,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_L3_MASK,
 };
@@ -245,7 +245,7 @@ static struct clockdomain core_l3_3xxx_clkdm = {
 static struct clockdomain core_l4_3xxx_clkdm = {
 	.name		= "core_l4_clkdm",
 	.pwrdm		= { .name = "core_pwrdm" },
-	.flags		= CLKDM_CAN_HWSUP,
+	.flags		= CLKDM_CAN_HWSUP | CLKDM_SKIP_MANUAL_TRANS,
 	.dep_bit	= OMAP3430_EN_CORE_SHIFT,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_L4_MASK,
 };
Signature · 1 line
1.7.4.1