Skip to content
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.

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

Add a gpio pinmap #10644

Merged
merged 7 commits into from
Jul 17, 2019
Merged

Add a gpio pinmap #10644

merged 7 commits into from
Jul 17, 2019

Conversation

fkjagodzinski
Copy link
Member

Description

This is an alternative to #10459 -- instead of giving a list of pins to avoid, provide a list of pins that are suitable for all gpio tests.

This patch adds a gpio_pinmap() function that every target has to override to provide a set of pins for GPIO testing with the FPGA shield.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@c1728p9 @maclobdell @0xc0170 @maciejbocianski

Release Notes

Add a weak gpio_pinmap(), that every target has to override, to provide a set of pins for GPIO testing.

@ciarmcom
Copy link
Member

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

@0xc0170
Copy link
Contributor

0xc0170 commented May 23, 2019

This is consistent with the rest of pinmap API. On the other hand, there's a bit of ROM taken to hold the entire gpio table (some chips are large - have lot of pins).

@ARMmbed/mbed-os-hal Please review

Copy link
Contributor

@c1728p9 c1728p9 left a comment

Choose a reason for hiding this comment

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

I left some comments, but the change looks good to me. @0xc0170 the extra ROM usage should only be for testing, so even for large devices this shouldn't be a problem.

{PTE11, GPIO_E, 1},
{PTE12, GPIO_E, 1},
// {PTE24, GPIO_E, 1}, // fixed pull-up (for I2C)
// {PTE25, GPIO_E, 1}, // fixed pull-up (for I2C)
Copy link
Contributor

Choose a reason for hiding this comment

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

This only applies to the pull up/down tests right? If so is there a way the GPIO tests could run all its normal testing on these pins except the pull up/down test?

@mbed-ci
Copy link

mbed-ci commented May 23, 2019

Test run: SUCCESS

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

@0xc0170
Copy link
Contributor

0xc0170 commented May 24, 2019

@fkjagodzinski Today is the day to complete this one :-)

Filip Jagodzinski added 2 commits May 24, 2019 16:33
This was unnecessary since all the pins may be used independently.
@fkjagodzinski
Copy link
Member Author

I've updated the code as @c1728p9 suggested. This is ready for CI again.

@0xc0170
Copy link
Contributor

0xc0170 commented May 27, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented May 27, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test
  • jenkins-ci/mbed-os-ci_exporter

@0xc0170
Copy link
Contributor

0xc0170 commented May 27, 2019

tests-mbed_hal-pinmap failures in tests - looks related to the changeset, please review

@fkjagodzinski
Copy link
Member Author

fkjagodzinski commented May 27, 2019

Two targets failed tests-mbed_hal-pinmap / pinmap - validation because of NC pins present in the Arduino form factor:


I added a dedicated gpio_pinmap() for each of these targets.

@fkjagodzinski
Copy link
Member Author

Ready to run CI tests again.

@0xc0170
Copy link
Contributor

0xc0170 commented May 30, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented May 30, 2019

Test run: SUCCESS

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

@adbridge
Copy link
Contributor

@maciejbocianski could you please review the additional changes?

Copy link
Contributor

@maciejbocianski maciejbocianski left a comment

Choose a reason for hiding this comment

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

LGTM

@SeppoTakalo SeppoTakalo merged commit 3d5489a into ARMmbed:master Jul 17, 2019
@fkjagodzinski fkjagodzinski deleted the hal-gpio_pinmap branch July 17, 2019 11:41
@mmahadevan108
Copy link
Contributor

How was it decided what pins go into the PinMap_GPIO array for each supported Freescale platform?

@fkjagodzinski
Copy link
Member Author

How was it decided what pins go into the PinMap_GPIO array for each supported Freescale platform?

I added pins based on the spec for each chip:
KW41Z -- MKW41Z512RM.pdf, section 2.2 Signal Multiplexing and Pin Assignments,
KW24D -- MKW2xDxxxRM.pdf, section 3.3 MKW2xD Pins,
K64F -- K64P144M120SF5RM.pdf, section 10.3.1 K64 Signal Multiplexing and Pin Assignments.

My intention was to add every possible pin, so we could later comment out any unsupported ones if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants