-
Notifications
You must be signed in to change notification settings - Fork 16
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
Update OpenTitan IP blocks #259
Conversation
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.
Thanks for doing this import (I know it's a lot of fiddly work). Just a few comments from my side before we can merge this. Does this PR come with any RTOS driver visible changes?
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 did you remove this patch? is this integrity stuff no longer in the template? Have you tested this by running util/reg_gen.sh
?
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 reggen stuff wasn't in the repo when I started; rebased now.
.intr_tx_empty_o (uart_interrupts[i][8]), // Interrupt was appended. | ||
.intr_rx_watermark_o (uart_interrupts[i][1]), | ||
.intr_tx_empty_o (uart_interrupts[i][2]), | ||
.intr_tx_done_o (uart_interrupts[i][2]), |
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 did you not leave empty at index 2 and append done at 8?
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.
Given this isn't software visible, we're just ORing it all together, does it matter?
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've annotated it above the connections; the more elegant solution would be to reorder the output ports in uart.sv
but I didn't really want to do that. My concern in shuffling the indexes of uart_interrupts
was simply that somebody expanding it in a waveform viewer may be misled by assuming that the indexes match the interrupt numbers.
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.
LGTM
@@ -1,122 +0,0 @@ | |||
diff --git a/data/rv_plic.hjson.tpl b/data/rv_plic.hjson.tpl |
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 we still need this? As part of this PR we should re-generate the PLIC with the updated templates.
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.
Rebased and regenerated PLIC now; I've opted to leave reggen/templates such that it includes the OpenTitan project
reference and so this is present in the generated PLIC, but left the Sonata-specific RTL (which I haven't needed to update) without that reference in the copyright headers.
This does mean - for now - that the output from util/reg_gen.sh mismatches trivially, but it sounds as though there are other twiddles to be done also at a later date.
Vendor Earl Grey Prod IP blocks into Sonata and modify the set of patch files accordingly.
Vendor in new IP files with patches applied.
tlul_adapter_sram instances do not require the readback functionality; tie off the ports. Introduce Verilator warning waivers for split I2C FSM logic (i2c_fsm -> i2c_controller_fsm and i2c_target_fsm).
The updated I2C controller needs to see its own signals; detection of SCL/SDA interference now causes the controller to halt and return to Idle. Merge the outbound I2C signalling into that generated by the DPI models within the Verilator test bench.
This PR updates the vendored-in IP blocks to the versions used in Earl Grey production, updates the vendor patch files accordingly, and makes the necessary RTL changes to accommodate altered ports and interrupts.
tests/test_runner
passes in Verilator simulation with all devices/tests, and the FPGA build is timing clean; local testing has been performed on FPGA except for those I2C devices that are unavailable.