2015-05-20 22:36:00

by Stefan Agner

[permalink] [raw]
Subject: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

Use the new config symbol ARM_SINGLE_ARMV7M which groups config
symbols used by modern ARMv7-M platforms. This allows supporting
multiple ARMv7-M platforms in one kernel image. However, a common
kernel image requires the combined platforms to share the same
main memory layout to be bootable.

Acked-by: Uwe Kleine-König <[email protected]>
Signed-off-by: Stefan Agner <[email protected]>
---
Since this is essentially only a shift of config symbols, it
should not change runtime behavior, at least when selecting
only one platform.

Uwe, this is essentially the same I had in my patchset, just
converting the other platforms too. I was bold and added your
Ack anyway...

Joachim, Maxime, I test compiled with your defconfigs, compiled
fine on my machine.

arch/arm/Kconfig | 86 ++++++++++++++++++--------------------------------
arch/arm/Kconfig.debug | 5 ++-
2 files changed, 32 insertions(+), 59 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 75920ed..9b777e3 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -334,6 +334,7 @@ config ARM_SINGLE_ARMV7M
depends on !MMU
select ARCH_WANT_OPTIONAL_GPIOLIB
select ARM_NVIC
+ select AUTO_ZRELADDR
select CLKSRC_OF
select COMMON_CLK
select CPU_V7M
@@ -411,24 +412,6 @@ config ARCH_EBSA110
Ethernet interface, two PCMCIA sockets, two serial ports and a
parallel port.

-config ARCH_EFM32
- bool "Energy Micro efm32"
- depends on !MMU
- select ARCH_REQUIRE_GPIOLIB
- select ARM_NVIC
- select AUTO_ZRELADDR
- select CLKSRC_OF
- select COMMON_CLK
- select CPU_V7M
- select GENERIC_CLOCKEVENTS
- select NO_DMA
- select NO_IOPORT_MAP
- select SPARSE_IRQ
- select USE_OF
- help
- Support for Energy Micro's (now Silicon Labs) efm32 Giant Gecko
- processors.
-
config ARCH_EP93XX
bool "EP93xx-based"
select ARCH_HAS_HOLES_MEMORYMODEL
@@ -599,26 +582,6 @@ config ARCH_W90X900
<http://www.nuvoton.com/hq/enu/ProductAndSales/ProductLines/
ConsumerElectronicsIC/ARMMicrocontroller/ARMMicrocontroller>

-config ARCH_LPC18XX
- bool "NXP LPC18xx/LPC43xx"
- depends on !MMU
- select ARCH_HAS_RESET_CONTROLLER
- select ARCH_REQUIRE_GPIOLIB
- select ARM_AMBA
- select ARM_NVIC
- select AUTO_ZRELADDR
- select CLKSRC_LPC32XX
- select COMMON_CLK
- select CPU_V7M
- select GENERIC_CLOCKEVENTS
- select NO_IOPORT_MAP
- select PINCTRL
- select SPARSE_IRQ
- select USE_OF
- help
- Support for NXP's LPC18xx Cortex-M3 and LPC43xx Cortex-M4
- high performance microcontrollers.
-
config ARCH_LPC32XX
bool "NXP LPC32XX"
select ARCH_REQUIRE_GPIOLIB
@@ -791,24 +754,6 @@ config ARCH_OMAP1
help
Support for older TI OMAP1 (omap7xx, omap15xx or omap16xx)

-config ARCH_STM32
- bool "STMicrolectronics STM32"
- depends on !MMU
- select ARCH_HAS_RESET_CONTROLLER
- select ARM_NVIC
- select ARMV7M_SYSTICK
- select AUTO_ZRELADDR
- select CLKSRC_OF
- select COMMON_CLK
- select CPU_V7M
- select GENERIC_CLOCKEVENTS
- select NO_IOPORT_MAP
- select RESET_CONTROLLER
- select SPARSE_IRQ
- select USE_OF
- help
- Support for STMicroelectronics STM32 processors.
-
endchoice

menu "Multiple platform selection"
@@ -1006,6 +951,35 @@ source "arch/arm/mach-zx/Kconfig"

source "arch/arm/mach-zynq/Kconfig"

+# ARMv7-M architecture
+config ARCH_EFM32
+ bool "Energy Micro efm32"
+ depends on ARM_SINGLE_ARMV7M
+ select ARCH_REQUIRE_GPIOLIB
+ help
+ Support for Energy Micro's (now Silicon Labs) efm32 Giant Gecko
+ processors.
+
+config ARCH_LPC18XX
+ bool "NXP LPC18xx/LPC43xx"
+ depends on ARM_SINGLE_ARMV7M
+ select ARCH_HAS_RESET_CONTROLLER
+ select ARM_AMBA
+ select CLKSRC_LPC32XX
+ select PINCTRL
+ help
+ Support for NXP's LPC18xx Cortex-M3 and LPC43xx Cortex-M4
+ high performance microcontrollers.
+
+config ARCH_STM32
+ bool "STMicrolectronics STM32"
+ depends on ARM_SINGLE_ARMV7M
+ select ARCH_HAS_RESET_CONTROLLER
+ select ARMV7M_SYSTICK
+ select RESET_CONTROLLER
+ help
+ Support for STMicroelectronics STM32 processors.
+
# Definitions to make life easier
config ARCH_ACORN
bool
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 9553759..e27fd3f 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1596,9 +1596,8 @@ config DEBUG_UNCOMPRESS
config UNCOMPRESS_INCLUDE
string
default "debug/uncompress.h" if ARCH_MULTIPLATFORM || ARCH_MSM || \
- PLAT_SAMSUNG || ARCH_EFM32 || \
- ARCH_SHMOBILE_LEGACY || \
- ARCH_LPC18XX || ARM_SINGLE_ARMV7M
+ PLAT_SAMSUNG || ARM_SINGLE_ARMV7M || \
+ ARCH_SHMOBILE_LEGACY
default "mach/uncompress.h"

config EARLY_PRINTK
--
2.4.1


2015-05-21 06:43:59

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On Thu, May 21, 2015 at 12:35:44AM +0200, Stefan Agner wrote:
> Use the new config symbol ARM_SINGLE_ARMV7M which groups config
> symbols used by modern ARMv7-M platforms. This allows supporting
> multiple ARMv7-M platforms in one kernel image. However, a common
> kernel image requires the combined platforms to share the same
> main memory layout to be bootable.
>
> Acked-by: Uwe Kleine-K?nig <[email protected]>
> Signed-off-by: Stefan Agner <[email protected]>
> ---
> Since this is essentially only a shift of config symbols, it
> should not change runtime behavior, at least when selecting
> only one platform.
>
> Uwe, this is essentially the same I had in my patchset, just
> converting the other platforms too. I was bold and added your
> Ack anyway...
That's fine.

Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2015-05-21 17:00:07

by Joachim Eastwood

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

Hi Stefan,

On 21 May 2015 at 00:35, Stefan Agner <[email protected]> wrote:
> Use the new config symbol ARM_SINGLE_ARMV7M which groups config
> symbols used by modern ARMv7-M platforms. This allows supporting
> multiple ARMv7-M platforms in one kernel image. However, a common
> kernel image requires the combined platforms to share the same
> main memory layout to be bootable.
>
> Acked-by: Uwe Kleine-König <[email protected]>
> Signed-off-by: Stefan Agner <[email protected]>

You should have your sign-off on the top and not Uwe's ack.

> ---
> Since this is essentially only a shift of config symbols, it
> should not change runtime behavior, at least when selecting
> only one platform.
>
> Uwe, this is essentially the same I had in my patchset, just
> converting the other platforms too. I was bold and added your
> Ack anyway...
>
> Joachim, Maxime, I test compiled with your defconfigs, compiled
> fine on my machine.
>
> arch/arm/Kconfig | 86 ++++++++++++++++++--------------------------------
> arch/arm/Kconfig.debug | 5 ++-
> 2 files changed, 32 insertions(+), 59 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 75920ed..9b777e3 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -334,6 +334,7 @@ config ARM_SINGLE_ARMV7M
> depends on !MMU
> select ARCH_WANT_OPTIONAL_GPIOLIB
> select ARM_NVIC
> + select AUTO_ZRELADDR
> select CLKSRC_OF
> select COMMON_CLK
> select CPU_V7M
> @@ -411,24 +412,6 @@ config ARCH_EBSA110
> Ethernet interface, two PCMCIA sockets, two serial ports and a
> parallel port.
[...]
> -config ARCH_LPC18XX
> - bool "NXP LPC18xx/LPC43xx"
> - depends on !MMU
> - select ARCH_HAS_RESET_CONTROLLER
> - select ARCH_REQUIRE_GPIOLIB
> - select ARM_AMBA
> - select ARM_NVIC
> - select AUTO_ZRELADDR
> - select CLKSRC_LPC32XX
> - select COMMON_CLK
> - select CPU_V7M
> - select GENERIC_CLOCKEVENTS
> - select NO_IOPORT_MAP
> - select PINCTRL
> - select SPARSE_IRQ
> - select USE_OF
> - help
> - Support for NXP's LPC18xx Cortex-M3 and LPC43xx Cortex-M4
> - high performance microcontrollers.
> -
[...]
> +config ARCH_LPC18XX
> + bool "NXP LPC18xx/LPC43xx"
> + depends on ARM_SINGLE_ARMV7M
> + select ARCH_HAS_RESET_CONTROLLER
> + select ARM_AMBA
> + select CLKSRC_LPC32XX
> + select PINCTRL
> + help
> + Support for NXP's LPC18xx Cortex-M3 and LPC43xx Cortex-M4
> + high performance microcontrollers.

The LPC18xx parts look good and it still builds and boots on my devkit so;

Acked-by: Joachim Eastwood <[email protected]>

regards,
Joachim Eastwood

