|
| 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