[clkmgr] Align behavior of jitter_enable CSR

Signed-off-by: Pirmin Vogel <[email protected]>

hw/ip_templates/clkmgr/data/clkmgr.hjson.tpl

@@ -409,8 +409,7 @@
           name: "EN",
           resval: "1"
           desc: '''
-            When 1, the value of !!JITTER_ENABLE can be changed.  When 0, writes have no
-            effect.
+            This register has no effect.
           '''
         },
       ]
@@ -429,13 +428,14 @@
           name: "VAL",
           desc: '''
             Enable jittery clock.
-            A value of kMultiBitBool4False disables the jittery clock,
-            while all other values enable jittery clock.
+            At reset, this register reads as kMultiBitBool4False and the jittery clock is disabled.
+            Any write to the register turns the value to kMultiBitBool4True and enables the jittery clock.
+            The value written doesn't matter.
+            The value then remains kMultiBitBool4True until reset.
           ''',
           resval: false
-          // avoid writing random values to this register as it could trigger transient checks
-          // in mubi sync
-          tags: ["excl:CsrAllTests:CsrExclWrite"]
+          // Exclude this register from any automated CSR tests as the behavior is non-standard.
+          tags: ["excl:CsrAllTests:CsrExclAll"]
         }
       ]
     },

hw/ip_templates/clkmgr/doc/registers.md.tpl

@@ -148,10 +148,10 @@ ${"###"} Fields
 {"reg": [{"name": "EN", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"bits": 31}], "config": {"lanes": 1, "fontsize": 10, "vspace": 80}}
 ```
 
-|  Bits  |  Type  |  Reset  | Name   | Description                                                                                            |
-|:------:|:------:|:-------:|:-------|:-------------------------------------------------------------------------------------------------------|
-|  31:1  |        |         |        | Reserved                                                                                               |
-|   0    |  rw0c  |   0x1   | EN     | When 1, the value of [`JITTER_ENABLE`](#jitter_enable) can be changed.  When 0, writes have no effect. |
+|  Bits  |  Type  |  Reset  | Name   | Description                  |
+|:------:|:------:|:-------:|:-------|:-----------------------------|
+|  31:1  |        |         |        | Reserved                     |
+|   0    |  rw0c  |   0x1   | EN     | This register has no effect. |
 
 ${"##"} JITTER_ENABLE
 Enable jittery clock
@@ -165,10 +165,16 @@ ${"###"} Fields
 {"reg": [{"name": "VAL", "bits": 4, "attr": ["rw"], "rotate": 0}, {"bits": 28}], "config": {"lanes": 1, "fontsize": 10, "vspace": 80}}
 ```
 
-|  Bits  |  Type  |  Reset  | Name   | Description                                                                                                                   |
-|:------:|:------:|:-------:|:-------|:------------------------------------------------------------------------------------------------------------------------------|
-|  31:4  |        |         |        | Reserved                                                                                                                      |
-|  3:0   |   rw   |   0x9   | VAL    | Enable jittery clock. A value of kMultiBitBool4False disables the jittery clock, while all other values enable jittery clock. |
+|  Bits  |  Type  |  Reset  | Name                       |
+|:------:|:------:|:-------:|:---------------------------|
+|  31:4  |        |         | Reserved                   |
+|  3:0   |   rw   |   0x9   | [VAL](#jitter_enable--val) |
+
+Enable jittery clock.
+At reset, this register reads as kMultiBitBool4False and the jittery clock is disabled.
+Any write to the register turns the value to kMultiBitBool4True and enables the jittery clock.
+The value written doesn't matter.
+The value then remains kMultiBitBool4True until reset.
 
 ${"##"} CLK_ENABLES
 Clock enable for software gateable clocks.

hw/ip_templates/clkmgr/dv/env/clkmgr_scoreboard.sv

@@ -290,7 +290,13 @@ class clkmgr_scoreboard extends cip_base_scoreboard #(
     // If incoming access is a write to a valid csr, update prediction right away.
     if (addr_phase_write) begin
       `uvm_info(`gfn, $sformatf("Writing 0x%0x to %s", item.a_data, csr.get_name()), UVM_MEDIUM)
-      void'(csr.predict(.value(item.a_data), .kind(UVM_PREDICT_WRITE), .be(item.a_mask)));
+      if (csr.get_name() == "jitter_enable") begin
+        // Any write to the jitter_enable CSR turns the value to MuBi4True. The value written
+        // doesn't matter.
+        void'(csr.predict(.value(prim_mubi_pkg::MuBi4True), .kind(UVM_PREDICT_WRITE)));
+      end else begin
+        void'(csr.predict(.value(item.a_data), .kind(UVM_PREDICT_WRITE), .be(item.a_mask)));
+      end
     end
 
     // Process the csr req:
@@ -318,8 +324,8 @@ class clkmgr_scoreboard extends cip_base_scoreboard #(
       "jitter_regwen": begin
       end
       "jitter_enable": begin
-        if (addr_phase_write && `gmv(ral.jitter_regwen)) begin
-          `DV_CHECK_EQ(prim_mubi_pkg::mubi4_t'(item.a_data), cfg.clkmgr_vif.jitter_enable_csr)
+        if (data_phase_write) begin
+          `DV_CHECK_EQ(prim_mubi_pkg::MuBi4True, cfg.clkmgr_vif.jitter_enable_csr)
         end
       end
       "clk_enables": begin

hw/ip_templates/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv

@@ -24,16 +24,13 @@ class clkmgr_smoke_vseq extends clkmgr_base_vseq;
   // This needs to be done outside the various CSR tests, since they update the jitter_enable
   // CSR, but the scoreboard is disabled for those tests.
   task test_jitter();
-    prim_mubi_pkg::mubi4_t jitter_value;
-    for (int i = 0; i < (1 << $bits(prim_mubi_pkg::mubi4_t)); ++i) begin
-      jitter_value = prim_mubi_pkg::mubi4_t'(i);
-      csr_wr(.ptr(ral.jitter_enable), .value(jitter_value));
-      csr_rd_check(.ptr(ral.jitter_enable), .compare_value(jitter_value));
-      // And set it back.
-      cfg.clk_rst_vif.wait_clks(6);
-      csr_wr(.ptr(ral.jitter_enable), .value('0));
-      csr_rd_check(.ptr(ral.jitter_enable), .compare_value('0));
-    end
+    // Write a random 32-bit value to the register. This will set it to enabled.
+    csr_wr(.ptr(ral.jitter_enable), .value($urandom()));
+    csr_rd_check(.ptr(ral.jitter_enable), .compare_value(prim_mubi_pkg::MuBi4True));
+    // And try to set it back - it must stay enabled until reset.
+    cfg.clk_rst_vif.wait_clks(6);
+    csr_wr(.ptr(ral.jitter_enable), .value(prim_mubi_pkg::MuBi4False));
+    csr_rd_check(.ptr(ral.jitter_enable), .compare_value(prim_mubi_pkg::MuBi4True));
   endtask
 
   // Flips all clk_enables bits from the reset value with all enabled. All is checked

hw/top_earlgrey/ip_autogen/clkmgr/data/clkmgr.hjson

@@ -414,8 +414,7 @@
           name: "EN",
           resval: "1"
           desc: '''
-            When 1, the value of !!JITTER_ENABLE can be changed.  When 0, writes have no
-            effect.
+            This register has no effect.
           '''
         },
       ]
@@ -434,13 +433,14 @@
           name: "VAL",
           desc: '''
             Enable jittery clock.
-            A value of kMultiBitBool4False disables the jittery clock,
-            while all other values enable jittery clock.
+            At reset, this register reads as kMultiBitBool4False and the jittery clock is disabled.
+            Any write to the register turns the value to kMultiBitBool4True and enables the jittery clock.
+            The value written doesn't matter.
+            The value then remains kMultiBitBool4True until reset.
           ''',
           resval: false
-          // avoid writing random values to this register as it could trigger transient checks
-          // in mubi sync
-          tags: ["excl:CsrAllTests:CsrExclWrite"]
+          // Exclude this register from any automated CSR tests as the behavior is non-standard.
+          tags: ["excl:CsrAllTests:CsrExclAll"]
         }
       ]
     },

hw/top_earlgrey/ip_autogen/clkmgr/doc/registers.md

@@ -148,10 +148,10 @@ Jitter write enable
 {"reg": [{"name": "EN", "bits": 1, "attr": ["rw0c"], "rotate": -90}, {"bits": 31}], "config": {"lanes": 1, "fontsize": 10, "vspace": 80}}
 ```
 
-|  Bits  |  Type  |  Reset  | Name   | Description                                                                                            |
-|:------:|:------:|:-------:|:-------|:-------------------------------------------------------------------------------------------------------|
-|  31:1  |        |         |        | Reserved                                                                                               |
-|   0    |  rw0c  |   0x1   | EN     | When 1, the value of [`JITTER_ENABLE`](#jitter_enable) can be changed.  When 0, writes have no effect. |
+|  Bits  |  Type  |  Reset  | Name   | Description                  |
+|:------:|:------:|:-------:|:-------|:-----------------------------|
+|  31:1  |        |         |        | Reserved                     |
+|   0    |  rw0c  |   0x1   | EN     | This register has no effect. |
 
 ## JITTER_ENABLE
 Enable jittery clock
@@ -165,10 +165,17 @@ Enable jittery clock
 {"reg": [{"name": "VAL", "bits": 4, "attr": ["rw"], "rotate": 0}, {"bits": 28}], "config": {"lanes": 1, "fontsize": 10, "vspace": 80}}
 ```
 
-|  Bits  |  Type  |  Reset  | Name   | Description                                                                                                                   |
-|:------:|:------:|:-------:|:-------|:------------------------------------------------------------------------------------------------------------------------------|
-|  31:4  |        |         |        | Reserved                                                                                                                      |
-|  3:0   |   rw   |   0x9   | VAL    | Enable jittery clock. A value of kMultiBitBool4False disables the jittery clock, while all other values enable jittery clock. |
+|  Bits  |  Type  |  Reset  | Name                       |
+|:------:|:------:|:-------:|:---------------------------|
+|  31:4  |        |         | Reserved                   |
+|  3:0   |   rw   |   0x9   | [VAL](#jitter_enable--val) |
+
+### JITTER_ENABLE . VAL
+Enable jittery clock.
+At reset, this register reads as kMultiBitBool4False and the jittery clock is disabled.
+Any write to the register turns the value to kMultiBitBool4True and enables the jittery clock.
+The value written doesn't matter.
+The value then remains kMultiBitBool4True until reset.
 
 ## CLK_ENABLES
 Clock enable for software gateable clocks.

hw/top_earlgrey/ip_autogen/clkmgr/dv/env/clkmgr_scoreboard.sv

@@ -290,7 +290,13 @@ class clkmgr_scoreboard extends cip_base_scoreboard #(
     // If incoming access is a write to a valid csr, update prediction right away.
     if (addr_phase_write) begin
       `uvm_info(`gfn, $sformatf("Writing 0x%0x to %s", item.a_data, csr.get_name()), UVM_MEDIUM)
-      void'(csr.predict(.value(item.a_data), .kind(UVM_PREDICT_WRITE), .be(item.a_mask)));
+      if (csr.get_name() == "jitter_enable") begin
+        // Any write to the jitter_enable CSR turns the value to MuBi4True. The value written
+        // doesn't matter.
+        void'(csr.predict(.value(prim_mubi_pkg::MuBi4True), .kind(UVM_PREDICT_WRITE)));
+      end else begin
+        void'(csr.predict(.value(item.a_data), .kind(UVM_PREDICT_WRITE), .be(item.a_mask)));
+      end
     end
 
     // Process the csr req:
@@ -318,8 +324,8 @@ class clkmgr_scoreboard extends cip_base_scoreboard #(
       "jitter_regwen": begin
       end
       "jitter_enable": begin
-        if (addr_phase_write && `gmv(ral.jitter_regwen)) begin
-          `DV_CHECK_EQ(prim_mubi_pkg::mubi4_t'(item.a_data), cfg.clkmgr_vif.jitter_enable_csr)
+        if (data_phase_write) begin
+          `DV_CHECK_EQ(prim_mubi_pkg::MuBi4True, cfg.clkmgr_vif.jitter_enable_csr)
         end
       end
       "clk_enables": begin

hw/top_earlgrey/ip_autogen/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv

@@ -24,16 +24,13 @@ class clkmgr_smoke_vseq extends clkmgr_base_vseq;
   // This needs to be done outside the various CSR tests, since they update the jitter_enable
   // CSR, but the scoreboard is disabled for those tests.
   task test_jitter();
-    prim_mubi_pkg::mubi4_t jitter_value;
-    for (int i = 0; i < (1 << $bits(prim_mubi_pkg::mubi4_t)); ++i) begin
-      jitter_value = prim_mubi_pkg::mubi4_t'(i);
-      csr_wr(.ptr(ral.jitter_enable), .value(jitter_value));
-      csr_rd_check(.ptr(ral.jitter_enable), .compare_value(jitter_value));
-      // And set it back.
-      cfg.clk_rst_vif.wait_clks(6);
-      csr_wr(.ptr(ral.jitter_enable), .value('0));
-      csr_rd_check(.ptr(ral.jitter_enable), .compare_value('0));
-    end
+    // Write a random 32-bit value to the register. This will set it to enabled.
+    csr_wr(.ptr(ral.jitter_enable), .value($urandom()));
+    csr_rd_check(.ptr(ral.jitter_enable), .compare_value(prim_mubi_pkg::MuBi4True));
+    // And try to set it back - it must stay enabled until reset.
+    cfg.clk_rst_vif.wait_clks(6);
+    csr_wr(.ptr(ral.jitter_enable), .value(prim_mubi_pkg::MuBi4False));
+    csr_rd_check(.ptr(ral.jitter_enable), .compare_value(prim_mubi_pkg::MuBi4True));
   endtask
 
   // Flips all clk_enables bits from the reset value with all enabled. All is checked

hw/top_earlgrey/ip_autogen/clkmgr/rtl/clkmgr_reg_top.sv

@@ -874,7 +874,7 @@ module clkmgr_reg_top (
 
     // from register interface
     .we     (jitter_enable_we),
-    .wd     (jitter_enable_wd),
+    .wd     (prim_mubi_pkg::MuBi4True),
 
     // from internal hardware
     .de     (1'b0),
@@ -2827,6 +2827,11 @@ module clkmgr_reg_top (
 
   // Unused signal tieoff
 
+  // Any write to the jitter_enable CSR writes MuBi4True.
+  // The actual write data is ignored.
+  logic unused_jitter_enable_wd;
+  assign unused_jitter_enable_wd = ^jitter_enable_wd;
+
   // wdata / byte enable are not always fully used
   // add a blanket unused statement to handle lint waivers
   logic unused_wdata;

sw/device/lib/dif/dif_clkmgr.c

@@ -88,8 +88,19 @@ dif_result_t dif_clkmgr_jitter_set_enabled(const dif_clkmgr_t *clkmgr,
     default:
       return kDifBadArg;
   }
-  mmio_region_write32(clkmgr->base_addr, CLKMGR_JITTER_ENABLE_REG_OFFSET,
-                      new_jitter_enable_val);
+
+  multi_bit_bool_t clk_jitter_enable_val =
+      mmio_region_read32(clkmgr->base_addr, CLKMGR_JITTER_ENABLE_REG_OFFSET);
+  if (clk_jitter_enable_val == kMultiBitBool4True &&
+      new_jitter_enable_val == kMultiBitBool4False) {
+    return kDifLocked;
+  }
+
+  if (new_jitter_enable_val == kMultiBitBool4True) {
+    mmio_region_write32(clkmgr->base_addr, CLKMGR_JITTER_ENABLE_REG_OFFSET,
+                        new_jitter_enable_val);
+  }
+
   return kDifOk;
 }
 

sw/device/lib/dif/dif_clkmgr.h

@@ -150,7 +150,7 @@ dif_result_t dif_clkmgr_jitter_get_enabled(const dif_clkmgr_t *clkmgr,
                                            dif_toggle_t *state);
 
 /**
- * Enable of Disable jitter.
+ * Enable or Disable jitter.
  * @param clkmgr Clock Manager Handle.
  * @param new_state whether to enable or disable jitter.
  * @returns The result of the operation.

sw/device/lib/dif/dif_clkmgr_unittest.cc

@@ -33,10 +33,11 @@ class JitterEnableTest : public ClkMgrTest {};
 // dif_clkmgr_jitter_set_enabled doesn't perform a read, just a write.
 TEST_F(JitterEnableTest, SetEnabled) {
   // Disable jitter.
-  EXPECT_WRITE32(CLKMGR_JITTER_ENABLE_REG_OFFSET, kMultiBitBool4False);
+  EXPECT_READ32(CLKMGR_JITTER_ENABLE_REG_OFFSET, kMultiBitBool4False);
   EXPECT_DIF_OK(dif_clkmgr_jitter_set_enabled(&clkmgr_, kDifToggleDisabled));
 
   // Enable jitter.
+  EXPECT_READ32(CLKMGR_JITTER_ENABLE_REG_OFFSET, kMultiBitBool4False);
   EXPECT_WRITE32(CLKMGR_JITTER_ENABLE_REG_OFFSET, kMultiBitBool4True);
   EXPECT_DIF_OK(dif_clkmgr_jitter_set_enabled(&clkmgr_, kDifToggleEnabled));
 }

util/reggen/fpv_csr.sv.tpl

@@ -86,6 +86,14 @@ module ${mod_base}_csr_assert_fpv import tlul_pkg::*;
   // accesses).
   bit [${hro_idx_width-1}:0] hro_idx;
   bit [${addr_msb}:0] normalized_addr;
+  % if lblock == "clkmgr":
+    % for hro_reg in hro_regs_list:
+      % if hro_reg.name.lower() == "jitter_enable":
+  // The jitter_enable CSR needs some special handling below.
+  localparam bit [${hro_idx_width-1}:0] JitterEnableHroIdx = ${hro_map.get(hro_reg.offset)[0]};
+      % endif
+    % endfor
+  % endif
 
   `ifdef FPV_ON
     // For FPV, we restrict the pend_trans array by giving its size a smaller upper bound. This
@@ -191,7 +199,14 @@ module ${mod_base}_csr_assert_fpv import tlul_pkg::*;
       if (d2h.d_valid) begin
         if (pend_trans[d_source_idx].wr_pending == 1) begin
           if (!d2h.d_error && regwen[hro_idx]) begin
+            % if lblock == "clkmgr":
+            if (hro_idx == JitterEnableHroIdx) begin
+              // Any write to the jitter_enable CSR writes MuBi4True.
+              exp_vals[hro_idx] <= prim_mubi_pkg::MuBi4True;
+            end else if (access_policy[hro_idx] == FpvRw0c) begin
+            % else:
             if (access_policy[hro_idx] == FpvRw0c) begin
+            % endif
               // Assume FpvWr0c policy only has one field that is wr0c.
               exp_vals[hro_idx] <= exp_vals[hro_idx][0] == 0 ? 0 : pend_trans[d_source_idx].wr_data;
             end else begin

util/reggen/reg_top.sv.tpl

@@ -824,6 +824,13 @@ ${rdata_gen(f, r.name.lower() + "_" + f.name.lower())}\
 % endif
 
   // Unused signal tieoff
+% if lblock == "clkmgr":
+
+  // Any write to the jitter_enable CSR writes MuBi4True.
+  // The actual write data is ignored.
+  logic unused_jitter_enable_wd;
+  assign unused_jitter_enable_wd = ^jitter_enable_wd;
+% endif
 % if rb.all_regs:
 
   // wdata / byte enable are not always fully used
@@ -904,13 +911,16 @@ ${bits.msb}\
       we_expr = f'{clk_base_name}{reg_name}{gated_suffix}_{we_suffix}'
 
       # when async, pick from the cdc handled data
-      wd_expr = f'{finst_name}_wd'
-      if reg.async_clk:
-        if field.bits.msb == field.bits.lsb:
-          bit_sel = f'{field.bits.msb}'
-        else:
-          bit_sel = f'{field.bits.msb}:{field.bits.lsb}'
-        wd_expr = f'{clk_base_name}{reg_name}_wdata[{bit_sel}]'
+      if lblock == "clkmgr" and reg_name == "jitter_enable":
+        wd_expr = "prim_mubi_pkg::MuBi4True"
+      else:
+        wd_expr = f'{finst_name}_wd'
+        if reg.async_clk:
+          if field.bits.msb == field.bits.lsb:
+            bit_sel = f'{field.bits.msb}'
+          else:
+            bit_sel = f'{field.bits.msb}:{field.bits.lsb}'
+          wd_expr = f'{clk_base_name}{reg_name}_wdata[{bit_sel}]'
 
     else:
       we_expr = "1'b0"