Skip to content

Commit 98600aa

Browse files
authored
RFC #59: Removing upwards propagation of clock domains
2 parents 055ffd6 + 00a5967 commit 98600aa

File tree

1 file changed

+66
-0
lines changed

1 file changed

+66
-0
lines changed
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
- Start Date: 2024-03-25
2+
- RFC PR: [amaranth-lang/rfcs#59](https://github.com/amaranth-lang/rfcs/pull/59)
3+
- Amaranth Issue: [amaranth-lang/amaranth#1242](https://github.com/amaranth-lang/amaranth/issues/1242)
4+
5+
# Get rid of upwards propagation of clock domains
6+
7+
## Summary
8+
[summary]: #summary
9+
10+
Upwards propagation of clock domains is removed, `local=True` effectively becomes the default.
11+
12+
## Motivation
13+
[motivation]: #motivation
14+
15+
Currently, a clock domain created anywhere in the hierarchy is, by default, propagated everywhere within that hierarchy. The other option is marking a clock domain with `local=True`, which restricts propagation to only downwards from the point of definition.
16+
17+
The default behavior constitutes the ultimate implicit spooky action at a distance, making it hard to reason about clock domain origins in complex Amaranth design. It is also rarely used. This dangerous default should be changed to something more reasonable.
18+
19+
## Guide and reference-level explanation
20+
[explanation]: #explanation
21+
22+
In version 0.5, upwards domain propagation becomes deprecated. Any use of a domain that wouldn't be valid without upwards propagation (ie. wouldn't be valid if the domain were `local=True`) triggers a deprecation warning.
23+
24+
In version 0.6, upwards domain propagation is removed entirely. The `local` argument to `ClockDomain` constructor becomes redundant and is deprecated.
25+
26+
In version 0.7, the `local` argument is removed.
27+
28+
## Drawbacks
29+
[drawbacks]: #drawbacks
30+
31+
A little bit of churn for designs with clock generator modules.
32+
33+
When propagating a domain upwards is actually desired (eg. for clock generator modules), a workaround is now necessary to route the domain to the toplevel. The simplest way is to set an eg. `cd_sync` attribute on the module in the constructor, then do `m.domains.sync = clkgen.cd_sync` in the top-level. This has the slight disadvantage that the clock generator module cannot decide on the attributes of the clock domain (clock polarity, reset asyncness) based on the platform.
34+
35+
## Rationale and alternatives
36+
[rationale-and-alternatives]: #rationale-and-alternatives
37+
38+
The core proposal is straightforward. The main question is how to handle the usecases currently served by purposeful use of upwards domain propagation. There are three alternatives:
39+
40+
1. None. Clock generator modules must export their domains through some other existing Amaranth functionality. This is the option proposed above.
41+
2. Keep the current mechanism as-is, but make it opt-in, such by requiring `ClockDomain(global=True)`.
42+
3. Add a mechanism to explicitly import a domain from a submodule. A draft is proposed here, for discussion purposes:
43+
44+
```py
45+
m.submodules.clkgen = ClockGenerator(...)
46+
m.domains.sync = ImportedDomain("clkgen", "sync") # Imports a domain defined in the submodule "clkgen" named "sync".
47+
```
48+
49+
We feel that 1. is the best option to take, as introducing a new mechanism when a full redesign of the system is pending is likely to just result in another thing that will have to be deprecated later.
50+
51+
## Prior art
52+
[prior-art]: #prior-art
53+
54+
None.
55+
56+
## Unresolved questions
57+
[unresolved-questions]: #unresolved-questions
58+
59+
None.
60+
61+
## Future possibilities
62+
[future-possibilities]: #future-possibilities
63+
64+
The concept of "private" clock domains has been proposed: clock domains that don't propagate at all, including downwards.
65+
66+
It's clear that clock domain overhaul with a larger scope needs to be done. This RFC is (among other things) intended to enable such overhaul, by removing a feature likely to make it infeasible.

0 commit comments

Comments
 (0)