diff mbox

[V2,3/9] ARM: stm32: prepare stm32 family to welcome armv7 architecture

Message ID 1513610272-7824-4-git-send-email-ludovic.Barre@st.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ludovic BARRE Dec. 18, 2017, 3:17 p.m. UTC
From: Ludovic Barre <ludovic.barre@st.com>

This patch prepares the STM32 machine for the integration of Cortex-A
based microprocessor (MPU), on top of the existing Cortex-M
microcontroller family (MCU). Since both MCUs and MPUs are sharing
common hardware blocks we can keep using ARCH_STM32 flag for most of
them. If a hardware block is specific to one family we can use either
ARM_SINGLE_ARMV7M or ARCH_MULTI_V7 flag.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 Documentation/arm/stm32/overview.rst               | 21 +++++++++--------
 arch/arm/mach-stm32/Kconfig                        | 27 ++++++++++++----------
 arch/arm/mach-stm32/Makefile                       |  2 +-
 arch/arm/mach-stm32/{board-dt.c => board-mcu-dt.c} |  0
 4 files changed, 28 insertions(+), 22 deletions(-)
 rename arch/arm/mach-stm32/{board-dt.c => board-mcu-dt.c} (100%)

Comments

Arnd Bergmann Dec. 18, 2017, 8:24 p.m. UTC | #1
On Mon, Dec 18, 2017 at 4:17 PM, Ludovic Barre <ludovic.Barre@st.com> wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> This patch prepares the STM32 machine for the integration of Cortex-A
> based microprocessor (MPU), on top of the existing Cortex-M
> microcontroller family (MCU). Since both MCUs and MPUs are sharing
> common hardware blocks we can keep using ARCH_STM32 flag for most of
> them. If a hardware block is specific to one family we can use either
> ARM_SINGLE_ARMV7M or ARCH_MULTI_V7 flag.
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>

Looks good overall. Two more small comments:


>
> +if ARCH_STM32
> +
>  config MACH_STM32F429
> -       bool "STMicrolectronics STM32F429"
> -       depends on ARCH_STM32
> +       bool "STMicroelectronics STM32F429"
> +       depends on ARM_SINGLE_ARMV7M
>         default y

Instead of the explicit dependency for each board, I'd leave the surrounding
'if ARM_SINGLE_ARMV7M'. I think you had in v1.

> diff --git a/arch/arm/mach-stm32/Makefile b/arch/arm/mach-stm32/Makefile
> index bd0b7b5..5940af1 100644
> --- a/arch/arm/mach-stm32/Makefile
> +++ b/arch/arm/mach-stm32/Makefile
> @@ -1 +1 @@
> -obj-y += board-dt.o
> +obj-$(CONFIG_ARM_SINGLE_ARMV7M) += board-mcu-dt.o
> diff --git a/arch/arm/mach-stm32/board-dt.c b/arch/arm/mach-stm32/board-mcu-dt.c
> similarity index 100%
> rename from arch/arm/mach-stm32/board-dt.c
> rename to arch/arm/mach-stm32/board-mcu-dt.c

Why the rename? I don't expect the new machines to have any notable
contents in a board file, if any at all, so just use one file for both.
I see the board-dt.c file refers to armv7m_restart, we can either put
that in an #ifdef, or find a way to make it the default for all armv7-m
platforms that don't provide any other restart method.

     Arnd
Alexandre TORGUE Dec. 19, 2017, 2:38 p.m. UTC | #2
On 12/18/2017 09:24 PM, Arnd Bergmann wrote:
> On Mon, Dec 18, 2017 at 4:17 PM, Ludovic Barre <ludovic.Barre@st.com> wrote:
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> This patch prepares the STM32 machine for the integration of Cortex-A
>> based microprocessor (MPU), on top of the existing Cortex-M
>> microcontroller family (MCU). Since both MCUs and MPUs are sharing
>> common hardware blocks we can keep using ARCH_STM32 flag for most of
>> them. If a hardware block is specific to one family we can use either
>> ARM_SINGLE_ARMV7M or ARCH_MULTI_V7 flag.
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> 
> Looks good overall. Two more small comments:
> 
> 
>>
>> +if ARCH_STM32
>> +
>>   config MACH_STM32F429
>> -       bool "STMicrolectronics STM32F429"
>> -       depends on ARCH_STM32
>> +       bool "STMicroelectronics STM32F429"
>> +       depends on ARM_SINGLE_ARMV7M
>>          default y
> 
> Instead of the explicit dependency for each board, I'd leave the surrounding
> 'if ARM_SINGLE_ARMV7M'. I think you had in v1.
> 
>> diff --git a/arch/arm/mach-stm32/Makefile b/arch/arm/mach-stm32/Makefile
>> index bd0b7b5..5940af1 100644
>> --- a/arch/arm/mach-stm32/Makefile
>> +++ b/arch/arm/mach-stm32/Makefile
>> @@ -1 +1 @@
>> -obj-y += board-dt.o
>> +obj-$(CONFIG_ARM_SINGLE_ARMV7M) += board-mcu-dt.o
>> diff --git a/arch/arm/mach-stm32/board-dt.c b/arch/arm/mach-stm32/board-mcu-dt.c
>> similarity index 100%
>> rename from arch/arm/mach-stm32/board-dt.c
>> rename to arch/arm/mach-stm32/board-mcu-dt.c
> 
> Why the rename? I don't expect the new machines to have any notable
> contents in a board file, if any at all, so just use one file for both.
> I see the board-dt.c file refers to armv7m_restart, we can either put
> that in an #ifdef, or find a way to make it the default for all armv7-m
> platforms that don't provide any other restart method.
> 
Currently "restart" is not functional on stm32 MCU (at least for 
stm32f746, I will check on others MCU). My fear is if Ludovic made some 
patches to make "armv7m_restart" the default ".restart" function for all 
armv7-m platform, he will not be able to test it on stm32 MCU (as it is 
not currently working). I propose to do it in 2 steps:

1-Keep as you suggest only one board-dt.c file for both (MCU and MPU) 
and remove ".restart" function.

2-Investigate and send patches around ".restart" for both in an other 
series.

regards
Alex


>       Arnd
>
Ludovic BARRE Dec. 19, 2017, 2:43 p.m. UTC | #3
On 12/18/2017 09:24 PM, Arnd Bergmann wrote:
> On Mon, Dec 18, 2017 at 4:17 PM, Ludovic Barre <ludovic.Barre@st.com> wrote:
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> This patch prepares the STM32 machine for the integration of Cortex-A
>> based microprocessor (MPU), on top of the existing Cortex-M
>> microcontroller family (MCU). Since both MCUs and MPUs are sharing
>> common hardware blocks we can keep using ARCH_STM32 flag for most of
>> them. If a hardware block is specific to one family we can use either
>> ARM_SINGLE_ARMV7M or ARCH_MULTI_V7 flag.
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> 
> Looks good overall. Two more small comments:
> 
> 
>>
>> +if ARCH_STM32
>> +
>>   config MACH_STM32F429
>> -       bool "STMicrolectronics STM32F429"
>> -       depends on ARCH_STM32
>> +       bool "STMicroelectronics STM32F429"
>> +       depends on ARM_SINGLE_ARMV7M
>>          default y
> 
> Instead of the explicit dependency for each board, I'd leave the surrounding
> 'if ARM_SINGLE_ARMV7M'. I think you had in v1.

As you suggest, I follow mach-at91 example.
The point is on "depends on ARM_SINGLE_ARMV7M" ?
You prefer this way:
config MACH_STM32F429
	bool "STMicroelectronics STM32F429" if ARM_SINGLE_ARMV7M
	default y

BR
Ludo

