Fix reading/writing small buffers

[bjoernQ]

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

• I have updated existing examples or added new ones (if applicable).
• I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
• My changes were added to the CHANGELOG.md in the proper section.
• My changes are in accordance to the esp-rs API guidelines

Extra:

• I have read the CONTRIBUTING.md guide and followed its instructions.

Pull Request Details 📖

Description

Fixes #1728

Testing

See #1728 for a minimal repro

[Dominaezzz]

I've been staring at the code for the last 30 minutes and I can't figure out why this PR fixes the linked issue.

On a somewhat unrelated note, I spotted something else that could also contribute to the transfer getting stuck.

This is the existing code before this PR.

        reset_dma_before_load_dma_dscr(reg_block);
        chain.fill_for_tx(false, ptr, len)?;
        unsafe {
            tx.prepare_transfer_without_start(self.dma_peripheral(), chain)
                .and_then(|_| tx.start_transfer())?;
        }
        self.clear_dma_interrupts();
        reset_dma_before_usr_cmd(reg_block);

        if listen {
            tx.listen_eof();
        }
        reg_block.cmd().modify(|_, w| w.usr().set_bit());

There's a race condition in it. tx.start_transfer() is called before self.clear_dma_interrupts().
This means the code could end up clearing interrupts from the started transfer that was just started.

In practice this is rare since most people should be doing DMA transfers with large buffers but if the DMA buffer is small enough, the DMA could end up filling the peripheral's FIFO before the peripheral has even been started.

Having said that, I now realize it's also incorrect to call reset_dma_before_usr_cmd(reg_block) after tx.start_transfer(), since the code would end up discarding data from the peripheral's FIFO that DMA may have just placed there. Though this only applies to GDMA and not PDMA.

(More stuff to exercise in the HIL tests I guess 😄)

[bjoernQ]

On ESP32 the interrupt triggered before reg_block.cmd().modify(|_, w| w.usr().set_bit()); and before tx.listen_eof() - so the interrupt cleared INT_ENA, then it gets set again making the future believe the transfer is still ongoing

[bjoernQ]

I agree on clearing the interrupts before preparing DMA, but regarding reset_dma_before_usr_cmd the TRM says

[Dominaezzz]

Ahhh I see, okay those are probably fine then. I wish the TRM had more details about how these buffers worked.

[JurajSadel]

LGTM, thanks!

[MabezDev]

The failed test test_symmetric_dma_transfer_huge_buffer could be a stack overflow, I've seen that defmt panic before when the global defmt flag gets trashed. Not sure why this has been induced from these changes though 🤔

[SergioGasquez]

The failed test test_symmetric_dma_transfer_huge_buffer could be a stack overflow.

The test also fails with const DMA_BUFFER_SIZE: usize = 50

[jessebraham]

Since Bjoern is on vacation this week, I will convert this to a draft for the time being.

[bjoernQ]

Turned out clearing the interrupts earlier (which seemed to make sense) caused problems for ESP32-S3 .... Very glad we have HIL-TESTs in place

[JurajSadel]

LGTM, thanks!

[SergioGasquez]

LGTM!