2015-05-21 19:04:18

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On Thu, May 21, 2015 at 07:00:02PM +0200, Joachim Eastwood wrote:
> Hi Stefan,
>
> On 21 May 2015 at 00:35, Stefan Agner <[email protected]> wrote:
> > Use the new config symbol ARM_SINGLE_ARMV7M which groups config
> > symbols used by modern ARMv7-M platforms. This allows supporting
> > multiple ARMv7-M platforms in one kernel image. However, a common
> > kernel image requires the combined platforms to share the same
> > main memory layout to be bootable.
> >
> > Acked-by: Uwe Kleine-K?nig <[email protected]>
> > Signed-off-by: Stefan Agner <[email protected]>
>
> You should have your sign-off on the top and not Uwe's ack.
I don't agree. IMHO the most sensible thing is to sign off (i.e. put
your sob line below) all the things that come from you. So it's

Signed-off-by: author
Acked-by: someone
Signed-off-by: forwarder
Signed-off-by: maintainer

if someone already acked before forwarder sent out the mail. If the ack
is between forwarder and maintainer it was the latter who added the Ack.
I'm not sure it's formalized this way, but like that it makes most sense
to me and I seem to recall having read somewhere that the footers are
supposed to tell something about the order of people involved.

So Stefan did it exactly as i would have done it.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2015-05-22 07:29:28

by Stefan Agner

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On 2015-05-21 21:04, Uwe Kleine-König wrote:
> On Thu, May 21, 2015 at 07:00:02PM +0200, Joachim Eastwood wrote:
>> Hi Stefan,
>>
>> On 21 May 2015 at 00:35, Stefan Agner <[email protected]> wrote:
>> > Use the new config symbol ARM_SINGLE_ARMV7M which groups config
>> > symbols used by modern ARMv7-M platforms. This allows supporting
>> > multiple ARMv7-M platforms in one kernel image. However, a common
>> > kernel image requires the combined platforms to share the same
>> > main memory layout to be bootable.
>> >
>> > Acked-by: Uwe Kleine-König <[email protected]>
>> > Signed-off-by: Stefan Agner <[email protected]>
>>
>> You should have your sign-off on the top and not Uwe's ack.
> I don't agree. IMHO the most sensible thing is to sign off (i.e. put
> your sob line below) all the things that come from you. So it's
>
> Signed-off-by: author
> Acked-by: someone
> Signed-off-by: forwarder
> Signed-off-by: maintainer
>
> if someone already acked before forwarder sent out the mail. If the ack
> is between forwarder and maintainer it was the latter who added the Ack.
> I'm not sure it's formalized this way, but like that it makes most sense
> to me and I seem to recall having read somewhere that the footers are
> supposed to tell something about the order of people involved.
>
> So Stefan did it exactly as i would have done it.

Hm, never give much thoughts about that order, its quite development
process driven here: I use git format-patch with --signoff. When I send
out v1, and get a Ack or Review, I add that to my commit. Hence, when I
send v2 of the patch, my sob line ends up below the Ack (that is what
happened in that case). If its the last revision, and the patch gets
picked up, the forwarder/maintainer will add the Ack and then it would
land after my sob...

--
Stefan

2015-05-22 07:54:30

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On Thursday 21 May 2015 00:35:44 Stefan Agner wrote:
> Use the new config symbol ARM_SINGLE_ARMV7M which groups config
> symbols used by modern ARMv7-M platforms. This allows supporting
> multiple ARMv7-M platforms in one kernel image. However, a common
> kernel image requires the combined platforms to share the same
> main memory layout to be bootable.
>
> Acked-by: Uwe Kleine-K?nig <[email protected]>
> Signed-off-by: Stefan Agner <[email protected]>
> ---
> Since this is essentially only a shift of config symbols, it
> should not change runtime behavior, at least when selecting
> only one platform.
>
> Uwe, this is essentially the same I had in my patchset, just
> converting the other platforms too. I was bold and added your
> Ack anyway...
>
> Joachim, Maxime, I test compiled with your defconfigs, compiled
> fine on my machine.
>
> arch/arm/Kconfig | 86 ++++++++++++++++++--------------------------------
> arch/arm/Kconfig.debug | 5 ++-
> 2 files changed, 32 insertions(+), 59 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 75920ed..9b777e3 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -334,6 +334,7 @@ config ARM_SINGLE_ARMV7M
> depends on !MMU
> select ARCH_WANT_OPTIONAL_GPIOLIB
> select ARM_NVIC
> + select AUTO_ZRELADDR
> select CLKSRC_OF
> select COMMON_CLK
> select CPU_V7M

I just got a build failure for VF610 because of the lack of AUTO_ZRELADDR,
so this patch should fix that. Good.

