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

kinetis_common: Refactor GPIO implementation #4042

Merged

Conversation

jnohlgard
Copy link
Member

This is a refactor of the GPIO implementation on Kinetis to follow the updated GPIO interface in #3095.

This implementation uses a linked list to handle the interrupt callbacks instead of statically allocating a few bytes per pin, this keeps the RAM usage down when not using all pins as interrupt inputs.

@jnohlgard jnohlgard added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 4, 2015
@jnohlgard jnohlgard added this to the Release NEXT MAJOR milestone Oct 4, 2015
@jnohlgard jnohlgard force-pushed the pr/kinetis-gpio-periph-refactor branch from c940779 to e67d13e Compare October 4, 2015 09:54
@LudwigKnuepfer
Copy link
Member

Compare #3996 (comment)

@jnohlgard jnohlgard force-pushed the pr/kinetis-gpio-periph-refactor branch from e67d13e to 462bcf7 Compare October 4, 2015 13:46
@jnohlgard jnohlgard added the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 4, 2015
@jnohlgard
Copy link
Member Author

Semi-depends on/has the same GPIO_PIN syntax as #3847

@jnohlgard
Copy link
Member Author

(Updated the commit message to explain what this commit actually changes)

@PeterKietzmann
Copy link
Member

I could test this on the phy-board soon, if it speeds up things. @gebart is this PR still waiting for an other?

@jnohlgard jnohlgard removed the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 21, 2015
@jnohlgard
Copy link
Member Author

@PeterKietzmann not waiting any more

@jnohlgard jnohlgard force-pushed the pr/kinetis-gpio-periph-refactor branch from 462bcf7 to 6ef83cf Compare October 21, 2015 14:22
@jnohlgard
Copy link
Member Author

rebased on latest master

@jnohlgard
Copy link
Member Author

ping @PeterKietzmann
Could you test on the phytec board? Do you have a freedom k64 board as well?

@jnohlgard jnohlgard force-pushed the pr/kinetis-gpio-periph-refactor branch from 6ef83cf to ef63371 Compare October 24, 2015 12:28
@jnohlgard
Copy link
Member Author

Rebased after #3485 was merged, updated GPIO pin for NVRAM CS, immediately squashed

@jfischer-no
Copy link
Contributor

I can test it tomorrow.

@PeterKietzmann
Copy link
Member

Aaah, dammit. Sorry for the delay. @jfischer-phytec-iot did you already test anything here?

@jfischer-no
Copy link
Contributor

@gebart @PeterKietzmann There is somewhere a bug when testing with init_int. Maybe I can dig deeper today afternoon.

@PeterKietzmann
Copy link
Member

@gebart, @jfischer-phytec-iot , @haukepetersen I tested some pins one the pba-d-01-kw2x with the functionalities from tests/periph_gpio. Everything went well. I don't have time for a complete review here

@PeterKietzmann
Copy link
Member

@jfischer-phytec-iot what is the bug looking like? I didn't see it

@jfischer-no
Copy link
Contributor

@PeterKietzmann Please try init_int 3 1 0 1 ("a Button") out.

@jfischer-no
Copy link
Contributor

 port->PCR[pin] &= ~(PORT_PCR_IRQC_MASK); /* Disable interrupt */
BITBAND_REG32(port->PCR[pin], PORT_PCR_ISF_SHIFT) = 1; /* Clear interrupt flag */
port->PCR[pin] |= config->irqc; /* Enable interrupt */       /* <= BUG*/
config->cb = cb;
config->arg = arg;
config->irqc = irqc;
/* Allow the callback to be found by the IRQ handler by setting the proper

@jfischer-no
Copy link
Contributor

@jnohlgard jnohlgard force-pushed the pr/kinetis-gpio-periph-refactor branch from 7ea1407 to 0ebeda9 Compare October 26, 2015 12:29
@jnohlgard
Copy link
Member Author

rebased on latest master, added the ISR fix by @jfischer-phytec-iot

@jnohlgard
Copy link
Member Author

@jfischer-phytec-iot does the current state of this PR work on your test set up?

@PeterKietzmann
Copy link
Member

I can confirm that the button didn't trigger an interrupt without Johan's PR but does work with it.

@jfischer-no
Copy link
Contributor

@gebart We test it, still need about 30 minutes.

@jonas-rem
Copy link
Contributor

Just tested the communication between two pba-d-01 boards with this PR -> works fine.

@jfischer-no
Copy link
Contributor

@gebart ACK, please throw cpu/stm32f1/periph/gpio.c out.

This is a rewrite of the Kinetis GPIO driver which follows the
refactored API in [1]. Pins are specified using the GPIO_PIN(PORT_x, y)
macro, e.g. GPIO_PIN(PORT_E, 25) for the PTE25 pin.

The interrupt pin handling is now implemented as a linked list, this
is more memory efficient, but with a minor variation in interrupt
latency depending on in what order the pins were initialized at
runtime.

Because the linked list entries are taken from a shared pool, there is
also the possibility of running out of available configuration slots,
define the preprocessor macro GPIO_INT_POOL_SIZE in periph_conf.h if
you need more than 16 pins configured for interrupts in the same
application.

[1]: RIOT-OS#3095
@jnohlgard jnohlgard force-pushed the pr/kinetis-gpio-periph-refactor branch from 0ebeda9 to 142c280 Compare October 28, 2015 13:12
@jnohlgard
Copy link
Member Author

rebased, squashed and threw out the unrelated change in stm32f1

@PeterKietzmann
Copy link
Member

Happy times and go

PeterKietzmann added a commit that referenced this pull request Oct 29, 2015
kinetis_common: Refactor GPIO implementation
@PeterKietzmann PeterKietzmann merged commit 6fcea29 into RIOT-OS:master Oct 29, 2015
@jnohlgard jnohlgard deleted the pr/kinetis-gpio-periph-refactor branch October 29, 2015 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants