[clkmgr] Allow multiple hint clocks in a block
There are several parts to this patch, which all have to be done at
once (because the design is broken if you just do some of them).
1. Rename the hints in hint_names_e to include the clock name as
well as the endpoint. This is needed because the "otbn" endpoint
has two different clocks with hints, which need different enum
entries.
2. Also, move the code that chooses these hint names to a method on
the TypedClocks class, so that other code can use it too and be
guaranteed to get the same results.
3. Use this code to determine the iteration order when connecting up
idle signals in merge.py. This (finally!) lets us remove the hack
that we were using to ensure each block only contributed one idle
signal.
4. Add a new "idle_otp_o" signal to OTBN. Wire it up properly and
add placeholder documentation stubs.
5. Teach the clkmgr DV code about the new clock. This is slightly
more work than "just add another entry to the list" because the
new output clock has io_div4 as a source clock, rather than main,
which was used for all the others. (Guillermo helped with the
changes here).
Co-authored-by: Guillermo Maturana <maturana@google.com>
Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
diff --git a/hw/ip/clkmgr/data/clkmgr.hjson.tpl b/hw/ip/clkmgr/data/clkmgr.hjson.tpl
index 98da908..a2d5c95 100644
--- a/hw/ip/clkmgr/data/clkmgr.hjson.tpl
+++ b/hw/ip/clkmgr/data/clkmgr.hjson.tpl
@@ -223,7 +223,7 @@
swaccess: "rw",
hwaccess: "hro",
fields: [
-% for clk in hint_clks:
+% for clk in hint_names.keys():
{
bits: "${loop.index}",
name: "${clk.upper()}_HINT",
@@ -249,7 +249,7 @@
swaccess: "ro",
hwaccess: "hwo",
fields: [
-% for clk in hint_clks.keys():
+% for clk in hint_names.keys():
{
bits: "${loop.index}",
name: "${clk.upper()}_VAL",
diff --git a/hw/ip/clkmgr/data/clkmgr.sv.tpl b/hw/ip/clkmgr/data/clkmgr.sv.tpl
index 4f7ed09..482c342 100644
--- a/hw/ip/clkmgr/data/clkmgr.sv.tpl
+++ b/hw/ip/clkmgr/data/clkmgr.sv.tpl
@@ -340,54 +340,49 @@
// clock target
////////////////////////////////////////////////////
-% for k in hint_clks.keys():
- logic ${k}_hint;
- logic ${k}_en;
+% for clk in hint_clks.keys():
+ logic ${clk}_hint;
+ logic ${clk}_en;
% endfor
-% for k, sig in hint_clks.items():
-<%
- ## Hint clocks should be connected to exactly one endpoint
- eps = list(set(ep_name for ep_name, ep_port in sig.endpoints))
- assert len(eps) == 1
-%>\
- assign ${k}_en = ${k}_hint | ~idle_i[${eps[0].capitalize()}];
+% for clk, sig in hint_clks.items():
+ assign ${clk}_en = ${clk}_hint | ~idle_i[${hint_names[clk]}];
prim_flop_2sync #(
.Width(1)
- ) u_${k}_hint_sync (
+ ) u_${clk}_hint_sync (
.clk_i(clk_${sig.src.name}_i),
.rst_ni(rst_${sig.src.name}_ni),
- .d_i(reg2hw.clk_hints.${k}_hint.q),
- .q_o(${k}_hint)
+ .d_i(reg2hw.clk_hints.${clk}_hint.q),
+ .q_o(${clk}_hint)
);
- lc_tx_t ${k}_scanmode;
+ lc_tx_t ${clk}_scanmode;
prim_lc_sync #(
.NumCopies(1),
.AsyncOn(0)
- ) u_${k}_scanmode_sync (
+ ) u_${clk}_scanmode_sync (
.clk_i(1'b0), //unused
.rst_ni(1'b1), //unused
.lc_en_i(scanmode_i),
- .lc_en_o(${k}_scanmode)
+ .lc_en_o(${clk}_scanmode)
);
prim_clock_gating #(
.NoFpgaGate(1'b1)
- ) u_${k}_cg (
+ ) u_${clk}_cg (
.clk_i(clk_${sig.src.name}_root),
- .en_i(${k}_en & clk_${sig.src.name}_en),
- .test_en_i(${k}_scanmode == lc_ctrl_pkg::On),
- .clk_o(clocks_o.${k})
+ .en_i(${clk}_en & clk_${sig.src.name}_en),
+ .test_en_i(${clk}_scanmode == lc_ctrl_pkg::On),
+ .clk_o(clocks_o.${clk})
);
% endfor
// state readback
-% for k in hint_clks.keys():
- assign hw2reg.clk_hints_status.${k}_val.de = 1'b1;
- assign hw2reg.clk_hints_status.${k}_val.d = ${k}_en;
+% for clk in hint_clks.keys():
+ assign hw2reg.clk_hints_status.${clk}_val.de = 1'b1;
+ assign hw2reg.clk_hints_status.${clk}_val.d = ${clk}_en;
% endfor
assign jitter_en_o = reg2hw.jitter_enable.q;
diff --git a/hw/ip/clkmgr/data/clkmgr_pkg.sv.tpl b/hw/ip/clkmgr/data/clkmgr_pkg.sv.tpl
index f59ab81..e6b10ac 100644
--- a/hw/ip/clkmgr/data/clkmgr_pkg.sv.tpl
+++ b/hw/ip/clkmgr/data/clkmgr_pkg.sv.tpl
@@ -11,21 +11,13 @@
package clkmgr_pkg;
typedef enum int {
-% for idx, blk_name in list(enumerate(hint_blocks)):
- ${blk_name.capitalize()} = ${idx}${"," if not loop.last else ""}
+% for idx, hint_name in list(enumerate(hint_names.values())):
+ ${hint_name} = ${idx}${"," if not loop.last else ""}
% endfor
} hint_names_e;
typedef struct packed {
-<%
-# Merge Clock Dicts together
-all_clocks = OrderedDict()
-all_clocks.update(ft_clks)
-all_clocks.update(hint_clks)
-all_clocks.update(rg_clks)
-all_clocks.update(sw_clks)
-%>\
-% for clk in all_clocks:
+% for clk in all_clks:
logic ${clk};
% endfor
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_env_cfg.sv b/hw/ip/clkmgr/dv/env/clkmgr_env_cfg.sv
index 0796081..8b7f3a9 100644
--- a/hw/ip/clkmgr/dv/env/clkmgr_env_cfg.sv
+++ b/hw/ip/clkmgr/dv/env/clkmgr_env_cfg.sv
@@ -13,6 +13,16 @@
virtual clk_rst_if usb_clk_rst_vif;
virtual clk_rst_if aon_clk_rst_vif;
+ // A map from trans clock to its source clock (giving the signal that will appear on this clock
+ // output if it's enabled).
+ src_e trans_to_src [int] = '{
+ TransAes: MainSrc,
+ TransHmac: MainSrc,
+ TransKmac: MainSrc,
+ TransOtbnIoDiv4: IoDiv4Src,
+ TransOtbnMain: MainSrc
+ };
+
`uvm_object_utils_begin(clkmgr_env_cfg)
`uvm_object_utils_end
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_env_pkg.sv b/hw/ip/clkmgr/dv/env/clkmgr_env_pkg.sv
index fe1e576..aa9f06f 100644
--- a/hw/ip/clkmgr/dv/env/clkmgr_env_pkg.sv
+++ b/hw/ip/clkmgr/dv/env/clkmgr_env_pkg.sv
@@ -27,7 +27,7 @@
// parameters
localparam int NUM_PERI = 4;
- localparam int NUM_TRANS = 4;
+ localparam int NUM_TRANS = 5;
// alerts
parameter uint NUM_ALERTS = 1;
@@ -36,7 +36,8 @@
// types
// The enum values for these match the bit order in the CSRs.
typedef enum int {PeriDiv4, PeriDiv2, PeriIo, PeriUsb} peri_e;
- typedef enum int {TransAes, TransHmac, TransKmac, TransOtbn} trans_e;
+ typedef enum int {TransAes, TransHmac, TransKmac, TransOtbnIoDiv4, TransOtbnMain} trans_e;
+ typedef enum int {MainSrc, IoDiv4Src} src_e;
// functions
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_if.sv b/hw/ip/clkmgr/dv/env/clkmgr_if.sv
index 395d40b..4641b2d 100644
--- a/hw/ip/clkmgr/dv/env/clkmgr_if.sv
+++ b/hw/ip/clkmgr/dv/env/clkmgr_if.sv
@@ -45,7 +45,8 @@
} clk_enables_t;
typedef struct packed {
- logic otbn;
+ logic otbn_main;
+ logic otbn_io_div4;
logic kmac;
logic hmac;
logic aes;
@@ -136,16 +137,21 @@
// Pipelines and clocking blocks for peripheral clocks.
logic [PIPELINE_DEPTH-1:0] clk_enable_div4_ffs;
+ logic [PIPELINE_DEPTH-1:0] clk_hint_otbn_div4_ffs;
logic [PIPELINE_DEPTH-1:0] ip_clk_en_div4_ffs;
always @(posedge clocks_o.clk_io_div4_powerup) begin
if (rst_n) begin
clk_enable_div4_ffs <= {clk_enable_div4_ffs[PIPELINE_DEPTH-2:0], clk_enables.io_div4_peri_en};
+ clk_hint_otbn_div4_ffs <= {clk_hint_otbn_div4_ffs[PIPELINE_DEPTH-2:0],
+ clk_hints[TransOtbnIoDiv4]};
ip_clk_en_div4_ffs <= {ip_clk_en_div4_ffs[PIPELINE_DEPTH-2:0], pwr_i.ip_clk_en};
end
end
clocking peri_div4_cb @(posedge clocks_o.clk_io_div4_powerup);
input ip_clk_en = ip_clk_en_div4_ffs[PIPELINE_DEPTH-1];
input clk_enable = clk_enable_div4_ffs[PIPELINE_DEPTH-1];
+ input clk_hint_otbn = clk_hint_otbn_div4_ffs[PIPELINE_DEPTH-1];
+ input otbn_idle = idle_i[TransOtbnIoDiv4];
endclocking
logic [PIPELINE_DEPTH-1:0] clk_enable_div2_ffs;
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_scoreboard.sv b/hw/ip/clkmgr/dv/env/clkmgr_scoreboard.sv
index 03f0832..ec1d5ee 100644
--- a/hw/ip/clkmgr/dv/env/clkmgr_scoreboard.sv
+++ b/hw/ip/clkmgr/dv/env/clkmgr_scoreboard.sv
@@ -111,7 +111,6 @@
monitor_div2_peri_clock();
monitor_io_peri_clock();
monitor_usb_peri_clock();
-
for (int i = 0; i < NUM_TRANS; ++i) begin
fork
automatic int trans_index = i;
@@ -195,13 +194,32 @@
endtask
task monitor_trans_clock(int trans_index);
- forever @cfg.clkmgr_vif.trans_cb begin
- logic hint = cfg.clkmgr_vif.trans_cb.clk_hints[trans_index];
- logic idle = cfg.clkmgr_vif.trans_cb.idle_i[trans_index];
- logic clk_en = cfg.clkmgr_vif.trans_cb.ip_clk_en;
- logic scan_en = cfg.clkmgr_vif.scanmode_i == lc_ctrl_pkg::On;
- logic gating_condition = (hint || !idle) && clk_en || scan_en;
- trans_e trans = trans_e'(trans_index);
+ trans_e trans = trans_e'(trans_index);
+ src_e src = cfg.trans_to_src[trans];
+
+ forever begin
+ logic hint, idle, clk_en, scan_en, gating_condition;
+
+ // Wait for the correct clocking block (to ensure that we sample when the output clock should
+ // be high if enabled), then read the relevant signals from that clocking block.
+ case (src)
+ MainSrc: begin
+ @(cfg.clkmgr_vif.trans_cb);
+ hint = cfg.clkmgr_vif.trans_cb.clk_hints[trans_index];
+ idle = cfg.clkmgr_vif.trans_cb.idle_i[trans_index];
+ clk_en = cfg.clkmgr_vif.trans_cb.ip_clk_en;
+ end
+ IoDiv4Src: begin
+ @(cfg.clkmgr_vif.peri_div4_cb);
+ hint = cfg.clkmgr_vif.peri_div4_cb.clk_hint_otbn;
+ idle = cfg.clkmgr_vif.peri_div4_cb.otbn_idle;
+ clk_en = cfg.clkmgr_vif.peri_div4_cb.ip_clk_en;
+ end
+ endcase
+
+ scan_en = cfg.clkmgr_vif.scanmode_i == lc_ctrl_pkg::On;
+ gating_condition = (hint || !idle) && clk_en || scan_en;
+
#0;
case (trans)
TransAes: begin
@@ -213,7 +231,10 @@
TransKmac: begin
check_clock(trans.name(), gating_condition, cfg.clkmgr_vif.clocks_o.clk_main_kmac);
end
- TransOtbn: begin
+ TransOtbnIoDiv4: begin
+ check_clock(trans.name(), gating_condition, cfg.clkmgr_vif.clocks_o.clk_io_div4_otbn);
+ end
+ TransOtbnMain: begin
check_clock(trans.name(), gating_condition, cfg.clkmgr_vif.clocks_o.clk_main_otbn);
end
endcase
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv
index 113cb64..75db6cb 100644
--- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv
+++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv
@@ -47,7 +47,8 @@
'{TransAes, ral.clk_hints.clk_main_aes_hint, ral.clk_hints_status.clk_main_aes_val},
'{TransHmac, ral.clk_hints.clk_main_hmac_hint, ral.clk_hints_status.clk_main_hmac_val},
'{TransKmac, ral.clk_hints.clk_main_kmac_hint, ral.clk_hints_status.clk_main_kmac_val},
- '{TransAes, ral.clk_hints.clk_main_otbn_hint, ral.clk_hints_status.clk_main_otbn_val}
+ '{TransOtbnIoDiv4, ral.clk_hints.clk_io_div4_otbn_hint, ral.clk_hints_status.clk_io_div4_otbn_val},
+ '{TransOtbnMain, ral.clk_hints.clk_main_otbn_hint, ral.clk_hints_status.clk_main_otbn_val}
};
idle = 0;
cfg.clkmgr_vif.update_idle(idle);
diff --git a/hw/ip/otbn/data/otbn.hjson b/hw/ip/otbn/data/otbn.hjson
index 831911d..008f7fd 100644
--- a/hw/ip/otbn/data/otbn.hjson
+++ b/hw/ip/otbn/data/otbn.hjson
@@ -5,7 +5,7 @@
clocking: [
{clock: "clk_i", reset: "rst_ni", idle: "idle_o", primary: true},
{clock: "clk_edn_i", reset: "rst_edn_ni", idle: "idle_o"},
- {clock: "clk_otp_i", reset: "rst_otp_ni", idle: "idle_o"}
+ {clock: "clk_otp_i", reset: "rst_otp_ni", idle: "idle_otp_o"}
]
bus_interfaces: [
{ protocol: "tlul", direction: "device" }
@@ -97,13 +97,20 @@
package: "edn_pkg"
},
- // OTBN is not performing any operation and can be clock/power-gated.
+ // OTBN is not performing any operation and can be clock/power-gated. One
+ // idle for each clock domain (see assignments in "clocking" dictionary above).
{ name: "idle",
type: "uni",
struct: "logic",
width: "1",
act: "req",
},
+ { name: "idle_otp",
+ type: "uni",
+ struct: "logic",
+ width: "1",
+ act: "req",
+ },
// ram configuration
{ struct: "ram_1p_cfg",
diff --git a/hw/ip/otbn/doc/_index.md b/hw/ip/otbn/doc/_index.md
index d5fe906..6a004f5 100644
--- a/hw/ip/otbn/doc/_index.md
+++ b/hw/ip/otbn/doc/_index.md
@@ -573,10 +573,16 @@
### Idle
OTBN exposes a single-bit `idle_o` signal, intended to be used by the clock manager to clock-gate the block when it is not in use.
-This signal is high when OTBN is not running.
+This signal is in the same clock domain as `clk_i`.
+It is high when OTBN is not running.
The cycle after a write to {{< regref "CMD.start" >}}, the signal goes low.
This remains low until the end of the operation (either from an {{< otbnInsnRef "ECALL" >}}) or an error, at which point it goes high again.
+OTBN also exposes another version of the idle signal as `idle_otp_o`.
+This works analogously, but is in the same clock domain as `clk_otp_i`.
+
+TODO: Specify interactions between `idle_o`, `idle_otp_o` and the clock manager fully.
+
### Data Integrity Protection {#design-details-data-integrity-protection}
OTBN stores and operates on data (state) in its dedicated memories, register files, and internal registers.
diff --git a/hw/ip/otbn/dv/uvm/tb.sv b/hw/ip/otbn/dv/uvm/tb.sv
index 1d536dc..36bad48 100644
--- a/hw/ip/otbn/dv/uvm/tb.sv
+++ b/hw/ip/otbn/dv/uvm/tb.sv
@@ -87,6 +87,8 @@
.tl_o(tl_if.d2h),
.idle_o(idle),
+ // TODO: Once this signal's behaviour is specified, check that it behaves as we expect.
+ .idle_otp_o (),
.intr_done_o(intr_done),
diff --git a/hw/ip/otbn/rtl/otbn.sv b/hw/ip/otbn/rtl/otbn.sv
index eafd3ab..b54cf97 100644
--- a/hw/ip/otbn/rtl/otbn.sv
+++ b/hw/ip/otbn/rtl/otbn.sv
@@ -32,6 +32,7 @@
// Inter-module signals
output logic idle_o,
+ output logic idle_otp_o,
// Interrupts
output logic intr_done_o,
@@ -111,6 +112,10 @@
// register interface?
assign idle_o = ~busy_q;
+ // TODO: These two signals aren't technically in the same clock domain. Sort out how we do the
+ // signalling properly.
+ assign idle_otp_o = idle_o;
+
// Lifecycle ==================================================================
lc_ctrl_pkg::lc_tx_t lc_escalate_en;
@@ -854,6 +859,7 @@
`ASSERT_KNOWN(TlODValidKnown_A, tl_o.d_valid)
`ASSERT_KNOWN(TlOAReadyKnown_A, tl_o.a_ready)
`ASSERT_KNOWN(IdleOKnown_A, idle_o)
+ `ASSERT_KNOWN(IdleOtpOKnown_A, idle_otp_o)
`ASSERT_KNOWN(IntrDoneOKnown_A, intr_done_o)
`ASSERT_KNOWN(AlertTxOKnown_A, alert_tx_o)
`ASSERT_KNOWN(EdnRndOKnown_A, edn_rnd_o)
diff --git a/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson b/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson
index a9217c6..a89a8dc 100644
--- a/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson
+++ b/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson
@@ -2656,8 +2656,8 @@
inst_name: clkmgr_aon
default: ""
package: ""
- end_idx: 4
- top_type: partial-one-to-N
+ end_idx: -1
+ top_type: one-to-N
top_signame: clkmgr_aon_idle
index: -1
}
@@ -5496,6 +5496,18 @@
default: ""
package: ""
top_signame: clkmgr_aon_idle
+ index: 4
+ }
+ {
+ name: idle_otp
+ struct: logic
+ type: uni
+ act: req
+ width: 1
+ inst_name: otbn
+ default: ""
+ package: ""
+ top_signame: clkmgr_aon_idle
index: 3
}
{
@@ -6760,6 +6772,7 @@
aes.idle
hmac.idle
kmac.idle
+ otbn.idle_otp
otbn.idle
]
pinmux_aon.lc_jtag:
@@ -14600,8 +14613,8 @@
inst_name: clkmgr_aon
default: ""
package: ""
- end_idx: 4
- top_type: partial-one-to-N
+ end_idx: -1
+ top_type: one-to-N
top_signame: clkmgr_aon_idle
index: -1
}
@@ -16250,6 +16263,18 @@
default: ""
package: ""
top_signame: clkmgr_aon_idle
+ index: 4
+ }
+ {
+ name: idle_otp
+ struct: logic
+ type: uni
+ act: req
+ width: 1
+ inst_name: otbn
+ default: ""
+ package: ""
+ top_signame: clkmgr_aon_idle
index: 3
}
{
@@ -18780,7 +18805,7 @@
signame: clkmgr_aon_idle
width: 5
type: uni
- end_idx: 4
+ end_idx: -1
act: rcv
suffix: ""
default: "'0"
diff --git a/hw/top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson b/hw/top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson
index d0a2d86..b467e1c 100644
--- a/hw/top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson
+++ b/hw/top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson
@@ -297,15 +297,6 @@
}
{
bits: "3",
- name: "CLK_MAIN_OTBN_HINT",
- resval: 1,
- desc: '''
- 0 CLK_MAIN_OTBN can be disabled.
- 1 CLK_MAIN_OTBN is enabled.
- '''
- }
- {
- bits: "4",
name: "CLK_IO_DIV4_OTBN_HINT",
resval: 1,
desc: '''
@@ -313,6 +304,15 @@
1 CLK_IO_DIV4_OTBN is enabled.
'''
}
+ {
+ bits: "4",
+ name: "CLK_MAIN_OTBN_HINT",
+ resval: 1,
+ desc: '''
+ 0 CLK_MAIN_OTBN can be disabled.
+ 1 CLK_MAIN_OTBN is enabled.
+ '''
+ }
]
// the CLK_HINT register cannot be written, otherwise there is the potential clocks could be
// disabled and the system will hang
@@ -357,15 +357,6 @@
}
{
bits: "3",
- name: "CLK_MAIN_OTBN_VAL",
- resval: 1,
- desc: '''
- 0 CLK_MAIN_OTBN is disabled.
- 1 CLK_MAIN_OTBN is enabled.
- '''
- }
- {
- bits: "4",
name: "CLK_IO_DIV4_OTBN_VAL",
resval: 1,
desc: '''
@@ -373,6 +364,15 @@
1 CLK_IO_DIV4_OTBN is enabled.
'''
}
+ {
+ bits: "4",
+ name: "CLK_MAIN_OTBN_VAL",
+ resval: 1,
+ desc: '''
+ 0 CLK_MAIN_OTBN is disabled.
+ 1 CLK_MAIN_OTBN is enabled.
+ '''
+ }
]
},
]
diff --git a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr.sv b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr.sv
index 793d5a3..3155082 100644
--- a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr.sv
+++ b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr.sv
@@ -572,7 +572,7 @@
logic clk_io_div4_otbn_hint;
logic clk_io_div4_otbn_en;
- assign clk_main_aes_en = clk_main_aes_hint | ~idle_i[Aes];
+ assign clk_main_aes_en = clk_main_aes_hint | ~idle_i[HintMainAes];
prim_flop_2sync #(
.Width(1)
@@ -603,7 +603,7 @@
.clk_o(clocks_o.clk_main_aes)
);
- assign clk_main_hmac_en = clk_main_hmac_hint | ~idle_i[Hmac];
+ assign clk_main_hmac_en = clk_main_hmac_hint | ~idle_i[HintMainHmac];
prim_flop_2sync #(
.Width(1)
@@ -634,7 +634,7 @@
.clk_o(clocks_o.clk_main_hmac)
);
- assign clk_main_kmac_en = clk_main_kmac_hint | ~idle_i[Kmac];
+ assign clk_main_kmac_en = clk_main_kmac_hint | ~idle_i[HintMainKmac];
prim_flop_2sync #(
.Width(1)
@@ -665,7 +665,7 @@
.clk_o(clocks_o.clk_main_kmac)
);
- assign clk_main_otbn_en = clk_main_otbn_hint | ~idle_i[Otbn];
+ assign clk_main_otbn_en = clk_main_otbn_hint | ~idle_i[HintMainOtbn];
prim_flop_2sync #(
.Width(1)
@@ -696,7 +696,7 @@
.clk_o(clocks_o.clk_main_otbn)
);
- assign clk_io_div4_otbn_en = clk_io_div4_otbn_hint | ~idle_i[Otbn];
+ assign clk_io_div4_otbn_en = clk_io_div4_otbn_hint | ~idle_i[HintIoDiv4Otbn];
prim_flop_2sync #(
.Width(1)
diff --git a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_pkg.sv b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_pkg.sv
index ff6c48b..a5b3967 100644
--- a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_pkg.sv
+++ b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_pkg.sv
@@ -13,10 +13,11 @@
package clkmgr_pkg;
typedef enum int {
- Aes = 0,
- Hmac = 1,
- Kmac = 2,
- Otbn = 3
+ HintMainAes = 0,
+ HintMainHmac = 1,
+ HintMainKmac = 2,
+ HintIoDiv4Otbn = 3,
+ HintMainOtbn = 4
} hint_names_e;
typedef struct packed {
diff --git a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_pkg.sv b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_pkg.sv
index 1446261..571f64b 100644
--- a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_pkg.sv
+++ b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_pkg.sv
@@ -57,10 +57,10 @@
} clk_main_kmac_hint;
struct packed {
logic q;
- } clk_main_otbn_hint;
+ } clk_io_div4_otbn_hint;
struct packed {
logic q;
- } clk_io_div4_otbn_hint;
+ } clk_main_otbn_hint;
} clkmgr_reg2hw_clk_hints_reg_t;
typedef struct packed {
@@ -79,11 +79,11 @@
struct packed {
logic d;
logic de;
- } clk_main_otbn_val;
+ } clk_io_div4_otbn_val;
struct packed {
logic d;
logic de;
- } clk_io_div4_otbn_val;
+ } clk_main_otbn_val;
} clkmgr_hw2reg_clk_hints_status_reg_t;
// Register -> HW type
diff --git a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_top.sv b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_top.sv
index 945fa12..bd7f2a3 100644
--- a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_top.sv
+++ b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_top.sv
@@ -131,15 +131,15 @@
logic clk_hints_clk_main_hmac_hint_wd;
logic clk_hints_clk_main_kmac_hint_qs;
logic clk_hints_clk_main_kmac_hint_wd;
- logic clk_hints_clk_main_otbn_hint_qs;
- logic clk_hints_clk_main_otbn_hint_wd;
logic clk_hints_clk_io_div4_otbn_hint_qs;
logic clk_hints_clk_io_div4_otbn_hint_wd;
+ logic clk_hints_clk_main_otbn_hint_qs;
+ logic clk_hints_clk_main_otbn_hint_wd;
logic clk_hints_status_clk_main_aes_val_qs;
logic clk_hints_status_clk_main_hmac_val_qs;
logic clk_hints_status_clk_main_kmac_val_qs;
- logic clk_hints_status_clk_main_otbn_val_qs;
logic clk_hints_status_clk_io_div4_otbn_val_qs;
+ logic clk_hints_status_clk_main_otbn_val_qs;
// Register instances
// R[alert_test]: V(True)
@@ -425,33 +425,7 @@
);
- // F[clk_main_otbn_hint]: 3:3
- prim_subreg #(
- .DW (1),
- .SWACCESS("RW"),
- .RESVAL (1'h1)
- ) u_clk_hints_clk_main_otbn_hint (
- .clk_i (clk_i),
- .rst_ni (rst_ni),
-
- // from register interface
- .we (clk_hints_we),
- .wd (clk_hints_clk_main_otbn_hint_wd),
-
- // from internal hardware
- .de (1'b0),
- .d ('0),
-
- // to internal hardware
- .qe (),
- .q (reg2hw.clk_hints.clk_main_otbn_hint.q),
-
- // to register interface (read)
- .qs (clk_hints_clk_main_otbn_hint_qs)
- );
-
-
- // F[clk_io_div4_otbn_hint]: 4:4
+ // F[clk_io_div4_otbn_hint]: 3:3
prim_subreg #(
.DW (1),
.SWACCESS("RW"),
@@ -477,6 +451,32 @@
);
+ // F[clk_main_otbn_hint]: 4:4
+ prim_subreg #(
+ .DW (1),
+ .SWACCESS("RW"),
+ .RESVAL (1'h1)
+ ) u_clk_hints_clk_main_otbn_hint (
+ .clk_i (clk_i),
+ .rst_ni (rst_ni),
+
+ // from register interface
+ .we (clk_hints_we),
+ .wd (clk_hints_clk_main_otbn_hint_wd),
+
+ // from internal hardware
+ .de (1'b0),
+ .d ('0),
+
+ // to internal hardware
+ .qe (),
+ .q (reg2hw.clk_hints.clk_main_otbn_hint.q),
+
+ // to register interface (read)
+ .qs (clk_hints_clk_main_otbn_hint_qs)
+ );
+
+
// R[clk_hints_status]: V(False)
// F[clk_main_aes_val]: 0:0
@@ -557,33 +557,7 @@
);
- // F[clk_main_otbn_val]: 3:3
- prim_subreg #(
- .DW (1),
- .SWACCESS("RO"),
- .RESVAL (1'h1)
- ) u_clk_hints_status_clk_main_otbn_val (
- .clk_i (clk_i),
- .rst_ni (rst_ni),
-
- // from register interface
- .we (1'b0),
- .wd ('0),
-
- // from internal hardware
- .de (hw2reg.clk_hints_status.clk_main_otbn_val.de),
- .d (hw2reg.clk_hints_status.clk_main_otbn_val.d),
-
- // to internal hardware
- .qe (),
- .q (),
-
- // to register interface (read)
- .qs (clk_hints_status_clk_main_otbn_val_qs)
- );
-
-
- // F[clk_io_div4_otbn_val]: 4:4
+ // F[clk_io_div4_otbn_val]: 3:3
prim_subreg #(
.DW (1),
.SWACCESS("RO"),
@@ -609,6 +583,32 @@
);
+ // F[clk_main_otbn_val]: 4:4
+ prim_subreg #(
+ .DW (1),
+ .SWACCESS("RO"),
+ .RESVAL (1'h1)
+ ) u_clk_hints_status_clk_main_otbn_val (
+ .clk_i (clk_i),
+ .rst_ni (rst_ni),
+
+ // from register interface
+ .we (1'b0),
+ .wd ('0),
+
+ // from internal hardware
+ .de (hw2reg.clk_hints_status.clk_main_otbn_val.de),
+ .d (hw2reg.clk_hints_status.clk_main_otbn_val.d),
+
+ // to internal hardware
+ .qe (),
+ .q (),
+
+ // to register interface (read)
+ .qs (clk_hints_status_clk_main_otbn_val_qs)
+ );
+
+
logic [6:0] addr_hit;
@@ -665,9 +665,9 @@
assign clk_hints_clk_main_kmac_hint_wd = reg_wdata[2];
- assign clk_hints_clk_main_otbn_hint_wd = reg_wdata[3];
+ assign clk_hints_clk_io_div4_otbn_hint_wd = reg_wdata[3];
- assign clk_hints_clk_io_div4_otbn_hint_wd = reg_wdata[4];
+ assign clk_hints_clk_main_otbn_hint_wd = reg_wdata[4];
// Read data return
always_comb begin
@@ -700,16 +700,16 @@
reg_rdata_next[0] = clk_hints_clk_main_aes_hint_qs;
reg_rdata_next[1] = clk_hints_clk_main_hmac_hint_qs;
reg_rdata_next[2] = clk_hints_clk_main_kmac_hint_qs;
- reg_rdata_next[3] = clk_hints_clk_main_otbn_hint_qs;
- reg_rdata_next[4] = clk_hints_clk_io_div4_otbn_hint_qs;
+ reg_rdata_next[3] = clk_hints_clk_io_div4_otbn_hint_qs;
+ reg_rdata_next[4] = clk_hints_clk_main_otbn_hint_qs;
end
addr_hit[6]: begin
reg_rdata_next[0] = clk_hints_status_clk_main_aes_val_qs;
reg_rdata_next[1] = clk_hints_status_clk_main_hmac_val_qs;
reg_rdata_next[2] = clk_hints_status_clk_main_kmac_val_qs;
- reg_rdata_next[3] = clk_hints_status_clk_main_otbn_val_qs;
- reg_rdata_next[4] = clk_hints_status_clk_io_div4_otbn_val_qs;
+ reg_rdata_next[3] = clk_hints_status_clk_io_div4_otbn_val_qs;
+ reg_rdata_next[4] = clk_hints_status_clk_main_otbn_val_qs;
end
default: begin
diff --git a/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv b/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv
index af5a7a0..1771266 100644
--- a/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv
+++ b/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv
@@ -2492,7 +2492,8 @@
.edn_rnd_i(edn1_edn_rsp[0]),
.edn_urnd_o(edn0_edn_req[6]),
.edn_urnd_i(edn0_edn_rsp[6]),
- .idle_o(clkmgr_aon_idle[3]),
+ .idle_o(clkmgr_aon_idle[4]),
+ .idle_otp_o(clkmgr_aon_idle[3]),
.ram_cfg_i(ast_ram_1p_cfg),
.lc_escalate_en_i(lc_ctrl_lc_escalate_en),
.tl_i(otbn_tl_req),
diff --git a/util/topgen.py b/util/topgen.py
index 00e2f17..be90d55 100755
--- a/util/topgen.py
+++ b/util/topgen.py
@@ -405,13 +405,8 @@
clocks = top['clocks']
assert isinstance(clocks, Clocks)
- by_type = clocks.typed_clocks()
-
- # The names of endpoints that use one or more sw hint clocks (clkmgr has an
- # "idle" feedback signal from each), in ascending order.
- hint_blocks = sorted(set(ep_name
- for sig in by_type.hint_clks.values()
- for ep_name, ep_port in sig.endpoints))
+ typed_clocks = clocks.typed_clocks()
+ hint_names = typed_clocks.hint_names()
for idx, tpl in enumerate(tpls):
out = ""
@@ -419,13 +414,14 @@
tpl = Template(fin.read())
try:
out = tpl.render(cfg=top,
- rg_srcs=by_type.rg_srcs,
- ft_clks=by_type.ft_clks,
- rg_clks=by_type.rg_clks,
- sw_clks=by_type.sw_clks,
- hint_clks=by_type.hint_clks,
+ rg_srcs=typed_clocks.rg_srcs,
+ ft_clks=typed_clocks.ft_clks,
+ rg_clks=typed_clocks.rg_clks,
+ sw_clks=typed_clocks.sw_clks,
+ hint_clks=typed_clocks.hint_clks,
+ all_clks=typed_clocks.all_clocks(),
export_clks=top['exported_clks'],
- hint_blocks=hint_blocks)
+ hint_names=hint_names)
except: # noqa: E722
log.error(exceptions.text_error_template().render())
diff --git a/util/topgen/clocks.py b/util/topgen/clocks.py
index 7d6ebff..929bb6c 100644
--- a/util/topgen/clocks.py
+++ b/util/topgen/clocks.py
@@ -4,6 +4,8 @@
from typing import Dict, List, NamedTuple, Tuple
+from .lib import Name
+
def _yn_to_bool(yn: object) -> bool:
yn_str = str(yn)
@@ -159,6 +161,42 @@
# don't bother gating the upstream source).
rg_srcs: List[str]
+ def all_clocks(self) -> Dict[str, ClockSignal]:
+ ret = {}
+ ret.update(self.ft_clks)
+ ret.update(self.hint_clks)
+ ret.update(self.rg_clks)
+ ret.update(self.sw_clks)
+ return ret
+
+ def hint_names(self) -> Dict[str, str]:
+ '''Return a dictionary with hint names for the hint clocks
+
+ These are used as enum items that name the clock hint signals. The
+ insertion ordering in this dictionary is important because it gives the
+ mapping from enum name to index.
+
+ '''
+ # A map from endpoint to the list of hint clocks that it uses.
+ ep_to_hints = {}
+ for sig in self.hint_clks.values():
+ for ep, port_name in sig.endpoints:
+ ep_to_hints.setdefault(ep, []).append(sig.name)
+
+ # A map from hint clock name to the associated enumeration name which
+ # will appear in hint_names_e in clkmgr_pkg.sv. Note that this is
+ # ordered alphabetically by endpoint: the precise ordering shouldn't
+ # matter, but it's probably nicer to keep endpoints' signals together.
+ hint_names = {}
+ for ep, clks in sorted(ep_to_hints.items()):
+ for clk in sorted(clks):
+ # Remove any "clk" prefix
+ clk_name = Name.from_snake_case(clk).remove_part('clk')
+ hint_name = Name(['hint']) + clk_name
+ hint_names[clk] = hint_name.as_camel_case()
+
+ return hint_names
+
class Clocks:
'''Clock connections for the chip'''
diff --git a/util/topgen/merge.py b/util/topgen/merge.py
index 6f1425c..c1f6701 100644
--- a/util/topgen/merge.py
+++ b/util/topgen/merge.py
@@ -604,17 +604,17 @@
for intf in top['exported_clks']:
external[f'{clkmgr_name}.clocks_{intf}'] = f"clks_{intf}"
- # TODO: If an endpoint has two clocks with idle signals, we don't yet have
- # a good solution for connecting them up properly. At the moment, we
- # just connect up the first idle signal. See issue #7170 for more
- # details.
- eps_with_idle = set()
+ typed_clocks = clocks.typed_clocks()
- # Set up intermodule connections for idle clocks. Note that these *must*
- # match the ordering of the hint_clks list from clocks.py, since this is
- # also used to derive an enum naming the bits of the connection.
+ # Set up intermodule connections for idle clocks. Iterating over
+ # hint_names() here ensures that we visit the clocks in the same order as
+ # the code that generates the enumeration in clkmgr_pkg.sv: important,
+ # since the order that we add entries to clkmgr_idle below gives the index
+ # of each hint in the "idle" signal bundle. These *must* match, or we'll
+ # have hard-to-debug mis-connections.
clkmgr_idle = []
- for sig in clocks.typed_clocks().hint_clks.values():
+ for clk_name in typed_clocks.hint_names().keys():
+ sig = typed_clocks.hint_clks[clk_name]
ep_names = list(set(ep_name for ep_name, ep_port in sig.endpoints))
if len(ep_names) != 1:
raise ValueError(f'There are {len(ep_names)} end-points connected '
@@ -622,13 +622,6 @@
f'should the idle signal come from?')
ep_name = ep_names[0]
- # TODO: This is a hack that needs replacing properly: see note above
- # definition of eps_with_idle. (In particular, it only works at
- # all if the only hit appears at the end of the list of clocks)
- if ep_name in eps_with_idle:
- continue
- eps_with_idle.add(ep_name)
-
# We've got the name of the endpoint, but that's not enough: we need to
# find the corresponding IpBlock. To do this, we have to do a (linear)
# search through top['module'] to find the instance that matches the
diff --git a/util/topgen/templates/chiplevel.sv.tpl b/util/topgen/templates/chiplevel.sv.tpl
index fb50e17..d5c4f37 100644
--- a/util/topgen/templates/chiplevel.sv.tpl
+++ b/util/topgen/templates/chiplevel.sv.tpl
@@ -1188,7 +1188,7 @@
always_comb begin : p_trigger
mio_out = mio_out_pre;
mio_out[MioIdxTrigger] = mio_out_pre[MioIdxTrigger] &
- ~top_${top["name"]}.clkmgr_aon_idle[clkmgr_pkg::Aes];
+ ~top_${top["name"]}.clkmgr_aon_idle[clkmgr_pkg::HintMainAes];
end
//////////////////////
