Skip to content

[I2C, DV] Refactoring / Simplifying functions in sequences#30066

Open
KinzaQamar wants to merge 2 commits intolowRISC:masterfrom
KinzaQamar:i2c_simplifications
Open

[I2C, DV] Refactoring / Simplifying functions in sequences#30066
KinzaQamar wants to merge 2 commits intolowRISC:masterfrom
KinzaQamar:i2c_simplifications

Conversation

@KinzaQamar
Copy link
Copy Markdown
Contributor

@KinzaQamar KinzaQamar commented May 7, 2026 •
edited
Loading

Refactoring in the sequences to avoid duplication and simplifies the way i2c_target_may_nack_seq randomizes ack and nack

@KinzaQamar KinzaQamar requested a review from a team as a code owner May 7, 2026 20:32
@KinzaQamar KinzaQamar requested review from elliotb-lowrisc, hcallahan-lowrisc, martin-velay and rswarbrick and removed request for a team May 7, 2026 20:32
@KinzaQamar KinzaQamar force-pushed the i2c_simplifications branch 3 times, most recently from 8724a91 to 88df9c5 Compare May 7, 2026 20:59
elliotb-lowrisc
elliotb-lowrisc reviewed May 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@elliotb-lowrisc elliotb-lowrisc left a comment

Choose a reason for hiding this comment

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

What's the rational for changing this? Looks like you lose the ability to distinguish between data and address responses, a feature that seems to be used by both the derivative sequences touched by this PR (though I'm not sure how essential or otherwise the distinction is).

@KinzaQamar
Copy link
Copy Markdown
Contributor Author

KinzaQamar commented May 8, 2026

What's the rational for changing this? Looks like you lose the ability to distinguish between data and address responses, a feature that seems to be used by both the derivative sequences touched by this PR (though I'm not sure how essential or otherwise the distinction is).

so, the base_seq use two methods as: drive_write_byte_ack() and drive_address_byte_ack and both are duplicate of each other so replace that with a single function. Although CI is failing so I'll mark this PR as draft and see what went wrong.

Thanks!

@KinzaQamar KinzaQamar marked this pull request as draft May 8, 2026 08:41
@KinzaQamar
Copy link
Copy Markdown
Contributor Author

KinzaQamar commented May 8, 2026

Converting it into draft as CI is failing

elliotb-lowrisc
elliotb-lowrisc suggested changes May 8, 2026
View reviewed changes
Comment thread hw/dv/sv/i2c_agent/seq_lib/i2c_device_response_seq.sv Outdated
@KinzaQamar
[I2C, DV] Use urandom_range() to simplify randomizing Ack / Nack
7ded8d5 
Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
@KinzaQamar KinzaQamar force-pushed the i2c_simplifications branch from 88df9c5 to 655b86e Compare May 8, 2026 14:40
@KinzaQamar
[I2C, DV] Refactoring the way ACKs / NACKs are driven
8efe888 
The motivation behind is to remove the duplication code.

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
@KinzaQamar KinzaQamar force-pushed the i2c_simplifications branch from 655b86e to 8efe888 Compare May 8, 2026 20:23
@KinzaQamar KinzaQamar requested a review from elliotb-lowrisc May 8, 2026 21:32
@KinzaQamar KinzaQamar marked this pull request as ready for review May 8, 2026 21:35
@KinzaQamar KinzaQamar changed the title [I2C, DV] Replace the duplicate functions with one [I2C, DV] Refactoring / Simplifying functions in sequences May 8, 2026
KinzaQamar
KinzaQamar commented May 8, 2026
View reviewed changes
Comment thread hw/dv/sv/i2c_agent/seq_lib/i2c_base_seq.sv
req.drv_type = DevAck;
`uvm_info(`gfn,
$sformatf("%sing the byte!", ((drv_type == DevAck) ? "ACK" : "NACK")),
UVM_LOW)
Copy link
Copy Markdown
Contributor Author

@KinzaQamar KinzaQamar May 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I now notices that this uvm_info will create alot of noise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hcallahan-lowrisc hcallahan-lowrisc Awaiting requested review from hcallahan-lowrisc hcallahan-lowrisc is a code owner automatically assigned from lowRISC/ot-dv-reviewers

@rswarbrick rswarbrick Awaiting requested review from rswarbrick

@martin-velay martin-velay Awaiting requested review from martin-velay

@elliotb-lowrisc elliotb-lowrisc Awaiting requested review from elliotb-lowrisc

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@KinzaQamar @elliotb-lowrisc