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

Regif update #323

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Regif update #323

wants to merge 1 commit into from

Conversation

c-thaler
Copy link
Contributor

I decoupled the doc generation from the BusIf structure to make it easier for users to implement their own generators. They can now simply implement a BusIfVisitor.
I also ported the CHeader and HTML generation to BusIfVisitor generators and added a JSON generator.
Regif now also supports Axi4-Lite bus.
Furthermore, I added an event signal for read/write (eventR/eventW).

@jijingg
Copy link
Collaborator

jijingg commented May 21, 2020

@c-thaler good job,
i'm not sure should we rename "regif" to "busif", i think the mem mapping and document also need supported later

@c-thaler
Copy link
Contributor Author

@jijingg I'm fine with "regif". Usually, I refer to such a thing as "register interface" or "register slave interface". But maybe in the long run it will replace the Bus Slave Factory and we can use a name closer to that. That is for @Dolu1990 to decide.

@Dolu1990
Copy link
Member

Idealy, both BusSlaveFactory and RegIf woiuld be merged into a single thing ^^
Then about the name, to be honnest, both BusSlaveFactory and RegIf aren't so great to me.
"Bus" can be understood as a bit vector, and register isn't great because it can map other things.

So maybe the best would be :
MemoryMappedSlaveFactory XD ?

@c-thaler
Copy link
Contributor Author

MemoryMappedSlaveFactory is a good description for what it does. Maybe MMSlaveFactory? I'm not sure if everybody interprets MM as "MemoryMapped". At least it also appears in "AvalonMM" :D

Idealy, both BusSlaveFactory and RegIf woiuld be merged into a single thing

Ok. I think that most of the BusSlaveFactory interface can be implemented as a kind of convenience methods in RegIf. So you don't have to deal with "regs" and "fields" which are the building blocks in RegIf.

@Dolu1990
Copy link
Member

Hi ^^

Aren't those conflict weird ?

@c-thaler
Copy link
Contributor Author

Hmmm, I did not touch most of these files :D

However, I'm working on an integration of RegIf and Bus Slave Factory. Thus, we can postpone the merge until I'm finished and then merge the whole thing.

@Dolu1990
Copy link
Member

@c-thaler Ok good :D

@andreasWallner
Copy link
Collaborator

@c-thaler I know I'm a bit late to the party, but my 0.02€ from using the regif stuff (just regarding the generators, not the other stuff):

I'm not sure whether the visitor like here it the best solution: In Java I'd understand, but scala has the match/case construct for pretty much exactly the use-case you'd do the visitor + double-dispatch in Java.
In my modified generators (e.g. https://github.com/andreasWallner/SpinalHDL/blob/typed_regif/lib/src/main/scala/spinal/lib/bus/regif/generator/CHeader.scala) I just iterate over the registers/fields and I need to do so multiple times.
The main reason for that is that I e.g. generate the CMSIS-usual struct for the registers, as well as pos/mask defines for the fields. Writing that one or my version of the HTML output with the visitor pattern quickly becomes ugly (no that my code is cleaned up yet... ;-) )

What are your thoughts on that? I'd be happy to remove all the other stuff that is present on my branch and open a PR depending on that.

Regarding visitor & scala: https://stackoverflow.com/questions/8618082/visitor-pattern-in-scala

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

Successfully merging this pull request may close these issues.

None yet

4 participants