* [PATCH] ARM: shmobile: Build code depending on PM only if PM is set
@ 2012-07-08 22:15 Rafael J. Wysocki
2012-07-09 1:19 ` Simon Horman
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2012-07-08 22:15 UTC (permalink / raw)
To: linux-sh
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) {}
+#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)
{
^ 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
` (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-09-11 6:51 Simon Horman
0 siblings, 0 replies; 8+ messages in thread
From: Simon Horman @ 2012-09-11 6:51 UTC (permalink / raw)
To: linux-sh
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.
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?
^ 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
end of thread, other threads:[~2012-09-12 0:23 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2012-09-11 20:44 ` Rafael J. Wysocki
2012-09-12 0:23 ` Simon Horman
-- strict thread matches above, loose matches on Subject: below --
2012-09-11 6:51 Simon Horman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).