Skip to content

Conversation

LBFFilho
Copy link
Contributor

@LBFFilho LBFFilho commented Sep 11, 2025

AXI Streaming doesn't allow for tvalid to depend on tready (AMBA AXI-Stream Protocol Specification, 2.2 Handshake signaling).
The previous version of util_axis_fifo_asym relied on the downstream user asserting tready whenever there was data at the output that was invalid due to tkeep=0, even if tvalid was 0 in this case.

In #1808 this forced us to introduce extra logic on the spi_engine as a workaround. With this PR, we can skip that extra logic. If this is merged after #1808, we'll need to add a commit removing the workaround logic.

This FIFO is used only in the spi_engine (after #1808 ) and data_offload IPs. I tried to make the fix so it only affects the TKEEP_EN=1 case, which would only affect spi_engine.

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)
  • Documentation

PR Checklist

  • I have followed the code style guidelines
  • I have performed a self-review of changes
  • I have compiled all hdl projects and libraries affected by this PR
  • I have tested in hardware affected projects, at least on relevant boards
  • I have commented my code, at least hard-to-understand parts
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
  • I have not introduced new Warnings/Critical Warnings on compilation
  • I have added new hdl testbenches or updated existing ones

@caosjr
Copy link
Contributor

caosjr commented Sep 12, 2025

I tested these changes with different tkeep masks on the SPI Engine and it works. I could not test different tkeep masks on the testbench here: https://github.com/analogdevicesinc/testbenches/tree/main/testbenches/ip/util_axis_fifo_asym
since the TB is not prepared for that.
I agree with the change.

@LBFFilho
Copy link
Contributor Author

LBFFilho commented Sep 12, 2025

Added a testbench for this (still a draft for now): analogdevicesinc/testbenches#246
Added a fix for a tlast corner case that I found with the new testbench (2.5.1 TKEEP qualification violation).

Fix a bug where m_axis_valid waits on m_axis_ready to be asserted a few times in
order to skip over data for which tkeep was all zeroes. This was a violation of
the AXI Streaming protocol.

Signed-off-by: Laez Barbosa <laez.barbosa@analog.com>
Fix tvalid low when tlast is present.

Before this commit, util_axis_fifo would suppress all transfers with tkeep=0,
even if tlast was asserted. This commit makes it so transfers with tlast
asserted are preserved.

The AXI-Streaming spec allows us to suppress transfers for which all tkeep bits
are 0 (all bytes are null bytes), but only in the case when tlast=0 as well.
Transfers where tlast is asserted can't be suppressed even when all bytes are
null.

Signed-off-by: Laez Barbosa <laez.barbosa@analog.com>
@sarpadi sarpadi requested a review from podgori September 24, 2025 18:59
Signed-off-by: Laez Barbosa <laez.barbosa@analog.com>
@LBFFilho LBFFilho requested a review from caosjr September 24, 2025 19:00
@LBFFilho LBFFilho marked this pull request as draft October 8, 2025 12:19
@LBFFilho
Copy link
Contributor Author

LBFFilho commented Oct 8, 2025

Moved to draft due to recent discussions on the FIFOs affecting this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants