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

periph/spi: Switching between CPOL=0,1 problems on Kinetis with software CS #6567

Open
jnohlgard opened this issue Feb 7, 2017 · 14 comments
Assignees
Labels
Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@jnohlgard
Copy link
Member

jnohlgard commented Feb 7, 2017

(I did not yet verify this with a scope)
The software CS is causing problems when moving between different clock polarity settings.
We discovered that the LIS3DH driver on mulle requires setting the SPI mode to 0, but it should be using mode 3 according to the data sheet, as well as when using the old SPI driver and when using a different driver in Contiki. Clearly there is something different with the new RIOT SPI driver.

The current driver also works in mode 3 if the LIS3DH configuration is changed to use hardware CS on Mulle (PR #6572).

I believe that the problem arises from the fact that the CS pin is asserted before the new clock polarity setting is applied to the SPI module, resulting in the device picking up the clock polarity change as a normal clock pulse, when in fact it should be ignored. However, this does not explain why it used to work in the old periph driver, which also was using software CS where the CS pin was managed by the LIS3DH driver itself instead of deferred to the SPI bus driver.

This needs investigation with a scope or logic analyzer.

@jnohlgard jnohlgard added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 7, 2017
@jnohlgard jnohlgard added this to the Release 2017.04 milestone Feb 7, 2017
@jnohlgard jnohlgard self-assigned this Feb 7, 2017
@jnohlgard
Copy link
Member Author

Also, the LIS3DH driver works if we retry the who am I query during init, but that's just hiding the problem because the first transfer will have placed the bus in the correct clock polarity for the second transfer.

@haukepetersen
Copy link
Contributor

Will look into this as soon as I can.

@miri64
Copy link
Member

miri64 commented Oct 25, 2017

@haukepetersen did you?

@PeterKietzmann
Copy link
Member

@MrKevinWeiss I just wanted you to be aware of this issue.

@MrKevinWeiss MrKevinWeiss self-assigned this Feb 21, 2019
@MrKevinWeiss
Copy link
Contributor

Thanks, I have kinetis boards and a scope. I will check it out!

@stale
Copy link

stale bot commented Aug 25, 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 25, 2019
@stale stale bot closed this as completed Sep 25, 2019
@kaspar030
Copy link
Contributor

ping @MrKevinWeiss

@kaspar030 kaspar030 reopened this Sep 25, 2019
@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Sep 25, 2019
@MrKevinWeiss
Copy link
Contributor

Darn, I will try to remember when I'm back from vacation.

@PeterKietzmann
Copy link
Member

@MrKevinWeiss obviously you didn't

@MrKevinWeiss
Copy link
Contributor

True... I'll check it out, thanks!

@MrKevinWeiss
Copy link
Contributor

I tested on the frdm-kw41z using test/periph_spi at 100 kHz
The procedure is:

> reboot
> init <args>
> send 123

args = init 0 0 0 -1 0 -> Mode 0, hw cs
spi0h

args = init 0 0 0 2 19 -> Mode 0, sw cs
spi0s

args = init 0 1 0 -1 0 -> Mode 1, hw cs
spi1h

args = init 0 1 0 2 19 -> Mode 1, sw cs
spi1s

args = init 0 2 0 -1 0 -> Mode 2, hw cs
spi2h

args = init 0 2 0 2 19 -> Mode 2, sw cs
spi2s

args = init 0 3 0 -1 0 -> Mode 3, hw cs
spi3h

args = init 0 3 0 2 19 -> Mode 3, sw cs
spi3s

@PeterKietzmann
Copy link
Member

@MrKevinWeiss many thanks! What is your interpretation of the results? On first sight CS behavior looks normal to me, although it returns to high very fast in some configurations. Furthermore, the logic analyzer didn't decode all cases correctly. Did you just miss to switch the decoder mode?

@MrKevinWeiss
Copy link
Contributor

I think there could be the problem when using soft CS and low power mode that @MichelRottleuthner pointed out on another platform (clock ends in the wrong polarity).

My interpretation is:
Mode0: POL=0 PHA=0
Mode1: POL=0 PHA=1
Mode2: POL=1 PHA=0
Mode3: POL=1 PHA=1

Which matches, the decoding was probably just a setting on the scope. My intention wasn't to show the decoding but the signals. Mode 0 and mode 3 both trigger the first rising edge, the only difference is the initial clock level, mode 3 starts high and mode 0 starts low.

I may have missed the initial capture of some of these actually and remember some problems that resulted in sending the command twice.

This may be because in mode 3, the clock pin starts low and when the CS pin goes low the device sees a clock pulse as the clock tries to go to its desired starting position. We don't see this the second time around because the SW CS leaves the clock high.

I would suggest setting the SCK based on the polarity setting during init.

@miri64 miri64 added this to the Release 2020.07 milestone Jul 1, 2020
@miri64
Copy link
Member

miri64 commented Jul 1, 2020

So... is this a bug? Can it be fixed? Did we fix it?

@aabadie aabadie added Area: drivers Area: Device drivers Area: cpu Area: CPU/MCU ports labels May 20, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

No branches or pull requests

7 participants