-
Notifications
You must be signed in to change notification settings - Fork 982
[alert,dv] Make the base driver explicit that it's just for alerts #29505
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: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,41 +1,41 @@ | ||
| // Copyright lowRISC contributors (OpenTitan project). | ||
| // Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
|
|
||
| // --------------------------------------------- | ||
| // Alert_esc_base driver | ||
| // --------------------------------------------- | ||
| class alert_esc_base_driver extends dv_base_driver#(alert_esc_seq_item, alert_esc_agent_cfg); | ||
| alert_esc_seq_item r_alert_ping_send_q[$], r_alert_rsp_q[$], r_esc_rsp_q[$], | ||
| s_alert_send_q[$], s_alert_ping_rsp_q[$]; | ||
| // Virtual base class used for alert_sender_driver and alert_receiver_driver | ||
|
|
||
| `uvm_component_utils(alert_esc_base_driver) | ||
| virtual class alert_base_driver extends dv_base_driver#(alert_esc_seq_item, alert_esc_agent_cfg); | ||
| alert_esc_seq_item r_alert_ping_send_q[$], r_alert_rsp_q[$], | ||
| s_alert_send_q[$], s_alert_ping_rsp_q[$]; | ||
|
|
||
| extern function new (string name, uvm_component parent); | ||
| // drive trans received from sequencer | ||
|
|
||
| // Take items from the sequencer and drive them. This implements a task from dv_base_driver. | ||
| extern virtual task get_and_drive(); | ||
| extern virtual task drive_req(); | ||
| extern virtual task get_req(); | ||
|
|
||
| endclass : alert_esc_base_driver | ||
| // Monitor seq_item_port and add the items to the various queues | ||
| // | ||
| // This is run by get_and_drive. | ||
| extern local task get_req(); | ||
|
|
||
| // Drive items that have been added to the various queues by get_req | ||
| // | ||
| // This should run forever and is started by get_and_drive. | ||
| pure virtual task drive_req(); | ||
| endclass | ||
|
|
||
| function alert_esc_base_driver::new (string name, uvm_component parent); | ||
| function alert_base_driver::new (string name, uvm_component parent); | ||
| super.new(name, parent); | ||
| endfunction : new | ||
|
|
||
| task alert_esc_base_driver::get_and_drive(); | ||
| task alert_base_driver::get_and_drive(); | ||
| fork | ||
| get_req(); | ||
| drive_req(); | ||
| join_none | ||
| endtask : get_and_drive | ||
| join | ||
| endtask | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: that follows one of our past discussion, I'd not remove the end of method labels as my preference would be to have it particularly when the rest of the class follows this |
||
|
|
||
| task alert_esc_base_driver::drive_req(); | ||
| `uvm_fatal(`gfn, "this is implemented as pure virtual task - please extend") | ||
| endtask : drive_req | ||
|
|
||
| task alert_esc_base_driver::get_req(); | ||
| task alert_base_driver::get_req(); | ||
| @(posedge cfg.vif.rst_n); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here isn't it best to point at a |
||
| forever begin | ||
| alert_esc_seq_item req_clone; | ||
|
|
@@ -45,12 +45,16 @@ task alert_esc_base_driver::get_req(); | |
| // receiver mode | ||
| if (req.r_alert_ping_send) r_alert_ping_send_q.push_back(req_clone); | ||
| if (req.r_alert_rsp) r_alert_rsp_q.push_back(req_clone); | ||
| if (req.r_esc_rsp) r_esc_rsp_q.push_back(req_clone); | ||
| // sender mode | ||
| if (req.s_alert_send) s_alert_send_q.push_back(req_clone); | ||
| if (req.s_alert_ping_rsp) s_alert_ping_rsp_q.push_back(req_clone); | ||
|
|
||
| if (req.r_esc_rsp) begin | ||
| `uvm_error(get_full_name(), "Alert driver cannot drive an esc item") | ||
| end | ||
|
|
||
| `uvm_info(`gfn, $sformatf({"Driver received item (after pushing): req.r_alert_ping_send=%0d", | ||
| " | req.r_alert_rsp=%0d | req.r_esc_rsp=%0d"}, | ||
| req.r_alert_ping_send, req.r_alert_rsp, req.r_esc_rsp), UVM_DEBUG) | ||
| " | req.r_alert_rsp=%0d"}, | ||
| req.r_alert_ping_send, req.r_alert_rsp), UVM_DEBUG) | ||
| end | ||
| endtask : get_req | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,54 +1,43 @@ | ||
| // Copyright lowRISC contributors (OpenTitan project). | ||
| // Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
|
|
||
| // --------------------------------------------- | ||
| // Esc sender driver | ||
| // --------------------------------------------- | ||
| class esc_sender_driver extends alert_esc_base_driver; | ||
| // Escalation sender driver | ||
|
|
||
| class esc_sender_driver extends dv_base_driver#(alert_esc_seq_item, alert_esc_agent_cfg); | ||
| `uvm_component_utils(esc_sender_driver) | ||
|
|
||
|
|
||
| extern function new (string name, uvm_component parent); | ||
| extern virtual task reset_signals(); | ||
| extern virtual task get_and_drive(); | ||
| extern virtual task drive_esc(); | ||
| extern virtual task do_reset(); | ||
|
|
||
| extern local task reset_esc(); | ||
| endclass : esc_sender_driver | ||
|
|
||
| function esc_sender_driver::new (string name, uvm_component parent); | ||
| super.new(name, parent); | ||
| endfunction : new | ||
|
|
||
| task esc_sender_driver::reset_signals(); | ||
| do_reset(); | ||
| wait(cfg.vif.rst_n !== 1'b1); | ||
| forever begin | ||
| @(negedge cfg.vif.rst_n); | ||
| under_reset = 1; | ||
| do_reset(); | ||
| @(posedge cfg.vif.rst_n); | ||
| reset_esc(); | ||
| wait(cfg.vif.rst_n === 1'b1); | ||
| under_reset = 0; | ||
|
|
||
| wait(cfg.vif.rst_n !== 1'b1); | ||
| end | ||
| endtask : reset_signals | ||
|
|
||
| task esc_sender_driver::get_and_drive(); | ||
| // LC_CTRL uses virtual interface to directly drive escalation requests. | ||
| // Other escalation handshakes are checked in prim_esc direct sequence. | ||
| // So the following task is not implemented. | ||
| drive_esc(); | ||
| endtask : get_and_drive | ||
|
|
||
| task esc_sender_driver::drive_esc(); | ||
| // LC_CTRL uses virtual interface to directly drive escalation requests. | ||
| // Other escalation handshakes are checked in prim_esc direct sequence. | ||
| // So this task is not implemented. | ||
| wait(!under_reset); | ||
| endtask : drive_esc | ||
| endtask : get_and_drive | ||
|
|
||
| task esc_sender_driver::do_reset(); | ||
| task esc_sender_driver::reset_esc(); | ||
| cfg.vif.esc_tx_int.esc_p <= 1'b0; | ||
| cfg.vif.esc_tx_int.esc_n <= 1'b1; | ||
| endtask : do_reset | ||
| endtask : reset_esc |
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 if we have a rule in our coding style, but I never declare multiple variables on the same line as it was forbidden in my previous company, and I think it's a good practice. The main reason was to avoid to change a signal type for one and which doesn't applies to the others (by negligence), diff are easier to track and we can add comments on the same line.
PS: I have checked and it's also part of Doulos guidelines -> look for "Have only one declaration or statement per line"