LPC1114: baremetal profile support #12623

toyowata · 2020-03-13T05:55:15Z

Summary of changes

LPC1114FN28: Flash 32kB, RAM 4kB

This PR add baremetal profile support for LPC1114.

Add supprorted_c_libs with small lib in targets.json
Remove the target from some storage tests which require RAM more than this device
Fix comment typo ARM linker script
Polish GCC_ARM linker script
Remove unused section in the startup code for GCC_ARM
Disable interrupt in us_ticker_init (detected by tests-mbed_hal-common_tickers test case)

Note that IAR toolchain is not tested, because it does not support small library.

Impact of changes

Migration actions required

Documentation

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)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

ARM test result (Arm compiler 6.13)
lpc1114-arm.txt
GCC_ARM test result (GCC compiler 9.2.1)
lpc1114-gcc_arm.txt

Reviewers

@0xc0170 @MarceloSalazar

ciarmcom · 2020-03-13T06:00:33Z

@toyowata, thank you for your changes.
@maclobdell @MarceloSalazar @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

bulislaw

@evedon please review
@jamesbeyond can we do something smart about these test cases? I don't want to accumulate list of exceptions like that

TESTS/configs/baremetal.json

targets/targets.json

evedon · 2020-03-13T15:11:10Z

@evedon please review
@jamesbeyond can we do something smart about these test cases? I don't want to accumulate list of exceptions like that

All Mbed 2 board should set "target.c_lib": "small" in targets.json instead of TESTS/configs/baremetal.json.

And we need to do something about IAR not supporting small library. The new tools will revert to standard library. The issue is what to do with Mbed OS 5 tools.

JojoS62 · 2020-03-14T12:20:22Z

I have had tested also a LPC824 with mbed5 bare-metal. This works well, and had also raised an issue about the default_lib, which is actually an indicator that this target needs to be tested with mbed5. But therefore targets.json has to be changed.
How about a more generic keyword in the targets description? Like an Info/Warning/Error message that could be displayed in the build process. For the mbed2 targets f.e., a 'warning: this target has not been tested with mbed-os 5' ?
Another issue I found during testing: the settings for STDIO_UART_RX/TX settings do not apply to LPC targets. Instead of changing this for each target individually, I suggest to change it in "LPCTarget" or even "Target" as the base class for all targets. To make this fully working, the Pinnames.h has to use the STDIO_UART pinnames.
For targets with only 4 k RAM, the default stack size has to be decreased.
Any comments? I don't now the roadmap, but the integration of mbed2 looks great. Maybe the next mbed should not be v6, but mbed-os 7. Because its now really 5+2 :)

toyowata · 2020-03-16T00:26:31Z

Applied review feedback from @evedon and rebased.

features/storage/TESTS/blockdevice/heap_block_device/main.cpp

MarceloSalazar

Minor comment - looks good - thanks!

evedon · 2020-03-17T16:30:05Z

Applied review feedback from @evedon and rebased.

You need to add ["5"] to "release_versions" in targets.json.
Otherwise changes look good.

evedon · 2020-03-17T16:40:29Z

How about a more generic keyword in the targets description? Like an Info/Warning/Error message that could be displayed in the build process. For the mbed2 targets f.e., a 'warning: this target has not been tested with mbed-os 5' ?

We will look into addressing this request in the migration guide for Mbed 2 targets. If a target "release_versions" only lists ["2"] then it is an indication that it was not tested with Mbed OS 5.

Another issue I found during testing: the settings for STDIO_UART_RX/TX settings do not apply to LPC targets. Instead of changing this for each target individually, I suggest to change it in "LPCTarget" or even "Target" as the base class for all targets. To make this fully working, the Pinnames.h has to use the STDIO_UART pinnames.

What change are you proposing for "LPCTarget"?

JojoS62 · 2020-03-17T17:06:58Z

What change are you proposing for "LPCTarget"?

I can create a PR, I have added this config section to the LPCTarget, but I think this config should be in "Target", there is already the setting for "console-uart".

    "LPCTarget": {
        "inherits": [
            "Target"
        ],
        "post_binary_hook": {
            "function": "LPCTargetCode.lpc_patch"
        },
        "config": {
                "stdio_uart_tx": {
                "help": "default TX STDIO pins is defined in PinNames.h file, but it can be overridden"
            },
            "stdio_uart_rx": {
                "help": "default RX STDIO pins is defined in PinNames.h file, but it can be overridden"
            }
        },
        "public": false

Actually, the LPC targets do not know about stdio_uart_tx/rx settings. When it is not used in PinNames.h, it does not break compatibility.

toyowata · 2020-03-18T04:11:12Z

@evedon

You need to add ["5"] to "release_versions" in targets.json.

Done. Pushed commit and rebased to master.

evedon · 2020-03-18T09:38:41Z

Which command did you use to run the greentea tests? I was expecting to see some tests skipped (those which are skipped in bare metal mode).

toyowata · 2020-03-18T13:59:01Z

@evedon

Which command did you use to run the greentea tests?

The command is:

mbed test -m lpc1114 -t arm --app-config TESTS/configs/baremetal.json

0xc0170 · 2020-03-18T15:00:41Z

CI started

0xc0170 · 2020-03-19T13:07:53Z

@toyowata There's an error in the logs: [2020-03-18T15:03:43.493Z] tools.utils.InvalidReleaseTargetException: Target 'LPC1114' must set the 'c_lib' to 'std' to be included in the mbed OS 5.0 official release.

hugueskamba · 2020-04-24T14:24:48Z

features/storage/TESTS/blockdevice/heap_block_device/main.cpp

@@ -23,6 +23,11 @@

 using namespace utest::v1;

+// TODO HACK, replace with available ram/heap property
+#if defined(TARGET_LPC1114)


This does not seem right. Replace TARGET_LPC1114 by what is preventing the target from running the test.
Do the same everywhere you have prevented a test from running.

@hugueskamba Thanks for your comment. How do you think about this?

#define TEST_BLOCK_SIZE 128 #define TEST_BLOCK_DEVICE_SIZE 32*TEST_BLOCK_SIZE #define TEST_BLOCK_COUNT 10 #define TEST_ERROR_MASK 16 #if (MBED_RAM_SIZE <= TEST_BLOCK_DEVICE_SIZE) #error [NOT_SUPPORTED] Insufficient heap for heap block device tests #endif

I am not sure why this test needs to be excluded for that board.
Here is a question, why do you need to exclude this test for that board?
If the reason is that the size of the block device is greater than the available RAM then it is fine.

Because the LPC1114 RAM size is 4kB which is not suitable to run this test case.

#define TEST_BLOCK_SIZE 128 #define TEST_BLOCK_DEVICE_SIZE 32*TEST_BLOCK_SIZE (snip) void test_read_write() { uint8_t *dummy = new (std::nothrow) uint8_t[TEST_BLOCK_DEVICE_SIZE];

Because the LPC1114 RAM size is 4kB which is not suitable to run this test case.

Then have that in your pre-processor macro.

toyowata · 2020-04-30T05:28:34Z

@hugueskamba I added a commit for some test cases to add memory check for RAM constraint device here, please can you review.

toyowata · 2020-05-08T01:02:28Z

rebased.

@0xc0170 @hugueskamba Any more work to do for this PR?

0xc0170 · 2020-05-08T10:56:42Z

CI started

mbed-ci · 2020-05-08T12:55:08Z

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 6
Build artifacts

hugueskamba · 2020-05-11T09:27:52Z

features/storage/TESTS/blockdevice/heap_block_device/main.cpp

@@ -28,6 +28,10 @@ using namespace utest::v1;
 #define TEST_BLOCK_COUNT 10
 #define TEST_ERROR_MASK 16

+#if ((MBED_RAM_SIZE - MBED_BOOT_STACK_SIZE) <= TEST_BLOCK_DEVICE_SIZE)


Why is this condition different than the other two files but the comment the same?

Because the heap_block_device test case allocates memory using TEST_BLOCK_DEVICE_SIZE.

mbed-os/features/storage/TESTS/blockdevice/heap_block_device/main.cpp

Line 45 in a707fd1

uint8_t *dummy = new (std::nothrow) uint8_t[TEST_BLOCK_DEVICE_SIZE];

And other test cases use different symbols.

mbed-os/features/storage/TESTS/blockdevice/mbr_block_device/main.cpp

Line 35 in a707fd1

uint8_t *dummy = new (std::nothrow) uint8_t[BLOCK_COUNT * BLOCK_SIZE];

hugueskamba · 2020-05-11T09:30:06Z

targets/targets.json

+            ]
+        },
+        "c_lib": "small",
+        "supported_application_modes": ["bare-metal"],


You still haven’t modified this.

toyowata · 2020-05-12T01:14:43Z

@hugueskamba @0xc0170
I fixed to use supported_application_profiles in the targets.json and rebased.

hugueskamba

Approved assuming you successfully ran mbed-os-example-blinky-baremetal on this target after the latest changes.

toyowata · 2020-05-13T11:17:19Z

@hugueskamba Yes, the mbed-os-example-blinky-baremetal works fine on this target.

0xc0170 · 2020-05-13T17:27:13Z

CI started

mbed-ci · 2020-05-13T18:17:03Z

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 8
Build artifacts

toyowata requested a review from 0xc0170 March 13, 2020 05:55

ciarmcom requested review from maclobdell, MarceloSalazar and a team March 13, 2020 06:00

ciarmcom added the needs: review label Mar 13, 2020

bulislaw reviewed Mar 13, 2020

View reviewed changes

evedon reviewed Mar 13, 2020

View reviewed changes

TESTS/configs/baremetal.json Outdated Show resolved Hide resolved

targets/targets.json Show resolved Hide resolved

targets/targets.json Outdated Show resolved Hide resolved

toyowata force-pushed the baremetal_lpc1114 branch from ad71d67 to dae80ab Compare March 16, 2020 00:20

MarceloSalazar reviewed Mar 16, 2020

View reviewed changes

features/storage/TESTS/blockdevice/heap_block_device/main.cpp Outdated Show resolved Hide resolved

MarceloSalazar reviewed Mar 16, 2020

View reviewed changes

toyowata force-pushed the baremetal_lpc1114 branch from e0a1414 to f642a0b Compare March 18, 2020 04:08

evedon previously approved these changes Mar 18, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Mar 18, 2020

VeijoPesonen approved these changes Mar 18, 2020

View reviewed changes

0xc0170 added needs: work and removed needs: CI labels Mar 19, 2020

hugueskamba requested changes Apr 24, 2020

View reviewed changes

toyowata force-pushed the baremetal_lpc1114 branch from fa59351 to c3614dc Compare April 30, 2020 05:22

toyowata force-pushed the baremetal_lpc1114 branch from d104e70 to 8037647 Compare May 8, 2020 01:00

hugueskamba reviewed May 11, 2020

View reviewed changes

toyowata force-pushed the baremetal_lpc1114 branch from df10ab7 to 2f37066 Compare May 12, 2020 01:06

toyowata added 6 commits May 12, 2020 10:07

Support bare-metal build for NXP LPC1114

d6779bb

LPC1114 - linker script & startup code improvement

ab791ae

LPC1114 - Add supported_application_modes

41435a0

[TESTS] Add memory check for RAM constraint device

5a57670

[LPC11XX] Disable interrupt in us_ticker_init

9cb0391

LPC1114 - add supported_application_profiles

58acc4e

toyowata force-pushed the baremetal_lpc1114 branch from 2f37066 to 58acc4e Compare May 12, 2020 01:07

0xc0170 added needs: CI and removed needs: work labels May 12, 2020

hugueskamba approved these changes May 12, 2020

View reviewed changes

0xc0170 approved these changes May 15, 2020

View reviewed changes

0xc0170 added ready for merge and removed needs: CI labels May 15, 2020

adbridge added the release-version: 6.0.0-rc1 label May 15, 2020

0xc0170 merged commit 6cbe22d into ARMmbed:master May 15, 2020

mergify bot removed the ready for merge label May 15, 2020

toyowata deleted the baremetal_lpc1114 branch June 6, 2020 16:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LPC1114: baremetal profile support #12623

LPC1114: baremetal profile support #12623

toyowata commented Mar 13, 2020 •

edited

ciarmcom commented Mar 13, 2020

bulislaw left a comment

evedon commented Mar 13, 2020

JojoS62 commented Mar 14, 2020 •

edited

toyowata commented Mar 16, 2020

MarceloSalazar left a comment •

edited

evedon commented Mar 17, 2020

evedon commented Mar 17, 2020

JojoS62 commented Mar 17, 2020

toyowata commented Mar 18, 2020

evedon commented Mar 18, 2020

toyowata commented Mar 18, 2020

0xc0170 commented Mar 18, 2020

0xc0170 commented Mar 19, 2020

hugueskamba Apr 24, 2020 •

edited

toyowata Apr 28, 2020 •

edited

hugueskamba Apr 28, 2020 •

edited

toyowata Apr 29, 2020

hugueskamba Apr 29, 2020

toyowata commented Apr 30, 2020

toyowata commented May 8, 2020

0xc0170 commented May 8, 2020

mbed-ci commented May 8, 2020

hugueskamba May 11, 2020

toyowata May 12, 2020

hugueskamba May 11, 2020

toyowata commented May 12, 2020

hugueskamba left a comment

toyowata commented May 13, 2020

0xc0170 commented May 13, 2020

mbed-ci commented May 13, 2020

LPC1114: baremetal profile support #12623

LPC1114: baremetal profile support #12623

Conversation

toyowata commented Mar 13, 2020 • edited

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Mar 13, 2020

bulislaw left a comment

Choose a reason for hiding this comment

evedon commented Mar 13, 2020

JojoS62 commented Mar 14, 2020 • edited

toyowata commented Mar 16, 2020

MarceloSalazar left a comment • edited

Choose a reason for hiding this comment

evedon commented Mar 17, 2020

evedon commented Mar 17, 2020

JojoS62 commented Mar 17, 2020

toyowata commented Mar 18, 2020

evedon commented Mar 18, 2020

toyowata commented Mar 18, 2020

0xc0170 commented Mar 18, 2020

0xc0170 commented Mar 19, 2020

hugueskamba Apr 24, 2020 • edited

Choose a reason for hiding this comment

toyowata Apr 28, 2020 • edited

Choose a reason for hiding this comment

hugueskamba Apr 28, 2020 • edited

Choose a reason for hiding this comment

toyowata Apr 29, 2020

Choose a reason for hiding this comment

hugueskamba Apr 29, 2020

Choose a reason for hiding this comment

toyowata commented Apr 30, 2020

toyowata commented May 8, 2020

0xc0170 commented May 8, 2020

mbed-ci commented May 8, 2020

Test run: SUCCESS

hugueskamba May 11, 2020

Choose a reason for hiding this comment

toyowata May 12, 2020

Choose a reason for hiding this comment

hugueskamba May 11, 2020

Choose a reason for hiding this comment

toyowata commented May 12, 2020

hugueskamba left a comment

Choose a reason for hiding this comment

toyowata commented May 13, 2020

0xc0170 commented May 13, 2020

mbed-ci commented May 13, 2020

Test run: SUCCESS

toyowata commented Mar 13, 2020 •

edited

JojoS62 commented Mar 14, 2020 •

edited

MarceloSalazar left a comment •

edited

hugueskamba Apr 24, 2020 •

edited

toyowata Apr 28, 2020 •

edited

hugueskamba Apr 28, 2020 •

edited