Skip to content

Exclude target-specific pins from GPIO tests with the FPGA shield #10459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Exclude target-specific pins from GPIO tests with the FPGA shield #10459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0b9b49
pinmap: Add a list of restricted GPIO pins
Apr 23, 2019
1587377
K64F: Add a list of restricted pins for GPIO test
Apr 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/design-documents/hal/0002-pinmap-extension.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,5 @@ MBED_WEAK const PinList *pinmap_restricted_pins()
return &pin_list;
}
```

In addition to `pinmap_restricted_pins()`, the `pinmap_restricted_pins_gpio()` is used to skip pins during GPIO testing. By default this function returns an empty list and any target can override it to provide a list of pins that should be skipped. For example, D14 and D15 pins present in the Arduino form factor of FRDM-K64F have fixed pull-up resistors. The `PullDown` `PinMode` is impossible to test. However, other peripherals like the I2C may be successfully tested on these pins.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its likely that more exclusion lists will be needed for various hardware quirks. What about something like pinmap_pin_info(PinName pin, pin_info_t *info) which returns a structure of info for the given pin? This could take the place of both pinmap_restricted_pins() and pinmap_restricted_pins_gpio() by having these as fields in the struct. As new types of hardware anomalies are found they could be added to the struct. The struct could look something like this (As an example):

typedef struct {
    bool restricted;    // Pin should be avoided for all tests
    PullType pull;      // Pin has a hardware pullup or pulldown or is not pulled
    bool i2c_shared;    // One or more I2C devices may be connected to this pin
    bool spi_shared;    // One or more SPI devices may be connected to this pin
    bool needs_jumper;  // This pin is unconnected with default board jumper setting
} pin_info_t;

@maclobdell what are your thoughts on something like this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good idea. A weak pinmap_pin_info() would do the trick. This would require updating https://github.com/ARMmbed/fpga-ci-test-shield/blob/master/mbed-code/test_utils.h, @maciejbocianski what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c1728p9 what will be use case of i2c_shared?
Actually we have only problems with gpio/gpio_irq tests. Limitation of others (i2c/spi/qspi/...) are or should be covered in PinMap_XXX. I think that for now having pinmap_restricted_pins_gpio list with pulled pins is enough

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @maciejbocianski -- I don't see any need for more exclusion lists for now.

8 changes: 8 additions & 0 deletions hal/mbed_pinmap_default.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,11 @@ MBED_WEAK const PinList *pinmap_restricted_pins()
return &pin_list;
}

//*** Default restricted pins for the GPIO test ***
MBED_WEAK const PinList *pinmap_restricted_pins_gpio()
{
static const PinName pins[] = {};
static const PinList pin_list = {0, pins};
return &pin_list;
}

19 changes: 19 additions & 0 deletions hal/pinmap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,25 @@ bool pinmap_list_has_pin(const PinList *list, PinName pin);
*/
const PinList *pinmap_restricted_pins(void);

/**
* Get the additional list of pins to avoid during GPIO testing
*
* Unlike a list returned by generic pinmap_restricted_pins(),
* that is used by *ALL* tests, this list is used only for GPIO tests.
* This list extends the one returned by pinmap_restricted_pins().
*
* For example, PTE25 & PTE24 on the FRDM-K64F have fixed pull-ups
* making the PullDown PinMode impossible to test. However, the I2C
* may be successfully tested on these pins.
*
* Targets should override the weak implementation of this
* function if they have additional pins which should be
* skipped during GPIO testing.
*
* @return Pointer to a pin list of pins to avoid
*/
const PinList *pinmap_restricted_pins_gpio(void);

#ifdef TARGET_FF_ARDUINO

/**
Expand Down
30 changes: 30 additions & 0 deletions targets/TARGET_Freescale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/TARGET_FRDM/pinmap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/* mbed Microcontroller Library
* Copyright (c) 2019 ARM Limited
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "hal/pinmap.h"

const PinList *pinmap_restricted_pins_gpio()
{
static const PinName pins[] = {
PTE25, PTE24 // fixed pull-ups (for I2C)
};
static const PinList pin_list = {
sizeof(pins) / sizeof(pins[0]),
pins
};
return &pin_list;
}