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

Bugs in FlashSafeDma<SpiDma> #1798

Closed
Dominaezzz opened this issue Jul 15, 2024 · 3 comments · Fixed by #1856
Closed

Bugs in FlashSafeDma<SpiDma> #1798

Dominaezzz opened this issue Jul 15, 2024 · 3 comments · Fixed by #1856
Labels
bug Something isn't working peripheral:dma DMA Peripheral

Comments

@Dominaezzz
Copy link
Collaborator

Discovered these while refactoring the DMA APIs. (I'm going to be removing them anyway for the new DMA Move API but I thought it'd be worth mentioning in case someone else runs into it)

Here the result of the transfer isn't copied back into user provided buffer.

async fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> {
Ok(
if !crate::soc::is_valid_ram_address(&write[0] as *const _ as u32) {
for (read, write) in read.chunks_mut(SIZE).zip(write.chunks(SIZE)) {
self.buffer[..write.len()].copy_from_slice(write);
self.inner
.transfer(read, &self.buffer[..write.len()])
.await?;
}
} else {
self.inner.transfer(read, write).await?;
},
)
}

fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> {
if !crate::soc::is_valid_ram_address(&write[0] as *const _ as u32) {
for (read, write) in read.chunks_mut(SIZE).zip(write.chunks(SIZE)) {
self.buffer[..write.len()].copy_from_slice(write);
self.inner.transfer(read, &self.buffer[..write.len()])?;
}
} else {
self.inner.transfer(read, write)?;
}
Ok(())
}

Here the DMA buffer isn't being used.

#[cfg(feature = "embedded-hal-02")]
impl<T: embedded_hal_02::blocking::spi::Transfer<u8>, const SIZE: usize>
embedded_hal_02::blocking::spi::Transfer<u8> for crate::FlashSafeDma<T, SIZE>
{
type Error = T::Error;
fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Self::Error> {
self.inner.transfer(words)
}
}

async fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> {
self.inner.read(words).await
}

async fn transfer_in_place(&mut self, words: &mut [u8]) -> Result<(), Self::Error> {
self.inner.transfer_in_place(words).await
}

@jessebraham jessebraham added bug Something isn't working peripheral:dma DMA Peripheral labels Jul 15, 2024
@plaes
Copy link
Contributor

plaes commented Jul 24, 2024

Two issues I noticed with current main (5d6354c) as of 2024.07.24 when communicating using async from embassy task on esp32 (revision v3.0).

Firstly, FlashSafeDma transfer hangs with certain buffer sizes: 3, 5, 6 and 8 words (tested up to 12 for now)

Hanging:
[INFO] - WRITE async FlashSafeDma: 0x3ffb0d5f, len: 3
[INFO] - WRITE async SpiDma: 0x3ffb0d5f, len: 3

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d5f, len: 5
[INFO] - WRITE async SpiDma: 0x3ffb0d5f, len: 5

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d5e, len: 6
[INFO] - WRITE async SpiDma: 0x3ffb0d5e, len: 6

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d58, len: 8
[INFO] - WRITE async SpiDma: 0x3ffb0d58, len: 8

Working:
[INFO] - WRITE async FlashSafeDma: 0x3ffb0eb4, len: 2
[INFO] - WRITE async SpiDma: 0x3ffb0eb4, len: 2

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d58, len: 4
[INFO] - WRITE async SpiDma: 0x3ffb0d58, len: 4

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d5f, len: 7
[INFO] - WRITE async SpiDma: 0x3ffb0d5f, len: 7

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d5f, len: 9
[INFO] - WRITE async SpiDma: 0x3ffb0d5f, len: 9

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d5e, len: 10
[INFO] - WRITE async SpiDma: 0x3ffb0d5e, len: 10

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d5f, len: 11
[INFO] - WRITE async SpiDma: 0x3ffb0d5f, len: 11

[INFO] - WRITE async FlashSafeDma: 0x3ffb0d58, len: 12
[INFO] - WRITE async SpiDma: 0x3ffb0d58, len: 12

Secondly received buffer seems to contain only the first byte of the payload (the way I'm currently testing this is sending message from one board to another using LoRa P2P), but I don't know which side is responsible yet.

@Dominaezzz
Copy link
Collaborator Author

Got any code to reproduce that?

@plaes
Copy link
Contributor

plaes commented Jul 24, 2024

Got any code to reproduce that?

Well, repository of my LoRa code is currently here - https://github.com/plaes/rust-lilygo-ttgo-lora32 which uses lora-phy for communication with sx1276 which internally uses embedded-hal-1.0's transfer method:
https://github.com/lora-rs/lora-rs/blob/36885275fbfed98578e1a3a48555d973638b6105/lora-phy/src/interface.rs#L40-L42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working peripheral:dma DMA Peripheral
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants