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

mprse · 2020-02-06T13:05:53Z

Summary of changes

Add a GPIO pinmap for DISCO_L475VG_IOT01A required for FPGA testing.

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-st-mcd

@jamesbeyond

@fkjagodzinski @maciejbocianski

LMESTM · 2020-02-06T13:28:09Z

.../TARGET_STM/TARGET_STM32L4/TARGET_STM32L475xG/TARGET_DISCO_L475VG_IOT01A/PeripheralPinMaps.h

@@ -52,6 +52,94 @@
 #include "PinNamesTypes.h"
 #include <mstd_cstddef>

+//*** GPIO ***
+// Pinmap used for FPGA GPIO testing only
+MSTD_CONSTEXPR_OBJ_11 PinMap PinMap_GPIO[] = {


I'm still not a big fan of this proposal.
espacially this is really are to read - this is supposed to be an exclusion table and it actually contains all the defined pins with dummy values ... can't we at least create an enum the list of pins that need to be excluded from the test ?
That would be easier to read and create .

This is done in the same way as other peripherals. Please check PWM pin-map table where also some pins are commented out because of some reason:

mbed-os/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L475xG/TARGET_DISCO_L475VG_IOT01A/PeripheralPinMaps.h

Lines 257 to 260 in 98af2d0

// {PB_6, PWM_4, STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM4, 1, 0)}, // TIM4_CH1 // Connected to STDIO_UART_TX

// {PB_6, PWM_16, STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF14_TIM16, 1, 1)}, // TIM16_CH1N // Connected to STDIO_UART_TX

// {PB_7, PWM_4, STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM4, 2, 0)}, // TIM4_CH2 // Connected to STDIO_UART_RX

// {PB_7, PWM_17, STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF14_TIM17, 1, 1)}, // TIM17_CH1N // Connected to STDIO_UART_RX

We have a ready testing utility functions which can deal with the pin-maps. So from our point of view making exceptions here is very problematic.

Except thePWM and other tables are maps between pins to HW block instances.
The GPIO one is a table mapping pins to nothing ... :-(

What you actually need is a simple list of pins on which you don't want to run a GPIO test. Do you need a table because other MCUs from other vendors a GPIO instance value ?

After a quick chat with @maciejbocianski @mprse , we think the better way of the achieve the same result as @LMESTM suggested is add const PinList *pinmap_restricted_gpio_pins() which exclude the GPIO pins can't be tested, this PR will be updated along with some other targets also need to be updated as well

Here I added a mechanism to handle list of restricted GPIO in FPGA testing:
#12379 (comment)

Also here I dropped base commit and added commit with the list of restricted GPIO pins instead.

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

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

jamesbeyond

LGTM, @LMESTM what do you think about this approach?

jeromecoutant

Hi

I didn't test, but I confirm this list of potential restrictions.
Note that maybe we could use some define value from PinNames.h:

RCC_OSC32_IN = PC_14,
RCC_OSC32_OUT = PC_15,
RCC_OSC_IN = PH_0,
RCC_OSC_OUT = PH_1,

and:
D14 = PB_9, // I2C_SCL
D15 = PB_8, // I2C_SDA

LMESTM

I definitely better like this approach (and Jerome proposal could make it even more generic for all STM32 targets !) - thank you for this update

Pull request has been modified.

mprse · 2020-02-07T10:52:19Z

Thanks, everyone for the discussion and finding optimal solutions for this case.

I addressed @jeromecoutant last comment and used define value from PinNames.h in the restricted gpio pin list. Test results below (this PR + PR #12379):

| target                      | platform_name       | test suite                              | result | elapsed_time (sec) | copy_method |
|-----------------------------|---------------------|-----------------------------------------|--------|--------------------|-------------|
| DISCO_L475VG_IOT01A-GCC_ARM | DISCO_L475VG_IOT01A | tests-mbed_hal_fpga_ci_test_shield-gpio | OK     | 19.18              | default     |
mbedgt: test suite results: 1 OK
mbedgt: test case report:
| target                      | platform_name       | test suite                              | test case                    | passed | failed | result | elapsed_time (sec) |
|-----------------------------|---------------------|-----------------------------------------|------------------------------|--------|--------|--------|--------------------|
| DISCO_L475VG_IOT01A-GCC_ARM | DISCO_L475VG_IOT01A | tests-mbed_hal_fpga_ci_test_shield-gpio | explicit init, input         | 1      | 0      | OK     | 1.57               |
| DISCO_L475VG_IOT01A-GCC_ARM | DISCO_L475VG_IOT01A | tests-mbed_hal_fpga_ci_test_shield-gpio | explicit init, output        | 1      | 0      | OK     | 1.58               |
| DISCO_L475VG_IOT01A-GCC_ARM | DISCO_L475VG_IOT01A | tests-mbed_hal_fpga_ci_test_shield-gpio | generic init, input & output | 1      | 0      | OK     | 1.63               |

@0xc0170 It would be better to first merge PR #12379 before this one.

0xc0170 · 2020-02-07T15:24:06Z

CI started

mbed-ci · 2020-02-07T17:39:49Z

Test run: SUCCESS

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

0xc0170 · 2020-02-11T10:21:17Z

Just in case, restarted CI once referenced PR is on master now.

mbed-ci · 2020-02-11T11:51:35Z

Test run: SUCCESS

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

mprse mentioned this pull request Feb 6, 2020

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

Merged

LMESTM reviewed Feb 6, 2020

View reviewed changes

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

ciarmcom added the needs: review label Feb 6, 2020

jamesbeyond mentioned this pull request Feb 6, 2020

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

Merged

mprse force-pushed the DISCO_L475VG_IOT01A_add_gpio_pinmap branch from 98af2d0 to c2cd2bb Compare February 7, 2020 10:04

mprse changed the title ~~DISCO_L475VG_IOT01A: Add a GPIO pinmap for testing~~ DISCO_L475VG_IOT01A: Add a list of restricted GPIO pins for testing Feb 7, 2020

jamesbeyond previously approved these changes Feb 7, 2020

View reviewed changes

jeromecoutant reviewed Feb 7, 2020

View reviewed changes

LMESTM approved these changes Feb 7, 2020

View reviewed changes

DISCO_L475VG_IOT01A: Add a list of restricted GPIO pins for testing

3a71f86

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

mprse force-pushed the DISCO_L475VG_IOT01A_add_gpio_pinmap branch from c2cd2bb to 3a71f86 Compare February 7, 2020 10:48

0xc0170 added the needs: preceding PR label Feb 7, 2020

jamesbeyond approved these changes Feb 7, 2020

View reviewed changes

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

mprse mentioned this pull request Feb 11, 2020

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

Merged

0xc0170 added needs: CI and removed needs: preceding PR ready for merge labels Feb 11, 2020

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

0xc0170 removed the ready for merge label Feb 11, 2020

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

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

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

mprse commented Feb 6, 2020

LMESTM Feb 6, 2020

mprse Feb 6, 2020

LMESTM Feb 6, 2020 •

edited

LMESTM Feb 6, 2020

jamesbeyond Feb 6, 2020

mprse Feb 7, 2020

ciarmcom commented Feb 6, 2020

jamesbeyond left a comment

jeromecoutant left a comment

LMESTM left a comment

mprse commented Feb 7, 2020

0xc0170 commented Feb 7, 2020

mbed-ci commented Feb 7, 2020

0xc0170 commented Feb 11, 2020

mbed-ci commented Feb 11, 2020

	// {PB_6, PWM_4, STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM4, 1, 0)}, // TIM4_CH1 // Connected to STDIO_UART_TX
	// {PB_6, PWM_16, STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF14_TIM16, 1, 1)}, // TIM16_CH1N // Connected to STDIO_UART_TX
	// {PB_7, PWM_4, STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM4, 2, 0)}, // TIM4_CH2 // Connected to STDIO_UART_RX
	// {PB_7, PWM_17, STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF14_TIM17, 1, 1)}, // TIM17_CH1N // Connected to STDIO_UART_RX

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

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

Conversation

mprse commented Feb 6, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

LMESTM Feb 6, 2020

Choose a reason for hiding this comment

mprse Feb 6, 2020

Choose a reason for hiding this comment

LMESTM Feb 6, 2020 • edited

Choose a reason for hiding this comment

LMESTM Feb 6, 2020

Choose a reason for hiding this comment

jamesbeyond Feb 6, 2020

Choose a reason for hiding this comment

mprse Feb 7, 2020

Choose a reason for hiding this comment

ciarmcom commented Feb 6, 2020

jamesbeyond left a comment

Choose a reason for hiding this comment

jeromecoutant left a comment

Choose a reason for hiding this comment

LMESTM left a comment

Choose a reason for hiding this comment

mprse commented Feb 7, 2020

0xc0170 commented Feb 7, 2020

mbed-ci commented Feb 7, 2020

Test run: SUCCESS

0xc0170 commented Feb 11, 2020

mbed-ci commented Feb 11, 2020

Test run: SUCCESS

LMESTM Feb 6, 2020 •

edited