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

MMSlaveFactory #352

Open
wants to merge 23 commits into
base: dev
Choose a base branch
from
Open

MMSlaveFactory #352

wants to merge 23 commits into from

Conversation

c-thaler
Copy link
Contributor

Half a year ago we discussed merging RegIf and BusSlaveFactory into a single thing. This is the current state of my work.

Currently, there are implementations for Axi4Lite and APB3. It supports Read-Only-Regs, Read Stream, Write Stream, Clear Regs. It generates C Header files, JSON files and HTML description for the interface.

It works well in my current project. Thus I think is mature enough to share.

@Dolu1990
Copy link
Member

Dolu1990 commented Nov 2, 2020

Hi,

Cool :)

About addField, is seems there is no explicit way to specify where it should be in the word ? ex add field at bit 8 of the word ?
Or i missed something ?

@c-thaler
Copy link
Contributor Author

c-thaler commented Nov 2, 2020

There is no explicit way. You can use reserved(bitwidth) to insert a gap into the word.

But I think I can add a function that allows explicitly adding fields to a position. Simply adding an index to the Field class and then sorting the Field objects in a ascending order when mapping to the register.

@Dolu1990
Copy link
Member

Dolu1990 commented Nov 6, 2020

But I think I can add a function that allows explicitly adding fields to a position. Simply adding an index to the Field class and then sorting the Field objects in a ascending order when mapping to the register.

You mean a fields to explicitly set the bit position ? or the field index ?

@c-thaler
Copy link
Contributor Author

c-thaler commented Nov 8, 2020

There is now a functions to add a field for a word with a parameter to explicitly set the bit position of the field within the word.

@Dolu1990
Copy link
Member

@c-thaler

Hi,

Sorry for the delay, i'm too busy those weeks :/

So, one question :

  val regAddr = slavFac.createReg("address", "Destination address")
  val address = regAddr.newField("addr", 32 bits, 0, "Address")

Is it possible to controle per field if you can Read / Write it ? (having a mixed Reg)

@c-thaler
Copy link
Contributor Author

No problem, same here :)

Is it possible to controle per field if you can Read / Write it ? (having a mixed Reg)

I'm afraid that's not possible. Access type is determined by the Entry implementation.

@Dolu1990
Copy link
Member

I'm afraid that's not possible. Access type is determined by the Entry implementation.

To me, it seem it is too limitating, isn't it ?

@c-thaler
Copy link
Contributor Author

c-thaler commented Dec 1, 2020

At least limiting when it comes to a dense packing of the registers :D

I struggled a lot with the different kinds of fields and was not sure how to handle it correctly.
Of course it makes sense to have RO and RW fields in the same word. But there are other combinations that cause some trouble. For instance a field connected to a FIFO and a RW field in the same word. The FIFO may cause some back pressure and the RW is not written until the FIFO is ready.
Do you think it is OK to leave this complex behavior relations to the caller of MMSlaveFactory? Or any ideas on how to deal with such situations?

I'm sure I can easily move all the access handling from the entries to the fields and just keep the address decoding and event generation on entry level.

@Dolu1990
Copy link
Member

Dolu1990 commented Dec 2, 2020

hmm so the design and API is realy different from https://github.com/SpinalHDL/SpinalHDL/tree/dev/lib/src/main/scala/spinal/lib/bus/regif
right ?

But there are other combinations that cause some trouble. For instance a field connected to a FIFO and a RW field in the same word. The FIFO may cause some back pressure and the RW is not written until the FIFO is ready.

Do not bother about this kind of mixes, it should never be done i would say.

@asicdruide
Copy link

I'd say in case somebody is trying to describe unreasonable/unsupported field combinations in one register, just complain and bail out.
For the beginning support for RO/WO/RW field combinations would be enough. More might come later if there is a real need.

@c-thaler
Copy link
Contributor Author

c-thaler commented Dec 2, 2020

Ok, so I leave this to the caller and may do some checks if the combination has weird consequences.

hmm so the design and API is realy different from https://github.com/SpinalHDL/SpinalHDL/tree/dev/lib/src/main/scala/spinal/lib/bus/regif
right ?

At the moment its different. But with the changes I will go back closer to the regif API. One function to create a register word. And several functions to add different fields, e.g. fieldRO, fieldRW, fieldClear.

@Readon
Copy link
Collaborator

Readon commented Dec 24, 2021

RegIf and MMSlaveFactory both do a good job. regIf or MMSlaveFactory, which is the preferred way forward?

I am planning to use such a mechanism in my design. I can either try to complete Axi4 support for RegIf or wait for the completion of MMSlaveFactory, but of course it is possible to contribute to this extension.

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