SUPERH platform development
 help / color / mirror / Atom feed
* [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
@ 2014-05-01  1:09 Geert Uytterhoeven
  2014-05-01  4:27 ` Simon Horman
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-05-01  1:09 UTC (permalink / raw)
  To: linux-sh

If the kernel is built to support multi-ARM configuration with shmobile
support built in, then drivers/sh is not built. This contains the PM
runtime code in drivers/sh/pm_runtime.c, which implicitly enables the
module clocks for all devices, and thus is quite essential.
Without this, the state of clocks depends on implicit reset state, or on
the bootloader.

If ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that
bits that may conflict (drivers/sh/clk if the common clock framework is
enabled) or are not used (drivers/sh/intc), are not built.
Also, only enable the PM runtime code when actually running on a shmobile
SoCs that needs it.

ARCH_SHMOBILE_MULTI was added a while ago by commit
efacfce5f8a523457e9419a25d52fe39db00b26a ("ARM: shmobile: Introduce
ARCH_SHMOBILE_MULTI"), but drivers/sh was compiled for both
ARCH_SHMOBILE_LEGACY and ARCH_SHMOBILE_MULTI until commit
bf98c1eac1d4a6bcf00532e4fa41d8126cd6c187 ("ARM: Rename ARCH_SHMOBILE to
ARCH_SHMOBILE_LEGACY").

Inspired by a patch from Ben Dooks <ben.dooks@codethink.co.uk>.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This is a minimal solution to fix the issue for multi-platform shmobile
kernels in a multi-platform-friendly way.
As it touches drivers/sh-specific code only, Simom should still be able to
get it in v3.15, and backported to v3.14-stable.

This is an RFC for several reasons:
  1. I'm at ELC, and away from my hardware to test it properly (also on
     non-shmobile).
  2. Can we already reduce the list of SoCs to check for?
     E.g. is this needed for emev2, which doesn't have MSTP clocks?

v3:
  - Add of_machine_is_compatible() checks for multi-platform
  - Remove Reviewed-by, Tested-by, as too much has changed,
  - Assumed authorship, as not much is left from the original patch from
    Ben (Ben: is that OK?),
v2:
  - Add Reviewed-by, Tested-by
---
 drivers/Makefile        |    2 +-
 drivers/sh/Makefile     |   14 ++++++++------
 drivers/sh/pm_runtime.c |   23 +++++++++++++++++++++--
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index d05d81b19b50..7183b6af5dac 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -119,7 +119,7 @@ obj-$(CONFIG_SGI_SN)		+= sn/
 obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
 obj-$(CONFIG_SUPERH)		+= sh/
-obj-$(CONFIG_ARCH_SHMOBILE_LEGACY)	+= sh/
+obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
 ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 obj-y				+= clocksource/
 endif
diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile
index fc67f564f02c..788ed9b59b4e 100644
--- a/drivers/sh/Makefile
+++ b/drivers/sh/Makefile
@@ -1,10 +1,12 @@
 #
 # Makefile for the SuperH specific drivers.
 #
-obj-y	:= intc/
+obj-$(CONFIG_SUPERH)			+= intc/
+obj-$(CONFIG_ARCH_SHMOBILE_LEGACY)	+= intc/
+ifneq ($(CONFIG_COMMON_CLK),y)
+obj-$(CONFIG_HAVE_CLK)			+= clk/
+endif
+obj-$(CONFIG_MAPLE)			+= maple/
+obj-$(CONFIG_SUPERHYWAY)		+= superhyway/
 
-obj-$(CONFIG_HAVE_CLK)		+= clk/
-obj-$(CONFIG_MAPLE)		+= maple/
-obj-$(CONFIG_SUPERHYWAY)	+= superhyway/
-
-obj-y				+= pm_runtime.o
+obj-y					+= pm_runtime.o
diff --git a/drivers/sh/pm_runtime.c b/drivers/sh/pm_runtime.c
index 8afa5a4589f2..cb4cb9f26896 100644
--- a/drivers/sh/pm_runtime.c
+++ b/drivers/sh/pm_runtime.c
@@ -50,8 +50,25 @@ static struct pm_clk_notifier_block platform_bus_notifier = {
 	.con_ids = { NULL, },
 };
 
-static int __init sh_pm_runtime_init(void)
+static bool default_pm_on;
+
+int __init sh_pm_runtime_init(void)
 {
+#ifdef CONFIG_ARCH_SHMOBILE_MULTI
+	if (!of_machine_is_compatible("renesas,emev2") &&
+	    !of_machine_is_compatible("renesas,r7s72100") &&
+	    !of_machine_is_compatible("renesas,r8a73a4") &&
+	    !of_machine_is_compatible("renesas,r8a7740") &&
+	    !of_machine_is_compatible("renesas,r8a7778") &&
+	    !of_machine_is_compatible("renesas,r8a7779") &&
+	    !of_machine_is_compatible("renesas,r8a7790") &&
+	    !of_machine_is_compatible("renesas,r8a7791") &&
+	    !of_machine_is_compatible("renesas,sh7372") &&
+	    !of_machine_is_compatible("renesas,sh73a0"))
+		return 0;
+#endif
+
+	default_pm_on = true;
 	pm_clk_add_notifier(&platform_bus_type, &platform_bus_notifier);
 	return 0;
 }
@@ -59,7 +76,9 @@ core_initcall(sh_pm_runtime_init);
 
 static int __init sh_pm_runtime_late_init(void)
 {
-	pm_genpd_poweroff_unused();
+	if (default_pm_on)
+		pm_genpd_poweroff_unused();
 	return 0;
 }
+
 late_initcall(sh_pm_runtime_late_init);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
  2014-05-01  1:09 [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Geert Uytterhoeven
@ 2014-05-01  4:27 ` Simon Horman
  2014-05-01  4:42 ` Geert Uytterhoeven
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-05-01  4:27 UTC (permalink / raw)
  To: linux-sh

On Thu, May 01, 2014 at 03:09:42AM +0200, Geert Uytterhoeven wrote:
> If the kernel is built to support multi-ARM configuration with shmobile
> support built in, then drivers/sh is not built. This contains the PM
> runtime code in drivers/sh/pm_runtime.c, which implicitly enables the
> module clocks for all devices, and thus is quite essential.
> Without this, the state of clocks depends on implicit reset state, or on
> the bootloader.
> 
> If ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that
> bits that may conflict (drivers/sh/clk if the common clock framework is
> enabled) or are not used (drivers/sh/intc), are not built.
> Also, only enable the PM runtime code when actually running on a shmobile
> SoCs that needs it.
> 
> ARCH_SHMOBILE_MULTI was added a while ago by commit
> efacfce5f8a523457e9419a25d52fe39db00b26a ("ARM: shmobile: Introduce
> ARCH_SHMOBILE_MULTI"), but drivers/sh was compiled for both
> ARCH_SHMOBILE_LEGACY and ARCH_SHMOBILE_MULTI until commit
> bf98c1eac1d4a6bcf00532e4fa41d8126cd6c187 ("ARM: Rename ARCH_SHMOBILE to
> ARCH_SHMOBILE_LEGACY").
> 
> Inspired by a patch from Ben Dooks <ben.dooks@codethink.co.uk>.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> This is a minimal solution to fix the issue for multi-platform shmobile
> kernels in a multi-platform-friendly way.
> As it touches drivers/sh-specific code only, Simom should still be able to
> get it in v3.15, and backported to v3.14-stable.
> 
> This is an RFC for several reasons:
>   1. I'm at ELC, and away from my hardware to test it properly (also on
>      non-shmobile).
>   2. Can we already reduce the list of SoCs to check for?
>      E.g. is this needed for emev2, which doesn't have MSTP clocks?

I will answer a related question: "Simon, can you test this?".

If its just a matter of testing that the board still boots to user-space
I can do so without too much effort for most shmobile boards including
the emev2 based kzm9g. The exceptions are:

* The r7s72100 based genmai
  - I think you have access to that
* The r8a7791 based henniger board.
  - I do have access to a koelsh board which is bsed on the same SoC

If the testing is a bit more involved then I don't have an automation
set up for it at this stage. That makes testing a bit more difficult
for me. Especially as I am going on holidays tomorrow.

Of course there is the caveat that a number of SoCs and their boards do
not support multiplatform yet.

> v3:
>   - Add of_machine_is_compatible() checks for multi-platform
>   - Remove Reviewed-by, Tested-by, as too much has changed,
>   - Assumed authorship, as not much is left from the original patch from
>     Ben (Ben: is that OK?),
> v2:
>   - Add Reviewed-by, Tested-by
> ---
>  drivers/Makefile        |    2 +-
>  drivers/sh/Makefile     |   14 ++++++++------
>  drivers/sh/pm_runtime.c |   23 +++++++++++++++++++++--
>  3 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index d05d81b19b50..7183b6af5dac 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -119,7 +119,7 @@ obj-$(CONFIG_SGI_SN)		+= sn/
>  obj-y				+= firmware/
>  obj-$(CONFIG_CRYPTO)		+= crypto/
>  obj-$(CONFIG_SUPERH)		+= sh/
> -obj-$(CONFIG_ARCH_SHMOBILE_LEGACY)	+= sh/
> +obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
>  ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
>  obj-y				+= clocksource/
>  endif
> diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile
> index fc67f564f02c..788ed9b59b4e 100644
> --- a/drivers/sh/Makefile
> +++ b/drivers/sh/Makefile
> @@ -1,10 +1,12 @@
>  #
>  # Makefile for the SuperH specific drivers.
>  #
> -obj-y	:= intc/
> +obj-$(CONFIG_SUPERH)			+= intc/
> +obj-$(CONFIG_ARCH_SHMOBILE_LEGACY)	+= intc/
> +ifneq ($(CONFIG_COMMON_CLK),y)
> +obj-$(CONFIG_HAVE_CLK)			+= clk/
> +endif
> +obj-$(CONFIG_MAPLE)			+= maple/
> +obj-$(CONFIG_SUPERHYWAY)		+= superhyway/
>  
> -obj-$(CONFIG_HAVE_CLK)		+= clk/
> -obj-$(CONFIG_MAPLE)		+= maple/
> -obj-$(CONFIG_SUPERHYWAY)	+= superhyway/
> -
> -obj-y				+= pm_runtime.o
> +obj-y					+= pm_runtime.o
> diff --git a/drivers/sh/pm_runtime.c b/drivers/sh/pm_runtime.c
> index 8afa5a4589f2..cb4cb9f26896 100644
> --- a/drivers/sh/pm_runtime.c
> +++ b/drivers/sh/pm_runtime.c
> @@ -50,8 +50,25 @@ static struct pm_clk_notifier_block platform_bus_notifier = {
>  	.con_ids = { NULL, },
>  };
>  
> -static int __init sh_pm_runtime_init(void)
> +static bool default_pm_on;
> +
> +int __init sh_pm_runtime_init(void)
>  {
> +#ifdef CONFIG_ARCH_SHMOBILE_MULTI
> +	if (!of_machine_is_compatible("renesas,emev2") &&
> +	    !of_machine_is_compatible("renesas,r7s72100") &&
> +	    !of_machine_is_compatible("renesas,r8a73a4") &&
> +	    !of_machine_is_compatible("renesas,r8a7740") &&
> +	    !of_machine_is_compatible("renesas,r8a7778") &&
> +	    !of_machine_is_compatible("renesas,r8a7779") &&
> +	    !of_machine_is_compatible("renesas,r8a7790") &&
> +	    !of_machine_is_compatible("renesas,r8a7791") &&
> +	    !of_machine_is_compatible("renesas,sh7372") &&
> +	    !of_machine_is_compatible("renesas,sh73a0"))
> +		return 0;
> +#endif
> +
> +	default_pm_on = true;
>  	pm_clk_add_notifier(&platform_bus_type, &platform_bus_notifier);
>  	return 0;
>  }
> @@ -59,7 +76,9 @@ core_initcall(sh_pm_runtime_init);
>  
>  static int __init sh_pm_runtime_late_init(void)
>  {
> -	pm_genpd_poweroff_unused();
> +	if (default_pm_on)
> +		pm_genpd_poweroff_unused();
>  	return 0;
>  }
> +
>  late_initcall(sh_pm_runtime_late_init);
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
  2014-05-01  1:09 [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Geert Uytterhoeven
  2014-05-01  4:27 ` Simon Horman
@ 2014-05-01  4:42 ` Geert Uytterhoeven
  2014-05-01  4:58 ` Simon Horman
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-05-01  4:42 UTC (permalink / raw)
  To: linux-sh

Hi Simon,

On Thu, May 1, 2014 at 6:27 AM, Simon Horman <horms@verge.net.au> wrote:
> On Thu, May 01, 2014 at 03:09:42AM +0200, Geert Uytterhoeven wrote:
>> If the kernel is built to support multi-ARM configuration with shmobile
>> support built in, then drivers/sh is not built. This contains the PM
>> runtime code in drivers/sh/pm_runtime.c, which implicitly enables the
>> module clocks for all devices, and thus is quite essential.
>> Without this, the state of clocks depends on implicit reset state, or on
>> the bootloader.
>>
>> If ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that
>> bits that may conflict (drivers/sh/clk if the common clock framework is
>> enabled) or are not used (drivers/sh/intc), are not built.
>> Also, only enable the PM runtime code when actually running on a shmobile
>> SoCs that needs it.
>>
>> ARCH_SHMOBILE_MULTI was added a while ago by commit
>> efacfce5f8a523457e9419a25d52fe39db00b26a ("ARM: shmobile: Introduce
>> ARCH_SHMOBILE_MULTI"), but drivers/sh was compiled for both
>> ARCH_SHMOBILE_LEGACY and ARCH_SHMOBILE_MULTI until commit
>> bf98c1eac1d4a6bcf00532e4fa41d8126cd6c187 ("ARM: Rename ARCH_SHMOBILE to
>> ARCH_SHMOBILE_LEGACY").
>>
>> Inspired by a patch from Ben Dooks <ben.dooks@codethink.co.uk>.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> This is a minimal solution to fix the issue for multi-platform shmobile
>> kernels in a multi-platform-friendly way.
>> As it touches drivers/sh-specific code only, Simom should still be able to
>> get it in v3.15, and backported to v3.14-stable.
>>
>> This is an RFC for several reasons:
>>   1. I'm at ELC, and away from my hardware to test it properly (also on
>>      non-shmobile).
>>   2. Can we already reduce the list of SoCs to check for?
>>      E.g. is this needed for emev2, which doesn't have MSTP clocks?

According to Magnus, emev2 doesn't rely on Runtime PM, so it can be
removed from the list.

> I will answer a related question: "Simon, can you test this?".

;-)

> If its just a matter of testing that the board still boots to user-space

Preferably after removing the clk_enables[] workarounds, and with MSTP
disable patches ;-)

