Skip to content

[chip, I2C, DV] Top level I2C host test#473

Open
KinzaQamar wants to merge 4 commits into
lowRISC:mainfrom
KinzaQamar:i2c_dv
Open

[chip, I2C, DV] Top level I2C host test#473
KinzaQamar wants to merge 4 commits into
lowRISC:mainfrom
KinzaQamar:i2c_dv

Conversation

@KinzaQamar

@KinzaQamar KinzaQamar commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Commands to test:
dvsim hw/top_chip/dv/top_chip_sim_cfg.hjson -i i2c_host_tx_rx -t xcelium -fs 1
dvsim hw/top_chip/dv/top_chip_sim_cfg.hjson -i i2c_host_tx_rx_cheri -t xcelium -fs 1

Close #507

@KinzaQamar KinzaQamar marked this pull request as draft April 27, 2026 11:46
@KinzaQamar KinzaQamar force-pushed the i2c_dv branch 3 times, most recently from fd42f2e to 0c033ce Compare April 28, 2026 13:40
@KinzaQamar KinzaQamar changed the title [I2C, dv] Initial I2C top level DV setup [chip, I2C, DV] Top level I2C host test Apr 28, 2026
@KinzaQamar KinzaQamar force-pushed the i2c_dv branch 2 times, most recently from dbb8d58 to 16ba0d4 Compare April 30, 2026 11:19
@KinzaQamar KinzaQamar force-pushed the i2c_dv branch 3 times, most recently from 4da05c4 to d5d15ad Compare May 7, 2026 15:03
@KinzaQamar KinzaQamar force-pushed the i2c_dv branch 10 times, most recently from f33a9e9 to 3a25ff6 Compare May 12, 2026 12:38
@KinzaQamar KinzaQamar assigned KinzaQamar and unassigned KinzaQamar May 12, 2026
@KinzaQamar KinzaQamar force-pushed the i2c_dv branch 4 times, most recently from d5393fb to a90365e Compare May 18, 2026 18:47
@KinzaQamar KinzaQamar force-pushed the i2c_dv branch 2 times, most recently from 6d98478 to e89e1dc Compare May 19, 2026 19:32
@KinzaQamar

Copy link
Copy Markdown
Contributor Author

Refer to the note about why host_test.c is failing on verilator507#issuecomment-4569471032). I've suggest few options but I need few opinions as well.

@KinzaQamar KinzaQamar force-pushed the i2c_dv branch 2 times, most recently from 1c9cec2 to 8987db7 Compare June 1, 2026 15:58
@KinzaQamar

Copy link
Copy Markdown
Contributor Author

Refer to the note about why host_test.c is failing on verilator507#issuecomment-4569471032). I've suggest few options but I need few opinions as well.

@marnovandermaas suggested to skip verilator and FPGA images for host_test

@elliotb-lowrisc elliotb-lowrisc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally looking decent to me. I have a few comments on making the SW test a bit clearer and in keeping with other tests

Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_tx_rx_test.c
Comment thread hw/top_chip/dv/tb/tb.sv Outdated
Comment thread sw/device/tests/CMakeLists.txt Outdated
Comment thread hw/top_chip/dv/tb/tb.sv
uvm_config_db#(virtual clk_rst_if)::set(null, "*", "sys_clk_if", sys_clk_if);
uvm_config_db#(virtual uart_if)::set(null, "*.env.m_uart_agent*", "vif", uart_if);
uvm_config_db#(virtual pins_if #(NUM_GPIOS))::set(null, "*.env", "gpio_vif", gpio_pins_if);
uvm_config_db#(virtual i2c_if)::set(null, "*.env.m_i2c_agent", "vif", i2c_if);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably best to add a wildcard as if a sub-component of i2c agent tries to get this handle it will fail. But it doesn't hurt us to give this possibility in case:

Suggested change
uvm_config_db#(virtual i2c_if)::set(null, "*.env.m_i2c_agent", "vif", i2c_if);
uvm_config_db#(virtual i2c_if)::set(null, "*.env.m_i2c_agent*", "vif", i2c_if);

Comment thread hw/top_chip/dv/env/top_chip_dv_env.sv Outdated
Comment thread hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv
Comment thread hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv
Comment thread hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv
@KinzaQamar KinzaQamar force-pushed the i2c_dv branch 5 times, most recently from 48917e0 to 0823c30 Compare June 3, 2026 23:26
Comment thread sw/device/lib/hal/i2c.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/lib/hal/i2c.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_tx_rx_test.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
@KinzaQamar

Copy link
Copy Markdown
Contributor Author

SW comments addressed

@ziuziakowska ziuziakowska left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a few nits.

Comment thread sw/device/tests/i2c/host_tx_rx_test.c Outdated
Comment thread sw/device/tests/i2c/host_test.c Outdated
Comment thread sw/device/tests/i2c/host_tx_rx_test.c Outdated
@KinzaQamar KinzaQamar mentioned this pull request Jun 7, 2026
@KinzaQamar

Copy link
Copy Markdown
Contributor Author

I've created #605 to address comments from @martin-velay and @elliotb-lowrisc on hal/i2c.c

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
Controller writes multiple bytes to the target's receiver and
then reads multiple bytes from the same address. At the end
of both read and write transfer, the test checks if the
transfer was succesful. If yes, then the test compares all
the read bytes with the data bytes that was sent as part of
write transfer.

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>

@elliotb-lowrisc elliotb-lowrisc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've run the latest changes locally and found the new test passes with multiple seeds and the changes don't seem to break any other UVM top-level tests.

I did spot one typo, but it's not worth holding back this PR over

Comment thread sw/device/tests/i2c/host_tx_rx_test.c Outdated
This vseq:
** reads the SW symbols to get the timing
   values
** calculates relevant timing parameter use by
   the agent to send Ack, Nack and Rdata.
** start the reactive sequence

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
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.

I2C DV -- Host TX-RX test

5 participants