AsyncFIFO cannot be composed with ResetInserter

[mbikovitsky]

Consider the following scenario:

1. A module defines a new clock domain.
2. The module defines an AsyncFIFO to transfer data from the sync domain into the new clock domain.

Then the module can no longer be reset using anything other than the sync domain's default reset signal.

Minimal reproducing sample:

from amaranth import ResetInserter, Signal
from amaranth.lib.fifo import AsyncFIFO
from amaranth.sim import Simulator, SimulatorContext

write_soft_rst = Signal()
fifo = ResetInserter({"write": write_soft_rst})(
    AsyncFIFO(width=8, depth=2, w_domain="write", r_domain="read")
)


async def testbench(ctx: SimulatorContext) -> None:
    VALUE = 0x13

    # Write some data info the FIFO
    await ctx.tick("write")
    assert ctx.get(fifo.w_rdy)
    ctx.set(fifo.w_en, True)
    ctx.set(fifo.w_data, VALUE)
    await ctx.tick("write")
    ctx.set(fifo.w_en, False)

    # Pop
    while not ctx.get(fifo.r_rdy):
        await ctx.tick("read")
    assert ctx.get(fifo.r_data) == VALUE
    ctx.set(fifo.r_en, True)
    await ctx.tick("read")
    ctx.set(fifo.r_en, False)

    # Soft reset
    await ctx.tick("write")
    ctx.set(write_soft_rst, True)
    await ctx.tick("write")
    ctx.set(write_soft_rst, False)

    # Wait a while
    await ctx.tick("write").repeat(2)

    # Write side should be ready
    assert ctx.get(fifo.w_rdy)

    # Read side should not be ready
    await ctx.tick("read")
    assert not ctx.get(fifo.r_rdy)


sim = Simulator(fifo)
sim.add_clock(1e-6, domain="write")
sim.add_clock(1e-6 / 2, domain="read")
sim.add_testbench(testbench)
with sim.write_vcd("fifo.vcd"):
    sim.run()

As I understand it, the problem stems from the fact that AsyncFIFO synchronizes the write domain's reset signal into the read domain: https://github.com/amaranth-lang/amaranth/blob/v0.5.1/amaranth/lib/fifo.py#L485-L490. However, if the write domain has any other reset signals defined through ResetInserter, then the read domain will no longer be properly reset.

This poses a problem for composing different Amaranth modules together. If a module uses an AsyncFIFO, then any module using it can no longer be reset using ResetInserter.

For example, this is the case in Glasgow, which has a separate reset signal for the applets: https://github.com/GlasgowEmbedded/glasgow/blob/346e5c4f167654dd76c70dcefa168196a540df46/software/glasgow/access/direct/multiplexer.py#L246. This means that applets cannot safely use an AsyncFIFO, since they will break when the "soft-reset" is applied.

[whitequark]

Thank you for the detailed report. This issue is directly related to #591, which also deals with reset handling in AsyncFIFO. They should probably be dealt with together.

[whitequark]

However, if the write domain has any other reset signals defined through ResetInserter, then the read domain will no longer be properly reset.

On re-read, this is actually a general problem where ResetSignal does not reflect sources of reset added via ResetInserter, rather than something that is specific to AsyncFIFO (or possibly both if I'm missing something). I do think that we should focus on reworking the clock domain system for the 0.6 release, but it is a major chunk of work that will require at least one large RFC.

cc @wanda-phi