> I can do so without too much effort for most shmobile boards including
> the emev2 based kzm9g. The exceptions are:
>
> * The r7s72100 based genmai
>   - I think you have access to that
> * The r8a7791 based henniger board.
>   - I do have access to a koelsh board which is bsed on the same SoC

Well, we shouldn't rush to get it in. 3.15 is still a few weeks away...

I can test it on Koelsch, Genmai, BeagleBone Black, and Armadillo after
ELC.

> If the testing is a bit more involved then I don't have an automation
> set up for it at this stage. That makes testing a bit more difficult
> for me. Especially as I am going on holidays tomorrow.

Sure, I understand. Enjoy your holidays! After which rc will you return?

> Of course there is the caveat that a number of SoCs and their boards do
> not support multiplatform yet.

Yep. But as long as there are no typos in the compatible checks, testing
well on one shmobile board and one non-shmobile board should suffice,
right?

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] 9+ messages in thread

* Re: [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
  2014-05-01  1:09 [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Geert Uytterhoeven
  2014-05-01  4:27 ` Simon Horman
  2014-05-01  4:42 ` Geert Uytterhoeven
@ 2014-05-01  4:58 ` Simon Horman
  2014-05-03 19:29 ` Magnus Damm
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-05-01  4:58 UTC (permalink / raw)
  To: linux-sh

On Thu, May 01, 2014 at 06:42:18AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, May 1, 2014 at 6:27 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, May 01, 2014 at 03:09:42AM +0200, Geert Uytterhoeven wrote:
> >> If the kernel is built to support multi-ARM configuration with shmobile
> >> support built in, then drivers/sh is not built. This contains the PM
> >> runtime code in drivers/sh/pm_runtime.c, which implicitly enables the
> >> module clocks for all devices, and thus is quite essential.
> >> Without this, the state of clocks depends on implicit reset state, or on
> >> the bootloader.
> >>
> >> If ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that
> >> bits that may conflict (drivers/sh/clk if the common clock framework is
> >> enabled) or are not used (drivers/sh/intc), are not built.
> >> Also, only enable the PM runtime code when actually running on a shmobile
> >> SoCs that needs it.
> >>
> >> ARCH_SHMOBILE_MULTI was added a while ago by commit
> >> efacfce5f8a523457e9419a25d52fe39db00b26a ("ARM: shmobile: Introduce
> >> ARCH_SHMOBILE_MULTI"), but drivers/sh was compiled for both
> >> ARCH_SHMOBILE_LEGACY and ARCH_SHMOBILE_MULTI until commit
> >> bf98c1eac1d4a6bcf00532e4fa41d8126cd6c187 ("ARM: Rename ARCH_SHMOBILE to
> >> ARCH_SHMOBILE_LEGACY").
> >>
> >> Inspired by a patch from Ben Dooks <ben.dooks@codethink.co.uk>.
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >> This is a minimal solution to fix the issue for multi-platform shmobile
> >> kernels in a multi-platform-friendly way.
> >> As it touches drivers/sh-specific code only, Simom should still be able to
> >> get it in v3.15, and backported to v3.14-stable.
> >>
> >> This is an RFC for several reasons:
> >>   1. I'm at ELC, and away from my hardware to test it properly (also on
> >>      non-shmobile).
> >>   2. Can we already reduce the list of SoCs to check for?
> >>      E.g. is this needed for emev2, which doesn't have MSTP clocks?
> 
> According to Magnus, emev2 doesn't rely on Runtime PM, so it can be
> removed from the list.
> 
> > I will answer a related question: "Simon, can you test this?".
> 
> ;-)
> 
> > If its just a matter of testing that the board still boots to user-space
> 
> Preferably after removing the clk_enables[] workarounds, and with MSTP
> disable patches ;-)

Ok, assuming such patches exist :)

