embassy icon indicating copy to clipboard operation
embassy copied to clipboard

RP2040: Add DMA transfer abort function

Open JomerDev opened this issue 1 year ago • 8 comments

This PR adds an abort function to RP2040 DMA transfers, which also returns the amount of bytes transferred up until this point. That can be useful in cases where a frame of dynamic length is read via PIO and the end of a frame is signaled via interrupt, so the code can stop the transfer and immediately know how long the frame was

JomerDev avatar Jul 13 '24 16:07 JomerDev

  • this makes the Transfer struct bigger which will affect all users even if they don't want this feature. Maybe yo ucan make it return the amount of bytes left instead, which you can read from trans_count so you don't need to store anything extra. it's a bit less ergonomic bvecause the user has to subtract but should still work.
  • abort should take self instead of &mut self, otherwise the dma transfer will be aborted again when dropped later.

Dirbaio avatar Jul 14 '24 06:07 Dirbaio

Fixed. I've also changed abort to return the data size as well, because without it the transfer count isn't as useful

JomerDev avatar Jul 14 '24 10:07 JomerDev

why? the user knows the data size already because they initiated it themselves.

Dirbaio avatar Jul 14 '24 11:07 Dirbaio

While that is technically true, I don't think many people make the connection that the size of the type they created the buffer with equals the data size that the dma transfers. Usually people will pick a u8 representation for byte buffers

JomerDev avatar Jul 14 '24 12:07 JomerDev

@Dirbaio Would you prefer it if the abort method always returns the transfer count in bytes and the users can calculate the transfer count for their type if they want to?

JomerDev avatar Jul 16 '24 13:07 JomerDev

the "natural" thing is to always use "number of words", where "word" is u8/u16/u32 depending on what the user passed in. For example, if you do a transfer with a &[u16] then .len() returns a number of u16 words, so it makes sense that abort() also returns number of u16 words.

I wouldn't "helpfully" convert it to bytes for the user.

Dirbaio avatar Jul 16 '24 15:07 Dirbaio

Alright, I've changed it so only the transfer count is returned I don't know why the build has now failed, I didn't change anything else

JomerDev avatar Jul 16 '24 19:07 JomerDev

to fix ci, can you rebase on latest main?

Dirbaio avatar Jul 16 '24 19:07 Dirbaio