Conversation
aolofsson
commented
Feb 6, 2026
- Dual mode, can work as device/endpoint or as two independent full duplex channels.
- In device mode, reads pull from the s2mm fifo, writes "done" on write to mm2s fifo
- Only posted writes in duplex mode
- S2MM command entry handled outside module, pulling it in would make it too heavy
- A bit funky, so definitely needs architectural/critical review. 😄
RiceShelley
left a comment
There was a problem hiding this comment.
Nice add, architecturally seems solid. Working on a Testbench here
|
|
||
| assign s2mm_fifo_write = usi_in_valid & ~s2mm_fifo_full; | ||
| assign s2mm_fifo_din[DW-1:0] = usi_in_data[DW-1:0]; | ||
| assign s2mm_fifo_din[DW] = usi_in_last; |
There was a problem hiding this comment.
usi_in_last is clocked into ififo_s2mm but never used on the umi_clk side of the fifo. The umi_out_cmd is driven only by s2mm_cmd when device mode is 0. I would expect that the syncronized usi_in_last would replace the EOM or EOF bit given by s2mm_cmd.
There was a problem hiding this comment.
Added issue, will fix in next PR.
| // Posted writes: only needs FIFO not full (works in both modes) | ||
| // Write with ack: need FIFO not full AND no stall (device mode only) | ||
| // Read: need s2mm not empty AND no stall (device mode only) | ||
| assign umi_in_ready = (cmd_write_posted & ~mm2s_fifo_full) | |
There was a problem hiding this comment.
Ready being combinationaly dependent on UMI CMD feels like an extension of the spec rule ready shall not be combinationaly dependent on valid. I think an easy fix would be have a buffer on UMI input and ready is driven high when the buffer is empty.
Line 462 in e57520f
There was a problem hiding this comment.
Added issue, will fix in next PR