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

Lots of K64F SPI fixes #315

Merged
merged 8 commits into from
Jul 28, 2024
Merged

Lots of K64F SPI fixes #315

merged 8 commits into from
Jul 28, 2024

Conversation

multiplemonomials
Copy link
Collaborator

@multiplemonomials multiplemonomials commented Jul 28, 2024

Summary of changes

I tested the Freescale FRDM_K64F on the CI test shield, and found that it had a number of issues with its SPI master implementation. This PR fixes those issues.

Incorrect fill data for 16-bit transactions

When the SPI word size was configured to 16 bits instead of 8, the SPI fill data would always be 0 for the upper word. This actually is due to an issue with the underlying FSL HAL not supporting this use case. I have put in a PR with MCUXpresso SDK for this change: nxp-mcuxpresso/mcux-sdk#204

SPI frequency reset from spi_format()

If you were foolish enough to call spi_format() after setting the frequency (jk), your SPI frequency would be reset back to 500kHz. This is because spi_format() reinitializes the SPI bus but forgets to set the frequency to what it was before in that API call. I think that the MIMXRT105x HAL had a similar issue, come to think of it. Maybe this is an easy footgun with FSL HAL.

SPI driver assertion failure when doing async transfers with word size > 8

The assertion in SPI.h had incorrect logic, so it was causing an assertion failure for valid code. Oops. Fixing this here!

Infinite hang if you used single-byte transfer after async transfer

If you did a single byte transfer after an async transfer, the SPI driver would hang forever. This is because the async transfers can leave the end of queue flag set and/or leave data in the Tx FIFO (it seems odd that this is the case but it's apparently by design, as most of the FSL HAL functions clear these flags at the start of the next transfer). Not clearing these flags causes the SPI write operation to loop forever waiting for them to clear. To fix it, we just have to clear the flags in the single byte write function to match the other functions' behavior.

DMA channels not freed when SPI deleted

spi_free() for K64F forgot to free any allocated DMA channels. This led to DMA not working with the SPI bus if it was deleted and recreated. I'm not 100% sure why, but I think maybe the old DMA channels were still active and still handling DMA requests from the SPI?

Double DMA SPI transactions

And for the biggest issue I found, some incorrect logic in an interrupt handler led to every "short" (< about 1000 bytes) DMA SPI transaction being done a second time after it completed! e.g. if you tried to send two bytes, it would actually send two copies of those two bytes. This is a very serious issue as it makes DMA SPI practically unusable for talking to real hardware.

Impact of changes

All of the above bugs fixed

Migration actions required

Documentation

None


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR

The FRDM K64F now passes the CI shield basic SPI test!


@multiplemonomials multiplemonomials changed the title K64F SPI fixes Lots of K64F SPI fixes Jul 28, 2024
@multiplemonomials multiplemonomials merged commit 5354079 into master Jul 28, 2024
10 checks passed
@multiplemonomials multiplemonomials deleted the dev/mk64f-spi-fixes branch July 28, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants