Hi Rafael, On Sun, Jun 12, 2011 at 5:40 AM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > From: Rafael J. Wysocki <rjw@xxxxxxx> > > Use the generic power domains support introduced by the previous > patch to implement support for power domains on SH7372. > > Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx> > --- Thanks for your work on this. I just tried this on my Mackerel board, but I can't seem to get the pd_power_up() and pd_power_down() callbacks to be executed. It is probably a misconfiguration from my side. Here's some feedback on the sh7372-specific code: > --- linux-2.6.orig/arch/arm/mach-shmobile/Kconfig > +++ linux-2.6/arch/arm/mach-shmobile/Kconfig > @@ -19,6 +19,7 @@ config ARCH_SH7372 > select CPU_V7 > select SH_CLK_CPG > select ARCH_WANT_OPTIONAL_GPIOLIB > + select PM_GENERIC_DOMAINS We want to support a single ARM binary for multiple boards, so this should be enabled for all SoCs in mach-shmobile as a whole. > --- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c > +++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c > @@ -15,16 +15,97 @@ > #include <linux/list.h> > #include <linux/err.h> > #include <linux/slab.h> > +#include <linux/pm_runtime.h> > +#include <linux/platform_device.h> > #include <asm/system.h> > #include <asm/io.h> > #include <asm/tlbflush.h> > #include <mach/common.h> > +#include <mach/sh7372.h> > > #define SMFRAM 0xe6a70000 > #define SYSTBCR 0xe6150024 > #define SBAR 0xe6180020 > #define APARMBAREA 0xe6f10020 > > +#define SPDCR 0xe6180008 > +#define SWUCR 0xe6180014 > +#define PSTR 0xe6180080 > + > +struct sh7372_domain_data { > + unsigned int bit_shift; > +}; Is it possible to make struct sh7372_domain_data include struct generic_pm_domain? I suspect so since these two data types seem to be linked together. I guess container_of() can be used for conversion between the types? > + > +static int pd_power_down(struct dev_pm_domain *domain) > +{ > + struct sh7372_domain_data *dd = domain->platform_data; > + unsigned int mask = 1 << dd->bit_shift; > + > + if (__raw_readl(PSTR) & mask) { > + __raw_writel(mask, SPDCR); > + > + while (__raw_readl(SPDCR) & mask) {} This probably wants a cpu_relax() in the polling loop. I realize that my prototype hack lacked that... > + pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n", > + mask, __raw_readl(PSTR)); > + } > + > + return 0; > +} > + > +static int pd_power_up(struct dev_pm_domain *domain) > +{ > + struct sh7372_domain_data *dd = domain->platform_data; > + unsigned int mask = 1 << dd->bit_shift; > + > + if (!(__raw_readl(PSTR) & mask)) { > + __raw_writel(mask, SWUCR); > + > + while (__raw_readl(SWUCR) & mask) {} Same cpu_relax() here. > +static int __init sh7372_power_domains_init(void) > +{ > + sh7372_init_domain(&sh7372_a4lc_domain, &sh7372_a4lc_domain_data); > + return 0; > +} > +core_initcall(sh7372_power_domains_init); This initcall is going to be executed regardless which SoC we're running on. We only want it called for sh7372 though. If you look at other SoC-specific code then you will notice that initcalls are only used for functions in mach-shmobile/ that are common for all SoCs implemented under mach-shmobile. You most likely want to initialize from sh7372_pm_init(), but for that to work you probably have to reorder your code inside mackerel_init(). Thanks, / magnus _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm