Skip to content

NXP LPSPI: Refactoring to remove dependency of HAL in XFER drivers #86939

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
Merged
merged 8 commits into from
Apr 4, 2025
Merged

NXP LPSPI: Refactoring to remove dependency of HAL in XFER drivers #86939

merged 8 commits into from
Apr 4, 2025

Conversation

decsny
Copy link
Member

@decsny decsny commented Mar 11, 2025 •
edited
Loading

This PR contains some of the changes from #85010 that are split into separate commits to make them more reviewable.

These changes are particularly important so that people can contribute to the main transfer driver files without having conflicts with #85010.

@decsny decsny marked this pull request as ready for review March 11, 2025 15:13
@zephyrbot zephyrbot added area: SPI SPI bus platform: NXP Drivers NXP Semiconductors, drivers labels Mar 11, 2025
EmilioCBen
EmilioCBen previously approved these changes Mar 20, 2025
View reviewed changes
@tbursztyka
Copy link
Contributor

Can you rebase? Afaik CI if failing due to external issues from this PR

@decsny
Copy link
Member Author

decsny commented Mar 21, 2025

Can you rebase? Afaik CI if failing due to external issues from this PR

yeah, the issue I am having in general right now is that there is a lot of work that needs done on this LPSPI and I am making many different PRs to try to satisfy all the people clamoring for what features they want ASAP while also making the PRs reviewable, it's kind of chaos as far as all the git branches I have right now. This PR I need to probably sequence with some other things, so I haven't updated it yet

@tbursztyka
Copy link
Contributor

@decsny I can imagine the pressure. No worries, ping me (if I am again lost in /dev/null) once you'll had time to rebase.

@decsny decsny force-pushed the lpspi_remove_sdk_from_xfer_files branch from 6fc6900 to 77dc96c Compare April 1, 2025 15:43
@decsny
Copy link
Member Author

decsny commented Apr 1, 2025 •
edited
Loading

I updated the PR to remove a commit that already merged in another PR, and remove another commit which is now in 3 other open PRs

@decsny decsny force-pushed the lpspi_remove_sdk_from_xfer_files branch from 48ebd6f to 9a24cdc Compare April 2, 2025 15:12
@decsny decsny linked an issue Apr 2, 2025 that may be closed by this pull request
spi_nxp_lpspi: Hardfault regression #87768
Closed
@dleach02
Copy link
Member

dleach02 commented Apr 2, 2025

@hakehuang can you regress test this on our HW range and give a +1 if clean.

dleach02
dleach02 previously approved these changes Apr 2, 2025
View reviewed changes
mmahadevan108
mmahadevan108 previously approved these changes Apr 2, 2025
View reviewed changes
@mmahadevan108 mmahadevan108 added the DNM This PR should not be merged (Do Not Merge) label Apr 2, 2025
@mmahadevan108
Copy link
Contributor

Awaiting @hakehuang test results.
@hakehuang , please remove the DNM label if it looks fine per your test and review.

@decsny
Copy link
Member Author

decsny commented Apr 3, 2025 •
edited
Loading

There is already multiple regressions on the main branch, and new test case added that don't pass on LPSPI while the hard fault was on main, the test results almost for sure won't pass, this PR is fixing the main hard fault

@hakehuang
Copy link
Contributor

Awaiting @hakehuang test results.
@hakehuang , please remove the DNM label if it looks fine per your test and review.

running, will feedback once done

@hakehuang hakehuang mentioned this pull request Apr 3, 2025
Closed
@hakehuang hakehuang added the block: HW Test Testing on hardware required before merging label Apr 3, 2025
decsny added 8 commits April 3, 2025 10:06
@decsny
spi_nxp_lpspi: Reset/clock peripheral
546a59b 
If there are HAL definitions available, do these two things:

Ungate the clock for the device from the zephyr driver. Eventually it
would be better to have a clocks property in the LPSPI DT node and get
the resources from there rather than the HAL.

Some platforms require the peripheral to be reset at system level, add
code to do this. Eventually it would be more ideal to have Zephyr
reset drivers for all of the NXP platforms and use DT to describe the
reset resources, but for now we can just do this to get the LPSPI
supported.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>
@decsny
spi_nxp_lpspi: Prevent edge case causing DMA error
1d16f8a 
Stop the transfer with error if at any point there is some
execution reached where transfer is being set up for 0 length, this can
cause problems where for example eDMA set up with this nonsense 0 length
channels can get an infinite error interrupt.

And this is probably an erroneously crafted transfer request anyways.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>
@decsny
spi_nxp_lpspi: Fix DMA driver async return
5bc4d26 
We should not release context until transfer ends. The code previously
would return from wait_for_completion and then release the context. This
is only supposed to be done in the dma callback except for the case of
error in the transceive call. For async transfer this was most likely
always happening wrong and probably broken for multi threads trying to
access the bus due to this premature release of the context.

Also we should not enable CS and leave enabled in case of error, move CS
enable to after the error check.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>
@decsny
spi_nxp_lpspi: Convert DMA version to native code
116e043 
Convert the DMA-based LPSPI driver to native code.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>
@decsny
spi_nxp_lpspi: Convert CPU version to native code
e8ebbad 
Convert the CPU-based lpspi driver to native code.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>
@decsny
spi_nxp_lpspi: Refactor validation args to func
b1ed4a1 
Minor refactor to make a separate function to validate configuration
arguments.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>
@decsny
spi_nxp_lpspi: Remove SDK items from header file
35c38ef 
Remove SDK types and defines from header file.

And since now the common file is the only consumer of SDK header, move
that include there.

Also rename the tristate boolean to be more clean about what it does
rather than trying to be similar to the SDK config name.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>
@decsny
spi_nxp_lpspi: Remove mcux branding from tokens
37e45cf 
Since these drivers mainly do not use MCUX except for the configure
function (which will soon also be changed), change namespace prefix to
lpspi_ instead of spi_mcux_ to avoid confusion.

Also improve descriptions of kconfigs to clarify what they are for.
Not changing the kconfig names for now since they are user-facing.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>
@decsny decsny dismissed stale reviews from mmahadevan108 and dleach02 via 37e45cf April 3, 2025 16:10
@decsny decsny force-pushed the lpspi_remove_sdk_from_xfer_files branch from 9a24cdc to 37e45cf Compare April 3, 2025 16:10
@decsny decsny removed DNM This PR should not be merged (Do Not Merge) block: HW Test Testing on hardware required before merging labels Apr 3, 2025
@decsny
Copy link
Member Author

decsny commented Apr 3, 2025

I cherry-picked the commits from ##87789 which fix the other issues to pass the current test on the platforms reported, now test should pass on all configuration and platform

@decsny decsny linked an issue Apr 3, 2025 that may be closed by this pull request
tests: drivers: spi : some NXP platform fail with spi_loopback tests #88090
Closed
@mmahadevan108 mmahadevan108 requested a review from hakehuang April 3, 2025 20:39
@hakehuang
Copy link
Contributor

I cherry-picked the commits from ##87789 which fix the other issues to pass the current test on the platforms reported, now test should pass on all configuration and platform

I restart the testing, will feedback once done

hakehuang
hakehuang approved these changes Apr 4, 2025
View reviewed changes
Copy link
Contributor

@hakehuang hakehuang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

board testing pass for v4.1.0-1894-g37e45cf692f9 on all tests/drivers/spi/spi_loopback/drivers.spi.loopback

@decsny decsny requested a review from mmahadevan108 April 4, 2025 11:15
@kartben kartben merged commit 0875499 into zephyrproject-rtos:main Apr 4, 2025
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hakehuang hakehuang hakehuang approved these changes

@mmahadevan108 mmahadevan108 mmahadevan108 approved these changes

@dleach02 dleach02 dleach02 left review comments

@EmilioCBen EmilioCBen EmilioCBen left review comments

@dbaluta dbaluta Awaiting requested review from dbaluta

@manuargue manuargue Awaiting requested review from manuargue

@Raymond0225 Raymond0225 Awaiting requested review from Raymond0225

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@teburd teburd Awaiting requested review from teburd

Assignees

@tbursztyka tbursztyka

@mmahadevan108 mmahadevan108

Labels
area: SPI SPI bus platform: NXP Drivers NXP Semiconductors, drivers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tests: drivers: spi : some NXP platform fail with spi_loopback tests spi_nxp_lpspi: Hardfault regression
8 participants
@decsny @tbursztyka @dleach02 @mmahadevan108 @hakehuang @EmilioCBen @kartben @zephyrbot