vegaluisjose commented on a change in pull request #32:
URL: https://github.com/apache/tvm-vta/pull/32#discussion_r700430871
##########
File path: hardware/chisel/src/main/scala/interface/axi/AXI.scala
##########
@@ -211,7 +211,7 @@ class AXIMaster(params: AXIParams) extends AXIBase(params) {
ar.bits.qos := params.qosConst.U
ar.bits.region := params.regionConst.U
ar.bits.size := params.sizeConst.U
- ar.bits.id := params.idConst.U
+ //do not override
Review comment:
Sure, the original intent of this method, back when we had support for
one small board/configuration, was to use this as a mechanism to derive other
params in the protocol. However, things have obviously evolved quite a bit
since then.
What if we do the same for both reads/writes in terms of `id` so future
contributors know that they have to handle `id` and `strb`?
Maybe we can even add comments on top of that `setConst` method saying what
they should implement and we can point to that documentation later if needed.
For the record, these suggestions are intended to help future contributions.
We really appreciate all your efforts on this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