> > I can do so without too much effort for most shmobile boards including
> > the emev2 based kzm9g. The exceptions are:
> >
> > * The r7s72100 based genmai
> >   - I think you have access to that
> > * The r8a7791 based henniger board.
> >   - I do have access to a koelsh board which is bsed on the same SoC
> 
> Well, we shouldn't rush to get it in. 3.15 is still a few weeks away...

Agreed.

> I can test it on Koelsch, Genmai, BeagleBone Black, and Armadillo after
> ELC.
> 
> > If the testing is a bit more involved then I don't have an automation
> > set up for it at this stage. That makes testing a bit more difficult
> > for me. Especially as I am going on holidays tomorrow.
> 
> Sure, I understand. Enjoy your holidays! After which rc will you return?

I expect to be back around rc5.
Or in more concrete terms, on the 13th.

> > Of course there is the caveat that a number of SoCs and their boards do
> > not support multiplatform yet.
> 
> Yep. But as long as there are no typos in the compatible checks, testing
> well on one shmobile board and one non-shmobile board should suffice,
> right?

Most likely.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
  2014-05-01  1:09 [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2014-05-01  4:58 ` Simon Horman
@ 2014-05-03 19:29 ` Magnus Damm
  2014-05-06 10:45 ` Ben Dooks
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2014-05-03 19:29 UTC (permalink / raw)
  To: linux-sh

Hi Geert,

On Thu, May 1, 2014 at 4:42 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Simon,
>
> On Thu, May 1, 2014 at 6:27 AM, Simon Horman <horms@verge.net.au> wrote:
>> On Thu, May 01, 2014 at 03:09:42AM +0200, Geert Uytterhoeven wrote:
>>> If the kernel is built to support multi-ARM configuration with shmobile
>>> support built in, then drivers/sh is not built. This contains the PM
>>> runtime code in drivers/sh/pm_runtime.c, which implicitly enables the
>>> module clocks for all devices, and thus is quite essential.
>>> Without this, the state of clocks depends on implicit reset state, or on
>>> the bootloader.
>>>
>>> If ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that
>>> bits that may conflict (drivers/sh/clk if the common clock framework is
>>> enabled) or are not used (drivers/sh/intc), are not built.
>>> Also, only enable the PM runtime code when actually running on a shmobile
>>> SoCs that needs it.
>>>
>>> ARCH_SHMOBILE_MULTI was added a while ago by commit
>>> efacfce5f8a523457e9419a25d52fe39db00b26a ("ARM: shmobile: Introduce
>>> ARCH_SHMOBILE_MULTI"), but drivers/sh was compiled for both
>>> ARCH_SHMOBILE_LEGACY and ARCH_SHMOBILE_MULTI until commit
>>> bf98c1eac1d4a6bcf00532e4fa41d8126cd6c187 ("ARM: Rename ARCH_SHMOBILE to
>>> ARCH_SHMOBILE_LEGACY").
>>>
>>> Inspired by a patch from Ben Dooks <ben.dooks@codethink.co.uk>.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> This is a minimal solution to fix the issue for multi-platform shmobile
>>> kernels in a multi-platform-friendly way.
>>> As it touches drivers/sh-specific code only, Simom should still be able to
>>> get it in v3.15, and backported to v3.14-stable.
>>>
>>> This is an RFC for several reasons:
>>>   1. I'm at ELC, and away from my hardware to test it properly (also on
>>>      non-shmobile).
>>>   2. Can we already reduce the list of SoCs to check for?
>>>      E.g. is this needed for emev2, which doesn't have MSTP clocks?

Thanks a lot for your efforts! I think this patch makes sense as next
step for Runtime PM since it will allow us to use DT-only for board
support together with Runtime PM.

> According to Magnus, emev2 doesn't rely on Runtime PM, so it can be
> removed from the list.

Correct, but let me clarify a bit. At this point we are not using
Runtime PM in any driver used by EMEV2. At some point it would however
be good to use Runtime PM, so I don't think it would hurt enabling it.
Actually, I think we should enable it for all our SoCs, so matching on
"renesas," may make even more sense.

Cheers,

/ magnus

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
  2014-05-01  1:09 [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Geert Uytterhoeven
                   ` (3 preceding siblings ...)
  2014-05-03 19:29 ` Magnus Damm
@ 2014-05-06 10:45 ` Ben Dooks
  2014-05-06 21:15 ` Geert Uytterhoeven
  2014-05-06 21:17 ` Geert Uytterhoeven
  6 siblings, 0 replies; 9+ messages in thread
From: Ben Dooks @ 2014-05-06 10:45 UTC (permalink / raw)
  To: linux-sh

On 01/05/14 02:09, Geert Uytterhoeven wrote:
> If the kernel is built to support multi-ARM configuration with shmobile
> support built in, then drivers/sh is not built. This contains the PM
> runtime code in drivers/sh/pm_runtime.c, which implicitly enables the
> module clocks for all devices, and thus is quite essential.
> Without this, the state of clocks depends on implicit reset state, or on
> the bootloader.
>
> If ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that
> bits that may conflict (drivers/sh/clk if the common clock framework is
> enabled) or are not used (drivers/sh/intc), are not built.
> Also, only enable the PM runtime code when actually running on a shmobile
> SoCs that needs it.
>
> ARCH_SHMOBILE_MULTI was added a while ago by commit
> efacfce5f8a523457e9419a25d52fe39db00b26a ("ARM: shmobile: Introduce
> ARCH_SHMOBILE_MULTI"), but drivers/sh was compiled for both
> ARCH_SHMOBILE_LEGACY and ARCH_SHMOBILE_MULTI until commit
> bf98c1eac1d4a6bcf00532e4fa41d8126cd6c187 ("ARM: Rename ARCH_SHMOBILE to
> ARCH_SHMOBILE_LEGACY").
>
> Inspired by a patch from Ben Dooks <ben.dooks@codethink.co.uk>.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> This is a minimal solution to fix the issue for multi-platform shmobile
> kernels in a multi-platform-friendly way.
> As it touches drivers/sh-specific code only, Simom should still be able to
> get it in v3.15, and backported to v3.14-stable.
>
> This is an RFC for several reasons:
>    1. I'm at ELC, and away from my hardware to test it properly (also on
>       non-shmobile).
>    2. Can we already reduce the list of SoCs to check for?
>       E.g. is this needed for emev2, which doesn't have MSTP clocks?

 From what I remember the EMEV2 does have hardware clock gates for
IP, just not the same way as the newer SoCs have. I have not tried
any newer kernels with this as the project I was on which used this
device is no-longer going.

> v3:
>    - Add of_machine_is_compatible() checks for multi-platform
>    - Remove Reviewed-by, Tested-by, as too much has changed,
>    - Assumed authorship, as not much is left from the original patch from
>      Ben (Ben: is that OK?),

Yes, that would fine. It looks like there is enough changes to
drop that. Thanks for sorting this out.

> -static int __init sh_pm_runtime_init(void)
> +static bool default_pm_on;
> +
> +int __init sh_pm_runtime_init(void)
>   {
> +#ifdef CONFIG_ARCH_SHMOBILE_MULTI
> +	if (!of_machine_is_compatible("renesas,emev2") &&
> +	    !of_machine_is_compatible("renesas,r7s72100") &&
> +	    !of_machine_is_compatible("renesas,r8a73a4") &&
> +	    !of_machine_is_compatible("renesas,r8a7740") &&
> +	    !of_machine_is_compatible("renesas,r8a7778") &&
> +	    !of_machine_is_compatible("renesas,r8a7779") &&
> +	    !of_machine_is_compatible("renesas,r8a7790") &&
> +	    !of_machine_is_compatible("renesas,r8a7791") &&
> +	    !of_machine_is_compatible("renesas,sh7372") &&
> +	    !of_machine_is_compatible("renesas,sh73a0"))
> +		return 0;
> +#endif

Is there a more generic compatible that could be matched on?

Could we do an if (IS_ENABLED(CONFIG_ARCH_SHMOBILE_MULTI)) instead
of an #ifdef block?

> +
> +	default_pm_on = true;
>   	pm_clk_add_notifier(&platform_bus_type, &platform_bus_notifier);
>   	return 0;
>   }
> @@ -59,7 +76,9 @@ core_initcall(sh_pm_runtime_init);
>
>   static int __init sh_pm_runtime_late_init(void)
>   {
> -	pm_genpd_poweroff_unused();
> +	if (default_pm_on)
> +		pm_genpd_poweroff_unused();
>   	return 0;
>   }
> +
>   late_initcall(sh_pm_runtime_late_init);
>


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
  2014-05-01  1:09 [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Geert Uytterhoeven
                   ` (4 preceding siblings ...)
  2014-05-06 10:45 ` Ben Dooks
@ 2014-05-06 21:15 ` Geert Uytterhoeven
  2014-05-06 21:17 ` Geert Uytterhoeven
  6 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-05-06 21:15 UTC (permalink / raw)
  To: linux-sh

Hi Magnus,

On Sat, May 3, 2014 at 9:29 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> According to Magnus, emev2 doesn't rely on Runtime PM, so it can be
>> removed from the list.
>
> Correct, but let me clarify a bit. At this point we are not using
> Runtime PM in any driver used by EMEV2. At some point it would however
> be good to use Runtime PM, so I don't think it would hurt enabling it.

OK, so let's keep it.

> Actually, I think we should enable it for all our SoCs, so matching on
> "renesas," may make even more sense.

Is there an easy way to match on "renesas,"? I haven't found one yet.

Anyway, I'm a bit reluctant to do that. The final fix may be SoC-dependent,
so having all SoCs listed explicitly allows to selectively remove lines when
needed.

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] 9+ messages in thread

* Re: [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
  2014-05-01  1:09 [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Geert Uytterhoeven
                   ` (5 preceding siblings ...)
  2014-05-06 21:15 ` Geert Uytterhoeven
@ 2014-05-06 21:17 ` Geert Uytterhoeven
  6 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-05-06 21:17 UTC (permalink / raw)
  To: linux-sh

Hi Ben,

On Tue, May 6, 2014 at 12:45 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>>    2. Can we already reduce the list of SoCs to check for?
>>       E.g. is this needed for emev2, which doesn't have MSTP clocks?
>
> From what I remember the EMEV2 does have hardware clock gates for
> IP, just not the same way as the newer SoCs have. I have not tried
> any newer kernels with this as the project I was on which used this
> device is no-longer going.

OK, will keep.

>> v3:
>>    - Add of_machine_is_compatible() checks for multi-platform
>>    - Remove Reviewed-by, Tested-by, as too much has changed,
>>    - Assumed authorship, as not much is left from the original patch from
>>      Ben (Ben: is that OK?),
>
> Yes, that would fine. It looks like there is enough changes to
> drop that. Thanks for sorting this out.

Thanks!

>> -static int __init sh_pm_runtime_init(void)
>> +static bool default_pm_on;
>> +
>> +int __init sh_pm_runtime_init(void)
>>   {
>> +#ifdef CONFIG_ARCH_SHMOBILE_MULTI
>> +       if (!of_machine_is_compatible("renesas,emev2") &&
>> +           !of_machine_is_compatible("renesas,r7s72100") &&
>> +           !of_machine_is_compatible("renesas,r8a73a4") &&
>> +           !of_machine_is_compatible("renesas,r8a7740") &&
>> +           !of_machine_is_compatible("renesas,r8a7778") &&
>> +           !of_machine_is_compatible("renesas,r8a7779") &&
>> +           !of_machine_is_compatible("renesas,r8a7790") &&
>> +           !of_machine_is_compatible("renesas,r8a7791") &&
>> +           !of_machine_is_compatible("renesas,sh7372") &&
>> +           !of_machine_is_compatible("renesas,sh73a0"))
>> +               return 0;
>> +#endif
>
> Is there a more generic compatible that could be matched on?

As I already replied to Magnus, I don't think so.

> Could we do an if (IS_ENABLED(CONFIG_ARCH_SHMOBILE_MULTI)) instead
> of an #ifdef block?

Good idea, that will increase compile coverage, while, the compiler will
still remove it when appropriate.

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] 9+ messages in thread

* [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI
  2014-05-13  7:42 [GIT PULL] SH Driver Updates for v3.15 Simon Horman
@ 2014-05-13  7:42 ` Simon Horman
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-05-13  7:42 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-sh, linux-kernel, Magnus Damm, Paul Mundt,
	Geert Uytterhoeven, Ben Dooks, Geert Uytterhoeven, Simon Horman

From: Geert Uytterhoeven <geert+renesas@glider.be>

If the kernel is built to support multi-ARM configuration with shmobile
support built in, then drivers/sh is not built. This contains the PM
runtime code in drivers/sh/pm_runtime.c, which implicitly enables the
module clocks for all devices, and thus is quite essential.
Without this, the state of clocks depends on implicit reset state, or on
the bootloader.

If ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that
bits that may conflict (drivers/sh/clk if the common clock framework is
enabled) or are not used (drivers/sh/intc), are not built.
Also, only enable the PM runtime code when actually running on a shmobile
SoCs that needs it.

ARCH_SHMOBILE_MULTI was added a while ago by commit
efacfce5f8a523457e9419a25d52fe39db00b26a ("ARM: shmobile: Introduce
ARCH_SHMOBILE_MULTI"), but drivers/sh was compiled for both
ARCH_SHMOBILE_LEGACY and ARCH_SHMOBILE_MULTI until commit
bf98c1eac1d4a6bcf00532e4fa41d8126cd6c187 ("ARM: Rename ARCH_SHMOBILE to
ARCH_SHMOBILE_LEGACY").

Inspired by a patch from Ben Dooks <ben.dooks@codethink.co.uk>.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 drivers/Makefile        |  2 +-
 drivers/sh/Makefile     | 14 ++++++++------
 drivers/sh/pm_runtime.c | 20 +++++++++++++++++++-
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index e3ced91..c033798 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -119,7 +119,7 @@ obj-$(CONFIG_SGI_SN)		+= sn/
 obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
 obj-$(CONFIG_SUPERH)		+= sh/
-obj-$(CONFIG_ARCH_SHMOBILE_LEGACY)	+= sh/
+obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
 ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 obj-y				+= clocksource/
 endif
diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile
index fc67f56..788ed9b 100644
--- a/drivers/sh/Makefile
+++ b/drivers/sh/Makefile
@@ -1,10 +1,12 @@
 #
 # Makefile for the SuperH specific drivers.
 #
-obj-y	:= intc/
+obj-$(CONFIG_SUPERH)			+= intc/
+obj-$(CONFIG_ARCH_SHMOBILE_LEGACY)	+= intc/
+ifneq ($(CONFIG_COMMON_CLK),y)
+obj-$(CONFIG_HAVE_CLK)			+= clk/
+endif
+obj-$(CONFIG_MAPLE)			+= maple/
+obj-$(CONFIG_SUPERHYWAY)		+= superhyway/
 
-obj-$(CONFIG_HAVE_CLK)		+= clk/
-obj-$(CONFIG_MAPLE)		+= maple/
-obj-$(CONFIG_SUPERHYWAY)	+= superhyway/
-
-obj-y				+= pm_runtime.o
+obj-y					+= pm_runtime.o
diff --git a/drivers/sh/pm_runtime.c b/drivers/sh/pm_runtime.c
index 8afa5a4..10c65eb 100644
--- a/drivers/sh/pm_runtime.c
+++ b/drivers/sh/pm_runtime.c
@@ -50,8 +50,25 @@ static struct pm_clk_notifier_block platform_bus_notifier = {
 	.con_ids = { NULL, },
 };
 
+static bool default_pm_on;
+
 static int __init sh_pm_runtime_init(void)
 {
+	if (IS_ENABLED(CONFIG_ARCH_SHMOBILE_MULTI)) {
+		if (!of_machine_is_compatible("renesas,emev2") &&
+		    !of_machine_is_compatible("renesas,r7s72100") &&
+		    !of_machine_is_compatible("renesas,r8a73a4") &&
+		    !of_machine_is_compatible("renesas,r8a7740") &&
+		    !of_machine_is_compatible("renesas,r8a7778") &&
+		    !of_machine_is_compatible("renesas,r8a7779") &&
+		    !of_machine_is_compatible("renesas,r8a7790") &&
+		    !of_machine_is_compatible("renesas,r8a7791") &&
+		    !of_machine_is_compatible("renesas,sh7372") &&
+		    !of_machine_is_compatible("renesas,sh73a0"))
+			return 0;
+	}
+
+	default_pm_on = true;
 	pm_clk_add_notifier(&platform_bus_type, &platform_bus_notifier);
 	return 0;
 }
@@ -59,7 +76,8 @@ core_initcall(sh_pm_runtime_init);
 
 static int __init sh_pm_runtime_late_init(void)
 {
-	pm_genpd_poweroff_unused();
+	if (default_pm_on)
+		pm_genpd_poweroff_unused();
 	return 0;
 }
 late_initcall(sh_pm_runtime_late_init);
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-05-13  7:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-01  1:09 [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Geert Uytterhoeven
2014-05-01  4:27 ` Simon Horman
2014-05-01  4:42 ` Geert Uytterhoeven
2014-05-01  4:58 ` Simon Horman
2014-05-03 19:29 ` Magnus Damm
2014-05-06 10:45 ` Ben Dooks
2014-05-06 21:15 ` Geert Uytterhoeven
2014-05-06 21:17 ` Geert Uytterhoeven
  -- strict thread matches above, loose matches on Subject: below --
2014-05-13  7:42 [GIT PULL] SH Driver Updates for v3.15 Simon Horman
2014-05-13  7:42 ` [PATCH] drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox