Add check owner support to DMA buffers

[Dominaezzz]

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.
• I have added necessary changes to user code to the Migration Guide.
• 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

Allows buffer implementations to configure the "check owner" bit of the DMA channel.
This is something that must be specified by buffer implementations for correct behavior.

No changelog as the Preparation struct isn't public yet and users shouldn't observe any different behaviour from this.

Testing

HIL tests

[bjoernQ]

LGTM

[bugadani]

WHy is this optional, what's the difference between None and Some(false)? The comment uses a lot of words to not explain this.

[Dominaezzz]

• Some(true): DMA channel must the owner bit
• Some(false): DMA channel must NOT the owner bit
• None: DMA channel can do whatever it likes/supports.

I can add this after the "This field allows buffer implementation to configure this behaviour." comment.

[bugadani]

None is somewhat problematic, though, isn't it? It preserves some random previous setting in the current implementation, can't that cause problems?

[Dominaezzz]

Ehhh, it does preserve some random previous setting, which I admit is a bit weird but I couldn't decide on a default haha. Like the doc says, for some buffer implementations, it doesn't matter whether the DMA channel checks or not. (And besides, the buffer trait is unsafe so we're trusting the user to pick the right option)

The main reason this None option exists is because SPI DMA (at least, I haven't checked all the other PDMA channels) doesn't support configuring check_owner, and I wanted a way for buffers to say "I don't care about this setting".

[bugadani]

(And besides, the buffer trait is unsafe so we're trusting the user to pick the right option)

The user can't implement the buffer traits anyway because the fields of Preparation aren't pub 🙃

[Dominaezzz]

Yeah 😅 I'm working towards that. (Still have to decide on builder pattern vs plain struct)

[bugadani]

IDK I think "Some(false): DMA channel must NOT [check?] the owner bit" sounds like a reasonable default instead of "None = whatever". You obviously know more here, so I would appreciate you explaining why accidentally owner-checking based on past use is a better option.

I think if the answer is "it makes no difference" then less options might be better.

[Dominaezzz]

So there's two parts of the equation:

1. What behavior the buffer implementation needs
2. What the DMA channel can provide

For 1, None needs to exist as an option because choosing either true or false means some DMA channels just won't work with the buffer implementation. If the buffer says true, DMA channels that cannot check will fail to prepare the buffer. If the buffer says false, DMA channels that can't not check will fail to prepare the buffer.
Having None means the buffer can work with any channel.

For 2, SPI DMA doesn't support configuring the "check owner" bit, so it'll reject false. Every other DMA channel (that is in the hal atm 😄) supports configuring this. In the None case, I left it as "past use" for simplicity but I can set it to true by default I suppose.

To eliminate the "accidentally owner-checking based on past use" I can configure the "check owner" bit to true when the buffer says it doesn't care. Then the behavior would be consistent.

[bugadani]

I still think we should be able to get away with a single bool instead of Option<bool> but maybe I'm just stubbornly not being to understand something. Let's move on, thanks for the PR.