search

efabless / caravel_mpw-one

Caravel is a standard SoC hardness with on chip resources to control and read/write operations from a user-dedicated space.
https://caravel-harness.readthedocs.io/
Apache License 2.0
133 stars 136 forks source link

Cannot simulate caravel module due to “`defaultnet_type none” and undeclared nets #3

Closed dan-rodrigues closed 3 years ago

dan-rodrigues commented 3 years ago

An example is this net in ring_osc2x13 which wasn’t explicitly declared, although there are many more. iverilog prints them all when attempting to build with the caravel module.

https://github.com/efabless/caravel/blob/4596e5f7a489ef9a31d71dd5ece7bdf3531ecbca/verilog/rtl/ring_osc2x13.v#L16

At the moment I’ve reverted the commit in my personal branch but am raising this issue as it’s up to the repo maintainers as to how/if the unconnected nets should be wired.

(I personally like using default_nettype none but was hesitant to include it my earlier wiring fix PR as I couldn't verify it at the time it by running the sim)

mattvenn commented 3 years ago

maybe I should have split this commit into the caravel tests and the wb tests. I only fixed the failing tests in the wb tests (by adding the missing signals).

Agree it would be good to know what to do with missing signals. I'm currently failing to run the soc simulations so I'm not making much progress with fixing up those tests.

mattvenn commented 3 years ago

damn there are a lot of missing signals.

mballance commented 3 years ago

I think a key question here is the right point for contributors to get involved. I happen to have forked the repo early enough that simulation does run for me, but I'm guessing that my fork is missing some key features needed for production. Are the primary developers looking for others to jump in and fix issues like this, or should we just hold off and wait for some level of stability before getting involved?

mattvenn commented 3 years ago

yes in retrospect I don't think this should have been merged.

RTimothyEdwards commented 3 years ago

@mballance : I am currently running through the testbench simulations after having cleaned up all the problems created by dropping the "defaultnet_type none" declaration everywhere.

RTimothyEdwards commented 3 years ago

@mballance : Should be fixed now, but since the "release" branch got frozen mid-day today, the fixes are currently in the new "develop" branch. There are some critical fixes in my last commit, so those will get merged into the release branch, hopefully soon, after review.

RTimothyEdwards commented 3 years ago

@mballance : All good to go? Shall I close this issue now?

mballance commented 3 years ago

@RTimothyEdwards, thanks, I'll certainly give the latest a try. Go ahead and close this for now and I'll raise a new issue if I have any problems with the latest changes you just committed.

Best Regards, Matthew

RTimothyEdwards commented 3 years ago

@mballance : Will do, thanks.