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

NXP LPSPI: Convert to native zephyr driver, remove MCUX shim #85010

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

decsny
Copy link
Member

@decsny decsny commented Feb 1, 2025

Convert the LPSPI driver completely to a native Zephyr driver.

Main commit message:

Remove the shim to the SDK driver, and only use direct register writes in the zephyr driver instead. This will save flash space.

Remove mcux branding from all code.

While doing this part of the rewrite, I discovered also some issues that I went ahead and fixed:
- The driver was hardcoded to always only use PCS 0 no matter what the user provided spi_config set slave to.
- Active high CS configuration was not respected.

Fixes an issue where SDK driver performed an althgorithm on 2,048 possible clock configurations before every transfer.
This was replaced with a binary search in the native version.
And make it so that this configuration does not need to happen every transfer, if the same spi_cfg is used, to improve performance.
Various optimizations also due to the fact that the use case is more clear than the SDK was written for, so we can be more straightforward and simpler.

Also, I have calculated the difference in the code size of the LPSPI driver in 3.7.0 (including HAL + shim driver) vs the version in this PR after all the recent changes, and have the following results:

Code Size Difference

Zephyr 3.7.0 LPSPI:
- 2.793 KB from HAL driver itself
- 1.488 KB from Zephyr shim driver
- 188 bytes from common Zephyr SPI code
= Total 4.465 KB (4,572 bytes)

Latest Changes + This PR (Hopefully Zephyr 4.1.0 LPSPI):
- 1.352 KB from CPU-based LPSPI specific driver (spi_nxp_lpspi.c)
- 1.230 KB from common LPSPI driver code (spi_nxp_lpspi_common.c)
- 190 bytes from common Zephyr SPI code
= Total 2.768 KB (2,834 bytes)

Tested on MCXN, pending testing on RT and MCXW

Depends on #84931
Depends on zephyrproject-rtos/hal_nxp#504
Depends on hardware testing for MCXW, and RT

For the CPU-based drivers, delete the old MCUX based RTIO driver and use
the default RTIO submit implementation instead.
Rationale: 300 LOC -> 1 LOC to maintain.

In the future a DMA based RTIO driver with custom implementation can be
designed, but for CPU based transfer, which is already not optimal
performance, code maintenance is more important. Only requirement is
asynchronous submit, which is accomplished by p4wq in rtio workq.

Signed-off-by: Declan Snyder <[email protected]>
@zephyrbot
Copy link
Collaborator

zephyrbot commented Feb 1, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@49ff7e3 (master) zephyrproject-rtos/hal_nxp#504 zephyrproject-rtos/hal_nxp#504/files

DNM label due to: 1 project with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Feb 1, 2025
@decsny decsny force-pushed the feature/lpspi_enhancements_4 branch from b25696f to eb1c476 Compare February 1, 2025 05:50
Remove the shim to the SDK driver, and only use direct register writes
in the zephyr driver instead. This will save flash space.

Remove mcux branding from all code.

While doing this part of the rewrite, I discovered also some issues that
I went ahead and fixed:
- The driver was hardcoded to always only use PCS 0 no matter
  what the user provided spi_config set slave to.
- Active high CS configuration was not respected.

Also fixes an issue where SDK driver performed an althgorithm on 2,048
possible clock configurations before every transfer. This was replaced
with a binary search in the native version. And make it so that this
configuration does not need to happen every transfer, if the same
spi_cfg is used, to improve performance. Various optimizations also
due to the fact that the use case is more clear than the SDK was
written for, so we can be more straightforward and simpler.

Signed-off-by: Declan Snyder <[email protected]>
@decsny decsny force-pushed the feature/lpspi_enhancements_4 branch from eb1c476 to 52f48c1 Compare February 1, 2025 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Clock Control area: SPI SPI bus block: HW Test Testing on hardware required before merging DNM This PR should not be merged (Do Not Merge) manifest manifest-hal_nxp platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants