Add static pin-map support: SDBlockDevice, kvstore, system storage (reduce ROM used by Mbed Cloud Client example) #12058

mprse · 2019-12-09T13:47:34Z

Summary of changes

This PR adds static pin-map support to:

SDBlockDevice component,
kvstore feature,
system storage feature.

This is done to reduce ROM used by the Mbed Cloud Client Example.

Related PR #12033 (fixes pin-map for SerialBase class and ensures that serial flow control functions/pinmaps are not pulled into the image if flow control is not used).

Memory usage details GCC 9.0/Cloud Client Example below.

mbed-os-5.15 vs mbed-os-5.15 + PR #12033 + this PR

| Module                                                                                                                   |         .text |    .data |      .bss |
|--------------------------------------------------------------------------------------------------------------------------|---------------|----------|-----------|
| [fill]                                                                                                                   |       743(-3) |   19(+0) | 2697(-32) |
| mbed-os\drivers\source\SPI.o                                                                                             |      940(-26) |    0(+0) |  1096(+0) |
| mbed-os\drivers\source\SerialBase.o                                                                                      |      1092(-2) |    0(+0) |     0(+0) |
| mbed-os\features\storage\kvstore\conf\kv_config.o                                                                        |     1755(+28) |    0(+0) |  1269(+0) |
| mbed-os\features\storage\system_storage\SystemStorage.o                                                                  |      236(+28) |    0(+0) |   309(+0) |
| mbed-os\hal\mbed_pinmap_common.o                                                                                         |       0(-185) |    0(+0) |     0(+0) |
| mbed-os\targets\TARGET_Freescale\TARGET_MCUXpresso_MCUS\TARGET_MCU_K64F\TARGET_FRDM\PeripheralPins.o                     |      0(-1032) |    0(+0) |     0(+0) |
| mbed-os\targets\TARGET_Freescale\TARGET_MCUXpresso_MCUS\TARGET_MCU_K64F\serial_api.o                                     |    1270(-344) |    0(+0) |   220(+0) |
| mbed-os\targets\TARGET_Freescale\TARGET_MCUXpresso_MCUS\TARGET_MCU_K64F\spi_api.o                                        |     644(-272) |    0(+0) |     0(+0) |
| Subtotals                                                                                                                | 416646(-1808) | 3368(+0) | 56392(+0) |

Example output:

Mbed Bootloader
No Update image
[DBG ] Active firmware up-to-date
booting...

Start Device Management Client
Using hardcoded Root of Trust, not suitable for production use.
Starting developer flow
Developer credentials already exist, continuing..
Application ready. Build at: Dec  9 2019 12:29:00
Mbed OS version 5.15.0
mcc_platform_interface_connect()
Connecting with interface: Ethernet
NSAPI_STATUS_CONNECTING
NSAPI_STATUS_GLOBAL_UP
IP: 192.168.70.19
Network initialized, registering...
Client registered
Endpoint Name: 016eeaaaf361000000000001001dccb8
Device ID: 016eeaaaf361000000000001001dccb8

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

Reviewers

@jamesbeyond
@bulislaw
@0xc0170

ciarmcom · 2019-12-09T14:00:28Z

@mprse, thank you for your changes.
@jamesbeyond @bulislaw @0xc0170 @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

adbridge · 2019-12-09T14:02:18Z

@mprse is this needed for RC2 then ?

SeppoTakalo · 2019-12-09T14:18:51Z

API change (extension), so cannot be patch release.
Also two weeks after the code freeze so should be pushed to next release.

adbridge · 2019-12-09T14:32:17Z

API change (extension), so cannot be patch release.
Also two weeks after the code freeze so should be pushed to next release.

RC2 is not a patch release. We have not yet released 5.15 so this can indeed go to the release. It is also highly important to the client team.

0xc0170 · 2019-12-09T16:10:51Z

This is done to reduce ROM used by the Mbed Cloud Client Example.

To use functionality we have already in rc1, just enable other components to use it. It was agreed to have this in rc2.

jamesbeyond

LGTM

0xc0170 · 2019-12-09T16:38:19Z

@mprse Please fix Travis (there is an error in the code)

mprse · 2019-12-09T16:51:04Z

That's odd. I was testing this with mbed 5.15 and cloud client. I cherry-picked the commit, but it looks like on master something has changed meanwhile.
I'll try to provide fix at the evening or tomorrow morning.

mprse · 2019-12-10T08:13:49Z

Fixed and tested again with Cloud Client.

0xc0170 · 2019-12-10T08:16:43Z

CI started

mbed-ci · 2019-12-10T08:55:47Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

mprse · 2019-12-10T09:14:34Z

Test run: FAILED

Checked a few failures and looks like some CI issue:

[mbed-16353] WARNING: Could not find mbed program in current path "/builds/ws/mbed-os-ci_build-ARM@2/mbed-os".
You can fix this by calling "mbed new ." in the root of your program.

mbed-ci · 2019-12-10T10:05:13Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

0xc0170 · 2019-12-10T10:34:09Z

Failures look related:

[Error] SystemStorage.cpp@50,11: unknown type name 'spi_pinmap_t'
[Error] SystemStorage.cpp@50,59: use of undeclared identifier 'MBED_CONF_SD_SPI_MOSI'
[Error] SystemStorage.cpp@50,82: use of undeclared identifier 'MBED_CONF_SD_SPI_MISO'
[Error] SystemStorage.cpp@50,105: use of undeclared identifier 'MBED_CONF_SD_SPI_CLK'
[ERROR] ./mbed-os/features/storage/system_storage/SystemStorage.cpp:50:11: error: unknown type name 'spi_pinmap_t'
constexpr spi_pinmap_t static_spi_pinmap = get_spi_pinmap(MBED_CONF_SD_SPI_MOSI, MBED_CONF_SD_SPI_MISO, MBED_CONF_SD_SPI_CLK, NC);

This is done in order to enable static pin-map for Mbed Cloud Client Example. This should give extra ROM savings, ~1KB.

mprse · 2019-12-10T11:31:36Z

Moved constexpr spi_pinmap_t static_spi_pinmap = get_spi_pinmap(MBED_CONF_SD_SPI_MOSI, MBED_CONF_SD_SPI_MISO, MBED_CONF_SD_SPI_CLK, NC); definition under #ifdef COMPONENT_SD. This should solve above failure.

@0xc0170 Can we run CI again.

0xc0170 · 2019-12-10T11:42:07Z

CI restarted

mbed-ci · 2019-12-10T13:05:06Z

Test run: SUCCESS

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

mprse · 2019-12-10T13:09:33Z

This one passed CI, but unfortunately, I can see that PR #12033 has already one failed test (CI is running): DISCO_L475VG_IOT01A IAR / ASYNCHRONOUS_DNS_TIMEOUTS – DISCO_L475VG_IOT01A-IAR.tests-netsocket-dns.

ciarmcom requested review from 0xc0170, bulislaw, jamesbeyond and a team December 9, 2019 14:00

ciarmcom added the needs: review label Dec 9, 2019

adbridge added the release-version: 5.15.0-rc2 label Dec 9, 2019

jamesbeyond approved these changes Dec 9, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Dec 9, 2019

mprse force-pushed the static_pinmap_for_cloud_client branch from 4e964c4 to babe42d Compare December 10, 2019 07:21

0xc0170 added needs: CI and removed needs: work labels Dec 10, 2019

0xc0170 approved these changes Dec 10, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: CI labels Dec 10, 2019

Add static pinmap support: SDBlockDevice, kvstore, system storage

ee5953a

This is done in order to enable static pin-map for Mbed Cloud Client Example. This should give extra ROM savings, ~1KB.

mprse force-pushed the static_pinmap_for_cloud_client branch from babe42d to ee5953a Compare December 10, 2019 11:26

SeppoTakalo approved these changes Dec 10, 2019

View reviewed changes

adbridge added needs: CI and removed needs: work labels Dec 10, 2019

0xc0170 removed the needs: CI label Dec 10, 2019

0xc0170 merged commit f2a1804 into ARMmbed:master Dec 10, 2019

Add static pin-map support: SDBlockDevice, kvstore, system storage (reduce ROM used by Mbed Cloud Client example) #12058

Add static pin-map support: SDBlockDevice, kvstore, system storage (reduce ROM used by Mbed Cloud Client example) #12058

Uh oh!

Conversation

mprse commented Dec 9, 2019

Summary of changes

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Dec 9, 2019

Uh oh!

adbridge commented Dec 9, 2019

Uh oh!

SeppoTakalo commented Dec 9, 2019

Uh oh!

adbridge commented Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Dec 9, 2019

Uh oh!

jamesbeyond left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Dec 9, 2019

Uh oh!

mprse commented Dec 9, 2019

Uh oh!

mprse commented Dec 10, 2019

Uh oh!

0xc0170 commented Dec 10, 2019

Uh oh!

mbed-ci commented Dec 10, 2019

Test run: FAILED

Uh oh!

mprse commented Dec 10, 2019

Uh oh!

mbed-ci commented Dec 10, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Dec 10, 2019

Uh oh!

mprse commented Dec 10, 2019

Uh oh!

0xc0170 commented Dec 10, 2019

Uh oh!

mbed-ci commented Dec 10, 2019

Test run: SUCCESS

Uh oh!

mprse commented Dec 10, 2019

Uh oh!

Uh oh!

adbridge commented Dec 9, 2019 •

edited

Loading