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.

Sign up for GitHub

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

Jump to bottom

periph/spi: Leftovers from SPI rework in #4780 #6437

Closed
10 of 23 tasks
PeterKietzmann opened this issue Jan 20, 2017 · 29 comments
Closed
10 of 23 tasks

periph/spi: Leftovers from SPI rework in #4780 #6437

PeterKietzmann opened this issue Jan 20, 2017 · 29 comments
Assignees
@haukepetersen @PeterKietzmann @A-Paul
Labels
Area: drivers Area: Device drivers State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Comments

@PeterKietzmann
Copy link
Member

PeterKietzmann commented Jan 20, 2017 •
edited by MrKevinWeiss
Loading

This issue acts as a place to collect and track leftovers and TODOs from the SPI rework done in #4780.

EDIT: Do the prformance analysis one SPI hardware testing is around
@PeterKietzmann PeterKietzmann added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: drivers Area: Device drivers State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Jan 20, 2017
@PeterKietzmann PeterKietzmann added this to the Release 2017.04 milestone Jan 20, 2017
@haukepetersen
Copy link
Contributor

Nice!

@PeterKietzmann PeterKietzmann mentioned this issue Jan 25, 2017
Merged
@PeterKietzmann
Copy link
Member Author

@ks156 this is a gentle reminder to test SPI on the WeIO board. Please report errors here or better, check the box in the issue description on success. You can find a procedure that we used for testing SPI drivers in this table

@lebrush
Copy link
Member

lebrush commented Jan 26, 2017

Please add to the list that the pn532 driver needs adaption

@PeterKietzmann
Copy link
Member Author

Dammit! Totally forgot that one. Thanks for the hint

@haukepetersen
Copy link
Contributor

@lebrush no problem, I can do the adaption quickly now, if you are volunteering to test it (as I don't have the device...).

@PeterKietzmann
Copy link
Member Author

We have one here

@haukepetersen
Copy link
Contributor

and here is the fix: #6478

@haukepetersen
Copy link
Contributor

build the test-app with PN532_MODE=spi BOARD=xxx make ...

@haukepetersen
Copy link
Contributor

Actually, the nvram driver on the mulle was just working fine, I just did not configure the correct CS pin when running the test application...

This was referenced Jan 26, 2017
Merged
Merged
@PeterKietzmann PeterKietzmann modified the milestones: Release 2017.01, Release 2017.04 Jan 26, 2017
@ks156
Copy link
Member

ks156 commented Jan 26, 2017

@PeterKietzmann I'm sorry but I won't be able to test the SPI now ... I don't have an oscilloscope under the hand anymore, and I don't currently have SPI devices for tests...

@kaspar030
Copy link
Contributor

Tested encx24j600 on nucleo-f334, works like a charm after disabling of onboard led (#6501)

@PeterKietzmann
Copy link
Member Author

@kaspar030 did you also test it on a stm32f4discovery board?

@PeterKietzmann
Copy link
Member Author

@haukepetersen when I run SPI (with POL:0, PHASE:0) it appears that CLK is on high level during idle. Just when the CS line pulls low, the SCK line goes low as well and when CS goes high, the CLK line goes high as well. That seems a bit strange to me. Did you observe the same during your tests?

@PeterKietzmann
Copy link
Member Author

BTW: Same applies to POL:0, PHASE:1. With POL:1, PHASE:* we also come from high level and result in high level before and after the transmission -as expected.

@PeterKietzmann PeterKietzmann mentioned this issue Jan 31, 2017
Closed
@PeterKietzmann PeterKietzmann modified the milestones: Release 2017.04, Release 2017.01 Feb 1, 2017
@PeterKietzmann
Copy link
Member Author

I unset the box for lis3dh because of #6567

@BytesGalore
Copy link
Member

@gebart I commented out the complete line for powering off [1], but no luck.

[1] https://github.com/RIOT-OS/RIOT/blob/master/cpu/kinetis_common/periph/spi.c#L155

@jnohlgard
Copy link
Member

@BytesGalore Sorry, I meant https://github.com/RIOT-OS/RIOT/blob/master/cpu/kinetis_common/periph/spi.c#L71. Leave the MDIS stuff in. If it doesn't make a difference, try removing both the SIM_SCGC line and the SPI_MCR MDIS line.

@BytesGalore
Copy link
Member

@gebart preventing clearing the SIM_SCGC when powering off (@ line 71 ) did the trick.
I tested it with 100kHz, 400kHz and 1MHz (my logic analyzer doesn't sample correctly higher frequencies).

@jnohlgard
Copy link
Member

I think turning off the clock domain for the SPI is a wrong move on Kinetis. It doesn't affect the power consumption as far as I can measure, and it have caused issues on multiple occasions.

@immesys
Copy link
Contributor

immesys commented Mar 1, 2017

It doesn't affect power consumption positively on SAMR21 either...

@dylad
Copy link
Member

dylad commented Mar 21, 2017

SPI doesn't work on SAML21 anymore.
see #6686 for the issue reference.
I also come up with the required fix #6687

@aabadie aabadie mentioned this issue Mar 21, 2017
Closed
@patgrosse patgrosse mentioned this issue Apr 4, 2017
Closed
@PeterKietzmann
Copy link
Member Author

@gebart I'd like to get rid of this old-timer. The only uncovered points are:

  • Test sensor driver adt7310
  • Problems with hardware CS on Kinetis (@ 100kHz and 400kHz)

Do you have a chance to test the adt7310? I would guess that after 1.5 years a SPI defect in this driver would have already popped up. For the latter issue I propose to cover it within #6567.

jnohlgard pushed a commit to jnohlgard/RIOT that referenced this issue Jun 27, 2018
kinetis: Clean up SPI module power handling
28fe4b8 
Fixes a problem with hardware CS when using slow SPI speeds,
mentioned in RIOT-OS#6437 (comment)
@jnohlgard jnohlgard mentioned this issue Jun 27, 2018
Merged
@jnohlgard
Copy link
Member

My adt7310 breakout is in a box somewhere but I will not be able to get to it until August.

Regarding the Kinetis SPI module disable issue: #9438

gdoffe pushed a commit to gdoffe/RIOT that referenced this issue Jul 7, 2018
kinetis: Clean up SPI module power handling
3f6dd4a 
Fixes a problem with hardware CS when using slow SPI speeds,
mentioned in RIOT-OS#6437 (comment)
pekkanikander pushed a commit to AaltoNEPPI/RIOT that referenced this issue Aug 11, 2018
@pekkanikander
kinetis: Clean up SPI module power handling
159e6c4 
Fixes a problem with hardware CS when using slow SPI speeds,
mentioned in RIOT-OS#6437 (comment)
@MrKevinWeiss
Copy link
Contributor

Tested on the adt7310 with nucleo-f401re, aside from not supporting the 10MHz in the driver test (changed to 400kHz) it seemed to pass. Can we close?

@PeterKietzmann
Copy link
Member Author

Yes, thank you very much!

pekkanikander pushed a commit to AaltoNEPPI/RIOT that referenced this issue Aug 28, 2018
@pekkanikander
kinetis: Clean up SPI module power handling
c54cd9a 
Fixes a problem with hardware CS when using slow SPI speeds,
mentioned in RIOT-OS#6437 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@haukepetersen haukepetersen

@PeterKietzmann PeterKietzmann

@A-Paul A-Paul

Labels
Area: drivers Area: Device drivers State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests