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

New split bus interface 2 / Verilator simulation support #146

Merged
merged 4 commits into from Dec 7, 2020
Merged

New split bus interface 2 / Verilator simulation support #146

merged 4 commits into from Dec 7, 2020

Conversation

flooklab
Copy link
Contributor

@flooklab flooklab commented Dec 4, 2020

Continue #145
Extend sbus support & add modifications to run simulation with Verilator.
The bus can be either fully split (use/add new modules using sbus)
or it can be also only split in the tb module (same old modules, no other changes required).

  • Adds simulation driver for split Basil bus.
  • Adds split bus tests for GPIO modules (for new "sbus GPIO module" and old "normal bus GPIO module").

To run with Verilator (and get .vcd output) one may currently add
mkfile += "EXTRA_ARGS += -Wno-UNSIGNED -Wno-WIDTH -Wno-WIDTHCONCAT --trace"
to the Makefile

@flooklab flooklab mentioned this pull request Dec 4, 2020
@codecov
Copy link

codecov bot commented Dec 4, 2020

Codecov Report

Merging #146 (34e3e0c) into master (e9542bc) will decrease coverage by 0.46%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #146      +/-   ##
==========================================
- Coverage   32.38%   31.92%   -0.47%     
==========================================
  Files          71       72       +1     
  Lines        4712     4780      +68     
==========================================
  Hits         1526     1526              
- Misses       3186     3254      +68     
Impacted Files Coverage Δ
basil/utils/sim/BasilBusDriver.py 0.00% <0.00%> (ø)
basil/utils/sim/BasilSbusDriver.py 0.00% <0.00%> (ø)
basil/utils/sim/SiLibUsbBusDriver.py 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9542bc...34e3e0c. Read the comment docs.

@themperek
Copy link
Member

Any idea of how we can reduce this repeating code tests/test_SimGpio_xxx.py maybe something like this: https://stackoverflow.com/questions/38729007/parametrize-class-tests-with-pytest#:~:text=From%20the%20docs%3A-,%40pytest.,test%20methods%20in%20the%20class.

@flooklab
Copy link
Contributor Author

flooklab commented Dec 4, 2020

Any idea of how we can reduce this repeating code tests/test_SimGpio_xxx.py maybe something like this: https://stackoverflow.com/questions/38729007/parametrize-class-tests-with-pytest#:~:text=From%20the%20docs%3A-,%40pytest.,test%20methods%20in%20the%20class.

The only line that changes is in the setUp() function. setUp cannot be parametrized like this.
I tried to inherit from the main test and override setUp instead. For some reason this does not work properly yet

Copy link
Member

@themperek themperek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea of how we can reduce this repeating code tests/test_SimGpio_xxx.py maybe something like this: https://stackoverflow.com/questions/38729007/parametrize-class-tests-with-pytest#:~:text=From%20the%20docs%3A-,%40pytest.,test%20methods%20in%20the%20class.

The only line that changes is in the setUp() function. setUp cannot be parametrized like this.
I tried to inherit from the main test and override setUp instead. For some reason this does not work properly yet

https://stackoverflow.com/questions/28695276/how-to-parameterize-python-unittest-setup-method ?

basil/firmware/modules/gpio/gpio.v Show resolved Hide resolved
basil/firmware/modules/utils/bus_to_ip.v Outdated Show resolved Hide resolved
yield RisingEdge(self.clock)
self.bus.ADD <= address + 0x4000
self.bus.BUS_DATA_IN <= int(value)
yield Timer(1) # This is hack for iverilog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be not needed?

Copy link
Contributor Author

@flooklab flooklab Dec 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes for sbus it (GPIO test) runs fine without hack with iverilog. For bus not.

basil/utils/sim/SiLibUsbSbusDriver.py Outdated Show resolved Hide resolved
examples/MIO_split_bus/ise/example.xise Outdated Show resolved Hide resolved
@flooklab
Copy link
Contributor Author

flooklab commented Dec 4, 2020

Any idea of how we can reduce this repeating code tests/test_SimGpio_xxx.py maybe something like this: https://stackoverflow.com/questions/38729007/parametrize-class-tests-with-pytest#:~:text=From%20the%20docs%3A-,%40pytest.,test%20methods%20in%20the%20class.

The only line that changes is in the setUp() function. setUp cannot be parametrized like this.
I tried to inherit from the main test and override setUp instead. For some reason this does not work properly yet

https://stackoverflow.com/questions/28695276/how-to-parameterize-python-unittest-setup-method ?

cab7212

@themperek
Copy link
Member

Thank you. Let it go in.
Can you squash all these commits to 2-3 that makes sense?
I can alternatively make 1 from GitHub GUI when merging.

@flooklab
Copy link
Contributor Author

flooklab commented Dec 7, 2020

Can you squash all these commits to 2-3 that makes sense?

I can have a look

@flooklab
Copy link
Contributor Author

flooklab commented Dec 7, 2020

Done @themperek

@themperek themperek merged commit af5e949 into SiLab-Bonn:master Dec 7, 2020
@themperek
Copy link
Member

It is suffering. Thank you!

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

2 participants