Skip to content

kinetis_common: add a option to enable hardware CS for kinetis spi#2817

Merged
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
jfischer-no:pr@kinetis-enable-hwcs
Apr 17, 2015
Merged

kinetis_common: add a option to enable hardware CS for kinetis spi#2817
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
jfischer-no:pr@kinetis-enable-hwcs

Conversation

@jfischer-no
Copy link
Copy Markdown
Contributor

@jfischer-no jfischer-no commented Apr 14, 2015

This PR adds a option to enable hardware CS for kinetis spi (PCS0 only) + some fixes.

@jfischer-no jfischer-no added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: drivers Area: Device drivers labels Apr 14, 2015
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Apr 15, 2015

@jfischer-phytec-iot I think I can live with that workaround, after all the discussions about th CS line :-). @gebart do you have time to test?

PeterKietzmann
PeterKietzmann reviewed Apr 15, 2015
View reviewed changes
cpu/kinetis_common/spi.c
Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann Apr 15, 2015

Choose a reason for hiding this comment

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

Wouldn't it make sense to move this switch to the periph_conf?

Copy link
Copy Markdown
Contributor Author

@jfischer-no jfischer-no Apr 15, 2015

Choose a reason for hiding this comment

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

@PeterKietzmann No, it is also necessary for the lines https://github.com/jfischer-phytec-iot/RIOT/commit/ba9db2d147ac99fe0efaa9e6d7d924708f393e52#diff-1cbad4f25cbb4ed1edc201509a26b427R346 and https://github.com/jfischer-phytec-iot/RIOT/commit/ba9db2d147ac99fe0efaa9e6d7d924708f393e52#diff-1cbad4f25cbb4ed1edc201509a26b427R764.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Apr 15, 2015

Still I wonder why you cannot move it to the periph_conf.. I have a phyWAVE-A-Eval-Board here, is it possible to test SPI with this board? And by the way: what happened to "RIOT/boards/pba-d-01-kw2x/*" ?

@jfischer-no
Copy link
Copy Markdown
Contributor Author

jfischer-no commented Apr 15, 2015

Still I wonder why you cannot move it to the periph_conf

It allows backward compatibility, if #define KINETIS_SPI_USE_HW_CS 1 does not exist in periph_conf.h.

I have a phyWAVE-A-Eval-Board here, is it possible to test SPI with this board?

Sure, just rebase #2059 on this PR, btw do you have time to review #2059 😄 ?

And by the way: what happened to "RIOT/boards/pba-d-01-kw2x/*" ?

❓ need more input ...

@jonas-rem jonas-rem mentioned this pull request Apr 15, 2015
Merged
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Apr 15, 2015

@jfischer-phytec-iot I will test this PR first. Your code changes look reasonable to me. Regarding a review of #2059 I can't promise anything at the moment but I'll try to keep it in mind.
The question about the pba-d-01 was just a personal confusion. For some reasons I thought I was already in the RIOT master...

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Apr 16, 2015

@jfischer-phytec-iot I've got a phyWAVE-A-Eval-Board (1st revision). Are the relevant SPI pins (including CS line) on the pinout? If so, on which pins are they?

@jfischer-no
Copy link
Copy Markdown
Contributor Author

jfischer-no commented Apr 16, 2015

@PeterKietzmann thx

Are the relevant SPI pins (including CS line) on the pinout? If so, on which pins are they?

It's like Arduino UNO Pinout (SPI0). 👀 -> 📧

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Apr 16, 2015

You're welcome! Will you also answer my question :-) ?

@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Apr 16, 2015

I am currently swamped at work and will not be able to test any of this in the near future, sorry.

@jfischer-no
Copy link
Copy Markdown
Contributor Author

jfischer-no commented Apr 16, 2015

@PeterKietzmann 😃 look at your mailbox

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Apr 16, 2015

@jfischer-phytec-iot I tend to ACK after you've squashed your commits and seeing travis happy. Here is my result for the SPI_0 bus (one can also see the benifit of hardware over software CS line):
bildschirmfoto vom 2015-04-16 11-31-41
Anyone NACKing this? I'm not too familiar with the kinetis series.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Apr 17, 2015

@jfischer-phytec-iot as nobody objects and I succesfully tested this PR I'd give my ACK. Will merge after squash

@jfischer-no
cpu/kinetis_common/spi.c: add a option to use hardware cs (PCS0 only)
36e8917 
  minor fixes:
  - get rid of compiler warning "value 'SPI_1' not handled in switch"
  - add a giant macro to avoid redundant code in spi_init_master
@jfischer-no
Copy link
Copy Markdown
Contributor Author

jfischer-no commented Apr 17, 2015

@PeterKietzmann squashed

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Apr 17, 2015

OK let's do it! &go

PeterKietzmann added a commit that referenced this pull request Apr 17, 2015
@PeterKietzmann
Merge pull request #2817 from jfischer-phytec-iot/pr@kinetis-enable-hwcs
3922f6e 
kinetis_common: add a option to enable hardware CS for kinetis spi
@PeterKietzmann PeterKietzmann merged commit 3922f6e into RIOT-OS:master Apr 17, 2015
@jfischer-no jfischer-no deleted the pr@kinetis-enable-hwcs branch April 17, 2015 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@PeterKietzmann PeterKietzmann

Labels

Area: drivers Area: Device drivers Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jfischer-no @PeterKietzmann @jnohlgard