-
Notifications
You must be signed in to change notification settings - Fork 47
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
Cl/eth astral #115
Cl/eth astral #115
Conversation
* Bender.yml: Update dependencies * axi_vga: Bump version to include an essential performance update * Bender.yml: Switch to released version * doc: Add new parameters, fix typo --------- Co-authored-by: Paul Scheffler <paulsc@iis.ee.ethz.ch>
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.
Some comments here and there.
Makefile
Outdated
@@ -13,7 +13,7 @@ include cheshire.mk | |||
|
|||
# Inside the repo, forward (prefixed) all, nonfree, and clean targets | |||
all: | |||
@$(MAKE) chs-all | |||
@$(MAKE) chs-all |
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.
Remove trailing spaces.
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.
but actually they are aligned with the rest of the mk file
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 trailing space is after the chs-all
target/sim/src/vip_cheshire_soc.sv
Outdated
initial begin | ||
@(posedge clk); | ||
$readmemh("rx_mem_init.vmem", i_rx_axi_sim_mem.mem); | ||
|
||
@(posedge clk); | ||
reg_drv_rx.send_write( 'h0300c000, 32'h98001032, 'hf, reg_error); //lower 32bits of MAC address | ||
@(posedge clk); | ||
|
||
reg_drv_rx.send_write( 'h0300c004, 32'h00002070, 'hf, reg_error); //upper 16bits of MAC address + other configuration set to false/0 | ||
@(posedge clk); | ||
|
||
reg_drv_rx.send_write( 'h0300c014, 32'h0, 'hf, reg_error ); // SRC_ADDR | ||
@(posedge clk); | ||
|
||
reg_drv_rx.send_write( 'h0300c018, 32'h0, 'hf, reg_error); // DST_ADDR | ||
@(posedge clk); | ||
|
||
reg_drv_rx.send_write( 'h0300c01c, 32'h40,'hf , reg_error); // Size in bytes | ||
@(posedge clk); | ||
|
||
reg_drv_rx.send_write( 'h0300c020, 32'h5,'hf , reg_error); // src protocol | ||
@(posedge clk); | ||
|
||
reg_drv_rx.send_write( 'h0300c024, 32'h0,'hf , reg_error); // dst protocol | ||
@(posedge clk); | ||
|
||
reg_drv_rx.send_write( 'h0300c03c, 'h1, 'hf , reg_error); // req valid | ||
@(posedge clk); | ||
|
||
reg_drv_rx.send_write( 'h0300c044, 'h1, 'hf, reg_error); | ||
|
||
while(1) begin | ||
reg_drv_rx.send_read( 'h0300c048, rx_rsp_valid, reg_error); | ||
if(rx_rsp_valid) begin | ||
reg_drv_rx.send_write( 'h0300c044, 32'h0, 'hf , reg_error); | ||
@(posedge clk); | ||
break; | ||
end | ||
@(posedge clk); | ||
end | ||
end |
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.
Maybe there is a cleaner way to do this entire thing using the memory map of the register interface returned by this function, and adding the various offsets (taken from the reg_pkg) to it.
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.
agreed. it is super tedious... that functions seems a good idea.
@@ -12,7 +12,7 @@ set TESTBENCH tb_cheshire_soc | |||
# Set voptargs only if not already set to make overridable. | |||
# Default on fast simulation flags. | |||
if {![info exists VOPTARGS]} { | |||
set VOPTARGS "-O5 +acc=p+tb_cheshire_soc. +noacc=p+cheshire_soc. +acc=r+stream_xbar" | |||
set VOPTARGS "-O5 +acc=p+tb_cheshire_soc. +acc=p+cheshire_soc. +acc=r+stream_xbar" |
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?
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 needed to add visibility to debug...my bad to have it pushed
`ifdef USE_ETHERNET | ||
input wire eth_rxck, | ||
input wire [3:0] eth_rxd, | ||
input wire eth_rxctl, | ||
output wire eth_txck, | ||
output wire [3:0] eth_txd, | ||
output wire eth_txctl, | ||
output wire eth_rst_n, | ||
output wire eth_mdc, | ||
inout wire eth_mdio, | ||
`endif |
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 think there is something not proper here. If you want to use the define like that, you should propagate it to the toplevel cheshire_soc
and also delete the ethernet interface, the same way it is done in Carfield with the Hyperbus. In my opinion, the clean way to do it would be to declare the signals anyway, and depending on the USE_ETHERNET
definition or not, you assign them or bind them correctly to the default..
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 am not sure i get the problem here. this is xilinx top level for FPGA. It is a wrapper including cheshire_soc
. you can find all the defs for fpga here
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 know what it is, let's keep also this for a discussion early the upcoming week!
`ifdef USE_ETHERNET | ||
|
||
logic eth_mdio_i; | ||
logic eth_mdio_o; | ||
logic eth_mdio_oe; | ||
|
||
IOBUF #( | ||
.DRIVE ( 12 ), // Specify the output drive strength | ||
.IBUF_LOW_PWR ( "FALSE" ), // Low Power - "TRUE", High Performance = "FALSE" | ||
.IOSTANDARD ( "DEFAULT" ), // Specify the I/O standard | ||
.SLEW ( "FAST" ) // Specify the output slew rate | ||
) i_md_iobuf ( | ||
.O ( eth_mdio_i ), // Buffer output | ||
.IO ( eth_mdio ), // Buffer inout port (connect directly to top-level port) | ||
.I ( eth_mdio_o ), // Buffer input | ||
.T ( ~eth_mdio_oe ) // 3-state enable input, high=input, low=output | ||
); | ||
`endif |
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.
See the comment above for the define.
Fix a typo in the `reg_rsp_t` parameter. This is not very critical as it isn't used, but perhaps it can prevent future headache. Signed-off-by: Nils Wistoff <nwistoff@iis.ee.ethz.ch>
0126bd6
to
3a908f4
Compare
* target/sim: Add JTAG tasks to read/write 32b registers * target/sim: Add JTAG task to halt and load binary Can be used by platforms to halt CVA6 and preload a shared memory when execution happens on domains different than Cheshire. * target/sim: Clean up added tasks --------- Co-authored-by: Paul Scheffler <paulsc@iis.ee.ethz.ch>
1d4e401
to
1d16b41
Compare
3ba061a
to
903658b
Compare
Update CHANGELOG_WEEKLY
0970289
to
515bbac
Compare
Closing as abandoned. |
ethernet integration @yvantor