Add STDIO UART as restricted for FPGA testing for all targets and support for restricting GPIO #12379

mprse · 2020-02-06T13:00:58Z

Summary of changes

Substantiation for this is that the STDIO UART peripheral is used by Mbed, so it should never be tested.

Also, solve the potential problem with accidentally skipped peripherals in FPGA testing. Currently, we have a one pinmap_restricted_peripherals() function for all interfaces (UART, I2C, SPI, etc.).
The problem can be encountered if different interfaces have the same peripheral ids (e.g. UART_0 = 0, SPI_0 = 0). In this case, if UART_0 is on the restricted list, then SPI tests will be also skipped for SPI_0.
The good news is that usually, the peripheral ids are the base addresses of the peripheral's register set, but we can't rely on this. It is also good that pinmap_restricted_peripherals() at this moment is only required for STDIO UART (Nuvoton and STM).

To solve this issue we will change name of pinmap_restricted_peripherals() to pinmap_uart_restricted_peripherals(), make STDIO UART restricted by default for all targets and update FPGA test utilily functions to use pinmap_uart_restricted_peripherals() to skip only uart peripherals.
In the future, if needed we can consider adding support to restrict peripherals of other interfaces(SPI, I2C, etc).

As a side effect of this change, we need to remove Nuvoton implementations of pinmap_restricted_peripherals(). This function is no more available. Was replaced by pinmap_uart_restricted_peripherals() and its weak implementation restricts FPGA testing on STDIO UART peripheral by default.

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

Reviewers

@ARMmbed/team-nuvoton

@maciejbocianski @fkjagodzinski

@jamesbeyond

LMESTM

Good to handle stdio_uart in a generic way ! thx for this

ciarmcom · 2020-02-06T14:00:43Z

@mprse, thank you for your changes.
@fkjagodzinski @maciejbocianski @Ronny-Liu @jamesbeyond @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-maintainers please review.

maciejbocianski · 2020-02-06T14:04:07Z

components/testing/COMPONENT_FPGA_CI_TEST_SHIELD/test_utils.h

@@ -20,6 +20,15 @@

 #include <list>

+#define UARTMAPS_NAME     "UART"


Make names consistent

jamesbeyond

good work, thanks for making changes this way, it makes things much easier. I'll tag @ARMmbed/team-nuvoton also have a look at the changes on their code

cyliangtw

So far, look good.
In the future, for flexibility, we could consider to use hash table to store "NAME" + ID for peripherals[] in pinmap_uart_restricted_peripherals().

Pull request has been modified.

Substantiation for this is that the STDIO UART peripheral is used by Mbed, so it should never be tested. Also solve the potential problem with accidenty skipped peripherals in FPGA testing. Currently, we have a one `pinmap_restricted_peripherals()` function for all interfaces (UART, I2C, SPI, etc.). The problem can be encountered if different interfaces have the same peripheral ids (e.g. `UART_0` = 0, `SPI_0` = 0). In this case, if `UART_0` is on the restricted list, then SPI tests will be also skipped for `SPI_0`. The good news is that usually, the peripheral ids are the base addresses of the peripheral's register set, but we can't rely on this. It is also good that `pinmap_restricted_peripherals()` at this moment is only required for STDIO UART (Nuvoton and STM). To solve this issue we will change name of `pinmap_restricted_peripherals()` to `pinmap_uart_restricted_peripherals()`, make STDIO UART restricted by default for all targets and update FPGA test utilily functions to use `pinmap_uart_restricted_peripherals()` to skip only uart peripherals. In the future if needed we can consider to add support to restrict peripherals of other interfaces(SPI, I2C, etc).

…art is restricted by default)

…pins This is a special case since targets do not provide by default GPIO pin-maps. This extension is required for FPGA GPIO tests - if some pins have limitations (e.g. fixed pull-up) we need to skip these pins while testing. To do that we were adding a dummy pin-map with commented out pins that can't be tested because of the limitations. This solution is redundant and the proposition is to provide a list of restricted gpio pins if required (by default weak implementation is provided with an empty list). This mechanism will be backward compatible, so the old method with dummy gpio pinmap will also work. The switch from dummy pin-maps to pinmap_gpio_restricted_pins() will be performed in separate commits/PRs.

mprse · 2020-02-07T09:46:41Z

Extend this patch and add similar functionality to restrict GPIO pins.

This is a special case since targets do not provide by default GPIO pin-maps. This extension is required for FPGA GPIO tests - if some pins have limitations (e.g. fixed pull-up) we need to skip these pins while testing. To do that we were adding a dummy pin-map with commented out pins that can't be tested because of the limitations.
This solution is redundant and the proposition is to provide a list of restricted GPIO pins if required (by default weak implementation is provided with an empty list).

This mechanism will be backward compatible, so the old method with dummy GPIO pinmap will also work. The switch from dummy pin-maps to pinmap_gpio_restricted_pins() will be performed in separate commits/PRs.

maciejbocianski · 2020-02-07T10:10:08Z

looks good

0xc0170 · 2020-02-07T15:23:34Z

CI started

mbed-ci · 2020-02-07T16:54:39Z

Test run: FAILED

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

Failed test jobs:

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

jamesbeyond · 2020-02-07T17:09:39Z

seems something went wrong with LPC55S69_S, when running

python tools/psa/release.py -m LPC55S69_S

throws


[2020-02-07T16:25:23.027Z] Compile [ 60.6%]: mbed_pinmap_default.cpp
[2020-02-07T16:25:23.027Z] [Error] mbed_pinmap_default.cpp@95,68: use of undeclared identifier 'serial_tx_pinmap'
[2020-02-07T16:25:23.027Z] [ERROR] ./hal/mbed_pinmap_default.cpp:95:68: error: use of undeclared identifier 'serial_tx_pinmap'
[2020-02-07T16:25:23.027Z]     static const int stdio_uart = pinmap_peripheral(STDIO_UART_TX, serial_tx_pinmap());

@mprse

mprse · 2020-02-07T18:15:57Z

In the build directory, I can see all passed.

Is there no serial available on LPC55S69_S?

…RIAL

Pull request has been modified.

mprse · 2020-02-10T07:21:24Z

Provided a fix for:
python tools/psa/release.py -m LPC55S69_S

@0xc0170 can we try again?

0xc0170 · 2020-02-10T12:55:45Z

CI restarted

mbed-ci · 2020-02-10T14:30:34Z

Test run: SUCCESS

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

fkjagodzinski · 2020-02-10T14:35:53Z

hal/pinmap.h

+ *
+ * @return Pointer to a peripheral list of peripheral to avoid
+ */
+const PinList *pinmap_gpio_restricted_pins(void);


It looks like we've come full circle and come back to the idea from #10459. 😄

jamesbeyond · 2020-02-10T16:41:09Z

please tag this PR as alpha-2-release @adbridge

mprse · 2020-02-11T09:56:28Z

@0xc0170 @adbridge @jamesbeyond
This one is ready to go. When this is in we can merge:

While testing it has been found that all tests are skipped in the FPGA uart test on the NRF52840 target. This is caused by the following change: ARMmbed#12379 - Add STDIO UART as restricted for FPGA testing for all targets NRF targets have MUXed pins and mainly do not provide pin-maps. There are only dummy pin-maps for testing. These pin-maps hold only pins and do not specify the peripheral or function of the pin (always 0). Because of that if we restrict STDIO uart peripheral (0) all FPGA uart test cases will be skipped. To fix this we will remove this restriction for NRF52840. Restriction for testing the USBTX, USBRX pins is sufficient in this case.

mprse mentioned this pull request Feb 6, 2020

STM32L4: Fix the UART RX & TX data reg bitmasks #12341

Merged

mprse requested a review from ccli8 February 6, 2020 13:19

LMESTM approved these changes Feb 6, 2020

View reviewed changes

ciarmcom requested review from fkjagodzinski, jamesbeyond, maciejbocianski, Ronny-Liu and a team February 6, 2020 14:00

ciarmcom added the needs: review label Feb 6, 2020

maciejbocianski reviewed Feb 6, 2020

View reviewed changes

mprse force-pushed the STDIO_UART_restricted_all branch from 92c86d6 to a7e70ba Compare February 6, 2020 14:35

jamesbeyond previously approved these changes Feb 6, 2020

View reviewed changes

cyliangtw approved these changes Feb 7, 2020

View reviewed changes

mprse force-pushed the STDIO_UART_restricted_all branch from a7e70ba to 50bb413 Compare February 7, 2020 09:43

mprse added 3 commits February 7, 2020 10:45

Remove pinmap_restricted_peripherals() function from Nuvoton (STDIO u…

a4e1354

…art is restricted by default)

mprse force-pushed the STDIO_UART_restricted_all branch from 50bb413 to 3c0982d Compare February 7, 2020 09:45

mprse changed the title ~~Add STDIO UART as restricted for FPGA testing for all targets~~ Add STDIO UART as restricted for FPGA testing for all targets and support for restricting GPIO Feb 7, 2020

mprse mentioned this pull request Feb 7, 2020

DISCO_L475VG_IOT01A: Add a list of restricted GPIO pins for testing #12380

Merged

maciejbocianski approved these changes Feb 7, 2020

View reviewed changes

jamesbeyond previously approved these changes Feb 7, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Feb 7, 2020

cyliangtw approved these changes Feb 7, 2020

View reviewed changes

mprse mentioned this pull request Feb 7, 2020

LPC55S69: Fix UART & GPIO HAL to pass FPGA CI test shield tests #12342

Merged

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

Provide pinmap_uart_restricted_peripherals() function under DEVICE_SE…

0d9a54f

…RIAL

0xc0170 added needs: CI and removed needs: work labels Feb 10, 2020

fkjagodzinski approved these changes Feb 10, 2020

View reviewed changes

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

jamesbeyond approved these changes Feb 11, 2020

View reviewed changes

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

0xc0170 merged commit d3078a3 into ARMmbed:master Feb 11, 2020

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

This was referenced Feb 11, 2020

Disable restricted uart peripheral for NRF52840 #12407

Merged

Remove GPIO pin-maps used for FPGA testing #12436

Merged

		@@ -20,6 +20,15 @@

		#include <list>

		#define UARTMAPS_NAME "UART"

Add STDIO UART as restricted for FPGA testing for all targets and support for restricting GPIO #12379

Add STDIO UART as restricted for FPGA testing for all targets and support for restricting GPIO #12379

Uh oh!

Conversation

mprse commented Feb 6, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

LMESTM left a comment

Choose a reason for hiding this comment

Uh oh!

ciarmcom commented Feb 6, 2020

Uh oh!

maciejbocianski Feb 6, 2020

Choose a reason for hiding this comment

Uh oh!

mprse Feb 6, 2020

Choose a reason for hiding this comment

Uh oh!

jamesbeyond left a comment

Choose a reason for hiding this comment

Uh oh!

cyliangtw left a comment

Choose a reason for hiding this comment

Uh oh!

mprse commented Feb 7, 2020

Uh oh!

maciejbocianski commented Feb 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Feb 7, 2020

Uh oh!

mbed-ci commented Feb 7, 2020

Test run: FAILED

Uh oh!

jamesbeyond commented Feb 7, 2020

Uh oh!

mprse commented Feb 7, 2020

Uh oh!

mprse commented Feb 10, 2020

Uh oh!

0xc0170 commented Feb 10, 2020

Uh oh!

mbed-ci commented Feb 10, 2020

Test run: SUCCESS

Uh oh!

fkjagodzinski Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

mprse Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

jamesbeyond commented Feb 10, 2020

Uh oh!

mprse commented Feb 11, 2020

Uh oh!

Uh oh!

maciejbocianski commented Feb 7, 2020 •

edited

Loading