Hi Paul, > -----Original Message----- > From: Paul Walmsley [mailto:paul@xxxxxxxxx] > Sent: Tuesday, January 11, 2011 6:36 AM > To: Rajendra Nayak > Cc: linux-omap@xxxxxxxxxxxxxxx; khilman@xxxxxxxxxxxxxxxxxxx; b-cousson@xxxxxx > Subject: Re: [PATCH 2/5] OMAP: clockdomain: Arch specific funcs to handle deps > > Hi Rajendra > > On Wed, 5 Jan 2011, Rajendra Nayak wrote: > > > Define the following architecture specific funtions for omap2/3 > > .clkdm_add_wkdep > > .clkdm_del_wkdep > > .clkdm_read_wkdep > > .clkdm_clear_all_wkdeps > > .clkdm_add_sleepdep > > .clkdm_del_sleepdep > > .clkdm_read_sleepdep > > .clkdm_clear_all_sleepdeps > > > > Convert the platform-independent framework to call these functions. > > With this also move the clkdm lookups for all wkdep_srcs and > > sleepdep_srcs at clkdm_init. > > > > Signed-off-by: Rajendra Nayak <rnayak@xxxxxx> > > --- > > arch/arm/mach-omap2/Makefile | 2 + > > arch/arm/mach-omap2/clockdomain.c | 80 ++++++--------- > > arch/arm/mach-omap2/clockdomain.h | 2 + > > arch/arm/mach-omap2/clockdomain2xxx_3xxx.c | 113 ++++++++++++++++++++++ > > arch/arm/mach-omap2/clockdomains2xxx_3xxx_data.c | 2 +- > > 5 files changed, 150 insertions(+), 49 deletions(-) > > create mode 100644 arch/arm/mach-omap2/clockdomain2xxx_3xxx.c > > > > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile > > index 4ab82f6..d28db0a 100644 > > --- a/arch/arm/mach-omap2/Makefile > > +++ b/arch/arm/mach-omap2/Makefile > > @@ -102,8 +102,10 @@ obj-$(CONFIG_ARCH_OMAP4) += $(powerdomain-common) \ > > > > # PRCM clockdomain control > > obj-$(CONFIG_ARCH_OMAP2) += clockdomain.o \ > > + clockdomain2xxx_3xxx.o \ > > clockdomains2xxx_3xxx_data.o > > obj-$(CONFIG_ARCH_OMAP3) += clockdomain.o \ > > + clockdomain2xxx_3xxx.o \ > > clockdomains2xxx_3xxx_data.o > > obj-$(CONFIG_ARCH_OMAP4) += clockdomain.o \ > > clockdomains44xx_data.o > > diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c > > index 3e40184..c32480c 100644 > > --- a/arch/arm/mach-omap2/clockdomain.c > > +++ b/arch/arm/mach-omap2/clockdomain.c > > @@ -308,6 +308,7 @@ void clkdm_init(struct clockdomain **clkdms, > > struct clockdomain **c = NULL; > > struct clockdomain *clkdm; > > struct clkdm_autodep *autodep = NULL; > > + struct clkdm_dep *cd; > > > > if (!custom_funcs) > > WARN(1, "No custom clkdm functions registered\n"); > > @@ -333,7 +334,18 @@ void clkdm_init(struct clockdomain **clkdms, > > else if (clkdm->flags & CLKDM_CAN_DISABLE_AUTO) > > omap2_clkdm_deny_idle(clkdm); > > > > + for (cd = clkdm->wkdep_srcs; cd && cd->clkdm_name; cd++) { > > + if (!omap_chip_is(cd->omap_chip)) > > + continue; > > + cd->clkdm = _clkdm_lookup(cd->clkdm_name); > > + } > > clkdm_clear_all_wkdeps(clkdm); > > + > > + for (cd = clkdm->sleepdep_srcs; cd && cd->clkdm_name; cd++) { > > + if (!omap_chip_is(cd->omap_chip)) > > + continue; > > + cd->clkdm = _clkdm_lookup(cd->clkdm_name); > > + } > > clkdm_clear_all_sleepdeps(clkdm); > > } > > } > > @@ -445,8 +457,8 @@ int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2) > > pr_debug("clockdomain: hardware will wake up %s when %s wakes " > > "up\n", clkdm1->name, clkdm2->name); > > > > - omap2_prm_set_mod_reg_bits((1 << clkdm2->dep_bit), > > - clkdm1->pwrdm.ptr->prcm_offs, PM_WKDEP); > > + if (arch_clkdm && arch_clkdm->clkdm_add_wkdep) > > + arch_clkdm->clkdm_add_wkdep(clkdm1, clkdm2); > > Putting this test inside the loop doesn't make sense to me; arch_clkdm and > arch_clkdm->clkdm_* only need to be tested once outside the loop. Please > do somtehing like this instead: Sure, will do the changes. > > cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs); > if (IS_ERR(cd)) > ret = PTR_ERR(cd); > > if (!arch_clkdm || !arch_clkdm->clkdm_add_wkdep) > ret = -EINVAL; > > if (ret) { > pr_debug("clockdomain: hardware cannot set/clear wake up of " > "%s when %s wakes up\n", clkdm1->name, clkdm2->name); > return ret; > } > > if (atomic_inc_return(&cd->wkdep_usecount) == 1) { > pr_debug("clockdomain: hardware will wake up %s when %s wakes " > "up\n", clkdm1->name, clkdm2->name); > > arch_clkdm->clkdm_add_wkdep(clkdm1, clkdm2); > } > > As in the above code, we should probably return an error if the function > pointers don't exist. That should allow us to get rid of the > > if (!cpu_is_omap34xx()) > return -EINVAL; > > tests in the sleepdep functions, since that can now be handled by just > leaving those function pointers NULL; see the below comments. > > These comments also apply to most of the other functions here. > > > @@ -480,8 +492,8 @@ int clkdm_del_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2) > > pr_debug("clockdomain: hardware will no longer wake up %s " > > "after %s wakes up\n", clkdm1->name, clkdm2->name); > > > > - omap2_prm_clear_mod_reg_bits((1 << clkdm2->dep_bit), > > - clkdm1->pwrdm.ptr->prcm_offs, PM_WKDEP); > > + if (arch_clkdm && arch_clkdm->clkdm_del_wkdep) > > + arch_clkdm->clkdm_del_wkdep(clkdm1, clkdm2); > > } > > > > return 0; > > @@ -516,8 +528,10 @@ int clkdm_read_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2) > > } > > > > /* XXX It's faster to return the atomic wkdep_usecount */ > > - return omap2_prm_read_mod_bits_shift(clkdm1->pwrdm.ptr->prcm_offs, PM_WKDEP, > > - (1 << clkdm2->dep_bit)); > > + if (arch_clkdm && arch_clkdm->clkdm_read_wkdep) > > + return arch_clkdm->clkdm_read_wkdep(clkdm1, clkdm2); > > + > > + return -EINVAL; > > } > > > > /** > > @@ -532,25 +546,11 @@ int clkdm_read_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2) > > */ > > int clkdm_clear_all_wkdeps(struct clockdomain *clkdm) > > { > > - struct clkdm_dep *cd; > > - u32 mask = 0; > > - > > if (!clkdm) > > return -EINVAL; > > > > - for (cd = clkdm->wkdep_srcs; cd && cd->clkdm_name; cd++) { > > - if (!omap_chip_is(cd->omap_chip)) > > - continue; > > - > > - if (!cd->clkdm && cd->clkdm_name) > > - cd->clkdm = _clkdm_lookup(cd->clkdm_name); > > - > > - /* PRM accesses are slow, so minimize them */ > > - mask |= 1 << cd->clkdm->dep_bit; > > - atomic_set(&cd->wkdep_usecount, 0); > > - } > > - > > - omap2_prm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs, PM_WKDEP); > > + if (arch_clkdm && arch_clkdm->clkdm_clear_all_wkdeps) > > + arch_clkdm->clkdm_clear_all_wkdeps(clkdm); > > > > return 0; > > } > > @@ -589,9 +589,8 @@ int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2) > > pr_debug("clockdomain: will prevent %s from sleeping if %s " > > "is active\n", clkdm1->name, clkdm2->name); > > > > - omap2_cm_set_mod_reg_bits((1 << clkdm2->dep_bit), > > - clkdm1->pwrdm.ptr->prcm_offs, > > - OMAP3430_CM_SLEEPDEP); > > + if (arch_clkdm && arch_clkdm->clkdm_add_sleepdep) > > + arch_clkdm->clkdm_add_sleepdep(clkdm1, clkdm2); > > } > > > > return 0; > > @@ -632,9 +631,8 @@ int clkdm_del_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2) > > "sleeping if %s is active\n", clkdm1->name, > > clkdm2->name); > > > > - omap2_cm_clear_mod_reg_bits((1 << clkdm2->dep_bit), > > - clkdm1->pwrdm.ptr->prcm_offs, > > - OMAP3430_CM_SLEEPDEP); > > + if (arch_clkdm && arch_clkdm->clkdm_del_sleepdep) > > + arch_clkdm->clkdm_del_sleepdep(clkdm1, clkdm2); > > } > > > > return 0; > > @@ -675,9 +673,10 @@ int clkdm_read_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2) > > } > > > > /* XXX It's faster to return the atomic sleepdep_usecount */ > > - return omap2_prm_read_mod_bits_shift(clkdm1->pwrdm.ptr->prcm_offs, > > - OMAP3430_CM_SLEEPDEP, > > - (1 << clkdm2->dep_bit)); > > + if (arch_clkdm && arch_clkdm->clkdm_read_sleepdep) > > + return arch_clkdm->clkdm_read_sleepdep(clkdm1, clkdm2); > > + > > + return -EINVAL; > > } > > > > /** > > @@ -692,29 +691,14 @@ int clkdm_read_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2) > > */ > > int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm) > > { > > - struct clkdm_dep *cd; > > - u32 mask = 0; > > - > > if (!cpu_is_omap34xx()) > > return -EINVAL; > > > > if (!clkdm) > > return -EINVAL; > > > > - for (cd = clkdm->sleepdep_srcs; cd && cd->clkdm_name; cd++) { > > - if (!omap_chip_is(cd->omap_chip)) > > - continue; > > - > > - if (!cd->clkdm && cd->clkdm_name) > > - cd->clkdm = _clkdm_lookup(cd->clkdm_name); > > - > > - /* PRM accesses are slow, so minimize them */ > > - mask |= 1 << cd->clkdm->dep_bit; > > - atomic_set(&cd->sleepdep_usecount, 0); > > - } > > - > > - omap2_prm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs, > > - OMAP3430_CM_SLEEPDEP); > > + if (arch_clkdm && arch_clkdm->clkdm_clear_all_sleepdeps) > > + arch_clkdm->clkdm_clear_all_sleepdeps(clkdm); > > > > return 0; > > } > > diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h > > index 346efa2..a9cda27 100644 > > --- a/arch/arm/mach-omap2/clockdomain.h > > +++ b/arch/arm/mach-omap2/clockdomain.h > > @@ -181,4 +181,6 @@ int omap2_clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk); > > extern void __init omap2_clockdomains_init(void); > > extern void __init omap44xx_clockdomains_init(void); > > > > +extern struct clkdm_ops omap2_clkdm_operations; > > + > > #endif > > diff --git a/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c b/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c > > new file mode 100644 > > index 0000000..a7303bd > > --- /dev/null > > +++ b/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c > > @@ -0,0 +1,113 @@ > > +/* > > + * OMAP2 and OMAP3 clockdomain control > > + * > > + * Copyright (C) 2008-2010 Texas Instruments, Inc. > > + * Copyright (C) 2008-2010 Nokia Corporation > > + * > > + * Derived from mach-omap2/clockdomain.c written by Paul Walmsley > > + * Rajendra Nayak <rnayak@xxxxxx> > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + */ > > + > > +#include <linux/types.h> > > +#include <plat/prcm.h> > > +#include "prm.h" > > +#include "prm2xxx_3xxx.h" > > +#include "cm.h" > > +#include "cm2xxx_3xxx.h" > > +#include "cm-regbits-24xx.h" > > +#include "cm-regbits-34xx.h" > > +#include "clockdomain.h" > > + > > +static void omap2_clkdm_add_wkdep(struct clockdomain *clkdm1, > > + struct clockdomain *clkdm2) > > +{ > > + omap2_prm_set_mod_reg_bits((1 << clkdm2->dep_bit), > > + clkdm1->pwrdm.ptr->prcm_offs, PM_WKDEP); > > +} > > + > > +static void omap2_clkdm_del_wkdep(struct clockdomain *clkdm1, > > + struct clockdomain *clkdm2) > > +{ > > + omap2_prm_clear_mod_reg_bits((1 << clkdm2->dep_bit), > > + clkdm1->pwrdm.ptr->prcm_offs, PM_WKDEP); > > +} > > + > > +static int omap2_clkdm_read_wkdep(struct clockdomain *clkdm1, > > + struct clockdomain *clkdm2) > > +{ > > + return omap2_prm_read_mod_bits_shift(clkdm1->pwrdm.ptr->prcm_offs, > > + PM_WKDEP, (1 << clkdm2->dep_bit)); > > +} > > + > > +static void omap2_clkdm_clear_all_wkdeps(struct clockdomain *clkdm) > > +{ > > + struct clkdm_dep *cd; > > + u32 mask = 0; > > + > > + for (cd = clkdm->wkdep_srcs; cd && cd->clkdm_name; cd++) { > > + if (!omap_chip_is(cd->omap_chip)) > > + continue; > > + > > + /* PRM accesses are slow, so minimize them */ > > + mask |= 1 << cd->clkdm->dep_bit; > > + atomic_set(&cd->wkdep_usecount, 0); > > + } > > + > > + omap2_prm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs, > > + PM_WKDEP); > > +} > > + > > +static void omap2_clkdm_add_sleepdep(struct clockdomain *clkdm1, > > + struct clockdomain *clkdm2) > > +{ > > + omap2_cm_set_mod_reg_bits((1 << clkdm2->dep_bit), > > + clkdm1->pwrdm.ptr->prcm_offs, > > + OMAP3430_CM_SLEEPDEP); > > +} > > OMAP2xxx didn't support software-controlled sleepdeps, so this should not > be present (note the 'OMAP3430' in the OMAP3430_CM_SLEEPDEP macro). Yup, realized after I sent the patches out. Will fix them in V2. Thanks Rajendra > > > + > > +static void omap2_clkdm_del_sleepdep(struct clockdomain *clkdm1, > > + struct clockdomain *clkdm2) > > +{ > > + omap2_cm_clear_mod_reg_bits((1 << clkdm2->dep_bit), > > + clkdm1->pwrdm.ptr->prcm_offs, > > + OMAP3430_CM_SLEEPDEP); > > +} > > as above > > > + > > +static int omap2_clkdm_read_sleepdep(struct clockdomain *clkdm1, > > + struct clockdomain *clkdm2) > > +{ > > + return omap2_prm_read_mod_bits_shift(clkdm1->pwrdm.ptr->prcm_offs, > > + OMAP3430_CM_SLEEPDEP, (1 << clkdm2->dep_bit)); > > +} > > as above > > > + > > +static void omap2_clkdm_clear_all_sleepdeps(struct clockdomain *clkdm) > > +{ > > + struct clkdm_dep *cd; > > + u32 mask = 0; > > + > > + for (cd = clkdm->sleepdep_srcs; cd && cd->clkdm_name; cd++) { > > + if (!omap_chip_is(cd->omap_chip)) > > + continue; > > + > > + /* PRM accesses are slow, so minimize them */ > > + mask |= 1 << cd->clkdm->dep_bit; > > + atomic_set(&cd->sleepdep_usecount, 0); > > + } > > + omap2_prm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs, > > + OMAP3430_CM_SLEEPDEP); > > +} > > as above > > > + > > +struct clkdm_ops omap2_clkdm_operations = { > > + .clkdm_add_wkdep = omap2_clkdm_add_wkdep, > > + .clkdm_del_wkdep = omap2_clkdm_del_wkdep, > > + .clkdm_read_wkdep = omap2_clkdm_read_wkdep, > > + .clkdm_clear_all_wkdeps = omap2_clkdm_clear_all_wkdeps, > > + .clkdm_add_sleepdep = omap2_clkdm_add_sleepdep, > > + .clkdm_del_sleepdep = omap2_clkdm_del_sleepdep, > > + .clkdm_read_sleepdep = omap2_clkdm_read_sleepdep, > > + .clkdm_clear_all_sleepdeps = omap2_clkdm_clear_all_sleepdeps, > > These four function pointers should not be present. OMAP2xxx did not > support software-controlled sleepdeps. You'll probably need to add > omap3_clkdm_operations in this patch. > > > +}; > > diff --git a/arch/arm/mach-omap2/clockdomains2xxx_3xxx_data.c b/arch/arm/mach- > omap2/clockdomains2xxx_3xxx_data.c > > index 8cab07a..ba0bbbd 100644 > > --- a/arch/arm/mach-omap2/clockdomains2xxx_3xxx_data.c > > +++ b/arch/arm/mach-omap2/clockdomains2xxx_3xxx_data.c > > @@ -856,5 +856,5 @@ static struct clockdomain *clockdomains_omap2[] __initdata = { > > > > void __init omap2_clockdomains_init(void) > > { > > - clkdm_init(clockdomains_omap2, clkdm_autodeps, NULL); > > + clkdm_init(clockdomains_omap2, clkdm_autodeps, &omap2_clkdm_operations); > > } > > -- > > 1.7.0.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
