-
Notifications
You must be signed in to change notification settings - Fork 0
Adding a Verilog example using PicoSoC #39
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
base: main
Are you sure you want to change the base?
Conversation
b426930 to
f9a4d9c
Compare
|
I think it belongs here. Defining the iopins here is better as well. Let's not add a makefile - it was removed from the others. |
Seems like it wasn't? e.g. https://github.com/ChipFlow/chipflow-examples/blob/main/mcu_soc/Makefile |
f9a4d9c to
7d0876e
Compare
91ced06 to
8e2c516
Compare
91ced06 to
210cfbd
Compare
210cfbd to
e7dde63
Compare
e7dde63 to
e64bf92
Compare
|
Hopefully once ChipFlow/chipflow-lib#129 is in this is ready to go! |
e64bf92 to
90cb442
Compare
| base = os.path.dirname(__file__) | ||
|
|
||
| verilog_sources = [ | ||
| f"{base}/picosoc_asic_top.v", | ||
| f"{base}/picorv32/picosoc/spimemio.v", | ||
| f"{base}/picorv32/picosoc/simpleuart.v", | ||
| f"{base}/picorv32/picosoc/picosoc.v", | ||
| f"{base}/picorv32/picorv32.v", | ||
| ] | ||
|
|
||
| for verilog_file in verilog_sources: | ||
| with open(verilog_file, 'r') as f: | ||
| platform.add_file(verilog_file, f) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering, could we have a chipflow.toml section to include external code, or maybe use a seperate toml file for defining the import?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any progress on this?
| # Clock and reset | ||
| i_clk=ClockSignal(), | ||
| i_resetn=~ResetSignal(), | ||
|
|
||
| # UART | ||
| o_ser_tx=self.uart_0.tx.o, | ||
| i_ser_rx=self.uart_0.rx.i, | ||
|
|
||
| # SPI flash | ||
| o_flash_csb=self.flash.csn.o, | ||
| o_flash_clk=self.flash.clk.o, | ||
|
|
||
| o_flash_io0_oe=self.flash.d.oe[0], | ||
| o_flash_io1_oe=self.flash.d.oe[1], | ||
| o_flash_io2_oe=self.flash.d.oe[2], | ||
| o_flash_io3_oe=self.flash.d.oe[3], | ||
|
|
||
| o_flash_io0_do=self.flash.d.o[0], | ||
| o_flash_io1_do=self.flash.d.o[1], | ||
| o_flash_io2_do=self.flash.d.o[2], | ||
| o_flash_io3_do=self.flash.d.o[3], | ||
|
|
||
| i_flash_io0_di=self.flash.d.i[0], | ||
| i_flash_io1_di=self.flash.d.i[1], | ||
| i_flash_io2_di=self.flash.d.i[2], | ||
| i_flash_io3_di=self.flash.d.i[3], | ||
|
|
||
| # LEDs | ||
| o_leds=self.gpio_0.gpio.o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, might be something worth putting in a toml description?
6b8a786 to
1e3fed9
Compare
|
@robtaylor as "proper" Verilog support has turned into a bigger project, shall we at least get this merged in its current form (I've rebased and updated it, I don't think any of the other PRs should affect it) so we have some kind of Verilog example to point to? |
Yes @gatecat , sounds sensible to me. Can you rebase now software-rework has landed? |
b0c279e to
9a82105
Compare
Signed-off-by: gatecat <gatecat@ds0.me>
9a82105 to
8086479
Compare
|
Rebasing ended up a bit more complex than expected, but with ChipFlow/chipflow-lib#143, I think this should now be working |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't think this should be necessary now?
if the chipflow-lib impl doesnt do something you need, lets fix that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a ton of work to get software building working with Verilog, and I don't think everyone is even going to want this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough, see other comment..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the flashio/cmd_read_*/set_flash parts here be in the QSPIFlash driver?
Maybe fix up chipflow-lib if drivers need a way to put stuff in start.S?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The picosoc qspi flash core isn't compatible with the one in chipflow-digital-ip so different drivers is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, i see. this is fine for now then i guess, as you're overriding software build. We should have a bit of a session on making sure we can have design specific drivers work alongside imported drivers.
| } | ||
|
|
||
| void print_dec(uint32_t v) | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not something like:
do // run at least once to print 0
{
d = x % 10;
stack[sp++] = d + '0'; // push
x /= 10;
} while (x);
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, this was just taken from claire's picosoc code as-is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how odd... Maybe note its origin in a comment?
| base = os.path.dirname(__file__) | ||
|
|
||
| verilog_sources = [ | ||
| f"{base}/picosoc_asic_top.v", | ||
| f"{base}/picorv32/picosoc/spimemio.v", | ||
| f"{base}/picorv32/picosoc/simpleuart.v", | ||
| f"{base}/picorv32/picosoc/picosoc.v", | ||
| f"{base}/picorv32/picorv32.v", | ||
| ] | ||
|
|
||
| for verilog_file in verilog_sources: | ||
| with open(verilog_file, 'r') as f: | ||
| platform.add_file(verilog_file, f) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any progress on this?
|
Also seems this is using the long-deprecated PinSignature?? Please check you're working against a current chipflow-lib |
|
Hm, where is that, I thought I moved to using all the predefined signatures in chipflow-lib so sim auto-generation works, but maybe some old code got left in by mistake... |
|
I also don't touch pyproject.toml or pdm.lock in this PR so CI (where the tests pass, only the known issue of minimal timing out in the cloud builder causes a failure) should be using latest chipflow-lib already |
|
Maybe my local branch got messed up somehow, let me check it! |
Everything else is ready for review, although I have a couple of questions: