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

RIOT port for the MKW22D512 SiP and Phytec PBA-D-01 PhyWave Evaluations-Board #2059

Merged
merged 2 commits into from
May 15, 2015
Merged

RIOT port for the MKW22D512 SiP and Phytec PBA-D-01 PhyWave Evaluations-Board #2059

merged 2 commits into from
May 15, 2015

Conversation

jfischer-no
Copy link
Contributor

This PR add support for Freescale MKW22D512 SiP and Phytec PBA-D-01 PhyWave Evaluations-Board.

@LudwigKnuepfer
Copy link
Member

Please sync your repos master with RIOT's master and rebase on it.
(Feel free to ask for details if this doesn't make sense to you.)

@LudwigKnuepfer LudwigKnuepfer added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Nov 20, 2014
@jfischer-no
Copy link
Contributor Author

@LudwigOrtmann: I do not know if I did it right, scolds me :-)

@haukepetersen
Copy link
Contributor

Maybe have a look at the comment from BytesGalore on the bottom of the comments in #2018. I think he explained the rebasing quite nicely...

And by the way: Welcome to RIOT!

@haukepetersen haukepetersen self-assigned this Nov 21, 2014
@haukepetersen haukepetersen added the Platform: ARM Platform: This PR/issue effects ARM-based platforms label Nov 21, 2014
@LudwigKnuepfer
Copy link
Member

OK, step by step. I assume git://github.com/RIOT-OS/RIOT.git is the remote called upstream and git://github.com/jfischer-phytec-iot/RIOT.git is called origin.

# get updates, reset master
git fetch --all
git checkout master
git reset --hard upstream/master

# rebase this branch
git checkout wip@mkw2xd
git checkout -b backup # create a backup, this also checks it out
git checkout wip@mkw2xd
git rebase -i master
# throw out all unrelated commits (there is an on-line help in the view that opens)

# in case of conflicts, you might need
git mergetool # to resolve conflicts
git rebase --continue # after resolving conflicts
git rebase --abort # if you messed up

# push changes
git checkout master
git push -f origin master
git checkout wip@mkw2xd
git push -f origin wip@mkw2xd

@jfischer-no
Copy link
Contributor Author

@LudwigOrtmann : done, but travis has a problem with unknown license header: 'cpu/mkw2x/include/MKW22D5.h'
@haukepetersen : thanks, we already know us (libopencm3, nrf51 :-) )

@LudwigKnuepfer
Copy link
Member

@jfischer-phytec-iot Would be nice to use your fresh first-hand experience to improve this wiki entry if necessary:
https://github.com/RIOT-OS/RIOT/wiki/Git-cheatsheet#how-to-rebase-your-master-on-a-current-riot-master

@LudwigKnuepfer
Copy link
Member

Regarding the license - I hope @haukepetersen can help.

@haukepetersen
Copy link
Contributor

@jfischer-phytec-iot Yes, I remembered by now... Got confused by your new github name... Nice port by the way! Hope I will get to reviewing it soon...

@haukepetersen
Copy link
Contributor

As to the license, as far as I see it the file doesn't have one... So I guess it should be safe to include it in the license checker?!

@jfischer-no
Copy link
Contributor Author

@haukepetersen I have created a new account for work. Can we add the description for our board in RIOT-wiki? What do you mean with "include it in the license checker"?

@haukepetersen
Copy link
Contributor

Of course, just go ahead! By the way, is the board freely available/buyable?

