[PATCH 3/3] OMAP4: clockdomain: Add wkup/sleep dependency support
Rajendra Nayak
rnayak at ti.com
Thu Feb 10 23:04:58 EST 2011
Hi Kevin/Paul,
> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Friday, February 11, 2011 7:36 AM
> To: Rajendra Nayak
> Cc: linux-omap at vger.kernel.org; khilman at ti.com; b-cousson at ti.com;
linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 3/3] OMAP4: clockdomain: Add wkup/sleep dependency
support
>
> Hi
>
> On Mon, 7 Feb 2011, Rajendra Nayak wrote:
>
> > Add OMAP4 platform specific implementation to support clkdm
> > wkup and sleep dependencies a.k.a static dependencies.
> >
> > Signed-off-by: Rajendra Nayak <rnayak at ti.com>
>
> This patch had a similar bug to the OMAP2/3 variants; updated patch
> follows -
Thanks for fixing this. I have one comment below...
>
>
> - Paul
>
> From: Rajendra Nayak <rnayak at ti.com>
> Date: Thu, 10 Feb 2011 18:58:04 -0700
> Subject: [PATCH] OMAP4: clockdomain: Add wkup/sleep dependency support
>
> Add OMAP4 platform specific implementation to support clkdm
> wkup and sleep dependencies a.k.a static dependencies.
>
> Signed-off-by: Rajendra Nayak <rnayak at ti.com>
> [paul at pwsan.com: removed comment about PRM; zero-prefixed STATICDEP
> register offset; fixed loop termination condition in
> omap4_clkdm_clear_all_wkup_sleep_deps(); thanks to Kevin Hilman for
finding
> and helping fix this bug]
> Cc: Kevin Hilman <khilman at deeprootsystems.com>
> Signed-off-by: Paul Walmsley <paul at pwsan.com>
> ---
> arch/arm/mach-omap2/clockdomain44xx.c | 58
+++++++++++++++++++++++++++++++++
> arch/arm/mach-omap2/cm44xx.h | 1 +
> 2 files changed, 59 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clockdomain44xx.c
b/arch/arm/mach-omap2/clockdomain44xx.c
> index c0ccc47..e6fd8da 100644
> --- a/arch/arm/mach-omap2/clockdomain44xx.c
> +++ b/arch/arm/mach-omap2/clockdomain44xx.c
> @@ -12,8 +12,58 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/kernel.h>
> #include "clockdomain.h"
> #include "cminst44xx.h"
> +#include "cm44xx.h"
> +
> +static int omap4_clkdm_add_wkup_sleep_dep(struct clockdomain *clkdm1,
> + struct clockdomain *clkdm2)
> +{
> + omap4_cminst_set_inst_reg_bits((1 << clkdm2->dep_bit),
> + clkdm1->prcm_partition,
> + clkdm1->cm_inst,
clkdm1->clkdm_offs +
> + OMAP4_CM_STATICDEP);
> + return 0;
> +}
> +
> +static int omap4_clkdm_del_wkup_sleep_dep(struct clockdomain *clkdm1,
> + struct clockdomain *clkdm2)
> +{
> + omap4_cminst_clear_inst_reg_bits((1 << clkdm2->dep_bit),
> + clkdm1->prcm_partition,
> + clkdm1->cm_inst,
clkdm1->clkdm_offs +
> + OMAP4_CM_STATICDEP);
> + return 0;
> +}
> +
> +static int omap4_clkdm_read_wkup_sleep_dep(struct clockdomain *clkdm1,
> + struct clockdomain *clkdm2)
> +{
> + return omap4_cminst_read_inst_reg_bits(clkdm1->prcm_partition,
> + clkdm1->cm_inst,
clkdm1->clkdm_offs +
> + OMAP4_CM_STATICDEP,
> + (1 << clkdm2->dep_bit));
> +}
> +
> +static int omap4_clkdm_clear_all_wkup_sleep_deps(struct clockdomain
*clkdm)
> +{
> + struct clkdm_dep *cd;
> + u32 mask = 0;
> +
> + for (cd = clkdm->wkdep_srcs; cd && cd->clkdm_name; cd++) {
My initial version actually did have a check for cd->clkdm_name instead
of cd->clkdm, and then I ran into aborts when a clkdm, though
belonging to the right chip version, failed lookup (in clkdm_init) and
left
the cd->clkdm pointer NULL. This however was due to the fact that the
clkdm_name populated was'nt matching the actual name, but that got
me thinking if its better to look for populated pointers instead of names.
I however seemed to have missed the point that there also could be a
cd->clkdm populated as NULL for non supported CHIP rev's.
> + if (!omap_chip_is(cd->omap_chip))
> + continue;
> +
Would it make sense to add an additional check here to avoid
an abort in case of mismatches in clkdm_name populated and
lookup's failing in clkdm_init?
Something like...
If (cd->clkdm) {
|= 1 << cd->clkdm->dep_bit;
atomic_set(&cd->wkdep_usecount, 0);
}
Regards
Rajendra
> + mask |= 1 << cd->clkdm->dep_bit;
> + atomic_set(&cd->wkdep_usecount, 0);
> + }
> +
> + omap4_cminst_clear_inst_reg_bits(mask, clkdm->prcm_partition,
> + clkdm->cm_inst, clkdm->clkdm_offs
+
> + OMAP4_CM_STATICDEP);
> + return 0;
> +}
>
> static int omap4_clkdm_sleep(struct clockdomain *clkdm)
> {
> @@ -68,6 +118,14 @@ static int omap4_clkdm_clk_disable(struct
clockdomain *clkdm)
> }
>
> struct clkdm_ops omap4_clkdm_operations = {
> + .clkdm_add_wkdep = omap4_clkdm_add_wkup_sleep_dep,
> + .clkdm_del_wkdep = omap4_clkdm_del_wkup_sleep_dep,
> + .clkdm_read_wkdep = omap4_clkdm_read_wkup_sleep_dep,
> + .clkdm_clear_all_wkdeps = omap4_clkdm_clear_all_wkup_sleep_deps,
> + .clkdm_add_sleepdep = omap4_clkdm_add_wkup_sleep_dep,
> + .clkdm_del_sleepdep = omap4_clkdm_del_wkup_sleep_dep,
> + .clkdm_read_sleepdep = omap4_clkdm_read_wkup_sleep_dep,
> + .clkdm_clear_all_sleepdeps =
omap4_clkdm_clear_all_wkup_sleep_deps,
> .clkdm_sleep = omap4_clkdm_sleep,
> .clkdm_wakeup = omap4_clkdm_wakeup,
> .clkdm_allow_idle = omap4_clkdm_allow_idle,
> diff --git a/arch/arm/mach-omap2/cm44xx.h b/arch/arm/mach-omap2/cm44xx.h
> index 48fc3f4..0b87ec8 100644
> --- a/arch/arm/mach-omap2/cm44xx.h
> +++ b/arch/arm/mach-omap2/cm44xx.h
> @@ -21,6 +21,7 @@
> #include "cm.h"
>
> #define OMAP4_CM_CLKSTCTRL 0x0000
> +#define OMAP4_CM_STATICDEP 0x0004
>
> /* Function prototypes */
> # ifndef __ASSEMBLER__
> --
> 1.7.2.3
More information about the linux-arm-kernel
mailing list