[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/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
//////////////////////
