[dv,pwm] Tidying and commenting

Reformatting and tidying of source, including dropping an
unused file. No functional changes.

Signed-off-by: Adrian Lees <[email protected]>

Author: alees24

hw/dv/sv/pwm_monitor/pwm_monitor_cfg.sv

@@ -2,10 +2,10 @@
 // Licensed under the Apache License, Version 2.0, see LICENSE for details.
 // SPDX-License-Identifier: Apache-2.0
 
-class pwm_monitor_cfg  extends dv_base_agent_cfg;
+class pwm_monitor_cfg extends dv_base_agent_cfg;
 
   int monitor_id = 0;
-  bit invert     = 1'b0; // 0: active high,  1: active low
+  bit invert     = 1'b0; // 0: active high, 1: active low
   bit active     = 1'b0; // 1: valid configuration, collect items 0: ignore
   int resolution = 0;
   int clk_div    = 0;

hw/ip/pwm/dv/env/pwm_env.core

@@ -13,7 +13,6 @@ filesets:
       - lowrisc:dv:pwm_monitor
     files:
       - pwm_env_pkg.sv
-      - pwm_seq_cfg.sv: {is_include_file: true}
       - pwm_env_cfg.sv: {is_include_file: true}
       - pwm_env_cov.sv: {is_include_file: true}
       - pwm_virtual_sequencer.sv: {is_include_file: true}

hw/ip/pwm/dv/env/pwm_env.sv

@@ -11,16 +11,17 @@ class pwm_env extends cip_base_env #(
   `uvm_component_utils(pwm_env)
   `uvm_component_new
 
-  pwm_monitor           m_pwm_monitor[PWM_NUM_CHANNELS];
+  // One monitor for each channel; the monitors operate independently.
+  pwm_monitor m_pwm_monitor[PWM_NUM_CHANNELS];
 
   function void build_phase(uvm_phase phase);
     super.build_phase(phase);
 
-    // instantiate pwm_monitor
-    foreach(m_pwm_monitor[i]) begin
+    // Instantiate PWM monitors.
+    foreach (m_pwm_monitor[i]) begin
       m_pwm_monitor[i] = pwm_monitor::type_id::create($sformatf("m_pwm_monitor%0d", i), this);
       uvm_config_db#(pwm_monitor_cfg)::set(this, $sformatf("m_pwm_monitor%0d", i), "cfg",
-                                           cfg.m_pwm_monitor_cfg[i]);
+                                                           cfg.m_pwm_monitor_cfg[i]);
       cfg.m_pwm_monitor_cfg[i].ok_to_end_delay_ns = 2000;
     end
 
@@ -31,8 +32,9 @@ class pwm_env extends cip_base_env #(
     end
 
     cfg.clk_rst_core_vif.set_freq_mhz(cfg.get_clk_core_freq());
-    `uvm_info(`gfn, $sformatf("\n  env_cfg: bus_clk %0d Mhz, core_clk %0d Mhz",
-        cfg.clk_rst_vif.clk_freq_mhz, cfg.clk_rst_core_vif.clk_freq_mhz), UVM_DEBUG)
+    `uvm_info(`gfn, $sformatf("\n  env_cfg: bus_clk %0d MHz, core_clk %0d MHz",
+                              cfg.clk_rst_vif.clk_freq_mhz, cfg.clk_rst_core_vif.clk_freq_mhz),
+              UVM_DEBUG)
   endfunction : build_phase
 
   function void connect_phase(uvm_phase phase);

hw/ip/pwm/dv/env/pwm_env_cfg.sv

