ARM: OMAP: pm: usecounting changes
Subsystems (5)
v4 cover ARM: OMAP: pm: usecounting changes
Hi,
Changes compared to previous version:
- added kerneldoc comments to new API functions
- added autoidle flagging support for omap3 dplls
- modified the clkdm code tweak required to fix omap3 per domain problems
* moved implementation to _clkdm_del_autodeps
* renamed the CLKDM_SKIP_MANUAL_TRANS to CLKDM_NO_AUTODEP_DISABLE
* modified comments accordingly
- ported for 3.5-rc6
- tested with omap3 beagle board, both ret / off + suspend / cpuidle
- functional branch available at:
git://gitorious.org/~kristo/omap-pm/omap-pm-work.git
branch: mainline-3.5-rc6-pwrdm-changes-v4
-Tero
PATCH v4 1/8 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.
--- 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
@@ -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
@@ -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
PATCH v4 1/8 ARM: OMAP: clk: add support for omap_clk_for_each
Quoted 2 lines · Tero Kristo
This works similarly to e.g.
pwrdm_for_each(). Needed by enhanced usecounting debug functionality that will be added to pm-debug.
OMAP clock framework has its own debugfs entry (/debug/clock) to expose usecounts for clocks, so does the COMMON clock framework (/debug/clk). Maybe there isn't one which dumps usecounts for the complete tree, instead there is one per each clock node.
I agree having a complete dump of the clock tree usecounts can be useful, but can we keep it in /debug/clock for now?
The reason I am saying this is because once we move from OMAP clock to COMMON clock, something like what 'omap_clk_for_each' does will not be possible anyway. We might have to add the support for dumping complete tree usecounts into the COMMON clock core.
Quoted 66 lines
Signed-off-by Tero Kristo<[email protected]>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@@ -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@@ -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 );
PATCH v4 1/8 ARM: OMAP: clk: add support for omap_clk_for_each
Quoted 9 lines · Rajendra Nayak
Quoted 2 lines · Tero Kristo
This works similarly to e.g.
pwrdm_for_each(). Needed by enhanced usecounting debug functionality that will be added to pm-debug.OMAP clock framework has its own debugfs entry (/debug/clock) to expose usecounts for clocks, so does the COMMON clock framework (/debug/clk). Maybe there isn't one which dumps usecounts for the complete tree, instead there is one per each clock node.
Quoted 6 lines
I agree having a complete dump of the clock tree usecounts can be useful, but can we keep it in /debug/clock for now?
The reason I am saying this is because once we move from OMAP clock to COMMON clock, something like what 'omap_clk_for_each' does will not be possible anyway. We might have to add the support for dumping complete tree usecounts into the COMMON clock core.
The main idea behind the dump was to allow dumping of whole clocktree status during failed suspend. It is not that useful as a debugfs feature (you can do the same with some scripts on userspace), but this is available as a side effect from the suspend dump.
-Tero
PATCH v4 2/8 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking
In reply to Tero KristoThis 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.
--- arch/arm/mach-omap2/clkt_iclk.c | 21 +++++++++++ arch/arm/mach-omap2/clockdomain.c | 59 +++++++++++++++++++++++++++++- arch/arm/mach-omap2/clockdomain.h | 2 + arch/arm/mach-omap2/dpll3xxx.c | 19 ++++++++++ arch/arm/mach-omap2/powerdomain.c | 35 ++++++++++++++++++ arch/arm/mach-omap2/powerdomain.h | 5 +++ arch/arm/plat-omap/clock.c | 6 +++ arch/arm/plat-omap/include/plat/clock.h | 2 + 8 files changed, 147 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clkt_iclk.c +21 -0
@@ -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 +57 -2
89 lines not on this page · raw message
arch/arm/mach-omap2/clockdomain.h +2 -0
@@ -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/dpll3xxx.c +19 -0
@@ -34,6 +34,7 @@#include "clock.h" #include "cm2xxx_3xxx.h" #include "cm-regbits-34xx.h" +#include "clockdomain.h" /* CM_AUTOIDLE_PLL*.AUTO_* bit values */ #define DPLL_AUTOIDLE_DISABLE 0x0
@@ -561,6 +562,15 @@ void omap3_dpll_allow_idle(struct clk *clk) v |=DPLL_AUTOIDLE_LOW_POWER_STOP <<__ffs (dd->autoidle_mask);__raw_writel (v, dd->autoidle_reg); +/* 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; }/**
@@ -590,6 +600,15 @@ void omap3_dpll_deny_idle(struct clk *clk) v |=DPLL_AUTOIDLE_DISABLE <<__ffs (dd->autoidle_mask);__raw_writel (v, dd->autoidle_reg); +/* 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; }/* Clock control for DPLL outputs */
arch/arm/mach-omap2/powerdomain.c +35 -0
@@ -981,6 +981,41 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)return ret; } +/** +* pwrdm_clkdm_enable - increment powerdomain usecount +* @pwrdm: struct powerdomain * +* +* Increases the usecount for a powerdomain. Called from clockdomain +* code once a clockdomain's usecount reaches zero, i.e. it is ready +* to idle. +*/ +void pwrdm_clkdm_enable (struct powerdomain *pwrdm) +{ +if (!pwrdm) +return ; + +atomic_inc (&pwrdm->usecount); +} + +/** +* pwrdm_clkdm_disable - decrease powerdomain usecount +* @pwrdm: struct powerdomain * +* +* Decreases the usecount for a powerdomain. Called from clockdomain +* code once a clockdomain becomes active. +*/ +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
@@ -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
@@ -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
@@ -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
PATCH v4 2/8 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking
Quoted 11 lines · 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<[email protected]>Cc Paul Walmsley<[email protected]>Cc Kevin Hilman<[email protected]>
Reviewed by: Rajendra Nayak <[email protected]>
Quoted 14 lines
--- arch/arm/mach-omap2/clkt_iclk.c | 21 +++++++++++ arch/arm/mach-omap2/clockdomain.c | 59 +++++++++++++++++++++++++++++- arch/arm/mach-omap2/clockdomain.h | 2 + arch/arm/mach-omap2/dpll3xxx.c | 19 ++++++++++ arch/arm/mach-omap2/powerdomain.c | 35 ++++++++++++++++++ arch/arm/mach-omap2/powerdomain.h | 5 +++ arch/arm/plat-omap/clock.c | 6 +++ arch/arm/plat-omap/include/plat/clock.h | 2 + 8 files changed, 147 insertions(+), 2 deletions(-)list_add(&clk->node,&clocks);
arch/arm/plat-omap/clock.c int clk_register(struct clk *clk) · line 290
Quoted 6 lines
arch/arm/plat-omap/clock.c +6
-0+/* +* If clock has no ops, it is handled by hardware and thus will +* idle automatically +*/ +if (clk->ops ==&clkops_null) + clk->autoidle = true;
I was a little skeptical about this when I first saw it. But I am sure you would have thought through :)
PATCH v4 2/8 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking
In reply to Tero KristoQuoted 11 lines · 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 <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>
Minor nit: please avoid using BUG_ON() which will panic the whole kernel. Proper error detection is preferred, but your usage, a WARN_ON() should suffice.
Otherwise,
Kevin
PATCH v4 2/8 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking
In reply to Tero KristoQuoted 11 lines · 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 <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>
[...]
Quoted 7 lines
/* Clock control for DPLL outputs */
arch/arm/mach-omap2/powerdomain.c +0
-0@@ -981,6 +981,41 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)return ret;
arch/arm/mach-omap2/powerdomain.c int pwrdm_state_switch(struct powerdomain *pwrdm) · line 991
Quoted 10 lines
arch/arm/mach-omap2/powerdomain.c +8
-0} +/** +* pwrdm_clkdm_enable - increment powerdomain usecount +* @pwrdm: struct powerdomain * +* +* Increases the usecount for a powerdomain. Called from clockdomain +* code once a clockdomain's usecount reaches zero, i.e. it is ready +* to idle. +*/
I think the comment here is wrong.
Quoted 5 lines
arch/arm/mach-omap2/powerdomain.c +5
-0+void pwrdm_clkdm_enable (struct powerdomain *pwrdm) +{ +if (!pwrdm) +return ; +
arch/arm/mach-omap2/powerdomain.c int pwrdm_state_switch(struct powerdomain *pwrdm) · line 1006
Quoted 10 lines
arch/arm/mach-omap2/powerdomain.c +10
-0+atomic_inc (&pwrdm->usecount); +} + +/** +* pwrdm_clkdm_disable - decrease powerdomain usecount +* @pwrdm: struct powerdomain * +* +* Decreases the usecount for a powerdomain. Called from clockdomain +* code once a clockdomain becomes active. +*/
Looks like these two comments need to be swapped?
arch/arm/mach-omap2/powerdomain.c int pwrdm_state_switch(struct powerdomain *pwrdm) · line 1018
Quoted 12 lines
arch/arm/mach-omap2/powerdomain.c +12
-0+void pwrdm_clkdm_disable (struct powerdomain *pwrdm) +{ +int val; + +if (!pwrdm) +return ; + + val =atomic_dec_return (&pwrdm->usecount); + +BUG_ON (val <0 ); +} +
Kevin
PATCH v4 2/8 ARM: OMAP3+: voltage/pwrdm/clkdm/clock add recursive usecount tracking
Quoted 45 lines · Kevin Hilman
Quoted 11 lines · 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 <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>[...]
Quoted 7 lines
/* Clock control for DPLL outputs */
arch/arm/mach-omap2/powerdomain.c +0
-0@@ -981,6 +981,41 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)return ret;arch/arm/mach-omap2/powerdomain.c int pwrdm_state_switch(struct powerdomain *pwrdm) · line 991
Quoted 10 lines
arch/arm/mach-omap2/powerdomain.c +8
-0} +/** +* pwrdm_clkdm_enable - increment powerdomain usecount +* @pwrdm: struct powerdomain * +* +* Increases the usecount for a powerdomain. Called from clockdomain +* code once a clockdomain's usecount reaches zero, i.e. it is ready +* to idle. +*/ I think the comment here is wrong.
Quoted 9 lines
arch/arm/mach-omap2/powerdomain.c +9
-0+void pwrdm_clkdm_enable (struct powerdomain *pwrdm) +{ +if (!pwrdm) +return ; + +atomic_inc (&pwrdm->usecount); +} + +/**
Quoted 8 lines
arch/arm/mach-omap2/powerdomain.c int pwrdm_state_switch(struct powerdomain *pwrdm) · line 1006
Quoted 6 lines
arch/arm/mach-omap2/powerdomain.c +6
-0+* pwrdm_clkdm_disable - decrease powerdomain usecount +* @pwrdm: struct powerdomain * +* +* Decreases the usecount for a powerdomain. Called from clockdomain +* code once a clockdomain becomes active. +*/ Looks like these two comments need to be swapped?
Ooops, that is true yes. Again my laziness in writing proper comments bites me.
-Tero
PATCH v4 3/8 ARM: OMAP3+: voltage: add support for voltagedomain usecounts
In reply to Tero KristoThese 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.
--- 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
@@ -994,7 +994,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); }/**
@@ -1013,6 +1014,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
@@ -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
PATCH v4 3/8 ARM: OMAP3+: voltage: add support for voltagedomain usecounts
Quoted 8 lines · Tero Kristo
These are updated based on powerdomain usecounts. Also added support for
voltdm->sleepandvoltdm->wakeupcalls 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<[email protected]>Cc Paul Walmsley<[email protected]>Cc Kevin Hilman<[email protected]>
I am working on a series to cleanup CPUidle, on top of this series, where I add similar callbacks at the pwrdm level too. Something like pwrdm->power_on and pwrdm->power_down. Helps get rid of a lot of code stuffed inside idle/suspend.
Quoted 150 lines
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@@ -994,7 +994,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); }/** @@ -1013,6 +1014,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@@ -38,6 +38,7 @@#include "voltage.h" #include "powerdomain.h" +#include "smartreflex.h" #include "vc.h" #include "vp.h" @@ -340,6 +341,61 @@ 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. +*/ +void voltdm_pwrdm_enable (struct voltagedomain *voltdm) +{ +if (!voltdm) +return ; + +if (atomic_inc_return (&voltdm->usecount) ==1 ) { +if (voltdm->wakeup) + voltdm->wakeup (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, and calculate the overall target +* state for the voltagedomain. +*/ +void voltdm_pwrdm_disable (struct voltagedomain *voltdm) +{ +u8 target_state =PWRDM_POWER_OFF ; +int state; +struct powerdomain *pwrdm; +int val; + +if (!voltdm) +return ; + + val =atomic_dec_return (&voltdm->usecount); + +BUG_ON (val<0 ); + +if (val ==0 ) { +/* 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 * arch/arm/mach-omap2/voltage.h +11
-0@@ -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,
PATCH v4 3/8 ARM: OMAP3+: voltage: add support for voltagedomain usecounts
In reply to Tero KristoTero,
Quoted 79 lines
-----Original Message-----
From: linux-omap-owner at vger.kernel.org [mailto:linux-omap- owner at vger.kernel.org] On Behalf Of Tero Kristo
Sent: Friday, July 13, 2012 9:20 AM
To: linux-omap at vger.kernel.org; paul at pwsan.com; khilman at ti.com
Cc: linux-arm-kernel at lists.infradead.org
Subject: [PATCHv4 3/8] ARM: OMAP3+: voltage: add support for voltagedomain usecountsThese are updated based on powerdomain usecounts. Also added support for
voltdm->sleepandvoltdm->wakeupcalls 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 <[email protected]>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- renamed from arch/arm/mach-omap2/powerdomain.c +0
-0
omap2/powerdomain.c
index 68bdf36..3b4b15d 100644arch/arm/mach-omap2/powerdomain.c +2
-1@@ -994,7 +994,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); }/**
arch/arm/mach-omap2/powerdomain.c +3
-0@@ -1013,6 +1014,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- renamed from arch/arm/mach-omap2/voltage.c +0
-0
omap2/voltage.c
index 4dc60e8..8c6439b 100644arch/arm/mach-omap2/voltage.c +5
-0@@ -38,6 +38,7 @@#include "voltage.h" #include "powerdomain.h" +#include "smartreflex.h" #include "vc.h" #include "vp.h" @@ -340,6 +341,61 @@ 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->wakeupcallback for this domain.
+ */
arch/arm/mach-omap2/voltage.c int voltdm_add_pwrdm(struct voltagedomain *voltdm, struct powerdomain *pwrdm) · line 360
Quoted 10 lines
arch/arm/mach-omap2/voltage.c +10
-0+void voltdm_pwrdm_enable (struct voltagedomain *voltdm) +{ +if (!voltdm) +return ; + +if (atomic_inc_return (&voltdm->usecount) ==1 ) { +if (voltdm->wakeup) + voltdm->wakeup (voltdm); + } +}
I think these usecounting functions are prone to race conditions and need to be protected using spin_lock or something like that (not just the usecounter). Otherwise right sequence will not be followed when multiple modules try to idle and enable simultaneously. You can see this issue when 2 cpus enter idle independently.
Regards
Vishwa
Quoted 104 lines
arch/arm/mach-omap2/voltage.c +6
-0+ +/** +* 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->sleepcallback, and calculate the overall target
+ * state for the voltagedomain.
+ */arch/arm/mach-omap2/voltage.c +17
-0+void voltdm_pwrdm_disable (struct voltagedomain *voltdm) +{ +u8 target_state =PWRDM_POWER_OFF ; +int state; +struct powerdomain *pwrdm; +int val; + +if (!voltdm) +return ; + + val =atomic_dec_return (&voltdm->usecount); + +BUG_ON (val <0 ); + +if (val ==0 ) { +/* Determine target state for voltdm */ + list_for_each_entry(pwrdm, &voltdm->pwrdm_list,
voltdm_node) {arch/arm/mach-omap2/voltage.c +11
-0+ 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 * arch/arm/mach- renamed from arch/arm/mach-omap2/voltage.h +0
-0
omap2/voltage.h
index 16a1b09..c1f4ae8 100644arch/arm/mach-omap2/voltage.h +0
-0@@ -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 supportedarch/arm/mach-omap2/voltage.h +7
-0@@ -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,
--
1.7.4.1--
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
PATCH v4 3/8 ARM: OMAP3+: voltage: add support for voltagedomain usecounts
Quoted 88 lines · Vishwanath Sripathy
Tero,
Quoted 86 lines
-----Original Message-----
From: linux-omap-owner at vger.kernel.org [mailto:linux-omap- owner at vger.kernel.org] On Behalf Of Tero Kristo
Sent: Friday, July 13, 2012 9:20 AM
To: linux-omap at vger.kernel.org; paul at pwsan.com; khilman at ti.com
Cc: linux-arm-kernel at lists.infradead.org
Subject: [PATCHv4 3/8] ARM: OMAP3+: voltage: add support for voltagedomain usecountsThese are updated based on powerdomain usecounts. Also added support for
voltdm->sleepandvoltdm->wakeupcalls 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 <[email protected]>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- renamed from arch/arm/mach-omap2/powerdomain.c +0
-0
omap2/powerdomain.c
index 68bdf36..3b4b15d 100644arch/arm/mach-omap2/powerdomain.c +2
-1@@ -994,7 +994,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); }/**
arch/arm/mach-omap2/powerdomain.c +3
-0@@ -1013,6 +1014,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- renamed from arch/arm/mach-omap2/voltage.c +0
-0
omap2/voltage.c
index 4dc60e8..8c6439b 100644arch/arm/mach-omap2/voltage.c +5
-0@@ -38,6 +38,7 @@#include "voltage.h" #include "powerdomain.h" +#include "smartreflex.h" #include "vc.h" #include "vp.h" @@ -340,6 +341,61 @@ 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->wakeupcallback for this domain.
+ */arch/arm/mach-omap2/voltage.c +7
-0+void voltdm_pwrdm_enable (struct voltagedomain *voltdm) +{ +if (!voltdm) +return ; + +if (atomic_inc_return (&voltdm->usecount) ==1 ) { +if (voltdm->wakeup)
Quoted 8 lines
arch/arm/mach-omap2/voltage.c int voltdm_add_pwrdm(struct voltagedomain *voltdm, struct powerdomain *pwrdm) · line 360
Quoted 3 lines
arch/arm/mach-omap2/voltage.c +3
-0+ voltdm->wakeup (voltdm); + } +}I think these usecounting functions are prone to race conditions and need to be protected using
spin_lockor something like that (not just the usecounter). Otherwise right sequence will not be followed when multiple modules try to idle and enable simultaneously. You can see this issue when 2 cpus enter idle independently.
Yes, you are right... something like this needs to be added for SMP once individual cpuidle is in place. I'll add this kind of fix for the next version.
-Tero
PATCH v4 4/8 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting
In reply to Tero Kristompu / 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.
--- arch/arm/mach-omap2/pm34xx.c | 3 ++ arch/arm/mach-omap2/pm44xx.c | 3 ++ arch/arm/mach-omap2/powerdomain.c | 64 +++++++++++++++++++++++++++++++++++++ arch/arm/mach-omap2/powerdomain.h | 3 ++ 4 files changed, 73 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/pm34xx.c +3 -0
@@ -758,6 +758,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
@@ -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 +64 -0
84 lines not on this page · raw message
arch/arm/mach-omap2/powerdomain.h +3 -0
@@ -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
PATCH v4 4/8 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting
Quoted 22 lines · 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.
Signed-off-by Tero Kristo<[email protected]>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 | 64 +++++++++++++++++++++++++++++++++++++ arch/arm/mach-omap2/powerdomain.h | 3 ++ 4 files changed, 73 insertions(+), 0 deletions(-)arch/arm/mach-omap2/pm34xx.c +0
-0
arch/arm/mach-omap2/pm34xx.c int __init omap3_pm_init(void) · line 763
Quoted 6 lines
arch/arm/mach-omap2/pm34xx.c +3
-0@@ -758,6 +758,9 @@ int __init omap3_pm_init(void) omap_pm_suspend = omap3_pm_suspend;#endif +/* Notify pwrdm usecounters about active CPU */ +pwrdm_cpu_wakeup (); +
These internally increment/decrement usecount for MPU and CORE but the name pwrdm_cpu_wakeup/idle seems somewhat misleading. But I don't know either what would be a better name, so..
PATCH v4 4/8 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting
In reply to Tero KristoQuoted 10 lines · 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.
Signed-off-by Tero Kristo <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>
IMO, the idea is fine, but I'm not crazy about the implementation inpowerdomain.c, which is meant for pwrdm generic code. In particular,
I'm not crazy about the pwrdm lookups in powerdomain.c.
Since pm<soc>.c already has references to mpu_pwrdm and core_pwrdm, why not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.c
Also, the changelog should be a bit more specific about why CORE powerdomain is also handled here when most of the code only talks about the CPU.
Kevin
PATCH v4 4/8 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting
Quoted 13 lines · Kevin Hilman
Quoted 10 lines · 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.
Signed-off-by Tero Kristo <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>
Quoted 6 lines
IMO, the idea is fine, but I'm not crazy about the implementation in
powerdomain.c, which is meant for pwrdm generic code. In particular,
I'm not crazy about the pwrdm lookups inpowerdomain.c.Since pm<soc>.c already has references to
mpu_pwrdmandcore_pwrdm, why not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.c
I think this was how the patch was in some earlier rev but I thought I'd try to be more clever with this. :) I can revert the implementation back to this.
Quoted 3 lines
Also, the changelog should be a bit more specific about why CORE powerdomain is also handled here when most of the code only talks about the CPU.
Yea, I'll add some beef to this also.
-Tero
PATCH v4 4/8 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting
Hi Tero,
Quoted 18 lines · Tero Kristo
Quoted 17 lines · Kevin Hilman
Quoted 10 lines · 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.
Signed-off-by Tero Kristo <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>IMO, the idea is fine, but I'm not crazy about the implementation in
powerdomain.c, which is meant for pwrdm generic code. In particular,
I'm not crazy about the pwrdm lookups inpowerdomain.c.
Quoted 6 lines
Quoted 2 lines
Since pm<soc>.c already has references to
mpu_pwrdmandcore_pwrdm, why not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.cI think this was how the patch was in some earlier rev but I thought I'd try to be more clever with this. :) I can revert the implementation back to this.
Furthermore after the changes in pre/post transitions [1], some more checks will be needed to identify the transitions on the mpu and core power domains.
Regards,
Jean
Quoted 13 lines
Quoted 4 lines
Also, the changelog should be a bit more specific about why CORE powerdomain is also handled here when most of the code only talks about the CPU.
Yea, I'll add some beef to this also.
-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
PATCH v4 4/8 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting
Quoted 27 lines · Jean Pihet
Hi Tero,
Quoted 18 lines · Tero Kristo
Quoted 17 lines · Kevin Hilman
Quoted 10 lines · 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.
Signed-off-by Tero Kristo <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>IMO, the idea is fine, but I'm not crazy about the implementation in
powerdomain.c, which is meant for pwrdm generic code. In particular,
I'm not crazy about the pwrdm lookups inpowerdomain.c.Quoted 6 lines
Quoted 2 lines
Since pm<soc>.c already has references to
mpu_pwrdmandcore_pwrdm, why not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.cI think this was how the patch was in some earlier rev but I thought I'd try to be more clever with this. :) I can revert the implementation back to this.
Quoted 8 lines
Furthermore after the changes in pre/post transitions [1], some more checks will be needed to identify the transitions on the mpu and core power domains.
Regards,
Jean
Hi Kevin,
What is the latest status regarding this one, seeing the patch mentioned got reverted due to problems? Should I do some changes for this or not?
I can look at moving the code away from the generic powerdomain.c at least, but is there anything else?
-Tero
Quoted 14 lines
Quoted 14 lines
Quoted 5 lines
Also, the changelog should be a bit more specific about why CORE powerdomain is also handled here when most of the code only talks about the CPU.
Yea, I'll add some beef to this also.
-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
PATCH v4 4/8 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting
Quoted 34 lines · Tero Kristo
Quoted 33 lines · Jean Pihet
Hi Tero,
Quoted 18 lines · Tero Kristo
Quoted 17 lines · Kevin Hilman
Quoted 10 lines · 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.
Signed-off-by Tero Kristo <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>IMO, the idea is fine, but I'm not crazy about the implementation in
powerdomain.c, which is meant for pwrdm generic code. In particular,
I'm not crazy about the pwrdm lookups inpowerdomain.c.Quoted 6 lines
Quoted 2 lines
Since pm<soc>.c already has references to
mpu_pwrdmandcore_pwrdm, why not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.cI think this was how the patch was in some earlier rev but I thought I'd try to be more clever with this. :) I can revert the implementation back to this.
Furthermore after the changes in pre/post transitions [1], some more checks will be needed to identify the transitions on the mpu and core power domains.
Quoted 7 lines
Quoted 2 lines
Regards,
JeanHi Kevin,
What is the latest status regarding this one, seeing the patch mentioned got reverted due to problems? Should I do some changes for this or not?
No, using latest minline should be fine.
Quoted 2 lines
I can look at moving the code away from the generic
powerdomain.cat least, but is there anything else?
Nothing else I can think of (but my vacation has purged my brain of most of the details here, so I may be forgetting something.)
Kevin
PATCH v4 4/8 ARM: OMAP3: add manual control for mpu / core pwrdm usecounting
In reply to Kevin HilmanQuoted 17 lines · Kevin Hilman
Quoted 10 lines · 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.
Signed-off-by Tero Kristo <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>IMO, the idea is fine, but I'm not crazy about the implementation in
powerdomain.c, which is meant for pwrdm generic code. In particular,
I'm not crazy about the pwrdm lookups inpowerdomain.c.
Quoted 6 lines
Since pm<soc>.c already has references to
mpu_pwrdmandcore_pwrdm, why not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.cAlso, the changelog should be a bit more specific about why CORE powerdomain is also handled here when most of the code only talks about the CPU.
After playing with this some more, and doing a test hack of my idea above, along with the per-pwrdm pre/post changes now in mainline, I thinks this series should go one step further.
Basically, we should be able to get rid of the calls to prdm_pre|post_transition that are outside the powerdomain code. With the addition of pwrdm_clkdm_enable|disable, it seems that the pre|post transition calls should be called directly from pwrdm_clkdm_enable|disable when the usecount transitions to/from zero.
Kevin
PATCH v4 5/8 ARM: OMAP3: set autoidle flag for sdrc_ick
In reply to Tero Kristosdrc_ick doesn't have autoidle flag on HW, but is always automatically idled. Thus mark the autoidle flag statically as true for it to reflect hardware behavior. The clock will no longer show as active in usecount dumps and will allow the voltdm->sleep / wakeup calls to work properly.
--- arch/arm/mach-omap2/clock3xxx_data.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock3xxx_data.c +1 -0
@@ -1751,6 +1751,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
PATCH v4 5/8 ARM: OMAP3: set autoidle flag for sdrc_ick
Quoted 8 lines · Tero Kristo
sdrc_ickdoesn't have autoidle flag on HW, but is always automatically idled. Thus mark the autoidle flag statically as true for it to reflect hardware behavior. The clock will no longer show as active in usecount dumps and will allow thevoltdm->sleep/ wakeup calls to work properly.Signed-off-by Tero Kristo<[email protected]>Cc Paul Walmsley<[email protected]>Cc Kevin Hilman<[email protected]>
Quoted 16 lines
--- arch/arm/mach-omap2/clock3xxx_data.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)arch/arm/mach-omap2/clock3xxx_data.c +1
-0@@ -1751,6 +1751,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 = {
PATCH v4 6/8 ARM: OMAP: pm-debug: enhanced usecount debug support
In reply to Tero KristoVoltdm, 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.
--- 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
@@ -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
@@ -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
PATCH v4 6/8 ARM: OMAP: pm-debug: enhanced usecount debug support
Quoted 4 lines · 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.
The only comment I had on this patch was what I also said in response to PATCH 1/8 about keeping the clock usecounts separate if possible. voltdm, pwrdm, clkdm and hwmod are OMAP specific frameworks (and so is clock framework for now) but sooner than later we would move to using COMMON clock framework and then supporting the clock usecounts in /debug/pm_debug/usecount would not be possible.
Quoted 204 lines
This patch also provices a way to do printk dumps from kernel code, by calling the
pm_dbg_dump_Xfunctions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.Signed-off-by Tero Kristo<[email protected]>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@@ -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@@ -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 )
PATCH v4 6/8 ARM: OMAP: pm-debug: enhanced usecount debug support
Quoted 6 lines · Rajendra Nayak
Quoted 4 lines · 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.
Quoted 6 lines
The only comment I had on this patch was what I also said in response to PATCH 1/8 about keeping the clock usecounts separate if possible. voltdm, pwrdm, clkdm and hwmod are OMAP specific frameworks (and so is clock framework for now) but sooner than later we would move to using COMMON clock framework and then supporting the clock usecounts in /debug/pm_debug/usecount would not be possible.
Well, I guess we can move these patches to a separate debug set for now in that case, and people who want to use them can do that.
Quoted 5 lines
Quoted 5 lines
This patch also provices a way to do printk dumps from kernel code, by calling the
pm_dbg_dump_Xfunctions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.
^
|
This part is the main motivation for this patch imo.
-Tero
PATCH v4 6/8 ARM: OMAP: pm-debug: enhanced usecount debug support
Quoted 7 lines · Tero Kristo
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
This patch also provices a way to do printk dumps from kernel code, by calling the
pm_dbg_dump_Xfunctions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.^
|
This part is the main motivation for this patch imo.
ah, okay. I missed that part completely.
PATCH v4 6/8 ARM: OMAP: pm-debug: enhanced usecount debug support
In reply to Tero KristoQuoted 5 lines · 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.
Quoted 7 lines
This patch also provices a way to do printk dumps from kernel code, by calling the
pm_dbg_dump_Xfunctions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.Signed-off-by Tero Kristo <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>
I think we should separate out the debug stuff as a separate patch set.
Where I want to go with PM debug is away from in-kernel debug prints and towards using tracepoints. We have tracepoints in the clock code that correspond to usecounts, but we need them in the clkdm, pwrdm and voltdm code as well. With that, you can use userspace tools (perf, ftrace) to trace a problem (e.g. failed suspend) and see voltdm/pwrdm/clkdm/clks are on when they shouldn't be.
Kevin
PATCH v4 6/8 ARM: OMAP: pm-debug: enhanced usecount debug support
Quoted 15 lines · Kevin Hilman
Quoted 5 lines · 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.
Quoted 7 lines
This patch also provices a way to do printk dumps from kernel code, by calling the
pm_dbg_dump_Xfunctions. The plan is to call these functions once an error condition is detected, e.g. failed suspend.Signed-off-by Tero Kristo <[email protected]>Cc Paul Walmsley <[email protected]>Cc Kevin Hilman <[email protected]>
Quoted 8 lines
I think we should separate out the debug stuff as a separate patch set.
Where I want to go with PM debug is away from in-kernel debug prints and towards using tracepoints. We have tracepoints in the clock code that correspond to usecounts, but we need them in the clkdm, pwrdm and voltdm code as well. With that, you can use userspace tools (perf, ftrace) to trace a problem (e.g. failed suspend) and see voltdm/pwrdm/clkdm/clks are on when they shouldn't be.
Yes, I can split out the debug stuff from this set for next rev. I'll also take a look at the tracepoints if they can be used reasonably for this purpose. We can add tracepoints for failed suspend for each powerdomain, but are tracepoints a good way to do this kind of recursive dumps (well, we could maybe register a trace event recursively from clkdm + clk code if a powerdomain fails to transition.)
-Tero
PATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
In reply to Tero KristoSome clockdomains bug out if their autodeps are deleted before idle. This happens namely with OMAP3 PER domain, it will bug out if it doesn't have wakedeps enabled when it enters off-mode. This patch adds support for new flag 'CLKDM_NO_AUTODEP_DISABLE' which does this.
--- arch/arm/mach-omap2/clockdomain.c | 3 +++ arch/arm/mach-omap2/clockdomain.h | 4 ++++ 2 files changed, 7 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clockdomain.c +3 -0
@@ -201,6 +201,9 @@ void _clkdm_del_autodeps(struct clockdomain *clkdm)if (!autodeps || clkdm->flags &CLKDM_NO_AUTODEPS )return ; +if (clkdm->flags &CLKDM_NO_AUTODEP_DISABLE ) +return ; +for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {if (IS_ERR (autodep->clkdm.ptr))continue ;
arch/arm/mach-omap2/clockdomain.h +4 -0
@@ -31,12 +31,16 @@* * CLKDM_NO_AUTODEPS: Prevent "autodeps" from being added/removed from this * clockdomain. (Currently, this applies to OMAP3 clockdomains only.) +* CLKDM_NO_AUTODEP_DISABLE: Prevent clockdomain code from deleting autodeps. +* Needed for PER domain on omap3, as it will bug out with off-mode if +* wakedeps are removed. */ #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_AUTODEP_DISABLE (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
PATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
Hi Tero,
Quoted 4 lines · Tero Kristo
Some clockdomains bug out if their autodeps are deleted before idle. This happens namely with OMAP3 PER domain, it will bug out if it doesn't have wakedeps enabled when it enters off-mode. This patch adds support for new flag '
CLKDM_NO_AUTODEP_DISABLE' which does this.
I had one more thought on how we could handle this (without adding a new flag :-))
How about marking OMAP3 PER with a CLKDM_NO_AUTODEPS (already existing flag) and setting a sleep/wakeup dependency of OMAP3 PER with MPU and IVA one time sometime during late PM init. Because thats what we intent to do, which is have a sleep/wakeup dependency set *always* and never try to remove it, right?
regards,
Rajendra
Quoted 42 lines
Signed-off-by Tero Kristo<[email protected]>--- arch/arm/mach-omap2/clockdomain.c | 3 +++ arch/arm/mach-omap2/clockdomain.h | 4 ++++ 2 files changed, 7 insertions(+), 0 deletions(-)arch/arm/mach-omap2/clockdomain.c +3
-0@@ -201,6 +201,9 @@ void _clkdm_del_autodeps(struct clockdomain *clkdm)if (!autodeps || clkdm->flags&CLKDM_NO_AUTODEPS )return ; +if (clkdm->flags&CLKDM_NO_AUTODEP_DISABLE ) +return ; +for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {if (IS_ERR (autodep->clkdm.ptr))continue ;arch/arm/mach-omap2/clockdomain.h +4
-0@@ -31,12 +31,16 @@* * CLKDM_NO_AUTODEPS: Prevent "autodeps" from being added/removed from this * clockdomain. (Currently, this applies to OMAP3 clockdomains only.) +* CLKDM_NO_AUTODEP_DISABLE: Prevent clockdomain code from deleting autodeps. +* Needed for PER domain on omap3, as it will bug out with off-mode if +* wakedeps are removed. */ #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_AUTODEP_DISABLE (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)
PATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
Quoted 8 lines · Rajendra Nayak
Hi Tero,
Quoted 4 lines · Tero Kristo
Some clockdomains bug out if their autodeps are deleted before idle. This happens namely with OMAP3 PER domain, it will bug out if it doesn't have wakedeps enabled when it enters off-mode. This patch adds support for new flag '
CLKDM_NO_AUTODEP_DISABLE' which does this.
Quoted 7 lines
I had one more thought on how we could handle this (without adding a new flag :-))
How about marking OMAP3 PER with aCLKDM_NO_AUTODEPS(already existing flag) and setting a sleep/wakeup dependency of OMAP3 PER with MPU and IVA one time sometime during late PM init. Because thats what we intent to do, which is have a sleep/wakeup dependency set *always* and never try to remove it, right?
I did some extra investigation on this, sorry for the delay. What is enough, is to just add a wakedep from wkup_clkdm to per_clkdm, as the wakedeps have usecounting so once this is done, the autodep handling can't remove the wakedep.
Anyway, it also looks like this fix is no longer needed with the latest kernel, something has changed with the gpio code / or latencies and it doesn't crash anymore. Thus, it looks like patches 7 & 8 can be dropped from this set for now. This is the behavior with beagleboard at least, if someone can verify this with some other omap3 hw that would be nice.
The underlying issue still remains, we have errata i582 which doesn't have any workarounds in the kernel yet. We should probably resurrect something like this:
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg38834.html
... or just pull the part which adds the dynamic wakedep add / remove for the per domain when attempting per OFF.
-Tero
Quoted 47 lines
regards,
RajendraQuoted 42 lines
Signed-off-by Tero Kristo<[email protected]>--- arch/arm/mach-omap2/clockdomain.c | 3 +++ arch/arm/mach-omap2/clockdomain.h | 4 ++++ 2 files changed, 7 insertions(+), 0 deletions(-)arch/arm/mach-omap2/clockdomain.c +3
-0@@ -201,6 +201,9 @@ void _clkdm_del_autodeps(struct clockdomain *clkdm)if (!autodeps || clkdm->flags&CLKDM_NO_AUTODEPS )return ; +if (clkdm->flags&CLKDM_NO_AUTODEP_DISABLE ) +return ; +for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {if (IS_ERR (autodep->clkdm.ptr))continue ;arch/arm/mach-omap2/clockdomain.h +4
-0@@ -31,12 +31,16 @@* * CLKDM_NO_AUTODEPS: Prevent "autodeps" from being added/removed from this * clockdomain. (Currently, this applies to OMAP3 clockdomains only.) +* CLKDM_NO_AUTODEP_DISABLE: Prevent clockdomain code from deleting autodeps. +* Needed for PER domain on omap3, as it will bug out with off-mode if +* wakedeps are removed. */ #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_AUTODEP_DISABLE (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)
Paul Walmsley
maintainerPATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
Hi
Quoted 8 lines · Tero Kristo
The underlying issue still remains, we have errata i582 which doesn't have any workarounds in the kernel yet. We should probably resurrect something like this:
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg38834.html
... or just pull the part which adds the dynamic wakedep add / remove for the per domain when attempting per OFF.
Yep looks like Kevin had some comments on that patch that no one followed up on. Guess we need to figure out who will have time to revise and update it.
A few comments on that patch.
1. Looks to me like the patch needs to be split into several smaller patches. One patch should deal with the serial changes. Another should deal with the pm34xx.c changes.
2. Looks like we also need a patch to run the McBSP2/3 sidetone loopback test. Then the pm34xx.c test code would be something like:
if (omap_uart_test_erratum_i582() || omap_mcbsp_test_erratum_i582()) {
pr_err("%s: erratum i582 encountered; applying workaround\n", __func__); .. etc.
}
3. When the erratum is encountered, shouldn't the code schedule a CORE OFF transition to occur at the earliest possible moment, rather than just emitting a message?
4. The bug is only a problem when the PER serial ports/McBSP sidetone devices are in use, right? So if those devices aren't in use then we can defer the device tests until right before one of those devices is brought into use, no?
5. There needs to be a better way of determining if a device is affected by this than by testing uart->num. Adding a hwmod dev_attr flag would be my first inclination.
- Paul
PATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
In reply to Tero KristoQuoted 5 lines · Tero Kristo
Anyway, it also looks like this fix is no longer needed with the latest kernel, something has changed with the gpio code / or latencies and it doesn't crash anymore. Thus, it looks like patches 7& 8 can be dropped from this set for now. This is the behavior with beagleboard at least, if someone can verify this with some other omap3 hw that would be nice.
I can test it on a omap3 SDP. What do you want me to test?
PATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
Quoted 8 lines · Rajendra Nayak
Quoted 5 lines · Tero Kristo
Anyway, it also looks like this fix is no longer needed with the latest kernel, something has changed with the gpio code / or latencies and it doesn't crash anymore. Thus, it looks like patches 7& 8 can be dropped from this set for now. This is the behavior with beagleboard at least, if someone can verify this with some other omap3 hw that would be nice.
I can test it on a omap3 SDP. What do you want me to test?
Just try suspend + cpuidle with and without off-mode enabled and see if there are any problems. I've usually seen problems with off-mode myself.
-Tero
PATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
Quoted 8 lines · Tero Kristo
Quoted 7 lines · Rajendra Nayak
Quoted 5 lines · Tero Kristo
Anyway, it also looks like this fix is no longer needed with the latest kernel, something has changed with the gpio code / or latencies and it
doesn't crash anymore. Thus, it looks like patches 7& 8 can be dropped
from this set for now. This is the behavior with beagleboard at least, if someone can verify this with some other omap3 hw that would be nice.
Quoted 4 lines
Quoted 1 line
I can test it on a omap3 SDP. What do you want me to test?
Just try suspend + cpuidle with and without off-mode enabled and see if there are any problems. I've usually seen problems with off-mode myself.
So I just knocked off the last 2 patches from 'mainline-3.5-rc6-pwrdm- changes-v4' and tested on my 3430 SDP.
I was able to hit RET and OFF in both suspend and cpuidle. Did not see any issues.
Quoted 4 lines
-Tero
PATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
Quoted 11 lines · Rajendra Nayak
Quoted 10 lines · Tero Kristo
Quoted 8 lines · Rajendra Nayak
Quoted 5 lines · Tero Kristo
Anyway, it also looks like this fix is no longer needed with the latest kernel, something has changed with the gpio code / or latencies and it
doesn't crash anymore. Thus, it looks like patches 7& 8 can be dropped
from this set for now. This is the behavior with beagleboard at least, if someone can verify this with some other omap3 hw that would be nice.I can test it on a omap3 SDP. What do you want me to test?
Quoted 8 lines
Quoted 2 lines
Just try suspend + cpuidle with and without off-mode enabled and see if there are any problems. I've usually seen problems with off-mode myself.
So I just knocked off the last 2 patches from 'mainline-3.5-rc6-pwrdm- changes-v4' and tested on my 3430 SDP.
I was able to hit RET and OFF in both suspend and cpuidle. Did not see any issues.
Okay thanks, so it looks pretty safe then.
-Tero
PATCH v4 7/8 ARM: OMAP: clockdomain: add support for preventing autodep delete
In reply to Rajendra NayakQuoted 11 lines · Rajendra Nayak
Quoted 10 lines · Tero Kristo
Quoted 8 lines · Rajendra Nayak
Quoted 5 lines · Tero Kristo
Anyway, it also looks like this fix is no longer needed with the latest kernel, something has changed with the gpio code / or latencies and it
doesn't crash anymore. Thus, it looks like patches 7& 8 can be dropped
from this set for now. This is the behavior with beagleboard at least, if someone can verify this with some other omap3 hw that would be nice.I can test it on a omap3 SDP. What do you want me to test?
Quoted 9 lines
Quoted 2 lines
Just try suspend + cpuidle with and without off-mode enabled and see if there are any problems. I've usually seen problems with off-mode myself.
So I just knocked off the last 2 patches from 'mainline-3.5-rc6-pwrdm- changes-v4' and tested on my 3430 SDP.
I was able to hit RET and OFF in both suspend and cpuidle. Did not see any issues.
FYI...
I tested this series without the last 2 patches on top of v3.5 and suspend/resume hangs on 3430/n900, 3530/Overo, 3730/OveroSTORM, but suspend/resume and cpuidle tests for retention and idle seem to work fine on 3730/Beagle-xM.
It works fine on all the platforms with the last 2 applied (note 6/8 needs a minor update to apply cleanly to mainline v3.5.)
Kevin
PATCH v4 8/8 ARM: OMAP3: do not delete per_clkdm autodeps during idle
In reply to Tero KristoPreviously, 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 be put to HWSUP sleep mode, which means its autodeps are deleted. Removing wakedeps for PER domain will cause 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
@@ -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_AUTODEP_DISABLE , .dep_bit =OMAP3430_EN_PER_SHIFT , .wkdep_srcs = per_wkdeps, .sleepdep_srcs = per_sleepdeps,
Signature · 1 line
1.7.4.1
Paul Walmsley
maintainerPATCH v4 8/8 ARM: OMAP3: do not delete per_clkdm autodeps during idle
Hi Tero,
just looking at the usecounting series to see what is mergeable and noticed this:
Quoted 3 lines · Tero Kristo
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.
Erratum i582 mentions that a wakeup dependency needs to exist between PER and WKUP such that PER will wake when CORE_L3 does. Our autodeps would not add this dependency; they just would attempt to add a wakeup dependency between PER and MPU/IVA2. So we need a different mechanism. What do you think about the following patch to handle part of the i582 workaround?
Also, do you have any of the other errata information handy, so we can track those down also? I'm pretty sure we need to enable the wakeup dependency between PER and CORE also for OMAP3, but can't recall the reference.
- Paul
From: Paul Walmsley <[email protected]>
Date: Tue, 18 Sep 2012 16:02:38 -0600
Subject: [PATCH] ARM: OMAP36xx: PM: apply part of the erratum i582 workaround
On OMAP36xx chips with ES < 1.2, if the PER powerdomain goes to OSWR or OFF while CORE stays at CSWR or ON, or if, upon chip wakeup from OSWR or OFF, the CORE powerdomain goes ON before PER, the UART3/4 FIFOs and McBSP2/3 SIDETONE memories will be unusable. This is erratum i582 in the OMAP36xx Silicon Errata document.
This patch implements one of several parts of the workaround: the addition of the wakeup dependency between the PER and WKUP clockdomains, such that PER will wake up at the same time CORE_L3 does.
This is not a complete workaround. For it to be complete:
1. the PER powerdomain's next power state must not be set to OSWR or OFF if the CORE powerdomain's next power state is set to CSWR or ON;
2. the UART3/4 FIFO and McBSP2/3 SIDETONE loopback tests should be run if the LASTPOWERSTATEENTERED bits for PER and CORE indicate that PER went OFF while CORE stayed on. If loopback tests fail, then those devices will be unusable until PER and CORE can undergo a transition from ON to OSWR/OFF and back ON.
--- arch/arm/mach-omap2/pm.h | 1 + arch/arm/mach-omap2/pm34xx.c | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-)
arch/arm/mach-omap2/pm.h +1 -0
@@ -91,6 +91,7 @@ extern void omap3_save_scratchpad_contents(void);#define PM_RTA_ERRATUM_i608 (1 << 0)#define PM_SDRC_WAKEUP_ERRATUM_i583 (1 << 1) +#define PM_PER_MEMORIES_ERRATUM_i582 (1 << 2)#if defined(CONFIG_PM ) && defined(CONFIG_ARCH_OMAP3 )extern u16 pm34xx_errata;
arch/arm/mach-omap2/pm34xx.c +23 -1
@@ -651,7 +651,8 @@ static void __init pm_errata_configure(void)/* Enable the l2 cache toggling in sleep logic */ enable_omap3630_toggle_l2_on_restore ();if (omap_rev () <OMAP3630_REV_ES1_2 ) - pm34xx_errata |= PM_SDRC_WAKEUP_ERRATUM_i583; + pm34xx_errata |= (PM_SDRC_WAKEUP_ERRATUM_i583 | + PM_PER_ERRATUM_i582); } }
@@ -726,6 +727,27 @@ int __init omap3_pm_init(void)if (IS_PM34XX_ERRATUM (PM_RTA_ERRATUM_i608))omap3630_ctrl_disable_rta (); +/* +* The UART3/4 FIFO and the sidetone memory in McBSP2/3 are +* not correctly reset when the PER powerdomain comes back +* from OFF or OSWR when the CORE powerdomain is kept active. +* See OMAP36xx Erratum i582 "PER Domain reset issue after +* Domain-OFF/OSWR Wakeup". This wakeup dependency is not a +* complete workaround. The kernel must also prevent the PER +* powerdomain from going to OSWR/OFF while the CORE +* powerdomain is not going to OSWR/OFF. And if PER last +* power state was off while CORE last power state was ON, the +* UART3/4 and McBSP2/3 SIDETONE devices need to run a +* self-test using their loopback tests; if that fails, those +* devices are unusable until the PER/CORE can complete a transition +* from ON to OSWR/OFF and then back to ON. +* +* XXX Technically this workaround is only needed if off-mode +* or OSWR is enabled. +*/ +if (IS_PM34XX_ERRATUM (PM_PER_MEMORIES_ERRATUM_i582)) +clkdm_add_wkdep (per_clkdm, wkup_clkdm); +clkdm_add_wkdep (neon_clkdm, mpu_clkdm);if (omap_type () !=OMAP2_DEVICE_TYPE_GP ) { omap3_secure_ram_storage =
Signature · 1 line
1.7.10.4
Paul Walmsley
maintainerPATCH v4 8/8 ARM: OMAP3: do not delete per_clkdm autodeps during idle
Quoted 2 lines · Paul Walmsley
What do you think about the following patch to handle part of the i582 workaround?
That patch was an older version; here's one that builds and boots - sorry about that. It passed the basic PM tests here on 3730ES1.0 Beagle XM and 3730ES1.2 EVM.
Kevin, care to ack it if you're happy with it? We still need to get the other parts of the i582 workaround into place, of course...
- Paul
From: Paul Walmsley <[email protected]>
Date: Tue, 18 Sep 2012 16:02:38 -0600
Subject: [PATCH] ARM: OMAP36xx: PM: apply part of the erratum i582 workaround
On OMAP36xx chips with ES < 1.2, if the PER powerdomain goes to OSWR or OFF while CORE stays at CSWR or ON, or if, upon chip wakeup from OSWR or OFF, the CORE powerdomain goes ON before PER, the UART3/4 FIFOs and McBSP2/3 SIDETONE memories will be unusable. This is erratum i582 in the OMAP36xx Silicon Errata document.
This patch implements one of several parts of the workaround: the addition of the wakeup dependency between the PER and WKUP clockdomains, such that PER will wake up at the same time CORE_L3 does.
This is not a complete workaround. For it to be complete:
1. the PER powerdomain's next power state must not be set to OSWR or OFF if the CORE powerdomain's next power state is set to CSWR or ON;
2. the UART3/4 FIFO and McBSP2/3 SIDETONE loopback tests should be run if the LASTPOWERSTATEENTERED bits for PER and CORE indicate that PER went OFF while CORE stayed on. If loopback tests fail, then those devices will be unusable until PER and CORE can undergo a transition from ON to OSWR/OFF and back ON.
--- arch/arm/mach-omap2/pm.h | 1 + arch/arm/mach-omap2/pm34xx.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/pm.h +1 -0
@@ -91,6 +91,7 @@ extern void omap3_save_scratchpad_contents(void);#define PM_RTA_ERRATUM_i608 (1 << 0)#define PM_SDRC_WAKEUP_ERRATUM_i583 (1 << 1) +#define PM_PER_MEMORIES_ERRATUM_i582 (1 << 2)#if defined(CONFIG_PM ) && defined(CONFIG_ARCH_OMAP3 )extern u16 pm34xx_errata;
arch/arm/mach-omap2/pm34xx.c +26 -2
@@ -651,14 +651,15 @@ static void __init pm_errata_configure(void)/* Enable the l2 cache toggling in sleep logic */ enable_omap3630_toggle_l2_on_restore ();if (omap_rev () <OMAP3630_REV_ES1_2 ) - pm34xx_errata |= PM_SDRC_WAKEUP_ERRATUM_i583; + pm34xx_errata |= (PM_SDRC_WAKEUP_ERRATUM_i583 | + PM_PER_MEMORIES_ERRATUM_i582); } }int __init omap3_pm_init (void ) {struct power_state *pwrst, *tmp; -struct clockdomain *neon_clkdm, *mpu_clkdm; +struct clockdomain *neon_clkdm, *mpu_clkdm, *per_clkdm, *wkup_clkdm;int ret;if (!omap3_has_io_chain_ctrl ())
@@ -710,6 +711,8 @@ int __init omap3_pm_init(void) neon_clkdm =clkdm_lookup ("neon_clkdm" ); mpu_clkdm =clkdm_lookup ("mpu_clkdm" ); + per_clkdm =clkdm_lookup ("per_clkdm" ); + wkup_clkdm =clkdm_lookup ("wkup_clkdm" );#ifdef CONFIG_SUSPEND omap_pm_suspend = omap3_pm_suspend;
@@ -726,6 +729,27 @@ int __init omap3_pm_init(void)if (IS_PM34XX_ERRATUM (PM_RTA_ERRATUM_i608))omap3630_ctrl_disable_rta (); +/* +* The UART3/4 FIFO and the sidetone memory in McBSP2/3 are +* not correctly reset when the PER powerdomain comes back +* from OFF or OSWR when the CORE powerdomain is kept active. +* See OMAP36xx Erratum i582 "PER Domain reset issue after +* Domain-OFF/OSWR Wakeup". This wakeup dependency is not a +* complete workaround. The kernel must also prevent the PER +* powerdomain from going to OSWR/OFF while the CORE +* powerdomain is not going to OSWR/OFF. And if PER last +* power state was off while CORE last power state was ON, the +* UART3/4 and McBSP2/3 SIDETONE devices need to run a +* self-test using their loopback tests; if that fails, those +* devices are unusable until the PER/CORE can complete a transition +* from ON to OSWR/OFF and then back to ON. +* +* XXX Technically this workaround is only needed if off-mode +* or OSWR is enabled. +*/ +if (IS_PM34XX_ERRATUM (PM_PER_MEMORIES_ERRATUM_i582)) +clkdm_add_wkdep (per_clkdm, wkup_clkdm); +clkdm_add_wkdep (neon_clkdm, mpu_clkdm);if (omap_type () !=OMAP2_DEVICE_TYPE_GP ) { omap3_secure_ram_storage =
Signature · 1 line
1.7.10.4
PATCH v4 8/8 ARM: OMAP3: do not delete per_clkdm autodeps during idle
Quoted 42 lines · Paul Walmsley
Quoted 2 lines · Paul Walmsley
What do you think about the following patch to handle part of the i582 workaround?
That patch was an older version; here's one that builds and boots - sorry about that. It passed the basic PM tests here on 3730ES1.0 Beagle XM and 3730ES1.2 EVM.
Kevin, care to ack it if you're happy with it? We still need to get the other parts of the i582 workaround into place, of course...
- Paul
From: Paul Walmsley <[email protected]>
Date: Tue, 18 Sep 2012 16:02:38 -0600
Subject: [PATCH] ARM: OMAP36xx: PM: apply part of the erratum i582 workaroundOn OMAP36xx chips with ES < 1.2, if the PER powerdomain goes to OSWR or OFF while CORE stays at CSWR or ON, or if, upon chip wakeup from OSWR or OFF, the CORE powerdomain goes ON before PER, the UART3/4 FIFOs and McBSP2/3 SIDETONE memories will be unusable. This is erratum i582 in the OMAP36xx Silicon Errata document.
This patch implements one of several parts of the workaround: the addition of the wakeup dependency between the PER and WKUP clockdomains, such that PER will wake up at the same time
CORE_L3does.This is not a complete workaround. For it to be complete:
1. the PER powerdomain's next power state must not be set to OSWR or OFF if the CORE powerdomain's next power state is set to CSWR or ON;
2. the UART3/4 FIFO and McBSP2/3 SIDETONE loopback tests should be run if the LASTPOWERSTATEENTERED bits for PER and CORE indicate that PER went OFF while CORE stayed on. If loopback tests fail, then those devices will be unusable until PER and CORE can undergo a transition from ON to OSWR/OFF and back ON.
Quoted 3 lines
Signed-off-by Paul Walmsley <[email protected]>Cc Tero Kristo <[email protected]>Cc Kevin Hilman <[email protected]>
Paul Walmsley
maintainerPATCH v4 8/8 ARM: OMAP3: do not delete per_clkdm autodeps during idle
Hi Kevin,
Here's an updated version of this one, with the erratum coverage expanded to include OMAP34xx/35xx.
I think this one can replace Tero's "[PATCHv6 06/11] ARM: OMAP:
clockdomain: add support for preventing autodep delete" and "[PATCHv6 07/11] ARM: OMAP3: do not delete per_clkdm autodeps during idle". Tero, please let us know if you feel otherwise.
The patch seems to pass testing on 37xx. Was not able to really test it on 35xx due to PM regressions in v3.7-rc1.
- Paul
From: Paul Walmsley <[email protected]>
Date: Tue, 16 Oct 2012 00:08:53 -0600
Subject: [PATCH] ARM: OMAP3: PM: apply part of the erratum i582 workaround
On OMAP34xx/35xx, and OMAP36xx chips with ES < 1.2, if the PER powerdomain goes to OSWR or OFF while CORE stays at CSWR or ON, or if, upon chip wakeup from OSWR or OFF, the CORE powerdomain goes ON before PER, the UART3/4 FIFOs and McBSP2/3 SIDETONE memories will be unusable. This is erratum i582 in the OMAP36xx Silicon Errata document.
This patch implements one of several parts of the workaround: the addition of the wakeup dependency between the PER and WKUP clockdomains, such that PER will wake up at the same time CORE_L3 does.
This is not a complete workaround. For it to be complete:
1. the PER powerdomain's next power state must not be set to OSWR or OFF if the CORE powerdomain's next power state is set to CSWR or ON;
2. the UART3/4 FIFO and McBSP2/3 SIDETONE loopback tests should be run if the LASTPOWERSTATEENTERED bits for PER and CORE indicate that PER went OFF while CORE stayed on. If loopback tests fail, then those devices will be unusable until PER and CORE can undergo a transition from ON to OSWR/OFF and back ON.
--- arch/arm/mach-omap2/pm.h | 1 + arch/arm/mach-omap2/pm34xx.c | 30 ++++++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/pm.h +1 -0
@@ -91,6 +91,7 @@ extern void omap3_save_scratchpad_contents(void);#define PM_RTA_ERRATUM_i608 (1 << 0)#define PM_SDRC_WAKEUP_ERRATUM_i583 (1 << 1) +#define PM_PER_MEMORIES_ERRATUM_i582 (1 << 2)#if defined(CONFIG_PM ) && defined(CONFIG_ARCH_OMAP3 )extern u16 pm34xx_errata;
arch/arm/mach-omap2/pm34xx.c +28 -2
@@ -651,14 +651,17 @@ static void __init pm_errata_configure(void)/* Enable the l2 cache toggling in sleep logic */ enable_omap3630_toggle_l2_on_restore ();if (omap_rev () <OMAP3630_REV_ES1_2 ) - pm34xx_errata |= PM_SDRC_WAKEUP_ERRATUM_i583; + pm34xx_errata |= (PM_SDRC_WAKEUP_ERRATUM_i583 | + PM_PER_MEMORIES_ERRATUM_i582); + }else if (cpu_is_omap34xx ()) { + pm34xx_errata |= PM_PER_MEMORIES_ERRATUM_i582; } }int __init omap3_pm_init (void ) {struct power_state *pwrst, *tmp; -struct clockdomain *neon_clkdm, *mpu_clkdm; +struct clockdomain *neon_clkdm, *mpu_clkdm, *per_clkdm, *wkup_clkdm;int ret;if (!omap3_has_io_chain_ctrl ())
@@ -710,6 +713,8 @@ int __init omap3_pm_init(void) neon_clkdm =clkdm_lookup ("neon_clkdm" ); mpu_clkdm =clkdm_lookup ("mpu_clkdm" ); + per_clkdm =clkdm_lookup ("per_clkdm" ); + wkup_clkdm =clkdm_lookup ("wkup_clkdm" );#ifdef CONFIG_SUSPEND omap_pm_suspend = omap3_pm_suspend;
@@ -726,6 +731,27 @@ int __init omap3_pm_init(void)if (IS_PM34XX_ERRATUM (PM_RTA_ERRATUM_i608))omap3630_ctrl_disable_rta (); +/* +* The UART3/4 FIFO and the sidetone memory in McBSP2/3 are +* not correctly reset when the PER powerdomain comes back +* from OFF or OSWR when the CORE powerdomain is kept active. +* See OMAP36xx Erratum i582 "PER Domain reset issue after +* Domain-OFF/OSWR Wakeup". This wakeup dependency is not a +* complete workaround. The kernel must also prevent the PER +* powerdomain from going to OSWR/OFF while the CORE +* powerdomain is not going to OSWR/OFF. And if PER last +* power state was off while CORE last power state was ON, the +* UART3/4 and McBSP2/3 SIDETONE devices need to run a +* self-test using their loopback tests; if that fails, those +* devices are unusable until the PER/CORE can complete a transition +* from ON to OSWR/OFF and then back to ON. +* +* XXX Technically this workaround is only needed if off-mode +* or OSWR is enabled. +*/ +if (IS_PM34XX_ERRATUM (PM_PER_MEMORIES_ERRATUM_i582)) +clkdm_add_wkdep (per_clkdm, wkup_clkdm); +clkdm_add_wkdep (neon_clkdm, mpu_clkdm);if (omap_type () !=OMAP2_DEVICE_TYPE_GP ) { omap3_secure_ram_storage =
Signature · 1 line
1.7.10.4
PATCH v4 8/8 ARM: OMAP3: do not delete per_clkdm autodeps during idle
Quoted 5 lines · Paul Walmsley
Hi Kevin,
Here's an updated version of this one, with the erratum coverage expanded to include OMAP34xx/35xx.
Quoted 7 lines
I think this one can replace Tero's "[PATCHv6 06/11] ARM: OMAP:
clockdomain: add support for preventing autodep delete" and "[PATCHv6 07/11] ARM: OMAP3: do not deleteper_clkdmautodeps during idle". Tero, please let us know if you feel otherwise.The patch seems to pass testing on 37xx. Was not able to really test it on 35xx due to PM regressions in v3.7-rc1.
With workarounds for some of the other regressions, this passes ret/off idle/suspend tests on 3530/Overo, 3730/OveroSTORM and 3730/Beagle-xM.
Queueing for v3.7-rc.
Thanks,
Kevin
PATCH v4 8/8 ARM: OMAP3: do not delete per_clkdm autodeps during idle
In reply to Paul WalmsleyHi Paul,
Quoted 9 lines · Paul Walmsley
Hi Kevin,
Here's an updated version of this one, with the erratum coverage expanded to include OMAP34xx/35xx.
I think this one can replace Tero's "[PATCHv6 06/11] ARM: OMAP:
clockdomain: add support for preventing autodep delete" and "[PATCHv6 07/11] ARM: OMAP3: do not deleteper_clkdmautodeps during idle". Tero, please let us know if you feel otherwise.
Yes, these are no longer necessary, just tested the set.
No other changes seem to be necessary for the set except trivial rebase. Would you like me to re-send the pwrdm-changes set or...?
-Tero
Quoted 116 lines
The patch seems to pass testing on 37xx. Was not able to really test it on 35xx due to PM regressions in v3.7-rc1.
- Paul
From: Paul Walmsley <[email protected]>
Date: Tue, 16 Oct 2012 00:08:53 -0600
Subject: [PATCH] ARM: OMAP3: PM: apply part of the erratum i582 workaroundOn OMAP34xx/35xx, and OMAP36xx chips with ES < 1.2, if the PER powerdomain goes to OSWR or OFF while CORE stays at CSWR or ON, or if, upon chip wakeup from OSWR or OFF, the CORE powerdomain goes ON before PER, the UART3/4 FIFOs and McBSP2/3 SIDETONE memories will be unusable. This is erratum i582 in the OMAP36xx Silicon Errata document.
This patch implements one of several parts of the workaround: the addition of the wakeup dependency between the PER and WKUP clockdomains, such that PER will wake up at the same time
CORE_L3does.This is not a complete workaround. For it to be complete:
1. the PER powerdomain's next power state must not be set to OSWR or OFF if the CORE powerdomain's next power state is set to CSWR or ON;
2. the UART3/4 FIFO and McBSP2/3 SIDETONE loopback tests should be run if the LASTPOWERSTATEENTERED bits for PER and CORE indicate that PER went OFF while CORE stayed on. If loopback tests fail, then those devices will be unusable until PER and CORE can undergo a transition from ON to OSWR/OFF and back ON.
Signed-off-by Paul Walmsley <[email protected]>Cc Tero Kristo <[email protected]>Cc Kevin Hilman <[email protected]>--- arch/arm/mach-omap2/pm.h | 1 + arch/arm/mach-omap2/pm34xx.c | 30 ++++++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-)arch/arm/mach-omap2/pm.h +1
-0@@ -91,6 +91,7 @@ extern void omap3_save_scratchpad_contents(void);#define PM_RTA_ERRATUM_i608 (1 << 0)#define PM_SDRC_WAKEUP_ERRATUM_i583 (1 << 1) +#define PM_PER_MEMORIES_ERRATUM_i582 (1 << 2)#if defined(CONFIG_PM ) && defined(CONFIG_ARCH_OMAP3 )extern u16 pm34xx_errata;arch/arm/mach-omap2/pm34xx.c +28
-2@@ -651,14 +651,17 @@ static void __init pm_errata_configure(void)/* Enable the l2 cache toggling in sleep logic */ enable_omap3630_toggle_l2_on_restore ();if (omap_rev () <OMAP3630_REV_ES1_2 ) - pm34xx_errata |= PM_SDRC_WAKEUP_ERRATUM_i583; + pm34xx_errata |= (PM_SDRC_WAKEUP_ERRATUM_i583 | + PM_PER_MEMORIES_ERRATUM_i582); + }else if (cpu_is_omap34xx ()) { + pm34xx_errata |= PM_PER_MEMORIES_ERRATUM_i582; } }int __init omap3_pm_init (void ) {struct power_state *pwrst, *tmp; -struct clockdomain *neon_clkdm, *mpu_clkdm; +struct clockdomain *neon_clkdm, *mpu_clkdm, *per_clkdm, *wkup_clkdm;int ret;if (!omap3_has_io_chain_ctrl ())@@ -710,6 +713,8 @@ int __init omap3_pm_init(void) neon_clkdm =clkdm_lookup ("neon_clkdm" ); mpu_clkdm =clkdm_lookup ("mpu_clkdm" ); + per_clkdm =clkdm_lookup ("per_clkdm" ); + wkup_clkdm =clkdm_lookup ("wkup_clkdm" );#ifdef CONFIG_SUSPEND omap_pm_suspend = omap3_pm_suspend;@@ -726,6 +731,27 @@ int __init omap3_pm_init(void)if (IS_PM34XX_ERRATUM (PM_RTA_ERRATUM_i608))omap3630_ctrl_disable_rta (); +/* +* The UART3/4 FIFO and the sidetone memory in McBSP2/3 are +* not correctly reset when the PER powerdomain comes back +* from OFF or OSWR when the CORE powerdomain is kept active. +* See OMAP36xx Erratum i582 "PER Domain reset issue after +* Domain-OFF/OSWR Wakeup". This wakeup dependency is not a +* complete workaround. The kernel must also prevent the PER +* powerdomain from going to OSWR/OFF while the CORE +* powerdomain is not going to OSWR/OFF. And if PER last +* power state was off while CORE last power state was ON, the +* UART3/4 and McBSP2/3 SIDETONE devices need to run a +* self-test using their loopback tests; if that fails, those +* devices are unusable until the PER/CORE can complete a transition +* from ON to OSWR/OFF and then back to ON. +* +* XXX Technically this workaround is only needed if off-mode +* or OSWR is enabled. +*/ +if (IS_PM34XX_ERRATUM (PM_PER_MEMORIES_ERRATUM_i582)) +clkdm_add_wkdep (per_clkdm, wkup_clkdm); +clkdm_add_wkdep (neon_clkdm, mpu_clkdm);if (omap_type () !=OMAP2_DEVICE_TYPE_GP ) { omap3_secure_ram_storage =
PATCH v4 8/8 ARM: OMAP3: do not delete per_clkdm autodeps during idle
In reply to Paul WalmsleyQuoted 11 lines · Paul Walmsley
Hi Tero,
just looking at the usecounting series to see what is mergeable and noticed this:
Quoted 3 lines · Tero Kristo
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 6 lines
Erratum i582 mentions that a wakeup dependency needs to exist between PER and WKUP such that PER will wake when
CORE_L3does. Our autodeps would not add this dependency; they just would attempt to add a wakeup dependency between PER and MPU/IVA2. So we need a different mechanism. What do you think about the following patch to handle part of the i582 workaround?
Yes, that looks good to me, someone will need to test it though.
Quoted 5 lines
Also, do you have any of the other errata information handy, so we can track those down also? I'm pretty sure we need to enable the wakeup dependency between PER and CORE also for OMAP3, but can't recall the reference.
GPIO errata i467 has a workaround which involves wakedeps. i582 is valid for omap3430 also. I am not able to find anything else regarding wakedeps right now, at least nothing regarding per vs core.
-Tero
Quoted 93 lines
- Paul
From: Paul Walmsley <[email protected]>
Date: Tue, 18 Sep 2012 16:02:38 -0600
Subject: [PATCH] ARM: OMAP36xx: PM: apply part of the erratum i582 workaroundOn OMAP36xx chips with ES < 1.2, if the PER powerdomain goes to OSWR or OFF while CORE stays at CSWR or ON, or if, upon chip wakeup from OSWR or OFF, the CORE powerdomain goes ON before PER, the UART3/4 FIFOs and McBSP2/3 SIDETONE memories will be unusable. This is erratum i582 in the OMAP36xx Silicon Errata document.
This patch implements one of several parts of the workaround: the addition of the wakeup dependency between the PER and WKUP clockdomains, such that PER will wake up at the same time
CORE_L3does.This is not a complete workaround. For it to be complete:
1. the PER powerdomain's next power state must not be set to OSWR or OFF if the CORE powerdomain's next power state is set to CSWR or ON;
2. the UART3/4 FIFO and McBSP2/3 SIDETONE loopback tests should be run if the LASTPOWERSTATEENTERED bits for PER and CORE indicate that PER went OFF while CORE stayed on. If loopback tests fail, then those devices will be unusable until PER and CORE can undergo a transition from ON to OSWR/OFF and back ON.
Signed-off-by Paul Walmsley <[email protected]>Cc Tero Kristo <[email protected]>Cc Kevin Hilman <[email protected]>--- arch/arm/mach-omap2/pm.h | 1 + arch/arm/mach-omap2/pm34xx.c | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-)arch/arm/mach-omap2/pm.h +1
-0@@ -91,6 +91,7 @@ extern void omap3_save_scratchpad_contents(void);#define PM_RTA_ERRATUM_i608 (1 << 0)#define PM_SDRC_WAKEUP_ERRATUM_i583 (1 << 1) +#define PM_PER_MEMORIES_ERRATUM_i582 (1 << 2)#if defined(CONFIG_PM ) && defined(CONFIG_ARCH_OMAP3 )extern u16 pm34xx_errata;arch/arm/mach-omap2/pm34xx.c +23
-1@@ -651,7 +651,8 @@ static void __init pm_errata_configure(void)/* Enable the l2 cache toggling in sleep logic */ enable_omap3630_toggle_l2_on_restore ();if (omap_rev () <OMAP3630_REV_ES1_2 ) - pm34xx_errata |= PM_SDRC_WAKEUP_ERRATUM_i583; + pm34xx_errata |= (PM_SDRC_WAKEUP_ERRATUM_i583 | + PM_PER_ERRATUM_i582); } }@@ -726,6 +727,27 @@ int __init omap3_pm_init(void)if (IS_PM34XX_ERRATUM (PM_RTA_ERRATUM_i608))omap3630_ctrl_disable_rta (); +/* +* The UART3/4 FIFO and the sidetone memory in McBSP2/3 are +* not correctly reset when the PER powerdomain comes back +* from OFF or OSWR when the CORE powerdomain is kept active. +* See OMAP36xx Erratum i582 "PER Domain reset issue after +* Domain-OFF/OSWR Wakeup". This wakeup dependency is not a +* complete workaround. The kernel must also prevent the PER +* powerdomain from going to OSWR/OFF while the CORE +* powerdomain is not going to OSWR/OFF. And if PER last +* power state was off while CORE last power state was ON, the +* UART3/4 and McBSP2/3 SIDETONE devices need to run a +* self-test using their loopback tests; if that fails, those +* devices are unusable until the PER/CORE can complete a transition +* from ON to OSWR/OFF and then back to ON. +* +* XXX Technically this workaround is only needed if off-mode +* or OSWR is enabled. +*/ +if (IS_PM34XX_ERRATUM (PM_PER_MEMORIES_ERRATUM_i582)) +clkdm_add_wkdep (per_clkdm, wkup_clkdm); +clkdm_add_wkdep (neon_clkdm, mpu_clkdm);if (omap_type () !=OMAP2_DEVICE_TYPE_GP ) { omap3_secure_ram_storage =