Re: [PATCH] ARM: shmobile: Build code depending on PM only if PM is set

[Magnus Damm <magnus.damm@xxxxxxxxx>]

On Mon, Jul 9, 2012 at 5:13 PM, Rafael J. Wysocki <rjw@xxxxxxx> 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 <rjw@xxxxxxx>
>> >
>> > 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 <rjw@xxxxxxx>
>> > ---
>> >  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

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.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html