@@ -8,11 +8,12 @@ class pwm_env_cfg extends cip_base_env_cfg #(.RAL_T(pwm_reg_block));
 
   `uvm_object_new
 
-  // configs
-  pwm_monitor_cfg       m_pwm_monitor_cfg[PWM_NUM_CHANNELS];
+  // Configuration for each monitor in turn.
+  pwm_monitor_cfg    m_pwm_monitor_cfg[PWM_NUM_CHANNELS];
 
-  // virtual ifs
-  virtual clk_rst_if    clk_rst_core_vif;
+  // Interface to the core clock on which the DUT logic itself runs. This logic remains active
+  // even when the bus interface is in a low power state (TL-UL clock stopped).
+  virtual clk_rst_if clk_rst_core_vif;
 
   // A scaling used to convert from the clock frequency in clk_rst_vif (the bus clock) to the core
   // clock. This is a ratio, but scaled so 1.0 is represented by 1024. The scaling is constrained to
@@ -23,26 +24,28 @@ class pwm_env_cfg extends cip_base_env_cfg #(.RAL_T(pwm_reg_block));
   // Method from dv_base_env_cfg. Construct RAL models and fill in monitor configs.
   extern virtual function void initialize(bit [31:0] csr_base_addr = '1);
 
-  // Return the scaled frequency in MHz for the core
+  // Return the scaled core clock frequency in MHz.
   extern virtual function int get_clk_core_freq();
 endclass : pwm_env_cfg
 
 function void pwm_env_cfg::initialize(bit [31:0] csr_base_addr = '1);
   list_of_alerts = pwm_env_pkg::LIST_OF_ALERTS;
   super.initialize(csr_base_addr);
 
-  // create pwm_agent_cfg
+  // Set up the configuration for each of the monitors.
   foreach (m_pwm_monitor_cfg[i]) begin
     m_pwm_monitor_cfg[i] = pwm_monitor_cfg::type_id::create($sformatf("m_pwm_monitor%0d_cfg", i));
     m_pwm_monitor_cfg[i].if_mode = Device;
     m_pwm_monitor_cfg[i].is_active = 0;
+    // Monitors identify themselves and their sequence items, for diagnostic/debugging purposes.
     m_pwm_monitor_cfg[i].monitor_id = i;
   end
 
-  // only support 1 outstanding TL items in tlul_adapter
+  // Support only a single outstanding TL item in tlul_adapter
   m_tl_agent_cfg.max_outstanding_req = 1;
 endfunction
 
+// Return the scaled core clock frequency in MHz.
 function int pwm_env_cfg::get_clk_core_freq();
   real scaled = clk_rst_vif.clk_freq_mhz * clk_scale / 1024;
   `DV_CHECK_FATAL(clk_rst_vif.clk_freq_mhz > 0)

hw/ip/pwm/dv/env/pwm_env_pkg.sv

@@ -3,7 +3,6 @@
 // SPDX-License-Identifier: Apache-2.0
 
 package pwm_env_pkg;
-  // dep packages
   import uvm_pkg::*;
   import top_pkg::*;
   import dv_utils_pkg::*;
@@ -18,32 +17,33 @@ package pwm_env_pkg;
 
   parameter uint PWM_NUM_CHANNELS = pwm_reg_pkg::NOutputs;
 
-  // macro includes
   `include "uvm_macros.svh"
   `include "dv_macros.svh"
 
-  // parameters
-  // alerts
+  // The DUT has a single alert signal.
   parameter uint NUM_ALERTS = 1;
   parameter string LIST_OF_ALERTS[] = {"fatal_fault"};
+
+  // Constants.
   parameter bit [31:0] MAX_32 = 32'hFFFF_FFFF;
   parameter bit [15:0] MAX_16 = 16'hFFFF;
   parameter bit [26:0] MAX_27 = 27'h7FF_FFFF;
+
+  // Duration of test in core clock cycles.
   parameter uint NUM_CYCLES = 'd1_049_000;
-  parameter bit [26:0] MAX_CLK_DIV = 15;
 
-  // datatype
-  typedef enum bit {
-    Enable  = 1'b1,
-    Disable = 1'b0
-  } pwm_status_e;
+  // The DUT has a very flexible interface, clearly intended to support a wide range of bus and
+  // core clock frequencies; however, large values of `CLK_DIV` are impractical in simulation.
+  parameter bit [26:0] MAX_CLK_DIV = 15;
 
+  // Phase counter configuration (CFG).
   typedef struct packed {
     bit [26:0]   ClkDiv;
     bit [3:0]    DcResn;
     bit          CntrEn;
   } cfg_reg_t;
 
+  // Channel parameters (PWM_PARAM_i register).
   typedef struct packed {
     bit          BlinkEn;
     bit          HtbtEn;
@@ -62,14 +62,14 @@ package pwm_env_pkg;
     bit [15:0]   X;
   } blink_param_t;
 
-  // the index of multi-reg is at the last char of the name
+  // The index of a multi-reg is given by the last character of the name, since there are fewer
+  // than ten channels.
   function automatic int get_multireg_idx(string name);
     string s = name.getc(name.len - 1);
     return s.atoi();
   endfunction
 
-  // package sources
-  `include "pwm_seq_cfg.sv"
+  // Package sources
   `include "pwm_env_cfg.sv"
   `include "pwm_env_cov.sv"
   `include "pwm_virtual_sequencer.sv"