Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

soc/interconnect/wishbone: Add Wishbone CDC #2158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

david-sawatzke
Copy link
Contributor

@david-sawatzke david-sawatzke commented Jan 13, 2025

Adds a wishbone CDC implementation. Inspired by BusSynchronizer, uses the PulseSynchronizer to begin/end transaction and ensure all bus bits are stable.

Most of it is handling the WB interface definitions to generate interfaces and signals for both directions.

@enjoy-digital
Copy link
Owner

Thanks @david-sawatzke, I'll try to review it soon. I also have an alternative version not yet upstreamed based on stream.ClockDomainCrosssing, that I used for a project recently, it would be interesting to compare each:

from migen import *

from litex.gen import *
from litex.gen.genlib.misc import WaitTimer

from litex.soc.interconnect import stream

# Wishbone Clock Crossing --------------------------------------------------------------------------

# This module is a simple/minimal clock crossing module for Wishbone transactions.

class WishboneClockCrossing(LiteXModule):
    def __init__(self, platform, wb_from, cd_from, wb_to, cd_to):
        # S2M CDC (cd_from -> cd_to).
        # ---------------------------
        self.cdc_s2m = cdc_s2m = stream.ClockDomainCrossing(
            layout  = [
                ("we",     1),
                ("adr",   32),
                ("sel",    4),
                ("dat_w", 32),
            ],
            cd_from = cd_from,
            cd_to   = cd_to,
        )
        # M2S CDC (cd_to -> cd_from).
        # ---------------------------
        self.cdc_m2s = cdc_m2s = stream.ClockDomainCrossing(
            layout  = [
                ("err",     1),
                ("dat_r",  32),
            ],
            cd_from = cd_to,
            cd_to   = cd_from,
        )

        # Cross FSM.
        # ----------
        self.cross_fsm = cross_fsm = ClockDomainsRenamer(cd_from)(FSM(reset_state="IDLE"))
        cross_fsm.act("IDLE",
            If(wb_from.cyc & wb_from.stb,
                NextState("SEND")
            )
        )
        cross_fsm.act("SEND",
            # Send request to CDC.
            cdc_s2m.sink.valid.eq(1),
            cdc_s2m.sink.we.eq(wb_from.we),
            cdc_s2m.sink.adr.eq(wb_from.adr),
            cdc_s2m.sink.sel.eq(wb_from.sel),
            cdc_s2m.sink.dat_w.eq(wb_from.dat_w),
            If(cdc_s2m.sink.ready,
                NextState("RECEIVE")
            )
        )
        cross_fsm.act("RECEIVE",
            # Get response from CDC.
            cdc_m2s.source.ready.eq(1),
            If(cdc_m2s.source.valid,
                wb_from.ack.eq(1),
                wb_from.err.eq(cdc_m2s.source.err),
                wb_from.dat_r.eq(cdc_m2s.source.dat_r),
                NextState("IDLE")
            )
        )

        # Access FSM.
        # -----------
        self.access_fsm   = access_fsm = ClockDomainsRenamer(cd_to)(FSM(reset_state="IDLE"))
        self.access_timer = access_timer = ClockDomainsRenamer(cd_to)(WaitTimer(64))
        access_fsm.act("IDLE",
            If(cdc_s2m.source.valid,
                NextState("ACCESS")
            )
        )
        access_fsm.act("ACCESS",
            access_timer.wait.eq(1),
            # Perform Wishbone access.
            wb_to.cyc.eq(1),
            wb_to.stb.eq(1),
            wb_to.we.eq(cdc_s2m.source.we),
            wb_to.adr.eq(cdc_s2m.source.adr),
            wb_to.sel.eq(cdc_s2m.source.sel),
            wb_to.dat_w.eq(cdc_s2m.source.dat_w),
            If(wb_to.ack | access_timer.done,
                # Send response back through CDC.
                cdc_s2m.source.ready.eq(1),
                cdc_m2s.sink.valid.eq(1),
                cdc_m2s.sink.err.eq(wb_to.err),
                cdc_m2s.sink.dat_r.eq(wb_to.dat_r),
                # Err on Timeout.
                If(access_timer.done,
                    cdc_m2s.sink.err.eq(1),
                    cdc_m2s.sink.dat_r.eq(0xffffffff)
                ),
                NextState("IDLE")
            )
        )

@david-sawatzke
Copy link
Contributor Author

david-sawatzke commented Jan 21, 2025

Taking a quick look:

  • MultiReg with manual synchronization with PulseSynchronizer vs. ClockDomainCrossing: The former is more complex to implement but might take less resources, as the stream AsyncFIFO is at least 4 words large
  • My implementation extracts the signals from the Interface which is less readable, but matches all variations of the Interface.
  • I think you're incorrectly handling err. According to Rule 3.45 either err or ack can be asserted (to end the cycle), but not both.
  • An additional timeout on the access FSM, which I don't think is required
  • A timeout happening on the master side is not propagated to the other side by me and can result in weird accesses. It's also not properly handled by your implementation (might generate errant ACKs on the master side), but much less likely due to the short slave timeout.

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