Remove GPIO pin-maps used for FPGA testing #12436

mprse · 2020-02-14T12:33:21Z

Summary of changes

This is the continuation of #12379 (restricting GPIOs in FPGA testing instead of using dummy GPIO pin-maps).

This PR provides the following changes:

Update find_ports() FPGA testing utility function to loop through the form factor pins instead the pin-map.

This change is required to fully remove GPIO pin-maps which were already added for FPGA testing.
One use case of adding GPIO pin-map was that pin-map must have the specific format - must be ended with NC pin. Functions that deal with pin-maps loops through the pin-map until NC pin is encountered.
Also, our FPGA testing utility function to find pins for testing does that. When GPIO pin-maps are fully removed we will have one generic GPIO pinmap which provides Arduino pins: D0, D1, D2, etc. (only Arduino form factor is supported at the moment).
In some cases may happen that an Arduino pin is not connected (e.g. KW24D: D4 == NC). As a result, we will have NC not only at the end but also in the middle of the GPIO pin-map.
In this case, find_ports() function will finish processing pin-map to early (when first NC is encountered).
The proposition is to change the find_ports() FPGA testing utility function to loop through form factor pins (instead pin-map) and then check if the pin is not NC and is available on the specific pin-map before using it for testing.

KW24D, KW41Z, K64F: Remove GPIO pin-maps (use restricted GPIO pins if needed)
Fix include order in the UART FPGA test.

Test results after applying the above changes:

	K64F	NUCLEO_F429ZI	DISCO_L475VG_IOT01A	NUCLEO_F411RE	NUCLEO_F303RE	LPC55S69	NRF52840_DK
analogin	OK	OK	OK	OK	OK	OK	OK
gpio	OK	OK	OK	OK	OK	OK	OK
gpio_irq	OK	OK	OK	OK	OK	OK	OK
i2c	FAIL	OK	OK	OK	OK	OK	FAIL
pwm	OK	OK	OK	OK	OK	OK	OK
spi	OK	OK	OK	OK	OK	OK	OK
uart	OK	OK	OK	OK	OK	OK	OK

We can see that now we have 2 failures while I2C testing: K64F and NRF52840_DK. The method of searching for the pins for testing has changed and now find_ports() function loops through form factor pins (instead of pin map). As a result, different pins than before can be selected to test peripherals.

Two issues which have been found must be addressed before this PR can be merged:

K64F: test fails when D12(PTD3) pin is used as SDA. I checked the reference manual and board schematic, but I didn't find anything special about this pin (b619c29). cc @ARMmbed/team-nxp please help.
NRF52840_DK: By default D0 - D3 pins are used for the bit-banged SPI com channel between mbed target and the FPGA-test-shield. For some reason, if pins were used as GPIOs and then reconfigured to I2C pins the I2C com does not work on NRF52840. I tried to implement gpio_free() function and use it to free GPIOs used for FPGA com channel, but without success. For now, I suggest skipping I2C tests on D0 - D3 pins. (897b4b3).

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

@jamesbeyond @0xc0170 @ARMmbed/team-nxp
@maciejbocianski @fkjagodzinski

0xc0170 · 2020-02-14T13:00:02Z

Rule for "Do not merge" label : add WIP: as prefix to the title. I fixed it the title, should be soon marked as don't merge.

ciarmcom · 2020-02-14T14:00:42Z

@mprse, thank you for your changes.
@fkjagodzinski @maclobdell @maciejbocianski @jamesbeyond @0xc0170 @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

mprse · 2020-02-17T06:35:20Z

cc @mmahadevan108

mprse · 2020-02-17T09:56:44Z

The second issue (I2C test failure on NRF52840_DK) has been fixed here: 3f7609f

fkjagodzinski

Looks good so far 👍

mprse · 2020-02-20T10:26:40Z

ping @mmahadevan108

fkjagodzinski · 2020-02-20T14:10:57Z

targets/TARGET_Freescale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/TARGET_FRDM/PeripheralPins.c

+// List of GPIOs with limited functionality
+const PinList *pinmap_gpio_restricted_pins()
 {
-    return PinMap_GPIO;
+    static const PinName pins[] = {
+        PTE24,  // fixed pull-up (for I2C)
+        PTE25,  // fixed pull-up (for I2C)
+    };
+    static const PinList pin_list = {
+        sizeof(pins) / sizeof(pins[0]),
+        pins
+    };
+    return &pin_list;


Note, there is an overlap with PR #12477 (1b894aa#diff-22a0fab58312a8cd5c56864402f4aa94). If #12477 gets merged first, we do not need this implementation of pinmap_gpio_restricted_pins() for K64F any more. The input pull mode tests will be skipped based on pin's capabilities now.

Thanks for the clarification. In this case, I think #12477 should go first. And then I rebase and update this one.

fkjagodzinski · 2020-02-20T14:13:06Z

@mprse, is this still a WIP?

mprse · 2020-02-24T07:11:23Z

@mprse, is this still a WIP?

Removed WIP, but this PR needs #12477. When #12477 is merged I will rebase this one and drop restricted pins for K64F.

mergify · 2020-02-24T07:48:39Z

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

… form factor pins instead the pin-map This change is required to fully remove gpio pin-maps which were already added for FPGA testing. One use case of adding gpio pinmap was that pin-map must have the specific format - must be ended with NC pin. Functions that deal with pin-maps loops through the pin-map until NC pin is encountered. Also, our FPGA testing utility function to find pins for testing does that. When gpio pinmaps are fully removed we will have one generic gpio pinmap which provides Arduino pins: D0, D1, D2, etc. (only Arduino form factor is supported at the moment). In some cases may happen that an arduino pin is not connected (e.g. KW24D: D4 == NC). As a result we will have NC not only at the end, but also in the middle of the gpio pin-map. In this case find_ports() function will finish processing pin-map to early (when first NC is encountered). The proposition is to change the find_ports() FPGA testing utility function to loop through form factor pins (instead pin-map) and then check if the pin is not NC and is available on the specific pin-map before using it for testing.

…removed)

By default D0 - D3 pins are used for the bit-banged SPI com channel between mbed target and the FPGA-test-shield. For some reason, if pins were used as GPIOs and then reconfigured to I2C pins the I2C com does not work on NRF52840. This commit modifies i2c_configure_twi_instance() function and adds proper initialization of the I2C pins.

mprse · 2020-02-24T14:28:21Z

Rebased on master after #12477 has been merged and removed restricted pins for K64F.
GPIO test results on K64F results:

| target       | platform_name | test suite                              | result | elapsed_time (sec) | copy_method |
|--------------|---------------|-----------------------------------------|--------|--------------------|-------------|
| K64F-GCC_ARM | K64F          | tests-mbed_hal_fpga_ci_test_shield-gpio | OK     | 25.09              | default     |
mbedgt: test suite results: 1 OK
mbedgt: test case report:
| target       | platform_name | test suite                              | test case             | passed | failed | result | elapsed_time (sec) |
|--------------|---------------|-----------------------------------------|-----------------------|--------|--------|--------|--------------------|
| K64F-GCC_ARM | K64F          | tests-mbed_hal_fpga_ci_test_shield-gpio | basic input & output  | 1      | 0      | OK     | 1.7                |
| K64F-GCC_ARM | K64F          | tests-mbed_hal_fpga_ci_test_shield-gpio | explicit init, input  | 1      | 0      | OK     | 1.82               |
| K64F-GCC_ARM | K64F          | tests-mbed_hal_fpga_ci_test_shield-gpio | explicit init, output | 1      | 0      | OK     | 1.74               |
| K64F-GCC_ARM | K64F          | tests-mbed_hal_fpga_ci_test_shield-gpio | input pull modes      | 1      | 0      | OK     | 1.83               |
mbedgt: test case results: 4 OK

This one is ready for CI.

0xc0170 · 2020-02-24T14:31:58Z

CI started

mbed-ci · 2020-02-24T15:32:41Z

Test run: SUCCESS

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

0xc0170 · 2020-02-25T13:53:22Z

@mprse Is this ready for final reviews? Please let us know who should approve (there are lot of reviewers requested here but not yet completed).

mprse · 2020-02-25T14:10:56Z

@mprse Is this ready for final reviews? Please let us know who should approve (there are lot of reviewers requested here but not yet completed).

This one is ready for review.
@mmahadevan108 already accepted this, so I assume that it is ok to remove PTD3 from K64F SDA pin-map. It would be also good to get approval from: @maciejbocianski @jamesbeyond.

bulislaw · 2020-03-03T15:26:45Z

@maciejbocianski @jamesbeyond ping

maciejbocianski

LGTM

0xc0170 · 2020-03-04T08:36:38Z

CI restarted

mbed-ci · 2020-03-04T11:15:08Z

Test run: SUCCESS

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

0xc0170 changed the title ~~Remove GPIO pin-maps used for FPGA testing - WIP~~ WIP: Remove GPIO pin-maps used for FPGA testing Feb 14, 2020