With license checker i meant the script that is run by Travis which compares every single license header found in RIOTs source code against a set of pre-defined headers. These can be found in RIOT/dist/tools/licenses/patterns/*. Just add a pattern for the Freescale header file included in your PR.

@jfischer-no
Copy link
Contributor Author

@haukepetersen We have presented our Modules and Board on Electronica last week. The Board should be available at the Embedded World, but we can provide some developer samples, specially for RIOT-developers :-).

@haukepetersen
Copy link
Contributor

Sounds great! And also one more reason to go to the Embedded World next year... I take it there is no public information (i.e. datasheets/manuals) available so far? If I had the right board on your website it looked quite interesting!

@LudwigKnuepfer
Copy link
Member

@haukepetersen The file says it is a CMSIS file - I thought there were replacements for that?
And either way - the file needs to have a license. We can't add a 'no license is fine' rule to the checker or whitelist individual files.

@jfischer-no
Copy link
Contributor Author

@haukepetersen I will clear the license type with Freescale.

@haukepetersen
Copy link
Contributor

perfect!

@PeterKietzmann
Copy link
Member

Tested i2c with #2119, #2121, #2123 and #2148. Worked fine

@@ -0,0 +1 @@
FEATURES_PROVIDED = transceiver periph_gpio periph_uart periph_spi periph_i2c
Copy link
Member

Choose a reason for hiding this comment

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

transceiver is not implemented yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will completely rework it in the next few days. Transceiver will also appear soon.

Copy link
Member

Choose a reason for hiding this comment

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

So, should we mark this as WIP for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@OlegHahm OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 9, 2014
@jfischer-no jfischer-no added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Dec 9, 2014
@jfischer-no jfischer-no added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Jan 6, 2015
@OlegHahm
Copy link
Member

OlegHahm commented Jan 6, 2015

Do I see it correctly, that the license issue has been resolved?

@jfischer-phytec-iot, what's left on the Todo list?

@jfischer-no
Copy link
Contributor Author

@OlegHahm It is a CMSIS header with 3c-BSD license now.
Todo list? You mean #2188?
I have moved all peripheral driver in kinetis_common. I'm also porting RIOT to KW01Z128 and will make a PR soon. Internally, we already use kinetis_common for two MCUs (KW22D512 and KW01Z128). I think kinetis_common should include initially only the peripheral (uart, spi, i2c, hwtimer) drivers.
Currently I do not know what I can improve on that branch :-).

@OlegHahm
Copy link
Member

For this PR, please go on and squash and merge at will.

@jfischer-no
Copy link
Contributor Author

fccbb18 makes travis unhappy (failed: driver_at86rf2xx, driver_xbee, unittests)
I will remove it and squash. After #2756 has merged, we can open a PR for fccbb18.

@OlegHahm
Copy link
Member

agreed

@jfischer-no jfischer-no removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels May 13, 2015
@jfischer-no
Copy link
Contributor Author

travis was canceled 😕 ?

@OlegHahm
Copy link
Member

I killed some of the outdated builds in Travis - if I accidentally killed the most current one, too, I'm sorry, but apparently it's running again.

@jonas-rem
Copy link
Contributor

As https://www.traviscistatus.com/ states, there was an issue today. Maybe it was related to that somehow.

@jonas-rem
Copy link
Contributor

Travis failed because the testapp for the isl29125 driver uses GPIO5, which is per default disabled in our board.

@OlegHahm
Copy link
Member

How about either modifying the Makefile with a special case for you board or (what I would prefer) blacklist the application?

@jonas-rem
Copy link
Contributor

I think we might not even need a special case for our board. Since GPIO_5 is just the default value we could change it to GPIO_0? This would also be consistent with the ng_at86rf2xx driver that counts the defaults from GPIO_0. GPIO_0 is defined by default and should be defined on all boards. As the comment in the Makefile (tests/driver_isl29125) says, this seems to be just a random value "set random default".

@OlegHahm
Copy link
Member

Sounds good, too. @LudwigOrtmann, it's "your" driver. Any reason against changing this?

@jfischer-no
Copy link
Contributor Author

I have reorganized the GPIOs, again, a few more can not hurt.

@jonas-rem
Copy link
Contributor

Oh, the changes in periph_conf.h that were neccessary to build the kw2xrf app are included in the postponed fccbb18 commit. However, if you comment-in GPIO 24 conf everything should be fine.

/* GPIO channel 24 config */
/*
#define GPIO_24_DEV         KW2XDRF_GPIO
#define GPIO_24_PORT        KW2XDRF_PORT
#define GPIO_24_PORT_BASE   KW2XDRF_PORT_BASE
#define GPIO_24_PIN         KW2XDRF_PCS0_PIN
#define GPIO_24_CLKEN()     KW2XDRF_PORT_CLKEN()
#define GPIO_24_IRQ         KW2XDRF_PORT_IRQn
#define KW2XRF_CS_GPIO      GPIO_24
*/

@jfischer-no
Copy link
Contributor Author

my fault, i will update 470781b. i will revert 470781b

@jfischer-no
Copy link
Contributor Author

@jremmert-phytec-iot
jonas-rem@3287e71#diff-26037c789932d7c8d7bea2a82565588dR8
I do not want to enable GPIO_24 because we are using hardware CS per default (KW2XRF_SHARED_SPI=1).

@jfischer-no jfischer-no added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 14, 2015
@jonas-rem
Copy link
Contributor

We have (KW2XRF_SHARED_SPI=0) as default. But yes, that means hardware CS.

I have also thought about that, but didn´t saw an alternative. How should we handle the parameter KWRF_CS then? We whould have to introduce two different parameter set of kw2xrf_init; i) without cs_pin and ii) with cs_pin and distinguish and let the preprocessor decide what function to use. I wanted to avoid that.

 54 int kw2xrf_spi_init(spi_t spi, spi_speed_t spi_speed,
 55                     gpio_t cs_pin)

Do you have other solutions in mind?

@LudwigKnuepfer
Copy link
Member

Just define a specific GPIO for your board in the drivers Makefile.

@jfischer-no
Copy link
Contributor Author

@OlegHahm ack & squash ?

openocd -f "$RIOTBASE/boards/pba-d-01-kw2x/dist/mkw22d512.cfg" \
-c "init" \
-c "reset halt" \
-c "flash write_image erase $elffile" \
Copy link
Member

Choose a reason for hiding this comment

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

superfluous space

@OlegHahm
Copy link
Member

Yes, please fix the tiny indentation thing and squash. ACK

@jfischer-no jfischer-no removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 15, 2015
@jfischer-no
Copy link
Contributor Author

GO?

@OlegHahm
Copy link
Member

Push the button!

jfischer-no pushed a commit that referenced this pull request May 15, 2015
RIOT port for the MKW22D512 SiP and Phytec PBA-D-01 PhyWave Evaluations-Board
@jfischer-no jfischer-no merged commit 1d46291 into RIOT-OS:master May 15, 2015
@jfischer-no jfischer-no deleted the wip@mkw2xd branch May 15, 2015 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: ARM Platform: This PR/issue effects ARM-based platforms 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.

9 participants