Bring static pinmap extension to master #11892

mprse · 2019-11-19T10:49:51Z

Description

In modern MCUs peripherals often can be mapped to different pins and each pin can have multiple functions. Mbed supports dynamic pin mapping, meaning that pins can be reconfigured at run time to be used by a different driver. That provides great flexibility, but it's not free. There's non-trivial ROM cost to maintain the pinmap tables and infrastructure to parse it. In some use cases, this flexibility is worth the cost. Quite often pin configuration is frozen at hw design stage and doesn't require runtime modification. Shifting this configuration to compile-time will allow us free memory associated with the dynamic approach.

Summary of change

HAL APIs making use of pins take these pins in their constructor and use those pins to lookup which peripheral/function to use. The process of looking up the peripheral/function requires there to be a pinmap table that maps pins to peripherals/functions. This pinmap table takes up ROM which could be saved if the pinmap wasn't used. The goal is to provide additional HAL API/constructors which takes pinmap as a parameter where pin/peripheral/function is specified statically and there is no need to use the pinmap tables. Also, add the constant expression utility functions to find pin mappings in compile time. This extension should give the following savings:

removed pinmap tables,
removed hal\mbed_pinmap_common library (required for pin lookup mechanism),
reduced HAL driver code.

More information can be found in the design document.

Documentation

Design doc:
https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-explicit-pinmap/docs/design-documents/hal/0003-static-pinmap-extension.md

Porting guide:
ARMmbed/mbed-os-5-docs#1156

Pull request type

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

Test results (required)

[] 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

@ARMmbed/mbed-os-hal

Release Notes

Summary of changes

Provide types which will hold static pinmaps for peripherals(PWM, AnalogIn, AnalogOut, SPI, I2C, UART, QSPI, CAN).
Provide xxx_init_direct(xxx_t *obj, static_pinmap_t *) functions to HAL API (these functions will not use pinmap tables).
Provide additional constructors in drivers layer which will use the xxx_init_direct(xxx_t *obj, static_pinmap_t*) HAL functions.
Provide default weak implementations of xxx_init_direct(static_pinmap_t *) functions. These functions will call standard xxx_init(xxx_t *obj, PinName, ...) function (backward compatibility for targets which do not support static pinmap mechanism).
Provide constexpr utility functions to lookup for pin mapping in compile time (requires C++14).
Initialize console using static pinmap mechanism, so hal\mbed_pinmap_common library is not needed and can be removed.
Modify FPGA tests to verify xxx_init_direct(xxx_t *obj, static_pinmap_t*) APIs.

ciarmcom · 2019-11-19T12:01:02Z

@mprse, thank you for your changes.
@maclobdell @ARMmbed/mbed-os-core @ARMmbed/mbed-os-test @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-maintainers please review.

mprse · 2019-11-19T13:49:42Z

cc @kjbracey-arm

bulislaw · 2019-11-19T16:32:00Z

Before I start reviewing it, is the merge commit ok in a PR? @ARMmbed/mbed-os-maintainers

0xc0170 · 2019-11-21T09:14:27Z

As this is targeting feature release, it would be. But simple rebase on the branch should remove it.

@mprse Can you rebase your branch and it should be nice linear log

0xc0170 · 2019-11-21T09:14:45Z

@mprse Is this targeting 5.15 ? Or 6.0?

bulislaw · 2019-11-21T14:10:05Z

5.15 if it can :P

bulislaw

I skimmed the files only. LGTM

@kjbracey-arm

0xc0170 · 2019-11-22T13:27:05Z

We shall finalize reviews here today, please go ahead!

I'll do mine soon

mprse · 2019-11-22T14:19:51Z

@mprse Can you rebase your branch and it should be nice linear log

Fixed

0xc0170 · 2019-11-25T07:57:57Z

CI started

0xc0170 · 2019-11-25T11:37:07Z

@mprse Can you not remove Release notes section (please add it back) ?

mprse · 2019-11-25T12:26:14Z

@mprse Can you not remove Release notes section (please add it back) ?

Fixed

mbed-ci · 2019-11-25T12:48:54Z

Test run: FAILED

Summary: 1 of 12 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

mprse · 2019-11-25T13:48:36Z

| K64F-GCC_ARM | K64F          | features-storage-tests-kvstore-general_tests_phase_1                         | FAIL   | 115.35             | default     | 
| DISCO_L475VG_IOT01A-ARMC6 | DISCO_L475VG_IOT01A | features-storage-tests-kvstore-direct_access_devicekey_test                  | TIMEOUT | 147.52             | default     |
| DISCO_L475VG_IOT01A-ARMC6 | DISCO_L475VG_IOT01A | features-storage-tests-kvstore-static_tests                                  | TIMEOUT | 329.23             | default     |
| DISCO_L475VG_IOT01A-ARMC6 | DISCO_L475VG_IOT01A | features-storage-tests-blockdevice-general_block_device                      | TIMEOUT | 323.21             | default     |
| DISCO_L475VG_IOT01A-ARMC6 | DISCO_L475VG_IOT01A | features-device_key-tests-device_key-functionality                           | TIMEOUT | 73.1               | default     |
| DISCO_L475VG_IOT01A-GCC_ARM | DISCO_L475VG_IOT01A | features-device_key-tests-device_key-functionality                           | TIMEOUT | 74.53              | default     |
| DISCO_L475VG_IOT01A-GCC_ARM | DISCO_L475VG_IOT01A | features-storage-tests-blockdevice-general_block_device                      | TIMEOUT | 325.74             | default     | 
| DISCO_L475VG_IOT01A-GCC_ARM | DISCO_L475VG_IOT01A | features-storage-tests-kvstore-static_tests                                  | FAIL    | 33.95              | default     | 
| DISCO_L475VG_IOT01A-GCC_ARM | DISCO_L475VG_IOT01A | tests-mbed_hal-qspi                                                          | FAIL    | 29.01              | default     | 
| DISCO_L475VG_IOT01A-IAR | DISCO_L475VG_IOT01A | features-device_key-tests-device_key-functionality                           | TIMEOUT | 72.05              | default     | 
| DISCO_L475VG_IOT01A-IAR | DISCO_L475VG_IOT01A | features-storage-tests-blockdevice-general_block_device                      | TIMEOUT | 323.19             | default     | 
| DISCO_L475VG_IOT01A-IAR | DISCO_L475VG_IOT01A | features-storage-tests-kvstore-static_tests                                  | FAIL    | 30.46              | default     | 
| DISCO_L475VG_IOT01A-IAR | DISCO_L475VG_IOT01A | tests-mbed_hal-qspi                                                          | FAIL    | 26.55              | default     | 
| NUCLEO_F411RE-ARMC6 | NUCLEO_F411RE | tests-mbed_hal-sleep_manager                                                 | FAIL   | 22.99              | default     | 
| NUCLEO_F411RE-IAR | NUCLEO_F411RE | tests-mbed_hal-sleep_manager                                                 | FAIL   | 23.02              | default     |

A lot of tests have failed. I'm not sure if these failures are related to the changes. I haven't seen such failures while working on feature branch. If this requires further investigation I will continue when I will be back from sick-leave (Thursday). Meanwhile can we repeat the test run?

0xc0170 · 2019-11-28T11:34:10Z

The storage fix landed, as I can see this needs now one more change. Once done, let us know to start CI asap

mprse · 2019-11-28T11:50:09Z

Rebased and added the fix for STM QSPI driver.
Checked that tests-mbed_hal-qspi test now passes. This should also fix problems with storage tests (I run a few of them with positive results).

@0xc0170 Can we run CI again?

0xc0170 · 2019-11-28T12:02:30Z

CI started

mbed-ci · 2019-11-28T14:41:15Z

Test run: FAILED

Summary: 1 of 12 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_wisun-mesh-test

updated

0xc0170 · 2019-11-28T14:45:15Z

jenkins-ci/mbed-os-ci_wisun-mesh-test

Failed prior restart, all fine here.

mprse · 2019-11-28T14:46:13Z

Test run: FAILED

It looks like all passed. The result is FAILED because jenkins-ci/wisun-mesh-test has failed, but was restarted and then passed.

ciarmcom requested review from maclobdell and a team November 19, 2019 12:01

ciarmcom added the needs: review label Nov 19, 2019

0xc0170 added the release-version: 5.15.0-rc1 label Nov 19, 2019

mprse mentioned this pull request Nov 20, 2019

Add the porting guide for the explicit pin-map extension. ARMmbed/mbed-os-5-docs#1156

Merged

bulislaw approved these changes Nov 21, 2019

View reviewed changes

0xc0170 requested a review from kjbracey November 21, 2019 14:58

mprse force-pushed the feature-hal-spec-explicit-pinmap branch from 3927aae to cd3989e Compare November 22, 2019 14:17

0xc0170 requested review from a team and removed request for a team November 25, 2019 07:58

kjbracey approved these changes Nov 25, 2019

View reviewed changes

0xc0170 approved these changes Nov 25, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Nov 25, 2019

mprse added 13 commits November 28, 2019 08:32

NUCLEO_F429ZI add CAN pinmaps

b35579b

QSPI: Add pointer to HAL init function

ef8a994

get_qspi_pinmap(): Fix invalid condition

caf438e

CAN: remove redundant loop

0147af8

prevent passing of temporary pinmap objects

933e774

Serial, QSPI: Fix style

7db9934

Add design document for explicit pinmap extension

ce2c10a

Use html tables instead of images, add missing details.

181461d

Change naming: explicit to static

1b3cce6

Update Overview and background section

fcef50f

Move GPIO_AF_NONE from PeripheralPins.h to PinNamesTypes.h

e3a34a5

Change explicit pinmap to static pinmap

b2dad08

Add pwmout_init_direct() stub (fix for unittests)

3d03d35

STM QSPI driver: return init status, fix pin function setting

2e79384

mprse force-pushed the feature-hal-spec-explicit-pinmap branch from cd3989e to 2e79384 Compare November 28, 2019 11:45

0xc0170 requested a review from jamesbeyond November 28, 2019 12:02

0xc0170 added needs: CI and removed needs: work labels Nov 28, 2019

0xc0170 removed the needs: CI label Nov 28, 2019

0xc0170 merged commit aa16859 into ARMmbed:master Nov 28, 2019

jeromecoutant mentioned this pull request Dec 16, 2019

STM32 can_init_freq() ignores frequency and uses 100000 instead #12104

Closed

mprse mentioned this pull request Feb 19, 2020

Allow MISO/MOSI set to NC during SPI initialisation (fix for issue #12435) #12460

Merged

multiplemonomials mentioned this pull request May 31, 2024

Fix PwmOut::resume() for static pinmap usage mbed-ce/mbed-os#281

Merged

Bring static pinmap extension to master #11892

Bring static pinmap extension to master #11892

Uh oh!

Conversation

mprse commented Nov 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Summary of change

Documentation

Pull request type

Test results (required)

Reviewers

Release Notes

Summary of changes

Uh oh!

ciarmcom commented Nov 19, 2019

Uh oh!

mprse commented Nov 19, 2019

Uh oh!

bulislaw commented Nov 19, 2019

Uh oh!

0xc0170 commented Nov 21, 2019

Uh oh!

0xc0170 commented Nov 21, 2019

Uh oh!

bulislaw commented Nov 21, 2019

Uh oh!

bulislaw left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Nov 22, 2019

Uh oh!

mprse commented Nov 22, 2019

Uh oh!

0xc0170 commented Nov 25, 2019

Uh oh!

0xc0170 commented Nov 25, 2019

Uh oh!

mprse commented Nov 25, 2019

Uh oh!

mbed-ci commented Nov 25, 2019

Test run: FAILED

Uh oh!

mprse commented Nov 25, 2019

Uh oh!

0xc0170 commented Nov 28, 2019

Uh oh!

mprse commented Nov 28, 2019

Uh oh!

0xc0170 commented Nov 28, 2019

Uh oh!

mbed-ci commented Nov 28, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Nov 28, 2019

Uh oh!

mprse commented Nov 28, 2019

Uh oh!

Uh oh!

mprse commented Nov 19, 2019 •

edited

Loading