mergify bot added the do not merge label Feb 14, 2020

ciarmcom requested review from 0xc0170, fkjagodzinski, jamesbeyond, maciejbocianski, maclobdell and a team February 14, 2020 14:00

ciarmcom added the needs: review label Feb 14, 2020

mprse force-pushed the fpga_remove_gpio_pinmaps branch from b619c29 to 5e5d0ff Compare February 17, 2020 09:54

fkjagodzinski reviewed Feb 17, 2020

View reviewed changes

fkjagodzinski reviewed Feb 20, 2020

View reviewed changes

mmahadevan108 approved these changes Feb 20, 2020

View reviewed changes

mprse changed the title ~~WIP: Remove GPIO pin-maps used for FPGA testing~~ Remove GPIO pin-maps used for FPGA testing Feb 24, 2020

mergify bot added the needs: work label Feb 24, 2020

mergify bot removed the needs: review label Feb 24, 2020

mprse added 4 commits February 24, 2020 14:44

KW24D, KW41Z: Remove GPIO pin-maps (use restricted GPIO pins if needed)

d36f2aa

tests-mbed_hal-pinmap: Remove testing GPIO pinmaps (GPIO pinmaps are …

6cd012d

…removed)

tests-mbed_hal_fpga_ci_test_shield-uart: fix include order

2a9e32e

mprse added 3 commits February 24, 2020 14:58

K64F: remove PTD3 pin from I2C SDA pinmap - fails FPGA test

2b52c58

i2c_api.c: fix style

32311b7

mprse force-pushed the fpga_remove_gpio_pinmaps branch from 5e5d0ff to 32311b7 Compare February 24, 2020 14:26

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

fkjagodzinski approved these changes Feb 24, 2020

View reviewed changes

0xc0170 added needs: review and removed do not merge needs: CI labels Feb 25, 2020

maciejbocianski approved these changes Mar 3, 2020

View reviewed changes

0xc0170 approved these changes Mar 4, 2020

View reviewed changes

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

mergify bot added ready for merge and removed needs: CI labels Mar 4, 2020

0xc0170 added the release-version: 6.0.0-alpha-3 label Mar 4, 2020

0xc0170 merged commit 22f3bc4 into ARMmbed:master Mar 4, 2020

mergify bot removed the ready for merge label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove GPIO pin-maps used for FPGA testing #12436

Remove GPIO pin-maps used for FPGA testing #12436

mprse commented Feb 14, 2020 •

edited

Loading

0xc0170 commented Feb 14, 2020

ciarmcom commented Feb 14, 2020

mprse commented Feb 17, 2020

mprse commented Feb 17, 2020

fkjagodzinski left a comment

mprse commented Feb 20, 2020

fkjagodzinski Feb 20, 2020

mprse Feb 24, 2020

fkjagodzinski commented Feb 20, 2020

mprse commented Feb 24, 2020

mergify bot commented Feb 24, 2020

mprse commented Feb 24, 2020

0xc0170 commented Feb 24, 2020

mbed-ci commented Feb 24, 2020

0xc0170 commented Feb 25, 2020

mprse commented Feb 25, 2020

bulislaw commented Mar 3, 2020

maciejbocianski left a comment

0xc0170 commented Mar 4, 2020

mbed-ci commented Mar 4, 2020

Remove GPIO pin-maps used for FPGA testing #12436

Remove GPIO pin-maps used for FPGA testing #12436

Conversation

mprse commented Feb 14, 2020 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

0xc0170 commented Feb 14, 2020

ciarmcom commented Feb 14, 2020

mprse commented Feb 17, 2020

mprse commented Feb 17, 2020

fkjagodzinski left a comment

Choose a reason for hiding this comment

mprse commented Feb 20, 2020

fkjagodzinski Feb 20, 2020

Choose a reason for hiding this comment

mprse Feb 24, 2020

Choose a reason for hiding this comment

fkjagodzinski commented Feb 20, 2020

mprse commented Feb 24, 2020

mergify bot commented Feb 24, 2020

mprse commented Feb 24, 2020

0xc0170 commented Feb 24, 2020

mbed-ci commented Feb 24, 2020

Test run: SUCCESS

0xc0170 commented Feb 25, 2020

mprse commented Feb 25, 2020

bulislaw commented Mar 3, 2020

maciejbocianski left a comment

Choose a reason for hiding this comment

0xc0170 commented Mar 4, 2020

mbed-ci commented Mar 4, 2020

Test run: SUCCESS

mprse commented Feb 14, 2020 •

edited

Loading