> 
>> diff --git a/arch/arm/mach-stm32/Makefile b/arch/arm/mach-stm32/Makefile
>> index bd0b7b5..5940af1 100644
>> --- a/arch/arm/mach-stm32/Makefile
>> +++ b/arch/arm/mach-stm32/Makefile
>> @@ -1 +1 @@
>> -obj-y += board-dt.o
>> +obj-$(CONFIG_ARM_SINGLE_ARMV7M) += board-mcu-dt.o
>> diff --git a/arch/arm/mach-stm32/board-dt.c b/arch/arm/mach-stm32/board-mcu-dt.c
>> similarity index 100%
>> rename from arch/arm/mach-stm32/board-dt.c
>> rename to arch/arm/mach-stm32/board-mcu-dt.c
> 
> Why the rename? I don't expect the new machines to have any notable
> contents in a board file, if any at all, so just use one file for both.
> I see the board-dt.c file refers to armv7m_restart, we can either put
> that in an #ifdef, or find a way to make it the default for all armv7-m
> platforms that don't provide any other restart method.
> 
>       Arnd
>
Arnd Bergmann Dec. 19, 2017, 3:26 p.m. UTC | #4
On Tue, Dec 19, 2017 at 3:43 PM, Ludovic BARRE <ludovic.barre@st.com> wrote:
>
>
> On 12/18/2017 09:24 PM, Arnd Bergmann wrote:
>>
>> On Mon, Dec 18, 2017 at 4:17 PM, Ludovic Barre <ludovic.Barre@st.com>
>> wrote:
>>>
>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>
>>> This patch prepares the STM32 machine for the integration of Cortex-A
>>> based microprocessor (MPU), on top of the existing Cortex-M
>>> microcontroller family (MCU). Since both MCUs and MPUs are sharing
>>> common hardware blocks we can keep using ARCH_STM32 flag for most of
>>> them. If a hardware block is specific to one family we can use either
>>> ARM_SINGLE_ARMV7M or ARCH_MULTI_V7 flag.
>>>
>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>
>>
>> Looks good overall. Two more small comments:
>>
>>
>>>
>>> +if ARCH_STM32
>>> +
>>>   config MACH_STM32F429
>>> -       bool "STMicrolectronics STM32F429"
>>> -       depends on ARCH_STM32
>>> +       bool "STMicroelectronics STM32F429"
>>> +       depends on ARM_SINGLE_ARMV7M
>>>          default y
>>
>>
>> Instead of the explicit dependency for each board, I'd leave the
>> surrounding
>> 'if ARM_SINGLE_ARMV7M'. I think you had in v1.
>
>
> As you suggest, I follow mach-at91 example.
> The point is on "depends on ARM_SINGLE_ARMV7M" ?
> You prefer this way:
> config MACH_STM32F429
>         bool "STMicroelectronics STM32F429" if ARM_SINGLE_ARMV7M
>         default y
>

No, that would be wrong, that way you would always enable
MACH_STM32F429 when ARM_SINGLE_ARMV7M is turned
off, which is exactly the wrong way round. What I meant is

if ARCH_STM32

if ARM_SINGLE_ARMV7M

config MACH_STM32F429
         bool "STMicrolectronics STM32F429"

config MACH_STM32...
          ...

endif # ARMv7-M

if ARCH_MULTI_V7

config MACH_STM32...
          ...

endif # ARMv7-A

endif # STM32

       Arnd
Ludovic BARRE Dec. 19, 2017, 3:54 p.m. UTC | #5
On 12/19/2017 04:26 PM, Arnd Bergmann wrote:
> On Tue, Dec 19, 2017 at 3:43 PM, Ludovic BARRE <ludovic.barre@st.com> wrote:
>>
>>
>> On 12/18/2017 09:24 PM, Arnd Bergmann wrote:
>>>
>>> On Mon, Dec 18, 2017 at 4:17 PM, Ludovic Barre <ludovic.Barre@st.com>
>>> wrote:
>>>>
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> This patch prepares the STM32 machine for the integration of Cortex-A
>>>> based microprocessor (MPU), on top of the existing Cortex-M
>>>> microcontroller family (MCU). Since both MCUs and MPUs are sharing
>>>> common hardware blocks we can keep using ARCH_STM32 flag for most of
>>>> them. If a hardware block is specific to one family we can use either
>>>> ARM_SINGLE_ARMV7M or ARCH_MULTI_V7 flag.
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>
>>>
>>> Looks good overall. Two more small comments:
>>>
>>>
>>>>
>>>> +if ARCH_STM32
>>>> +
>>>>    config MACH_STM32F429
>>>> -       bool "STMicrolectronics STM32F429"
>>>> -       depends on ARCH_STM32
>>>> +       bool "STMicroelectronics STM32F429"
>>>> +       depends on ARM_SINGLE_ARMV7M
>>>>           default y
>>>
>>>
>>> Instead of the explicit dependency for each board, I'd leave the
>>> surrounding
>>> 'if ARM_SINGLE_ARMV7M'. I think you had in v1.
>>
>>
>> As you suggest, I follow mach-at91 example.
>> The point is on "depends on ARM_SINGLE_ARMV7M" ?
>> You prefer this way:
>> config MACH_STM32F429
>>          bool "STMicroelectronics STM32F429" if ARM_SINGLE_ARMV7M
>>          default y
>>
> 
> No, that would be wrong, that way you would always enable
> MACH_STM32F429 when ARM_SINGLE_ARMV7M is turned
> off, which is exactly the wrong way round. What I meant is
> 
> if ARCH_STM32
> 
> if ARM_SINGLE_ARMV7M
> 
> config MACH_STM32F429
>           bool "STMicrolectronics STM32F429"
> 
> config MACH_STM32...
>            ...
> 
> endif # ARMv7-M
> 
> if ARCH_MULTI_V7
> 
> config MACH_STM32...
>            ...
> 
> endif # ARMv7-A
> 
> endif # STM32
> 
>         Arnd
> 
Ok, it's clear :-)
Ludovic BARRE Dec. 21, 2017, 4:39 p.m. UTC | #6
hi Arnd

just a ping, on Alex's proposal (below) about armv7m_restart
(Currently "restart" is not functional on stm32 MCU).

BR
Ludo
On 12/19/2017 03:38 PM, Alexandre Torgue wrote:
> 
> 
> On 12/18/2017 09:24 PM, Arnd Bergmann wrote:
>> On Mon, Dec 18, 2017 at 4:17 PM, Ludovic Barre <ludovic.Barre@st.com> 
>> wrote:
>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>
>>> This patch prepares the STM32 machine for the integration of Cortex-A
>>> based microprocessor (MPU), on top of the existing Cortex-M
>>> microcontroller family (MCU). Since both MCUs and MPUs are sharing
>>> common hardware blocks we can keep using ARCH_STM32 flag for most of
>>> them. If a hardware block is specific to one family we can use either
>>> ARM_SINGLE_ARMV7M or ARCH_MULTI_V7 flag.
>>>
>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>
>> Looks good overall. Two more small comments:
>>
>>
>>>
>>> +if ARCH_STM32
>>> +
>>>   config MACH_STM32F429
>>> -       bool "STMicrolectronics STM32F429"
>>> -       depends on ARCH_STM32
>>> +       bool "STMicroelectronics STM32F429"
>>> +       depends on ARM_SINGLE_ARMV7M
>>>          default y
>>
>> Instead of the explicit dependency for each board, I'd leave the 
>> surrounding
>> 'if ARM_SINGLE_ARMV7M'. I think you had in v1.
>>
>>> diff --git a/arch/arm/mach-stm32/Makefile b/arch/arm/mach-stm32/Makefile
>>> index bd0b7b5..5940af1 100644
>>> --- a/arch/arm/mach-stm32/Makefile
>>> +++ b/arch/arm/mach-stm32/Makefile
>>> @@ -1 +1 @@
>>> -obj-y += board-dt.o
>>> +obj-$(CONFIG_ARM_SINGLE_ARMV7M) += board-mcu-dt.o
>>> diff --git a/arch/arm/mach-stm32/board-dt.c 
>>> b/arch/arm/mach-stm32/board-mcu-dt.c
>>> similarity index 100%
>>> rename from arch/arm/mach-stm32/board-dt.c
>>> rename to arch/arm/mach-stm32/board-mcu-dt.c
>>
>> Why the rename? I don't expect the new machines to have any notable
>> contents in a board file, if any at all, so just use one file for both.
>> I see the board-dt.c file refers to armv7m_restart, we can either put
>> that in an #ifdef, or find a way to make it the default for all armv7-m
>> platforms that don't provide any other restart method.
>>
> Currently "restart" is not functional on stm32 MCU (at least for 
> stm32f746, I will check on others MCU). My fear is if Ludovic made some 
> patches to make "armv7m_restart" the default ".restart" function for all 
> armv7-m platform, he will not be able to test it on stm32 MCU (as it is 
> not currently working). I propose to do it in 2 steps:
> 
> 1-Keep as you suggest only one board-dt.c file for both (MCU and MPU) 
> and remove ".restart" function.
> 
> 2-Investigate and send patches around ".restart" for both in an other 
> series.

I resend

> 
> regards
> Alex
> 
> 
>>       Arnd
>>
Arnd Bergmann Dec. 21, 2017, 8:48 p.m. UTC | #7
On Thu, Dec 21, 2017 at 5:39 PM, Ludovic BARRE <ludovic.barre@st.com> wrote:

>>>
>> Currently "restart" is not functional on stm32 MCU (at least for
>> stm32f746, I will check on others MCU). My fear is if Ludovic made some
>> patches to make "armv7m_restart" the default ".restart" function for all
>> armv7-m platform, he will not be able to test it on stm32 MCU (as it is not
>> currently working). I propose to do it in 2 steps:
>>
>> 1-Keep as you suggest only one board-dt.c file for both (MCU and MPU) and
>> remove ".restart" function.
>>
>> 2-Investigate and send patches around ".restart" for both in an other
>> series.
>
>
> I resend

Yes, that seems fine.

       Arnd
diff mbox

Patch

