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

Structure suggestion #23

Merged
merged 1 commit into from
Nov 28, 2022
Merged

Conversation

numero-744
Copy link
Collaborator

@numero-744 numero-744 commented Nov 18, 2022

Draft structure suggestion according to SpinalHDL/SpinalDoc-RTD#147

There are no unit / regression tests yet; I won't add anything for that before SpinalHDL/SpinalHDL#965 is merged and released (feedback welcome btw), or closed.

Don't merge before:

hw/spinal/config.scala Outdated Show resolved Hide resolved
hw/spinal/config.scala Outdated Show resolved Hide resolved
hw/spinal/config.scala Outdated Show resolved Hide resolved
@numero-744
Copy link
Collaborator Author

@Dolu1990 you started a review, so do I assume agreement on the structure?

So:

@Dolu1990
Copy link
Member

One issue is the MyTopLevel folder :

  • The package structure and the folder structure need to match. I already had issues with tools when it is done otherwise.

So it realy has to be removed
Which also kind of trash the xxx.scala structure you have. In someway, i think it need to be reverted back to something more similar to the original structure MyTopLevelGen.scala and so on

@numero-744
Copy link
Collaborator Author

Wilco for MyTopLevelGen.scala-like names with no MyTopLevel folder. It is a little bit less structured but that's not a big deal as it is compliant with lexical order so all is fine 👍

Other things to change? Can I start documenting?

@numero-744
Copy link
Collaborator Author

For MyTopLevelVerilog and MyTopLevelVhdl they are not in a MyTopLevelXxx file, isn't it an issue there too?

Maybe we should have this, which generates all by default, and depending on the synthesis tool used the users can remove the ones they don't need?

object MyTopLevelGen extends App {
  config.spinal.generateVerilog(MyTopLevel())
  config.spinal.generateVhdl(MyTopLevel())
}

@Dolu1990
Copy link
Member

Other things to change? Can I start documenting?

So this and the other topics which were already opened, that's all ^^

For MyTopLevelVerilog and MyTopLevelVhdl they are not in a MyTopLevelXxx file, isn't it an issue there too?

Not an issue they can be both into MyTopLevelGen.scala

object MyTopLevelGen

Hmmm i think the best is to keep a Verilog and a Vhdl object instead of running two time the generation (which is something that normaly nobody do)

@numero-744
Copy link
Collaborator Author

Tested with mill running:

mill projectname.runMain projectname.MyTopLevelSim

@numero-744 numero-744 marked this pull request as ready for review November 23, 2022 19:26
@numero-744
Copy link
Collaborator Author

Rebased to clean history.

@Dolu1990 for merging

@numero-744
Copy link
Collaborator Author

@Dolu1990 I added two little commits as the PR is still open, but it can be merged I think

@Dolu1990
Copy link
Member

Nice, thanks :)

@Dolu1990 Dolu1990 merged commit 07a5bea into SpinalHDL:master Nov 28, 2022
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.

2 participants