* Re: [PATCH] ARM: shmobile: Build code depending on PM only if PM is set
2012-07-08 22:15 [PATCH] ARM: shmobile: Build code depending on PM only if PM is set Rafael J. Wysocki
@ 2012-07-09 1:19 ` Simon Horman
2012-07-09 8:13 ` Rafael J. Wysocki
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Simon Horman @ 2012-07-09 1:19 UTC (permalink / raw)
To: linux-sh
On Mon, Jul 09, 2012 at 12:15:12AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> 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@sisk.pl>
> ---
> 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.
> +#endif
> extern void sh7372_resume_core_standby_sysc(void);
> extern int sh7372_do_idle_sysc(unsigned long sleep_mode);
> extern struct clk sh7372_extal1_clk;
> Index: linux/arch/arm/mach-shmobile/pm-r8a7740.c
> =================================> --- linux.orig/arch/arm/mach-shmobile/pm-r8a7740.c
> +++ linux/arch/arm/mach-shmobile/pm-r8a7740.c
> @@ -11,7 +11,6 @@
> #include <linux/console.h>
> #include <mach/pm-rmobile.h>
>
> -#ifdef CONFIG_PM
> static int r8a7740_pd_a4s_suspend(void)
> {
> /*
> @@ -50,5 +49,3 @@ struct rmobile_pm_domain r8a7740_pd_a4lc
> .genpd.name = "A4LC",
> .bit_shift = 1,
> };
> -
> -#endif /* CONFIG_PM */
> Index: linux/arch/arm/mach-shmobile/pm-rmobile.c
> =================================> --- linux.orig/arch/arm/mach-shmobile/pm-rmobile.c
> +++ linux/arch/arm/mach-shmobile/pm-rmobile.c
> @@ -27,7 +27,6 @@
> #define PSTR_RETRIES 100
> #define PSTR_DELAY_US 10
>
> -#ifdef CONFIG_PM
> static int rmobile_pd_power_down(struct generic_pm_domain *genpd)
> {
> struct rmobile_pm_domain *rmobile_pd = to_rmobile_pd(genpd);
> @@ -164,4 +163,3 @@ void rmobile_pm_add_subdomain(struct rmo
> {
> pm_genpd_add_subdomain(&rmobile_pd->genpd, &rmobile_sd->genpd);
> }
> -#endif /* CONFIG_PM */
> Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
> =================================> --- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
> +++ linux/arch/arm/mach-shmobile/pm-sh7372.c
> @@ -69,8 +69,6 @@
> /* AP-System Core */
> #define APARMBAREA 0xe6f10020
>
> -#ifdef CONFIG_PM
> -
> struct rmobile_pm_domain sh7372_pd_a4lc = {
> .genpd.name = "A4LC",
> .bit_shift = 1,
> @@ -149,8 +147,6 @@ struct rmobile_pm_domain sh7372_pd_a3sg
> .bit_shift = 13,
> };
>
> -#endif /* CONFIG_PM */
> -
> #ifdef CONFIG_SUSPEND
> static void sh7372_set_reset_vector(unsigned long address)
> {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: shmobile: Build code depending on PM only if PM is set
2012-07-08 22:15 [PATCH] ARM: shmobile: Build code depending on PM only if PM is set Rafael J. Wysocki
2012-07-09 1:19 ` Simon Horman
@ 2012-07-09 8:13 ` Rafael J. Wysocki
2012-07-09 9:56 ` Magnus Damm
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2012-07-09 8:13 UTC (permalink / raw)
To: linux-sh
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@sisk.pl>
> >
> > 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@sisk.pl>
> > ---
> > 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?
Thanks,
Rafael
> > +#endif
> > extern void sh7372_resume_core_standby_sysc(void);
> > extern int sh7372_do_idle_sysc(unsigned long sleep_mode);
> > extern struct clk sh7372_extal1_clk;
> > Index: linux/arch/arm/mach-shmobile/pm-r8a7740.c
> > =================================> > --- linux.orig/arch/arm/mach-shmobile/pm-r8a7740.c
> > +++ linux/arch/arm/mach-shmobile/pm-r8a7740.c
> > @@ -11,7 +11,6 @@
> > #include <linux/console.h>
> > #include <mach/pm-rmobile.h>
> >
> > -#ifdef CONFIG_PM
> > static int r8a7740_pd_a4s_suspend(void)
> > {
> > /*
> > @@ -50,5 +49,3 @@ struct rmobile_pm_domain r8a7740_pd_a4lc
> > .genpd.name = "A4LC",
> > .bit_shift = 1,
> > };
> > -
> > -#endif /* CONFIG_PM */
> > Index: linux/arch/arm/mach-shmobile/pm-rmobile.c
> > =================================> > --- linux.orig/arch/arm/mach-shmobile/pm-rmobile.c
> > +++ linux/arch/arm/mach-shmobile/pm-rmobile.c
> > @@ -27,7 +27,6 @@
> > #define PSTR_RETRIES 100
> > #define PSTR_DELAY_US 10
> >
> > -#ifdef CONFIG_PM
> > static int rmobile_pd_power_down(struct generic_pm_domain *genpd)
> > {
> > struct rmobile_pm_domain *rmobile_pd = to_rmobile_pd(genpd);
> > @@ -164,4 +163,3 @@ void rmobile_pm_add_subdomain(struct rmo
> > {
> > pm_genpd_add_subdomain(&rmobile_pd->genpd, &rmobile_sd->genpd);
> > }
> > -#endif /* CONFIG_PM */
> > Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
> > =================================> > --- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
> > +++ linux/arch/arm/mach-shmobile/pm-sh7372.c
> > @@ -69,8 +69,6 @@
> > /* AP-System Core */
> > #define APARMBAREA 0xe6f10020
> >
> > -#ifdef CONFIG_PM
> > -
> > struct rmobile_pm_domain sh7372_pd_a4lc = {
> > .genpd.name = "A4LC",
> > .bit_shift = 1,
> > @@ -149,8 +147,6 @@ struct rmobile_pm_domain sh7372_pd_a3sg
> > .bit_shift = 13,
> > };
> >
> > -#endif /* CONFIG_PM */
> > -
> > #ifdef CONFIG_SUSPEND
> > static void sh7372_set_reset_vector(unsigned long address)
> > {
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: shmobile: Build code depending on PM only if PM is set
2012-07-08 22:15 [PATCH] ARM: shmobile: Build code depending on PM only if PM is set Rafael J. Wysocki
2012-07-09 1:19 ` Simon Horman
2012-07-09 8:13 ` Rafael J. Wysocki
@ 2012-07-09 9:56 ` Magnus Damm
2012-07-09 16:49 ` Rafael J. Wysocki
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Magnus Damm @ 2012-07-09 9:56 UTC (permalink / raw)
To: linux-sh
On Mon, Jul 9, 2012 at 5:13 PM, Rafael J. Wysocki <rjw@sisk.pl> 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@sisk.pl>
>> >
>> > 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@sisk.pl>
>> > ---
>> > 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: shmobile: Build code depending on PM only if PM is set
2012-07-08 22:15 [PATCH] ARM: shmobile: Build code depending on PM only if PM is set Rafael J. Wysocki
` (2 preceding siblings ...)
2012-07-09 9:56 ` Magnus Damm
@ 2012-07-09 16:49 ` Rafael J. Wysocki
2012-09-11 20:44 ` Rafael J. Wysocki
2012-09-12 0:23 ` Simon Horman
5 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2012-07-09 16:49 UTC (permalink / raw)
To: linux-sh
On Monday, July 09, 2012, Magnus Damm wrote:
> On Mon, Jul 9, 2012 at 5:13 PM, Rafael J. Wysocki <rjw@sisk.pl> 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@sisk.pl>
> >> >
> >> > 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@sisk.pl>
> >> > ---
> >> > 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: shmobile: Build code depending on PM only if PM is set
2012-07-08 22:15 [PATCH] ARM: shmobile: Build code depending on PM only if PM is set Rafael J. Wysocki
` (3 preceding siblings ...)
2012-07-09 16:49 ` Rafael J. Wysocki
@ 2012-09-11 20:44 ` Rafael J. Wysocki
2012-09-12 0:23 ` Simon Horman
5 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2012-09-11 20:44 UTC (permalink / raw)
To: linux-sh
On Tuesday, September 11, 2012, Simon Horman wrote:
> On Mon, Jul 09, 2012 at 06:49:15PM +0200, Rafael J. Wysocki wrote:
> > On Monday, July 09, 2012, Magnus Damm wrote:
> > > On Mon, Jul 9, 2012 at 5:13 PM, Rafael J. Wysocki <rjw@sisk.pl> 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@sisk.pl>
> > > >> >
> > > >> > 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@sisk.pl>
> > > >> > ---
> > > >> > 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.
>
> Hi Rafael,
>
> I may have missed something, but it seems that this patch wasn't merged.
No, it wasn't, because we addressed the problem in a different way.
> Aside from whether it still applies or not, is it still appropriate?
> And if so, is it appropriate to go through the linux-pm tree?
I believe commit a1ee61b8f4b56e5e6ced16b83d5098e0f4238a45
(ARM: shmobile: Take cpuidle dependencies into account correctly) from Magnus
fixed this particular problem.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ARM: shmobile: Build code depending on PM only if PM is set
2012-07-08 22:15 [PATCH] ARM: shmobile: Build code depending on PM only if PM is set Rafael J. Wysocki
` (4 preceding siblings ...)
2012-09-11 20:44 ` Rafael J. Wysocki
@ 2012-09-12 0:23 ` Simon Horman
5 siblings, 0 replies; 8+ messages in thread
From: Simon Horman @ 2012-09-12 0:23 UTC (permalink / raw)
To: linux-sh
On Tue, Sep 11, 2012 at 10:44:40PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, September 11, 2012, Simon Horman wrote:
> > On Mon, Jul 09, 2012 at 06:49:15PM +0200, Rafael J. Wysocki wrote:
> > > On Monday, July 09, 2012, Magnus Damm wrote:
> > > > On Mon, Jul 9, 2012 at 5:13 PM, Rafael J. Wysocki <rjw@sisk.pl> 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@sisk.pl>
> > > > >> >
> > > > >> > 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@sisk.pl>
> > > > >> > ---
> > > > >> > 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.
> >
> > Hi Rafael,
> >
> > I may have missed something, but it seems that this patch wasn't merged.
>
> No, it wasn't, because we addressed the problem in a different way.
>
> > Aside from whether it still applies or not, is it still appropriate?
> > And if so, is it appropriate to go through the linux-pm tree?
>
> I believe commit a1ee61b8f4b56e5e6ced16b83d5098e0f4238a45
> (ARM: shmobile: Take cpuidle dependencies into account correctly) from Magnus
> fixed this particular problem.
Thanks, case closed :)
^ permalink raw reply [flat|nested] 8+ messages in thread