ucb-bar / dsptools

A Library of Chisel3 Tools for Digital Signal Processing
Apache License 2.0
227 stars 38 forks source link

Issue with TileLink ready-valid logic in TLMasterModel #175

Open tymcauley opened 5 years ago

tymcauley commented 5 years ago

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:

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 commented 5 years ago

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 commented 5 years ago

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).