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

boards: Add support for KEA128LEDLIGHTRD #9655

Closed
wants to merge 2 commits into from
Closed

Conversation

OYTIS
Copy link
Contributor

@OYTIS OYTIS commented Jul 31, 2018

The PR includes support for S9KEA128 MCU with a limited set of peripherals (LEDs, GPIOs, PIT timer and CAN) and support for the board.
I had to add some conditional compilation to kinetis codebase, because KEA family has significant differences from what was supported so far (different clock generation/distribution, different pin multiplexing).
I've also run into some issues with CAN subsystem and added some hacks to my driver as a workaround, but it's worth debugging these problems to the cause.

@PeterKietzmann
Copy link
Member

I guess it is this board? Nice! @smlng might be interested as well.

@PeterKietzmann PeterKietzmann added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports labels Aug 1, 2018
@smlng
Copy link
Member

smlng commented Aug 1, 2018

looks really nice, @PeterKietzmann can you order the board 😀 For the CPU (kinetis) I'd like to have @gebart opinions, too.

@smlng smlng requested a review from jnohlgard August 1, 2018 09:18
@smlng
Copy link
Member

smlng commented Aug 1, 2018

Just from a quick view on this, it might be best to sort and separate this out a bit. I see 3 major contributions in this PR:

  1. add KEA CPU support
  2. add KEAxyz board support
  3. add NXP/Kinetis CAN periph driver

which I would pack into distinct PRs based on one-another in that order. Though the first cannot be used without the second, it still would ensure that the changes can be tested with existing kinetis CPUs. For the CAN driver I would suggest to rename it a bit and put it into periph/can.c see #6178, too.

@smlng smlng self-assigned this Aug 1, 2018
@OYTIS
Copy link
Contributor Author

OYTIS commented Aug 1, 2018

Sure, I can break the PR into three (or four with Olimex debugger support) if you find it more convenient for review.
Note that CAN driver is KEA-specific, K series uses a different IP core (FlexCAN vs MSCAN).

@OYTIS
Copy link
Contributor Author

OYTIS commented Aug 1, 2018

By the way, what's the purpose of typedef int dont_be_pedantic declarations?

@smlng
Copy link
Member

smlng commented Aug 1, 2018

@OYTIS its more about the #ifdef #else where the file would be empty in some cases (ie. the ifdef does not hold) and with certain compiler flags we would get errors then. So the compiler should not be pedantic here 😁

@OYTIS
Copy link
Contributor Author

OYTIS commented Aug 1, 2018

@smlng Thanks! The first PR is there, I'll move mscan.c under periph later and make a second one.

@OYTIS
Copy link
Contributor Author

OYTIS commented Aug 5, 2018

@smlng Split this PR into three. Previous two can be found at #9666 and #9711. MSCAN is now under periph/.

@smlng
Copy link
Member

smlng commented Aug 6, 2018

@OYTIS thanks!

@RIOT-OS RIOT-OS deleted a comment Sep 14, 2018
@bergzand bergzand mentioned this pull request Sep 25, 2018
@OYTIS OYTIS force-pushed the s9kea branch 4 times, most recently from d9b3775 to 0f91e5c Compare December 2, 2018 15:55
@RIOT-OS RIOT-OS deleted a comment Dec 2, 2018
@smlng smlng added the State: waiting for other PR State: The PR requires another PR to be merged first label Jan 11, 2019
@smlng
Copy link
Member

smlng commented Jan 11, 2019

requires #9666

@smlng smlng changed the title Add support for KEA128LEDLIGHTRD board boards: Add support for KEA128LEDLIGHTRD Feb 13, 2019
@smlng
Copy link
Member

smlng commented Feb 13, 2019

@OYTIS this seems to include #9711, too. Can you rebase on master and remove the MSCAN dependency here. Then this can move independently.

@OYTIS
Copy link
Contributor Author

OYTIS commented Feb 13, 2019

@smlng Sure. Then MSCAN support on KEA128LEDLIGHTRD should come as a separate PR, right?

@smlng
Copy link
Member

smlng commented Feb 13, 2019

yes, what I mean is: leave #9711 as is. But remove any MSCAN related config and code stuff from this PR. This might require a followup PR to enable the periph_can feature for this board later.

@OYTIS
Copy link
Contributor Author

OYTIS commented Feb 13, 2019

@smlng Done. Also fixed a bug in #9711.

@stale
Copy link

stale bot commented Aug 17, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 17, 2019
@OYTIS
Copy link
Contributor Author

OYTIS commented Aug 18, 2019

@smlng @gebart Oh this one. To be honest, I don't have access to the KEA128LEDLIGHTRD board any more. So unless someone wants to take over, or test and merge, that can only be closed I suppose.

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Aug 18, 2019
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Sep 10, 2019
@stale
Copy link

stale bot commented Feb 19, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Feb 19, 2020
@stale stale bot closed this Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: stale State: The issue / PR has no activity for >185 days Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants