Skip to content

feat(dmac)!: add Interrupts type parameter to DMA channels#988

Draft
QuartzShard wants to merge 13 commits intoatsamd-rs:masterfrom
QuartzShard:dma-typelevel-interrupt-state
Draft

feat(dmac)!: add Interrupts type parameter to DMA channels#988
QuartzShard wants to merge 13 commits intoatsamd-rs:masterfrom
QuartzShard:dma-typelevel-interrupt-state

Conversation

@QuartzShard
Copy link
Contributor

@QuartzShard QuartzShard commented Feb 23, 2026

Summary

Follow on from #977: Adds a new Typelevel enum Interrupts with states Available and Blocked. This gates enable_interrupts and associated methods so that a Blocked channel cannot change interrupt configuration.

This allows the implementation of From<> to convert Async-mode channels (ReadyFuture), into Sync-mode channels (Ready) (and vice versa) to permit the use of Transfer when the controller is configured in Future mode - without risking Incorrect behavior why conflicting with the ISR that the async DMAC uses. My motivation for this was requiring both async usage for UART and a Circular transfer for reading from a DVP camera - but a ReadyFuture channel cannot function with the Transfer api.

Checklist

  • All new or modified code is well documented, especially public items
  • No new warnings or clippy suggestions have been introduced - CI will deny clippy warnings by default! You may #[allow] certain lints where reasonable, but ideally justify those with a short comment.

These new types track whether a channel can safely and correctly
enable/disable interrupts, based on whether the async ISR is in use.
Additionally, a set of `From<>` impls allow conversion of Future-mode
channels to Sync-mode channels for use with the Transfer API (such as
for circular transfers) - which is possible now by tracking the
Interrupt availability.
These channels don't work with the `begin()` method so transfers made
from them are useless. This makes the compiler catch it earlier.
DISCLAIMER: LLM assistance was used for this commit. Docs have been read
over before committing, but proceed with appropriate skepticism.
@QuartzShard
Copy link
Contributor Author

Failing on clippy::type_complexity lint, would like advice on how to address.
The pattern Transfer<Channel<Ch::ID, Busy, Ch::Interrupts>, BufferPair<B, Self>> shows up in a few spots, and with the new param, upsets clippy. Is this a valid ignore, or should an alias be introduced?

@rnd-ash
Copy link
Contributor

rnd-ash commented Feb 24, 2026

I guess introducing a type alias for a Busy channel with Interrupts should work, since a similar concept exists for other peripherals, just not sure what to include exactly in the type Alias...

@QuartzShard
Copy link
Contributor Author

I'm happy with the state of this now - finished with janitorial docs stuff

Allows the HAL and users to accept Channel<Id, Ready, Blocked> as a
type-level promise that interrupts are not enabled, even when not using
the async API.
@QuartzShard QuartzShard force-pushed the dma-typelevel-interrupt-state branch 2 times, most recently from 4772b61 to d4de5fa Compare March 2, 2026 22:34
QuartzShard and others added 4 commits March 2, 2026 22:34
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 6 to 7.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@v6...v7)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-version: '7'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 7 to 8.
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v7...v8)

---
updated-dependencies:
- dependency-name: actions/download-artifact
  dependency-version: '8'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@QuartzShard QuartzShard marked this pull request as draft March 9, 2026 15:49
@QuartzShard
Copy link
Contributor Author

I've been writing a Ring Buffer on top of this, both to actually use and as a stress test of the API, and on reflection, I am not convinced this is the correct approach.

The main point of contention is that the current ISR for the Async DMA unconditionally disables channels when TCMPL fires, which is correct for one-shot channels (which is the entire current story for async DMA). This is wrong for circular/linked transfers, which was what I was attempting to solve by disallowing the use of interrupts when the ISR would cause issues; But really, shouldn't the ISR be made less opinionated? The channel can be disabled in poll() for TransferFutures before returning Ready.

I'm thinking to either move channel management into poll calls, or make the disable conditional on a static CHANNEL_MODE: [AtomicBool; NUM_CHANNELS], which then makes all of this unecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants