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

Async SPI transaction with [write, read] operation broken on esp32 #2059

Closed
plaes opened this issue Sep 2, 2024 · 12 comments · Fixed by #2131
Closed

Async SPI transaction with [write, read] operation broken on esp32 #2059

plaes opened this issue Sep 2, 2024 · 12 comments · Fixed by #2131
Assignees
Labels
bug Something isn't working chip:esp32s2 Issue related to ESP32-S2 chip chip:esp32 Issue related to ESP32 chip peripheral:spi SPI peripheral
Milestone

Comments

@plaes
Copy link
Contributor

plaes commented Sep 2, 2024

I have a driver that does spi.transaction with sequential write and read operations.

let write_buffer = [0; 1];
let mut read_buffer = [0xa; 4];
let mut ops = [Operation::Write(write_buffer), Operation::Read(read_buffer)];
spi.transaction(&mut ops).await?;

It seems that whenever the read_buffer is longer than one byte, it doesn't get updated with read data, ie with this code read_buffer still contains [0xa, 0xa, 0xa, 0xa] after transaction.

This is issue is present with 0.20.1 and current HEAD c5e342a

@github-project-automation github-project-automation bot moved this to Todo in esp-rs Sep 2, 2024
@MabezDev MabezDev added this to the 0.21.0 milestone Sep 2, 2024
@Dominaezzz
Copy link
Collaborator

Thanks for the snippet, mind making it bigger? Is that with DMA? If so, how big are the buffers?
A full MVP would be appreciated then I'll be able to run it and see what's going on later this evening.

@MabezDev MabezDev added bug Something isn't working peripheral:spi SPI peripheral chip:esp32 Issue related to ESP32 chip labels Sep 2, 2024
@plaes
Copy link
Contributor Author

plaes commented Sep 2, 2024

Thanks for the snippet, mind making it bigger? Is that with DMA? If so, how big are the buffers? A full MVP would be appreciated then I'll be able to run it and see what's going on later this evening.

Yes, with async fullduplex DMA with buffer length of 1024.
I'll see whether I can whip up something, as I was blocked by the xtensa-lx-rt issue.

edit: I can't seem to reproduce this with single device based HIL test though.

@plaes
Copy link
Contributor Author

plaes commented Sep 2, 2024

@Dominaezzz I managed to find some kind of "anomaly":

    let write_buf = &[0xaa];
    let mut read: [u8; 2] = [0xff; 2];
    let mut ops = [Operation::Write(write_buf), Operation::Read(&mut read)];
    let _ = spi_dev.transaction(&mut ops).await;
    defmt::info!("Test 1... result: {:?}", read);
    // why do we get 0xff, 0xff here?
    assert_eq!(read, [0xff, 0xff]);


    let write_buf = &[0xaa];
    let mut read: [u8; 1] = [0xffu8; 1];
    let mut ops = [Operation::Write(write_buf), Operation::Read(&mut read)];
    let _ = spi_dev.transaction(&mut ops).await;
    defmt::info!("Test 2... result: {:?}", read);
    // ... but 0x00 here?
    assert_eq!(read, [0xff]);

Self contained test-case can be found from here: https://github.com/plaes/rust-lilygo-ttgo-lora32/blob/c79b7f0cb16310beaee593b894472bdf5e45adb0/src/spi_dma_transaction.rs#L79-L93

@Dominaezzz
Copy link
Collaborator

Try changing let mut read: [u8; 2] = [0xff; 2]; to let mut read: [u8; 2] = [0xab; 2];.

I'm curious to see if the driver isn't writing anything to the buffer at all or if it's write 1s (from a pull up or something).

@plaes
Copy link
Contributor Author

plaes commented Sep 2, 2024

Well, no pullup issue:

[INFO] - Test 1... result: [171, 171]
[INFO] - Test 2... result: [0]
[INFO] - Test 1... result: [171, 171]

@Dominaezzz
Copy link
Collaborator

Does the same happen for blocking mode?

@plaes
Copy link
Contributor Author

plaes commented Sep 2, 2024

Does the same happen for blocking mode?

I haven't yet figured out blocking + DMA + transaction, but blocking + transaction gives following output:

[INFO] - Test 1... result: [0, 0]
[INFO] - Test 2... result: [0]
[INFO] - Test 3... result: [0, 0]

@Dominaezzz
Copy link
Collaborator

I've been able to reproduce it. (Without your full example I wouldn't have been able to, it only comes out in particular circumstances it seems)

@Dominaezzz
Copy link
Collaborator

Dominaezzz commented Sep 2, 2024

Basically, it seems once a write happens, reads stop working correctly. Only one byte is read.

Happens in both blocking and async mode at least, and it seems the peripheral itself just won't return more data to the DMA.

transfer also doesn't work correctly after a write is done

With a larger buffer to read, the read works as expected until the last 8 bytes....

I'm able to reproduce this with older commits, I can reproduce it before the DMA Move Api PR at least 😅 .
Even before #1894 landed, I can still reproduce this issue. I fear this may have always been broken...

transfer then read is fine, but write then read is not.... hhmmm

@Dominaezzz
Copy link
Collaborator

It appears to be a silicon bug in the base ESP32.
espressif/esp-idf@366e439
(An ESP32 #ifdef was added later. espressif/esp-idf@0c3653b)

This stuff should really go in the errata.

Looks like the ESP32 will need special logic in the SPI driver.... 😞

@Dominaezzz
Copy link
Collaborator

To reproduce, take the embassy_spi example and change it from transfer to write + read.

@Dominaezzz Dominaezzz mentioned this issue Sep 2, 2024
9 tasks
@bjoernQ bjoernQ self-assigned this Sep 10, 2024
@bjoernQ
Copy link
Contributor

bjoernQ commented Sep 10, 2024

I also see problems on ESP32-S2: there a write and read only succeeds once - then it fails

@bjoernQ bjoernQ added the chip:esp32s2 Issue related to ESP32-S2 chip label Sep 10, 2024
@bjoernQ bjoernQ linked a pull request Sep 10, 2024 that will close this issue
6 tasks
@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chip:esp32s2 Issue related to ESP32-S2 chip chip:esp32 Issue related to ESP32 chip peripheral:spi SPI peripheral
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants