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

[RTL872x] multiple SPI fixes #2768

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Conversation

XuGuohui
Copy link
Member

Problem

  1. SPI.transfer(byte) may lose data
  2. SPI settings before SPI.begin() won't apply
  3. SPI.transfer(txBuf, nullptr, size, callback) will lose data if CS pin is deasserted after the callback is invoked
  4. SPI slave DMA state machine is messed up if master just toggles the CS pin

Steps to Test

user/tests/wiring/spi_master_slave

References

N/A


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@XuGuohui
Copy link
Member Author

Tests to be done to make sure we don't run into regressions:

  1. FRAM read/write
  2. SD card
  3. Neopixel
  4. Gen3 <-> Gen4 mixed SPI master/slave tests

@eberseth
Copy link
Contributor

So far the master passes everything but the slave fails. Commit 6334474

Test 23_SPI_Master_Slave_Master_Variable_Length_Transfer_DMA_Synchronous_MODE2_LSB passed.
Test 24_SPI_Master_Slave_Master_Const_String_Transfer_DMA passed.
Test 25_SPI_Master_Slave_Master_Reception passed.
Test summary: 26 passed, 0 failed, and 0 skipped, out of 26 test(s).
Test 00_SPI_Master_Slave_Slave_Transfer passed.
Test 01_SPI_Master_Slave_Slave_Receiption passed.
Assertion failed: (SPI1.available()=0) == (strlen(txString)=1000), file tests/wiring/spi_master_slave/spi_slave/spi_slave.cpp, line 366.
Test 02_SPI_Master_Slave_Slave_Const_String_Transfer_DMA failed.
Test summary: 2 passed, 1 failed, and 0 skipped, out of 3 test(s).

@XuGuohui
Copy link
Member Author

I have repeatedly run the 00_SPI_Master_Slave_Slave_Transfer for whole night and so far so good. This is the most tricky test I've experienced, cos it just failed occassionally. I'll jump to tun the whole tests to figureout the failure of 02_SPI_Master_Slave_Slave_Const_String_Transfer_DMA.

@XuGuohui
Copy link
Member Author

@eberseth Have you updated the system-part and test application for both SPI master and slave device? I just ran the whole test several times and all passed.

@XuGuohui
Copy link
Member Author

The SPI master/slave fixture test always passed for me. I was using two Photon2(s) to run the test.

@eberseth
Copy link
Contributor

I am running two Borons. I have updated the system parts on both.

@scott-brust scott-brust modified the milestones: 6.0.0, 5.8.2 May 6, 2024
Copy link
Member

@scott-brust scott-brust left a comment

Choose a reason for hiding this comment

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

I tested the spi_master_slave wiring tests

  • Photon 2 master SPI <--> Photon 2 slave SPI1
    • All fail: I assume SPI1 slave mode on RTL platform is still broken?
  • Photon 2 Master SPI <--> Boron slave SPI1
    • All passed: Test summary: 26 passed, 0 failed, and 0 skipped, out of 26 test(s).
  • Argon master SPI <--> Photon 2 slave SPI1
    • Mostly passed:
Assertion failed: (strncmp((const char *)SPI_Master_Rx_Buffer, "SPI Slave is doing good", sizeof("SPI Slave is doing good")) == 0=0) == (true=1), file tests/wiring/spi_master_s
lave/spi_master/spi_master.cpp, line 309.
Test 17_SPI_Master_Slave_Master_Variable_Length_Transfer_DMA_Synchronous_MODE1_LSB failed.
Assertion failed: (SPI_Master_Slave_Change_Mode(0x02, 1, transferFunc)=0) == (true=1), file tests/wiring/spi_master_slave/spi_master/spi_master.cpp, line 495.
Test 18_SPI_Master_Slave_Master_Variable_Length_Transfer_No_DMA_MODE2_MSB failed.
Assertion failed: (SPI_Master_Slave_Change_Mode(0x02, 1, transferFunc)=0) == (true=1), file tests/wiring/spi_master_slave/spi_master/spi_master.cpp, line 503.
Test 19_SPI_Master_Slave_Master_Variable_Length_Transfer_DMA_MODE2_MSB failed.
Assertion failed: (SPI_Master_Slave_Change_Mode(0x02, 1, transferFunc)=0) == (true=1), file tests/wiring/spi_master_slave/spi_master/spi_master.cpp, line 511.
Test 20_SPI_Master_Slave_Master_Variable_Length_Transfer_DMA_Synchronous_MODE2_MSB failed.
Assertion failed: (SPI_Master_Slave_Change_Mode(0x02, 0, transferFunc)=0) == (true=1), file tests/wiring/spi_master_slave/spi_master/spi_master.cpp, line 521.
Test 21_SPI_Master_Slave_Master_Variable_Length_Transfer_No_DMA_MODE2_LSB failed.
Assertion failed: (SPI_Master_Slave_Change_Mode(0x02, 0, transferFunc)=0) == (true=1), file tests/wiring/spi_master_slave/spi_master/spi_master.cpp, line 529.
Test 22_SPI_Master_Slave_Master_Variable_Length_Transfer_DMA_MODE2_LSB failed.
Assertion failed: (SPI_Master_Slave_Change_Mode(0x02, 0, transferFunc)=0) == (true=1), file tests/wiring/spi_master_slave/spi_master/spi_master.cpp, line 537.
Test 23_SPI_Master_Slave_Master_Variable_Length_Transfer_DMA_Synchronous_MODE2_LSB failed.
Test 24_SPI_Master_Slave_Master_Const_String_Transfer_DMA passed.
Assertion failed: (strncmp((const char *)SPI_Master_Rx_Buffer_Supper, txString, strlen(txString)) == 0=0) == (true=1), file tests/wiring/spi_master_slave/spi_master/spi_master.
cpp, line 598.
Test 25_SPI_Master_Slave_Master_Reception failed.
Test summary: 18 passed, 8 failed, and 0 skipped, out of 26 test(s).

I will review the DMA configuration / operation changes next. This looks like the most relevant changes in this PR
I read through the DMA operation and it seems to make sense to me. I think the biggest change here is the DCache invalidation steps with the tx/rx buffers.
I would have to debug the tests and HAL in depth to understand the above test failures since nothing is obviously wrong to me...

hal/src/rtl872x/spi_hal.cpp Show resolved Hide resolved
hal/src/rtl872x/spi_hal.cpp Show resolved Hide resolved
hal/src/rtl872x/spi_hal.cpp Show resolved Hide resolved
hal/src/rtl872x/spi_hal.cpp Show resolved Hide resolved
hal/src/rtl872x/spi_hal.cpp Show resolved Hide resolved
@XuGuohui
Copy link
Member Author

@scott-brust Have you updated the system-part that is built against the branch in this PR for Photon2 slave role as well?

@scott-brust
Copy link
Member

scott-brust commented May 15, 2024

@scott-brust Have you updated the system-part that is built against the branch in this PR for Photon2 slave role as well?

Yes, Ill double check it tomorrow. Should all the tests pass with P2 Master <-> P2 slave? Or just Gen3 Master <-> P2 slave?

@XuGuohui
Copy link
Member Author

The fixture tests all passed with P2 Master <-> P2 slave on my side.

@scott-brust
Copy link
Member

The fixture tests all passed with P2 Master <-> P2 slave on my side.

I cant get them to consistently pass. I always get failures at some point in the test run. Things break after that.
Here is a log and capture of the tests failing, around 79 seconds into the capture. I have two devices connected with jumper wire on a breadboard, not sure if this is introducing errors or what.
spi_master_slave_test_failures.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants