* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
@ 2014-09-04 4:12 ` Simon Horman
2014-09-04 4:17 ` Magnus Damm
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2014-09-04 4:12 UTC (permalink / raw)
To: linux-sh
On Thu, Sep 04, 2014 at 12:39:42PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
>
> Simplify the r8a7740 PM code by using CONFIG_PM_RMOBILE instead
> of having a complex #if expression embedded in the C code.
>
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
Thanks Magnus,
this looks good to me.
The dependencies for this changes are a bit messy so I would like
to defer it until v3.19. Could you repost it once v3.18-rc6 is released?
> ---
>
> Built on top of renesas-devel-20140904-v3.17-rc3
>
> arch/arm/mach-shmobile/pm-r8a7740.c | 6 ++++--
> arch/arm/mach-shmobile/r8a7740.h | 7 +------
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> --- 0001/arch/arm/mach-shmobile/pm-r8a7740.c
> +++ work/arch/arm/mach-shmobile/pm-r8a7740.c 2014-09-04 12:29:12.000000000 +0900
> @@ -13,7 +13,7 @@
> #include "common.h"
> #include "pm-rmobile.h"
>
> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> +#ifdef CONFIG_PM_RMOBILE
> static int r8a7740_pd_a4s_suspend(void)
> {
> /*
> @@ -58,7 +58,9 @@ void __init r8a7740_init_pm_domains(void
> rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
> pm_genpd_add_subdomain_names("A4S", "A3SP");
> }
> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> +#else
> +void r8a7740_init_pm_domains(void) {}
> +#endif
>
> #ifdef CONFIG_SUSPEND
> static int r8a7740_enter_suspend(suspend_state_t suspend_state)
> --- 0001/arch/arm/mach-shmobile/r8a7740.h
> +++ work/arch/arm/mach-shmobile/r8a7740.h 2014-09-04 12:29:12.000000000 +0900
> @@ -52,11 +52,6 @@ extern void r8a7740_add_standard_devices
> extern void r8a7740_clock_init(u8 md_ck);
> extern void r8a7740_pinmux_init(void);
> extern void r8a7740_pm_init(void);
> -
> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> -extern void __init r8a7740_init_pm_domains(void);
> -#else
> -static inline void r8a7740_init_pm_domains(void) {}
> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> +extern void r8a7740_init_pm_domains(void);
>
> #endif /* __ASM_R8A7740_H__ */
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
2014-09-04 4:12 ` Simon Horman
@ 2014-09-04 4:17 ` Magnus Damm
2014-09-04 4:25 ` Simon Horman
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Magnus Damm @ 2014-09-04 4:17 UTC (permalink / raw)
To: linux-sh
Hi Simon,
On Thu, Sep 4, 2014 at 1:12 PM, Simon Horman <horms@verge.net.au> wrote:
> On Thu, Sep 04, 2014 at 12:39:42PM +0900, Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> Simplify the r8a7740 PM code by using CONFIG_PM_RMOBILE instead
>> of having a complex #if expression embedded in the C code.
>>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>
> Thanks Magnus,
>
> this looks good to me.
>
> The dependencies for this changes are a bit messy so I would like
> to defer it until v3.19. Could you repost it once v3.18-rc6 is released?
Sure, will do! Does that apply to this patch only, or also to other
things like the new (sh73a0 and/or r8a73a4) multiplatform bits and
legacy board code removal?
Thanks,
/ magnus
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
2014-09-04 4:12 ` Simon Horman
2014-09-04 4:17 ` Magnus Damm
@ 2014-09-04 4:25 ` Simon Horman
2014-09-04 4:29 ` Simon Horman
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2014-09-04 4:25 UTC (permalink / raw)
To: linux-sh
On Thu, Sep 04, 2014 at 01:17:54PM +0900, Magnus Damm wrote:
> Hi Simon,
>
> On Thu, Sep 4, 2014 at 1:12 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Sep 04, 2014 at 12:39:42PM +0900, Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >>
> >> Simplify the r8a7740 PM code by using CONFIG_PM_RMOBILE instead
> >> of having a complex #if expression embedded in the C code.
> >>
> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >
> > Thanks Magnus,
> >
> > this looks good to me.
> >
> > The dependencies for this changes are a bit messy so I would like
> > to defer it until v3.19. Could you repost it once v3.18-rc6 is released?
>
> Sure, will do! Does that apply to this patch only, or also to other
> things like the new (sh73a0 and/or r8a73a4) multiplatform bits and
> legacy board code removal?
At this time I would like to handle-things on a case-by-case basis.
In general it would make my life slightly easier if all futher
cleanups were deferred until v3.19. But at this stage I'm still happy
to assess each patch on its merits.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
` (2 preceding siblings ...)
2014-09-04 4:25 ` Simon Horman
@ 2014-09-04 4:29 ` Simon Horman
2014-09-04 6:31 ` Geert Uytterhoeven
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2014-09-04 4:29 UTC (permalink / raw)
To: linux-sh
On Thu, Sep 04, 2014 at 01:25:05PM +0900, Simon Horman wrote:
> On Thu, Sep 04, 2014 at 01:17:54PM +0900, Magnus Damm wrote:
> > Hi Simon,
> >
> > On Thu, Sep 4, 2014 at 1:12 PM, Simon Horman <horms@verge.net.au> wrote:
> > > On Thu, Sep 04, 2014 at 12:39:42PM +0900, Magnus Damm wrote:
> > >> From: Magnus Damm <damm+renesas@opensource.se>
> > >>
> > >> Simplify the r8a7740 PM code by using CONFIG_PM_RMOBILE instead
> > >> of having a complex #if expression embedded in the C code.
> > >>
> > >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> > >
> > > Thanks Magnus,
> > >
> > > this looks good to me.
> > >
> > > The dependencies for this changes are a bit messy so I would like
> > > to defer it until v3.19. Could you repost it once v3.18-rc6 is released?
That should read v3.17-rc6.
> > Sure, will do! Does that apply to this patch only, or also to other
> > things like the new (sh73a0 and/or r8a73a4) multiplatform bits and
> > legacy board code removal?
>
> At this time I would like to handle-things on a case-by-case basis.
>
> In general it would make my life slightly easier if all futher
> cleanups were deferred until v3.19. But at this stage I'm still happy
> to assess each patch on its merits.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
` (3 preceding siblings ...)
2014-09-04 4:29 ` Simon Horman
@ 2014-09-04 6:31 ` Geert Uytterhoeven
2014-09-24 0:54 ` Simon Horman
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2014-09-04 6:31 UTC (permalink / raw)
To: linux-sh
Hi Magnus,
On Thu, Sep 4, 2014 at 5:39 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> @@ -58,7 +58,9 @@ void __init r8a7740_init_pm_domains(void
> rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
> pm_genpd_add_subdomain_names("A4S", "A3SP");
> }
> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> +#else
> +void r8a7740_init_pm_domains(void) {}
> +#endif
>
> #ifdef CONFIG_SUSPEND
> static int r8a7740_enter_suspend(suspend_state_t suspend_state)
> --- 0001/arch/arm/mach-shmobile/r8a7740.h
> +++ work/arch/arm/mach-shmobile/r8a7740.h 2014-09-04 12:29:12.000000000 +0900
> @@ -52,11 +52,6 @@ extern void r8a7740_add_standard_devices
> extern void r8a7740_clock_init(u8 md_ck);
> extern void r8a7740_pinmux_init(void);
> extern void r8a7740_pm_init(void);
> -
> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> -extern void __init r8a7740_init_pm_domains(void);
> -#else
> -static inline void r8a7740_init_pm_domains(void) {}
> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> +extern void r8a7740_init_pm_domains(void);
>
> #endif /* __ASM_R8A7740_H__ */
Why do you make r8a7740_init_pm_domains() out-of-line in the
!CONFIG_PM_RMOBILE case?
It increases code size (call to an empty function that cannot be optimized
away), and prevents us from building pm-r8a7740.c conditionally on
CONFIG_PM_RMOBILE and CONFIG_ARCH_R8A7740 in the future.
For the latter, we can turn pm-rmobile into a library, and do something
like
pm-rmobile-objs-$(CONFIG_ARCH_R8A7740) += pm-r8a7740.o
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
` (4 preceding siblings ...)
2014-09-04 6:31 ` Geert Uytterhoeven
@ 2014-09-24 0:54 ` Simon Horman
2014-09-30 4:25 ` Simon Horman
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2014-09-24 0:54 UTC (permalink / raw)
To: linux-sh
On Thu, Sep 04, 2014 at 08:31:45AM +0200, Geert Uytterhoeven wrote:
> Hi Magnus,
>
> On Thu, Sep 4, 2014 at 5:39 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> > @@ -58,7 +58,9 @@ void __init r8a7740_init_pm_domains(void
> > rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
> > pm_genpd_add_subdomain_names("A4S", "A3SP");
> > }
> > -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> > +#else
> > +void r8a7740_init_pm_domains(void) {}
> > +#endif
> >
> > #ifdef CONFIG_SUSPEND
> > static int r8a7740_enter_suspend(suspend_state_t suspend_state)
> > --- 0001/arch/arm/mach-shmobile/r8a7740.h
> > +++ work/arch/arm/mach-shmobile/r8a7740.h 2014-09-04 12:29:12.000000000 +0900
> > @@ -52,11 +52,6 @@ extern void r8a7740_add_standard_devices
> > extern void r8a7740_clock_init(u8 md_ck);
> > extern void r8a7740_pinmux_init(void);
> > extern void r8a7740_pm_init(void);
> > -
> > -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> > -extern void __init r8a7740_init_pm_domains(void);
> > -#else
> > -static inline void r8a7740_init_pm_domains(void) {}
> > -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> > +extern void r8a7740_init_pm_domains(void);
> >
> > #endif /* __ASM_R8A7740_H__ */
>
> Why do you make r8a7740_init_pm_domains() out-of-line in the
> !CONFIG_PM_RMOBILE case?
> It increases code size (call to an empty function that cannot be optimized
> away), and prevents us from building pm-r8a7740.c conditionally on
> CONFIG_PM_RMOBILE and CONFIG_ARCH_R8A7740 in the future.
>
> For the latter, we can turn pm-rmobile into a library, and do something
> like
>
> pm-rmobile-objs-$(CONFIG_ARCH_R8A7740) += pm-r8a7740.o
This seems to have slipped through the cracks.
For the first part of the problem how about this update to Magnus's patch?
[PATCH v1.1] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
Simplify the r8a7740 PM code by using CONFIG_PM_RMOBILE instead
of having a complex #if expression embedded in the C code.
Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
[horms+renesas@verge.net.au: do not make r8a7740_init_pm_domains
out-of-line for !CONFIG_PM_RMOBILE case]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
arch/arm/mach-shmobile/pm-r8a7740.c | 4 ++--
arch/arm/mach-shmobile/r8a7740.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-shmobile/pm-r8a7740.c b/arch/arm/mach-shmobile/pm-r8a7740.c
index e3f1464..225be5e 100644
--- a/arch/arm/mach-shmobile/pm-r8a7740.c
+++ b/arch/arm/mach-shmobile/pm-r8a7740.c
@@ -13,7 +13,7 @@
#include "common.h"
#include "pm-rmobile.h"
-#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
+#ifdef CONFIG_PM_RMOBILE
static int r8a7740_pd_a4s_suspend(void)
{
/*
@@ -56,7 +56,7 @@ void __init r8a7740_init_pm_domains(void)
rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
pm_genpd_add_subdomain_names("A4S", "A3SP");
}
-#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
+#endif
#ifdef CONFIG_SUSPEND
static int r8a7740_enter_suspend(suspend_state_t suspend_state)
diff --git a/arch/arm/mach-shmobile/r8a7740.h b/arch/arm/mach-shmobile/r8a7740.h
index f369b4b..5ba292e 100644
--- a/arch/arm/mach-shmobile/r8a7740.h
+++ b/arch/arm/mach-shmobile/r8a7740.h
@@ -53,10 +53,10 @@ extern void r8a7740_clock_init(u8 md_ck);
extern void r8a7740_pinmux_init(void);
extern void r8a7740_pm_init(void);
-#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
+#ifdef CONFIG_PM_RMOBILE
extern void __init r8a7740_init_pm_domains(void);
#else
static inline void r8a7740_init_pm_domains(void) {}
-#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
+#endif
#endif /* __ASM_R8A7740_H__ */
--
2.0.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
` (5 preceding siblings ...)
2014-09-24 0:54 ` Simon Horman
@ 2014-09-30 4:25 ` Simon Horman
2014-09-30 7:12 ` Geert Uytterhoeven
2014-09-30 8:11 ` Simon Horman
8 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2014-09-30 4:25 UTC (permalink / raw)
To: linux-sh
On Wed, Sep 24, 2014 at 09:54:49AM +0900, Simon Horman wrote:
> On Thu, Sep 04, 2014 at 08:31:45AM +0200, Geert Uytterhoeven wrote:
> > Hi Magnus,
> >
> > On Thu, Sep 4, 2014 at 5:39 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> > > @@ -58,7 +58,9 @@ void __init r8a7740_init_pm_domains(void
> > > rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
> > > pm_genpd_add_subdomain_names("A4S", "A3SP");
> > > }
> > > -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> > > +#else
> > > +void r8a7740_init_pm_domains(void) {}
> > > +#endif
> > >
> > > #ifdef CONFIG_SUSPEND
> > > static int r8a7740_enter_suspend(suspend_state_t suspend_state)
> > > --- 0001/arch/arm/mach-shmobile/r8a7740.h
> > > +++ work/arch/arm/mach-shmobile/r8a7740.h 2014-09-04 12:29:12.000000000 +0900
> > > @@ -52,11 +52,6 @@ extern void r8a7740_add_standard_devices
> > > extern void r8a7740_clock_init(u8 md_ck);
> > > extern void r8a7740_pinmux_init(void);
> > > extern void r8a7740_pm_init(void);
> > > -
> > > -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> > > -extern void __init r8a7740_init_pm_domains(void);
> > > -#else
> > > -static inline void r8a7740_init_pm_domains(void) {}
> > > -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> > > +extern void r8a7740_init_pm_domains(void);
> > >
> > > #endif /* __ASM_R8A7740_H__ */
> >
> > Why do you make r8a7740_init_pm_domains() out-of-line in the
> > !CONFIG_PM_RMOBILE case?
> > It increases code size (call to an empty function that cannot be optimized
> > away), and prevents us from building pm-r8a7740.c conditionally on
> > CONFIG_PM_RMOBILE and CONFIG_ARCH_R8A7740 in the future.
> >
> > For the latter, we can turn pm-rmobile into a library, and do something
> > like
> >
> > pm-rmobile-objs-$(CONFIG_ARCH_R8A7740) += pm-r8a7740.o
>
> This seems to have slipped through the cracks.
Geert, are you happy with this?
> For the first part of the problem how about this update to Magnus's patch?
>
> [PATCH v1.1] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
>
> Simplify the r8a7740 PM code by using CONFIG_PM_RMOBILE instead
> of having a complex #if expression embedded in the C code.
>
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> [horms+renesas@verge.net.au: do not make r8a7740_init_pm_domains
> out-of-line for !CONFIG_PM_RMOBILE case]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> arch/arm/mach-shmobile/pm-r8a7740.c | 4 ++--
> arch/arm/mach-shmobile/r8a7740.h | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/pm-r8a7740.c b/arch/arm/mach-shmobile/pm-r8a7740.c
> index e3f1464..225be5e 100644
> --- a/arch/arm/mach-shmobile/pm-r8a7740.c
> +++ b/arch/arm/mach-shmobile/pm-r8a7740.c
> @@ -13,7 +13,7 @@
> #include "common.h"
> #include "pm-rmobile.h"
>
> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> +#ifdef CONFIG_PM_RMOBILE
> static int r8a7740_pd_a4s_suspend(void)
> {
> /*
> @@ -56,7 +56,7 @@ void __init r8a7740_init_pm_domains(void)
> rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
> pm_genpd_add_subdomain_names("A4S", "A3SP");
> }
> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> +#endif
>
> #ifdef CONFIG_SUSPEND
> static int r8a7740_enter_suspend(suspend_state_t suspend_state)
> diff --git a/arch/arm/mach-shmobile/r8a7740.h b/arch/arm/mach-shmobile/r8a7740.h
> index f369b4b..5ba292e 100644
> --- a/arch/arm/mach-shmobile/r8a7740.h
> +++ b/arch/arm/mach-shmobile/r8a7740.h
> @@ -53,10 +53,10 @@ extern void r8a7740_clock_init(u8 md_ck);
> extern void r8a7740_pinmux_init(void);
> extern void r8a7740_pm_init(void);
>
> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> +#ifdef CONFIG_PM_RMOBILE
> extern void __init r8a7740_init_pm_domains(void);
> #else
> static inline void r8a7740_init_pm_domains(void) {}
> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> +#endif
>
> #endif /* __ASM_R8A7740_H__ */
> --
> 2.0.1
>
>
> --
> 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] 10+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
` (6 preceding siblings ...)
2014-09-30 4:25 ` Simon Horman
@ 2014-09-30 7:12 ` Geert Uytterhoeven
2014-09-30 8:11 ` Simon Horman
8 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2014-09-30 7:12 UTC (permalink / raw)
To: linux-sh
Hi Simon, Magnus,
On Tue, Sep 30, 2014 at 6:25 AM, Simon Horman <horms@verge.net.au> wrote:
> On Wed, Sep 24, 2014 at 09:54:49AM +0900, Simon Horman wrote:
>> On Thu, Sep 04, 2014 at 08:31:45AM +0200, Geert Uytterhoeven wrote:
>> > On Thu, Sep 4, 2014 at 5:39 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> > > @@ -58,7 +58,9 @@ void __init r8a7740_init_pm_domains(void
>> > > rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
>> > > pm_genpd_add_subdomain_names("A4S", "A3SP");
>> > > }
>> > > -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
>> > > +#else
>> > > +void r8a7740_init_pm_domains(void) {}
>> > > +#endif
>> > >
>> > > #ifdef CONFIG_SUSPEND
>> > > static int r8a7740_enter_suspend(suspend_state_t suspend_state)
>> > > --- 0001/arch/arm/mach-shmobile/r8a7740.h
>> > > +++ work/arch/arm/mach-shmobile/r8a7740.h 2014-09-04 12:29:12.000000000 +0900
>> > > @@ -52,11 +52,6 @@ extern void r8a7740_add_standard_devices
>> > > extern void r8a7740_clock_init(u8 md_ck);
>> > > extern void r8a7740_pinmux_init(void);
>> > > extern void r8a7740_pm_init(void);
>> > > -
>> > > -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
>> > > -extern void __init r8a7740_init_pm_domains(void);
>> > > -#else
>> > > -static inline void r8a7740_init_pm_domains(void) {}
>> > > -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
>> > > +extern void r8a7740_init_pm_domains(void);
>> > >
>> > > #endif /* __ASM_R8A7740_H__ */
>> >
>> > Why do you make r8a7740_init_pm_domains() out-of-line in the
>> > !CONFIG_PM_RMOBILE case?
>> > It increases code size (call to an empty function that cannot be optimized
>> > away), and prevents us from building pm-r8a7740.c conditionally on
>> > CONFIG_PM_RMOBILE and CONFIG_ARCH_R8A7740 in the future.
>> >
>> > For the latter, we can turn pm-rmobile into a library, and do something
>> > like
>> >
>> > pm-rmobile-objs-$(CONFIG_ARCH_R8A7740) += pm-r8a7740.o
>>
>> This seems to have slipped through the cracks.
>
> Geert, are you happy with this?
I'd like to ask to drop this patch, as I'll have to revert this hunk
in pm-r8a7740.c:
-#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
+#ifdef CONFIG_PM_RMOBILE
to add DT PM domain support.
Is that OK for you?
BTW, changing
-#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
+#if defined(CONFIG_PM_RMOBILE) && !defined(CONFIG_ARCH_MULTIPLATFORM)
is fine for me, though, but I don't know it's worth the effort.
>> For the first part of the problem how about this update to Magnus's patch?
>>
>> [PATCH v1.1] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
>>
>> Simplify the r8a7740 PM code by using CONFIG_PM_RMOBILE instead
>> of having a complex #if expression embedded in the C code.
>>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> [horms+renesas@verge.net.au: do not make r8a7740_init_pm_domains
>> out-of-line for !CONFIG_PM_RMOBILE case]
>> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>> ---
>> arch/arm/mach-shmobile/pm-r8a7740.c | 4 ++--
>> arch/arm/mach-shmobile/r8a7740.h | 4 ++--
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mach-shmobile/pm-r8a7740.c b/arch/arm/mach-shmobile/pm-r8a7740.c
>> index e3f1464..225be5e 100644
>> --- a/arch/arm/mach-shmobile/pm-r8a7740.c
>> +++ b/arch/arm/mach-shmobile/pm-r8a7740.c
>> @@ -13,7 +13,7 @@
>> #include "common.h"
>> #include "pm-rmobile.h"
>>
>> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
>> +#ifdef CONFIG_PM_RMOBILE
>> static int r8a7740_pd_a4s_suspend(void)
>> {
>> /*
>> @@ -56,7 +56,7 @@ void __init r8a7740_init_pm_domains(void)
>> rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
>> pm_genpd_add_subdomain_names("A4S", "A3SP");
>> }
>> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
>> +#endif
>>
>> #ifdef CONFIG_SUSPEND
>> static int r8a7740_enter_suspend(suspend_state_t suspend_state)
>> diff --git a/arch/arm/mach-shmobile/r8a7740.h b/arch/arm/mach-shmobile/r8a7740.h
>> index f369b4b..5ba292e 100644
>> --- a/arch/arm/mach-shmobile/r8a7740.h
>> +++ b/arch/arm/mach-shmobile/r8a7740.h
>> @@ -53,10 +53,10 @@ extern void r8a7740_clock_init(u8 md_ck);
>> extern void r8a7740_pinmux_init(void);
>> extern void r8a7740_pm_init(void);
>>
>> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
>> +#ifdef CONFIG_PM_RMOBILE
>> extern void __init r8a7740_init_pm_domains(void);
>> #else
>> static inline void r8a7740_init_pm_domains(void) {}
>> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
>> +#endif
>>
>> #endif /* __ASM_R8A7740_H__ */
>> --
>> 2.0.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
2014-09-04 3:39 [PATCH] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage Magnus Damm
` (7 preceding siblings ...)
2014-09-30 7:12 ` Geert Uytterhoeven
@ 2014-09-30 8:11 ` Simon Horman
8 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2014-09-30 8:11 UTC (permalink / raw)
To: linux-sh
On Tue, Sep 30, 2014 at 09:12:24AM +0200, Geert Uytterhoeven wrote:
> Hi Simon, Magnus,
>
> On Tue, Sep 30, 2014 at 6:25 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Sep 24, 2014 at 09:54:49AM +0900, Simon Horman wrote:
> >> On Thu, Sep 04, 2014 at 08:31:45AM +0200, Geert Uytterhoeven wrote:
> >> > On Thu, Sep 4, 2014 at 5:39 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >> > > @@ -58,7 +58,9 @@ void __init r8a7740_init_pm_domains(void
> >> > > rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
> >> > > pm_genpd_add_subdomain_names("A4S", "A3SP");
> >> > > }
> >> > > -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> >> > > +#else
> >> > > +void r8a7740_init_pm_domains(void) {}
> >> > > +#endif
> >> > >
> >> > > #ifdef CONFIG_SUSPEND
> >> > > static int r8a7740_enter_suspend(suspend_state_t suspend_state)
> >> > > --- 0001/arch/arm/mach-shmobile/r8a7740.h
> >> > > +++ work/arch/arm/mach-shmobile/r8a7740.h 2014-09-04 12:29:12.000000000 +0900
> >> > > @@ -52,11 +52,6 @@ extern void r8a7740_add_standard_devices
> >> > > extern void r8a7740_clock_init(u8 md_ck);
> >> > > extern void r8a7740_pinmux_init(void);
> >> > > extern void r8a7740_pm_init(void);
> >> > > -
> >> > > -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> >> > > -extern void __init r8a7740_init_pm_domains(void);
> >> > > -#else
> >> > > -static inline void r8a7740_init_pm_domains(void) {}
> >> > > -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> >> > > +extern void r8a7740_init_pm_domains(void);
> >> > >
> >> > > #endif /* __ASM_R8A7740_H__ */
> >> >
> >> > Why do you make r8a7740_init_pm_domains() out-of-line in the
> >> > !CONFIG_PM_RMOBILE case?
> >> > It increases code size (call to an empty function that cannot be optimized
> >> > away), and prevents us from building pm-r8a7740.c conditionally on
> >> > CONFIG_PM_RMOBILE and CONFIG_ARCH_R8A7740 in the future.
> >> >
> >> > For the latter, we can turn pm-rmobile into a library, and do something
> >> > like
> >> >
> >> > pm-rmobile-objs-$(CONFIG_ARCH_R8A7740) += pm-r8a7740.o
> >>
> >> This seems to have slipped through the cracks.
> >
> > Geert, are you happy with this?
>
> I'd like to ask to drop this patch, as I'll have to revert this hunk
> in pm-r8a7740.c:
>
> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> +#ifdef CONFIG_PM_RMOBILE
>
> to add DT PM domain support.
>
> Is that OK for you?
Yes, that is fine. I'll mark it as rejected.
> BTW, changing
>
> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> +#if defined(CONFIG_PM_RMOBILE) && !defined(CONFIG_ARCH_MULTIPLATFORM)
>
> is fine for me, though, but I don't know it's worth the effort.
I don't think it is worth the noise it creates.
>
> >> For the first part of the problem how about this update to Magnus's patch?
> >>
> >> [PATCH v1.1] ARM: shmobile: r8a7740: Cleanup PM Kconfig usage
> >>
> >> Simplify the r8a7740 PM code by using CONFIG_PM_RMOBILE instead
> >> of having a complex #if expression embedded in the C code.
> >>
> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >> [horms+renesas@verge.net.au: do not make r8a7740_init_pm_domains
> >> out-of-line for !CONFIG_PM_RMOBILE case]
> >> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >> ---
> >> arch/arm/mach-shmobile/pm-r8a7740.c | 4 ++--
> >> arch/arm/mach-shmobile/r8a7740.h | 4 ++--
> >> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-shmobile/pm-r8a7740.c b/arch/arm/mach-shmobile/pm-r8a7740.c
> >> index e3f1464..225be5e 100644
> >> --- a/arch/arm/mach-shmobile/pm-r8a7740.c
> >> +++ b/arch/arm/mach-shmobile/pm-r8a7740.c
> >> @@ -13,7 +13,7 @@
> >> #include "common.h"
> >> #include "pm-rmobile.h"
> >>
> >> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> >> +#ifdef CONFIG_PM_RMOBILE
> >> static int r8a7740_pd_a4s_suspend(void)
> >> {
> >> /*
> >> @@ -56,7 +56,7 @@ void __init r8a7740_init_pm_domains(void)
> >> rmobile_init_domains(r8a7740_pm_domains, ARRAY_SIZE(r8a7740_pm_domains));
> >> pm_genpd_add_subdomain_names("A4S", "A3SP");
> >> }
> >> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> >> +#endif
> >>
> >> #ifdef CONFIG_SUSPEND
> >> static int r8a7740_enter_suspend(suspend_state_t suspend_state)
> >> diff --git a/arch/arm/mach-shmobile/r8a7740.h b/arch/arm/mach-shmobile/r8a7740.h
> >> index f369b4b..5ba292e 100644
> >> --- a/arch/arm/mach-shmobile/r8a7740.h
> >> +++ b/arch/arm/mach-shmobile/r8a7740.h
> >> @@ -53,10 +53,10 @@ extern void r8a7740_clock_init(u8 md_ck);
> >> extern void r8a7740_pinmux_init(void);
> >> extern void r8a7740_pm_init(void);
> >>
> >> -#if defined(CONFIG_PM) && !defined(CONFIG_ARCH_MULTIPLATFORM)
> >> +#ifdef CONFIG_PM_RMOBILE
> >> extern void __init r8a7740_init_pm_domains(void);
> >> #else
> >> static inline void r8a7740_init_pm_domains(void) {}
> >> -#endif /* CONFIG_PM && !CONFIG_ARCH_MULTIPLATFORM */
> >> +#endif
> >>
> >> #endif /* __ASM_R8A7740_H__ */
> >> --
> >> 2.0.1
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
> --
> 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] 10+ messages in thread