From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Date: Mon, 09 Jul 2012 16:49:15 +0000 Subject: Re: [PATCH] ARM: shmobile: Build code depending on PM only if PM is set Message-Id: <201207091849.15951.rjw@sisk.pl> List-Id: References: <201207090015.12531.rjw@sisk.pl> In-Reply-To: <201207090015.12531.rjw@sisk.pl> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-sh@vger.kernel.org On Monday, July 09, 2012, Magnus Damm wrote: > On Mon, Jul 9, 2012 at 5:13 PM, Rafael J. Wysocki wrote: > > On Monday, July 09, 2012, Simon Horman wrote: > >> On Mon, Jul 09, 2012 at 12:15:12AM +0200, Rafael J. Wysocki wrote: > >> > From: Rafael J. Wysocki > >> > > >> > There are a few files under arch/arm/mach-shmobile/ whose entire > >> > contents depend on CONFIG_PM, but they are compiled even if > >> > CONFIG_PM is unset. It is cleaner to modify the Makefile to > >> > avoid building those files entirely for CONFIG_PM unset and > >> > remove #ifdef CONFIG_PM directives from them. > >> > > >> > Signed-off-by: Rafael J. Wysocki > >> > --- > >> > arch/arm/mach-shmobile/Makefile | 4 +++- > >> > arch/arm/mach-shmobile/include/mach/common.h | 4 ++++ > >> > arch/arm/mach-shmobile/pm-r8a7740.c | 3 --- > >> > arch/arm/mach-shmobile/pm-rmobile.c | 2 -- > >> > arch/arm/mach-shmobile/pm-sh7372.c | 4 ---- > >> > 5 files changed, 7 insertions(+), 10 deletions(-) > >> > > >> > Index: linux/arch/arm/mach-shmobile/Makefile > >> > =================================> >> > --- linux.orig/arch/arm/mach-shmobile/Makefile > >> > +++ linux/arch/arm/mach-shmobile/Makefile > >> > @@ -37,11 +37,13 @@ obj-$(CONFIG_ARCH_SH7372) += entry-intc. > >> > obj-$(CONFIG_ARCH_R8A7740) += entry-intc.o > >> > > >> > # PM objects > >> > -obj-$(CONFIG_SUSPEND) += suspend.o > >> > obj-$(CONFIG_CPU_IDLE) += cpuidle.o > >> > +ifeq ($(CONFIG_PM),y) > >> > +obj-$(CONFIG_SUSPEND) += suspend.o > >> > obj-$(CONFIG_ARCH_SHMOBILE) += pm-rmobile.o > >> > obj-$(CONFIG_ARCH_SH7372) += pm-sh7372.o sleep-sh7372.o > >> > obj-$(CONFIG_ARCH_R8A7740) += pm-r8a7740.o > >> > +endif > >> > obj-$(CONFIG_ARCH_R8A7779) += pm-r8a7779.o > >> > > >> > # Board objects > >> > Index: linux/arch/arm/mach-shmobile/include/mach/common.h > >> > =================================> >> > --- linux.orig/arch/arm/mach-shmobile/include/mach/common.h > >> > +++ linux/arch/arm/mach-shmobile/include/mach/common.h > >> > @@ -41,7 +41,11 @@ extern void sh7372_add_early_devices(voi > >> > extern void sh7372_add_standard_devices(void); > >> > extern void sh7372_clock_init(void); > >> > extern void sh7372_pinmux_init(void); > >> > +#ifdef CONFIG_PM > >> > extern void sh7372_pm_init(void); > >> > +#else > >> > +static inline void sh7372_pm_init(void) {} > >> > >> This seems to slightly alter the behaviour in the case where > >> CONFIG_PM is not set. I'm unsure if that is a problem or not. > > > > I don't think it would be a problem, all of those settings are PM-related. > > > > Magnus, what do you think? > > Cleaning up the code is always nice, but I wonder if you take the > following into consideration? > > CONFIG_CPU_IDLE=y > CONFIG_PM=n This actually doesn't work (please see below). > Also, slightly off topic, but on top of that we have the ARM specific > CONFIG_ARM_CPU_SUSPEND which is required to build cpu_resume() in > sleep-sh7372.c, and this in turn in needed for CPUIdle or > Suspend-to-RAM on sh7372. > > It would be nice to simplify all this somehow. From the ARM arch code > we do want to use cpu_resume() and cpu_suspend() both from CPUIdle and > Suspend-to-RAM - this since the same low level sleep code is used for > both cases. Yes, please see my fix for the build bug reported by Guennadi. cpu_resume() and cpu_suspend() are not available if CONFIG_SUSPEND is unset. Thanks, Rafael