Skip to content

Update axi_ram.v#94

Open
pavel-shushunov wants to merge 1 commit intoalexforencich:masterfrom
pavel-shushunov:patch-1
Open

Update axi_ram.v#94
pavel-shushunov wants to merge 1 commit intoalexforencich:masterfrom
pavel-shushunov:patch-1

Conversation

@pavel-shushunov
Copy link
Copy Markdown

Alex, you write addr before data in AXI realization. You cannot skip the WRITE_STATE_RESP state. Otherwise, when processing a sequence of packets, you will end up writing to the previous address.

Alex, you write addr before data in AXI realization. You cannot skip the WRITE_STATE_RESP state. Otherwise, when processing a sequence of packets, you will end up writing to the previous address.
@alexforencich
Copy link
Copy Markdown
Owner

alexforencich commented Jan 23, 2026

Did you actually test that change? Or should I let the CI flow run so you can see how bad the protocol violation is?

@alexforencich
Copy link
Copy Markdown
Owner

alexforencich commented Jan 23, 2026

Screw it, let's watch it fail.

I don't see what issue it is that you're trying to solve here. In the WRITE_STATE_BURST state, when mem_wr_en is set to 1, the registered address from the previous clock cycle (either captured from awaddr in WRITE_STATE_IDLE, or an incremented address in WRITE_STATE_BURST) is used to write the current data in wdata. Note that mem_wr_en is combinatorial. At the end of the burst, it can immediately return to idle to latch in the next address and start the next operation, the only reason the WRITE_STATE_RESP is needed is to handle back-pressure on the B channel.

Your change results in duplicate write responses. Fixing that would result in an unnecessary stall cycle when there is no B channel back-pressure.

@pavel-shushunov
Copy link
Copy Markdown
Author

Alex, I've resolved my issue with the axi_ram connection at top level. In my case, AW was captured incorrectly due to a misaligned FSM initial state, which resulted in an outdated address.

I fully agree that you can skip the stall cycle when there's no back-pressure on the B channel.

Thank you for your response. The issue has been resolved.

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