diff --git a/Documentation/arm/stm32/overview.rst b/Documentation/arm/stm32/overview.rst
index 6be6059..3da6a8e 100644
--- a/Documentation/arm/stm32/overview.rst
+++ b/Documentation/arm/stm32/overview.rst
@@ -5,16 +5,17 @@  STM32 ARM Linux Overview
 Introduction
 ------------
 
-The STMicroelectronics family of Cortex-M based MCUs are supported by the
-'STM32' platform of ARM Linux. Currently only the STM32F429 (Cortex-M4)
-and STM32F746 (Cortex-M7) are supported.
+The STMicroelectronics STM32 family of Cortex-A microprocessors (MPUs) and
+Cortex-M microcontrollers (MCUs) are supported by the 'STM32' platform of
+ARM Linux.
 
 Configuration
 -------------
 
-A generic configuration is provided for STM32 family, and can be used as the
-default by
+For MCUs, use the provided default configuration:
         make stm32_defconfig
+For MPUs, use multi_v7 configuration:
+        make multi_v7_defconfig
 
 Layout
 ------
@@ -22,10 +23,12 @@  Layout
 All the files for multiple machine families are located in the platform code
 contained in arch/arm/mach-stm32
 
-There is a generic board board-dt.c in the mach folder which support
-Flattened Device Tree, which means, it works with any compatible board with
-Device Trees.
+There are generic boards board-mcu-dt.c and board-mpu-dt.c files in the mach
+folder which support Flattened Device Tree, which means, they work with any
+compatible board with Device Trees.
 
 :Authors:
 
-Maxime Coquelin <mcoquelin.stm32@gmail.com>
+- Maxime Coquelin <mcoquelin.stm32@gmail.com>
+- Ludovic Barre <ludovic.barre@st.com>
+- Gerald Baeza <gerald.baeza@st.com>
diff --git a/arch/arm/mach-stm32/Kconfig b/arch/arm/mach-stm32/Kconfig
index 0d1889b..e0f744b 100644
--- a/arch/arm/mach-stm32/Kconfig
+++ b/arch/arm/mach-stm32/Kconfig
@@ -1,8 +1,7 @@ 
-config ARCH_STM32
-	bool "STMicrolectronics STM32"
-	depends on ARM_SINGLE_ARMV7M
+menuconfig ARCH_STM32
+	bool "STMicroelectronics STM32 family" if ARM_SINGLE_ARMV7M || ARCH_MULTI_V7
+	select ARMV7M_SYSTICK if ARM_SINGLE_ARMV7M
 	select ARCH_HAS_RESET_CONTROLLER
-	select ARMV7M_SYSTICK
 	select CLKSRC_STM32
 	select PINCTRL
 	select RESET_CONTROLLER
@@ -10,22 +9,26 @@  config ARCH_STM32
 	help
 	  Support for STMicroelectronics STM32 processors.
 
+if ARCH_STM32
+
 config MACH_STM32F429
-	bool "STMicrolectronics STM32F429"
-	depends on ARCH_STM32
+	bool "STMicroelectronics STM32F429"
+	depends on ARM_SINGLE_ARMV7M
 	default y
 
 config MACH_STM32F469
-	bool "STMicrolectronics STM32F469"
-	depends on ARCH_STM32
+	bool "STMicroelectronics STM32F469"
+	depends on ARM_SINGLE_ARMV7M
 	default y
 
 config MACH_STM32F746
-	bool "STMicrolectronics STM32F746"
-	depends on ARCH_STM32
+	bool "STMicroelectronics STM32F746"
+	depends on ARM_SINGLE_ARMV7M
 	default y
 
 config MACH_STM32H743
-	bool "STMicrolectronics STM32H743"
-	depends on ARCH_STM32
+	bool "STMicroelectronics STM32H743"
+	depends on ARM_SINGLE_ARMV7M
 	default y
+
+endif
diff --git a/arch/arm/mach-stm32/Makefile b/arch/arm/mach-stm32/Makefile
index bd0b7b5..5940af1 100644
--- a/arch/arm/mach-stm32/Makefile
+++ b/arch/arm/mach-stm32/Makefile
@@ -1 +1 @@ 
-obj-y += board-dt.o
+obj-$(CONFIG_ARM_SINGLE_ARMV7M) += board-mcu-dt.o
diff --git a/arch/arm/mach-stm32/board-dt.c b/arch/arm/mach-stm32/board-mcu-dt.c
similarity index 100%
rename from arch/arm/mach-stm32/board-dt.c
rename to arch/arm/mach-stm32/board-mcu-dt.c