[Design] Various AXI4 suggestions

[bgamari]

While attempting to use clash-protocols in a project, I found that the separate type-level records used to represent each of the sub-busses of AXI4 made the library very painful to use. In my fork I have merged these into a single type-level record; while this is perhaps slightly less principled than the status quo, it is quite a bit less bothersome in practice. Moreover, I think it is quite defensible as one does tend to use at very least all of the read or all of the write sub-busses in conjunction.

Other changes in my fork include:

• Introduction of HasPortRep instances for my clash-port-name package, allowing convenient naming of top-level entity ports. With these instances, Vivado's port type inference works without any user intervention.
• Introduced Address and TransactionId newtypes as these are very frequently needed and have clear semantics
• Reworking the handling of strobe signals to support the port naming bits above.
• Introduction of an Axi4Lite type synonym and a few smart constructors
• Various utilities for testing designs with my clash-testbench package

[bgamari]

For the record, the type-records package which my fork uses to generate type-level field selectors can be found here. Frankly, it's simple enough that it's probably not worth a package of its own.

[bgamari]

@martijnbastiaan, what is your long-term vision for the AXI4 implementation in clash-protocols? Would any of the above be welcome additions? I would be happy to offer patches if so.

In particular, the type-level record change is IMHO a significant improvement in usability. Currently the type signatures required to use AXI with clash-protocols are extremely verbose. Not only does this compromise readability but it makes it hard to abstract (e.g. it entirely eliminates the need for the repetition introduced in #25). If consolidating the configuration of all five AXI channels in a single type-level record is deemed too coarse-grained, an alternative design would be to consolidate configuration for the read and write halves. This would already significantly reduce the amount of repetition needed.

Naturally, I would be happy to keep the clash-port-name instances in an orphans package. However, the strobe signal rework is a prerequisite for using clash-port-name at all.

[martijnbastiaan]

Thanks for the ping! First off all, clash-protocols is very much an experimental library, so I'm very much looking for feedback like this. So far the team I'm in has only used Wishbone, so AXI hasn't got much attention..

In my fork I have merged these into a single type-level record; while this is perhaps slightly less principled than the status quo, it is quite a bit less bothersome in practice. Moreover, I think it is quite defensible as one does tend to use at very least all of the read or all of the write sub-busses in conjunction.

Agreed. Pragmatism beats purism in this case.

Introduction of HasPortRep instances for my clash-port-name package, allowing convenient naming of top-level entity ports. With these instances, Vivado's port type inference works without any user intervention.

I'm assuming this is in context of Vivado Block Design? If the strobe signal rework is necessary for your project to work, then that's a good argument for doing so. I'm also interested in the clash-port-name itself though - Clash's current approach is lacking. I'm certainly not opposed to clash-protocols relying on packages like that in the future. One caveat is that CI insists on having complete documentation for exposed binders in this project, given its level of abstraction (=> hurdle for beginners). New dependencies that are exposed through this project should follow a similar standard for documentation IMO.

Introduced Address and TransactionId newtypes as these are very frequently needed and have clear semantics

I did look it up in your fork, but I'm not sure what its purpose is. Could you elaborate?

Reworking the handling of strobe signals to support the port naming bits above.

Makes sense!

---

I'll invite you as a collaborator on clash-protocols. Feel free to reject. The only thing I'd ask is to have at least one person look at PRs before merging - even though this isn't forced (mostly to reduce friction when trying to merge typo-like PRs).

[bgamari]

Thanks, @martijnbastiaan !

... so far the team I'm in has only used Wishbone, so AXI hasn't got much attention..

This is good to know; I was worried that introducing refactorings like those suggested here might cause undue pain on existing users. It sounds like that isn't much of a concern, thankfully.

I'm assuming this is in context of Vivado Block Design?

Correct; with clash-port-name Vivado's AXI port inference works out-of-the-box.

I'm also interested in the clash-port-name itself though - Clash's current approach is lacking

I was hoping this would be the case but have been hesitant to suggest it while the documentation remains so dire. I willl try to to rectify this today.

I did look it up in your fork, but I'm not sure what its purpose is. Could you elaborate?

The point here is merely to capture more information in the types and to avoid "BitVector blindness". However, I don't have a strong opinion here. In my recent rewrite of axi-register I have tried to abstract away AXI addresses (and, for that matter, AXI entirely) meaning that the type safety afforded by these newtypes isn't as significant as it was in the previous iteration.

I'll invite you as a collaborator on clash-protocols

Thanks; I have accepted although I will continue to go through the usual MR process in all but the most trivial of patches.

[bgamari]

@martijnbastiaan , I have pushed a README and some Haddocks to clash-port-name. I suspect that more still could be done here, but the situation is much better than it was.