From: hvaibhav@ti.com (Vaibhav Hiremath)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints
Date: Wed, 12 Dec 2012 15:51:54 +0530 [thread overview]
Message-ID: <50C85AC2.7010203@ti.com> (raw)
In-Reply-To: <20121209012342.19716.66436.stgit@dusk.lan>
On 12/9/2012 6:53 AM, Paul Walmsley wrote:
> The atomic usecounts seem to be confusing, and are no longer needed
> since the operations that they are attached to really should take
> place under lock. Replace the atomic counters with simple integers,
> protected by the enclosing powerdomain spinlock.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> arch/arm/mach-omap2/clockdomain.c | 88 +++++++++++++++++++++++++++++++-----
> arch/arm/mach-omap2/clockdomain.h | 6 +-
> arch/arm/mach-omap2/cm3xxx.c | 6 +-
> arch/arm/mach-omap2/cminst44xx.c | 2 -
> arch/arm/mach-omap2/pm-debug.c | 6 +-
> arch/arm/mach-omap2/pm.c | 3 +
> arch/arm/mach-omap2/prm2xxx_3xxx.c | 3 +
> 7 files changed, 88 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
> index 9d5b69f..ed8ac2f 100644
> --- a/arch/arm/mach-omap2/clockdomain.c
> +++ b/arch/arm/mach-omap2/clockdomain.c
> @@ -210,7 +210,8 @@ static int _clkdm_add_wkdep(struct clockdomain *clkdm1,
> return ret;
> }
>
> - if (atomic_inc_return(&cd->wkdep_usecount) == 1) {
> + cd->wkdep_usecount++;
> + if (cd->wkdep_usecount == 1) {
> pr_debug("clockdomain: hardware will wake up %s when %s wakes up\n",
> clkdm1->name, clkdm2->name);
>
> @@ -252,7 +253,8 @@ static int _clkdm_del_wkdep(struct clockdomain *clkdm1,
> return ret;
> }
>
> - if (atomic_dec_return(&cd->wkdep_usecount) == 0) {
> + cd->wkdep_usecount--;
> + if (cd->wkdep_usecount == 0) {
> pr_debug("clockdomain: hardware will no longer wake up %s after %s wakes up\n",
> clkdm1->name, clkdm2->name);
>
> @@ -296,7 +298,8 @@ static int _clkdm_add_sleepdep(struct clockdomain *clkdm1,
> return ret;
> }
>
> - if (atomic_inc_return(&cd->sleepdep_usecount) == 1) {
> + cd->sleepdep_usecount++;
> + if (cd->sleepdep_usecount == 1) {
> pr_debug("clockdomain: will prevent %s from sleeping if %s is active\n",
> clkdm1->name, clkdm2->name);
>
> @@ -340,7 +343,8 @@ static int _clkdm_del_sleepdep(struct clockdomain *clkdm1,
> return ret;
> }
>
> - if (atomic_dec_return(&cd->sleepdep_usecount) == 0) {
> + cd->sleepdep_usecount--;
> + if (cd->sleepdep_usecount == 0) {
> pr_debug("clockdomain: will no longer prevent %s from sleeping if %s is active\n",
> clkdm1->name, clkdm2->name);
>
> @@ -567,7 +571,21 @@ struct powerdomain *clkdm_get_pwrdm(struct clockdomain *clkdm)
> */
> int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> {
> - return _clkdm_add_wkdep(clkdm1, clkdm2);
> + struct clkdm_dep *cd;
> + int ret;
> +
> + if (!clkdm1 || !clkdm2)
> + return -EINVAL;
> +
> + cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
> + if (IS_ERR(cd))
> + return PTR_ERR(cd);
> +
> + pwrdm_lock(cd->clkdm->pwrdm.ptr);
> + ret = _clkdm_add_wkdep(clkdm1, clkdm2);
> + pwrdm_unlock(cd->clkdm->pwrdm.ptr);
> +
> + return ret;
> }
>
> /**
> @@ -582,7 +600,21 @@ int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> */
> int clkdm_del_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> {
> - return _clkdm_del_wkdep(clkdm1, clkdm2);
> + struct clkdm_dep *cd;
> + int ret;
> +
> + if (!clkdm1 || !clkdm2)
> + return -EINVAL;
> +
> + cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
> + if (IS_ERR(cd))
> + return PTR_ERR(cd);
> +
> + pwrdm_lock(cd->clkdm->pwrdm.ptr);
> + ret = _clkdm_del_wkdep(clkdm1, clkdm2);
> + pwrdm_unlock(cd->clkdm->pwrdm.ptr);
> +
> + return ret;
> }
>
> /**
> @@ -620,7 +652,7 @@ int clkdm_read_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> return ret;
> }
>
> - /* XXX It's faster to return the atomic wkdep_usecount */
> + /* XXX It's faster to return the wkdep_usecount */
> return arch_clkdm->clkdm_read_wkdep(clkdm1, clkdm2);
> }
>
> @@ -659,7 +691,21 @@ int clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
> */
> int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> {
> - return _clkdm_add_sleepdep(clkdm1, clkdm2);
> + struct clkdm_dep *cd;
> + int ret;
> +
> + if (!clkdm1 || !clkdm2)
> + return -EINVAL;
> +
> + cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
> + if (IS_ERR(cd))
> + return PTR_ERR(cd);
> +
> + pwrdm_lock(cd->clkdm->pwrdm.ptr);
> + ret = _clkdm_add_sleepdep(clkdm1, clkdm2);
> + pwrdm_unlock(cd->clkdm->pwrdm.ptr);
> +
> + return ret;
> }
>
> /**
> @@ -676,7 +722,21 @@ int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> */
> int clkdm_del_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> {
> - return _clkdm_del_sleepdep(clkdm1, clkdm2);
> + struct clkdm_dep *cd;
> + int ret;
> +
> + if (!clkdm1 || !clkdm2)
> + return -EINVAL;
> +
> + cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
> + if (IS_ERR(cd))
> + return PTR_ERR(cd);
> +
> + pwrdm_lock(cd->clkdm->pwrdm.ptr);
> + ret = _clkdm_del_sleepdep(clkdm1, clkdm2);
> + pwrdm_unlock(cd->clkdm->pwrdm.ptr);
> +
> + return ret;
> }
>
> /**
> @@ -716,7 +776,7 @@ int clkdm_read_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> return ret;
> }
>
> - /* XXX It's faster to return the atomic sleepdep_usecount */
> + /* XXX It's faster to return the sleepdep_usecount */
> return arch_clkdm->clkdm_read_sleepdep(clkdm1, clkdm2);
> }
>
> @@ -1063,7 +1123,8 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
> * should be called for every clock instance or hwmod that is
> * enabled, so the clkdm can be force woken up.
> */
> - if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps) {
> + clkdm->usecount++;
> + if (clkdm->usecount > 1 && autodeps) {
> pwrdm_unlock(clkdm->pwrdm.ptr);
> return 0;
> }
> @@ -1084,13 +1145,14 @@ static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
>
> pwrdm_lock(clkdm->pwrdm.ptr);
>
> - if (atomic_read(&clkdm->usecount) == 0) {
> + if (clkdm->usecount == 0) {
> pwrdm_unlock(clkdm->pwrdm.ptr);
> WARN_ON(1); /* underflow */
> return -ERANGE;
> }
>
> - if (atomic_dec_return(&clkdm->usecount) > 0) {
> + clkdm->usecount--;
> + if (clkdm->usecount > 0) {
> pwrdm_unlock(clkdm->pwrdm.ptr);
> return 0;
> }
> diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
> index 50c3cd8..2da3765 100644
> --- a/arch/arm/mach-omap2/clockdomain.h
> +++ b/arch/arm/mach-omap2/clockdomain.h
> @@ -91,8 +91,8 @@ struct clkdm_autodep {
> struct clkdm_dep {
> const char *clkdm_name;
> struct clockdomain *clkdm;
> - atomic_t wkdep_usecount;
> - atomic_t sleepdep_usecount;
> + s16 wkdep_usecount;
> + s16 sleepdep_usecount;
> };
>
> /* Possible flags for struct clockdomain._flags */
> @@ -136,7 +136,7 @@ struct clockdomain {
> const u16 clkdm_offs;
> struct clkdm_dep *wkdep_srcs;
> struct clkdm_dep *sleepdep_srcs;
> - atomic_t usecount;
> + int usecount;
> struct list_head node;
> };
>
> diff --git a/arch/arm/mach-omap2/cm3xxx.c b/arch/arm/mach-omap2/cm3xxx.c
> index b94af4c..9061c30 100644
> --- a/arch/arm/mach-omap2/cm3xxx.c
> +++ b/arch/arm/mach-omap2/cm3xxx.c
> @@ -186,7 +186,7 @@ static int omap3xxx_clkdm_clear_all_sleepdeps(struct clockdomain *clkdm)
> continue; /* only happens if data is erroneous */
>
> mask |= 1 << cd->clkdm->dep_bit;
> - atomic_set(&cd->sleepdep_usecount, 0);
> + cd->sleepdep_usecount = 0;
> }
> omap2_cm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs,
> OMAP3430_CM_SLEEPDEP);
> @@ -209,7 +209,7 @@ static int omap3xxx_clkdm_wakeup(struct clockdomain *clkdm)
>
> static void omap3xxx_clkdm_allow_idle(struct clockdomain *clkdm)
> {
> - if (atomic_read(&clkdm->usecount) > 0)
> + if (clkdm->usecount > 0)
> clkdm_add_autodeps(clkdm);
>
> omap3xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
> @@ -221,7 +221,7 @@ static void omap3xxx_clkdm_deny_idle(struct clockdomain *clkdm)
> omap3xxx_cm_clkdm_disable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
> clkdm->clktrctrl_mask);
>
> - if (atomic_read(&clkdm->usecount) > 0)
> + if (clkdm->usecount > 0)
> clkdm_del_autodeps(clkdm);
> }
>
> diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
> index 7f9a464..f0290f5 100644
> --- a/arch/arm/mach-omap2/cminst44xx.c
> +++ b/arch/arm/mach-omap2/cminst44xx.c
> @@ -393,7 +393,7 @@ static int omap4_clkdm_clear_all_wkup_sleep_deps(struct clockdomain *clkdm)
> continue; /* only happens if data is erroneous */
>
> mask |= 1 << cd->clkdm->dep_bit;
> - atomic_set(&cd->wkdep_usecount, 0);
> + cd->wkdep_usecount = 0;
> }
>
> omap4_cminst_clear_inst_reg_bits(mask, clkdm->prcm_partition,
> diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c
> index 3cf4fdf..806a06b 100644
> --- a/arch/arm/mach-omap2/pm-debug.c
> +++ b/arch/arm/mach-omap2/pm-debug.c
> @@ -84,10 +84,8 @@ static int clkdm_dbg_show_counter(struct clockdomain *clkdm, void *user)
> 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");
> + seq_printf(s, "%s->%s (%d)\n", clkdm->name, clkdm->pwrdm.ptr->name,
> + clkdm->usecount);
>
> return 0;
> }
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index 2e2a897..6b7cb7c 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -78,11 +78,12 @@ static void __init omap2_init_processor_devices(void)
>
> int __init omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
> {
> + /* XXX The usecount test is racy */
> if ((clkdm->flags & CLKDM_CAN_ENABLE_AUTO) &&
> !(clkdm->flags & CLKDM_MISSING_IDLE_REPORTING))
> clkdm_allow_idle(clkdm);
> else if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP &&
> - atomic_read(&clkdm->usecount) == 0)
> + clkdm->usecount == 0)
> clkdm_sleep(clkdm);
> return 0;
> }
> diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
> index a3e121f..947f6ad 100644
> --- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
> +++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
> @@ -210,6 +210,7 @@ int omap2_clkdm_read_wkdep(struct clockdomain *clkdm1,
> PM_WKDEP, (1 << clkdm2->dep_bit));
> }
>
> +/* XXX Caller must hold the clkdm's powerdomain lock */
Isn't this comment applicable for all the internal api's
omap2_clkdm_clear_all_wkdeps,
omap3xxx_clkdm_add_sleepdep,
omap3xxx_clkdm_del_sleepdep,
omap3xxx_clkdm_read_sleepdep,
omap3xxx_clkdm_clear_all_sleepdeps,
> int omap2_clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
Why this function is non-static? typo??? May be I am missing something.
It was static before, but api became public by below commit -
ARM: OMAP2/3: clockdomain/PRM/CM: move the low-level clockdomain
functions into PRM/CM
Thanks,
Vaibhav
> {
> struct clkdm_dep *cd;
> @@ -221,7 +222,7 @@ int omap2_clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
>
> /* PRM accesses are slow, so minimize them */
> mask |= 1 << cd->clkdm->dep_bit;
> - atomic_set(&cd->wkdep_usecount, 0);
> + cd->wkdep_usecount = 0;
> }
>
> omap2_prm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs,
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
next prev parent reply other threads:[~2012-12-12 10:21 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
2012-12-09 1:23 ` [PATCH 02/10] ARM: OMAP2+: clockdomain: add pwrdm_state_switch() call to clkdm_sleep() Paul Walmsley
2012-12-09 1:23 ` [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings Paul Walmsley
2012-12-12 8:31 ` Vaibhav Hiremath
2012-12-13 5:41 ` Paul Walmsley
2012-12-13 5:55 ` Hiremath, Vaibhav
2012-12-13 7:29 ` Paul Walmsley
2012-12-13 9:18 ` Hiremath, Vaibhav
2012-12-12 9:28 ` Santosh Shilimkar
2012-12-09 1:23 ` [PATCH 03/10] ARM: OMAP2xxx: PM: clean up some crufty powerstate programming code Paul Walmsley
2012-12-09 1:23 ` [PATCH 04/10] ARM: OMAP2: PM/powerdomain: drop unnecessary pwrdm_wait_transition() Paul Walmsley
2012-12-09 1:23 ` [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code Paul Walmsley
2012-12-12 9:31 ` Jean Pihet
2013-01-29 20:59 ` Paul Walmsley
2012-12-12 10:21 ` Vaibhav Hiremath
2013-01-09 17:43 ` Russell King - ARM Linux
2012-12-12 10:31 ` Jean Pihet
2012-12-09 1:23 ` [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock Paul Walmsley
2012-12-12 9:41 ` Jean Pihet
2013-01-29 21:13 ` Paul Walmsley
2012-12-12 10:28 ` Jean Pihet
2012-12-09 1:23 ` [PATCH 07/10] ARM: OMAP2xxx: CM: remove autodep handling Paul Walmsley
2012-12-09 1:23 ` [PATCH 08/10] ARM: OMAP2+: clockdomain: work on wkdep/sleepdep functions Paul Walmsley
2012-12-09 1:23 ` [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints Paul Walmsley
2012-12-12 10:21 ` Vaibhav Hiremath [this message]
2012-12-26 6:31 ` Bedia, Vaibhav
2012-12-09 1:23 ` [PATCH 10/10] ARM: OMAP2+: powerdomain: fix whitespace, improve flag comments Paul Walmsley
2013-01-04 13:07 ` [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Tero Kristo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50C85AC2.7010203@ti.com \
--to=hvaibhav@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).