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

Issue with TileLink ready-valid logic in TLMasterModel #175

Open
tymcauley opened this issue Oct 21, 2019 · 2 comments
Open

Issue with TileLink ready-valid logic in TLMasterModel #175

tymcauley opened this issue Oct 21, 2019 · 2 comments
Assignees

Comments

@tymcauley
Copy link
Contributor

In the TLMasterModel Trait, there are tlWrite* and tlRead* functions for each TileLink channel. All of them contain logic that looks something like this:

def tlWriteA(a: AChannel): Unit = {
  poke(memTL.a.valid, 1)
  pokeA(a)

  while(peek(memTL.a.ready) != BigInt(0)) {
    step(1)
  }
  step(1)
  poke(memTL.a.valid, 0)
}

I'm a bit confused by the line while(peek(memTL.a.ready) != BigInt(0)) {. That would suggest the logic of this function looks something like this:

  • Assert valid.
  • Wait for ready to deassert.
  • Wait a cycle, then deassert valid.

Since the TileLink transaction fires when both ready and valid are high, shouldn't we wait for ready to assert, not deassert? Should that line read while(peek(memTL.a.ready) == BigInt(0)) { or while(peek(memTL.a.ready) != BigInt(1)) {?

@grebe grebe self-assigned this Nov 6, 2019
@grebe
Copy link
Contributor

grebe commented Nov 6, 2019

Sorry for the slow reply.

Yeah, you're absolutely correct. TLMasterModel has always been a bit rickety compared to AXI, perhaps this is one reason why. I'll assign fixing this to myself. The model should really be tested more.

@tymcauley
Copy link
Contributor Author

No problem, thanks for the confirmation! Let me know if I can do anything to help out. I can certainly submit a PR to fix these logical bugs in the TLMasterModel trait, but since I'm not so familiar with the code base, I don't know if I'll really be able to add new tests (which it sounds like you'd like to do).

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

No branches or pull requests

2 participants