> menu "Multiple platform selection"
> @@ -1006,6 +951,35 @@ source "arch/arm/mach-zx/Kconfig"
>
> source "arch/arm/mach-zynq/Kconfig"
>
> +# ARMv7-M architecture
> +config ARCH_EFM32
> + bool "Energy Micro efm32"
> + depends on ARM_SINGLE_ARMV7M
> + select ARCH_REQUIRE_GPIOLIB
> + help
> + Support for Energy Micro's (now Silicon Labs) efm32 Giant Gecko
> + processors.
> +
> +config ARCH_LPC18XX
> + bool "NXP LPC18xx/LPC43xx"
> + depends on ARM_SINGLE_ARMV7M
> + select ARCH_HAS_RESET_CONTROLLER
> + select ARM_AMBA
> + select CLKSRC_LPC32XX
> + select PINCTRL
> + help
> + Support for NXP's LPC18xx Cortex-M3 and LPC43xx Cortex-M4
> + high performance microcontrollers.
> +
> +config ARCH_STM32
> + bool "STMicrolectronics STM32"
> + depends on ARM_SINGLE_ARMV7M
> + select ARCH_HAS_RESET_CONTROLLER
> + select ARMV7M_SYSTICK
> + select RESET_CONTROLLER
> + help
> + Support for STMicroelectronics STM32 processors.
> +

Should we move those options into the respective subdirectories,
for consistency with the other platforms?

The current top-level Kconfig file is much too large at the moment,
so that would reduce the clutter a bit, but then again, all three
of these currently don't need a Kconfig file for themselves, so
that might be a bit silly as well.

Another option might be to consolidate these three into a single
directory, if someone can come up with a good name. The machine
files are all trivial, so they could even be merged into one as
far as I can tell, we just need slightly different 'select'
statements above.

If we do that, is it possible to merge Vybrid into that as well?
I guess the main question here is how much other infrastructure
(if any) from mach-imx is used on vf610, and if there is some other
way to do that.

Arnd

2015-05-22 08:56:39

by Stefan Agner

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On 2015-05-22 09:53, Arnd Bergmann wrote:
> On Thursday 21 May 2015 00:35:44 Stefan Agner wrote:
<snip>
>> +# ARMv7-M architecture
>> +config ARCH_EFM32
>> + bool "Energy Micro efm32"
>> + depends on ARM_SINGLE_ARMV7M
>> + select ARCH_REQUIRE_GPIOLIB
>> + help
>> + Support for Energy Micro's (now Silicon Labs) efm32 Giant Gecko
>> + processors.
>> +
>> +config ARCH_LPC18XX
>> + bool "NXP LPC18xx/LPC43xx"
>> + depends on ARM_SINGLE_ARMV7M
>> + select ARCH_HAS_RESET_CONTROLLER
>> + select ARM_AMBA
>> + select CLKSRC_LPC32XX
>> + select PINCTRL
>> + help
>> + Support for NXP's LPC18xx Cortex-M3 and LPC43xx Cortex-M4
>> + high performance microcontrollers.
>> +
>> +config ARCH_STM32
>> + bool "STMicrolectronics STM32"
>> + depends on ARM_SINGLE_ARMV7M
>> + select ARCH_HAS_RESET_CONTROLLER
>> + select ARMV7M_SYSTICK
>> + select RESET_CONTROLLER
>> + help
>> + Support for STMicroelectronics STM32 processors.
>> +
>
> Should we move those options into the respective subdirectories,
> for consistency with the other platforms?
>
> The current top-level Kconfig file is much too large at the moment,
> so that would reduce the clutter a bit, but then again, all three
> of these currently don't need a Kconfig file for themselves, so
> that might be a bit silly as well.

But their machine folder is there anyway, so it wouldn't clutter
arch/arm/ more than it is today.

> Another option might be to consolidate these three into a single
> directory, if someone can come up with a good name. The machine
> files are all trivial, so they could even be merged into one as
> far as I can tell, we just need slightly different 'select'
> statements above.
>
> If we do that, is it possible to merge Vybrid into that as well?
> I guess the main question here is how much other infrastructure
> (if any) from mach-imx is used on vf610, and if there is some other
> way to do that.

I think it is nice today to use the same Kconfig symbol (SOC_VF610) as
we use for the primary Cortex-A5 processor. After all, the kernel is
running on the same SoC... Its just a different processor we are running
on.

Today, I rely on several config symbols set by ARCH_MXC or SOC_VF610,
some of that quite SoC specific (PINCTRL_VF610). However, after the move
of the clock stuff which is in imx-next, there is really almost no
machine specific code required from mach-imx. However, I plan to add PM
support, which probably still will land in the machine folder?

I also think that it would a bit risky to do this for that release. So
maybe as first step, simply split out the machines in individual Kconfig
files?

What do you think about the defconfig dependency Uwe pointed out? Shall
I create a single patch on-top of a soc/defconfig merged branch?

--
Stefan




2015-05-22 09:37:32

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On Friday 22 May 2015 09:27:38 Stefan Agner wrote:
> On 2015-05-21 21:04, Uwe Kleine-K?nig wrote:
> > On Thu, May 21, 2015 at 07:00:02PM +0200, Joachim Eastwood wrote:
> >> Hi Stefan,
> >>
> >> On 21 May 2015 at 00:35, Stefan Agner <[email protected]> wrote:
> >> > Use the new config symbol ARM_SINGLE_ARMV7M which groups config
> >> > symbols used by modern ARMv7-M platforms. This allows supporting
> >> > multiple ARMv7-M platforms in one kernel image. However, a common
> >> > kernel image requires the combined platforms to share the same
> >> > main memory layout to be bootable.
> >> >
> >> > Acked-by: Uwe Kleine-K?nig <[email protected]>
> >> > Signed-off-by: Stefan Agner <[email protected]>
> >>
> >> You should have your sign-off on the top and not Uwe's ack.
> > I don't agree. IMHO the most sensible thing is to sign off (i.e. put
> > your sob line below) all the things that come from you. So it's
> >
> > Signed-off-by: author
> > Acked-by: someone
> > Signed-off-by: forwarder
> > Signed-off-by: maintainer
> >
> > if someone already acked before forwarder sent out the mail. If the ack
> > is between forwarder and maintainer it was the latter who added the Ack.
> > I'm not sure it's formalized this way, but like that it makes most sense
> > to me and I seem to recall having read somewhere that the footers are
> > supposed to tell something about the order of people involved.
> >
> > So Stefan did it exactly as i would have done it.
>
> Hm, never give much thoughts about that order, its quite development
> process driven here: I use git format-patch with --signoff. When I send
> out v1, and get a Ack or Review, I add that to my commit. Hence, when I
> send v2 of the patch, my sob line ends up below the Ack (that is what
> happened in that case). If its the last revision, and the patch gets
> picked up, the forwarder/maintainer will add the Ack and then it would
> land after my sob...

I think it's not very important either way. The order that Uwe suggested
is what we tend to end up with when everyone just adds additional tags
on the bottom, but I also see the acks added elsewhere.

The order for the signed-off-by lines is more important though, as it
documents the patch flow.

Arnd

2015-05-22 09:39:57

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On Friday 22 May 2015 10:54:48 Stefan Agner wrote:
> > Another option might be to consolidate these three into a single
> > directory, if someone can come up with a good name. The machine
> > files are all trivial, so they could even be merged into one as
> > far as I can tell, we just need slightly different 'select'
> > statements above.
> >
> > If we do that, is it possible to merge Vybrid into that as well?
> > I guess the main question here is how much other infrastructure
> > (if any) from mach-imx is used on vf610, and if there is some other
> > way to do that.
>
> I think it is nice today to use the same Kconfig symbol (SOC_VF610) as
> we use for the primary Cortex-A5 processor. After all, the kernel is
> running on the same SoC... Its just a different processor we are running
> on.

Yes, though you can in fact have multiple definitions of the same symbol,
so that would still work if you define CONFIG_SOC_VF610 outside again
outside of mach-imx. It would be a little ugly and possibly confusing
though.

> Today, I rely on several config symbols set by ARCH_MXC or SOC_VF610,
> some of that quite SoC specific (PINCTRL_VF610). However, after the move
> of the clock stuff which is in imx-next, there is really almost no
> machine specific code required from mach-imx. However, I plan to add PM
> support, which probably still will land in the machine folder?

What kind of PM support? We should have driver subsystems for most of
it now, and the goal has always been that you're able to do it without
platform specific code.

> I also think that it would a bit risky to do this for that release. So
> maybe as first step, simply split out the machines in individual Kconfig
> files?
>
> What do you think about the defconfig dependency Uwe pointed out? Shall
> I create a single patch on-top of a soc/defconfig merged branch?

I think for efm32, we want to change the defconfig in the same commit
that moves the option around, to it keeps working across bisection.

For the other ones, it is probably easier as a separate patch, as the
defconfigs do not exist in the next/soc branch yet and there is no
possible regression either.

Arnd

2015-05-22 13:06:25

by Maxime Coquelin

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

2015-05-21 0:35 GMT+02:00 Stefan Agner <[email protected]>:
> Use the new config symbol ARM_SINGLE_ARMV7M which groups config
> symbols used by modern ARMv7-M platforms. This allows supporting
> multiple ARMv7-M platforms in one kernel image. However, a common
> kernel image requires the combined platforms to share the same
> main memory layout to be bootable.
>
> Acked-by: Uwe Kleine-König <[email protected]>
> Signed-off-by: Stefan Agner <[email protected]>
> ---
> Since this is essentially only a shift of config symbols, it
> should not change runtime behavior, at least when selecting
> only one platform.
>
> Uwe, this is essentially the same I had in my patchset, just
> converting the other platforms too. I was bold and added your
> Ack anyway...
>
> Joachim, Maxime, I test compiled with your defconfigs, compiled
> fine on my machine.
>
> arch/arm/Kconfig | 86 ++++++++++++++++++--------------------------------
> arch/arm/Kconfig.debug | 5 ++-
> 2 files changed, 32 insertions(+), 59 deletions(-)
>

For the stm32 part:
Acked-by: Maxime Coquelin <[email protected]>

Thanks!
Maxime

2015-05-22 14:51:14

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On Thursday 21 May 2015 00:35:44 Stefan Agner wrote:
> Use the new config symbol ARM_SINGLE_ARMV7M which groups config
> symbols used by modern ARMv7-M platforms. This allows supporting
> multiple ARMv7-M platforms in one kernel image. However, a common
> kernel image requires the combined platforms to share the same
> main memory layout to be bootable.
>
> Acked-by: Uwe Kleine-K?nig <[email protected]>
> Signed-off-by: Stefan Agner <[email protected]>
> ---
> Since this is essentially only a shift of config symbols, it
> should not change runtime behavior, at least when selecting
> only one platform.
>
> Uwe, this is essentially the same I had in my patchset, just
> converting the other platforms too. I was bold and added your
> Ack anyway...
>
> Joachim, Maxime, I test compiled with your defconfigs, compiled
> fine on my machine.

I've applied the patch now with all three Acks added (and reordered
with regard to the Signed-off-by).

Thanks!

[one small request as I have four armv7-m folks on Cc already:
could one of you try to fix the warning that I get with every
single build: "/git/arm-soc/arch/arm/kernel/head-nommu.S: Assembler
messages: /git/arm-soc/arch/arm/kernel/head-nommu.S:167: Warning:
Use of r13 as a source register is deprecated when r15 is the
destination register."]

Arnd

2015-05-22 15:29:23

by Maxime Coquelin

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

2015-05-22 16:50 GMT+02:00 Arnd Bergmann <[email protected]>:
> [one small request as I have four armv7-m folks on Cc already:
> could one of you try to fix the warning that I get with every
> single build: "/git/arm-soc/arch/arm/kernel/head-nommu.S: Assembler
> messages: /git/arm-soc/arch/arm/kernel/head-nommu.S:167: Warning:
> Use of r13 as a source register is deprecated when r15 is the
> destination register."]

Moving r13 to r12 and returning r12 seems to do the job (see below).
But I don't know if there is a more elegant way, and if it is also
valid for other architectures than armv7-m.
I can propose a patch if someone can confirm it is valid.

Regards,
Maxime

-------------------------------------------------------------------------------------------------------------

diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index aebfbf7..e84bdad 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -164,7 +164,8 @@ __after_proc_init:
#endif
mcr p15, 0, r0, c1, c0, 0 @ write control reg
#endif /* CONFIG_CPU_CP15 */
- ret r13
+ mov r12, r13
+ ret r12
ENDPROC(__after_proc_init)
.ltorg

2015-05-22 15:38:33

by Stefan Agner

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On 2015-05-22 17:29, Maxime Coquelin wrote:
> 2015-05-22 16:50 GMT+02:00 Arnd Bergmann <[email protected]>:
>> [one small request as I have four armv7-m folks on Cc already:
>> could one of you try to fix the warning that I get with every
>> single build: "/git/arm-soc/arch/arm/kernel/head-nommu.S: Assembler
>> messages: /git/arm-soc/arch/arm/kernel/head-nommu.S:167: Warning:
>> Use of r13 as a source register is deprecated when r15 is the
>> destination register."]
>
> Moving r13 to r12 and returning r12 seems to do the job (see below).
> But I don't know if there is a more elegant way, and if it is also
> valid for other architectures than armv7-m.
> I can propose a patch if someone can confirm it is valid.

>
> -------------------------------------------------------------------------------------------------------------
>
> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> index aebfbf7..e84bdad 100644
> --- a/arch/arm/kernel/head-nommu.S
> +++ b/arch/arm/kernel/head-nommu.S
> @@ -164,7 +164,8 @@ __after_proc_init:
> #endif
> mcr p15, 0, r0, c1, c0, 0 @ write control reg
> #endif /* CONFIG_CPU_CP15 */
> - ret r13
> + mov r12, r13
> + ret r12
> ENDPROC(__after_proc_init)
> .ltorg

That is actually a patch I have here too, altough I used r11 back
then... :-)

However, I don't think this is a nice solution. We should avoid using
r13 for that address in the first place, e.g. using a different register
to get the value when calling __mmap_switched. However, for that one
need to know what registers are guaranteed to be not used within
PROCINFO_INITFUNC...

--
Stefan

2015-05-22 15:56:13

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On 22/05/15 16:29, Maxime Coquelin wrote:
> 2015-05-22 16:50 GMT+02:00 Arnd Bergmann <[email protected]>:
>> [one small request as I have four armv7-m folks on Cc already:
>> could one of you try to fix the warning that I get with every
>> single build: "/git/arm-soc/arch/arm/kernel/head-nommu.S: Assembler
>> messages: /git/arm-soc/arch/arm/kernel/head-nommu.S:167: Warning:
>> Use of r13 as a source register is deprecated when r15 is the
>> destination register."]
>
> Moving r13 to r12 and returning r12 seems to do the job (see below).
> But I don't know if there is a more elegant way, and if it is also
> valid for other architectures than armv7-m.

Why not just s/r13/r11/?

(works for me but I'm only working on single core system)

2015-05-22 16:30:07

by Stefan Agner

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On 2015-05-22 17:56, Daniel Thompson wrote:
> On 22/05/15 16:29, Maxime Coquelin wrote:
>> 2015-05-22 16:50 GMT+02:00 Arnd Bergmann <[email protected]>:
>>> [one small request as I have four armv7-m folks on Cc already:
>>> could one of you try to fix the warning that I get with every
>>> single build: "/git/arm-soc/arch/arm/kernel/head-nommu.S: Assembler
>>> messages: /git/arm-soc/arch/arm/kernel/head-nommu.S:167: Warning:
>>> Use of r13 as a source register is deprecated when r15 is the
>>> destination register."]
>>
>> Moving r13 to r12 and returning r12 seems to do the job (see below).
>> But I don't know if there is a more elegant way, and if it is also
>> valid for other architectures than armv7-m.
>
> Why not just s/r13/r11/?
>
> (works for me but I'm only working on single core system)

For ARMv7-M this works, since r11 is not used in the processors
PROCINFO_INITFUNC function (__cpu_flush in struct proc_info_list, which
is __v7m_setup in proc-v7m.S).

However, afaik, head-nommu.S can be used by different processors too,
hence that register needs to be free to use for all possible __cpu_flush
implementations.

That said, proc-v7.S stores r11 on the stack, so it really seems that
r11 is ok to use?

--
Stefan

2015-05-22 17:57:12

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On Fri, May 22, 2015 at 05:29:20PM +0200, Maxime Coquelin wrote:
> 2015-05-22 16:50 GMT+02:00 Arnd Bergmann <[email protected]>:
> > [one small request as I have four armv7-m folks on Cc already:
> > could one of you try to fix the warning that I get with every
> > single build: "/git/arm-soc/arch/arm/kernel/head-nommu.S: Assembler
> > messages: /git/arm-soc/arch/arm/kernel/head-nommu.S:167: Warning:
> > Use of r13 as a source register is deprecated when r15 is the
> > destination register."]
>
> Moving r13 to r12 and returning r12 seems to do the job (see below).
> But I don't know if there is a more elegant way, and if it is also
> valid for other architectures than armv7-m.
> I can propose a patch if someone can confirm it is valid.

Please follow the ARM code example, rather than inventing alternative
solutions to the same problem that was solved there. We use r3 for
this there:

mov r3, r13
ret r3

Consistency _is_ important.

Thanks.

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-05-22 18:07:03

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On Fri, May 22, 2015 at 06:28:16PM +0200, Stefan Agner wrote:
> On 2015-05-22 17:56, Daniel Thompson wrote:
> > On 22/05/15 16:29, Maxime Coquelin wrote:
> >> 2015-05-22 16:50 GMT+02:00 Arnd Bergmann <[email protected]>:
> >>> [one small request as I have four armv7-m folks on Cc already:
> >>> could one of you try to fix the warning that I get with every
> >>> single build: "/git/arm-soc/arch/arm/kernel/head-nommu.S: Assembler
> >>> messages: /git/arm-soc/arch/arm/kernel/head-nommu.S:167: Warning:
> >>> Use of r13 as a source register is deprecated when r15 is the
> >>> destination register."]
> >>
> >> Moving r13 to r12 and returning r12 seems to do the job (see below).
> >> But I don't know if there is a more elegant way, and if it is also
> >> valid for other architectures than armv7-m.
> >
> > Why not just s/r13/r11/?
> >
> > (works for me but I'm only working on single core system)
>
> For ARMv7-M this works, since r11 is not used in the processors
> PROCINFO_INITFUNC function (__cpu_flush in struct proc_info_list, which
> is __v7m_setup in proc-v7m.S).
>
> However, afaik, head-nommu.S can be used by different processors too,
> hence that register needs to be free to use for all possible __cpu_flush
> implementations.
>
> That said, proc-v7.S stores r11 on the stack, so it really seems that
> r11 is ok to use?

Please use r3 (as I just said). We don't need random deviations between
MMU and noMMU stuff - that just makes maintanence of other code more
difficult.

You can also avoid the issues of having it passed through the processor
specific init function (which isn't guaranteed to preserve r13) by
doing this:

- ldr r13, =__mmap_switched @ address to jump to after
- @ initialising sctlr
badr lr, 1f @ return (PIC) address
ldr r12, [r10, #PROCINFO_INITFUNC]
add r12, r12, r10
ret r12
- 1: b __after_proc_init
+1: ldr r13, =__mmap_switched @ address to jump to after
+ @ initialising sctlr
b __after_proc_init

However, because you have no MMU to turn on, and no address switch,
you actually don't need any of this. __after_proc_init can become
a "function" which returns via the link register.

You can then do:

1: bl __after_proc_init
b __mmap_switched

You'll need to fix secondary_startup in there as well:

- adr r4, __secondary_data
- ldmia r4, {r7, r12}
ldr r7, __secondary_data

#ifdef CONFIG_ARM_MPU
/* Use MPU region info supplied by __cpu_up */
ldr r6, [r7] @ get secondary_data.mpu_szr
bl __setup_mpu @ Initialize the MPU
#endif

- badr lr, __after_proc_init @ return address
- mov r13, r12 @ __secondary_switched address
+ badr lr, 1f @ return (PIC) address
ldr r12, [r10, #PROCINFO_INITFUNC]
add r12, r12, r10
ret r12
-ENDPROC(secondary_startup)
-
-ENTRY(__secondary_switched)
+1: bl __after_proc_init
ldr sp, [r7, #12] @ set up the stack pointer
mov fp, #0
b secondary_start_kernel
-ENDPROC(__secondary_switched)
+ENDPROC(secondary_startup)


--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-05-22 19:35:49

by Stefan Agner

[permalink] [raw]
Subject: Re: [PATCH soc] ARM: use ARM_SINGLE_ARMV7M for ARMv7-M platforms

On 2015-05-22 20:06, Russell King - ARM Linux wrote:
> On Fri, May 22, 2015 at 06:28:16PM +0200, Stefan Agner wrote:
>> On 2015-05-22 17:56, Daniel Thompson wrote:
>> > On 22/05/15 16:29, Maxime Coquelin wrote:
>> >> 2015-05-22 16:50 GMT+02:00 Arnd Bergmann <[email protected]>:
>> >>> [one small request as I have four armv7-m folks on Cc already:
>> >>> could one of you try to fix the warning that I get with every
>> >>> single build: "/git/arm-soc/arch/arm/kernel/head-nommu.S: Assembler
>> >>> messages: /git/arm-soc/arch/arm/kernel/head-nommu.S:167: Warning:
>> >>> Use of r13 as a source register is deprecated when r15 is the
>> >>> destination register."]
>> >>
>> >> Moving r13 to r12 and returning r12 seems to do the job (see below).
>> >> But I don't know if there is a more elegant way, and if it is also
>> >> valid for other architectures than armv7-m.
>> >
>> > Why not just s/r13/r11/?
>> >
>> > (works for me but I'm only working on single core system)
>>
>> For ARMv7-M this works, since r11 is not used in the processors
>> PROCINFO_INITFUNC function (__cpu_flush in struct proc_info_list, which
>> is __v7m_setup in proc-v7m.S).
>>
>> However, afaik, head-nommu.S can be used by different processors too,
>> hence that register needs to be free to use for all possible __cpu_flush
>> implementations.
>>
>> That said, proc-v7.S stores r11 on the stack, so it really seems that
>> r11 is ok to use?
>
> Please use r3 (as I just said). We don't need random deviations between
> MMU and noMMU stuff - that just makes maintanence of other code more
> difficult.
>
> You can also avoid the issues of having it passed through the processor
> specific init function (which isn't guaranteed to preserve r13) by
> doing this:
>
> - ldr r13, =__mmap_switched @ address to jump to after
> - @ initialising sctlr
> badr lr, 1f @ return (PIC) address
> ldr r12, [r10, #PROCINFO_INITFUNC]
> add r12, r12, r10
> ret r12
> - 1: b __after_proc_init
> +1: ldr r13, =__mmap_switched @ address to jump to after
> + @ initialising sctlr
> b __after_proc_init

Hm, this is looks sensible, could also be used for head.S I guess...
secondary_startup would need a similar approach then.

>
> However, because you have no MMU to turn on, and no address switch,
> you actually don't need any of this. __after_proc_init can become
> a "function" which returns via the link register.
>
> You can then do:
>
> 1: bl __after_proc_init
> b __mmap_switched
>
> You'll need to fix secondary_startup in there as well:
>
> - adr r4, __secondary_data
> - ldmia r4, {r7, r12}
> ldr r7, __secondary_data
>
> #ifdef CONFIG_ARM_MPU
> /* Use MPU region info supplied by __cpu_up */
> ldr r6, [r7] @ get secondary_data.mpu_szr
> bl __setup_mpu @ Initialize the MPU
> #endif
>
> - badr lr, __after_proc_init @ return address
> - mov r13, r12 @ __secondary_switched address
> + badr lr, 1f @ return (PIC) address
> ldr r12, [r10, #PROCINFO_INITFUNC]
> add r12, r12, r10
> ret r12
> -ENDPROC(secondary_startup)
> -
> -ENTRY(__secondary_switched)
> +1: bl __after_proc_init
> ldr sp, [r7, #12] @ set up the stack pointer
> mov fp, #0
> b secondary_start_kernel
> -ENDPROC(__secondary_switched)
> +ENDPROC(secondary_startup)

Sounds like a much simpler approach. Will test that and send out a patch
in case it works here.

--
Stefan