Add new target: NUCLEO_L452RE-P #12286

pea-pod · 2020-01-21T04:01:33Z

Summary of changes

Add new target: ST Nucleo L452RE-P and also here.

Impact of changes

New target.

I was not able to test with IAR or ARM6, so there is a non-zero chance that those are horribly wrong despite my best efforts.

Migration actions required

None.

Documentation

Only the mbed site documentation and pin description.

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

Reviewers

@jeromecoutant
nucleo_l452re-p test results.txt

ciarmcom · 2020-01-21T06:00:33Z

@pea-pod, thank you for your changes.
@jeromecoutant @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-core please review.

jeromecoutant

Thanks for the contribution

@ARMmbed/team-st-mcd

jeromecoutant · 2020-01-21T12:35:42Z

targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L452xE/TARGET_NUCLEO_L452RE_P/PeripheralPins.c

+// TIM<x> cannot be used because already used by the us_ticker
+// You have to comment all PWM using TIM_MST defined in hal_tick.h file
+//  or update python script (check TIM_MST_LIST) and re-run it


TIM2 is used by us_ticker,
so you have to comment all PWM_2

I presume this means it disappears from the PWMName enum in PeripheralNames.h as well?

You're right, but I have never done it... :-)

Well, I tried a little bit ago, and this is the happy message I received:

[Error] pwmout_device.c@38,6: 'PWM_2' undeclared here (not in a function); did you mean 'PWM_6'?

I did some pythoning, and found that different L4 devices use different timers for their us_ticker. Since there are no #ifdef checks excluding those from compiling in pwmout_device.c found in the TARGET_STM32L4 folder, I do not see a good reason to start now. In fact, I am weary of trying to do that without having at least one or two other affected boards.

jeromecoutant · 2020-01-21T12:36:54Z

targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L452xE/device/TOOLCHAIN_IAR/stm32l452xx.icf

+place at address mem:__intvec_start__ { readonly section .intvec };
+
+place in ROM_region   { readonly };
+place in CSTACK_region { block CSTACK };


Compilation issue: remove this CSTACK_region line, as it is already described above

jeromecoutant · 2020-01-21T12:49:19Z

targets/targets.json

+        },
+        "macros_add": [
+            "STM32L452xx",
+            "STM32L452RE",


STM32L452RE not needed ?

jeromecoutant · 2020-01-21T12:50:33Z

targets/targets.json

+            "STM32L452",
+            "STM32L452xx",
+            "STM32L452xE",
+            "STM32L452RE"


STM32L452, STM32L452xx and STM32L452RE not needed ?

Which of these lines (any of the target.json lines), if any, are required for adding the "bootloader_supported": true feature in order to make it work?

Incidentally, when I did have that one added, the mbed script would complain that it could not find STM32L452RE in some file. This may be by design, but it was beyond my understanding either way.

"bootloader_supported" needs the correct device_name value, see below comment

jeromecoutant · 2020-01-21T16:41:59Z

targets/targets.json

+        "release_versions": [
+            "5"
+        ],
+        "device_name": "STM32L452RE"


Update value with "STM32L452RETx"
(value chosen in tools/arm_pack_manager/index.json)

Thank you. I believe this resolved the build warning, as I also added "bootloader_supported": true and did not see any additional details in my latest test run.

jeromecoutant · 2020-01-21T16:42:52Z

targets/targets.json

+            "STM32L452",
+            "STM32L452xx",
+            "STM32L452xE",
+            "STM32L452RE"


"bootloader_supported" needs the correct device_name value, see below comment

pea-pod · 2020-01-22T06:51:26Z

After making the changes, I re-ran the tests, which are attached here.

nucleo_l452re-p test results 2020-01-21 2137.txt

mergify · 2020-01-23T10:53:14Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

pea-pod · 2020-01-23T14:43:43Z

I added the sector start value and sector size to the tools/arm_pack_manager/index.json file pulled from the datasheet (start at 0x08000000, sector size 2k) to the STM32L452RETx section to allow the tools-py*.* tests to pass.

I also had to resolve the conflict in platform/mbed_lib.json file based on the Samsung #12106 pull request.

@jeromecoutant or anyone else: is there anything else required to merge?

adbridge · 2020-01-23T16:37:40Z

@pea-pod I'm sorry but one of our requirements is that targets should support all 3 compile chains. So we cannot accept this until you have ensured all the compilers work. Why are you unable to test against the other compilers ?

jeromecoutant · 2020-01-23T16:40:11Z

Why are you unable to test against the other compilers ?

Because other are not free :-)

I started tests, let's see tomorrow morning results

pea-pod · 2020-01-24T04:17:48Z

Why are you unable to test against the other compilers ?

Because other are not free :-)

Yes, exactly. I am aware that there is the 800 ~~pound gorilla~~ megabyte IDE which also includes the clang compiler, but I do not know how to run the equivalent of mbed test with it. I did look for what it installed, but I could not find any executables that I could attempt to point it to with a path update.

I started tests, let's see tomorrow morning results

Thank you very much!

jeromecoutant · 2020-01-24T08:41:51Z

Hi

Results:

GCC: Status OK : 704 / 704 (100.0%)
IAR: Status OK : 704 / 704 (100.0%)
ARM: Status OK : 702 / 704 (99.7%)

Issue is with 'tests-mbedmicro-rtos-mbed-heap_and_stack'

[1579809876.64][CONN][RXD] >>> Running case #4: 'Test isr stack in range'...
[1579809876.68][CONN][INF] found KV pair in stream: {{__testcase_start;Test isr stack in range}}, queued...
mbedgt: :243::FAIL: Interrupt stack in wrong location

Linker script has be be checked

adbridge · 2020-01-24T12:58:22Z

Why are you unable to test against the other compilers ?

Because other are not free :-)

I started tests, let's see tomorrow morning results

Thanks Jerome :)

pea-pod · 2020-01-27T14:43:25Z

After some digging (and perhaps not in the X that marks the spot on the treasure map) the best I can come up with in the liker script is that I did not have the minimum heap size set to be larger than the smaller RAM bank. I had it smaller. I am not at all sure how this would make much of a difference where the linker puts the ISR interrupt handler stack, however.

I also corrected a number comments in the same file, one of them having to do with remaining SRAM.

@jeromecoutant I am at your mercy for testing these linker scripts. If there is any guidance as to what to do, or a way to test this using the mbed Studio, I am all ears. In fact, if there was a way to test using mbed Studio, I would be happy to learn about it.

Finally, I keep force pushing to my branch, because I believe that's the right way to keep the merges squashed. If there is a better way to do this, please let me know, as I am still learning the preferred style this site.

jeromecoutant · 2020-01-27T16:13:20Z

Mmm seems there is the same issue:

[1580138117.42][CONN][RXD] >>> Running case #4: 'Test isr stack in range'...
[1580138117.46][CONN][INF] found KV pair in stream: {{__testcase_start;Test isr stack in range}}, queued...
mbedgt: :243::FAIL: Interrupt stack in wrong location
[1580138117.51][CONN][RXD] :243::FAIL: Interrupt stack in wrong location

pea-pod · 2020-01-28T00:49:05Z

Ok, so I think I found it. I compared startup_stm32l452xx.s with another similar file in the L4 family and found that aside from the differences between vectors themselves, there were some of the original HEAP_SIZE definitions remaining. At this point, I do not remember when I modifying it, but I'm sure it was when I wasn't a linker expert. (I am still not a linker expert).

@jeromecoutant Please let me know if this works. I appreciate it a lot.

jeromecoutant · 2020-01-29T08:00:05Z

PR12286_ARM.txt
PR12286_IAR.txt

100% OK

pea-pod · 2020-01-30T13:59:46Z

@adbridge Is there anything remaining that is required to merge? I believe I have satisfied the requirements, but I could be wrong.

@jeromecoutant Thank you for your help. I really appreciate it.

pea-pod · 2020-02-01T16:27:16Z

@0xc0170 Is this ready to go in? @jeromecoutant has approved the two compilers I do not have access to. I have done gcc. Is there something else I need to do?

adbridge · 2020-02-03T11:23:27Z

Let's get this into CI

mbed-ci · 2020-02-03T13:01:08Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

mbed-ci · 2020-02-03T16:07:13Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

mergify bot added the needs: work label Jan 21, 2020

ciarmcom requested review from jeromecoutant and a team January 21, 2020 06:00

ciarmcom added the needs: review label Jan 21, 2020

jeromecoutant reviewed Jan 21, 2020

View reviewed changes

pea-pod force-pushed the target-nucleo_l452re-p branch from 8ac388d to d226d14 Compare January 22, 2020 06:51

mergify bot removed the needs: review label Jan 22, 2020

pea-pod force-pushed the target-nucleo_l452re-p branch from d226d14 to 9451de9 Compare January 23, 2020 14:31

adbridge added the scope: new-target label Jan 23, 2020

pea-pod force-pushed the target-nucleo_l452re-p branch from 9451de9 to 095a0bd Compare January 27, 2020 14:41

Add new target: NUCLEO_L452RE-P

f7c4693

pea-pod force-pushed the target-nucleo_l452re-p branch from 095a0bd to f7c4693 Compare January 28, 2020 00:42

jeromecoutant approved these changes Jan 29, 2020

View reviewed changes

kjbracey approved these changes Feb 3, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Feb 3, 2020

mergify bot added ready for merge and removed needs: CI labels Feb 3, 2020

0xc0170 added the release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 label Feb 3, 2020

0xc0170 merged commit 250e581 into ARMmbed:master Feb 3, 2020

mergify bot removed the ready for merge label Feb 3, 2020

Add new target: NUCLEO_L452RE-P #12286

Add new target: NUCLEO_L452RE-P #12286

Uh oh!

Conversation

pea-pod commented Jan 21, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Jan 21, 2020

Uh oh!

jeromecoutant left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pea-pod commented Jan 22, 2020

Uh oh!

mergify bot commented Jan 23, 2020

Uh oh!

pea-pod commented Jan 23, 2020

Uh oh!

adbridge commented Jan 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeromecoutant commented Jan 23, 2020

Uh oh!

pea-pod commented Jan 24, 2020

Uh oh!

jeromecoutant commented Jan 24, 2020

Uh oh!

adbridge commented Jan 24, 2020

Uh oh!

pea-pod commented Jan 27, 2020

Uh oh!

jeromecoutant commented Jan 27, 2020

Uh oh!

pea-pod commented Jan 28, 2020

Uh oh!

jeromecoutant commented Jan 29, 2020

Uh oh!

pea-pod commented Jan 30, 2020

Uh oh!

pea-pod commented Feb 1, 2020

Uh oh!

adbridge commented Feb 3, 2020

Uh oh!

mbed-ci commented Feb 3, 2020

Uh oh!

mbed-ci commented Feb 3, 2020

adbridge commented Jan 23, 2020 •

edited

Loading