Skip to content

Add passthrough mode for mem_to_banks with NumBanks==1#226

Closed
micprog wants to merge 1 commit intov2-devfrom
michaero/mem_to_banks_passthrough
Closed

Add passthrough mode for mem_to_banks with NumBanks==1#226
micprog wants to merge 1 commit intov2-devfrom
michaero/mem_to_banks_passthrough

Conversation

@micprog
Copy link
Copy Markdown
Member

@micprog micprog commented Aug 20, 2024

No description provided.

@micprog micprog requested a review from niwis August 20, 2024 16:32
Copy link
Copy Markdown
Collaborator

@niwis niwis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @micprog! I guess that changing the functional behaviour of mem_to_banks_detailed for NumBanks == 1 would require a major release? Also, I think we should somehow handle conflicting parametrisations (e.g. NumBanks == 1 && HideStrb == 1). Perhaps it would be cleaner to change this in the instantiating module and just add a warning here if NumBanks == 1. What do you think?

@niwis niwis added the v2 label Feb 4, 2025
@niwis niwis added this to the v2.0 milestone Feb 4, 2025
@colluca colluca mentioned this pull request Apr 17, 2026
14 tasks
@phsauter phsauter changed the base branch from master to v2-dev April 23, 2026 13:23
@phsauter
Copy link
Copy Markdown
Collaborator

phsauter commented May 6, 2026

I have been thinking about this and the correct solution to me seems to be warning and assertions.
This change could potentially introduce critical paths due to the FIFO removal and it functionally changes what it does in the case of back-pressure.
So in my opinion this should be considered closed and we should add the warnings instead (I will leave it open until this is done).

@phsauter
Copy link
Copy Markdown
Collaborator

phsauter commented May 7, 2026

The asserts/warnings will be in v2 with #299 , the rest is dropped. I will archive the branch.

@phsauter phsauter closed this May 7, 2026
@phsauter phsauter deleted the michaero/mem_to_banks_passthrough branch May 7, 2026 16:20
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.

3 participants